fix(fs): keep one editor path contract
This commit is contained in:
@@ -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-str-replace-editor/README.md
|
||||
README.md: 8ac6a22f24ddcdd3818b346e3426e58e718027e2
|
||||
README.zh.md: cf82b132b63730af209c159066a70f6a18b77f39
|
||||
README.md: 12224537ab2ca2d2ba97e93fe8dc2192fa9ac1aa
|
||||
README.zh.md: 5481723f8a3077ee329ec202b12a67b678abc691
|
||||
|
||||
@@ -10,12 +10,10 @@ Standalone model-facing `str_replace_editor` over `ctx.fs`. It can be composed w
|
||||
|---|---:|---|
|
||||
| `maxOutputChars` | `16000` | Prefix characters retained for file and directory views. |
|
||||
| `description` | Editor command guide | Model-facing tool description. |
|
||||
| `requireAbsolutePath` | `true` | Reject relative paths; disable only for deployments with a deliberate session-cwd contract. |
|
||||
| `expandTabsOnMutation` | `true` | Preserve the canonical Claude SWE behavior that expands tabs across the whole file before replace/insert. Set `false` for atomic literal replacement that preserves unrelated tabs. |
|
||||
|
||||
## Tool
|
||||
|
||||
The schema provides `view`, `create`, `str_replace`, and `insert`. File views use one-based line numbers; directory views omit hidden, dependency, and Python-cache entries and descend two levels. Replacement requires one unique literal match and reports errors only in the public `old_str` vocabulary. Insert follows the selected zero-based insertion boundary without adding an implicit trailing newline.
|
||||
The schema provides `view`, `create`, `str_replace`, and `insert` over absolute paths. File views use one-based line numbers; directory views omit hidden, dependency, and Python-cache entries and descend two levels. Replacement requires one unique literal match and reports errors only in the public `old_str` vocabulary. Insert follows the selected zero-based insertion boundary without adding an implicit trailing newline. Mutations preserve tabs outside the requested edit.
|
||||
|
||||
## Model Experience
|
||||
|
||||
@@ -51,5 +49,4 @@ Append-only tool results follow the reusable request prefix.
|
||||
|
||||
- Operations target UTF-8 text; binary files are unsupported.
|
||||
- `str_replace` intentionally rejects zero or multiple matches and has no `replace_all` argument.
|
||||
- Canonical mode (`expandTabsOnMutation: true`) expands tabs in the entire file before replacement or insertion, including lines outside the edited region. Set it to `false` for Makefiles and other tab-sensitive files.
|
||||
- Every mutation goes through `fs/write-intent` or `fs/edit-intent`, resolves the current session sandbox policy, and delegates enforcement to the mounted filesystem and policy plugins.
|
||||
|
||||
@@ -10,12 +10,10 @@
|
||||
|---|---:|---|
|
||||
| `maxOutputChars` | `16000` | 文件和目录查看结果保留的前缀字符数。 |
|
||||
| `description` | 编辑器命令指南 | 面向模型的工具描述。 |
|
||||
| `requireAbsolutePath` | `true` | 拒绝相对路径;仅当部署明确约定 session cwd 时才应关闭。 |
|
||||
| `expandTabsOnMutation` | `true` | 保留 Claude SWE 参考行为:替换/插入前展开整个文件的制表符。设为 `false` 时使用原子字面量替换,并保留未触及的制表符。 |
|
||||
|
||||
## 工具
|
||||
|
||||
Schema 提供 `view`、`create`、`str_replace` 与 `insert`。文件查看使用从一开始的行号;目录查看忽略隐藏、依赖与 Python 缓存条目并下探两层。替换要求字面量唯一匹配,错误只使用公开的 `old_str` 词汇。插入遵循所选的零基插入边界,不会隐式补尾换行。
|
||||
Schema 提供针对绝对路径的 `view`、`create`、`str_replace` 与 `insert`。文件查看使用从一开始的行号;目录查看忽略隐藏、依赖与 Python 缓存条目并下探两层。替换要求字面量唯一匹配,错误只使用公开的 `old_str` 词汇。插入遵循所选的零基插入边界,不会隐式补尾换行。修改操作会保留请求编辑范围之外的制表符。
|
||||
|
||||
## 模型体验
|
||||
|
||||
@@ -51,5 +49,4 @@ Schema 提供 `view`、`create`、`str_replace` 与 `insert`。文件查看使
|
||||
|
||||
- 操作面向 UTF-8 文本,不支持二进制文件。
|
||||
- `str_replace` 刻意拒绝零匹配或多匹配,且没有 `replace_all` 参数。
|
||||
- 规范模式(`expandTabsOnMutation: true`)会在替换或插入前展开整个文件中的制表符,包括未编辑区域。Makefile 等依赖制表符的文件应设为 `false`。
|
||||
- 每个修改操作都会经过 `fs/write-intent` 或 `fs/edit-intent`,解析当前 session 的沙箱策略,并交由挂载的文件系统与策略插件执行。
|
||||
|
||||
@@ -105,12 +105,11 @@ class MutationPolicy {
|
||||
async function resolveTarget(
|
||||
ctx: Context,
|
||||
path: string,
|
||||
requireAbsolutePath: boolean,
|
||||
exec: ToolRunContext,
|
||||
workspaceRoot?: string,
|
||||
): Promise<FsTarget> {
|
||||
if (path.trim().length === 0) throw new Error('path must be a non-empty string')
|
||||
if (requireAbsolutePath && !isAbsolute(path)) {
|
||||
if (!isAbsolute(path)) {
|
||||
throw new Error(`The path ${path} is not an absolute path, it should start with \`/\`. Maybe you meant /${path}?`)
|
||||
}
|
||||
const cwd = exec.agent?.session.header.cwd ?? workspaceRoot
|
||||
@@ -237,10 +236,9 @@ async function viewPath(
|
||||
path: string,
|
||||
viewRange: number[] | undefined,
|
||||
maxOutputChars: number,
|
||||
requireAbsolutePath: boolean,
|
||||
exec: ToolRunContext,
|
||||
): Promise<string> {
|
||||
const target = await resolveTarget(ctx, path, requireAbsolutePath, exec)
|
||||
const target = await resolveTarget(ctx, path, exec)
|
||||
const info = await statExisting(ctx, target, 'view', exec)
|
||||
if (info.type === 'directory') {
|
||||
if (viewRange !== undefined) {
|
||||
@@ -261,12 +259,11 @@ async function createFile(
|
||||
policy: MutationPolicy,
|
||||
path: string,
|
||||
fileText: string | undefined,
|
||||
requireAbsolutePath: boolean,
|
||||
exec: ToolRunContext,
|
||||
): Promise<string> {
|
||||
const content = requiredForCommand(fileText, 'file_text', 'create')
|
||||
const sandboxPolicy = policy.resolve(exec)
|
||||
const target = await resolveTarget(ctx, path, requireAbsolutePath, exec, sandboxPolicy?.workspaceRoot)
|
||||
const target = await resolveTarget(ctx, path, exec, sandboxPolicy?.workspaceRoot)
|
||||
if (await ctx.fs.stat(target, exec.signal) !== undefined) {
|
||||
throw new Error(`File already exists at: ${target.displayPath}. Cannot overwrite files using command \`create\`.`)
|
||||
}
|
||||
@@ -298,11 +295,10 @@ async function replaceInFile(
|
||||
path: string,
|
||||
oldStr: string | undefined,
|
||||
newStr: string | undefined,
|
||||
requireAbsolutePath: boolean,
|
||||
exec: ToolRunContext,
|
||||
): Promise<string> {
|
||||
const sandboxPolicy = policy.resolve(exec)
|
||||
const target = await resolveTarget(ctx, path, requireAbsolutePath, exec, sandboxPolicy?.workspaceRoot)
|
||||
const target = await resolveTarget(ctx, path, exec, sandboxPolicy?.workspaceRoot)
|
||||
const intent = await ctx.waterfall('fs/edit-intent', target, exec, () => undefined)
|
||||
const oldValue = requiredForCommand(oldStr, 'old_str', 'str_replace', false)
|
||||
const newValue = newStr ?? ''
|
||||
@@ -327,10 +323,12 @@ async function replaceInFile(
|
||||
}
|
||||
let outcome
|
||||
try {
|
||||
outcome = await ctx.fs.editText(
|
||||
outcome = await ctx.fs.writeText(
|
||||
target,
|
||||
{ oldString: oldValue, newString: newValue, replaceAll: false },
|
||||
intent ?? { version: info.version },
|
||||
before.replace(oldValue, newValue),
|
||||
intent === undefined
|
||||
? { kind: 'replaceIfVersion', version: info.version }
|
||||
: { kind: 'replaceIfVersion', version: intent.version },
|
||||
exec.signal,
|
||||
sandboxPolicy,
|
||||
)
|
||||
@@ -347,13 +345,12 @@ async function insertInFile(
|
||||
path: string,
|
||||
insertLine: number | undefined,
|
||||
newStr: string | undefined,
|
||||
requireAbsolutePath: boolean,
|
||||
exec: ToolRunContext,
|
||||
): Promise<string> {
|
||||
if (insertLine === undefined) throw new Error('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, requireAbsolutePath, exec, sandboxPolicy?.workspaceRoot)
|
||||
const target = await resolveTarget(ctx, path, exec, sandboxPolicy?.workspaceRoot)
|
||||
const intent = await ctx.waterfall('fs/edit-intent', target, exec, () => undefined)
|
||||
const info = await statExisting(ctx, target, 'insert', exec)
|
||||
if (info.type !== 'file') {
|
||||
@@ -387,7 +384,6 @@ async function insertInFile(
|
||||
interface ResolvedConfig {
|
||||
maxOutputChars: number
|
||||
description: string
|
||||
requireAbsolutePath: boolean
|
||||
}
|
||||
|
||||
function presentEditorCall(args: {
|
||||
@@ -484,9 +480,9 @@ function registerStrReplaceEditor(ctx: Context, config: ResolvedConfig): void {
|
||||
async execute(args, exec) {
|
||||
switch (args.command) {
|
||||
case 'view':
|
||||
return viewPath(ctx, args.path, args.view_range, config.maxOutputChars, config.requireAbsolutePath, exec)
|
||||
return viewPath(ctx, args.path, args.view_range, config.maxOutputChars, exec)
|
||||
case 'create':
|
||||
return createFile(ctx, policy, args.path, args.file_text, config.requireAbsolutePath, exec)
|
||||
return createFile(ctx, policy, args.path, args.file_text, exec)
|
||||
case 'str_replace':
|
||||
return replaceInFile(
|
||||
ctx,
|
||||
@@ -494,7 +490,6 @@ function registerStrReplaceEditor(ctx: Context, config: ResolvedConfig): void {
|
||||
args.path,
|
||||
args.old_str,
|
||||
args.new_str,
|
||||
config.requireAbsolutePath,
|
||||
exec,
|
||||
)
|
||||
case 'insert':
|
||||
@@ -504,7 +499,6 @@ function registerStrReplaceEditor(ctx: Context, config: ResolvedConfig): void {
|
||||
args.path,
|
||||
args.insert_line,
|
||||
args.new_str,
|
||||
config.requireAbsolutePath,
|
||||
exec,
|
||||
)
|
||||
}
|
||||
@@ -522,15 +516,12 @@ export interface Config {
|
||||
maxOutputChars?: number
|
||||
/** Model-facing tool description. */
|
||||
description?: string
|
||||
/** Require local absolute paths like the canonical editor contract (default true). */
|
||||
requireAbsolutePath?: boolean
|
||||
}
|
||||
|
||||
/** Runtime configuration schema for the string-replacement editor tool. */
|
||||
export const Config: z<Config> = z.object({
|
||||
maxOutputChars: z.number().default(16_000),
|
||||
description: z.string().default(DEFAULT_DESCRIPTION),
|
||||
requireAbsolutePath: z.boolean().default(true),
|
||||
})
|
||||
|
||||
/** Register one `str_replace_editor` tool over `ctx.fs`. */
|
||||
@@ -538,7 +529,6 @@ export function apply(ctx: Context, config: Config): void {
|
||||
const resolved: ResolvedConfig = {
|
||||
maxOutputChars: config.maxOutputChars ?? 16_000,
|
||||
description: config.description ?? DEFAULT_DESCRIPTION,
|
||||
requireAbsolutePath: config.requireAbsolutePath ?? true,
|
||||
}
|
||||
if (!Number.isSafeInteger(resolved.maxOutputChars) || resolved.maxOutputChars <= 0) {
|
||||
throw new Error('tool-str-replace-editor: maxOutputChars must be a positive safe integer')
|
||||
|
||||
@@ -316,6 +316,16 @@ describe('tool-str-replace-editor', () => {
|
||||
expect(text(repeatedMultiline))
|
||||
.toContain('Multiple occurrences of old_str `alpha\nbeta` in lines [1, 4]')
|
||||
|
||||
const mixedEol = join(root, 'mixed-eol.txt')
|
||||
await writeFile(mixedEol, 'alpha\r\nbeta\nmiddle\nalpha\nbeta')
|
||||
expect((await call(ctx, owner, {
|
||||
command: 'str_replace',
|
||||
path: mixedEol,
|
||||
old_str: 'alpha\r\nbeta',
|
||||
new_str: 'replaced',
|
||||
})).isError).toBe(false)
|
||||
expect(await readFile(mixedEol, 'utf8')).toBe('replaced\nmiddle\nalpha\nbeta')
|
||||
|
||||
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')
|
||||
@@ -378,13 +388,6 @@ describe('tool-str-replace-editor', () => {
|
||||
})).error).toMatchObject({ info: { code: 'FS_NOT_REGULAR_FILE' } })
|
||||
})
|
||||
|
||||
it('can opt into session-relative paths for non-canonical deployments', async () => {
|
||||
const { ctx, root, owner } = await setup({ requireAbsolutePath: false })
|
||||
await writeFile(join(root, 'relative.txt'), 'relative')
|
||||
expect(text(await call(ctx, owner, { command: 'view', path: 'relative.txt' })))
|
||||
.toContain("Here's the content of")
|
||||
})
|
||||
|
||||
it('delegates read-before-edit decisions to fs-policy', async () => {
|
||||
const { ctx, root, owner } = await setup({}, { fsPolicy: true })
|
||||
const existing = join(root, 'existing.txt')
|
||||
@@ -490,7 +493,7 @@ describe('tool-str-replace-editor', () => {
|
||||
const failWrite = async (): Promise<never> => {
|
||||
throw new Error('backend write failed')
|
||||
}
|
||||
ctx.fs.editText = failWrite
|
||||
ctx.fs.writeText = failWrite
|
||||
|
||||
const replace = await call(ctx, owner, {
|
||||
command: 'str_replace',
|
||||
@@ -501,7 +504,6 @@ describe('tool-str-replace-editor', () => {
|
||||
expect(replace.isError).toBe(true)
|
||||
expect(text(replace)).toContain('backend write failed')
|
||||
|
||||
ctx.fs.writeText = failWrite
|
||||
const insert = await call(ctx, owner, {
|
||||
command: 'insert',
|
||||
path,
|
||||
|
||||
Reference in New Issue
Block a user