refactor(acp-example): derive llm-replay script from the session JSONL
Per a design revision, the per-scenario snapshot fixture becomes EXACTLY the persisted session JSONL (<scenario>/session.jsonl) rather than a hand-authored llm.json. The log already holds all LLM behavior (assistant/chunk carries every StreamChunk) AND all harness behavior (tool/call, tool/result, turn/*, usage), so one artifact drives replay and doubles as a behavioral golden. llm-replay becomes replay-only (the record-tee is removed; recording is now "run the real agent once and harvest the .jsonl", done by the harness in a later commit). deriveReplayScript(events) groups assistant/chunk by (turn,step) in log order — exact because the loop makes one ctx.llm.stream() call per step and tags each chunk with the current (turn,step). The two failure modes the log can't express (a thrown stream — no terminal finish; cancel/hang — timing) use an optional replay.override.json sidecar. Hardens against a Codex review finding: a derived group is only valid if it ends in a `finish` chunk. A group without one is the fingerprint of a thrown stream() and is NOT silently replayed as a clean stop — deriveReplayScript throws, naming the (turn,step), so a missing sidecar override fails loud. Updates the unit tests (parse/derive/load helpers, sidecar override, finish- terminated grouping, HMR), the example README, and the RFC prose to the JSONL format. Two goldens (stdout transcript + re-persisted JSONL) and the harness wiring land in the next commit.
This commit is contained in:
@@ -14,44 +14,54 @@ This RFC records the decision to add a third test tier — **snapshot tests**
|
||||
|
||||
## Decision
|
||||
|
||||
A snapshot test boots the **real** `examples/acp-agent` subprocess, drives it over real ACP stdio with a deterministic input script, and diffs its (normalized) stdout transcript against a committed golden file. The model is made deterministic by **recording its streamed responses once** against the real API and **replaying them** on every subsequent run.
|
||||
A snapshot test boots the **real** `examples/acp-agent` subprocess, drives it over real ACP stdio with a deterministic input script, and diffs its (normalized) output against committed golden files. The model is made deterministic by **recording a real run's session log once** against the real API and **replaying it** on every subsequent run. The committed fixture IS the persisted session JSONL — the same append-only log the harness writes for any session.
|
||||
|
||||
### Record/replay at the `llm/stream` waterfall
|
||||
### The fixture is the persisted session JSONL
|
||||
|
||||
The record/replay seam is the provider-agnostic `llm/stream` waterfall ([packages/llm/src/index.ts](../../../packages/llm/src/index.ts)), not an HTTP-record library and not an adapter swap. The agent loop calls `ctx.llm.stream()` for every model call (and `generate()` also drains `stream()` internally), so a single waterfall listener intercepts every model interaction regardless of which adapter (deepseek, pi-ai) is installed. The recorded unit is the parsed `StreamChunk` — provider-neutral, JSON-serializable, and already the unit the loop treats as "the replay record" ([packages/agent-loop/src/loop.ts](../../../packages/agent-loop/src/loop.ts)).
|
||||
The per-scenario fixture is `<scenario>/session.jsonl`: the exact log produced by running the scenario once against the real API (the snapshot harness harvests the file the JSONL persistence backend writes). This log already contains everything needed to reproduce the run deterministically: its `assistant/chunk` events carry every parsed `StreamChunk` (the LLM's behavior), and its `tool/call`/`tool/result`/`turn/*`/`assistant/message`/`usage` events carry the harness's behavior. One artifact captures both, and it is the format the codebase already treats as the authoritative replay record ([packages/session/src/types.ts](../../../packages/session/src/types.ts): "raw chunks are the replay record").
|
||||
|
||||
A byte-level HTTP-record library (Polly/nock/MSW) was rejected: it would be adapter-specific (the two adapters share only the `StreamChunk` contract, not transport internals), it handles streaming SSE awkwardly, and it records at a lower level than the thing under test. Recording parsed chunks is simpler, robust, and human-reviewable.
|
||||
An earlier draft used a hand-authored `llm.json` of model chunks; reusing the real session log instead means the fixture is a genuine product of the system (not a hand-built mock), and it doubles as a behavioral golden (see below). A byte-level HTTP-record library (Polly/nock/MSW) was rejected: adapter-specific, awkward with streaming SSE, and lower-level than the thing under test.
|
||||
|
||||
### Fixture entry schema honors the full LLM contract
|
||||
### Replay derives the model script from the log
|
||||
|
||||
The LLM contract ([packages/llm/src/types.ts](../../../packages/llm/src/types.ts)) allows an adapter to report failure two ways: *throw from `stream()`*, or *end the stream with a `finish {kind:'error'|'aborted'}` chunk*. A fixture of bare `StreamChunk[]` could only replay the second. So each `llm.json` entry is a discriminated record:
|
||||
The replay seam is the provider-agnostic `llm/stream` waterfall ([packages/llm/src/index.ts](../../../packages/llm/src/index.ts)) — a single listener intercepts every model call regardless of adapter (deepseek, pi-ai), because the loop routes all model calls through `ctx.llm.stream()`. The `llm-replay` plugin short-circuits that waterfall (never calls `next()`) and serves back streams reconstructed from the log: `deriveReplayScript(events)` groups `assistant/chunk` events by `(turn, step)` in log order, yielding one model stream per group. This grouping is exact because the agent loop makes **exactly one `ctx.llm.stream()` call per step** and tags every chunk with the current `(turn, step)` ([packages/agent-loop/src/loop.ts](../../../packages/agent-loop/src/loop.ts)): `step` increments once per loop iteration, so `(turn, step)` is unique per model call. A `finish {kind:'error'}` chunk is part of its group and replays naturally — no special-casing.
|
||||
|
||||
### The in-memory replay entry honors the full LLM contract
|
||||
|
||||
`deriveReplayScript` produces a list of `ReplayEntry`, the in-memory unit the replay listener serves positionally:
|
||||
|
||||
```
|
||||
{ kind: 'chunks', chunks: StreamChunk[] }
|
||||
| { kind: 'throw', message: string, code: string, status?: number }
|
||||
| { kind: 'throw', chunks: StreamChunk[], message: string, code: string, status?: number }
|
||||
| { kind: 'hang' }
|
||||
```
|
||||
|
||||
`throw` replays the thrown-error branch (e.g. a provider 401), and `hang` (one chunk, then wait for abort) replays cancellation — mirroring the `hang` marker in the existing [MockAdapter](../../../packages/agent-loop/tests/mock-adapter.ts). This keeps both contract branches exercised through the real consumer, per the "honor cross-seam contracts on BOTH sides" defensive pattern.
|
||||
`chunks` is what the log derives. The other two cover the LLM contract's failure branches the log **cannot** reconstruct from `assistant/chunk` alone: a *pure throw before any chunk* (e.g. an HTTP 401 — the log holds only a `turn/end {error}`, no chunks) and a *cancel/hang* (a timing behavior, not chunk content). A scenario needing those supplies an optional `<scenario>/replay.override.json` (a `ReplayEntry[]`) that **replaces** the derived script. The `throw` entry carries any prefix chunks so a mid-stream failure replays its partial output before throwing — the "honor cross-seam contracts on BOTH sides" defensive pattern. Synthesizing throw/cancel from the log's `turn/end {kind:error|aborted}` was rejected: it would couple `llm-replay` to loop-internal turn-closing semantics and the `turn/end` reason is lossy (it can't distinguish a thrown 401 from a finish-error). An explicit sidecar is the cleaner seam.
|
||||
|
||||
### Positional replay, one in-flight stream
|
||||
|
||||
Replay is positional: the Nth `stream()` call in a scenario returns the Nth recorded entry (a cursor with `shift()` semantics, like MockAdapter). This is deterministic **only when at most one model stream is in flight at a time**. The first cut runs one ACP session per scenario, which guarantees that. Multi-session concurrency (the bridge multiplexes N sessions, which can prompt concurrently) would let scheduling decide which model call consumes which entry — so concurrent-session snapshots are explicitly out of scope until fixture entries are keyed by request rather than position. A scenario whose control flow changes the number/order of model calls must be re-recorded; the cursor **fails loud on overrun** rather than silently reusing or skipping an entry.
|
||||
Replay is positional: the Nth `stream()` call serves the Nth `ReplayEntry`. This is deterministic **only when at most one model stream is in flight at a time**. The first cut runs one ACP session per scenario, which guarantees that. Multi-session concurrency (the bridge multiplexes N sessions, which can prompt concurrently) would let scheduling decide which model call consumes which entry — so concurrent-session snapshots are out of scope until entries are keyed by request rather than position. A scenario whose control flow changes the number/order of model calls must be re-recorded; the cursor **fails loud on overrun** rather than silently reusing or skipping an entry. A missing `session.jsonl` in replay fails loud too ("record first") — never a silent skip.
|
||||
|
||||
### Record flushes per-stream, not on dispose
|
||||
### Recording harvests the log; keyless replay needs a providerless config
|
||||
|
||||
The example subprocess is terminated with `SIGKILL` by the test teardown, and `start.ts` has no disposal path, so a `ctx.effect` disposer that flushed `llm.json` at teardown would never run. Record mode therefore flushes the fixture **atomically after each completed stream** (write-temp-then-rename). A minimal graceful-shutdown path (SIGTERM / stdin-end → `await ctx.dispose()`) is added for cleanliness, but fixture durability does not depend on it.
|
||||
Recording runs the scenario with the real `llm-deepseek` adapter and the JSONL persistence backend, then copies the produced `.jsonl` into the scenario dir. Per-event appends are durable, but the harness shuts the subprocess down gracefully (close stdin → `await ctx.dispose()`) before harvesting so the final events are flushed. `llm-replay` itself does no recording — it is replay-only.
|
||||
|
||||
### Keyless replay needs a providerless config
|
||||
`examples/base.yml` always loads `@deepseek-ai/dsh-llm-deepseek`, whose `apply` throws when no API key is present ([packages/llm-deepseek/src/index.ts](../../../packages/llm-deepseek/src/index.ts)). So replay cannot reuse the normal config — it uses a dedicated `examples/acp-agent/cordis.snapshot.yml` that omits `llm-deepseek` and installs `llm-replay` in its place. Recording uses a config that loads the real adapter (no `llm-replay`). In replay mode `start.ts` also skips `.env` loading so a stray key cannot trigger a live call.
|
||||
|
||||
`examples/base.yml` always loads `@deepseek-ai/dsh-llm-deepseek`, whose `apply` throws when no API key is present ([packages/llm-deepseek/src/index.ts](../../../packages/llm-deepseek/src/index.ts)). So replay cannot reuse the normal config — it uses a dedicated `examples/acp-agent/cordis.snapshot.yml` that omits `llm-deepseek` and installs the replay plugin in its place. Record mode loads the real adapter *and* the replay plugin (which tees it). In replay mode `start.ts` also skips `.env` loading so a stray key cannot trigger a live call.
|
||||
### Two goldens: normalize, then snapshot
|
||||
|
||||
### Normalize, then snapshot parsed frames
|
||||
A snapshot run asserts **two** normalized goldens, because the harness's external surfaces are distinct:
|
||||
|
||||
The transcript contains non-deterministic values: `randomUUID()` session ids, the temp `mkdtemp` cwd (which appears in terminal-card `_meta`), JSON-RPC ids, and timing. A pure normalization function replaces these with stable tokens (`{{sessionId}}`, `{{cwd}}`, sequenced ids; volatile numerics dropped/rounded) **before** the snapshot. The golden holds normalized, stable-stringified frames (not opaque bytes) for clean, line-diffable PRs; a separate raw-purity assertion keeps the existing guarantee that every stdout line parses as JSON (no logger leak onto the protocol channel). Vitest's `toMatchFileSnapshot` provides the golden store and the `-u`/`--update` "accept the diff" workflow.
|
||||
1. The **stdout transcript** — the framed `session/update` JSON-RPC the editor sees. Catches regressions in the ACP bridge's event→update translation (`streamSessionEventUpdate`).
|
||||
2. The **re-derived session JSONL** — the log the replay run itself persists, compared against the recorded fixture. Catches regressions in the loop, tool dispatch, and turn/step structure that never surface on stdout.
|
||||
|
||||
The two are genuinely additive: stdout is the bridge's *lossy projection* of the log (it drops `usage`, `step/*`, exact `seq`/`time`, and renders tool I/O differently), so a loop/tool/turn-structure regression can change the JSONL while leaving the stdout projection identical, and a bridge-translation regression can change stdout while the JSONL is untouched. Asserting the JSONL equality also echoes the proposed [universal replay fixture](../proposed/2026-06-11-deterministic-and-stress-testing.md) idea.
|
||||
|
||||
Both surfaces contain non-deterministic values that a pure normalization function scrubs **before** the snapshot: `randomUUID()` session ids → `{{sessionId}}`, the temp `mkdtemp` cwd → `{{cwd}}` (it appears in terminal-card `_meta` and the log header), JSON-RPC ids → a stable sequence, and the log's per-event `time` (epoch ms) + header `createdAt` dropped or zeroed. Real bash runs during replay, so the JSONL normalizer additionally stabilizes tool-output volatility (any embedded paths/pids/timestamps) — scenarios keep bash commands tightly constrained (`echo`, file writes; no `date`/`env`/background/large-output) so this surface is small. The goldens hold normalized, stable-stringified frames/events (not opaque bytes) for clean, line-diffable PRs; a separate raw-purity assertion keeps the guarantee that every stdout line parses as JSON (no logger leak onto the protocol channel). Vitest's `toMatchFileSnapshot` provides the golden store and the `-u`/`--update` "accept the diff" workflow.
|
||||
|
||||
### Isolation: normalization now, sandbox later
|
||||
|
||||
|
||||
Determinism of the tool environment comes from a per-test `mkdtemp` cwd, the executor's existing secret-scrubbing env (`/KEY|SECRET|TOKEN/i`), the fresh non-login `bash -c` per call, and the normalization pass — **not** from an OS sandbox. A real rootless sandbox (bwrap on Linux, sandbox-exec/Seatbelt on macOS) is the established cross-platform pattern (Claude Code, Codex), but it is per-OS, fragile on newer kernels (Ubuntu 24.04+ AppArmor blocks unprivileged user namespaces), and unnecessary for transcript determinism. It is reserved as a future tier via the documented `BashExecutor` capability seam ([a sandboxing executor replaces dsh-bash-local without touching a tool schema](2026-06-13-capability-seams.md)) — a new `bash-*` package, not a change here. Scenarios keep bash commands tightly constrained (no `date`/`env`/background/large-output) so the temp-dir tier suffices.
|
||||
|
||||
### Example-local plugin, not a new package
|
||||
@@ -60,10 +70,10 @@ The replay plugin lives at `examples/acp-agent/src/llm-replay.ts`, referenced fr
|
||||
|
||||
### Two subcommands, replay in the default gate
|
||||
|
||||
`pnpm run test:snapshot` runs replay (keyless) and is composed into the default `pnpm run test` gate so every PR gets the regression check (the main `vitest.config.ts` include stays narrow; the gate is `test && test:snapshot`). `pnpm run test:snapshot:record` requires `DEEPSEEK_API_KEY` (loaded from repo `.env` first), hits the real API, and `--update`s both `llm.json` and the golden in one pass. Both forward a scenario filter. A missing fixture in replay **fails loud** with a "record first" message rather than self-skipping (the e2e self-skip rule is a CI-secret accommodation, not appropriate here — a committed-fixture test that silently vanishes is a coverage hole). No-model scenarios commit `llm.json` as `[]` so fail-loud and the no-model case don't conflict. An orphan-fixture guard test fails on a golden/fixture not referenced by any scenario (Vitest does not prune orphaned raw goldens).
|
||||
`pnpm run test:snapshot` runs replay (keyless) and is composed into the default `pnpm run test` gate so every PR gets the regression check (the main `vitest.config.ts` include stays narrow; the gate is `test && test:snapshot`). `pnpm run test:snapshot:record` requires `DEEPSEEK_API_KEY` (loaded from repo `.env` first), hits the real API, harvests the produced `session.jsonl`, and `--update`s both goldens in one pass. Both forward a scenario filter. A missing fixture in replay **fails loud** with a "record first" message rather than self-skipping (the e2e self-skip rule is a CI-secret accommodation, not appropriate here — a committed-fixture test that silently vanishes is a coverage hole). A no-model scenario's `session.jsonl` simply has no `assistant/chunk` events (empty derived script); fail-loud still applies if a model call happens with no entry. An orphan-fixture guard test fails on a golden/fixture not referenced by any scenario (Vitest does not prune orphaned raw goldens).
|
||||
|
||||
## Consequences
|
||||
|
||||
A new test tier and its fixtures to maintain: each scenario is a directory of `input.json` + `llm.json` + `stdout.golden.txt`, committed and reviewed. Re-recording when the model's phrasing changes churns the goldens — visible in review, which is the point of committing them. Bought: deterministic, keyless, full-transcript regression coverage that boots the real Loader (so it still guards the export-shape bug class), exercises the real bash executor, and gives a one-command accept-the-diff loop. The tier is ACP-first but the harness (subprocess + tee + input-DSL + normalization + record/replay waterfall) is example-agnostic and extends to other examples.
|
||||
A new test tier and its fixtures to maintain: each scenario is a directory of `input.json` (the client stdin script) + `session.jsonl` (the recorded log) + an optional `replay.override.json` + the two `*.golden` files, committed and reviewed. Re-recording when the model's phrasing changes churns the goldens — visible in review, which is the point of committing them. Bought: deterministic, keyless, full-transcript regression coverage that boots the real Loader (so it still guards the export-shape bug class), exercises the real bash executor, and gives a one-command accept-the-diff loop. The tier is ACP-first but the harness (subprocess + tee + input-DSL + normalization + JSONL-derived replay) is example-agnostic and extends to other examples.
|
||||
|
||||
This RFC relates to but does not supersede the [proposed determinism RFC](../proposed/2026-06-11-deterministic-and-stress-testing.md): that proposal's "universal replay fixture" re-derives session *message history* after every test (an internal-consistency invariant), whereas snapshot tests pin the *external protocol output*. They are complementary — one guards the event-sourcing invariant, the other guards the editor-facing contract.
|
||||
|
||||
Reference in New Issue
Block a user