Fix doc cross-links for the hierarchy; add package-path + shape gates
Merge brought in the RFC-classification reorg and two new doc gates; rewrite every drifted packages/<name> cross-link (Markdown link targets, moved-README relative depths, and .ts comment paths) to the grouped paths. Add two doc-sync/hygiene gates so the manual checks this restructure needed become automated: - verify-package-paths.ts: flags a packages/<path> reference (in Markdown or a .ts comment/string) that does not resolve AND names a real package in a segment — i.e. a stale path to a MOVED package. A path naming a non-existent package (a forward-looking proposal) is left alone, so it applies uniformly across proposed/implemented/rejected. - check-workspace-constraints: assert the packages/<group>/<pkg> depth-2 shape (group dirs carry no package.json; no flat or over-nested packages). Group names stay open; only the shape is fixed.
This commit is contained in:
Tianyi Cui
@@ -6,7 +6,7 @@ The file-by-file checklist for a new `@deepseek-ai/dsh-<name>` package. (Verifie
|
||||
|
||||
```
|
||||
packages/<name>/
|
||||
package.json # copy from packages/tools, adjust name/description/deps
|
||||
package.json # copy from packages/core/tools, adjust name/description/deps
|
||||
tsconfig.json # extends ../../tsconfig.base.json, rootDir src, outDir lib,
|
||||
# references: vendor/cosmokit, vendor/cordis (+ vendor/schemastery
|
||||
# if you use Config, + ../<dep> for each dsh dependency)
|
||||
@@ -25,7 +25,7 @@ package.json invariants (enforced by `pnpm run constraints` / `scripts/check-wor
|
||||
| `tsconfig.typecheck.json` | same entry (this file overrides the map wholesale) |
|
||||
| `tsconfig.build.json` | add `{ "path": "./packages/<name>" }` to `references` |
|
||||
| `scripts/publint-all.ts` | add `'packages/<name>'` to the array |
|
||||
| `knip.json` | only if the package has non-`*.spec.ts` entries (e.g. `*.e2e.ts` → add a per-workspace override like `packages/llm-deepseek`) |
|
||||
| `knip.json` | only if the package has non-`*.spec.ts` entries (e.g. `*.e2e.ts` → add a per-workspace override like `packages/llm/llm-deepseek`) |
|
||||
|
||||
Covered automatically by globs — no edits needed: root `package.json` workspaces, `tsdown.config.ts`, `vitest.config.ts`, `eslint.config.mjs`.
|
||||
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
# Cookbook: adding a tool
|
||||
|
||||
How to give the model a new capability. Reference implementations: `examples/echo-agent/src/echo-tool.ts` (minimal) and `packages/tool-bash` (production-grade, three-package seam).
|
||||
How to give the model a new capability. Reference implementations: `examples/echo-agent/src/echo-tool.ts` (minimal) and `packages/bash/tool-bash` (production-grade, three-package seam).
|
||||
|
||||
## The minimal shape
|
||||
|
||||
@@ -50,4 +50,4 @@ Prefer not to build policy into the tool. The seam is the `tools/execute` waterf
|
||||
|
||||
## Tests every tool needs
|
||||
|
||||
Arg-validation rejections, result shaping for every outcome, the HMR disposal test, and — for tools with side effects — an integration spec that drives the tool through the agent loop with a scripted `MockAdapter` (`packages/agent-loop/tests/mock-adapter.ts`), asserting the `tool/call` / `tool/result` session events.
|
||||
Arg-validation rejections, result shaping for every outcome, the HMR disposal test, and — for tools with side effects — an integration spec that drives the tool through the agent loop with a scripted `MockAdapter` (`packages/core/agent-loop/tests/mock-adapter.ts`), asserting the `tool/call` / `tool/result` session events.
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
# Cookbook: adding an LLM adapter
|
||||
|
||||
How to connect a new model provider. Reference implementations: `packages/llm-deepseek` (hand-rolled HTTP/SSE) and `packages/llm-pi-ai` (wrapping an LLM library). Read the `StreamChunk` doc in `packages/llm/src/types.ts` first — it records the protocol conventions both adapters were verified against.
|
||||
How to connect a new model provider. Reference implementations: `packages/llm/llm-deepseek` (hand-rolled HTTP/SSE) and `packages/llm/llm-pi-ai` (wrapping an LLM library). Read the `StreamChunk` doc in `packages/llm/llm/src/types.ts` first — it records the protocol conventions both adapters were verified against.
|
||||
|
||||
## The shape
|
||||
|
||||
|
||||
@@ -57,7 +57,7 @@ export function apply(ctx: Context) {
|
||||
|
||||
A *client driver* is a UI plugin whose "user" is another program speaking a wire protocol rather than a human at a terminal. It owns the process's stdio (so it must run with **no stdout logger** — every non-protocol byte corrupts the stream), creates/resumes agents on demand through the `dsh-agent` factory seam, translates harness events (`session/event`, `agent/*`) into outbound protocol messages, and translates inbound requests back into `agent.send()` / `agent.abort()`. Two harness-specific contracts make it correct: resolve each request exactly once off a settle signal (the turn can end without its `agent/turn-end` event firing — fall back through the logged `turn/end` record), and on disposal reach quiescence (`await agent.whenIdle()` after `abort()`), not just request it.
|
||||
|
||||
`packages/acp` is the worked example: it bridges the agent to the Agent Client Protocol (JSON-RPC over stdio) so Zed and other ACP editors can drive it. See its README for the full method surface and the deferred-permission-gate note.
|
||||
`packages/ui/acp` is the worked example: it bridges the agent to the Agent Client Protocol (JSON-RPC over stdio) so Zed and other ACP editors can drive it. See its README for the full method surface and the deferred-permission-gate note.
|
||||
|
||||
```ts
|
||||
import type { Context } from 'cordis'
|
||||
|
||||
@@ -1,8 +1,8 @@
|
||||
# Bash Executor
|
||||
|
||||
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.
|
||||
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/bash), `ctx.bash`), implementation ([dsh-bash-local](../../packages/bash/bash-local), local subprocesses), and consumer ([dsh-tool-bash](../../packages/bash/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)
|
||||
Source: [`packages/bash/bash/src/types.ts`](../../packages/bash/bash/src/types.ts)
|
||||
|
||||
## Request vs. spec: the `resolve()` split
|
||||
|
||||
@@ -120,4 +120,4 @@ interface BashTaskRead {
|
||||
|
||||
## The service
|
||||
|
||||
`BashExecutor` (`ctx.bash`, abstract — defined in [`packages/bash/src/index.ts`](../../packages/bash/src/index.ts)) mirrors the `LlmService`/`LlmAdapter` split: `resolve` (request → spec), `run` (foreground), `start` (background), `get`/`ownerOf`/`list`/`readOutput`/`kill`, and `onTaskDone` (a `BashTaskListener` completion callback). Spawned commands get a **scrubbed env** (dropping `*KEY*`/`*SECRET*`/`*TOKEN*`) and spill files use a private 0700 dir with random names and owner-only opens — model output never gets the ambient environment or a predictable path. The implementation that provides all this is `dsh-bash-local`; the model-facing `bash`/`bash_output`/`bash_kill` schemas that call it are in `dsh-tool-bash` (and present as terminals via the [tool-presentation vocabulary](tools.md#tool-presentation-ui-vocabulary)).
|
||||
`BashExecutor` (`ctx.bash`, abstract — defined in [`packages/bash/bash/src/index.ts`](../../packages/bash/bash/src/index.ts)) mirrors the `LlmService`/`LlmAdapter` split: `resolve` (request → spec), `run` (foreground), `start` (background), `get`/`ownerOf`/`list`/`readOutput`/`kill`, and `onTaskDone` (a `BashTaskListener` completion callback). Spawned commands get a **scrubbed env** (dropping `*KEY*`/`*SECRET*`/`*TOKEN*`) and spill files use a private 0700 dir with random names and owner-only opens — model output never gets the ambient environment or a predictable path. The implementation that provides all this is `dsh-bash-local`; the model-facing `bash`/`bash_output`/`bash_kill` schemas that call it are in `dsh-tool-bash` (and present as terminals via the [tool-presentation vocabulary](tools.md#tool-presentation-ui-vocabulary)).
|
||||
|
||||
@@ -61,7 +61,7 @@ Two large discriminated unions are the ones consumers `switch` over most: **`Str
|
||||
|
||||
IDs that cross package boundaries are **branded** — structurally strings, but non-interchangeable at the type level (an `AgentId` can't be passed where a `CallId` is expected). Construction goes through a per-type factory; comparison, logging, and JSON behave as ordinary strings.
|
||||
|
||||
Source: [`packages/llm/src/brand.ts`](../../packages/llm/src/brand.ts)
|
||||
Source: [`packages/llm/llm/src/brand.ts`](../../packages/llm/llm/src/brand.ts)
|
||||
|
||||
```ts type-equiv
|
||||
type Branded<B extends string> = string & { readonly [BRAND]: B }
|
||||
@@ -73,7 +73,7 @@ The three core IDs: `CallId` (correlates a tool call with its result; dsh-llm),
|
||||
|
||||
A conversation is `Message`s; a message is an array of typed **content blocks**. The block union derives from `ContentBlockMap`.
|
||||
|
||||
Source: [`packages/llm/src/types.ts`](../../packages/llm/src/types.ts)
|
||||
Source: [`packages/llm/llm/src/types.ts`](../../packages/llm/llm/src/types.ts)
|
||||
|
||||
```ts type-equiv
|
||||
interface ContentBlockMap {
|
||||
@@ -116,7 +116,7 @@ The full union, the adapter contract (usage-before-finish, raw-JSON tool argumen
|
||||
|
||||
One model call is a fully-assembled `GenerateOptions`; the non-streaming result is `GenerateResult`.
|
||||
|
||||
Source: [`packages/llm/src/types.ts`](../../packages/llm/src/types.ts)
|
||||
Source: [`packages/llm/llm/src/types.ts`](../../packages/llm/llm/src/types.ts)
|
||||
|
||||
```ts type-equiv
|
||||
interface GenerateOptions {
|
||||
@@ -180,7 +180,7 @@ The model-facing `ToolSchema` is the wire shape; the registered `ToolDefinition`
|
||||
|
||||
A `Session` is an **append-only log** of typed `SessionEvent`s — the single source of truth. The LLM message history is *derived* from the log (`deriveMessages()`), not stored separately. The event vocabulary derives from `SessionEventMap`:
|
||||
|
||||
Source: [`packages/session/src/types.ts`](../../packages/session/src/types.ts)
|
||||
Source: [`packages/core/session/src/types.ts`](../../packages/core/session/src/types.ts)
|
||||
|
||||
```ts type-equiv
|
||||
type SessionEvent<T extends SessionEventType = SessionEventType> = {
|
||||
@@ -201,7 +201,7 @@ The thirteen event variants (`turn/start`, `turn/end`, `step/start`, `step/end`,
|
||||
|
||||
`Agent` is the surface every plugin (UI, hooks, orchestrators) programs against. The concrete implementation is `ReactLoopAgent` in dsh-agent-loop; nothing outside the loop depends on the implementation.
|
||||
|
||||
Source: [`packages/agent/src/types.ts`](../../packages/agent/src/types.ts)
|
||||
Source: [`packages/core/agent/src/types.ts`](../../packages/core/agent/src/types.ts)
|
||||
|
||||
```ts type-equiv
|
||||
interface Agent {
|
||||
|
||||
@@ -1,8 +1,8 @@
|
||||
# LLM Streaming
|
||||
|
||||
The wire-level streaming vocabulary of [dsh-llm](../../packages/llm). [core.md](core.md) introduces `StreamChunk`, `Message`, and `ContentBlock`; this page owns the full chunk protocol, the adapter contract every adapter must obey, and the shared assembler.
|
||||
The wire-level streaming vocabulary of [dsh-llm](../../packages/llm/llm). [core.md](core.md) introduces `StreamChunk`, `Message`, and `ContentBlock`; this page owns the full chunk protocol, the adapter contract every adapter must obey, and the shared assembler.
|
||||
|
||||
Source: [`packages/llm/src/types.ts`](../../packages/llm/src/types.ts)
|
||||
Source: [`packages/llm/llm/src/types.ts`](../../packages/llm/llm/src/types.ts)
|
||||
|
||||
## `StreamChunk` — the raw protocol
|
||||
|
||||
@@ -45,7 +45,7 @@ interface TokenUsage {
|
||||
|
||||
## `BlockAssembler`
|
||||
|
||||
`BlockAssembler` ([`packages/llm/src/assembler.ts`](../../packages/llm/src/assembler.ts)) is the single shared implementation that folds a `StreamChunk` stream back into `ContentBlock`s and a final `Message`. The loop logs the raw chunks (for replay fidelity) while feeding the same chunks through an assembler — so the canonical log keeps token-level detail and the derived message is rebuilt deterministically. A consumer that needs the assembled result without re-implementing the fold uses this.
|
||||
`BlockAssembler` ([`packages/llm/llm/src/assembler.ts`](../../packages/llm/llm/src/assembler.ts)) is the single shared implementation that folds a `StreamChunk` stream back into `ContentBlock`s and a final `Message`. The loop logs the raw chunks (for replay fidelity) while feeding the same chunks through an assembler — so the canonical log keeps token-level detail and the derived message is rebuilt deterministically. A consumer that needs the assembled result without re-implementing the fold uses this.
|
||||
|
||||
## The seam
|
||||
|
||||
|
||||
@@ -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/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 seam is a textbook [capability seam](../rfc/implemented/architecture/2026-06-13-capability-seams.md): one abstract service ([dsh-session-persistence](../../packages/session-persistence/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
|
||||
|
||||
@@ -16,7 +16,7 @@ A backend that reloads a log crashed mid-turn finds an open `turn/start` with no
|
||||
|
||||
Per-session metadata travels **separately** from the event log: format version, cwd, and lineage are storage concerns, not conversation events, so they stay out of `SessionEventMap` and never reach `deriveMessages()`. The header is attached to a `Session` via `session.header`.
|
||||
|
||||
Source: [`packages/session/src/types.ts`](../../packages/session/src/types.ts)
|
||||
Source: [`packages/core/session/src/types.ts`](../../packages/core/session/src/types.ts)
|
||||
|
||||
```ts type-equiv
|
||||
interface SessionHeader {
|
||||
@@ -57,7 +57,7 @@ Replay/fork is therefore `ctx.sessions.create(id, { seed: seedEvents })`; resumi
|
||||
|
||||
Both implement the same abstract `SessionPersistence` (create/append/load/list/has/delete over `SessionEvent`) and pass `runPersistenceContract`, proving the seam is genuinely backend-agnostic:
|
||||
|
||||
- **[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.
|
||||
- **[dsh-session-persistence-jsonl](../../packages/session-persistence/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/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/architecture/2026-06-18-shared-persistence-write-coordinator.md).
|
||||
|
||||
@@ -1,8 +1,8 @@
|
||||
# Sessions
|
||||
|
||||
The in-memory, event-sourced model of [dsh-session](../../packages/session). A `Session` is an **append-only log** of typed `SessionEvent`s — the single source of truth for an agent's whole interaction history. The LLM message history is *derived* from the log, never stored separately; replay is re-derivation from the same events. How the log is made **durable** (the persistence seam, backends, crash recovery) is the sibling concern on [persistence.md](persistence.md).
|
||||
The in-memory, event-sourced model of [dsh-session](../../packages/core/session). A `Session` is an **append-only log** of typed `SessionEvent`s — the single source of truth for an agent's whole interaction history. The LLM message history is *derived* from the log, never stored separately; replay is re-derivation from the same events. How the log is made **durable** (the persistence seam, backends, crash recovery) is the sibling concern on [persistence.md](persistence.md).
|
||||
|
||||
Source: [`packages/session/src/types.ts`](../../packages/session/src/types.ts)
|
||||
Source: [`packages/core/session/src/types.ts`](../../packages/core/session/src/types.ts)
|
||||
|
||||
## `SessionEventMap` — the event vocabulary
|
||||
|
||||
|
||||
@@ -1,8 +1,8 @@
|
||||
# Tools
|
||||
|
||||
The tool pipeline of [dsh-tools](../../packages/tools). [core.md](core.md) introduces `ToolDefinition` as the one pipeline-authoring type promoted to the spine and `ToolSchema` as the model-facing wire shape. This page owns the full `ToolDefinition`, the typed schema DSL that builds it, the waterfall execution shapes, and the UI-presentation vocabulary.
|
||||
The tool pipeline of [dsh-tools](../../packages/core/tools). [core.md](core.md) introduces `ToolDefinition` as the one pipeline-authoring type promoted to the spine and `ToolSchema` as the model-facing wire shape. This page owns the full `ToolDefinition`, the typed schema DSL that builds it, the waterfall execution shapes, and the UI-presentation vocabulary.
|
||||
|
||||
Source: [`packages/tools/src/index.ts`](../../packages/tools/src/index.ts) · [`packages/tools/src/schema.ts`](../../packages/tools/src/schema.ts)
|
||||
Source: [`packages/core/tools/src/index.ts`](../../packages/core/tools/src/index.ts) · [`packages/core/tools/src/schema.ts`](../../packages/core/tools/src/schema.ts)
|
||||
|
||||
## `ToolDefinition` — a registered tool
|
||||
|
||||
@@ -36,7 +36,7 @@ interface ToolDefinition extends ToolSchema {
|
||||
|
||||
Plugin authors write per-property specs with a boolean `required: true`, and a type-level helper maps the spec to the `execute` argument type — zero casts. The DSL is *machinery that types* `ToolDefinition`; it is intentionally a sub-page detail, not core.
|
||||
|
||||
Source: [`packages/tools/src/schema.ts`](../../packages/tools/src/schema.ts)
|
||||
Source: [`packages/core/tools/src/schema.ts`](../../packages/core/tools/src/schema.ts)
|
||||
|
||||
```ts type-equiv
|
||||
interface SchemaProp {
|
||||
@@ -109,4 +109,4 @@ How a tool wants its call shown in a UI (an editor tool-call card, a CLI log lin
|
||||
|
||||
> These shapes carry a `FIXME(tool-presentation)` in source: they grew incrementally and the call-vs-result terminal split is muddy. Before more tools/UIs depend on them, they will be redesigned (a tagged union over card kinds) and pinned in an RFC, migrating `dsh-tool-bash` and the ACP bridge together. Treat the field-level shapes here as provisional; the source is authoritative.
|
||||
|
||||
The full presentation field docs live in [`packages/tools/src/index.ts`](../../packages/tools/src/index.ts). The bash tool's own schemas (`bash`/`bash_output`/`bash_kill`) and the executor they drive are on [bash.md](bash.md).
|
||||
The full presentation field docs live in [`packages/core/tools/src/index.ts`](../../packages/core/tools/src/index.ts). The bash tool's own schemas (`bash`/`bash_output`/`bash_kill`) and the executor they drive are on [bash.md](bash.md).
|
||||
|
||||
@@ -135,7 +135,7 @@ Pick the tag that matches the urgency so anyone scanning the code can tell a rel
|
||||
The [core data structures](core-data-structures/core.md) docs paste real type definitions so a reader sees the exact shape. To keep a paste from drifting when source changes, fence it as ` ```ts type-equiv ` (instead of ` ```ts `) and register it in `scripts/type-equiv.manifest.json` with the source file and symbol it mirrors:
|
||||
|
||||
```json
|
||||
{ "doc": "docs/core-data-structures/session.md", "symbol": "SessionEvent", "source": "packages/session/src/types.ts" }
|
||||
{ "doc": "docs/core-data-structures/session.md", "symbol": "SessionEvent", "source": "packages/core/session/src/types.ts" }
|
||||
```
|
||||
|
||||
`pnpm run verify-type-equiv` (part of `doc-sync`) then extracts that symbol's declaration from source via the TypeScript parser and asserts the block matches it (whitespace- and comment-insensitive, so a doc block may show a clean definition and the prose can carry the semantics). It also enforces a 1:1 correspondence: every `ts type-equiv` block has exactly one manifest entry and vice-versa, so a block can't go silently unchecked and a stale entry can't linger. `doc-typecheck` skips `ts type-equiv` blocks (they aren't standalone-compilable) and excludes them from its opt-out ratio. When you change a documented type, the gate fails until you update the paste; when you add or remove a block, update the manifest in the same change.
|
||||
|
||||
@@ -24,7 +24,7 @@ The ACP server could not create or load a single session — the two RPCs an edi
|
||||
|
||||
## Root cause #1 — `export default apply` drops the plugin's `inject` (broke `session/new`)
|
||||
|
||||
`packages/acp/src/index.ts` is a *namespace plugin*: it exports `name`, `inject`, `Config`, and `apply` as separate named exports — the same shape as every other plugin in the repo (`invariants`, `llm-deepseek`, `tool-bash`, `stdio-chat`, …). But it *also* ended with one extra line no other plugin had:
|
||||
`packages/ui/acp/src/index.ts` is a *namespace plugin*: it exports `name`, `inject`, `Config`, and `apply` as separate named exports — the same shape as every other plugin in the repo (`invariants`, `llm-deepseek`, `tool-bash`, `stdio-chat`, …). But it *also* ended with one extra line no other plugin had:
|
||||
|
||||
```ts ignore-check
|
||||
export const name = 'acp'
|
||||
@@ -97,8 +97,8 @@ Both bugs share one root process gap: **no test exercised the plugin through its
|
||||
|
||||
## Guardrails added
|
||||
|
||||
- **Removed `export default apply`** (`packages/acp/src/index.ts`) — the Bug #1 fix.
|
||||
- **`AgentLoop.resume` reads `this.ctx.get('sessionPersistence')`** (`packages/agent-loop/src/index.ts`) — the Bug #2 fix, with a comment explaining the shadow-walk trap.
|
||||
- **Removed `export default apply`** (`packages/ui/acp/src/index.ts`) — the Bug #1 fix.
|
||||
- **`AgentLoop.resume` reads `this.ctx.get('sessionPersistence')`** (`packages/core/agent-loop/src/index.ts`) — the Bug #2 fix, with a comment explaining the shadow-walk trap.
|
||||
- **No-key `session/new` e2e over real stdio** (`examples/acp-agent/tests/acp.e2e.ts`): boots the example as a subprocess through the real Loader and asserts `session/new` resolves. This fails loudly on Bug #1 with no API key. Verified it fails when `export default apply` is restored.
|
||||
- **`TSX_TSCONFIG_PATH` in the e2e spawn**: the subprocess runs from a temp cwd, where tsx cannot find the repo-root tsconfig `paths` map by searching upward — so dsh-* imports silently fell back to built `lib/`. Pointing tsx at the repo tsconfig makes resolution cwd-independent and ensures the test runs *source*, not a possibly-stale build.
|
||||
- **AGENTS.md defensive pattern**: "Line coverage is not behavior coverage; test the REAL entry path, not a synthetic stand-in" — codifies the lesson for every future plugin.
|
||||
|
||||
@@ -4,7 +4,7 @@ Status: implemented
|
||||
|
||||
## Problem
|
||||
|
||||
The ACP bridge lets each tool own its call rendering via `presentCall`/`presentResult` (see [tool-call UI presentation](../../proposed/feature/2026-06-14-acp-agent-client-protocol.md) and `packages/tools`). For `bash` we surface the exact command as the `tool_call` title, the model's `description` as a content text block, `kind: 'execute'`, and the completed output wrapped in a fenced ` ```console ` text block.
|
||||
The ACP bridge lets each tool own its call rendering via `presentCall`/`presentResult` (see [tool-call UI presentation](../../proposed/feature/2026-06-14-acp-agent-client-protocol.md) and `packages/core/tools`). For `bash` we surface the exact command as the `tool_call` title, the model's `description` as a content text block, `kind: 'execute'`, and the completed output wrapped in a fenced ` ```console ` text block.
|
||||
|
||||
That is a correct, capability-free baseline, but not how the reference editors render a *terminal* tool at its best. An editor like Zed has a dedicated terminal tool-call card — a header showing the working directory, the command as the label, the command output rendered as a terminal, and an exit-status pill — but it only builds that card when the `tool_call` carries terminal metadata (below). With a plain text block the output appears as static markdown and there is no cwd header. (Zed also HIDES `rawInput` for `kind: 'execute'`, which is why the command IS the title — both reference adapters do the same. The human-readable description rides as a separate content block above the card; note this is a DELIBERATE divergence — claude-agent-acp DROPS the description in terminal mode and renders only the card — we keep the summary visible alongside.)
|
||||
|
||||
|
||||
@@ -18,13 +18,13 @@ A snapshot test boots the **real** `examples/acp-agent` subprocess, drives it ov
|
||||
|
||||
### The fixture is the persisted session JSONL
|
||||
|
||||
The per-scenario fixture is `<scenario>/session.jsonl`: the exact log produced by running the scenario once against the real API (the snapshot harness harvests the file the JSONL persistence backend writes). This log already contains everything needed to reproduce the run deterministically: its `assistant/chunk` events carry every parsed `StreamChunk` (the LLM's behavior), and its `tool/call`/`tool/result`/`turn/*`/`assistant/message`/`usage` events carry the harness's behavior. One artifact captures both, and it is the format the codebase already treats as the authoritative replay record ([packages/session/src/types.ts](../../../../packages/session/src/types.ts): "raw chunks are the replay record").
|
||||
The per-scenario fixture is `<scenario>/session.jsonl`: the exact log produced by running the scenario once against the real API (the snapshot harness harvests the file the JSONL persistence backend writes). This log already contains everything needed to reproduce the run deterministically: its `assistant/chunk` events carry every parsed `StreamChunk` (the LLM's behavior), and its `tool/call`/`tool/result`/`turn/*`/`assistant/message`/`usage` events carry the harness's behavior. One artifact captures both, and it is the format the codebase already treats as the authoritative replay record ([packages/core/session/src/types.ts](../../../../packages/core/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/llm/src/index.ts](../../../../packages/llm/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/core/agent-loop/src/loop.ts](../../../../packages/core/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/llm-deepseek/src/index.ts](../../../../packages/llm/llm-deepseek/src/index.ts)). So replay cannot reuse the normal config — it uses a dedicated `examples/acp-agent/cordis.snapshot.yml` that installs `llm-replay` in place of the adapter. 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
|
||||
|
||||
@@ -66,7 +66,7 @@ Determinism of the tool environment comes from a per-test `mkdtemp` cwd, the exe
|
||||
|
||||
### 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/support/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
|
||||
|
||||
|
||||
@@ -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/llm-deepseek/src/index.ts](../../../../packages/llm/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
|
||||
|
||||
@@ -3,7 +3,7 @@
|
||||
Status: proposed
|
||||
|
||||
<!-- XXX: legacy ADR/RFC body format, not yet normalized to a unified RFC template. -->
|
||||
> **Implementation status (MVP landed):** steps 1, 2, 3, 4, 6, 7, 8 are implemented in `packages/acp` + `examples/acp-agent`. **Step 5 (the `session/request_permission` permission gate) is deferred** — the bridge ships a pass-through (tools run with the executor's full authority) marked `TODO(rfc010-permission-gate)`, and lays down only the `WeakMap<Agent, sessionId>` ownership seam the gate will build on. Status stays `proposed` until the gate lands. `session/cancel` is the queue-aware `agent.cancel()`: it aborts a running step, clears queued + steering work, and drops a turn that is about to start, so a queued-but-not-yet-started prompt never runs and a later prompt cannot be batched into the cancelled turn. **Per-session `cwd` is now honored** (lifting the original "launch the server in the workspace root" restriction — see § Deferred): `session/new` accepts any absolute `cwd`, and `session/load` requires the request `cwd` to match the persisted session `cwd` so the editor and bash executor agree on the workspace.
|
||||
> **Implementation status (MVP landed):** steps 1, 2, 3, 4, 6, 7, 8 are implemented in `packages/ui/acp` + `examples/acp-agent`. **Step 5 (the `session/request_permission` permission gate) is deferred** — the bridge ships a pass-through (tools run with the executor's full authority) marked `TODO(rfc010-permission-gate)`, and lays down only the `WeakMap<Agent, sessionId>` ownership seam the gate will build on. Status stays `proposed` until the gate lands. `session/cancel` is the queue-aware `agent.cancel()`: it aborts a running step, clears queued + steering work, and drops a turn that is about to start, so a queued-but-not-yet-started prompt never runs and a later prompt cannot be batched into the cancelled turn. **Per-session `cwd` is now honored** (lifting the original "launch the server in the workspace root" restriction — see § Deferred): `session/new` accepts any absolute `cwd`, and `session/load` requires the request `cwd` to match the persisted session `cwd` so the editor and bash executor agree on the workspace.
|
||||
|
||||
## Problem
|
||||
|
||||
@@ -17,7 +17,7 @@ This RFC has a hard prerequisite on [session persistence](../../implemented/arch
|
||||
|
||||
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.
|
||||
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/ui/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.
|
||||
|
||||
The mapping between ACP and existing harness seams — each row names the seam and any required extension:
|
||||
|
||||
@@ -43,9 +43,9 @@ Lifecycle and disposal: the connection, listeners, and in-flight permission prom
|
||||
|
||||
## 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/ui/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/core/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<Agent, sessionId>` 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/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.
|
||||
|
||||
@@ -3,7 +3,7 @@
|
||||
Status: proposed
|
||||
|
||||
<!-- XXX: legacy ADR/RFC body format, not yet normalized to a unified RFC template. -->
|
||||
> **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.
|
||||
> **Implementation status:** the multi-session bridge (steps 1, 3, 4) and the bash task-ownership isolation are implemented in `packages/ui/acp` + `packages/bash/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<SessionId, AcpSession>` 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).
|
||||
|
||||
|
||||
@@ -4,7 +4,7 @@ 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/llm/src/index.ts](../../../../packages/llm/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/simplification/2026-06-19-drop-mutable-session-summary.md) pattern at a smaller scale.
|
||||
|
||||
@@ -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](../../../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.
|
||||
- Update the event taxonomy table in [docs/architecture.md](../../../architecture.md) and [packages/llm/llm/README.md](../../../../packages/llm/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?
|
||||
|
||||
|
||||
@@ -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/llm/src/index.ts](../../../../packages/llm/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/llm/src/index.ts)).
|
||||
- `generate()` — one fully-assembled `GenerateResult`, dispatched through a second `llm/generate` waterfall ([index.ts:151-157](../../../../packages/llm/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/core/agent-loop/src/loop.ts](../../../../packages/core/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/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/llm/src/assembler.ts:138-168](../../../../packages/llm/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
|
||||
|
||||
@@ -34,7 +34,7 @@ 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](../../../architecture.md), and module docs no longer mention the removed convenience surfaces.
|
||||
- `packages/llm/llm/README.md`, [docs/architecture.md](../../../architecture.md), and module docs no longer mention the removed convenience surfaces.
|
||||
|
||||
## Risks
|
||||
|
||||
|
||||
@@ -8,13 +8,13 @@ Two capability seams ([interface / implementation / consumer](../../implemented/
|
||||
|
||||
### `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.
|
||||
The abstract service declares four operations beyond create/append: `load`, `list`, `has`, `delete` ([packages/session-persistence/session-persistence/src/index.ts:142-151](../../../../packages/session-persistence/session-persistence/src/index.ts)). Production consumers of `ctx.sessionPersistence` use only two of them: the agent-loop resume path calls `load()` ([packages/core/agent-loop/src/index.ts:176-194](../../../../packages/core/agent-loop/src/index.ts)), and the ACP bridge calls `list()` for `session/list` ([packages/ui/acp/src/index.ts](../../../../packages/ui/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/ui/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.
|
||||
`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/session-persistence/src/coordinator.ts:298-310](../../../../packages/session-persistence/session-persistence/src/coordinator.ts)). `delete()` drags the `deleteStored` backend hook ([coordinator.ts:99](../../../../packages/session-persistence/session-persistence/src/coordinator.ts), [coordinator.ts:313-319](../../../../packages/session-persistence/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.
|
||||
The bash seam declares `get(id)` ("look up a background task by id") and `list()` ("all tracked background tasks") ([packages/bash/bash/src/index.ts:88-107](../../../../packages/bash/bash/src/index.ts)), both implemented by `LocalBashExecutor` ([packages/bash/bash-local/src/index.ts:179-191](../../../../packages/bash/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/bash/tests/service.spec.ts](../../../../packages/bash/bash/tests/service.spec.ts), [packages/bash/bash-local/tests/executor.spec.ts](../../../../packages/bash/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/bash/tool-bash/tests/tools.spec.ts](../../../../packages/bash/tool-bash/tests/tools.spec.ts), [packages/bash/tool-bash/tests/integration.spec.ts](../../../../packages/bash/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
|
||||
|
||||
@@ -22,7 +22,7 @@ Remove the methods nothing consumes, from the abstract seam, the implementation,
|
||||
|
||||
- `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.
|
||||
- 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/session-persistence/README.md](../../../../packages/session-persistence/session-persistence/README.md)'s `has(id)`/`delete(id)` API row and its "delegates its six public service methods" prose → four, [packages/bash/bash/README.md](../../../../packages/bash/bash/README.md)'s `get(id)`/`list()` row), the backend READMEs that describe `has`/`list` semantics ([packages/session-persistence/session-persistence-sqlite/README.md](../../../../packages/session-persistence/session-persistence-sqlite/README.md), [packages/session-persistence/session-persistence-jsonl/README.md](../../../../packages/session-persistence/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/session-persistence/src/index.ts](../../../../packages/session-persistence/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/session-persistence/src/coordinator.ts](../../../../packages/session-persistence/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"?
|
||||
|
||||
|
||||
@@ -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/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.
|
||||
|
||||
@@ -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/ui/acp/README.md) describe fresh-session support only.
|
||||
|
||||
## What we give up
|
||||
|
||||
|
||||
@@ -20,7 +20,7 @@ The implementing PR should update the [capability seams](../../implemented/archi
|
||||
- `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/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.
|
||||
- [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/session-persistence/README.md) explain why backend implementations remain separate.
|
||||
|
||||
## What we give up
|
||||
|
||||
|
||||
@@ -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/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.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user