From 5ab8f2e328f0f9b762b9a4fd1697786c681d868d Mon Sep 17 00:00:00 2001 From: Dudu-0223 Date: Tue, 14 Jul 2026 11:56:27 +0800 Subject: [PATCH] docs: correct fs/observed concurrency-safety wording The read tool's isConcurrencySafe rationale called the fs/observed recorder "commutative" and said concurrent reads "converge to one observed version", overstating the guarantee: the WeakMap record is last-writer-wins. Safety comes from write/edit re-checking the version in their in-lock CAS (a stale observation only forces a later edit to fail closed with FS_STALE_VERSION), as the RFC already states. Align the read comment, the ToolDefinition JSDoc, both READMEs, and the regenerated catalogs. --- docs/config-catalog.md | 2 +- docs/cordis-catalog/services.md | 2 +- docs/core-data-structures/tools.md | 6 ++++-- packages/core/tools/README.md | 2 +- packages/core/tools/src/index.ts | 6 ++++-- packages/fs/tool-fs/README.md | 2 +- packages/fs/tool-fs/src/read.ts | 10 ++++++---- 7 files changed, 18 insertions(+), 12 deletions(-) diff --git a/docs/config-catalog.md b/docs/config-catalog.md index 0a242d31b1..b06942002d 100644 --- a/docs/config-catalog.md +++ b/docs/config-catalog.md @@ -975,7 +975,7 @@ export interface Config { export type ToolPresentationMode = 'native' | 'code' | 'both' ``` -Source: [`packages/core/tools/src/index.ts:482`](../packages/core/tools/src/index.ts) +Source: [`packages/core/tools/src/index.ts:484`](../packages/core/tools/src/index.ts) ## `@deepseek-ai/dsh-user-approval` diff --git a/docs/cordis-catalog/services.md b/docs/cordis-catalog/services.md index e8ab41247f..8b427f9bff 100644 --- a/docs/cordis-catalog/services.md +++ b/docs/cordis-catalog/services.md @@ -277,7 +277,7 @@ async execute(exec: ToolExecutionInput): Promise Types: [ToolDefinition](../core-data-structures/tools.md) · [ToolExecutionInput](../core-data-structures/tools.md) · [ToolExecutionMode](../core-data-structures/tools.md) · [ToolExecutionResult](../core-data-structures/tools.md) -Source: [`packages/core/tools/src/index.ts:574`](../../packages/core/tools/src/index.ts) +Source: [`packages/core/tools/src/index.ts:576`](../../packages/core/tools/src/index.ts) ## `ctx.userInteraction` — `UserInteractionService` diff --git a/docs/core-data-structures/tools.md b/docs/core-data-structures/tools.md index bc231d6b2e..2e4a269537 100644 --- a/docs/core-data-structures/tools.md +++ b/docs/core-data-structures/tools.md @@ -40,8 +40,10 @@ interface ToolDefinition extends ToolSchema { * step outputs are the returned content, `meta`, structured error, and * `additionalContext` carried through the loop's ordered post-execute path. * The narrow exception is a synchronous, side-effect-only recorder whose - * updates are commutative for concurrent calls by the same session (the - * `fs/observed` version recorder is the worked example). + * updates are commutative OR fail closed for concurrent calls by the same + * session (the `fs/observed` version recorder is the worked example: its + * WeakMap record is last-writer-wins, and a stale observation only makes a + * later write/edit fail closed at its in-lock version CAS). */ isConcurrencySafe?(args: unknown): boolean /** diff --git a/packages/core/tools/README.md b/packages/core/tools/README.md index 8866de6d8c..4f29f4b6e1 100644 --- a/packages/core/tools/README.md +++ b/packages/core/tools/README.md @@ -85,7 +85,7 @@ See `defineTool`, `validateArgs`, `ToolArgsError`, `SchemaSpec`, `InferArgs`, an `defineTool` also validates an optional `timeoutMs` at definition time when present: it must be a positive finite number, or the helper throws — the budget is attached to the produced `ToolDefinition` (for `@deepseek-ai/dsh-timeout-policy`) and never reaches the model. -`defineTool` also accepts an optional `isConcurrencySafe(args): boolean` — the per-call concurrency classifier the agent-loop scheduler reads via `executionMode`. `args` is the typed `InferArgs` shape. It is soft-validated exactly like the presenters: an arg mismatch yields `false` (the conservative exclusive default), never the hard `ToolArgsError`. Declaring `true` is a contract — the tool body must not mutate parent-owned async state (`exec.agent.session.append`, `agent.inject`) during `execute`; its only ordered outputs are the returned content, `meta`, error, and `additionalContext`. The one exception is a synchronous, side-effect-only commutative recorder (the `fs/observed` version recorder is the worked example); anything richer stays exclusive. Host-only, never model-visible. +`defineTool` also accepts an optional `isConcurrencySafe(args): boolean` — the per-call concurrency classifier the agent-loop scheduler reads via `executionMode`. `args` is the typed `InferArgs` shape. It is soft-validated exactly like the presenters: an arg mismatch yields `false` (the conservative exclusive default), never the hard `ToolArgsError`. Declaring `true` is a contract — the tool body must not mutate parent-owned async state (`exec.agent.session.append`, `agent.inject`) during `execute`; its only ordered outputs are the returned content, `meta`, error, and `additionalContext`. The one exception is a synchronous, side-effect-only recorder whose updates are commutative or fail closed (the `fs/observed` version recorder is the worked example: its record is last-writer-wins, and a stale observation only makes a later write/edit fail closed at its in-lock version CAS); anything richer stays exclusive. Host-only, never model-visible. ### Structured-output schema subset diff --git a/packages/core/tools/src/index.ts b/packages/core/tools/src/index.ts index f2eac7349b..0a26ff7089 100644 --- a/packages/core/tools/src/index.ts +++ b/packages/core/tools/src/index.ts @@ -217,8 +217,10 @@ export interface ToolDefinition extends ToolSchema { * step outputs are the returned content, `meta`, structured error, and * `additionalContext` carried through the loop's ordered post-execute path. * The narrow exception is a synchronous, side-effect-only recorder whose - * updates are commutative for concurrent calls by the same session (the - * `fs/observed` version recorder is the worked example). + * updates are commutative OR fail closed for concurrent calls by the same + * session (the `fs/observed` version recorder is the worked example: its + * WeakMap record is last-writer-wins, and a stale observation only makes a + * later write/edit fail closed at its in-lock version CAS). */ isConcurrencySafe?(args: unknown): boolean /** diff --git a/packages/fs/tool-fs/README.md b/packages/fs/tool-fs/README.md index 5817c0eb95..7cd9b3d641 100644 --- a/packages/fs/tool-fs/README.md +++ b/packages/fs/tool-fs/README.md @@ -46,6 +46,6 @@ The tool passes `exec` (the tool-execution context) as the opaque `actor` on eve `fs/observed` fires AFTER the read/write/edit already succeeded, via a plain `ctx.emit`. A listener is contractually a synchronous, side-effect-only recorder (`@deepseek-ai/dsh-fs-policy`'s is a `WeakMap.set`); the tool does not guard the emit, so a listener that throws would surface as the tool's `isError` result — async or fallible observation does not belong on this event. -This is exactly why `read` declares `isConcurrencySafe: () => true` while `write`/`edit` do not: `read`'s only side effect is that synchronous commutative recorder (same-target concurrent reads converge to one observed version), so the agent loop may run sibling reads in parallel. `write`/`edit` mutate the filesystem and stay exclusive barriers — the provider re-checks the observed version inside its per-target lock before mutating, so a stale read never corrupts (it only forces a re-read). See the [parallel tool-call RFC](../../../docs/rfc/implemented/feature/2026-07-10-parallel-tool-call-execution.md). +This is exactly why `read` declares `isConcurrencySafe: () => true` while `write`/`edit` do not: `read`'s only side effect is that synchronous version recorder (same-target concurrent reads race last-writer-wins on the observed version), so the agent loop may run sibling reads in parallel. `write`/`edit` mutate the filesystem and stay exclusive barriers — the provider re-checks the observed version inside its per-target lock before mutating, so a stale read never corrupts (it only forces a re-read via `FS_STALE_VERSION`). See the [parallel tool-call RFC](../../../docs/rfc/implemented/feature/2026-07-10-parallel-tool-call-execution.md). The read rendering (line windowing + output formatting) lives in `src/read-render.ts` (Cordis-free, independently unit-tested); `src/read.ts`/`write.ts`/`edit.ts` are the tool executors and `src/index.ts` composes them. diff --git a/packages/fs/tool-fs/src/read.ts b/packages/fs/tool-fs/src/read.ts index dc43177726..ad60dc7afb 100644 --- a/packages/fs/tool-fs/src/read.ts +++ b/packages/fs/tool-fs/src/read.ts @@ -92,10 +92,12 @@ export function applyReadTool(ctx: Context, caps: ReadToolCaps): void { offset: { type: 'number', description: '1-based first line to return. Defaults to 1.' }, limit: { type: 'number', description: `Maximum number of lines to return. Defaults to ${caps.limit}.` }, }, - // Read-only. Its one side effect is the synchronous, commutative `fs/observed` - // version recorder (a WeakMap write; see below and the fs-policy plugin), so - // concurrent same-target reads converge to one observed version. write/edit - // stay exclusive barriers and re-check versions in-lock before mutating. + // Read-only. Its one side effect is the synchronous `fs/observed` version + // recorder (a WeakMap write; see below and the fs-policy plugin): concurrent + // same-target reads race last-writer-wins on that record, which is safe because + // it is NOT the safety boundary — write/edit stay exclusive barriers and + // re-check the version in-lock, so a stale observation only makes a later edit + // fail closed with FS_STALE_VERSION. isConcurrencySafe: () => true, async execute(args, exec): Promise { const input = parseReadArgs(args, caps.limit)