Repository navigation
feat(testing): run base and head in one worker - #6521
Conversation
🏆 Review leaderboardOct 3–10, 2026 · UTC
Ranked by distinct external PRs reviewed, then review rounds, then PR comments. Self-activity and bot activity are excluded. Show 6 more reviewers
|
Rishet11
left a comment
Rishet11
left a comment
There was a problem hiding this comment.
Changes requested for commit bb076a2. Three required fixes are in inline comments: selected-revision preparation, conflicting worker skill instructions, and the leg-start request-body contract.
Reviewed the diff against base 6634181, including service lifecycle, session/MCP wiring, CLI and HTTP routes, storage, API contracts, and new tests. Tests and builds were not run under the reviewer rules. CI lint, API drift, sqlc drift and native jobs had passed when checked; other jobs were still running. Live base/head behavior remains unverified.
Rishet11
left a comment
Rishet11
left a comment
There was a problem hiding this comment.
Approved for this code review at commit e2900c9. No required changes found.
The three previous findings are addressed by inspection. The merged target adapter prepares the selected pinned revision, the installed testing skill keeps both legs in the same worker, and the leg-start handler accepts the bodyless POST declared by its API contract. Regression coverage now checks those boundaries. The new foreground guards reject unsupported configuration, stored background recipes, and conflicting per-tool delivery modes before input dispatch, with refusal entries in the action journal.
Reviewed the diff against base 960e7ba, including service lifecycle, session/MCP wiring, CLI and HTTP routes, persistence, API contracts, and changed tests. Tests and builds were not run under the reviewer rules. CI was still running when checked. The live same-worker base/head/cancellation proof remains unverified, so this code-review approval does not claim that runtime result.
Code changes: +837 -128
Tests: +1036 -12
Others: +1973 -14
Testing MCP credentials were tied to one attempt, so switching revisions required a fresh investigator. This change attaches tools before the first worker turn and resolves the session's current attempt on every call.
ao testing leg start base|headjoins the previous target's cleanup and starts the selected pinned revision in the same worker. Calls using the old attempt or screenshot are refused.Includes service lifecycle and persisted comparison legs, the new CLI/controller routes, session MCP wiring, checked target contexts, and generated API contracts. Worker cancellation joins target cleanup before provider teardown. Cleanup receipts retain failures, and screenshots, clips and reports remain in the evidence store.
A's #6518 intake, owned launch and worker skill are merged from feature
960e7ba56. Native evidence capture depends on C's #6522 at0375450f2; C is combined only in the separate local proof checkout. The B branch retains normal merge history and contains no C changes.The three native review findings are addressed separately: a real adapter regression covers pinned head preparation after base cleanup, the installed skill and worker prompt require the same investigator for both legs, and the bodyless leg POST now matches its API contract. A fourth commit requires foreground input and records refusals without dispatching input when actual delivery differs.
Validation:
e2900c904, including full race, generated contract drift and native platform jobs. AO native code review approved that head; all three previous review threads are resolved.Live proof on a separate immutable A+B+C build exercised PR #6266 base then head in one worker. Both native Harness/search actions and both 48-test frontend checks passed. Conversation, model and exact Codex/MCP process births stayed the same; one old-base frame was refused. Cancelling head finalized the recording and removed all captured app, daemon, driver, recorder, provider and MCP processes, listeners and control files. The live AO daemon stayed unchanged. The comparison, screenshots, original clips and cleanup observations are attached in AO.
The proof found two dependency gaps. Both target roots retained three private files despite complete cleanup receipts; A owns the minimal fix in feature commit
84d72b78d. Original QuickTime MOV clips stalled in AO Browser, while MP4 copies made with built-inavconvertpassthrough played and advanced. All encoded H.264 packet hashes match the originals, with no re-encoding. C owns the recorder container follow-up. These findings are recorded in the AO comparison; a fully clean integrated trial remains pending those fixes.Part of #6473.