From a44f7f34864af21b484839d1826740224650a98a Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Sun, 14 Jun 2026 21:14:51 +0800 Subject: [PATCH 1/3] docs: add RFCs 009-011 (session persistence + ACP support) Three proposal documents, numbered in dependency order: - RFC 009: an abstract, append-only, event-based SessionPersistence service over the existing SessionEvent log (no parallel persisted type), a JSONL impl, a SessionMeta header seam, and an async AgentLoop.resume path. Design informed by Codex/Claude Code/ opencode/pi. Core design point; unblocks resume + ACP session/load. - RFC 010: ACP (Agent Client Protocol) support as a dsh-acp client-driver plugin on @agentclientprotocol/sdk, mapping ACP onto the agent/* events and the tools/execute permission seam. Builds on 009 for session/load; single active session. - RFC 011: multiplex concurrent ACP sessions over one connection (bridge-layer change; downstream of 010). --- ...09-session-persistence-and-resumability.md | 60 ++++++++++++++++ docs/rfc/010-acp-agent-client-protocol.md | 70 +++++++++++++++++++ docs/rfc/011-acp-multi-session.md | 32 +++++++++ docs/rfc/README.md | 3 + 4 files changed, 165 insertions(+) create mode 100644 docs/rfc/009-session-persistence-and-resumability.md create mode 100644 docs/rfc/010-acp-agent-client-protocol.md create mode 100644 docs/rfc/011-acp-multi-session.md diff --git a/docs/rfc/009-session-persistence-and-resumability.md b/docs/rfc/009-session-persistence-and-resumability.md new file mode 100644 index 0000000000..1330af7b1c --- /dev/null +++ b/docs/rfc/009-session-persistence-and-resumability.md @@ -0,0 +1,60 @@ +# RFC 009: Durable session persistence — an abstract, append-only, event-based store + +Status: proposed + +## Problem + +Sessions live only in memory. The example `session-jsonl.ts` plugin (duplicated byte-for-byte in both `examples/coding-agent` and `examples/echo-agent`) is write-only telemetry: it buffers `session/event` and appends JSON lines, but has no read/replay path, no crash-safety (no fsync, no atomic write, and a fire-and-forget dispose drain), no listing, and no format versioning. [ADR 0003](../adr/0003-event-sourced-sessions.md) and [docs/architecture.md](../architecture.md) both park "real persistence backends (JSONL session dirs, sqlite)" and the session-event-vocabulary review as deferred TODOs "once the loop and the first persistence plugin coexist" — that time is now. + +Because nothing can rehydrate a past session from disk into a live agent, durable resume ("continue yesterday's task"), durable forking, and the ACP `session/load` method (RFC 010) are all impossible. (In-memory replay/fork via `ctx.sessions.create(id, seed)` already exists and is tested; what is missing is the durable store behind it and a first-class agent-loop resume path.) + +The event-sourced model (ADR 0003) makes the log the single source of truth and derives LLM history from it. Persistence must stay faithful to that: it should persist the existing `SessionEvent` directly — there must be no parallel "persisted message" type that the log has to be converted to and from. We also want the backend to be swappable: a file store now, a database store later, behind one interface. + +## Proposal + +Mirror the codebase's capability-seam pattern ([ADR 0009](../adr/0009-capability-seams.md), the `bash` template: an abstract `Service` interface, a concrete implementation, and consumers) for persistence. + +**1. Abstract service `SessionPersistence`** — a new interface package `@deepseek-ai/dsh-session-persistence` owning `ctx.sessionPersistence`, depending only on `cordis` and `dsh-session`. Its persisted unit IS `SessionEvent` (`{ type, seq, time, data }`), reused verbatim — no conversion type. The method surface: + +- `create(meta: SessionMeta): Promise` — register a new session's header. The backend MAY defer the physical write until the first `append` (lazy materialization); `has`/`list` semantics for a zero-event session are specified, not left implicit. +- `append(id, events: readonly SessionEvent[]): Promise` — durably persist a batch (called from the flush drain). Append-only; never rewrites. **Contract**: the first event's `seq` MUST equal the backend's stored next-seq (a DB impl asserts this inside a transaction; the file impl appends at EOF). All persisted `event.data` MUST be JSON-serializable. +- `load(id): Promise<{ meta: SessionMeta; events: SessionEvent[] }>` — replay header plus the full event log. Returns `meta` AND `events` so the live session is reconstructed with its `cwd`/lineage, not just its log. **Validation**: `events[i].seq === i` (contiguous, zero-based) or the load rejects. +- `list(): Promise` — lightweight listing from headers, no full-log parse. +- `has(id)` / `delete(id)` — existence and removal. +- `update(id, summary: Partial): Promise` — update mutable header fields without touching the append-only event log. + +The new `SessionMeta` splits into an immutable `SessionHeader` (`{ id, version, createdAt, cwd?, parentSession? }`) and a mutable `SessionSummary` (`{ updatedAt, title?, firstPrompt? }`); `SessionMeta = SessionHeader & SessionSummary`. Every reference system writes such a header (pi's `version: 3` header line, Codex's `SessionMeta`, Claude Code's tail metadata). It is kept *separate from the event log* deliberately: format-version, cwd, and lineage are storage concerns, not conversation events, so they stay out of `SessionEventMap` and never reach `deriveMessages()`. The alternative — a merge-extensible `session/meta` event as log line 0 — was considered: an in-log event would ride along with a seeded/forked session for free, whereas an out-of-log header must be threaded through a seam (item 3a). It was rejected because metadata is not replayable conversation state; the explicit metadata seam is the cleaner cost. + +**2. Concrete impl `SessionPersistenceJsonl`** — a new package `@deepseek-ai/dsh-session-persistence-jsonl`. One file per session: a header line (`{ type: 'session', version, id, cwd, createdAt, parentSession? }`) followed by one `SessionEvent` JSON per line. On disk: a configured root with per-cwd subdirectories (pi-style `--encoded-cwd--/_.jsonl`) so sessions group by project. `list()` reads only each file's header line. Resilience over the example: append plus explicit flush; on `load`, drop only a corrupt/incomplete *trailing* line but reject a parse error or `seq` gap in the *middle* (a hole would desync `seq = log.length` appends); lazy materialization (no file until the first real event, so abandoned sessions leave nothing behind). + +**2a. `assistant/chunk` persistence policy** (decided here, not deferred). The loop appends one `assistant/chunk` per raw stream chunk, but `deriveMessages()` skips chunks entirely — the assembled `assistant/message` is authoritative for history. It is tempting to drop chunks from the durable log (Codex's `policy.rs` filters deltas from its rollout). But `seq = log.length` and the load-validation `events[i].seq === i` require a *contiguous* log: filtering chunks out would leave holes (`[0,1,4,6,8]`) and break both the contract and resume. **Decision: the canonical durable log persists every `SessionEvent` verbatim, including `assistant/chunk`** — this keeps `seq` contiguous, keeps "persist `SessionEvent` directly" literally true, and lets RFC 010 replay streamed turns on `session/load`. A chunk-filtered *projection* (for export or a compacted listing) is possible later as a derived view with its own renumbering, but it is NOT the canonical log and NOT the default. The round-trip test asserts byte-identical events. + +**3. Write path** lives in the impl plugin, generalizing the example: subscribe to `session/event` (buffer write-behind, keyed by session), drain to `append()` at the awaited `session/flush` checkpoint and on dispose — the seam the loop already fires at every turn end. The loop's write path needs no change. + +**3a. Metadata seam** (the one `dsh-session` change). Today `Session` has only `id` plus the log, and `session/event` carries `(session, event)` — there is nowhere for `cwd`/lineage to live, so a plugin listening to events alone cannot know a session's `cwd`. Add a minimal seam: `SessionStore.create(id, { seed?, meta? })` attaches a `SessionHeader` to the `Session` (a new readonly `session.header`), and the persistence plugin captures it on `session/created`. This is additive; `deriveMessages()` and the log are untouched. + +**4. Resume path** — an async helper, NOT a change to the synchronous `create`. `AgentLoop.create(agentId, options)` is synchronous (the `AgentLoop` constructor calls it for configured agents), so it cannot `await` persistence. Add a separate `async resume(agentId, resumeSessionId, options?): Promise` that awaits `ctx.sessionPersistence.load(resumeSessionId)`, then calls `ctx.sessions.create(resumeSessionId, { seed: events, meta })`, then constructs/registers/starts the `LoopAgent` on that session. Three distinct identities are kept separate: the `agentId` (the handle), the live `sessionId` (here the resumed one, NOT `${agentId}-session`), and the `resumeSessionId` being loaded. Downstream already works: `Session`'s constructor shallow-copies the seed, `lastTurnNumber()` in `loop.ts` continues turn numbering, and `deriveMessages()` rebuilds history. + +Seed handling has two cases that the plugin must distinguish, and neither is the naive "re-append on flush" hazard. Seed events are copied into `Session` by the constructor *without* emitting `session/event` (the store installs `onAppend` only after construction), so the write-behind buffer never sees them — there is no double-write on a plain resume. (1) **Resume / adopt** an existing on-disk session: the events are already persisted, so the plugin initializes its per-session write cursor to the loaded length and appends only events with `seq >= loadedLength`. (2) **Fork** a brand-new session whose seed came from another session: that seed is NOT yet on disk under the new id, so the plugin must persist the full seed once (on `session/created`, via `create(meta)` + an initial `append`) and then set the cursor to the seed length. The `append` seq-contract makes both safe — a re-append of a stored seq is rejected, never silently duplicated. + +**5. DB-backend feasibility** (proven by the design, implemented later). `SessionEvent` maps 1:1 onto a row `(session_id TEXT, seq INTEGER, type TEXT, time INTEGER, data JSON, PRIMARY KEY(session_id, seq))` — `seq` already exists and is monotonic. `append` is INSERT (in a transaction asserting the contiguous-seq contract), `load` is SELECT … ORDER BY seq, `list` is SELECT from a `sessions` header table. A future `@deepseek-ai/dsh-session-persistence-sqlite` is a drop-in `SessionPersistence` subclass with no interface change (opencode runs exactly this `session_message(session_id, seq, type, data)` shape on SQLite/WAL). Because `SessionEventMap` is merge-extensible and `data` is typed only as `SessionEventMap[K]`, the interface requires all persisted `event.data` to be JSON-serializable; `append` rejects non-serializable data with an error naming the offending event type, and the plugin snapshots (serializes/clones) each event when it buffers on `session/event`, since `session.events` hands out the live mutable object. A canonical SQLite backend is one such drop-in; a Codex-style derived search/listing index over the JSONL files would instead be a separate projector service, NOT a `SessionPersistence` replacement. `SessionId` is an unvalidated branded string, so the file impl MUST sanitize/encode it before using it in a path (no traversal, no collision). + +## Plan + +1. Interface package `packages/session-persistence/` per [the cookbook](../cookbook/adding-a-package.md): abstract `SessionPersistence extends Service` (`super(ctx, 'sessionPersistence')`), the `declare module 'cordis'` ctx key, the `SessionHeader`/`SessionSummary`/`SessionMeta` types, and method contracts documented in JSDoc (durability, append-only, contiguous-seq, JSON-serializable, error semantics). +2. `dsh-session` changes: add the three meta types beside `SessionId`; add the metadata seam. `SessionStore.create(id?, seed?)` becomes `create(id?, options?: { seed?; meta? })` — a breaking signature change (callers pass `seed` positionally today: `AgentLoop.create`, and ~20+ call sites across `session`/`invariants`/`agent-loop` tests), so either migrate every caller or keep a deprecated overload during transition. Add a readonly `session.header`. Persistence captures the header on `session/created` (a synchronous event), so the impl must hold a per-session init promise that every `session/flush` awaits before `append`, and must seed existing live sessions via `ctx.sessions.list()` on plugin apply (HMR does not replay `session/created`, mirroring `dsh-invariants`). Do NOT add meta to `SessionEventMap`. +3. JSONL impl `packages/session-persistence-jsonl/`: header plus event lines (all events, verbatim — see 2a), sanitized per-cwd dirs and filenames, lazy materialize (header + first batch written atomically), append plus flush, trailing-line-tolerant and mid-gap-rejecting `load` that truncates back to the last complete `turn/end` rather than resuming a half-written turn, header-only `list`, the per-session write cursor, and snapshot-on-buffer. `static Config` for root dir and flush policy. +4. Generalize the write-path plugin: the impl subscribes to `session/created` (capture header, persist any seed for forks), `session/event` (snapshot + buffer), and `session/flush`/dispose (drain), replacing the per-example `session-jsonl.ts`; both examples load the shared plugin. +5. Resume seam: the async `AgentLoop.resume(agentId, resumeSessionId, options?)`; initialize the write cursor to the loaded length; verify `lastTurnNumber`/`deriveMessages` continuity. `AgentLoop` does NOT hard-inject `sessionPersistence` (that would break non-persistent examples) — `resume` checks for the service and throws a typed "persistence not configured" error; consumers that need resume (ACP) load the persistence plugin. +6. Tests (event-sourcing makes these strong): a round-trip property (persist an arbitrary log → reload → byte-identical events and identical `deriveMessages()` output — the replay equivalence ADR 0003 promises); resume vs fork (resume appends no duplicate seqs; a fork persists its seed once); contiguous-seq enforcement (mid-log gap rejected, re-append of a stored seq rejected); crash tolerance (a truncated final turn truncates back to the last `turn/end`); JSON-serializability rejection for a plugin-added event carrying non-serializable data; mutation-after-`session/event` does not corrupt the persisted snapshot; SessionId path-traversal is neutralized; lazy materialization (no file until the first event); `has`/`list` semantics for a zero-event session; HMR-safety (dispose drains buffers and closes file handles; apply seeds existing live sessions); concurrent sessions do not cross buffers. +7. Docs: update the "Event-sourced sessions" durability-seam paragraph and the "Deferred work" list in [docs/architecture.md](../architecture.md) (persistence is no longer deferred); sync the affected package READMEs/JSDoc (`dsh-session` for the `create`/`session.header` change); add a [cookbook](../cookbook/) note on writing a persistence backend; resolve the `TODO(review)` on the event vocabulary now that a real persistence plugin coexists with the loop. On implementation this likely graduates to an ADR — "persistence is an abstract service over the existing `SessionEvent`; verbatim append-only log; file canonical, DB drop-in" is durable, contested, and surprising enough to record. + +## Risks + +Format versioning: the header carries a `version`; `load` must reject or migrate unknown versions (pi rejects, Codex relies on serde forward-compat). Fix the policy before shipping so v1 files stay loadable. + +Crash-safety bounds: append-only plus flush is robust to partial trailing writes (tolerated on load) but not to fsync-less power loss mid-line. State the guarantee honestly; a DB/WAL backend is the stronger option later. + +`SessionMeta` placement is a small public-surface decision (the immutability concern from [ADR 0012](../adr/0012-dev-invariants-over-deep-readonly.md)); pick its owning package deliberately and freeze the shape. + +Event-vocabulary churn: persisting the log freezes its shape more firmly, so this is the moment to complete ADR 0003's `TODO(review)` — especially the `assistant/chunk` fidelity question — before committing to an on-disk format. diff --git a/docs/rfc/010-acp-agent-client-protocol.md b/docs/rfc/010-acp-agent-client-protocol.md new file mode 100644 index 0000000000..63a11a7db3 --- /dev/null +++ b/docs/rfc/010-acp-agent-client-protocol.md @@ -0,0 +1,70 @@ +# RFC 010: Agent Client Protocol (ACP) support — drive the coding agent from external editors + +Status: proposed + +## Problem + +The coding agent is reachable only through the readline `stdio-chat` plugin: it reads lines from stdin, calls `agent.send()`, and prints `agent/stream-chunk` to stdout. There is no structured protocol, so the agent cannot be embedded in an editor — no streaming render, no tool-call display, no permission UI, no resumable sessions. + +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 RFC 009: 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 010 must land after, or in the same change as, 009, and pins to 009's `resume(agentId, resumeSessionId)` contract. RFC 009 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 [ADR 0009](../adr/0009-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) — zero runtime dependencies, Apache-2.0, actively versioned. 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: + +| ACP (client ⇄ agent) | Harness seam | Notes | +|---|---|---| +| `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}` → `{sessionId}` | `ctx.agentLoop.create` | agent generates `sessionId`; reject a 2nd session (single-session MVP, see RFC 011); `cwd` validated (require absolute) with "launch the server in the workspace root" documented until the workdir seam exists; `mcpServers` ignored (no `mcpCapabilities` advertised) | +| `session/load {sessionId, cwd, mcpServers}` | RFC 009's async `AgentLoop.resume(agentId, sessionId)` | 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 | +| `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) | turn-end carries the real reason including `max-tokens`; 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 | +| `session/update: agent_thought_chunk` | `agent/stream-chunk` `reasoning-delta` | | +| `session/update: tool_call` (pending→in_progress) | `session/event` `tool/call` | demux via a Session→sessionId map; `kind` inferred from the tool name | +| `session/update: tool_call_update` (completed/failed) | `session/event` `tool/result` | a throwing `tools/execute` yields NO `tool/result` → fail the pending tool UI from `agent/error`/turn-end | +| `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.abort(reason)` | 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. + +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 await `agent.done`. 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. + +## Plan + +1. Package scaffold `packages/acp/` per [the cookbook](../cookbook/adding-a-package.md); add `@agentclientprotocol/sdk`; `inject: ['agents', 'agentLoop', 'sessions', 'tools', 'sessionPersistence']` (the last is required because `session/load` advertises `loadSession: true`). +2. Connection plus `initialize`/`session/new`: wire `AgentSideConnection` to stdin/stdout; protocolVersion negotiation; the single-session guard; 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. +4. Prompt-turn streaming plus load: translate `agent/stream-chunk` and `session/event` into `session/update`; resolve `session/prompt` on settle. 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` with the authoritative `stopReason`; 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 RFC 009's `AgentLoop.resume`. +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 RFC 009 — required for `session/load`), omits the stdout logger (see Risks), and adds `yarn 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: RFC 001 / [ADR 0013](../adr/0013-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; write an ADR only if a decision 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 → RFC 011. +- `cwd` honoring. There is no current path from `session/new.cwd` to the bash workdir (`AgentLoop.create` takes only `AgentOptions`; `tool-bash` forwards only an explicit `args.workdir`; `LocalBashExecutor.resolve` defaults to its own config or `process.cwd()`). The MVP validates `cwd` (require absolute) and requires the server to be launched in the workspace root, erroring on a mismatch rather than silently running tools in the wrong directory; honoring an arbitrary `cwd` later means extending the agent-creation seam to carry a workdir. +- Client `terminal/*` proxying (a live editor terminal) and `fs/*` (editor-rendered diffs) — a future `BashExecutor` over the [ADR 0009](../adr/0009-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 — [ADR 0001](../adr/0001-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. + +Permission-await and disposal hangs: a pending `request_permission` whose connection closes or whose turn aborts must settle exactly once; disposal must reach quiescence (await `agent.done`), not orphan awaits on a closed pipe. + +The 100% per-file coverage gate (repo policy) makes a branch-heavy protocol bridge real work. Accepted deliberately, surfaced so it isn't a surprise at PR time. + +ACP protocol-shape details (exact method names, `session/update` variants, permission option kinds, stop reasons) are taken from the ACP spec and the `@agentclientprotocol/sdk` types; they are not independently verifiable until the dependency is added, so the implementation pins the SDK version and conforms to its types rather than to this RFC's prose where they differ. + diff --git a/docs/rfc/011-acp-multi-session.md b/docs/rfc/011-acp-multi-session.md new file mode 100644 index 0000000000..2ad08f4be1 --- /dev/null +++ b/docs/rfc/011-acp-multi-session.md @@ -0,0 +1,32 @@ +# RFC 011: Multiplex concurrent ACP sessions over one connection + +Status: proposed + +## Problem + +RFC 010 ships ACP support 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. + +## Proposal + +The harness core already supports many agents (`AgentRegistry.list()` and `AgentLoop.create` impose no count limit), so multiplexing is a bridge-layer change in `@deepseek-ai/dsh-acp`, not a loop or core change. + +- Lift the single-session guard in `session/new`; allow N live sessions, each mapped to its own `LoopAgent`. +- The bridge's `sessionId→agent` and `Session→sessionId` maps (introduced single-entry in RFC 010) become true multi-entry, plus a third `agent→sessionId` reverse map: the `tools/execute` permission gate receives only `exec.agent` (no sessionId), so it needs an O(1) reverse lookup to find the owning session. Every `agent/*` event and every `session/event` is demuxed strictly by id, so two sessions streaming at once never interleave their `session/update` notifications. +- Per-session prompt queues: RFC 010's single-entry in-flight-prompt state becomes multi-entry — one in-flight prompt *per session*, tracked per `sessionId`. +- Per-session cancel routing: `session/cancel` aborts only its own session's agent and settles only that session's in-flight prompt. `agent.abort()` drives a per-agent `AbortController`, so the per-session `exec.signal` is the natural isolation fence. +- Per-session permission ownership: a `session/request_permission` and its outcome are bound to the originating session via the reverse map, so a permission prompt or a cancel in one session can never resolve another session's pending permission. + +## Plan + +1. Generalize the two id maps to multi-entry and add the `agent→sessionId` reverse map; add a per-session record holding the agent, the in-flight-prompt state, the pending-permission registry, and the session's child context (see step 2). +2. Give each session its own child Cordis context (`ctx.extend()`) and register that session's listeners on it, so per-session listeners are fiber-scoped — disposing one session's child fiber removes exactly its listeners while the other N-1 sessions (and the bridge root) keep running. Demux every `agent/*` and `session/event` by id into the right session record. Note the single global `tools/execute` listener stays on the bridge root (it must see all agents) and routes via the reverse map. +3. Lift the `session/new` guard; keep `session/load` (RFC 010) working per session. +4. Tests for cross-session isolation: two sessions streaming and permission-prompting concurrently never interleave; a cancel/abort in one session leaves the other's stream and pending permission untouched; per-session in-flight-prompt enforcement holds independently; disposing one session leaves the others running. + +## Risks + +Listener fan-out cost: each session adds listeners; ensure disposal of one session removes exactly its own and the connection teardown (RFC 010) still reaches quiescence across all sessions. + +The subtle correctness trap is cross-session leakage — a cancel or abort on one session settling another session's pending permission. The per-session permission ownership rule (routed via the `agent→sessionId` reverse map) and its isolation test are the guard. + +Shared background-task state: the bash executor's task ids are global and predictable (`bash-1`, `bash-2`, …), and `bash_output`/`bash_kill` look up by id without checking the caller. Under one session this is benign; under N sessions one session's agent could read or kill another's background task. This is a pre-existing `tool-bash` gap that multi-session turns into a real isolation hole — fixing it (validate the caller against the task owner) belongs with this RFC or a companion `tool-bash` change. diff --git a/docs/rfc/README.md b/docs/rfc/README.md index 64992d1e93..e6dc5005bf 100644 --- a/docs/rfc/README.md +++ b/docs/rfc/README.md @@ -12,3 +12,6 @@ Proposals for substantial future work — reviewed before implementation, unlike | [006](006-doc-sync-and-api-reports.md) | Doc-sync enforcement and API extractor reports | implemented (pts 1-2; pt 3 deferred) | | [007](007-supply-chain-and-vendor-drift.md) | Supply chain checks and vendor drift verification | proposed | | [008](008-immutable-public-surfaces.md) | Deep-readonly public surfaces | implemented (revised) | +| [009](009-session-persistence-and-resumability.md) | Durable session persistence — abstract, append-only, event-based store | proposed | +| [010](010-acp-agent-client-protocol.md) | Agent Client Protocol (ACP) support for external editors | proposed | +| [011](011-acp-multi-session.md) | Multiplex concurrent ACP sessions over one connection | proposed | From 64ce7caedc35f646869c2729e27b639c5e59ade3 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Sun, 14 Jun 2026 21:43:16 +0800 Subject: [PATCH 2/3] docs: address review on RFCs 009-011 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolve the inline review feedback on PR #18 (all verified against the codebase, the published @agentclientprotocol/sdk@0.25.1 tarball, and Cordis fiber semantics): - 009: dsh-session owns SessionMeta (persistence re-exports) to avoid a package cycle; split mutable summary into a sidecar so the event log stays append-only and list/load can return it; pick one load-repair rule (resume from the last complete turn/end, overwrite the orphan). - 010: SDK has a zod peer dep + runtime zod/v4 import (drop "zero runtime deps"); session/new needs a create seam taking {sessionId, meta}; propose an abstract create/resume factory on dsh-agent so the bridge depends on the interface not the loop, and observe agent/status for quiescence since agent.done is LoopAgent-only; add the explicit TurnEndReason -> ACP StopReason wire mapping + test; reject non-empty additionalDirectories for the MVP; remove the EOF blank line. - 011: ctx.extend() does not create a disposable fiber — use a real per-session disposer scope. --- ...09-session-persistence-and-resumability.md | 8 ++++---- docs/rfc/010-acp-agent-client-protocol.md | 19 ++++++++++--------- docs/rfc/011-acp-multi-session.md | 4 ++-- 3 files changed, 16 insertions(+), 15 deletions(-) diff --git a/docs/rfc/009-session-persistence-and-resumability.md b/docs/rfc/009-session-persistence-and-resumability.md index 1330af7b1c..9b15dc55e1 100644 --- a/docs/rfc/009-session-persistence-and-resumability.md +++ b/docs/rfc/009-session-persistence-and-resumability.md @@ -14,18 +14,18 @@ The event-sourced model (ADR 0003) makes the log the single source of truth and Mirror the codebase's capability-seam pattern ([ADR 0009](../adr/0009-capability-seams.md), the `bash` template: an abstract `Service` interface, a concrete implementation, and consumers) for persistence. -**1. Abstract service `SessionPersistence`** — a new interface package `@deepseek-ai/dsh-session-persistence` owning `ctx.sessionPersistence`, depending only on `cordis` and `dsh-session`. Its persisted unit IS `SessionEvent` (`{ type, seq, time, data }`), reused verbatim — no conversion type. The method surface: +**1. Abstract service `SessionPersistence`** — a new interface package `@deepseek-ai/dsh-session-persistence` owning `ctx.sessionPersistence`, depending only on `cordis` and `dsh-session`. The `SessionHeader`/`SessionSummary`/`SessionMeta` types are owned by **`dsh-session`** (they live beside `SessionId` because `Session.header` is typed by them — see item 3a); the persistence package imports/re-exports them. Owning them in the persistence package would force `dsh-session` to depend back on it to type `Session.header`, a package cycle. Its persisted unit IS `SessionEvent` (`{ type, seq, time, data }`), reused verbatim — no conversion type. The method surface: - `create(meta: SessionMeta): Promise` — register a new session's header. The backend MAY defer the physical write until the first `append` (lazy materialization); `has`/`list` semantics for a zero-event session are specified, not left implicit. - `append(id, events: readonly SessionEvent[]): Promise` — durably persist a batch (called from the flush drain). Append-only; never rewrites. **Contract**: the first event's `seq` MUST equal the backend's stored next-seq (a DB impl asserts this inside a transaction; the file impl appends at EOF). All persisted `event.data` MUST be JSON-serializable. -- `load(id): Promise<{ meta: SessionMeta; events: SessionEvent[] }>` — replay header plus the full event log. Returns `meta` AND `events` so the live session is reconstructed with its `cwd`/lineage, not just its log. **Validation**: `events[i].seq === i` (contiguous, zero-based) or the load rejects. +- `load(id): Promise<{ meta: SessionMeta; events: SessionEvent[] }>` — replay header plus the event log up to the last durable checkpoint. Returns `meta` AND `events` so the live session is reconstructed with its `cwd`/lineage, not just its log. **Validation/repair**: the returned events MUST be contiguous (`events[i].seq === i`); a parse error or `seq` gap in the *middle* of the log makes the session unloadable (reject). The loop only flushes at `turn/end`, so a crash can leave a half-written final turn — `load` returns events only up to the **last complete `turn/end`** and the impl sets its write cursor to that length, so a subsequent `append` overwrites the orphaned tail rather than appending past it (no seq desync). This is the single chosen behavior: reject incomplete final turns, resume from the last clean checkpoint. - `list(): Promise` — lightweight listing from headers, no full-log parse. - `has(id)` / `delete(id)` — existence and removal. - `update(id, summary: Partial): Promise` — update mutable header fields without touching the append-only event log. The new `SessionMeta` splits into an immutable `SessionHeader` (`{ id, version, createdAt, cwd?, parentSession? }`) and a mutable `SessionSummary` (`{ updatedAt, title?, firstPrompt? }`); `SessionMeta = SessionHeader & SessionSummary`. Every reference system writes such a header (pi's `version: 3` header line, Codex's `SessionMeta`, Claude Code's tail metadata). It is kept *separate from the event log* deliberately: format-version, cwd, and lineage are storage concerns, not conversation events, so they stay out of `SessionEventMap` and never reach `deriveMessages()`. The alternative — a merge-extensible `session/meta` event as log line 0 — was considered: an in-log event would ride along with a seeded/forked session for free, whereas an out-of-log header must be threaded through a seam (item 3a). It was rejected because metadata is not replayable conversation state; the explicit metadata seam is the cleaner cost. -**2. Concrete impl `SessionPersistenceJsonl`** — a new package `@deepseek-ai/dsh-session-persistence-jsonl`. One file per session: a header line (`{ type: 'session', version, id, cwd, createdAt, parentSession? }`) followed by one `SessionEvent` JSON per line. On disk: a configured root with per-cwd subdirectories (pi-style `--encoded-cwd--/_.jsonl`) so sessions group by project. `list()` reads only each file's header line. Resilience over the example: append plus explicit flush; on `load`, drop only a corrupt/incomplete *trailing* line but reject a parse error or `seq` gap in the *middle* (a hole would desync `seq = log.length` appends); lazy materialization (no file until the first real event, so abandoned sessions leave nothing behind). +**2. Concrete impl `SessionPersistenceJsonl`** — a new package `@deepseek-ai/dsh-session-persistence-jsonl`. Per session: an append-only `.jsonl` event log (a `SessionHeader` line — `{ type: 'session', version, id, cwd, createdAt, parentSession? }` — followed by one `SessionEvent` JSON per line), plus a small sidecar `..summary.json` holding the mutable `SessionSummary` (`updatedAt`, `title?`, `firstPrompt?`). The split keeps the event log strictly append-only: `update(id, summary)` rewrites only the tiny sidecar (atomic temp-write + rename), never the log; `load`/`list` read the header line from the log and merge the sidecar to return a full `SessionMeta` (sidecar absent → summary fields default). On disk: a configured root with per-cwd subdirectories (pi-style `--encoded-cwd--/_.jsonl`) so sessions group by project. `list()` reads only each file's header line plus its sidecar. Resilience over the example: append plus explicit flush; **on `load`, an incomplete final turn is rejected back to the last complete `turn/end`** — see the load-repair rule below; lazy materialization (no file until the first real event, so abandoned sessions leave nothing behind). **2a. `assistant/chunk` persistence policy** (decided here, not deferred). The loop appends one `assistant/chunk` per raw stream chunk, but `deriveMessages()` skips chunks entirely — the assembled `assistant/message` is authoritative for history. It is tempting to drop chunks from the durable log (Codex's `policy.rs` filters deltas from its rollout). But `seq = log.length` and the load-validation `events[i].seq === i` require a *contiguous* log: filtering chunks out would leave holes (`[0,1,4,6,8]`) and break both the contract and resume. **Decision: the canonical durable log persists every `SessionEvent` verbatim, including `assistant/chunk`** — this keeps `seq` contiguous, keeps "persist `SessionEvent` directly" literally true, and lets RFC 010 replay streamed turns on `session/load`. A chunk-filtered *projection* (for export or a compacted listing) is possible later as a derived view with its own renumbering, but it is NOT the canonical log and NOT the default. The round-trip test asserts byte-identical events. @@ -43,7 +43,7 @@ Seed handling has two cases that the plugin must distinguish, and neither is the 1. Interface package `packages/session-persistence/` per [the cookbook](../cookbook/adding-a-package.md): abstract `SessionPersistence extends Service` (`super(ctx, 'sessionPersistence')`), the `declare module 'cordis'` ctx key, the `SessionHeader`/`SessionSummary`/`SessionMeta` types, and method contracts documented in JSDoc (durability, append-only, contiguous-seq, JSON-serializable, error semantics). 2. `dsh-session` changes: add the three meta types beside `SessionId`; add the metadata seam. `SessionStore.create(id?, seed?)` becomes `create(id?, options?: { seed?; meta? })` — a breaking signature change (callers pass `seed` positionally today: `AgentLoop.create`, and ~20+ call sites across `session`/`invariants`/`agent-loop` tests), so either migrate every caller or keep a deprecated overload during transition. Add a readonly `session.header`. Persistence captures the header on `session/created` (a synchronous event), so the impl must hold a per-session init promise that every `session/flush` awaits before `append`, and must seed existing live sessions via `ctx.sessions.list()` on plugin apply (HMR does not replay `session/created`, mirroring `dsh-invariants`). Do NOT add meta to `SessionEventMap`. -3. JSONL impl `packages/session-persistence-jsonl/`: header plus event lines (all events, verbatim — see 2a), sanitized per-cwd dirs and filenames, lazy materialize (header + first batch written atomically), append plus flush, trailing-line-tolerant and mid-gap-rejecting `load` that truncates back to the last complete `turn/end` rather than resuming a half-written turn, header-only `list`, the per-session write cursor, and snapshot-on-buffer. `static Config` for root dir and flush policy. +3. JSONL impl `packages/session-persistence-jsonl/`: append-only event log (header line + all events verbatim — see 2a) plus an atomic `.summary.json` sidecar for mutable fields, sanitized per-cwd dirs and filenames, lazy materialize (header + first batch written atomically), append plus flush, a `load` that returns events up to the last complete `turn/end` and sets the write cursor there (rejects mid-log gaps; overwrites the orphaned tail on next append), `list` from header + sidecar, the per-session write cursor, and snapshot-on-buffer. `static Config` for root dir and flush policy. 4. Generalize the write-path plugin: the impl subscribes to `session/created` (capture header, persist any seed for forks), `session/event` (snapshot + buffer), and `session/flush`/dispose (drain), replacing the per-example `session-jsonl.ts`; both examples load the shared plugin. 5. Resume seam: the async `AgentLoop.resume(agentId, resumeSessionId, options?)`; initialize the write cursor to the loaded length; verify `lastTurnNumber`/`deriveMessages` continuity. `AgentLoop` does NOT hard-inject `sessionPersistence` (that would break non-persistent examples) — `resume` checks for the service and throws a typed "persistence not configured" error; consumers that need resume (ACP) load the persistence plugin. 6. Tests (event-sourcing makes these strong): a round-trip property (persist an arbitrary log → reload → byte-identical events and identical `deriveMessages()` output — the replay equivalence ADR 0003 promises); resume vs fork (resume appends no duplicate seqs; a fork persists its seed once); contiguous-seq enforcement (mid-log gap rejected, re-append of a stored seq rejected); crash tolerance (a truncated final turn truncates back to the last `turn/end`); JSON-serializability rejection for a plugin-added event carrying non-serializable data; mutation-after-`session/event` does not corrupt the persisted snapshot; SessionId path-traversal is neutralized; lazy materialization (no file until the first event); `has`/`list` semantics for a zero-event session; HMR-safety (dispose drains buffers and closes file handles; apply seeds existing live sessions); concurrent sessions do not cross buffers. diff --git a/docs/rfc/010-acp-agent-client-protocol.md b/docs/rfc/010-acp-agent-client-protocol.md index 63a11a7db3..6210400975 100644 --- a/docs/rfc/010-acp-agent-client-protocol.md +++ b/docs/rfc/010-acp-agent-client-protocol.md @@ -14,17 +14,17 @@ This RFC has a hard prerequisite on RFC 009: it assumes durable session persiste 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 [ADR 0009](../adr/0009-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) — zero runtime dependencies, Apache-2.0, actively versioned. 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/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: | ACP (client ⇄ agent) | Harness seam | Notes | |---|---|---| | `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}` → `{sessionId}` | `ctx.agentLoop.create` | agent generates `sessionId`; reject a 2nd session (single-session MVP, see RFC 011); `cwd` validated (require absolute) with "launch the server in the workspace root" documented until the workdir seam exists; `mcpServers` ignored (no `mcpCapabilities` advertised) | -| `session/load {sessionId, cwd, mcpServers}` | RFC 009's async `AgentLoop.resume(agentId, sessionId)` | 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 | +| `session/new {cwd, mcpServers, additionalDirectories}` → `{sessionId}` | a new `agentLoop` create seam (see 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 `create(id)` hardcodes `${id}-session` and takes no metadata); reject a 2nd session (single-session MVP, see RFC 011); `cwd` validated (require absolute) with "launch the server in the workspace root" documented until the workdir seam exists; `mcpServers` ignored (no `mcpCapabilities` advertised); non-empty `additionalDirectories` rejected for the MVP (the bridge cannot yet widen bash/tool filesystem scope, so silently ignoring them would desync the client's filesystem-scope UI) | +| `session/load {sessionId, cwd, mcpServers, additionalDirectories}` | RFC 009's async `agentLoop` resume seam | 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; `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) | turn-end carries the real reason including `max-tokens`; honor the batch-into-one-turn and send-not-synchronously-running settle semantics | +| 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 | | `session/update: agent_thought_chunk` | `agent/stream-chunk` `reasoning-delta` | | | `session/update: tool_call` (pending→in_progress) | `session/event` `tool/call` | demux via a Session→sessionId map; `kind` inferred from the tool name | @@ -34,14 +34,16 @@ The mapping between ACP and existing harness seams — each row names the seam a 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 await `agent.done`. 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. +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 `LoopAgent`, 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. ## Plan -1. Package scaffold `packages/acp/` per [the cookbook](../cookbook/adding-a-package.md); add `@agentclientprotocol/sdk`; `inject: ['agents', 'agentLoop', 'sessions', 'tools', 'sessionPersistence']` (the last is required because `session/load` advertises `loadSession: true`). -2. Connection plus `initialize`/`session/new`: wire `AgentSideConnection` to stdin/stdout; protocolVersion negotiation; the single-session guard; the `sessionId↔agent` and `Session↔sessionId` maps. +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. -4. Prompt-turn streaming plus load: translate `agent/stream-chunk` and `session/event` into `session/update`; resolve `session/prompt` on settle. 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` with the authoritative `stopReason`; 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 RFC 009's `AgentLoop.resume`. +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 RFC 009's 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 RFC 009 — required for `session/load`), omits the stdout logger (see Risks), and adds `yarn 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: RFC 001 / [ADR 0013](../adr/0013-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. @@ -67,4 +69,3 @@ Permission-await and disposal hangs: a pending `request_permission` whose connec The 100% per-file coverage gate (repo policy) makes a branch-heavy protocol bridge real work. Accepted deliberately, surfaced so it isn't a surprise at PR time. ACP protocol-shape details (exact method names, `session/update` variants, permission option kinds, stop reasons) are taken from the ACP spec and the `@agentclientprotocol/sdk` types; they are not independently verifiable until the dependency is added, so the implementation pins the SDK version and conforms to its types rather than to this RFC's prose where they differ. - diff --git a/docs/rfc/011-acp-multi-session.md b/docs/rfc/011-acp-multi-session.md index 2ad08f4be1..dca24569f8 100644 --- a/docs/rfc/011-acp-multi-session.md +++ b/docs/rfc/011-acp-multi-session.md @@ -18,8 +18,8 @@ The harness core already supports many agents (`AgentRegistry.list()` and `Agent ## Plan -1. Generalize the two id maps to multi-entry and add the `agent→sessionId` reverse map; add a per-session record holding the agent, the in-flight-prompt state, the pending-permission registry, and the session's child context (see step 2). -2. Give each session its own child Cordis context (`ctx.extend()`) and register that session's listeners on it, so per-session listeners are fiber-scoped — disposing one session's child fiber removes exactly its listeners while the other N-1 sessions (and the bridge root) keep running. Demux every `agent/*` and `session/event` by id into the right session record. Note the single global `tools/execute` listener stays on the bridge root (it must see all agents) and routes via the reverse map. +1. Generalize the two id maps to multi-entry and add the `agent→sessionId` reverse map; add a per-session record holding the agent, the in-flight-prompt state, the pending-permission registry, and the session's disposer scope (see step 2). +2. Give each session a real per-session disposer scope, NOT `ctx.extend()` — in Cordis `ctx.extend()` only creates a child context/prototype, but `ctx.on()` registered on it is still owned by the current plugin fiber, so disposing it would not remove that session's listeners. Use a genuine child fiber (load a per-session sub-plugin, e.g. `ctx.plugin(...)` returning a fork, or collect each session's `ctx.on` disposers in its session record and call them on teardown). Demux every `agent/*` and `session/event` by id into the right session record. Note the single global `tools/execute` listener stays on the bridge root (it must see all agents) and routes via the reverse map. 3. Lift the `session/new` guard; keep `session/load` (RFC 010) working per session. 4. Tests for cross-session isolation: two sessions streaming and permission-prompting concurrently never interleave; a cancel/abort in one session leaves the other's stream and pending permission untouched; per-session in-flight-prompt enforcement holds independently; disposing one session leaves the others running. From de73afbe39660b022a2045054c1439f1f3bed4b8 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Sun, 14 Jun 2026 21:57:59 +0800 Subject: [PATCH 3/3] docs: resolve self-consistency follow-ups on RFCs 009-010 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 009: the crash-tail "overwrite" contradicted the append-only contract. Name it explicitly as a one-time truncation-repair (ftruncate+fsync to the last complete turn/end byte offset) that removes only the never-committed crash tail; committed events are never rewritten. Qualify the append/impl/ADR wording to match. - 010: remove the remaining concrete-loop references — the session/new and session/load table rows now point at the dsh-agent create/resume factory, and the Risks disposal line uses the interface-level settle signal (agent/status) instead of LoopAgent-only agent.done. --- docs/rfc/009-session-persistence-and-resumability.md | 10 +++++----- docs/rfc/010-acp-agent-client-protocol.md | 6 +++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/docs/rfc/009-session-persistence-and-resumability.md b/docs/rfc/009-session-persistence-and-resumability.md index 9b15dc55e1..9e07eebdbc 100644 --- a/docs/rfc/009-session-persistence-and-resumability.md +++ b/docs/rfc/009-session-persistence-and-resumability.md @@ -17,15 +17,15 @@ Mirror the codebase's capability-seam pattern ([ADR 0009](../adr/0009-capability **1. Abstract service `SessionPersistence`** — a new interface package `@deepseek-ai/dsh-session-persistence` owning `ctx.sessionPersistence`, depending only on `cordis` and `dsh-session`. The `SessionHeader`/`SessionSummary`/`SessionMeta` types are owned by **`dsh-session`** (they live beside `SessionId` because `Session.header` is typed by them — see item 3a); the persistence package imports/re-exports them. Owning them in the persistence package would force `dsh-session` to depend back on it to type `Session.header`, a package cycle. Its persisted unit IS `SessionEvent` (`{ type, seq, time, data }`), reused verbatim — no conversion type. The method surface: - `create(meta: SessionMeta): Promise` — register a new session's header. The backend MAY defer the physical write until the first `append` (lazy materialization); `has`/`list` semantics for a zero-event session are specified, not left implicit. -- `append(id, events: readonly SessionEvent[]): Promise` — durably persist a batch (called from the flush drain). Append-only; never rewrites. **Contract**: the first event's `seq` MUST equal the backend's stored next-seq (a DB impl asserts this inside a transaction; the file impl appends at EOF). All persisted `event.data` MUST be JSON-serializable. -- `load(id): Promise<{ meta: SessionMeta; events: SessionEvent[] }>` — replay header plus the event log up to the last durable checkpoint. Returns `meta` AND `events` so the live session is reconstructed with its `cwd`/lineage, not just its log. **Validation/repair**: the returned events MUST be contiguous (`events[i].seq === i`); a parse error or `seq` gap in the *middle* of the log makes the session unloadable (reject). The loop only flushes at `turn/end`, so a crash can leave a half-written final turn — `load` returns events only up to the **last complete `turn/end`** and the impl sets its write cursor to that length, so a subsequent `append` overwrites the orphaned tail rather than appending past it (no seq desync). This is the single chosen behavior: reject incomplete final turns, resume from the last clean checkpoint. +- `append(id, events: readonly SessionEvent[]): Promise` — durably persist a batch (called from the flush drain). Committed events (at or below a flushed `turn/end`) are append-only and never rewritten; the only exception is the one-time truncation-repair of a never-committed crash tail on the first `append` after a `load` (see `load`). **Contract**: the first event's `seq` MUST equal the backend's stored next-seq after any such repair (a DB impl asserts this inside a transaction; the file impl appends at EOF). All persisted `event.data` MUST be JSON-serializable. +- `load(id): Promise<{ meta: SessionMeta; events: SessionEvent[] }>` — replay header plus the event log up to the last durable checkpoint. Returns `meta` AND `events` so the live session is reconstructed with its `cwd`/lineage, not just its log. **Validation/repair**: the returned events MUST be contiguous (`events[i].seq === i`); a parse error or `seq` gap in the *middle* of the log makes the session unloadable (reject). The loop only flushes at `turn/end`, so a crash can leave a half-written final turn *below* the last committed checkpoint — `load` returns events only up to the **last complete `turn/end`**, and a subsequent `append` runs the **truncation-repair** step (see impl) that physically discards the orphaned tail before writing. This keeps the append-only contract honest: only the never-committed crash tail is ever removed; events at or below a flushed `turn/end` are never rewritten. - `list(): Promise` — lightweight listing from headers, no full-log parse. - `has(id)` / `delete(id)` — existence and removal. - `update(id, summary: Partial): Promise` — update mutable header fields without touching the append-only event log. The new `SessionMeta` splits into an immutable `SessionHeader` (`{ id, version, createdAt, cwd?, parentSession? }`) and a mutable `SessionSummary` (`{ updatedAt, title?, firstPrompt? }`); `SessionMeta = SessionHeader & SessionSummary`. Every reference system writes such a header (pi's `version: 3` header line, Codex's `SessionMeta`, Claude Code's tail metadata). It is kept *separate from the event log* deliberately: format-version, cwd, and lineage are storage concerns, not conversation events, so they stay out of `SessionEventMap` and never reach `deriveMessages()`. The alternative — a merge-extensible `session/meta` event as log line 0 — was considered: an in-log event would ride along with a seeded/forked session for free, whereas an out-of-log header must be threaded through a seam (item 3a). It was rejected because metadata is not replayable conversation state; the explicit metadata seam is the cleaner cost. -**2. Concrete impl `SessionPersistenceJsonl`** — a new package `@deepseek-ai/dsh-session-persistence-jsonl`. Per session: an append-only `.jsonl` event log (a `SessionHeader` line — `{ type: 'session', version, id, cwd, createdAt, parentSession? }` — followed by one `SessionEvent` JSON per line), plus a small sidecar `..summary.json` holding the mutable `SessionSummary` (`updatedAt`, `title?`, `firstPrompt?`). The split keeps the event log strictly append-only: `update(id, summary)` rewrites only the tiny sidecar (atomic temp-write + rename), never the log; `load`/`list` read the header line from the log and merge the sidecar to return a full `SessionMeta` (sidecar absent → summary fields default). On disk: a configured root with per-cwd subdirectories (pi-style `--encoded-cwd--/_.jsonl`) so sessions group by project. `list()` reads only each file's header line plus its sidecar. Resilience over the example: append plus explicit flush; **on `load`, an incomplete final turn is rejected back to the last complete `turn/end`** — see the load-repair rule below; lazy materialization (no file until the first real event, so abandoned sessions leave nothing behind). +**2. Concrete impl `SessionPersistenceJsonl`** — a new package `@deepseek-ai/dsh-session-persistence-jsonl`. Per session: an append-only `.jsonl` event log (a `SessionHeader` line — `{ type: 'session', version, id, cwd, createdAt, parentSession? }` — followed by one `SessionEvent` JSON per line), plus a small sidecar `..summary.json` holding the mutable `SessionSummary` (`updatedAt`, `title?`, `firstPrompt?`). The split keeps committed events untouched: `update(id, summary)` rewrites only the tiny sidecar (atomic temp-write + rename), never the log; `load`/`list` read the header line from the log and merge the sidecar to return a full `SessionMeta` (sidecar absent → summary fields default). On disk: a configured root with per-cwd subdirectories (pi-style `--encoded-cwd--/_.jsonl`) so sessions group by project. `list()` reads only each file's header line plus its sidecar. Resilience over the example: append plus explicit flush; **truncation-repair on the first append after a crash** — `load` computes the byte offset of the last complete `turn/end`, and the impl truncates the file to that offset (`ftruncate`, then `fsync`) before its first append, atomically discarding the never-committed tail. Only the uncommitted crash tail is ever removed. Lazy materialization (no file until the first real event, so abandoned sessions leave nothing behind). **2a. `assistant/chunk` persistence policy** (decided here, not deferred). The loop appends one `assistant/chunk` per raw stream chunk, but `deriveMessages()` skips chunks entirely — the assembled `assistant/message` is authoritative for history. It is tempting to drop chunks from the durable log (Codex's `policy.rs` filters deltas from its rollout). But `seq = log.length` and the load-validation `events[i].seq === i` require a *contiguous* log: filtering chunks out would leave holes (`[0,1,4,6,8]`) and break both the contract and resume. **Decision: the canonical durable log persists every `SessionEvent` verbatim, including `assistant/chunk`** — this keeps `seq` contiguous, keeps "persist `SessionEvent` directly" literally true, and lets RFC 010 replay streamed turns on `session/load`. A chunk-filtered *projection* (for export or a compacted listing) is possible later as a derived view with its own renumbering, but it is NOT the canonical log and NOT the default. The round-trip test asserts byte-identical events. @@ -43,11 +43,11 @@ Seed handling has two cases that the plugin must distinguish, and neither is the 1. Interface package `packages/session-persistence/` per [the cookbook](../cookbook/adding-a-package.md): abstract `SessionPersistence extends Service` (`super(ctx, 'sessionPersistence')`), the `declare module 'cordis'` ctx key, the `SessionHeader`/`SessionSummary`/`SessionMeta` types, and method contracts documented in JSDoc (durability, append-only, contiguous-seq, JSON-serializable, error semantics). 2. `dsh-session` changes: add the three meta types beside `SessionId`; add the metadata seam. `SessionStore.create(id?, seed?)` becomes `create(id?, options?: { seed?; meta? })` — a breaking signature change (callers pass `seed` positionally today: `AgentLoop.create`, and ~20+ call sites across `session`/`invariants`/`agent-loop` tests), so either migrate every caller or keep a deprecated overload during transition. Add a readonly `session.header`. Persistence captures the header on `session/created` (a synchronous event), so the impl must hold a per-session init promise that every `session/flush` awaits before `append`, and must seed existing live sessions via `ctx.sessions.list()` on plugin apply (HMR does not replay `session/created`, mirroring `dsh-invariants`). Do NOT add meta to `SessionEventMap`. -3. JSONL impl `packages/session-persistence-jsonl/`: append-only event log (header line + all events verbatim — see 2a) plus an atomic `.summary.json` sidecar for mutable fields, sanitized per-cwd dirs and filenames, lazy materialize (header + first batch written atomically), append plus flush, a `load` that returns events up to the last complete `turn/end` and sets the write cursor there (rejects mid-log gaps; overwrites the orphaned tail on next append), `list` from header + sidecar, the per-session write cursor, and snapshot-on-buffer. `static Config` for root dir and flush policy. +3. JSONL impl `packages/session-persistence-jsonl/`: append-only event log (header line + all events verbatim — see 2a) plus an atomic `.summary.json` sidecar for mutable fields, sanitized per-cwd dirs and filenames, lazy materialize (header + first batch written atomically), append plus flush, a `load` that returns events up to the last complete `turn/end` and computes its byte offset; the first post-load `append` runs truncation-repair (`ftruncate` to that offset + `fsync`, discarding only the uncommitted crash tail) before writing (rejects mid-log gaps), `list` from header + sidecar, the per-session write cursor, and snapshot-on-buffer. `static Config` for root dir and flush policy. 4. Generalize the write-path plugin: the impl subscribes to `session/created` (capture header, persist any seed for forks), `session/event` (snapshot + buffer), and `session/flush`/dispose (drain), replacing the per-example `session-jsonl.ts`; both examples load the shared plugin. 5. Resume seam: the async `AgentLoop.resume(agentId, resumeSessionId, options?)`; initialize the write cursor to the loaded length; verify `lastTurnNumber`/`deriveMessages` continuity. `AgentLoop` does NOT hard-inject `sessionPersistence` (that would break non-persistent examples) — `resume` checks for the service and throws a typed "persistence not configured" error; consumers that need resume (ACP) load the persistence plugin. 6. Tests (event-sourcing makes these strong): a round-trip property (persist an arbitrary log → reload → byte-identical events and identical `deriveMessages()` output — the replay equivalence ADR 0003 promises); resume vs fork (resume appends no duplicate seqs; a fork persists its seed once); contiguous-seq enforcement (mid-log gap rejected, re-append of a stored seq rejected); crash tolerance (a truncated final turn truncates back to the last `turn/end`); JSON-serializability rejection for a plugin-added event carrying non-serializable data; mutation-after-`session/event` does not corrupt the persisted snapshot; SessionId path-traversal is neutralized; lazy materialization (no file until the first event); `has`/`list` semantics for a zero-event session; HMR-safety (dispose drains buffers and closes file handles; apply seeds existing live sessions); concurrent sessions do not cross buffers. -7. Docs: update the "Event-sourced sessions" durability-seam paragraph and the "Deferred work" list in [docs/architecture.md](../architecture.md) (persistence is no longer deferred); sync the affected package READMEs/JSDoc (`dsh-session` for the `create`/`session.header` change); add a [cookbook](../cookbook/) note on writing a persistence backend; resolve the `TODO(review)` on the event vocabulary now that a real persistence plugin coexists with the loop. On implementation this likely graduates to an ADR — "persistence is an abstract service over the existing `SessionEvent`; verbatim append-only log; file canonical, DB drop-in" is durable, contested, and surprising enough to record. +7. Docs: update the "Event-sourced sessions" durability-seam paragraph and the "Deferred work" list in [docs/architecture.md](../architecture.md) (persistence is no longer deferred); sync the affected package READMEs/JSDoc (`dsh-session` for the `create`/`session.header` change); add a [cookbook](../cookbook/) note on writing a persistence backend; resolve the `TODO(review)` on the event vocabulary now that a real persistence plugin coexists with the loop. On implementation this likely graduates to an ADR — "persistence is an abstract service over the existing `SessionEvent`; verbatim append-only log (committed events never rewritten; only an uncommitted crash tail is truncation-repaired); file canonical, DB drop-in" is durable, contested, and surprising enough to record. ## Risks diff --git a/docs/rfc/010-acp-agent-client-protocol.md b/docs/rfc/010-acp-agent-client-protocol.md index 6210400975..42f1bd5c56 100644 --- a/docs/rfc/010-acp-agent-client-protocol.md +++ b/docs/rfc/010-acp-agent-client-protocol.md @@ -21,8 +21,8 @@ The mapping between ACP and existing harness seams — each row names the seam a | ACP (client ⇄ agent) | Harness seam | Notes | |---|---|---| | `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}` | a new `agentLoop` create seam (see 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 `create(id)` hardcodes `${id}-session` and takes no metadata); reject a 2nd session (single-session MVP, see RFC 011); `cwd` validated (require absolute) with "launch the server in the workspace root" documented until the workdir seam exists; `mcpServers` ignored (no `mcpCapabilities` advertised); non-empty `additionalDirectories` rejected for the MVP (the bridge cannot yet widen bash/tool filesystem scope, so silently ignoring them would desync the client's filesystem-scope UI) | -| `session/load {sessionId, cwd, mcpServers, additionalDirectories}` | RFC 009's async `agentLoop` resume seam | 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; `additionalDirectories` rejected as in `session/new` | +| `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 RFC 011); `cwd` validated (require absolute) with "launch the server in the workspace root" documented until the workdir seam exists; `mcpServers` ignored (no `mcpCapabilities` advertised); non-empty `additionalDirectories` rejected for the MVP (the bridge cannot yet widen bash/tool filesystem scope, so silently ignoring them would desync the client's filesystem-scope UI) | +| `session/load {sessionId, cwd, mcpServers, additionalDirectories}` | the `dsh-agent` resume factory (RFC 009 + 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; `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 | @@ -64,7 +64,7 @@ New third-party runtime dependency plus protocol drift: `@agentclientprotocol/sd 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 (await `agent.done`), not orphan awaits on a closed pipe. +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 `LoopAgent`-only), not orphan awaits on a closed pipe. The 100% per-file coverage gate (repo policy) makes a branch-heavy protocol bridge real work. Accepted deliberately, surfaced so it isn't a surprise at PR time.