diff --git a/docs/rfc/implemented/testing/2026-07-08-shared-acp-snapshot-package.md b/docs/rfc/implemented/testing/2026-07-08-shared-acp-snapshot-package.md index 8c95c1f049..910f179313 100644 --- a/docs/rfc/implemented/testing/2026-07-08-shared-acp-snapshot-package.md +++ b/docs/rfc/implemented/testing/2026-07-08-shared-acp-snapshot-package.md @@ -12,7 +12,7 @@ A second ACP example wanting snapshot coverage — the sandbox/approval composit The machinery lives in [`packages/support/acp-snapshot`](../../../../packages/support/acp-snapshot/README.md) (`@deepseek-ai/dsh-acp-snapshot`); an example's `*.snapshot.ts` is its scenario table, its agent paths, and one factory call, over its own `snapshots/` fixtures and `cordis.snapshot.yml` overlay ([single-source replay config](2026-07-04-single-source-acp-replay-config.md)). Reading `DSH_SNAPSHOT` stays at that edge — the library takes a resolved `mode`. -**`src/harness.ts`** — `runScenario` and the input-script/result types, parameterized by an `AgentUnderTest` (`binScript`, `configPath`, `tsconfigPath`; absolute paths the consuming suite resolves from its own `import.meta.url`). The client's `session/request_permission` handler consumes an optional `InputScript.permissionAnswers` FIFO queue, each entry selecting by option **kind** (ids are agent-issued randoms a committed script cannot know; kinds are the ACP-stable vocabulary, mapped to the offered `optionId` at answer time); an absent or exhausted queue answers `cancelled`, and a kind the request never offered fails loud. This is what lets an approval suite drive allow/reject round-trips deterministically from `input.json`. +**`src/harness.ts`** — `runScenario` and the input-script/result types, parameterized by an `AgentUnderTest` (`binScript`, `configPath`, `tsconfigPath`; absolute paths the consuming suite resolves from its own `import.meta.url`). The client's `session/request_permission` handler consumes an optional `InputScript.permissionAnswers` FIFO queue, each entry selecting by option **kind** (ids are agent-issued randoms a committed script cannot know; kinds are the ACP-stable vocabulary, mapped to the offered `optionId` at answer time); an absent or exhausted queue answers `cancelled`, and a kind the request never offered rejects the run — the agent itself is answered `cancelled`, so the scenario bug fails the harness rather than being absorbed as an agent-side denial. This is what lets an approval suite drive allow/reject round-trips deterministically from `input.json`. **`src/normalize.ts`** — the pure normalizers, hook-free by policy: when a future event carries a new volatile field (an approval duration, say), the shared normalizer learns it in the same change, keeping one home for what "normalized" means rather than per-suite scrub extensions. diff --git a/packages/support/acp-snapshot/README.md b/packages/support/acp-snapshot/README.md index a47a51d053..8c0b514c07 100644 --- a/packages/support/acp-snapshot/README.md +++ b/packages/support/acp-snapshot/README.md @@ -33,4 +33,4 @@ defineAcpSnapshotSuite({ The example also ships a `cordis.snapshot.yml` replay overlay next to its `cordis.yml` (the bin swaps them under `DSH_SNAPSHOT=replay` — [single-source replay config RFC](../../../docs/rfc/implemented/testing/2026-07-04-single-source-acp-replay-config.md)); replay fixtures are served by [`dsh-llm-replay`](../llm-replay/README.md), which this package points at via the `DSH_SNAPSHOT_*` env vars it sets on the child. Fixture roles, record/replay semantics, and scenario-table fields are documented on `Scenario` and in the [snapshot RFC](../../../docs/rfc/implemented/testing/2026-06-19-acp-snapshot-tests.md). -Constraints: `suite.ts` imports vitest, so the package is importable only inside a vitest run (the harness and normalizers have no such dependency but ship from the same entry). ACP-specific by design — the harness speaks the SDK's `ClientSideConnection`. Permission round-trips are scriptable: `InputScript.permissionAnswers` is a FIFO queue of option-kind selections (`allow_once`, `reject_once`, …) the client maps to the agent-issued `optionId` at answer time; an absent or exhausted queue answers `cancelled`, and a kind the request never offered fails loud. +Constraints: `suite.ts` imports vitest, so the package is importable only inside a vitest run (the harness and normalizers have no such dependency but ship from the same entry). ACP-specific by design — the harness speaks the SDK's `ClientSideConnection`. Permission round-trips are scriptable: `InputScript.permissionAnswers` is a FIFO queue of option-kind selections (`allow_once`, `reject_once`, …) the client maps to the agent-issued `optionId` at answer time; an absent or exhausted queue answers `cancelled`, and a kind the request never offered rejects the run (the agent is answered `cancelled`, so a tolerant agent cannot absorb the scenario bug). diff --git a/packages/support/acp-snapshot/src/harness.ts b/packages/support/acp-snapshot/src/harness.ts index da640f2dce..538b81d57e 100644 --- a/packages/support/acp-snapshot/src/harness.ts +++ b/packages/support/acp-snapshot/src/harness.ts @@ -97,7 +97,9 @@ export interface InputScript { * kind → the offered `optionId` at answer time. A request beyond the queue * (or with no queue at all) is answered `cancelled` — the stub behavior a * scenario without approvals relies on. A scripted kind the request does - * not offer fails loud: the scenario scripted an impossible click. + * not offer REJECTS the run: the scenario scripted an impossible click, + * and {@link runScenario} throws once the in-flight step settles (the + * agent itself just sees `cancelled`, so it cannot absorb the bug). */ permissionAnswers?: PermissionAnswer[] } @@ -240,6 +242,14 @@ export async function runScenario(input: InputScript, opts: RunOptions): Promise // Permission answers are consumed FIFO across the whole run; exhaustion // falls back to `cancelled` so approval-free scenarios keep the plain stub. const permissionQueue = [...input.permissionAnswers ?? []] + // A scenario bug detected inside a client callback (a scripted permission + // kind the agent never offered). It cannot fail the run from in there: a + // callback throw only becomes a JSON-RPC error RESPONSE to the agent, and + // a tolerant agent treats that as a denial and carries on — the run (or + // worse, a record) would absorb the impossible click silently. So the + // callback answers `cancelled` (a well-defined path for the agent), + // captures the error here, and the step loop fails the run on it. + let scriptError: Error | undefined const makeClient = (_agent: AcpAgent): Client => ({ sessionUpdate(params: SessionNotification): Promise { for (let i = updateWaiters.length - 1; i >= 0; i--) { @@ -262,12 +272,13 @@ export async function runScenario(input: InputScript, opts: RunOptions): Promise const option = params.options.find(o => o.kind === answer.kind) if (option === undefined) { // The scenario scripted a click the agent never offered — a scenario - // bug. Throwing here surfaces as a JSON-RPC error on the permission - // request, which the transcript (and usually the run) fails on. - throw new Error( + // bug. Captured (last one wins; same bug class either way) and + // answered `cancelled`; the step loop rejects the run on it. + scriptError = new Error( `snapshot-harness: scripted permission answer ${answer.kind} not among ` + `the offered options [${params.options.map(o => o.kind).join(', ')}]`, ) + return Promise.resolve({ outcome: { outcome: 'cancelled' } }) } return Promise.resolve({ outcome: { outcome: 'selected', optionId: option.optionId } }) }, @@ -276,6 +287,11 @@ export async function runScenario(input: InputScript, opts: RunOptions): Promise for (const step of input.steps) { await runStep(client, step, cwd, waitForUpdate, () => sessionId, (id) => { sessionId = id }) + // A permission exchange happens while a step's request is in flight, so + // by the time the step settles any script bug it exposed is captured — + // fail the run HERE, as a harness error, rather than hoping the agent's + // reaction to the answer perturbs the transcript. + if (scriptError !== undefined) throw scriptError } // Done driving: close stdin so the server disposes gracefully (flushing // persistence) and exits. Then await exit so the harvested log is complete. diff --git a/packages/support/acp-snapshot/tests/harness.spec.ts b/packages/support/acp-snapshot/tests/harness.spec.ts index d674e01818..683d2aaf80 100644 --- a/packages/support/acp-snapshot/tests/harness.spec.ts +++ b/packages/support/acp-snapshot/tests/harness.spec.ts @@ -257,16 +257,16 @@ describe('runScenario', () => { expect(result.rawStdout).toContain('permission:{\\"outcome\\":\\"selected\\",\\"optionId\\":\\"opt-reject\\"}') }) - it('fails loud on a scripted permission kind the agent never offered', { timeout: 20_000 }, async () => { + it('rejects the run on a scripted permission kind the agent never offered', { timeout: 20_000 }, async () => { const { fixtureFile } = await scenario({ permissionProbe: true }) // The fake bin offers allow_once/reject_once; scripting allow_always is a - // scenario bug. The client handler throws, the SDK surfaces it as a - // JSON-RPC error on the permission request, and the fake bin echoes the - // missing outcome as null. - const result = await runScenario( + // scenario bug. The agent is answered `cancelled` (it must not be able to + // absorb the bug as an error-means-denial), and the RUN fails: a callback + // throw would only reach the agent as a JSON-RPC error response, letting + // a tolerant agent carry on and the scenario pass — or record. + await expect(runScenario( { steps: [...boot, { op: 'prompt', text: 'impossible click' }], permissionAnswers: [{ kind: 'allow_always' }] }, { agent: AGENT, mode: 'replay', fixtureFile }, - ) - expect(result.rawStdout).toContain('permission:null') + )).rejects.toThrow(/allow_always not among the offered options \[allow_once, reject_once\]/) }) })