test(acp): assert the patch target's name; state the overlay's true failure modes
Codex review, with its own probes, showed the RFC overclaimed: the disable patch carried no name assertion despite the text crediting one, and an id rename is not fail-loud — the skipped patch's warning needs a logger the replay app deliberately lacks, and the resulting keyless adapter entry fails inside its fiber without reaching the unhandled-rejection guard (verified by a subprocess probe of the real installFailLoud + boot composition). The overlay now asserts name: dsh-llm-deepseek on the patch (a reused id can never disable the wrong plugin), and the RFC records the honest residual: an id rename degrades to config rot with replay output still correct (llm-replay owns the stream short-circuit), plus the insert-collision last-wins fact.
This commit is contained in:
@@ -8,7 +8,7 @@ Status: implemented
|
||||
|
||||
## 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).
|
||||
`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.
|
||||
|
||||
@@ -19,5 +19,5 @@ Keeping the full twin with a symmetry verify-gate was the recorded fallback —
|
||||
## 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.
|
||||
- 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.
|
||||
|
||||
Reference in New Issue
Block a user