From fc8656e321703d9f1abc41a748588629bad8d8a0 Mon Sep 17 00:00:00 2001 From: Aiden Cline Date: Mon, 27 Jul 2026 14:58:51 +0000 Subject: [PATCH] fix(core): restore v1 edit behavior --- packages/core/src/tool/plugin/edit-replace.ts | 490 ++++++++++++++++++ packages/core/src/tool/plugin/edit.ts | 178 ++++--- packages/core/test/tool-edit.test.ts | 151 +++++- 3 files changed, 701 insertions(+), 118 deletions(-) create mode 100644 packages/core/src/tool/plugin/edit-replace.ts diff --git a/packages/core/src/tool/plugin/edit-replace.ts b/packages/core/src/tool/plugin/edit-replace.ts new file mode 100644 index 0000000000..a5185ec3d3 --- /dev/null +++ b/packages/core/src/tool/plugin/edit-replace.ts @@ -0,0 +1,490 @@ +// Ported from the V1 edit tool, whose correction strategies are sourced from: +// https://github.com/cline/cline/blob/main/evals/diff-edits/diff-apply/diff-06-23-25.ts +// https://github.com/google-gemini/gemini-cli/blob/main/packages/core/src/utils/editCorrector.ts +// https://github.com/cline/cline/blob/main/evals/diff-edits/diff-apply/diff-06-26-25.ts + +export type Replacer = (content: string, find: string) => Generator + +// Similarity thresholds for block anchor fallback matching +const SINGLE_CANDIDATE_SIMILARITY_THRESHOLD = 0.65 +const MULTIPLE_CANDIDATES_SIMILARITY_THRESHOLD = 0.65 + +/** + * Levenshtein distance algorithm implementation + */ +function levenshtein(a: string, b: string): number { + // Handle empty strings + if (a === "" || b === "") { + return Math.max(a.length, b.length) + } + const matrix = Array.from({ length: a.length + 1 }, (_, i) => + Array.from({ length: b.length + 1 }, (_, j) => (i === 0 ? j : j === 0 ? i : 0)), + ) + + for (let i = 1; i <= a.length; i++) { + for (let j = 1; j <= b.length; j++) { + const cost = a[i - 1] === b[j - 1] ? 0 : 1 + matrix[i][j] = Math.min(matrix[i - 1][j] + 1, matrix[i][j - 1] + 1, matrix[i - 1][j - 1] + cost) + } + } + return matrix[a.length][b.length] +} + +export const SimpleReplacer: Replacer = function* (_content, find) { + yield find +} + +export const LineTrimmedReplacer: Replacer = function* (content, find) { + const originalLines = content.split("\n") + const searchLines = find.split("\n") + + if (searchLines[searchLines.length - 1] === "") { + searchLines.pop() + } + + for (let i = 0; i <= originalLines.length - searchLines.length; i++) { + let matches = true + + for (let j = 0; j < searchLines.length; j++) { + const originalTrimmed = originalLines[i + j].trim() + const searchTrimmed = searchLines[j].trim() + + if (originalTrimmed !== searchTrimmed) { + matches = false + break + } + } + + if (matches) { + let matchStartIndex = 0 + for (let k = 0; k < i; k++) { + matchStartIndex += originalLines[k].length + 1 + } + + let matchEndIndex = matchStartIndex + for (let k = 0; k < searchLines.length; k++) { + matchEndIndex += originalLines[i + k].length + if (k < searchLines.length - 1) { + matchEndIndex += 1 // Add newline character except for the last line + } + } + + yield content.substring(matchStartIndex, matchEndIndex) + } + } +} + +export const BlockAnchorReplacer: Replacer = function* (content, find) { + const originalLines = content.split("\n") + const searchLines = find.split("\n") + + if (searchLines.length < 3) { + return + } + + if (searchLines[searchLines.length - 1] === "") { + searchLines.pop() + } + + const firstLineSearch = searchLines[0].trim() + const lastLineSearch = searchLines[searchLines.length - 1].trim() + const searchBlockSize = searchLines.length + const maxLineDelta = Math.max(1, Math.floor(searchBlockSize * 0.25)) + + // Collect all candidate positions where both anchors match + const candidates: Array<{ startLine: number; endLine: number }> = [] + for (let i = 0; i < originalLines.length; i++) { + if (originalLines[i].trim() !== firstLineSearch) { + continue + } + + // Look for the matching last line after this first line + for (let j = i + 2; j < originalLines.length; j++) { + if (originalLines[j].trim() === lastLineSearch) { + const actualBlockSize = j - i + 1 + if (Math.abs(actualBlockSize - searchBlockSize) <= maxLineDelta) { + candidates.push({ startLine: i, endLine: j }) + } + break // Only match the first occurrence of the last line + } + } + } + + // Return immediately if no candidates + if (candidates.length === 0) { + return + } + + // Handle single candidate scenario (using relaxed threshold) + if (candidates.length === 1) { + const { startLine, endLine } = candidates[0] + const actualBlockSize = endLine - startLine + 1 + + let similarity = 0 + const linesToCheck = Math.min(searchBlockSize - 2, actualBlockSize - 2) // Middle lines only + + if (linesToCheck > 0) { + for (let j = 1; j < searchBlockSize - 1 && j < actualBlockSize - 1; j++) { + const originalLine = originalLines[startLine + j].trim() + const searchLine = searchLines[j].trim() + const maxLen = Math.max(originalLine.length, searchLine.length) + if (maxLen === 0) { + continue + } + const distance = levenshtein(originalLine, searchLine) + similarity += (1 - distance / maxLen) / linesToCheck + + // Exit early when threshold is reached + if (similarity >= SINGLE_CANDIDATE_SIMILARITY_THRESHOLD) { + break + } + } + } else { + // No middle lines to compare, just accept based on anchors + similarity = 1.0 + } + + if (similarity >= SINGLE_CANDIDATE_SIMILARITY_THRESHOLD) { + let matchStartIndex = 0 + for (let k = 0; k < startLine; k++) { + matchStartIndex += originalLines[k].length + 1 + } + let matchEndIndex = matchStartIndex + for (let k = startLine; k <= endLine; k++) { + matchEndIndex += originalLines[k].length + if (k < endLine) { + matchEndIndex += 1 // Add newline character except for the last line + } + } + yield content.substring(matchStartIndex, matchEndIndex) + } + return + } + + // Calculate similarity for multiple candidates + let bestMatch: { startLine: number; endLine: number } | null = null + let maxSimilarity = -1 + + for (const candidate of candidates) { + const { startLine, endLine } = candidate + const actualBlockSize = endLine - startLine + 1 + + let similarity = 0 + const linesToCheck = Math.min(searchBlockSize - 2, actualBlockSize - 2) // Middle lines only + + if (linesToCheck > 0) { + for (let j = 1; j < searchBlockSize - 1 && j < actualBlockSize - 1; j++) { + const originalLine = originalLines[startLine + j].trim() + const searchLine = searchLines[j].trim() + const maxLen = Math.max(originalLine.length, searchLine.length) + if (maxLen === 0) { + continue + } + const distance = levenshtein(originalLine, searchLine) + similarity += 1 - distance / maxLen + } + similarity /= linesToCheck // Average similarity + } else { + // No middle lines to compare, just accept based on anchors + similarity = 1.0 + } + + if (similarity > maxSimilarity) { + maxSimilarity = similarity + bestMatch = candidate + } + } + + // Threshold judgment + if (maxSimilarity >= MULTIPLE_CANDIDATES_SIMILARITY_THRESHOLD && bestMatch) { + const { startLine, endLine } = bestMatch + let matchStartIndex = 0 + for (let k = 0; k < startLine; k++) { + matchStartIndex += originalLines[k].length + 1 + } + let matchEndIndex = matchStartIndex + for (let k = startLine; k <= endLine; k++) { + matchEndIndex += originalLines[k].length + if (k < endLine) { + matchEndIndex += 1 + } + } + yield content.substring(matchStartIndex, matchEndIndex) + } +} + +export const WhitespaceNormalizedReplacer: Replacer = function* (content, find) { + const normalizeWhitespace = (text: string) => text.replace(/\s+/g, " ").trim() + const normalizedFind = normalizeWhitespace(find) + + // Handle single line matches + const lines = content.split("\n") + for (let i = 0; i < lines.length; i++) { + const line = lines[i] + if (normalizeWhitespace(line) === normalizedFind) { + yield line + } else { + // Only check for substring matches if the full line doesn't match + const normalizedLine = normalizeWhitespace(line) + if (normalizedLine.includes(normalizedFind)) { + // Find the actual substring in the original line that matches + const words = find.trim().split(/\s+/) + if (words.length > 0) { + const pattern = words.map((word) => word.replace(/[.*+?^${}()|[\]\\]/g, "\\$&")).join("\\s+") + try { + const regex = new RegExp(pattern) + const match = line.match(regex) + if (match) { + yield match[0] + } + } catch { + // Invalid regex pattern, skip + } + } + } + } + } + + // Handle multi-line matches + const findLines = find.split("\n") + if (findLines.length > 1) { + for (let i = 0; i <= lines.length - findLines.length; i++) { + const block = lines.slice(i, i + findLines.length) + if (normalizeWhitespace(block.join("\n")) === normalizedFind) { + yield block.join("\n") + } + } + } +} + +export const IndentationFlexibleReplacer: Replacer = function* (content, find) { + const removeIndentation = (text: string) => { + const lines = text.split("\n") + const nonEmptyLines = lines.filter((line) => line.trim().length > 0) + if (nonEmptyLines.length === 0) return text + + const minIndent = Math.min( + ...nonEmptyLines.map((line) => { + const match = line.match(/^(\s*)/) + return match ? match[1].length : 0 + }), + ) + + return lines.map((line) => (line.trim().length === 0 ? line : line.slice(minIndent))).join("\n") + } + + const normalizedFind = removeIndentation(find) + const contentLines = content.split("\n") + const findLines = find.split("\n") + + for (let i = 0; i <= contentLines.length - findLines.length; i++) { + const block = contentLines.slice(i, i + findLines.length).join("\n") + if (removeIndentation(block) === normalizedFind) { + yield block + } + } +} + +export const EscapeNormalizedReplacer: Replacer = function* (content, find) { + const unescapeString = (str: string): string => { + return str.replace(/\\(n|t|r|'|"|`|\\|\n|\$)/g, (match, capturedChar) => { + switch (capturedChar) { + case "n": + return "\n" + case "t": + return "\t" + case "r": + return "\r" + case "'": + return "'" + case '"': + return '"' + case "`": + return "`" + case "\\": + return "\\" + case "\n": + return "\n" + case "$": + return "$" + default: + return match + } + }) + } + + const unescapedFind = unescapeString(find) + + // Try direct match with unescaped find string + if (content.includes(unescapedFind)) { + yield unescapedFind + } + + // Also try finding escaped versions in content that match unescaped find + const lines = content.split("\n") + const findLines = unescapedFind.split("\n") + + for (let i = 0; i <= lines.length - findLines.length; i++) { + const block = lines.slice(i, i + findLines.length).join("\n") + const unescapedBlock = unescapeString(block) + + if (unescapedBlock === unescapedFind) { + yield block + } + } +} + +export const MultiOccurrenceReplacer: Replacer = function* (content, find) { + // This replacer yields all exact matches, allowing the replace function + // to handle multiple occurrences based on replaceAll parameter + let startIndex = 0 + + while (true) { + const index = content.indexOf(find, startIndex) + if (index === -1) break + + yield find + startIndex = index + find.length + } +} + +export const TrimmedBoundaryReplacer: Replacer = function* (content, find) { + const trimmedFind = find.trim() + + if (trimmedFind === find) { + // Already trimmed, no point in trying + return + } + + // Try to find the trimmed version + if (content.includes(trimmedFind)) { + yield trimmedFind + } + + // Also try finding blocks where trimmed content matches + const lines = content.split("\n") + const findLines = find.split("\n") + + for (let i = 0; i <= lines.length - findLines.length; i++) { + const block = lines.slice(i, i + findLines.length).join("\n") + + if (block.trim() === trimmedFind) { + yield block + } + } +} + +export const ContextAwareReplacer: Replacer = function* (content, find) { + const findLines = find.split("\n") + if (findLines.length < 3) { + // Need at least 3 lines to have meaningful context + return + } + + // Remove trailing empty line if present + if (findLines[findLines.length - 1] === "") { + findLines.pop() + } + + const contentLines = content.split("\n") + + // Extract first and last lines as context anchors + const firstLine = findLines[0].trim() + const lastLine = findLines[findLines.length - 1].trim() + + // Find blocks that start and end with the context anchors + for (let i = 0; i < contentLines.length; i++) { + if (contentLines[i].trim() !== firstLine) continue + + // Look for the matching last line + for (let j = i + 2; j < contentLines.length; j++) { + if (contentLines[j].trim() === lastLine) { + // Found a potential context block + const blockLines = contentLines.slice(i, j + 1) + const block = blockLines.join("\n") + + // Check if the middle content has reasonable similarity + // (simple heuristic: at least 50% of non-empty lines should match when trimmed) + if (blockLines.length === findLines.length) { + let matchingLines = 0 + let totalNonEmptyLines = 0 + + for (let k = 1; k < blockLines.length - 1; k++) { + const blockLine = blockLines[k].trim() + const findLine = findLines[k].trim() + + if (blockLine.length > 0 || findLine.length > 0) { + totalNonEmptyLines++ + if (blockLine === findLine) { + matchingLines++ + } + } + } + + if (totalNonEmptyLines === 0 || matchingLines / totalNonEmptyLines >= 0.5) { + yield block + break // Only match the first occurrence + } + } + break + } + } + } +} + +export function replace(content: string, oldString: string, newString: string, replaceAll = false): string { + if (oldString === newString) { + throw new Error("No changes to apply: oldString and newString are identical.") + } + if (oldString === "") { + throw new Error( + "oldString cannot be empty when editing an existing file. Provide the exact text to replace, or use write for an intentional full-file replacement.", + ) + } + + let notFound = true + + for (const replacer of [ + SimpleReplacer, + LineTrimmedReplacer, + BlockAnchorReplacer, + WhitespaceNormalizedReplacer, + IndentationFlexibleReplacer, + EscapeNormalizedReplacer, + TrimmedBoundaryReplacer, + ContextAwareReplacer, + MultiOccurrenceReplacer, + ]) { + for (const search of replacer(content, oldString)) { + const index = content.indexOf(search) + if (index === -1) continue + notFound = false + if (isDisproportionateMatch(search, oldString)) { + throw new Error( + "Refusing replacement because the matched span is much larger than oldString. Re-read the file and provide the full exact oldString for the intended replacement.", + ) + } + if (replaceAll) { + return content.replaceAll(search, newString) + } + const lastIndex = content.lastIndexOf(search) + if (index !== lastIndex) continue + return content.substring(0, index) + newString + content.substring(index + search.length) + } + } + + if (notFound) { + throw new Error( + "Could not find oldString in the file. It must match exactly, including whitespace, indentation, and line endings.", + ) + } + throw new Error("Found multiple matches for oldString. Provide more surrounding context to make the match unique.") +} + +function isDisproportionateMatch(search: string, oldString: string) { + const oldLines = oldString.split("\n").length + const searchLines = search.split("\n").length + if (searchLines >= Math.max(oldLines + 3, oldLines * 2)) return true + if (oldLines === 1) return false + return search.trim().length > Math.max(oldString.trim().length + 500, oldString.trim().length * 4) +} diff --git a/packages/core/src/tool/plugin/edit.ts b/packages/core/src/tool/plugin/edit.ts index 3807020ab2..63e9263343 100644 --- a/packages/core/src/tool/plugin/edit.ts +++ b/packages/core/src/tool/plugin/edit.ts @@ -15,6 +15,8 @@ import { FileMutation } from "../../file-mutation" import { FSUtil } from "@opencode-ai/util/fs-util" import { LocationMutation } from "../../location-mutation" import { Permission } from "../../permission" +import { KeyedMutex } from "../../effect/keyed-mutex" +import { replace } from "./edit-replace" export const name = "edit" @@ -23,7 +25,7 @@ export const Input = Schema.Struct({ description: "File path to edit. Relative paths resolve within the active Location. Absolute paths inside that Location are accepted; external absolute paths require external_directory approval.", }), - oldString: Schema.String.annotate({ description: "Exact text to replace" }), + oldString: Schema.String.annotate({ description: "The text to replace" }), newString: Schema.String.annotate({ description: "Replacement text, which must differ from oldString" }), replaceAll: Schema.Boolean.pipe(Schema.optional).annotate({ description: "Replace all exact occurrences of oldString (default false)", @@ -50,7 +52,6 @@ const decodeUtf8 = (content: Uint8Array) => { } const countOccurrences = (content: string, search: string) => { - if (search === "") return content.length + 1 let count = 0 let offset = 0 while ((offset = content.indexOf(search, offset)) !== -1) { @@ -78,7 +79,6 @@ export const toModelOutput = (output: Output, oldString: string, newString: stri ].join("\n") /** Deferred edit behavior and UX integrations remain visible at the model-facing seam. */ -// TODO: Port V1 fuzzy correction strategies only after exact-edit behavior is established: line-trimmed matching, block-anchor fallback, indentation correction, and similarity-threshold review. // TODO: Add formatter integration after formatter runtime exists. // TODO: Publish watcher/file-edit events after watcher integration exists. // TODO: Add snapshots / undo after design exists. @@ -91,92 +91,78 @@ export const Plugin = { const files = yield* FileMutation.Service const fs = yield* FSUtil.Service const permission = yield* Permission.Service + const locks = KeyedMutex.makeUnsafe() yield* ctx.tool .transform((draft) => - draft.add( - ({ - name, - options: { codemode: false, permission: "edit" }, - description: - "Replace exact text in one file. Relative paths resolve within the active Location. Absolute paths inside the Location are accepted. Explicit external absolute paths require external_directory approval before edit approval.", - input: Input, - output: Output, - execute: (input, context) => { - const unableToEdit = (effect: Effect.Effect) => - effect.pipe( - Effect.mapError((error) => - error instanceof FileMutation.StaleContentError - ? new ToolFailure({ - message: "File changed after permission approval. Read it again before editing.", - error, - }) - : new ToolFailure({ message: `Unable to edit ${input.path}`, error }), - ), - ) + draft.add({ + name, + options: { codemode: false, permission: "edit" }, + description: + "Replace text in one file. Relative paths resolve within the active Location. Absolute paths inside the Location are accepted. Explicit external absolute paths require external_directory approval before edit approval.", + input: Input, + output: Output, + execute: (input, context) => { + const unableToEdit = (effect: Effect.Effect) => + effect.pipe( + Effect.mapError((error) => new ToolFailure({ message: `Unable to edit ${input.path}`, error })), + ) - return Effect.gen(function* () { - const permissionSource = { - type: "tool" as const, - messageID: context.messageID, - callID: context.callID, - } - if (input.oldString === input.newString) { + return Effect.gen(function* () { + const permissionSource = { + type: "tool" as const, + messageID: context.messageID, + callID: context.callID, + } + if (input.oldString === input.newString) { + return yield* new ToolFailure({ + message: "No changes to apply: oldString and newString are identical.", + }) + } + const target = yield* unableToEdit(mutation.resolve({ path: input.path, kind: "file" })) + const external = target.externalDirectory + if (external) { + yield* unableToEdit( + permission.assert({ + ...LocationMutation.externalDirectoryPermission(external), + sessionID: context.sessionID, + agent: context.agent, + source: permissionSource, + }), + ) + } + + return yield* locks.withLock(target.canonical)( + Effect.gen(function* () { + const content = yield* fs.readFile(target.canonical).pipe( + Effect.catchReason("PlatformError", "NotFound", () => Effect.succeed(undefined)), + unableToEdit, + ) + if (input.oldString === "" && content !== undefined) { return yield* new ToolFailure({ - message: "No changes to apply: oldString and newString are identical.", + message: + "oldString cannot be empty when editing an existing file. Provide the exact text to replace, or use write for an intentional full-file replacement.", }) } - if (input.oldString === "") { - return yield* new ToolFailure({ - message: "oldString must not be empty. Use write to create or overwrite a file.", - }) + if (content === undefined && input.oldString !== "") { + return yield* new ToolFailure({ message: `File ${input.path} not found` }) } - - const target = yield* unableToEdit(mutation.resolve({ path: input.path, kind: "file" })) - const external = target.externalDirectory - if (external) { - yield* unableToEdit( - permission.assert({ - ...LocationMutation.externalDirectoryPermission(external), - sessionID: context.sessionID, - agent: context.agent, - source: permissionSource, - }), - ) - } - - yield* unableToEdit( - permission.assert({ - action: "edit", - resources: [target.resource], - save: ["*"], - sessionID: context.sessionID, - agent: context.agent, - source: permissionSource, - }), - ) - const source = decodeUtf8(yield* unableToEdit(fs.readFile(target.canonical))) + const source = decodeUtf8(content ?? new Uint8Array()) const ending = detectLineEnding(source.text) const oldString = convertToLineEnding(input.oldString, ending) const newString = convertToLineEnding(input.newString, ending) - const replacements = countOccurrences(source.text, oldString) - if (replacements === 0) { - return yield* new ToolFailure({ - message: - "Could not find oldString in the file. It must match exactly, including whitespace and indentation.", - }) - } - if (replacements > 1 && input.replaceAll !== true) { - return yield* new ToolFailure({ - message: - "Found multiple exact matches for oldString. Provide more surrounding context or set replaceAll to true.", - }) - } - const replaced = - input.replaceAll === true - ? source.text.replaceAll(oldString, newString) - : source.text.replace(oldString, newString) + input.oldString === "" + ? newString + : yield* Effect.try({ + try: () => replace(source.text, oldString, newString, input.replaceAll), + catch: (error) => + new ToolFailure({ + message: error instanceof Error ? error.message : `Unable to edit ${input.path}`, + }), + }) + const replacements = + input.oldString === "" ? 1 : Math.max(1, countOccurrences(source.text, oldString)) const counts = diffLines(source.text, replaced).reduce( (result, item) => ({ additions: result.additions + (item.added ? (item.count ?? 0) : 0), @@ -185,10 +171,21 @@ export const Plugin = { { additions: 0, deletions: 0 }, ) const next = splitBom(replaced) + const patch = createTwoFilesPatch(target.resource, target.resource, source.text, replaced) + yield* unableToEdit( + permission.assert({ + action: "edit", + resources: [target.resource], + save: ["*"], + metadata: { filepath: target.canonical, diff: patch }, + sessionID: context.sessionID, + agent: context.agent, + source: permissionSource, + }), + ) const result = yield* unableToEdit( - files.writeIfUnchanged({ + files.write({ target, - expected: source.content, content: joinBom(next.text, source.bom || next.bom), }), ) @@ -196,23 +193,24 @@ export const Plugin = { files: [ { file: result.resource, - patch: createTwoFilesPatch(result.resource, result.resource, source.text, replaced), + patch, status: "modified" as const, ...counts, }, ], replacements, } satisfies Output - }).pipe( - Effect.map((output) => ({ - output, - content: toModelOutput(output, input.oldString, input.newString), - metadata: { files: output.files }, - })), - ) - }, - }), - ), + }), + ) + }).pipe( + Effect.map((output) => ({ + output, + content: toModelOutput(output, input.oldString, input.newString), + metadata: { files: output.files }, + })), + ) + }, + }), ) .pipe(Effect.orDie) }), diff --git a/packages/core/test/tool-edit.test.ts b/packages/core/test/tool-edit.test.ts index a22ba21f39..8efafa490d 100644 --- a/packages/core/test/tool-edit.test.ts +++ b/packages/core/test/tool-edit.test.ts @@ -99,13 +99,7 @@ const withTool = (directory: string, body: (registry: Tool.Interface) = }).pipe( Effect.provide( AppNodeBuilder.build( - LayerNode.group([ - Tool.node, - Tool.node, - LocationMutation.node, - FileMutation.node, - editToolNode, - ]), + LayerNode.group([Tool.node, Tool.node, LocationMutation.node, FileMutation.node, editToolNode]), [ [FSUtil.node, filesystem], [Location.node, activeLocation], @@ -302,7 +296,7 @@ describe("EditTool", () => { error: { type: "permission.rejected", message: "Permission denied: edit" }, }) expect(assertions.map((input) => input.action)).toEqual(["external_directory", "edit"]) - expect(reads).toBe(0) + expect(reads).toBe(1) expect(writes).toEqual([]) expect(yield* Effect.promise(() => fs.readFile(external, "utf8"))).toBe("before") }), @@ -313,7 +307,7 @@ describe("EditTool", () => { ), ) - it.live("denied edit reads no target content and does not disclose whether oldString matches", () => + it.live("validates the replacement before requesting edit approval", () => Effect.acquireUseRelease( Effect.promise(() => tmpdir()), (tmp) => { @@ -337,9 +331,20 @@ describe("EditTool", () => { status: "error", error: { type: "permission.rejected", message: "Permission denied: edit" }, }) - expect(missing).toEqual(matching) - expect(assertions.map((input) => input.action)).toEqual(["edit", "edit"]) - expect(reads).toBe(0) + expect(missing).toEqual({ + status: "error", + error: { + type: "tool.execution", + message: + "Could not find oldString in the file. It must match exactly, including whitespace, indentation, and line endings.", + }, + }) + expect(assertions.map((input) => input.action)).toEqual(["edit"]) + expect(assertions[0]?.metadata).toMatchObject({ + filepath: yield* Effect.promise(() => fs.realpath(target)), + diff: expect.stringContaining("+replacement"), + }) + expect(reads).toBe(2) expect(writes).toEqual([]) }), ), @@ -350,7 +355,7 @@ describe("EditTool", () => { ), ) - it.live("rejects no-op, empty, missing, and ambiguous exact replacements", () => + it.live("rejects no-op, invalid empty, missing, and ambiguous replacements", () => Effect.acquireUseRelease( Effect.promise(() => tmpdir()), (tmp) => { @@ -375,7 +380,8 @@ describe("EditTool", () => { status: "error", error: { type: "tool.execution", - message: "oldString must not be empty. Use write to create or overwrite a file.", + message: + "oldString cannot be empty when editing an existing file. Provide the exact text to replace, or use write for an intentional full-file replacement.", }, }) expect( @@ -385,7 +391,7 @@ describe("EditTool", () => { error: { type: "tool.execution", message: - "Could not find oldString in the file. It must match exactly, including whitespace and indentation.", + "Could not find oldString in the file. It must match exactly, including whitespace, indentation, and line endings.", }, }) expect( @@ -395,7 +401,7 @@ describe("EditTool", () => { error: { type: "tool.execution", message: - "Found multiple exact matches for oldString. Provide more surrounding context or set replaceAll to true.", + "Found multiple matches for oldString. Provide more surrounding context to make the match unique.", }, }) expect(writes).toEqual([]) @@ -408,6 +414,61 @@ describe("EditTool", () => { ), ) + it.live("creates a missing file when oldString is empty", () => + Effect.acquireUseRelease( + Effect.promise(() => tmpdir()), + (tmp) => { + reset() + const target = path.join(tmp.path, "nested", "new.txt") + return withTool(tmp.path, (registry) => + executeTool(registry, call({ path: "nested/new.txt", oldString: "", newString: "new content" })), + ).pipe( + Effect.andThen((settled) => + Effect.gen(function* () { + expect(settled.status).toBe("completed") + expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("new content") + expect(assertions[0]?.metadata).toMatchObject({ diff: expect.stringContaining("+new content") }) + }), + ), + ) + }, + (tmp) => Effect.promise(() => tmp[Symbol.asyncDispose]()), + ), + ) + + it.live("uses the V1 fallback matchers", () => + Effect.acquireUseRelease( + Effect.promise(() => tmpdir()), + (tmp) => { + reset() + const target = path.join(tmp.path, "fallback.ts") + return Effect.promise(() => fs.writeFile(target, "function value() {\n return 1\n}\n")).pipe( + Effect.andThen( + withTool(tmp.path, (registry) => + executeTool( + registry, + call({ + path: "fallback.ts", + oldString: "function value() {\nreturn 1\n}", + newString: "function value() {\n return 2\n}", + }), + ), + ), + ), + Effect.andThen((settled) => + Effect.gen(function* () { + expect(settled.status).toBe("completed") + expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe( + "function value() {\n return 2\n}\n", + ) + }), + ), + ) + }, + (tmp) => Effect.promise(() => tmp[Symbol.asyncDispose]()), + ), + ) + it.live("replaces every exact occurrence when replaceAll is true", () => Effect.acquireUseRelease( Effect.promise(() => tmpdir()), @@ -455,7 +516,7 @@ describe("EditTool", () => { ), ) - it.live("rejects an in-place content change after matching but before conditional commit", () => + it.live("applies the approved edit when another process writes after the read", () => Effect.acquireUseRelease( Effect.promise(() => tmpdir()), (tmp) => { @@ -470,17 +531,51 @@ describe("EditTool", () => { ), Effect.andThen((result) => Effect.gen(function* () { - // The message-less StaleContentError cause must not erase the tool's - // curated failure message; the canonical error is the sole authority. - expect(result).toEqual({ - status: "error", - error: { - type: "tool.execution", - message: "File changed after permission approval. Read it again before editing.", - }, - }) - expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("newer\n") - expect(writes).toEqual([]) + expect(result.status).toBe("completed") + expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("after\n") + expect(writes).toHaveLength(1) + }), + ), + ) + }, + (tmp) => Effect.promise(() => tmp[Symbol.asyncDispose]()), + ), + ) + + it.live("serializes concurrent edits to different sections of one file", () => + Effect.acquireUseRelease( + Effect.promise(() => tmpdir()), + (tmp) => { + reset() + const target = path.join(tmp.path, "concurrent-sections.txt") + afterRead = () => (reads === 1 ? Effect.sleep("50 millis") : Effect.void) + return Effect.promise(() => fs.writeFile(target, "top = 0\nmiddle = keep\nbottom = 0\n")).pipe( + Effect.andThen( + withTool(tmp.path, (registry) => + Effect.all( + [ + executeTool( + registry, + call({ path: "concurrent-sections.txt", oldString: "top = 0", newString: "top = 1" }, "top"), + ), + executeTool( + registry, + call( + { path: "concurrent-sections.txt", oldString: "bottom = 0", newString: "bottom = 2" }, + "bottom", + ), + ), + ], + { concurrency: "unbounded" }, + ), + ), + ), + Effect.andThen((results) => + Effect.gen(function* () { + expect(results.map((result) => result.status)).toEqual(["completed", "completed"]) + expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe( + "top = 1\nmiddle = keep\nbottom = 2\n", + ) }), ), )