Merge pull request #155 from deepseek-harness/simpl-b3-replay

test(acp): derive the replay config from the live config
This commit is contained in:
Tianyi Cui
2026-07-04 23:09:17 +08:00
committed by GitHub
6 changed files with 57 additions and 152 deletions

View File

@@ -81,7 +81,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
@@ -185,6 +184,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

View File

@@ -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

View File

@@ -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

View File

@@ -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 (matched by id AND asserted by `name`, so a reused id can never disable the wrong plugin) and insert the `llm-replay` entry ([the vendored include plugin](../../../../vendor/include/src/index.ts)'s patch mechanism: by-id overrides with an optional name assertion, 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. The `name` assertion on the disable patch guards mis-targeting (a reused id skips the patch instead of disabling the wrong plugin). An id RENAME degrades the patch to a skip whose warning needs a logger the replay app deliberately lacks — the observable result is a futile keyless `llm-deepseek` entry alongside `llm-replay`, with replay output still correct (`llm-replay` owns the stream short-circuit); config rot for review to catch, not wrong snapshots. A top-level insert whose id collides with an existing entry resolves last-wins through the loader's id map — the current config has no collision, and a new patch line is where one would be introduced.
- 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.

View 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.

View File

@@ -1,125 +1,33 @@
# Snapshot-test REPLAY config: the acp-agent plugin tree with the model backend
# swapped to llm-replay (serves a recorded session JSONL — no API key, no
# network). The dsh-acp-agent bin selects this file for DSH_SNAPSHOT=replay.
# Snapshot-test REPLAY overlay: the SAME app tree as cordis.yml, derived from
# it by an include — the one difference is the model backend. A keyless replay
# run cannot boot the real adapter (llm-deepseek's apply() throws without
# DEEPSEEK_API_KEY), so the include patches the live tree at load time: the
# llm-deepseek entry is disabled by id, and the llm-replay entry (which serves
# a recorded session JSONL — no API key, no network) is inserted. Every other
# entry — the app, the bash executor, the fs/subagent/todo tools, both hook
# bridges, the system prompt — IS the live tree, so replay exercises exactly
# what ships and an app-shape change lands once, in cordis.yml.
#
# Same app as cordis.yml (@deepseek-ai/dsh-acp-agent: the agent-core spine +
# JSONL persistence + the ACP bridge) — only the LLM backend differs: llm-replay
# here, llm-deepseek there. It can't reuse the real adapter because llm-deepseek's
# apply() throws without DEEPSEEK_API_KEY, killing a keyless replay run at boot.
#
# stdout is reserved for the ACP JSON-RPC protocol — no stdout logger (the app
# package omits it). The replay fixture path comes from $DSH_SNAPSHOT_FILE (and
# an optional $DSH_SNAPSHOT_OVERRIDE sidecar), set by the snapshot harness.
# The replay adapter: short-circuits llm/stream with the recorded log's chunks,
# in place of llm-deepseek.
- id: llm-replay
name: '@deepseek-ai/dsh-llm-replay'
# Local bash executor for agent-core's tool-bash schema.
# FIXME(config-comments): keep this executor note from implying bash is the
# whole tool set; filesystem, subagent, and todo_write are loaded below.
- id: bash
name: '@deepseek-ai/dsh-bash-local'
# The dsh-acp-agent bin selects this file for DSH_SNAPSHOT=replay. The replay
# fixture path comes from $DSH_SNAPSHOT_FILE (and an optional
# $DSH_SNAPSHOT_OVERRIDE sidecar), set by the snapshot harness. stdout stays
# reserved for the ACP JSON-RPC protocol (the app package loads no stdout
# logger). Patches apply when the include loads the file — a one-shot replay
# boot, so the load-time-only patch semantics are exactly enough.
- id: base
name: '@cordisjs/plugin-include'
config:
timeoutMs: 60000
# The ACP server app — identical to cordis.yml's entry.
- id: acp-agent
name: '@deepseek-ai/dsh-acp-agent'
config:
model: deepseek-v4-flash
persistenceRoot: !!js process.env.DSH_SNAPSHOT_SESSIONS_ROOT ?? './.sessions'
systemPrompt: |
You are a coding assistant driven over the Agent Client Protocol.
Your tools are read/write/edit for file operations, bash (plus
bash_output/bash_kill for background tasks), and subagent. Use read to
inspect UTF-8 text files, write to create or replace files, and edit for
targeted literal replacements. Use bash for shell commands, tests,
searches, and operations that are not ordinary file reads or edits. Each
bash call runs in a fresh shell — pass workdir instead of cd. Check the
[exit code: N] marker; verify your work. Keep answers brief and factual.
Use the subagent tool to delegate a focused, self-contained subtask to
a fresh child agent (it works in its own context and returns only its
final result) — give it a complete, standalone instruction. Use
subagent_fork instead when the subtask needs THIS conversation's
context: the child inherits the log so far.
For multi-step work, use the todo_write tool to track a task list:
send the WHOLE list each call (it replaces the previous one), keep at
most one task in_progress (exactly one while work remains), and mark a
task completed as soon as it is done. Skip it for trivial single-step
tasks.
# The subagent seam + both in-process backends + two model-facing tools —
# identical to cordis.yml's wiring (only the LLM backend differs above): spawn
# and fork are each reachable via a dsh-tool-subagent bound to it with a distinct
# toolName (subagent → spawn, subagent_fork → fork).
- id: subagent
name: '@deepseek-ai/dsh-subagent'
- id: subagent-spawn
name: '@deepseek-ai/dsh-subagent-spawn'
config:
providerName: spawn
- id: subagent-fork
name: '@deepseek-ai/dsh-subagent-fork'
config:
providerName: fork
- id: tool-subagent
name: '@deepseek-ai/dsh-tool-subagent'
config:
provider: spawn
toolName: subagent
- id: tool-subagent-fork
name: '@deepseek-ai/dsh-tool-subagent'
config:
provider: fork
toolName: subagent_fork
# The model-facing todo_write tool — identical to cordis.yml's wiring, so a
# replayed todo_write tool call resolves to a real tool during snapshot replay.
- id: tool-todo
name: '@deepseek-ai/dsh-tool-todo'
# Filesystem capability stack — identical to cordis.yml's wiring, so replayed
# read/write/edit tool calls resolve to the real tools during snapshot replay.
- id: fs-local
name: '@deepseek-ai/dsh-fs-local'
config:
cwd: !!js process.cwd()
- id: fs-policy
name: '@deepseek-ai/dsh-fs-policy'
- id: tool-fs
name: '@deepseek-ai/dsh-tool-fs'
# The Claude Code hook bridge. `configPath` is read ONCE at load and resolves
# `./hooks.json` against the PROCESS cwd (not per-session) — in these snapshot
# runs the harness launches the subprocess with process cwd = the scenario's temp
# workspace, so a scenario that ships `workspace/hooks.json` (copied into that cwd
# before the run) exercises the hooks path end-to-end; every other scenario has no
# such file, so the parse fails-soft and the bridge registers nothing (a silent
# no-op — the ACP app loads no logger exporter, so the warning never reaches
# stdout). Hooks themselves run in the session cwd (the bridge passes it as workdir).
- id: hooks-claude
name: '@deepseek-ai/dsh-hooks-claude'
config:
configPath: ./hooks.json
# The Codex hook bridge, loaded alongside the Claude one (symmetric with
# cordis.yml so a recorded Codex scenario fires the hook during recording too). It
# reads its OWN file `./codex-hooks.json` (Codex's dialect) — the two bridges
# cannot share one config. Same fails-soft-when-absent contract: a scenario that
# ships `workspace/codex-hooks.json` exercises the Codex path end-to-end; a
# scenario without one registers nothing (a silent no-op, never reaching stdout).
- id: hooks-codex
name: '@deepseek-ai/dsh-hooks-codex'
config:
configPath: ./codex-hooks.json
path: ./cordis.yml
patches:
# The name is an assertion, not an override: the include skips the patch
# (warning if a logger exists) when the id points at a different plugin,
# so this can never disable the wrong entry. If cordis.yml ever RENAMES
# the id, the patch degrades to a skip — replay output stays correct
# (llm-replay still short-circuits the stream) but the stale patch and a
# futile keyless adapter entry linger until review catches them.
- id: llm-deepseek
name: '@deepseek-ai/dsh-llm-deepseek'
disabled: true
- insert:
- id: llm-replay
name: '@deepseek-ai/dsh-llm-replay'