test(acp): derive the replay config from the live config
examples/acp-agent/cordis.snapshot.yml is a 26-line declarative overlay: one entry mounts @cordisjs/plugin-include on ./cordis.yml with patches that disable the llm-deepseek entry by id and insert llm-replay. Every other entry is the live tree loaded through the include, so the replay tier exercises exactly what ships and an app-shape change lands once — the silent-drift class the hand-mirrored 125-line twin invited is structurally gone. The bin is untouched; recording still boots cordis.yml; assertEntriesLoaded tolerates the disabled entry by design. All snapshot scenarios pass unchanged, byte-identical goldens included; the include applies patches at load time only, which a one-shot replay boot is exactly. Implements docs/rfc/implemented/testing/2026-07-04-single-source-acp-replay-config.md (moved from proposed/ and amended); the acp-snapshot-tests and hook-snapshot-matrix RFCs' replay-config facts are amended in the same change.
This commit is contained in:
@@ -84,7 +84,6 @@ Do NOT write one for a mechanical or local choice (a variable name, a one-file r
|
||||
|---|---|
|
||||
| [Deterministic tests, the replay invariant fixture, and race stress](proposed/testing/2026-06-11-deterministic-and-stress-testing.md) | 2026-06-11 |
|
||||
| [Mutation testing as the coverage counterweight](proposed/testing/2026-06-11-mutation-testing.md) | 2026-06-11 |
|
||||
| [Single-source the acp-agent replay config](proposed/testing/2026-07-04-single-source-acp-replay-config.md) | 2026-07-04 |
|
||||
<!-- gen-rfc-index:end proposed -->
|
||||
|
||||
## Implemented
|
||||
@@ -183,6 +182,7 @@ Do NOT write one for a mechanical or local choice (a variable name, a one-file r
|
||||
| [Record fork and mixed spawn+fork snapshot scenarios](implemented/testing/2026-06-22-fork-snapshot-scenarios.md) | 2026-06-22 |
|
||||
| [Per-session snapshot replay for nested agents](implemented/testing/2026-06-22-subagent-snapshot-replay.md) | 2026-06-22 |
|
||||
| [Hook snapshot matrix — end-to-end goldens for both bridges](implemented/testing/2026-07-04-hook-snapshot-matrix.md) | 2026-07-04 |
|
||||
| [Single-source the acp-agent replay config](implemented/testing/2026-07-04-single-source-acp-replay-config.md) | 2026-07-04 |
|
||||
<!-- gen-rfc-index:end implemented -->
|
||||
|
||||
## Rejected
|
||||
|
||||
@@ -46,7 +46,7 @@ Replay is positional: the Nth `stream()` call serves the Nth `ReplayEntry`. This
|
||||
|
||||
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.
|
||||
|
||||
The ACP server app loads `@deepseek-ai/dsh-llm-deepseek`, whose `apply` throws when no API key is present ([packages/llm/llm-deepseek/src/index.ts](../../../../packages/llm/llm-deepseek/src/index.ts)). So replay cannot reuse the normal config — it uses a dedicated `examples/acp-agent/cordis.snapshot.yml` that installs `llm-replay` in place of the adapter. The rest of the tree is not duplicated: both the normal `examples/acp-agent/cordis.yml` and the replay config load the same `@deepseek-ai/dsh-acp-agent` app entry (which bundles the agent-core spine + JSONL persistence + the ACP bridge), differing only in the LLM backend (`llm-deepseek` vs `llm-replay`) and the bash executor line. Recording reuses the normal `cordis.yml` (real adapter) — its persistence root reads `$DSH_SNAPSHOT_SESSIONS_ROOT` when the harness sets it — so there is no separate record config. The `dsh-acp-agent` bin selects `cordis.snapshot.yml` for `DSH_SNAPSHOT=replay` and skips `.env` loading in that mode so a stray key cannot trigger a live call.
|
||||
The ACP server app loads `@deepseek-ai/dsh-llm-deepseek`, whose `apply` throws when no API key is present ([packages/llm/llm-deepseek/src/index.ts](../../../../packages/llm/llm-deepseek/src/index.ts)). So replay cannot boot the normal config as-is — `examples/acp-agent/cordis.snapshot.yml` is an include-overlay of `cordis.yml` that disables the `llm-deepseek` entry by id and inserts `llm-replay` (see [single-source the acp-agent replay config](2026-07-04-single-source-acp-replay-config.md)); every other entry IS the live tree, loaded through the include. Recording reuses the normal `cordis.yml` (real adapter) — its persistence root reads `$DSH_SNAPSHOT_SESSIONS_ROOT` when the harness sets it — so there is no separate record config. The `dsh-acp-agent` bin selects `cordis.snapshot.yml` for `DSH_SNAPSHOT=replay` and skips `.env` loading in that mode so a stray key cannot trigger a live call.
|
||||
|
||||
### Two surfaces: normalize, then compare
|
||||
|
||||
|
||||
@@ -18,7 +18,7 @@ Two coupled changes, in one PR:
|
||||
|
||||
It is safe because a bridge whose config file is absent is a **silent no-op**: `apply()` catches the read failure, logs through `ctx.logger`, and registers nothing — zero listeners, zero session events. The `acp-agent` app ships no stdout logger, so the warning cannot reach the ACP JSON-RPC channel. A scenario (or a real project) that wants only Claude hooks ships only `hooks.json`; the Codex bridge sees no `codex-hooks.json` and vanishes. This was verified empirically: with both bridges loaded, all pre-existing snapshots (none of which ship a `codex-hooks.json`) are byte-identical.
|
||||
|
||||
Loading both is the minimum that lets the snapshot tier exercise each dialect against the same real app the product ships. Recording (which boots `cordis.yml`) must load both too, so a recorded Codex scenario captures the transcript with its hook genuinely active — hence the symmetric edit to both configs.
|
||||
Loading both is the minimum that lets the snapshot tier exercise each dialect against the same real app the product ships. Recording (which boots `cordis.yml`) loads both by construction, and replay inherits them the same way: `cordis.snapshot.yml` is an include-overlay of `cordis.yml` that swaps only the llm entry (see [single-source the acp-agent replay config](2026-07-04-single-source-acp-replay-config.md)), so a bridge added to the live tree is in the replay tree with no second edit.
|
||||
|
||||
### 2. A snapshot scenario per hook point × its headline outcome, both dialects
|
||||
|
||||
|
||||
@@ -0,0 +1,23 @@
|
||||
# RFC: Single-source the acp-agent replay config
|
||||
|
||||
Status: implemented
|
||||
|
||||
## Problem
|
||||
|
||||
`examples/acp-agent` shipped two hand-maintained configs: `cordis.yml` (the live tree) and a `cordis.snapshot.yml` that mirrored it entry-for-entry with only the llm backend swapped — stripped of comments, the entire difference was the eight-line `llm-deepseek` stanza versus the two-line `llm-replay` stanza. Every app-shape change had to be made twice, and nothing gated the symmetry: if the copies drifted, the snapshot tier would silently exercise a different app than the one that ships — the ["green units, broken product" class of gap](../../../postmortem/0001-acp-default-export-drops-inject.md) the snapshot tier exists to close, reintroduced one level up, with reviewer vigilance as the only defense.
|
||||
|
||||
## Decision
|
||||
|
||||
`cordis.snapshot.yml` is a declarative overlay, not a copy: its single entry mounts `@cordisjs/plugin-include` on `./cordis.yml` with `patches` that disable the `llm-deepseek` entry by id and insert the `llm-replay` entry ([the vendored include plugin](../../../../vendor/include/src/index.ts)'s patch mechanism: by-id overrides with a name-assertion guard, plus top-level inserts). Every other entry — the app, the bash executor, the fs/subagent/todo tools, both hook bridges, the system prompt — is the live tree itself, loaded through the include, so replay exercises exactly what ships and an app-shape change lands once. The `dsh-acp-agent` bin is untouched (it still just selects this file for `DSH_SNAPSHOT=replay`); recording still boots `cordis.yml` directly; the bin's `assertEntriesLoaded` guard tolerates the disabled entry by design (a disabled entry is the one legitimate fiber-less state).
|
||||
|
||||
One vendored-plugin fact the overlay depends on, deliberately: the include applies `patches` when it loads the file — its `refresh()`/`internal/update` paths re-read without re-patching — which is exactly enough for a one-shot replay boot (the replay app loads no `hmr` and nothing rewrites the config mid-run). The snapshot suite is the proof: all scenarios pass unchanged on the overlay, byte-identical goldens included.
|
||||
|
||||
## Why not the alternatives?
|
||||
|
||||
Keeping the full twin with a symmetry verify-gate was the recorded fallback — it would have removed the silent-drift class but kept a 125-line near-copy whose only content was one entry's difference, growing with every plugin the app gains. A bin-side swap (parse the config, replace the entry, delete the file) would have put YAML surgery inside a published artifact and moved the replay delta out of sight; the overlay keeps the delta declarative, readable, and next to the base config — the teaching value the twin's defenders actually wanted.
|
||||
|
||||
## Consequences
|
||||
|
||||
- A plugin added to `cordis.yml` is in the replay tree with no second edit; the drift class is structurally gone rather than gated.
|
||||
- The overlay depends on entries carrying stable `id:`s — which the live config already does, and which the include's name-assertion patch guard makes checkable.
|
||||
- If a future replay tree needs a second divergence (another backend swapped), it is one more patch line, not a second fork of the file.
|
||||
@@ -1,26 +0,0 @@
|
||||
# RFC: Single-source the acp-agent replay config
|
||||
|
||||
Status: proposed
|
||||
|
||||
## Problem
|
||||
|
||||
`examples/acp-agent` ships two hand-maintained configs: `cordis.yml` (the live tree) and `cordis.snapshot.yml` (the keyless replay tree). Stripped of comments and blanks, their entire difference is ONE plugin entry — the eight-line `llm-deepseek` stanza (with its `!!js` env keys and model list) versus the two-line `llm-replay` stanza. Every other entry is byte-identical, including the multi-line system prompt and both hook-bridge stanzas. Every app-shape change must therefore be made twice, and the [hook-snapshot-matrix RFC](../../implemented/testing/2026-07-04-hook-snapshot-matrix.md) records paying exactly that tax: "hence the symmetric edit to both configs".
|
||||
|
||||
Nothing gates the symmetry. If the copies drift, the snapshot tier silently exercises a different app than the one that ships — the ["green units, broken product" class of gap](../../../postmortem/0001-acp-default-export-drops-inject.md) the snapshot tier exists to close, reintroduced one level up, with reviewer vigilance as the only defense.
|
||||
|
||||
## Proposal
|
||||
|
||||
Make the replay tree derive from the live tree instead of mirroring it. Preferred endpoint: a single source — either `cordis.snapshot.yml` becomes a thin overlay that includes `cordis.yml` and swaps only the llm entry (if the vendored loader/include config supports entry-level override), or the acp-agent bin's existing `DSH_SNAPSHOT=replay` branch performs the one-entry swap on the parsed config and `cordis.snapshot.yml` is deleted. Fallback endpoint, if single-sourcing is judged too magical for a teaching example: keep both files and add a boring verify gate (in the `doc-sync`/`hygiene` family) asserting the two configs' entry sets are equal modulo the llm entry. The implementing PR picks after checking the loader's include/override capability, updates the recording docs, and amends the snapshot RFCs' facts per [implemented/AGENTS.md](../../implemented/AGENTS.md).
|
||||
|
||||
## Why not keep the twin?
|
||||
|
||||
An explicit replay file is transparently readable and teaches replay semantics — the strongest counterargument, and the reason the fallback keeps the file and adds only the gate. YAML surgery inside the published bin is real complexity in a shipping artifact, and an include-overlay depends on loader capability that may not exist. But the status quo — a 125-line hand-maintained near-copy of a 141-line file whose one meaningful difference is two lines, defended by nothing — is the one option with a silent failure mode, and it grows with every plugin the app gains (the hook-bridge stanzas are twins in both files).
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- Either one config file plus a mechanical llm-entry swap exercised by the snapshot suite itself, or two files plus a symmetry gate that fails CI on any non-llm divergence.
|
||||
- All snapshot scenarios (hook matrix included) pass unchanged; `pnpm run test:snapshot:record` still boots the live tree.
|
||||
|
||||
## Risks
|
||||
|
||||
The include-overlay shape may be unsupported by the vendored loader — then the bin-side swap or the gate. `echo-agent`/`coding-agent` are unaffected (no snapshot twin). If the gate route is chosen, it is one more bespoke verify script — the cost the repo's gate-friendly policy explicitly accepts for encoding an invariant no human reliably remembers.
|
||||
Reference in New Issue
Block a user