Repository navigation
cmux-next: screenshot and video tours for UI pull requests - #16427
Merged
Merged
Conversation
A new cmux-next-media workflow builds the head as a tagged dev app on an owned Mac's gui runner, launches it on a private control socket, drives a tour through the bundled cmux CLI and captures a screenshot per step plus frames for a video. The publish job uploads them to pr-media/<pr>/<sha8>/ and keeps one sticky comment on the pull request. Tours are JSON in scripts/cmux-next/media/tours so other lanes can add steps. The runner is picked by scripts/ci/cmux_next_media_runner.py and never falls back to Blacksmith, whose sessions cannot capture. Co-Authored-By: Claude Opus 5.5 (1M context) <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 |
Contributor
|
|
This comment has been minimized.
This comment has been minimized.
Contributor
|
Passes: CI passes on CI passes on Written by |
Contributor
|
cmux-next tour of The tour job failed; failed steps are below, and the run log has the build output. Core UI: 7 ok, 3 failed, 2 unavailable
Tours are data in |
…rget function)
ResourceControl.run declares `let target = try await MainActor.run { try
target(...) }`. Swift 6.3 (Xcode 27) resolves the call to the static
function; Swift 6.2 (Xcode 26.3 and 26.6, which the fleet workers use)
resolves it to the local being declared and fails with "generic parameter
'T' could not be inferred", so every fleet build of feat-cmux-next since
3d9c39f failed (fleet job 20ae3ebca821c9f69f0b857f). Call Self.target.
Reproduced and checked with a minimal file under Xcode 26.3 swiftc.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
(cherry picked from commit 822db55)
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
8 issues found and verified against the latest diff
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/cmux-next/media/publish.py">
<violation number="1" location="scripts/cmux-next/media/publish.py:187">
P2: The generated MP4 bypasses `pr-media.py`'s 25 MB file limit. Check the video with `pr_media.check_size(local, "video")` before uploading, otherwise larger tours leave partial media and no comment.</violation>
</file>
<file name="tests/test_cmux_next_media.py">
<violation number="1" location="tests/test_cmux_next_media.py:60">
P3: `TourFormat.write()` and `Comment.test_dry_run_renders_from_tour_output()` create temp directories with `tempfile.mkdtemp()` and never remove them, leaking a directory in `/tmp` for every test run (9 in the `test_bad_steps` subTests alone). Track them and clean up in `tearDown`, or use `tempfile.TemporaryDirectory()` as a context manager in each test.</violation>
</file>
<file name="scripts/cmux-next/media/tours/core.json">
<violation number="1" location="scripts/cmux-next/media/tours/core.json:13">
P3: The tour screenshots the sidebar only in its hidden state: "Hide the sidebar" captures a shot, while "Show the sidebar" sets `shot: false`. For UI visual evidence of the sidebar feature this skips the one state a reviewer wants to see. Capture the visible state instead (e.g., drop `shot: false` here, or put it on the Hide step if the hidden-state shot is redundant).</violation>
</file>
<file name="scripts/cmux-next/media/tour.py">
<violation number="1" location="scripts/cmux-next/media/tour.py:174">
P2: The screenshot rect comes from `NSScreen.frame` (`Screen.__init__`/`follow`), which is in points, while `screencapture -R` interprets its rectangle in the display's backing pixel space. On a Retina (2x) runner the point-based rect covers only the top-left quarter of the actual window pixels, cropping every step screenshot and every video frame. Verify the owned runner's display scale, and multiply by `backingScaleFactor` (or capture by CGWindow ID) if it is 2x.</violation>
<violation number="2" location="scripts/cmux-next/media/tour.py:201">
P3: A failed frame capture can leave an empty `frame-NNNNN.jpg` behind: `capture()` treats `st_size == 0` as an error but never deletes the file, and the recorder only skips appending the frame name. When the failing capture is the last one, ffmpeg's `frame-%05d.jpg` pattern reads the empty jpg and `make_video` drops the entire mp4/GIF for the tour. Unlink the file when a capture fails (also for step screenshots, so a failed screenshot does not leave a 0-byte png that confuses uploads).</violation>
<violation number="3" location="scripts/cmux-next/media/tour.py:272">
P1: Run daemon-scoped CLI calls as the console user, or pass the daemon’s explicit socket path. With a different runner and console UID, `--session cmux-app[-tag]` resolves under the runner’s runtime directory and cannot reach the daemon started by the app, causing required terminal and notification tour steps to fail on the supported cross-user setup.</violation>
<violation number="4" location="scripts/cmux-next/media/tour.py:411">
P2: Treat `shot_error` as a tour failure. The current status check returns success when `screencapture` fails for an otherwise successful required step, so CI can publish a green tour without the promised screenshot.</violation>
</file>
<file name=".github/workflows/cmux-next-media.yml">
<violation number="1" location=".github/workflows/cmux-next-media.yml:35">
P3: Fork PRs never receive the explanatory comment this workflow promises. The `if` on the `route` job skips the entire job for fork heads, so `needs.route.outputs.reason` is empty, and `publish`'s `if: ${{ !cancelled() && needs.route.result == 'success' }}` evaluates false (route is `skipped`) — publish is skipped too, so no comment is posted at all. That also makes `cmux_next_media_runner.py`'s `if not same_repo` branch unreachable. The header comment and PR description state the comment explains why the tour did not run; for forks it never does. If the skip is intentional (fork-controlled `publish.py` must not run with the pr-media write token), update the header/description to state forks are excluded without a comment, and drop the dead fork branch or route only forks into a reason-less path.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| leading = [word for word in args[:1] if word in ("--json", "--jsonl", "--quiet")] | ||
| argv = [str(self.cli_path), *leading, *scope, *args[len(leading):]] | ||
| try: | ||
| return subprocess.run(argv, capture_output=True, text=True, timeout=timeout, env=self.environment()) |
There was a problem hiding this comment.
P1: Run daemon-scoped CLI calls as the console user, or pass the daemon’s explicit socket path. With a different runner and console UID, --session cmux-app[-tag] resolves under the runner’s runtime directory and cannot reach the daemon started by the app, causing required terminal and notification tour steps to fail on the supported cross-user setup.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At scripts/cmux-next/media/tour.py, line 272:
<comment>Run daemon-scoped CLI calls as the console user, or pass the daemon’s explicit socket path. With a different runner and console UID, `--session cmux-app[-tag]` resolves under the runner’s runtime directory and cannot reach the daemon started by the app, causing required terminal and notification tour steps to fail on the supported cross-user setup.</comment>
<file context>
@@ -0,0 +1,447 @@
+ leading = [word for word in args[:1] if word in ("--json", "--jsonl", "--quiet")]
+ argv = [str(self.cli_path), *leading, *scope, *args[len(leading):]]
+ try:
+ return subprocess.run(argv, capture_output=True, text=True, timeout=timeout, env=self.environment())
+ except subprocess.TimeoutExpired as expired:
+ return subprocess.CompletedProcess(argv, 124, expired.stdout or "", f"timed out after {timeout:.0f}s")
</file context>
Comment on lines
+187
to
+189
| for local in (mp4, gif): | ||
| if local: | ||
| files[local.name] = local |
There was a problem hiding this comment.
P2: The generated MP4 bypasses pr-media.py's 25 MB file limit. Check the video with pr_media.check_size(local, "video") before uploading, otherwise larger tours leave partial media and no comment.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At scripts/cmux-next/media/publish.py, line 187:
<comment>The generated MP4 bypasses `pr-media.py`'s 25 MB file limit. Check the video with `pr_media.check_size(local, "video")` before uploading, otherwise larger tours leave partial media and no comment.</comment>
<file context>
@@ -0,0 +1,209 @@
+ into = Path(scratch) / name
+ into.mkdir()
+ mp4, gif = make_video(directory, manifest, into, pr_media)
+ for local in (mp4, gif):
+ if local:
+ files[local.name] = local
</file context>
Suggested change
| for local in (mp4, gif): | |
| if local: | |
| files[local.name] = local | |
| for local in (mp4, gif): | |
| if local: | |
| pr_media.check_size(local, "video" if local.suffix == ".mp4" else "gif") | |
| files[local.name] = local |
|
|
||
|
|
||
| def passed(manifest: dict[str, Any]) -> bool: | ||
| return "error" not in manifest and all(step["status"] != "failed" for step in manifest["steps"]) |
There was a problem hiding this comment.
P2: Treat shot_error as a tour failure. The current status check returns success when screencapture fails for an otherwise successful required step, so CI can publish a green tour without the promised screenshot.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At scripts/cmux-next/media/tour.py, line 411:
<comment>Treat `shot_error` as a tour failure. The current status check returns success when `screencapture` fails for an otherwise successful required step, so CI can publish a green tour without the promised screenshot.</comment>
<file context>
@@ -0,0 +1,447 @@
+
+
+def passed(manifest: dict[str, Any]) -> bool:
+ return "error" not in manifest and all(step["status"] != "failed" for step in manifest["steps"])
+
+
</file context>
Suggested change
| return "error" not in manifest and all(step["status"] != "failed" for step in manifest["steps"]) | |
| return "error" not in manifest and all(step["status"] != "failed" and "shot_error" not in step for step in manifest["steps"]) |
| {"title": "New terminal tab", "cmux": ["tab", "new-terminal"]}, | ||
| {"title": "Browser tab", "cmux": ["tab", "new-browser", "--url", "https://example.com"], "wait": 5}, | ||
| {"title": "Hide the sidebar", "cmux": ["action", "run", "toggleSidebar"]}, | ||
| {"title": "Show the sidebar", "cmux": ["action", "run", "toggleSidebar"], "shot": false}, |
There was a problem hiding this comment.
P3: The tour screenshots the sidebar only in its hidden state: "Hide the sidebar" captures a shot, while "Show the sidebar" sets shot: false. For UI visual evidence of the sidebar feature this skips the one state a reviewer wants to see. Capture the visible state instead (e.g., drop shot: false here, or put it on the Hide step if the hidden-state shot is redundant).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At scripts/cmux-next/media/tours/core.json, line 13:
<comment>The tour screenshots the sidebar only in its hidden state: "Hide the sidebar" captures a shot, while "Show the sidebar" sets `shot: false`. For UI visual evidence of the sidebar feature this skips the one state a reviewer wants to see. Capture the visible state instead (e.g., drop `shot: false` here, or put it on the Hide step if the hidden-state shot is redundant).</comment>
<file context>
@@ -0,0 +1,21 @@
+ {"title": "New terminal tab", "cmux": ["tab", "new-terminal"]},
+ {"title": "Browser tab", "cmux": ["tab", "new-browser", "--url", "https://example.com"], "wait": 5},
+ {"title": "Hide the sidebar", "cmux": ["action", "run", "toggleSidebar"]},
+ {"title": "Show the sidebar", "cmux": ["action", "run", "toggleSidebar"], "shot": false},
+ {"title": "Notification", "daemon": true,
+ "cmux": ["notify", "--title", "Tour", "--body", "Hello from CI"], "wait": 2},
</file context>
Suggested change
| {"title": "Show the sidebar", "cmux": ["action", "run", "toggleSidebar"], "shot": false}, | |
| {"title": "Show the sidebar", "cmux": ["action", "run", "toggleSidebar"]}, |
| jobs: | ||
| route: | ||
| name: Pick the tour runner | ||
| if: ${{ github.repository_owner == 'manaflow-ai' && github.event.pull_request.head.repo.full_name == github.repository }} |
There was a problem hiding this comment.
P3: Fork PRs never receive the explanatory comment this workflow promises. The if on the route job skips the entire job for fork heads, so needs.route.outputs.reason is empty, and publish's if: ${{ !cancelled() && needs.route.result == 'success' }} evaluates false (route is skipped) — publish is skipped too, so no comment is posted at all. That also makes cmux_next_media_runner.py's if not same_repo branch unreachable. The header comment and PR description state the comment explains why the tour did not run; for forks it never does. If the skip is intentional (fork-controlled publish.py must not run with the pr-media write token), update the header/description to state forks are excluded without a comment, and drop the dead fork branch or route only forks into a reason-less path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At .github/workflows/cmux-next-media.yml, line 35:
<comment>Fork PRs never receive the explanatory comment this workflow promises. The `if` on the `route` job skips the entire job for fork heads, so `needs.route.outputs.reason` is empty, and `publish`'s `if: ${{ !cancelled() && needs.route.result == 'success' }}` evaluates false (route is `skipped`) — publish is skipped too, so no comment is posted at all. That also makes `cmux_next_media_runner.py`'s `if not same_repo` branch unreachable. The header comment and PR description state the comment explains why the tour did not run; for forks it never does. If the skip is intentional (fork-controlled `publish.py` must not run with the pr-media write token), update the header/description to state forks are excluded without a comment, and drop the dead fork branch or route only forks into a reason-less path.</comment>
<file context>
@@ -0,0 +1,181 @@
+jobs:
+ route:
+ name: Pick the tour runner
+ if: ${{ github.repository_owner == 'manaflow-ai' && github.event.pull_request.head.repo.full_name == github.repository }}
+ runs-on: ${{ github.repository_owner != 'manaflow-ai' && 'ubuntu-24.04' || github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository && 'blacksmith-4vcpu-ubuntu-2404' || vars.LINUX_RUNNER || 'blacksmith-4vcpu-ubuntu-2404' }}
+ timeout-minutes: 5
</file context>
| started = time.monotonic() | ||
| path = self.directory / f"frame-{len(self.frames):05d}.jpg" | ||
| problem = self.screen.capture(path, "jpg") | ||
| if problem: |
There was a problem hiding this comment.
P3: A failed frame capture can leave an empty frame-NNNNN.jpg behind: capture() treats st_size == 0 as an error but never deletes the file, and the recorder only skips appending the frame name. When the failing capture is the last one, ffmpeg's frame-%05d.jpg pattern reads the empty jpg and make_video drops the entire mp4/GIF for the tour. Unlink the file when a capture fails (also for step screenshots, so a failed screenshot does not leave a 0-byte png that confuses uploads).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At scripts/cmux-next/media/tour.py, line 201:
<comment>A failed frame capture can leave an empty `frame-NNNNN.jpg` behind: `capture()` treats `st_size == 0` as an error but never deletes the file, and the recorder only skips appending the frame name. When the failing capture is the last one, ffmpeg's `frame-%05d.jpg` pattern reads the empty jpg and `make_video` drops the entire mp4/GIF for the tour. Unlink the file when a capture fails (also for step screenshots, so a failed screenshot does not leave a 0-byte png that confuses uploads).</comment>
<file context>
@@ -0,0 +1,447 @@
+ started = time.monotonic()
+ path = self.directory / f"frame-{len(self.frames):05d}.jpg"
+ problem = self.screen.capture(path, "jpg")
+ if problem:
+ self.error = problem
+ else:
</file context>
…ne and tab steps by action id Run 36903298606: every capture failed with "could not create image from rect", and pane split-right / tab new-terminal were taken by the daemon grammar as terminal ids, so those steps run their app actions by id. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
4 issues found and verified against the latest diff
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="tests/test_cmux_next_media.py">
<violation number="1" location="tests/test_cmux_next_media.py:139">
P3: `test_dry_run_renders_from_tour_output` asserts only that `publish.main` exits 0, but the dry-run path returns 0 even when `manifests()` finds nothing (it would print "The tour produced no media."). A regression that broke media discovery or rendering would still make this test pass. Capture the printed body and assert it contains the tour content, e.g. `### Core UI`.</violation>
</file>
<file name="scripts/ci/cmux_next_media_runner.py">
<violation number="1" location="scripts/ci/cmux_next_media_runner.py:47">
P1: This skips every tour when the valid pool configuration omits an explicit GUI slot count, even if an owned GUI runner is online. Use live GUI-runner discovery for this route, or pass a GUI-capacity source alongside the pool slots before returning no runner.</violation>
</file>
<file name="scripts/cmux-next/media/publish.py">
<violation number="1" location="scripts/cmux-next/media/publish.py:151">
P3: `upsert_comment` patches only `existing[0]`, the first marker comment listed, and never reconciles the rest. If a PR ever carries two marker comments (e.g. one posted by a run whose successor/ancestor both finished, or a duplicate left by a manual edit), only the oldest is updated and the second goes stale forever, breaking the one-sticky-comment invariant. Update the newest marker comment and delete the others, or have each run remove every marker comment it does not own.</violation>
<violation number="2" location="scripts/cmux-next/media/publish.py:176">
P2: A missing or empty artifact directory becomes a successful `The tour produced no media.` comment, masking artifact-download failures after a successful tour. Treat absent manifests as an error unless an explicit `--note` explains the skipped or failed tour.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| slots = pr_runner_pool.slots(owned_slots, pr_xcode_app) | ||
| for pool in pools: | ||
| gui = pr_runner_pool.gui_label(pool) | ||
| if gui and slots.get(gui, 0) > 0: |
There was a problem hiding this comment.
P1: This skips every tour when the valid pool configuration omits an explicit GUI slot count, even if an owned GUI runner is online. Use live GUI-runner discovery for this route, or pass a GUI-capacity source alongside the pool slots before returning no runner.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At scripts/ci/cmux_next_media_runner.py, line 47:
<comment>This skips every tour when the valid pool configuration omits an explicit GUI slot count, even if an owned GUI runner is online. Use live GUI-runner discovery for this route, or pass a GUI-capacity source alongside the pool slots before returning no runner.</comment>
<file context>
@@ -0,0 +1,72 @@
+ slots = pr_runner_pool.slots(owned_slots, pr_xcode_app)
+ for pool in pools:
+ gui = pr_runner_pool.gui_label(pool)
+ if gui and slots.get(gui, 0) > 0:
+ return gui, f"{slots[gui]} gui runner(s) on {pool}"
+ return "", "no owned pool has gui runners in CI_OWNED_POOL_SLOTS"
</file context>
| pr_media = load_pr_media() | ||
| sections = [] | ||
| with tempfile.TemporaryDirectory() as scratch: | ||
| for directory, manifest in manifests(args.media) if args.media.is_dir() else []: |
There was a problem hiding this comment.
P2: A missing or empty artifact directory becomes a successful The tour produced no media. comment, masking artifact-download failures after a successful tour. Treat absent manifests as an error unless an explicit --note explains the skipped or failed tour.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At scripts/cmux-next/media/publish.py, line 176:
<comment>A missing or empty artifact directory becomes a successful `The tour produced no media.` comment, masking artifact-download failures after a successful tour. Treat absent manifests as an error unless an explicit `--note` explains the skipped or failed tour.</comment>
<file context>
@@ -0,0 +1,209 @@
+ pr_media = load_pr_media()
+ sections = []
+ with tempfile.TemporaryDirectory() as scratch:
+ for directory, manifest in manifests(args.media) if args.media.is_dir() else []:
+ name = manifest.get("name") or directory.name
+ folder = f"{args.pr}/{args.sha[:8]}/next-{name}"
</file context>
| (media / "core" / "shots").mkdir(parents=True) | ||
| (media / "core" / "shots" / "01-launch.png").write_bytes(b"\x89PNG\r\n\x1a\n") | ||
| (media / "core" / "manifest.json").write_text(json.dumps(self.MANIFEST), encoding="utf-8") | ||
| self.assertEqual(publish.main(["--media", str(media), "--pr", "1", "--sha", "b" * 40, "--dry-run"]), 0) |
There was a problem hiding this comment.
P3: test_dry_run_renders_from_tour_output asserts only that publish.main exits 0, but the dry-run path returns 0 even when manifests() finds nothing (it would print "The tour produced no media."). A regression that broke media discovery or rendering would still make this test pass. Capture the printed body and assert it contains the tour content, e.g. ### Core UI.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At tests/test_cmux_next_media.py, line 139:
<comment>`test_dry_run_renders_from_tour_output` asserts only that `publish.main` exits 0, but the dry-run path returns 0 even when `manifests()` finds nothing (it would print "The tour produced no media."). A regression that broke media discovery or rendering would still make this test pass. Capture the printed body and assert it contains the tour content, e.g. `### Core UI`.</comment>
<file context>
@@ -0,0 +1,143 @@
+ (media / "core" / "shots").mkdir(parents=True)
+ (media / "core" / "shots" / "01-launch.png").write_bytes(b"\x89PNG\r\n\x1a\n")
+ (media / "core" / "manifest.json").write_text(json.dumps(self.MANIFEST), encoding="utf-8")
+ self.assertEqual(publish.main(["--media", str(media), "--pr", "1", "--sha", "b" * 40, "--dry-run"]), 0)
+
+
</file context>
| existing = ids.split() | ||
| payload = json.dumps({"body": body}) | ||
| if existing: | ||
| gh(["api", "-X", "PATCH", f"repos/{repo}/issues/comments/{existing[0]}", "--input", "-"], payload) |
There was a problem hiding this comment.
P3: upsert_comment patches only existing[0], the first marker comment listed, and never reconciles the rest. If a PR ever carries two marker comments (e.g. one posted by a run whose successor/ancestor both finished, or a duplicate left by a manual edit), only the oldest is updated and the second goes stale forever, breaking the one-sticky-comment invariant. Update the newest marker comment and delete the others, or have each run remove every marker comment it does not own.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At scripts/cmux-next/media/publish.py, line 151:
<comment>`upsert_comment` patches only `existing[0]`, the first marker comment listed, and never reconciles the rest. If a PR ever carries two marker comments (e.g. one posted by a run whose successor/ancestor both finished, or a duplicate left by a manual edit), only the oldest is updated and the second goes stale forever, breaking the one-sticky-comment invariant. Update the newest marker comment and delete the others, or have each run remove every marker comment it does not own.</comment>
<file context>
@@ -0,0 +1,209 @@
+ existing = ids.split()
+ payload = json.dumps({"body": body})
+ if existing:
+ gh(["api", "-X", "PATCH", f"repos/{repo}/issues/comments/{existing[0]}", "--input", "-"], payload)
+ else:
+ gh(["api", "-X", "POST", f"repos/{repo}/issues/{pr}/comments", "--input", "-"], payload)
</file context>
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
1 issue found across 4 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/cmux-next/media/tour.py">
<violation number="1" location="scripts/cmux-next/media/tour.py:164">
P2: These checks treat a nonempty file as success even when `screencapture` exits nonzero. Require `returncode == 0` in the probe and both capture checks, otherwise failed or partial images can be published as valid evidence.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| said = (done.stderr or done.stdout).strip() or f"exit {done.returncode}" | ||
| except (OSError, subprocess.SubprocessError) as error: | ||
| said = str(error) | ||
| if image.is_file() and image.stat().st_size > 0: |
There was a problem hiding this comment.
P2: These checks treat a nonempty file as success even when screencapture exits nonzero. Require returncode == 0 in the probe and both capture checks, otherwise failed or partial images can be published as valid evidence.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At scripts/cmux-next/media/tour.py, line 164:
<comment>These checks treat a nonempty file as success even when `screencapture` exits nonzero. Require `returncode == 0` in the probe and both capture checks, otherwise failed or partial images can be published as valid evidence.</comment>
<file context>
@@ -119,25 +119,58 @@ def command_line(argv: list[str]) -> str:
+ said = (done.stderr or done.stdout).strip() or f"exit {done.returncode}"
+ except (OSError, subprocess.SubprocessError) as error:
+ said = str(error)
+ if image.is_file() and image.stat().st_size > 0:
+ self.capture_prefix, self.capture_mode = prefix, mode
+ return
</file context>
… data, capture through the cua-ssh helper The publish job holds the pr-media write token, so it checks out its scripts from the base SHA and validates tour names and shot paths taken from the artifact. Run 36904395488 showed the gui runner has no Screen Recording and no passwordless sudo, so the tour also tries an approved CuaSshScreenCapture.app (main's release-media helper backend), and the comment says why when nothing could be captured. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…re, inert comment text) - tour.py: SIGTERM unwinds through cleanup; the app stops before the recorder; a leftover app or daemon from a cancelled run is stopped before launch; recording starts once the window is on screen; shots never fall back to the whole shared display; the probe image is deleted; the helper runs in the console session; the crop rect is clamped to the display; the recorder's frame list is locked. - publish.py: manifest steps are normalized to plain strings and ints, HTML, Markdown and @mentions are escaped, a bad tour is reported instead of failing the job, the video has a fixed size, and the head is checked again right before the comment is written. - The typing step runs before the new workspace so it lands on screen. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
3 issues found across 4 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/cmux-next/media/tour.py">
<violation number="1" location="scripts/cmux-next/media/tour.py:60">
P1: This resolves the helper from the runner's home, but `Session` explicitly supports a different console user. Resolve the helper under the console user and launch it through that user's session, otherwise the fallback is skipped and the tour produces no captures.</violation>
</file>
<file name=".github/workflows/cmux-next-media.yml">
<violation number="1" location=".github/workflows/cmux-next-media.yml:164">
P2: This checkout pins the publisher scripts to `base.sha`, but the media artifact they consume is produced by the head's `tour.py`, and the artifact format is an unversioned, moving contract. Within this PR alone the format already changed between commits (tour folder naming, `capture_mode` in manifest.json). On a future PR that changes `tour.py`'s output — shot naming, manifest fields, or tour folder layout — the base `publish.py` will silently drop the media: `shot_file()` returns None for anything not matching `SHOT_PATH` without a warning, and non-dict steps are filtered quietly, so the posted comment loses screenshots/video while the job stays green. That PR's author also cannot demonstrate working media for their own PR because the publisher is the last merged base revision. Version the artifact (e.g., a `format_version` field in manifest.json) and have `publish.py` fail fast with a clear message when it sees a newer version than it supports, or at least emit `::warning::` for each skipped step/shot so skew is visible in the run.</violation>
</file>
<file name="scripts/cmux-next/media/publish.py">
<violation number="1" location="scripts/cmux-next/media/publish.py:81">
P1: `shot_file` checks only the leaf, so a symlinked tour directory or `shots/` can make the publisher upload a file outside the artifact. Reject every path component (including the tour directory in `manifests`) before reading or uploading it.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| SOCKET_TIMEOUT = 60.0 | ||
| FRAME_INTERVAL = 0.5 | ||
| # scripts/release-media/host_agent.py on main looks in the same places. | ||
| HELPER_CANDIDATES = (Path.home() / "Applications/CuaSshScreenCapture.app", |
There was a problem hiding this comment.
P1: This resolves the helper from the runner's home, but Session explicitly supports a different console user. Resolve the helper under the console user and launch it through that user's session, otherwise the fallback is skipped and the tour produces no captures.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At scripts/cmux-next/media/tour.py, line 60:
<comment>This resolves the helper from the runner's home, but `Session` explicitly supports a different console user. Resolve the helper under the console user and launch it through that user's session, otherwise the fallback is skipped and the tour produces no captures.</comment>
<file context>
@@ -56,6 +56,9 @@
SOCKET_TIMEOUT = 60.0
FRAME_INTERVAL = 0.5
+# scripts/release-media/host_agent.py on main looks in the same places.
+HELPER_CANDIDATES = (Path.home() / "Applications/CuaSshScreenCapture.app",
+ Path("/Applications/CuaSshScreenCapture.app"))
# A long tour would make a GIF past the inline limit; split it instead.
</file context>
| if not isinstance(shot, str) or not SHOT_PATH.fullmatch(shot): | ||
| return None | ||
| path = directory / shot | ||
| if path.is_symlink() or not path.is_file(): |
There was a problem hiding this comment.
P1: shot_file checks only the leaf, so a symlinked tour directory or shots/ can make the publisher upload a file outside the artifact. Reject every path component (including the tour directory in manifests) before reading or uploading it.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At scripts/cmux-next/media/publish.py, line 81:
<comment>`shot_file` checks only the leaf, so a symlinked tour directory or `shots/` can make the publisher upload a file outside the artifact. Reject every path component (including the tour directory in `manifests`) before reading or uploading it.</comment>
<file context>
@@ -50,15 +53,36 @@ def load_pr_media() -> Any:
+ if not isinstance(shot, str) or not SHOT_PATH.fullmatch(shot):
+ return None
+ path = directory / shot
+ if path.is_symlink() or not path.is_file():
+ return None
+ return path
</file context>
| - name: Checkout the publisher from the base revision | ||
| uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 | ||
| with: | ||
| ref: ${{ github.event.pull_request.base.sha }} |
There was a problem hiding this comment.
P2: This checkout pins the publisher scripts to base.sha, but the media artifact they consume is produced by the head's tour.py, and the artifact format is an unversioned, moving contract. Within this PR alone the format already changed between commits (tour folder naming, capture_mode in manifest.json). On a future PR that changes tour.py's output — shot naming, manifest fields, or tour folder layout — the base publish.py will silently drop the media: shot_file() returns None for anything not matching SHOT_PATH without a warning, and non-dict steps are filtered quietly, so the posted comment loses screenshots/video while the job stays green. That PR's author also cannot demonstrate working media for their own PR because the publisher is the last merged base revision. Version the artifact (e.g., a format_version field in manifest.json) and have publish.py fail fast with a clear message when it sees a newer version than it supports, or at least emit ::warning:: for each skipped step/shot so skew is visible in the run.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At .github/workflows/cmux-next-media.yml, line 164:
<comment>This checkout pins the publisher scripts to `base.sha`, but the media artifact they consume is produced by the head's `tour.py`, and the artifact format is an unversioned, moving contract. Within this PR alone the format already changed between commits (tour folder naming, `capture_mode` in manifest.json). On a future PR that changes `tour.py`'s output — shot naming, manifest fields, or tour folder layout — the base `publish.py` will silently drop the media: `shot_file()` returns None for anything not matching `SHOT_PATH` without a warning, and non-dict steps are filtered quietly, so the posted comment loses screenshots/video while the job stays green. That PR's author also cannot demonstrate working media for their own PR because the publisher is the last merged base revision. Version the artifact (e.g., a `format_version` field in manifest.json) and have `publish.py` fail fast with a clear message when it sees a newer version than it supports, or at least emit `::warning::` for each skipped step/shot so skew is visible in the run.</comment>
<file context>
@@ -154,8 +156,12 @@ jobs:
+ - name: Checkout the publisher from the base revision
+ uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
with:
+ ref: ${{ github.event.pull_request.base.sha }}
persist-credentials: false
sparse-checkout: |
</file context>
…failing the publish Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…e app stops, signal-safe cleanup) - A newline or CR in manifest text or a command can no longer end the code span or the table row. - The recorder stops before the app is killed, so no frame shows what was behind the window; a window off the display clears the rect. - Cleanup ignores further signals, and the step execs python so the runner's cancel signal reaches it. - The helper's raw desktop image is written outside shots/ and frames/. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…e window rect Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Collaborator
Author
|
Review: third subagent pass over 92303ae (tour.py, publish.py, workflow). Fixed in 034e69a:
Left: none. Signals, the recorder stop lock, Head: 034e69a — Zoetrope g1 🗝️ |
teamleaderleo
added a commit
that referenced
this pull request
Oct 1, 2026
teamleaderleo
added a commit
that referenced
this pull request
Oct 1, 2026
teamleaderleo
added a commit
that referenced
this pull request
Oct 1, 2026
teamleaderleo
added a commit
that referenced
this pull request
Oct 1, 2026
teamleaderleo
added a commit
that referenced
this pull request
Oct 1, 2026
teamleaderleo
added a commit
that referenced
this pull request
Oct 1, 2026
teamleaderleo
added a commit
that referenced
this pull request
Oct 1, 2026
teamleaderleo
added a commit
that referenced
this pull request
Oct 1, 2026
teamleaderleo
added a commit
that referenced
this pull request
Oct 1, 2026
…und ffmpeg Run 6 still launched into two earlier Tour workspaces: the daemon outlives the app and keeps its workspaces, so deleting the state directory under it changed nothing. The tour now ends the tagged daemon through its socket (server stop --end-terminals) before the pkill fallback and the state removal. An untagged build's daemon is the user's and is never stopped. Run 6's publish job spent its 15 minutes in apt installing ffmpeg. The install is now capped at 4 minutes and may fail (the comment then has screenshots and no video), and nothing installs when the base revision has no publisher. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
2 issues found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".github/workflows/cmux-next-media.yml">
<violation number="1" location=".github/workflows/cmux-next-media.yml:199">
P2: A step that exceeds `timeout-minutes` cancels the entire job: GitHub's runner treats the timed-out step as cancelled, and `continue-on-error: true` only applies to steps that fail, not to job cancellation. So a slow or stuck `apt-get` here cancels the publish job outright and the sticky comment is never posted — the opposite of the intent stated in the comment above the step. Wrap the apt command with coreutils `timeout` (e.g. `timeout 240`) and keep `continue-on-error: true` for a genuinely failed install; that fails only the step, and publish.py still posts the screenshots-only comment.</violation>
</file>
<file name="scripts/cmux-next/media/tour.py">
<violation number="1" location="scripts/cmux-next/media/tour.py:381">
P2: This stops all terminals in a tagged daemon even for the default local mode without `--fresh-state`, including terminals the developer already owns. Gate the stop on tour ownership rather than tag presence; at minimum, honor `fresh_state` here.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| # video, so a slow or stuck apt never holds the comment back. | ||
| - name: Install ffmpeg | ||
| if: ${{ steps.publisher.outputs.present == 'true' }} | ||
| timeout-minutes: 4 |
There was a problem hiding this comment.
P2: A step that exceeds timeout-minutes cancels the entire job: GitHub's runner treats the timed-out step as cancelled, and continue-on-error: true only applies to steps that fail, not to job cancellation. So a slow or stuck apt-get here cancels the publish job outright and the sticky comment is never posted — the opposite of the intent stated in the comment above the step. Wrap the apt command with coreutils timeout (e.g. timeout 240) and keep continue-on-error: true for a genuinely failed install; that fails only the step, and publish.py still posts the screenshots-only comment.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At .github/workflows/cmux-next-media.yml, line 199:
<comment>A step that exceeds `timeout-minutes` cancels the entire job: GitHub's runner treats the timed-out step as cancelled, and `continue-on-error: true` only applies to steps that fail, not to job cancellation. So a slow or stuck `apt-get` here cancels the publish job outright and the sticky comment is never posted — the opposite of the intent stated in the comment above the step. Wrap the apt command with coreutils `timeout` (e.g. `timeout 240`) and keep `continue-on-error: true` for a genuinely failed install; that fails only the step, and publish.py still posts the screenshots-only comment.</comment>
<file context>
@@ -183,10 +183,27 @@ jobs:
- name: Install ffmpeg
- run: command -v ffmpeg >/dev/null || (sudo apt-get update -qq && sudo apt-get install -y -qq ffmpeg >/dev/null)
+ if: ${{ steps.publisher.outputs.present == 'true' }}
+ timeout-minutes: 4
+ continue-on-error: true
+ env:
</file context>
| directory, so deleting that directory under a live daemon changes nothing. | ||
| An untagged build's session is the user's own, so it is never stopped. | ||
| """ | ||
| if self.bundle_tag: |
There was a problem hiding this comment.
P2: This stops all terminals in a tagged daemon even for the default local mode without --fresh-state, including terminals the developer already owns. Gate the stop on tour ownership rather than tag presence; at minimum, honor fresh_state here.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At scripts/cmux-next/media/tour.py, line 381:
<comment>This stops all terminals in a tagged daemon even for the default local mode without `--fresh-state`, including terminals the developer already owns. Gate the stop on tour ownership rather than tag presence; at minimum, honor `fresh_state` here.</comment>
<file context>
@@ -371,13 +371,25 @@ def state_directory(self) -> Path | None:
+ directory, so deleting that directory under a live daemon changes nothing.
+ An untagged build's session is the user's own, so it is never stopped.
+ """
+ if self.bundle_tag:
+ self.cli(["server", "stop", "--end-terminals"], daemon=True, timeout=20)
+
</file context>
Collaborator
Author
|
Tour state fixed at d82961b (run).
The tour job stays red on split right, split down and new terminal tab (the terminal_id bug in the base, reported on #16174). This PR waits for #16174 to merge and retarget it. Head: d82961b — Zoetrope g1 🗝️ |
A stuck tour recorded a frame every half second for as long as it ran. The recorder now stops at five minutes of frames and the publisher's ffmpeg reads no more than that, so neither the runner nor the publish job grows with a slow tour. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
2 issues found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/cmux-next/media/publish.py">
<violation number="1" location="scripts/cmux-next/media/publish.py:41">
P3: MAX_FRAMES is duplicated in tour.py and this constant, and only a comment keeps them in sync. If tour.py's cap changes, the comment drifts and the mp4 cap silently diverges from the recorder cap. tests/test_cmux_next_media.py already imports both modules, so assert equality there to keep the coupling enforced.</violation>
</file>
<file name="scripts/cmux-next/media/tour.py">
<violation number="1" location="scripts/cmux-next/media/tour.py:313">
P3: The truncation message set on this branch is never surfaced: run_tour's finally block only writes recorder.error into the manifest when the frames list is empty (tour.py:579-580), but this branch fires only once 600 frames exist, so the guard is always false and the "video stops at 600 frames" notice never reaches the manifest (publish.py ignores frames_error, so nothing else can report it). A video cut short by the cap therefore looks finished with no indication it stopped at five minutes. Record the truncation with a dedicated flag (for example self.truncated = True here) and, in the finally block, write frames_error when that flag is set, instead of assigning an error this consumer cannot report.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| GIF_FPS = 4 | ||
| VIDEO_SIZE = (1280, 800) | ||
| # tour.py's MAX_FRAMES: a larger artifact does not make a longer video. | ||
| MAX_FRAMES = 600 |
There was a problem hiding this comment.
P3: MAX_FRAMES is duplicated in tour.py and this constant, and only a comment keeps them in sync. If tour.py's cap changes, the comment drifts and the mp4 cap silently diverges from the recorder cap. tests/test_cmux_next_media.py already imports both modules, so assert equality there to keep the coupling enforced.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At scripts/cmux-next/media/publish.py, line 41:
<comment>MAX_FRAMES is duplicated in tour.py and this constant, and only a comment keeps them in sync. If tour.py's cap changes, the comment drifts and the mp4 cap silently diverges from the recorder cap. tests/test_cmux_next_media.py already imports both modules, so assert equality there to keep the coupling enforced.</comment>
<file context>
@@ -37,6 +37,8 @@
GIF_FPS = 4
VIDEO_SIZE = (1280, 800)
+# tour.py's MAX_FRAMES: a larger artifact does not make a longer video.
+MAX_FRAMES = 600
GIF_WIDTHS = (720, 560, 440)
TOUR_NAME = re.compile(r"[a-z0-9][a-z0-9-]{0,39}")
</file context>
| if self.stopping.is_set(): | ||
| break | ||
| if len(self.frames) >= MAX_FRAMES: | ||
| self.error = f"the video stops at {MAX_FRAMES} frames" |
There was a problem hiding this comment.
P3: The truncation message set on this branch is never surfaced: run_tour's finally block only writes recorder.error into the manifest when the frames list is empty (tour.py:579-580), but this branch fires only once 600 frames exist, so the guard is always false and the "video stops at 600 frames" notice never reaches the manifest (publish.py ignores frames_error, so nothing else can report it). A video cut short by the cap therefore looks finished with no indication it stopped at five minutes. Record the truncation with a dedicated flag (for example self.truncated = True here) and, in the finally block, write frames_error when that flag is set, instead of assigning an error this consumer cannot report.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At scripts/cmux-next/media/tour.py, line 313:
<comment>The truncation message set on this branch is never surfaced: run_tour's finally block only writes recorder.error into the manifest when the frames list is empty (tour.py:579-580), but this branch fires only once 600 frames exist, so the guard is always false and the "video stops at 600 frames" notice never reaches the manifest (publish.py ignores frames_error, so nothing else can report it). A video cut short by the cap therefore looks finished with no indication it stopped at five minutes. Record the truncation with a dedicated flag (for example self.truncated = True here) and, in the finally block, write frames_error when that flag is set, instead of assigning an error this consumer cannot report.</comment>
<file context>
@@ -306,6 +309,9 @@ def run(self) -> None:
if self.stopping.is_set():
break
+ if len(self.frames) >= MAX_FRAMES:
+ self.error = f"the video stops at {MAX_FRAMES} frames"
+ break
path = self.directory / f"frame-{len(self.frames):05d}.jpg"
</file context>
This was referenced Oct 6, 2026
Bugbot is paused — on-demand spend limit reachedBugbot 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. |
* ci: format staged files in the trusted pre-commit hook * ci: guard formatter scope for staged files * ci: scope Rust formatter guard to its package * ci: use direct rustfmt and skip merge formatting * ci: pass the cmux-tui Rust edition to rustfmt
* test(ci): red test, PRs check rustfmt on the cmux-tui packages they touch PR CI never runs cmux-tui.yml (dispatch only), so #18706 landed unformatted Rust and broke base cargo fmt --check. These tests pin a helper that runs cargo fmt --check once per touched package, checks every package when a rustfmt config changes or there is no base, and names each unformatted package. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * ci: check rustfmt on the cmux-tui packages a PR touches PR CI never runs cmux-tui.yml (dispatch only), so #18706 landed unformatted Rust and broke base cargo fmt --check. cmux-tui-spec.yml already runs on every PR and push touching cmux-tui/** on hosted Linux; its new rustfmt job runs cmux-tui-fmt-changed.py, which maps each changed .rs file or Cargo.toml to its nearest tracked package and runs cargo fmt --check on it with the pinned toolchain. No build and no dependency download. A rustfmt config or toolchain change, or no base, checks every package; vendored crates and deleted packages are skipped. A PR compares with its merge commit's first parent, a push with the commit before it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * style(cmux-tui): rustfmt acpmux's draft tests and the relay circuit The new rustfmt job's full run found two base packages that fail cargo fmt --check: acpmux, from #18706's draft tests and their module order, and the Cloudflare relay's circuit.rs. Formatting only, exactly the hosted rustfmt diff. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * style(cmux-tui): leave the relay circuit to its own repair Touching the Cloudflare relay runs its lane, whose npm audit is red on base (sharp and undici advisories through wrangler, fixed only by a wrangler bump outside the stated range). That is the relay's own repair, so this PR keeps acpmux's formatting only. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
* ci: add mini-first Swift canary for cmux-next * ci: remove unit tests from Swift canary change
…e cmux-tui (#18698) * test(cmux-next): red: a PR that changes no cmux-tui fails same-tree on an unpublished merge tree #16614, #17551 and #18605 change no cmux-tui or ghostty-next path, yet same-tree cmux-tui failed them because the base tip's tree was never published (its artifacts run was cancelled). Such a PR should reuse the newest published tree in its history (tree_fetch_key, fetched through CMUX_TUI_TREE_KEY), or skip the tree jobs when none is published, never fail. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * ci(cmux-next): same-tree cmux-tui is a hard check only for PRs that change cmux-tui Path routing records whether the pull request's own diff touches cmux-tui/ or ghostty-next. For one that does not, a merge tree that is not ready no longer fails: the probe (CMUX_TUI_TREE_BASE_FALLBACK=1) resolves the newest published tree in the merge's history and reports ready with tree_fetch_key, which the daemon tests and the Debug scheme compile fetch through CMUX_TUI_TREE_KEY; with none published it reports skipped. Pushes, dispatches and PRs that change cmux-tui keep the hard check. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(cmux-next): the newest-published search deepens only this commit's history A bare `git fetch --deepen=50 origin` deepens every branch of origin: on this PR's first run it took path routing past its 5-minute limit (job 113609407061) before the probe's base fallback, which then resolved in under a second. Deepening only HEAD takes about 30 s on a depth-2 clone of feat-cmux-next. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
* docs(cmux-next): map UI parity reference sources * docs(cmux-next): add media preview reference source * docs(cmux-next): define reference freshness checks
* test(nightly): the notes step cannot spend the publish budget before the upload (red) Run 37611677848 spent 29 of publish-nightly's 30 minutes on release notes and was cancelled mid-upload, leaving partial release assets. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(nightly): bound the publish notes step at 10 minutes The notes take seconds since 0115b26. A regression now fails the step before any asset upload, instead of spending publish-nightly's 30 minutes and being cancelled mid-upload with partial assets left on the release. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…self (#17622) * test(cmux-next): the cmux-tui gate pins and dispatches an unpublished tree itself Publisher run 37505519359 failed transiently and its retry was replaced by a newer feat-cmux-next push, so the tree was never published and lanes pushed cmux-tui-pin-* branches by hand to unblock their gates. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(cmux-next): the tree gate watches the base and may pin the tree Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * ci(cmux-next): the cmux-tui gate pins and dispatches an unpublished tree When two run checks find no artifacts run that will publish the tree, the gate creates cmux-tui-pin-<key12> at the publisher commit and dispatches the artifacts workflow there, once, then keeps waiting. A pull request now watches its base commit (the merge's first parent) instead of no publisher, so it neither waits 45 minutes for a tree nobody builds nor needs a lane to push a pin branch by hand. A superseded push still skips; forks keep a read-only token and no autopin. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(cmux-tui): a transient wrong binary read must not fail publication Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(cmux-tui): read a mismatched published binary again before failing R2 objects are write-once, so a digest mismatch on a binary the run just uploaded is a bad read. Fetch it again from a cache-busting URL up to three times, and name the response headers when it still differs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(cmux-next): a pull request gate pins a tree its base never builds Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * ci(cmux-next): a pull request gate pins a tree its base never builds A pull request that changes cmux-tui has a merge tree no push builds, and the artifacts workflow's pull_request_target trigger has never started a run, so its gate waited 45 minutes with the run check off. With CMUX_TUI_TREE_AUTOPIN the gate now pins its own checkout at once and watches that run. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * ci(cmux-next): retain base tree dispatch tests --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
* ci(gallery): capture narrow responsive states * test(gallery): cover workflow matrix changes * fix(gallery): dedupe equal responsive widths * ci: shard gallery matrix renders * ci: retrigger sharded gallery validation
tests/test-execution.toml: took base. Dropped the PR's unit test tests/test_cmux_next_media.py and its ci-guards step (no unit tests in cmux-next). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Found 1 test failure on Blacksmith runners: Failure
|
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light.svg)
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 9eab0ac. Configure here.
| contents: read | ||
| timeout-minutes: 20 | ||
| env: | ||
| CMUX_NEXT_RUNNER: ${{ needs.macos-placement.outputs.runner || github.repository_owner != 'manaflow-ai' && 'macos-26' || (github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository && 'blacksmith-6vcpu-macos-26' || vars.CI_PR_POOL_OWNED == '1' && contains(fromJSON('["pull_request","push","workflow_dispatch"]'), github.event_name) && (github.run_attempt <= 2 || github.triggering_actor != 'github-actions[bot]') && vars.CI_SIDE_LANE_RUNNER || vars.MACOS_RUNNER_26 || 'blacksmith-6vcpu-macos-26') }} |
There was a problem hiding this comment.
Canary ignores attempt-four overflow
Medium Severity
The new swift-canary job always prefers macos-placement's runner label. Sibling Mac jobs now drop that label on bot attempt 4 so a refused mini overflows to Blacksmith. A canary refusal stays on the same owned label, so the merge gate for package-only Swift PRs can queue indefinitely.
Reviewed by Cursor Bugbot for commit 9eab0ac. Configure here.
| for stage in base head repeat; do | ||
| mkdir -p "$evidence/runs/$stage" | ||
| jq -s 'add' "$evidence"/runs/$stage/shard-*/results.json > "$evidence/runs/$stage/results.json" | ||
| done |
There was a problem hiding this comment.
Sharded screenshots miss compare paths
High Severity
Each render shard writes screenshots and results.json under runs/<stage>/shard-N/, but the compare job only concatenates those JSON files into runs/<stage>/results.json. Image paths stay one directory deeper than compare expects, so before/after evidence can look broken or missing.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 9eab0ac. Configure here.
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
3 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.
Part of the cmux-next parity work listed in #16174 (comment).
Problem
UI pull requests into
feat-cmux-nexthave no visual evidence from CI. The old app's media pipeline (pr-media.yml,test-e2e.yml,DogfoodScenarioUITests) depends on pieces this branch deleted, and cmux-next CI only runsswift test.What this adds
cmux-next-media.ymlruns on pull requests intofeat-cmux-nextthat touchPackages/macOS/CmuxNext/Sources,webviews/src(agent pane) or the tour files:route picks an owned Mac's gui runner (
scripts/ci/cmux_next_media_runner.py, built onpr_runner_pool). It never falls back to Blacksmith, whose macOS sessions sit at a locked screen and cannot capture. Fork heads and an empty gui pool skip the tour, and the comment says why.tour builds the head with
scripts/reload.sh --tag ci-media. The fixed tag keeps the mini's DerivedData warm. The job then launches the app on a private socket (thesmoke-signed-app-cli.shknobs) and runsscripts/cmux-next/media/tour.py. The tour drives the app only through the bundledcmuxCLI from cmux-next: the Rust cmux CLI (cmux-tui) with acpmux and cmux acp; delete the Swift CLI #16174 and captures:Capture runs in the console user's session, as main's E2E recorder does.
publish makes an mp4 and a GIF from the frames. It uploads them and the screenshots to
pr-media/<pr>/<sha8>/next-<tour>/(the layoutprune_pr_media.pyalready bounds) throughscripts/pr-media.py's writer, and keeps one sticky comment with:Tours are data:
scripts/cmux-next/media/tours/*.json. A step is a title plus acmuxargv (withdaemon: truefor daemon verbs),wait,shot, andoptional: "<why>"for features not ported yet. Other lanes add steps or a tour file.tour.py --checkvalidates them, and the tour job runs that check too.The starter tour covers:
bindUnavailabletoday; they show as "unavailable" until cmux-next: agent pane hosts the React acpmux pane (New Agent Chat) #16231 and the sidebar/notifications lane land.cmux-next has no XCUITest target, so the tour is the UI check: a failed step fails the tour job and its screenshot shows inline in the comment.
Choices to review
feat-cmux-next-acpmux, stacked. The tour drives cmux-next: the Rust cmux CLI (cmux-tui) with acpmux and cmux acp; delete the Swift CLI #16174's CLI (--app-socket,action run); onfeat-cmux-nextthe app still bundles the Swift CLI. This stays open until cmux-next: the Rust cmux CLI (cmux-tui) with acpmux and cmux acp; delete the Swift CLI #16174 merges and GitHub retargets it, then gets feat-cmux-next merged in and re-checked. It carries 822db55 (the Swift 6.2ResourceControlbuild fix already on feat-cmux-next) so the base compiles for the tour; that commit drops out of the diff after the retarget. Red checks inherited from the base (cmux-next swift test, the 239-commit gap; the fork-routing guard oncmux-next.yml:60) are not from this diff.contents: writeandpull-requests: write, on same-repository heads only. It never checks out or runs the pull request's code:publish.pyandpr-media.pycome from the base SHA, and the tour artifact is read as data (tour folders must be slugs, shot paths must beshots/NN-slug.pngregular files inside the tour folder, PNG by content, and manifest text is HTML-escaped). The route and tour jobs staycontents: read. Until the base haspublish.py(this PR's own runs), the publish job notes that and leaves the media in the run artifact.What the CI runs found
reload.sh --tag ci-mediabuilds on the mini, the app answers on its private socket, andworkspace new, terminal typing,tab new-browser, the sidebar toggle andnotifyall pass.action run splitRight/splitDown/newSurfacefail on this base withterminal_id must be a 32-character lowercase UUIDv4 hex value: an app-side bug the tour caught (reported on cmux-next: the Rust cmux CLI (cmux-tui) with acpmux and cmux acp; delete the Swift CLI #16174). They stay required steps, so the tour job is red until it is fixed.screencaptureand the TCC grant main's E2E lane uses do not work there. The tour now also tries an approvedCuaSshScreenCapture.app(main's release-media helper), and the comment says which route failed when none works.Testing
tests/test_cmux_next_media.py(wired into the preflight guard group) covers:scripts/ci/guards-local.sh --allpasses except for failures that already exist onfeat-cmux-next-acpmux(fork routing incmux-next.yml:60, the determinism gate, the iOS conventions, the bash shell checks). None of them touch these files.Example output
From CI run 4 on be0c62d, published by hand with publish.py while the base has no publisher yet. Split and new tab fail on an app bug in the acpmux base (reported on #16174).
Core UI: 7 ok, 3 failed, 2 unavailable
Video (mp4)
cmux workspace new --name Tourcmux terminal current write --text "echo hello from the cmux-next tour\r"cmux action run splitRightcmux action run splitDowncmux action run newSurfacecmux tab new-browser --url https://example.comcmux action run toggleSidebarcmux action run toggleSidebarcmux notify --title Tour --body "Hello from CI"cmux action run showNotificationscmux agent new-chatFailed steps:
4. Split right
5. Split down
6. New terminal tab
Screenshots (8)
1. Launch
2. New workspace
3. Type in the terminal
7. Browser tab
8. Hide the sidebar
10. Notification
11. Notifications panel
12. Agent chat
🤖 Generated with Claude Code
— Zoetrope g1 🗝️ · run
run_cc_next_ci_media_20261001_e712de82Note
Medium Risk
Touches cmux-next routing, cmux-tui tree pinning, and a publish job with write tokens; behavior is guarded but mis-routing could skip or run wrong Mac tiers.
Overview
Adds cmux-next PR media CI: a new workflow builds the head app on an owned Mac GUI runner, runs JSON-defined CLI tours (
tour.py), and posts GIF/video/screenshots viapublish.pyfrom the base revision (same-repo only; forks skip with an explanation).cmux-next CI gains a Swift canary job for small package-only
CmuxNextedits (debugswift buildon changed production targets); Release compile stays advisory on that PR class. cmux-tui same-tree probing can fall back to the newest published tree (or skip tree jobs) when a PR does not touchcmux-tui/ghostty-next, withCMUX_TUI_TREE_KEYwired into daemon/scheme jobs. Mac job placement now keeps minis through bot attempt 3 and sends attempt 4 to Blacksmith after rescue re-runs.cmux-tui-spec runs scoped
cargo fmt --checkviacmux-tui-fmt-changed.py(plus unit tests). Gallery PR renders narrow and normal widths, shards evidence across four parallel jobs, and merges results before compare. Pre-commit formats staged webviews (vp fmt) andcmux-tuiRust (rustfmt), skips merges, and documents the behavior in CONTRIBUTING.Other fixes: manifest publish verification retries remote binary digest mismatches with fresh URLs; nightly release-notes step gets its own timeout;
gh-merge-greenrequires the canary check; new UI reference sources plan doc and tier doc updates.Reviewed by Cursor Bugbot for commit 9eab0ac. Bugbot is set up for automated code reviews on this repo. Configure here.