Repository navigation
ci: stop skipped linux jobs from transitively skipping staged macOS jobs - #7620
Merged
Merged
Conversation
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.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 8 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 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 |
Contributor
Greptile SummaryThis PR fixes macOS CI jobs that were skipped through the Linux preflight dependency chain. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (2): Last reviewed commit: "tests: macOS staging guard requires expl..." | Re-trigger Greptile |
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 8, 2026
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>
Contributor
|
Note: |
4 tasks done
Contributor
|
Friendly nudge: all required checks are green here (only the non-required Vercel deploy statuses pending). Five review-clean memory-audit PRs for #7596 (#7616, #7617, #7621, #7624, #7625) are queued behind this fix — they all fail only on the #7618 aggregator-skip signature and will re-trigger against main as soon as this lands. Happy to help if anything's still open on it. |
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 (web-typecheck, react-apps-check, web-db-migrations, remote-daemon-tests, agent-session-web-resources) legitimately skip on such PRs; GitHub's implicitsuccess()job gate evaluates the transitive needs chain, soapp-host-unit-tests,swift-package-tests,tests-build-and-lag, andrelease-buildall report skipped even thoughlinux-preflightitself ran withif: always()and succeeded. Thetestsgate then fails withapp-host unit tests were required but did not pass: skipped, andci-statusfails with it.Evidence: every pull_request CI run created after b51ee74 merged (08:24Z) shows the identical signature, e.g. https://github.com/manaflow-ai/cmux/actions/runs/28929597020 (feat-diff-viewer-fast-first-paint), https://github.com/manaflow-ai/cmux/actions/runs/28929672257, https://github.com/manaflow-ai/cmux/actions/runs/28929644066 — all with
changes.macos=true,linux-preflight=success, macOS jobs skipped.Fix: replace the implicit gate on the four staged macOS jobs with an explicit direct-needs condition:
!cancelled()(which disables the implicit transitivesuccess()) plusresult == 'success'for each direct need (changes,linux-preflight, and for release-build alsoswift-package-tests), keeping thechanges.outputs.macos == 'true'route filter. Staging semantics are preserved: macOS jobs still only start after linux-preflight passes.Verification caveat: #7583's own PR run missed this because workflow-file changes set every path filter true, so no routed job skipped there. This PR's own run has the same property, so the skip path is only provable on a macOS-only PR after merge (e.g. re-run #7605 by updating its branch).
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Note
Low Risk
Changes are limited to GitHub Actions job
ifexpressions and workflow guard tests; staging semantics (macOS only after preflight) are preserved.Overview
Fixes false CI failures on macOS-only PRs where required macOS jobs were skipped even though
linux-preflightsucceeded.After staging macOS work behind
linux-preflight, GitHub’s implicitsuccess()on jobneedsstill walks the transitive dependency graph. Routed Linux jobs (web-typecheck,remote-daemon-tests, etc.) legitimately skip when path filters don’t match; that skip propagated and skippedapp-host-unit-tests,swift-package-tests,tests-build-and-lag, andrelease-build, which then broke thetestsandci-statusgates.The four jobs now use an explicit
if:!cancelled()plusneeds.changes/needs.linux-preflight(andneeds.swift-package-testsfor release-build)result == 'success', still gated onchanges.outputs.macos == 'true'.tests/test_ci_change_areas.pyasserts those conditions so the old bare macOS-onlyifcannot regress.Reviewed by Cursor Bugbot for commit 2a58d9f. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Fixes CI so macOS jobs no longer get skipped when routed Linux jobs skip. Adds explicit direct-needs checks so macOS jobs run after
linux-preflightsucceeds and the macOS path filter matches, and adds a test to enforce this guard.!cancelled()+ explicitneeds.*.result == 'success'.needs.changes.outputs.macos == 'true'.Written for commit 2a58d9f. Summary will update on new commits.