Skip to content

Stage required macOS CI behind linux preflight - #7583

Merged
azooz2003-bit merged 14 commits into
mainfrom
task-required-ci-shared-build
Jul 8, 2026
Merged

azooz2003-bit merged 14 commits into
mainfrom
task-required-ci-shared-build

Conversation

@azooz2003-bit

@azooz2003-bit azooz2003-bit commented Jul 8, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Add required linux-preflight so routed Linux checks and workflow guards finish before expensive macOS jobs enter the queue.
  • Collapse required display/runtime macOS work into tests-build-and-lag: one Debug build-for-testing now feeds Swift warning budget, CoreAnimation startup, lag, display-resolution UI, and browser-find UI regressions.
  • Remove the separate ui-regressions macOS job.
  • Remove the separate release-ghostty-cli-helper macOS job. The existing macOS 15 swift-package-tests lane now builds/uploads the universal Ghostty helper before switching to the Xcode 26 package-test SDK, and release-build downloads that artifact before validating app/CLI/helper slices on macOS 26.
  • Keep app-host unit tests sharded 4 ways because collapsing those shards would increase wall time.
  • Add lane-specific Xcode fast paths through CMUX_CI_XCODE_APP_MACOS_15 and CMUX_CI_XCODE_APP_MACOS_26, with an early CMUX_CI_REQUIRED_MACOS_SDK_MAJOR=26 guard so stale pinned Xcodes fail before expensive builds.

Resulting required PR split

  • Cheap Linux/routing layer: changes, workflow-guard-tests, routed Linux checks, linux-preflight.
  • Parallel macOS layer after preflight: four app-host unit tests shards, swift-package-tests, tests-build-and-lag, release-build.
  • Aggregates: tests, ci-status.

Repo settings

Already set on manaflow-ai/cmux:

  • CMUX_CI_XCODE_APP_MACOS_15=/Applications/Xcode_26.3.app
  • CMUX_CI_XCODE_APP_MACOS_26=/Applications/Xcode_26.5.app

Optional helper override, only needed if you want to force a specific SDK 15 helper Xcode instead of scanning installed Xcodes:

  • CMUX_CI_HELPER_XCODE_APP_MACOS_15=/Applications/<sdk-15-xcode>.app

Ruleset impact

Update required checks after this lands if the ruleset currently names individual checks:

  • Remove ui-regressions.
  • Remove release-ghostty-cli-helper.
  • Add linux-preflight if requiring direct job names instead of only ci-status.
  • Keep tests, tests-build-and-lag, release-build, swift-package-tests, and app-host shard checks.

Verification

  • bash -n scripts/ci/run-display-ui-regressions.sh scripts/select-ci-xcode.sh tests/test_ci_xcode_selection_fast_path.sh tests/test_ci_release_sdk_lane.sh tests/test_ci_self_hosted_guard.sh tests/test_ci_release_build_timeout.sh
  • python3 tests/test_ci_change_areas.py
  • ./tests/test_ci_xcode_selection_fast_path.sh
  • ./tests/test_ci_release_sdk_lane.sh
  • ./tests/test_ci_release_build_timeout.sh
  • ./tests/test_ci_self_hosted_guard.sh
  • git diff --check

Note

Medium Risk
Large CI workflow restructuring changes when macOS runs and how release helper artifacts are produced; mistakes could skip regressions or break release-build handoff, but extensive guard scripts and Python tests lock the intended graph.

Overview
Adds a required linux-preflight gate so workflow guards and routed Linux jobs must pass before any macOS work is queued; tests and ci-status now depend on it.

Job consolidation: removes standalone ui-regressions and release-ghostty-cli-helper. Display-resolution and browser-find UI regressions move into tests-build-and-lag via new scripts/ci/run-display-ui-regressions.sh, sharing one Debug build-for-testing with CA/lag checks (timeout 75m, stronger virtual-display teardown). The universal Ghostty CLI helper is built and uploaded from swift-package-tests (helper Xcode pinned to SDK 15, then package tests on SDK 26); release-build waits on that job and downloads the artifact instead of building the helper on macOS 26.

Xcode selection: select-ci-xcode.sh gains pinned-app fast paths (CMUX_CI_XCODE_APP / lane vars) and CMUX_CI_REQUIRED_MACOS_SDK_MAJOR validation so wrong SDKs fail before long builds. macOS jobs set lane-specific Xcode env vars.

UITest configureSocketLaunch forwards CMUX_UI_TEST_TARGET_DISPLAY_ID for the browser-find regression on the persistent virtual display. Guard tests and test_ci_change_areas.py encode the new topology.

Reviewed by Cursor Bugbot for commit 652eb84. Bugbot is set up for automated code reviews on this repo. Configure here.

@vercel

vercel Bot commented Jul 8, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
cmux Canceled Canceled Jul 8, 2026 2:18pm
cmux-staging Building Building Preview, Comment Jul 8, 2026 2:18pm

@coderabbitai

coderabbitai Bot commented Jul 8, 2026 •

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This PR updates CI routing and release job layout, refactors Xcode selection into a shared helper, and adds a display UI regression runner with matching workflow and test coverage.

Changes

CI preflight and release flow

Layer / File(s) Summary
Add linux-preflight job
.github/workflows/ci.yml
Adds linux-preflight to validate routed job results and print route outputs from changes.
Wire macOS jobs and tests to preflight
.github/workflows/ci.yml
Updates app-host-unit-tests, tests, swift-package-tests, tests-build-and-lag, and ci-status to depend on linux-preflight and include its result in routing checks.
Build helper in package lane
.github/workflows/ci.yml
Builds and uploads the universal Ghostty CLI helper in swift-package-tests and updates release-build to use the macOS 26 selector settings.
Collapse runtime regressions and update routing
.github/workflows/ci.yml
Runs display UI regressions inline in tests-build-and-lag, changes the build to build-for-testing, and updates ci-status routing inputs.

Xcode selection refactor

Layer / File(s) Summary
Centralize developer dir selection
scripts/select-ci-xcode.sh
Adds select_developer_dir() and uses it for pinned and best-match Xcode selection paths.
Fast path test and workflow guard
tests/test_ci_xcode_selection_fast_path.sh, .github/workflows/ci.yml
Adds a shell test for pinned Xcode selection behavior and wires it into workflow guard coverage.

UI regression runner and CI guard updates

Layer / File(s) Summary
Run display UI regressions
scripts/ci/run-display-ui-regressions.sh
Adds the virtual-display churn flow, the follow-up browser regression run, and the sequential orchestration between them.
Update CI guard tests
tests/test_ci_release_sdk_lane.sh, tests/test_ci_self_hosted_guard.sh, tests/test_ci_change_areas.py
Updates workflow assertions for the new helper handoff, the collapsed runtime regression path, and the removed standalone helper and UI regression jobs.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant changes
  participant linux_preflight
  participant tests
  participant swift_package_tests

  changes->>linux_preflight: route outputs
  linux_preflight->>tests: require success for routed jobs
  linux_preflight->>swift_package_tests: require success or skipped routing
Loading
sequenceDiagram
  participant select_ci_xcode_sh
  participant xcode_select
  participant xcodebuild

  select_ci_xcode_sh->>xcode_select: set developer dir
  select_ci_xcode_sh->>xcodebuild: read version and SDK path
  xcodebuild-->>select_ci_xcode_sh: version and SDK diagnostics
Loading

Possibly related issues

Suggested reviewers: lawrencecchen

🚥 Pre-merge checks | ✅ 25
✅ Passed checks (25 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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 Swift Actor Isolation ✅ Passed No production Swift files changed; the PR only touches workflow/test/script files, so no actor-isolation regression is introduced.
Cmux Swift Blocking Runtime ✅ Passed No Swift source files changed; the PR only touches workflows/tests, so the Swift blocking-runtime rule is not implicated.
Cmux Browser Automation Off-Main ✅ Passed Diff only touches CI/test scripts; no changes in Sources/TerminalController.swift or ControlCommandExecutionPolicy.swift, so the browser-socket routing rule isn't implicated.
Cmux Expensive Synchronous Load ✅ Passed The diff only changes CI workflows and test scripts; no production Swift files or main-actor load paths were touched, so the sync-load rule doesn’t apply.
Cmux Cache Substitution Correctness ✅ Passed PASS: The diff only touches workflow/test shell/Python files; no production Swift/TS/JS persistence/history/undo/snapshot code changed, so the cache rule doesn’t apply.
Cmux No Hacky Sleeps ✅ Passed PASS: The only new waits are in CI-only UI regression scaffolding; scripts/select-ci-xcode.sh adds no sleeps, and the policy allows test-only/CI orchestration delays.
Cmux Algorithmic Complexity ✅ Passed The diff only touches CI workflow and test files; no production Swift/TS/JS/shell/runtime code was changed, so algorithmic-complexity rules aren’t implicated.
Cmux Swift Concurrency ✅ Passed The PR only changes CI YAML and test scripts; no Swift source files were touched, so it doesn't add legacy Swift async patterns.
Cmux Swift @Concurrent ✅ Passed No Swift source files changed in the diff, so the @concurrent concurrency rule is not applicable.
Cmux Swift File And Package Boundaries ✅ Passed No production Swift files or package-boundary changes are in the diff; only CI YAML and test scripts changed.
Cmux Swiftpm Lockfiles ✅ Passed No cmux-owned .gitignore, Package.swift, Xcode project, or Package.resolved files changed; the diff only touches CI workflow/tests/scripts.
Cmux Swift Logging ✅ Passed No production Swift files changed; the diff only touches workflow/tests/scripts, so the Swift logging rule isn’t implicated.
Cmux User-Facing Error Privacy ✅ Passed Only CI/workflow/test files changed; the rule allows tests/docs/operational runbooks, and no end-user-facing error copy was introduced.
Cmux Full Internationalization ✅ Passed Diff only touches CI workflow/tests; no Swift/UI/web/messages/xcstrings/Info.plist or other user-facing localized text was added.
Cmux Swiftui State Layout ✅ Passed No SwiftUI code changed; the PR only touches CI workflow and test scripts, so the swiftui-state-layout rules are not implicated.
Cmux Architecture Rethink ✅ Passed Diff only touches CI/workflow and shell/test files; no Swift source changed, so the Swift-architecture rethink rule isn’t implicated.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PR touches only CI workflow/tests/scripts; no Swift window code or auxiliary-window identifiers changed, so the rule is not applicable.
Cmux Source Artifacts ✅ Passed Only workflow/tests/scripts changed; no artifact directories or generated outputs were added, and the rule explicitly allows source and test-system files.
Cmux No Test Or Debug Seam In Production Source ✅ Passed No production Sources/**/*.swift files changed; the PR only touches workflows, scripts, and tests, so it adds no test/debug seam in shipping source.
Cmux No Ambient Global State ✅ Passed No production Swift files changed in HEAD; the PR only touches CI workflow/scripts and Python/bash tests, so the ambient-global-state rule is not implicated.
Title check ✅ Passed It clearly captures the main change: gating required macOS CI behind a new linux preflight job.
Description check ✅ Passed It provides a detailed summary and verification of the change, covering the main template requirements.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch task-required-ci-shared-build

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.

@greptile-apps

greptile-apps Bot commented Jul 8, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR reorganizes the required CI path so Linux checks finish before macOS work starts. The main changes are:

  • Added a required linux-preflight gate before macOS jobs.
  • Merged display UI regressions into tests-build-and-lag.
  • Moved the Ghostty CLI helper artifact build into swift-package-tests.
  • Updated release-build to consume the helper artifact from swift-package-tests.
  • Added pinned Xcode SDK validation and matching CI coverage.

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.
  • The pinned Xcode path now checks the required SDK before accepting the selection.
  • The new fast-path test covers rejecting an older pinned SDK.

Important Files Changed

Filename Overview
scripts/select-ci-xcode.sh Pinned Xcode selection now checks the required macOS SDK before using the fast path.
.github/workflows/ci.yml The workflow now gates macOS jobs on Linux preflight and moves the helper artifact through swift-package-tests.
tests/test_ci_xcode_selection_fast_path.sh Adds coverage for pinned Xcode selection, wrong-SDK rejection, scan fallback, and missing pinned paths.
scripts/ci/run-display-ui-regressions.sh Extracts display UI regression orchestration into a shared CI script.
cmuxUITests/BrowserPaneNavigationKeybindUITests.swift Passes the target display ID into the UI test app launch environment.

Reviews (15): Last reviewed commit: "Clean display helper on final trap" | Re-trigger Greptile

Comment thread scripts/select-ci-xcode.sh
@azooz2003-bit
azooz2003-bit force-pushed the task-required-ci-shared-build branch from 9f84228 to 047d99c Compare July 8, 2026 04:21

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/test_ci_xcode_selection_fast_path.sh`:
- Around line 48-60: Add a test covering the CMUX_CI_XCODE_APP fast path in the
same test script by exercising the script through the CMUX_CI_XCODE_APP entry
point instead of only CMUX_CI_DEVELOPER_DIR. Verify the logic in the CI
selection path that derives the developer directory from
${CMUX_CI_XCODE_APP%/}/Contents/Developer, including the trailing-slash
stripping behavior and the expected selection outcome, so regressions in that
path are caught.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 8e2d4fa5-d6c3-41cf-a144-dd9decf06407

📥 Commits

Reviewing files that changed from the base of the PR and between da3707b and 047d99c.

📒 Files selected for processing (4)
  • .github/workflows/ci.yml
  • scripts/select-ci-xcode.sh
  • tests/test_ci_change_areas.py
  • tests/test_ci_xcode_selection_fast_path.sh

Comment thread tests/test_ci_xcode_selection_fast_path.sh
@azooz2003-bit
azooz2003-bit force-pushed the task-required-ci-shared-build branch from 047d99c to ff8c378 Compare July 8, 2026 04:37
Comment thread scripts/select-ci-xcode.sh

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
tests/test_ci_xcode_selection_fast_path.sh (1)

48-60: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Still missing CMUX_CI_XCODE_APP test coverage.

The CMUX_CI_XCODE_APP → CMUX_CI_DEVELOPER_DIR derivation path (select-ci-xcode.sh lines 55–57, including ${CMUX_CI_XCODE_APP%/}/Contents/Developer trailing-slash stripping) remains untested. This is the primary fast-path mechanism used by the CI workflow per the PR objectives. The past review comment suggested adding this test case; it has not been addressed.

♻️ Suggested additional test case
 # After the existing CMUX_CI_DEVELOPER_DIR success test (line 84), add:

 # Test: CMUX_CI_XCODE_APP derives the developer dir correctly
 app_output="$(
   PATH="$bin_dir:/usr/bin:/bin" \
     GITHUB_ENV="$env_file" \
     CMUX_TEST_XCODE_SELECT_LOG="$xcode_select_log" \
     CMUX_CI_XCODE_APP="$pinned_app" \
     CMUX_XCODE_APPLICATIONS_DIR="$tmp_dir/no-apps" \
     "$SCRIPT"
 )"

 if ! grep -Fq "Selected pinned Xcode (DEVELOPER_DIR): $pinned_developer (macOS SDK 26.2)" <<< "$app_output"; then
   echo "FAIL: CMUX_CI_XCODE_APP did not derive the correct developer dir"
   printf '%s\n' "$app_output" >&2
   exit 1
 fi
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/test_ci_xcode_selection_fast_path.sh` around lines 48 - 60, Add test
coverage for the CMUX_CI_XCODE_APP fast path in the CI Xcode selection script by
setting CMUX_CI_XCODE_APP and verifying select-ci-xcode.sh derives
CMUX_CI_DEVELOPER_DIR from it, including the
${CMUX_CI_XCODE_APP%/}/Contents/Developer trailing-slash handling. Extend
tests/test_ci_xcode_selection_fast_path.sh alongside the existing
pinned_app/pinned_developer scenario, and assert the derived developer dir is
used rather than relying only on CMUX_CI_DEVELOPER_DIR input.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@scripts/ci/run-display-ui-regressions.sh`:
- Around line 344-358: The browser-find regression run is creating a persistent
virtual display but never exports its display identifier to the test process. In
run_browser_find_focus, read the persistent display ID from PERSISTENT_ID_PATH
and pass it into the xcodebuild invocation as CMUX_UI_TEST_TARGET_DISPLAY_ID so
the app under test targets the same screen as the virtual display.

---

Duplicate comments:
In `@tests/test_ci_xcode_selection_fast_path.sh`:
- Around line 48-60: Add test coverage for the CMUX_CI_XCODE_APP fast path in
the CI Xcode selection script by setting CMUX_CI_XCODE_APP and verifying
select-ci-xcode.sh derives CMUX_CI_DEVELOPER_DIR from it, including the
${CMUX_CI_XCODE_APP%/}/Contents/Developer trailing-slash handling. Extend
tests/test_ci_xcode_selection_fast_path.sh alongside the existing
pinned_app/pinned_developer scenario, and assert the derived developer dir is
used rather than relying only on CMUX_CI_DEVELOPER_DIR input.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 788da195-0d2f-451a-8b50-e1e8bfed89b0

📥 Commits

Reviewing files that changed from the base of the PR and between 047d99c and ff8c378.

📒 Files selected for processing (7)
  • .github/workflows/ci.yml
  • scripts/ci/run-display-ui-regressions.sh
  • scripts/select-ci-xcode.sh
  • tests/test_ci_change_areas.py
  • tests/test_ci_release_sdk_lane.sh
  • tests/test_ci_self_hosted_guard.sh
  • tests/test_ci_xcode_selection_fast_path.sh

Comment thread scripts/ci/run-display-ui-regressions.sh

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (1)
tests/test_ci_xcode_selection_fast_path.sh (1)

48-61: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Add a test case for the CMUX_CI_XCODE_APP entry point (previously flagged).

The test still only exercises CMUX_CI_DEVELOPER_DIR directly. The CI workflow (.github/workflows/ci.yml:400-404) sets CMUX_CI_XCODE_APP — not CMUX_CI_DEVELOPER_DIR — so the primary fast-path mechanism (deriving the developer dir via ${CMUX_CI_XCODE_APP%/}/Contents/Developer) remains untested. A regression in the trailing-slash stripping or Contents/Developer suffix logic would silently break the main CI selection path.

♻️ Suggested additional test case
 # After the existing CMUX_CI_DEVELOPER_DIR success test (line 84), add:

 # Test: CMUX_CI_XCODE_APP derives the developer dir correctly
 app_output="$(
   PATH="$bin_dir:/usr/bin:/bin" \
     GITHUB_ENV="$env_file" \
     CMUX_TEST_XCODE_SELECT_LOG="$xcode_select_log" \
     CMUX_CI_XCODE_APP="$pinned_app" \
     CMUX_CI_REQUIRED_MACOS_SDK_MAJOR=26 \
     CMUX_XCODE_APPLICATIONS_DIR="$tmp_dir/no-apps" \
     "$SCRIPT"
 )"

 if ! grep -Fq "Selected pinned Xcode (DEVELOPER_DIR): $pinned_developer (macOS SDK 26.2)" <<< "$app_output"; then
   echo "FAIL: CMUX_CI_XCODE_APP did not derive the correct developer dir"
   printf '%s\n' "$app_output" >&2
   exit 1
 fi
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/test_ci_xcode_selection_fast_path.sh` around lines 48 - 61, Add
coverage for the CMUX_CI_XCODE_APP fast path in
tests/test_ci_xcode_selection_fast_path.sh instead of only setting
CMUX_CI_DEVELOPER_DIR. Update the test to exercise the selection logic that
derives the developer dir from CMUX_CI_XCODE_APP in the same path used by the CI
workflow, and verify the existing xcode-select assertions still pass through the
relevant helper/script entry point. Make sure the test covers the trailing-slash
handling and Contents/Developer suffix derivation so regressions in that path
are detected.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Duplicate comments:
In `@tests/test_ci_xcode_selection_fast_path.sh`:
- Around line 48-61: Add coverage for the CMUX_CI_XCODE_APP fast path in
tests/test_ci_xcode_selection_fast_path.sh instead of only setting
CMUX_CI_DEVELOPER_DIR. Update the test to exercise the selection logic that
derives the developer dir from CMUX_CI_XCODE_APP in the same path used by the CI
workflow, and verify the existing xcode-select assertions still pass through the
relevant helper/script entry point. Make sure the test covers the trailing-slash
handling and Contents/Developer suffix derivation so regressions in that path
are detected.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 71a4da8f-f2dd-4ec7-9fe5-a3fb0629e81e

📥 Commits

Reviewing files that changed from the base of the PR and between ff8c378 and 074e87a.

📒 Files selected for processing (4)
  • .github/workflows/ci.yml
  • scripts/select-ci-xcode.sh
  • tests/test_ci_change_areas.py
  • tests/test_ci_xcode_selection_fast_path.sh

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit e923d84. Configure here.

Comment thread .github/workflows/ci.yml

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
.github/workflows/ci.yml (1)

985-1063: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Add a test for routed-on jobs that are skipped
tests/test_ci_change_areas.py only checks that linux-preflight accepts routed jobs in success or skipped states. Add a case where a route output is true but the routed job is skipped, so preflight fails instead of letting macOS start.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/ci.yml around lines 985 - 1063, The linux-preflight gate
only verifies routed jobs by result state, but it does not have coverage for the
case where a routed job is marked true and still ends up skipped. Update
tests/test_ci_change_areas.py to add a scenario covering a routed output from
changes (for example macos/web/go/agent_session_web) being true while the
corresponding routed job result is skipped, and assert that the preflight logic
fails rather than allowing macOS work to continue.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/ci.yml:
- Around line 834-868: The release-build job is unnecessarily serialized through
the swift-package-tests artifact flow; build the Ghostty CLI helper directly
inside release-build using the existing Install zig, Cache Zig packages, and
Build universal Ghostty CLI helper steps, then remove the upload/download
artifact handoff and drop the release-build dependency on swift-package-tests.
Keep the helper creation and verification aligned with the existing
build-ghostty-cli-helper.sh and upload-ghostty-cli-helper step names so the lane
stays parallel and self-contained.

---

Outside diff comments:
In @.github/workflows/ci.yml:
- Around line 985-1063: The linux-preflight gate only verifies routed jobs by
result state, but it does not have coverage for the case where a routed job is
marked true and still ends up skipped. Update tests/test_ci_change_areas.py to
add a scenario covering a routed output from changes (for example
macos/web/go/agent_session_web) being true while the corresponding routed job
result is skipped, and assert that the preflight logic fails rather than
allowing macOS work to continue.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: c93fc67f-6a03-4a8c-8988-f7f5b9dc06ba

📥 Commits

Reviewing files that changed from the base of the PR and between 074e87a and e923d84.

📒 Files selected for processing (4)
  • .github/workflows/ci.yml
  • tests/test_ci_change_areas.py
  • tests/test_ci_release_sdk_lane.sh
  • tests/test_ci_self_hosted_guard.sh

Comment thread .github/workflows/ci.yml
@azooz2003-bit
azooz2003-bit enabled auto-merge (squash) July 8, 2026 08:19
@azooz2003-bit
azooz2003-bit merged commit b51ee74 into main Jul 8, 2026
28 of 30 checks passed
@azooz2003-bit
azooz2003-bit deleted the task-required-ci-shared-build branch July 8, 2026 08:24
austinywang added a commit that referenced this pull request Jul 8, 2026
Staging macOS CI behind linux-preflight (#7583) left the staged jobs
with plain conditions. linux-preflight survives its skipped web-job
ancestors via always(), but app-host-unit-tests, swift-package-tests,
tests-build-and-lag, and release-build did not use !cancelled(), so on
macOS-only diffs GitHub propagated the ancestors' skip through
linux-preflight and skipped every required macOS job; the tests
aggregation then failed with 'required but did not pass: skipped'.
The staging PR's own run masked this because it touched .github and ran
all web jobs. Require linux-preflight (and for release-build,
swift-package-tests) to have succeeded explicitly instead.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@austinywang

Copy link
Copy Markdown
Contributor

Heads-up: this staging setup skips all required macOS jobs on macOS-only diffs. linux-preflight survives its skipped web-job ancestors via always(), but app-host-unit-tests / swift-package-tests / tests-build-and-lag / release-build use plain if: conditions, so GitHub propagates the ancestors' skip through linux-preflight and skips them — then tests fails with "app-host unit tests were required but did not pass: skipped". This PR's own run masked it because touching .github made every web job run. Repro: https://github.com/manaflow-ai/cmux/actions/runs/28929880123 (macOS-only diff, all web jobs skipped, all staged macOS jobs skipped). Fix (adds !cancelled() && needs.linux-preflight.result == 'success' to the four staged jobs) is in #7533 commit 7b5b343 — feel free to cherry-pick to main so other branches unblock.

lawrencecchen added a commit that referenced this pull request Jul 8, 2026
#7622)

The change-area detector treats any path not explicitly macos-neutral as
a macOS change, so mux-only PRs resolved macos=true. Combined with the
new linux-preflight staging (#7583), that made the required app-host
Swift tests skip while the routing guard required them, failing 'tests'
and 'ci-status' on every mux-only PR (e.g. #7609).

cmux-mux is a standalone Rust project gated by its own 'mux' workflow and
never affects the macOS app build or app-host tests, so 'mux/' belongs in
is_macos_neutral. Adds test_mux_only_skips_macos.
lawrencecchen added a commit that referenced this pull request Jul 8, 2026
…obs (#7620)

* ci: stop skipped linux jobs from transitively skipping staged macOS jobs

Since #7583 staged macOS CI behind
linux-preflight, every PR that does not touch web/go/agent-session paths
fails CI: the routed linux jobs skip, GitHub's implicit success() gate
evaluates the transitive needs chain, and app-host-unit-tests,
swift-package-tests, tests-build-and-lag, and release-build all report
skipped even though linux-preflight itself succeeded. The tests gate then
fails with 'app-host unit tests were required but did not pass: skipped'.

Replace the implicit gate with an explicit direct-needs condition:
!cancelled() plus result == 'success' for each direct need, keeping the
macos route filter. #7583's own PR run missed this because workflow file
changes set every path filter true, so no routed job skipped there; the
same applies to this PR's run, so the skip path is provable only on a
macOS-only PR after merge.

* tests: macOS staging guard requires explicit direct-needs gate

test_macos_jobs_wait_for_linux_preflight asserted the exact bare macos
route literal, which is the condition that reintroduces the transitive
skip. Assert the !cancelled() + direct-needs form instead, and reject the
bare literal.
austinywang added a commit that referenced this pull request Jul 9, 2026
* Add failing regression tests for discarded browser webview restore retry (#7504)

A discarded browser webview whose restore navigation never commits
(connection refused, WebKit content-process death, dead localhost dev
server) permanently consumes its discard state, so every later reveal,
reload, or automation touch no-ops and the pane stays black forever.

Red tests only, per the two-commit regression policy:
- R1: manager-level — a restore whose navigation never starts/commits
  must leave the pane discarded and retryable.
- R2: panel end-to-end — connection-refused restore must leave the next
  restore touch able to retry.

CI on this commit is expected to fail these tests; the fix lands in the
next commit.

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

* Fix browser panes stuck black after a failed discard-restore (#7504)

Keep the discard state armed until a restore navigation actually
commits, so failed restores retry on the next touch instead of leaving
the pane permanently black:

- BrowserHiddenWebViewDiscardManager: restoreIfNeeded no longer clears
  the discard state before navigating; new isRestoreNavigationPending
  state machine (noteRestoreNavigationStarted / Committed /
  DidNotCommit) driven by real navigation-delegate signals; in-flight
  restores dedupe instead of double-navigating;
  reactivateWithoutNavigation no longer consumes state without a
  commit.
- BrowserPanel: didCommit / didFailNavigation /
  didCancelProvisionalNavigation hooks drive the state machine;
  error-page commits do not clear the state; stall detection retries
  silently-dead restores on the next reveal/automation touch;
  blank-shell heal re-navigates a never-committed shell that still has
  a URL intent on reveal transitions (never on visibility heartbeats,
  and never while an insecure-HTTP consent alert is pending);
  restore_pending / has_committed_document diagnostics.
- BrowserDiscardRestoreHeal (new): pure, unit-testable predicates for
  heal and stall eligibility, plus relocated lifecycle diagnostics
  helpers to stay inside the BrowserPanel.swift length budget.
- Green tests for the new state machine and heal predicates.

Fixes #7504

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

* Fix remote queued discard restore state

* Handle discarded restore review edge cases

* Ignore about:blank commits when tracking discarded-restore recovery

A navigation commit to about:blank (e.g. the placeholder document) must not
count as a successful discarded-webview restore; gate the restore-commit
bookkeeping on a real committed URL. Harden the retry test to wait for the
restore-pending flag to clear instead of only waiting for loading to settle.

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

* Refresh Swift file length budget for BrowserNavigationDelegate download callback

The discarded-restore fix adds a didBecomeDownload callback (property plus
two delegate call sites, +3 lines) to BrowserNavigationDelegate.swift.
Accept the growth in the checked-in budget.

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

* Mark pure lifecycle formatter helpers nonisolated

webViewLifecycleTimestamp and webViewHiddenDurationMilliseconds are pure
formatters and do not need MainActor isolation; align them with the
sibling nonisolated helpers in BrowserDiscardRestoreHeal.

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

* Fix download and no-URL edge cases in discarded webview restore

Two review findings on the restore retry state machine:

- A main-frame download cleared discard state but never committed a
  document, so blank-shell healing re-navigated to the download URL on
  every reveal, restarting the download. Treat a main-frame download as
  a committed terminal outcome for the replaced web view.

- A discarded pane whose restore URL is nil or about:blank navigated (or
  skipped navigating) into a state whose commit is intentionally ignored,
  leaving the manager marked discarded (or restore-pending) forever and
  blocking future discards. Reactivate such panes in place through the
  existing reactivateWithoutNavigation path.

Widen navigationDelegate to internal so the download regression test can
drive the didBecomeDownload callback via @testable import, and raise the
test settle timeout for loaded CI hosts.

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

* Seed committed flag for adopted prewarmed webviews; move commit predicate

A prewarmed webview is only claimable after its load finished, but the
commit happened under the pool's delegate, so the panel's
hasCommittedDocumentSinceWebViewReplacement stayed false and blank-shell
healing reloaded the adopted page on first reveal. Seed the flag at
adoption.

Move shouldTreatCommitAsDiscardedRestoreCommit next to its sibling
restore-heal predicates in BrowserDiscardRestoreHeal.swift and refresh
the BrowserPanel.swift length budget for the net restore-retry growth.

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

* Gate blank-shell heal off during pending WebContent crash recovery

A webview replaced after WebContent process termination waits for the
user's explicit Reload (hasRecoverableWebContentTermination). The
blank-shell heal predicate did not know about that gate, so a hidden
crashed pane would auto-navigate on the next reveal, clear the recovery
overlay, and could re-enter the crash loop. Add the recovery flag to
shouldHealBlankShell and cover it in the predicate tests.

Also move the no-restorable-URL restore fallback into
BrowserDiscardRestoreHeal so BrowserPanel.swift stays below its pre-PR
length (the guard job's hard cap forbids any growth of files over 900
lines), and drop the now-unneeded budget bump.

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

* Cache the lifecycle-payload ISO8601 formatter

webViewLifecycleTopPayload runs on the polled debug-socket/top path for
every browser panel; allocate the documented-thread-safe formatter once
instead of per timestamp field, matching CmuxEventBus and Workspace.

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

* Scope committed-document tracking to each discarded restore attempt

didCommit sets hasCommittedDocumentSinceWebViewReplacement even for
error-page commits, where the discard manager intentionally stays
discarded. A later restore retry that produced no navigation callbacks
was then never detected as stalled, leaving the pane stuck pending.
Reset the flag when a discarded restore navigation starts so each
attempt tracks its own commit.

Move the restore-milestone helpers next to the other discard-restore
logic in BrowserDiscardRestoreHeal.

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

* ci: keep staged macOS jobs alive when web jobs skip

Staging macOS CI behind linux-preflight (#7583) left the staged jobs
with plain conditions. linux-preflight survives its skipped web-job
ancestors via always(), but app-host-unit-tests, swift-package-tests,
tests-build-and-lag, and release-build did not use !cancelled(), so on
macOS-only diffs GitHub propagated the ancestors' skip through
linux-preflight and skipped every required macOS job; the tests
aggregation then failed with 'required but did not pass: skipped'.
The staging PR's own run masked this because it touched .github and ran
all web jobs. Require linux-preflight (and for release-build,
swift-package-tests) to have succeeded explicitly instead.

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

* ci: align staged macOS job gates with upstream fix and update guard test

Adopt the exact conditions from ci-fix-macos-staged-skip (PR #7620) so the
staged macOS jobs survive skipped routed linux ancestors, and teach
tests/test_ci_change_areas.py the new explicit direct-needs gate (it
asserted the old literal if-string, which also fails PR #7620 as pushed).

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

* Keep restore-stall detection armed after an about:blank commit

A restore navigation that dead-ends in WebKit's about:blank placeholder
set hasCommittedDocumentSinceWebViewReplacement, which disabled the
stall detector while the discard manager stayed pending, wedging the
pane in restore bookkeeping. Only real document commits (including
error pages) set the flag now, so the next restore touch detects the
stall, clears the pending state, and retries.

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

* Let explicit reloads restart pending restores; attribute restore callbacks

Two review findings on the pending-restore state:

- restoreIfNeeded deduplicated while a restore navigation was pending,
  so an explicit reload/hard-reload during an in-flight restore was
  swallowed as handled. Add a force flag that clears the pending bit and
  restarts the restore; reload paths pass it.

- WebKit can deliver an older provisional load's failure/cancellation
  after a newer attempt already started; the shared callbacks then
  cleared the pending bit for the active attempt, letting a visibility
  touch hijack the in-flight navigation with a restore reload. Track the
  WKNavigation returned by the restore load and only clear pending state
  for callbacks that match it.

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

* Never blank-shell-heal over an explicit user Stop

Stopping a pre-commit load left the heal predicate satisfied (rendered,
idle, no committed document, non-blank intent URL), so the next reveal
silently restarted the stopped navigation. Track an explicit-stop flag
per webview replacement, clear it when a new navigation starts, and fail
the heal predicate closed while it is set; covered in the predicate test.

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

* Make explicit Stop sticky for discarded restores; move restore flow to heal file

A user Stop during a discarded-webview restore left the manager
discarded, so the next visibility touch restarted the stopped load
through restoreIfNeeded. Honor the explicit-stop flag in the restore
touch as well, with explicit reload (forceRestartPendingRestore) as the
override that clears it.

Move restoreDiscardedWebViewIfNeeded and healBlankRestoredWebViewIfNeeded
next to the rest of the discard-restore logic in BrowserDiscardRestoreHeal,
widening the members they use, so BrowserPanel.swift stays under its
no-growth cap.

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

* Never blank-shell-heal over the browser error page

The error page commits as about:blank (baseURL nil), so the commit gate
left it looking uncommitted and healing re-requested the failed URL on
the next reveal. Treat an active error page as content awaiting the
user's Reload in shouldHealBlankShell. The discarded-restore retry path
is unaffected (it flows through the manager, not healing).

Split the pure predicate coverage into
BrowserDiscardRestoreHealPredicateTests (wired into the Xcode project)
so the retry test file stays under the 500-line tracking threshold.

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

* Add queued remote restore regression test

* Deduplicate queued remote discard restores

* Complete policy-cancelled discard restores

* Handle policy-cancelled browser restores explicitly

* Keep intent fallback restores retryable

* Complete insecure HTTP prompt restores

* Preserve current restore attempts on stale cancels

* Defer external prompt restore completion

* Clear stale restore navigation on stalls

* Tokenize browser restore policy cancels

* Keep insecure HTTP restore prompts retryable

* Avoid browser panel budget growth

* Preserve restore tokens through policy prompts

* Scope restore downloads to attempts

* Complete terminal restore handling for nil-target tabs

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

This branch was successfully deployed

1 active deployment
Preview – cmux — 652eb846 Deployed Jul 8, 2026 by vercel[bot]
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.

2 participants