The ToolCallPresentation / ToolResultPresentation / ToolTerminal shapes grew
incrementally and the responsibility split is now muddy (overlapping call/result
terminal fields, the bridge stitching content + terminal + rawInput per call).
Flag it as a release-blocking FIXME to redesign around a tool's render INTENT
(a tagged union over card kinds) and pin it in an RFC before more tools/UIs
depend on the current bag-of-optionals.
Review nit: the KNOWN RESIDUAL comment named only the [exit code: N] case, but
the same end-of-string spoof applies to [killed by signal: SIG]. Reword to cover
both markers.
Codex + an independent review pass found three real defects in the prior commit:
1. parseExitStatus could misreport a SUCCESSFUL command as a failure: a clean
exit 0 appends no marker, so output ending in "[exit code: 5]" (no trailing
newline) was read as the marker. Anchor the parse to a LEADING newline —
renderResult always inserts one before a real marker, so a body that merely
ends in marker-like text no longer matches. A narrow residual (a clean exit 0
whose final line is exactly the marker) is inherent to the replay-only-sees-
text design and documented; the complete fix (a structured exit on the event)
is the RFC's named escape hatch.
2. A run_in_background start and an isError result were rendered as exited
terminal cards with a false exit-0 pill. A background start returns a task-id
ack (not a streamed terminal) and is no longer marked terminal; an isError
result (spawn failure / abort) carries no exit pill.
3. The terminal capability was re-read live on the result path, so a second
initialize between a call and its result could desync them (orphan
terminal_output or clobbered card). Snapshot the capability per session at
creation (SessionRecord.terminalEnabled) so call and result always agree.
Also reword the reference-parity claim: keeping the description as a content
block in terminal mode is a DELIBERATE divergence (claude-agent-acp drops it).
Tests added for each; with-key e2e still green.
Match claude-agent-acp / codex-acp: the bash tool_call title IS the command
(an execute card hides rawInput), the model description rides as a content
text block above the card, and the completed card carries an exit-status pill
via _meta.terminal_exit.
Bridge fixes found in review of the prior terminal-card commit:
- tool_call_update.content is OMITTED in terminal mode (an ACP update.content
REPLACES the call's content collection in Zed, so the fenced ```console block
would clobber the terminal content block).
- terminal.output preserves RAW newlines (terminal renderers rely on exact
bytes); only the fenced fallback trims trailing blank lines.
- a relative workdir is resolved against the session cwd for the card header,
matching where the command actually ran.
- result-side terminal output is gated on the pending call having registered a
terminal (no orphan _meta.terminal_output for a terminal Zed never made).
The exit pill is recovered by parsing renderResult's status markers (the pure
presentResult seam sees only content blocks); a round-trip test pins the parse
to the marker emission. Neutral ToolTerminal gains exitCode/signal; widened
ToolCallPresentation with a content block. Docs (RFC + 3 READMEs) updated;
with-key e2e verifies the card + exit pill against the real model.
When the client advertises clientCapabilities._meta.terminal_output (Zed), a
bash tool call now renders as a real TERMINAL card — a cwd header + the command
+ its output — instead of the plain ```console text block. Keeps agent-side
dsh-bash execution; rejects the spec's client-side terminal/create (which would
bypass sandbox/env-scrub/ownership/cwd). Matches what claude-agent-acp and
codex-acp do; wire contract verified against Zed's source.
- dsh-tools: a provider-neutral ToolTerminal shape ({ cwd?, output? }) on
ToolCallPresentation/ToolResultPresentation — a tool asks "render me as a
terminal"; no ACP types leak in.
- dsh-tool-bash: bash presentCall marks terminal (cwd from an explicit absolute
workdir, else left for the bridge to fill from the session cwd); presentResult
carries the output alongside the ```console fallback.
- dsh-acp: initialize reads/remembers the _meta.terminal_output capability;
streamSessionEventUpdate maps a terminal presentation to
content:[{type:'terminal',terminalId}] + _meta.terminal_info on the call and
_meta.terminal_output on the update WHEN capable — else the unchanged text
path. terminalId is the callId; cwd defaults to the session header. The pure
translator gained a TerminalRendering {enabled,cwd} param (off by default).
Tests via the REAL tool-bash + bash-local: capability ON -> terminal content +
_meta; OFF -> no _meta (text path). The with-key e2e adds a real-model terminal
card case (echo over ACP with the capability on). 773 tests, 100% coverage.
The exit-status pill (_meta.terminal_exit), live streaming
(_meta.terminal_output_delta), and command classification are RFC follow-ups.
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.
- Fix two root-AGENTS.md cross-links that the depth bump left pointing at the
new docs/AGENTS.md instead of the root file they cite (capability-seams,
optional-code-mode). These resolved on disk so verify-md-links passed — the
gate checks existence, not which file you meant; corrected to ../../../.
- Broaden verify-md-links scope to .agents/skills/**/*.md: this PR rewrote the
dsh-code-review skill's links into the RFC tree, but the skill dir was outside
the gate, so a broken skill link would have passed silently.
- Percent-decode the path component before the existence check, so a valid
encoded relative target (My%20File.md) is not falsely reported broken; a
malformed escape (%zz) is reported broken rather than crashing the gate.
- Drop the merged property-testing RFC's "nightly CI job 100x" claim: that line
came from the original proposal, not the accepted decision, and CI has only
push/pull_request triggers — note it as possible future work instead.
doc-sync (incl. verify-md-links over 58 files), doc-typecheck, lint pass.
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.
Codex round 2 caught that the quality-gates hard blocker omitted the
documented doc-sync gate (and module-graph freshness), so a docs-specific
gate failure could be read as outside the blocking checklist. Add both, and
note doc-sync's coverage so the manual prose-drift check (#1) reads as
additional to it, not a duplicate.
Codex adversarial review flagged that "guidance not authority" over-rotated:
it could be read as licensing a reviewer to waive genuinely-blocking checks
(HMR-safety test, doc-sync, ADR 0007 gates), and the "Where to look first"
section claimed to list things "not in the docs" while its first two items
ARE documented requirements.
Fix both: scope independent judgment to *what to look at* and *how to apply a
rule*, not whether documented requirements hold; state AGENTS.md /
packages/AGENTS.md / ADR 0007 stay authoritative. Split the checks into "Hard
blockers (documented requirements)" and "Reviewer-only checks (judgment
required)", so doc-sync/HMR-safety/gates aren't mixed with reviewer-only
heuristics. Keep the independent/broad-reasoning intent.
The skill read like an authoritative checklist; clearing its items could be
mistaken for a complete review. Reframe it explicitly as guidance — a
where-to-look map that lowers startup cost, not a definition of a sufficient
review. Add a "How to think about a review" section drawing on the patterns
from the /code-review, /review, requesting-code-review, and
receiving-code-review skills: reason from the code independently, sweep all
aspects (correctness, concurrency, security, design/approach, tests, docs,
…), verify before flagging, calibrate confidence and suppress noise, and lead
with severity. Add a receiving-review note so authors evaluate findings on
merit rather than following them blindly.
