diff --git a/docs/rfc/README.md b/docs/rfc/README.md index dab85690d3..54865f095c 100644 --- a/docs/rfc/README.md +++ b/docs/rfc/README.md @@ -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 | ## 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 | ## Rejected diff --git a/docs/rfc/implemented/testing/2026-06-19-acp-snapshot-tests.md b/docs/rfc/implemented/testing/2026-06-19-acp-snapshot-tests.md index 7f6cd47b48..cd7058487f 100644 --- a/docs/rfc/implemented/testing/2026-06-19-acp-snapshot-tests.md +++ b/docs/rfc/implemented/testing/2026-06-19-acp-snapshot-tests.md @@ -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 diff --git a/docs/rfc/implemented/testing/2026-07-04-hook-snapshot-matrix.md b/docs/rfc/implemented/testing/2026-07-04-hook-snapshot-matrix.md index f8ec029d22..ad61bd3b84 100644 --- a/docs/rfc/implemented/testing/2026-07-04-hook-snapshot-matrix.md +++ b/docs/rfc/implemented/testing/2026-07-04-hook-snapshot-matrix.md @@ -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 diff --git a/docs/rfc/implemented/testing/2026-07-04-single-source-acp-replay-config.md b/docs/rfc/implemented/testing/2026-07-04-single-source-acp-replay-config.md new file mode 100644 index 0000000000..c5f836b9f7 --- /dev/null +++ b/docs/rfc/implemented/testing/2026-07-04-single-source-acp-replay-config.md @@ -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. diff --git a/docs/rfc/proposed/testing/2026-07-04-single-source-acp-replay-config.md b/docs/rfc/proposed/testing/2026-07-04-single-source-acp-replay-config.md deleted file mode 100644 index 3cb986a3e7..0000000000 --- a/docs/rfc/proposed/testing/2026-07-04-single-source-acp-replay-config.md +++ /dev/null @@ -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. diff --git a/examples/acp-agent/cordis.snapshot.yml b/examples/acp-agent/cordis.snapshot.yml index 3bef79d83a..fd8a9dcad2 100644 --- a/examples/acp-agent/cordis.snapshot.yml +++ b/examples/acp-agent/cordis.snapshot.yml @@ -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'