Repository navigation
ci(cmux-next media): let tours run side by side on one Mac - #22046
Conversation
The tour never activates its app, a build without a window snapshot is captured by its window server id rather than a rectangle of the display, and a tour holds its tag for the rest of its job. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
tour.py no longer activates the app it drives through its socket, and its fallback for a build without debug.window_snapshot captures that window alone (screencapture -l), never the display. A new step holds the tour's tag (media_tag_lock.py) so one tag's DerivedData, sockets and daemon take one tour at a time while other tags run beside it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
|
cmux status (one comment, edited in place; each line is updated by the job that owns it) Review App of this head (app.zip, kept 1 day)
|
|
Bead |
The app refuses activation itself, so tours side by side never take focus or the key window from each other. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
Red: prune_media_derived_data.py has no --lock-dir and removes a held tag's base and head. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ld it - prune skips a tag whose lock a tour holds, so a tour of another pull request never deletes DerivedData a tour is about to build into - the hold step fetches media_tag_lock.py from the run's commit when the pull request head predates it, and waits at most 30 minutes - the last-resort window capture picks the on-screen window that matches the app's frame Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Review (subagent, correctness first) on f0fe23e, and what 7468a34 does about each finding:
|
|
Found 1 test failure on Blacksmith runners: Failure
|
[Medium impact] The changes affect shared Mac resources used by the cmux-next media-tour workflow. Fix the display-shot caller before merging; it currently stops tours that request a display shot. Findings
SummaryAllows media tours to share one Mac while keeping each tour's captures and build files separate.
Both earlier unnumbered findings are fixed: the prune keeps its lock through deletion, and older heads receive both lock-aware scripts. Diagram%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Tour starts] --> B[Acquire tag lock]
B --> C[Prune idle DerivedData]
C --> D[Build base and head]
D --> E[Launch without activation]
E --> F[Capture app window]
F --> G[Job ends and releases lock]
C --> H[Skip tags held by other tours]
Reviews (6) · Last reviewed commit: "Merge branch 'feat-cmux-next' into ci/to..." · Reviewed by Greptile |
The prune probes a dir's tag lock and lets it go before its rmtree, so a tour can take the tag and start building into a dir that is being removed. This drives the real prune main with the real media_tag_lock.py hold asking for the tag inside the delete. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claim_tag takes the tag's lock (creating it, as media_tag_lock.py does) without waiting and keeps it across allocated_gib and rmtree. A tour that asks for the tag meanwhile waits on its lock. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…b can run Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A dispatch of feat-cmux-next waits behind a merge tour that holds the tag for up to an hour warm (54 min seen) and longer cold; 30 min failed it. Wait the tour job's 120 min timeout instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A head branched before the tag lock has a prune that ignores held tags, and the prune step ran it from the head. The hold step now takes both scripts from the run's commit for such a head and hands their dir to the prune step. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Bugbot needs on-demand usage enabledBugbot uses usage-based billing for this team and requires on-demand usage to be enabled. A team admin can enable on-demand usage in the Cursor dashboard. |
|
Pushed 82b9d03 (merges feat-cmux-next 86efbba):
Reds on 1b8a89f that aren't this PR's:
Local runs on Huge Blue: test_media_tag_lock.py, test_ci_prune_media_derived_data.py and test_cmux_next_tour.py all pass. actionlint is clean. |
tests/test_media_tag_lock.py sits in the linux-guard lane but no workflow ran it, so its tour-wait and prune-lock checks never executed in CI. The release-notary group now runs it next to the prune test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The job timeout counts from job start, so a 120-min wait for a 120-min job is never reached: a starved tour is cancelled instead of failing with "still held", and one that gets the tag late times out mid-build. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The wait ends while the waiter still has time to build and tour. The old-head fallback fetches the run's commit without --depth so the reused self-hosted checkout does not turn shallow. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
For whoever lands this: #22174 is built on this branch's head (82b9d03) and replaces the per-PR tag with a per-Mac slot claimed through this PR's lock ( |
|
Re-review (subagent, correctness first) of the commits since the last review: no defects. SHIP at . |
|
|
||
| def use_display(self) -> None: | ||
| """Capture the window's rect on the display from now on; follow() again after.""" | ||
| def use_display(self, pid: Any) -> None: |
There was a problem hiding this comment.
Screen.use_display() now requires pid, but display_capture() still calls it without arguments. When a screen capture route is available, a step with display: true raises an unhandled TypeError and stops the tour. The standard core.json tour hits this through its Profile menu step, even though that step is optional.
Update display_capture() to pass the app's PID.
…ur-slots Conflicts came from #22046 (squash-merged into base) and #22170. Each conflicting file is base's version plus this PR's own slot-claim diff: - cmux-next-media.yml: the claim step replaces base's one-hour hold; the old-head fallback keeps base's fetch without --depth. - test_ci_media_module_cache_stamp.py: #22170's docstring and warm rebuild check stay; the slot tours run in the slot-numbered dirs. - test_media_tag_lock.py: the claim tests replace base's hold --wait test, since a tour no longer waits for a tag.
|
Shipped in:
|
… tour (#22717) All merge (push) runs shared one concurrency group. Each merge waited behind the running tour, and the next merge replaced the waiting run: 95 of the 100 cancelled cmux-next-media runs on 2026-10-20 were zero-job push runs replaced this way, and each lost its tour and its PR comment. Every merge now gets its own group; tours already run side by side in a Mac's slots (#22046), and a merge with nothing for a Mac to tour still ends in the route job. Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=e8f63fd9-94e8-4878-bcd4-704659aba1f2&host=cosmic-gray
…id, reload shim test, fleet-test-compile runners) (#23553) * fix(cmux-next media): a display step's shot passes the app's pid to the screen route Screen.use_display takes the app's pid since #22046 (it finds the window server id by owner pid), and run_tour passes it; display_capture still called use_display() with no argument, so every `display` tour step died with a TypeError. tests/test_cmux_next_tour.py's display-step cases were red on feat-cmux-next (preflight guard group). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15c31-89b3-7d62-8e8c-b116a23ae693&host=cosmic-gray * fix(ci): the reload DerivedData test loads the shim writer from scripts/lib/cmux-dev-shim-install reload.sh now sources write_dev_cli_shim and reload_socket_is_live from scripts/lib/cmux-dev-shim-install (the one source of the managed shim), so the test's awk extraction from reload.sh found nothing and failed with "write_dev_cli_shim not found in reload.sh" on feat-cmux-next (preflight guard group). The test sources the library as reload.sh does, and checks reload.sh still sources it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15c31-89b3-7d62-8e8c-b116a23ae693&host=cosmic-gray * fix(ci): fleet-test-compile.yml runs on Blacksmith, not a GitHub-hosted label #23423's plan job and its empty-plan compile leg selected ubuntu-24.04, which tests/test_ci_self_hosted_guard.sh refuses (a GitHub billing block must not stop CI): "Validate macOS runner guards" was red on feat-cmux-next (preflight guard group). Both take the standard selector: the fork branch, then vars.LINUX_RUNNER with the Blacksmith fallback; a needed plan still takes vars.CI_COMPILE_CHECK_GATEWAY. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15c31-89b3-7d62-8e8c-b116a23ae693&host=cosmic-gray * fix(ci): fleet-test-compile.yml sends fork pull requests to Blacksmith before any runner variable The previous commit put vars.LINUX_RUNNER (and the compile gateway) right after the owner branch, so tests/test_ci_fork_runner_routing.py flagged both lines: a fork pull request could reach an owned runner label. Both selectors now use the standard form (agent-brands.yml, backend.yml): the fork pull-request branch first, then the gateway and LINUX_RUNNER. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15c31-89b3-7d62-8e8c-b116a23ae693&host=cosmic-gray --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Media tours can now run several at once on one Mac, each capturing only its own app's window. That lets the runner hook drop its one-tour-per-Mac limit.
What changes:
The tour no longer activates its app, and launches it with
CMUX_NEXT_NO_ACTIVATE=1. It drives the app through the app's own control socket and screenshots it withdebug.window_snapshot, so being in front never mattered. Concurrent tours would otherwise trade focus and the key window back and forth.The fallback captures the window, not the screen. A build without
debug.window_snapshotused to be captured withscreencapture -R, a rectangle of the display, which would include whatever covers the window, such as another tour's app. It now captures that window alone:screencapture -l <id>, or the screen-capture helper'swindow <id>. The window server id comes from the window list by the app's pid. With no id there is no shot, never the display. The manifest recordscapture_window. This route leaves out child windows (sheets, popovers) thatdebug.window_snapshotincludes; it is only for old bases without that verb.A tour holds its tag (
scripts/ci/media_tag_lock.py, new step "Hold this tour's tag"). One tag's warm DerivedData, sockets and daemon session take one tour at a time; other tags never wait. The concurrency group already keeps two runs of a pull request apart, but not these cases:feat-cmux-nexttag.The lock lets go when the job's
Runner.Workerexits, or after 3 h. A second tour of a tag waits up to 60 min: longer than a warm merge tour holds the shared tag (54 min seen), while leaving the waiter 45+ min of its 120-min job for a cold build and the tour. A longer wait would only end in a job-timeout cancel instead of the "still held" error. When the pull request head predates the lock, the step takes the lock script and the lock-aware prune from the run's commit, so unrebased PRs still run and their prune never deletes a held tag.The DerivedData prune skips a held tag and holds a tag's lock itself until its dir is gone, so a tour never builds into DerivedData that is being deleted, or has its DerivedData deleted by another pull request's tour.
What still needs the console, and stays on the console runner, is unchanged here: app-host XCTest (testmanagerd), E2E video recording and frame-pacing benches.
Validation
tests/test_cmux_next_tour.pyand the newtests/test_media_tag_lock.py(linux-guard lane), red then green:tour.pyand with no lock script,test_the_tour_never_activates_the_appandtest_without_a_window_snapshot_the_tour_uses_a_screen_routefail, and so do 3 of the 4 lock tests.--lock-dir. At 7468a34 it passes, and so do the lock and tour tests.actionlint .github/workflows/cmux-next-media.ymlis clean.7200 not less than or equal to 4500); at bd085a7 it passes.tests/test_media_tag_lock.pywas in the linux-guard lane but no workflow ran it;ci-guards.ymlnow runs it in the release-notary group. All four tour/lock/prune tests pass on Huge Blue at bd085a7.CMUX_NEXT_NO_ACTIVATE), so focus styling looks inactive in tour media.ExplicitPrecompiledModules/Darwin-*.pcmtour failures (14 of 22 recent failed tours) are not from the prune: every one I checked (jobs 118285853270, 118197207461, 118076888558, 118006980751) followedmedia_module_cache_stamp.py clearremovingExplicitPrecompiledModulesafter a binary pin change, while the build's dependency records still point at those .pcm files. The prune only removes whole idlecmux-ci-media-*dirs, and now only under the tag lock. fix(cmux-next media): a pin change also drops Xcode's build description #22181 fixes that clear.release-notaryfailsNEXT post-publication dependency contractthe same way on feat-cmux-next (job 118292665776), andcmux-next checksfails the same six checks (state roots, concurrency, strings, type scale, script tests, web bundle key) as merged agent pane: permission UI keeps its last state across a chat switch (no blink) #22054 (job 118309312276), being fixed in cmux-next: clear every base red in cmux-next checks (type scale, strings, tree key, state root, concurrency, bundle key) #22117.tour.py, which still activates the app and captures a rectangle, until they merge main.Runtime test: tests/test_media_tag_lock.py, tests/test_ci_prune_media_derived_data.py and tests/test_cmux_next_tour.py in the release-notary/preflight guard jobs at bd085a7
Changelog
none
Review: SHIP at bd085a7 (cmux-review subagent; correctness first)
🤖 Generated with Claude Code