From 9db3ce73138c92669de8b684a6ea828a324541a8 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Sat, 4 Jul 2026 17:54:17 +0800 Subject: [PATCH] test(acp): assert the patch target's name; state the overlay's true failure modes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../testing/2026-07-04-single-source-acp-replay-config.md | 4 ++-- examples/acp-agent/cordis.snapshot.yml | 7 +++++++ 2 files changed, 9 insertions(+), 2 deletions(-) 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 index 56a4cfc552..c5f836b9f7 100644 --- 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 @@ -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. diff --git a/examples/acp-agent/cordis.snapshot.yml b/examples/acp-agent/cordis.snapshot.yml index 0826c49b29..fd8a9dcad2 100644 --- a/examples/acp-agent/cordis.snapshot.yml +++ b/examples/acp-agent/cordis.snapshot.yml @@ -19,7 +19,14 @@ config: 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