test(server): support replay gates for ACP providers - #13594
Conversation
|
Macroscope skipped reviewing this pull request. Per-review cost limit exceeded (workspace setting). This review would cost an estimated $17.50, which exceeds your per-review limit of $15.00. The top 3 files driving up this estimate:
Tip To get this pull request reviewed, you can:
|
github-actions
Bot
added
vouch:trusted
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| import { GROK_ACP_CANCEL_META } from "../../provider/acp/GrokAcpSupport.ts"; | ||
| import { makeXAiPromptCompletionRuntime } from "../../provider/acp/XAiAcpExtension.ts"; | ||
| import { layer as idAllocatorLayer, IdAllocatorV2 } from "../IdAllocator.ts"; | ||
| import { ProviderContinuationRequests } from "../ProviderContinuationRequests.ts"; |
There was a problem hiding this comment.
This new service dependency uses a named import at the adapter boundary. Please import the module as * as ProviderContinuationRequests and acquire ProviderContinuationRequests.ProviderContinuationRequests to retain its public service namespace.
Posted via Macroscope — Effect Service Conventions
| import { ACP_PROTOCOL } from "../src/orchestration-v2/Adapters/AcpAdapterV2.ts"; | ||
| import * as IdAllocator from "../src/orchestration-v2/IdAllocator.ts"; | ||
| import type { ProviderAdapterV2SessionRuntime } from "../src/orchestration-v2/ProviderAdapter.ts"; | ||
| import { ProviderContinuationRequests } from "../src/orchestration-v2/ProviderContinuationRequests.ts"; |
There was a problem hiding this comment.
This new service dependency uses a named import at the recorder's adapter boundary. Please import the module as * as ProviderContinuationRequests and acquire ProviderContinuationRequests.ProviderContinuationRequests.
Posted via Macroscope — Effect Service Conventions
ApprovabilityVerdict: Would Approve Macroscope's review found this PR approvable — This PR adds replay-gate and background-wake coverage for ACP/Grok test harnesses, with production adapter behavior unchanged unless optional test hooks are explicitly supplied. The large addition is chiefly fixture data and test infrastructure, with no product-default, schema, security, billing, deployment, or static-analysis changes. Not approved because:
Review your spending limits in Billing settings, or comment |
Thread transfer impact✅ Thread transfer remains within every enforced ceiling.
Baseline: unavailable · PR result: Scenario and decoded snapshot size10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.
Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed. |
ACP replay emitted every recorded inbound frame as soon as the one before it was sent, so it could not express frames that arrive after a turn settles. The Grok and ACP registry harnesses now honor the scenario's replay gates like the Codex and Claude testkits: the runtime holds the replay agent's inbound lines at their transcript labels until the scenario releases them, and releases everything on teardown. The Grok replay harness also wires the continuation request queue into the adapter, as production does, so a fixture that runs the continuation worker replays Grok's own wake turns as continuation runs. Before this, post-settle continuation was off in every Grok replay. The adapter's finish debounce runs on the replay test clock. A new `advanceClockWhenQuiet` option on `await_run_status` advances the clock each time no event has been stored for a short wall-clock window, so a run held open for background work settles once its provider is quiet. Recording background work: - The scenario still runs on the replay test clock so dispatch order matches replay; the Grok session runs on wall time, so the adapter's finish debounce and safety holds behave as they do live. - The recording stays open until no Grok session has a queued or running prompt, no background task runs, and every spawned subagent finished, so Grok's own wake turns (`task-completed-*`, `notifications-*`, `subagent-completed-*`) are captured. - Frame labels carry the prompt id they belong to, and background task ids are normalized like session ids, so a fixture can gate on the start of one of Grok's wake turns. - A fixture that gates records its dispatches only; live, Grok and wall time pace the run. grok_monitor is the first gated ACP fixture: a live Grok Monitor that outlives its root turn, held open until `_x.ai/task_completed`, with Grok's reply to the finished monitor replayed as a continuation run. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
7850f65 to
f056fe7
Compare
Review fixes for the ACP replay gates: - A failing step left recorded frames held at a gate, and the harness's release finalizer only ran after provider shutdown, which waited on those frames: a failing gated fixture hung for two 120 s timeouts with no diagnostic. The scenario now releases every gate when it ends, so the step's own error reports in about a minute instead. - `advanceClockWhenQuiet` polled wall time for a quiet window. It is replaced with a receipt: the adapter's new `onDeferredFinalizeScheduled` test hook reports each armed finish debounce, the Grok replay harness records it on the replay gate, and a `finish_held_run` step advances the test clock by exactly that debounce until the run settles. A rearm supersedes the previous receipt. The debounce is named (ACP_DEFERRED_FINALIZE_DEBOUNCE) and its rationale moved with it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 participant
ACP replay had no way to express frames that arrive after a turn settles. The replay agent emits every recorded inbound frame as soon as the one before it is sent, so a background task that outlives its root turn could not be replayed. The Codex and Claude testkits already honor
release_replay_gate; the ACP harnesses ignored it. Recording had the matching gap: the recorder stopped when T3's run settled, before Grok's own wake turns.Replay
makeAcpReplayRuntimetakes the scenario'sProviderReplayGateand holds the replay agent's stdout lines at their labels. The agent writes one line peremit_inboundentry in transcript order, so the Nth line carries the Nth entry's label. Lines after a held one wait with it, which keeps wire order. The Grok and ACP registry harnesses pass the gate through and release everything on teardown, like the Codex harness. This is the existing gate model (beforeEmit/release); nothing new in the orchestrator.ProviderContinuationRequests, so post-settle continuation was off in every Grok replay (postSettleContinuationEnabledwas false). It now uses the same queue the continuation worker drains, like production. The registry adapter has no continuation support, in production either, so its harness only gains gates.finish_held_runstep finishes a run held open for background work on a receipt, not on wall-time quiet: the ACP adapter reports each armed finish debounce through a test hook (onDeferredFinalizeScheduled), which the Grok harness records on the gate. The step waits for that receipt, advances the TestClock by exactly the reported debounce, and returns once the run reaches its status. If a later frame re-arms the debounce instead, it takes the next receipt. No real-time polling window is left.Recording
openSessionand its turn methods) runs on the wall clock, so the adapter's timers behave as they do live. Previously the recording stalled: the debounce waited on a TestClock nobody advanced.x.ai/queue/changedagainstturn_completed), no background task runs (background_tasks), every spawned subagent finished (subagent_spawnedagainstsubagent_finished), and 5 s pass without a frame. Without this, the transcript missed Grok's own wake turns._meta.promptId, orturn_completed.prompt_id), and background task ids are normalized like session ids. A fixture can therefore gate on the start of a specific Grok wake turn.Source
Grok schedules its own wake turns, and these are what the gates hold (xai-org/grok-build at f0e3be1):
task-completed-<task id>:crates/codegen/xai-grok-shell/src/tools/notification_bridge.rs~423, after_x.ai/task_completednotifications-<uuid v7>: batched monitor events,crates/codegen/xai-grok-shell/src/session/acp_session_impl/notification_drain.rs~753subagent-completed-<subagent id>:crates/codegen/xai-grok-shell/src/agent/subagent/spawn.rs~524Fixture:
grok_monitorRecorded live from Grok 1.0.41: a Monitor on
for i in 1 2 3; do sleep 8; echo tick $i; done, and the root turn ends withROOT_DONEwhile it runs. Replay asserts:runningwith the output so far_x.ai/task_completed, and run 1 settles after thatagent:provider), the same way Claude and Codex background wakes projectWith #13557's normalizer fix reverted, this fixture fails:
the monitor completed before tick 1.Verification
vp test runonOrchestratorReplayFixtures.integration.test.ts,.contract.test.ts,AcpAdapterV2.test.ts,XAiAcpExtension.test.ts,ProviderSwitch.integration.test.ts: 322 passed. The Grok replays passed on 3 consecutive runs.grok_monitorthat waits on a status it never reaches while a frame is held): with the teardown release, the step fails withOrchestratorV2ScenarioStepErroronce its own 60 s wait expires, and teardown is immediate. Without it, the same failure took 254 s and ended in twoTest timed out in 120000ms.vp exec tsc --noEmit -p .in apps/server: 0error TS/warning TS.vp run knip:check: clean.vp linton touched files: no new warnings. Diff grepped for renamed service imports and raw data in errors: none.Model: Claude Opus 5.5 (Claude Code)
🤖 Generated with Claude Code