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.
This commit is contained in:
1 parent
4e4b4eb9b6
commit
36d45eca9c
5 files changed
+54
-50
No files matched your search
@@ -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
|
||||
|
||||
@@ -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> 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> 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`.
|
||||
|
||||
-----
|
||||
|
||||
|
||||
@@ -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> mode]` 标记。路径必须是绝对路径;相对路径会被拒绝并给出提示。
|
||||
`view`、`str_replace` 或 `insert` 发生元数据未命中时,工具会在返回 `FS_NOT_FOUND` 前记录确认缺失,因此后续 `create` 可以通过已挂载策略的防护创建流程恢复外部删除的路径;缺失状态绝不会授权 `str_replace` 或 `insert`。防护变更继承策略插件的错误码与恢复指令——`FS_NOT_OBSERVED`(先读取文件再重试)、`FS_STALE_VERSION`(先重新读取再重试)——沙箱拒绝则表现为 `[sandbox: file access denied under <mode> mode]` 标记。路径必须是绝对路径;相对路径会被拒绝并给出提示。参数模式无法表达的参数错误,例如相对路径、缺少命令参数或 `view_range`、`insert_line` 越界,抛出 `ToolArgsError`,错误码为 `INVALID_ARGS`;对已存在路径执行 `create` 报告 `FS_ALREADY_EXISTS`。
|
||||
|
||||
-----
|
||||
|
||||
|
||||
@@ -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 = '<response clipped><NOTE>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.</NOTE>'
|
||||
@@ -92,9 +92,9 @@ async function resolveTarget(
|
||||
path: string,
|
||||
signal: AbortSignal,
|
||||
): Promise<FsTarget> {
|
||||
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<string> {
|
||||
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<string> {
|
||||
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),
|
||||
|
||||
@@ -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<string, unknown>, { 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')
|
||||
|
||||
|
||||
Reference in new issue
Block a user