The writeText comment still said a null `before` (a create or an
undiffable binary file) means "a consumer renders no result-time diff,
only the call-time whole-file card." That is stale since write's
presentResult renders a whole-file diff for a null before-image. Correct
it: a null `before` gives no contextual-hunk basis, so the consumer falls
back to a whole-file diff — the tool still renders a result diff card, not
the raw result text.
The acp README's presentResult card list still described the `diff` result
as always "the APPLIED hunks computed from before/after". Qualify it like
the other surfaces: typically the applied hunks, or a whole-file diff for a
create, and a successful mutation always returns it so the result text
can't clobber the diff.
The write-diff-card fix made write's presentResult return an args-derived
whole-file diff (oldText:null) for a create / unchanged overwrite, but the
DiffResultView contract and its mirrored docs still said `diffs` is ALWAYS
the applied contextual hunks computed from before/after. Correct the type
JSDoc, the write execute-side comment, and the four mirrored surfaces
(tools.md, tools README, acp-feature-support, adding-a-tool cookbook) to
say: typically the applied hunks, or a whole-file diff when there is no
before-image (a create) — and that a mutation returns the diff result even
when it duplicates the call-time card, since a tool_call_update.content
replace would otherwise clobber the diff with the model-facing text.
Regenerate the cordis catalog (source line shift).
A Write CREATE rendered its completed tool_call_update as the model-facing
result TEXT (`<path>…</path>…Created file`), which — because an ACP
tool_call_update.content REPLACES the call's content — clobbered the
new-file diff the pending call installed. So Zed showed the diff, then
replaced it with raw XML-ish text; only overwrite/edit looked right
(their result re-sends a diff).
write's presentResult now ALWAYS returns a diff card for a successful
write: the applied contextual hunk from `meta` when there is one
(overwrite), else an args-derived whole-file diff (`oldText: null`) for a
create or an unchanged-content overwrite. This matches claude-agent-acp,
where the create diff rides on the update and no result text replaces it.
An error still falls through to generic rendering so its message shows.
edit is unchanged (it always has a hunk; no whole-file fallback).
Re-recorded fs-write / fs-write-overwrite goldens; the create's completed
update is now a {type:'diff'} block, not the XML result text.
Same stale enumeration as the presenter-note fix, in the ToolPresenter
JSDoc: it said the tool/result event "carries only { callId, content,
isError }". The event also carries error and meta; the load-bearing fact
is that it omits the tool name/args (why the presenter remembers them per
callId). State that instead of an exhaustive list that drifts.
The pasted type-equiv block's JSDoc still said the meta payload is
`{@link JsonValue}`; the source comment is `unknown` (the meta channel is
opaque at the seam). verify-type-equiv compares type structure, not the
comment, so the drift slipped through — align the doc comment.
The callId→args map note said the tool/result event "carries only
{ callId, content, isError }" — an exhaustive field list that drifts as
the event grows (it also carries error, and now meta). State the load-
bearing fact instead: the event omits the tool name/args, which is why
the bridge remembers them per callId.
Address the applied-hunk-diffs review:
- CRLF write overwrite emitted bogus every-line-changed hunks: write's
`before` was LF-normalized but `after` kept the raw model content, so a
CRLF rewrite of an LF file diffed every line. Normalize write's `after`
to LF so both sides share the diff basis (edit already did). Regression
test proves it fails on the raw-after path.
- The tool-private `meta` payload is now typed `unknown` (opaque) at every
seam instead of `JsonValue`. This drops the `dsh-tools -> dsh-session`
package edge that existed only to name the type, and removes the
`FileDiff` index signature that had been widening the type solely for
JsonValue-assignability. Serializability is still enforced at runtime by
`Session.append`'s isJsonValue check, which was always the real guard.
- Sync the docs the new result/meta surface left stale: ToolResultView's
diff card + ToolExecutionResult.meta in tools.md/session.md type-equiv
blocks, the acp/tools READMEs, and the adding-a-tool cookbook; regenerate
the cordis catalog and module graph.
A four-way parallel review of every EN<->ZH pair (one reviewer per pair,
findings verified before applying) surfaced:
- terminology: stale-sense 过期 -> 陈旧 in development.zh.md (the table
reserves 过期 for expired); GitHub workflow -> GitHub 工作流 per the
table; dropped the invented 质量 in 'CI gates' -> CI 门禁; taxonomy ->
分类体系 in translation-rules.zh.md; skill first-occurrence annotation
added in the i18n README
- verbatim spans: <current-blob> restored inside an inline code span the
translation had localized
- typography: full-width dashes normalized to no surrounding spaces
across all four files (the rules' own MUST); one 顿号 between clauses
-> comma; 顿号 before 以及 dropped
- fidelity/wording: must-not rendered 不得 (not 不应); local setup ->
本地环境搭建; enforce -> 强制执行; verified surface -> 受验证的范围;
batch-lands-before-neighbors nuance restored; 更新粘贴内容
freshness -> 新鲜度 confirmed against MDN's zh HTTP caching docs (freshness
lifetime -> 新鲜度生命周期); precedent now cited in the table. The same
source pairs stale with 陈旧, not 过期 (过期 maps to expired), so the stale
entry and the i18n README translation now say 陈旧译文.
Every new term the bilingual-docs work introduced, with precedented
renderings where precedent exists:
- orphan -> 孤立 (git's official zh l10n renders orphan as 孤立, e.g.
孤立分支 — not 孤儿; the translations were corrected to match)
- info string -> 信息字符串 (CommonMark zh convention; corrected in the
i18n README translation)
- fenced code block -> 围栏代码块 (MDN zh), staged -> 暂存 (git zh),
event-sourced -> 事件溯源 (DDD convention), smoke test -> 冒烟测试,
fail-fast -> 快速失败, plus fingerprint/pairing/freshness/stale/contract
- mechanism names coined by this repo, marked as such in the notes:
language switcher -> 语言切换行, structural signature -> 结构签名,
enforcement frontier -> 强制边界
- keep-English entries so future translators don't guess: backlog,
blob hash, CI, doc-sync, e2e, monorepo, PR, worktree
fs write/edit now emit a result-time contextual-diff tool_call_update
(the applied hunk with ±3 context lines, one hunk per replace_all site),
matching what claude-agent-acp sends and what makes an editor render the
change in place. The call-time snippet diff stays; the result hunk
supersedes it (ACP content-replace).
Mechanism:
- A persisted tool-private `meta` channel: execute may return
`{ content, meta }`; `meta` (JsonValue) rides on the tool/result event
and is handed back to presentResult, so the diff reproduces on replay
(event-sourced). JsonValue is now exported from dsh-session.
- The backend returns raw before/after text (storage facts) on
FsWriteOutcome/FsEditOutcome; the tool computes the hunk via the npm
`diff` package's structuredPatch. A create has no before → no result
diff; a failed/aborted mutation carries no meta.
- ToolResultView gains a DiffResultView; the bridge's result-side switch
renders it as {type:'diff'} content blocks.
RFC: docs/rfc/implemented/architecture/2026-07-02-result-time-applied-hunk-diffs.md
(justifies the npm `diff` runtime dep over vendoring and the meta channel);
the render-intent-union RFC's Non-goal is updated to record this shipped.
All fs snapshot goldens re-recorded; edit/overwrite gain the contextual
result diff, create/read/policy-reject unchanged in structure.
Review findings addressed:
- The gate compared only heading and code-block COUNTS, understating the
contract it claims to enforce. It now compares ordered structural
signatures: heading depths, fenced code blocks verbatim (info string +
content), table column counts, list kinds, and every link target except
the language switcher. Proven red on a heading demotion, a reworded
code-block comment, and a retargeted link; green on all existing pairs.
- Stated the gate's limit explicitly (header comment + docs/i18n/README.md
both languages): green means fresh and structurally sound, NOT verified —
translation quality is the reviewer's half of the contract.
- first-line extraction no longer silently drops the last character of a
newline-less file (split with limit instead of indexOf slice).
- isExcluded documents the trailing-slash-is-the-boundary invariant.
- Rollout guidance: grow the required frontier at the pace translation
review is resourced.
- dsh-code-review's doc-sync sublist is now the exhaustive chain.
docs/i18n/README.zh.md updated via the minimal-diff workflow and
re-fingerprinted.
The numResults (exa) and searchRecency (perplexity) conditional spreads
in apply() were only exercised on their absent side, leaving the 100%
per-file branch gate red. Add plugin-registration tests that pass those
config fields and assert they reach the request body.
First backlog item translated with the dsh-translate-docs skill: full-file
translation, terminology per docs/i18n/terminology.md, structure locked to
the source (11 headings, 10 byte-identical code blocks), fingerprinted and
added to the manifest's required list.
The Exa and Perplexity providers hard-coded request parameters that
deployments should control while defaults are still unsettled. Exa gains
searchType, numResults, and highlightsPerResult; Perplexity gains
maxTokens (it previously sent none) and an optional searchRecency. Each
follows the deepseek provider's shape: a defaulted Config field, a
DEFAULT_* constant, and a positive-integer status() check for numeric
limits. The call-level maxResults still flows through WebSearchRequest
and wins over the configured default, keeping the seam layering intact.
Addresses tianyicui's "make everything configurable" review comment.
The closed WebErrorCode union leaked fetch-transport details (redirect,
too-large, content-type) into the seam's shared vocabulary and made web
the only seam with a closed error-code union. Drop it and let WebError
carry an open code: string like LlmError/SubagentError; document the
codes grouped by owner (seam-neutral vs dsh-web-fetch-local transport).
Addresses tianyicui's leaky-abstraction review comment on WebErrorCode.
Establish EN->ZH bilingual documentation for the README and docs tree:
- docs/i18n/README.md — the pairing contract: sibling foo.md <-> foo.zh.md,
English canonical, blob-hash source fingerprints, language switchers,
scope/exclusions, and a manifest-driven rollout ratchet.
- docs/i18n/translation-rules.md — how to translate: faithfulness, structure
preservation, terminology discipline over docs/i18n/terminology.md, and
typography rules grounded in MDN/K8s/Vue/clreq conventions.
- .agents/skills/dsh-translate-docs — the committed agent workflow, following
the dsh-code-review pattern of deferring to docs as sources of truth.
- scripts/verify-translation-pairing.ts + manifest — a doc-sync gate: required
pairs exist; every existing .zh.md is fresh (fingerprint = current source
blob), switcher-linked, structure-matched, and non-orphaned; excluded
(generated) docs stay unpaired. --list prints the translation work list.
- RFC (implemented/process) recording the decision and the alternatives.
- Dogfood: README.zh.md and the two i18n docs translated under their own rules.
Gates: doc-sync green including the new gate; red/green proven for stale
fingerprint, orphan, and excluded-file violations.
Use the new filesystem seam for skill file reads and system skill writes when ctx.fs is available, and include the skill tool in the generated tool catalog.
Address the render-intent-union review:
- toolResultUpdate branched on `if (card === 'terminal')` with a generic
fallthrough; ToolResultView is a closed union, so make it an exhaustive
`switch (view.card)` ending in assertNever (matching the call-side
renderer and the § Conventions closed-union rule). Adding a result card
later now fails to compile at the switch. Regression test: a rogue result
card throws.
- displayTitle's `rel.startsWith('..')` guard mis-rejected an in-workspace
target whose relative form merely begins with the chars `..` (e.g.
`..cache/x`, a real sibling name), leaving its title absolute. Test for a
`..` SEGMENT (`..` alone or `..<sep>…`) so such paths relativize, matching
claude-agent-acp's `cwd + sep` prefix check. Regression test added.
Replace the "bag of optional fields" tool-presentation types
(ToolCallPresentation / ToolResultPresentation / ToolTerminal) with a
card-tagged discriminated union — the standing FIXME(tool-presentation).
A tool declares one render intent per call/result and the ACP bridge
switches on `card`:
ToolCallView = generic | terminal | diff
ToolResultView = generic | terminal
The `diff` card is new: fs write/edit now emit an ACP {type:'diff'}
content block (an editor's inline diff), which the old shapes could not
express. The bridge also relativizes a file card's title against the
session cwd (mirroring claude-agent-acp's toDisplayPath) while keeping
locations/diff paths raw, and derives the no-capability fenced console
fallback from a terminal result's output. read gains the window-in-title
(`Read foo.txt (5 - 8)`) and an always-set location line, matching the
reference adapter field-for-field.
Migrates all three producer families (tool-fs, tool-bash, tool-todo) and
the sole consumer (the ACP bridge) together — the source does not compile
piecewise. Adds snapshot coverage for the terminal _meta path (a new
capability-advertising scenario) and re-records the fs goldens to show the
diff cards. Applied-hunk (result-time, context-line) diffs need a new
result/event shape and are a follow-up.
RFC: docs/rfc/implemented/architecture/2026-07-02-tool-render-intent-union.md
Codex convergence verified the retrospective's claims against the code and
caught two inaccuracies (everything else — the spawn-overload claim, create()
dropping meta, ask-codex invocation, test:coverage guidance, waitFor, markdown
— checked out):
- The meta-threading factory `ctx.agents.create()` (createAgent) is
SYNCHRONOUS, not "the async factory path"; the async one is `resume` (which
reloads the persisted header). Reworded.
- The generated-artifact bullet conflated triggers: gen-cordis-catalog reads
`interface Events`/`Context` member JSDoc (not top module docs);
gen-module-graph is driven by package peerDependencies (not event/doc edits);
and module-graph freshness is `verify-module-graph`, a SEPARATE gate from
`doc-sync`. Split the guidance per artifact.
Retrospective on the whole stacked-PR effort. Adds a "Landing changes cleanly:
gates, Codex, and scope" section capturing the workflow lessons the effort
surfaced, and two codebase-specific traps to Defensive patterns.
The through-line: a mechanical gate proves lines ran and types check, never
that a test guards anything or that prose is accurate — so layer the human/AI
judgment on top, in order, and keep each unit honestly scoped:
- prove every regression test RED on the unfixed code (top-billed, not a nit)
- run the FULL test:coverage, not an isolated -t filter (test-isolation bugs)
- spend Codex on what gates can't see (prose/RFC/comment drift, self-introduced
fix bugs), scoped to ONE concern per review (a two-fix prompt timed out)
- a mid-review cleanup that exceeds the reviewed RFC scope goes in a NEW stacked
PR; enumerate consumers + grill before deleting a seam
- regenerate a generated artifact as part of the invalidating edit, not as a
gate to fail; lint:fix before hand-fixing
- read a failure before reacting: ENOSPC watcher exhaustion is environmental,
not a code regression
Defensive-patterns additions (both bit us this cycle): spawn narrows non-null
stdout/stderr only from a literal stdio tuple; AgentLoop.create() drops
options.meta (only the async factory threads it).
Follow-up to the README fix: the dsh-agent README's emit section had lost its
streaming entry, leaving a "Streaming + tool" heading with no pointer to where
the token stream now lives. Rename the section to "Live control notifications"
and add a line directing readers to the durable `session/event` `assistant/chunk`
feed.
Codex convergence caught stale prose the doc-sync gate does not scan: the
dsh-agent README still listed `agent/stream-chunk` as an emit and the
dsh-agent-loop README still told UI plugins to listen on it. A plugin author
following either would silently get no chunks. Point both at the
`session/event` `assistant/chunk` feed (the agent-loop UI line also names the
`agent/*` control events a UI still uses).
The loop recorded every model token delta as a durable `assistant/chunk`
session event AND emitted an identical live `agent/stream-chunk` Cordis event
one line later. Same StreamChunk, same turn/step; the emit added only the live
Agent handle, which the sole consumer discarded. This is the boundary-mirror
duplication the event-domain work removed for turn/step boundaries, applied to
the token stream — a follow-up the boundary RFC explicitly deferred.
The premise is settled: chunk persistence is authoritative (the proposal to
stop persisting chunks was rejected — replay/snapshots depend on it), so
`assistant/chunk` on `session/event` is the load-bearing token stream and
`agent/stream-chunk` is pure redundancy.
- Remove the `agent/stream-chunk` declaration + emit; drop the now-unused
StreamChunk import from dsh-agent's types.
- Migrate `dsh-ui-stdio` (the only live consumer; ACP already reads
assistant/chunk off session/event) to render assistant/chunk in its existing
session/event listener. Consolidating to one listener also makes the
inReasoning dim-SGR flag deterministic across chunk/boundary events (they no
longer race across two listeners).
- Repoint the agent-loop tests (cancel/loop) and ui-stdio tests to the
session/event assistant/chunk feed.
- New RFC (implemented/simplification/2026-07-02-remove-stream-chunk-mirror);
amend the boundary RFC's retained-list entry to cross-link; update
architecture, cookbook, event-domain-semantics, the ACP proposal, and the
regenerated cordis catalog.
Snapshot goldens unchanged (ACP never used the mirror), confirming no
editor-facing transcript change.
The fs-policy gate throws FS_NOT_OBSERVED when the model edits a file it
never read; that rejection surfaces as a failed tool_call_update, but no
snapshot pinned it — a regression that dropped or mis-rendered the failed
card would pass every gate. Record a scenario that edits a seeded file
without a preceding read: the edit is vetoed, the file stays unchanged on
disk, and the transcript shows the pending edit card followed by a
status:'failed' update carrying the policy error.
The new packages/fs/tool-fs/tests/*.e2e.ts (+ its harness.ts) need an explicit
knip workspace entry — mirroring the other e2e-bearing packages — so knip
follows them and does not flag the files or their dsh-agent-loop/dsh-llm-deepseek
devDeps as unused.
Behavioral gaps from the coverage audit (line coverage was already 100%; these
close BEHAVIOR gaps):
- fs-local: service-level writeText/editText pre-abort → FS_ABORTED (file
unchanged); concurrent guarded-write race and mixed write-vs-edit race (one
wins, one FS_STALE_VERSION, locks released); edit→edit version refresh at the
provider; the replaceIfVersion post-write version matches a fresh stat. fsio:
a mid-stream abort → FS_ABORTED (previously only pre-abort was covered).
- fs-policy: the agent-without-session owner rung ({agent:{}} → no owner →
createIfAbsent / FS_NOT_OBSERVED); fs/write-intent first-wins (symmetric to
the existing edit-intent test).
- tool-fs: abort-through-the-tool for read/write/edit (isError FS_ABORTED, file
unchanged); a deterministic tool-tier concurrent-edit race via a shared read;
the throwing-fs/observed contract (a throwing listener surfaces as isError but
the mutation already hit disk); the replace_all edit message; parseReadArgs
rejects fractional/NaN offset and zero/negative limit.
- dsh-fs: FsError chains a cause through ErrorOptions.
New with-key e2e (packages/fs/tool-fs/tests/fs-tools.e2e.ts, self-skips without
DEEPSEEK_API_KEY): a real model drives the real read/write/edit tools to create
→ read → edit a file, verified on disk; a second test proves a relative path
resolves against the per-session cwd (factory meta.cwd) not config.cwd. Booted
via a plain tests/harness.ts. Added dsh-agent-loop + dsh-llm-deepseek devDeps.
Five recorded ACP snapshot scenarios exercising read/write/edit end-to-end
through the real acp-agent subprocess, replayed keyless in CI:
- fs-read — read a seeded file (read tool + presentation + observed-state)
- fs-write — create a file (write, no prior version guard)
- fs-edit — read then literal-replace (read-before-edit authorization)
- fs-write-overwrite — read then rewrite (replaceIfVersion after a read)
- fs-read-window — read lines 5-8 with offset/limit (windowing + the offset
surfaced as the tool_call location line)
The goldens confirm the tools render with their new presentation — Read/Write/
Edit <path> titles, read/edit kinds, and `locations` (fs-read-window carries
`{path, line:5}`) — and that the prompts steered the model to the fs tools, not
bash (zero bash calls in any golden). Recorded against the real API, filtered to
the new scenarios so no existing fixture churned.
Codex convergence findings on the delegate-and-fold fix (code path verified
correct, prose only):
- The hook-bridges RFC claimed a downstream `block` "carries the bridge context
too" for BOTH seams. True for `tools/post-execute` (PostToolDecision.block has
an additionalContext field) but false for `agent/prompt-submit`
(PromptDecision.block is `{kind,reason}` with no context field). The code is
already correct — a blocked prompt drops the context, which is right since the
prompt never reaches the model. Reworded the RFC to state the per-seam
difference accurately.
- Two test comments narrated "Before the fix…", which the current-state-only
doc rule forbids. Reworded to describe the behavior, not its history.
- Documented on concatContext (both bridges) why the merged block carries a
single source: a HookContext holds one MessageSource and the seam cannot
represent mixed provenance; rendering distinguishes only by source.kind, so a
downstream plugin's text stays framed as plugin context.
Load dsh-fs-local + dsh-fs-policy + dsh-tool-fs after tool-todo (mirroring the
acp-agent wiring), and steer the system prompt to prefer read/write/edit for
file ops with bash for shell/tests/search. Update the welcome line and the
FIXME(config-comments) bash note.
Doc sweep now that both demos ship the fs tools and the seam resolves per-session
cwd: architecture.md and the event-gate RFC no longer say the demos do file ops
through bash / that no config wires the tools; the coding-agent + examples
READMEs and the AGENTS.md layout blurb list the fs tools; the acp-agent README
drops the launch-dir caveat (per-session cwd now works, so the server can launch
anywhere).
(stdio-agent is single-session, so fs-local's cwd = process.cwd() is the
workspace. Keyless boot smoke is blocked locally by an unrelated inotify
watcher-limit ENOSPC that also hits demo:echo; the config parses and the same fs
stack boots green in the acp-agent snapshot tier.)
The fs tools rendered as generic cards (title = tool name, raw file content) in
an ACP editor. Give them tool-owned presentation like bash/subagent have:
- read → title "Read <path>", kind read, offset/limit as rawInput
- write → title "Write <path>", kind edit
- edit → title "Edit <path>", kind edit, a clipped old→new rawInput summary
Add a provider-neutral `locations: { path, line? }[]` to ToolCallPresentation —
the files a call reads/modifies — so a capable editor can follow along / jump to
the file (read carries its offset as the line). The ACP bridge forwards it onto
the wire `tool_call` (ResolvedCallPresentation + call() + the tool_call build in
streamSessionEventUpdate). This flips the `locations` cell in the ACP feature
matrix to supported. The SDK already carries `tool_call.locations`
(ToolCallLocation `{ path, line? }`), so no ACP types leak into dsh-tools.
presentResult is intentionally omitted: it only receives `{ content, isError }`,
not the write/edit outcome, so titling by create-vs-overwrite or replacement
count would mean parsing the model-facing text — the static title stays.
Tests: pure presentCall assertions for all three tools incl. locations and the
edit rawInput clip; a bridge test drives the REAL fs tools through ToolPresenter
and asserts locations reaches the wire tool_call (proven to fail without the
forwarding line). New withFs harness option + dsh-fs devDeps on dsh-acp.
Tianyi Cui
Ziya
Dudu-0223
Yichen Jiang