diff --git a/.agents/skills/dsh-code-review/SKILL.md b/.agents/skills/dsh-code-review/SKILL.md index d97bb1bd77..54510133db 100644 --- a/.agents/skills/dsh-code-review/SKILL.md +++ b/.agents/skills/dsh-code-review/SKILL.md @@ -7,7 +7,7 @@ description: Use when reviewing a pull request in the deepseek-harness repo — **This skill is guidance, not a complete checklist.** It is a where-to-look map that lowers your startup cost on an unfamiliar PR — clearing every item here does not mean the PR is good. You are the reviewer: reason independently from the code in front of you, and think broadly across every dimension a change can fail on. The items below are the failure modes this repo has already paid for; a real review also catches the ones nobody has written down yet. -Independent judgment governs *what to look at* and *how to apply a rule to this case* — not whether the repo's documented requirements still hold. AGENTS.md, packages/AGENTS.md, and the [quality gates](../../../docs/rfc/implemented/2026-06-11-quality-gates.md) remain authoritative; a missing HMR-safety test or out-of-sync docs is a blocking gap regardless of your judgment, not a suggestion you can waive. Use your own reasoning to go *beyond* these checks and to weigh genuine edge cases against an RFC (raise it as a discussion, don't silently override) — never to demote a documented blocker to optional. +Independent judgment governs *what to look at* and *how to apply a rule to this case* — not whether the repo's documented requirements still hold. AGENTS.md, packages/AGENTS.md, and the [quality gates](../../../docs/rfc/implemented/process/2026-06-11-quality-gates.md) remain authoritative; a missing HMR-safety test or out-of-sync docs is a blocking gap regardless of your judgment, not a suggestion you can waive. Use your own reasoning to go *beyond* these checks and to weigh genuine edge cases against an RFC (raise it as a discussion, don't silently override) — never to demote a documented blocker to optional. ## How to think about a review @@ -25,7 +25,7 @@ These define the conventions and gates this repo is checked against, and they ar - **AGENTS.md § Defensive patterns (hard-won)** — each bullet is a bug class that bit us. Reviewing anything touching process lifecycle, async/await, disposal, or adapter error paths? Re-read this first — then look for the *adjacent* mistake it doesn't name. - **AGENTS.md § Type Safety and Documentation** — the doc-sync rule (code change ⇒ update README + JSDoc in the SAME commit) and the no-hard-wrap markdown convention. - **[packages/AGENTS.md](../../../packages/AGENTS.md)** — per-package conventions (file layout, the HMR-safety test requirement). -- **[RFC index](../../../docs/rfc/README.md)** — the *why* behind the architecture. Especially [quality gates](../../../docs/rfc/implemented/2026-06-11-quality-gates.md) (what a PR must pass) and [capability seams](../../../docs/rfc/implemented/2026-06-13-capability-seams.md) (the three-package split). If a change seems to fight an RFC, that's a discussion, not a silent override — and not an automatic veto either: an RFC can be wrong for this case, so reason about it. +- **[RFC index](../../../docs/rfc/README.md)** — the *why* behind the architecture. Especially [quality gates](../../../docs/rfc/implemented/process/2026-06-11-quality-gates.md) (what a PR must pass) and [capability seams](../../../docs/rfc/implemented/architecture/2026-06-13-capability-seams.md) (the three-package split). If a change seems to fight an RFC, that's a discussion, not a silent override — and not an automatic veto either: an RFC can be wrong for this case, so reason about it. ## Hard blockers (documented requirements — missing one blocks merge) @@ -44,7 +44,7 @@ Where your independent reasoning earns its keep. Start here, then keep going acr - **Plugin export shape + real-loader coverage.** A new/changed `cordis.yml`-loaded plugin: is it a function/namespace plugin (`name`/`inject`/`Config`/`apply` named exports) with NO `export default`? A stray default export makes the Loader's `unwrapExports` drop `inject` and the plugin crashes at load with `cannot get property … without inject` — invisible to hand-built `ctx.plugin({...})` tests and to line coverage. Confirm there's a test driving it through the REAL loader path (the no-key subprocess e2e for ACP is the model). And any opportunistic read of a service NOT in `static inject` should use `ctx.get(name)`, not `ctx.` (the property proxy throws through a foreign shadow). See [packages/AGENTS.md](../../../packages/AGENTS.md) and [docs/postmortem/0001](../../../docs/postmortem/0001-acp-default-export-drops-inject.md). - **Seam discipline.** New swappable capability? Check it's split per the capability-seams RFC (interface / impl / consumer), and that the consumer injects the interface key, never an implementation type. - **Test quality — sufficiency, not just coverage.** 100% per-file coverage and a green suite are necessary, not sufficient: they prove the lines *ran*, not that the feature *works the way it ships*. Judge whether the tests are sufficient on two axes. (1) **Would they fail if the behavior regressed?** A test that passes but asserts the wrong thing — or restates the implementation instead of the contract (events fired, disposal reached, the world changed) — is worse than none. (2) **Do they exercise the REAL thing, the way it's actually used?** Prefer the genuine collaborator over a fake, drive the change through its real entry path (the cordis Loader, the ACP bridge, a booted subprocess — not a hand-built `ctx.plugin({...})` that bypasses `unwrapExports`), and verify the WORLD (re-read the file/log/registry externally), not the agent's self-report. A test that fakes the inputs just enough to cover every line will agree with whatever the author assumed; the real thing won't. When a test sets up a *clean/happy* path to reach a line, ask whether the line's PURPOSE is exercised — e.g. a durability/teardown path "tested" by a fully-completed turn never proves the mid-flight teardown it exists for; a torn-tail recovery branch covered by a well-formed log never proves recovery. Flag tests that hit the line but not the scenario. See AGENTS.md § Defensive patterns "Line coverage is not behavior coverage" and "Prefer the REAL implementation over a mock/stand-in in tests". -- **Snapshot coverage for transcript/UX changes.** If the PR changes the editor-facing transcript or end-to-end agent UX — the ACP bridge's event→update translation, the agent loop's observable output, tool presentation, or anything an editor renders — it must add or update a snapshot scenario (`examples/*/tests/**/*.snapshot.ts`, goldens under `examples/acp-agent/tests/snapshots/`) or note explicitly why none applies (AGENTS.md § Conventions). Review the golden diff itself: a changed `stdout.golden.txt` / `session.golden.txt` is a behavior change in disguise — confirm it's intended, not an accidental regression someone re-recorded away. A pure internal refactor with no observable-output change is exempt, but the PR should say so. See [docs/rfc/implemented/2026-06-19-acp-snapshot-tests.md](../../../docs/rfc/implemented/2026-06-19-acp-snapshot-tests.md). +- **Snapshot coverage for transcript/UX changes.** If the PR changes the editor-facing transcript or end-to-end agent UX — the ACP bridge's event→update translation, the agent loop's observable output, tool presentation, or anything an editor renders — it must add or update a snapshot scenario (`examples/*/tests/**/*.snapshot.ts`, goldens under `examples/acp-agent/tests/snapshots/`) or note explicitly why none applies (AGENTS.md § Conventions). Review the golden diff itself: a changed `stdout.golden.txt` / `session.golden.txt` is a behavior change in disguise — confirm it's intended, not an accidental regression someone re-recorded away. A pure internal refactor with no observable-output change is exempt, but the PR should say so. See [docs/rfc/implemented/testing/2026-06-19-acp-snapshot-tests.md](../../../docs/rfc/implemented/testing/2026-06-19-acp-snapshot-tests.md). - **Intent and contracts.** Does the change do what the PR says, and honor the documented contract on *both* sides of every seam it touches (see AGENTS.md "Honor cross-seam contracts on BOTH sides")? ## How to respond diff --git a/.agents/skills/dsh-find-simplifications/SKILL.md b/.agents/skills/dsh-find-simplifications/SKILL.md index ac8565e210..2dcea37c66 100644 --- a/.agents/skills/dsh-find-simplifications/SKILL.md +++ b/.agents/skills/dsh-find-simplifications/SKILL.md @@ -11,7 +11,7 @@ This skill helps turn a broad "find things to simplify" request into evidence-ba - Read `AGENTS.md`, especially the pre-release stance, tests-document-behavior section, conventions, defensive patterns, and Type Safety and Documentation section. - Skim [docs/architecture.md](../../../docs/architecture.md) before judging anything under `packages/`; simplifications that fight the service map or event taxonomy need extra evidence. -- Use the RFC index ([docs/rfc/README.md](../../../docs/rfc/README.md)) to understand intentional architecture. The most relevant implemented examples are [drop mutable session summary](../../../docs/rfc/implemented/2026-06-19-drop-mutable-session-summary.md), [shared persistence write coordinator](../../../docs/rfc/implemented/2026-06-18-shared-persistence-write-coordinator.md), [capability seams](../../../docs/rfc/implemented/2026-06-13-capability-seams.md), and the twin adapter / dual persistence backend RFCs. +- Use the RFC index ([docs/rfc/README.md](../../../docs/rfc/README.md)) to understand intentional architecture. The most relevant implemented examples are [drop mutable session summary](../../../docs/rfc/implemented/simplification/2026-06-19-drop-mutable-session-summary.md), [shared persistence write coordinator](../../../docs/rfc/implemented/architecture/2026-06-18-shared-persistence-write-coordinator.md), [capability seams](../../../docs/rfc/implemented/architecture/2026-06-13-capability-seams.md), and the twin adapter / dual persistence backend RFCs. - Treat dual LLM adapters and dual persistence backends as intentional by default. Do not propose deleting either twin/backend as "low effort" unless the user explicitly overrides that constraint. Removing an unused method or hook inside a protected seam can still be valid if it does not collapse the protected design. ## What Counts As A Strong Candidate diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index 2de3a6cea6..7a8c9dc88c 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -23,7 +23,7 @@ name: E2E (real DeepSeek API) # in the BASE repo's context WITH secrets while still able to check out untrusted # fork code — a textbook key-leak vector, especially once this repo is public. # The fork/secret model and its public-repo implications are recorded in -# docs/rfc/implemented/2026-06-19-real-api-e2e-ci.md. +# docs/rfc/implemented/testing/2026-06-19-real-api-e2e-ci.md. # # Note: scheduled triggers are auto-disabled after 60 days of repo inactivity; # push/pull_request/workflow_dispatch act as backstops. diff --git a/AGENTS.md b/AGENTS.md index 8b8319ade0..1c3bc64e30 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -14,7 +14,7 @@ A passing test pins the behavior the code **currently** has — not necessarily Before you preserve a behavior solely to keep a test green, ask: is this behavior load-bearing (a real consumer depends on it, a contract promises it, a user observes it), or is it an artifact? If it's an artifact, **change the behavior AND its test together, in the same change, and say why in the PR** — do not contort new code to keep an obsolete assertion passing, and do not treat "but the test expects X" as a reason X must stay. Conversely, do not delete a test just because it is inconvenient: the discipline cuts both ways — you must show the *behavior* is dead, not merely that the test is in your way. -The worked example is [Drop the mutable session summary](docs/rfc/implemented/2026-06-19-drop-mutable-session-summary.md): an entire `SessionSummary` type, a `SessionPersistence.update()` method, a JSONL sidecar, and SQLite columns existed and were exercised by their own contract test — yet **nothing in production CONSUMED any of it, and `update()` had no production caller**. (The backends did *write* summary state — JSONL touched the sidecar after a durable append, SQLite bumped `updated_at` in the append transaction — but those writes fed only reads that nothing performed.) The tests documented the behavior perfectly; the behavior was dead. Deleting the behavior and its tests together removed ~400 lines and erased a durability divergence the next refactor would have had to model. (This is the test-tier echo of "verify the world, not a synthetic stand-in" in § Defensive patterns: a test agrees with whatever it was written to assert; only a real consumer proves the behavior matters.) +The worked example is [Drop the mutable session summary](docs/rfc/implemented/simplification/2026-06-19-drop-mutable-session-summary.md): an entire `SessionSummary` type, a `SessionPersistence.update()` method, a JSONL sidecar, and SQLite columns existed and were exercised by their own contract test — yet **nothing in production CONSUMED any of it, and `update()` had no production caller**. (The backends did *write* summary state — JSONL touched the sidecar after a durable append, SQLite bumped `updated_at` in the append transaction — but those writes fed only reads that nothing performed.) The tests documented the behavior perfectly; the behavior was dead. Deleting the behavior and its tests together removed ~400 lines and erased a durability divergence the next refactor would have had to model. (This is the test-tier echo of "verify the world, not a synthetic stand-in" in § Defensive patterns: a test agrees with whatever it was written to assert; only a real consumer proves the behavior matters.) ## Architecture @@ -65,8 +65,10 @@ examples/ Runnable demos (not workspaces; see examples/AGENTS.md). echo-agent docs/ architecture.md — the design doc. module-graph.md — generated inter-package dependency graph (Mermaid; `pnpm run gen-module-graph`). rfc/ — design decisions and proposals, one kind of doc grouped by - lifecycle into proposed/ implemented/ rejected/ (the why behind - vendoring, event-sourcing, the schema DSL, …). See rfc/README.md. + lifecycle (proposed/ implemented/ rejected/) then by class + (feature/ bug-fix/ simplification/ architecture/ process/ testing/); + the why behind vendoring, event-sourcing, the schema DSL, …. See + rfc/README.md. postmortem/ — incident write-ups: a bug that escaped to a user/merge/release, why the safety nets missed it, the guardrails added. cookbook/ — step-by-step guides: adding a package, a tool, @@ -109,7 +111,12 @@ pnpm run gen-cordis-catalog # regenerate docs/cordis-catalog/events-and-service pnpm run verify-cordis-catalog # assert that generated catalog is not stale pnpm run verify-md-wrap # assert no hard-wrapped prose paragraphs in README.md, # docs/**/*.md, packages/*/*.md, AGENTS.md (one line per paragraph) -pnpm run doc-sync # doc-typecheck + verify-cordis-catalog + verify-md-wrap + verify-md-links + verify-type-equiv (CI runs this) +pnpm run verify-doc-refs # assert every docs/*.md path cited in a packages|examples + # TypeScript comment resolves (catches a moved/renamed doc) +pnpm run verify-rfc-classification # assert every RFC lives in a valid + # {lifecycle}/{class}/ folder and docs/rfc/README.md lists it + # under the matching heading (closed class set + index completeness) +pnpm run doc-sync # doc-typecheck + verify-cordis-catalog + verify-md-wrap + verify-md-links + verify-doc-refs + verify-rfc-classification + verify-type-equiv (CI runs this) pnpm run demo:echo # run examples/echo-agent (no API key; type "echo hi" to # see a tool call) — the mock skeleton pnpm run demo:coding # run examples/coding-agent — the real agent (needs @@ -153,7 +160,7 @@ Dev/test/demo run **unbuilt** via tsx + the `paths` map in the root `tsconfig.js - **TODO markers**: use `FIXME`/`TODO`/`XXX` to flag known issues by urgency — see [docs/development.md](docs/development.md) for the semantics of each. - **Tests**: vitest, colocated under `packages//tests/*.spec.ts`. Every registry needs an HMR-safety test (dispose the contributing fiber, assert cleanup). **Excessive tests are welcome** — when in doubt, write the test; err on the side of covering edge cases, error paths, event ordering, and concurrency races even if they seem unlikely. Review findings get regression tests (see `packages/agent-loop/tests/review-fixes.spec.ts`). The same generosity applies to **real-API (with-key) e2e tests — inference is cheap here (we are DeepSeek), so do not ration them**: cover the agent's real flows (a real prompt that writes a file, multi-turn, tool use, cancellation) and run them frequently while developing, especially cheap **smoke tests** that boot the real example and check the world. A green mock/no-key suite proves the plumbing, not the product — the with-key smoke test is what catches "green units, broken product". See § Secrets / .env for the with-key policy and why self-skip is a CI accommodation, not a verdict that real-API tests are expensive. - **Prefer the REAL implementation over a mock/stand-in in tests.** When the genuine collaborator is available in the repo, wire it up instead of hand-rolling a fake — a test that registers an inline `defineTool({ name: 'bash', … })` to stand in for `dsh-tool-bash` proves the *bridge* moves bytes but not that the *shipping tool* renders the way the test asserts; the two drift and the test passes while the product is wrong. Mock only the genuinely expensive/non-deterministic boundary (the LLM adapter, the network, the clock) and keep everything downstream real: a bridge tool-call test runs the scripted mock MODEL but the REAL tool + REAL executor (e.g. `makeBridgeHarness({ withBash: true })` plugs `dsh-bash-local` + `dsh-tool-bash` and runs an actual `echo`), so it verifies the actual `presentCall`/`presentResult` an editor sees. This is the unit-test echo of "verify the world, not a synthetic stand-in" (see § Defensive patterns) — a fake you wrote will agree with whatever you assumed; the real thing won't. -- **A change that affects the editor-facing transcript or end-to-end agent UX needs a snapshot test (or an explicit note in the PR why none applies).** The snapshot tier (`examples/*/tests/**/*.snapshot.ts`, `pnpm run test:snapshot`) boots the real example subprocess, replays a recorded session JSONL deterministically (keyless), and diffs the normalized stdout transcript + re-persisted session log against committed goldens — the full-transcript regression net that mock-level unit tests structurally cannot be (it is what catches a bridge-translation or loop-structure regression that leaves every unit green). When you change the ACP bridge, the agent loop's observable output, tool presentation, or anything an editor renders, add or update a scenario under `examples/acp-agent/tests/snapshots/` and re-record with `pnpm run test:snapshot:record`. Reviewing the golden diff is part of the review. The rule is scoped to transcript/UX-affecting changes — a pure internal refactor with no observable-output change does not need one, but say so. See [docs/rfc/implemented/2026-06-19-acp-snapshot-tests.md](docs/rfc/implemented/2026-06-19-acp-snapshot-tests.md). +- **A change that affects the editor-facing transcript or end-to-end agent UX needs a snapshot test (or an explicit note in the PR why none applies).** The snapshot tier (`examples/*/tests/**/*.snapshot.ts`, `pnpm run test:snapshot`) boots the real example subprocess, replays a recorded session JSONL deterministically (keyless), and diffs the normalized stdout transcript + re-persisted session log against committed goldens — the full-transcript regression net that mock-level unit tests structurally cannot be (it is what catches a bridge-translation or loop-structure regression that leaves every unit green). When you change the ACP bridge, the agent loop's observable output, tool presentation, or anything an editor renders, add or update a scenario under `examples/acp-agent/tests/snapshots/` and re-record with `pnpm run test:snapshot:record`. Reviewing the golden diff is part of the review. The rule is scoped to transcript/UX-affecting changes — a pure internal refactor with no observable-output change does not need one, but say so. See [docs/rfc/implemented/testing/2026-06-19-acp-snapshot-tests.md](docs/rfc/implemented/testing/2026-06-19-acp-snapshot-tests.md). ## Defensive patterns (hard-won) @@ -175,7 +182,7 @@ This codebase aims to be **very type-safe and well documented** for maintainabil In the **core** packages (`packages/llm`, `packages/tools`, `packages/agent`, `packages/agent-loop`, `packages/session`, `packages/system-prompt`), **type gymnastics are acceptable when they improve the DX of plugin authors** for common plugin types. The `defineTool` typed schema DSL in `dsh-tools` is the canonical example: the `SchemaSpec` to `InferArgs` type-level mapping gives tool authors zero-cast typed `execute` args, and the cost of the conditional types stays inside the core package. -Verbose documentation is fine **as long as docs and code stay strictly in sync**. Out-of-sync docs are worse than no docs. **When you change code, update its docs in the SAME change** — grep the package README and the module/JSDoc comments for the old behavior (config keys, defaults, error codes, wire field names, event names) and fix every hit. CI runs `pnpm run doc-sync` (`doc-typecheck` + `verify-cordis-catalog` + `verify-md-wrap` + `verify-md-links` + `verify-type-equiv`), which typechecks every fenced `ts` block in `README.md`, `docs/**/*.md`, and `packages/*/*.md`, regenerates the cordis events/services catalog from source and fails if the committed copy is stale, asserts no hard-wrapped prose paragraphs, checks that every relative Markdown cross-link resolves, and checks that every ` ```ts type-equiv ` doc block still matches its source type — across those files plus `AGENTS.md` / `packages/AGENTS.md` — but that scope does NOT catch prose drift in `AGENTS.md` / `packages/AGENTS.md` / `packages/README.md` (config keys, defaults, error codes), so keeping those in sync remains on the author. Every module has a module-level doc comment explaining its role. Every exported class, interface, type, function, and non-obvious method has a JSDoc that explains semantics (not just the name) — contracts (what events fire when), disposal behavior, error behavior, and extension intent. Internal helpers get docs only where non-obvious. Prefer one-liners when one line suffices. +Verbose documentation is fine **as long as docs and code stay strictly in sync**. Out-of-sync docs are worse than no docs. **When you change code, update its docs in the SAME change** — grep the package README and the module/JSDoc comments for the old behavior (config keys, defaults, error codes, wire field names, event names) and fix every hit. CI runs `pnpm run doc-sync` (`doc-typecheck` + `verify-cordis-catalog` + `verify-md-wrap` + `verify-md-links` + `verify-doc-refs` + `verify-rfc-classification` + `verify-type-equiv`), which typechecks every fenced `ts` block in `README.md`, `docs/**/*.md`, and `packages/*/*.md`, regenerates the cordis events/services catalog from source and fails if the committed copy is stale, asserts no hard-wrapped prose paragraphs, checks that every relative Markdown cross-link resolves, checks that every `docs/*.md` path cited in a source comment resolves, checks that every RFC is filed under a valid class folder and listed in its index, and checks that every ` ```ts type-equiv ` doc block still matches its source type — across those files plus `AGENTS.md` / `packages/AGENTS.md` — but that scope does NOT catch prose drift in `AGENTS.md` / `packages/AGENTS.md` / `packages/README.md` (config keys, defaults, error codes), so keeping those in sync remains on the author. Every module has a module-level doc comment explaining its role. Every exported class, interface, type, function, and non-obvious method has a JSDoc that explains semantics (not just the name) — contracts (what events fire when), disposal behavior, error behavior, and extension intent. Internal helpers get docs only where non-obvious. Prefer one-liners when one line suffices. **Tag every new event with `@mode`.** The cordis events/services catalog ([docs/cordis-catalog/events-and-services.md](docs/cordis-catalog/events-and-services.md)) is GENERATED from source by `scripts/gen-cordis-catalog.ts` — never hand-edit it; run `pnpm run gen-cordis-catalog` and commit the result. When you add an event to an `interface Events` block, its JSDoc MUST carry a `@mode emit|waterfall|parallel` tag (the generator hard-errors without it): use `waterfall` when the signature ends with a `next: () => …` parameter (the listener transforms or vetoes via `next()`), `parallel` when the loop awaits a fan-out with no veto (e.g. an awaited `Promise | void` checkpoint like `session/flush`), and `emit` for plain fire-and-forget notifications. The generator also cross-checks the tag against the signature where the shape is conclusive (a trailing `next` ⇒ waterfall) and hard-errors on a contradiction. Write the rest of the event's JSDoc to stand alone — it is the catalog entry's prose. diff --git a/docs/AGENTS.md b/docs/AGENTS.md index 6d23352bff..7571209aa2 100644 --- a/docs/AGENTS.md +++ b/docs/AGENTS.md @@ -4,12 +4,12 @@ Conventions for authoring everything under `docs/` (architecture, RFCs, cookbook ## Cross-reference with machine-checkable links, never free prose -When one doc refers to another doc, an RFC, a package README, or any file in the repo, link it with a **relative Markdown link** to the actual path — `[capability seams](rfc/implemented/2026-06-13-capability-seams.md)`, `[architecture.md](architecture.md)`. Do NOT refer to it by bare prose or by a number ("see ADR 0009", "per RFC 005"): a number is not checkable, goes stale the moment a file is renamed, and forces the reader to go hunting. A relative link is verified mechanically — `pnpm run verify-md-links` (part of `doc-sync`, see [the cross-link lint RFC](rfc/implemented/2026-06-18-markdown-cross-link-lint.md)) fails CI and the pre-push hook if any relative target does not exist, so a rename that orphans a link is caught before review rather than rotting silently. +When one doc refers to another doc, an RFC, a package README, or any file in the repo, link it with a **relative Markdown link** to the actual path — `[capability seams](rfc/implemented/architecture/2026-06-13-capability-seams.md)`, `[architecture.md](architecture.md)`. Do NOT refer to it by bare prose or by a number ("see ADR 0009", "per RFC 005"): a number is not checkable, goes stale the moment a file is renamed, and forces the reader to go hunting. A relative link is verified mechanically — `pnpm run verify-md-links` (part of `doc-sync`, see [the cross-link lint RFC](rfc/implemented/process/2026-06-18-markdown-cross-link-lint.md)) fails CI and the pre-push hook if any relative target does not exist, so a rename that orphans a link is caught before review rather than rotting silently. -This is why the RFC tree carries no stable numbers: files are named `yyyy-mm-dd-topic-title.md` and referred to by link, so they survive moves between `proposed/`/`implemented/`/`rejected/` without a dangling reference. When you move or rename a doc, the gate tells you every inbound link you still need to fix. +This is why the RFC tree carries no stable numbers: files are named `yyyy-mm-dd-topic-title.md` and referred to by link, so they survive moves between lifecycle folders (`proposed/`/`implemented/`/`rejected/`) and class folders without a dangling reference. When you move or rename a doc, the gate tells you every inbound link you still need to fix. The gate checks file *existence*, not `#anchor` validity — a link to a real file with a wrong heading fragment still passes. Prefer linking the file (and a heading when it helps the reader), but don't rely on the gate to catch a stale anchor. ## RFCs -Design decisions and proposals live in [rfc/](rfc/) — one kind of doc, grouped by lifecycle into `proposed/`/`implemented/`/`rejected/`. See [rfc/README.md](rfc/README.md) for the naming scheme and when to write one. +Design decisions and proposals live in [rfc/](rfc/) — one kind of doc, grouped by lifecycle (`proposed/`/`implemented/`/`rejected/`) then by class (`feature`/`bug-fix`/`simplification`/`architecture`/`process`/`testing`). See [rfc/README.md](rfc/README.md) for the class definitions, the naming scheme, and when to write one. diff --git a/docs/architecture.md b/docs/architecture.md index e4b9014904..2bc782cd43 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -111,7 +111,7 @@ Tool schemas are deliberately **part of the assembly**: "what the model is told - `send(content)` — queued message; starts a turn when idle, else next turn - `steer(content)` — mid-turn injection, drained **between steps**; behaves like `send` when idle -- `inject(content)` — in-session context (`context/message` event); the next request sees it (Claude Code attachment / system-reminder analog). An inject made while the agent is *running* joins the open turn; an inject while *idle* is wrapped in a one-shot turn (`turn/start{trigger:injection}` → `context/message` → `turn/end`) so every event stays turn-enclosed (see [the turn-enclosure invariant](rfc/implemented/2026-06-15-turn-enclosure-invariant.md)). +- `inject(content)` — in-session context (`context/message` event); the next request sees it (Claude Code attachment / system-reminder analog). An inject made while the agent is *running* joins the open turn; an inject while *idle* is wrapped in a one-shot turn (`turn/start{trigger:injection}` → `context/message` → `turn/end`) so every event stays turn-enclosed (see [the turn-enclosure invariant](rfc/implemented/architecture/2026-06-15-turn-enclosure-invariant.md)). - `abort(reason)` — aborts the in-flight step via `AbortSignal` - `cancel(reason)` — the broad cancel: clears queued + steering work, aborts the in-flight step, and drops a turn about to start (the pre-step window) so a queued-but-not-started prompt never runs and cannot be batched into the cancelled turn. `abort()` is the narrower step-only verb; `cancel()` is what a UI/ACP `session/cancel` maps to. - `whenIdle()` — resolves once the agent reaches quiescence after settling out of `running` (resolves immediately when already idle; awaits the loop exit when disposed). The teardown signal: `abort()` then `await whenIdle()` guarantees the in-flight turn has fully stopped. Observes the transition without disposing the agent. @@ -163,9 +163,9 @@ Error containment: a throwing `agent/turn-continuation` listener or a broken ste Turn-end reasons: a turn ends with one `TurnEndReason` — `completed`, `aborted`, `error`, `disposed`, or `max-tokens`. `max-tokens` mirrors the model-call `FinishReason` of the same name (DeepSeek's `length`): a step that hit the output-token ceiling makes the turn end `max-tokens` rather than `completed`, by the rule *any `max-tokens` step in the turn surfaces as `max-tokens`* (a continuation plugin may run further steps after one, but the cut-short fact wins; the `disposed`/`aborted`/`error` outcomes still take precedence). This lets a consumer distinguish a clean stop from a truncated one (the ACP bridge maps it to the `max_tokens` stop reason). `TurnEndReason` is merge-extensible; `refusal` and `max_turn_requests` are the next variants to add when an adapter/loop first emits them. -A failure that happens once the turn is already closed has no in-turn position for a session `error` event (appending one after `turn/end` would put it past the persistence commit boundary, where it is dropped as a crash tail — [the turn-enclosure invariant](rfc/implemented/2026-06-15-turn-enclosure-invariant.md)). So a rejecting `session/flush` (the post-`turn/end` durability checkpoint) and a throwing `agent/turn-end` listener are reported via `agent/error` + the logger only, NOT as a session event; the turn stays balanced and the persistence backend keeps its buffered events for the next flush. +A failure that happens once the turn is already closed has no in-turn position for a session `error` event (appending one after `turn/end` would put it past the persistence commit boundary, where it is dropped as a crash tail — [the turn-enclosure invariant](rfc/implemented/architecture/2026-06-15-turn-enclosure-invariant.md)). So a rejecting `session/flush` (the post-`turn/end` durability checkpoint) and a throwing `agent/turn-end` listener are reported via `agent/error` + the logger only, NOT as a session event; the turn stays balanced and the persistence backend keeps its buffered events for the next flush. -**Turn-enclosure invariant**: every session event lives inside a turn (between a `turn/start` and its `turn/end`). The loop appends queued `user/message` events *after* `turn/start`, and an idle `agent.inject()` wraps its `context/message` in a one-shot `injection` turn. This makes the turn the single durability/replay boundary: a persistence backend can treat anything after the last `turn/end` as an interrupted-crash tail without risking the loss of legitimately-recorded between-turn context. The `dsh-invariants` plugin enforces it in dev (a message event outside an open turn throws). See [the turn-enclosure invariant](rfc/implemented/2026-06-15-turn-enclosure-invariant.md). +**Turn-enclosure invariant**: every session event lives inside a turn (between a `turn/start` and its `turn/end`). The loop appends queued `user/message` events *after* `turn/start`, and an idle `agent.inject()` wraps its `context/message` in a one-shot `injection` turn. This makes the turn the single durability/replay boundary: a persistence backend can treat anything after the last `turn/end` as an interrupted-crash tail without risking the loss of legitimately-recorded between-turn context. The `dsh-invariants` plugin enforces it in dev (a message event outside an open turn throws). See [the turn-enclosure invariant](rfc/implemented/architecture/2026-06-15-turn-enclosure-invariant.md). ### Event taxonomy diff --git a/docs/cookbook/adding-a-tool.md b/docs/cookbook/adding-a-tool.md index 8deb9510ae..330a2db17c 100644 --- a/docs/cookbook/adding-a-tool.md +++ b/docs/cookbook/adding-a-tool.md @@ -33,7 +33,7 @@ Registration is effect-based: disposing the plugin fiber unregisters the tool (w ## Rules of the execute() contract -- **Args are validated for you.** `defineTool` validates the model-generated `arguments` against the `SchemaSpec` before `execute` runs (type, required keys, enum membership, nested objects/arrays — [runtime arg validation](../rfc/implemented/2026-06-11-runtime-arg-validation.md)), so inside `execute` the args already match `InferArgs`. You still hand-check value constraints the DSL can't express (non-empty strings, positive numbers, cross-field rules); throw a descriptive Error for those. Raw JSON-Schema tools registered directly (MCP) are NOT validated by the harness — they validate their own input. +- **Args are validated for you.** `defineTool` validates the model-generated `arguments` against the `SchemaSpec` before `execute` runs (type, required keys, enum membership, nested objects/arrays — [runtime arg validation](../rfc/implemented/architecture/2026-06-11-runtime-arg-validation.md)), so inside `execute` the args already match `InferArgs`. You still hand-check value constraints the DSL can't express (non-empty strings, positive numbers, cross-field rules); throw a descriptive Error for those. Raw JSON-Schema tools registered directly (MCP) are NOT validated by the harness — they validate their own input. - **Throwing means isError.** The registry catches anything `execute()` throws and returns `{isError: true}` to the model. Use that for infrastructure failures (bad input, spawn errors, aborts) — but REPORT domain failures in the result text instead (e.g. tool-bash returns `[exit code: 9]` with `isError: false`: the model decides what a failing command means). - **Honor `exec.signal`.** Cancel in-flight work when it fires. - **Use `exec.agent` for async notifications.** `agent.inject(content, {source: {kind: 'plugin', plugin: ''}})` appends durable context the NEXT model request sees — it is not a wake-up (an idle agent stays idle). Guard against disposed agents (try/catch). diff --git a/docs/cookbook/adding-a-vendored-package.md b/docs/cookbook/adding-a-vendored-package.md index fa3c754cb8..71eadb9108 100644 --- a/docs/cookbook/adding-a-vendored-package.md +++ b/docs/cookbook/adding-a-vendored-package.md @@ -1,6 +1,6 @@ # Cookbook: adding a vendored package -When the harness needs another upstream Cordis package (e.g. `@cordisjs/plugin-http`), it is **vendored** as pinned source under `vendor/`, not added as an npm dependency — see [the vendoring decision](../rfc/implemented/2026-06-11-vendor-cordis-as-source.md) for why. [vendor/README.md](../../vendor/README.md) covers *updating* an already-vendored package; this guide is the file-by-file checklist for adding a **new** one. (Verified against the existing vendored set; if it drifts, fix it here.) +When the harness needs another upstream Cordis package (e.g. `@cordisjs/plugin-http`), it is **vendored** as pinned source under `vendor/`, not added as an npm dependency — see [the vendoring decision](../rfc/implemented/process/2026-06-11-vendor-cordis-as-source.md) for why. [vendor/README.md](../../vendor/README.md) covers *updating* an already-vendored package; this guide is the file-by-file checklist for adding a **new** one. (Verified against the existing vendored set; if it drifts, fix it here.) ## 1. Copy the source in diff --git a/docs/core-data-structures/bash.md b/docs/core-data-structures/bash.md index fc24bbd386..043140e25e 100644 --- a/docs/core-data-structures/bash.md +++ b/docs/core-data-structures/bash.md @@ -1,6 +1,6 @@ # Bash Executor -The bash execution seam — the canonical [capability seam](../rfc/implemented/2026-06-13-capability-seams.md) example, split across three packages: interface ([dsh-bash](../../packages/bash), `ctx.bash`), implementation ([dsh-bash-local](../../packages/bash-local), local subprocesses), and consumer ([dsh-tool-bash](../../packages/tool-bash), the `bash`/`bash_output`/`bash_kill` tool schemas). Bash is **one optional capability**, not part of the agent-loop spine — so its vocabulary lives here, not in [core.md](core.md). A sandboxed, containerized, or remote backend is a sibling package implementing the same interface. +The bash execution seam — the canonical [capability seam](../rfc/implemented/architecture/2026-06-13-capability-seams.md) example, split across three packages: interface ([dsh-bash](../../packages/bash), `ctx.bash`), implementation ([dsh-bash-local](../../packages/bash-local), local subprocesses), and consumer ([dsh-tool-bash](../../packages/tool-bash), the `bash`/`bash_output`/`bash_kill` tool schemas). Bash is **one optional capability**, not part of the agent-loop spine — so its vocabulary lives here, not in [core.md](core.md). A sandboxed, containerized, or remote backend is a sibling package implementing the same interface. Source: [`packages/bash/src/types.ts`](../../packages/bash/src/types.ts) diff --git a/docs/core-data-structures/persistence.md b/docs/core-data-structures/persistence.md index f232fae1c9..797299b1cb 100644 --- a/docs/core-data-structures/persistence.md +++ b/docs/core-data-structures/persistence.md @@ -2,7 +2,7 @@ The **durability seam** for the event log. [session.md](session.md) describes the in-memory `Session` — the append-only `SessionEvent` log that is the source of truth. This page describes how that log is made durable: the abstract `SessionPersistence` service, its backends, the flush checkpoint, crash recovery, and the metadata header that travels alongside the log. -The seam is a textbook [capability seam](../rfc/implemented/2026-06-13-capability-seams.md): one abstract service ([dsh-session-persistence](../../packages/session-persistence), `ctx.sessionPersistence`) defining create/append/load/list/has/delete over the existing `SessionEvent` — **no parallel persisted type** — and two interchangeable backends that pass the same `runPersistenceContract` suite. See the [session-persistence RFC](../rfc/implemented/2026-06-14-session-persistence.md). +The seam is a textbook [capability seam](../rfc/implemented/architecture/2026-06-13-capability-seams.md): one abstract service ([dsh-session-persistence](../../packages/session-persistence), `ctx.sessionPersistence`) defining create/append/load/list/has/delete over the existing `SessionEvent` — **no parallel persisted type** — and two interchangeable backends that pass the same `runPersistenceContract` suite. See the [session-persistence RFC](../rfc/implemented/architecture/2026-06-14-session-persistence.md). ## The flush checkpoint @@ -60,4 +60,4 @@ Both implement the same abstract `SessionPersistence` (create/append/load/list/h - **[dsh-session-persistence-jsonl](../../packages/session-persistence-jsonl)** — an append-only JSONL log per session with crash-safe atomic writes, the interrupted-turn crash recovery above, and a read/replay path. - **[dsh-session-persistence-sqlite](../../packages/session-persistence-sqlite)** — `node:sqlite`, one row per `SessionEvent`. The row shape `(session_id, seq, type, time, data)` maps 1:1 onto the event, so there is no parallel persisted schema to keep in sync. -Multiple backends sharing one on-disk session coordinate writes through the [shared persistence write-coordinator](../rfc/implemented/2026-06-18-shared-persistence-write-coordinator.md). +Multiple backends sharing one on-disk session coordinate writes through the [shared persistence write-coordinator](../rfc/implemented/architecture/2026-06-18-shared-persistence-write-coordinator.md). diff --git a/docs/core-data-structures/session.md b/docs/core-data-structures/session.md index 5803641c27..6856daeabc 100644 --- a/docs/core-data-structures/session.md +++ b/docs/core-data-structures/session.md @@ -110,7 +110,7 @@ interface TurnEndReasonMap { ## The turn-enclosure invariant -Every session event lives **inside** a turn (between a `turn/start` and its `turn/end`). The loop appends queued `user/message` events *after* `turn/start`, and an idle `agent.inject()` wraps its `context/message` in a one-shot `injection` turn. This makes the turn the single durability/replay boundary: a backend can treat anything after the last `turn/end` as an interrupted-crash tail without risking the loss of legitimately-recorded between-turn context. The `dsh-invariants` plugin enforces it in dev (a message event outside an open turn throws). See [the turn-enclosure invariant RFC](../rfc/implemented/2026-06-15-turn-enclosure-invariant.md). +Every session event lives **inside** a turn (between a `turn/start` and its `turn/end`). The loop appends queued `user/message` events *after* `turn/start`, and an idle `agent.inject()` wraps its `context/message` in a one-shot `injection` turn. This makes the turn the single durability/replay boundary: a backend can treat anything after the last `turn/end` as an interrupted-crash tail without risking the loss of legitimately-recorded between-turn context. The `dsh-invariants` plugin enforces it in dev (a message event outside an open turn throws). See [the turn-enclosure invariant RFC](../rfc/implemented/architecture/2026-06-15-turn-enclosure-invariant.md). ## Durability contract diff --git a/docs/rfc/README.md b/docs/rfc/README.md index c1299bb23a..6f295957ab 100644 --- a/docs/rfc/README.md +++ b/docs/rfc/README.md @@ -4,91 +4,160 @@ One kind of design doc lives here. An **RFC** records a decision or proposal tha ## Layout and naming -Files are grouped by lifecycle into three folders, and an RFC moves between them as its status changes: +Every RFC has two axes, both encoded in its **path** — `{lifecycle}/{class}/yyyy-mm-dd-topic-title.md`: -- **`proposed/`** — proposals reviewed before implementation; not yet built (or only partly). -- **`implemented/`** — the decision shipped. The file records what was decided and what was rejected, and is **kept current with what actually shipped**: when the code later moves a file, renames a package, or changes a key/default, the RFC is updated in the same change to match (facts only — paths, names, structure — not the decision itself). See [implemented/AGENTS.md](implemented/AGENTS.md). -- **`rejected/`** — the proposal was considered and declined. Kept for the record so the rejection isn't re-litigated. +- **Lifecycle** (the top-level folder) is the RFC's status, and an RFC moves between folders as that status changes: + - **`proposed/`** — proposals reviewed before implementation; not yet built (or only partly). + - **`implemented/`** — the decision shipped. The file records what was decided and what was rejected, and is **kept current with what actually shipped**: when the code later moves a file, renames a package, or changes a key/default, the RFC is updated in the same change to match (facts only — paths, names, structure — not the decision itself). See [implemented/AGENTS.md](implemented/AGENTS.md). + - **`rejected/`** — the proposal was considered and declined. Kept for the record so the rejection isn't re-litigated. +- **Class** (the nested folder) is the *kind* of decision — see [Classification](#classification) below. -Each file is named `yyyy-mm-dd-topic-title.md`, where the date is when the topic was **first proposed** (per git history). Cross-references between RFCs use relative markdown links (`[topic](../implemented/2026-…-….md)`) — never bare prose or numbers — so they are mechanically checkable and survive moves between folders. +The date in the filename is when the topic was **first proposed** (per git history). Cross-references between RFCs use relative markdown links (`[topic](../../implemented/architecture/2026-…-….md)`) — never bare prose or numbers — so they are mechanically checkable and survive moves between folders. + +## Classification + +Each RFC is filed under exactly one **class** — the kind of decision it records. The class is encoded in the path (the folder *is* the label, so a file's location declares its class) and the set is **closed**: `scripts/verify-rfc-classification.ts` rejects any folder outside the set and asserts this index lists every RFC under the heading matching its path. Adding a new class means amending that gate and this section, not just dropping a new folder. See [the classification RFC](implemented/process/2026-06-20-rfc-classification.md) for why the taxonomy is path-encoded and gated. + +| Class | What it 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 — gates, the package manager, vendoring — 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. (`refactor` is deliberately absent — it overlaps `simplification`, whose discriminator, "does observable behavior change?", already covers it.) ## When to write one -Write an RFC when a decision is **durable** (it shapes the codebase beyond a single function or package), **contested** (there was a real alternative a reasonable engineer might have chosen), and **surprising** (a future reader would otherwise ask "why on earth is it done this way?"). A proposal for substantial future work starts in `proposed/`; a decision already made starts in `implemented/`. +Write an RFC when a decision is **durable** (it shapes the codebase beyond a single function or package), **contested** (there was a real alternative a reasonable engineer might have chosen), and **surprising** (a future reader would otherwise ask "why on earth is it done this way?"). A proposal for substantial future work starts in `proposed/`; a decision already made starts in `implemented/`. Pick the class folder that matches the decision (see [Classification](#classification)). Do NOT write one for a mechanical or local choice (a variable name, a one-file refactor), for anything already enforced and explained by a gate or a convention in AGENTS.md, or for a still-provisional decision tagged `TODO(...)` in the code — record those as TODOs and promote to an RFC only once they settle. An RFC is never edited into a *different decision*: supersede it with a new one and cross-link. (Editing an `implemented/` RFC to track where its already-made decision now *lives* — a moved file, a renamed package — is not a different decision and is required, not forbidden; see [implemented/AGENTS.md](implemented/AGENTS.md).) ## Proposed +### Feature + | Title | First proposed | |---|---| -| [Mutation testing as the coverage counterweight](proposed/2026-06-11-mutation-testing.md) | 2026-06-11 | -| [Deterministic tests, the replay invariant fixture, and race stress](proposed/2026-06-11-deterministic-and-stress-testing.md) | 2026-06-11 | -| [Architectural conformance — dependency rules and the adapter kit](proposed/2026-06-11-architectural-conformance.md) | 2026-06-11 | -| [API extractor reports](proposed/2026-06-11-api-extractor-reports.md) | 2026-06-11 | -| [Supply chain checks and vendor drift verification](proposed/2026-06-11-supply-chain-and-vendor-drift.md) | 2026-06-11 | -| [Agent Client Protocol (ACP) support for external editors](proposed/2026-06-14-acp-agent-client-protocol.md) | 2026-06-14 | -| [Multiplex concurrent ACP sessions over one connection](proposed/2026-06-14-acp-multi-session.md) | 2026-06-14 | -| [Optional Code Mode — model writes TypeScript against an SDK of all tools](proposed/2026-06-15-optional-code-mode.md) | 2026-06-15 | -| [Runtime schemas for the event vocabulary (Zod vs the merge-extensible-map pattern)](proposed/2026-06-16-typed-event-schemas.md) | 2026-06-16 | -| [Unify the agent id and the session id](proposed/2026-06-20-unify-agent-and-session-id.md) | 2026-06-20 | -| [Stop mirroring durable boundaries as agent events](proposed/2026-06-20-remove-agent-boundary-mirror-events.md) | 2026-06-20 | -| [Keep one public stop primitive](proposed/2026-06-20-public-agent-stop-surface.md) | 2026-06-20 | -| [Drop unconsumed assembled LLM convenience surfaces](proposed/2026-06-20-drop-unconsumed-llm-assembled-surfaces.md) | 2026-06-20 | -| [Drop the unconsumed `llm/adapter-change` event](proposed/2026-06-20-drop-unconsumed-llm-adapter-change-event.md) | 2026-06-20 | -| [Prune dead methods from the persistence and bash seams](proposed/2026-06-20-prune-dead-seam-methods.md) | 2026-06-20 | -| [Fold trace-only session facts into load-bearing events](proposed/2026-06-20-collapse-trace-only-session-events.md) | 2026-06-20 | -| [Extract a generic long-running tool runtime](proposed/2026-06-20-generic-long-running-tool-runtime.md) | 2026-06-20 | -| [Make the shared example base providerless](proposed/2026-06-20-providerless-example-base.md) | 2026-06-20 | -| [Use `session.jsonl` as the only snapshot session-log artifact](proposed/2026-06-20-remove-redundant-snapshot-log-goldens.md) | 2026-06-20 | -| [Reorganize packages into a modular hierarchy](proposed/2026-06-20-package-hierarchy.md) | 2026-06-20 | -| [Discover package inventories instead of maintaining static lists](proposed/2026-06-20-discover-package-inventory.md) | 2026-06-20 | +| [Agent Client Protocol (ACP) support for external editors](proposed/feature/2026-06-14-acp-agent-client-protocol.md) | 2026-06-14 | +| [Multiplex concurrent ACP sessions over one connection](proposed/feature/2026-06-14-acp-multi-session.md) | 2026-06-14 | +| [Optional Code Mode — model writes TypeScript against an SDK of all tools](proposed/feature/2026-06-15-optional-code-mode.md) | 2026-06-15 | + +### Simplification + +| Title | First proposed | +|---|---| +| [Unify the agent id and the session id](proposed/simplification/2026-06-20-unify-agent-and-session-id.md) | 2026-06-20 | +| [Stop mirroring durable boundaries as agent events](proposed/simplification/2026-06-20-remove-agent-boundary-mirror-events.md) | 2026-06-20 | +| [Keep one public stop primitive](proposed/simplification/2026-06-20-public-agent-stop-surface.md) | 2026-06-20 | +| [Drop unconsumed assembled LLM convenience surfaces](proposed/simplification/2026-06-20-drop-unconsumed-llm-assembled-surfaces.md) | 2026-06-20 | +| [Drop the unconsumed `llm/adapter-change` event](proposed/simplification/2026-06-20-drop-unconsumed-llm-adapter-change-event.md) | 2026-06-20 | +| [Prune dead methods from the persistence and bash seams](proposed/simplification/2026-06-20-prune-dead-seam-methods.md) | 2026-06-20 | +| [Fold trace-only session facts into load-bearing events](proposed/simplification/2026-06-20-collapse-trace-only-session-events.md) | 2026-06-20 | + +### Architecture + +| Title | First proposed | +|---|---| +| [Runtime schemas for the event vocabulary (Zod vs the merge-extensible-map pattern)](proposed/architecture/2026-06-16-typed-event-schemas.md) | 2026-06-16 | +| [Extract a generic long-running tool runtime](proposed/architecture/2026-06-20-generic-long-running-tool-runtime.md) | 2026-06-20 | +| [Make the shared example base providerless](proposed/architecture/2026-06-20-providerless-example-base.md) | 2026-06-20 | +| [Reorganize packages into a modular hierarchy](proposed/architecture/2026-06-20-package-hierarchy.md) | 2026-06-20 | + +### Process + +| Title | First proposed | +|---|---| +| [Architectural conformance — dependency rules and the adapter kit](proposed/process/2026-06-11-architectural-conformance.md) | 2026-06-11 | +| [API extractor reports](proposed/process/2026-06-11-api-extractor-reports.md) | 2026-06-11 | +| [Supply chain checks and vendor drift verification](proposed/process/2026-06-11-supply-chain-and-vendor-drift.md) | 2026-06-11 | +| [Discover package inventories instead of maintaining static lists](proposed/process/2026-06-20-discover-package-inventory.md) | 2026-06-20 | + +### Testing + +| Title | First proposed | +|---|---| +| [Mutation testing as the coverage counterweight](proposed/testing/2026-06-11-mutation-testing.md) | 2026-06-11 | +| [Deterministic tests, the replay invariant fixture, and race stress](proposed/testing/2026-06-11-deterministic-and-stress-testing.md) | 2026-06-11 | +| [Use `session.jsonl` as the only snapshot session-log artifact](proposed/testing/2026-06-20-remove-redundant-snapshot-log-goldens.md) | 2026-06-20 | ## Implemented +### Feature + | Title | First proposed | |---|---| -| [Vendor Cordis as source, not npm dependencies](implemented/2026-06-11-vendor-cordis-as-source.md) | 2026-06-11 | -| [Microkernel: extension via Cordis event taxonomy, one concrete loop](implemented/2026-06-11-microkernel-event-taxonomy.md) | 2026-06-11 | -| [Event-sourced sessions with derived message history](implemented/2026-06-11-event-sourced-sessions.md) | 2026-06-11 | -| [Provider-neutral content-block vocabulary owned by dsh-llm](implemented/2026-06-11-content-block-vocabulary.md) | 2026-06-11 | -| [Custom typed tool-schema DSL instead of schemastery](implemented/2026-06-11-custom-schema-dsl.md) | 2026-06-11 | -| [Tool schemas are part of the system-prompt assembly](implemented/2026-06-11-tool-schemas-in-prompt-assembly.md) | 2026-06-11 | -| [Mechanical quality gates over prose guidelines](implemented/2026-06-11-quality-gates.md) | 2026-06-11 | -| [tsdown for JS bundling instead of dumble](implemented/2026-06-11-tsdown-over-dumble.md) | 2026-06-11 | -| [Runtime arg validation at the model boundary](implemented/2026-06-11-runtime-arg-validation.md) | 2026-06-11 | -| [Dev-mode invariants over compile-time deep-readonly](implemented/2026-06-11-dev-invariants-over-deep-readonly.md) | 2026-06-11 | -| [Property-based testing for protocol-shaped code](implemented/2026-06-11-property-based-testing.md) | 2026-06-11 | -| [Doc-sync enforcement](implemented/2026-06-11-doc-sync-enforcement.md) | 2026-06-11 | -| [Markdown cross-link validity linting](implemented/2026-06-18-markdown-cross-link-lint.md) | 2026-06-18 | -| [Structured error taxonomy](implemented/2026-06-11-structured-error-taxonomy.md) | 2026-06-11 | -| [Capability seams — interface / implementation / consumer split](implemented/2026-06-13-capability-seams.md) | 2026-06-13 | -| [Two LLM adapters as a design-verification twin](implemented/2026-06-13-twin-llm-adapters.md) | 2026-06-13 | -| [Session persistence as an abstract service over `SessionEvent`](implemented/2026-06-14-session-persistence.md) | 2026-06-14 | -| [Every session event is enclosed in a turn](implemented/2026-06-15-turn-enclosure-invariant.md) | 2026-06-15 | -| [pnpm as the package manager instead of Yarn 4](implemented/2026-06-16-pnpm-over-yarn.md) | 2026-06-16 | -| [Rich ACP bash rendering — the terminal card (`_meta`) and command classification](implemented/2026-06-18-acp-terminal-and-tool-rendering.md) | 2026-06-18 | -| [ACP snapshot tests — record-once / replay-deterministic](implemented/2026-06-19-acp-snapshot-tests.md) | 2026-06-19 | -| [Real-API e2e in CI against the external DeepSeek API](implemented/2026-06-19-real-api-e2e-ci.md) | 2026-06-19 | -| [Drop the mutable session summary](implemented/2026-06-19-drop-mutable-session-summary.md) | 2026-06-19 | -| [Shared persistence write coordinator](implemented/2026-06-18-shared-persistence-write-coordinator.md) | 2026-06-18 | -| [Agent lifecycle and ownership seams](implemented/2026-06-18-agent-lifecycle-and-ownership-seams.md) | 2026-06-18 | -| [Core-data-structures catalog and the `ts type-equiv` drift gate](implemented/2026-06-20-core-data-structures-catalog.md) | 2026-06-20 | -| [Generated cordis events + services catalog](implemented/2026-06-20-generated-cordis-catalog.md) | 2026-06-20 | +| [Rich ACP bash rendering — the terminal card (`_meta`) and command classification](implemented/feature/2026-06-18-acp-terminal-and-tool-rendering.md) | 2026-06-18 | + +### Simplification + +| Title | First proposed | +|---|---| +| [Drop the mutable session summary](implemented/simplification/2026-06-19-drop-mutable-session-summary.md) | 2026-06-19 | + +### Architecture + +| Title | First proposed | +|---|---| +| [Microkernel: extension via Cordis event taxonomy, one concrete loop](implemented/architecture/2026-06-11-microkernel-event-taxonomy.md) | 2026-06-11 | +| [Event-sourced sessions with derived message history](implemented/architecture/2026-06-11-event-sourced-sessions.md) | 2026-06-11 | +| [Provider-neutral content-block vocabulary owned by dsh-llm](implemented/architecture/2026-06-11-content-block-vocabulary.md) | 2026-06-11 | +| [Custom typed tool-schema DSL instead of schemastery](implemented/architecture/2026-06-11-custom-schema-dsl.md) | 2026-06-11 | +| [Tool schemas are part of the system-prompt assembly](implemented/architecture/2026-06-11-tool-schemas-in-prompt-assembly.md) | 2026-06-11 | +| [Runtime arg validation at the model boundary](implemented/architecture/2026-06-11-runtime-arg-validation.md) | 2026-06-11 | +| [Dev-mode invariants over compile-time deep-readonly](implemented/architecture/2026-06-11-dev-invariants-over-deep-readonly.md) | 2026-06-11 | +| [Structured error taxonomy](implemented/architecture/2026-06-11-structured-error-taxonomy.md) | 2026-06-11 | +| [Capability seams — interface / implementation / consumer split](implemented/architecture/2026-06-13-capability-seams.md) | 2026-06-13 | +| [Two LLM adapters as a design-verification twin](implemented/architecture/2026-06-13-twin-llm-adapters.md) | 2026-06-13 | +| [Session persistence as an abstract service over `SessionEvent`](implemented/architecture/2026-06-14-session-persistence.md) | 2026-06-14 | +| [Every session event is enclosed in a turn](implemented/architecture/2026-06-15-turn-enclosure-invariant.md) | 2026-06-15 | +| [Shared persistence write coordinator](implemented/architecture/2026-06-18-shared-persistence-write-coordinator.md) | 2026-06-18 | +| [Agent lifecycle and ownership seams](implemented/architecture/2026-06-18-agent-lifecycle-and-ownership-seams.md) | 2026-06-18 | + +### Process + +| Title | First proposed | +|---|---| +| [Vendor Cordis as source, not npm dependencies](implemented/process/2026-06-11-vendor-cordis-as-source.md) | 2026-06-11 | +| [Mechanical quality gates over prose guidelines](implemented/process/2026-06-11-quality-gates.md) | 2026-06-11 | +| [tsdown for JS bundling instead of dumble](implemented/process/2026-06-11-tsdown-over-dumble.md) | 2026-06-11 | +| [Doc-sync enforcement](implemented/process/2026-06-11-doc-sync-enforcement.md) | 2026-06-11 | +| [pnpm as the package manager instead of Yarn 4](implemented/process/2026-06-16-pnpm-over-yarn.md) | 2026-06-16 | +| [Markdown cross-link validity linting](implemented/process/2026-06-18-markdown-cross-link-lint.md) | 2026-06-18 | +| [Core-data-structures catalog and the `ts type-equiv` drift gate](implemented/process/2026-06-20-core-data-structures-catalog.md) | 2026-06-20 | +| [Generated cordis events + services catalog](implemented/process/2026-06-20-generated-cordis-catalog.md) | 2026-06-20 | +| [Classify RFCs by kind via path-encoded subdirectories](implemented/process/2026-06-20-rfc-classification.md) | 2026-06-20 | + +### Testing + +| Title | First proposed | +|---|---| +| [Property-based testing for protocol-shaped code](implemented/testing/2026-06-11-property-based-testing.md) | 2026-06-11 | +| [ACP snapshot tests — record-once / replay-deterministic](implemented/testing/2026-06-19-acp-snapshot-tests.md) | 2026-06-19 | +| [Real-API e2e in CI against the external DeepSeek API](implemented/testing/2026-06-19-real-api-e2e-ci.md) | 2026-06-19 | ## Rejected +### Simplification + | Title | First proposed | |---|---| -| [Deep-readonly public surfaces](rejected/2026-06-11-immutable-public-surfaces.md) | 2026-06-11 | -| [Persist assembled assistant messages, not stream chunks](rejected/2026-06-20-assembled-assistant-messages-only.md) | 2026-06-20 | -| [Drop ACP session/load until resume has a product shape](rejected/2026-06-20-drop-acp-session-load.md) | 2026-06-20 | -| [Drop ACP terminal `_meta` rendering](rejected/2026-06-20-drop-acp-terminal-meta.md) | 2026-06-20 | -| [Drop bash full-output spill files](rejected/2026-06-20-drop-bash-output-spill-files.md) | 2026-06-20 | -| [Drop durable step boundary events](rejected/2026-06-20-drop-durable-step-boundaries.md) | 2026-06-20 | -| [Drop unused session lineage metadata](rejected/2026-06-20-drop-unused-session-lineage.md) | 2026-06-20 | -| [Fold the persistence interface into dsh-session](rejected/2026-06-20-fold-session-persistence-interface.md) | 2026-06-20 | -| [Collapse tool-owned UI presentation](rejected/2026-06-20-generic-tool-rendering.md) | 2026-06-20 | -| [Retire mid-turn steering](rejected/2026-06-20-retire-mid-turn-steering.md) | 2026-06-20 | -| [Return the ACP bridge to one live session per connection](rejected/2026-06-20-single-session-acp-bridge.md) | 2026-06-20 | -| [Truncate interrupted final turns on load](rejected/2026-06-20-truncate-interrupted-turns.md) | 2026-06-20 | +| [Persist assembled assistant messages, not stream chunks](rejected/simplification/2026-06-20-assembled-assistant-messages-only.md) | 2026-06-20 | +| [Drop ACP session/load until resume has a product shape](rejected/simplification/2026-06-20-drop-acp-session-load.md) | 2026-06-20 | +| [Drop ACP terminal `_meta` rendering](rejected/simplification/2026-06-20-drop-acp-terminal-meta.md) | 2026-06-20 | +| [Drop bash full-output spill files](rejected/simplification/2026-06-20-drop-bash-output-spill-files.md) | 2026-06-20 | +| [Drop durable step boundary events](rejected/simplification/2026-06-20-drop-durable-step-boundaries.md) | 2026-06-20 | +| [Drop unused session lineage metadata](rejected/simplification/2026-06-20-drop-unused-session-lineage.md) | 2026-06-20 | +| [Fold the persistence interface into dsh-session](rejected/simplification/2026-06-20-fold-session-persistence-interface.md) | 2026-06-20 | +| [Collapse tool-owned UI presentation](rejected/simplification/2026-06-20-generic-tool-rendering.md) | 2026-06-20 | +| [Retire mid-turn steering](rejected/simplification/2026-06-20-retire-mid-turn-steering.md) | 2026-06-20 | +| [Return the ACP bridge to one live session per connection](rejected/simplification/2026-06-20-single-session-acp-bridge.md) | 2026-06-20 | +| [Truncate interrupted final turns on load](rejected/simplification/2026-06-20-truncate-interrupted-turns.md) | 2026-06-20 | + +### Architecture + +| Title | First proposed | +|---|---| +| [Deep-readonly public surfaces](rejected/architecture/2026-06-11-immutable-public-surfaces.md) | 2026-06-11 | diff --git a/docs/rfc/implemented/2026-06-11-content-block-vocabulary.md b/docs/rfc/implemented/architecture/2026-06-11-content-block-vocabulary.md similarity index 100% rename from docs/rfc/implemented/2026-06-11-content-block-vocabulary.md rename to docs/rfc/implemented/architecture/2026-06-11-content-block-vocabulary.md diff --git a/docs/rfc/implemented/2026-06-11-custom-schema-dsl.md b/docs/rfc/implemented/architecture/2026-06-11-custom-schema-dsl.md similarity index 100% rename from docs/rfc/implemented/2026-06-11-custom-schema-dsl.md rename to docs/rfc/implemented/architecture/2026-06-11-custom-schema-dsl.md diff --git a/docs/rfc/implemented/2026-06-11-dev-invariants-over-deep-readonly.md b/docs/rfc/implemented/architecture/2026-06-11-dev-invariants-over-deep-readonly.md similarity index 86% rename from docs/rfc/implemented/2026-06-11-dev-invariants-over-deep-readonly.md rename to docs/rfc/implemented/architecture/2026-06-11-dev-invariants-over-deep-readonly.md index 518cd7168d..b9a182a4e9 100644 --- a/docs/rfc/implemented/2026-06-11-dev-invariants-over-deep-readonly.md +++ b/docs/rfc/implemented/architecture/2026-06-11-dev-invariants-over-deep-readonly.md @@ -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` 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` 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. diff --git a/docs/rfc/implemented/2026-06-11-event-sourced-sessions.md b/docs/rfc/implemented/architecture/2026-06-11-event-sourced-sessions.md similarity index 100% rename from docs/rfc/implemented/2026-06-11-event-sourced-sessions.md rename to docs/rfc/implemented/architecture/2026-06-11-event-sourced-sessions.md diff --git a/docs/rfc/implemented/2026-06-11-microkernel-event-taxonomy.md b/docs/rfc/implemented/architecture/2026-06-11-microkernel-event-taxonomy.md similarity index 100% rename from docs/rfc/implemented/2026-06-11-microkernel-event-taxonomy.md rename to docs/rfc/implemented/architecture/2026-06-11-microkernel-event-taxonomy.md diff --git a/docs/rfc/implemented/2026-06-11-runtime-arg-validation.md b/docs/rfc/implemented/architecture/2026-06-11-runtime-arg-validation.md similarity index 90% rename from docs/rfc/implemented/2026-06-11-runtime-arg-validation.md rename to docs/rfc/implemented/architecture/2026-06-11-runtime-arg-validation.md index 9998c85d35..5c0aad9eb2 100644 --- a/docs/rfc/implemented/2026-06-11-runtime-arg-validation.md +++ b/docs/rfc/implemented/architecture/2026-06-11-runtime-arg-validation.md @@ -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. diff --git a/docs/rfc/implemented/2026-06-11-structured-error-taxonomy.md b/docs/rfc/implemented/architecture/2026-06-11-structured-error-taxonomy.md similarity index 100% rename from docs/rfc/implemented/2026-06-11-structured-error-taxonomy.md rename to docs/rfc/implemented/architecture/2026-06-11-structured-error-taxonomy.md diff --git a/docs/rfc/implemented/2026-06-11-tool-schemas-in-prompt-assembly.md b/docs/rfc/implemented/architecture/2026-06-11-tool-schemas-in-prompt-assembly.md similarity index 100% rename from docs/rfc/implemented/2026-06-11-tool-schemas-in-prompt-assembly.md rename to docs/rfc/implemented/architecture/2026-06-11-tool-schemas-in-prompt-assembly.md diff --git a/docs/rfc/implemented/2026-06-13-capability-seams.md b/docs/rfc/implemented/architecture/2026-06-13-capability-seams.md similarity index 90% rename from docs/rfc/implemented/2026-06-13-capability-seams.md rename to docs/rfc/implemented/architecture/2026-06-13-capability-seams.md index 3446fce9af..e66440c3ee 100644 --- a/docs/rfc/implemented/2026-06-13-capability-seams.md +++ b/docs/rfc/implemented/architecture/2026-06-13-capability-seams.md @@ -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. diff --git a/docs/rfc/implemented/2026-06-13-twin-llm-adapters.md b/docs/rfc/implemented/architecture/2026-06-13-twin-llm-adapters.md similarity index 93% rename from docs/rfc/implemented/2026-06-13-twin-llm-adapters.md rename to docs/rfc/implemented/architecture/2026-06-13-twin-llm-adapters.md index 6d6ded31e4..ecd21b5cc3 100644 --- a/docs/rfc/implemented/2026-06-13-twin-llm-adapters.md +++ b/docs/rfc/implemented/architecture/2026-06-13-twin-llm-adapters.md @@ -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. diff --git a/docs/rfc/implemented/2026-06-14-session-persistence.md b/docs/rfc/implemented/architecture/2026-06-14-session-persistence.md similarity index 89% rename from docs/rfc/implemented/2026-06-14-session-persistence.md rename to docs/rfc/implemented/architecture/2026-06-14-session-persistence.md index 3e66be3730..9bcd1f8c6f 100644 --- a/docs/rfc/implemented/2026-06-14-session-persistence.md +++ b/docs/rfc/implemented/architecture/2026-06-14-session-persistence.md @@ -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). diff --git a/docs/rfc/implemented/2026-06-15-turn-enclosure-invariant.md b/docs/rfc/implemented/architecture/2026-06-15-turn-enclosure-invariant.md similarity index 100% rename from docs/rfc/implemented/2026-06-15-turn-enclosure-invariant.md rename to docs/rfc/implemented/architecture/2026-06-15-turn-enclosure-invariant.md diff --git a/docs/rfc/implemented/2026-06-18-agent-lifecycle-and-ownership-seams.md b/docs/rfc/implemented/architecture/2026-06-18-agent-lifecycle-and-ownership-seams.md similarity index 95% rename from docs/rfc/implemented/2026-06-18-agent-lifecycle-and-ownership-seams.md rename to docs/rfc/implemented/architecture/2026-06-18-agent-lifecycle-and-ownership-seams.md index 839e5d6c9f..dc4b2428a1 100644 --- a/docs/rfc/implemented/2026-06-18-agent-lifecycle-and-ownership-seams.md +++ b/docs/rfc/implemented/architecture/2026-06-18-agent-lifecycle-and-ownership-seams.md @@ -35,7 +35,7 @@ Background-task ownership moved from a `tool-bash` plugin-local `Map` **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` 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` 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. diff --git a/docs/rfc/implemented/process/2026-06-20-rfc-classification.md b/docs/rfc/implemented/process/2026-06-20-rfc-classification.md new file mode 100644 index 0000000000..d3fc95399b --- /dev/null +++ b/docs/rfc/implemented/process/2026-06-20-rfc-classification.md @@ -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. diff --git a/docs/rfc/implemented/2026-06-19-drop-mutable-session-summary.md b/docs/rfc/implemented/simplification/2026-06-19-drop-mutable-session-summary.md similarity index 63% rename from docs/rfc/implemented/2026-06-19-drop-mutable-session-summary.md rename to docs/rfc/implemented/simplification/2026-06-19-drop-mutable-session-summary.md index 6f73664c26..24f693e6b6 100644 --- a/docs/rfc/implemented/2026-06-19-drop-mutable-session-summary.md +++ b/docs/rfc/implemented/simplification/2026-06-19-drop-mutable-session-summary.md @@ -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. diff --git a/docs/rfc/implemented/2026-06-11-property-based-testing.md b/docs/rfc/implemented/testing/2026-06-11-property-based-testing.md similarity index 92% rename from docs/rfc/implemented/2026-06-11-property-based-testing.md rename to docs/rfc/implemented/testing/2026-06-11-property-based-testing.md index 9e4300cf65..19e371b1b0 100644 --- a/docs/rfc/implemented/2026-06-11-property-based-testing.md +++ b/docs/rfc/implemented/testing/2026-06-11-property-based-testing.md @@ -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 diff --git a/docs/rfc/implemented/2026-06-19-acp-snapshot-tests.md b/docs/rfc/implemented/testing/2026-06-19-acp-snapshot-tests.md similarity index 75% rename from docs/rfc/implemented/2026-06-19-acp-snapshot-tests.md rename to docs/rfc/implemented/testing/2026-06-19-acp-snapshot-tests.md index cf61386509..a0f6497c2a 100644 --- a/docs/rfc/implemented/2026-06-19-acp-snapshot-tests.md +++ b/docs/rfc/implemented/testing/2026-06-19-acp-snapshot-tests.md @@ -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 `/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 `/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 `/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. diff --git a/docs/rfc/implemented/2026-06-19-real-api-e2e-ci.md b/docs/rfc/implemented/testing/2026-06-19-real-api-e2e-ci.md similarity index 88% rename from docs/rfc/implemented/2026-06-19-real-api-e2e-ci.md rename to docs/rfc/implemented/testing/2026-06-19-real-api-e2e-ci.md index ed52c9f23c..b6fc29ee90 100644 --- a/docs/rfc/implemented/2026-06-19-real-api-e2e-ci.md +++ b/docs/rfc/implemented/testing/2026-06-19-real-api-e2e-ci.md @@ -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 diff --git a/docs/rfc/proposed/2026-06-20-providerless-example-base.md b/docs/rfc/proposed/2026-06-20-providerless-example-base.md deleted file mode 100644 index a824dd4fa1..0000000000 --- a/docs/rfc/proposed/2026-06-20-providerless-example-base.md +++ /dev/null @@ -1,27 +0,0 @@ -# RFC: Make the shared example base providerless - -Status: proposed - -## Problem - -The examples have two shared base files: [examples/base-core.yml](../../../examples/base-core.yml) is providerless, while [examples/base.yml](../../../examples/base.yml) includes that core plus the real `llm-deepseek` adapter. Snapshot replay needs the providerless core with `llm-replay`, because loading the real adapter without a key throws. The normal demos need the real adapter. The result is a naming inversion: the file named `base.yml` is not the reusable base for all examples, while the true base is `base-core.yml`. - -The split is understandable, but it makes every config explanation longer. It also leads to awkward test setup like a keyless smoke test carrying a dummy API key so an adapter can boot even though the model is not called. - -## Proposal - -Rename the providerless core to [examples/base.yml](../../../examples/base.yml) and make adapter selection explicit in each concrete example. The coding and ACP real configs add a tiny `llm-deepseek` include or local block; snapshot config adds `llm-replay`. Delete [examples/base-core.yml](../../../examples/base-core.yml). - -The shared base should contain only provider-neutral services and tools: `llm`, sessions, system prompt, tools, agents, invariants, bash executor, and bash tool schemas. Anything that chooses a model provider belongs at the leaf config. - -## Acceptance criteria - -- [examples/base.yml](../../../examples/base.yml) is providerless. -- [examples/base-core.yml](../../../examples/base-core.yml) is deleted. -- Real demo configs explicitly add the DeepSeek adapter. -- Snapshot replay config includes the same providerless base and its replay adapter. -- The [examples README](../../../examples/README.md), example-specific READMEs, and RFC references stop explaining "base = base-core plus adapter". - -## What we give up - -Real demos lose one layer of convenience: each must opt into the adapter. That is the right default for examples, because adapter choice is the variable part and providerless wiring is the shared product core. diff --git a/docs/rfc/proposed/2026-06-20-prune-dead-seam-methods.md b/docs/rfc/proposed/2026-06-20-prune-dead-seam-methods.md deleted file mode 100644 index d38583c60b..0000000000 --- a/docs/rfc/proposed/2026-06-20-prune-dead-seam-methods.md +++ /dev/null @@ -1,49 +0,0 @@ -# RFC: Prune dead methods from the persistence and bash capability seams - -Status: proposed - -## Problem - -Two capability seams ([interface / implementation / consumer](../implemented/2026-06-13-capability-seams.md)) carry abstract methods that no consumer calls. The seam exists to let implementations and consumers evolve independently — but a method no consumer programs against is not a seam, it is speculative surface every implementation must still implement and test. - -### `SessionPersistence.has()` and `.delete()` - -The abstract service declares four operations beyond create/append: `load`, `list`, `has`, `delete` ([packages/session-persistence/src/index.ts:142-151](../../../packages/session-persistence/src/index.ts)). Production consumers of `ctx.sessionPersistence` use only two of them: the agent-loop resume path calls `load()` ([packages/agent-loop/src/index.ts:176-194](../../../packages/agent-loop/src/index.ts)), and the ACP bridge calls `list()` for `session/list` ([packages/acp/src/index.ts](../../../packages/acp/src/index.ts)). Grepping every `sessionPersistence.*` / `persistence.*` use across `packages/*/src` and `examples/` finds no `has(` and no `delete(` on the service. The `.has(`/`.delete(` calls in `packages/acp/src/index.ts` are on the in-memory `SessionStore` and a local `Set` of loading ids, not persistence. The only callers of `has`/`delete` are the contract suites and per-backend specs. - -`has()` is not just unused — it is the most intricate branch in the shared coordinator: a tracked-vs-untracked dual-probe (`loadLive(id, cwd)` for a live-tracked session vs `loadStored(id)` for an untracked one) with a multi-line rationale ([packages/session-persistence/src/coordinator.ts:298-310](../../../packages/session-persistence/src/coordinator.ts)). `delete()` drags the `deleteStored` backend hook ([coordinator.ts:99](../../../packages/session-persistence/src/coordinator.ts), [coordinator.ts:313-319](../../../packages/session-persistence/src/coordinator.ts)) that every backend must implement. This is the [drop-mutable-session-summary](../implemented/2026-06-19-drop-mutable-session-summary.md) pattern: a contract test exercises both, but no shipping code asks "is this session persisted?" or removes one. - -### `BashExecutor.get()` and `.list()` - -The bash seam declares `get(id)` ("look up a background task by id") and `list()` ("all tracked background tasks") ([packages/bash/src/index.ts:88-107](../../../packages/bash/src/index.ts)), both implemented by `LocalBashExecutor` ([packages/bash-local/src/index.ts:179-191](../../../packages/bash-local/src/index.ts)). The sole production consumer — `dsh-tool-bash` — drives tasks via `ownerOf`, `onTaskDone`, `start`, `readOutput`, `kill`, `resolve`, `run`; it never calls `get`/`list` in shipping code, and there is no `bash_list` tool exposing a task roster to the model. So both are dead production seam surface. They are used by tests, more broadly than a single idiom: the bash seam/executor specs assert them directly ([packages/bash/tests/service.spec.ts](../../../packages/bash/tests/service.spec.ts), [packages/bash-local/tests/executor.spec.ts](../../../packages/bash-local/tests/executor.spec.ts) both call `get()`/`list()`), and several `dsh-tool-bash` tests reach through `ctx.bash.get(id)` to await a task's `done`, read its `status`, or inspect task fields ([packages/tool-bash/tests/tools.spec.ts](../../../packages/tool-bash/tests/tools.spec.ts), [packages/tool-bash/tests/integration.spec.ts](../../../packages/tool-bash/tests/integration.spec.ts)). These are test-harness conveniences, not shipping consumers — but they are real test code an implementing PR must migrate or delete. - -## Proposal - -Remove the methods nothing consumes, from the abstract seam, the implementation, and the contract/spec suites that exist only to exercise them: - -- `SessionPersistence.has()` / `.delete()`: delete the abstract declarations, the coordinator's `has`/`delete`/`deleteCore`, and the `PersistenceBackend.deleteStored` hook. Remove the `has`/`delete` rows from the contract suite and the per-backend specs (jsonl + sqlite each implement `deleteStored` only to satisfy the hook — that implementation goes too). The backends are the [dual-backend](../implemented/2026-06-14-session-persistence.md) design and otherwise out of scope, but removing a hook they implement for no consumer is part of removing the hook, not a backend redesign. -- `BashExecutor.get()` / `.list()`: delete the abstract declarations and the `LocalBashExecutor` impls. The seam/executor specs that assert `get()`/`list()` directly (`bash/tests/service.spec.ts`, `bash-local/tests/executor.spec.ts`) lose those assertions (the behavior is being removed). The `dsh-tool-bash` tests that reach through `ctx.bash.get(id)` to await `done`, read `status`, or inspect task fields switch to the public completion/status seam they should use — `onTaskDone` (or the `done` promise and status the `start()` return already exposes) — keeping their coverage without the removed lookup method. -- Update every doc and source-comment reference to the removed methods — not only literal `has(`/`delete(`/`get(`/`list(`/`deleteStored` call spellings, but also `{@link has}`/`{@link delete}` JSDoc links and prose that counts the methods (removing 2 of the persistence service's 6 public methods makes any "six public methods" phrasing wrong). The implementing PR greps `has`/`delete`/`get`/`list`/`deleteStored`/`{@link `/`six ` across `docs/`, `packages/*/README.md`, and source comments, and fixes each. The known doc sites: the seam READMEs ([packages/session-persistence/README.md](../../../packages/session-persistence/README.md)'s `has(id)`/`delete(id)` API row and its "delegates its six public service methods" prose → four, [packages/bash/README.md](../../../packages/bash/README.md)'s `get(id)`/`list()` row), the backend READMEs that describe `has`/`list` semantics ([packages/session-persistence-sqlite/README.md](../../../packages/session-persistence-sqlite/README.md), [packages/session-persistence-jsonl/README.md](../../../packages/session-persistence-jsonl/README.md) — reword "absent from `has()`/`list()`" to just `list()`), the service-map / seam docs in [docs/architecture.md](../../../docs/architecture.md), and the persistence prose in the [session-persistence RFC](../implemented/2026-06-14-session-persistence.md) and [shared write-coordinator RFC](../implemented/2026-06-18-shared-persistence-write-coordinator.md). The known source-comment sites: the abstract `create()` JSDoc's `{@link has}/{@link list}` link ([packages/session-persistence/src/index.ts](../../../packages/session-persistence/src/index.ts) — drop the `has` link), the coordinator's "six public methods"/"six public service methods" module + class JSDoc and its lazy-materialization JSDoc justifying the `materialized` flag by "the signal `has`/`list` rely on" ([packages/session-persistence/src/coordinator.ts](../../../packages/session-persistence/src/coordinator.ts)), the JSONL backend's `loadStored`/`deleteStored` comment, and the SQLite backend's `schema.ts` and `index.ts` comments that mention "absent from `has`/`list`" — all reworded to the surviving four-method, `list()`-only contract. - -## Why not keep them as "the seam should be complete"? - -The instinct that a persistence seam "should" offer delete, or a task executor "should" offer enumeration, is real — and it is exactly the speculative-completeness the pre-release stance warns against ([AGENTS.md](../../../AGENTS.md): optimize for the correct foundation, not for hypothetical callers you do not have). Each of these is one method to re-add the day a consumer needs it: - -- A session-management UI that deletes old sessions will want `delete()` — add it then, designed against that UI's real needs (soft-delete? cascade? confirmation?), not guessed now. -- A `bash_list` tool that shows the model its running tasks will want `list()` — add it with the tool. - -Re-adding a seam method with a live consumer is cheap and better-designed than the speculative version, because the consumer pins the contract. Carrying it unused means every implementation (and every future backend) must implement and test a method that does nothing. - -## Acceptance criteria - -- `has`/`delete`/`deleteStored` and `get`/`list` are gone from their seams, impls, and contract suites; `pnpm run knip` reports no new dead exports. -- The remaining seam operations (`create`/`append`/`load`/`list` for persistence; `run`/`start`/`ownerOf`/`onTaskDone`/`readOutput`/`kill`/`resolve` for bash) are untouched; ACP `session/list`, bash tool flows, and crash-recovery behave identically. -- `pnpm run test:coverage` stays 100% per-file (the contract/spec rows for the removed methods are deleted with them). -- Seam READMEs and `docs/architecture.md` no longer list the removed methods. - -## Risks - -- **`delete()` is the kind of operation a product eventually wants.** True — but "eventually" is the point. Deleting it now and re-adding it against a real consumer is strictly better than shipping a guessed contract. The dual backends each shed a `deleteStored` impl, which is a bounded edit in otherwise-out-of-scope packages. -- **`list()` on the bash seam is the natural seed for a future `bash_list`.** Acknowledged in the [pre-release foundation stance](../../../AGENTS.md): add the seed when the tool lands. The executor still tracks tasks internally (the `tasks` map backs `ownerOf`/`readOutput`/`kill`); exposing an enumeration is a one-line re-add. -- **Low coupling.** Both removals are confined to their seam + impl + tests; no cross-package consumer references the removed methods, so there is no ripple beyond the docs. - -Modest size, but it converts two seams from "what an implementation must provide for nobody" back to "exactly what a consumer uses." diff --git a/docs/rfc/proposed/2026-06-16-typed-event-schemas.md b/docs/rfc/proposed/architecture/2026-06-16-typed-event-schemas.md similarity index 93% rename from docs/rfc/proposed/2026-06-16-typed-event-schemas.md rename to docs/rfc/proposed/architecture/2026-06-16-typed-event-schemas.md index 7ee53a6cb4..a48caf11d4 100644 --- a/docs/rfc/proposed/2026-06-16-typed-event-schemas.md +++ b/docs/rfc/proposed/architecture/2026-06-16-typed-event-schemas.md @@ -6,9 +6,9 @@ Status: proposed ## Problem -The harness models its core vocabulary — content blocks, message sources, finish reasons, turn triggers, turn-end reasons, and session events — as **merge-extensible maps**: a TypeScript `interface` (e.g. `SessionEventMap`, `ContentBlockMap`) that plugins augment via declaration merging, with the public union derived as `Map[keyof Map]`. This is the repo's universal extension pattern, documented in [docs/architecture.md](../../architecture.md) ("The same merge-extensible-map pattern is used for `MessageSource`, `FinishReason`, `TurnTrigger`, and `TurnEndReason`") and relied on by the `defineTool` `InferArgs` DSL and the `assertNever` exhaustiveness convention. +The harness models its core vocabulary — content blocks, message sources, finish reasons, turn triggers, turn-end reasons, and session events — as **merge-extensible maps**: a TypeScript `interface` (e.g. `SessionEventMap`, `ContentBlockMap`) that plugins augment via declaration merging, with the public union derived as `Map[keyof Map]`. This is the repo's universal extension pattern, documented in [docs/architecture.md](../../../architecture.md) ("The same merge-extensible-map pattern is used for `MessageSource`, `FinishReason`, `TurnTrigger`, and `TurnEndReason`") and relied on by the `defineTool` `InferArgs` DSL and the `assertNever` exhaustiveness convention. -The pattern is **compile-time only**. The types vanish at runtime: there is no schema object to validate an incoming value against, parse untrusted input with, or enumerate at runtime. Two concrete consequences surfaced in review of [the session-persistence work](../implemented/2026-06-14-session-persistence.md) (#33): +The pattern is **compile-time only**. The types vanish at runtime: there is no schema object to validate an incoming value against, parse untrusted input with, or enumerate at runtime. Two concrete consequences surfaced in review of [the session-persistence work](../../implemented/architecture/2026-06-14-session-persistence.md) (#33): 1. **Persistence treats `event.data` as opaque JSON.** The JSONL/SQLite backends `JSON.stringify`/`JSON.parse` each event verbatim; the only runtime guard is `isJsonValue` (round-trip serializability — rejects BigInt, functions, cycles, non-finite numbers, …), NOT structural validation. A corrupted-but-still-JSON event datum (wrong field types, missing fields) round-trips silently and is only caught later, if at all, by a consumer's `switch`. 2. **No runtime contract for plugin-added variants.** A plugin that declaration-merges a new `SessionEventMap` key gets compile-time typing for its own code, but nothing validates that the values it produces match the shape it declared — at the producer, at the persistence boundary, or on reload. @@ -32,7 +32,7 @@ A migration of the event/vocabulary surface to runtime schemas touches, at minim - **The event producers** — 16 `session.append(...)` call sites in the loop — unchanged in shape but now validated at the boundary. - **~7 switch-consumers** that branch on these unions: `deriveMessages` (`dsh-session`), `BlockAssembler` (`dsh-llm`), the `dsh-invariants` plugin, both LLM adapters (`dsh-llm-deepseek`, `dsh-llm-pi-ai`), and the tool schema layer (`dsh-tools`). The `assertNever`-on-closed-unions vs fall-through-on-extensible-unions convention (a documented lint rule) would need rethinking — runtime variants are not statically exhaustive. - **The `defineTool` `InferArgs` DSL** (`dsh-tools`), which derives zero-cast `execute` arg types from a compile-time schema spec — the showcase of the current approach. -- **Docs**: architecture.md (the pattern is described as foundational), [dev-mode invariants](../implemented/2026-06-11-dev-invariants-over-deep-readonly.md), and any RFC that references the pattern. +- **Docs**: architecture.md (the pattern is described as foundational), [dev-mode invariants](../../implemented/architecture/2026-06-11-dev-invariants-over-deep-readonly.md), and any RFC that references the pattern. This is a HUGE change. It is not in scope for the RFC-009 session-persistence work and must not be smuggled in through it. diff --git a/docs/rfc/proposed/2026-06-20-generic-long-running-tool-runtime.md b/docs/rfc/proposed/architecture/2026-06-20-generic-long-running-tool-runtime.md similarity index 85% rename from docs/rfc/proposed/2026-06-20-generic-long-running-tool-runtime.md rename to docs/rfc/proposed/architecture/2026-06-20-generic-long-running-tool-runtime.md index 825fb133fe..4f034e3020 100644 --- a/docs/rfc/proposed/2026-06-20-generic-long-running-tool-runtime.md +++ b/docs/rfc/proposed/architecture/2026-06-20-generic-long-running-tool-runtime.md @@ -6,7 +6,7 @@ Status: proposed The bash capability seam supports both foreground commands and long-running background tasks. Background support is large: the abstract executor exposes `start`, `get`, `ownerOf`, `list`, `readOutput`, `kill`, and `onTaskDone`; the local executor tracks tasks, incremental reads, owner tokens, process cleanup, and completion listeners; the model sees three tools (`bash`, `bash_output`, `bash_kill`); the tool plugin injects completion notices back into the owning agent's session. The local executor fences task access behind owner tokens because predictable global task ids are a cross-session read/kill hazard. -The [tool cookbook](../../cookbook/adding-a-tool.md) already points at the real design smell: background bash is really generic long-running-tool infrastructure living inside one tool. If future tools need background execution, polling, kill, ownership, and completion notices, those semantics should not be hidden in `dsh-bash`. +The [tool cookbook](../../../cookbook/adding-a-tool.md) already points at the real design smell: background bash is really generic long-running-tool infrastructure living inside one tool. If future tools need background execution, polling, kill, ownership, and completion notices, those semantics should not be hidden in `dsh-bash`. ## Proposal @@ -28,7 +28,7 @@ The runtime should own: - A shared long-running-task service or tool layer owns those semantics and is documented as the path for any future background-capable tool. - Bash background behavior remains available through the shared layer, with tests proving cross-session isolation still holds. - ACP and snapshot fixtures render background bash through the shared task vocabulary, not through bash-only lifecycle semantics. -- The [tool cookbook](../../cookbook/adding-a-tool.md) points long-running tools at the shared runtime instead of telling each tool to invent its own task protocol. +- The [tool cookbook](../../../cookbook/adding-a-tool.md) points long-running tools at the shared runtime instead of telling each tool to invent its own task protocol. ## What we give up diff --git a/docs/rfc/proposed/2026-06-20-package-hierarchy.md b/docs/rfc/proposed/architecture/2026-06-20-package-hierarchy.md similarity index 88% rename from docs/rfc/proposed/2026-06-20-package-hierarchy.md rename to docs/rfc/proposed/architecture/2026-06-20-package-hierarchy.md index 7351cefc93..5eccadaef8 100644 --- a/docs/rfc/proposed/2026-06-20-package-hierarchy.md +++ b/docs/rfc/proposed/architecture/2026-06-20-package-hierarchy.md @@ -4,7 +4,7 @@ Status: proposed ## Problem -`packages/` is flat. Core product packages, provider integrations, capability seams, example UI support, and snapshot-only replay support all sit at the same level and look equally foundational. The [package README](../../../packages/README.md) already has a `FIXME(package-hierarchy)` noting that `ui-stdio` and `llm-replay` were extracted from examples mostly for reuse and coverage. The flat layout makes support packages appear more product-shaped than they are and forces publish/lint/doc scripts to encode intent through comments or static lists. +`packages/` is flat. Core product packages, provider integrations, capability seams, example UI support, and snapshot-only replay support all sit at the same level and look equally foundational. The [package README](../../../../packages/README.md) already has a `FIXME(package-hierarchy)` noting that `ui-stdio` and `llm-replay` were extracted from examples mostly for reuse and coverage. The flat layout makes support packages appear more product-shaped than they are and forces publish/lint/doc scripts to encode intent through comments or static lists. This is not just cosmetic. A package's location currently says little about whether it is core API, a swappable capability, an adapter integration, an example harness helper, or test infrastructure. That makes future removal harder because every top-level package looks like part of the same public surface. diff --git a/docs/rfc/proposed/architecture/2026-06-20-providerless-example-base.md b/docs/rfc/proposed/architecture/2026-06-20-providerless-example-base.md new file mode 100644 index 0000000000..2061248596 --- /dev/null +++ b/docs/rfc/proposed/architecture/2026-06-20-providerless-example-base.md @@ -0,0 +1,27 @@ +# RFC: Make the shared example base providerless + +Status: proposed + +## Problem + +The examples have two shared base files: [examples/base-core.yml](../../../../examples/base-core.yml) is providerless, while [examples/base.yml](../../../../examples/base.yml) includes that core plus the real `llm-deepseek` adapter. Snapshot replay needs the providerless core with `llm-replay`, because loading the real adapter without a key throws. The normal demos need the real adapter. The result is a naming inversion: the file named `base.yml` is not the reusable base for all examples, while the true base is `base-core.yml`. + +The split is understandable, but it makes every config explanation longer. It also leads to awkward test setup like a keyless smoke test carrying a dummy API key so an adapter can boot even though the model is not called. + +## Proposal + +Rename the providerless core to [examples/base.yml](../../../../examples/base.yml) and make adapter selection explicit in each concrete example. The coding and ACP real configs add a tiny `llm-deepseek` include or local block; snapshot config adds `llm-replay`. Delete [examples/base-core.yml](../../../../examples/base-core.yml). + +The shared base should contain only provider-neutral services and tools: `llm`, sessions, system prompt, tools, agents, invariants, bash executor, and bash tool schemas. Anything that chooses a model provider belongs at the leaf config. + +## Acceptance criteria + +- [examples/base.yml](../../../../examples/base.yml) is providerless. +- [examples/base-core.yml](../../../../examples/base-core.yml) is deleted. +- Real demo configs explicitly add the DeepSeek adapter. +- Snapshot replay config includes the same providerless base and its replay adapter. +- The [examples README](../../../../examples/README.md), example-specific READMEs, and RFC references stop explaining "base = base-core plus adapter". + +## What we give up + +Real demos lose one layer of convenience: each must opt into the adapter. That is the right default for examples, because adapter choice is the variable part and providerless wiring is the shared product core. diff --git a/docs/rfc/proposed/2026-06-14-acp-agent-client-protocol.md b/docs/rfc/proposed/feature/2026-06-14-acp-agent-client-protocol.md similarity index 70% rename from docs/rfc/proposed/2026-06-14-acp-agent-client-protocol.md rename to docs/rfc/proposed/feature/2026-06-14-acp-agent-client-protocol.md index 3dc779ffa4..7056c74977 100644 --- a/docs/rfc/proposed/2026-06-14-acp-agent-client-protocol.md +++ b/docs/rfc/proposed/feature/2026-06-14-acp-agent-client-protocol.md @@ -11,11 +11,11 @@ The coding agent is reachable only through the readline `stdio-chat` plugin: it Editors are converging on the Agent Client Protocol (ACP), which Zed and others speak: JSON-RPC 2.0 over newline-delimited stdio, modeled on the Language Server Protocol. An editor boots the agent as a subprocess and exchanges `initialize` / `session/new` / `session/prompt`, rendering streamed `session/update` notifications and `session/request_permission` prompts. The goal is for the agent to be a drop-in ACP server — implement the protocol once and run in any ACP client, with no per-editor glue. -This RFC has a hard prerequisite on [session persistence](../implemented/2026-06-14-session-persistence.md): it assumes durable session persistence (the `SessionPersistence` service and the async `AgentLoop.resume` seam) is implemented, so resuming a session via `session/load` is in scope. None of those APIs exist yet — `AgentLoop` currently exposes only the synchronous `create` — so ACP must land after, or in the same change as, [session persistence](../implemented/2026-06-14-session-persistence.md), and pins to its `resume(agentId, resumeSessionId)` contract. Session persistence persists every `SessionEvent` verbatim (including `assistant/chunk`), so a loaded session has the stream chunks needed to replay turns to the client. +This RFC has a hard prerequisite on [session persistence](../../implemented/architecture/2026-06-14-session-persistence.md): it assumes durable session persistence (the `SessionPersistence` service and the async `AgentLoop.resume` seam) is implemented, so resuming a session via `session/load` is in scope. None of those APIs exist yet — `AgentLoop` currently exposes only the synchronous `create` — so ACP must land after, or in the same change as, [session persistence](../../implemented/architecture/2026-06-14-session-persistence.md), and pins to its `resume(agentId, resumeSessionId)` contract. Session persistence persists every `SessionEvent` verbatim (including `assistant/chunk`), so a loaded session has the stream chunks needed to replay turns to the client. ## Proposal -A new plugin package `@deepseek-ai/dsh-acp` — a client-driver / UI plugin, the structured analogue of `stdio-chat`. It is NOT a change to the loop and NOT an [capability seams](../implemented/2026-06-13-capability-seams.md) interface/implementation/consumer capability split; it consumes the existing `agent/*` event taxonomy and the `tools/execute` waterfall. +A new plugin package `@deepseek-ai/dsh-acp` — a client-driver / UI plugin, the structured analogue of `stdio-chat`. It is NOT a change to the loop and NOT an [capability seams](../../implemented/architecture/2026-06-13-capability-seams.md) interface/implementation/consumer capability split; it consumes the existing `agent/*` event taxonomy and the `tools/execute` waterfall. It depends on the official `@agentclientprotocol/sdk` (the `AgentSideConnection` class) — Apache-2.0, actively versioned. The SDK declares a `zod` peer dependency and imports `zod/v4` at runtime, so `packages/acp` must declare `zod` itself (per the workspace dependency constraints). This is the renamed successor to `@zed-industries/agent-client-protocol`, which is now deprecated on npm. @@ -25,7 +25,7 @@ The mapping between ACP and existing harness seams — each row names the seam a |---|---|---| | `initialize` | static handler | negotiate `protocolVersion` (echo the supported version, else error); advertise text-only `promptCapabilities` and `loadSession: true`; report agent name/version | | `session/new {cwd, mcpServers, additionalDirectories}` → `{sessionId}` | the `dsh-agent` create factory (see Dependency note + Plan) | the seam must accept `{ sessionId, meta }` so the ACP-generated `sessionId` becomes the live/persisted session id and the validated `cwd` is attached as the `SessionHeader` (today `AgentLoop.create(id)` hardcodes `${id}-session` and takes no metadata); reject a 2nd session (single-session MVP, see [ACP multi-session](2026-06-14-acp-multi-session.md)); `cwd` validated (require absolute) — any absolute cwd is honored: it becomes the session's `SessionHeader.cwd` and the default bash workdir (per-session cwd, see § Deferred → RESOLVED), so the server need not launch in the workspace; non-empty `mcpServers` and `additionalDirectories` are rejected for the MVP because silently ignoring requested servers/roots would desync the client's tool and filesystem-scope UI | -| `session/load {sessionId, cwd, mcpServers, additionalDirectories}` | the `dsh-agent` resume factory ([session persistence](../implemented/2026-06-14-session-persistence.md) + Dependency note) | load `{ meta, events }`, seed the session, re-derive history via `deriveMessages()`, replay prior turns to the client as `session/update` per the ACP load contract; `mcpServers` and `additionalDirectories` rejected as in `session/new` | +| `session/load {sessionId, cwd, mcpServers, additionalDirectories}` | the `dsh-agent` resume factory ([session persistence](../../implemented/architecture/2026-06-14-session-persistence.md) + Dependency note) | load `{ meta, events }`, seed the session, re-derive history via `deriveMessages()`, replay prior turns to the client as `session/update` per the ACP load contract; `mcpServers` and `additionalDirectories` rejected as in `session/new` | | `session/prompt {prompt}` | `agent.send()` (idle) | text blocks → `TextBlock`; reject image/audio per advertised capabilities; one in-flight prompt per session | | resolve `session/prompt` → `{stopReason}` | `agent/turn-end` (extended, see Plan) | map the harness kebab `TurnEndReason` to the ACP snake_case `StopReason` wire enum: `completed`→`end_turn`, `max-tokens`→`max_tokens`, `aborted`(cancel)→`cancelled`, plus `refusal`/`max_turn_requests` when applicable; honor the batch-into-one-turn and send-not-synchronously-running settle semantics | | `session/update: agent_message_chunk` | `agent/stream-chunk` `text-delta` only | do NOT also emit on `block-end(TextBlock)` — it carries the fully-assembled block and would duplicate the streamed text | @@ -35,37 +35,37 @@ The mapping between ACP and existing harness seams — each row names the seam a | `session/request_permission {sessionId, toolCall, options}` | prepended `tools/execute` listener | no-op unless `exec.agent` is ACP-owned; await the outcome; `selected/allow_*` → `next()`; `reject_*`/`cancelled` → veto `ToolExecutionResult{isError}` | | `session/cancel` (notification) | `agent.cancel(reason)` | the queue-aware cancel (abort running step, clear queued + steering, drop an about-to-start turn); settle the in-flight prompt as `cancelled`; resolve any pending permission as `cancelled` exactly once | -The permission gate is the first real consumer of the `tools/execute` veto seam (the documented "single veto/sandbox/permission seam" plus the deferred "Permission system" TODO in [docs/architecture.md](../../architecture.md)). It is a single global listener registered with `prepend: true` so it runs before any other tool wrapper. `ToolExecution.agent` is optional and the `Agent` interface carries no origin marker, so the bridge tracks ownership itself: it records each agent it creates in a `WeakMap` and the gate no-ops (calls `next()` immediately) for any `exec.agent` it does not own — non-ACP agents and the no-agent case pass straight through. For an owned agent it resolves the session, issues `session/request_permission`, and stores the pending resolver on that session's record so the outcome — or a `session/cancel`/connection-close — settles it exactly once. +The permission gate is the first real consumer of the `tools/execute` veto seam (the documented "single veto/sandbox/permission seam" plus the deferred "Permission system" TODO in [docs/architecture.md](../../../architecture.md)). It is a single global listener registered with `prepend: true` so it runs before any other tool wrapper. `ToolExecution.agent` is optional and the `Agent` interface carries no origin marker, so the bridge tracks ownership itself: it records each agent it creates in a `WeakMap` and the gate no-ops (calls `next()` immediately) for any `exec.agent` it does not own — non-ACP agents and the no-agent case pass straight through. For an owned agent it resolves the session, issues `session/request_permission`, and stores the pending resolver on that session's record so the outcome — or a `session/cancel`/connection-close — settles it exactly once. Lifecycle and disposal: the connection, listeners, and in-flight permission promises register via `ctx.effect`/`ctx.on`; teardown is async and awaits quiescence — close the connection, settle/reject pending permissions, `agent.abort()`, and wait for the agent to settle. The disposal-settle signal must come from the `dsh-agent` interface, not the loop: `agent.done` exists only on the concrete `ReactLoopAgent`, so the bridge instead observes `agent/status` reaching `idle`/`disposed` (or the RFC lifts a quiescence promise onto the `Agent` interface). Every listener contains its `send()` exceptions (log, never reject the turn) because stream chunks are emitted inside the model step, so a throwing listener would corrupt the turn. -**Dependency note (architecture rule).** [docs/architecture.md](../../architecture.md) states "plugins depend on interface packages, never on `dsh-agent-loop`." Creating and resuming agents is currently only on the concrete `AgentLoop` (`ctx.agentLoop`), so this RFC proposes adding an **abstract create/resume factory** to the `dsh-agent` interface (registry-level `create({ sessionId, meta })` / `resume(...)`), implemented by the loop, so `dsh-acp` injects only `agents` (the interface) and the dependency rule holds. The alternative — injecting the concrete `agentLoop` and recording a documented exception in the architecture doc — is explicitly the non-preferred fallback. +**Dependency note (architecture rule).** [docs/architecture.md](../../../architecture.md) states "plugins depend on interface packages, never on `dsh-agent-loop`." Creating and resuming agents is currently only on the concrete `AgentLoop` (`ctx.agentLoop`), so this RFC proposes adding an **abstract create/resume factory** to the `dsh-agent` interface (registry-level `create({ sessionId, meta })` / `resume(...)`), implemented by the loop, so `dsh-acp` injects only `agents` (the interface) and the dependency rule holds. The alternative — injecting the concrete `agentLoop` and recording a documented exception in the architecture doc — is explicitly the non-preferred fallback. ## Plan -1. Package scaffold `packages/acp/` per [the cookbook](../../cookbook/adding-a-package.md); add `@agentclientprotocol/sdk` and `zod`. Add the abstract create/resume factory to `dsh-agent` (the interface) so the bridge can `inject: ['agents', 'sessions', 'tools', 'sessionPersistence']` without depending on the concrete loop; `sessionPersistence` is required because `session/load` advertises `loadSession: true`. (Fallback only if the factory is judged not worth it: inject `agentLoop` directly and record the architecture-rule exception in `docs/architecture.md`.) +1. Package scaffold `packages/acp/` per [the cookbook](../../../cookbook/adding-a-package.md); add `@agentclientprotocol/sdk` and `zod`. Add the abstract create/resume factory to `dsh-agent` (the interface) so the bridge can `inject: ['agents', 'sessions', 'tools', 'sessionPersistence']` without depending on the concrete loop; `sessionPersistence` is required because `session/load` advertises `loadSession: true`. (Fallback only if the factory is judged not worth it: inject `agentLoop` directly and record the architecture-rule exception in `docs/architecture.md`.) 2. Connection plus `initialize`/`session/new`: wire `AgentSideConnection` to stdin/stdout; protocolVersion negotiation; the single-session guard; create the live session through the new `{ sessionId, meta }` factory seam (so the ACP `sessionId` and validated `cwd` become the session's id and header); the `sessionId↔agent` and `Session↔sessionId` maps. -3. Internal edit — turn-end reason fidelity (sanctioned: edit internals to fit ACP). Extend `TurnEndReasonMap` in the proper places: (a) declaration-merge a `max-tokens` variant in the owning package (`packages/session/src/types.ts`, alongside `completed|aborted|error|disposed`) — add `max-tokens` because `FinishReasonMap` produces it (DeepSeek maps `length` → `max-tokens`); do not add `refusal`, since no current adapter produces it (unknown DeepSeek finish reasons collapse to `error`), but leave a comment in `TurnEndReasonMap` noting `refusal` should be added when an adapter first emits it (`FinishReasonMap` is merge-extensible); (b) make `agent-loop`'s `loop.ts` populate the reason from the model `finish` chunk — `assembler.finish` lives inside `runStep`, so `runStep` must return it up to `runTurn`, and the rule is "the last step's finish reason wins, but any `max-tokens` in the turn surfaces as `max-tokens`"; (c) no consumer exhaustively switches over `TurnEndReason` today (the invariants plugin switches on `SessionEventType`, and `deriveMessages` ignores `turn/end`), so adding `max-tokens` is a non-breaking extension — but recheck before landing; (d) update [docs/architecture.md](../../architecture.md) (the CI-verified loop-lifecycle/event-taxonomy doc) and the affected package READMEs/JSDoc (`dsh-session`, `dsh-agent`, `dsh-agent-loop`) per the repo doc-sync policy. This replaces a fragile "observe the finish chunk in the bridge" hack with a real, documented contract. +3. Internal edit — turn-end reason fidelity (sanctioned: edit internals to fit ACP). Extend `TurnEndReasonMap` in the proper places: (a) declaration-merge a `max-tokens` variant in the owning package (`packages/session/src/types.ts`, alongside `completed|aborted|error|disposed`) — add `max-tokens` because `FinishReasonMap` produces it (DeepSeek maps `length` → `max-tokens`); do not add `refusal`, since no current adapter produces it (unknown DeepSeek finish reasons collapse to `error`), but leave a comment in `TurnEndReasonMap` noting `refusal` should be added when an adapter first emits it (`FinishReasonMap` is merge-extensible); (b) make `agent-loop`'s `loop.ts` populate the reason from the model `finish` chunk — `assembler.finish` lives inside `runStep`, so `runStep` must return it up to `runTurn`, and the rule is "the last step's finish reason wins, but any `max-tokens` in the turn surfaces as `max-tokens`"; (c) no consumer exhaustively switches over `TurnEndReason` today (the invariants plugin switches on `SessionEventType`, and `deriveMessages` ignores `turn/end`), so adding `max-tokens` is a non-breaking extension — but recheck before landing; (d) update [docs/architecture.md](../../../architecture.md) (the CI-verified loop-lifecycle/event-taxonomy doc) and the affected package READMEs/JSDoc (`dsh-session`, `dsh-agent`, `dsh-agent-loop`) per the repo doc-sync policy. This replaces a fragile "observe the finish chunk in the bridge" hack with a real, documented contract. 4. Prompt-turn streaming plus load: translate `agent/stream-chunk` and `session/event` into `session/update`; resolve `session/prompt` on settle, mapping the harness `TurnEndReason` to the ACP `StopReason` wire enum (`completed`→`end_turn`, `max-tokens`→`max_tokens`, `aborted`→`cancelled`) — a small total function with a test asserting the exact wire strings, since the SDK rejects an unknown `stopReason`. Concrete correlation, since the loop batches queued messages into one turn and `send()` does not synchronously flip to running: install listeners before `send()`; gate on an observed `agent/turn-start` (confirms work was accepted) then resolve on the next `agent/turn-end`; reject an empty/whitespace prompt up front rather than calling `send()` (no turn would ever start, so the RPC would hang). Implement `session/load` on the session-persistence resume seam. 5. Permission gate: a single `tools/execute` listener registered with `prepend: true`, owning a `WeakMap` of bridge-created agents; no-op (`next()`) for unowned/no-agent calls; for owned calls → `session/request_permission` → allow (`next()`) / veto; settle the stored resolver exactly once on outcome, cancel, or connection close. -6. Example wiring (extract a shared base). `@cordisjs/plugin-include` is itself a plugin entry that resets `ctx.baseUrl` and loads a path, so a child `cordis.yml` can nest-include a shared base; the extraction is safe because every dependent plugin declares `inject` (loader groups initialize via `Promise.all`, so YAML order is NOT the dependency mechanism — never rely on it). Extract the provider/tool core (`llm, sessions, system-prompt, tools, agents, invariants, llm-deepseek, bash-local, tool-bash`) into `examples/base.yml`; have both `coding-agent` and a new `examples/acp-agent/` include it and add their own UI plugin plus logger. Keep `agent-loop` per-example (NOT in the base): `AgentLoop` creates its configured agents in its constructor, and the two examples disagree — `coding-agent` needs a pre-created `main` (its `stdio-chat` calls `ctx.agents.get('main')`), while `acp-agent` must pre-create none (ACP `session/new` creates agents). So `coding-agent` declares `agent-loop` with `agents: [{ id: main, … }]` and `acp-agent` with `agents: []`. `acp-agent` loads `dsh-session-persistence-jsonl` (from [session persistence](../implemented/2026-06-14-session-persistence.md) — required for `session/load`), omits the stdout logger (see Risks), and adds `pnpm run demo:acp` plus the Zed `agent_servers` snippet. -7. Tests (the repo cares a lot here): a property-based test for the protocol shape (precedent: [property-based testing](../implemented/2026-06-11-property-based-testing.md)) — fuzz arbitrary harness event sequences and assert ACP-stream invariants (never a `tool_call_update` before its `tool_call`; exactly one `session/prompt` resolution per prompt; monotonic, well-formed ordering; `stopReason` in the legal set); codec unit tests over an in-memory `Duplex` pair (drive `AgentSideConnection` without a subprocess; assert exact frames for `initialize`, `session/new`, a full prompt turn); the mandatory HMR-safety test (dispose the fiber; assert the connection closed, all `ctx.on` listeners gone, any in-flight `request_permission` settled); failure-path tests (connection closes mid-stream; closes with a permission pending; a notification `send()` rejects but the turn survives; `finish{kind:'error'|'aborted'}`; a `tools/execute` throw with no `tool/result`; a second `session/new` rejected; a `session/prompt` while one is in flight; an empty prompt rejected without hanging; a `session/load` re-derives identical history and replays it); and an e2e (`*.e2e.ts`, self-skips without `DEEPSEEK_API_KEY`) that boots `examples/acp-agent`, connects a `ClientSideConnection`, sends a real prompt, owns and disposes the harness in `afterEach`, and verifies the world (files on disk), not the agent's self-report. -8. Docs: module/JSDoc plus a package README; extend [the extension cookbook](../../cookbook/extension-cookbook.md) with the client-driver pattern. Flip Status to `implemented` on landing; record a decision in this RFC only if it proves durable, contested, and surprising (candidates: the `tools/execute` permission-ownership rule, the npm-dependency choice) — not auto-required. +6. Example wiring (extract a shared base). `@cordisjs/plugin-include` is itself a plugin entry that resets `ctx.baseUrl` and loads a path, so a child `cordis.yml` can nest-include a shared base; the extraction is safe because every dependent plugin declares `inject` (loader groups initialize via `Promise.all`, so YAML order is NOT the dependency mechanism — never rely on it). Extract the provider/tool core (`llm, sessions, system-prompt, tools, agents, invariants, llm-deepseek, bash-local, tool-bash`) into `examples/base.yml`; have both `coding-agent` and a new `examples/acp-agent/` include it and add their own UI plugin plus logger. Keep `agent-loop` per-example (NOT in the base): `AgentLoop` creates its configured agents in its constructor, and the two examples disagree — `coding-agent` needs a pre-created `main` (its `stdio-chat` calls `ctx.agents.get('main')`), while `acp-agent` must pre-create none (ACP `session/new` creates agents). So `coding-agent` declares `agent-loop` with `agents: [{ id: main, … }]` and `acp-agent` with `agents: []`. `acp-agent` loads `dsh-session-persistence-jsonl` (from [session persistence](../../implemented/architecture/2026-06-14-session-persistence.md) — required for `session/load`), omits the stdout logger (see Risks), and adds `pnpm run demo:acp` plus the Zed `agent_servers` snippet. +7. Tests (the repo cares a lot here): a property-based test for the protocol shape (precedent: [property-based testing](../../implemented/testing/2026-06-11-property-based-testing.md)) — fuzz arbitrary harness event sequences and assert ACP-stream invariants (never a `tool_call_update` before its `tool_call`; exactly one `session/prompt` resolution per prompt; monotonic, well-formed ordering; `stopReason` in the legal set); codec unit tests over an in-memory `Duplex` pair (drive `AgentSideConnection` without a subprocess; assert exact frames for `initialize`, `session/new`, a full prompt turn); the mandatory HMR-safety test (dispose the fiber; assert the connection closed, all `ctx.on` listeners gone, any in-flight `request_permission` settled); failure-path tests (connection closes mid-stream; closes with a permission pending; a notification `send()` rejects but the turn survives; `finish{kind:'error'|'aborted'}`; a `tools/execute` throw with no `tool/result`; a second `session/new` rejected; a `session/prompt` while one is in flight; an empty prompt rejected without hanging; a `session/load` re-derives identical history and replays it); and an e2e (`*.e2e.ts`, self-skips without `DEEPSEEK_API_KEY`) that boots `examples/acp-agent`, connects a `ClientSideConnection`, sends a real prompt, owns and disposes the harness in `afterEach`, and verifies the world (files on disk), not the agent's self-report. +8. Docs: module/JSDoc plus a package README; extend [the extension cookbook](../../../cookbook/extension-cookbook.md) with the client-driver pattern. Flip Status to `implemented` on landing; record a decision in this RFC only if it proves durable, contested, and surprising (candidates: the `tools/execute` permission-ownership rule, the npm-dependency choice) — not auto-required. Deferred (each names its owning future work): - Multiplexing concurrent sessions → [ACP multi-session](2026-06-14-acp-multi-session.md). - ~~`cwd` honoring.~~ **RESOLVED.** Originally there was no path from `session/new.cwd` to the bash workdir (`tool-bash` forwarded only an explicit `args.workdir`; `LocalBashExecutor.resolve` defaulted to its own config or `process.cwd()`), so the MVP validated `cwd` (require absolute) AND required the server to launch in the workspace root, erroring on a mismatch. This is now lifted: the validated `cwd` is stored as `SessionHeader.cwd`, and `dsh-tool-bash` defaults the bash workdir to the calling agent's `session.header.cwd` (an explicit model `workdir` still wins; a relative one resolves against it). Any absolute `cwd` is honored — the server need not launch in the workspace, and N sessions can each target a different directory. Widening scope beyond the single cwd (`additionalDirectories`) remains deferred. -- Client `terminal/*` proxying (a live editor terminal) and `fs/*` (editor-rendered diffs) — a future `BashExecutor` over the [capability seams](../implemented/2026-06-13-capability-seams.md) bash seam, gated on `clientCapabilities.terminal`. +- Client `terminal/*` proxying (a live editor terminal) and `fs/*` (editor-rendered diffs) — a future `BashExecutor` over the [capability seams](../../implemented/architecture/2026-06-13-capability-seams.md) bash seam, gated on `clientCapabilities.terminal`. - Image/audio prompts (blocked on the DeepSeek adapter, which skips `image` blocks today), modes, auth, `available_commands`/slash-commands, `plan`, and `usage_update`. ## Risks stdout is the protocol — guaranteed by config, not by monkey-patching. The console logger writes through `console.log` to stdout, so any stdout UI/logger plugin corrupts JSON-RPC. The guarantee is config-only: the `acp-agent` example loads no stdout plugin (no console logger, no `stdio-chat`) and, if logging is wanted, uses a stderr exporter. A defensive process-wide `process.stdout.write`/`console.log` hijack inside `dsh-acp` is explicitly rejected — it lives outside Cordis' effect-scoped, HMR-friendly plugin model, races the connection's own stdout handoff, and fights the logger. A test asserts the example emits only framed JSON-RPC on stdout. -New third-party runtime dependency plus protocol drift: `@agentclientprotocol/sdk` is young (0.25.x, recently renamed) and evolving. Pin the version and isolate churn to the one bridge package. This is not a vendoring-policy violation — [vendoring Cordis as source](../implemented/2026-06-11-vendor-cordis-as-source.md) vendors the framework; genuine third-party deps already live on npm (`@earendil-works/pi-ai`). +New third-party runtime dependency plus protocol drift: `@agentclientprotocol/sdk` is young (0.25.x, recently renamed) and evolving. Pin the version and isolate churn to the one bridge package. This is not a vendoring-policy violation — [vendoring Cordis as source](../../implemented/process/2026-06-11-vendor-cordis-as-source.md) vendors the framework; genuine third-party deps already live on npm (`@earendil-works/pi-ai`). -Turn-settle and prompt-correlation hazards: honor "queued messages batch into one turn" and "`send()` does not synchronously flip to running" (see `stdio-chat.ts` and the defensive-patterns section of [docs/architecture.md](../../architecture.md)); gate resolution on an observed running→idle transition and handle the empty-prompt / no-work branch so an RPC can't hang. +Turn-settle and prompt-correlation hazards: honor "queued messages batch into one turn" and "`send()` does not synchronously flip to running" (see `stdio-chat.ts` and the defensive-patterns section of [docs/architecture.md](../../../architecture.md)); gate resolution on an observed running→idle transition and handle the empty-prompt / no-work branch so an RPC can't hang. Permission-await and disposal hangs: a pending `request_permission` whose connection closes or whose turn aborts must settle exactly once; disposal must reach quiescence (observe the interface-level settle signal — `agent/status` reaching `idle`/`disposed`, since `agent.done` is `ReactLoopAgent`-only), not orphan awaits on a closed pipe. diff --git a/docs/rfc/proposed/2026-06-14-acp-multi-session.md b/docs/rfc/proposed/feature/2026-06-14-acp-multi-session.md similarity index 90% rename from docs/rfc/proposed/2026-06-14-acp-multi-session.md rename to docs/rfc/proposed/feature/2026-06-14-acp-multi-session.md index 3b73726607..faa8a48f65 100644 --- a/docs/rfc/proposed/2026-06-14-acp-multi-session.md +++ b/docs/rfc/proposed/feature/2026-06-14-acp-multi-session.md @@ -3,15 +3,15 @@ Status: proposed -> **Implementation status:** the multi-session bridge (steps 1, 3, 4) and the bash task-ownership isolation are implemented in `packages/acp` + `packages/tool-bash`. **Per-session *permission* ownership is deferred** — it depends on [the ACP support permission gate](2026-06-14-acp-agent-client-protocol.md) (`TODO(rfc010-permission-gate)`), which is itself deferred; the `agent→sessionId` reverse map the gate will route through is in place. Step 2's per-session disposer scope is now implemented (see [agent lifecycle & ownership seams](../implemented/2026-06-18-agent-lifecycle-and-ownership-seams.md)): the factory returns a per-agent `AgentHandle` whose `dispose()` stops the loop, awaits quiescence, unregisters the agent, and removes its session, so a bare client disconnect leaves no registered agent or session-store entry. Status stays `proposed` until per-session permission ownership lands. +> **Implementation status:** the multi-session bridge (steps 1, 3, 4) and the bash task-ownership isolation are implemented in `packages/acp` + `packages/tool-bash`. **Per-session *permission* ownership is deferred** — it depends on [the ACP support permission gate](2026-06-14-acp-agent-client-protocol.md) (`TODO(rfc010-permission-gate)`), which is itself deferred; the `agent→sessionId` reverse map the gate will route through is in place. Step 2's per-session disposer scope is now implemented (see [agent lifecycle & ownership seams](../../implemented/architecture/2026-06-18-agent-lifecycle-and-ownership-seams.md)): the factory returns a per-agent `AgentHandle` whose `dispose()` stops the loop, awaits quiescence, unregisters the agent, and removes its session, so a bare client disconnect leaves no registered agent or session-store entry. Status stays `proposed` until per-session permission ownership lands. -> **Target-client note:** Zed is the current target ACP client, and its ACP client maintains a `HashMap` plus `pending_sessions` for concurrent `session/load` calls. The competing simplification to return to one live session per connection was rejected after checking that target-client shape; this RFC remains the path for finishing multiplexing and per-session permission ownership. See [the rejected simplification](../rejected/2026-06-20-single-session-acp-bridge.md). +> **Target-client note:** Zed is the current target ACP client, and its ACP client maintains a `HashMap` plus `pending_sessions` for concurrent `session/load` calls. The competing simplification to return to one live session per connection was rejected after checking that target-client shape; this RFC remains the path for finishing multiplexing and per-session permission ownership. See [the rejected simplification](../../rejected/simplification/2026-06-20-single-session-acp-bridge.md). ## Problem [ACP support](2026-06-14-acp-agent-client-protocol.md) ships with a single active session per connection: a second `session/new` is rejected. Editors expect to run several conversations over one agent subprocess — a user opens multiple threads, or a client pre-warms sessions. The single-session guard is a deliberate MVP scope cut, not an architectural limit; this RFC lifts it. -This paragraph is historical: the multi-session bridge has landed. The remaining proposed work is per-session permission ownership plus the lifecycle seams now tracked in [agent lifecycle and ownership seams](../implemented/2026-06-18-agent-lifecycle-and-ownership-seams.md). +This paragraph is historical: the multi-session bridge has landed. The remaining proposed work is per-session permission ownership plus the lifecycle seams now tracked in [agent lifecycle and ownership seams](../../implemented/architecture/2026-06-18-agent-lifecycle-and-ownership-seams.md). ## Proposal diff --git a/docs/rfc/proposed/2026-06-15-optional-code-mode.md b/docs/rfc/proposed/feature/2026-06-15-optional-code-mode.md similarity index 85% rename from docs/rfc/proposed/2026-06-15-optional-code-mode.md rename to docs/rfc/proposed/feature/2026-06-15-optional-code-mode.md index 598986a521..eb41750d1d 100644 --- a/docs/rfc/proposed/2026-06-15-optional-code-mode.md +++ b/docs/rfc/proposed/feature/2026-06-15-optional-code-mode.md @@ -6,7 +6,7 @@ Status: proposed ## Problem -Today the agent loop advertises every registered tool to the model as a native JSON-schema function definition. `ToolRegistry` feeds its schemas into `ctx.systemPrompt`, the loop puts them on `GenerateOptions.tools`, and the adapter serializes them to the provider's function-calling wire format. The model then invokes one `tool-call` block per step, the loop dispatches each call through `ctx.tools.execute()` **sequentially** (parallel tool execution is an explicit open TODO in `dsh-tools` and [docs/architecture.md](../../architecture.md)), and **every** intermediate `tool-result` re-enters the model's context on the next request. +Today the agent loop advertises every registered tool to the model as a native JSON-schema function definition. `ToolRegistry` feeds its schemas into `ctx.systemPrompt`, the loop puts them on `GenerateOptions.tools`, and the adapter serializes them to the provider's function-calling wire format. The model then invokes one `tool-call` block per step, the loop dispatches each call through `ctx.tools.execute()` **sequentially** (parallel tool execution is an explicit open TODO in `dsh-tools` and [docs/architecture.md](../../../architecture.md)), and **every** intermediate `tool-result` re-enters the model's context on the next request. For multi-step tool work this is token-heavy and serial. The model cannot compose tools — loop over a result set, branch on an intermediate value, fan out, post-process — without a full model round-trip per call, and each of those round-trips drags the entire intermediate result back into context whether the model needs it or not. @@ -16,9 +16,9 @@ This RFC proposes an **optional** Code Mode for the DeepSeek Harness, covering * ## Proposal -The design follows the codebase's capability-seam pattern ([capability seams](../implemented/2026-06-13-capability-seams.md), the `bash` template) as a three-package split, plus one consumer plugin. Nothing in `dsh-session`, `dsh-agent`, `dsh-agent-loop`, `dsh-llm`, `dsh-tools`, or `dsh-system-prompt` changes. +The design follows the codebase's capability-seam pattern ([capability seams](../../implemented/architecture/2026-06-13-capability-seams.md), the `bash` template) as a three-package split, plus one consumer plugin. Nothing in `dsh-session`, `dsh-agent`, `dsh-agent-loop`, `dsh-llm`, `dsh-tools`, or `dsh-system-prompt` changes. -**Prior art.** `@cloudflare/codemode` validates this shape directly and several of its decisions are adopted below. Its `Executor` interface is deliberately tiny — `execute(code, fns) → { result, error?, logs? }` — with a production `DynamicWorkerExecutor` (isolated Workers) and a six-line `NodeVMExecutor` example as two implementations behind it: exactly the interface/implementation split [the capability-seam pattern](../implemented/2026-06-13-capability-seams.md) prescribes. It generates TypeScript type definitions from tools for the model's context and runs the generated JavaScript in a sandbox, capturing console output alongside the return value. It normalizes model output into an async arrow function via AST parsing (acorn) and sanitizes tool names into valid JS identifiers (`my-tool` → `my_tool`, `delete` → `delete_`). It blocks outbound network by default. The transferable lessons — minimal executor contract, host-side type derivation, capture-output-and-return-value, name sanitization, AST-normalize the code, isolate by default — are folded into the design below. What does **not** transfer is the substrate: Cloudflare's isolation is Workers-specific; our equivalent hardened substrate is the deferred follow-up. +**Prior art.** `@cloudflare/codemode` validates this shape directly and several of its decisions are adopted below. Its `Executor` interface is deliberately tiny — `execute(code, fns) → { result, error?, logs? }` — with a production `DynamicWorkerExecutor` (isolated Workers) and a six-line `NodeVMExecutor` example as two implementations behind it: exactly the interface/implementation split [the capability-seam pattern](../../implemented/architecture/2026-06-13-capability-seams.md) prescribes. It generates TypeScript type definitions from tools for the model's context and runs the generated JavaScript in a sandbox, capturing console output alongside the return value. It normalizes model output into an async arrow function via AST parsing (acorn) and sanitizes tool names into valid JS identifiers (`my-tool` → `my_tool`, `delete` → `delete_`). It blocks outbound network by default. The transferable lessons — minimal executor contract, host-side type derivation, capture-output-and-return-value, name sanitization, AST-normalize the code, isolate by default — are folded into the design below. What does **not** transfer is the substrate: Cloudflare's isolation is Workers-specific; our equivalent hardened substrate is the deferred follow-up. **Prompt-budget tradeoff (Code Mode is not unconditionally cheaper).** Deriving the SDK types host-side costs no extra *discovery* round-trip, but the generated `.d.ts` is injected into the system prompt (§3a), so the type definitions themselves **do** consume context — and for an all-tools SDK that cost scales with every registered tool and can be comparable to, or larger than, the native JSON schemas it replaces. Code Mode's saving is on the **output/result** side (the model curates what comes back; intermediate results never re-enter context) and on **round-trips** (compose many calls in one program), not on the input-side tool description. The net win is workload-dependent: it pays off for multi-call, large-intermediate-result workflows and can cost *more* for a single call against a large tool surface. The `.d.ts` section is a prefix-stable prompt prefix, so prompt caching amortizes its per-turn cost across a session; the RFC notes that caching is what keeps the injected SDK affordable, and that a deployment with a very large tool surface should weigh the SDK size against native schemas rather than assume Code Mode is strictly cheaper. @@ -29,7 +29,7 @@ The design follows the codebase's capability-seam pattern ([capability seams](.. - a readonly `safe: boolean` on the `CodeRuntime` service — `false` for an unsandboxed stub, `true` only for a real isolating substrate; consumers gate on it (§2). - `SdkBinding = { namespace: string; fns: Record Promise> }` -Per the "explicit > implicit at seams" convention, the request spells out every field the runtime acts on; defaulting (e.g. an output cap, a timeout derived from `signal`) is the implementation's explicit job, not a hidden `?? default` inside `run()`. The split into interface + implementation is justified under [the capability-seam pattern](../implemented/2026-06-13-capability-seams.md) because there is **genuinely more than one planned implementation** — the node:vm stub *and* the hardened substrate (a real isolate, or the generated program run as a sandboxed process through the existing `ctx.bash` seam) that is scheduled follow-up work, not speculative optionality. The capability-seam pattern warns against splitting preemptively when only one implementation is conceivable; here a second is not just conceivable but required before any untrusted use, so the seam earns its keep. +Per the "explicit > implicit at seams" convention, the request spells out every field the runtime acts on; defaulting (e.g. an output cap, a timeout derived from `signal`) is the implementation's explicit job, not a hidden `?? default` inside `run()`. The split into interface + implementation is justified under [the capability-seam pattern](../../implemented/architecture/2026-06-13-capability-seams.md) because there is **genuinely more than one planned implementation** — the node:vm stub *and* the hardened substrate (a real isolate, or the generated program run as a sandboxed process through the existing `ctx.bash` seam) that is scheduled follow-up work, not speculative optionality. The capability-seam pattern warns against splitting preemptively when only one implementation is conceivable; here a second is not just conceivable but required before any untrusted use, so the seam earns its keep. **Backends can differ by language/runtime, not only by trust level.** The two implementations above (unsafe stub vs. hardened substrate) differ along the *trust* axis while staying TypeScript/JS, but nothing in the `CodeRuntime` contract — a program string plus a set of named async SDK bindings in, and a `{ result, logs, error? }` out — is bound to one source language. The same seam can host backends that differ along the *language* axis, executing a program written in something other than TypeScript. Two illustrative directions: @@ -38,7 +38,7 @@ Per the "explicit > implicit at seams" convention, the request spells out every These are illustrations of the seam's reach, **not commitments** — the MVP ships only the TypeScript path. The honest caveat is that the *execution* contract is language-agnostic but the *presentation* is not: the SDK-generation pipeline below (§3a and the `jsonSchemaToTs` codegen, which emits a TypeScript `.d.ts`) is TypeScript-specific, so a non-TS backend pairs the shared `CodeRuntime` contract with its own language-appropriate SDK generator and system-prompt section (a `.pyi` stub and Python usage instructions for the Python backend, AssemblyScript-flavored types for that one). The runtime seam is reused as-is; only the codegen/prompt half is per-language. -**2. Implementation package `packages/code-runtime-vm/`** — a new package `@deepseek-ai/dsh-code-runtime-vm`, the `node:vm` reference stub. It type-erases the model's TypeScript via the compiler's `transpileModule` (or sucrase) — the types exist only to guide the model; the runtime is plain JS — then wraps the body in an async IIFE for top-level `await` (Cloudflare's `NodeVMExecutor` does literally `new AsyncFunction("codemode", "return await (${code})()")`), runs it in a `vm.Context` whose globals are a capturing `console` and the SDK namespace objects, awaits the IIFE, and captures the return value, the buffered logs, and any thrown error (as `error: string`). It applies an **output cap** (truncate captured logs) and a **timeout tied to `request.signal`**. These caps limit blast radius; **they are not a security boundary**. node:vm is **not** isolation: withholding `require`/`process` does not contain anything (code escapes via `constructor`/prototype reflection), and per [AGENTS.md](../../../AGENTS.md) the harness must never hand model output the ambient environment. +**2. Implementation package `packages/code-runtime-vm/`** — a new package `@deepseek-ai/dsh-code-runtime-vm`, the `node:vm` reference stub. It type-erases the model's TypeScript via the compiler's `transpileModule` (or sucrase) — the types exist only to guide the model; the runtime is plain JS — then wraps the body in an async IIFE for top-level `await` (Cloudflare's `NodeVMExecutor` does literally `new AsyncFunction("codemode", "return await (${code})()")`), runs it in a `vm.Context` whose globals are a capturing `console` and the SDK namespace objects, awaits the IIFE, and captures the return value, the buffered logs, and any thrown error (as `error: string`). It applies an **output cap** (truncate captured logs) and a **timeout tied to `request.signal`**. These caps limit blast radius; **they are not a security boundary**. node:vm is **not** isolation: withholding `require`/`process` does not contain anything (code escapes via `constructor`/prototype reflection), and per [AGENTS.md](../../../../AGENTS.md) the harness must never hand model output the ambient environment. **The unsafe-runtime guard is enforceable, not a README warning.** Because a README caveat is not a control — and AGENTS.md's "never hand model output ambient authority" is a hard rule, not advice — the design makes the danger refuse to run by construction. Two layers: @@ -63,11 +63,11 @@ These are illustrations of the seam's reach, **not commitments** — the MVP shi **Sub-call CallIds.** Real tool calls dispatched from inside `run_code` need ids, but `CallId` is normally provider-issued (a branded string for correlating a call with its result — only brand-wrapped via `CallId()`, with no generator and no documented session-global-uniqueness guarantee). The plugin mints deterministic sub-ids scoped to the parent: `` `${exec.callId}:code:${n}` `` with a per-run counter `n`. These are unique within one `run_code` run (assuming the parent `callId` is unique, which the provider guarantees per turn); the `code/dispatch` event additionally carries the session log's `seq` so the UI and persistence can order and disambiguate globally without relying on the id alone. `ToolExecution.agent` is optional; the normal loop always supplies it (and with it `exec.agent.session`, the log `code/dispatch` appends to). A `run_code` execution arriving without `exec.agent` still runs (sub-calls propagate `agent: undefined`, exactly as the loop's own contract allows) but **skips session-log observability** — with no session to append to, those direct runs are simply not logged. -**Observability without context cost.** Each sub-dispatch emits a session event **declared by the `dsh-code-mode` plugin itself** via `SessionEventMap` declaration merging (the map is merge-extensible precisely so plugins can add events without touching `dsh-session`). Shape: `code/dispatch` with `{ parentCallId, subCallId, name, arguments (or redacted), isError, summary }`, ordered by the session log's own `seq`. `deriveMessages()` does **not** translate it into a model message — an unknown event type falls through its `default`, per the merge-extensible-union convention — so the UI and persistence ([session persistence](../implemented/2026-06-14-session-persistence.md)) can render every sub-call while the model's context only ever receives the single `run_code` tool-result. Because the event lives in the plugin, this adds no core change. +**Observability without context cost.** Each sub-dispatch emits a session event **declared by the `dsh-code-mode` plugin itself** via `SessionEventMap` declaration merging (the map is merge-extensible precisely so plugins can add events without touching `dsh-session`). Shape: `code/dispatch` with `{ parentCallId, subCallId, name, arguments (or redacted), isError, summary }`, ordered by the session log's own `seq`. `deriveMessages()` does **not** translate it into a model message — an unknown event type falls through its `default`, per the merge-extensible-union convention — so the UI and persistence ([session persistence](../../implemented/architecture/2026-06-14-session-persistence.md)) can render every sub-call while the model's context only ever receives the single `run_code` tool-result. Because the event lives in the plugin, this adds no core change. **SDK codegen.** A pure `jsonSchemaToTs(schema)` in `code-mode` maps the JSON-schema subset the `defineTool` DSL produces (object/string/number/boolean/array, `properties`, `required[]`, `enum` → string-literal union, nested objects, array `items`) to a TS type literal. It is **total**: any unsupported construct (`$ref`, `oneOf`/`anyOf`, `integer`, `null`, `additionalProperties`, or any raw MCP shape it does not recognize) degrades to `unknown` without throwing — it never crashes codegen. Typing is best-effort, not a guarantee, because MCP tools accept arbitrary JSON Schema and `ToolSchema.parameters` is typed only as `Record`. Because `ToolSchema.name` is an arbitrary string (not necessarily a valid TS identifier), the SDK is generated as a **namespace with quoted access** (e.g. `tools["some-mcp-tool"](args)`) plus safe camelCase aliases where the name is a clean identifier; alias collisions and TS reserved words fall back to quoted-only access (no duplicate alias emitted). This mirrors Cloudflare's `sanitizeToolName`. `run_code` itself is filtered out of the SDK. The MVP surfaces text content only; image and other block types in sub-results are deferred (noted as a limitation). -**Concurrency — serialized by default (the binding must enforce it).** The SDK functions are async, so a model writing `await Promise.all([tools.a(...), tools.b(...)])` would *start both* immediately, and each would call `ctx.tools.execute` right away — i.e. the binding shape makes concurrent dispatch the **default**, not an opt-in. Because the tool contract carries **no concurrency-safety metadata today** (parallel tool execution and a concurrency-safety hint are an open TODO in both `dsh-tools` and [docs/architecture.md](../../architecture.md): "phase 1 executes tool calls sequentially"), concurrent dispatch through a not-yet-hardened tool may race. So a prose "may serialize" is not sufficient. **Decision: the MVP SDK bindings enforce serialization** — each `run_code` invocation owns a per-run dispatch queue, and every `invoke()` chains onto it (`tail = tail.then(() => ctx.tools.execute(...))`), so even `Promise.all` over SDK calls executes them one at a time in submission order. This is a hard acceptance criterion, with a test that issues `Promise.all([...])` from a program and asserts the underlying `ctx.tools.execute` calls did **not** overlap (e.g. a probe tool records enter/exit and the test asserts no interleaving). The `.d.ts` may *describe* the model-visible functions as async (they are), but the implementation guarantees serial execution. Lifting serialization is deferred: only once a tool can declare itself read-only / concurrency-safe does the binding allow those specific tools to overlap. The same per-run queue is where the before/after abort checks (§3c) live, so an aborted run drains no further queued dispatches. +**Concurrency — serialized by default (the binding must enforce it).** The SDK functions are async, so a model writing `await Promise.all([tools.a(...), tools.b(...)])` would *start both* immediately, and each would call `ctx.tools.execute` right away — i.e. the binding shape makes concurrent dispatch the **default**, not an opt-in. Because the tool contract carries **no concurrency-safety metadata today** (parallel tool execution and a concurrency-safety hint are an open TODO in both `dsh-tools` and [docs/architecture.md](../../../architecture.md): "phase 1 executes tool calls sequentially"), concurrent dispatch through a not-yet-hardened tool may race. So a prose "may serialize" is not sufficient. **Decision: the MVP SDK bindings enforce serialization** — each `run_code` invocation owns a per-run dispatch queue, and every `invoke()` chains onto it (`tail = tail.then(() => ctx.tools.execute(...))`), so even `Promise.all` over SDK calls executes them one at a time in submission order. This is a hard acceptance criterion, with a test that issues `Promise.all([...])` from a program and asserts the underlying `ctx.tools.execute` calls did **not** overlap (e.g. a probe tool records enter/exit and the test asserts no interleaving). The `.d.ts` may *describe* the model-visible functions as async (they are), but the implementation guarantees serial execution. Lifting serialization is deferred: only once a tool can declare itself read-only / concurrency-safe does the binding allow those specific tools to overlap. The same per-run queue is where the before/after abort checks (§3c) live, so an aborted run drains no further queued dispatches. **Tool visibility tiers (design intentionally skipped).** A natural extension is to mark each tool with a *visibility tier*: some tools "direct-call eligible" (still offered as native wire tools alongside `run_code`), some "code-mode only" (reachable solely from within a `run_code` program, never on the wire), and the default "both." This would let a deployment keep a few high-frequency or approval-gated tools as direct calls while routing the long tail through Code Mode, or hide composition-only primitives from the native surface entirely. This RFC notes the possibility but **intentionally skips the detailed design** — the per-tool metadata, how it interacts with the `agent/request` enforcement in 3b, and the presentation split in 3a are left to a follow-up. The MVP is the simple two-state model: Code Mode on (everything via `run_code`) or off (everything native). @@ -75,7 +75,7 @@ These are illustrations of the seam's reach, **not commitments** — the MVP shi ## Alternatives -**Result elision / summarization over native tool-calling (the narrower route).** The Problem has two halves — context bloat (every intermediate `tool-result` re-enters context) and serial composition (one tool call per round-trip). The context-bloat half can be addressed *without* any code-execution runtime: keep provider tool-calling exactly as it is, and add a plugin on the `agent/request` waterfall (or a compaction pass akin to [the session-persistence work](../implemented/2026-06-14-session-persistence.md)) that elides or summarizes older `tool-result` blocks before they re-enter the model's context — drop them past a window, replace large payloads with a digest, or keep only the blocks the model still references. This is strictly less invasive than Code Mode: no new runtime seam, no model-written programs, no new safety surface. It is the right tool if context growth is the only pain. +**Result elision / summarization over native tool-calling (the narrower route).** The Problem has two halves — context bloat (every intermediate `tool-result` re-enters context) and serial composition (one tool call per round-trip). The context-bloat half can be addressed *without* any code-execution runtime: keep provider tool-calling exactly as it is, and add a plugin on the `agent/request` waterfall (or a compaction pass akin to [the session-persistence work](../../implemented/architecture/2026-06-14-session-persistence.md)) that elides or summarizes older `tool-result` blocks before they re-enter the model's context — drop them past a window, replace large payloads with a digest, or keep only the blocks the model still references. This is strictly less invasive than Code Mode: no new runtime seam, no model-written programs, no new safety surface. It is the right tool if context growth is the only pain. It is insufficient for the **composition / round-trip** half, which is the decisive reason this RFC does not stop there. Elision still pays one model round-trip per tool call: a loop over N items is N turns, a branch on an intermediate value is a turn to fetch then a turn to act, and post-processing (filter, join, reduce) either happens in the model's head over full payloads or not at all. Code Mode collapses all of that into one program — the loop, the branch, the join run in the runtime, and only the curated result returns. Elision also cannot express fan-out or data-dependent control flow; it only shrinks what comes back. So the two are complementary, not competing: elision could even layer *under* Code Mode for the residual native-tool paths. The RFC chooses Code Mode because the round-trip/composition cost is the larger structural limit, and accepts the new code-execution surface as the price — which is exactly why the execution substrate is gated behind the enforceable safety guard (§2) and the hardened backend is a hard prerequisite for untrusted use. @@ -83,12 +83,12 @@ It is insufficient for the **composition / round-trip** half, which is the decis ## Plan -1. Scaffold the interface package `packages/code-runtime/` per [the cookbook](../../cookbook/adding-a-package.md): abstract `CodeRuntime extends Service` (`super(ctx, 'codeRuntime')`) with a readonly `safe: boolean`, the `declare module 'cordis'` ctx key, the `CodeRunRequest`/`CodeRunResult`/`SdkBinding` vocabulary, method contracts documented in JSDoc (what `run` captures, abort semantics, that an error is a result field not a throw, what `safe` means). HMR-safety test (dispose the contributing fiber, assert `ctx.codeRuntime` is gone). +1. Scaffold the interface package `packages/code-runtime/` per [the cookbook](../../../cookbook/adding-a-package.md): abstract `CodeRuntime extends Service` (`super(ctx, 'codeRuntime')`) with a readonly `safe: boolean`, the `declare module 'cordis'` ctx key, the `CodeRunRequest`/`CodeRunResult`/`SdkBinding` vocabulary, method contracts documented in JSDoc (what `run` captures, abort semantics, that an error is a result field not a throw, what `safe` means). HMR-safety test (dispose the contributing fiber, assert `ctx.codeRuntime` is gone). 2. Scaffold the implementation package `packages/code-runtime-vm/`: the node:vm stub — `safe = false`, a constructor that **throws unless given `{ unsafe: true }`**, transpile/type-erase, async-IIFE wrap, capturing `console`, SDK globals, return-value/logs/error capture, output cap, signal-tied timeout. Tests for output capture, return value, error-as-field, abort, the constructor refusal without `unsafe`, and a README documenting the "not a sandbox, trusted-only" caveat prominently. 3. Scaffold the consumer plugin `packages/code-mode/`: `jsonSchemaToTs` codegen with namespace/quoted-access + alias handling (unit tests, including non-identifier MCP names and unsupported-shape → `unknown`); the registered lazy `ctx.systemPrompt.section()` carrying the SDK `.d.ts`; the `agent/request` listener (`prepend: true`) collapsing `request.tools` to `[run_code]` after `await next()`; the **unsafe-runtime gate** (refuse to register `run_code` when `ctx.codeRuntime.safe === false` unless `allowUnsafeRuntime` is set); the `run_code` tool with the dispatch bridge (per-run serialization queue, deterministic sub-call ids, before/after abort checks, `CodeRunError` on error results); and the `code/dispatch` event declared here via `SessionEventMap` merge. Declare `inject = ['tools', 'systemPrompt', 'codeRuntime']`. 4. Tests: HMR-safety (dispose removes the tool, the section, and the listener); a waterfall test that the wire tool list is exactly `[run_code]` (spy adapter, asserting via `agent/request` and optionally `llm/stream`); an integration test that a program calling two tools returns only its printed/returned output (verify the world, not the self-report); a **serialization test** that `Promise.all([...])` over SDK calls does not overlap the underlying `ctx.tools.execute` invocations (a probe tool records enter/exit; assert no interleaving); `deriveMessages()` ignores `code/dispatch`; abort mid-program stops further dispatches; `CodeRunError` surfaces as `isError: true`; and the **unsafe-runtime refusal test** (§3, the VM-guard): with the unsafe flag unset, a non-mock agent's `run_code` is refused; with it set, the program runs. 5. Wire an example: `examples/coding-agent-code-mode` (or a config flag on the existing example) loading the trio. Running it against the node:vm stub requires both opt-ins (`VmCodeRuntime({ unsafe: true })` and `code-mode`'s `allowUnsafeRuntime`); the example sets them explicitly and comments why, or uses a mock model — a real model never reaches the unsandboxed stub without those deliberate flags. Add a `pnpm run demo:*` entry. -6. Docs: update [docs/architecture.md](../../architecture.md) (a `ctx.codeRuntime` row in the service map, a Code Mode note under the tool pipeline / capability seams sections); add a [cookbook](../../cookbook/) note on writing a `CodeRuntime` backend; and **file the follow-up RFC for the hardened execution substrate** (the isolate/sandboxed-process design, the additional-language backends sketched in §1 — AssemblyScript/WASM, Python — with their per-language SDK generators, plus the tool-visibility-tier design skipped here). On landing, move this file to `implemented/` and update its row in [the RFC index](../README.md). +6. Docs: update [docs/architecture.md](../../../architecture.md) (a `ctx.codeRuntime` row in the service map, a Code Mode note under the tool pipeline / capability seams sections); add a [cookbook](../../../cookbook) note on writing a `CodeRuntime` backend; and **file the follow-up RFC for the hardened execution substrate** (the isolate/sandboxed-process design, the additional-language backends sketched in §1 — AssemblyScript/WASM, Python — with their per-language SDK generators, plus the tool-visibility-tier design skipped here). On landing, move this file to `implemented/` and update its row in [the RFC index](../../README.md). ## Risks diff --git a/docs/rfc/proposed/2026-06-11-api-extractor-reports.md b/docs/rfc/proposed/process/2026-06-11-api-extractor-reports.md similarity index 86% rename from docs/rfc/proposed/2026-06-11-api-extractor-reports.md rename to docs/rfc/proposed/process/2026-06-11-api-extractor-reports.md index db004eb8b9..c1099a6cce 100644 --- a/docs/rfc/proposed/2026-06-11-api-extractor-reports.md +++ b/docs/rfc/proposed/process/2026-06-11-api-extractor-reports.md @@ -4,7 +4,7 @@ Status: proposed -> Split out from the original "Doc-sync and API reports" RFC (2026-06-11). Parts 1-2 (doc-block typechecking, event-taxonomy verification) shipped — see [doc-sync enforcement](../implemented/2026-06-11-doc-sync-enforcement.md). This is the deferred part 3, kept as a standalone proposal. +> Split out from the original "Doc-sync and API reports" RFC (2026-06-11). Parts 1-2 (doc-block typechecking, event-taxonomy verification) shipped — see [doc-sync enforcement](../../implemented/process/2026-06-11-doc-sync-enforcement.md). This is the deferred part 3, kept as a standalone proposal. ## Problem diff --git a/docs/rfc/proposed/2026-06-11-architectural-conformance.md b/docs/rfc/proposed/process/2026-06-11-architectural-conformance.md similarity index 83% rename from docs/rfc/proposed/2026-06-11-architectural-conformance.md rename to docs/rfc/proposed/process/2026-06-11-architectural-conformance.md index 1268e6dc49..6eaf8b03a7 100644 --- a/docs/rfc/proposed/2026-06-11-architectural-conformance.md +++ b/docs/rfc/proposed/process/2026-06-11-architectural-conformance.md @@ -6,7 +6,7 @@ Status: proposed ## Problem -Two architectural guarantees currently live only in prose: (1) nothing depends on the concrete loop package ([the microkernel promise](../implemented/2026-06-11-microkernel-event-taxonomy.md)), and (2) every LlmAdapter speaks the chunk protocol correctly. Both should be mechanical ([the quality-gates principle](../implemented/2026-06-11-quality-gates.md)). +Two architectural guarantees currently live only in prose: (1) nothing depends on the concrete loop package ([the microkernel promise](../../implemented/architecture/2026-06-11-microkernel-event-taxonomy.md)), and (2) every LlmAdapter speaks the chunk protocol correctly. Both should be mechanical ([the quality-gates principle](../../implemented/process/2026-06-11-quality-gates.md)). ## Proposal @@ -18,7 +18,7 @@ Two architectural guarantees currently live only in prose: (1) nothing depends o - `vendor/*` must not import from `packages/*`. - Layering: dsh-llm imports nothing from other dsh packages; dsh-session only dsh-llm; etc. (the dependency table in packages/README.md, enforced). -**Adapter conformance kit** in dsh-llm (`@deepseek-ai/dsh-llm/conformance`): a reusable vitest suite parameterized by an adapter factory, asserting the chunk-protocol contract — index monotonicity per block, no deltas after `block-end` for an index, exactly one `finish`, usage at most once, every `tool-call-delta` carries the call id, abort honored promptly. Run it against the mocks now; the DeepSeek V4 adapter inherits it on day one. Optionally a dev-mode `strictAdapter()` wrapper enforcing the same at runtime behind a debug flag (pairs with [the dev-mode invariants](../implemented/2026-06-11-dev-invariants-over-deep-readonly.md)). +**Adapter conformance kit** in dsh-llm (`@deepseek-ai/dsh-llm/conformance`): a reusable vitest suite parameterized by an adapter factory, asserting the chunk-protocol contract — index monotonicity per block, no deltas after `block-end` for an index, exactly one `finish`, usage at most once, every `tool-call-delta` carries the call id, abort honored promptly. Run it against the mocks now; the DeepSeek V4 adapter inherits it on day one. Optionally a dev-mode `strictAdapter()` wrapper enforcing the same at runtime behind a debug flag (pairs with [the dev-mode invariants](../../implemented/architecture/2026-06-11-dev-invariants-over-deep-readonly.md)). ## Plan diff --git a/docs/rfc/proposed/2026-06-11-supply-chain-and-vendor-drift.md b/docs/rfc/proposed/process/2026-06-11-supply-chain-and-vendor-drift.md similarity index 79% rename from docs/rfc/proposed/2026-06-11-supply-chain-and-vendor-drift.md rename to docs/rfc/proposed/process/2026-06-11-supply-chain-and-vendor-drift.md index c187c85355..e35e79f0a5 100644 --- a/docs/rfc/proposed/2026-06-11-supply-chain-and-vendor-drift.md +++ b/docs/rfc/proposed/process/2026-06-11-supply-chain-and-vendor-drift.md @@ -6,7 +6,7 @@ Status: proposed ## Problem -The vendor manifest ([the vendoring decision](../implemented/2026-06-11-vendor-cordis-as-source.md)) is enforced at commit time in the *forward* direction (vendored change ⇒ manifest update) but nothing verifies the manifest's *claims*: that vendor/ actually equals upstream-at-SHA plus exactly the logged modifications. And the handful of true npm dependencies have no advisory monitoring or update cadence. +The vendor manifest ([the vendoring decision](../../implemented/process/2026-06-11-vendor-cordis-as-source.md)) is enforced at commit time in the *forward* direction (vendored change ⇒ manifest update) but nothing verifies the manifest's *claims*: that vendor/ actually equals upstream-at-SHA plus exactly the logged modifications. And the handful of true npm dependencies have no advisory monitoring or update cadence. ## Proposal diff --git a/docs/rfc/proposed/2026-06-20-discover-package-inventory.md b/docs/rfc/proposed/process/2026-06-20-discover-package-inventory.md similarity index 59% rename from docs/rfc/proposed/2026-06-20-discover-package-inventory.md rename to docs/rfc/proposed/process/2026-06-20-discover-package-inventory.md index bb2f3e95ee..281648eb51 100644 --- a/docs/rfc/proposed/2026-06-20-discover-package-inventory.md +++ b/docs/rfc/proposed/process/2026-06-20-discover-package-inventory.md @@ -4,13 +4,13 @@ Status: proposed ## Problem -Package and gate inventories are repeated by hand. [scripts/publint-all.ts](../../../scripts/publint-all.ts) has a static list of publishable packages. The [package cookbook](../../cookbook/adding-a-package.md) tells authors to update several files. The [package README](../../../packages/README.md) carries a hand-written dependency graph. [CI](../../../.github/workflows/ci.yml) and [development docs](../../development.md) can drift from the actual `doc-sync` subcommands when new gates are added. These lists are small today, but every new package or gate creates another manual synchronization point. +Package and gate inventories are repeated by hand. [scripts/publint-all.ts](../../../../scripts/publint-all.ts) has a static list of publishable packages. The [package cookbook](../../../cookbook/adding-a-package.md) tells authors to update several files. The [package README](../../../../packages/README.md) carries a hand-written dependency graph. [CI](../../../../.github/workflows/ci.yml) and [development docs](../../../development.md) can drift from the actual `doc-sync` subcommands when new gates are added. These lists are small today, but every new package or gate creates another manual synchronization point. Static lists are appropriate when they encode policy; they are needless friction when they duplicate manifest data or layout facts that already exist in `package.json`, workspace globs, or the package hierarchy. ## Proposal -Make package/gate inventories discoverable. Publishability should come from the deliberate [package hierarchy](2026-06-20-package-hierarchy.md) plus package manifests, not from a static array in a script or the npm `private` flag. Module graph generation should read package manifests. `doc-sync` should be the one command that defines and prints its sub-gates, with docs linking to that command rather than restating a second list. +Make package/gate inventories discoverable. Publishability should come from the deliberate [package hierarchy](../architecture/2026-06-20-package-hierarchy.md) plus package manifests, not from a static array in a script or the npm `private` flag. Module graph generation should read package manifests. `doc-sync` should be the one command that defines and prints its sub-gates, with docs linking to that command rather than restating a second list. The hierarchy does not need to encode every fact about a package, but it should encode the broad maintenance policy: core/product packages, integrations, capability seams, and support/test/example packages should not all require a hand-maintained exception list before scripts can tell them apart. diff --git a/docs/rfc/proposed/2026-06-20-collapse-trace-only-session-events.md b/docs/rfc/proposed/simplification/2026-06-20-collapse-trace-only-session-events.md similarity index 100% rename from docs/rfc/proposed/2026-06-20-collapse-trace-only-session-events.md rename to docs/rfc/proposed/simplification/2026-06-20-collapse-trace-only-session-events.md diff --git a/docs/rfc/proposed/2026-06-20-drop-unconsumed-llm-adapter-change-event.md b/docs/rfc/proposed/simplification/2026-06-20-drop-unconsumed-llm-adapter-change-event.md similarity index 83% rename from docs/rfc/proposed/2026-06-20-drop-unconsumed-llm-adapter-change-event.md rename to docs/rfc/proposed/simplification/2026-06-20-drop-unconsumed-llm-adapter-change-event.md index ab7f23296b..be39827bf0 100644 --- a/docs/rfc/proposed/2026-06-20-drop-unconsumed-llm-adapter-change-event.md +++ b/docs/rfc/proposed/simplification/2026-06-20-drop-unconsumed-llm-adapter-change-event.md @@ -4,9 +4,9 @@ Status: proposed ## Problem -`LlmService.registerAdapter()` emits `llm/adapter-change` on registration and disposal ([packages/llm/src/index.ts](../../../packages/llm/src/index.ts)). Grepping `llm/adapter-change` across `packages/*/src` and `examples/*/src` finds only the declaration, emit sites, docs, and tests; no production listener subscribes to it. +`LlmService.registerAdapter()` emits `llm/adapter-change` on registration and disposal ([packages/llm/src/index.ts](../../../../packages/llm/src/index.ts)). Grepping `llm/adapter-change` across `packages/*/src` and `examples/*/src` finds only the declaration, emit sites, docs, and tests; no production listener subscribes to it. -This differs from `tools/change` and `system-prompt/change`. Those two events are also unconsumed today, but they are plausible registry-change signals for future live tool/prompt UIs. LLM adapter registration is more of a boot-time implementation detail: adapters are not a user-visible palette and the real model-call interception seam is `llm/stream`. Keeping an adapter-change event with no listener repeats the [drop-the-dead-summary](../implemented/2026-06-19-drop-mutable-session-summary.md) pattern at a smaller scale. +This differs from `tools/change` and `system-prompt/change`. Those two events are also unconsumed today, but they are plausible registry-change signals for future live tool/prompt UIs. LLM adapter registration is more of a boot-time implementation detail: adapters are not a user-visible palette and the real model-call interception seam is `llm/stream`. Keeping an adapter-change event with no listener repeats the [drop-the-dead-summary](../../implemented/simplification/2026-06-19-drop-mutable-session-summary.md) pattern at a smaller scale. The event is not free. `registerAdapter()` yields its rollback disposer before emitting `llm/adapter-change` so a throwing listener unwinds the mutation instead of leaking an adapter entry, and the package carries tests for that listener-throw path. That defensive ordering protects a failure mode only tests can trigger. @@ -19,7 +19,7 @@ Remove only `llm/adapter-change`: - Simplify `registerAdapter()`'s effect generator: keep the mutation and rollback disposer for HMR/disposal, but drop the listener-throw rollback ordering that exists only for the removed event. - Remove the "Emits `llm/adapter-change` on registration and disposal" sentence from `LlmService.registerAdapter`'s JSDoc. - Rewrite the adapter-disposer test to assert the returned disposer removes the adapter without subscribing to `llm/adapter-change`; delete the listener-throw rollback test that exists solely for the removed event. -- Update the event taxonomy table in [docs/architecture.md](../../../docs/architecture.md) and [packages/llm/README.md](../../../packages/llm/README.md). The [doc-sync-enforcement RFC](../implemented/2026-06-11-doc-sync-enforcement.md) should avoid using `llm/adapter-change` as an example once the event is gone. +- Update the event taxonomy table in [docs/architecture.md](../../../architecture.md) and [packages/llm/README.md](../../../../packages/llm/README.md). The [doc-sync-enforcement RFC](../../implemented/process/2026-06-11-doc-sync-enforcement.md) should avoid using `llm/adapter-change` as an example once the event is gone. ## Why not remove every registry change event? diff --git a/docs/rfc/proposed/2026-06-20-drop-unconsumed-llm-assembled-surfaces.md b/docs/rfc/proposed/simplification/2026-06-20-drop-unconsumed-llm-assembled-surfaces.md similarity index 61% rename from docs/rfc/proposed/2026-06-20-drop-unconsumed-llm-assembled-surfaces.md rename to docs/rfc/proposed/simplification/2026-06-20-drop-unconsumed-llm-assembled-surfaces.md index 85198e7d84..1f0a1efadd 100644 --- a/docs/rfc/proposed/2026-06-20-drop-unconsumed-llm-assembled-surfaces.md +++ b/docs/rfc/proposed/simplification/2026-06-20-drop-unconsumed-llm-assembled-surfaces.md @@ -4,17 +4,17 @@ Status: proposed ## Problem -`LlmService` ([packages/llm/src/index.ts](../../../packages/llm/src/index.ts)) exposes three call surfaces over a model: +`LlmService` ([packages/llm/src/index.ts](../../../../packages/llm/src/index.ts)) exposes three call surfaces over a model: - `stream()` — raw `StreamChunk`s, dispatched through the `llm/stream` waterfall. -- `streamBlocks()` — a "convenience view" that runs the chunks through a `BlockAssembler` and yields completed `ContentBlock`s in stream order ([index.ts:137-144](../../../packages/llm/src/index.ts)). -- `generate()` — one fully-assembled `GenerateResult`, dispatched through a second `llm/generate` waterfall ([index.ts:151-157](../../../packages/llm/src/index.ts)). +- `streamBlocks()` — a "convenience view" that runs the chunks through a `BlockAssembler` and yields completed `ContentBlock`s in stream order ([index.ts:137-144](../../../../packages/llm/src/index.ts)). +- `generate()` — one fully-assembled `GenerateResult`, dispatched through a second `llm/generate` waterfall ([index.ts:151-157](../../../../packages/llm/src/index.ts)). -The only production consumer of the LLM service is the agent loop, and it uses `stream()` exclusively — feeding raw chunks through its own `BlockAssembler` so it can log chunks for replay fidelity while assembling in parallel ([packages/agent-loop/src/loop.ts](../../../packages/agent-loop/src/loop.ts), the `ctx.llm.stream(req)` step). Grepping `streamBlocks` and `ctx.llm.generate` across `packages/*/src` and `examples/*/src` finds no production callers. The references are the service methods, docs, and tests; adapter tests use `generate()` as a convenient driver, but they can hand-drain `stream()` through the same assembler helper without preserving a public production API. +The only production consumer of the LLM service is the agent loop, and it uses `stream()` exclusively — feeding raw chunks through its own `BlockAssembler` so it can log chunks for replay fidelity while assembling in parallel ([packages/agent-loop/src/loop.ts](../../../../packages/agent-loop/src/loop.ts), the `ctx.llm.stream(req)` step). Grepping `streamBlocks` and `ctx.llm.generate` across `packages/*/src` and `examples/*/src` finds no production callers. The references are the service methods, docs, and tests; adapter tests use `generate()` as a convenient driver, but they can hand-drain `stream()` through the same assembler helper without preserving a public production API. -This is the [drop-mutable-session-summary](../implemented/2026-06-19-drop-mutable-session-summary.md) pattern: assembled-view APIs with tested contracts, consumed by tests rather than production. They were built speculatively for consumers that do not care about token-level deltas, but the one real consumer cares about deltas precisely so it can persist high-fidelity replay data. +This is the [drop-mutable-session-summary](../../implemented/simplification/2026-06-19-drop-mutable-session-summary.md) pattern: assembled-view APIs with tested contracts, consumed by tests rather than production. They were built speculatively for consumers that do not care about token-level deltas, but the one real consumer cares about deltas precisely so it can persist high-fidelity replay data. -`streamBlocks()` drags a dedicated slice of `BlockAssembler` behind it: `flushReady()` and `flushRemaining()` ([packages/llm/src/assembler.ts:138-168](../../../packages/llm/src/assembler.ts)) plus the `flushed` cursor field exist only to support incremental in-order yield. `generate()` drags `GenerateResult`, `BlockAssembler.result()`, and the `llm/generate` waterfall as a second interception surface over the same underlying stream. The loop's assembler usage is `push()` / `message()` / `usage` / `finish` — not streaming flush or one-shot service assembly. +`streamBlocks()` drags a dedicated slice of `BlockAssembler` behind it: `flushReady()` and `flushRemaining()` ([packages/llm/src/assembler.ts:138-168](../../../../packages/llm/src/assembler.ts)) plus the `flushed` cursor field exist only to support incremental in-order yield. `generate()` drags `GenerateResult`, `BlockAssembler.result()`, and the `llm/generate` waterfall as a second interception surface over the same underlying stream. The loop's assembler usage is `push()` / `message()` / `usage` / `finish` — not streaming flush or one-shot service assembly. ## Proposal @@ -24,9 +24,9 @@ Make `stream()` the only public LLM call surface: - Remove `LlmService.generate()`, the `llm/generate` waterfall event, and `GenerateResult` if no surviving API needs that named result shape. - Remove `BlockAssembler.flushReady()`, `BlockAssembler.flushRemaining()`, and the `flushed` cursor field. - Remove `BlockAssembler.result()` if it is only a helper for the deleted `generate()` service path and tests. -- Replace adapter-test use of `ctx.llm.generate()` with a small test helper that calls `ctx.llm.stream()`, pushes chunks into `BlockAssembler`, and returns the assembled message, usage, and finish reason needed by that test. That keeps the [twin-adapter design](../implemented/2026-06-13-twin-llm-adapters.md) intact while avoiding a public method whose only callers are tests. +- Replace adapter-test use of `ctx.llm.generate()` with a small test helper that calls `ctx.llm.stream()`, pushes chunks into `BlockAssembler`, and returns the assembled message, usage, and finish reason needed by that test. That keeps the [twin-adapter design](../../implemented/architecture/2026-06-13-twin-llm-adapters.md) intact while avoiding a public method whose only callers are tests. - Remove or rework the `flushReady`/`flushRemaining`-dependent tests. Keep assembler invariants that still apply to `push()` / `blocks()` / `message()`; delete behavior that only pins the removed flush API. -- Update every doc/comment reference to `streamBlocks`, `generate`, `GenerateResult`, and `llm/generate` across `docs/`, package READMEs, and source comments. The `ctx.llm` service-map row in [docs/architecture.md](../../../docs/architecture.md) becomes `stream()` only, the event taxonomy drops `llm/generate`, and the [property-based-testing RFC](../implemented/2026-06-11-property-based-testing.md) names block-assembly invariants without referring to removed convenience methods. +- Update every doc/comment reference to `streamBlocks`, `generate`, `GenerateResult`, and `llm/generate` across `docs/`, package READMEs, and source comments. The `ctx.llm` service-map row in [docs/architecture.md](../../../architecture.md) becomes `stream()` only, the event taxonomy drops `llm/generate`, and the [property-based-testing RFC](../../implemented/testing/2026-06-11-property-based-testing.md) names block-assembly invariants without referring to removed convenience methods. ## Acceptance criteria @@ -34,11 +34,11 @@ Make `stream()` the only public LLM call surface: - `pnpm run test:coverage` stays at 100% per-file (the deleted methods take their dedicated tests with them; no remaining line goes uncovered). - Adapter tests still exercise both real adapters through `stream()` and the shared assembler, not through a test-only public shortcut. - The loop behaves identically — verified by unchanged ACP snapshot goldens. -- `packages/llm/README.md`, [docs/architecture.md](../../../docs/architecture.md), and module docs no longer mention the removed convenience surfaces. +- `packages/llm/README.md`, [docs/architecture.md](../../../architecture.md), and module docs no longer mention the removed convenience surfaces. ## Risks -- **It removes public methods from a core vocabulary package.** A future plugin that wants assembled blocks without deltas would need to call `stream()` and use `BlockAssembler` directly or reintroduce a focused helper with a real consumer. Given the pre-release "foundation over speculative future" stance ([AGENTS.md](../../../AGENTS.md)), this is the right time to cut test-only public shape. +- **It removes public methods from a core vocabulary package.** A future plugin that wants assembled blocks without deltas would need to call `stream()` and use `BlockAssembler` directly or reintroduce a focused helper with a real consumer. Given the pre-release "foundation over speculative future" stance ([AGENTS.md](../../../../AGENTS.md)), this is the right time to cut test-only public shape. - **Adapter tests get a little more explicit.** They lose the ergonomic `generate()` wrapper, but that is useful pressure: tests exercise the same streaming path production uses. - **Waterfall users lose `llm/generate`.** No production listener exists. Any future caching/retry/logging plugin should wrap `llm/stream`, which remains the single provider call path. diff --git a/docs/rfc/proposed/simplification/2026-06-20-prune-dead-seam-methods.md b/docs/rfc/proposed/simplification/2026-06-20-prune-dead-seam-methods.md new file mode 100644 index 0000000000..de79c3f355 --- /dev/null +++ b/docs/rfc/proposed/simplification/2026-06-20-prune-dead-seam-methods.md @@ -0,0 +1,49 @@ +# RFC: Prune dead methods from the persistence and bash capability seams + +Status: proposed + +## Problem + +Two capability seams ([interface / implementation / consumer](../../implemented/architecture/2026-06-13-capability-seams.md)) carry abstract methods that no consumer calls. The seam exists to let implementations and consumers evolve independently — but a method no consumer programs against is not a seam, it is speculative surface every implementation must still implement and test. + +### `SessionPersistence.has()` and `.delete()` + +The abstract service declares four operations beyond create/append: `load`, `list`, `has`, `delete` ([packages/session-persistence/src/index.ts:142-151](../../../../packages/session-persistence/src/index.ts)). Production consumers of `ctx.sessionPersistence` use only two of them: the agent-loop resume path calls `load()` ([packages/agent-loop/src/index.ts:176-194](../../../../packages/agent-loop/src/index.ts)), and the ACP bridge calls `list()` for `session/list` ([packages/acp/src/index.ts](../../../../packages/acp/src/index.ts)). Grepping every `sessionPersistence.*` / `persistence.*` use across `packages/*/src` and `examples/` finds no `has(` and no `delete(` on the service. The `.has(`/`.delete(` calls in `packages/acp/src/index.ts` are on the in-memory `SessionStore` and a local `Set` of loading ids, not persistence. The only callers of `has`/`delete` are the contract suites and per-backend specs. + +`has()` is not just unused — it is the most intricate branch in the shared coordinator: a tracked-vs-untracked dual-probe (`loadLive(id, cwd)` for a live-tracked session vs `loadStored(id)` for an untracked one) with a multi-line rationale ([packages/session-persistence/src/coordinator.ts:298-310](../../../../packages/session-persistence/src/coordinator.ts)). `delete()` drags the `deleteStored` backend hook ([coordinator.ts:99](../../../../packages/session-persistence/src/coordinator.ts), [coordinator.ts:313-319](../../../../packages/session-persistence/src/coordinator.ts)) that every backend must implement. This is the [drop-mutable-session-summary](../../implemented/simplification/2026-06-19-drop-mutable-session-summary.md) pattern: a contract test exercises both, but no shipping code asks "is this session persisted?" or removes one. + +### `BashExecutor.get()` and `.list()` + +The bash seam declares `get(id)` ("look up a background task by id") and `list()` ("all tracked background tasks") ([packages/bash/src/index.ts:88-107](../../../../packages/bash/src/index.ts)), both implemented by `LocalBashExecutor` ([packages/bash-local/src/index.ts:179-191](../../../../packages/bash-local/src/index.ts)). The sole production consumer — `dsh-tool-bash` — drives tasks via `ownerOf`, `onTaskDone`, `start`, `readOutput`, `kill`, `resolve`, `run`; it never calls `get`/`list` in shipping code, and there is no `bash_list` tool exposing a task roster to the model. So both are dead production seam surface. They are used by tests, more broadly than a single idiom: the bash seam/executor specs assert them directly ([packages/bash/tests/service.spec.ts](../../../../packages/bash/tests/service.spec.ts), [packages/bash-local/tests/executor.spec.ts](../../../../packages/bash-local/tests/executor.spec.ts) both call `get()`/`list()`), and several `dsh-tool-bash` tests reach through `ctx.bash.get(id)` to await a task's `done`, read its `status`, or inspect task fields ([packages/tool-bash/tests/tools.spec.ts](../../../../packages/tool-bash/tests/tools.spec.ts), [packages/tool-bash/tests/integration.spec.ts](../../../../packages/tool-bash/tests/integration.spec.ts)). These are test-harness conveniences, not shipping consumers — but they are real test code an implementing PR must migrate or delete. + +## Proposal + +Remove the methods nothing consumes, from the abstract seam, the implementation, and the contract/spec suites that exist only to exercise them: + +- `SessionPersistence.has()` / `.delete()`: delete the abstract declarations, the coordinator's `has`/`delete`/`deleteCore`, and the `PersistenceBackend.deleteStored` hook. Remove the `has`/`delete` rows from the contract suite and the per-backend specs (jsonl + sqlite each implement `deleteStored` only to satisfy the hook — that implementation goes too). The backends are the [dual-backend](../../implemented/architecture/2026-06-14-session-persistence.md) design and otherwise out of scope, but removing a hook they implement for no consumer is part of removing the hook, not a backend redesign. +- `BashExecutor.get()` / `.list()`: delete the abstract declarations and the `LocalBashExecutor` impls. The seam/executor specs that assert `get()`/`list()` directly (`bash/tests/service.spec.ts`, `bash-local/tests/executor.spec.ts`) lose those assertions (the behavior is being removed). The `dsh-tool-bash` tests that reach through `ctx.bash.get(id)` to await `done`, read `status`, or inspect task fields switch to the public completion/status seam they should use — `onTaskDone` (or the `done` promise and status the `start()` return already exposes) — keeping their coverage without the removed lookup method. +- Update every doc and source-comment reference to the removed methods — not only literal `has(`/`delete(`/`get(`/`list(`/`deleteStored` call spellings, but also `{@link has}`/`{@link delete}` JSDoc links and prose that counts the methods (removing 2 of the persistence service's 6 public methods makes any "six public methods" phrasing wrong). The implementing PR greps `has`/`delete`/`get`/`list`/`deleteStored`/`{@link `/`six ` across `docs/`, `packages/*/README.md`, and source comments, and fixes each. The known doc sites: the seam READMEs ([packages/session-persistence/README.md](../../../../packages/session-persistence/README.md)'s `has(id)`/`delete(id)` API row and its "delegates its six public service methods" prose → four, [packages/bash/README.md](../../../../packages/bash/README.md)'s `get(id)`/`list()` row), the backend READMEs that describe `has`/`list` semantics ([packages/session-persistence-sqlite/README.md](../../../../packages/session-persistence-sqlite/README.md), [packages/session-persistence-jsonl/README.md](../../../../packages/session-persistence-jsonl/README.md) — reword "absent from `has()`/`list()`" to just `list()`), the service-map / seam docs in [docs/architecture.md](../../../architecture.md), and the persistence prose in the [session-persistence RFC](../../implemented/architecture/2026-06-14-session-persistence.md) and [shared write-coordinator RFC](../../implemented/architecture/2026-06-18-shared-persistence-write-coordinator.md). The known source-comment sites: the abstract `create()` JSDoc's `{@link has}/{@link list}` link ([packages/session-persistence/src/index.ts](../../../../packages/session-persistence/src/index.ts) — drop the `has` link), the coordinator's "six public methods"/"six public service methods" module + class JSDoc and its lazy-materialization JSDoc justifying the `materialized` flag by "the signal `has`/`list` rely on" ([packages/session-persistence/src/coordinator.ts](../../../../packages/session-persistence/src/coordinator.ts)), the JSONL backend's `loadStored`/`deleteStored` comment, and the SQLite backend's `schema.ts` and `index.ts` comments that mention "absent from `has`/`list`" — all reworded to the surviving four-method, `list()`-only contract. + +## Why not keep them as "the seam should be complete"? + +The instinct that a persistence seam "should" offer delete, or a task executor "should" offer enumeration, is real — and it is exactly the speculative-completeness the pre-release stance warns against ([AGENTS.md](../../../../AGENTS.md): optimize for the correct foundation, not for hypothetical callers you do not have). Each of these is one method to re-add the day a consumer needs it: + +- A session-management UI that deletes old sessions will want `delete()` — add it then, designed against that UI's real needs (soft-delete? cascade? confirmation?), not guessed now. +- A `bash_list` tool that shows the model its running tasks will want `list()` — add it with the tool. + +Re-adding a seam method with a live consumer is cheap and better-designed than the speculative version, because the consumer pins the contract. Carrying it unused means every implementation (and every future backend) must implement and test a method that does nothing. + +## Acceptance criteria + +- `has`/`delete`/`deleteStored` and `get`/`list` are gone from their seams, impls, and contract suites; `pnpm run knip` reports no new dead exports. +- The remaining seam operations (`create`/`append`/`load`/`list` for persistence; `run`/`start`/`ownerOf`/`onTaskDone`/`readOutput`/`kill`/`resolve` for bash) are untouched; ACP `session/list`, bash tool flows, and crash-recovery behave identically. +- `pnpm run test:coverage` stays 100% per-file (the contract/spec rows for the removed methods are deleted with them). +- Seam READMEs and `docs/architecture.md` no longer list the removed methods. + +## Risks + +- **`delete()` is the kind of operation a product eventually wants.** True — but "eventually" is the point. Deleting it now and re-adding it against a real consumer is strictly better than shipping a guessed contract. The dual backends each shed a `deleteStored` impl, which is a bounded edit in otherwise-out-of-scope packages. +- **`list()` on the bash seam is the natural seed for a future `bash_list`.** Acknowledged in the [pre-release foundation stance](../../../../AGENTS.md): add the seed when the tool lands. The executor still tracks tasks internally (the `tasks` map backs `ownerOf`/`readOutput`/`kill`); exposing an enumeration is a one-line re-add. +- **Low coupling.** Both removals are confined to their seam + impl + tests; no cross-package consumer references the removed methods, so there is no ripple beyond the docs. + +Modest size, but it converts two seams from "what an implementation must provide for nobody" back to "exactly what a consumer uses." diff --git a/docs/rfc/proposed/2026-06-20-public-agent-stop-surface.md b/docs/rfc/proposed/simplification/2026-06-20-public-agent-stop-surface.md similarity index 100% rename from docs/rfc/proposed/2026-06-20-public-agent-stop-surface.md rename to docs/rfc/proposed/simplification/2026-06-20-public-agent-stop-surface.md diff --git a/docs/rfc/proposed/2026-06-20-remove-agent-boundary-mirror-events.md b/docs/rfc/proposed/simplification/2026-06-20-remove-agent-boundary-mirror-events.md similarity index 100% rename from docs/rfc/proposed/2026-06-20-remove-agent-boundary-mirror-events.md rename to docs/rfc/proposed/simplification/2026-06-20-remove-agent-boundary-mirror-events.md diff --git a/docs/rfc/proposed/2026-06-20-unify-agent-and-session-id.md b/docs/rfc/proposed/simplification/2026-06-20-unify-agent-and-session-id.md similarity index 97% rename from docs/rfc/proposed/2026-06-20-unify-agent-and-session-id.md rename to docs/rfc/proposed/simplification/2026-06-20-unify-agent-and-session-id.md index 05895f51f1..19bde62d01 100644 --- a/docs/rfc/proposed/2026-06-20-unify-agent-and-session-id.md +++ b/docs/rfc/proposed/simplification/2026-06-20-unify-agent-and-session-id.md @@ -16,7 +16,7 @@ The agent factory carries TWO ids for what is, in every live consumer, one thing Everywhere a live consumer actually looks an agent up — the **ACP bridge, the only production path** — the two are already unified: `agentId === sessionId === `. -The separation is **latent generality no consumer exercises**: nothing reads a *stable* `agentId` back across runs (each process starts fresh, and persistence keys off the session id, never the agent id). The config path's "stable agentId, fresh sessionId" buys nothing concrete — it is cosmetic. And the `agentId !== sessionId` case is precisely what opens the bash owner-token alias hole: the bash completion-notice routes by `session.header.id`, but the registry enforces uniqueness only on `agentId`, so a programmatic caller registering two agents with different agent ids but the SAME session id can mis-route a notice (see [agent lifecycle and ownership seams](../implemented/2026-06-18-agent-lifecycle-and-ownership-seams.md) § Seam precondition). The current code documents this as a precondition rather than guaranteeing it. +The separation is **latent generality no consumer exercises**: nothing reads a *stable* `agentId` back across runs (each process starts fresh, and persistence keys off the session id, never the agent id). The config path's "stable agentId, fresh sessionId" buys nothing concrete — it is cosmetic. And the `agentId !== sessionId` case is precisely what opens the bash owner-token alias hole: the bash completion-notice routes by `session.header.id`, but the registry enforces uniqueness only on `agentId`, so a programmatic caller registering two agents with different agent ids but the SAME session id can mis-route a notice (see [agent lifecycle and ownership seams](../../implemented/architecture/2026-06-18-agent-lifecycle-and-ownership-seams.md) § Seam precondition). The current code documents this as a precondition rather than guaranteeing it. ## Proposal diff --git a/docs/rfc/proposed/2026-06-11-deterministic-and-stress-testing.md b/docs/rfc/proposed/testing/2026-06-11-deterministic-and-stress-testing.md similarity index 100% rename from docs/rfc/proposed/2026-06-11-deterministic-and-stress-testing.md rename to docs/rfc/proposed/testing/2026-06-11-deterministic-and-stress-testing.md diff --git a/docs/rfc/proposed/2026-06-11-mutation-testing.md b/docs/rfc/proposed/testing/2026-06-11-mutation-testing.md similarity index 79% rename from docs/rfc/proposed/2026-06-11-mutation-testing.md rename to docs/rfc/proposed/testing/2026-06-11-mutation-testing.md index 8a1e422f7b..209b0cdbd0 100644 --- a/docs/rfc/proposed/2026-06-11-mutation-testing.md +++ b/docs/rfc/proposed/testing/2026-06-11-mutation-testing.md @@ -6,7 +6,7 @@ Status: proposed ## Problem -The per-file 100% coverage gate ([the quality-gates decision](../implemented/2026-06-11-quality-gates.md)) proves every line *executes* under test — not that any assertion would notice if the line were wrong. Under agent-written tests, coverage pressure can produce execution-without-assertion. Mutation testing measures what coverage cannot: whether the suite *kills* deliberately injected bugs. +The per-file 100% coverage gate ([the quality-gates decision](../../implemented/process/2026-06-11-quality-gates.md)) proves every line *executes* under test — not that any assertion would notice if the line were wrong. Under agent-written tests, coverage pressure can produce execution-without-assertion. Mutation testing measures what coverage cannot: whether the suite *kills* deliberately injected bugs. ## Proposal diff --git a/docs/rfc/proposed/2026-06-20-remove-redundant-snapshot-log-goldens.md b/docs/rfc/proposed/testing/2026-06-20-remove-redundant-snapshot-log-goldens.md similarity index 95% rename from docs/rfc/proposed/2026-06-20-remove-redundant-snapshot-log-goldens.md rename to docs/rfc/proposed/testing/2026-06-20-remove-redundant-snapshot-log-goldens.md index 15b996cf61..55325a7ef7 100644 --- a/docs/rfc/proposed/2026-06-20-remove-redundant-snapshot-log-goldens.md +++ b/docs/rfc/proposed/testing/2026-06-20-remove-redundant-snapshot-log-goldens.md @@ -24,7 +24,7 @@ Stdout goldens remain unchanged; they are the editor-facing projection and are n - The snapshot test derives the expected session log from `session.jsonl` for every model scenario. - Authored sidecar scenarios commit their expected produced log in `session.jsonl`; `replay.override.json` remains the model-behavior override. - Orphan-fixture guards understand which files are required by scenario kind. -- The [ACP snapshot tests RFC](../implemented/2026-06-19-acp-snapshot-tests.md) is updated to describe the reduced fixture set. +- The [ACP snapshot tests RFC](../../implemented/testing/2026-06-19-acp-snapshot-tests.md) is updated to describe the reduced fixture set. ## What we give up diff --git a/docs/rfc/rejected/2026-06-11-immutable-public-surfaces.md b/docs/rfc/rejected/architecture/2026-06-11-immutable-public-surfaces.md similarity index 72% rename from docs/rfc/rejected/2026-06-11-immutable-public-surfaces.md rename to docs/rfc/rejected/architecture/2026-06-11-immutable-public-surfaces.md index 8433862e1f..0e791ef689 100644 --- a/docs/rfc/rejected/2026-06-11-immutable-public-surfaces.md +++ b/docs/rfc/rejected/architecture/2026-06-11-immutable-public-surfaces.md @@ -1,6 +1,6 @@ # RFC: Deep-readonly public surfaces -Status: rejected — the pervasive `DeepReadonly` type flip was rejected in favor of an always-on `deriveMessages` clone plus dev-mode `Object.freeze` + invariants. The immutability *goal* shipped via that alternative; see [dev-mode invariants](../implemented/2026-06-11-dev-invariants-over-deep-readonly.md). +Status: rejected — the pervasive `DeepReadonly` type flip was rejected in favor of an always-on `deriveMessages` clone plus dev-mode `Object.freeze` + invariants. The immutability *goal* shipped via that alternative; see [dev-mode invariants](../../implemented/architecture/2026-06-11-dev-invariants-over-deep-readonly.md). @@ -10,18 +10,18 @@ The session log is append-only by contract, but `session.events` returns `readon ## Proposal -> **Implemented differently — see the Status line and [dev-mode invariants](../implemented/2026-06-11-dev-invariants-over-deep-readonly.md).** The `DeepReadonly` design below was rejected as written (compile-only, high type-noise, castable). What shipped: an always-on deep clone in `deriveMessages` (closing the request/adapter aliasing path) plus a dev-mode `Object.freeze` + invariants plugin. The proposal text is kept for the record. +> **Implemented differently — see the Status line and [dev-mode invariants](../../implemented/architecture/2026-06-11-dev-invariants-over-deep-readonly.md).** The `DeepReadonly` design below was rejected as written (compile-only, high type-noise, castable). What shipped: an always-on deep clone in `deriveMessages` (closing the request/adapter aliasing path) plus a dev-mode `Object.freeze` + invariants plugin. The proposal text is kept for the record. Make immutability part of the type where mutation is corruption: - `SessionEvent` data becomes `DeepReadonly` on the way OUT of a session (`events`, `session/event` listeners); `append()` keeps taking plain mutable input. A `DeepReadonly` utility type lands in dsh-llm next to the brand/never helpers. - `deriveMessages()` returns deep-readonly messages; the loop clones before handing a mutable request to the `agent/request` waterfall (mutation there is sanctioned — the clone makes the boundary explicit and cheap, once per step). - `PromptAssembly` stays mutable through its waterfall (sanctioned) but the registry's internal section list is cloned per assembly (already true). -- Optionally, dev-mode `Object.freeze` of event data behind [the dev-mode invariants](../implemented/2026-06-11-dev-invariants-over-deep-readonly.md) flag, so sanctioned-mutation violations throw in tests rather than corrupting silently. +- Optionally, dev-mode `Object.freeze` of event data behind [the dev-mode invariants](../../implemented/architecture/2026-06-11-dev-invariants-over-deep-readonly.md) flag, so sanctioned-mutation violations throw in tests rather than corrupting silently. ## Plan -Introduce `DeepReadonly`, flip the session read paths, fix resulting compile errors in consumers (expected: a handful in tests), add the freeze-in-dev option alongside [the dev-mode invariants](../implemented/2026-06-11-dev-invariants-over-deep-readonly.md) plugin. +Introduce `DeepReadonly`, flip the session read paths, fix resulting compile errors in consumers (expected: a handful in tests), add the freeze-in-dev option alongside [the dev-mode invariants](../../implemented/architecture/2026-06-11-dev-invariants-over-deep-readonly.md) plugin. ## Risks diff --git a/docs/rfc/rejected/2026-06-20-assembled-assistant-messages-only.md b/docs/rfc/rejected/simplification/2026-06-20-assembled-assistant-messages-only.md similarity index 78% rename from docs/rfc/rejected/2026-06-20-assembled-assistant-messages-only.md rename to docs/rfc/rejected/simplification/2026-06-20-assembled-assistant-messages-only.md index 173b63e8aa..73dd601139 100644 --- a/docs/rfc/rejected/2026-06-20-assembled-assistant-messages-only.md +++ b/docs/rfc/rejected/simplification/2026-06-20-assembled-assistant-messages-only.md @@ -4,7 +4,7 @@ Status: rejected — high-fidelity chunk replay, partial failed streams, and sna ## Problem -The canonical session log currently persists every `assistant/chunk` exactly as streamed by the model. The [session persistence RFC](../implemented/2026-06-14-session-persistence.md) chose this for token-level replay fidelity and contiguous `seq`, but the cost has grown: JSONL fixtures are dominated by tiny delta records, snapshot scenarios replay the model by grouping chunk events, ACP load reconstructs prior assistant output from chunks, and any future log reader must distinguish durable message history from token-level trace. +The canonical session log currently persists every `assistant/chunk` exactly as streamed by the model. The [session persistence RFC](../../implemented/architecture/2026-06-14-session-persistence.md) chose this for token-level replay fidelity and contiguous `seq`, but the cost has grown: JSONL fixtures are dominated by tiny delta records, snapshot scenarios replay the model by grouping chunk events, ACP load reconstructs prior assistant output from chunks, and any future log reader must distinguish durable message history from token-level trace. For successful steps that assemble completed content, the loop already appends an `assistant/message`. That is the event `deriveMessages()` uses for the next model request. In other words, the normal resumable conversation state is already present without the chunks; chunks are a live rendering and deterministic-test artifact, not required conversation history. Failed or aborted streams are different: partial assistant output may exist only as chunks, and empty max-token steps may produce no `assistant/message` at all. @@ -17,7 +17,7 @@ ACP `session/load` can replay prior assistant messages as complete content block ## Acceptance criteria - `SessionEventMap` drops `assistant/chunk`, or marks it as non-persisted if a transitional live event is needed. -- [Session persistence docs](../../../packages/session-persistence/README.md) no longer require every stream chunk to be stored verbatim. +- [Session persistence docs](../../../../packages/session-persistence/README.md) no longer require every stream chunk to be stored verbatim. - `llm-replay` and ACP snapshots use an explicit replay fixture format or sidecar for model chunks. - `session/load` renders completed assistant messages from `assistant/message`. - Stored logs get much smaller and remain `seq`-contiguous without chunk holes. @@ -29,4 +29,4 @@ The canonical user session no longer reconstructs the exact token stream of an o ## Related -This supersedes the chunk-persistence choice in [session persistence](../implemented/2026-06-14-session-persistence.md) and affects [ACP snapshot tests](../implemented/2026-06-19-acp-snapshot-tests.md), whose current replay plugin derives its script from `assistant/chunk` events. +This supersedes the chunk-persistence choice in [session persistence](../../implemented/architecture/2026-06-14-session-persistence.md) and affects [ACP snapshot tests](../../implemented/testing/2026-06-19-acp-snapshot-tests.md), whose current replay plugin derives its script from `assistant/chunk` events. diff --git a/docs/rfc/rejected/2026-06-20-drop-acp-session-load.md b/docs/rfc/rejected/simplification/2026-06-20-drop-acp-session-load.md similarity index 96% rename from docs/rfc/rejected/2026-06-20-drop-acp-session-load.md rename to docs/rfc/rejected/simplification/2026-06-20-drop-acp-session-load.md index 5714dc058f..c8d86066bf 100644 --- a/docs/rfc/rejected/2026-06-20-drop-acp-session-load.md +++ b/docs/rfc/rejected/simplification/2026-06-20-drop-acp-session-load.md @@ -18,7 +18,7 @@ For now, ACP starts fresh sessions only. `initialize` advertises `loadSession: f - `initialize` does not advertise load support. - The `session/load` handler, loading-id tracking, cwd preflight for loaded sessions, and load replay tests are removed. - Snapshot fixtures no longer rely on load replay presentation. -- [ACP docs](../../../packages/acp/README.md) describe fresh-session support only. +- [ACP docs](../../../../packages/acp/README.md) describe fresh-session support only. ## What we give up diff --git a/docs/rfc/rejected/2026-06-20-drop-acp-terminal-meta.md b/docs/rfc/rejected/simplification/2026-06-20-drop-acp-terminal-meta.md similarity index 75% rename from docs/rfc/rejected/2026-06-20-drop-acp-terminal-meta.md rename to docs/rfc/rejected/simplification/2026-06-20-drop-acp-terminal-meta.md index 89b0335275..4b7ca91a0c 100644 --- a/docs/rfc/rejected/2026-06-20-drop-acp-terminal-meta.md +++ b/docs/rfc/rejected/simplification/2026-06-20-drop-acp-terminal-meta.md @@ -4,7 +4,7 @@ Status: rejected — Zed is the current target client, and the terminal `_meta` ## Problem -The ACP bridge implements a Zed-specific terminal-card convention through `_meta.terminal_info`, `_meta.terminal_output`, and `_meta.terminal_exit`. The implemented [rich ACP bash rendering RFC](../implemented/2026-06-18-acp-terminal-and-tool-rendering.md) deliberately avoided ACP's client-side `terminal/create` because bash execution belongs in the harness, but still adopted the reference agents' display-only `_meta` convention. That gives a nicer Zed card at the cost of bridge state, capability negotiation, terminal ids, special update mapping, text fallback tests, and exit-pill parsing in `dsh-tool-bash`. +The ACP bridge implements a Zed-specific terminal-card convention through `_meta.terminal_info`, `_meta.terminal_output`, and `_meta.terminal_exit`. The implemented [rich ACP bash rendering RFC](../../implemented/feature/2026-06-18-acp-terminal-and-tool-rendering.md) deliberately avoided ACP's client-side `terminal/create` because bash execution belongs in the harness, but still adopted the reference agents' display-only `_meta` convention. That gives a nicer Zed card at the cost of bridge state, capability negotiation, terminal ids, special update mapping, text fallback tests, and exit-pill parsing in `dsh-tool-bash`. The fallback path already exists: render the tool call and completed output as normal ACP content blocks. Non-Zed clients rely on that path anyway, but the Zed terminal card is a current target-client feature rather than speculative decoration. @@ -20,7 +20,7 @@ This proposal is narrower than [collapsing tool-owned UI presentation](2026-06-2 - `TerminalRendering`, terminal ids, terminal cwd resolution, and `_meta.terminal_*` update mapping disappear from `@deepseek-ai/dsh-acp`. - `ToolTerminal` disappears from `@deepseek-ai/dsh-tools`, or is unused and deleted with the presentation cleanup. - Bash result presentation no longer parses exit status for terminal pills. -- The implemented [rich ACP bash rendering RFC](../implemented/2026-06-18-acp-terminal-and-tool-rendering.md) stays in `implemented/` as shipped history and is cross-linked from this proposal if superseded. +- The implemented [rich ACP bash rendering RFC](../../implemented/feature/2026-06-18-acp-terminal-and-tool-rendering.md) stays in `implemented/` as shipped history and is cross-linked from this proposal if superseded. ## What we give up diff --git a/docs/rfc/rejected/2026-06-20-drop-bash-output-spill-files.md b/docs/rfc/rejected/simplification/2026-06-20-drop-bash-output-spill-files.md similarity index 85% rename from docs/rfc/rejected/2026-06-20-drop-bash-output-spill-files.md rename to docs/rfc/rejected/simplification/2026-06-20-drop-bash-output-spill-files.md index ba533bba6d..a6a47c66a0 100644 --- a/docs/rfc/rejected/2026-06-20-drop-bash-output-spill-files.md +++ b/docs/rfc/rejected/simplification/2026-06-20-drop-bash-output-spill-files.md @@ -12,7 +12,7 @@ This solves a real problem, but in a narrow and leaky way. A spill path is a pro Keep tail truncation, drop full-output spill files. A bash result contains the bounded tail plus a clear truncation marker; no path is emitted. If users need full-output recovery, add a generic artifact/blob service with explicit ownership, cleanup, and UI rendering, then let bash attach large outputs to that service. -This proposal can land independently of [a generic long-running tool runtime](../proposed/2026-06-20-generic-long-running-tool-runtime.md). If background tasks stay, `bash_output` should still report that output was dropped, but without advertising a spill path. +This proposal can land independently of [a generic long-running tool runtime](../../proposed/architecture/2026-06-20-generic-long-running-tool-runtime.md). If background tasks stay, `bash_output` should still report that output was dropped, but without advertising a spill path. ## Acceptance criteria @@ -20,7 +20,7 @@ This proposal can land independently of [a generic long-running tool runtime](.. - `OutputCollector` keeps bounded buffers only and deletes the temp-file machinery. - `renderResult()` reports truncation without a filesystem path. - Tests cover tail truncation and no longer assert full-output file contents. -- Security guidance in [root AGENTS.md](../../../AGENTS.md) stops treating private spill files as a model-visible interface. +- Security guidance in [root AGENTS.md](../../../../AGENTS.md) stops treating private spill files as a model-visible interface. ## What we give up diff --git a/docs/rfc/rejected/2026-06-20-drop-durable-step-boundaries.md b/docs/rfc/rejected/simplification/2026-06-20-drop-durable-step-boundaries.md similarity index 94% rename from docs/rfc/rejected/2026-06-20-drop-durable-step-boundaries.md rename to docs/rfc/rejected/simplification/2026-06-20-drop-durable-step-boundaries.md index 60b2af391a..833fae5bc6 100644 --- a/docs/rfc/rejected/2026-06-20-drop-durable-step-boundaries.md +++ b/docs/rfc/rejected/simplification/2026-06-20-drop-durable-step-boundaries.md @@ -20,7 +20,7 @@ The invariants plugin should enforce that step-scoped events have valid positive - The loop has no `closeStep()` finalization path. - ACP snapshots and persistence contract fixtures stop expecting step-boundary lines. - `deriveMessages()` and replay derive the same message history from step-scoped events. -- The [event taxonomy docs](../../architecture.md) describe turns as the durable boundary and steps as a field on step-scoped records. +- The [event taxonomy docs](../../../architecture.md) describe turns as the durable boundary and steps as a field on step-scoped records. - The session format version and recorded fixtures are refreshed; non-current stored logs are rejected per the pre-release format policy. ## What we give up diff --git a/docs/rfc/rejected/2026-06-20-drop-unused-session-lineage.md b/docs/rfc/rejected/simplification/2026-06-20-drop-unused-session-lineage.md similarity index 100% rename from docs/rfc/rejected/2026-06-20-drop-unused-session-lineage.md rename to docs/rfc/rejected/simplification/2026-06-20-drop-unused-session-lineage.md diff --git a/docs/rfc/rejected/2026-06-20-fold-session-persistence-interface.md b/docs/rfc/rejected/simplification/2026-06-20-fold-session-persistence-interface.md similarity index 76% rename from docs/rfc/rejected/2026-06-20-fold-session-persistence-interface.md rename to docs/rfc/rejected/simplification/2026-06-20-fold-session-persistence-interface.md index da19617793..6731b50211 100644 --- a/docs/rfc/rejected/2026-06-20-fold-session-persistence-interface.md +++ b/docs/rfc/rejected/simplification/2026-06-20-fold-session-persistence-interface.md @@ -12,7 +12,7 @@ The capability-seam split made sense when persistence was a new swappable backen Move the abstract `SessionPersistence` service, the coordinator, and persistence contract helpers into `dsh-session`. Keep JSONL and SQLite as separate backend packages that register the session-owned service. This preserves backend swappability while deleting one support package and one cross-package seam. -The implementing PR should update the [capability seams](../implemented/2026-06-13-capability-seams.md) guidance with the exception: persistence is not like bash or LLM because its vocabulary and lifecycle events are already the session package's core domain. +The implementing PR should update the [capability seams](../../implemented/architecture/2026-06-13-capability-seams.md) guidance with the exception: persistence is not like bash or LLM because its vocabulary and lifecycle events are already the session package's core domain. ## Acceptance criteria @@ -20,7 +20,7 @@ The implementing PR should update the [capability seams](../implemented/2026-06- - `dsh-session` exports the persistence service type, coordinator, and contract helpers. - JSONL and SQLite backend packages depend on `dsh-session` directly. - `agent-loop` resume uses the session-owned service key. -- [Session persistence](../implemented/2026-06-14-session-persistence.md), [shared persistence write coordinator](../implemented/2026-06-18-shared-persistence-write-coordinator.md), and [package docs](../../../packages/session-persistence/README.md) explain why backend implementations remain separate. +- [Session persistence](../../implemented/architecture/2026-06-14-session-persistence.md), [shared persistence write coordinator](../../implemented/architecture/2026-06-18-shared-persistence-write-coordinator.md), and [package docs](../../../../packages/session-persistence/README.md) explain why backend implementations remain separate. ## What we give up diff --git a/docs/rfc/rejected/2026-06-20-generic-tool-rendering.md b/docs/rfc/rejected/simplification/2026-06-20-generic-tool-rendering.md similarity index 100% rename from docs/rfc/rejected/2026-06-20-generic-tool-rendering.md rename to docs/rfc/rejected/simplification/2026-06-20-generic-tool-rendering.md diff --git a/docs/rfc/rejected/2026-06-20-retire-mid-turn-steering.md b/docs/rfc/rejected/simplification/2026-06-20-retire-mid-turn-steering.md similarity index 100% rename from docs/rfc/rejected/2026-06-20-retire-mid-turn-steering.md rename to docs/rfc/rejected/simplification/2026-06-20-retire-mid-turn-steering.md diff --git a/docs/rfc/rejected/2026-06-20-single-session-acp-bridge.md b/docs/rfc/rejected/simplification/2026-06-20-single-session-acp-bridge.md similarity index 86% rename from docs/rfc/rejected/2026-06-20-single-session-acp-bridge.md rename to docs/rfc/rejected/simplification/2026-06-20-single-session-acp-bridge.md index ab26b9c3c9..b2a2c3b84f 100644 --- a/docs/rfc/rejected/2026-06-20-single-session-acp-bridge.md +++ b/docs/rfc/rejected/simplification/2026-06-20-single-session-acp-bridge.md @@ -4,7 +4,7 @@ Status: rejected — Zed is the current target ACP client and its ACP implementa ## Problem -The ACP bridge now supports multiple live sessions on one JSON-RPC connection. That capability brings multi-entry session maps, reverse session/agent lookups, per-session prompt state, loading ids, demux for every event, cross-session teardown, and isolation concerns for future permission prompts and background tasks. The older [multi-session ACP proposal](../proposed/2026-06-14-acp-multi-session.md) still tracks the unfinished permission-ownership piece; this RFC is the competing simplification path. +The ACP bridge now supports multiple live sessions on one JSON-RPC connection. That capability brings multi-entry session maps, reverse session/agent lookups, per-session prompt state, loading ids, demux for every event, cross-session teardown, and isolation concerns for future permission prompts and background tasks. The older [multi-session ACP proposal](../../proposed/feature/2026-06-14-acp-multi-session.md) still tracks the unfinished permission-ownership piece; this RFC is the competing simplification path. The product target has proven it needs concurrent editor conversations over one harness process: Zed's ACP connection owns multiple sessions and load states. The snapshot replay tier still avoids concurrent model streams because its replay entries are positional; that is a test-fixture limitation, not a reason to remove bridge multiplexing. @@ -20,7 +20,7 @@ Remove the multi-session maps and demux where a single `SessionRecord | undefine - `session/new` and `session/load` reject while that record exists. - Event handlers no longer demux across a `Map`. - Multi-session tests are removed or moved under the proposal that continues to defend multiplexing. -- The existing [multi-session ACP proposal](../proposed/2026-06-14-acp-multi-session.md) is updated to link this RFC and remains the live direction. +- The existing [multi-session ACP proposal](../../proposed/feature/2026-06-14-acp-multi-session.md) is updated to link this RFC and remains the live direction. ## What we give up diff --git a/docs/rfc/rejected/2026-06-20-truncate-interrupted-turns.md b/docs/rfc/rejected/simplification/2026-06-20-truncate-interrupted-turns.md similarity index 84% rename from docs/rfc/rejected/2026-06-20-truncate-interrupted-turns.md rename to docs/rfc/rejected/simplification/2026-06-20-truncate-interrupted-turns.md index 771388ede9..ed8e41d598 100644 --- a/docs/rfc/rejected/2026-06-20-truncate-interrupted-turns.md +++ b/docs/rfc/rejected/simplification/2026-06-20-truncate-interrupted-turns.md @@ -19,7 +19,7 @@ This makes the persisted turn boundary simple: a completed `turn/end` is the che - `TurnEndReasonMap` drops the `interrupted` variant. - `interruptedTurnClosers()` and its tests disappear. - The persistence coordinator's repair hook truncates backend-specific torn/open tail state without appending closers. -- [Session persistence docs](../../../packages/session-persistence/README.md) say load returns the last completed turn, plus no partial final turn. +- [Session persistence docs](../../../../packages/session-persistence/README.md) say load returns the last completed turn, plus no partial final turn. - Snapshot and contract tests update together with the behavior they pin. - The session format version and recorded fixtures are refreshed; non-current stored logs are rejected per the pre-release format policy, with no migration path. @@ -29,4 +29,4 @@ A crash can lose real work from the final turn: assistant text, tool calls, and ## Related -This is a direct simplification of [session persistence](../implemented/2026-06-14-session-persistence.md) and [turn enclosure](../implemented/2026-06-15-turn-enclosure-invariant.md). It also removes much of the motivation for durable step boundary events, making [drop durable step boundary events](2026-06-20-drop-durable-step-boundaries.md) smaller. +This is a direct simplification of [session persistence](../../implemented/architecture/2026-06-14-session-persistence.md) and [turn enclosure](../../implemented/architecture/2026-06-15-turn-enclosure-invariant.md). It also removes much of the motivation for durable step boundary events, making [drop durable step boundary events](2026-06-20-drop-durable-step-boundaries.md) smaller. diff --git a/examples/acp-agent/tests/snapshot-harness.ts b/examples/acp-agent/tests/snapshot-harness.ts index d66ee45d7f..54d64c3174 100644 --- a/examples/acp-agent/tests/snapshot-harness.ts +++ b/examples/acp-agent/tests/snapshot-harness.ts @@ -10,7 +10,7 @@ * shutdown flush. Two pure normalizers turn the captured stdout frames and the * session-log events into stable, snapshot-able text. * - * See docs/rfc/implemented/2026-06-19-acp-snapshot-tests.md. + * See docs/rfc/implemented/testing/2026-06-19-acp-snapshot-tests.md. */ import { spawn, type ChildProcessWithoutNullStreams } from 'node:child_process' diff --git a/examples/acp-agent/tests/snapshot-normalize.ts b/examples/acp-agent/tests/snapshot-normalize.ts index f863913591..db0d493535 100644 --- a/examples/acp-agent/tests/snapshot-normalize.ts +++ b/examples/acp-agent/tests/snapshot-normalize.ts @@ -11,7 +11,7 @@ * `time` (epoch ms) and header `createdAt` → 0. NOT scrubbed: the log's `seq` * (deterministic — `seq = log.length`, part of the event-log contract). * - * See docs/rfc/implemented/2026-06-19-acp-snapshot-tests.md. + * See docs/rfc/implemented/testing/2026-06-19-acp-snapshot-tests.md. */ const SESSION_ID = '{{sessionId}}' diff --git a/package.json b/package.json index eeb5dd7d07..fadc903cbb 100644 --- a/package.json +++ b/package.json @@ -26,13 +26,15 @@ "doc-typecheck": "tsx scripts/doc-typecheck.ts", "verify-md-wrap": "tsx scripts/verify-md-wrap.ts", "verify-md-links": "tsx scripts/verify-md-links.ts", + "verify-doc-refs": "tsx scripts/verify-doc-refs.ts", + "verify-rfc-classification": "tsx scripts/verify-rfc-classification.ts", "verify-type-equiv": "tsx scripts/verify-type-equiv.ts", "gen-cordis-catalog": "tsx scripts/gen-cordis-catalog.ts", "verify-cordis-catalog": "tsx scripts/gen-cordis-catalog.ts --check", "gen-module-graph": "tsx scripts/gen-module-graph.ts", "verify-module-graph": "tsx scripts/gen-module-graph.ts --check", "constraints": "tsx scripts/check-workspace-constraints.ts", - "doc-sync": "pnpm run doc-typecheck && pnpm run verify-cordis-catalog && pnpm run verify-md-wrap && pnpm run verify-md-links && pnpm run verify-type-equiv", + "doc-sync": "pnpm run doc-typecheck && pnpm run verify-cordis-catalog && pnpm run verify-md-wrap && pnpm run verify-md-links && pnpm run verify-doc-refs && pnpm run verify-rfc-classification && pnpm run verify-type-equiv", "hygiene": "pnpm run knip && pnpm run publint && pnpm run constraints", "demo:echo": "node --expose-internals --import tsx examples/echo-agent/start.ts", "demo:coding": "node --expose-internals --import tsx examples/coding-agent/start.ts", diff --git a/packages/README.md b/packages/README.md index b9fe1aa5f1..68d7600c85 100644 --- a/packages/README.md +++ b/packages/README.md @@ -30,7 +30,7 @@ dsh-ui-stdio ← dsh-agent, dsh-llm, dsh-session (stdio readline UI plugin) dsh-llm-replay ← dsh-llm, dsh-session (record/replay adapter for keyless snapshot tests) ``` -The rule: plugins depend on interfaces, never on the concrete loop. `dsh-agent-loop` is swappable — UI/hook/tool plugins keep working against the `dsh-agent` vocabulary if the loop is replaced. A swappable capability splits into interface / implementation / consumer packages (the bash trio is the template — see [capability seams](../docs/rfc/implemented/2026-06-13-capability-seams.md)). +The rule: plugins depend on interfaces, never on the concrete loop. `dsh-agent-loop` is swappable — UI/hook/tool plugins keep working against the `dsh-agent` vocabulary if the loop is replaced. A swappable capability splits into interface / implementation / consumer packages (the bash trio is the template — see [capability seams](../docs/rfc/implemented/architecture/2026-06-13-capability-seams.md)). ## What goes where diff --git a/packages/acp/README.md b/packages/acp/README.md index 2c6d531a60..b0334a0a2a 100644 --- a/packages/acp/README.md +++ b/packages/acp/README.md @@ -1,8 +1,8 @@ # @deepseek-ai/dsh-acp -The **Agent Client Protocol (ACP)** bridge: exposes the DeepSeek Harness coding agent as an ACP server over JSON-RPC stdio, so editors (Zed and other ACP clients) can drive it — streaming render, tool-call display, and resumable sessions. Zed is the current target client: baseline ACP behavior should remain reasonable for other clients, but bridge capabilities and compatibility decisions are evaluated against Zed first. **N concurrent sessions per connection** (see [ACP multi-session](../../docs/rfc/proposed/2026-06-14-acp-multi-session.md)): each maps to its own `ReactLoopAgent`, and every event is demuxed strictly by session id so two sessions streaming at once never interleave. +The **Agent Client Protocol (ACP)** bridge: exposes the DeepSeek Harness coding agent as an ACP server over JSON-RPC stdio, so editors (Zed and other ACP clients) can drive it — streaming render, tool-call display, and resumable sessions. Zed is the current target client: baseline ACP behavior should remain reasonable for other clients, but bridge capabilities and compatibility decisions are evaluated against Zed first. **N concurrent sessions per connection** (see [ACP multi-session](../../docs/rfc/proposed/feature/2026-06-14-acp-multi-session.md)): each maps to its own `ReactLoopAgent`, and every event is demuxed strictly by session id so two sessions streaming at once never interleave. -It is a **client-driver / UI plugin**, the structured analogue of the readline `stdio-chat` plugin — NOT a loop change and NOT a [capability seam](../../docs/rfc/implemented/2026-06-13-capability-seams.md). It consumes the existing `agent/*` event taxonomy, the `dsh-agent` create/resume factory, and `dsh-session-persistence`. +It is a **client-driver / UI plugin**, the structured analogue of the readline `stdio-chat` plugin — NOT a loop change and NOT a [capability seam](../../docs/rfc/implemented/architecture/2026-06-13-capability-seams.md). It consumes the existing `agent/*` event taxonomy, the `dsh-agent` create/resume factory, and `dsh-session-persistence`. ## Service / plugin @@ -53,7 +53,7 @@ A tool whose call IS a shell command (`bash`) can render as a real **terminal ca - `tool_call`: `content:[…, {type:'terminal', terminalId}]` + `_meta.terminal_info.{terminal_id, cwd}` — the terminal id is the harness `callId`; the cwd is the tool's explicit absolute `terminal.cwd`, else a relative `terminal.cwd` resolved against the session cwd, else the session's workspace cwd (the bridge fills the default, since the pure tool presenter can't see it). Any pending `content` the tool supplied (e.g. bash's `description`) renders BEFORE the terminal block, so the description sits above the card. - `tool_call_update`: `_meta.terminal_output.{terminal_id, data}` (the captured output) plus `_meta.terminal_exit.{terminal_id, exit_code | signal}` when the tool reported a structured exit. In terminal mode the update's `content` is OMITTED — an ACP `tool_call_update.content` REPLACES the call's content, so sending the fenced text block would clobber the terminal content block from the call. -When the client does NOT advertise the capability, none of the `_meta`/terminal content is emitted: the `tool_call` shows the `description` content block and the `tool_call_update` carries the ` ```console ` text block (above) as the rendering — so a non-Zed client is never worse off. The `_meta` object is ACP's spec-blessed extensibility point; the specific `terminal_info`/`terminal_output`/`terminal_exit` keys are a Zed convention, not the ACP `terminal/create` sub-protocol (which would make the editor execute the command, bypassing `dsh-bash`'s sandbox/env-scrub/ownership/cwd). Live incremental streaming and command classification are follow-ups. See [the terminal-rendering RFC](../../docs/rfc/implemented/2026-06-18-acp-terminal-and-tool-rendering.md). +When the client does NOT advertise the capability, none of the `_meta`/terminal content is emitted: the `tool_call` shows the `description` content block and the `tool_call_update` carries the ` ```console ` text block (above) as the rendering — so a non-Zed client is never worse off. The `_meta` object is ACP's spec-blessed extensibility point; the specific `terminal_info`/`terminal_output`/`terminal_exit` keys are a Zed convention, not the ACP `terminal/create` sub-protocol (which would make the editor execute the command, bypassing `dsh-bash`'s sandbox/env-scrub/ownership/cwd). Live incremental streaming and command classification are follow-ups. See [the terminal-rendering RFC](../../docs/rfc/implemented/feature/2026-06-18-acp-terminal-and-tool-rendering.md). ## Settle-exactly-once @@ -65,12 +65,12 @@ Teardown reaches quiescence: for EVERY live session settle any pending prompt as ## Known limitations (tracked TODOs) -- **`TODO(rfc010-permission-gate)`** — the `tools/execute` permission gate (`session/request_permission`) is NOT implemented; tools run with the executor's full authority. The `agent→sessionId` reverse map is in place so the gate can route a permission request (which receives only `exec.agent`) back to its originating session. [ACP support](../../docs/rfc/proposed/2026-06-14-acp-agent-client-protocol.md) and [ACP multi-session](../../docs/rfc/proposed/2026-06-14-acp-multi-session.md) stay `proposed` until the gate (and per-session permission ownership) land. +- **`TODO(rfc010-permission-gate)`** — the `tools/execute` permission gate (`session/request_permission`) is NOT implemented; tools run with the executor's full authority. The `agent→sessionId` reverse map is in place so the gate can route a permission request (which receives only `exec.agent`) back to its originating session. [ACP support](../../docs/rfc/proposed/feature/2026-06-14-acp-agent-client-protocol.md) and [ACP multi-session](../../docs/rfc/proposed/feature/2026-06-14-acp-multi-session.md) stay `proposed` until the gate (and per-session permission ownership) land. - **`additionalDirectories`** — rejected. A session operates in its single `cwd` (see Per-session cwd); widening the tool/filesystem scope to extra roots is a separate sandbox concern, not yet implemented. ## stdout is the protocol -The JSON-RPC frames go on stdout, so this plugin MUST run in an example that loads **no stdout logger** (the console logger writes to stdout and would corrupt the frames). The guarantee is config-only — see `examples/acp-agent` (no console logger) and [ACP support risks](../../docs/rfc/proposed/2026-06-14-acp-agent-client-protocol.md#risks). A stderr exporter is fine for logging. +The JSON-RPC frames go on stdout, so this plugin MUST run in an example that loads **no stdout logger** (the console logger writes to stdout and would corrupt the frames). The guarantee is config-only — see `examples/acp-agent` (no console logger) and [ACP support risks](../../docs/rfc/proposed/feature/2026-06-14-acp-agent-client-protocol.md#risks). A stderr exporter is fine for logging. ## Running diff --git a/packages/acp/acp-feature-support.md b/packages/acp/acp-feature-support.md index 04b1a2f258..d5dd379bf5 100644 --- a/packages/acp/acp-feature-support.md +++ b/packages/acp/acp-feature-support.md @@ -92,7 +92,7 @@ These are capabilities the bridge would *drive* on the editor. The harness runs ## 5. Tool-call rendering -Tool-call presentation is **owned by each tool** (`presentCall` / `presentResult` on the `dsh-tools` definition), not special-cased in the bridge — see the [terminal-and-tool-rendering RFC](../../docs/rfc/implemented/2026-06-18-acp-terminal-and-tool-rendering.md). +Tool-call presentation is **owned by each tool** (`presentCall` / `presentResult` on the `dsh-tools` definition), not special-cased in the bridge — see the [terminal-and-tool-rendering RFC](../../docs/rfc/implemented/feature/2026-06-18-acp-terminal-and-tool-rendering.md). | Feature | Stable | Bridge | Claude | Codex | Notes | |---|---|---|---|---|---| @@ -130,7 +130,7 @@ The bridge rejects unsupported prompt blocks rather than silently dropping them | Feature | Stable | Bridge | Notes | |---|---|---|---| | `StopReason` mapping | S | ✅ | `turnEndToStopReason` is total over harness turn-end reasons → `end_turn`/`max_tokens`/`cancelled`. | -| Multi-session (N per connection) | S | ✅ | Strict per-session demux; concurrent streams never interleave. See the [multi-session RFC](../../docs/rfc/proposed/2026-06-14-acp-multi-session.md). | +| Multi-session (N per connection) | S | ✅ | Strict per-session demux; concurrent streams never interleave. See the [multi-session RFC](../../docs/rfc/proposed/feature/2026-06-14-acp-multi-session.md). | | Disconnect / disposal teardown | S | ✅ | Quiesces every live session on client disconnect or Cordis disposal. | | `_meta` extensibility | S | ⚠️ | Consumed (Zed terminal cap) and emitted (terminal `_meta`); no other custom extensions. | | Background-task ownership isolation | — | ✅ | `bash_output`/`bash_kill` reject another session's task via an opaque owner token. | diff --git a/packages/agent-loop/README.md b/packages/agent-loop/README.md index 5d79d90147..c51c9132b6 100644 --- a/packages/agent-loop/README.md +++ b/packages/agent-loop/README.md @@ -13,7 +13,7 @@ This is the only package in the harness that contains concrete loop logic. Every `AgentLoop` also implements the `AgentFactory` seam and registers itself via `ctx.agents.setFactory(this)`, so plugins create/resume agents through `ctx.agents` (the interface): - `ctx.agents.create({ agentId, sessionId, meta?, agentOptions? }): AgentHandle` — programmatic create on a caller-supplied `sessionId` (e.g. an ACP-generated id), NOT `${id}-session`. Returns an [`AgentHandle`](../agent/README.md) — the owner disposes it to tear down exactly this agent (stop loop + await quiescence + unregister + remove session). -- `ctx.agents.resume({ agentId, resumeSessionId, agentOptions? }): Promise` — load a persisted session via `ctx.sessionPersistence` ([session persistence](../../docs/rfc/implemented/2026-06-14-session-persistence.md)) and resume an agent on it. The live session id is the resumed id; turn numbering and derived history continue from the loaded log. Requires a session-persistence backend (NOT hard-injected — non-persistent demos still work; `resume` rejects with a clear error when persistence is absent). Returns an `AgentHandle`. +- `ctx.agents.resume({ agentId, resumeSessionId, agentOptions? }): Promise` — load a persisted session via `ctx.sessionPersistence` ([session persistence](../../docs/rfc/implemented/architecture/2026-06-14-session-persistence.md)) and resume an agent on it. The live session id is the resumed id; turn numbering and derived history continue from the loaded log. Requires a session-persistence backend (NOT hard-injected — non-persistent demos still work; `resume` rejects with a clear error when persistence is absent). Returns an `AgentHandle`. The config-driven `ctx.agentLoop.create()` path keeps its agent owned by the loop fiber (it discards the handle) — only the programmatic factory callers (the ACP bridge) hold a handle and own per-agent teardown. diff --git a/packages/agent/README.md b/packages/agent/README.md index 846ab17444..39fba37fd0 100644 --- a/packages/agent/README.md +++ b/packages/agent/README.md @@ -18,7 +18,7 @@ Agent *creation* is provided by whichever plugin implements `AgentFactory` (phas - `ctx.agents.setFactory(factory: AgentFactory): () => void` — register the creation factory (the loop calls this on construction). Throws on a second factory; the slot clears on dispose. - `ctx.agents.create(options: CreateAgentOptions): AgentHandle` — construct, start, AND register a new agent on a caller-supplied `sessionId` (with optional `meta.cwd`). Distinct from `register` (which only records). Throws if no factory is registered. -- `ctx.agents.resume(options: ResumeAgentOptions): Promise` — load a persisted session ([session persistence](../../docs/rfc/implemented/2026-06-14-session-persistence.md)) and resume an agent on it. Async; rejects if no factory is registered, or if the factory finds session persistence unconfigured. +- `ctx.agents.resume(options: ResumeAgentOptions): Promise` — load a persisted session ([session persistence](../../docs/rfc/implemented/architecture/2026-06-14-session-persistence.md)) and resume an agent on it. Async; rejects if no factory is registered, or if the factory finds session persistence unconfigured. `AgentHandle = { agent: Agent; dispose(): Promise }`. The disposer is a **capability** — only the holder can tear this agent down. `dispose()` stops the loop, `await`s its exit (quiescence — NOT just the `disposed` status flip), unregisters the agent, and removes its session from the store, in an order that captures the loop's final `session/flush` before the session is detached. `ctx.agents.get(id)` still returns a bare `Agent` — the handle is only for the OWNER that created it. The ACP bridge is the production consumer (one handle per session, disposed on disconnect/teardown); config-created agents are owned by the loop fiber and never need a handle. @@ -55,7 +55,7 @@ The handle every plugin programs against: - `agent.send(content, options?)` — queue a message; starts a turn when idle - `agent.steer(content, options?)` — steer a running turn (inject between steps); behaves like `send` when idle -- `agent.inject(content, options?)` — inject in-session context (context/message event); the next request sees it. Does not run the model. While a turn is open it joins that turn; while idle it is wrapped in a one-shot `injection` turn so every event stays turn-enclosed ([the turn-enclosure invariant](../../docs/rfc/implemented/2026-06-15-turn-enclosure-invariant.md)) +- `agent.inject(content, options?)` — inject in-session context (context/message event); the next request sees it. Does not run the model. While a turn is open it joins that turn; while idle it is wrapped in a one-shot `injection` turn so every event stays turn-enclosed ([the turn-enclosure invariant](../../docs/rfc/implemented/architecture/2026-06-15-turn-enclosure-invariant.md)) - `agent.abort(reason?)` — abort the in-flight step (the narrow, step-only verb) - `agent.cancel(reason?)` — cancel ALL pending work: clears the queued + steering FIFOs, aborts the in-flight step, and drops a turn about to start (the pre-step window) so a queued-but-not-started prompt never runs. A UI/ACP `session/cancel` maps to this. Idle with nothing pending → a safe no-op. - `agent.whenIdle()` — resolve once the agent reaches quiescence after settling out of `running` (idle → immediately; disposed → awaits the loop exit), the signal a teardown awaits (`abort()` then `await whenIdle()`). Observes the transition without disposing the agent. diff --git a/packages/invariants/README.md b/packages/invariants/README.md index 094b7e13a0..a3901a0f5d 100644 --- a/packages/invariants/README.md +++ b/packages/invariants/README.md @@ -44,7 +44,7 @@ On any violation it throws `InvariantError` (`code: 'INVARIANT'`). ## Why runtime, not deep-readonly types -A `DeepReadonly` is high type-noise across every log consumer, and a plugin can cast straight through it. A dev-mode freeze plus these assertions catch real corruption at zero production cost and zero type noise. The always-on half of that defense — cloning derived messages so request/adapter mutation can't reach back into the log — lives in `dsh-session`'s `deriveMessages`. This package is the dev-mode tripwire. See [dev-mode invariants](../../docs/rfc/implemented/2026-06-11-dev-invariants-over-deep-readonly.md). +A `DeepReadonly` is high type-noise across every log consumer, and a plugin can cast straight through it. A dev-mode freeze plus these assertions catch real corruption at zero production cost and zero type noise. The always-on half of that defense — cloning derived messages so request/adapter mutation can't reach back into the log — lives in `dsh-session`'s `deriveMessages`. This package is the dev-mode tripwire. See [dev-mode invariants](../../docs/rfc/implemented/architecture/2026-06-11-dev-invariants-over-deep-readonly.md). ## Seeded sessions diff --git a/packages/llm-replay/src/index.ts b/packages/llm-replay/src/index.ts index 9a25cc6e0c..a1c5f4f1d1 100644 --- a/packages/llm-replay/src/index.ts +++ b/packages/llm-replay/src/index.ts @@ -5,7 +5,7 @@ * waterfall (never calls `next()`) and yields model streams reconstructed from * a recorded **session JSONL** fixture — so a snapshot test can boot the real * agent against a fixed model transcript with no API key. See - * docs/rfc/implemented/2026-06-19-acp-snapshot-tests.md. + * docs/rfc/implemented/testing/2026-06-19-acp-snapshot-tests.md. * * The fixture IS the persisted session log (`/session.jsonl`): its * `assistant/chunk` events carry every {@link StreamChunk}, so grouping them by diff --git a/packages/llm/README.md b/packages/llm/README.md index 3c16c3153c..02758d404c 100644 --- a/packages/llm/README.md +++ b/packages/llm/README.md @@ -43,4 +43,4 @@ Streaming is a raw chunk protocol (`block-start`, `text-delta`, `reasoning-delta ### Real adapters -Two adapters implement `LlmAdapter` against this vocabulary, deliberately built on different internals to keep the contract honest (see [the twin LLM adapters](../../docs/rfc/implemented/2026-06-13-twin-llm-adapters.md)): [`@deepseek-ai/dsh-llm-deepseek`](../llm-deepseek) (hand-rolled fetch/SSE) and [`@deepseek-ai/dsh-llm-pi-ai`](../llm-pi-ai) (via `@earendil-works/pi-ai`). The pair pinned down the `StreamChunk` conventions now documented in `types.ts` (usage before finish, raw-string tool arguments, the two sanctioned error paths). +Two adapters implement `LlmAdapter` against this vocabulary, deliberately built on different internals to keep the contract honest (see [the twin LLM adapters](../../docs/rfc/implemented/architecture/2026-06-13-twin-llm-adapters.md)): [`@deepseek-ai/dsh-llm-deepseek`](../llm-deepseek) (hand-rolled fetch/SSE) and [`@deepseek-ai/dsh-llm-pi-ai`](../llm-pi-ai) (via `@earendil-works/pi-ai`). The pair pinned down the `StreamChunk` conventions now documented in `types.ts` (usage before finish, raw-string tool arguments, the two sanctioned error paths). diff --git a/packages/session-persistence-jsonl/README.md b/packages/session-persistence-jsonl/README.md index 640f132e78..92899de4b5 100644 --- a/packages/session-persistence-jsonl/README.md +++ b/packages/session-persistence-jsonl/README.md @@ -23,7 +23,7 @@ The JSONL durable session-persistence backend — a concrete `SessionPersistence - **Lazy materialization.** `create(meta)` writes nothing; the `.jsonl` (header + first batch) is written atomically (temp-write + `fsync` + rename) on the first `append`. A created-but-never-appended session leaves nothing on disk and is absent from `has`/`list`. - **Append-only.** Committed events (at or below a flushed `turn/end`) are never rewritten. Subsequent appends are line appends at EOF + `fsync`. -- **Crash recovery — close, don't truncate.** A crash can leave a log whose final turn never closed (real events after the last `turn/end`). `load` PRESERVES those events (a turn can be huge — they are real work) and closes the orphaned turn by durably appending synthetic boundary events: an error `tool/result` for every `tool-call` the crash left unanswered (the loop logs the assistant message before running the tools, so a mid-tool crash leaves dangling calls — and `deriveMessages()` would replay an assistant tool-call with no result, which providers reject), then a `step/end` if a step was open, then `turn/end {kind:'interrupted'}`, returning a balanced log. Only a never-fully-written **torn tail fragment** (a final line with no newline / unparseable) is `ftruncate`d away before the closers are written. See [session persistence](../../docs/rfc/implemented/2026-06-14-session-persistence.md). +- **Crash recovery — close, don't truncate.** A crash can leave a log whose final turn never closed (real events after the last `turn/end`). `load` PRESERVES those events (a turn can be huge — they are real work) and closes the orphaned turn by durably appending synthetic boundary events: an error `tool/result` for every `tool-call` the crash left unanswered (the loop logs the assistant message before running the tools, so a mid-tool crash leaves dangling calls — and `deriveMessages()` would replay an assistant tool-call with no result, which providers reject), then a `step/end` if a step was open, then `turn/end {kind:'interrupted'}`, returning a balanced log. Only a never-fully-written **torn tail fragment** (a final line with no newline / unparseable) is `ftruncate`d away before the closers are written. See [session persistence](../../docs/rfc/implemented/architecture/2026-06-14-session-persistence.md). - **Contiguous-seq.** `load` rejects a mid-log parse error or `seq` gap (unloadable); `append` rejects a batch whose first `seq` does not continue the stored log, and rejects non-JSON-serializable `event.data` naming the offending event type. - **Format version.** Only v1 is supported; `load` rejects an unknown version. While the harness is unreleased a format change bumps the version and rejects non-current logs — there is no migration (no persisted user data to preserve). diff --git a/packages/session-persistence-sqlite/README.md b/packages/session-persistence-sqlite/README.md index d821b56446..74e79ac8f3 100644 --- a/packages/session-persistence-sqlite/README.md +++ b/packages/session-persistence-sqlite/README.md @@ -1,6 +1,6 @@ # @deepseek-ai/dsh-session-persistence-sqlite -A SQLite durable session-persistence backend — a second `SessionPersistence` implementation ([session persistence](../../docs/rfc/implemented/2026-06-14-session-persistence.md)), built to validate that the abstract seam and the shared `runPersistenceContract` suite are genuinely backend-agnostic. It satisfies the SAME contract as `dsh-session-persistence-jsonl` (append-only, contiguous-seq, lazy materialization, interrupted-turn close on load), expressed over `node:sqlite` rows instead of file bytes. +A SQLite durable session-persistence backend — a second `SessionPersistence` implementation ([session persistence](../../docs/rfc/implemented/architecture/2026-06-14-session-persistence.md)), built to validate that the abstract seam and the shared `runPersistenceContract` suite are genuinely backend-agnostic. It satisfies the SAME contract as `dsh-session-persistence-jsonl` (append-only, contiguous-seq, lazy materialization, interrupted-turn close on load), expressed over `node:sqlite` rows instead of file bytes. > **TODO:** this backend talks to `node:sqlite` directly. If a cordis database service (`cordis/db` / a `@cordisjs` SQL driver plugin) is adopted, route through that instead of holding a raw `DatabaseSync` here — the contract surface (`SessionPersistence`) would not change, only the storage driver. diff --git a/packages/session-persistence/README.md b/packages/session-persistence/README.md index 3ec9f7860b..42fb137287 100644 --- a/packages/session-persistence/README.md +++ b/packages/session-persistence/README.md @@ -1,6 +1,6 @@ # @deepseek-ai/dsh-session-persistence -The abstract durable session-persistence seam (`ctx.sessionPersistence`). Defines WHAT a persistence backend does — durably store, reload, and list sessions — without saying HOW. Mirrors the `dsh-bash` capability-seam template ([capability seams](../../docs/rfc/implemented/2026-06-13-capability-seams.md)): an abstract service here, a concrete implementation in a sibling package, consumers that inject the interface. +The abstract durable session-persistence seam (`ctx.sessionPersistence`). Defines WHAT a persistence backend does — durably store, reload, and list sessions — without saying HOW. Mirrors the `dsh-bash` capability-seam template ([capability seams](../../docs/rfc/implemented/architecture/2026-06-13-capability-seams.md)): an abstract service here, a concrete implementation in a sibling package, consumers that inject the interface. The persisted unit IS the existing `SessionEvent` (event-sourced model — the log is the single source of truth), so there is no parallel "persisted message" type. Metadata that is NOT replayable conversation state (format version, cwd, lineage) travels separately as `SessionHeader`, owned by `dsh-session` and re-exported here. @@ -39,7 +39,7 @@ The `PersistenceBackend` hooks (the only seam between the coordinato | `deleteStored(id)` / `list()` | Remove a stored artifact / list all stored metadata. | | `close?()` | Optional lifecycle teardown (e.g. close a db handle), awaited after the dispose drain. | -The `tornMarker` is fully OPAQUE: the coordinator only tests `!== undefined` and round-trips it to `commitRepair`, never inspecting its value (the JSONL backend uses the byte offset to truncate to, the SQLite backend the seq to delete from). The public `SessionPersistence` service shape is unchanged, so a third-party backend MAY still implement the abstract service directly without the coordinator. See [the write-coordinator RFC](../../docs/rfc/implemented/2026-06-18-shared-persistence-write-coordinator.md). +The `tornMarker` is fully OPAQUE: the coordinator only tests `!== undefined` and round-trips it to `commitRepair`, never inspecting its value (the JSONL backend uses the byte offset to truncate to, the SQLite backend the seq to delete from). The public `SessionPersistence` service shape is unchanged, so a third-party backend MAY still implement the abstract service directly without the coordinator. See [the write-coordinator RFC](../../docs/rfc/implemented/architecture/2026-06-18-shared-persistence-write-coordinator.md). ## Testing backends diff --git a/packages/session-persistence/src/coordinator.ts b/packages/session-persistence/src/coordinator.ts index 5371873097..ad21f3a8ca 100644 --- a/packages/session-persistence/src/coordinator.ts +++ b/packages/session-persistence/src/coordinator.ts @@ -18,7 +18,7 @@ * a coordinator it composes), so a third-party backend MAY implement the service * directly without using the coordinator at all. * - * See the write-coordinator RFC (docs/rfc/implemented/2026-06-18-shared-persistence-write-coordinator.md) + * See the write-coordinator RFC (docs/rfc/implemented/architecture/2026-06-18-shared-persistence-write-coordinator.md) * for the design rationale (composition over inheritance, the opaque torn marker). * * @module @deepseek-ai/dsh-session-persistence/coordinator diff --git a/scripts/verify-doc-refs.ts b/scripts/verify-doc-refs.ts new file mode 100644 index 0000000000..322368b724 --- /dev/null +++ b/scripts/verify-doc-refs.ts @@ -0,0 +1,96 @@ +/** + * Doc-sync gate: verify that doc references written in TypeScript COMMENTS + * resolve to a file that exists. Source comments cite docs by root-relative + * prose path — `see docs/rfc/implemented/testing/2026-06-19-acp-snapshot-tests.md`, + * `docs/architecture.md § plugin checklist`. `verify-md-links` parses Markdown + * link AST and never sees these, so a doc rename or move could silently orphan + * a `.ts` comment that points at it. The RFC classification reorg + * ([the classification RFC](../docs/rfc/implemented/process/2026-06-20-rfc-classification.md)) + * is the motivating case: it moved every RFC under a `{class}/` folder, and + * several `.ts` doc comments cite RFC paths that changed. + * + * Detection is a token scan, NOT an AST walk: doc refs live in free prose inside + * comments, not in a structured form. We match `docs/.md` tokens and + * REQUIRE the `.md` extension, so extensionless prose (`docs/postmortem/0001`, + * `docs/architecture.md § plugin checklist` — the section suffix is outside the + * token) is left alone rather than misread as a path. Each token is resolved + * ROOT-RELATIVE (the way the comments are written) and must exist on disk. This + * is checker, not fixer: it reports and never rewrites. + * + * Scope is repo-authored TypeScript under `packages/**` and `examples/**`, + * excluding built output (`lib/`, `*.d.ts`) and `vendor/` (pinned upstream + * source we do not own). The scan is purely textual, so it does not distinguish + * a token in a comment from one in a string literal — a `docs/….md` string in + * code is checked too, which is harmless (such a path should resolve anyway). + * + * Run: `tsx scripts/verify-doc-refs.ts`. + */ + +import { existsSync, readFileSync } from 'node:fs' +import { relative, resolve } from 'node:path' +import { glob } from 'node:fs/promises' + +const root = resolve(import.meta.dirname, '..') + +/** Repo-authored TypeScript that may cite docs in comments. */ +const PATTERNS = ['packages/**/*.ts', 'examples/**/*.ts'] + +/** Paths excluded from the scan: built output and vendored upstream source. */ +const isExcluded = (p: string): boolean => + p.includes('/lib/') || p.endsWith('.d.ts') || p.startsWith('vendor/') + +/** + * Match a `docs/…​.md` reference token. The `.md` extension is required so a + * bare `docs/postmortem/0001` (no extension) does not register as a path. The + * character class stops at whitespace, backticks, parens, and the section sign, + * so trailing prose (`… .md § plugin checklist`) is not swallowed into the path. + */ +const DOC_REF = /\bdocs\/[A-Za-z0-9._/-]+\.md/g + +/** A broken doc reference: a root-relative `docs/….md` token with no file. */ +interface Violation { + file: string + /** 1-based line where the reference appears. */ + line: number + ref: string +} + +/** Find every broken `docs/….md` reference in one TypeScript file. */ +function findViolations(absPath: string): Violation[] { + const file = relative(root, absPath) + const source = readFileSync(absPath, 'utf8') + const out: Violation[] = [] + const lines = source.split('\n') + for (let i = 0; i < lines.length; i++) { + const line = lines[i] + if (line === undefined) continue + for (const m of line.matchAll(DOC_REF)) { + const ref = m[0] + if (!existsSync(resolve(root, ref))) { + out.push({ file, line: i + 1, ref }) + } + } + } + return out +} + +const all: Violation[] = [] +let checked = 0 +for (const pattern of PATTERNS) { + for await (const match of glob(pattern, { cwd: root })) { + if (isExcluded(match)) continue + checked++ + all.push(...findViolations(resolve(root, match))) + } +} + +if (all.length === 0) { + console.log(`verify-doc-refs: ${checked} file(s) checked, all docs/*.md references resolve.`) + process.exit(0) +} + +console.error('verify-doc-refs: broken docs/*.md references found in source comments (target does not exist):') +for (const v of all) { + console.error(` ${v.file}:${v.line} ${v.ref}`) +} +process.exit(1) diff --git a/scripts/verify-rfc-classification.ts b/scripts/verify-rfc-classification.ts new file mode 100644 index 0000000000..011ce9591a --- /dev/null +++ b/scripts/verify-rfc-classification.ts @@ -0,0 +1,160 @@ +/** + * Doc-sync gate: enforce the RFC classification scheme + * ([the classification RFC](../docs/rfc/implemented/process/2026-06-20-rfc-classification.md)). + * Every RFC is filed at `docs/rfc/{lifecycle}/{class}/yyyy-mm-dd-topic.md`; the + * folder IS the label. This gate is the machine source of truth for the closed + * class set and keeps the README index honest. + * + * Two checks: + * + * 1. STRUCTURE — every `.md` under a lifecycle folder lives in a class folder + * from CLASSES, named `yyyy-mm-dd-*.md`. A loose `.md` directly under a + * lifecycle root (other than the README/AGENTS allowlist) fails; an unknown + * class folder fails; a stray file at an unexpected depth fails. This is what + * makes the set CLOSED: a new class folder can't appear without amending + * CLASSES here (and the README's Classification section, per the RFC). + * + * 2. COMPLETENESS — `docs/rfc/README.md` lists every RFC exactly once, under the + * `### {Class}` heading inside the `## {Lifecycle}` section that matches the + * file's path. A missing entry, a duplicate, or an entry under the wrong + * heading fails. This mirrors `verify-event-taxonomy`: a curated doc table + * checked against the on-disk source of truth, so the index can't drift. + * + * The class DESCRIPTIONS in the README prose are not checked (they are + * explanatory text); only the per-class index tables are. This is checker, not + * fixer: it reports and never rewrites. + * + * Run: `tsx scripts/verify-rfc-classification.ts`. + */ + +import { readFileSync } from 'node:fs' +import { relative, resolve } from 'node:path' +import { glob } from 'node:fs/promises' + +const root = resolve(import.meta.dirname, '..') +const rfcRoot = resolve(root, 'docs/rfc') + +/** The closed set of RFC lifecycles (top-level folders under docs/rfc/). */ +const LIFECYCLES = ['proposed', 'implemented', 'rejected'] as const + +/** + * The closed set of RFC classes (nested folder under each lifecycle). Adding a + * class is a deliberate act: extend this list AND the README's Classification + * section. The gate rejects any folder not listed here. + */ +const CLASSES = ['feature', 'bug-fix', 'simplification', 'architecture', 'process', 'testing'] as const + +/** Non-RFC Markdown allowed to sit directly at a lifecycle root. */ +const ROOT_ALLOWLIST = new Set(['AGENTS.md', 'CLAUDE.md']) + +/** Title-case a class/lifecycle folder name for README heading comparison. */ +const heading = (s: string): string => s.charAt(0).toUpperCase() + s.slice(1) + +const errors: string[] = [] + +// --- Check 1: structure ----------------------------------------------------- +// Every Markdown file anywhere under a lifecycle folder, at any depth. +interface Rfc { + lifecycle: string + cls: string + base: string + /** Path relative to docs/rfc, for the README link check. */ + rel: string +} +const rfcs: Rfc[] = [] + +for (const lifecycle of LIFECYCLES) { + for await (const match of glob(`${lifecycle}/**/*.md`, { cwd: rfcRoot })) { + const segs = match.split('/') + // Allowlisted file directly at the lifecycle root (e.g. implemented/AGENTS.md). + if (segs.length === 2 && ROOT_ALLOWLIST.has(segs[1] ?? '')) continue + const cls = segs[1] + const base = segs[2] + if (segs.length !== 3 || cls === undefined || base === undefined) { + errors.push(`structure: ${match} — expected {lifecycle}/{class}/file.md (got depth ${segs.length})`) + continue + } + if (!(CLASSES as readonly string[]).includes(cls)) { + errors.push(`structure: ${match} — unknown class folder "${cls}" (allowed: ${CLASSES.join(', ')})`) + continue + } + if (!/^\d{4}-\d{2}-\d{2}-.+\.md$/.test(base)) { + errors.push(`structure: ${match} — filename must be yyyy-mm-dd-topic.md`) + continue + } + rfcs.push({ lifecycle, cls, base, rel: match }) + } +} + +// --- Check 2: README completeness ------------------------------------------- +// Parse the index into (lifecycle, class) -> set of linked rel paths, by +// tracking the current `## {Lifecycle}` and `### {Class}` headings and reading +// every `](path)` link target underneath. A link target is normalized to its +// path relative to docs/rfc. +const readmePath = resolve(rfcRoot, 'README.md') +const readme = readFileSync(readmePath, 'utf8') +const lifecycleByHeading = new Map(LIFECYCLES.map((l): [string, string] => [heading(l), l])) +const classByHeading = new Map(CLASSES.map((c): [string, string] => [heading(c), c])) + +/** README-listed RFC link targets, keyed `lifecycle/class` -> set of rel paths. */ +const listed = new Map>() +let curLifecycle: string | null = null +let curClass: string | null = null + +for (const line of readme.split('\n')) { + const h2 = /^##\s+(.+?)\s*$/.exec(line) + if (h2?.[1] !== undefined) { + curLifecycle = lifecycleByHeading.get(h2[1].trim()) ?? null + curClass = null + continue + } + const h3 = /^###\s+(.+?)\s*$/.exec(line) + if (h3?.[1] !== undefined) { + curClass = classByHeading.get(h3[1].trim()) ?? null + continue + } + if (!curLifecycle || !curClass) continue + // Collect every relative .md link target on this line. + for (const m of line.matchAll(/\]\(([^)]+\.md)[^)]*\)/g)) { + const target = m[1] + if (target === undefined) continue + // README links are relative to docs/rfc; normalize and key by location. + const rel = relative(rfcRoot, resolve(rfcRoot, target)) + const key = `${curLifecycle}/${curClass}` + const set = listed.get(key) ?? new Set() + set.add(rel) + listed.set(key, set) + } +} + +// Every on-disk RFC must be listed under the heading matching its path. +const seenOnDisk = new Set() +for (const rfc of rfcs) { + seenOnDisk.add(rfc.rel) + const key = `${rfc.lifecycle}/${rfc.cls}` + if (!listed.get(key)?.has(rfc.rel)) { + errors.push( + `index: ${rfc.rel} is not listed in README under "## ${heading(rfc.lifecycle)}" → "### ${heading(rfc.cls)}"`, + ) + } +} + +// Every README entry must point at a real RFC under that same heading (catches a +// misfiled or stale row). +for (const [key, targets] of listed) { + for (const rel of targets) { + if (!seenOnDisk.has(rel)) { + errors.push(`index: README lists "${rel}" under "${key}", but no such RFC exists`) + } + } +} + +// --- Report ----------------------------------------------------------------- +if (errors.length === 0) { + console.log(`verify-rfc-classification: ${rfcs.length} RFC(s) checked, structure and index consistent.`) + process.exit(0) +} + +console.error('verify-rfc-classification: violations found:') +for (const e of errors) console.error(` ${e}`) +process.exit(1)