From 36d45eca9cd87f4ef05f9527c02e2bb5b9283e16 Mon Sep 17 00:00:00 2001 From: sjw1231 Date: Thu, 5 Nov 2026 17:28:43 +0800 Subject: [PATCH] feat(str-replace-editor): give argument and create failures error codes Hand-checked argument failures (relative or empty path, missing command parameters, invalid view_range, out-of-range insert_line, null new_str) throw ToolArgsError (INVALID_ARGS), and create on an existing path throws FsError FS_ALREADY_EXISTS, so tool results persist error.info. --- .../tool-str-replace-editor/README.i18n.yaml | 4 +- packages/fs/tool-str-replace-editor/README.md | 2 +- .../fs/tool-str-replace-editor/README.zh.md | 2 +- .../fs/tool-str-replace-editor/src/index.ts | 36 +++++------ .../tests/tools.spec.ts | 60 ++++++++++--------- 5 files changed, 54 insertions(+), 50 deletions(-) diff --git a/packages/fs/tool-str-replace-editor/README.i18n.yaml b/packages/fs/tool-str-replace-editor/README.i18n.yaml index 7bd12ad505..b1cc3a78da 100644 --- a/packages/fs/tool-str-replace-editor/README.i18n.yaml +++ b/packages/fs/tool-str-replace-editor/README.i18n.yaml @@ -27,8 +27,8 @@ en: 554e8446946fbd96 zh: 3af12627c19a0111 /deepseek-ai-dsh-tool-str-replace-editor/use-this-package/failures-and-recovery: - en: 2b249fcb0b5621aa - zh: d172b3de43c337c4 + en: 68d14f5993c734d5 + zh: 36eab11474947735 /deepseek-ai-dsh-tool-str-replace-editor/understand-the-implementation: en: b0b36a22cd4421eb zh: db47ef5b71dd51df diff --git a/packages/fs/tool-str-replace-editor/README.md b/packages/fs/tool-str-replace-editor/README.md index f1880a5217..563ddf6ce1 100644 --- a/packages/fs/tool-str-replace-editor/README.md +++ b/packages/fs/tool-str-replace-editor/README.md @@ -52,7 +52,7 @@ A backend, optionally the policy plugin, then the tool; the editor composes with ### Failures and recovery -A metadata miss from `view`, `str_replace`, or `insert` records confirmed absence before returning `FS_NOT_FOUND`, so a later `create` can recover an externally deleted path through the mounted policy's guarded-create flow; absence never authorizes `str_replace` or `insert`. Guarded mutations inherit the policy plugin's codes and remedies — `FS_NOT_OBSERVED` (read the file, then retry), `FS_STALE_VERSION` (re-read, then retry) — and sandbox denials surface as the `[sandbox: file access denied under mode]` marker. Paths must be absolute; a relative path is refused with a hint. +A metadata miss from `view`, `str_replace`, or `insert` records confirmed absence before returning `FS_NOT_FOUND`, so a later `create` can recover an externally deleted path through the mounted policy's guarded-create flow; absence never authorizes `str_replace` or `insert`. Guarded mutations inherit the policy plugin's codes and remedies — `FS_NOT_OBSERVED` (read the file, then retry), `FS_STALE_VERSION` (re-read, then retry) — and sandbox denials surface as the `[sandbox: file access denied under mode]` marker. Paths must be absolute; a relative path is refused with a hint. Argument mistakes the schema cannot express, such as a relative path, a missing command parameter, or an out-of-range `view_range` or `insert_line`, throw `ToolArgsError` with `INVALID_ARGS`; `create` on an existing path reports `FS_ALREADY_EXISTS`. ----- diff --git a/packages/fs/tool-str-replace-editor/README.zh.md b/packages/fs/tool-str-replace-editor/README.zh.md index 9c69505356..5182960718 100644 --- a/packages/fs/tool-str-replace-editor/README.zh.md +++ b/packages/fs/tool-str-replace-editor/README.zh.md @@ -52,7 +52,7 @@ kind: "package-reference" ### 失败与恢复 -`view`、`str_replace` 或 `insert` 发生元数据未命中时，工具会在返回 `FS_NOT_FOUND` 前记录确认缺失，因此后续 `create` 可以通过已挂载策略的防护创建流程恢复外部删除的路径；缺失状态绝不会授权 `str_replace` 或 `insert`。防护变更继承策略插件的错误码与恢复指令——`FS_NOT_OBSERVED`（先读取文件再重试）、`FS_STALE_VERSION`（先重新读取再重试）——沙箱拒绝则表现为 `[sandbox: file access denied under mode]` 标记。路径必须是绝对路径；相对路径会被拒绝并给出提示。 +`view`、`str_replace` 或 `insert` 发生元数据未命中时，工具会在返回 `FS_NOT_FOUND` 前记录确认缺失，因此后续 `create` 可以通过已挂载策略的防护创建流程恢复外部删除的路径；缺失状态绝不会授权 `str_replace` 或 `insert`。防护变更继承策略插件的错误码与恢复指令——`FS_NOT_OBSERVED`（先读取文件再重试）、`FS_STALE_VERSION`（先重新读取再重试）——沙箱拒绝则表现为 `[sandbox: file access denied under mode]` 标记。路径必须是绝对路径；相对路径会被拒绝并给出提示。参数模式无法表达的参数错误，例如相对路径、缺少命令参数或 `view_range`、`insert_line` 越界，抛出 `ToolArgsError`，错误码为 `INVALID_ARGS`；对已存在路径执行 `create` 报告 `FS_ALREADY_EXISTS`。 ----- diff --git a/packages/fs/tool-str-replace-editor/src/index.ts b/packages/fs/tool-str-replace-editor/src/index.ts index 3542ee1770..099d1a6d73 100644 --- a/packages/fs/tool-str-replace-editor/src/index.ts +++ b/packages/fs/tool-str-replace-editor/src/index.ts @@ -12,7 +12,7 @@ import { truncateWithoutSplittingSurrogatePair } from '@deepseek-ai/dsh-output-r import { sandboxDenialMarker } from '@deepseek-ai/dsh-sandbox' import type { SandboxExecutionPolicy } from '@deepseek-ai/dsh-sandbox' import type { SandboxPolicyService } from '@deepseek-ai/dsh-sandbox-policy' -import { defineTool } from '@deepseek-ai/dsh-tools' +import { ToolArgsError, defineTool } from '@deepseek-ai/dsh-tools' import type { ToolCallView, ToolRunContext } from '@deepseek-ai/dsh-tools' const TRUNCATED_MESSAGE = 'To save on context only part of this file has been shown to you. You should retry this tool after you have searched inside the file with `grep -n` in order to find the line numbers of what you are looking for.' @@ -92,9 +92,9 @@ async function resolveTarget( path: string, signal: AbortSignal, ): Promise { - if (path.trim().length === 0) throw new Error('path must be a non-empty string') + if (path.trim().length === 0) throw new ToolArgsError(['path must be a non-empty string']) if (!isAbsolute(path)) { - throw new Error(`The path ${path} is not an absolute path, it should start with \`/\`. Maybe you meant /${path}?`) + throw new ToolArgsError([`The path ${path} is not an absolute path, it should start with \`/\`. Maybe you meant /${path}?`]) } return ctx.fs.resolve(path, { signal }) } @@ -128,9 +128,9 @@ function requiredForCommand( command: string, allowEmpty = true, ): string { - if (value === undefined) throw new Error(`Parameter \`${parameter}\` is required for command: ${command}`) + if (value === undefined) throw new ToolArgsError([`Parameter \`${parameter}\` is required for command: ${command}`]) if (!allowEmpty && value.length === 0) { - throw new Error(`Parameter \`${parameter}\` is empty for command: ${command}`) + throw new ToolArgsError([`Parameter \`${parameter}\` is empty for command: ${command}`]) } return value } @@ -154,24 +154,24 @@ function formatFileView( || requestedFinalLine === undefined || !viewRange.every(Number.isInteger) ) { - throw new Error('Invalid `view_range`. It should be a list of two integers.') + throw new ToolArgsError(['Invalid `view_range`. It should be a list of two integers.']) } initialLine = requestedInitialLine finalLine = requestedFinalLine if (initialLine < 1 || initialLine > allLines.length) { - throw new Error( + throw new ToolArgsError([ `Invalid \`view_range\`: [${viewRange.join(', ')}]. Its first element \`${initialLine}\` should be within the range of lines of the file: [1, ${allLines.length}]`, - ) + ]) } if (finalLine > allLines.length) { - throw new Error( + throw new ToolArgsError([ `Invalid \`view_range\`: [${viewRange.join(', ')}]. Its second element \`${finalLine}\` should be smaller than the number of lines in the file: \`${allLines.length}\``, - ) + ]) } if (finalLine !== -1 && finalLine < initialLine) { - throw new Error( + throw new ToolArgsError([ `Invalid \`view_range\`: [${viewRange.join(', ')}]. Its second element \`${finalLine}\` should be larger or equal than its first \`${initialLine}\``, - ) + ]) } lines = finalLine === -1 ? allLines.slice(initialLine - 1) @@ -226,7 +226,7 @@ async function viewPath( const info = await statExisting(ctx, target, 'view', exec) if (info.type === 'directory') { if (viewRange !== undefined) { - throw new Error('The `view_range` parameter is not allowed when `path` points to a directory.') + throw new ToolArgsError(['The `view_range` parameter is not allowed when `path` points to a directory.']) } return listDirectory(ctx, target, maxOutputChars, exec) } @@ -249,7 +249,7 @@ async function createFile( const sandboxPolicy = policy.resolve(exec) const target = await resolveTarget(ctx, path, exec.signal) if (await ctx.fs.stat(target, exec.signal) !== undefined) { - throw new Error(`File already exists at: ${target.displayPath}. Cannot overwrite files using command \`create\`.`) + throw new FsError(`File already exists at: ${target.displayPath}. Cannot overwrite files using command \`create\`.`, 'FS_ALREADY_EXISTS') } const intent = await ctx.waterfall( 'fs/write-intent', @@ -282,7 +282,7 @@ async function replaceInFile( exec: ToolRunContext, ): Promise { if (newStr === null) { - throw new Error('Parameter `new_str` must be omitted or contain a string for command: str_replace') + throw new ToolArgsError(['Parameter `new_str` must be omitted or contain a string for command: str_replace']) } const sandboxPolicy = policy.resolve(exec) const target = await resolveTarget(ctx, path, exec.signal) @@ -335,7 +335,7 @@ async function insertInFile( newStr: string | undefined, exec: ToolRunContext, ): Promise { - if (insertLine === undefined) throw new Error('Parameter `insert_line` is required for command: insert') + if (insertLine === undefined) throw new ToolArgsError(['Parameter `insert_line` is required for command: insert']) const value = requiredForCommand(newStr, 'new_str', 'insert') const sandboxPolicy = policy.resolve(exec) const target = await resolveTarget(ctx, path, exec.signal) @@ -347,9 +347,9 @@ async function insertInFile( const before = await ctx.fs.readText(target, exec.signal) const lines = before.split('\n') if (!Number.isInteger(insertLine) || insertLine < 0 || insertLine > lines.length) { - throw new Error( + throw new ToolArgsError([ `Invalid \`insert_line\` parameter: ${insertLine}. It should be within the range of lines of the file: [0, ${lines.length}]`, - ) + ]) } const after = [ ...lines.slice(0, insertLine), diff --git a/packages/fs/tool-str-replace-editor/tests/tools.spec.ts b/packages/fs/tool-str-replace-editor/tests/tools.spec.ts index 14de14c4cd..e08c539c59 100644 --- a/packages/fs/tool-str-replace-editor/tests/tools.spec.ts +++ b/packages/fs/tool-str-replace-editor/tests/tools.spec.ts @@ -460,6 +460,7 @@ describe('tool-str-replace-editor', () => { const relative = await call(ctx, owner, { command: 'view', path: 'ambiguous.txt' }) expect(relative.isError).toBe(true) expect(text(relative)).toContain('is not an absolute path') + expect(relative.error?.info).toEqual({ name: 'ToolArgsError', code: 'INVALID_ARGS' }) expect(await readFile(ambiguous, 'utf8')).toBe('alpha\nbeta\nmiddle\nalpha\nbeta') }) @@ -476,35 +477,38 @@ describe('tool-str-replace-editor', () => { await writeFile(threeLines, 'one\ntwo\nthree') await mkdir(directory) - const cases = [ - { command: null, path: ambiguous }, - { command: 'view', path: null }, - { command: 'view', path: '' }, - { command: 'view', path: join(root, 'missing.txt') }, - { command: 'view', path: ambiguous, view_range: [1] }, - { command: 'view', path: ambiguous, view_range: [0, 1] }, - { command: 'view', path: ambiguous, view_range: [1.5, 2] }, - { command: 'view', path: threeLines, view_range: [1, 99] }, - { command: 'view', path: threeLines, view_range: [2, 1] }, - { command: 'view', path: directory, view_range: [1, 1] }, - { command: 'create', path: join(root, 'new.txt') }, - { command: 'create', path: join(root, 'new.txt'), file_text: null }, - { command: 'create', path: ambiguous, file_text: 'overwrite' }, - { command: 'str_replace', path: ambiguous, new_str: 'x' }, - { command: 'str_replace', path: ambiguous, old_str: null, new_str: 'x' }, - { command: 'str_replace', path: ambiguous, old_str: 'same same', new_str: null }, - { command: 'str_replace', path: ambiguous, old_str: '', new_str: 'x' }, - { command: 'insert', path: ambiguous, new_str: 'x' }, - { command: 'insert', path: ambiguous, insert_line: null, new_str: 'x' }, - { command: 'insert', path: ambiguous, insert_line: 0, new_str: null }, - { command: 'insert', path: ambiguous, insert_line: -1, new_str: 'x' }, - { command: 'insert', path: ambiguous, insert_line: 1.5, new_str: 'x' }, - { command: 'insert', path: ambiguous, insert_line: 99, new_str: 'x' }, - { command: 'insert', path: empty, insert_line: 2, new_str: 'x' }, - { command: 'insert', path: directory, insert_line: 0, new_str: 'x' }, + const args = { name: 'ToolArgsError', code: 'INVALID_ARGS' } + const cases: [Record, { name: string; code: string }][] = [ + [{ command: null, path: ambiguous }, args], + [{ command: 'view', path: null }, args], + [{ command: 'view', path: '' }, args], + [{ command: 'view', path: join(root, 'missing.txt') }, { name: 'FsError', code: 'FS_NOT_FOUND' }], + [{ command: 'view', path: ambiguous, view_range: [1] }, args], + [{ command: 'view', path: ambiguous, view_range: [0, 1] }, args], + [{ command: 'view', path: ambiguous, view_range: [1.5, 2] }, args], + [{ command: 'view', path: threeLines, view_range: [1, 99] }, args], + [{ command: 'view', path: threeLines, view_range: [2, 1] }, args], + [{ command: 'view', path: directory, view_range: [1, 1] }, args], + [{ command: 'create', path: join(root, 'new.txt') }, args], + [{ command: 'create', path: join(root, 'new.txt'), file_text: null }, args], + [{ command: 'create', path: ambiguous, file_text: 'overwrite' }, { name: 'FsError', code: 'FS_ALREADY_EXISTS' }], + [{ command: 'str_replace', path: ambiguous, new_str: 'x' }, args], + [{ command: 'str_replace', path: ambiguous, old_str: null, new_str: 'x' }, args], + [{ command: 'str_replace', path: ambiguous, old_str: 'same same', new_str: null }, args], + [{ command: 'str_replace', path: ambiguous, old_str: '', new_str: 'x' }, args], + [{ command: 'insert', path: ambiguous, new_str: 'x' }, args], + [{ command: 'insert', path: ambiguous, insert_line: null, new_str: 'x' }, args], + [{ command: 'insert', path: ambiguous, insert_line: 0, new_str: null }, args], + [{ command: 'insert', path: ambiguous, insert_line: -1, new_str: 'x' }, args], + [{ command: 'insert', path: ambiguous, insert_line: 1.5, new_str: 'x' }, args], + [{ command: 'insert', path: ambiguous, insert_line: 99, new_str: 'x' }, args], + [{ command: 'insert', path: empty, insert_line: 2, new_str: 'x' }, args], + [{ command: 'insert', path: directory, insert_line: 0, new_str: 'x' }, { name: 'FsError', code: 'FS_NOT_REGULAR_FILE' }], ] - for (const args of cases) { - expect((await call(ctx, owner, args)).isError).toBe(true) + for (const [callArgs, info] of cases) { + const result = await call(ctx, owner, callArgs) + expect(result.isError).toBe(true) + expect({ args: callArgs, info: result.error?.info }).toEqual({ args: callArgs, info }) } expect(await readFile(ambiguous, 'utf8')).toBe('same same')