diff --git a/docs/cookbook/adding-a-tool.md b/docs/cookbook/adding-a-tool.md index c5e7931871..84247e3f3d 100644 --- a/docs/cookbook/adding-a-tool.md +++ b/docs/cookbook/adding-a-tool.md @@ -36,6 +36,7 @@ Registration is effect-based: disposing the plugin fiber unregisters the tool (w - **Args are validated for you.** `defineTool` validates the model-generated `arguments` against the `SchemaSpec` before `execute` runs (type, required keys, enum membership, nested objects/arrays — [runtime arg validation](../rfc/implemented/architecture/2026-06-11-runtime-arg-validation.md)), so inside `execute` the args already match `InferArgs`. You still hand-check value constraints the DSL can't express (non-empty strings, positive numbers, cross-field rules); throw a descriptive Error for those. Raw JSON-Schema tools registered directly (MCP) are NOT validated by the harness — they validate their own input. - **Throwing means isError.** The registry catches anything `execute()` throws and returns `{isError: true}` to the model. Use that for infrastructure failures (bad input, spawn errors, aborts) — but REPORT domain failures in the result text instead (e.g. tool-bash returns `[exit code: 9]` with `isError: false`: the model decides what a failing command means). - **Honor `exec.signal`.** Cancel in-flight work when it fires. +- **Attach durable card data with `meta` (optional).** `execute` may return `{ content, meta }` instead of a bare `ContentBlock[]` — `meta` is a JSON-serializable payload the core treats as opaque, persisted on the `tool/result` event and handed back to your `presentResult` (so a card that needs more than `args`, like `write`/`edit`'s applied-hunk diff, survives a session replay). Keep UI-only data here, never in the model-facing `content`. - **Use `exec.agent` for async notifications.** `agent.inject(content, {source: {kind: 'plugin', plugin: ''}})` appends durable context the NEXT model request sees — it is not a wake-up (an idle agent stays idle). Guard against disposed agents (try/catch). ## Long-running work @@ -58,7 +59,7 @@ Both methods return a **`card`-tagged render intent** — pick the card kind tha - `{ card: 'generic', title, kind?, rawInput?, content?, locations? }` — the default. Set `kind` for an icon (`read`/`search`/…); set `locations: [{ path, line? }]` for any file your tool touches so a capable editor follows along / jumps to it. - `{ card: 'terminal', title, description?, cwd? }` — your call IS a shell command. `title` is the command, `description` renders above the terminal card. (tool-bash.) - `{ card: 'diff', title, diffs, locations? }` — your call creates or modifies a file. `diffs: [{ path, oldText, newText }]` (`oldText: null` for a new file) renders as an inline diff card. (tool-fs `write`/`edit`.) -- `presentResult(args, { content, isError })` → a `ToolResultView` (the COMPLETED card): `{ card: 'generic', title?, content? }` or `{ card: 'terminal', title?, output?, exitCode?, signal? }` (the run's captured output + exit — the bridge shows an exit pill and derives a fenced ` ```console ` fallback for editors without the terminal capability). +- `presentResult(args, { content, isError, meta? })` → a `ToolResultView` (the COMPLETED card): `{ card: 'generic', title?, content? }`, `{ card: 'terminal', title?, output?, exitCode?, signal? }` (the run's captured output + exit — the bridge shows an exit pill and derives a fenced ` ```console ` fallback for editors without the terminal capability), or `{ card: 'diff', title?, diffs }` (the APPLIED hunks of a completed file mutation, computed from the before/after content — `write`/`edit` attach the hunks via the `meta` channel and read them back here). `result.meta` is your tool's own optional presentation payload, attached from `execute` (see below) and persisted so a replay reproduces the card. Hard rules (they bite if broken): diff --git a/docs/cordis-catalog/events-and-services.md b/docs/cordis-catalog/events-and-services.md index 90419feeb9..6854e56e2e 100644 --- a/docs/cordis-catalog/events-and-services.md +++ b/docs/cordis-catalog/events-and-services.md @@ -337,7 +337,7 @@ A tool was registered or unregistered (the available tool set changed). 'tools/change'(): void ``` -Source: [`packages/core/tools/src/index.ts:49`](../../packages/core/tools/src/index.ts) +Source: [`packages/core/tools/src/index.ts:48`](../../packages/core/tools/src/index.ts) #### `tools/execute` — waterfall @@ -349,7 +349,7 @@ Waterfall around every tool execution — the single seam where sandbox, permiss Types: [ToolExecution](../core-data-structures/tools.md) · [ToolExecutionResult](../core-data-structures/tools.md) -Source: [`packages/core/tools/src/index.ts:44`](../../packages/core/tools/src/index.ts) +Source: [`packages/core/tools/src/index.ts:43`](../../packages/core/tools/src/index.ts) ## Services @@ -547,7 +547,7 @@ async execute(exec: ToolExecution): Promise Types: [ToolDefinition](../core-data-structures/tools.md) · [ToolExecution](../core-data-structures/tools.md) · [ToolExecutionResult](../core-data-structures/tools.md) -Source: [`packages/core/tools/src/index.ts:370`](../../packages/core/tools/src/index.ts) +Source: [`packages/core/tools/src/index.ts:363`](../../packages/core/tools/src/index.ts) ## Inherited tier (cordis core + loader/hmr/timer) diff --git a/docs/core-data-structures/session.md b/docs/core-data-structures/session.md index 789890680f..41cad4660d 100644 --- a/docs/core-data-structures/session.md +++ b/docs/core-data-structures/session.md @@ -32,7 +32,7 @@ interface SessionEventMap { */ 'assistant/message': { turn: number; step: number; content: ContentBlock[]; usage?: TokenUsage } 'tool/call': { turn: number; step: number; callId: CallId; name: string; arguments: string } - 'tool/result': { turn: number; step: number; callId: CallId; content: ContentBlock[]; isError: boolean; error?: { name: string; code: string }; meta?: JsonValue } + 'tool/result': { turn: number; step: number; callId: CallId; content: ContentBlock[]; isError: boolean; error?: { name: string; code: string }; meta?: unknown } /** Steering content injected between steps of a running turn. */ 'steering/message': { turn: number; content: ContentBlock[]; source: MessageSource } /** diff --git a/docs/core-data-structures/tools.md b/docs/core-data-structures/tools.md index ac765d25cd..4f9be38e21 100644 --- a/docs/core-data-structures/tools.md +++ b/docs/core-data-structures/tools.md @@ -106,7 +106,7 @@ interface ToolExecutionResult { * {@link ToolResult} for `presentResult`. Opaque {@link JsonValue}; absent when * the tool attached none or the call failed. */ - meta?: JsonValue + meta?: unknown } ``` @@ -117,7 +117,7 @@ A waterfall listener receives `(exec, next)`: call `next()` to proceed (possibly How a tool wants its call shown in a UI (an editor tool-call card, a CLI log line), provider-neutral so a tool describes itself without depending on any client protocol. `presentCall`/`presentResult` return a **`card`-tagged render intent** — a discriminated union a UI bridge switches on: - `ToolCallView` (pending): `{ card: 'generic', title, kind?, rawInput?, content?, locations? }` (the default card; `locations` is `{ path, line? }[]` files the call reads/modifies, for editor follow-along), `{ card: 'terminal', title, description?, cwd? }` (a shell command → a terminal card), or `{ card: 'diff', title, diffs, locations? }` (a file create/modify → an inline diff card; `diffs` is `{ path, oldText, newText }[]`, `oldText: null` for a new file). -- `ToolResultView` (completed): `{ card: 'generic', title?, content? }` or `{ card: 'terminal', title?, output?, exitCode?, signal? }` (the captured run output + exit; a capable UI shows an exit-status pill, an incapable one gets a fenced ` ```console ` fallback the bridge derives from `output`). +- `ToolResultView` (completed): `{ card: 'generic', title?, content? }`, `{ card: 'terminal', title?, output?, exitCode?, signal? }` (the captured run output + exit; a capable UI shows an exit-status pill, an incapable one gets a fenced ` ```console ` fallback the bridge derives from `output`), or `{ card: 'diff', title?, diffs }` (a completed file mutation → the APPLIED hunks with context lines, one entry per changed site, computed from the before/after file content — distinct from the call-time whole-snippet `diff`, which it supersedes). `ToolCallKind` (`'read' | 'edit' | 'delete' | 'move' | 'search' | 'execute' | 'fetch' | 'other'`) picks an icon on a generic card. `FileLocation` (`{ path, line? }`) and `FileDiff` (`{ path, oldText, newText }`) are the shared file-card vocabulary. The design is pinned in [the render-intent-union RFC](../rfc/implemented/architecture/2026-07-02-tool-render-intent-union.md); the ACP bridge maps a `diff` card to a `{ type: 'diff' }` content block, a `terminal` card to the `_meta` terminal convention, and relativizes a file card's title against the session cwd. diff --git a/docs/module-graph.md b/docs/module-graph.md index d09fd76eaf..8de73f8d80 100644 --- a/docs/module-graph.md +++ b/docs/module-graph.md @@ -40,7 +40,6 @@ graph TD session-persistence-sqlite --> session-persistence tools --> agent tools --> llm - tools --> session tools --> system-prompt ui-stdio --> agent ui-stdio --> llm @@ -130,7 +129,7 @@ graph TD | `invariants` | `agent`, `llm`, `session` | | `session-persistence-jsonl` | `session`, `session-persistence` | | `session-persistence-sqlite` | `session`, `session-persistence` | -| `tools` | `agent`, `llm`, `session`, `system-prompt` | +| `tools` | `agent`, `llm`, `system-prompt` | | `ui-stdio` | `agent`, `llm`, `session` | | `acp` | `agent`, `llm`, `session`, `session-persistence`, `tools` | | `agent-loop` | `agent`, `llm`, `session`, `session-persistence`, `system-prompt`, `tools` | diff --git a/docs/rfc/implemented/architecture/2026-07-02-result-time-applied-hunk-diffs.md b/docs/rfc/implemented/architecture/2026-07-02-result-time-applied-hunk-diffs.md index 591dbb4779..20328041f0 100644 --- a/docs/rfc/implemented/architecture/2026-07-02-result-time-applied-hunk-diffs.md +++ b/docs/rfc/implemented/architecture/2026-07-02-result-time-applied-hunk-diffs.md @@ -16,13 +16,13 @@ Add a **persisted, tool-private presentation channel** so a tool's `execute` can ### 1. A `meta` channel on the tool result (core) -`ToolDefinition.execute` may now return either its model-facing `ContentBlock[]` (unchanged, the common case) OR `{ content: ContentBlock[]; meta?: JsonValue }`: +`ToolDefinition.execute` may now return either its model-facing `ContentBlock[]` (unchanged, the common case) OR `{ content: ContentBlock[]; meta?: unknown }`: ```ts ignore-check -type ToolExecuteReturn = ContentBlock[] | { content: ContentBlock[]; meta?: JsonValue } +type ToolExecuteReturn = ContentBlock[] | { content: ContentBlock[]; meta?: unknown } ``` -`meta` is an opaque, JSON-serializable payload the core never interprets. The registry threads it onto the `tool/result` **session event** (`{ …, meta?: JsonValue }`), so it is persisted with the log; on replay the same `meta` is read back and handed to `presentResult` via a widened `ToolResult` (`{ content, isError, meta? }`). Because the payload lives in the event log, the diff reproduces on session reload / snapshot replay **for free** — the event-sourcing guarantee, not a re-computation. `JsonValue` is exported from `dsh-session` (paired with the existing `isJsonValue` predicate that already gates every event's serializability at `append`). +`meta` is an opaque payload the core never interprets — typed `unknown` at every seam (the tool that produced it owns and narrows its shape). It MUST be JSON-serializable: the registry threads it onto the `tool/result` **session event**, and `Session.append` runtime-validates all event data with the existing `isJsonValue` predicate, so a non-serializable `meta` is rejected at the source. On replay the same `meta` is read back and handed to `presentResult` via a widened `ToolResult` (`{ content, isError, meta? }`). Because the payload lives in the event log, the diff reproduces on session reload / snapshot replay **for free** — the event-sourcing guarantee, not a re-computation. Typing `meta` as `unknown` (rather than a shared serializable-value type) keeps the tools core free of a dependency it would otherwise take just to name the type, and the runtime `isJsonValue` gate — not the static type — is what actually enforces serializability. This is the general shape ("a tool attaches durable result presentation"), not an fs-specific one — any tool can use it. @@ -39,7 +39,7 @@ Per the [capability-seam split](2026-06-13-capability-seams.md), the storage bac ### The diff algorithm — a third-party runtime dependency over vendoring -Computing hunks-with-context is a solved problem with sharp edge cases (grouping, context coalescing, the trailing-newline marker). Rather than hand-roll it, `dsh-tool-fs` takes a runtime dependency on the npm [`diff`](https://www.npmjs.com/package/diff) package (v9, ships its own types) and uses its `structuredPatch`. The repo's default is to vendor Cordis-framework source, but that policy is about the *framework*; a leaf tool package taking a small, well-known, self-typed utility dependency is the same shape as `dsh-acp` depending on `@agentclientprotocol/sdk`. Vendoring a diff algorithm would be re-implementing a battle-tested one for no benefit — the [pre-release "foundation over blast radius"](../../../../AGENTS.md) reasoning does not argue for re-deriving standard algorithms. The dependency is pinned and its output is normalized in one small module (`packages/fs/tool-fs/src/diff.ts`). +Computing hunks-with-context is a solved problem with sharp edge cases (grouping, context coalescing, the trailing-newline marker). Rather than hand-roll it, `dsh-tool-fs` takes a runtime dependency on the npm [`diff`](https://www.npmjs.com/package/diff) package (a `^9.0.0` range, exact-pinned by the lockfile; it ships its own types) and uses its `structuredPatch`. The repo's default is to vendor Cordis-framework source, but that policy is about the *framework*; a leaf tool package taking a small, well-known, self-typed utility dependency is the same shape as `dsh-acp` depending on `@agentclientprotocol/sdk`. Vendoring a diff algorithm would be re-implementing a battle-tested one for no benefit — the [pre-release "foundation over blast radius"](../../../../AGENTS.md) reasoning does not argue for re-deriving standard algorithms. The dependency's output is normalized in one small module (`packages/fs/tool-fs/src/diff.ts`). ## Non-goals diff --git a/packages/core/session/src/types.ts b/packages/core/session/src/types.ts index 6a9eaa0fc1..62c3a33e49 100644 --- a/packages/core/session/src/types.ts +++ b/packages/core/session/src/types.ts @@ -1,6 +1,5 @@ import type { Branded } from '@deepseek-ai/dsh-brand' import type { CallId, ContentBlock, MessageSource, StreamChunk, TokenUsage } from '@deepseek-ai/dsh-llm' -import type { JsonValue } from './json.ts' /** Identifies one session in the store (and its persistence artifacts). */ export type SessionId = Branded<'SessionId'> @@ -213,14 +212,14 @@ export interface SessionEventMap { 'tool/call': { turn: number; step: number; callId: CallId; name: string; arguments: string } /** * A completed tool call's model-facing result, plus an optional tool-private - * `meta` presentation payload. `meta` is opaque to the core — the producing - * tool owns its shape and reads it back in `presentResult` — and is a - * {@link JsonValue} so it persists in the durable log and reproduces on replay - * (a UI bridge renders the identical card from a loaded session). Absent unless - * the tool attaches one (e.g. `dsh-tool-fs` carries its result-time contextual - * diff here). + * `meta` presentation payload. `meta` is opaque to the core (`unknown` — the + * producing tool owns its shape and reads it back in `presentResult`) but MUST + * be JSON-serializable: `Session.append` runtime-validates all event data with + * `isJsonValue`, so a non-serializable `meta` is rejected at the source, and the + * durable log reproduces the identical card on replay. Absent unless the tool + * attaches one (e.g. `dsh-tool-fs` carries its result-time contextual diff here). */ - 'tool/result': { turn: number; step: number; callId: CallId; content: ContentBlock[]; isError: boolean; error?: { name: string; code: string }; meta?: JsonValue } + 'tool/result': { turn: number; step: number; callId: CallId; content: ContentBlock[]; isError: boolean; error?: { name: string; code: string }; meta?: unknown } /** Steering content injected between steps of a running turn. */ 'steering/message': { turn: number; content: ContentBlock[]; source: MessageSource } /** diff --git a/packages/core/tools/README.md b/packages/core/tools/README.md index f6694269ea..c8a3ddba4b 100644 --- a/packages/core/tools/README.md +++ b/packages/core/tools/README.md @@ -26,7 +26,7 @@ Tool registry and execution waterfall. Tool plugins register their schemas and e - `ToolDefinition` — `ToolSchema` + `execute(args, exec): Promise` (the bare array is the model-facing content; the object form additionally attaches an opaque, JSON-serializable `meta` presentation payload persisted on the `tool/result` event and handed back to `presentResult`), plus optional `presentCall(args)` / `presentResult(args, result)` for tool-owned UI presentation (see below). - `ToolExecution` — one pending tool call: `{ callId, name, arguments, agent?, signal? }`. -- `ToolExecutionResult` — outcome: `{ callId, content, isError, error? }`. On failure with a `HarnessError`, `error: { name, code }` carries the structured failure class alongside the model-facing text (the loop forwards it onto the `tool/result` session event for retry/sandbox plugins and replay). +- `ToolExecutionResult` — outcome: `{ callId, content, isError, error?, meta? }`. On failure with a `HarnessError`, `error: { name, code }` carries the structured failure class alongside the model-facing text. `meta` is the tool's opaque presentation payload from a successful `execute` (the object return form); the loop forwards both `error` and `meta` onto the `tool/result` session event (for retry/sandbox plugins, replay, and result-card rendering). - `ToolCallView` / `ToolResultView` — provider-neutral `card`-tagged render intents a tool returns from `presentCall` / `presentResult` to own how a UI renders ITS calls (see "Tool-owned UI presentation"). ### Extension points @@ -81,7 +81,7 @@ A tool owns how ITS calls render in a UI (an editor's tool-call card, a CLI log - `{ card: 'terminal', title?, output?, exitCode?, signal? }` — a terminal run's captured `output` and exit status. A capable UI shows an exit-status pill; an incapable UI gets a fenced ` ```console ` fallback the BRIDGE derives from `output` (the tool does not encode the fences). - `{ card: 'diff', title?, diffs }` — a completed file mutation as an inline diff. `diffs` is `FileDiff[]` — the APPLIED hunks with surrounding context (one entry per changed site), computed from the before/after file content, distinct from the call-time whole-snippet `diff`. Used by `write`/`edit`; a `tool_call_update.content` replaces the call's content, so this supersedes the pending snippet. -Returning `undefined` (or omitting a method) tells a UI to fall back to a generic presentation (title = tool name, raw args as input, raw result content). Both methods must be **pure and side-effect-free**: a UI may call them during live streaming AND during a session-log replay, so they depend only on their arguments. `result.meta` is the tool's own optional presentation payload (`JsonValue`), attached by `execute` (see below) and persisted on the `tool/result` event, so a `presentResult` reading it stays replay-deterministic (the same `meta` is read back from the log). With `defineTool`, `args` is the typed `InferArgs` shape; the helper soft-validates before calling (a malformed/older logged arg shape yields `undefined` rather than throwing, since display must never crash a replay). The views are provider-neutral — the ACP bridge (`dsh-acp`) maps each `card` to ACP `tool_call`/`tool_call_update` wire fields (a `diff` card to a `{ type: 'diff' }` content block, a `terminal` card to the `_meta` terminal convention), and relativizes a file card's title against the session cwd. See the render-intent-union RFC (`docs/rfc/implemented/architecture/2026-07-02-tool-render-intent-union.md`) and the applied-hunk-diffs RFC (`docs/rfc/implemented/architecture/2026-07-02-result-time-applied-hunk-diffs.md`); `dsh-tool-bash` (terminal) and `dsh-tool-fs` (diff/generic) are the reference implementations. +Returning `undefined` (or omitting a method) tells a UI to fall back to a generic presentation (title = tool name, raw args as input, raw result content). Both methods must be **pure and side-effect-free**: a UI may call them during live streaming AND during a session-log replay, so they depend only on their arguments. `result.meta` is the tool's own optional presentation payload (opaque `unknown`, JSON-serializable), attached by `execute` (see below) and persisted on the `tool/result` event, so a `presentResult` reading it stays replay-deterministic (the same `meta` is read back from the log). With `defineTool`, `args` is the typed `InferArgs` shape; the helper soft-validates before calling (a malformed/older logged arg shape yields `undefined` rather than throwing, since display must never crash a replay). The views are provider-neutral — the ACP bridge (`dsh-acp`) maps each `card` to ACP `tool_call`/`tool_call_update` wire fields (a `diff` card to a `{ type: 'diff' }` content block, a `terminal` card to the `_meta` terminal convention), and relativizes a file card's title against the session cwd. See the render-intent-union RFC (`docs/rfc/implemented/architecture/2026-07-02-tool-render-intent-union.md`) and the applied-hunk-diffs RFC (`docs/rfc/implemented/architecture/2026-07-02-result-time-applied-hunk-diffs.md`); `dsh-tool-bash` (terminal) and `dsh-tool-fs` (diff/generic) are the reference implementations. ```ts import { defineTool } from '@deepseek-ai/dsh-tools' diff --git a/packages/core/tools/package.json b/packages/core/tools/package.json index 05394ea118..a6d3bbe0ca 100644 --- a/packages/core/tools/package.json +++ b/packages/core/tools/package.json @@ -24,14 +24,12 @@ "peerDependencies": { "@deepseek-ai/dsh-agent": "^0.0.1", "@deepseek-ai/dsh-llm": "^0.0.1", - "@deepseek-ai/dsh-session": "^0.0.1", "@deepseek-ai/dsh-system-prompt": "^0.0.1", "cordis": "^4.0.0-rc.6" }, "devDependencies": { "@deepseek-ai/dsh-agent": "workspace:^", "@deepseek-ai/dsh-llm": "workspace:^", - "@deepseek-ai/dsh-session": "workspace:^", "@deepseek-ai/dsh-system-prompt": "workspace:^", "cordis": "^4.0.0-rc.6" } diff --git a/packages/core/tools/src/index.ts b/packages/core/tools/src/index.ts index 0aa19a7b9a..acae166abe 100644 --- a/packages/core/tools/src/index.ts +++ b/packages/core/tools/src/index.ts @@ -11,7 +11,6 @@ import { Context, Service } from 'cordis' import type { CallId, ContentBlock, ToolSchema } from '@deepseek-ai/dsh-llm' import { HarnessError } from '@deepseek-ai/dsh-llm' import type { Agent } from '@deepseek-ai/dsh-agent' -import type { JsonValue } from '@deepseek-ai/dsh-session' import type {} from '@deepseek-ai/dsh-system-prompt' export { @@ -88,13 +87,6 @@ export interface FileDiff { oldText: string | null /** Content after the change. */ newText: string - /** - * Index signature so a `FileDiff` is a valid {@link JsonValue} member — a tool - * persists result-time diffs as `tool/result` `meta`, which must round-trip - * through the session log. Every declared field is already JSON-compatible; - * this only makes the structural compatibility explicit. - */ - [key: string]: string | null } /** @@ -247,12 +239,13 @@ export interface DiffResultView { /** * What a tool's `execute` returns. The bare {@link ContentBlock}`[]` form is the * common case (model-facing content only); the object form additionally attaches - * a tool-private `meta` presentation payload ({@link JsonValue}) that the - * registry threads onto the `tool/result` session event and hands back to the - * tool's `presentResult`. `meta` is opaque to the core — the tool owns its shape - * and validates it on the way out — and persists so replay reproduces the card. + * a tool-private `meta` presentation payload that the registry threads onto the + * `tool/result` session event and hands back to the tool's `presentResult`. + * `meta` is opaque to the core (`unknown` — the tool owns and narrows its shape), + * and MUST be JSON-serializable: it persists on the durable log (the session + * enforces this at `append`), so replay reproduces the card. */ -export type ToolExecuteReturn = ContentBlock[] | { content: ContentBlock[]; meta?: JsonValue } +export type ToolExecuteReturn = ContentBlock[] | { content: ContentBlock[]; meta?: unknown } /** A registered tool: its schema plus the execution function. */ export interface ToolDefinition extends ToolSchema { @@ -286,10 +279,10 @@ export interface ToolResult { /** * The tool-private presentation payload the tool attached from `execute` (via * the object return form), threaded verbatim from the `tool/result` event. - * Opaque {@link JsonValue}; the tool narrows it back to its own shape. Absent - * when the tool attached none. + * Opaque (`unknown`); the tool narrows it back to its own shape. Absent when + * the tool attached none. */ - meta?: JsonValue + meta?: unknown } /** One pending tool call, as it flows through the execution waterfall. */ @@ -336,10 +329,10 @@ export interface ToolExecutionResult { /** * The tool-private presentation payload from a successful `execute` (the object * return form). Threaded onto the `tool/result` session event and back into - * {@link ToolResult} for `presentResult`. Opaque {@link JsonValue}; absent when - * the tool attached none or the call failed. + * {@link ToolResult} for `presentResult`. Opaque (`unknown`); absent when the + * tool attached none or the call failed. */ - meta?: JsonValue + meta?: unknown } /** diff --git a/packages/fs/fs-local/src/fsio.ts b/packages/fs/fs-local/src/fsio.ts index 9b77678f82..e2aead2bfc 100644 --- a/packages/fs/fs-local/src/fsio.ts +++ b/packages/fs/fs-local/src/fsio.ts @@ -429,4 +429,4 @@ export function applyLiteralEdit( return { content: content.split(oldNorm).join(newNorm), replacements } } -export { restoreLineEndings } +export { normalizeLineEndings, restoreLineEndings } diff --git a/packages/fs/fs-local/src/index.ts b/packages/fs/fs-local/src/index.ts index 3ce0fa2f92..30c65058c9 100644 --- a/packages/fs/fs-local/src/index.ts +++ b/packages/fs/fs-local/src/index.ts @@ -26,6 +26,7 @@ import type { } from '@deepseek-ai/dsh-fs' import { applyLiteralEdit, + normalizeLineEndings, probe, readForEdit, readTextForDiff, @@ -156,7 +157,10 @@ export class LocalFileSystem extends FileSystem { operation: existing ? 'update' : 'create', version: this.versionAfterWrite(after, target), before, - after: content, + // LF-normalized to share the diff basis with `before` (also LF): a CRLF + // overwrite must not read as every line changed. Line-ending restoration + // is a storage detail the applied-hunk diff ignores. + after: normalizeLineEndings(content), } }) } diff --git a/packages/fs/fs-local/tests/filesystem.spec.ts b/packages/fs/fs-local/tests/filesystem.spec.ts index b6686b9627..63baacd47b 100644 --- a/packages/fs/fs-local/tests/filesystem.spec.ts +++ b/packages/fs/fs-local/tests/filesystem.spec.ts @@ -203,11 +203,15 @@ describe('writeText', () => { expect(outcome.after).toBe('new body') }) - it('an overwrite of a CRLF file returns LF-normalized before content', async () => { - await writeFile(join(dir, 'a.txt'), 'a\r\nb\r\n') + it('an overwrite returns LF-normalized before AND after (a CRLF rewrite is not every-line-changed)', async () => { + // The applied-hunk diff bases on `before`/`after`; if `after` kept CRLF while + // `before` is LF-normalized, a CRLF rewrite would read as every line changed. + // Both sides are LF so only the genuinely-changed line diffs. + await writeFile(join(dir, 'a.txt'), 'a\r\nb\r\nc\r\n') const target = await fs.resolve('a.txt') - const outcome = await fs.writeText(target, 'a\nB\n') - expect(outcome.before).toBe('a\nb\n') + const outcome = await fs.writeText(target, 'a\r\nB\r\nc\r\n') + expect(outcome.before).toBe('a\nb\nc\n') + expect(outcome.after).toBe('a\nB\nc\n') }) it('an overwrite of a BINARY prior file reports before:null (undiffable), still succeeds', async () => { diff --git a/packages/fs/fs/src/types.ts b/packages/fs/fs/src/types.ts index 2bc6f27765..1b768724cf 100644 --- a/packages/fs/fs/src/types.ts +++ b/packages/fs/fs/src/types.ts @@ -103,11 +103,11 @@ export interface FsWriteOutcome { version: FsVersion /** * The file's content BEFORE the write, or `null` when the file did not exist - * (a create). Raw storage text (LF-normalized by the backend), never a diff — - * a consumer computes the result-time contextual diff from `before`/`after`. + * (a create). LF-normalized storage text (the diff basis), never a diff — a + * consumer computes the result-time contextual diff from `before`/`after`. */ before: string | null - /** The file's content AFTER the write (the text that was written). */ + /** The file's content AFTER the write, LF-normalized to share `before`'s diff basis. */ after: string } diff --git a/packages/fs/tool-fs/src/diff.ts b/packages/fs/tool-fs/src/diff.ts index 51d39bb3d0..273fe0b033 100644 --- a/packages/fs/tool-fs/src/diff.ts +++ b/packages/fs/tool-fs/src/diff.ts @@ -14,17 +14,17 @@ import { structuredPatch } from 'diff' import type { FileDiff } from '@deepseek-ai/dsh-tools' -import type { JsonValue } from '@deepseek-ai/dsh-session' /** Context lines shown on each side of an applied hunk (matches claude-agent-acp). */ export const DIFF_CONTEXT = 3 /** * The `write`/`edit` tools' private `tool/result` `meta` payload: the applied - * contextual-diff hunks. A {@link JsonValue} (persisted with the session log, so - * `presentResult` reproduces the diff card on replay). The producing tool owns - * this shape; the bridge only sees the opaque `meta` and the tool narrows it back - * via {@link diffsFromMeta}. + * contextual-diff hunks. Attached opaquely (as `unknown`) on the tool result and + * persisted with the session log — it must be JSON-serializable (the session + * validates this at `append`), so `presentResult` reproduces the diff card on + * replay. The producing tool owns this shape; the bridge only sees the opaque + * `meta` and the tool narrows it back via {@link diffsFromMeta}. */ export type FsDiffMeta = { diffs: FileDiff[] } @@ -68,9 +68,9 @@ export function computeHunkDiffs(path: string, before: string, after: string): F } /** Whether `value` is a valid {@link FileDiff} (defensive narrowing from opaque `meta`). */ -function isFileDiff(value: JsonValue): value is FileDiff & JsonValue { +function isFileDiff(value: unknown): value is FileDiff { if (typeof value !== 'object' || value === null || Array.isArray(value)) return false - const { path, oldText, newText } = value + const { path, oldText, newText } = value as Record return typeof path === 'string' && (oldText === null || typeof oldText === 'string') && typeof newText === 'string' @@ -83,9 +83,9 @@ function isFileDiff(value: JsonValue): value is FileDiff & JsonValue { * it validates defensively rather than trusting the payload — a bad `meta` yields * no diff card (the generic result rendering) instead of a thrown presenter. */ -export function diffsFromMeta(meta: JsonValue | undefined): FileDiff[] | undefined { +export function diffsFromMeta(meta: unknown): FileDiff[] | undefined { if (typeof meta !== 'object' || meta === null || Array.isArray(meta)) return undefined - const diffs = meta.diffs + const diffs = (meta as Record).diffs if (!Array.isArray(diffs) || diffs.length === 0 || !diffs.every(isFileDiff)) return undefined return diffs } diff --git a/packages/ui/acp/README.md b/packages/ui/acp/README.md index 32e4326b99..ac38f36f58 100644 --- a/packages/ui/acp/README.md +++ b/packages/ui/acp/README.md @@ -48,7 +48,7 @@ How a tool call renders in the editor is owned by the TOOL, not the bridge — t - `{ card: 'terminal', title, description?, cwd? }` — a shell command → a terminal card (see Terminal card). - `{ card: 'diff', title, diffs, locations? }` — a file create/modify → an inline diff card; `diffs` is `FileDiff[]` (`{ path, oldText, newText }`, `oldText: null` ⇒ new file). The bridge emits each diff as an ACP `{ type: 'diff', path, oldText, newText }` `tool_call.content` block, which Zed renders as an inline diff / new-file preview. -`presentResult` returns a `ToolResultView`, one of two cards: `{ card: 'generic', title?, content? }` (an optional replacement `title` and reformatted `content`) or `{ card: 'terminal', title?, output?, exitCode?, signal? }` (the captured run output + exit — see Terminal card). The bridge looks the definition up by name in `ctx.tools` and `switch (view.card)`es to build the ACP `tool_call`/`tool_call_update` wire shape per card; a tool that declares neither gets a generic fallback (title = tool name, raw parsed args as `rawInput`, kind inferred from the name). For example `dsh-tool-bash` returns a `terminal` card for a foreground `bash` (title = the exact `command` "ls -la src", `description` = the model description); the `dsh-tool-fs` `write`/`edit` tools return a `diff` card and `read` returns a `generic` card (`kind: 'read'`, the read window in its title — `Read foo.txt (5 - 8)` — and a `locations` entry for the file). For a file card the bridge **relativizes the title** against the session cwd (mirroring `claude-agent-acp`'s `toDisplayPath` — `Read src/foo.ts`, not the absolute path) while keeping `locations[]`/`diffs[].path` **raw** so the editor opens the real path. +`presentResult` returns a `ToolResultView`, one of three cards: `{ card: 'generic', title?, content? }` (an optional replacement `title` and reformatted `content`), `{ card: 'terminal', title?, output?, exitCode?, signal? }` (the captured run output + exit — see Terminal card), or `{ card: 'diff', title?, diffs }` (a completed file mutation → the APPLIED hunks with context lines computed from the before/after content, which supersede the call-time snippet). The bridge looks the definition up by name in `ctx.tools` and `switch (view.card)`es to build the ACP `tool_call`/`tool_call_update` wire shape per card; a tool that declares neither gets a generic fallback (title = tool name, raw parsed args as `rawInput`, kind inferred from the name). For example `dsh-tool-bash` returns a `terminal` card for a foreground `bash` (title = the exact `command` "ls -la src", `description` = the model description); the `dsh-tool-fs` `write`/`edit` tools return a `diff` call card and a `diff` result card, and `read` returns a `generic` card (`kind: 'read'`, the read window in its title — `Read foo.txt (5 - 8)` — and a `locations` entry for the file). For a file card the bridge **relativizes the title** against the session cwd (mirroring `claude-agent-acp`'s `toDisplayPath` — `Read src/foo.ts`, not the absolute path) while keeping `locations[]`/`diffs[].path` **raw** so the editor opens the real path. The `tool/result` session event carries only `{ callId, content, isError }` — not the tool name or args — so to call a tool's `presentResult` the bridge keeps a small per-session map from `callId` to the in-flight call's `(name, args)`, populated on `tool/call` and removed as each result is presented (it holds only currently-in-flight calls, never finished ones). This is bridge-local state — NOT a change to the event schema or a core service. The map lives on the `SessionRecord`, so two concurrent sessions never cross their in-flight tool state; a `session/load` replay uses a throwaway presenter that pairs each `tool/call` with its `tool/result` as the log replays in order, so replayed tool cards render identically to live ones. diff --git a/packages/ui/acp/src/index.ts b/packages/ui/acp/src/index.ts index 631a741447..24cdc58f19 100644 --- a/packages/ui/acp/src/index.ts +++ b/packages/ui/acp/src/index.ts @@ -66,7 +66,7 @@ import { assertNever, CallId } from '@deepseek-ai/dsh-llm' import type { Agent, AgentStatus } from '@deepseek-ai/dsh-agent' import { AgentId } from '@deepseek-ai/dsh-agent' import { SessionId } from '@deepseek-ai/dsh-session' -import type { JsonValue, SessionEvent, TodoItem, TurnEndReason } from '@deepseek-ai/dsh-session' +import type { SessionEvent, TodoItem, TurnEndReason } from '@deepseek-ai/dsh-session' import type { ToolCallKind, ToolCallView, ToolRegistry, ToolResultView, TerminalResultView } from '@deepseek-ai/dsh-tools' // Side-effect type import: declaration-merges `ctx.sessionPersistence` onto // Context (the bridge injects it and reads `list()` for load cwd validation). @@ -919,7 +919,7 @@ export class ToolPresenter { } /** Completed-state render intent for a `tool/result`; consumes the remembered `(name, args, card)`. */ - result(callId: CallId, content: ContentBlock[], isError: boolean, meta?: JsonValue): ToolResultView { + result(callId: CallId, content: ContentBlock[], isError: boolean, meta?: unknown): ToolResultView { const call = this.pending.get(callId) this.pending.delete(callId) // No remembered call (unknown/late callId) → nothing to present from; raw content. diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index db4b513b10..9b974809cf 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -271,9 +271,6 @@ importers: '@deepseek-ai/dsh-llm': specifier: workspace:^ version: link:../../llm/llm - '@deepseek-ai/dsh-session': - specifier: workspace:^ - version: link:../session '@deepseek-ai/dsh-system-prompt': specifier: workspace:^ version: link:../system-prompt