Skip to content

ci: stop routing-policy PRs that add a guard test from compiling macOS - #13722

Merged
teamleaderleo merged 1 commit into
mainfrom
ci/trusted-router-package-closure
Sep 22, 2026
Merged

teamleaderleo merged 1 commit into
mainfrom
ci/trusted-router-package-closure

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Sep 22, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

When a PR edits CI routing policy, ci.yml classifies the PR's product files with the trusted base router, which copies a handful of files from the base commit. Two gaps made those PRs compile the macOS app for nothing:

  1. Test references came only from the base workflows. A tests/ file counts as macOS-neutral only when a workflow names it in Linux jobs alone. A guard test the PR adds is named only by the PR's own ci-guards.yml, so the base router treated it as macOS-relevant. That scheduled compile admission and a Release build.
  2. No Xcode project or package manifests. The trusted copy logged Could not derive macOS local-package dependency closure; keeping Packages/iOS macOS-relevant: … trusted-ci-router/cmux.xcodeproj/project.pbxproj.

Observed on #13703 (run 35753992765): the product files were one iOS script and one new guard test. changes resolved macos=true release_build=true, and compile admission spent 34+ min in "Compile app-host test product". That run also missed every cache because it is a fork PR, which #13717 addresses separately.

Resulting behavior

  • Merged references: load_macos_job_test_references now merges the PR head's references when CMUX_CI_HEAD_TEST_REFERENCE_ROOT is set. ci.yml sets it to the workspace for the trusted-router invocation. A path that either the base or the head names in a macOS job stays macOS-relevant, and if the head can't be read, only the base is used.
  • Project and manifests: the trusted copy now also receives the base cmux.xcodeproj/project.pbxproj and every Packages/*/*/Package.swift.
  • ci(ios): shallow checkout and zig only on fallback for TestFlight uploads #13703 replayed locally: macos=true release_build=true becomes macos=false release_build=false. That used the new detector, base workflows as the trusted root, and the PR's checkout as head.

Validation and remaining gap

  • New test test_trusted_router_reads_new_guard_tests_from_the_pr_head covers three cases: the base alone keeps the new test macOS-relevant; merging head Linux references makes it guard-only; and a head macOS job naming it keeps it macOS-relevant.
  • All of tests/test_ci_change_areas.py passes locally (with RUNNER_TEMP set, as CI does), along with test_ci_linux_guard_routing.py, test_ci_release_guard_structure.py and test_ci_self_hosted_guard.sh.
  • Remaining gap: the trusted router runs the base detector, so the reference merge only takes effect for PRs opened after this lands. This PR's own run still uses the old behavior.

Part of #13095.

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Stops routing-policy PRs that add a guard test from compiling the macOS app for nothing, since the trusted base router previously treated the new tests/ file as macOS-relevant and lacked the Xcode project and package manifests needed to derive the local-package closure.

  • Merges the PR head's workflow test references when CMUX_CI_HEAD_TEST_REFERENCE_ROOT is set; a path named in a macOS job by either side stays macOS-relevant.
  • Copies the base cmux.xcodeproj/project.pbxproj and all Packages/*/*/Package.swift into the trusted router's workspace.
  • Adds a test covering base-only, merged-head, and head-macOS-named cases; the existing CI area tests pass.
  • Only takes effect for PRs opened after this lands, since routing-policy PRs are classified by the base detector.

Part of #13095.

Written for commit 3dd4b60. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Bug Fixes

    • Improved change detection for CI routing policies and package updates.
    • CI now more accurately identifies macOS-relevant changes, reducing unnecessary checks while ensuring affected changes are validated.
    • Routing decisions now account for workflow and guard-test updates introduced in the current change.
  • Tests

    • Added coverage for classifying newly introduced CI guard tests and workflow references.

When a PR edits CI routing policy, ci.yml classifies its product files with
the trusted base router. That router read test references only from the base
workflows, which never name a guard test the PR adds, so the new tests/ file
counted as macOS-relevant and the PR paid for a full app compile and Release
build (e.g. #13703: 34 min cold compile for one guard test + one iOS script).

Merge the PR head's test references in: a path either side names in a macOS
job stays macOS-relevant. Also give the trusted router the base Xcode project
and Package.swift manifests, whose absence logged 'Could not derive macOS
local-package dependency closure' and kept every iOS package edit macOS.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@cursor

cursor Bot commented Sep 22, 2026

Copy link
Copy Markdown

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.

@teamleaderleo
teamleaderleo enabled auto-merge (squash) September 22, 2026 17:24
@github-actions

Copy link
Copy Markdown
Contributor

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

@coderabbitai

coderabbitai Bot commented Sep 22, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

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 configuration

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

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 9498e603-d87b-4fe3-9e62-075bb680aac1

📥 Commits

Reviewing files that changed from the base of the PR and between 9b144a2 and 3dd4b60.

📒 Files selected for processing (3)
  • .github/workflows/ci.yml
  • scripts/ci/detect_ci_change_areas.py
  • tests/test_ci_change_areas.py

Included review availability: Your plan provides up to 10 included reviews per hour; 1 remains after this review.


📝 Walkthrough

Walkthrough

Changes

CI reference routing

Layer / File(s) Summary
Root-aware test reference loading
scripts/ci/detect_ci_change_areas.py
load_macos_job_test_references reads references from a supplied root and merges references from the optional head root.
Trusted router base and head integration
.github/workflows/ci.yml, tests/test_ci_change_areas.py
The workflow copies the trusted base Xcode and package files and sets the head reference root. Tests cover head-only guard references in Linux and macOS jobs.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant CIWorkflow
  participant TrustedRouter
  participant detect_ci_change_areas
  participant BaseRoot
  participant HeadRoot
  CIWorkflow->>TrustedRouter: copy base Xcode project and Package.swift files
  CIWorkflow->>TrustedRouter: set CMUX_CI_HEAD_TEST_REFERENCE_ROOT
  TrustedRouter->>detect_ci_change_areas: classify product inputs
  detect_ci_change_areas->>BaseRoot: load base workflow references
  detect_ci_change_areas->>HeadRoot: load head workflow references
  detect_ci_change_areas-->>TrustedRouter: merge references and return classification
Loading

Merge Risk: ⚪ Minimal · up to 3dd4b

This change refines CI routing so guard-only and package-specific changes avoid unnecessary macOS work; no merge-blocking risk is identified.

🚥 Pre-merge checks | ✅ 24 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (24 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the primary change: preventing CI routing-policy pull requests that add guard tests from triggering unnecessary macOS compilation.
Description check ✅ Passed The description provides a detailed problem statement, resulting behavior, implementation details, validation results, and a documented remaining gap. It does not reproduce the template's Demo Video, …
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Cmux Cloud Persistent Session And Early Input ✅ Passed PASS: The pull request changes only CI routing, the change-area detector, and its tests. The diff adds trusted-base file loading and merged workflow test references. It does not change Cloud terminal …
Cmux Swift Actor Isolation ✅ Passed PASS: The PR changes only .github/workflows/ci.yml, scripts/ci/detect_ci_change_areas.py, and tests/test_ci_change_areas.py. The authoritative diff contains no Swift files or Swift declarations,…
Cmux Swift Blocking Runtime ✅ Passed The PR changes only GitHub Actions YAML, Python CI logic, and Python tests. It adds no Swift files or Swift runtime synchronization. The diff introduces no semaphores, blocking waits, sleeps, delayed …
Cmux Browser Automation Off-Main ✅ Passed PASS: The pull request changes only CI workflow/Python detection code and its tests. The rule-scoped browser automation files are unchanged, and no added lines contain browser socket commands, WebKit/…
Cmux Expensive Synchronous Load ✅ Passed The authoritative PR diff changes only .github/workflows/ci.yml, scripts/ci/detect_ci_change_areas.py, and tests/test_ci_change_areas.py. It adds CI shell logic and Python test-reference handlin…
Cmux Cache Substitution Correctness ✅ Passed PASS: The pull request changes only .github/workflows/ci.yml and Python files. It does not change production Swift, TypeScript, or JavaScript, so the cache-substitution condition does not apply.
Cmux No Hacky Sleeps ✅ Passed PASS: The diff adds no sleep, timer, fixed delay, polling, retry backoff, or wall-clock wait. The new shell loop consumes git ls-tree output, and the Python changes only load and merge workflow refe…
Cmux Algorithmic Complexity ✅ Passed The diff does not introduce a prohibited complexity pattern. The new shell loop processes matched package paths linearly. The Python code scans four fixed workflow paths per root and performs set unio…
Cmux Swift Concurrency ✅ Passed PASS: The pull request changes only .github/workflows/ci.yml, scripts/ci/detect_ci_change_areas.py, and tests/test_ci_change_areas.py. It adds no cmux-owned Swift code and introduces no Swift co…
Cmux Swift @Concurrent ✅ Passed PASS: The PR changes only GitHub Actions YAML, Python CI logic, and Python tests. The authoritative diff contains no Swift files, Swift functions, or Swift call sites, so the @concurrent check is not …
Cmux Swift Package Boundaries ✅ Passed The pull request changes only .github/workflows/ci.yml and Python files. The diff contains no Swift production changes, so the Swift package boundary rule does not apply.
Cmux Swiftpm Lockfiles ✅ Passed PASS: The PR changes only .github/workflows/ci.yml, scripts/ci/detect_ci_change_areas.py, and its test. It does not modify a cmux-owned .gitignore, Package.swift, Package.resolved, or `cmux.…
Cmux Swift Logging ✅ Passed The PR changes only CI YAML and Python test/detector files. The authoritative diff contains no Swift or other app/runtime logging changes, so the Swift logging failure conditions do not apply.
Cmux User-Facing Error Privacy ✅ Passed PASS — The diff changes only GitHub Actions CI routing, an internal CI detector, and tests. Added messages and comments describe trusted-router loading and CI area selection; they are not product UI, …
Cmux Full Internationalization ✅ Passed The pull request changes only CI workflow logic, a CI detection script, and CI tests. The diff adds no Swift UI text, app catalog or Info.plist entries, web UI/API copy, locale data, or message entrie…
Cmux Swiftui State Layout ✅ Passed PASS: The pull request changes only .github/workflows/ci.yml, scripts/ci/detect_ci_change_areas.py, and tests/test_ci_change_areas.py. The diff contains no Swift/SwiftUI files or SwiftUI state/l…
Cmux Architecture Rethink ✅ Passed PASS: The reviewed range changes only .github/workflows/ci.yml, scripts/ci/detect_ci_change_areas.py, and tests/test_ci_change_areas.py. No .swift file changes or SwiftUI/AppKit lifecycle chan…
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PASS: The pull request changes only .github/workflows/ci.yml, scripts/ci/detect_ci_change_areas.py, and tests/test_ci_change_areas.py. It contains no Swift changes and no standalone cmux-owned w…
Cmux Source Artifacts ✅ Passed The PR changes only .github/workflows/ci.yml, scripts/ci/detect_ci_change_areas.py, and tests/test_ci_change_areas.py. These are intentional CI configuration, source, and test files. The diff ad…
Cmux No Test Or Debug Seam In Production Source ✅ Passed The pull request changes only .github/workflows/ci.yml, scripts/ci/detect_ci_change_areas.py, and tests/test_ci_change_areas.py. It adds no Swift file under a production Sources/ path, so it c…
Full details: Docstring Coverage

Explanation

Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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

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

@teamleaderleo
teamleaderleo merged commit f239d4e into main Sep 22, 2026
35 of 37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant