Repository navigation
ci: stop buying a universal Release build for CI janitors and reporters - #13912
Conversation
Editing scripts/ci/queue_janitor.py, triage-radar.py, notify-indexnow.py or eleven similar helpers selected macOS, web, agent-session-web and a universal Release build. Each of them runs only in a Linux workflow and none is read by the Xcode product; they were simply unclassified, so they hit the fail-open default the way the two transport helpers did before #13895. Classify the operational set -- janitors, census and reporting, registry validation, R2 canaries, build diagnostics -- as control-plane-only. scripts/ci/web_subareas.py needs both halves: it is ci-web.yml's subarea router, so it keeps the web area through is_web_change while dropping macOS and Release. Neutralizing it without that second edit would stop running web validation on the web router itself. Deliberately excluded: detect_ci_change_areas.py, detect_linux_guard_changes.py and workflow_guard_groups.py. Those decide routing, so they should keep native coverage rather than certify themselves. cmux_unit_test_shard.py and xcodebuild_noninteractive.py are real macOS build inputs. Linux guard coverage is unaffected: guard routing takes macos as an input but resolves linux_guard_tests=true either way; only ghosttykit_release drops, which is correct when no macOS build runs. Measured with classify_files() over the last 357 first-parent commits on main, this frees 8 changes from both macOS and the Release build. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Repository: manaflow-ai/cmux/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches📝 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 |
|
All contributors have signed the CLA ✍️ ✅ |
Review caught that the previous commit's carveout comment was false for one file. scripts/ci/run_python_test_lane.py imports test_execution_registry, and ci-macos.yml runs that lane helper on a macOS runner at four call sites, so the registry module does execute on a Mac. That is the same indirect reachability that keeps cache_restore_receipt.py (via the cache-restore composite action) and xcodebuild_noninteractive.py (via run-app-host-xcodebuild.sh) out of the carveout. Treating it differently was an inconsistency, not a judgement. Drop it from CI_CONTROL_PLANE_ONLY, correct the comment to say no workflow runs these on a macOS runner directly or through a wrapper, and record why the registry module is excluded. Also close the two test gaps the review found: the operational test covered ten of the helpers rather than all of them, and the boundary test pinned two of the six exclusions the description claimed. Both now cover the full set, including the file dropped here, so a future widening across this boundary reddens. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Independent agent review — Thornquay 💠 I wrote this change, so the review below was done by a separate agent told to falsify it. It returned merge with changes, and it was right. Pushed as The defect it found. The carveout comment claimed all fourteen helpers "run only in a Linux workflow." That is false for one of them: What makes that a real miss rather than a judgement call: this PR already excluded It would not have been a silent hole — Fixed: dropped it (14 → 13), corrected the comment to say no workflow runs these on a macOS runner directly or through a wrapper, and recorded why the registry module is out. Two test gaps it also found, both closed: the operational test covered ten of the helpers rather than all of them, and the boundary test pinned two of the six exclusions the description claimed to pin. Both now cover the full set including the dropped file, verified to redden under two independent widenings (extending the frozenset to all Independently confirmed from the review:
Not acting on, deliberately: the review noted that Validation after the fix: all 132 registered |
ca867b7 ci(ios): bound the xcodebuild test invocation so a teardown wedge fails fast (manaflow-ai#13927) 9ffbb6a ci: compile the E2E test product once, in its own job (manaflow-ai#13908) 827f614 ci: let the macOS 15 and 26 pools share one Swift package cache (manaflow-ai#13925) b79a83b Price GPT-6 models in coderouter API-equivalent estimates (manaflow-ai#13892) ff20a22 Expose in-flight drag intent to custom JavaScript sidebars (manaflow-ai#13841) 3344583 Capture Cloud Desktop click destinations before queued opens (manaflow-ai#13897) ac041c1 test(ios): assert the letterbox a daemon-push shrink actually produces (manaflow-ai#13920) ce1c55c Catch guard-group drift between ci.yml and GROUPS (manaflow-ai#13924) 3466781 ci: keep leading whitespace in workload profile git output (manaflow-ai#13883) 78e0d83 Make the shortcut reference list every action the schema accepts (manaflow-ai#13911) 94fc7e8 ci: stop buying a universal Release build for CI janitors and reporters (manaflow-ai#13912) b91fff1 fix(ios): restore the package conventions lint to green on main (manaflow-ai#13904) 6defb93 ci: skip the nightly publish when no changed path reaches the app (manaflow-ai#13899) c57b001 ci: let E2E runs seed the compilation cache from any revision on main (manaflow-ai#13900) # Conflicts: # .github/workflows/nightly.yml # .github/workflows/perf-activation.yml # .github/workflows/test-depot.yml # .github/workflows/test-e2e.yml # .github/workflows/test-ios.yml
Editing
scripts/ci/queue_janitor.py,triage-radar.py,notify-indexnow.pyor ten similar helpers selects macOS, web, agent-session-web and a universal Release build. Each runs only in a Linux workflow and none is read by the Xcode product — they were simply unclassified, so they hit the fail-open default the way the two transport helpers did before #13895.Resulting behavior
web_subareas.pyneeds both halvesIt is
ci-web.yml's subarea router, so it keeps the web area throughis_web_changewhile dropping macOS and Release. Neutralizing it without that second edit would stop running web validation on the web router itself — a false negative.test_web_subarea_router_keeps_web_without_macospins that.Editing it still exercises every web subarea, because the helper lists itself in its own
ALL_SUBAREA_INPUTS(web_subareas.py:56-59).Deliberately excluded
detect_ci_change_areas.py,detect_linux_guard_changes.py,workflow_guard_groups.py— these decide routing. They should keep native coverage rather than certify themselves.cmux_unit_test_shard.py,xcodebuild_noninteractive.py— real macOS build inputs, referenced from macOS workflows.cache_restore_receipt.py— reached from macOS jobs through thecache-restorecomposite action (.github/actions/cache-restore/action.yml:88).test_execution_registry.py— dropped from the carveout after review.scripts/ci/run_python_test_lane.py:13imports it andci-macos.ymlruns that lane helper on a macOS runner at four call sites (:1975,:1993,:2002,:2003), so it does execute on a Mac. Same indirect reachability as the two above; an earlier revision of this PR classified it by mistake.CLA.md— prose, but the CLA guard reads it.test_routing_policy_and_build_helpers_still_run_macospins all seven excluded paths, so the carveout cannot silently widen across that boundary.test_operational_ci_helpers_skip_product_areaspins all thirteen carved-out helpers.Linux guard coverage is unaffected
Guard routing takes
macosas an input, so this was the thing worth checking rather than assuming.detect_linux_guard_changes.pyresolveslinux_guard_tests=truefor these paths with--macos trueand--macos false; onlyghosttykit_releasedrops, which is correct when no macOS build runs.Tradeoff
A helper added later is still unclassified and still fails open — expensive, never wrong. This narrows fourteen known files rather than changing the default. For why the default itself should stay fail-open, see #13905: 72.0% of changes genuinely select macOS.
Validation
linux-guardtests: 0 failures.tests/test_ci_change_areas.pypasses with three new cases.classify_files()over the last 357 first-parent commits onmain: 8 changes freed from both macOS and the Release build, on top of ci: stop routing contributor prose to macOS and the release build #13905. (Measured on the 14-helper revision; droppingtest_execution_registry.pydoes not change the sampled figure.)🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Given the initial reject/non-acceptance tag, confidently handling this further edited as working iterations. Since no major constraints.
Written for commit 7e495f1. Summary will update on new commits.
Summary by CodeRabbit