Records the verified design before implementing: keep dsh-bash agent-side
execution and render Zed's terminal tool-call card via the `_meta` convention
(terminal_info/terminal_output/terminal_exit), capability-gated on
clientCapabilities._meta.terminal_output, with the ```console text block as the
no-capability fallback. Rejects the spec's client-side terminal/create path (it
would bypass dsh-bash's sandbox/env-scrub/ownership/cwd). Studied
claude-agent-acp, codex-acp, and Zed's renderer to ground the wire contract.
Live streaming and command classification are noted as separate follow-ups.
- bash presentCall title is now "description — command" (e.g. "List files in
src — ls -la src"). An execute-kind ACP card HIDES rawInput (Zed renders it
only for non-terminal tools), so the command must ride in the always-visible
title to be seen — matching how claude-agent-acp/codex-acp title execute
tools. The command stays in rawInput too for non-execute UIs that show it.
- Rework the acp tool-call presentation tests (turns + load replay) to drive the
REAL dsh-tool-bash + dsh-bash-local via a new makeBridgeHarness({ withBash })
option, running an actual `echo` — instead of an inline fake bash tool. The
mock MODEL still scripts the call (deterministic, no key), but the tool and
executor are real, so the test verifies the shipping presentCall/presentResult.
- AGENTS.md: add the principle "prefer the REAL implementation over a mock/
stand-in in tests" (mock only the expensive/non-deterministic boundary).
- RFC (proposed): the ACP terminal sub-protocol + command classification — the
capability-gated rich rendering (live cwd-header terminal card, classify a
`cat` as a read / `grep` as a search) that the reference adapters do; the
fenced ```console text block stays the no-capability baseline. Studied
codex-acp, claude-agent-acp, and Zed's renderer to ground it.
- schemas() builds the model-facing ToolSchema by EXPLICIT allowlist
({name, description, parameters, strict?}) instead of stripping `execute` —
presentCall/presentResult are functions that must never leak into a model
request, and an allowlist can't drift when a new ToolDefinition member lands.
- session/load replay uses a THROWAWAY ToolPresenter, not record.presenter, so
a historical interrupted-mid-tool turn (tool/call with no tool/result) can't
leave stale in-flight state on the live presenter that serves later events.
- ToolPresenter.call/result contain a throwing presentCall/presentResult: log
via an onError sink and fall back to the generic presentation, so a buggy
display callback can never fail a live turn or a load replay.
- acp README inject list now includes `tools`.
- remove a stray blank line at EOF (git diff --check gate).
Regressions added: schemas() drops presenter callbacks (+ keeps `strict`);
session/load replays a tool call with the tool-owned presentation; a throwing
presenter is contained (direct + through the real bridge) with and without an
onError sink.
In Zed the tool-call card showed only "bash" — the bare tool name — instead
of what the command does. Fix it by letting each TOOL own how its calls render,
rather than the bridge special-casing names.
dsh-tools: add an optional two-state presentation seam to ToolDefinition /
defineTool — `presentCall(args)` (pending: title, kind, rawInput) and
`presentResult(args, result)` (completed: title?, content?). Provider-neutral
`ToolCallKind`/`ToolCallPresentation`/`ToolResultPresentation` vocabulary so
tools never depend on ACP. defineTool soft-validates args (display runs on log
replay, so a malformed/old shape returns undefined instead of throwing).
dsh-tool-bash: bash declares presentCall (model `description` → title, exact
`command` → rawInput, kind execute) and presentResult (wrap output in a fenced
```console block — a UI-only affordance kept out of the model-facing result);
bash_output/bash_kill present task-scoped titles.
dsh-acp: inject `tools`; a per-session `ToolPresenter` looks the tool up by name
and maps its neutral presentation to the ACP tool_call/tool_call_update wire
shape, with a generic fallback (title = name) for tools that declare nothing.
Because the `tool/result` event carries only {callId, content, isError}, the
presenter keeps a small bridge-local map of ONLY in-flight calls' (name, args),
keyed by callId and removed as each result is presented — no event-schema or
core change. Replay uses a throwaway presenter so loaded sessions render
identically to live ones.
Tests: dsh-tools defineTool presenters (typed args, soft-validate), tool-bash
bash/bash_output/bash_kill presenters, acp ToolPresenter (tool-owned mapping,
unknown-callId fallback, in-flight-only map), and an end-to-end turn through the
bridge. The key-gated e2e now asserts a real bash call's title is the model
description (not "bash") and rawInput is the command — verified against the real
DeepSeek model. The test harness derives its inject from the bridge's exported
`inject` so it can't drift again.
- AgentLoop.resume uses `this.ctx.get('sessionPersistence')` (strict) instead
of the `, false` overload: still topology-independent, but an inactive/
absent backend reads as undefined (rejected by the existing guard) rather
than being handed back mid-teardown.
- Correct the bridge teardown comment: an ACP-created agent's registry entry
binds to the BRIDGE fiber (the factory is reached through the bridge's
traceable proxy, so AgentLoop.start's `this.ctx.effect` registration uses the
caller context), not the AgentLoop fiber — so an ACP-only HMR dispose
reclaims it. Add a regression test pinning that ownership.
- Sync the ctx.get guidance in the post-mortem, packages/AGENTS.md, and the
dsh-code-review skill to the strict form.
Two independent bugs made the ACP server crash the moment an editor (Zed)
connected, despite 178 green unit tests at 100% coverage:
1. `session/new` threw `cannot get property "agents" without inject`. Root
cause: a stray `export default apply` made the cordis Loader's
`unwrapExports` (`exports.default ?? exports`) collapse the module to the
bare `apply` function, discarding the sibling `inject`/`name`/`Config`
named exports. The plugin fiber was built with empty `inject`, so every
`ctx.<service>` read in `apply` threw at load. Fix: remove the default
export so the Loader uses the namespace.
2. `session/load` threw `cannot get property "sessionPersistence" without
inject`. `AgentLoop.resume` read `this.ctx.sessionPersistence` (a service
it deliberately does NOT inject); the property proxy's ancestor-only fiber
walk fails through the bridge's traceable shadow. Fix: read it via
`this.ctx.get('sessionPersistence', false)`, the topology-independent
global-store lookup.
Why the suite missed both: every test mounted the plugin by hand
(`ctx.plugin({name,inject,apply})`), bypassing `unwrapExports` entirely, and
the only test driving these RPCs was key-gated (skipped in CI). Added a no-key
`session/new` e2e that boots the real example through the real Loader — it
fails loudly on bug #1 without an API key. Set `TSX_TSCONFIG_PATH` in the e2e
spawn so the subprocess resolves workspace `paths` from a temp cwd (it was
silently falling back to a stale built `lib/`).
Docs: post-mortem 0001; AGENTS.md "line coverage is not behavior coverage" +
with-key/smoke-test philosophy; packages/AGENTS.md plugin-export-shape and
ctx.get rules; dsh-code-review SKILL checks.
Collapse docs/adr/ and docs/rfc/ into a single docs/rfc/ with proposed/,
implemented/, and rejected/ subfolders. Every file is renamed to
yyyy-mm-dd-topic-title.md, where the date is when the topic was first
proposed (from git history). ADRs and RFCs that covered exactly the same
topic are merged (property-based testing, session persistence); the
umbrella RFC 005 stays split across its three implemented decisions, and
RFC 006's deferred part-3 (API extractor reports) splits into its own
proposed RFC. All cross-references become machine-checkable relative
links instead of bare "ADR NNNN" / "RFC NNN" prose.
Add a verify-md-links doc-sync gate (scripts/verify-md-links.ts) that
checks every relative Markdown cross-link resolves, wired into doc-sync
alongside verify-md-wrap. This makes the reorganization self-verifying:
the same change that rewrote ~forty inter-doc links adds the check that
proves none dangle. Document the cross-link convention in a new
docs/AGENTS.md and record the gate as an implemented RFC.
doc-sync, typecheck, lint, and the full test suite (667) all pass.
Lifts the RFC 010 § Deferred restriction that the server had to launch in the
workspace ("cwd must equal the launch directory"). An editor can now open any
project folder, and N concurrent sessions over one connection can each target a
different directory.
- packages/acp: drop the `cwd === process.cwd()` guard in validateWorkspaceParams
(keep "must be absolute" — the cwd becomes the session header / bash workdir),
and drop the persisted-cwd-vs-launch-dir check in session/load (a resumed
session keeps its original header.cwd, so its bash tools run in its workspace).
- packages/tool-bash: the missing link — default the bash workdir to the calling
agent's session cwd (`exec.agent.session.header.cwd`) via a new resolveWorkdir
helper. An explicit model `workdir` still wins; a relative one resolves against
the session cwd. This is the only correct spot for multi-session: N sessions
share one ctx.bash executor, so the workdir must come per-call from exec.agent,
not executor config. Falls back to the executor default when no session cwd is
available (preserves non-ACP behavior).
- Trust: the cwd originates from the ACP client (the user's editor) at
session/new — same trust level as the old launch dir; no new untrusted-input
path. `additionalDirectories` (scope widening / sandbox) stays rejected.
- Tests: bridge accepts any absolute cwd + records it on the header; session/load
honors the persisted cwd; bash defaults to / resolves relative against the
session cwd; two sessions with different cwds each run bash in their own dir;
non-absolute cwd still rejected. 100% per-file coverage maintained.
- Docs: RFC 010 status + § Deferred cwd bullet marked RESOLVED; acp README adds a
Per-session cwd section; tool-bash + example READMEs and e2e comments updated.
The loop logs the assistant/message (carrying tool-call blocks) BEFORE running
the tools, so a crash mid-tool leaves durable tool calls with no matching
tool/result. interruptedTurnClosers only added step/end + turn/end, so a resumed
session's deriveMessages() replayed a dangling assistant tool-call — which every
provider rejects as an invalid transcript on the next request.
interruptedTurnClosers now scans the interrupted turn for tool-call blocks
without a matching tool/result and synthesizes an error tool/result for each
(before the step/end), so the rehydrated history is a valid transcript. Adds a
dedicated repair.spec.ts and a shared-contract case proving both backends pair
every orphaned call with a result. Docs (ADR 0018, both persistence READMEs,
load() JSDoc) updated.
Also fixes the echo-agent README session-cleanup path: demo:echo runs from the
repo root, so sessions land in <repo-root>/.sessions/_no-cwd/, not
examples/echo-agent/.sessions/ (review #33).
Mirror the JSONL backend's crash-recovery contract change: load() now PRESERVES
the real events of an interrupted final turn (a turn can be huge — truncating it
would destroy work) and durably CLOSES the orphaned turn with synthetic boundary
events (step/end if open, then turn/end {interrupted}) inside one transaction
that also deletes any torn tail row. load() is therefore mutating; the deferred
truncation-repair on the next append is gone. Replaces cutAtLastTurnEnd with
scanRows (longest preserved prefix + torn-tail offset). Updates the sqlite tests
and README to the preserve-and-close semantics; the shared runPersistenceContract
suite now holds both backends to identical interrupted-turn behavior.
A config agent with `resumeSessionId` set continues a persisted session
instead of starting a fresh `${id}-session-<uuid>`. The id is sourced from
an env var in cordis.yml, so the coding-agent demo can resume a prior
conversation without code changes. The resume is deferred until the
`sessionPersistence` backend loads (via ctx.inject) and is contained: a
missing/unreadable id logs a warning and starts no agent. Adds a real-API
resume e2e proving cross-process continuity through the JSONL backend.
A crash can leave a durable log whose final turn never closed. The old
behavior truncated everything after the last turn/end as a "crash tail".
But a single turn can be HUGE in a long-horizon task (many steps, large
tool output), so truncating it silently destroys real, durably-written
work — truncating a turn is wrong.
New crash recovery (ADR 0018): load() PRESERVES the interrupted turn's
events and CLOSES the orphaned turn by durably appending synthetic
boundary events — a step/end if a step was open, then a turn/end carrying
the new merge-extensible TurnEndReason {kind:'interrupted'}. load()
returns the balanced log, so a resumed session is immediately usable. Only
a never-fully-written TORN tail fragment is discarded; corruption in the
committed region is still unloadable.
- dsh-session: TurnEndReason {kind:'interrupted'} + shared
interruptedTurnClosers() repair helper.
- JSONL backend: scanLog preserves the longest contiguous prefix
(including a partial final turn); loadCore truncates a torn fragment and
durably writes the closers, returning the balanced log.
- runPersistenceContract gains a crash-recovery test (both backends + mock).
- Docs: ADR 0018/0017, architecture.md, package READMEs.
Also (review #33): RFC 013 records the "move event vocabulary to Zod"
question (merge-extensible maps → runtime schema registry) + blast radius;
deferred, not done here.
The `materialized` INTEGER column was redundant: create()/update() already
keep a lazy session in memory and write no row, so a `sessions` row is
written only by the first append. Its EXISTENCE is the materialization
signal — has()/list() now report exactly the sessions that have a row,
matching the JSONL backend's "file exists ⇔ materialized".
The column only existed to force has()/list() to FALSE for an all-tail
crash (a partial first turn, zero committed events). That actually
DIVERGED from the JSONL backend, whose file (and thus has()=true) survives
a first append that never reached turn/end. Removing the column drops that
special case: an all-tail session keeps its row and stays present, the
same as JSONL. The orphaned tail rows are still removed by the deferred
truncation-repair on the next append, and load() stays non-mutating.
Also: add a TODO to route through a cordis db service if one is adopted,
and correct the README's Node-version framing to the repo's engines (>=24).
Lifts the RFC 010 single-session-per-connection cap: the bridge now runs N
concurrent sessions over one connection, each mapped to its own LoopAgent.
- packages/acp: live sessions held in a Map<sessionId, SessionRecord> with an
agent→sessionId reverse WeakMap so agent/* events (which carry only the
Agent) demux in O(1). Every session/event and agent/status is routed strictly
to its owning record — concurrent sessions never cross-settle or interleave
their session/update notifications. Per-session state: one in-flight prompt
each, session/cancel aborts+settles only its own agent/prompt, session/load
reserves a per-id load slot (distinct ids load concurrently; re-loading a live
id is rejected), and disposal drains every live session in parallel to
quiescence.
- packages/tool-bash: record each background task's owning agent at spawn and
keep it for the executor's lifetime (NOT cleared on completion).
bash_output/bash_kill reject a task owned by a different agent (a task with no
owner is open; a no-agent caller can't access an owned task). Task ids are
global and predictable, so this is the fence that stops one session's agent
from reading/killing another session's background task.
- Per-session permission ownership and a per-agent disposer seam stay deferred
(depend on the deferred permission gate); the reverse map the gate will route
through is in place. RFC 011 stays `proposed`.
- Tests: two sessions stream concurrently without interleave; cross-session
cancel isolation; per-session in-flight enforcement; dispose-all-to-quiescence;
bash cross-session read/kill rejected (+ no-agent and unowned-task cases).
- Docs: RFC 011 implementation-status note; acp + tool-bash READMEs; example
MVP-limitations updated. 100% per-file coverage maintained.
Implements the RFC 010 MVP: a new `@deepseek-ai/dsh-acp` package bridges the
harness agent to the Agent Client Protocol (JSON-RPC 2.0 over newline-delimited
stdio), so Zed and other ACP editors can drive the coding agent — streaming
render, tool-call display, and resumable sessions via `session/load`.
- packages/acp: AgentSideConnection wiring; initialize/newSession/loadSession/
prompt/cancel; a total TurnEndReason→StopReason codec; settle-once with a
fallback chain (agent/turn-end → logged turn/end → idle); single-session
guard; cwd-must-equal-launch-dir validation; load replays from the persisted
event log (assistant/chunk→agent_message_chunk, tool/call/result→tool_call*).
- agent: add Agent.whenIdle() quiescence signal to the interface; LoopAgent
implements it (resolves on the first running→idle/disposed transition). The
bridge awaits it on disposal so teardown reaches quiescence, not just abort.
- examples: extract the shared provider/tool core into examples/base.yml;
coding-agent nest-includes it; new examples/acp-agent serves the agent over
ACP with JSONL persistence and no stdout logger (stdout is the protocol).
- Permission gate deferred (TODO(rfc010-permission-gate)): tools run with the
executor's full authority; only the Agent→sessionId ownership seam is laid
down. Cancel is best-effort for a not-yet-started queued turn
(TODO(rfc010-cancel-prestep)). RFC 010 stays `proposed`.
- Docs: package README + Zed snippet; client-driver cookbook section; root and
packages layout/commands; RFC 010 implementation-status note.
48 bridge tests + whenIdle coverage; 100% per-file coverage; e2e boots the
example as a subprocess and verifies a written file on disk (key-gated, with a
no-key stdout-purity check).
master's pnpm migration claimed ADR 0016 (0016-pnpm-over-yarn), which
collides with this stack's session-persistence ADR. Renumbering the
session-persistence ADR to 0018 (turn-enclosure stays 0017); update the
json.ts module-doc reference accordingly.
Add a `max-tokens` variant to `TurnEndReasonMap` and carry the model
finish reason up from `runStep` to `runTurn`, applying the rule "any
max-tokens step in the turn surfaces as max-tokens" (disposed/aborted/
error still take precedence). This lets consumers distinguish a clean
stop from a truncated one — the contract RFC 010's ACP bridge maps to
the `max_tokens` stop reason.
Also add an AGENTS.md rule: write an ADR when (and only when) a PR makes
a durable, contested, surprising decision.
materialize() removed the temp hard link in a finally that ran BEFORE
syncDir() and before state.materialized/cursor advanced. If link()
succeeded (log published) but the temp rm then threw, materialize()
rejected after publishing — leaving state.materialized false, so the
buffered events stayed unpersisted and every retry wedged on the
"already exists" exists() backstop.
Restructured to the robust shape: track link() success; on link failure
remove the temp (the only reference) before propagating; on success
fsync the directory, mark materialized, THEN best-effort remove the
now-redundant temp link (a leftover *.tmp is harmless and never read).
A temp-rm failure can no longer reject a session whose log published.
The prior fix handled a throwing session/event listener on the turn/start
append, but the SAME push-before-notify hazard remained on the three
FINALIZER appends. Session.append pushes the event before notifying, so a
throwing listener on a finalizer event left the event logged but aborted
the rest of finalization — stranding the turn open.
- failTurn(): set `reason` BEFORE appending the `error` event, and contain
a throwing session/event listener on it (the event is already logged
either way). Otherwise reason stayed unset, agent/error was skipped, and
the caller's closeTurn(false) never ran → open turn.
- closeStep(): the try/catch wrapped only the agent/step-end EMIT, not the
step/end APPEND. A throwing session/event listener on step/end escaped —
fatal when closeStep runs from the outer catch during finalization
(turn/start + step/end but no turn/end). Now both the append and the emit
are contained and surface as a turn error via failTurn.
- closeTurn(): contain a throwing session/event listener on the turn/end
append (it would propagate to the runLoop backstop from closeTurn(false),
or skip the turn-end emit from closeTurn(true)). turn/end is logged
either way, so the turn stays balanced.
Regressions: a throwing session/event listener on the error event, on
step/end during finalization (driven by a throwing agent/step-start), and
on turn/end — each leaves a balanced turn and the loop survives.
- load() no longer DELETEs the crash tail — it stays non-mutating w.r.t.
the event log and records a repair point (repairFrom). The next append
runs the DELETE inside its own transaction before inserting. This makes
the SQLite backend honor the SAME public contract as JSONL (load returns
the committed prefix; the subsequent append performs the one-time
physical truncation-repair), instead of mutating during load.
- All-tail load: when the discarded crash tail was the session's only
committed content (committed.length === 0), the metadata row still read
materialized = 1 from the prior append, so has()/list() reported a
session load() had just emptied. load() now flips the row's materialized
flag to 0 (metadata only — the orphaned event rows are still removed by
the deferred repair), so has()/list() are immediately consistent.
- Schema version: openDatabase now stores SCHEMA_VERSION in PRAGMA
user_version on a fresh database and rejects opening one whose
user_version is newer than this build supports, protecting against a
future incompatible layout.
Regression tests: load is non-mutating (tail rows survive until the next
append), all-tail load makes has()/list() false, and a newer-schema
database is rejected on open.
- agent/README: the inject() line said "without triggering a turn",
regressing the #32 turn-enclosure model. Restored the running-vs-idle
wording (idle inject wraps a one-shot injection turn; ADR 0017) to match
the interface JSDoc.
- agent-loop resume() JSDoc said "throws a typed error" but the code
throws a plain Error (consistent with the sibling assertAgentIdFree
throw). Softened to "rejects with a clear error" — no behavior change;
plain Error is intentional (no consumer needs a structured code here).
A durable persistence backend must not treat a storage fault as absence.
listCwdDirs() and exists() swallowed EVERY error and reported "no
sessions" / "not found", so EACCES/ENOTDIR/transient I/O could make
list() return nothing, load() report not-found, and collision checks
proceed under a false absence assumption.
- Add an isENOENT() helper; listCwdDirs() and exists() now return the
empty/absent result ONLY for ENOENT and rethrow every other error.
Regression tests drive ENOTDIR through both paths.
TODO-level hardening also addressed:
- writeSidecar() now uses an exclusive owner-only temp open ('wx', 0o600)
like the log-materialization path, instead of a truncating writeFile —
the sidecar can carry user data (title/firstPrompt), so a predictable/
pre-existing temp path must never be silently followed.
- The shared runPersistenceContract serializability case now exercises
EVERY value isJsonValue rejects (BigInt, undefined, Infinity, function,
symbol, Map, circular), not just BigInt, so a backend cannot pass the
contract while accepting values that corrupt the round-trip. The mock
MemoryPersistence now validates via the canonical isJsonValue.
Session.append pushes the event BEFORE notifying session/event listeners,
so a throwing listener leaves the event in the log while the line after
the append (a boolean flag) never runs. Both turn-balance decisions were
gated on such flags, so a throwing listener could strand an open turn or
skip a durability checkpoint.
- loop.ts: the outer catch decided "turn/end owed" from `turnStarted`.
A throwing listener on the turn/start append left turn/start logged but
the flag false → catch rethrew and skipped turn/end → permanently open
turn (violating ADR 0017). Now decided from the log (this turn's
turn/start present), so the turn is always balanced; only a genuine
pre-push failure (non-serializable trigger — turn/start never logged) is
rethrown to the runLoop backstop. Removed the now-dead `turnStarted`.
- agent.ts inject(): the idle one-shot-turn flush was gated on a
`turnRecorded` flag set after append('turn/end'); a throwing turn/end
listener skipped the flush, losing the balanced in-memory injection turn
on crash. Now the flush decision is read from the log, the synthetic
turn/end append contains a throwing listener (turn stays balanced), and
a failing idle flush is reported via agent/error (step 0 convention) AND
the logger — mirroring the loop's post-turn/end flush path — with a
throwing agent/error listener contained.
Rewrote the test that encoded the old (buggy) "turn/start listener throw
is rethrown, no turn/end" semantics to assert the balanced-turn contract,
and added regressions for the throwing-turn/end-listener flush and the
agent/error report. Updated Agent.inject JSDoc.
