From 1ee167aeaca76ef483db6d2e3a2c6ba1f110161f Mon Sep 17 00:00:00 2001 From: _Kerman Date: Mon, 3 Aug 2026 19:49:30 +0800 Subject: [PATCH 1/2] feat(fs): append recovery remedy to guarded-mutation errors write/edit failures with FS_STALE_VERSION or FS_NOT_OBSERVED now reach the model with the correct recovery instruction appended (re-read / read, then retry) while preserving the structured code and chaining the cause. The edit-intent waterfall sits inside the same try, so the policy's FS_NOT_OBSERVED refusal is remediated too. Re-recorded the fs-policy-reject keyless snapshot and the bilingual README pairs. --- .../snapshots/fs-policy-reject/session.jsonl | 2 +- packages/fs/fs-policy/README.i18n.yaml | 4 +- packages/fs/fs-policy/README.md | 2 +- packages/fs/fs-policy/README.zh.md | 2 +- packages/fs/tool-fs/README.i18n.yaml | 4 +- packages/fs/tool-fs/README.md | 2 +- packages/fs/tool-fs/README.zh.md | 2 +- packages/fs/tool-fs/src/edit.ts | 14 +++- packages/fs/tool-fs/src/error.ts | 34 ++++++++ packages/fs/tool-fs/src/write.ts | 6 +- packages/fs/tool-fs/tests/error.spec.ts | 35 ++++++++ packages/fs/tool-fs/tests/integration.spec.ts | 80 +++++++++++++++++++ packages/fs/tool-fs/tests/tools.spec.ts | 3 +- 13 files changed, 174 insertions(+), 16 deletions(-) create mode 100644 packages/fs/tool-fs/src/error.ts create mode 100644 packages/fs/tool-fs/tests/error.spec.ts diff --git a/examples/acp-agent/tests/snapshots/fs-policy-reject/session.jsonl b/examples/acp-agent/tests/snapshots/fs-policy-reject/session.jsonl index 07d1408c9e..87cd7427f9 100644 --- a/examples/acp-agent/tests/snapshots/fs-policy-reject/session.jsonl +++ b/examples/acp-agent/tests/snapshots/fs-policy-reject/session.jsonl @@ -16,7 +16,7 @@ {"type":"assistant/chunk","seq":78,"time":1785487602271,"data":{"turn":1,"step":1,"chunk":{"type":"finish","reason":{"kind":"tool-calls"}}}} {"type":"assistant/message","seq":79,"time":1785487602271,"data":{"turn":1,"step":1,"message":{"role":"assistant","content":[{"type":"reasoning","text":"The user wants me to use the edit tool to replace \"blue\" with \"green\" in settings.txt without reading the file first, and then reply with just \"DONE\"."},{"type":"tool-call","id":"call_00_x0zlnXl5JOxLrAYL9y7P0119","name":"edit","arguments":"{\"file_path\": \"settings.txt\", \"old_string\": \"blue\", \"new_string\": \"green\"}"}],"source":{"kind":"model","provider":"deepseek-official","model":"deepseek-v4-flash"},"id":"8bd34189-fb62-4106-9c25-b6022d48e059"},"usage":{"inputTokens":3132,"outputTokens":115,"cacheReadTokens":0,"reasoningTokens":36}},"sourceEventSeqs":[7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40,41,42,43,44,45,46,47,48,49,50,51,52,53,54,55,56,57,58,59,60,61,62,63,64,65,66,67,68,69,70,71,72,73,74,75,76,77,78],"surfaceOp":"append"} {"type":"tool/call","seq":80,"time":1785487602272,"data":{"turn":1,"step":1,"callId":"call_00_x0zlnXl5JOxLrAYL9y7P0119","name":"edit","arguments":"{\"file_path\": \"settings.txt\", \"old_string\": \"blue\", \"new_string\": \"green\"}"}} -{"type":"tool/result","seq":81,"time":1785487602280,"data":{"turn":1,"step":1,"message":{"source":{"kind":"tool","callId":"call_00_x0zlnXl5JOxLrAYL9y7P0119"},"content":[{"type":"tool-result","toolCallId":"call_00_x0zlnXl5JOxLrAYL9y7P0119","content":[{"type":"text","text":"Error: edit requires reading \"{{cwd}}/settings.txt\" first"}],"isError":true}],"role":"user","id":"c4018c31-b6fd-4f14-af3c-e609863bf501"},"error":{"name":"FsError","code":"FS_NOT_OBSERVED"}},"sourceEventSeqs":[80],"surfaceOp":"append"} +{"type":"tool/result","seq":81,"time":1785487602280,"data":{"turn":1,"step":1,"message":{"source":{"kind":"tool","callId":"call_00_x0zlnXl5JOxLrAYL9y7P0119"},"content":[{"type":"tool-result","toolCallId":"call_00_x0zlnXl5JOxLrAYL9y7P0119","content":[{"type":"text","text":"Error: edit requires reading \"{{cwd}}/settings.txt\" first — read the file, then retry"}],"isError":true}],"role":"user","id":"c4018c31-b6fd-4f14-af3c-e609863bf501"},"error":{"name":"FsError","code":"FS_NOT_OBSERVED"}},"sourceEventSeqs":[80],"surfaceOp":"append"} {"type":"step/end","seq":82,"time":1785487602280,"data":{"turn":1,"step":1}} {"type":"step/start","seq":83,"time":1785487602287,"data":{"turn":1,"step":2}} {"type":"assistant/chunk","seq":84,"time":1783611704931,"data":{"turn":1,"step":2,"chunk":{"type":"block-start","index":0,"blockType":"reasoning"}}} diff --git a/packages/fs/fs-policy/README.i18n.yaml b/packages/fs/fs-policy/README.i18n.yaml index 6690227dbc..5168b43d34 100644 --- a/packages/fs/fs-policy/README.i18n.yaml +++ b/packages/fs/fs-policy/README.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write packages/fs/fs-policy/README.md -README.md: dc4e9377793570c80b8d71ec84196bebe7fe583a -README.zh.md: aa0cb25899f5906ac9f531583ba48d01ad6095b4 +README.md: f6b3292bdc6e5565df0393a59c50d4e594921401 +README.zh.md: 2b30e6223719301df776b5d1cb7c674cb7ef7ff7 diff --git a/packages/fs/fs-policy/README.md b/packages/fs/fs-policy/README.md index dc4e937779..f6b3292bdc 100644 --- a/packages/fs/fs-policy/README.md +++ b/packages/fs/fs-policy/README.md @@ -55,7 +55,7 @@ Because the plugin influences the world only through events, removing it does no #### What the model sees -This plugin adds no prompt or schema. It rejects an edit without a prior read with code `FS_NOT_OBSERVED` and exact message `edit requires reading "" first`. Guarded mutations whose observed version is stale propagate the provider-owned `FS_STALE_VERSION` error. [`dsh-tool-fs`](../tool-fs/README.md) owns the model-facing error wrapper; observation state is never shown. +This plugin adds no prompt or schema. It rejects an edit without a prior read with code `FS_NOT_OBSERVED` and exact message `edit requires reading "" first`. Guarded mutations whose observed version is stale propagate the provider-owned `FS_STALE_VERSION` error. [`dsh-tool-fs`](../tool-fs/README.md) owns the model-facing error wrapper, which appends the recovery instruction to `FS_STALE_VERSION` (`— re-read the file, then retry`) and `FS_NOT_OBSERVED` (`— read the file, then retry`) messages while preserving the code; observation state is never shown. #### Token effect diff --git a/packages/fs/fs-policy/README.zh.md b/packages/fs/fs-policy/README.zh.md index aa0cb25899..2b30e62237 100644 --- a/packages/fs/fs-policy/README.zh.md +++ b/packages/fs/fs-policy/README.zh.md @@ -55,7 +55,7 @@ await ctx.plugin(FsPolicy) #### 模型看到的内容 -该插件不添加提示词或 schema。编辑前未读取时，它会以代码 `FS_NOT_OBSERVED` 和精确消息 `edit requires reading "" first` 拒绝。观察版本陈旧的防护变更会传播由提供方拥有的 `FS_STALE_VERSION` 错误。[`dsh-tool-fs`](../tool-fs/README.md)拥有面向模型的错误包装；观察状态绝不会显示。 +该插件不添加提示词或 schema。编辑前未读取时，它会以代码 `FS_NOT_OBSERVED` 和精确消息 `edit requires reading "" first` 拒绝。观察版本陈旧的防护变更会传播由提供方拥有的 `FS_STALE_VERSION` 错误。[`dsh-tool-fs`](../tool-fs/README.md)拥有面向模型的错误包装，会为 `FS_STALE_VERSION` 消息追加恢复指令（`— re-read the file, then retry`）、为 `FS_NOT_OBSERVED` 消息追加恢复指令（`— read the file, then retry`），同时保留错误码；观察状态绝不会显示。 #### Token 影响 diff --git a/packages/fs/tool-fs/README.i18n.yaml b/packages/fs/tool-fs/README.i18n.yaml index fbe2e69043..8f462ed19b 100644 --- a/packages/fs/tool-fs/README.i18n.yaml +++ b/packages/fs/tool-fs/README.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write packages/fs/tool-fs/README.md -README.md: a695d0ba8fb1d600689d2b68763e8423d1591da5 -README.zh.md: 5c600ab70b46da640637aec64efc1c0f0d0d54c0 +README.md: 246b1c8797e9a2ddc630724729edf8e2f1185bfc +README.zh.md: 6cfc3d750b0f6ffc9ee886f4d0f058885bc19083 diff --git a/packages/fs/tool-fs/README.md b/packages/fs/tool-fs/README.md index a695d0ba8f..246b1c8797 100644 --- a/packages/fs/tool-fs/README.md +++ b/packages/fs/tool-fs/README.md @@ -136,7 +136,7 @@ Append-only; newly visible content follows the reusable request prefix and does #### What the model sees -Failures are normalized as `Error: `. This package's stable validation and read messages are `file_path must be a non-empty string`, `limit must be less than or equal to `, `old_string must be a non-empty string`, `old_string and new_string must differ`, `cannot read "": not found`, `cannot read "": not a regular file`, and `offset is out of range for "" ( lines)`; provider and policy templates are quoted in their package READMEs. +Failures are normalized as `Error: `. This package's stable validation and read messages are `file_path must be a non-empty string`, `limit must be less than or equal to `, `old_string must be a non-empty string`, `old_string and new_string must differ`, `cannot read "": not found`, `cannot read "": not a regular file`, and `offset is out of range for "" ( lines)`; provider and policy templates are quoted in their package READMEs. Guarded-mutation failures additionally carry their recovery instruction in the message, appended by this package's model-facing error wrapper: `FS_STALE_VERSION` (including a missing edit target) gets `— re-read the file, then retry`, `FS_NOT_OBSERVED` gets `— read the file, then retry`; the structured code is preserved. #### Token effect diff --git a/packages/fs/tool-fs/README.zh.md b/packages/fs/tool-fs/README.zh.md index 5c600ab70b..6cfc3d750b 100644 --- a/packages/fs/tool-fs/README.zh.md +++ b/packages/fs/tool-fs/README.zh.md @@ -136,7 +136,7 @@ Use the edit tool for targeted changes to existing UTF-8 text files. It replaces #### 模型看到的内容 -失败会规范化为 `Error: `。本包稳定的校验和读取消息是 `file_path must be a non-empty string`、`limit must be less than or equal to `、`old_string must be a non-empty string`、`old_string and new_string must differ`、`cannot read "": not found`、`cannot read "": not a regular file` 和 `offset is out of range for "" ( lines)`；提供方和策略模板在各自包的 README 中逐字列出。 +失败会规范化为 `Error: `。本包稳定的校验和读取消息是 `file_path must be a non-empty string`、`limit must be less than or equal to `、`old_string must be a non-empty string`、`old_string and new_string must differ`、`cannot read "": not found`、`cannot read "": not a regular file` 和 `offset is out of range for "" ( lines)`；提供方和策略模板在各自包的 README 中逐字列出。防护变更失败还会在消息中携带恢复指令，由本包面向模型的错误包装追加：`FS_STALE_VERSION`（包括编辑目标缺失）追加 `— re-read the file, then retry`，`FS_NOT_OBSERVED` 追加 `— read the file, then retry`；结构化错误码保持不变。 #### Token 影响 diff --git a/packages/fs/tool-fs/src/edit.ts b/packages/fs/tool-fs/src/edit.ts index 951c0b7b57..fcd04cb17c 100644 --- a/packages/fs/tool-fs/src/edit.ts +++ b/packages/fs/tool-fs/src/edit.ts @@ -11,6 +11,7 @@ import type { DiffCallView, DiffResultView, ToolResult } from '@deepseek-ai/dsh- import type {} from '@deepseek-ai/dsh-fs' import type {} from '@deepseek-ai/dsh-system-prompt' import { computeHunkDiffs, diffsFromMeta } from './diff.ts' +import { remediateFsError } from './error.ts' import { sessionResolveOptions } from './session-cwd.ts' import type { FsSandboxSurface } from './sandbox.ts' @@ -116,10 +117,13 @@ export function applyEditTool(ctx: Context, sandbox: FsSandboxSurface): void { const target = await ctx.fs.resolve(input.filePath, sessionResolveOptions(exec, input.filePath, sandboxPolicy?.workspaceRoot)) // Single-slot decision: the policy plugin returns { version: vObserved } or // throws FS_NOT_OBSERVED; the bare default is undefined (unconditional edit). - // No stat — the bare default never manufactures a version basis. - const intent = await ctx.waterfall('fs/edit-intent', target, exec, () => undefined) + // No stat — the bare default never manufactures a version basis. The intent + // slot itself can throw FS_NOT_OBSERVED for an unread target, so it sits + // inside the try: both that refusal and the provider's guarded-mutation + // failure get the model-facing remedy below. let outcome try { + const intent = await ctx.waterfall('fs/edit-intent', target, exec, () => undefined) outcome = await ctx.fs.editText( target, { oldString: input.oldString, newString: input.newString, replaceAll: input.replaceAll }, @@ -128,8 +132,10 @@ export function applyEditTool(ctx: Context, sandbox: FsSandboxSurface): void { sandboxPolicy, ) } catch (error: unknown) { - // A sandbox denial becomes the shared [sandbox: …] marker; any other error passes through. - throw sandbox.mapError(error, sandboxPolicy) + // A sandbox denial becomes the shared [sandbox: …] marker (the model + // recognizes it from bash); stale/not-observed failures gain their + // model-facing remedy; anything else passes through. + throw remediateFsError(sandbox.mapError(error, sandboxPolicy)) } // Record the observed version (a no-op when no policy plugin listens). ctx.emit('fs/observed', target, outcome.version, exec) diff --git a/packages/fs/tool-fs/src/error.ts b/packages/fs/tool-fs/src/error.ts new file mode 100644 index 0000000000..e67616887f --- /dev/null +++ b/packages/fs/tool-fs/src/error.ts @@ -0,0 +1,34 @@ +/** + * Model-facing remediation for guarded-mutation failures. The provider's + * `FS_STALE_VERSION` and `FS_NOT_OBSERVED` messages state the condition but + * not the only correct recovery (re-read / read the file), so this package + * appends the remedy at the model boundary; provider messages stay + * machine-oriented and unchanged. + * @module @deepseek-ai/dsh-tool-fs/src/error + */ + +import { FsError } from '@deepseek-ai/dsh-fs' +import type { FsErrorCode } from '@deepseek-ai/dsh-fs' + +/** The remedy appended to each remediable failure code's message. */ +const REMEDIES: Partial> = { + FS_STALE_VERSION: 're-read the file, then retry', + FS_NOT_OBSERVED: 'read the file, then retry', +} + +/** + * Append the correct recovery instruction to a guarded-mutation failure's + * message. `FS_STALE_VERSION` (the file changed since this session's last + * observation, including a missing target) recovers only by re-reading; + * `FS_NOT_OBSERVED` (no prior read by this session) by reading. The `FsError` + * code is preserved so retry/permission/UI layers keep routing on it, and the + * original error chains as `cause`. Anything else passes through untouched. + * @param error - the caught value from a write/edit execution. + * @returns a remediated `FsError` for the two guarded-mutation codes, else the original value. + */ +export function remediateFsError(error: unknown): unknown { + if (!(error instanceof FsError)) return error + const remedy = REMEDIES[error.code] + if (!remedy) return error + return new FsError(`${error.message} — ${remedy}`, error.code, { cause: error }) +} diff --git a/packages/fs/tool-fs/src/write.ts b/packages/fs/tool-fs/src/write.ts index 37a6d67e59..56e2be488b 100644 --- a/packages/fs/tool-fs/src/write.ts +++ b/packages/fs/tool-fs/src/write.ts @@ -12,6 +12,7 @@ import type { FsWriteOutcome } from '@deepseek-ai/dsh-fs' import type {} from '@deepseek-ai/dsh-fs' import type {} from '@deepseek-ai/dsh-system-prompt' import { computeHunkDiffs, diffsFromMeta } from './diff.ts' +import { remediateFsError } from './error.ts' import { sessionResolveOptions } from './session-cwd.ts' import type { FsSandboxSurface } from './sandbox.ts' @@ -113,8 +114,9 @@ export function applyWriteTool(ctx: Context, sandbox: FsSandboxSurface): void { outcome = await ctx.fs.writeText(target, input.content, intent, exec.signal, sandboxPolicy) } catch (error: unknown) { // A sandbox denial becomes the shared [sandbox: …] marker (the model - // recognizes it from bash); any other error passes through. - throw sandbox.mapError(error, sandboxPolicy) + // recognizes it from bash); stale/not-observed failures gain their + // model-facing remedy; anything else passes through. + throw remediateFsError(sandbox.mapError(error, sandboxPolicy)) } // Record the observed version (a no-op when no policy plugin listens). ctx.emit('fs/observed', target, outcome.version, exec) diff --git a/packages/fs/tool-fs/tests/error.spec.ts b/packages/fs/tool-fs/tests/error.spec.ts new file mode 100644 index 0000000000..671eb32d9d --- /dev/null +++ b/packages/fs/tool-fs/tests/error.spec.ts @@ -0,0 +1,35 @@ +/** + * Unit tests for the model-facing error remediation: the remedy appended to + * guarded-mutation failures, code preservation, and passthrough behavior. + */ + +import { describe, expect, it } from 'vitest' +import { FsError } from '@deepseek-ai/dsh-fs' +import { remediateFsError } from '../src/error.ts' + +describe('remediateFsError', () => { + it('appends the re-read remedy to FS_STALE_VERSION, preserving the code and chaining the cause', () => { + const original = new FsError('cannot edit "x": file changed since it was read', 'FS_STALE_VERSION') + const remedied = remediateFsError(original) as FsError + expect(remedied).toBeInstanceOf(FsError) + expect(remedied.message).toBe('cannot edit "x": file changed since it was read — re-read the file, then retry') + expect(remedied.code).toBe('FS_STALE_VERSION') + expect(remedied.cause).toBe(original) + }) + + it('appends the read remedy to FS_NOT_OBSERVED', () => { + const remedied = remediateFsError(new FsError('edit requires reading "x" first', 'FS_NOT_OBSERVED')) as FsError + expect(remedied.message).toBe('edit requires reading "x" first — read the file, then retry') + expect(remedied.code).toBe('FS_NOT_OBSERVED') + }) + + it('leaves other FsError codes untouched', () => { + const original = new FsError('no match anywhere', 'FS_EDIT_NOT_FOUND') + expect(remediateFsError(original)).toBe(original) + }) + + it('leaves non-FsError values untouched', () => { + const original = new Error('boom') + expect(remediateFsError(original)).toBe(original) + }) +}) diff --git a/packages/fs/tool-fs/tests/integration.spec.ts b/packages/fs/tool-fs/tests/integration.spec.ts index c835baebb9..3482b38569 100644 --- a/packages/fs/tool-fs/tests/integration.spec.ts +++ b/packages/fs/tool-fs/tests/integration.spec.ts @@ -71,6 +71,9 @@ describe('default deployment (with dsh-fs-policy)', () => { const result = await call('write', { file_path: 'a.txt', content: 'clobber' }) expect(result.isError).toBe(true) expect(result.error).toMatchObject({ info: { code: 'FS_NOT_OBSERVED' } }) + // The model-facing text names the remedy, not just the condition. + expect(text(result)).toContain('without reading it first') + expect(text(result)).toContain('read the file, then retry') expect(await readFile(join(dir, 'a.txt'), 'utf8')).toBe('original') }) @@ -89,6 +92,23 @@ describe('default deployment (with dsh-fs-policy)', () => { const result = await call('write', { file_path: 'a.txt', content: 'replaced' }) expect(result.isError).toBe(true) expect(result.error).toMatchObject({ info: { code: 'FS_STALE_VERSION' } }) + // The model-facing text names the remedy, not just the condition. + expect(text(result)).toContain('file changed since it was read') + expect(text(result)).toContain('re-read the file, then retry') + }) + + it('the stale remedy is actionable: re-reading the changed file unblocks the retried write', async () => { + await writeFile(join(dir, 'a.txt'), 'original') + await call('read', { file_path: 'a.txt' }) + await writeFile(join(dir, 'a.txt'), 'changed-externally') // out-of-band change + const stale = await call('write', { file_path: 'a.txt', content: 'replaced' }) + expect(stale.isError).toBe(true) + expect(stale.error).toMatchObject({ info: { code: 'FS_STALE_VERSION' } }) + // Follow the remedy: re-read (refreshes the observed version), then retry. + expect((await call('read', { file_path: 'a.txt' })).isError).toBe(false) + const retried = await call('write', { file_path: 'a.txt', content: 'replaced' }) + expect(retried.isError).toBe(false) + expect(await readFile(join(dir, 'a.txt'), 'utf8')).toBe('replaced') }) }) @@ -131,6 +151,9 @@ describe('default deployment (with dsh-fs-policy)', () => { const result = await call('edit', { file_path: 'a.txt', old_string: 'world', new_string: 'there' }) expect(result.isError).toBe(true) expect(result.error).toMatchObject({ info: { code: 'FS_NOT_OBSERVED' } }) + // The policy's refusal reaches the model with the read remedy appended. + expect(text(result)).toContain('edit requires reading') + expect(text(result)).toContain('read the file, then retry') expect(await readFile(join(dir, 'a.txt'), 'utf8')).toBe('hello world') }) @@ -155,6 +178,23 @@ describe('default deployment (with dsh-fs-policy)', () => { const result = await call('edit', { file_path: 'a.txt', old_string: 'world', new_string: 'there' }) expect(result.isError).toBe(true) expect(result.error).toMatchObject({ info: { code: 'FS_STALE_VERSION' } }) + // The model-facing text names the remedy, not just the condition. + expect(text(result)).toContain('file changed since it was read') + expect(text(result)).toContain('re-read the file, then retry') + }) + + it('the stale remedy is actionable: re-reading the changed file unblocks the retried edit', async () => { + await writeFile(join(dir, 'a.txt'), 'hello world') + await call('read', { file_path: 'a.txt' }) + await writeFile(join(dir, 'a.txt'), 'hello brave world') // out-of-band change + const stale = await call('edit', { file_path: 'a.txt', old_string: 'world', new_string: 'there' }) + expect(stale.isError).toBe(true) + expect(stale.error).toMatchObject({ info: { code: 'FS_STALE_VERSION' } }) + // Follow the remedy: re-read (refreshes the observed version), then retry. + expect((await call('read', { file_path: 'a.txt' })).isError).toBe(false) + const retried = await call('edit', { file_path: 'a.txt', old_string: 'world', new_string: 'there' }) + expect(retried.isError).toBe(false) + expect(await readFile(join(dir, 'a.txt'), 'utf8')).toBe('hello brave there') }) it('rejects an ambiguous match without replace_all', async () => { @@ -194,6 +234,43 @@ describe('default deployment (with dsh-fs-policy)', () => { }) }) + describe('deleted observed target (fail-closed corner)', () => { + it('a deleted observed file stays un-writable and un-editable in-session: the remedy cannot unblock it', async () => { + await writeFile(join(dir, 'a.txt'), 'original') + await call('read', { file_path: 'a.txt' }) + await rm(join(dir, 'a.txt')) // out-of-band deletion + + // Edit of the missing target: stale (the missing-target path shares the + // stale code and the re-read remedy). + const edit = await call('edit', { file_path: 'a.txt', old_string: 'original', new_string: 'x' }) + expect(edit.isError).toBe(true) + expect(edit.error).toMatchObject({ info: { code: 'FS_STALE_VERSION' } }) + + // Re-reading the missing file FAILS with FS_NOT_FOUND and records no + // observation, so the retried edit fails identically: the observed entry + // is never cleared for a deleted target. + const reread = await call('read', { file_path: 'a.txt' }) + expect(reread.isError).toBe(true) + expect(reread.error).toMatchObject({ info: { code: 'FS_NOT_FOUND' } }) + const retriedEdit = await call('edit', { file_path: 'a.txt', old_string: 'original', new_string: 'x' }) + expect(retriedEdit.isError).toBe(true) + expect(retriedEdit.error).toMatchObject({ info: { code: 'FS_STALE_VERSION' } }) + + // Write cannot recreate it either: the stale observation still forces + // replaceIfVersion, which rejects a missing target ("file no longer exists"). + const write = await call('write', { file_path: 'a.txt', content: 'fresh' }) + expect(write.isError).toBe(true) + expect(write.error).toMatchObject({ info: { code: 'FS_STALE_VERSION' } }) + + // The dead end lifts once the file exists again and is freshly observed. + await writeFile(join(dir, 'a.txt'), 'restored') + expect((await call('read', { file_path: 'a.txt' })).isError).toBe(false) + const recovered = await call('write', { file_path: 'a.txt', content: 'fresh' }) + expect(recovered.isError).toBe(false) + expect(await readFile(join(dir, 'a.txt'), 'utf8')).toBe('fresh') + }) + }) + describe('stat budget', () => { it('read stats once; write and edit never stat in the tool (the gate stats zero too)', async () => { await writeFile(join(dir, 'a.txt'), 'hello world') @@ -264,6 +341,9 @@ describe('bare provider (no dsh-fs-policy)', () => { const result = await call('edit', { file_path: 'missing.txt', old_string: 'a', new_string: 'b' }) expect(result.isError).toBe(true) expect(result.error).toMatchObject({ info: { code: 'FS_STALE_VERSION' } }) + // Even without policy, the stale text carries the re-read remedy. + expect(text(result)).toContain('file changed since it was read') + expect(text(result)).toContain('re-read the file, then retry') }) it('edit still enforces literal-match codes (FS_EDIT_NOT_FOUND), unrelated to freshness', async () => { diff --git a/packages/fs/tool-fs/tests/tools.spec.ts b/packages/fs/tool-fs/tests/tools.spec.ts index 914a1bf7de..ad01237c2b 100644 --- a/packages/fs/tool-fs/tests/tools.spec.ts +++ b/packages/fs/tool-fs/tests/tools.spec.ts @@ -397,12 +397,13 @@ describe('write tool', () => { expect(text(result)).toContain('file_path must be a non-empty string') }) - it('propagates a backend FsError as an isError result carrying its code', async () => { + it('propagates a backend FsError as an isError result carrying its code and remedy', async () => { const { ctx, fs } = await setup() fs.rejectWith = new FsError('blocked', 'FS_STALE_VERSION') const result = await call(ctx, 'write', { file_path: 'a.txt', content: 'hi' }) expect(result.isError).toBe(true) expect(result.error).toMatchObject({ info: { name: 'FsError', code: 'FS_STALE_VERSION' } }) + expect(text(result)).toContain('re-read the file, then retry') }) }) From 044df0e0c8b30f1dab97db8e473317dc66b060c0 Mon Sep 17 00:00:00 2001 From: _Kerman Date: Mon, 3 Aug 2026 19:49:38 +0800 Subject: [PATCH 2/2] docs(notes): record model-facing error remedy decision The tool-fs error wrapper decision: guarded-mutation failures gain their recovery instruction at the model boundary while the provider messages and structured codes stay unchanged; includes the deleted-target fail-closed corner. --- .../2026-08-03-fs-tool-error-remedy.i18n.yaml | 6 ++++ .../2026-08-03-fs-tool-error-remedy.md | 32 +++++++++++++++++++ .../2026-08-03-fs-tool-error-remedy.zh.md | 32 +++++++++++++++++++ 3 files changed, 70 insertions(+) create mode 100644 .agents/notes/implemented/feature/2026-08-03-fs-tool-error-remedy.i18n.yaml create mode 100644 .agents/notes/implemented/feature/2026-08-03-fs-tool-error-remedy.md create mode 100644 .agents/notes/implemented/feature/2026-08-03-fs-tool-error-remedy.zh.md diff --git a/.agents/notes/implemented/feature/2026-08-03-fs-tool-error-remedy.i18n.yaml b/.agents/notes/implemented/feature/2026-08-03-fs-tool-error-remedy.i18n.yaml new file mode 100644 index 0000000000..98500c284e --- /dev/null +++ b/.agents/notes/implemented/feature/2026-08-03-fs-tool-error-remedy.i18n.yaml @@ -0,0 +1,6 @@ +# Bilingual-pair consistency record (docs/i18n/README.md): the git blob hash of each +# side as of the last confirmed-consistent state. Both languages carry equal authority; +# after editing either side, bring the other along and re-record with: +# pnpm run verify-translation-pairing --write .agents/notes/implemented/feature/2026-08-03-fs-tool-error-remedy.md +2026-08-03-fs-tool-error-remedy.md: f227c31365725652b130e097d70c79d3daab3684 +2026-08-03-fs-tool-error-remedy.zh.md: 11acd0cf48924833ced91591d5ea1424735969cd diff --git a/.agents/notes/implemented/feature/2026-08-03-fs-tool-error-remedy.md b/.agents/notes/implemented/feature/2026-08-03-fs-tool-error-remedy.md new file mode 100644 index 0000000000..f227c31365 --- /dev/null +++ b/.agents/notes/implemented/feature/2026-08-03-fs-tool-error-remedy.md @@ -0,0 +1,32 @@ +# Agent Note: Guarded-mutation errors append the recovery instruction at the model boundary + +Status: implemented + +English | [中文](2026-08-03-fs-tool-error-remedy.zh.md) + +## Problem + +Guarded `write` and `edit` failures reach the model with messages that state the condition but not the only correct recovery: `FS_STALE_VERSION` ("file changed since it was read") and `FS_NOT_OBSERVED` ("edit requires reading … first"). The model must guess that the recovery is a re-read (or a first read) followed by a retry, and the retry/permission/UI layers that route on the structured code see the same message text. The provider-owned messages are part of the storage seam's machine-oriented vocabulary ([filesystem capability seam](../architecture/2026-06-17-filesystem-capability-seam.md)), so the remedy cannot live there without leaking model-facing wording into every consumer of `FsError`. + +## Decision + +`dsh-tool-fs` owns a model-facing error wrapper, `remediateFsError` in `src/error.ts`, applied in `write.ts` and `edit.ts` after the sandbox denial mapping. It appends the recovery instruction to the two guarded-mutation codes and passes everything else through untouched: + +- `FS_STALE_VERSION` (including a missing edit target, which shares the stale code) gains `— re-read the file, then retry`. +- `FS_NOT_OBSERVED` gains `— read the file, then retry`. + +The structured `FsError` code is preserved so retry/permission/UI layers keep routing on it, and the original error chains as `cause`. Provider messages stay machine-oriented and unchanged. + +In `edit.ts` the `fs/edit-intent` waterfall now sits inside the same `try` as the provider mutation, so the policy plugin's `FS_NOT_OBSERVED` refusal thrown from the intent slot also receives the remedy — both refusal paths reach the model with the same recovery wording. + +## Alternatives considered + +- **Append the remedy to the provider messages in `dsh-fs` / `dsh-fs-local`.** Rejected because those messages are machine-oriented seam vocabulary consumed by retry, permission, and UI layers as well as the model surface; model-facing wording belongs at the model boundary, where `dsh-tool-fs` already owns result formatting ([filesystem capability seam](../architecture/2026-06-17-filesystem-capability-seam.md)). +- **Add the recovery to prompt guidance instead.** Rejected because the failure arrives mid-task; a static instruction does not reliably reach the retry decision, while the error message is present exactly when the model must act. +- **Signal the remedy with a new `FsError` code.** Rejected because the two failures are the same conditions retry layers already handle; splitting the code would fork routing on identical semantics. + +## Consequences + +Model-visible text for the two codes changes; the `fs-policy-reject` keyless snapshot is re-recorded, and the READMEs of `dsh-tool-fs` and `dsh-fs-policy` pin the exact appended text. Unit tests cover the wrapper directly (remedy text, code preservation, cause chaining, passthrough of other codes and non-`FsError` values) and the assembled tool paths assert the remedy reaches the model for both codes. + +The remedy is not a promise: a deleted observed target cannot be unblocked, because re-reading a missing file fails with `FS_NOT_FOUND` and records no observation. That dead end is pinned fail-closed in the integration tests — the retried mutation fails identically until the target exists again and is freshly observed. diff --git a/.agents/notes/implemented/feature/2026-08-03-fs-tool-error-remedy.zh.md b/.agents/notes/implemented/feature/2026-08-03-fs-tool-error-remedy.zh.md new file mode 100644 index 0000000000..11acd0cf48 --- /dev/null +++ b/.agents/notes/implemented/feature/2026-08-03-fs-tool-error-remedy.zh.md @@ -0,0 +1,32 @@ +# Agent Note: Guarded-mutation errors append the recovery instruction at the model boundary + +Status: implemented + +[English](2026-08-03-fs-tool-error-remedy.md) | 中文 + +## Problem + +受防护的 `write` 与 `edit` 失败以只陈述条件、不给出唯一正确恢复方式的消息到达模型：`FS_STALE_VERSION`（"file changed since it was read"）与 `FS_NOT_OBSERVED`（"edit requires reading … first"）。模型必须自行猜测恢复方式是重新读取（或首次读取）后重试，而基于结构化错误码路由的重试/权限/UI 层看到的也是同一段消息文本。提供方拥有的消息属于存储接缝的面向机器词汇（[filesystem capability seam](../architecture/2026-06-17-filesystem-capability-seam.md)），因此恢复指令不能放在那里，否则会把面向模型的措辞泄漏给 `FsError` 的每个消费者。 + +## Decision + +`dsh-tool-fs` 拥有一个面向模型的错误包装 `remediateFsError`（位于 `src/error.ts`），在 `write.ts` 与 `edit.ts` 中于沙箱拒绝映射之后应用。它为两个受防护变更错误码追加恢复指令，其余错误原样透传： + +- `FS_STALE_VERSION`（包括缺失的编辑目标——它与陈旧错误共用同一错误码）追加 `— re-read the file, then retry`。 +- `FS_NOT_OBSERVED` 追加 `— read the file, then retry`。 + +结构化 `FsError` 错误码保持不变，使重试/权限/UI 层继续基于它路由；原始错误作为 `cause` 链入。提供方消息保持面向机器且不变。 + +在 `edit.ts` 中，`fs/edit-intent` waterfall 现在与提供方变更位于同一个 `try` 内，因此策略插件从 intent 槽抛出的 `FS_NOT_OBSERVED` 拒绝也会获得恢复指令——两条拒绝路径都以相同的恢复措辞到达模型。 + +## Alternatives considered + +- **在 `dsh-fs` / `dsh-fs-local` 的提供方消息中追加恢复指令。** 被拒绝：这些消息是面向机器的接缝词汇，除模型表面外还被重试、权限与 UI 层消费；面向模型的措辞应位于模型边界，即 `dsh-tool-fs` 已经拥有结果格式化之处（[filesystem capability seam](../architecture/2026-06-17-filesystem-capability-seam.md)）。 +- **改为在提示词引导中加入恢复方式。** 被拒绝：失败发生在任务中途；静态指令无法可靠地影响重试决策，而错误消息恰好在模型必须行动时出现。 +- **用新的 `FsError` 错误码表达恢复指令。** 被拒绝：这两种失败本就是重试层已处理的相同条件；拆分错误码会让语义相同的路由分叉。 + +## Consequences + +两个错误码的模型可见文本发生变化；`fs-policy-reject` 无密钥快照被重新录制，`dsh-tool-fs` 与 `dsh-fs-policy` 的 README 逐字固定追加后的文本。单元测试直接覆盖包装器（恢复指令文本、错误码保留、cause 链、其他错误码与非 `FsError` 值的透传），组装后的工具路径断言两个错误码的恢复指令都到达模型。 + +恢复指令不是承诺：已删除的观察目标无法被解除阻塞，因为重新读取缺失文件会以 `FS_NOT_FOUND` 失败且不记录观察。这一死胡同在集成测试中以 fail-closed 方式固定——在目标重新存在并被新鲜观察之前，重试的变更以相同方式失败。