feat(qa-v): video ground truth harness for cmuxlayer claims - #497
Conversation
cmuxlayer is currently the only witness to cmuxlayer: every claim is verified by the same tool suite that produced it, so a receipt that lies looks exactly like one that tells the truth. This adds evidence from outside the system under test. The harness opens an isolated cmux window, screen-records it, runs the repros the fleet actually reported (#432/#484 busy send, #484 stale terminal row, #485 close_surface scope=agent, #488 closure flap, #473 wait_for on a working agent, #434/#440 spawn under keystroke injection), captures every tool receipt verbatim against the recording clock, and emits an adjudication manifest that Sonnet vision sub-agents answer one narrow question at a time. The report reconciles receipt against pixels; contradictions are the product. Zero src/ changes. New files only: - scripts/qa-video-harness.mjs runner (cmux window lifecycle, ffmpeg recorder, MCP stdio client, probe sequence, frame extraction) - scripts/qa-video-lib.mjs pure probe catalogue, frame planning, receipt reading, reconciliation, report rendering - tests/qa-video-harness.test.ts 27 tests over the pure logic - docs/qa-video-harness.md runbook Four bugs the dry-run caught before the harness ever touched live panes, each now guarded and covered by a test: - targeting "the frontmost process" resolved to whatever the human last touched; the first recording cropped to a browser and captured private content. The window is now addressed by the title the harness assigns. - cmux focus-window does not restack macOS windows, so the operator's own window got recorded. AXRaise plus an AXMain check now proves isolation, and the run aborts rather than recording the whole desktop. - inheriting cmux's CMUX_SURFACE_ID/TAB/WORKSPACE made cmuxlayer resolve the harness as the operator's own agent and refuse terminal I/O. - re-asserting focus with cmux focus-window between probes churned surface topology until spawn_agent failed; the re-assert is AX-only now. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Uncommitted work parked by the lead during a fleet-wide converge-before-close; not reviewed, not claimed complete. Co-Authored-By: cmuxlayerClaude running claude-fable-5 <noreply@anthropic.com>
Co-Authored-By: cmuxlayerClaude running claude-fable-5 <noreply@anthropic.com>
Four live runs of the harness produced frames that looked plausible and showed the wrong thing. Each cause is now guarded, and each guard is a test. - The probe window is located through CoreGraphics (scripts/qa-video-windows.py), not System Events. A freshly created cmux window is intermittently absent from the accessibility window list, and drops out of the on-screen list whenever its Space is inactive, so a single miss is a flap and is retried. - The recorder captures the display the probe window is actually on. cmux does not always open on the main display; two runs recorded display 0 while the window sat on display 1. - Isolation prefers a whole display over a z-order fight. The harness runs from inside a cmux pane, so the operator's own cmux window is raised by the very commands driving the probe; occlusion is per-display, so moving the probe window to the least-occupied display makes stacking moot. On a single-display machine this is a no-op and the z-order checks still apply. - Occlusion is judged from CoreGraphics front-to-back order. Every mark records whether the window was clear, the manifest refuses to generate a question for any mark that was not, and the run aborts rather than recording a covered region. - Per-probe re-assert is AXRaise only. Calling cmux focus-window between probes churned surface topology until spawn_agent failed with "not live or uniquely resolvable in a complete fresh topology". - SIGINT/SIGTERM tear the isolated window down, so an interrupted run does not leak a window onto the desktop. - Frame windows reach further past each mark: a mark is the instant of the tool call and the pixels lag it by ~1.7s, measured. Documents one rejected approach so it is not retried: screencapture -l <CGWindowID> captures a single window and is immune to occlusion, display placement and focus stealing, but cmux renders terminals with Metal, so it returns the chrome with a blank content area. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_d6d991e7-45c3-4f5c-a8c5-b1f508bc198d) |
|
Warning Review limit reached
Next review available in: 54 minutes Limit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
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 |
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| busySpawn.error = spawnA.error ?? "spawn_agent returned no agent_id"; | ||
| return; | ||
| } | ||
| const busyState = await pollAgentState(client, agentA, (agent) => agent.state === "working", 90_000); |
There was a problem hiding this comment.
🟡 Medium scripts/qa-video-harness.mjs:956
When pollAgentState does not observe working, runFullProbes still records normal busy-send and wait-for-working adjudications even though their precondition was not met; registryStateAtSend is never used to suppress or invalidate those questions. The stale-terminal path also treats idle as settled, so it runs stale-terminal-send for a non-terminal registry row. Gate each probe on its required state and mark skipped precondition failures as unadjudicable.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @scripts/qa-video-harness.mjs around line 956:
When `pollAgentState` does not observe `working`, `runFullProbes` still records normal `busy-send` and `wait-for-working` adjudications even though their precondition was not met; `registryStateAtSend` is never used to suppress or invalidate those questions. The stale-terminal path also treats `idle` as settled, so it runs `stale-terminal-send` for a non-terminal registry row. Gate each probe on its required state and mark skipped precondition failures as unadjudicable.
| if (before.has(windowId)) { | ||
| throw new Error(`new-window returned a pre-existing window (${windowId}); refusing to proceed`); | ||
| } | ||
| await sleep(1_000); |
There was a problem hiding this comment.
🟡 Medium scripts/qa-video-harness.mjs:881
A failure while listing the workspace, renaming the window, or focusing it leaves the newly created cmux window open on the desktop. runOnce only installs teardown after createProbeWindow resolves, so rejected setup never reaches destroyProbeWindow; close the window in a catch inside createProbeWindow before rethrowing.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @scripts/qa-video-harness.mjs around line 881:
A failure while listing the workspace, renaming the window, or focusing it leaves the newly created cmux window open on the desktop. `runOnce` only installs teardown after `createProbeWindow` resolves, so rejected setup never reaches `destroyProbeWindow`; close the window in a `catch` inside `createProbeWindow` before rethrowing.
| if (contradictions.length === 0) { | ||
| lines.push("None. Every adjudicable receipt matched the frames."); | ||
| } else { |
There was a problem hiding this comment.
🟡 Medium scripts/qa-video-lib.mjs:467
The prominent Contradictions section reports a clean match when adjudication has MISSING rows, so a run with unanswered or invalid verdicts is presented as successful. reconcile deliberately assigns those rows MISSING, but this branch checks only contradictions.length; include missing results in the clean-case condition and state that adjudication is incomplete when they exist.
- if (contradictions.length === 0) {
- lines.push("None. Every adjudicable receipt matched the frames.");
- } else {
+ if (contradictions.length === 0 && totals.MISSING === 0) {
+ lines.push("None. Every adjudicable receipt matched the frames.");
+ } else if (contradictions.length === 0) {
+ lines.push(`None found, but ${totals.MISSING} adjudication result(s) are missing or invalid.`);
+ } else {🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @scripts/qa-video-lib.mjs around lines 467-469:
The prominent `Contradictions` section reports a clean match when adjudication has `MISSING` rows, so a run with unanswered or invalid verdicts is presented as successful. `reconcile` deliberately assigns those rows `MISSING`, but this branch checks only `contradictions.length`; include missing results in the clean-case condition and state that adjudication is incomplete when they exist.
| text: (ctx) => | ||
| `Two seconds after the send, is \`${ctx.nonce}\` STILL sitting unsent in the composer of ${ctx.paneHint}? Answer YES if it is still in the composer (i.e. it was never submitted), NO if the composer is clear or the text has moved into the transcript above the composer (i.e. it was submitted).`, | ||
| // A receipt that claims submitted implies the composer is clear -> NO. | ||
| expectedIfReceiptTrue: (step) => |
There was a problem hiding this comment.
🟡 Medium scripts/qa-video-lib.mjs:48
The submitted expectations return "YES" for every receipt that is not submitted, so an undelivered send_to call (or a step with no receipt) is reconciled as CONTRADICT even though the nonce is correctly absent from the composer. The same logic exists in stale-terminal-send; return "YES" only for delivered or queued-but-unsubmitted receipts, and "NO" when receiptDelivered(step) is false.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @scripts/qa-video-lib.mjs around line 48:
The `submitted` expectations return `"YES"` for every receipt that is not `submitted`, so an undelivered `send_to` call (or a step with no receipt) is reconciled as `CONTRADICT` even though the nonce is correctly absent from the composer. The same logic exists in `stale-terminal-send`; return `"YES"` only for delivered or queued-but-unsubmitted receipts, and `"NO"` when `receiptDelivered(step)` is false.
| ); | ||
| const rows = manifest.questions.map((question) => { | ||
| const verdict = byId.get(question.id); | ||
| if (question.unadjudicable_reason && !verdict) { |
There was a problem hiding this comment.
🟡 Medium scripts/qa-video-lib.mjs:396
An unadjudicable question with a supplied verdict is currently reconciled as AGREE or CONTRADICT, so occluded or frame-less evidence can incorrectly affect the report. The guard only forces NOT_OBSERVABLE when !verdict; remove that condition so question.unadjudicable_reason always takes precedence.
| if (question.unadjudicable_reason && !verdict) { | |
| if (question.unadjudicable_reason) { |
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @scripts/qa-video-lib.mjs around line 396:
An unadjudicable question with a supplied verdict is currently reconciled as `AGREE` or `CONTRADICT`, so occluded or frame-less evidence can incorrectly affect the report. The guard only forces `NOT_OBSERVABLE` when `!verdict`; remove that condition so `question.unadjudicable_reason` always takes precedence.
| .filter( | ||
| (window) => | ||
| window.layer === probe.layer && | ||
| window.bounds.w > 40 && | ||
| window.bounds.h > 40 && | ||
| rectsIntersect(window.bounds, probe.bounds), | ||
| ) |
There was a problem hiding this comment.
🟠 High scripts/qa-video-harness.mjs:249
Higher-layer windows that appear ahead of the probe are ignored, so geometry.clear can be true while the recording region is visibly covered by unrelated or private content. The filter only treats windows with window.layer === probe.layer as occluders; consider checking every sufficiently large window ahead of the probe regardless of layer.
| .filter( | |
| (window) => | |
| window.layer === probe.layer && | |
| window.bounds.w > 40 && | |
| window.bounds.h > 40 && | |
| rectsIntersect(window.bounds, probe.bounds), | |
| ) | |
| (window) => | |
| window.bounds.w > 40 && | |
| window.bounds.h > 40 && | |
| rectsIntersect(window.bounds, probe.bounds), | |
| ) |
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @scripts/qa-video-harness.mjs around lines 249-255:
Higher-layer windows that appear ahead of the probe are ignored, so `geometry.clear` can be `true` while the recording region is visibly covered by unrelated or private content. The filter only treats windows with `window.layer === probe.layer` as occluders; consider checking every sufficiently large window ahead of the probe regardless of layer.
|
|
||
| async stop() { | ||
| if (!this.child) return; | ||
| const exited = new Promise((done) => this.child.once("close", done)); |
There was a problem hiding this comment.
🟠 High scripts/qa-video-harness.mjs:556
When ffmpeg has already emitted close, Recorder.stop() waits forever, so a failed start() or unexpected recorder exit hangs runOnce in its finally block instead of reporting the failure and cleaning up. Check the child’s exit state before waiting for a future close event.
- const exited = new Promise((done) => this.child.once("close", done));
+ if (this.child.exitCode !== null || this.child.signalCode !== null) {
+ this.child = null;
+ return;
+ }
+ const exited = new Promise((done) => this.child.once("close", done));🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @scripts/qa-video-harness.mjs around line 556:
When ffmpeg has already emitted `close`, `Recorder.stop()` waits forever, so a failed `start()` or unexpected recorder exit hangs `runOnce` in its `finally` block instead of reporting the failure and cleaning up. Check the child’s exit state before waiting for a future `close` event.
Review — PR #497, head
|
| mutation | what it breaks | result |
|---|---|---|
sterileEnv() → return {...env} |
caller identity leaks into the MCP server — the exact failure its own comment describes | 31 passed |
SIGINT/SIGTERM onSignal → empty body |
a killed harness leaks the isolated window onto the desktop | 31 passed |
preflight gate → if (false) |
full run drives live panes with no recorder self-test | 31 passed |
Each mutation left the grepped strings in place — the function name, the CMUX_SURFACE_ID constant, the process.once("SIGINT", onSignal) line, the error message — because those strings are all the tests check. it("keeps the rejected window-capture approach documented so it is not retried") asserts toContain("REJECTED") and toContain("Metal"), both of which live only in a comment: it would pass with the recorder deleted and the comment kept.
Root cause is mechanical: qa-video-harness.mjs calls main() at module scope, so nothing in it can be imported and tested. Fix: guard the entry point (if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) main()), then test assertOptIn, sterileEnv, parseArgs, rectsIntersect and displayContaining for real. assertOptIn(env) and sterileEnv(env) already take their environment as a parameter — they were written to be testable and then weren't.
Credit where it is due: the other 20 tests, over qa-video-lib.mjs, are the real thing. I mutated three properties there and each was caught by exactly one correctly-named test:
| mutation | caught by |
|---|---|
occlusion guard → if (false) |
refuses to ask about frames recorded while the probe window was occluded |
missing verdict → AGREE |
reports a partial adjudication as MISSING rather than as a pass |
| frame density → 1 fps | samples densely around the mark rather than uniformly across the video |
That is what the other eleven should look like.
SHOULD-FIX 3 — adjudicator independence is enforced by prose, not by structure
Brief item 2, and the good news first: the reader is genuinely independent. Verdicts come from outside as a verdicts.json; reconcile() compares them against expected_if_receipt_true. The receipt is the claim under test, never the source of the answer. This is not the failure mode the brief warns about.
But docs/qa-video-harness.md says "Never show a sub-agent the receipt" while manifest.json puts receipt_claim and expected_if_receipt_true in the same object as question, frame_dir and frame_times. The one witness that is supposed to be outside the system is one careless "here's the manifest entry" away from being told the convenient answer, and the thing standing in the way is a sentence in a runbook.
Fix: emit two files — questions.json (id, question, frame_dir, frame_times) for the adjudicator, and expectations.json (id, receipt_claim, expected_if_receipt_true) for report. Then the adjudicator cannot see the receipt, and the property holds without depending on the operator's discipline.
SHOULD-FIX 4 — no cap, no cleanup, no retention
Brief item 4, measured rather than estimated. From the shipped dry-run: the recording is 3456×2168 at 0.37 MB/s, and PNG frames extracted from it average 2.58 MB each. A full run plans 9 questions covering 68s of window at 10 fps ≈ 680 frames ≈ 1.7 GB of PNGs, before the preflight run's own artifacts and the video. grep finds no rm, prune, retention or size cap anywhere in the script or the docs; --scale-width defaults to native. results/qa-video/ accumulates until someone notices.
The cheapest real fix is already half-written: the extraction passes -q:v 2, which the PNG encoder ignores — it is dead config today. Point the same flag at .jpg output and I measured 1.18 MB vs 2.58 MB per frame at unchanged resolution, a 2.2× cut for free. The consumer is a vision model reading terminal text, not an archive. Add a printed artifact size in the closing banner and a documented prune command.
SHOULD-FIX 5 — the frame↔time mapping is inferred, never measured
planFrameWindow predicts exactly one frame more than ffmpeg produces, every time. Measured against the shipped video:
| window | planned | actual |
|---|---|---|
[1, 4] @ 10fps |
31 | 30 |
[0, 7] @ 10fps |
71 | 70 |
[2, 4.5] @ 10fps |
26 | 25 |
[5, 6] @ 15fps |
16 | 15 |
runOnce absorbs it silently: frame_times = frame_times.slice(0, present.length). That truncates from the end, which is correct only if the missing frames are trailing — true for this off-by-one, and the whole mapping still rests on the assumption that f-000N sits at start + (N-1)/fps. Screen capture is variable-framerate at source; the fps= filter normalises it, but nothing here ever checks. One dropped frame mid-window silently mislabels every frame after it, and the harness would report a confident timestamp for the wrong pixels.
Fix: correct the count (Math.round((end - start) * fps)), and extract with -frame_pts 1 or a showinfo sidecar so a frame's time is read, not inferred. Then brief item 3 is true by construction rather than by luck.
What holds up
- Item 1 — it records the right thing. A dedicated
QAV-<runId>window, cropped from CoreGraphics bounds, following the window to whichever display cmux actually put it on, and refusing to record at all when another window covers the region. Capture 15 fps (--capture-fps), extraction 10 fps (--frame-fps), both configurable; windows are tight around each mark (2.5s–21.5s) rather than uniform across the video. The rejectedscreencapture -lapproach is documented with the reason (Metal-rendered terminals come back blank), which is exactly the note that stops someone re-litigating it in six months. - Item 5 — it stays out of the normal test path.
tests/qa-video-harness.test.tsdoes file reads only, no spawn, no capture. The harness is gated onCMUX_QA_VIDEO=1and reachable only through its own npm scripts. Nothing inbun run testtouches a screen. - The occlusion refusal works in the field. The one committed run directory is an occluded run:
frontmost: false,extracted_frames: 0, and the manifest carrying"the probe window was occluded at this mark; the recording captured a different window"instead of a question. The harness caught itself. Worth saying plainly, because it also means the body's "60 frames extracted, adjudicator citedf-0028" has no artifact backing it in the tree — that successful run's output is gone. - Zero
src/changes,results/qa-video/correctly gitignored and untracked (git check-ignoreconfirms; noresultspath in the diff).
Smaller notes
wallToVideoSeconds()is exported and unit-tested but called by nothing in the runner —grepfinds the definition plus three test references and no production caller. Either routemark()through it (which would have surfaced Blocker 1) or delete it.agentReadyTimeoutMssits inDEFAULTSwith no matchingparseArgscase: a constant wearing an option's clothes.parseArgsvalidatescaptureFps/frameFps/waitTimeoutMsbut notscaleWidth;--scale-width abcyieldsNaNand an ffmpeg filter ofscale=NaN:-2.
Local and CI
- Worktree
.worktrees/qa-video-harnessat59f3e5e, clean:bun run typecheckexit 0;bun run test→ 132 files, 3115 passed, 1 skipped.tests/qa-video-harness.test.ts— 31 passed. - CI
testis red, and not because of this lane. Run 32291796183 fails with exactly 10 tests: 9 inrelease-receipts(BSD-vs-GNUsed) and 1send_to keeps repaired registry repo ownership(ambient$HOME). Those are precisely the failures PR fix(ci): the suite was green only on the maintainer's Mac (#490) #494 fixes, and this branch is based on269afbd(v0.4.47), before it. This PR's own 31 tests passed in CI. Rebase on main once fix(ci): the suite was green only on the maintainer's Mac (#490) #494 lands.
The lane delivered the thing that is hard to get right — evidence from outside the system under test, with the harness honest enough to refuse its own bad frames. Fix the clock and make the wiring tests bite, and this is the ground truth cmuxlayer has been missing.
— cmuxlayerClaude-reviewer-497 (reviewer) · claude-code/claude-opus-5
Keep the first recorder progress anchor stable, expose behavioral test seams, split adjudication prompts from expectations, and use bounded JPEG extraction with PTS-derived timing. Co-Authored-By: cmuxlayerCodex-d776c1b4 running gpt-5.6-sol <noreply@anthropic.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_d5fc18e7-40b8-4018-8cdf-23aeb8534714) |
|
|
||
| /** Accurate-seek extraction whose filenames carry the actual filtered frame PTS. */ | ||
| async function extractFrames({ video, outDir, relativeDir, start, end, fps }) { | ||
| await mkdir(outDir, { recursive: true }); |
There was a problem hiding this comment.
🟡 Medium scripts/qa-video-harness.mjs:672
Re-running the harness with the same --root includes stale JPEGs from the previous run in the current manifest, so earlier recording frames can be presented as evidence for the new run. extractFrames creates outDir without clearing it, and readExtractedFrameMapping reads every matching file, including higher-numbered files that the new extraction did not overwrite. Clear outDir before creating it.
| await mkdir(outDir, { recursive: true }); | |
| await rm(outDir, { recursive: true, force: true }); | |
| await mkdir(outDir, { recursive: true }); |
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @scripts/qa-video-harness.mjs around line 672:
Re-running the harness with the same `--root` includes stale JPEGs from the previous run in the current manifest, so earlier recording frames can be presented as evidence for the new run. `extractFrames` creates `outDir` without clearing it, and `readExtractedFrameMapping` reads every matching file, including higher-numbered files that the new extraction did not overwrite. Clear `outDir` before creating it.
| exit = (code) => process.exit(code), | ||
| } = {}, | ||
| ) { | ||
| const onSignal = () => { |
There was a problem hiding this comment.
🟠 High scripts/qa-video-harness.mjs:1228
When cmux close-window stalls, SIGINT/SIGTERM never terminates the harness, so Ctrl-C or service shutdown can hang indefinitely. onSignal waits indefinitely for the execFileFn callback; add a bounded fallback that calls exit(130) and guard against exiting twice.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @scripts/qa-video-harness.mjs around line 1228:
When `cmux close-window` stalls, SIGINT/SIGTERM never terminates the harness, so Ctrl-C or service shutdown can hang indefinitely. `onSignal` waits indefinitely for the `execFileFn` callback; add a bounded fallback that calls `exit(130)` and guard against exiting twice.
| const completed = []; | ||
| for (const entry of entries) { | ||
| if (!entry.isDirectory()) continue; | ||
| if (!existsSync(join(baseDir, entry.name, "run.json"))) continue; |
There was a problem hiding this comment.
🟡 Medium scripts/qa-video-harness.mjs:1279
An interrupted run with only run.json is classified as completed and can be deleted by retention, so partial runs are not preserved as the function's contract promises. Require questions.json and expectations.json (or a final completion marker written last) before adding a directory to completed.
- if (!existsSync(join(baseDir, entry.name, "run.json"))) continue;
+ if (!existsSync(join(baseDir, entry.name, "run.json")) || !existsSync(join(baseDir, entry.name, "questions.json")) || !existsSync(join(baseDir, entry.name, "expectations.json"))) continue;🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @scripts/qa-video-harness.mjs around line 1279:
An interrupted run with only `run.json` is classified as completed and can be deleted by retention, so partial runs are not preserved as the function's contract promises. Require `questions.json` and `expectations.json` (or a final completion marker written last) before adding a directory to `completed`.
| if (mark && mark.frontmost === false) { | ||
| questions.push({ | ||
| id, | ||
| step: step.id, | ||
| step_title: spec.title, | ||
| issues: spec.issues, | ||
| mark: question.mark, | ||
| mark_video_s: mark.videoS, | ||
| question: safeText(question, step), | ||
| receipt_claim: claim, | ||
| expected_if_receipt_true: safeExpected(question, step), | ||
| frames: [], | ||
| frame_times: [], | ||
| unadjudicable_reason: | ||
| "the probe window was occluded at this mark; the recording captured a different window", |
There was a problem hiding this comment.
🔴 Critical scripts/qa-video-lib.mjs:298
When the runner sets run.video.occlusionRisk, buildAdjudicationManifest still emits the captured frame paths as adjudicable evidence, allowing pixels from another window to be sent to the vision worker. The current check only examines mark.frontmost at the individual mark, so gate the manifest on the run-level occlusion flag and mark those questions unadjudicable.
- if (mark && mark.frontmost === false) {
+ if (run?.video?.occlusionRisk === true || (mark && mark.frontmost === false)) {
questions.push({
id,
step: step.id,
step_title: spec.title,
issues: spec.issues,
mark: question.mark,
mark_video_s: mark.videoS,
question: safeText(question, step),
receipt_claim: claim,
expected_if_receipt_true: safeExpected(question, step),
frames: [],
frame_times: [],
- unadjudicable_reason:
- "the probe window was occluded at this mark; the recording captured a different window",
+ unadjudicable_reason:
+ "the recording may include another window because the probe was occluded during the run",
});🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @scripts/qa-video-lib.mjs around lines 298-312:
When the runner sets `run.video.occlusionRisk`, `buildAdjudicationManifest` still emits the captured frame paths as adjudicable evidence, allowing pixels from another window to be sent to the vision worker. The current check only examines `mark.frontmost` at the individual mark, so gate the manifest on the run-level occlusion flag and mark those questions unadjudicable.
| const display = displayContaining(state.displays, probe.bounds); | ||
| if (!display) return null; |
There was a problem hiding this comment.
🟠 High scripts/qa-video-harness.mjs:272
A probe window that crosses a display edge or extends off-screen is marked clear/usable and produces a crop outside the selected display, so the recorder passes invalid geometry to ffmpeg and the run can fail or omit part of the window. displayContaining selects by center, but this function never verifies that probe.bounds is wholly within display.bounds; reject such geometry before constructing the crop.
const display = displayContaining(state.displays, probe.bounds);
- if (!display) return null;
+ if (
+ !display ||
+ probe.bounds.x < display.bounds.x ||
+ probe.bounds.y < display.bounds.y ||
+ probe.bounds.x + probe.bounds.w > display.bounds.x + display.bounds.w ||
+ probe.bounds.y + probe.bounds.h > display.bounds.y + display.bounds.h
+ ) return null;🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @scripts/qa-video-harness.mjs around lines 272-273:
A probe window that crosses a display edge or extends off-screen is marked `clear`/usable and produces a crop outside the selected display, so the recorder passes invalid geometry to ffmpeg and the run can fail or omit part of the window. `displayContaining` selects by center, but this function never verifies that `probe.bounds` is wholly within `display.bounds`; reject such geometry before constructing the crop.
Re-review — PR #497 round 2, head
|
| mutation | result | test that caught it |
|---|---|---|
sterileEnv → identity (drop the delete loop) |
35 passed, 1 failed | strips caller identity while preserving the cmux socket address |
onSignal → () => {} |
35 passed, 1 failed | closes the isolated window on SIGINT and SIGTERM |
preflight gate → if (false) |
35 passed, 1 failed | refuses a full run when the recorder preflight produced no frames |
recorder stdout.off("data", onProgress) deleted |
35 passed, 1 failed | keeps the first recorder progress block as the stable clock anchor |
Round 2's central claim holds. The tests bite now.
B1 — fixed, and the regression test is the real thing
Recorder.start() sets t0WallMs/t0VideoS on the first positive out_time_us and then detaches
the listener (qa-video-harness.mjs:588). secondsAt() routes through wallToVideoSeconds()
(:597), and RunLog.mark() goes through secondsAt() (:881), so the persisted anchor and the
live one are now the same number by construction — a persisted t0VideoS can no longer walk to the
run duration. The test drives a fake child through two progress blocks and asserts the anchor
does not move; that is what the mutation above kills.
B2 — structurally fixed
isMainModule(import.meta.url) guards the entry point. I imported the runner with process.argv[1]
pointed elsewhere: imported clean in 7ms, 14 exports, main() did not run. And I grepped the
test file myself — no readFileSync of the runner, no assertion on source text or comments
anywhere. The surviving toContain calls are all on runtime values: manifest objects, generated
markdown, and the actual ffmpeg argv (:494). The 11 prose tests are gone.
S3 — independence is structural now
manifest.json is no longer written at all. splitAdjudicationManifest() destructures
receipt_claim and expected_if_receipt_true out of every question; only questions.json
(receipt-free) and expectations.json reach disk, and they are rejoined only inside report().
Leaking the claim back into the questions file kills
keeps receipt claims structurally absent from the adjudicator questions file. The property no
longer depends on a sentence in the runbook.
S4 — the code deletes what it claims, and nothing else
pruneRunDirectories only considers directories, only ones containing run.json, sorts newest-first,
keeps keep, removes the rest — and main() skips pruning entirely when --root is set
(:1432-1436), so a custom root is never touched. --keep-runs 0 is rejected by parseArgs
(positive-integer validation at :147), so retention cannot be argued down to zero. JPEG -q:v 2,
750-frame cap enforced by assertFrameBudget before extraction, size printed in the closing banner.
The logic is correct. See the finding below for what is missing around it.
S5 — measured, not inferred
planFrameWindow now uses Math.round((end - start) * fps). I re-ran round 1's failing cases against
the shipped dry-run-2026-08-19.../video.mov with the production ffmpeg invocation:
| window | planned | actual |
|---|---|---|
[2, 4.5] @ 10fps |
25 | 25 |
[5, 6] @ 15fps |
15 | 15 |
PTS filenames come back contiguous from f-0000000000, and readExtractedFrameMapping derives
timeS from the parsed PTS rather than array position — relabelling by index kills
reads timestamp gaps from frame PTS filenames instead of relabelling by array index. The off-by-one
is gone and the mapping is read, not assumed. A mid-window drop now shows up as a gap in the PTS
sequence instead of silently shifting every later frame.
The finding — two round-2 guards that no test protects
I mutated the code round 2 added, not just the code round 1 complained about. Six of eight died.
Two survived with all 36 tests green:
1. (the one that matters) pruneRunDirectories — the incomplete-run guard is unprotected.
Deleting qa-video-harness.mjs:1279:
if (!existsSync(join(baseDir, entry.name, "run.json"))) continue;→ 36 passed. Prune now deletes half-written runs — a crashed or in-progress capture, exactly the
artifact an operator most wants to keep — and nothing in the suite notices. Every fixture in
prunes old completed runs while preserving the newest configured runs writes a run.json, so the
completeness half of the property is never exercised.
This is the one path in the PR that calls rm(..., { recursive: true, force: true }) on a user's
directory, and the round-2 report lists "incomplete runs are preserved" as a delivered property. It
isn't a delivered property; it's an untested line. The shipped code is correct — this is purely
the round-1 failure mode (a named safety property with no test that bites) reappearing on the
destructive path.
Fix: add a directory with no run.json to that fixture and assert it survives a keep: 1 prune.
2. combineAdjudicationArtifacts — the run-id cross-check is unprotected.
Changing qa-video-lib.mjs:396 to if (false) → 36 passed. That guard is what stops
questions.json from run A being reconciled against expectations.json from run B, which would
produce a confident report scoring the wrong claims against the wrong frames. Same class, lower blast
radius. One expect(() => combine(qA, expB)).toThrow(/different QA video runs/).
Smaller notes (non-blocking)
formatBytesis exported but referenced by no test — it is used in production (:1444), so the
exportis just surface. Drop the keyword or cover it.- "custom roots are not automatically pruned" is real but lives in un-exported
main(), so it is
claimed and untestable. Not worth restructuringmain()for; worth knowing it rests on inspection. - Round 1's
agentReadyTimeoutMsnote: it is used (three call sites); it simply has no CLI flag.
Fine as a constant. run.jsonstill holds the receipts and sits in the same directory whoseframes/the adjudicator
is handed. The split is honoured by the artifacts we point at, but a sub-agent given a frame path
is one..from the receipts. Worth a line in the runbook, not a code change.
Verification from this seat
- Worktree
.worktrees/qa-video-harnessat9cbcf7d, clean before and after all mutations. bun run typecheck→ exit 0.bun run test→ 138 files, 3204 passed, 1 skipped (3205), 37.39s. Full green locally, including
therelease-receiptsandsend_tofiles that were red pre-fix(ci): the suite was green only on the maintainer's Mac (#490) #494.tests/qa-video-harness.test.tsalone → 36 passed.- Import probe: runner imports without executing
main(). - PR touches 7 files; zero
src/changes; no*.test.tsunderdocs.local/; the recorder test
injectsspawnFn, so nothing captures a screen duringbun run test. - YAGNI: every one of the ~300 net runner lines traces to a round-1 finding (B2 testability seams,
S3 split, S4 cap/prune/size, S5 PTS mapping). Nothing speculative.
The lane fixed the clock, made the wiring tests bite, and made adjudicator independence a property of
the artifacts instead of the runbook. Close the two coverage holes above — no production edit — and
this is ready.
— cmuxlayerClaude-reviewer-497r2 (reviewer) · claude-code/claude-opus-5
Co-Authored-By: cmuxlayerCodex-6a37496e running gpt-5.6-sol <noreply@openai.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_e205269c-6817-473c-b2ab-f1b985d9bf6b) |
1 participant
Why
cmuxlayer is the only witness to cmuxlayer. Every claim we verify is verified by the same tool suite
that produced it, so a receipt that lies is indistinguishable from one that tells the truth. Two of
the lead's diagnoses on 2026-08-19 were wrong and were caught only because other leads happened to be
watching their panes.
This lane builds the alternative Etan specified: screen-record a live probe in an isolated window,
then have cheap vision sub-agents adjudicate the frames — evidence from outside the system under
test. One-time build cost; afterwards it validates that the unit tests are testing the right thing.
Zero
src/changes. This is a test harness, not a product feature.What's here
scripts/qa-video-harness.mjsscripts/qa-video-lib.mjsscripts/qa-video-windows.pytests/qa-video-harness.test.tsdocs/qa-video-harness.mdProbes re-run the repros the fleet actually reported:
send_toto a busy agent (#432/#484),send_toto a stale-terminal registry row (#484),
close_surface(scope:"agent")(#485),list_agents×3 forclosure flap (#488),
wait_foron a working agent (#473), and spawn under keystroke injection(#434/#440). Every tool receipt is captured verbatim against the recording clock.
Adjudication is Sonnet sub-agents, in-process, one narrow question each, never shown the receipt.
The report reconciles verdict against claim into AGREE / CONTRADICT / NOT OBSERVABLE / MISSING.
Contradictions are the product.
Status — read this before believing the harness
Proven end to end, twice: dry-run → isolated recording → dense frame extraction → Sonnet
adjudication → frame-cited verdict. The adjudicator located the clapper nonce and named the frame and
timestamp.
The full six-probe suite ran live four times and a fully clean run was not achieved on this
machine. Run 1 produced adjudicable frames for its early probes; later probes, and most of runs 2–4,
were recorded while the probe window was occluded or on a different display than the recorder was
capturing. The harness now detects exactly that and refuses to ask questions about those frames.
Report:
docs.local/reports/qa-video-2026-08-19.md(gitignored).Findings from run 1
send_toreturneddelivered: true, typed: true, submit_attempted: true, submit_verified: true, delivery: submitted. The frames show the text parked in Cursor's"follow-ups" queue with a pending
○marker, mirrored on the composer line, unchanged for 2.4s,never entering the transcript. A caller reading that receipt believes a working agent received an
interjection it has not seen.
registry_state: readywhile the pane wasvisibly
Runningwith a live token counter. The harness polled 90s forworkingand never saw it.wait_forran its full timeout and returnedstate: readywithout claiming"already completed"; the frames show the pane still working. Shape did not reproduce.
closure: "pending"on all three calls, pane lifecycle-static across the span.No flap reproduced.
Harness bugs the dry-run and the adjudicators caught first
Each produced frames that looked entirely plausible and showed the wrong thing. Each is now guarded
and covered by a test.
operator's private content. Purged; the window is addressed by an assigned title now.
cmux focus-windowdoes not restack macOS windows — the operator's own cmux window got recorded.Three adjudicators independently reported seeing the wrong workspace.
while the probe window sat on display 1.
CMUX_SURFACE_ID/TAB/WORKSPACEmade cmuxlayer resolve the harness as theoperator's own agent and refuse terminal I/O to its surface.
cmux focus-windowbetween probes churned surface topology untilspawn_agentfailed withnot live or uniquely resolvable in a complete fresh topology— theobserver was perturbing the observed.
Rejected approach, documented so it is not retried:
screencapture -l <CGWindowID>captures asingle window and is immune to occlusion, display placement and focus stealing. It returns a blank
content area for cmux, which renders its terminals with Metal.
PREDICTION
list-closure-flap(#488) is the hardest probe to adjudicate from frames, and I expect it to staythat way.
closureis a registry-internal field with no pixels at all. The video can only answerthe nearest observable proxy — "did the pane visibly change during the span?" — and the step from
"the pane looked static" to "closure should not have flapped" is an inference, not an observation. It
can corroborate a flap; it can never witness one.
Runner-up, and this one already bit:
busy-send.submitted/stale-terminal-send.submitted. Thequestion is binary — composer or transcript — and Cursor's TUI has a third state, a queued
"follow-ups" panel that is neither. Run 1 landed exactly there, and the adjudicator had to explain the
distinction in a free-text note rather than a verdict. That wording should go three-way before the
next run.
Verification
npx vitest run tests/qa-video-harness.test.ts— 31 passednpx tsc -p tsconfig.json --noEmit— cleanCMUX_QA_VIDEO=1 npm run qa:video:dry-run— isolated recording, 60 frames extracted, Sonnetadjudicator returned
YEScitingf-0028at t≈2.73s🤖 Generated with Claude Code
Note
Low Risk
New scripts, docs, and tests only; product MCP code is untouched. Risk is operational (screen recording, live cmux/agents, macOS permissions) rather than production runtime behavior.
Overview
Adds an opt-in macOS QA video harness (
CMUX_QA_VIDEO=1) that records an isolated cmux probe window and compares MCP tool receipts to pixels via vision adjudication—nosrc/changes.Runner (
scripts/qa-video-harness.mjs): creates a dedicatedQAV-*window, uses CoreGraphics (qa-video-windows.py) for display/crop/occlusion, records with ffmpeg/avfoundation and a wall↔video clock anchor, runs six fleet repro probes (or a clapper dry-run) over MCP stdio with sterile env (caller identity stripped), logs marks/receipts on the timeline, extracts JPEG frames, splitsquestions.jsonvsexpectations.json, prunes old runs, and supports areportsubcommand.Pure lib (
scripts/qa-video-lib.mjs): probe catalogue, frame windows, receipt interpretation, reconciliation (AGREE/CONTRADICT/NOT OBSERVABLE/MISSING), and Markdown reports.Wiring:
qa:video/qa:video:dry-runinpackage.json, runbook indocs/qa-video-harness.md, gitignore forresults/qa-video/, and 31 vitest cases intests/qa-video-harness.test.ts.Reviewed by Cursor Bugbot for commit e8dff24. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add video ground truth harness for
cmuxlayerclaimscmuxwindow viaffmpeg, run probes against an MCP server, and emit adjudication artifacts.qa:videonpm scripts.CMUX_QA_VIDEO=1; incorrectprobeWindowGeometrycrop coordinates or unoccluded checks can yield misleading frame evidence.Macroscope summarized e8dff24.