docs(rfc): classify RFCs by kind via path-encoded subdirectories

Add a second axis to every RFC — its class (feature, bug-fix,
simplification, architecture, process, testing) — encoded in the path
as docs/rfc/{lifecycle}/{class}/file.md. The folder is the label, so
the closed set is enforced by structure rather than a parsed field.

Two new doc-sync gates back it:
- verify-rfc-classification: every RFC sits in a valid class folder and
  the README index lists it under the matching lifecycle→class heading.
- verify-doc-refs: every docs/*.md path cited in a packages|examples TS
  comment resolves — closes a drift class verify-md-links can't see, and
  catches the four comment refs this reorg moved.

The README gains a Classification section explaining the taxonomy and
per-class index sub-sections. A self-referential process RFC records why
the scheme is path-encoded and gated.
This commit is contained in:
Tianyi Cui
2026-06-20 22:29:45 +08:00
parent 906705e353
commit 605587e79c
92 changed files with 673 additions and 293 deletions

View File

@@ -8,7 +8,7 @@ Status: implemented (accepted 2026-06-13)
The session log is append-only by contract, but the types don't enforce it: `session.events` returns `readonly SessionEvent[]` whose *elements* are mutable, and `deriveMessages()` handed the logged `content` arrays/blocks out by reference. The loop then passes those derived messages into the `agent/request` waterfall and on to adapters, where mutating the request is sanctioned — so a request middleware could reach back and rewrite history, silently breaking replay equivalence and the derived-history guarantee. Separately, the event taxonomy (turn/step nesting, seq monotonicity, tool-call/result pairing, legal status transitions) was asserted only where individual tests happened to look.
Two ways to defend the log: make immutability part of the type (`DeepReadonly<SessionEvent>` on the way out), or catch corruption at runtime in dev. The runtime-validation proposal took the runtime route; [the deep-readonly proposal](../rejected/2026-06-11-immutable-public-surfaces.md) took the type route.
Two ways to defend the log: make immutability part of the type (`DeepReadonly<SessionEvent>` on the way out), or catch corruption at runtime in dev. The runtime-validation proposal took the runtime route; [the deep-readonly proposal](../../rejected/architecture/2026-06-11-immutable-public-surfaces.md) took the type route.
## Decision
@@ -26,4 +26,4 @@ The invariants encode the *real* contract, not an idealized one: a `tool/call` m
- History corruption is caught loudly in tests and demos, at zero production cost and zero type noise. The trade-off is that the guarantee is dynamic (a dev-mode tripwire) rather than static.
- The invariants plugin doubles as executable documentation of the event taxonomy — the assertions are the contract.
- `Session.events` keeps its `readonly SessionEvent[]` type; no consumer churn.
- This folds in [the deep-readonly proposal](../rejected/2026-06-11-immutable-public-surfaces.md) — there is no separate deep-readonly record; this records the decision to *not* pursue that approach. `InvariantError` is a plain `Error` with a `code` for now; a later taxonomy change can promote it.
- This folds in [the deep-readonly proposal](../../rejected/architecture/2026-06-11-immutable-public-surfaces.md) — there is no separate deep-readonly record; this records the decision to *not* pursue that approach. `InvariantError` is a plain `Error` with a `code` for now; a later taxonomy change can promote it.

View File

@@ -17,6 +17,6 @@ The validator mirrors `schemaSpecToJsonSchema` semantics exactly — same struct
## Consequences
- The model gets actionable feedback on its own malformed calls instead of an opaque crash, closing the gap between `InferArgs`'s promise and runtime reality.
- The validator and `InferArgs` must stay in agreement; that drift risk is to be closed by a property test ([property-based testing](2026-06-11-property-based-testing.md), not yet landed) generating args that satisfy `InferArgs` and asserting they pass `validateArgs`. Until then the agreement rests on the example tests and the shared converter structure.
- The validator and `InferArgs` must stay in agreement; that drift risk is to be closed by a property test ([property-based testing](../testing/2026-06-11-property-based-testing.md), not yet landed) generating args that satisfy `InferArgs` and asserting they pass `validateArgs`. Until then the agreement rests on the example tests and the shared converter structure.
- `ToolArgsError` is a plain `Error` with a `code` field for now; if a harness-wide error taxonomy lands it becomes a subclass without changing callers that read `.message`.
- Validation cost is negligible next to a model call.

View File

@@ -26,4 +26,4 @@ The split is not mandatory when the parts are genuinely one concern: the LLM sea
## Consequences
More packages and more boilerplate per capability (a `package.json`/`tsconfig`/README trio, the inject wiring). Bought: implementations and consumers ship and version independently, and a new backend never risks the model-facing contract. The rule is documented in [AGENTS.md](../../../AGENTS.md) § Conventions ("Capability seams are three packages") and [architecture.md](../../architecture.md) § "Capability seams"; the bash trio is the reference template. When to fold vs. split is a judgment call the architecture doc spells out — this RFC records *why* the default is to split.
More packages and more boilerplate per capability (a `package.json`/`tsconfig`/README trio, the inject wiring). Bought: implementations and consumers ship and version independently, and a new backend never risks the model-facing contract. The rule is documented in [AGENTS.md](../../../../AGENTS.md) § Conventions ("Capability seams are three packages") and [architecture.md](../../../architecture.md) § "Capability seams"; the bash trio is the reference template. When to fold vs. split is a judgment call the architecture doc spells out — this RFC records *why* the default is to split.

View File

@@ -21,4 +21,4 @@ Alternatives considered: **a single adapter** — less code and half the e2e cos
## Consequences
Double the adapter maintenance and double the key-gated e2e surface (both adapters cover V4 Flash and Pro across representative thinking/effort modes). Bought: a continuously-verified neutrality guarantee for the most leak-prone abstraction in the codebase, and a worked second example for adapter authors. The two share the core Config shape (`apiKey`/`baseURL`/`models`) so a deployment swaps mostly one line, but the reasoning knob differs — `dsh-llm-deepseek` takes `thinking`/`reasoningEffort`, `dsh-llm-pi-ai` takes a single `reasoning` level — so a swap translates that field. If the maintenance cost ever outweighs the verification value (e.g. once conformance tests from [architectural conformance](../proposed/2026-06-11-architectural-conformance.md) cover the contract mechanically), retiring the twin to a single adapter + the conformance kit would be a new RFC superseding this one.
Double the adapter maintenance and double the key-gated e2e surface (both adapters cover V4 Flash and Pro across representative thinking/effort modes). Bought: a continuously-verified neutrality guarantee for the most leak-prone abstraction in the codebase, and a worked second example for adapter authors. The two share the core Config shape (`apiKey`/`baseURL`/`models`) so a deployment swaps mostly one line, but the reasoning knob differs — `dsh-llm-deepseek` takes `thinking`/`reasoningEffort`, `dsh-llm-pi-ai` takes a single `reasoning` level — so a swap translates that field. If the maintenance cost ever outweighs the verification value (e.g. once conformance tests from [architectural conformance](../../proposed/process/2026-06-11-architectural-conformance.md) cover the contract mechanically), retiring the twin to a single adapter + the conformance kit would be a new RFC superseding this one.

View File

@@ -8,7 +8,7 @@ Status: implemented (proposed 2026-06-14, accepted 2026-06-15)
## Context
Sessions lived only in memory. The example `session-jsonl.ts` plugin (duplicated byte-for-byte in both examples) was write-only telemetry: it buffered `session/event` and appended JSON lines, with no read/replay path, no crash-safety (no fsync, no atomic write, a fire-and-forget dispose drain), no listing, and no format versioning. Nothing could rehydrate a past session from disk into a live agent, so durable resume ("continue yesterday's task"), durable forking, and the ACP `session/load` method ([ACP support](../proposed/2026-06-14-acp-agent-client-protocol.md)) were all impossible.
Sessions lived only in memory. The example `session-jsonl.ts` plugin (duplicated byte-for-byte in both examples) was write-only telemetry: it buffered `session/event` and appended JSON lines, with no read/replay path, no crash-safety (no fsync, no atomic write, a fire-and-forget dispose drain), no listing, and no format versioning. Nothing could rehydrate a past session from disk into a live agent, so durable resume ("continue yesterday's task"), durable forking, and the ACP `session/load` method ([ACP support](../../proposed/feature/2026-06-14-acp-agent-client-protocol.md)) were all impossible.
The [event-sourced model](2026-06-11-event-sourced-sessions.md) makes the append-only log the single source of truth and derives LLM history from it. Persistence had to stay faithful to that: persist the existing `SessionEvent` directly, with no parallel "persisted message" type that the log is converted to and from. The backend also had to be swappable — a file store now, a database store later — behind one interface.
@@ -24,11 +24,11 @@ Key choices recorded here because they are durable, contested, and surprising:
- **The canonical durable log persists every `SessionEvent` verbatim, including `assistant/chunk`.** `deriveMessages()` skips chunks, and a chunk-filtered rollout (Codex's `policy.rs`) is tempting — but `seq = log.length` and the load-validation `events[i].seq === i` require a *contiguous* log; filtering chunks out would leave holes and break both the contract and resume. A chunk-filtered projection is possible later as a derived view with its own renumbering, but it is NOT the canonical log.
- **Append-only; a crashed turn is closed, never truncated.** Committed events — those at or below a flushed `turn/end` — are never rewritten. The loop only flushes at `turn/end`, so a crash can leave a durable log whose final turn never closed: real, fully-written events sit after the last `turn/end`. **A single turn can be huge in a long-horizon task** (many steps, large tool output spanning a long autonomous run), so discarding the interrupted turn would silently destroy a large amount of real work — truncating a turn is wrong. Instead, on reload `load` PRESERVES those events and CLOSES the orphaned turn by durably appending the minimal synthetic boundary events: an error `tool/result` for every `tool-call` the crash left unanswered, then a `step/end` if a step was still open, then a `turn/end` carrying the merge-extensible `{ kind: 'interrupted' }` reason (a marker that records the turn was cut short by a crash, not completed by the model — no loop ever emits it). The synthetic tool results matter for resume correctness: the loop logs the `assistant/message` (carrying the `tool-call` blocks) BEFORE running the tools, so a crash mid-tool leaves calls without results; `deriveMessages()` would then replay a dangling assistant tool-call, which every provider rejects as an invalid transcript on the next request. Answering each orphaned call with an error result keeps the rehydrated history valid. `load` returns the balanced log, so a resumed session is immediately usable. The ONLY thing discarded is a never-fully-written **torn tail fragment** — a final record whose bytes (JSONL) or row were never completely flushed; that fragment is not a valid event and is dropped before the synthetic closers are written. A parse error or `seq` gap in the COMMITTED region (at or before the last real `turn/end`) is genuine corruption and makes the session unloadable.
- **File backend canonical, DB backend a proven drop-in.** `SessionEvent` maps 1:1 onto a row `(session_id, seq, type, time, data)``append` is INSERT (in a transaction asserting the contiguous-seq contract), `load` is SELECT … ORDER BY seq. `dsh-session-persistence-sqlite` is exactly this: a `SessionPersistence` subclass with no interface change (opencode runs this exact shape on SQLite/WAL), and it passes the same `runPersistenceContract` suite as the JSONL backend — so the contract holds both backends to identical semantics (lazy materialization, interrupted-turn close on load, contiguous-seq), expressed once over file bytes and once over rows.
- **Metadata is out-of-log.** Format version, cwd, and lineage are storage concerns, not replayable conversation state, so they live in a `SessionHeader` owned by `dsh-session` and attached to a `Session` via a new readonly `session.header` — never in `SessionEventMap`, never reaching `deriveMessages()`. The alternative (a merge-extensible `session/meta` event as log line 0) was rejected: an in-log event would ride along with a seeded/forked session for free, but metadata is not replayable state, so the explicit out-of-log header seam is the cleaner cost. (The header was originally split into an immutable `SessionHeader` plus a mutable `SessionSummary` whose union was `SessionMeta`; the mutable summary was later removed as dead state — see [Drop the mutable session summary](2026-06-19-drop-mutable-session-summary.md).)
- **Metadata is out-of-log.** Format version, cwd, and lineage are storage concerns, not replayable conversation state, so they live in a `SessionHeader` owned by `dsh-session` and attached to a `Session` via a new readonly `session.header` — never in `SessionEventMap`, never reaching `deriveMessages()`. The alternative (a merge-extensible `session/meta` event as log line 0) was rejected: an in-log event would ride along with a seeded/forked session for free, but metadata is not replayable state, so the explicit out-of-log header seam is the cleaner cost. (The header was originally split into an immutable `SessionHeader` plus a mutable `SessionSummary` whose union was `SessionMeta`; the mutable summary was later removed as dead state — see [Drop the mutable session summary](../simplification/2026-06-19-drop-mutable-session-summary.md).)
- **Resume is an async factory, not a change to synchronous create.** `ctx.agents.resume({ resumeSessionId })` awaits `ctx.sessionPersistence.load`, recreates the live session with the loaded events (so `lastTurnNumber`/`deriveMessages` continue), and starts a fresh agent on the resumed id (NOT `${agentId}-session`). The agent-loop does NOT hard-inject `sessionPersistence` (that would pend non-persistent demos forever); `resume` rejects with a clear error when it is absent.
Format versioning: the header carries a `version`; `load` rejects an unknown version (no v1 migration). Stated honestly: append-only + flush is robust to partial trailing writes (tolerated on load) but not to fsync-less power loss mid-line; a DB/WAL backend is the stronger option later.
## Consequences
Two new packages and the metadata seam in `dsh-session` (`session.header`, the `create(id?, options?)` signature). Bought: durable resume/fork, a read/replay path, crash tolerance, and the foundation the ACP `session/load` ([ACP support](../proposed/2026-06-14-acp-agent-client-protocol.md)) needs — all over the existing event-sourced log, with the backend swappable behind one interface. The reusable `runPersistenceContract` suite holds every backend to the same append-only / contiguous-seq / lazy-materialization / serializability semantics. This completes [event-sourced sessions](2026-06-11-event-sourced-sessions.md)'s deferred "real persistence backend" and resolves its `TODO(review)` on the event vocabulary: persisting the log freezes its shape, and the `assistant/chunk` fidelity question is answered above (persist verbatim).
Two new packages and the metadata seam in `dsh-session` (`session.header`, the `create(id?, options?)` signature). Bought: durable resume/fork, a read/replay path, crash tolerance, and the foundation the ACP `session/load` ([ACP support](../../proposed/feature/2026-06-14-acp-agent-client-protocol.md)) needs — all over the existing event-sourced log, with the backend swappable behind one interface. The reusable `runPersistenceContract` suite holds every backend to the same append-only / contiguous-seq / lazy-materialization / serializability semantics. This completes [event-sourced sessions](2026-06-11-event-sourced-sessions.md)'s deferred "real persistence backend" and resolves its `TODO(review)` on the event vocabulary: persisting the log freezes its shape, and the `assistant/chunk` fidelity question is answered above (persist verbatim).

View File

@@ -35,7 +35,7 @@ Background-task ownership moved from a `tool-bash` plugin-local `Map<string, Age
The bash owner-token comparison relies on `session.header.id` being unique among live agents. The agent registry does NOT enforce this — it rejects a duplicate *agentId*, not a duplicate session id, and `createAgent` accepts an arbitrary `sessionId`. This is NOT reachable via ACP (UUID sessionId, `agentId === sessionId`, duplicate-load rejected), so it is not a live product hole, but a programmatic caller that registers two agents with the same session id would break bash isolation and mis-route the completion notice. The access *policy* (token comparison) stays in `tool-bash` (the consumer); the bash seam stores only an opaque `owner` string and never interprets it — the correct interface/impl/consumer split.
The planned resolution is to remove the precondition by construction — see [unify the agent id and the session id](../proposed/2026-06-20-unify-agent-and-session-id.md): once an agent IS its session (one id), the registry's existing unique-`agentId` check is a unique-session-id guarantee and no two live agents can share a session token.
The planned resolution is to remove the precondition by construction — see [unify the agent id and the session id](../../proposed/simplification/2026-06-20-unify-agent-and-session-id.md): once an agent IS its session (one id), the registry's existing unique-`agentId` check is a unique-session-id guarantee and no two live agents can share a session token.
## Notes

View File

@@ -4,7 +4,7 @@ Status: implemented
## Problem
The ACP bridge lets each tool own its call rendering via `presentCall`/`presentResult` (see [tool-call UI presentation](../proposed/2026-06-14-acp-agent-client-protocol.md) and `packages/tools`). For `bash` we surface the exact command as the `tool_call` title, the model's `description` as a content text block, `kind: 'execute'`, and the completed output wrapped in a fenced ` ```console ` text block.
The ACP bridge lets each tool own its call rendering via `presentCall`/`presentResult` (see [tool-call UI presentation](../../proposed/feature/2026-06-14-acp-agent-client-protocol.md) and `packages/tools`). For `bash` we surface the exact command as the `tool_call` title, the model's `description` as a content text block, `kind: 'execute'`, and the completed output wrapped in a fenced ` ```console ` text block.
That is a correct, capability-free baseline, but not how the reference editors render a *terminal* tool at its best. An editor like Zed has a dedicated terminal tool-call card — a header showing the working directory, the command as the label, the command output rendered as a terminal, and an exit-status pill — but it only builds that card when the `tool_call` carries terminal metadata (below). With a plain text block the output appears as static markdown and there is no cwd header. (Zed also HIDES `rawInput` for `kind: 'execute'`, which is why the command IS the title — both reference adapters do the same. The human-readable description rides as a separate content block above the card; note this is a DELIBERATE divergence — claude-agent-acp DROPS the description in terminal mode and renders only the card — we keep the summary visible alongside.)

View File

@@ -15,7 +15,7 @@ Two gates, mirroring the existing `scripts/` style (tsx ESM, one job each):
1. **`doc-typecheck`** extracts every fenced ` ```ts ` block from `README.md`, `docs/**`, and `packages/*/README.md`, writes them to a temp project, and compiles with `tsc --noEmit`. The temp tsconfig copies only resolution-relevant options and the workspace `paths` map from `tsconfig.typecheck.json` (vendor → built `lib`, harness → `src`) — resolving vendor to `lib` is essential, or tsc type-checks raw vendor source and floods the run. A block that is a deliberate sketch opts out with an explicit ` ```ts ignore-check ` info string; the script reports the opt-out ratio and fails if it exceeds half, so the escape hatch can't quietly become the norm.
2. **`verify-event-taxonomy`** extracts the event names from the `interface Events` blocks across `packages/*/src` and from the taxonomy table in `docs/architecture.md`, and asserts the two sets match exactly. Verify, don't generate: the table keeps its hand-written Mode/Purpose columns; only the set of names is checked. (Landing this surfaced three events the table had been missing — `tools/change`, `llm/adapter-change`, `system-prompt/change`.) **Superseded** by [the generated cordis catalog](2026-06-20-generated-cordis-catalog.md): this gate and its `architecture.md` table are retired in favor of a fully-generated `docs/cordis-catalog/events-and-services.md` and its `verify-cordis-catalog` freshness gate. The other gates here (`doc-typecheck`, and the `verify-md-wrap` amendment below) are unaffected.
Both run via a shared `doc-sync` package.json script that the lefthook pre-push hook and CI both invoke ([mechanical quality gates](2026-06-11-quality-gates.md): hooks and CI call the same scripts, so the gate fires locally before a push — not only after it). They run after `pnpm run typecheck` (which emits the vendor `lib/` that doc-typecheck resolves against). API-extractor golden reports ([the deferred API-extractor-reports proposal](../proposed/2026-06-11-api-extractor-reports.md)) were deliberately **deferred** — low value for an internal monorepo where reviewers already see the source diff, and a heavy, finicky dependency.
Both run via a shared `doc-sync` package.json script that the lefthook pre-push hook and CI both invoke ([mechanical quality gates](2026-06-11-quality-gates.md): hooks and CI call the same scripts, so the gate fires locally before a push — not only after it). They run after `pnpm run typecheck` (which emits the vendor `lib/` that doc-typecheck resolves against). API-extractor golden reports ([the deferred API-extractor-reports proposal](../../proposed/process/2026-06-11-api-extractor-reports.md)) were deliberately **deferred** — low value for an internal monorepo where reviewers already see the source diff, and a heavy, finicky dependency.
**Amendment (2026-06-17):** a third gate, **`verify-md-wrap`**, was later folded into `doc-sync`. It parses each in-scope Markdown file (`README.md`, `docs/**`, `packages/*/README.md`, plus `AGENTS.md` / `packages/AGENTS.md`) with `mdast-util-from-markdown` + GFM and fails on any `paragraph` node spanning more than one source line, enforcing the AGENTS.md "Markdown is not hard-wrapped" convention. Same verify-don't-generate principle: it reports hard-wraps and never rewrites, so it adds no formatting churn. `doc-sync` is now three gates.

View File

@@ -22,4 +22,4 @@ Every AGENTS.md promise gets a command that exits non-zero, wired into git hooks
- Conventions survive agent turnover; violations fail fast and locally.
- The gates themselves are code to maintain; config changes are reviewed like any change.
- 100%-coverage pressure can produce assertion-free tests — mutation testing is the planned counterweight (see [the mutation-testing proposal](../proposed/2026-06-11-mutation-testing.md)).
- 100%-coverage pressure can produce assertion-free tests — mutation testing is the planned counterweight (see [the mutation-testing proposal](../../proposed/testing/2026-06-11-mutation-testing.md)).

View File

@@ -25,4 +25,4 @@ This gate checks *existence*, not anchor validity: a link to a real file with a
- Renames and moves that orphan a cross-link now fail the pre-push hook and CI instead of waiting for a reader to click a dead link. This made the RFC reorganization that introduced the gate self-verifying: the same PR that rewrote forty links also added the check that proves none dangle.
- One more fast tsx script in the `doc-sync` chain; no new dependency (the mdast/GFM stack is already in devDependencies for `verify-md-wrap`).
- Fragment/anchor validity remains unchecked — a known, deliberate scope cut.
- The convention this enforces — cross-reference docs by machine-checkable relative link, never by bare prose or a number — is documented in [docs/AGENTS.md](../../AGENTS.md) so authors know the gate exists and why.
- The convention this enforces — cross-reference docs by machine-checkable relative link, never by bare prose or a number — is documented in [docs/AGENTS.md](../../../AGENTS.md) so authors know the gate exists and why.

View File

@@ -6,7 +6,7 @@ Status: implemented (accepted 2026-06-20)
## Context
A reader trying to understand the harness could find its *behavior* in [architecture.md](../../architecture.md) (the service map, the session/turn/step lifecycle, the event taxonomy) but had no single place describing its *vocabulary* — the data structures that behavior moves around. The type shapes lived only in source, scattered across `packages/*/src/types.ts`, so understanding "what is a `Message`, a `SessionEvent`, a `StreamChunk`" meant reading the declarations directly. A prose catalog would help, but a catalog that paraphrases or paste-copies type definitions rots the instant a field changes — and an out-of-sync type doc is worse than none, because a reader trusts it.
A reader trying to understand the harness could find its *behavior* in [architecture.md](../../../architecture.md) (the service map, the session/turn/step lifecycle, the event taxonomy) but had no single place describing its *vocabulary* — the data structures that behavior moves around. The type shapes lived only in source, scattered across `packages/*/src/types.ts`, so understanding "what is a `Message`, a `SessionEvent`, a `StreamChunk`" meant reading the declarations directly. A prose catalog would help, but a catalog that paraphrases or paste-copies type definitions rots the instant a field changes — and an out-of-sync type doc is worse than none, because a reader trusts it.
So the work had two intertwined questions: **what belongs in such a catalog** (the scoping problem — a harness has dozens of cross-package types and dumping all of them helps no one), and **how to keep pasted type definitions from drifting** (the durability problem). This RFC records both decisions. Its sibling, [the generated cordis events + services catalog](2026-06-20-generated-cordis-catalog.md), is the *wiring*-axis complement: this one catalogs the data structures, that one the events and services that move them.

View File

@@ -8,7 +8,7 @@ Status: implemented (accepted 2026-06-20)
A plugin author needs two reference surfaces that no single document gave them: every cordis **event** they can listen to (with its exact signature and dispatch mode) and every `ctx.<key>` **service** they can call (with its exact interface). The pieces existed but were scattered — a hand-maintained event-taxonomy *table* in `docs/architecture.md` (names + prose Mode/Purpose, name-set-checked by `verify-event-taxonomy`), a Service-map table (8 rows of role prose), and the `interface Events` / `interface Context` declarations themselves. The taxonomy table also could not catch a brand-new *undocumented* event: a name-set verifier only checks the names that are already in the table on both sides.
This is the wiring-axis complement to the [core-data-structures catalog](../../core-data-structures/core.md) ([its RFC](2026-06-20-core-data-structures-catalog.md)): that one catalogs the *data structures* the loop moves around (verified hand-pastes); this one catalogs the *events and services* that move them.
This is the wiring-axis complement to the [core-data-structures catalog](../../../core-data-structures/core.md) ([its RFC](2026-06-20-core-data-structures-catalog.md)): that one catalogs the *data structures* the loop moves around (verified hand-pastes); this one catalogs the *events and services* that move them.
## Decision
@@ -20,7 +20,7 @@ Pure generation is correct here because the codebase is disciplined enough that
Specific choices:
- **`@mode` tag, cross-checked.** Each harness event's JSDoc carries an explicit `@mode emit|waterfall|parallel` tag; the generator hard-errors on a missing tag. Where the signature shape is conclusive — a trailing `next: () => …` parameter is structurally a waterfall — it asserts the tag agrees and hard-errors on a contradiction. The emit-vs-parallel distinction is not structurally visible (`session/flush` returns `Promise<void> | void` with no `next`), so it is trusted from the tag. The authoring rule lives in [AGENTS.md](../../../AGENTS.md).
- **`@mode` tag, cross-checked.** Each harness event's JSDoc carries an explicit `@mode emit|waterfall|parallel` tag; the generator hard-errors on a missing tag. Where the signature shape is conclusive — a trailing `next: () => …` parameter is structurally a waterfall — it asserts the tag agrees and hard-errors on a contradiction. The emit-vs-parallel distinction is not structurally visible (`session/flush` returns `Promise<void> | void` with no `next`), so it is trusted from the tag. The authoring rule lives in [AGENTS.md](../../../../AGENTS.md).
- **Tiered scope.** The harness tier (the 8 `@deepseek-ai/dsh-*` services + their events) is rendered in full from source. The inherited tier (cordis-core `ctx.on/emit/effect/provide/…` + the `internal/*` events + loader/hmr/timer) is pinned vendor source a plugin also sees; it is rendered tersely (name + one-line + source pointer) from a curated table in the generator, NOT walked from the vendor AST — the cordis-core `Context` mixes true ctx members with non-service fields (`root`, `baseUrl`, `logger`), and the vendor surface changes only on a deliberate vendor sync.
- **Cross-links to the data-structure catalog.** A type name in a signature (`GenerateOptions`, `StreamChunk`, `ToolDefinition`, …) links to the core-data-structures page that documents it. The map is a small hand-curated const in the generator — NOT `type-equiv.manifest.json`, which documents the `…Map` symbols while signatures reference the derived union names, and lists a few symbols on two pages.
- **A dedicated fence.** Signature blocks use a ` ```ts cordis-catalog ` info string that `doc-typecheck` recognizes and skips (a bare signature fragment is not standalone-compilable), excluded from the opt-out ratio — the same treatment `type-equiv` blocks get.

View File

@@ -0,0 +1,46 @@
# RFC: Classify RFCs by kind via path-encoded subdirectories
Status: implemented (proposed 2026-06-20, accepted 2026-06-20)
## Context
`docs/rfc/` grouped RFCs by **lifecycle** only — `proposed/` / `implemented/` / `rejected/`. Nothing recorded what *kind* of decision each RFC was. The index was one flat list per lifecycle, with no way to scan "show me every simplification" or "every testing-strategy decision." A wave of simplification RFCs landing on the same day made the gap concrete: a reader skimming `proposed/` could not tell a new capability from a removal from a tooling-policy change without opening each file.
The repo's standing bias is [mechanical quality gates over prose guidelines](2026-06-11-quality-gates.md): a convention that isn't machine-checked rots. So a classification scheme here had to be enforceable, not an honor-system header.
## Decision
Add a second axis — the RFC's **class** — and encode it in the path: `{lifecycle}/{class}/yyyy-mm-dd-topic.md`. The folder *is* the label. A file's location declares its class, the closed set is "these folders and no others," and the existing [verify-md-links](2026-06-18-markdown-cross-link-lint.md) gate already protects the path rewrites the move required.
### The closed set of six classes
| Class | Covers |
|---|---|
| `feature` | A new user- or model-facing capability. |
| `bug-fix` | Corrects a defect or closes a gap a postmortem surfaced. |
| `simplification` | Removes code, behavior, or surface area without adding a capability. |
| `architecture` | A structural decision about the **shipped source** — how packages relate, what the runtime vocabulary is. |
| `process` | Tooling, policy, or workflow **around** the code, not runtime behavior. |
| `testing` | Test infrastructure and strategy. |
The `architecture` / `process` line: **architecture** is about the source we ship; **process** is the surrounding tooling and workflow. This RFC is itself a `process` decision — it changes how the repo is organized and gated, not what the harness does at runtime — so it lives under `implemented/process/`.
### Two gates
Both are `doc-sync` members, in the `verify-md-wrap` style (tsx ESM, verify-don't-generate, exit non-zero on the first violation):
- **`scripts/verify-rfc-classification.ts`** — the closed set and index completeness. It asserts every file under a lifecycle folder lives in a class folder from the canonical set (a loose `.md` at a lifecycle root, or an unknown class folder, fails), and that `README.md` lists every RFC exactly once under the `###` heading matching its `{lifecycle}/{class}` path. The canonical class set lives as a `const` in this script — the machine source of truth — and [the index](../../README.md) documents it in prose; the two are kept in sync by hand (the README's completeness is gated, its class *descriptions* are not). This mirrors `verify-event-taxonomy`, which checks a doc table against source.
- **`scripts/verify-doc-refs.ts`** — source comments that cite docs. RFC paths are referenced not only from Markdown but from TypeScript doc comments (root-relative prose like `docs/rfc/implemented/testing/2026-06-19-acp-snapshot-tests.md`). `verify-md-links` never saw those, so the reorg could have silently orphaned them. This gate scans repo-authored `.ts` under `packages/**` and `examples/**` (excluding built `lib/` and `vendor/`) for `docs/….md` tokens, resolves each root-relative, and asserts it exists. It requires the `.md` extension so extensionless prose (`docs/postmortem/0001`, `docs/architecture.md § plugin checklist`) is left alone.
### Rejected alternatives
- **A `Classification:` prose line** in each file (next to `Status:`), parsed by the gate. Workable, but it duplicates into the file a fact the path can already carry, and a line can disagree with its folder. Path-encoding makes the label and its storage the same thing — there is nothing to keep in sync.
- **A `refactor` class.** It overlaps `simplification` almost entirely; the only discriminator anyone reached for was "does observable behavior change?", which `simplification` already encodes (it does not). One class, not two.
- **Auto-generating the README index** from the filesystem. Rejected to keep the index hand-written like every other doc here; the completeness gate gives the same drift-protection without generated Markdown in a curated file.
## Consequences
- Every RFC now sits under a class folder, and the index groups by class within each lifecycle. A reader scans one heading to see all simplifications, or all testing decisions.
- Two more fast tsx scripts in the `doc-sync` chain; no new dependency (the mdast/GFM stack was already present for `verify-md-wrap`/`verify-md-links`).
- Adding a class is a deliberate act: amend the `const` in `verify-rfc-classification.ts` and the [Classification section](../../README.md#classification), not just `mkdir` a folder. The gate rejects an unknown folder, so an ad-hoc class can't slip in.
- Source-comment doc references are now gated too — a moved or renamed doc that a `.ts` comment cites fails the pre-push hook, closing a drift class `verify-md-links` structurally could not see.

View File

@@ -4,7 +4,7 @@ Status: implemented (proposed and accepted 2026-06-19)
## Context
The [session-persistence seam](2026-06-14-session-persistence.md) split a session's out-of-log metadata into two types owned by `dsh-session`: an immutable `SessionHeader` (`version`, `id`, `createdAt`, `cwd?`, `parentSession?`) written once at creation, and a mutable `SessionSummary` (`updatedAt`, `title?`, `firstPrompt?`) "updateable without touching the append-only log". Their union was `SessionMeta = SessionHeader & SessionSummary`, and the abstract `SessionPersistence` service carried a seventh method — `update(id, summary)` — for rewriting the summary. Each backend implemented the mutable store its own way: JSONL wrote a separate atomic `.summary.json` **sidecar** beside the log (temp-write + rename, best-effort), SQLite kept `updated_at`/`title`/`first_prompt` **columns** bumped inside the append transaction.
The [session-persistence seam](../architecture/2026-06-14-session-persistence.md) split a session's out-of-log metadata into two types owned by `dsh-session`: an immutable `SessionHeader` (`version`, `id`, `createdAt`, `cwd?`, `parentSession?`) written once at creation, and a mutable `SessionSummary` (`updatedAt`, `title?`, `firstPrompt?`) "updateable without touching the append-only log". Their union was `SessionMeta = SessionHeader & SessionSummary`, and the abstract `SessionPersistence` service carried a seventh method — `update(id, summary)` — for rewriting the summary. Each backend implemented the mutable store its own way: JSONL wrote a separate atomic `.summary.json` **sidecar** beside the log (temp-write + rename, best-effort), SQLite kept `updated_at`/`title`/`first_prompt` **columns** bumped inside the append transaction.
The summary was designed for a future session picker (recency ordering via `updatedAt`, a `title`/`firstPrompt` preview). That picker was never built. An audit of the whole repo found the entire `SessionSummary` surface is **dead state**:
@@ -20,12 +20,12 @@ Delete the mutable session summary entirely. `SessionSummary` and the `SessionMe
Anything the summary was meant to provide is **derivable from the append-only log** when a consumer actually needs it (`firstPrompt` = first `user/message`; recency = the last event's `time` or the file mtime) or already lives in the immutable header (`createdAt`, `cwd`). The one thing *not* derivable — a user-*edited* title — had no implementation and is pure YAGNI; it can return as its own log event or header field if a real feature ever needs it.
This is recorded as a decision because it is **durable** (it narrows a public service contract and an on-disk format across two backends), **contested** (the summary was a deliberate forward-looking design, not an accident), and **surprising** (a future reader finding `SessionHeader` where the original RFC describes `SessionMeta` would otherwise ask why the summary vanished). It also unblocks the [shared persistence write coordinator](2026-06-18-shared-persistence-write-coordinator.md): with no mutable summary, the coordinator's hook interface needs no `updateSummary` hook and the JSONL-sidecar-vs-SQLite-column durability divergence disappears, so the two backends' write paths converge.
This is recorded as a decision because it is **durable** (it narrows a public service contract and an on-disk format across two backends), **contested** (the summary was a deliberate forward-looking design, not an accident), and **surprising** (a future reader finding `SessionHeader` where the original RFC describes `SessionMeta` would otherwise ask why the summary vanished). It also unblocks the [shared persistence write coordinator](../architecture/2026-06-18-shared-persistence-write-coordinator.md): with no mutable summary, the coordinator's hook interface needs no `updateSummary` hook and the JSONL-sidecar-vs-SQLite-column durability divergence disappears, so the two backends' write paths converge.
## No migration
This is unreleased software (see [root AGENTS.md](../../../AGENTS.md) § "Pre-release stance: foundation over blast radius"), so there are no on-disk databases or logs to preserve. SQLite does not migrate a v1 database: the `openDatabase` guard now rejects any non-current on-disk `user_version` (`onDisk !== 0 && onDisk !== SCHEMA_VERSION`) — older *or* newer — so a stale v1 DB is cleanly rejected rather than half-read against the new column set. A fresh database stamps the current version; that is the only path that needs to work.
This is unreleased software (see [root AGENTS.md](../../../../AGENTS.md) § "Pre-release stance: foundation over blast radius"), so there are no on-disk databases or logs to preserve. SQLite does not migrate a v1 database: the `openDatabase` guard now rejects any non-current on-disk `user_version` (`onDisk !== 0 && onDisk !== SCHEMA_VERSION`) — older *or* newer — so a stale v1 DB is cleanly rejected rather than half-read against the new column set. A fresh database stamps the current version; that is the only path that needs to work.
## What we gave up
A future session picker now has to derive its preview/ordering from the log (or reintroduce a typed field) rather than reading a ready-made summary row. That is the correct cost: a cache for a feature that does not exist is dead weight that every backend pays to maintain and every contract test pays to assert. The principle — **a passing test pins current behavior, not necessarily correct behavior; behavior can be an artifact of a past compromise** — is now recorded as a standalone convention in [root AGENTS.md](../../../AGENTS.md), with this change as its worked example.
A future session picker now has to derive its preview/ordering from the log (or reintroduce a typed field) rather than reading a ready-made summary row. That is the correct cost: a cache for a feature that does not exist is dead weight that every backend pays to maintain and every contract test pays to assert. The principle — **a passing test pins current behavior, not necessarily correct behavior; behavior can be an artifact of a past compromise** — is now recorded as a standalone convention in [root AGENTS.md](../../../../AGENTS.md), with this change as its worked example.

View File

@@ -16,7 +16,7 @@ Adopt `fast-check` (a root devDependency) with one `tests/properties.spec.ts` pe
- **dsh-llm / BlockAssembler:** arbitrary chunk streams (valid + malformed: duplicate indices, stragglers, missing block-start). Invariants: `flushReady()+flushRemaining() ≡ blocks()` in order; the streamed prefix is always a prefix of final `blocks()`; partial count ≤ distinct indices; re-assembly idempotent.
- **dsh-session:** arbitrary event logs. Invariants: `deriveMessages` deterministic; replay-from-seed identical; seq strictly monotonic; non-message events never affect derived history; derived content is decoupled from the log.
- **dsh-tools:** arbitrary `SchemaSpec`. Invariants: JSON Schema `required` equals the `required:true` keys at every level; conversion total; **and the composition with [runtime arg validation](2026-06-11-runtime-arg-validation.md)** — generated args satisfying a spec pass `validateArgs`, and targeted corruptions (dropped required key, non-object top level) are rejected. This closes the validator/`InferArgs` drift risk.
- **dsh-tools:** arbitrary `SchemaSpec`. Invariants: JSON Schema `required` equals the `required:true` keys at every level; conversion total; **and the composition with [runtime arg validation](../architecture/2026-06-11-runtime-arg-validation.md)** — generated args satisfying a spec pass `validateArgs`, and targeted corruptions (dropped required key, non-object top level) are rejected. This closes the validator/`InferArgs` drift risk.
- **dsh-agent-loop:** arbitrary send schedules against a never-exhausting adapter, driven through the `agent/status` settle signal (no wall-clock sleeps). Invariants: no message lost; turn numbers strictly increase; status transitions stay on the legal machine.
## Consequences

View File

@@ -6,7 +6,7 @@ Status: implemented (accepted 2026-06-19)
## Context
The harness has two test tiers: keyless unit `.spec.ts` (the 100%-per-file coverage gate) and real-API `.e2e.ts` (key-gated, self-skipping in CI). Neither continuously verifies the **complete output transcript** an ACP editor (Zed) sees on its stdin/stdout. The existing ACP e2e ([examples/acp-agent/tests/acp.e2e.ts](../../../examples/acp-agent/tests/acp.e2e.ts)) is the closest end-to-end check, but it is key-gated and asserts on a handful of *structured fields* (`stopReason`, a `tool_call` title), not the byte-for-byte stream of `session/update` frames. That leaves the "green units, broken product" gap: every unit test can pass while the actual editor-facing protocol output regresses — the same class of failure that shipped the inject bug ([docs/postmortem/0001](../../postmortem/0001-acp-default-export-drops-inject.md)), where 178 hand-mounted tests stayed green while a real Zed session crashed instantly.
The harness has two test tiers: keyless unit `.spec.ts` (the 100%-per-file coverage gate) and real-API `.e2e.ts` (key-gated, self-skipping in CI). Neither continuously verifies the **complete output transcript** an ACP editor (Zed) sees on its stdin/stdout. The existing ACP e2e ([examples/acp-agent/tests/acp.e2e.ts](../../../../examples/acp-agent/tests/acp.e2e.ts)) is the closest end-to-end check, but it is key-gated and asserts on a handful of *structured fields* (`stopReason`, a `tool_call` title), not the byte-for-byte stream of `session/update` frames. That leaves the "green units, broken product" gap: every unit test can pass while the actual editor-facing protocol output regresses — the same class of failure that shipped the inject bug ([docs/postmortem/0001](../../../postmortem/0001-acp-default-export-drops-inject.md)), where 178 hand-mounted tests stayed green while a real Zed session crashed instantly.
The blocker for a full-transcript test is the model: the agent's output is driven by a non-deterministic LLM, and a key-gated test that hits the real API on every run is neither deterministic nor CI-runnable. We want the fidelity of a real run with the determinism of a fixture.
@@ -18,13 +18,13 @@ A snapshot test boots the **real** `examples/acp-agent` subprocess, drives it ov
### The fixture is the persisted session JSONL
The per-scenario fixture is `<scenario>/session.jsonl`: the exact log produced by running the scenario once against the real API (the snapshot harness harvests the file the JSONL persistence backend writes). This log already contains everything needed to reproduce the run deterministically: its `assistant/chunk` events carry every parsed `StreamChunk` (the LLM's behavior), and its `tool/call`/`tool/result`/`turn/*`/`assistant/message`/`usage` events carry the harness's behavior. One artifact captures both, and it is the format the codebase already treats as the authoritative replay record ([packages/session/src/types.ts](../../../packages/session/src/types.ts): "raw chunks are the replay record").
The per-scenario fixture is `<scenario>/session.jsonl`: the exact log produced by running the scenario once against the real API (the snapshot harness harvests the file the JSONL persistence backend writes). This log already contains everything needed to reproduce the run deterministically: its `assistant/chunk` events carry every parsed `StreamChunk` (the LLM's behavior), and its `tool/call`/`tool/result`/`turn/*`/`assistant/message`/`usage` events carry the harness's behavior. One artifact captures both, and it is the format the codebase already treats as the authoritative replay record ([packages/session/src/types.ts](../../../../packages/session/src/types.ts): "raw chunks are the replay record").
An earlier draft used a hand-authored `llm.json` of model chunks; reusing the real session log instead means the fixture is a genuine product of the system (not a hand-built mock), and it doubles as a behavioral golden (see below). A byte-level HTTP-record library (Polly/nock/MSW) was rejected: adapter-specific, awkward with streaming SSE, and lower-level than the thing under test.
### Replay derives the model script from the log
The replay seam is the provider-agnostic `llm/stream` waterfall ([packages/llm/src/index.ts](../../../packages/llm/src/index.ts)) — a single listener intercepts every model call regardless of adapter (deepseek, pi-ai), because the loop routes all model calls through `ctx.llm.stream()`. The `llm-replay` plugin short-circuits that waterfall (never calls `next()`) and serves back streams reconstructed from the log: `deriveReplayScript(events)` groups `assistant/chunk` events by `(turn, step)` in log order, yielding one model stream per group. This grouping is exact because the agent loop makes **exactly one `ctx.llm.stream()` call per step** and tags every chunk with the current `(turn, step)` ([packages/agent-loop/src/loop.ts](../../../packages/agent-loop/src/loop.ts)): `step` increments once per loop iteration, so `(turn, step)` is unique per model call. A `finish {kind:'error'}` chunk is part of its group and replays naturally — no special-casing.
The replay seam is the provider-agnostic `llm/stream` waterfall ([packages/llm/src/index.ts](../../../../packages/llm/src/index.ts)) — a single listener intercepts every model call regardless of adapter (deepseek, pi-ai), because the loop routes all model calls through `ctx.llm.stream()`. The `llm-replay` plugin short-circuits that waterfall (never calls `next()`) and serves back streams reconstructed from the log: `deriveReplayScript(events)` groups `assistant/chunk` events by `(turn, step)` in log order, yielding one model stream per group. This grouping is exact because the agent loop makes **exactly one `ctx.llm.stream()` call per step** and tags every chunk with the current `(turn, step)` ([packages/agent-loop/src/loop.ts](../../../../packages/agent-loop/src/loop.ts)): `step` increments once per loop iteration, so `(turn, step)` is unique per model call. A `finish {kind:'error'}` chunk is part of its group and replays naturally — no special-casing.
### The in-memory replay entry honors the full LLM contract
@@ -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.
`examples/base.yml` always loads `@deepseek-ai/dsh-llm-deepseek`, whose `apply` throws when no API key is present ([packages/llm-deepseek/src/index.ts](../../../packages/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. To avoid duplicating the rest of the tree, the providerless core is factored into `examples/base-core.yml` (shared by `base.yml = base-core + llm-deepseek` and the replay config = `base-core + llm-replay`), and the agent-loop/persistence/ACP-bridge tail into `examples/acp-agent/acp-tail.yml` (shared by `cordis.yml` and the replay config). 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. In replay mode `start.ts` skips `.env` loading so a stray key cannot trigger a live call.
`examples/base.yml` always loads `@deepseek-ai/dsh-llm-deepseek`, whose `apply` throws when no API key is present ([packages/llm-deepseek/src/index.ts](../../../../packages/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. To avoid duplicating the rest of the tree, the providerless core is factored into `examples/base-core.yml` (shared by `base.yml = base-core + llm-deepseek` and the replay config = `base-core + llm-replay`), and the agent-loop/persistence/ACP-bridge tail into `examples/acp-agent/acp-tail.yml` (shared by `cordis.yml` and the replay config). 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. In replay mode `start.ts` skips `.env` loading so a stray key cannot trigger a live call.
### Two goldens: normalize, then snapshot
@@ -55,18 +55,18 @@ A snapshot run asserts **two** normalized goldens, because the harness's externa
1. The **stdout transcript** — the framed `session/update` JSON-RPC the editor sees. Catches regressions in the ACP bridge's event→update translation (`streamSessionEventUpdate`).
2. The **re-derived session JSONL** — the log the replay run itself persists, compared against the recorded fixture. Catches regressions in the loop, tool dispatch, and turn/step structure that never surface on stdout.
The two are genuinely additive: stdout is the bridge's *lossy projection* of the log (it drops `usage`, `step/*`, exact `seq`/`time`, and renders tool I/O differently), so a loop/tool/turn-structure regression can change the JSONL while leaving the stdout projection identical, and a bridge-translation regression can change stdout while the JSONL is untouched. Asserting the JSONL equality also echoes the proposed [universal replay fixture](../proposed/2026-06-11-deterministic-and-stress-testing.md) idea.
The two are genuinely additive: stdout is the bridge's *lossy projection* of the log (it drops `usage`, `step/*`, exact `seq`/`time`, and renders tool I/O differently), so a loop/tool/turn-structure regression can change the JSONL while leaving the stdout projection identical, and a bridge-translation regression can change stdout while the JSONL is untouched. Asserting the JSONL equality also echoes the proposed [universal replay fixture](../../proposed/testing/2026-06-11-deterministic-and-stress-testing.md) idea.
Both surfaces contain non-deterministic values that a pure normalization function scrubs **before** the snapshot: `randomUUID()` session ids → `{{sessionId}}`, the temp `mkdtemp` cwd → `{{cwd}}` (it appears in terminal-card `_meta` and the log header), JSON-RPC ids → a stable sequence, and the log's per-event `time` (epoch ms) + header `createdAt` dropped or zeroed (the log's `seq` is left intact — it is deterministic by contract, `seq = log.length`). Real bash runs during replay, so the JSONL normalizer additionally stabilizes tool-output volatility (any embedded paths/pids/timestamps) — scenarios keep bash commands tightly constrained (`echo`, file writes; no `date`/`env`/background/large-output) so this surface is small. The goldens are themselves **JSONL** — one compact, normalized record per line, in the same shape as the surfaces they mirror (NDJSON on the wire, JSONL on disk: `stdout.golden.jsonl`, `session.golden.jsonl`), so they stay `grep`/`jq`-able and faithful to what the agent actually emits. A separate raw-purity assertion keeps the guarantee that every stdout line parses as JSON (no logger leak onto the protocol channel). Vitest's `toMatchFileSnapshot` provides the golden store and the `-u`/`--update` "accept the diff" workflow.
### Isolation: normalization now, sandbox later
Determinism of the tool environment comes from a per-test `mkdtemp` cwd, the executor's existing secret-scrubbing env (`/KEY|SECRET|TOKEN/i`), the fresh non-login `bash -c` per call, and the normalization pass — **not** from an OS sandbox. A real rootless sandbox (bwrap on Linux, sandbox-exec/Seatbelt on macOS) is the established cross-platform pattern (Claude Code, Codex), but it is per-OS, fragile on newer kernels (Ubuntu 24.04+ AppArmor blocks unprivileged user namespaces), and unnecessary for transcript determinism. It is reserved as a future tier via the documented `BashExecutor` capability seam ([a sandboxing executor replaces dsh-bash-local without touching a tool schema](2026-06-13-capability-seams.md)) — a new `bash-*` package, not a change here. Scenarios keep bash commands tightly constrained (no `date`/`env`/background/large-output) so the temp-dir tier suffices.
Determinism of the tool environment comes from a per-test `mkdtemp` cwd, the executor's existing secret-scrubbing env (`/KEY|SECRET|TOKEN/i`), the fresh non-login `bash -c` per call, and the normalization pass — **not** from an OS sandbox. A real rootless sandbox (bwrap on Linux, sandbox-exec/Seatbelt on macOS) is the established cross-platform pattern (Claude Code, Codex), but it is per-OS, fragile on newer kernels (Ubuntu 24.04+ AppArmor blocks unprivileged user namespaces), and unnecessary for transcript determinism. It is reserved as a future tier via the documented `BashExecutor` capability seam ([a sandboxing executor replaces dsh-bash-local without touching a tool schema](../architecture/2026-06-13-capability-seams.md)) — a new `bash-*` package, not a change here. Scenarios keep bash commands tightly constrained (no `date`/`env`/background/large-output) so the temp-dir tier suffices.
### The replay plugin is its own package
The replay plugin lives in its own package, `@deepseek-ai/dsh-llm-replay` (`packages/llm-replay/`), and the snapshot config references it by package name. It is the keyless replacement for the real LLM adapter: it installs an `llm/stream` waterfall listener and short-circuits it, serving model streams reconstructed from a recorded session JSONL. Its sole consumer is the ACP snapshot harness here, but it is a package (not example-local glue like echo-agent's [mock-llm.ts](../../../examples/echo-agent/src/mock-llm.ts)) so that its derive/parse/replay branches fall under the per-file 100% coverage gate on package `src` trees — logic under `examples/` is not measured by that gate, which would leave those branches unguarded.
The replay plugin lives in its own package, `@deepseek-ai/dsh-llm-replay` (`packages/llm-replay/`), and the snapshot config references it by package name. It is the keyless replacement for the real LLM adapter: it installs an `llm/stream` waterfall listener and short-circuits it, serving model streams reconstructed from a recorded session JSONL. Its sole consumer is the ACP snapshot harness here, but it is a package (not example-local glue like echo-agent's [mock-llm.ts](../../../../examples/echo-agent/src/mock-llm.ts)) so that its derive/parse/replay branches fall under the per-file 100% coverage gate on package `src` trees — logic under `examples/` is not measured by that gate, which would leave those branches unguarded.
### Two subcommands, replay in the default gate
@@ -76,4 +76,4 @@ The replay plugin lives in its own package, `@deepseek-ai/dsh-llm-replay` (`pack
A new test tier and its fixtures to maintain: each scenario is a directory of `input.json` (the client stdin script) + `session.jsonl` (the recorded log) + an optional `replay.override.json` + an optional `workspace/` seed dir + the two `*.golden.jsonl` files, committed and reviewed. A scenario that needs the agent to operate on existing files (read, edit, grep) ships a `<scenario>/workspace/` directory; the harness copies its contents into the temp cwd before the run, so the seeded files are present for both record and replay (the cwd is normalized in the goldens, so the seeded paths stay stable). Re-recording when the model's phrasing changes churns the goldens — visible in review, which is the point of committing them. Bought: deterministic, keyless, full-transcript regression coverage that boots the real Loader (so it still guards the export-shape bug class), exercises the real bash executor, and gives a one-command accept-the-diff loop. The tier is ACP-first but the harness (subprocess + tee + input-DSL + workspace seeding + normalization + JSONL-derived replay) is example-agnostic and extends to other examples.
This RFC relates to but does not supersede the [proposed determinism RFC](../proposed/2026-06-11-deterministic-and-stress-testing.md): that proposal's "universal replay fixture" re-derives session *message history* after every test (an internal-consistency invariant), whereas snapshot tests pin the *external protocol output*. They are complementary — one guards the event-sourcing invariant, the other guards the editor-facing contract.
This RFC relates to but does not supersede the [proposed determinism RFC](../../proposed/testing/2026-06-11-deterministic-and-stress-testing.md): that proposal's "universal replay fixture" re-derives session *message history* after every test (an internal-consistency invariant), whereas snapshot tests pin the *external protocol output*. They are complementary — one guards the event-sourcing invariant, the other guards the editor-facing contract.

View File

@@ -6,15 +6,15 @@ Status: implemented (accepted 2026-06-19)
## Context
The harness leans hard on real-API tests by policy: AGENTS.md § Secrets argues that a no-key suite proves the plumbing but not the product, and the [ACP inject postmortem](../../postmortem/0001-acp-default-export-drops-inject.md) is the standing proof — 178 keyless tests stayed green while a real editor session crashed instantly. The real-API e2e suite (`pnpm run test:e2e`, the `*.e2e.ts` files) exists precisely to close that gap: it drives the agent against the live DeepSeek API — real model calls, real bash tools, multi-turn, resume, ACP-over-stdio.
The harness leans hard on real-API tests by policy: AGENTS.md § Secrets argues that a no-key suite proves the plumbing but not the product, and the [ACP inject postmortem](../../../postmortem/0001-acp-default-export-drops-inject.md) is the standing proof — 178 keyless tests stayed green while a real editor session crashed instantly. The real-API e2e suite (`pnpm run test:e2e`, the `*.e2e.ts` files) exists precisely to close that gap: it drives the agent against the live DeepSeek API — real model calls, real bash tools, multi-turn, resume, ACP-over-stdio.
But until this change **nothing in CI ran it**. The default gate ([.github/workflows/ci.yml](../../../.github/workflows/ci.yml)) is deliberately keyless — it carries no secret, runs on every push and PR including from forks, and stays green for any contributor. `test:e2e` self-skips without a key (`describe.skipIf(!process.env.DEEPSEEK_API_KEY)`), so even if ci.yml invoked it, a keyless runner would skip it green. The real-API safety net therefore only fired when a developer happened to run it locally with a key in their environment — i.e. unreliably, and never as a merge gate.
But until this change **nothing in CI ran it**. The default gate ([.github/workflows/ci.yml](../../../../.github/workflows/ci.yml)) is deliberately keyless — it carries no secret, runs on every push and PR including from forks, and stays green for any contributor. `test:e2e` self-skips without a key (`describe.skipIf(!process.env.DEEPSEEK_API_KEY)`), so even if ci.yml invoked it, a keyless runner would skip it green. The real-API safety net therefore only fired when a developer happened to run it locally with a key in their environment — i.e. unreliably, and never as a merge gate.
This RFC records the decision to add a **second, secret-consuming workflow** that runs the real-API suite in CI, and — because introducing the first CI secret into a repo that may later go public is a security/isolation decision — the threat model it relies on and what changes when the repo becomes public.
## Decision
Add a dedicated workflow, [.github/workflows/e2e.yml](../../../.github/workflows/e2e.yml), separate from ci.yml. It runs only `pnpm run test:e2e` against the external API using a repo secret, on trusted events, with a preflight that converts a missing secret into a loud failure instead of a false green. ci.yml is left untouched.
Add a dedicated workflow, [.github/workflows/e2e.yml](../../../../.github/workflows/e2e.yml), separate from ci.yml. It runs only `pnpm run test:e2e` against the external API using a repo secret, on trusted events, with a preflight that converts a missing secret into a loud failure instead of a false green. ci.yml is left untouched.
### A separate workflow, not a job in ci.yml
@@ -51,7 +51,7 @@ The repo secret is named `DEEPSEEK_API_KEY_EXTERNAL`; it is mapped to the `DEEPS
- **Step-scoped secret.** `DEEPSEEK_API_KEY` is set in the `env:` of only the preflight and e2e steps, never job-level — so checkout/setup-node/install never see it. A compromised install-time lifecycle script in a dependency cannot read a secret that isn't in its environment.
- **`permissions: contents: read`.** The job only reads the repo to run tests; it needs no write scopes (no PR comments, no status writes), so the `GITHUB_TOKEN` is dropped to least privilege.
- **`DEEPSEEK_BASE_URL` pinned** to `https://api.deepseek.com` on the e2e step. The adapter would default to this when unset ([packages/llm-deepseek/src/index.ts](../../../packages/llm-deepseek/src/index.ts) `PUBLIC_BASE_URL`), but pinning is self-documenting and hermetic — a stray repo-root `.env` (which `vitest.e2e.config.ts` loads if present) cannot silently redirect the run to another endpoint.
- **`DEEPSEEK_BASE_URL` pinned** to `https://api.deepseek.com` on the e2e step. The adapter would default to this when unset ([packages/llm-deepseek/src/index.ts](../../../../packages/llm-deepseek/src/index.ts) `PUBLIC_BASE_URL`), but pinning is self-documenting and hermetic — a stray repo-root `.env` (which `vitest.e2e.config.ts` loads if present) cannot silently redirect the run to another endpoint.
- **No secret echoed.** The preflight prints only `DEEPSEEK_API_KEY present.` — not the value, not its length. (An earlier draft echoed `${#KEY}`; dropped as needless metadata.)
### Scope, runtime shape