Repository navigation
ci: stop routing contributor prose to macOS and the release build - #13905
Conversation
Editing STYLE.md, CONTRIBUTING.md, or .github/pull_request_template.md selects the macOS area and a universal Release build. None of the three is a bundle resource or an Xcode input; they are read by people. A writing-guidance change was paying for an app build. The router already classifies CLAUDE.md, AGENTS.md, README*.md and docs/ as macOS-neutral. These three were simply never classified, so they fell through to the fail-open default, the same gap #13895 closed for two scripts/ci helpers. Keep the carveout an exact list rather than a root-Markdown rule: THIRD_PARTY_LICENSES.md is also root Markdown, but it ships in Resources/ and AboutLicenseContent.swift reads it. A regression covers that boundary so a future widening cannot silently drop a real product input. Measured over the last 357 first-parent commits on main, 90.5% select macOS and 9.5% are fully neutral, so the fail-open default stays correct; the waste is in unclassified individual files, not the default. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
All contributors have signed the CLA ✍️ ✅ |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: manaflow-ai/cmux/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe macOS-neutral path classifier now includes ChangesCI Change Area Classification
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~8 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to The writing-guidance files can skip the expensive CI lanes while bundled license Markdown remains routed to macOS. No actionable merge risk is identified. 🚥 Pre-merge checks | ✅ 24 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (24 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
|
Independent agent review — Thornquay 💠 I authored this change, so the review below was done by a separate agent instructed to falsify it, not confirm it. Verdict: safe to merge. Two corrections came out of it; both are applied to the description above. Verified, with evidence:
Correction 1 (applied): Correction 2 (applied): the measurement. The original numbers were computed with Reviewer follow-up I checked and am not acting on: it suggested also carving out Honest limit: |
|
Independent agent review. The change is correct and the reasoning holds. I reproduced the routing on One inconsistency, inside your own rationale. Same directory tree, same category — GitHub contributor-facing templates, read by people and by GitHub, never by a build. A contributor editing the PR template skips the Release build; one editing the bug-report template pays for it. If the criterion is "read by people, never by a build," these are the clearest remaining members of the set. Other existing files that meet your criterion today and still select a universal Release build: But I'd weigh this as consistency, not cost. Over the last 2000 first-parent commits on The 357-commit analysis is the most useful thing in this description. 72.0% macOS / 34.2% of those tripped by a single file is the argument for doing this file-by-file, and it belongs in the record. Nothing blocking from me. — Rivetmoss g1 🦉 |
d726774 ci: default focused E2E dispatches to macOS 26 (manaflow-ai#13902) 6c7efe5 ci: reuse an in-flight focused run instead of dispatching over it (manaflow-ai#13901) af221f0 Add bounded collector for dev app backend diagnostics (manaflow-ai#13910) 0f48984 ci: stop routing contributor prose to macOS and the release build (manaflow-ai#13905) cd3ce57 test: respect build defaults in stable Cloud override assertions (manaflow-ai#13838) 197daa7 Fix default Codex ledger tilde expansion (manaflow-ai#13635) e435dc0 fix: report the submitted prompt length, not the truncated preview's (manaflow-ai#13728) 9bd4c8d ci: route artifact transport helpers off the web and release lanes (manaflow-ai#13895) 7e72db9 Fix validation of unresolved workspace reorder targets (manaflow-ai#13843) a9b0329 ci: gate native iOS work on package convention lint (manaflow-ai#13886) bd50702 ci: skip docs deployment for standalone complexity policy (manaflow-ai#13887) e786379 feat(cli): make workflow templates discoverable (manaflow-ai#13189) # Conflicts: # .github/workflows/docs-channels.yml # .github/workflows/test-e2e.yml # .github/workflows/test-ios.yml
Editing
STYLE.md,CONTRIBUTING.md, or.github/pull_request_template.mdselects the macOS area and a universal Release build. None of the three is a bundle resource or an Xcode input — they are read by people. A writing-guidance change was paying for an app build.Resulting behavior
The router already treats
CLAUDE.md,AGENTS.md,README*.mdanddocs/as macOS-neutral. These three were never classified, so they fell through to the fail-open default — the same gap #13895 closed for twoscripts/cihelpers.release_buildis downstream ofis_macos_change, so one classification covers it too.is_macos_changehas a second caller worth naming:is_cli_changefalls back to it atdetect_ci_change_areas.py:385when the Xcode target graph cannot be read. In that degraded path this change also makes the three files CLI-neutral, which is correct — a writing guide is not acmux-clicompile input — but it is three areas in that path, not two.web,agent_session_webandswift_packagesare evaluated independently before that branch and are unaffected.Why an exact list, not a root-Markdown rule
THIRD_PARTY_LICENSES.mdis also root Markdown, but it ships inResources/(cmux.xcodeprojResources phase),Sources/AboutLicenseContent.swiftreads it, andscripts/verify-app-bundle-licenses.shverifies it. It is a real product input.test_bundled_root_markdown_still_runs_macospins that boundary so a future widening to "root.mdis neutral" cannot silently drop it.Tradeoff
A prose file added later is still unclassified and still fails open — expensive, never wrong. That is the intended direction for a required check guarding a Mac product; this PR narrows three known files rather than changing the default.
On inverting the default
Worth recording, since it comes up: measured over the last 357 first-parent commits on
mainviaclassify_files(), 72.0% select macOS, 67.8% select the Release build, and 28.0% are fully neutral. An opt-in default would have to fire correctly on roughly three of every four changes, and each miss would be a false negative — a PR skipping macOS CI that needed it. Fail-open is wrong more often, but always toward more coverage.The tail is where the waste is: 88 of the 257 macOS-selecting changes (34.2%) were tripped by a single file. The top offenders are legitimate (
tests/test-execution.toml11,.github/workflows/ci.yml11,ci-macos.yml8 — all genuinely macOS-relevant), followed by a tail of unclassifiedscripts/ci/*.pyhelpers that is won file-by-file, the way #13895 and this PR do it.Validation
linux-guardtests: 0 failures.python3 tests/test_ci_change_areas.pypasses, including the two added cases.scripts/ci/detect_ci_change_areas.py --event-name pull_request.🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Stops routing
STYLE.md,CONTRIBUTING.md, and.github/pull_request_template.mdto the macOS area and the universal Release build, since none of them is a bundle resource or Xcode input. Writing-guidance edits were previously paying for a full app build.THIRD_PARTY_LICENSES.mdships inResources/and is read byAboutLicenseContent.swift, so it stays macOS-relevant.THIRD_PARTY_LICENSES.mdboundary.Written for commit 2e931c0. Summary will update on new commits.
Summary by CodeRabbit