fix(fs): preserve tabs during editor mutations
This commit is contained in:
@@ -299,21 +299,18 @@ async function replaceInFile(
|
|||||||
oldStr: string | undefined,
|
oldStr: string | undefined,
|
||||||
newStr: string | undefined,
|
newStr: string | undefined,
|
||||||
requireAbsolutePath: boolean,
|
requireAbsolutePath: boolean,
|
||||||
expandTabsOnMutation: boolean,
|
|
||||||
exec: ToolRunContext,
|
exec: ToolRunContext,
|
||||||
): Promise<string> {
|
): Promise<string> {
|
||||||
const sandboxPolicy = policy.resolve(exec)
|
const sandboxPolicy = policy.resolve(exec)
|
||||||
const target = await resolveTarget(ctx, path, requireAbsolutePath, exec, sandboxPolicy?.workspaceRoot)
|
const target = await resolveTarget(ctx, path, requireAbsolutePath, exec, sandboxPolicy?.workspaceRoot)
|
||||||
const intent = await ctx.waterfall('fs/edit-intent', target, exec, () => undefined)
|
const intent = await ctx.waterfall('fs/edit-intent', target, exec, () => undefined)
|
||||||
const rawOldValue = requiredForCommand(oldStr, 'old_str', 'str_replace', false)
|
const oldValue = requiredForCommand(oldStr, 'old_str', 'str_replace', false)
|
||||||
const oldValue = expandTabsOnMutation ? expandTabs(rawOldValue) : rawOldValue
|
const newValue = newStr ?? ''
|
||||||
const newValue = expandTabsOnMutation ? expandTabs(newStr ?? '') : newStr ?? ''
|
|
||||||
const info = await statExisting(ctx, target, 'str_replace', exec)
|
const info = await statExisting(ctx, target, 'str_replace', exec)
|
||||||
if (info.type !== 'file') {
|
if (info.type !== 'file') {
|
||||||
throw new FsError(`cannot edit "${target.displayPath}": not a regular file`, 'FS_NOT_REGULAR_FILE')
|
throw new FsError(`cannot edit "${target.displayPath}": not a regular file`, 'FS_NOT_REGULAR_FILE')
|
||||||
}
|
}
|
||||||
const rawBefore = await ctx.fs.readText(target, exec.signal)
|
const before = await ctx.fs.readText(target, exec.signal)
|
||||||
const before = expandTabsOnMutation ? expandTabs(rawBefore) : rawBefore
|
|
||||||
const offsets = matchOffsets(before, oldValue)
|
const offsets = matchOffsets(before, oldValue)
|
||||||
if (offsets.length === 0) {
|
if (offsets.length === 0) {
|
||||||
throw new FsError(
|
throw new FsError(
|
||||||
@@ -330,23 +327,13 @@ async function replaceInFile(
|
|||||||
}
|
}
|
||||||
let outcome
|
let outcome
|
||||||
try {
|
try {
|
||||||
outcome = expandTabsOnMutation
|
outcome = await ctx.fs.editText(
|
||||||
? await ctx.fs.writeText(
|
target,
|
||||||
target,
|
{ oldString: oldValue, newString: newValue, replaceAll: false },
|
||||||
before.replace(oldValue, newValue),
|
intent ?? { version: info.version },
|
||||||
intent === undefined
|
exec.signal,
|
||||||
? { kind: 'replaceIfVersion', version: info.version }
|
sandboxPolicy,
|
||||||
: { kind: 'replaceIfVersion', version: intent.version },
|
)
|
||||||
exec.signal,
|
|
||||||
sandboxPolicy,
|
|
||||||
)
|
|
||||||
: await ctx.fs.editText(
|
|
||||||
target,
|
|
||||||
{ oldString: oldValue, newString: newValue, replaceAll: false },
|
|
||||||
intent ?? { version: info.version },
|
|
||||||
exec.signal,
|
|
||||||
sandboxPolicy,
|
|
||||||
)
|
|
||||||
} catch (error: unknown) {
|
} catch (error: unknown) {
|
||||||
throw policy.mapError(error, sandboxPolicy)
|
throw policy.mapError(error, sandboxPolicy)
|
||||||
}
|
}
|
||||||
@@ -361,12 +348,10 @@ async function insertInFile(
|
|||||||
insertLine: number | undefined,
|
insertLine: number | undefined,
|
||||||
newStr: string | undefined,
|
newStr: string | undefined,
|
||||||
requireAbsolutePath: boolean,
|
requireAbsolutePath: boolean,
|
||||||
expandTabsOnMutation: boolean,
|
|
||||||
exec: ToolRunContext,
|
exec: ToolRunContext,
|
||||||
): Promise<string> {
|
): Promise<string> {
|
||||||
if (insertLine === undefined) throw new Error('Parameter `insert_line` is required for command: insert')
|
if (insertLine === undefined) throw new Error('Parameter `insert_line` is required for command: insert')
|
||||||
const rawValue = requiredForCommand(newStr, 'new_str', 'insert')
|
const value = requiredForCommand(newStr, 'new_str', 'insert')
|
||||||
const value = expandTabsOnMutation ? expandTabs(rawValue) : rawValue
|
|
||||||
const sandboxPolicy = policy.resolve(exec)
|
const sandboxPolicy = policy.resolve(exec)
|
||||||
const target = await resolveTarget(ctx, path, requireAbsolutePath, exec, sandboxPolicy?.workspaceRoot)
|
const target = await resolveTarget(ctx, path, requireAbsolutePath, exec, sandboxPolicy?.workspaceRoot)
|
||||||
const intent = await ctx.waterfall('fs/edit-intent', target, exec, () => undefined)
|
const intent = await ctx.waterfall('fs/edit-intent', target, exec, () => undefined)
|
||||||
@@ -374,8 +359,7 @@ async function insertInFile(
|
|||||||
if (info.type !== 'file') {
|
if (info.type !== 'file') {
|
||||||
throw new FsError(`cannot insert into "${target.displayPath}": not a regular file`, 'FS_NOT_REGULAR_FILE')
|
throw new FsError(`cannot insert into "${target.displayPath}": not a regular file`, 'FS_NOT_REGULAR_FILE')
|
||||||
}
|
}
|
||||||
const rawBefore = await ctx.fs.readText(target, exec.signal)
|
const before = await ctx.fs.readText(target, exec.signal)
|
||||||
const before = expandTabsOnMutation ? expandTabs(rawBefore) : rawBefore
|
|
||||||
const lines = before.split('\n')
|
const lines = before.split('\n')
|
||||||
if (!Number.isInteger(insertLine) || insertLine < 0 || insertLine > lines.length) {
|
if (!Number.isInteger(insertLine) || insertLine < 0 || insertLine > lines.length) {
|
||||||
throw new Error(
|
throw new Error(
|
||||||
@@ -404,7 +388,6 @@ interface ResolvedConfig {
|
|||||||
maxOutputChars: number
|
maxOutputChars: number
|
||||||
description: string
|
description: string
|
||||||
requireAbsolutePath: boolean
|
requireAbsolutePath: boolean
|
||||||
expandTabsOnMutation: boolean
|
|
||||||
}
|
}
|
||||||
|
|
||||||
function presentEditorCall(args: {
|
function presentEditorCall(args: {
|
||||||
@@ -512,7 +495,6 @@ function registerStrReplaceEditor(ctx: Context, config: ResolvedConfig): void {
|
|||||||
args.old_str,
|
args.old_str,
|
||||||
args.new_str,
|
args.new_str,
|
||||||
config.requireAbsolutePath,
|
config.requireAbsolutePath,
|
||||||
config.expandTabsOnMutation,
|
|
||||||
exec,
|
exec,
|
||||||
)
|
)
|
||||||
case 'insert':
|
case 'insert':
|
||||||
@@ -523,7 +505,6 @@ function registerStrReplaceEditor(ctx: Context, config: ResolvedConfig): void {
|
|||||||
args.insert_line,
|
args.insert_line,
|
||||||
args.new_str,
|
args.new_str,
|
||||||
config.requireAbsolutePath,
|
config.requireAbsolutePath,
|
||||||
config.expandTabsOnMutation,
|
|
||||||
exec,
|
exec,
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
@@ -543,8 +524,6 @@ export interface Config {
|
|||||||
description?: string
|
description?: string
|
||||||
/** Require local absolute paths like the canonical editor contract (default true). */
|
/** Require local absolute paths like the canonical editor contract (default true). */
|
||||||
requireAbsolutePath?: boolean
|
requireAbsolutePath?: boolean
|
||||||
/** Expand tabs across the full file before each mutation, matching the canonical editor (default true). */
|
|
||||||
expandTabsOnMutation?: boolean
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Runtime configuration schema for the string-replacement editor tool. */
|
/** Runtime configuration schema for the string-replacement editor tool. */
|
||||||
@@ -552,7 +531,6 @@ export const Config: z<Config> = z.object({
|
|||||||
maxOutputChars: z.number().default(16_000),
|
maxOutputChars: z.number().default(16_000),
|
||||||
description: z.string().default(DEFAULT_DESCRIPTION),
|
description: z.string().default(DEFAULT_DESCRIPTION),
|
||||||
requireAbsolutePath: z.boolean().default(true),
|
requireAbsolutePath: z.boolean().default(true),
|
||||||
expandTabsOnMutation: z.boolean().default(true),
|
|
||||||
})
|
})
|
||||||
|
|
||||||
/** Register one `str_replace_editor` tool over `ctx.fs`. */
|
/** Register one `str_replace_editor` tool over `ctx.fs`. */
|
||||||
@@ -561,7 +539,6 @@ export function apply(ctx: Context, config: Config): void {
|
|||||||
maxOutputChars: config.maxOutputChars ?? 16_000,
|
maxOutputChars: config.maxOutputChars ?? 16_000,
|
||||||
description: config.description ?? DEFAULT_DESCRIPTION,
|
description: config.description ?? DEFAULT_DESCRIPTION,
|
||||||
requireAbsolutePath: config.requireAbsolutePath ?? true,
|
requireAbsolutePath: config.requireAbsolutePath ?? true,
|
||||||
expandTabsOnMutation: config.expandTabsOnMutation ?? true,
|
|
||||||
}
|
}
|
||||||
if (!Number.isSafeInteger(resolved.maxOutputChars) || resolved.maxOutputChars <= 0) {
|
if (!Number.isSafeInteger(resolved.maxOutputChars) || resolved.maxOutputChars <= 0) {
|
||||||
throw new Error('tool-str-replace-editor: maxOutputChars must be a positive safe integer')
|
throw new Error('tool-str-replace-editor: maxOutputChars must be a positive safe integer')
|
||||||
|
|||||||
@@ -444,8 +444,8 @@ describe('tool-str-replace-editor', () => {
|
|||||||
expect(ownerless.error).toMatchObject({ info: { code: 'FS_SANDBOX_DENIED' } })
|
expect(ownerless.error).toMatchObject({ info: { code: 'FS_SANDBOX_DENIED' } })
|
||||||
})
|
})
|
||||||
|
|
||||||
it('can preserve tabs outside the edited region', async () => {
|
it('preserves tabs outside the edited region', async () => {
|
||||||
const { ctx, root, owner } = await setup({ expandTabsOnMutation: false })
|
const { ctx, root, owner } = await setup()
|
||||||
const path = join(root, 'Makefile')
|
const path = join(root, 'Makefile')
|
||||||
await writeFile(path, 'target:\n\told\nremove\n')
|
await writeFile(path, 'target:\n\told\nremove\n')
|
||||||
await call(ctx, owner, {
|
await call(ctx, owner, {
|
||||||
@@ -487,9 +487,10 @@ describe('tool-str-replace-editor', () => {
|
|||||||
const { ctx, root, owner } = await setup()
|
const { ctx, root, owner } = await setup()
|
||||||
const path = join(root, 'backend-error.txt')
|
const path = join(root, 'backend-error.txt')
|
||||||
await writeFile(path, 'old\n')
|
await writeFile(path, 'old\n')
|
||||||
ctx.fs.writeText = async () => {
|
const failWrite = async (): Promise<never> => {
|
||||||
throw new Error('backend write failed')
|
throw new Error('backend write failed')
|
||||||
}
|
}
|
||||||
|
ctx.fs.editText = failWrite
|
||||||
|
|
||||||
const replace = await call(ctx, owner, {
|
const replace = await call(ctx, owner, {
|
||||||
command: 'str_replace',
|
command: 'str_replace',
|
||||||
@@ -500,6 +501,7 @@ describe('tool-str-replace-editor', () => {
|
|||||||
expect(replace.isError).toBe(true)
|
expect(replace.isError).toBe(true)
|
||||||
expect(text(replace)).toContain('backend write failed')
|
expect(text(replace)).toContain('backend write failed')
|
||||||
|
|
||||||
|
ctx.fs.writeText = failWrite
|
||||||
const insert = await call(ctx, owner, {
|
const insert = await call(ctx, owner, {
|
||||||
command: 'insert',
|
command: 'insert',
|
||||||
path,
|
path,
|
||||||
|
|||||||
Reference in New Issue
Block a user