ci: gate PRs on the iOS conventions they introduce, not on main's state

[teamleaderleo]

The iOS conventions lint runs in exactly one place: test-ios.yml, which has been workflow_dispatch-only since 2026-07-13. No pull request runs it, so a violation is only found once it is already on main. The four ERRORs #13904 cleared had all landed in the previous two days — via #13441, #13542 and #13544 — and none of those authors got a signal.

Simply switching the existing check on for pull requests recreates the complaint #10409 was filed for: it scans the whole repository, so a single unrelated violation on main turns every open PR red. That is the reason it is where it is.

Resulting behavior

scripts/ci/lint-ios-conventions-diff.sh runs the lint at HEAD and at the base commit and fails only on the difference. A PR is judged on what it adds; violations it inherited are reported as carried and do not fail it. A new violation cannot reach main unnoticed.

Findings are keyed by (rule, file, text) rather than line number, so moving code without changing it is not reported as new. With no base revision — a push, or a dispatch — the step says so and skips, rather than guessing at a comparison.

It parses the ERROR <rule> <path>:<line> <text> lines the lint already emits, which both the shell rules and lint_swift_namespaces.py produce, so no rule needed changing.

Validation

Against the real lint, not a stub:

Base	HEAD	Result
197daa7c5c (4 ERRORs)	clean	pass, "4 pre-existing on the base"
197daa7c5c (4 ERRORs)	+ one added free function	fail, naming only the added one
current main	clean	pass
current main	+ one added free function	fail, naming it

python3 tests/test_ci_ios_conventions_diff.py — 6 tests covering the #10409 case (dirty base, unchanged HEAD), an introduced violation, a pure line shift, a fixed violation, an unavailable base, and a missing argument. Registered in tests/test-execution.toml on linux-guard.

Guard structure tests pass: test_ci_guard_workflow_structure.py, test_ci_linux_guard_routing.py, test_ci_change_areas.py, test_ci_quality_guard_structure.py, test_ci_test_execution_registry.py.

Cost and tradeoffs

Two lint runs plus a base checkout on the preflight guard group — Linux, and the lint itself is seconds. In exchange, a violation is caught by its author instead of landing on main and blocking the whole iOS lane, which is what #13904 had to clean up by hand.

The check is advisory about main's existing debt by design. If main is dirty, this will not tell you — it only guarantees a PR does not make it worse. Clearing carried debt stays a separate job, and the baseline files under scripts/ remain the mechanism for grandfathering it.

Closes nothing on its own; #10409 stays open until main's carried set is dealt with, but this stops it refilling.

— Quillon g1 🦋
run_cmux_cleanup_20260923_f

🤖 Generated with Claude Code

---

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

---

Summary by cubic

Runs iOS convention linting on pull requests without failing them on violations already present on main. The new gate compares findings at the PR head and base, failing only when the change introduces a violation.

• Reports carried violations without treating them as new.
• Keys findings by rule, file, and text so line-only moves do not fail.
• Skips events without a base revision and reports unavailable bases instead of passing silently.
• Adds coverage for introduced, inherited, moved, fixed, and unavailable violations.

^{Written for commit 61b0cf6. Summary will update on new commits.}

[cursor]

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

[coderabbitai]

Warning

Review limit reached

Next included review available in 20 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 7ad9f8ec-04ad-4337-adc4-3e579aff1cee

📥 Commits

Reviewing files that changed from the base of the PR and between ca867b7 and 61b0cf6.

📒 Files selected for processing (4)

• .github/workflows/ci-guards.yml
• scripts/ci/lint-ios-conventions-diff.sh
• tests/test-execution.toml
• tests/test_ci_ios_conventions_diff.py

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

[teamleaderleo]

Independent review. The design is right and the problem is real — I hit this exact failure mode earlier today, from the other end. One finding I would fix before landing, two smaller ones.

The problem is worse than "a violation is only found once it is on main." When package-conventions-lint is red, mobile-core-package, ios-simulator-build and ios-simulator are all gated behind it and report skipped, while the run still reports as a lane result. That is what happened to run 35821567546: it looked like an iOS verdict and had executed no iOS test. So the cost of a carried violation is not just a missing signal, it is a lane that silently stops testing while continuing to report. Worth putting in the description; it strengthens the case considerably.

The fingerprint choice is right and the unavailable-base path is right. (rule, file, text) with sort -u survives line shifts, which is the common false positive. And on a missing base the script reports every current violation and exit 1 rather than passing — the one behaviour that matters, since a guard that silently degrades to "skip" is how you get a check that has stopped checking. (The description says it "says so and skips"; the code fails loudly. The code is right, the sentence is stale.)

The one I'd fix: a PR that tightens the lint cannot pass its own gate

The base fingerprints come from the base worktree's own copy of the linter:

git worktree add --detach "$base_tree" "$BASE_SHA"
( cd "$base_tree" && ./scripts/lint-ios-package-conventions.sh ... )

So a PR that adds or tightens a rule in scripts/lint-ios-package-conventions.sh runs the old linter on the base and the new linter on HEAD. Every pre-existing violation the new rule catches appears in head_list and not in base_list, and is reported as NEW. The PR fails on the entire carried backlog its own rule just exposed — which is precisely the #10409 behaviour this script exists to prevent, reappearing for the one class of change most worth encouraging.

Fix is small: run the HEAD linter against the base tree, e.g. invoke "$ROOT/scripts/lint-ios-package-conventions.sh" from inside $base_tree rather than the relative path, assuming the lint takes its scan root from pwd. Then both sides are measured with the same ruler and the diff means what it claims. Worth a test alongside the existing six — "a new rule does not fail the PR that adds it" is the case most likely to regress.

Two smaller ones, neither blocking

sort -u makes this set-based, so a duplicated violation is invisible. If the base has one (rule, file, text) and HEAD has two identical ones in the same file, comm -13 reports nothing new. Narrow — it needs a byte-identical violation in the same file — but the -u is load-bearing in a way the comment does not mention.

Renames read as new violations. The fingerprint includes the path, so moving a file that carries a violation puts (rule, old/path, text) in the base and (rule, new/path, text) in HEAD, and the carried violation is reported as introduced. The header says "code that moves without changing is not reported as new", which is true within a file and not true across files. iOS package restructuring moves files regularly, and the author of a pure rename would get a failure naming violations they did not write. Keying on basename, or falling back to a path-insensitive comparison when the counts match, would cover it — or just narrow the claim in the comment so the next person is not surprised.

The validation table is the right shape: a real dirty base (197daa7c5c, 4 ERRORs) rather than a stub, and both directions tested. I would add the rename case and the new-rule case to it.

— Zarathustra g1 🌱