From 8a2c1ebe93856ecad473d79731d946a0ee993a79 Mon Sep 17 00:00:00 2001 From: Bertan Ari Date: Tue, 13 Oct 2026 21:24:14 -0700 Subject: [PATCH 01/15] fix(apply-diff): port bounded failure reporting from upstream PR15 Port https://github.com/simurg79/Roo-Code/pull/15. Upstream-commit: d9d96e47006786acbb05d6ef0ee7a927f91f8f12 Retain Zoo diagnostics and telemetry; adapt regression tests to typed mocks and verify repeated-failure reporting. Exclude upstream changeset. --- .../multi-search-replace-marker-hint.spec.ts | 87 +++++ .../diff/strategies/multi-search-replace.ts | 27 +- src/core/tools/ApplyDiffTool.ts | 83 ++++- .../applyDiffTool.failureReporting.spec.ts | 309 ++++++++++++++++++ 4 files changed, 491 insertions(+), 15 deletions(-) create mode 100644 src/core/diff/strategies/__tests__/multi-search-replace-marker-hint.spec.ts create mode 100644 src/core/tools/__tests__/applyDiffTool.failureReporting.spec.ts diff --git a/src/core/diff/strategies/__tests__/multi-search-replace-marker-hint.spec.ts b/src/core/diff/strategies/__tests__/multi-search-replace-marker-hint.spec.ts new file mode 100644 index 0000000000..034dbfc05e --- /dev/null +++ b/src/core/diff/strategies/__tests__/multi-search-replace-marker-hint.spec.ts @@ -0,0 +1,87 @@ +import { MultiSearchReplaceDiffStrategy } from "../multi-search-replace" + +// Guards the POST-FAILURE "LIKELY CAUSE" hint for mistyped start-line markers. +// The heuristic must never act as an upfront rejection gate. +describe("MultiSearchReplaceDiffStrategy malformed start-line marker hint", () => { + let strategy: MultiSearchReplaceDiffStrategy + + beforeEach(() => { + strategy = new MultiSearchReplaceDiffStrategy() + }) + + const nonMatchingFile = ["alpha", "beta", "gamma", "delta", "epsilon"].join("\n") + + function buildDiff(searchBody: string, replaceBody: string): string { + return `<<<<<<< SEARCH\n${searchBody}\n=======\n${replaceBody}\n>>>>>>> REPLACE` + } + + function firstFailureError(result: Awaited>): string { + expect(result.success).toBe(false) + const failures = result.failParts ?? [] + expect(failures.length).toBeGreaterThan(0) + const failure = failures[0] + if (failure.success || !failure.error) { + throw new Error("Expected a failed block with error details") + } + return failure.error + } + + it("adds LIKELY CAUSE when the first search line is a bare ':3' marker and the block fails", async () => { + const diffContent = buildDiff(":3\nnot-in-the-file", "replacement") + const result = await strategy.applyDiff(nonMatchingFile, diffContent) + + const errorText = firstFailureError(result) + expect(errorText).toContain("LIKELY CAUSE") + expect(errorText).toContain(":start_line:") + }) + + const markerVariants = ["start_line:3", ":start-line 3", ": start_line : 3", ":START_LINE:3", "Start_Line: 3"] + + it.each(markerVariants)("adds LIKELY CAUSE for the malformed marker variant %j", async (markerVariant) => { + const diffContent = buildDiff(`${markerVariant}\nnot-in-the-file`, "replacement") + const result = await strategy.applyDiff(nonMatchingFile, diffContent) + + expect(firstFailureError(result)).toContain("LIKELY CAUSE") + }) + + it("still applies cleanly when the file genuinely contains a ':3' line (hint is not a gate)", async () => { + const fileWithMarkerLikeLine = ["alpha", ":3", "beta", "gamma"].join("\n") + const diffContent = buildDiff(":3\nbeta", ":3\nBETA") + + const result = await strategy.applyDiff(fileWithMarkerLikeLine, diffContent) + + expect(result.success).toBe(true) + if (!result.success) { + throw new Error("Expected marker-like file content to match") + } + expect(result.content).toContain("BETA") + expect(result.content).not.toContain("LIKELY CAUSE") + expect(result.failParts ?? []).toHaveLength(0) + }) + + it("omits the hint for ordinary non-matching content but keeps the file-changed tip", async () => { + const diffContent = buildDiff("completely unrelated content\nsecond line", "replacement") + const result = await strategy.applyDiff(nonMatchingFile, diffContent) + + const errorText = firstFailureError(result) + expect(errorText).not.toContain("LIKELY CAUSE") + expect(errorText).toContain("the file content may have changed") + }) + + it("does not flag the correct ':start_line:3' marker form", async () => { + const diffContent = `<<<<<<< SEARCH\n:start_line:3\n-------\nnot-in-the-file\n=======\nreplacement\n>>>>>>> REPLACE` + const result = await strategy.applyDiff(nonMatchingFile, diffContent) + + expect(firstFailureError(result)).not.toContain("LIKELY CAUSE") + }) + + it("adds LIKELY CAUSE when a well-formed ':start_line:N' line goes unparsed and swallows its separator", async () => { + // Indenting the marker makes the parser miss it, so both it and the "-------" become search text. + const diffContent = buildDiff(" :start_line:3\n-------\nnot-in-the-file", "replacement") + const result = await strategy.applyDiff(nonMatchingFile, diffContent) + + const errorText = firstFailureError(result) + expect(errorText).toContain("LIKELY CAUSE") + expect(errorText).toContain('together with the "-------" line after it') + }) +}) diff --git a/src/core/diff/strategies/multi-search-replace.ts b/src/core/diff/strategies/multi-search-replace.ts index cac52c5486..5bf45cb628 100644 --- a/src/core/diff/strategies/multi-search-replace.ts +++ b/src/core/diff/strategies/multi-search-replace.ts @@ -575,9 +575,34 @@ export class MultiSearchReplaceDiffStrategy implements DiffStrategy { ? distance(normalizeString(searchChunk), normalizeString(bestMatchContent)) : -1 + // Only diagnose markers after matching fails: marker-like text can be valid file content. + const hintLines = searchChunk.split("\n") + const firstSearchLine = hintLines[0]?.trim() ?? "" + const separatorSwallowed = hintLines[1]?.trim() === "-------" + const malformedMarkerHint = /^(?::\s*\d{1,9}:?|:?\s*start[_-]?line\s*:?\s*\d{1,9}:?)$/i.test( + firstSearchLine, + ) + ? `\n- LIKELY CAUSE: the first line of your search content is "${firstSearchLine}", which looks like a malformed start-line marker and was therefore searched for as literal file text${separatorSwallowed ? ', together with the "-------" line after it' : ""}. The only recognized form is ":start_line:" on the line immediately after "<<<<<<< SEARCH", optionally followed by a "-------" line. Remove or correct that line and retry.` + : "" + diffResults.push({ success: false, - error: `No sufficiently similar match found${lineRange} (${Math.floor(bestMatchScore * 100)}% similar, needs ${Math.floor(this.fuzzyThreshold * 100)}%)\n\nDebug Info:\n- Similarity Score: ${Math.floor(bestMatchScore * 100)}%\n- Required Threshold: ${Math.floor(this.fuzzyThreshold * 100)}%\n- Search Range: ${startLine ? `starting at line ${startLine}` : "start to end"}\n- Levenshtein Distance: ${levenDist >= 0 ? `${levenDist} characters` : "N/A"}\n- Search Length: ${searchChunk.length} characters\n- Best Match Length: ${bestMatchContent ? bestMatchContent.length : 0} characters\n- Tried both standard and aggressive line number stripping\n- Tip: Use the read_file tool to get the latest content of the file before attempting to use the apply_diff tool again, as the file content may have changed\n- Tip: The SEARCH block must contain complete, whole lines — partial-line (substring) matching is not supported. If the target line is very long, copy the entire line into the SEARCH block\n\nSearch Content:\n${searchChunk}${bestMatchSection}${originalContentSection}`, + error: + `No sufficiently similar match found${lineRange} ` + + `(${Math.floor(bestMatchScore * 100)}% similar, needs ${Math.floor(this.fuzzyThreshold * 100)}%)\n\n` + + `Debug Info:\n` + + `- Similarity Score: ${Math.floor(bestMatchScore * 100)}%\n` + + `- Required Threshold: ${Math.floor(this.fuzzyThreshold * 100)}%\n` + + `- Search Range: ${startLine ? `starting at line ${startLine}` : "start to end"}\n` + + `- Levenshtein Distance: ${levenDist >= 0 ? `${levenDist} characters` : "N/A"}\n` + + `- Search Length: ${searchChunk.length} characters\n` + + `- Best Match Length: ${bestMatchContent ? bestMatchContent.length : 0} characters\n` + + `- Tried both standard and aggressive line number stripping${malformedMarkerHint}\n` + + `- Tip: Use the read_file tool to get the latest content of the file before attempting ` + + `to use the apply_diff tool again, as the file content may have changed\n` + + `- Tip: The SEARCH block must contain complete, whole lines — partial-line (substring) matching is not supported. ` + + `If the target line is very long, copy the entire line into the SEARCH block\n\n` + + `Search Content:\n${searchChunk}${bestMatchSection}${originalContentSection}`, }) continue } diff --git a/src/core/tools/ApplyDiffTool.ts b/src/core/tools/ApplyDiffTool.ts index 3b664b3bd2..6574d16db0 100644 --- a/src/core/tools/ApplyDiffTool.ts +++ b/src/core/tools/ApplyDiffTool.ts @@ -12,7 +12,7 @@ import { RecordSource } from "../context-tracking/FileContextTrackerTypes" import { unescapeHtmlEntities } from "../../utils/text-normalization" import { EXPERIMENT_IDS, experiments } from "../../shared/experiments" import { computeDiffStats, sanitizeUnifiedDiff } from "../diff/stats" -import type { ToolUse } from "../../shared/tools" +import type { DiffResult, ToolUse } from "../../shared/tools" import { BaseTool, ToolCallbacks } from "./BaseTool" @@ -21,6 +21,12 @@ interface ApplyDiffParams { diff: string } +// Failures embed file content; cap both detail size and the number of summaries. +const DETAILED_APPLY_DIFF_FAILURES = 5 +const MAX_DETAILED_FAILURE_CHARACTERS = 4000 +const MAX_SUMMARY_FAILURE_CHARACTERS = 200 +const MAX_ABBREVIATED_FAILURES = 50 + export class ApplyDiffTool extends BaseTool<"apply_diff"> { readonly name = "apply_diff" as const @@ -88,17 +94,13 @@ export class ApplyDiffTool extends BaseTool<"apply_diff"> { TelemetryService.instance.captureDiffApplicationError(task.taskId, currentCount) if (diffResult.failParts && diffResult.failParts.length > 0) { - for (const failPart of diffResult.failParts) { - if (failPart.success) { - continue - } - - const errorDetails = failPart.details ? JSON.stringify(failPart.details, null, 2) : "" + const failedParts = diffResult.failParts.filter((part) => !part.success) + const summary = + failedParts.length > 1 + ? `${failedParts.length} diff blocks failed to apply. Fix the failures below before retrying.\n\n` + : "" - formattedError = `\n${ - failPart.error - }${errorDetails ? `\n\nDetails:\n${errorDetails}` : ""}\n` - } + formattedError = `\n${summary}${this.formatFailedParts(failedParts)}\n` } else { const errorDetails = diffResult.details ? JSON.stringify(diffResult.details, null, 2) : "" @@ -231,17 +233,25 @@ export class ApplyDiffTool extends BaseTool<"apply_diff"> { // Used to determine if we should wait for busy terminal to update before sending api request task.didEditFile = true + + // Literal SEARCH markers inside content can inflate this advisory count. + const searchBlocks = (diffContent.match(/<<<<<<< SEARCH/g) || []).length + const unappliedParts = diffResult.failParts?.filter((part) => !part.success) ?? [] let partFailHint = "" - if (diffResult.failParts && diffResult.failParts.length > 0) { - partFailHint = `But unable to apply all diff parts to file: ${absolutePath}. Use the read_file tool to check the newest file version and re-apply diffs.\n` + if (unappliedParts.length > 0) { + const appliedBlocks = Math.max(0, searchBlocks - unappliedParts.length) + partFailHint = + `Applied ${appliedBlocks} of ${searchBlocks} diff blocks to ${absolutePath}. ` + + `The other ${unappliedParts.length} did NOT apply, so those changes are NOT in the file. ` + + `Fix the failures below and resend ONLY the blocks that failed; the blocks that already applied will no longer match.\n\n` + + `\n${this.formatFailedParts(unappliedParts)}\n\n\n` } // Get the formatted response message const message = await task.diffViewProvider.pushToolWriteResult(task, task.cwd, !fileExists) // Check for single SEARCH/REPLACE block warning - const searchBlocks = (diffContent.match(/<<<<<<< SEARCH/g) || []).length const singleBlockNotice = searchBlocks === 1 ? "\nMaking multiple related changes in a single apply_diff is more efficient. If other changes are needed in this file, please include them as additional SEARCH/REPLACE blocks." @@ -269,6 +279,51 @@ export class ApplyDiffTool extends BaseTool<"apply_diff"> { } } + private formatFailedParts(failedParts: Extract[]): string { + const renderedCount = Math.min(failedParts.length, MAX_ABBREVIATED_FAILURES) + const sections = failedParts.slice(0, renderedCount).map((failPart, index) => { + const label = failedParts.length > 1 ? `--- Diff block ${index + 1} of ${failedParts.length} ---\n` : "" + const error = failPart.error ?? "Unknown apply_diff failure" + + if (index >= DETAILED_APPLY_DIFF_FAILURES) { + const summaryLine = error.split("\n")[0] + return `${label}${this.truncate(summaryLine, MAX_SUMMARY_FAILURE_CHARACTERS)}` + } + + const errorDetails = failPart.details ? JSON.stringify(failPart.details, null, 2) : "" + const detailedSection = `${label}${error}${errorDetails ? `\n\nDetails:\n${errorDetails}` : ""}` + + return this.truncate(detailedSection, MAX_DETAILED_FAILURE_CHARACTERS) + }) + + const trailers: string[] = [] + const abbreviatedRangeStart = DETAILED_APPLY_DIFF_FAILURES + 1 + + if (renderedCount > DETAILED_APPLY_DIFF_FAILURES) { + trailers.push( + `(Failures ${abbreviatedRangeStart}-${renderedCount} are shown as a single summary line each; retry those blocks individually for full detail.)`, + ) + } + + const omitted = failedParts.length - renderedCount + + if (omitted > 0) { + trailers.push(`(+${omitted} more failures not shown; retry those blocks individually for full detail.)`) + } + + return [sections.join("\n\n"), ...trailers].join("\n\n") + } + + private truncate(text: string, maxCharacters: number): string { + if (text.length <= maxCharacters) { + return text + } + + const omitted = text.length - maxCharacters + + return `${text.slice(0, maxCharacters)}\n... [truncated; ${omitted} characters omitted — retry this block alone for full detail]` + } + override async handlePartial(task: Task, block: ToolUse<"apply_diff">): Promise { const relPath: string | undefined = block.params.path const diffContent: string | undefined = block.params.diff diff --git a/src/core/tools/__tests__/applyDiffTool.failureReporting.spec.ts b/src/core/tools/__tests__/applyDiffTool.failureReporting.spec.ts new file mode 100644 index 0000000000..697d695ac0 --- /dev/null +++ b/src/core/tools/__tests__/applyDiffTool.failureReporting.spec.ts @@ -0,0 +1,309 @@ +import fs from "fs/promises" + +import type { DiffResult, ToolResponse } from "../../../shared/tools" +import type { Task } from "../../task/Task" +import { fileExistsAtPath } from "../../../utils/fs" +import type { ToolCallbacks } from "../BaseTool" +import { applyDiffTool } from "../ApplyDiffTool" + +vi.mock("@roo-code/telemetry", () => ({ + TelemetryService: { instance: { captureDiffApplicationError: vi.fn() } }, +})) + +vi.mock("fs/promises", () => ({ + default: { + readFile: vi.fn().mockResolvedValue(""), + }, +})) + +vi.mock("../../../utils/fs", () => ({ + fileExistsAtPath: vi.fn().mockResolvedValue(true), +})) + +vi.mock("../../../utils/path", () => ({ + getReadablePath: vi.fn().mockReturnValue("test/file.txt"), +})) + +vi.mock("../../prompts/responses", () => ({ + formatResponse: { + toolError: vi.fn((message: string) => `Error: ${message}`), + rooIgnoreError: vi.fn((filePath: string) => `Access denied: ${filePath}`), + createPrettyPatch: vi.fn(() => "mock-diff"), + }, +})) + +vi.mock("../../diff/stats", () => ({ + sanitizeUnifiedDiff: vi.fn((diff: string) => diff), + computeDiffStats: vi.fn(() => ({ additions: 1, deletions: 1 })), +})) + +vi.mock("vscode", () => ({ + window: { showWarningMessage: vi.fn() }, + env: { openExternal: vi.fn() }, + Uri: { parse: vi.fn() }, +})) + +type FailPart = Extract + +describe("ApplyDiffTool failure reporting", () => { + const testFilePath = "test/file.txt" + + let mockTask: Task + let mockAskApproval: ReturnType> + let mockHandleError: ReturnType> + let pushedResult: ToolResponse | undefined + + beforeEach(() => { + vi.clearAllMocks() + pushedResult = undefined + vi.mocked(fileExistsAtPath).mockResolvedValue(true) + vi.mocked(fs.readFile).mockResolvedValue("line one\nline two\nline three\n") + + mockAskApproval = vi.fn().mockResolvedValue(true) + mockHandleError = vi.fn().mockResolvedValue(undefined) + + // Only the provider, edit tracking, and diff-view members used by execute are needed. + mockTask = { + cwd: "/workspace", + consecutiveMistakeCount: 0, + consecutiveMistakeCountForApplyDiff: new Map(), + didEditFile: false, + api: { getModel: () => ({ id: "claude-test" }) }, + rooIgnoreController: { validateAccess: vi.fn().mockReturnValue(true) }, + rooProtectedController: { isWriteProtected: vi.fn().mockReturnValue(false) }, + providerRef: { + deref: vi.fn().mockReturnValue({ + getState: vi.fn().mockResolvedValue({ + diagnosticsEnabled: true, + writeDelayMs: 0, + experiments: {}, + }), + }), + }, + diffViewProvider: { + editType: undefined, + originalContent: "", + open: vi.fn().mockResolvedValue(undefined), + update: vi.fn().mockResolvedValue(undefined), + reset: vi.fn().mockResolvedValue(undefined), + revertChanges: vi.fn().mockResolvedValue(undefined), + saveChanges: vi.fn().mockResolvedValue(undefined), + saveDirectly: vi.fn().mockResolvedValue(undefined), + scrollToFirstDiff: vi.fn(), + pushToolWriteResult: vi.fn().mockResolvedValue("Changes successfully applied."), + }, + fileContextTracker: { trackFileContext: vi.fn().mockResolvedValue(undefined) }, + say: vi.fn().mockResolvedValue(undefined), + ask: vi.fn().mockResolvedValue(undefined), + recordToolError: vi.fn(), + recordToolUsage: vi.fn(), + processQueuedMessages: vi.fn(), + sayAndCreateMissingParamError: vi.fn().mockResolvedValue("Missing param error"), + diffStrategy: undefined, + } as unknown as Task + }) + + function makeFailure(errorText: string | undefined, details?: FailPart["details"]): FailPart { + return { success: false, error: errorText, details, failParts: [] } + } + + function buildDiffContent(blockCount: number): string { + const blocks: string[] = [] + for (let blockIndex = 0; blockIndex < blockCount; blockIndex++) { + blocks.push(`<<<<<<< SEARCH\nsearch-${blockIndex}\n=======\nreplace-${blockIndex}\n>>>>>>> REPLACE`) + } + return blocks.join("\n") + } + + async function runApplyDiff(diffResult: DiffResult, blockCount: number): Promise { + mockTask.diffStrategy = { + getName: () => "test", + applyDiff: vi.fn().mockResolvedValue(diffResult), + getProgressStatus: vi.fn().mockReturnValue({ icon: "diff-multiple", text: "x" }), + } + + await applyDiffTool.execute({ path: testFilePath, diff: buildDiffContent(blockCount) }, mockTask, { + askApproval: mockAskApproval, + handleError: mockHandleError, + pushToolResult: vi.fn((result: ToolResponse) => { + pushedResult = result + }), + }) + + expect(mockHandleError).not.toHaveBeenCalled() + return String(pushedResult ?? "") + } + + function makeFailureSet(count: number): FailPart[] { + const failures: FailPart[] = [] + for (let failureIndex = 1; failureIndex <= count; failureIndex++) { + failures.push(makeFailure(`FAILURE-MARKER-${failureIndex}\nsecond line of failure ${failureIndex}`)) + } + return failures + } + + describe("failure accumulation and rendering", () => { + it("reports every failed block, not just the last one", async () => { + const output = await runApplyDiff({ success: false, failParts: makeFailureSet(2) }, 2) + + expect(output).toContain("FAILURE-MARKER-1") + expect(output).toContain("FAILURE-MARKER-2") + expect(output).toContain("--- Diff block 1 of 2 ---") + expect(output).toContain("--- Diff block 2 of 2 ---") + }) + + it("records every failure and shows the same details on the second consecutive error", async () => { + const result: DiffResult = { success: false, failParts: makeFailureSet(2) } + const firstOutput = await runApplyDiff(result, 2) + expect(mockTask.say).not.toHaveBeenCalled() + expect(mockTask.recordToolError).toHaveBeenLastCalledWith("apply_diff", firstOutput) + + const secondOutput = await runApplyDiff(result, 2) + expect(mockTask.say).toHaveBeenCalledWith("diff_error", secondOutput) + expect(secondOutput).toContain("FAILURE-MARKER-1") + expect(secondOutput).toContain("FAILURE-MARKER-2") + }) + + it("renders exactly five failures in full detail with no abbreviation trailer", async () => { + const output = await runApplyDiff({ success: false, failParts: makeFailureSet(5) }, 5) + + for (let failureIndex = 1; failureIndex <= 5; failureIndex++) { + expect(output).toContain(`FAILURE-MARKER-${failureIndex}`) + expect(output).toContain(`second line of failure ${failureIndex}`) + } + expect(output).not.toContain("are shown as a single summary line each") + }) + + it("abbreviates the sixth failure and emits a trailer naming the range", async () => { + const output = await runApplyDiff({ success: false, failParts: makeFailureSet(6) }, 6) + + expect(output).toContain("FAILURE-MARKER-6") + expect(output).not.toContain("second line of failure 6") + expect(output).toContain("second line of failure 5") + expect(output).toContain("(Failures 6-6 are shown as a single summary line each") + }) + + it("omits the diff block label when there is a single failure", async () => { + const output = await runApplyDiff({ success: false, failParts: makeFailureSet(1) }, 1) + + expect(output).toContain("FAILURE-MARKER-1") + expect(output).not.toContain("--- Diff block") + }) + + it("produces no stray separators for an empty failures array", async () => { + const output = await runApplyDiff({ success: false, failParts: [], error: "nothing applied" }, 1) + + expect(output).not.toContain("--- Diff block") + expect(output).not.toContain("are shown as a single summary line each") + }) + + it("substitutes a placeholder when a failure has no error text", async () => { + const output = await runApplyDiff({ success: false, failParts: [makeFailure(undefined)] }, 1) + + expect(output).toContain("Unknown apply_diff failure") + }) + + it("appends Details for detailed failures and omits them for abbreviated ones", async () => { + const failures = makeFailureSet(6) + failures[0] = makeFailure("FAILURE-MARKER-1\nsecond line of failure 1", { bestMatch: "DETAIL-FIRST" }) + failures[5] = makeFailure("FAILURE-MARKER-6\nsecond line of failure 6", { bestMatch: "DETAIL-SIXTH" }) + + const output = await runApplyDiff({ success: false, failParts: failures }, 6) + + expect(output).toContain("Details:") + expect(output).toContain("DETAIL-FIRST") + expect(output).not.toContain("DETAIL-SIXTH") + }) + + it("truncates a detailed failure that exceeds the character cap", async () => { + const oversizedError = `FAILURE-MARKER-1\n${"x".repeat(6000)}` + const output = await runApplyDiff({ success: false, failParts: [makeFailure(oversizedError)] }, 1) + + expect(output).toContain("characters omitted — retry this block alone for full detail") + expect(output).not.toContain("x".repeat(5000)) + }) + + it("leaves a detailed failure just under the character cap untruncated", async () => { + const nearCapError = `FAILURE-MARKER-1\n${"x".repeat(3900)}` + const output = await runApplyDiff({ success: false, failParts: [makeFailure(nearCapError)] }, 1) + + expect(output).not.toContain("[truncated") + expect(output).toContain("x".repeat(3900)) + }) + + it("truncates an abbreviated one-line summary that exceeds the summary cap", async () => { + const failures = makeFailureSet(6) + failures[5] = makeFailure(`FAILURE-MARKER-6 ${"y".repeat(500)}\nsecond line of failure 6`) + + const output = await runApplyDiff({ success: false, failParts: failures }, 6) + + expect(output).not.toContain("y".repeat(400)) + expect(output).toContain("characters omitted — retry this block alone for full detail") + }) + + it("emits a real trailer range when eight failures are reported", async () => { + const output = await runApplyDiff({ success: false, failParts: makeFailureSet(8) }, 8) + + expect(output).toContain("(Failures 6-8 are shown as a single summary line each") + expect(output).toContain("FAILURE-MARKER-8") + expect(output).not.toContain("second line of failure 8") + }) + + it("caps the number of abbreviated failures and reports the remainder as a count", async () => { + const output = await runApplyDiff({ success: false, failParts: makeFailureSet(60) }, 60) + + expect(output).toContain("(Failures 6-50 are shown as a single summary line each") + expect(output).toContain("(+10 more failures not shown") + expect(output).not.toContain("FAILURE-MARKER-51") + }) + }) + + describe("partial apply path", () => { + function partialResult(failures: FailPart[]): DiffResult { + return { success: true, content: "updated content", failParts: failures } + } + + it("reports how many blocks applied and asks for only the failed blocks to be resent", async () => { + const output = await runApplyDiff(partialResult(makeFailureSet(1)), 2) + + expect(output).toContain("Applied 1 of 2") + expect(output).toContain("FAILURE-MARKER-1") + expect(output).toContain("resend ONLY the blocks that failed") + }) + + it("does not tell the model to use read_file to re-apply the diffs", async () => { + const output = await runApplyDiff(partialResult(makeFailureSet(1)), 2) + + expect(output).not.toContain("re-apply") + }) + + it("applies the abbreviation cap on the partial path as well", async () => { + const output = await runApplyDiff(partialResult(makeFailureSet(6)), 8) + + expect(output).toContain("Applied 2 of 8") + expect(output).toContain("(Failures 6-6 are shown as a single summary line each") + expect(output).not.toContain("second line of failure 6") + }) + + it("emits no partial-apply hint when every block applies", async () => { + const output = await runApplyDiff({ success: true, content: "updated content", failParts: [] }, 1) + + expect(output).not.toContain("Applied") + expect(output).toContain("Changes successfully applied.") + expect(output).toContain("Making multiple related changes in a single apply_diff is more efficient") + }) + + it("counts and renders only the unsuccessful entries in failParts", async () => { + const mixedParts: DiffResult[] = [ + { success: true, content: "first change" }, + makeFailure("FAILURE-MARKER-1\nsecond line of failure 1"), + { success: true, content: "third change" }, + ] + const output = await runApplyDiff({ success: true, content: "updated content", failParts: mixedParts }, 3) + + expect(output).toContain("Applied 2 of 3") + expect(output).toContain("FAILURE-MARKER-1") + expect(output).not.toContain("--- Diff block") + }) + }) +}) From 541a0e2d81d1907a5eb331867a85a0f754d81175 Mon Sep 17 00:00:00 2001 From: Bertan Ari Date: Wed, 14 Oct 2026 12:29:18 -0700 Subject: [PATCH 02/15] fix(apply-diff): name target file in multi-block failure output --- src/core/tools/ApplyDiffTool.ts | 2 +- .../__tests__/applyDiffTool.failureReporting.spec.ts | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/core/tools/ApplyDiffTool.ts b/src/core/tools/ApplyDiffTool.ts index 6574d16db0..dc29adc53d 100644 --- a/src/core/tools/ApplyDiffTool.ts +++ b/src/core/tools/ApplyDiffTool.ts @@ -100,7 +100,7 @@ export class ApplyDiffTool extends BaseTool<"apply_diff"> { ? `${failedParts.length} diff blocks failed to apply. Fix the failures below before retrying.\n\n` : "" - formattedError = `\n${summary}${this.formatFailedParts(failedParts)}\n` + formattedError = `Unable to apply diff to file: ${absolutePath}\n\n\n${summary}${this.formatFailedParts(failedParts)}\n` } else { const errorDetails = diffResult.details ? JSON.stringify(diffResult.details, null, 2) : "" diff --git a/src/core/tools/__tests__/applyDiffTool.failureReporting.spec.ts b/src/core/tools/__tests__/applyDiffTool.failureReporting.spec.ts index 697d695ac0..f47d175645 100644 --- a/src/core/tools/__tests__/applyDiffTool.failureReporting.spec.ts +++ b/src/core/tools/__tests__/applyDiffTool.failureReporting.spec.ts @@ -1,4 +1,5 @@ import fs from "fs/promises" +import path from "path" import type { DiffResult, ToolResponse } from "../../../shared/tools" import type { Task } from "../../task/Task" @@ -152,6 +153,13 @@ describe("ApplyDiffTool failure reporting", () => { expect(output).toContain("--- Diff block 2 of 2 ---") }) + it("names the target file before the multi-block failure details", async () => { + const output = await runApplyDiff({ success: false, failParts: makeFailureSet(2) }, 2) + + const expectedPrefix = `Unable to apply diff to file: ${path.resolve("/workspace", testFilePath)}\n\n\n` + expect(output.startsWith(expectedPrefix)).toBe(true) + }) + it("records every failure and shows the same details on the second consecutive error", async () => { const result: DiffResult = { success: false, failParts: makeFailureSet(2) } const firstOutput = await runApplyDiff(result, 2) From 1cc062bb7e1b712178d288016df362aabac578ba Mon Sep 17 00:00:00 2001 From: Bertan Ari Date: Thu, 15 Oct 2026 05:42:41 -0700 Subject: [PATCH 03/15] test(apply-diff): kill surviving mutants in failure reporting and marker hint --- .../multi-search-replace-marker-hint.spec.ts | 56 ++++++++++++++++- .../applyDiffTool.failureReporting.spec.ts | 62 ++++++++++++++++++- 2 files changed, 115 insertions(+), 3 deletions(-) diff --git a/src/core/diff/strategies/__tests__/multi-search-replace-marker-hint.spec.ts b/src/core/diff/strategies/__tests__/multi-search-replace-marker-hint.spec.ts index 034dbfc05e..3e269b0910 100644 --- a/src/core/diff/strategies/__tests__/multi-search-replace-marker-hint.spec.ts +++ b/src/core/diff/strategies/__tests__/multi-search-replace-marker-hint.spec.ts @@ -44,6 +44,56 @@ describe("MultiSearchReplaceDiffStrategy malformed start-line marker hint", () = expect(firstFailureError(result)).toContain("LIKELY CAUSE") }) + it.each([": 3", ":42", "startline:3", "start_line:42"])( + "adds LIKELY CAUSE for the spacing, multi-digit or separator-less marker %j", + async (markerVariant) => { + const diffContent = buildDiff(`${markerVariant}\nnot-in-the-file`, "replacement") + const result = await strategy.applyDiff(nonMatchingFile, diffContent) + + expect(firstFailureError(result)).toContain(`first line of your search content is "${markerVariant}"`) + }, + ) + + it.each(["value:3", ":3 trailing", "x start_line:3"])( + "omits LIKELY CAUSE when %j only contains a marker-like fragment", + async (firstLine) => { + const diffContent = buildDiff(`${firstLine}\nnot-in-the-file`, "replacement") + const result = await strategy.applyDiff(nonMatchingFile, diffContent) + + expect(firstFailureError(result)).not.toContain("LIKELY CAUSE") + }, + ) + + it("does not claim a separator was swallowed when none follows the marker", async () => { + const diffContent = buildDiff(":3\nnot-in-the-file", "replacement") + const result = await strategy.applyDiff(nonMatchingFile, diffContent) + + const errorText = firstFailureError(result) + expect(errorText).toContain( + "looks like a malformed start-line marker and was therefore searched for as literal file text. The only recognized form", + ) + expect(errorText).not.toContain("together with") + }) + + it("detects a swallowed separator even when it is indented", async () => { + const diffContent = buildDiff(" :start_line:3\n -------\nnot-in-the-file", "replacement") + const result = await strategy.applyDiff(nonMatchingFile, diffContent) + + expect(firstFailureError(result)).toContain('together with the "-------" line after it') + }) + + it("reports the real similarity, threshold and distance for a near miss", async () => { + const diffContent = buildDiff("alpha\nbetx", "replacement") + const result = await strategy.applyDiff(nonMatchingFile, diffContent) + + const errorText = firstFailureError(result) + const similarity = /\((\d+)% similar, needs 100%\)\n\nDebug Info:\n- Similarity Score: (\d+)%\n/.exec(errorText) + expect(similarity).not.toBeNull() + expect(Number(similarity?.[1])).toBeGreaterThan(50) + expect(similarity?.[2]).toBe(similarity?.[1]) + expect(errorText).toMatch(/- Levenshtein Distance: [1-9]\d* characters\n/) + }) + it("still applies cleanly when the file genuinely contains a ':3' line (hint is not a gate)", async () => { const fileWithMarkerLikeLine = ["alpha", ":3", "beta", "gamma"].join("\n") const diffContent = buildDiff(":3\nbeta", ":3\nBETA") @@ -65,7 +115,11 @@ describe("MultiSearchReplaceDiffStrategy malformed start-line marker hint", () = const errorText = firstFailureError(result) expect(errorText).not.toContain("LIKELY CAUSE") - expect(errorText).toContain("the file content may have changed") + expect(errorText).toContain( + "- Tried both standard and aggressive line number stripping\n" + + "- Tip: Use the read_file tool to get the latest content of the file before attempting " + + "to use the apply_diff tool again, as the file content may have changed\n\n", + ) }) it("does not flag the correct ':start_line:3' marker form", async () => { diff --git a/src/core/tools/__tests__/applyDiffTool.failureReporting.spec.ts b/src/core/tools/__tests__/applyDiffTool.failureReporting.spec.ts index f47d175645..b63f2d9bf3 100644 --- a/src/core/tools/__tests__/applyDiffTool.failureReporting.spec.ts +++ b/src/core/tools/__tests__/applyDiffTool.failureReporting.spec.ts @@ -116,14 +116,15 @@ describe("ApplyDiffTool failure reporting", () => { return blocks.join("\n") } - async function runApplyDiff(diffResult: DiffResult, blockCount: number): Promise { + async function runApplyDiff(diffResult: DiffResult, blockCount: number, diffOverride?: string): Promise { mockTask.diffStrategy = { getName: () => "test", applyDiff: vi.fn().mockResolvedValue(diffResult), getProgressStatus: vi.fn().mockReturnValue({ icon: "diff-multiple", text: "x" }), } - await applyDiffTool.execute({ path: testFilePath, diff: buildDiffContent(blockCount) }, mockTask, { + const diff = diffOverride ?? buildDiffContent(blockCount) + await applyDiffTool.execute({ path: testFilePath, diff }, mockTask, { askApproval: mockAskApproval, handleError: mockHandleError, pushToolResult: vi.fn((result: ToolResponse) => { @@ -257,6 +258,44 @@ describe("ApplyDiffTool failure reporting", () => { expect(output).not.toContain("second line of failure 8") }) + it("prefixes multiple failures with a count summary that ignores successful parts", async () => { + const mixedParts: DiffResult[] = [{ success: true, content: "applied" }, ...makeFailureSet(2)] + const output = await runApplyDiff({ success: false, failParts: mixedParts }, 3) + + expect(output).toContain( + "\n2 diff blocks failed to apply. Fix the failures below before retrying.\n\n--- Diff block 1 of 2 ---\n", + ) + expect(output).toContain("second line of failure 1\n\n--- Diff block 2 of 2 ---\nFAILURE-MARKER-2") + }) + + it("renders a single failure verbatim with no summary, label, details or trailer", async () => { + const output = await runApplyDiff({ success: false, failParts: makeFailureSet(1) }, 1) + + expect(output).toContain("\nFAILURE-MARKER-1\nsecond line of failure 1\n") + }) + + it("separates the abbreviated trailer from the last summary and omits the remainder trailer", async () => { + const output = await runApplyDiff({ success: false, failParts: makeFailureSet(6) }, 6) + + expect(output).toContain("FAILURE-MARKER-6\n\n(Failures 6-6 are shown") + expect(output).not.toContain("more failures not shown") + }) + + it("does not truncate a detailed failure of exactly the character cap", async () => { + const exactCapError = "z".repeat(4000) + const output = await runApplyDiff({ success: false, failParts: [makeFailure(exactCapError)] }, 1) + + expect(output).not.toContain("[truncated") + expect(output).toContain(`${exactCapError}\n`) + }) + + it("reports the exact number of omitted characters when truncating", async () => { + const oversizedError = `FAILURE-MARKER-1\n${"x".repeat(6000)}` + const output = await runApplyDiff({ success: false, failParts: [makeFailure(oversizedError)] }, 1) + + expect(output).toContain(`[truncated; ${oversizedError.length - 4000} characters omitted`) + }) + it("caps the number of abbreviated failures and reports the remainder as a count", async () => { const output = await runApplyDiff({ success: false, failParts: makeFailureSet(60) }, 60) @@ -285,6 +324,25 @@ describe("ApplyDiffTool failure reporting", () => { expect(output).not.toContain("re-apply") }) + it("states how many blocks did not apply", async () => { + const output = await runApplyDiff(partialResult(makeFailureSet(2)), 3) + + expect(output).toContain(`Applied 1 of 3 diff blocks to ${path.resolve("/workspace", testFilePath)}. `) + expect(output).toContain("The other 2 did NOT apply, so those changes are NOT in the file. ") + }) + + it("tolerates a successful result without failParts or SEARCH markers", async () => { + const output = await runApplyDiff( + { success: true, content: "updated content" }, + 0, + "diff text without block markers", + ) + + expect(output).not.toContain("Applied") + expect(output).not.toContain("Making multiple related changes") + expect(output).toContain("Changes successfully applied.") + }) + it("applies the abbreviation cap on the partial path as well", async () => { const output = await runApplyDiff(partialResult(makeFailureSet(6)), 8) From 738d68b4194b9a557e2605d685883f5d51aeb828 Mon Sep 17 00:00:00 2001 From: Bertan Ari Date: Thu, 15 Oct 2026 06:05:25 -0700 Subject: [PATCH 04/15] test(apply-diff): pass toolCallId required by ToolCallbacks on main --- src/core/tools/__tests__/applyDiffTool.failureReporting.spec.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/core/tools/__tests__/applyDiffTool.failureReporting.spec.ts b/src/core/tools/__tests__/applyDiffTool.failureReporting.spec.ts index b63f2d9bf3..70d09c3592 100644 --- a/src/core/tools/__tests__/applyDiffTool.failureReporting.spec.ts +++ b/src/core/tools/__tests__/applyDiffTool.failureReporting.spec.ts @@ -127,6 +127,7 @@ describe("ApplyDiffTool failure reporting", () => { await applyDiffTool.execute({ path: testFilePath, diff }, mockTask, { askApproval: mockAskApproval, handleError: mockHandleError, + toolCallId: "tool-call-1", pushToolResult: vi.fn((result: ToolResponse) => { pushedResult = result }), From 92d8de195fa12b752b37aea325f49a909d53485a Mon Sep 17 00:00:00 2001 From: Bertan Ari Date: Sat, 17 Oct 2026 14:36:43 -0700 Subject: [PATCH 05/15] fix(apply-diff): count line-level markers and retain main guidance --- .../multi-search-replace-marker-hint.spec.ts | 4 +++- src/core/tools/ApplyDiffTool.ts | 3 +-- .../applyDiffTool.failureReporting.spec.ts | 21 +++++++++++++++++++ 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/src/core/diff/strategies/__tests__/multi-search-replace-marker-hint.spec.ts b/src/core/diff/strategies/__tests__/multi-search-replace-marker-hint.spec.ts index 3e269b0910..1c946fcbc8 100644 --- a/src/core/diff/strategies/__tests__/multi-search-replace-marker-hint.spec.ts +++ b/src/core/diff/strategies/__tests__/multi-search-replace-marker-hint.spec.ts @@ -118,7 +118,9 @@ describe("MultiSearchReplaceDiffStrategy malformed start-line marker hint", () = expect(errorText).toContain( "- Tried both standard and aggressive line number stripping\n" + "- Tip: Use the read_file tool to get the latest content of the file before attempting " + - "to use the apply_diff tool again, as the file content may have changed\n\n", + "to use the apply_diff tool again, as the file content may have changed\n" + + "- Tip: The SEARCH block must contain complete, whole lines — partial-line (substring) matching is not supported. " + + "If the target line is very long, copy the entire line into the SEARCH block\n\n", ) }) diff --git a/src/core/tools/ApplyDiffTool.ts b/src/core/tools/ApplyDiffTool.ts index dc29adc53d..2c8ce7ee48 100644 --- a/src/core/tools/ApplyDiffTool.ts +++ b/src/core/tools/ApplyDiffTool.ts @@ -234,8 +234,7 @@ export class ApplyDiffTool extends BaseTool<"apply_diff"> { // Used to determine if we should wait for busy terminal to update before sending api request task.didEditFile = true - // Literal SEARCH markers inside content can inflate this advisory count. - const searchBlocks = (diffContent.match(/<<<<<<< SEARCH/g) || []).length + const searchBlocks = (diffContent.match(/^<<<<<<< SEARCH>?[^\S\r\n]*\r?$/gm) || []).length const unappliedParts = diffResult.failParts?.filter((part) => !part.success) ?? [] let partFailHint = "" diff --git a/src/core/tools/__tests__/applyDiffTool.failureReporting.spec.ts b/src/core/tools/__tests__/applyDiffTool.failureReporting.spec.ts index 70d09c3592..0c1fedde1c 100644 --- a/src/core/tools/__tests__/applyDiffTool.failureReporting.spec.ts +++ b/src/core/tools/__tests__/applyDiffTool.failureReporting.spec.ts @@ -325,6 +325,27 @@ describe("ApplyDiffTool failure reporting", () => { expect(output).not.toContain("re-apply") }) + it.each(["\n", "\r\n"])( + "counts only unescaped line-level SEARCH markers with %j endings", + async (lineEnding) => { + const diff = [ + "<<<<<<< SEARCH> ", + "\\<<<<<<< SEARCH", + "inline <<<<<<< SEARCH", + "<<<<<<< SEARCHING", + "=======", + "replacement", + ">>>>>>> REPLACE", + buildDiffContent(1), + ] + .join("\n") + .replace(/\n/g, lineEnding) + const output = await runApplyDiff(partialResult(makeFailureSet(1)), 2, diff) + + expect(output).toContain("Applied 1 of 2 diff blocks") + }, + ) + it("states how many blocks did not apply", async () => { const output = await runApplyDiff(partialResult(makeFailureSet(2)), 3) From 560a68de9905b1f4e8f2e6f6a0b27fa9a54aa6d2 Mon Sep 17 00:00:00 2001 From: Bertan Ari Date: Sat, 17 Oct 2026 14:59:49 -0700 Subject: [PATCH 06/15] fix(apply-diff): avoid claiming failed blocks are absent; assert full replacement content --- .../__tests__/multi-search-replace-marker-hint.spec.ts | 2 +- src/core/tools/ApplyDiffTool.ts | 5 +++-- .../tools/__tests__/applyDiffTool.failureReporting.spec.ts | 7 +++++-- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/core/diff/strategies/__tests__/multi-search-replace-marker-hint.spec.ts b/src/core/diff/strategies/__tests__/multi-search-replace-marker-hint.spec.ts index 1c946fcbc8..a8929f8048 100644 --- a/src/core/diff/strategies/__tests__/multi-search-replace-marker-hint.spec.ts +++ b/src/core/diff/strategies/__tests__/multi-search-replace-marker-hint.spec.ts @@ -104,7 +104,7 @@ describe("MultiSearchReplaceDiffStrategy malformed start-line marker hint", () = if (!result.success) { throw new Error("Expected marker-like file content to match") } - expect(result.content).toContain("BETA") + expect(result.content).toBe(["alpha", ":3", "BETA", "gamma"].join("\n")) expect(result.content).not.toContain("LIKELY CAUSE") expect(result.failParts ?? []).toHaveLength(0) }) diff --git a/src/core/tools/ApplyDiffTool.ts b/src/core/tools/ApplyDiffTool.ts index 2c8ce7ee48..0656b56cae 100644 --- a/src/core/tools/ApplyDiffTool.ts +++ b/src/core/tools/ApplyDiffTool.ts @@ -242,8 +242,9 @@ export class ApplyDiffTool extends BaseTool<"apply_diff"> { const appliedBlocks = Math.max(0, searchBlocks - unappliedParts.length) partFailHint = `Applied ${appliedBlocks} of ${searchBlocks} diff blocks to ${absolutePath}. ` + - `The other ${unappliedParts.length} did NOT apply, so those changes are NOT in the file. ` + - `Fix the failures below and resend ONLY the blocks that failed; the blocks that already applied will no longer match.\n\n` + + `The other ${unappliedParts.length} did NOT apply. ` + + `Inspect the saved file before retrying, since an earlier block may already contain the intended change. ` + + `Then resend ONLY the failed blocks that are still needed; the blocks that already applied will no longer match.\n\n` + `\n${this.formatFailedParts(unappliedParts)}\n\n\n` } diff --git a/src/core/tools/__tests__/applyDiffTool.failureReporting.spec.ts b/src/core/tools/__tests__/applyDiffTool.failureReporting.spec.ts index 0c1fedde1c..59ab2bea69 100644 --- a/src/core/tools/__tests__/applyDiffTool.failureReporting.spec.ts +++ b/src/core/tools/__tests__/applyDiffTool.failureReporting.spec.ts @@ -316,7 +316,7 @@ describe("ApplyDiffTool failure reporting", () => { expect(output).toContain("Applied 1 of 2") expect(output).toContain("FAILURE-MARKER-1") - expect(output).toContain("resend ONLY the blocks that failed") + expect(output).toContain("resend ONLY the failed blocks that are still needed") }) it("does not tell the model to use read_file to re-apply the diffs", async () => { @@ -350,7 +350,10 @@ describe("ApplyDiffTool failure reporting", () => { const output = await runApplyDiff(partialResult(makeFailureSet(2)), 3) expect(output).toContain(`Applied 1 of 3 diff blocks to ${path.resolve("/workspace", testFilePath)}. `) - expect(output).toContain("The other 2 did NOT apply, so those changes are NOT in the file. ") + expect(output).toContain( + "The other 2 did NOT apply. Inspect the saved file before retrying, since an earlier block may already contain the intended change. ", + ) + expect(output).not.toContain("NOT in the file") }) it("tolerates a successful result without failParts or SEARCH markers", async () => { From 64ddf474a17f5a65c9978960f4752fea5d329f25 Mon Sep 17 00:00:00 2001 From: Bertan Ari Date: Sat, 17 Oct 2026 15:22:09 -0700 Subject: [PATCH 07/15] fix(apply-diff): report partial-apply totals from the parsed block count --- .../multi-search-replace-marker-hint.spec.ts | 14 ++++++++++++++ src/core/diff/strategies/multi-search-replace.ts | 1 + src/core/tools/ApplyDiffTool.ts | 4 +++- .../applyDiffTool.failureReporting.spec.ts | 11 +++++++++++ src/shared/tools.ts | 2 +- 5 files changed, 30 insertions(+), 2 deletions(-) diff --git a/src/core/diff/strategies/__tests__/multi-search-replace-marker-hint.spec.ts b/src/core/diff/strategies/__tests__/multi-search-replace-marker-hint.spec.ts index a8929f8048..d0a3737b7a 100644 --- a/src/core/diff/strategies/__tests__/multi-search-replace-marker-hint.spec.ts +++ b/src/core/diff/strategies/__tests__/multi-search-replace-marker-hint.spec.ts @@ -109,6 +109,20 @@ describe("MultiSearchReplaceDiffStrategy malformed start-line marker hint", () = expect(result.failParts ?? []).toHaveLength(0) }) + it("reports the parsed block count when repair splits an inline SEARCH marker", async () => { + const original = ["alpha", "beta"].join("\n") + const diffContent = "<<<<<<< SEARCH\nalpha\n=======\nALPHA x <<<<<<< SEARCH\nmissing\n>>>>>>> REPLACE" + + const result = await strategy.applyDiff(original, diffContent) + + expect(result.success).toBe(true) + if (!result.success) { + throw new Error("Expected the first repaired block to apply") + } + expect(result.blockCount).toBe(2) + expect(result.failParts?.filter((part) => !part.success)).toHaveLength(1) + }) + it("omits the hint for ordinary non-matching content but keeps the file-changed tip", async () => { const diffContent = buildDiff("completely unrelated content\nsecond line", "replacement") const result = await strategy.applyDiff(nonMatchingFile, diffContent) diff --git a/src/core/diff/strategies/multi-search-replace.ts b/src/core/diff/strategies/multi-search-replace.ts index 5bf45cb628..c2997ee105 100644 --- a/src/core/diff/strategies/multi-search-replace.ts +++ b/src/core/diff/strategies/multi-search-replace.ts @@ -666,6 +666,7 @@ export class MultiSearchReplaceDiffStrategy implements DiffStrategy { success: true, content: finalContent, failParts: diffResults, + blockCount: matches.length, } } diff --git a/src/core/tools/ApplyDiffTool.ts b/src/core/tools/ApplyDiffTool.ts index 0656b56cae..fc47a12ec6 100644 --- a/src/core/tools/ApplyDiffTool.ts +++ b/src/core/tools/ApplyDiffTool.ts @@ -234,7 +234,9 @@ export class ApplyDiffTool extends BaseTool<"apply_diff"> { // Used to determine if we should wait for busy terminal to update before sending api request task.didEditFile = true - const searchBlocks = (diffContent.match(/^<<<<<<< SEARCH>?[^\S\r\n]*\r?$/gm) || []).length + // Prefer the parser's count: truncation repair can split inline markers into extra blocks. + const searchBlocks = + diffResult.blockCount ?? (diffContent.match(/^<<<<<<< SEARCH>?[^\S\r\n]*\r?$/gm) || []).length const unappliedParts = diffResult.failParts?.filter((part) => !part.success) ?? [] let partFailHint = "" diff --git a/src/core/tools/__tests__/applyDiffTool.failureReporting.spec.ts b/src/core/tools/__tests__/applyDiffTool.failureReporting.spec.ts index 59ab2bea69..836df38cd3 100644 --- a/src/core/tools/__tests__/applyDiffTool.failureReporting.spec.ts +++ b/src/core/tools/__tests__/applyDiffTool.failureReporting.spec.ts @@ -346,6 +346,17 @@ describe("ApplyDiffTool failure reporting", () => { }, ) + it("prefers the parser-reported block count over the raw marker count", async () => { + const output = await runApplyDiff( + { success: true, content: "updated content", failParts: makeFailureSet(1), blockCount: 2 }, + 1, + buildDiffContent(1), + ) + + expect(output).toContain("Applied 1 of 2 diff blocks") + expect(output).toContain("The other 1 did NOT apply.") + }) + it("states how many blocks did not apply", async () => { const output = await runApplyDiff(partialResult(makeFailureSet(2)), 3) diff --git a/src/shared/tools.ts b/src/shared/tools.ts index b9a56d7d3d..8730fd4446 100644 --- a/src/shared/tools.ts +++ b/src/shared/tools.ts @@ -251,7 +251,7 @@ export const TOOL_ALIASES: Record = { } as const export type DiffResult = - | { success: true; content: string; failParts?: DiffResult[] } + | { success: true; content: string; failParts?: DiffResult[]; blockCount?: number } | ({ success: false error?: string From f2cf1c60bd985c81e7a8715f95865b6cbe635f6d Mon Sep 17 00:00:00 2001 From: Bertan Ari Date: Sat, 17 Oct 2026 15:42:52 -0700 Subject: [PATCH 08/15] test(apply-diff): assert repaired first-block content --- .../__tests__/multi-search-replace-marker-hint.spec.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/core/diff/strategies/__tests__/multi-search-replace-marker-hint.spec.ts b/src/core/diff/strategies/__tests__/multi-search-replace-marker-hint.spec.ts index d0a3737b7a..080bb8f1e9 100644 --- a/src/core/diff/strategies/__tests__/multi-search-replace-marker-hint.spec.ts +++ b/src/core/diff/strategies/__tests__/multi-search-replace-marker-hint.spec.ts @@ -119,6 +119,7 @@ describe("MultiSearchReplaceDiffStrategy malformed start-line marker hint", () = if (!result.success) { throw new Error("Expected the first repaired block to apply") } + expect(result.content).toBe(["ALPHA x", "beta"].join("\n")) expect(result.blockCount).toBe(2) expect(result.failParts?.filter((part) => !part.success)).toHaveLength(1) }) From cf041bfad7474e5f6b235104a9d813f7d405510e Mon Sep 17 00:00:00 2001 From: Bertan Ari Date: Sat, 17 Oct 2026 17:39:28 -0700 Subject: [PATCH 09/15] fix(apply_diff): limit file excerpts in multi-failure and partial-apply reports Only the first failed block keeps Search/Best Match/Original Content sections (main parity); later blocks and the partial-apply result keep diagnostics only. --- src/core/tools/ApplyDiffTool.ts | 17 ++++- .../applyDiffTool.failureReporting.spec.ts | 67 +++++++++++++++++++ 2 files changed, 81 insertions(+), 3 deletions(-) diff --git a/src/core/tools/ApplyDiffTool.ts b/src/core/tools/ApplyDiffTool.ts index fc47a12ec6..bdc594a9a5 100644 --- a/src/core/tools/ApplyDiffTool.ts +++ b/src/core/tools/ApplyDiffTool.ts @@ -23,6 +23,8 @@ interface ApplyDiffParams { // Failures embed file content; cap both detail size and the number of summaries. const DETAILED_APPLY_DIFF_FAILURES = 5 +// Strategy errors put file excerpts after this heading; main exposed at most one such excerpt. +const FILE_CONTENT_SECTION_HEADING = "\n\nSearch Content:\n" const MAX_DETAILED_FAILURE_CHARACTERS = 4000 const MAX_SUMMARY_FAILURE_CHARACTERS = 200 const MAX_ABBREVIATED_FAILURES = 50 @@ -100,7 +102,7 @@ export class ApplyDiffTool extends BaseTool<"apply_diff"> { ? `${failedParts.length} diff blocks failed to apply. Fix the failures below before retrying.\n\n` : "" - formattedError = `Unable to apply diff to file: ${absolutePath}\n\n\n${summary}${this.formatFailedParts(failedParts)}\n` + formattedError = `Unable to apply diff to file: ${absolutePath}\n\n\n${summary}${this.formatFailedParts(failedParts, 1)}\n` } else { const errorDetails = diffResult.details ? JSON.stringify(diffResult.details, null, 2) : "" @@ -247,7 +249,7 @@ export class ApplyDiffTool extends BaseTool<"apply_diff"> { `The other ${unappliedParts.length} did NOT apply. ` + `Inspect the saved file before retrying, since an earlier block may already contain the intended change. ` + `Then resend ONLY the failed blocks that are still needed; the blocks that already applied will no longer match.\n\n` + - `\n${this.formatFailedParts(unappliedParts)}\n\n\n` + `\n${this.formatFailedParts(unappliedParts, 0)}\n\n\n` } // Get the formatted response message @@ -281,7 +283,11 @@ export class ApplyDiffTool extends BaseTool<"apply_diff"> { } } - private formatFailedParts(failedParts: Extract[]): string { + /** Only the first `fileContentParts` failures may include file excerpts and details. */ + private formatFailedParts( + failedParts: Extract[], + fileContentParts: number, + ): string { const renderedCount = Math.min(failedParts.length, MAX_ABBREVIATED_FAILURES) const sections = failedParts.slice(0, renderedCount).map((failPart, index) => { const label = failedParts.length > 1 ? `--- Diff block ${index + 1} of ${failedParts.length} ---\n` : "" @@ -292,6 +298,11 @@ export class ApplyDiffTool extends BaseTool<"apply_diff"> { return `${label}${this.truncate(summaryLine, MAX_SUMMARY_FAILURE_CHARACTERS)}` } + if (index >= fileContentParts) { + const diagnostics = error.split(FILE_CONTENT_SECTION_HEADING)[0] + return this.truncate(`${label}${diagnostics}`, MAX_DETAILED_FAILURE_CHARACTERS) + } + const errorDetails = failPart.details ? JSON.stringify(failPart.details, null, 2) : "" const detailedSection = `${label}${error}${errorDetails ? `\n\nDetails:\n${errorDetails}` : ""}` diff --git a/src/core/tools/__tests__/applyDiffTool.failureReporting.spec.ts b/src/core/tools/__tests__/applyDiffTool.failureReporting.spec.ts index 836df38cd3..8055f58fa1 100644 --- a/src/core/tools/__tests__/applyDiffTool.failureReporting.spec.ts +++ b/src/core/tools/__tests__/applyDiffTool.failureReporting.spec.ts @@ -306,6 +306,73 @@ describe("ApplyDiffTool failure reporting", () => { }) }) + describe("file content exposure", () => { + const SECRET = "SECRET_TOKEN=abc123" + + function makeContentFailure(index: number): FailPart { + return makeFailure( + `No sufficiently similar match found ${index}\n\nDebug Info:\n- Similarity Score: 10%` + + `\n\nSearch Content:\nsearch-${index}\n\nBest Match Found:\n${SECRET}\n\nOriginal Content:\n${SECRET}`, + { similarity: 0.1 }, + ) + } + + it("keeps file content for only the first failed block on the failure path", async () => { + const output = await runApplyDiff( + { success: false, failParts: [1, 2, 3].map((index) => makeContentFailure(index)) }, + 3, + ) + + expect(output.split("Original Content:").length - 1).toBe(1) + expect(output.split(SECRET).length - 1).toBe(2) + expect(output.split("Details:").length - 1).toBe(1) + expect(output).toContain("No sufficiently similar match found 2\n\nDebug Info:\n- Similarity Score: 10%") + expect(output).toContain("No sufficiently similar match found 3\n\nDebug Info:\n- Similarity Score: 10%") + }) + + it("keeps the full content of a single failed block", async () => { + const output = await runApplyDiff({ success: false, failParts: [makeContentFailure(1)] }, 1) + + expect(output).toContain(`Original Content:\n${SECRET}`) + }) + + it("cuts at the strategy heading even when the search text repeats it", async () => { + const repeated = makeFailure( + `Diagnostics kept\n\nSearch Content:\nx\n\nSearch Content:\ny\n\nOriginal Content:\n${SECRET}`, + ) + const output = await runApplyDiff({ success: false, failParts: [makeContentFailure(1), repeated] }, 2) + const secondSection = output.split("--- Diff block 2 of 2 ---\n")[1] + + expect(secondSection).toContain("Diagnostics kept") + expect(secondSection).not.toContain("Search Content:") + expect(secondSection).not.toContain(SECRET) + }) + + it("shows the same content-limited text in the diff_error message", async () => { + const result: DiffResult = { success: false, failParts: [1, 2].map((index) => makeContentFailure(index)) } + await runApplyDiff(result, 2) + const secondOutput = await runApplyDiff(result, 2) + + expect(mockTask.say).toHaveBeenCalledWith("diff_error", secondOutput) + expect(secondOutput.split(SECRET).length - 1).toBe(2) + }) + + it("excludes all file content from the partial-apply result", async () => { + const output = await runApplyDiff( + { success: true, content: "x", blockCount: 3, failParts: [makeContentFailure(1)] }, + 3, + ) + + expect(output).toContain("Applied 2 of 3 diff blocks") + expect(output).toContain("No sufficiently similar match found 1\n\nDebug Info:\n- Similarity Score: 10%") + expect(output).not.toContain("Search Content:") + expect(output).not.toContain("Best Match Found:") + expect(output).not.toContain("Original Content:") + expect(output).not.toContain("Details:") + expect(output).not.toContain(SECRET) + }) + }) + describe("partial apply path", () => { function partialResult(failures: FailPart[]): DiffResult { return { success: true, content: "updated content", failParts: failures } From b5b8cf8fa7036af4c7c00b026420bb8739e6bf54 Mon Sep 17 00:00:00 2001 From: Bertan Ari Date: Sun, 18 Oct 2026 16:51:36 -0700 Subject: [PATCH 10/15] fix(apply_diff): report only non-sensitive diagnostics on diff failures Stop echoing search, best-match and original file content from failed SEARCH blocks and never forward DiffResult details. Failures now carry numeric scores, line locations and constant hints for single, multi-block, partial-success and fallback paths. --- .../multi-search-replace-marker-hint.spec.ts | 2 +- .../__tests__/multi-search-replace.spec.ts | 34 +++-- .../diff/strategies/multi-search-replace.ts | 44 ++----- src/core/tools/ApplyDiffTool.ts | 34 ++--- .../applyDiffTool.failureReporting.spec.ts | 124 +++++++++++------- 5 files changed, 118 insertions(+), 120 deletions(-) diff --git a/src/core/diff/strategies/__tests__/multi-search-replace-marker-hint.spec.ts b/src/core/diff/strategies/__tests__/multi-search-replace-marker-hint.spec.ts index 080bb8f1e9..2fc441113b 100644 --- a/src/core/diff/strategies/__tests__/multi-search-replace-marker-hint.spec.ts +++ b/src/core/diff/strategies/__tests__/multi-search-replace-marker-hint.spec.ts @@ -135,7 +135,7 @@ describe("MultiSearchReplaceDiffStrategy malformed start-line marker hint", () = "- Tip: Use the read_file tool to get the latest content of the file before attempting " + "to use the apply_diff tool again, as the file content may have changed\n" + "- Tip: The SEARCH block must contain complete, whole lines — partial-line (substring) matching is not supported. " + - "If the target line is very long, copy the entire line into the SEARCH block\n\n", + "If the target line is very long, copy the entire line into the SEARCH block", ) }) diff --git a/src/core/diff/strategies/__tests__/multi-search-replace.spec.ts b/src/core/diff/strategies/__tests__/multi-search-replace.spec.ts index 07ffc419a4..81adddb9c9 100644 --- a/src/core/diff/strategies/__tests__/multi-search-replace.spec.ts +++ b/src/core/diff/strategies/__tests__/multi-search-replace.spec.ts @@ -900,7 +900,7 @@ replacement content here expect(error).toContain("No sufficiently similar match found") expect(error).toContain("at line: 999") expect(error).toContain("Search Range: starting at line 999") - expect(error).toContain("Best Match Found:\n(no match)") + expect(error).toContain("Best Match Location: none") expect(error).toContain("Levenshtein Distance: N/A") expect(error).toContain("Best Match Length: 0 characters") }) @@ -928,7 +928,7 @@ replacement content here expect(error).toContain("Tip: The SEARCH block must contain complete, whole lines") }) - it("should include scoped original content when search fails with start_line that has a low-score match", async () => { + it("reports only numeric diagnostics when search fails with start_line that has a low-score match", async () => { const originalContent = "function existing() {\n return 42;\n}\n" const diffContent = `test.ts <<<<<<< SEARCH @@ -954,8 +954,9 @@ function newVersion() { expect(error).toContain("No sufficiently similar match found") expect(error).toContain("at line: 1") expect(error).toContain("Search Range: starting at line 1") - expect(error).toContain("Best Match Found:") - expect(error).toContain("Original Content:\n1 | function existing()") + expect(error).toContain("Best Match Location: lines 1-") + expect(error).not.toContain("function existing()") + expect(error).not.toContain("function different()") }) it("should include best-match debug info when unscoped search is below threshold", async () => { @@ -984,7 +985,9 @@ function processUsers(data) { expect(error).toContain("Levenshtein Distance:") expect(error).toContain("characters") expect(error).toContain("Best Match Length:") - expect(error).toContain("Best Match Found:\n1 | function processUsers(data)") + expect(error).toContain("Best Match Location: lines 1-3") + expect(error).not.toContain("user.username") + expect(error).not.toContain("user.name") }) it("should include zero-match info when unscoped search finds no similarity at all", async () => { @@ -1007,11 +1010,24 @@ aaaaaa : "" expect(error).toContain("No sufficiently similar match found") expect(error).toContain("Search Range: start to end") - expect(error).toContain("Best Match Found:\n(no match)") + expect(error).toContain("Best Match Location: none") expect(error).toContain("Levenshtein Distance: N/A") expect(error).toContain("Best Match Length: 0 characters") - expect(error).toContain("Original Content:") - expect(error).toContain("1 | xxxxxx") + expect(error).not.toContain("xxxxxx") + expect(error).not.toContain("!!!!!!") + }) + + it("never echoes search, best-match or surrounding file text in failure diagnostics", async () => { + const originalContent = "const a = 1\nAPI_KEY=FILE-SECRET-1\nconst b = 2\nAPI_KEY=FILE-SECRET-2\n" + const diffContent = + "<<<<<<< SEARCH\n:start_line:2\n-------\nAPI_KEY=SEARCH-SECRET-0\n=======\nAPI_KEY=x\n>>>>>>> REPLACE" + + const result = await strategy.applyDiff(originalContent, diffContent) + const serialized = JSON.stringify(result) + + expect(result.success).toBe(false) + expect(serialized).toContain("No sufficiently similar match found") + expect(serialized).not.toMatch(/FILE-SECRET|SEARCH-SECRET/) }) it("should match content with extra whitespace", async () => { @@ -1610,7 +1626,7 @@ function sum(a, b) { expect(failedPart).toHaveProperty("error") const errorMsg = (failedPart as { error: string }).error expect(errorMsg).toContain("No sufficiently similar match found") - expect(errorMsg).toContain("Best Match Found:") + expect(errorMsg).toContain("Best Match Location:") expect(errorMsg).toContain("Levenshtein Distance:") expect(errorMsg).toContain("Search Range: start to end") }) diff --git a/src/core/diff/strategies/multi-search-replace.ts b/src/core/diff/strategies/multi-search-replace.ts index c2997ee105..06be958736 100644 --- a/src/core/diff/strategies/multi-search-replace.ts +++ b/src/core/diff/strategies/multi-search-replace.ts @@ -2,7 +2,7 @@ import { distance } from "fastest-levenshtein" import { ToolProgressStatus, DEFAULT_DIFF_FUZZY_THRESHOLD } from "@roo-code/types" -import { addLineNumbers, everyLineHasLineNumbers, stripLineNumbers } from "../../../integrations/misc/extract-text" +import { everyLineHasLineNumbers, stripLineNumbers } from "../../../integrations/misc/extract-text" import { ToolUse, DiffStrategy, DiffResult } from "../../../shared/tools" import { normalizeString } from "../../../utils/text-normalization" @@ -468,8 +468,6 @@ export class MultiSearchReplaceDiffStrategy implements DiffStrategy { continue } - const endLine = replacement.startLine + searchLines.length - 1 - // Initialize search variables let matchIndex = -1 let bestMatchScore = 0 @@ -538,38 +536,12 @@ export class MultiSearchReplaceDiffStrategy implements DiffStrategy { searchLines = aggressiveSearchLines replaceLines = replaceContent ? replaceContent.split(/\r?\n/) : [] } else { - // No match found with either method - const originalContentSection = - startLine && endLine - ? `\n\nOriginal Content:\n${addLineNumbers( - resultLines - .slice( - Math.max(0, startLine - 1 - this.bufferLines), - Math.min(resultLines.length, endLine + this.bufferLines), - ) - .join("\n"), - Math.max(1, startLine - this.bufferLines), - )}` - : `\n\nOriginal Content:\n${addLineNumbers( - resultLines - .slice( - Math.max(0, (matchIndex >= 0 ? matchIndex : 0) - this.bufferLines), - Math.min( - resultLines.length, - (matchIndex >= 0 ? matchIndex : 0) + - searchLines.length + - this.bufferLines, - ), - ) - .join("\n"), - Math.max(1, (matchIndex >= 0 ? matchIndex : 0) - this.bufferLines + 1), - )}` - - const bestMatchSection = bestMatchContent - ? `\n\nBest Match Found:\n${addLineNumbers(bestMatchContent, matchIndex + 1)}` - : `\n\nBest Match Found:\n(no match)` - + // File and search text are never echoed: they may hold secrets. Report only numbers and constants. const lineRange = startLine ? ` at line: ${startLine}` : "" + const bestMatchLocation = + bestMatchContent && matchIndex >= 0 + ? `lines ${matchIndex + 1}-${matchIndex + bestMatchContent.split("\n").length}` + : "none" const levenDist = bestMatchContent ? distance(normalizeString(searchChunk), normalizeString(bestMatchContent)) @@ -596,13 +568,13 @@ export class MultiSearchReplaceDiffStrategy implements DiffStrategy { `- Search Range: ${startLine ? `starting at line ${startLine}` : "start to end"}\n` + `- Levenshtein Distance: ${levenDist >= 0 ? `${levenDist} characters` : "N/A"}\n` + `- Search Length: ${searchChunk.length} characters\n` + + `- Best Match Location: ${bestMatchLocation}\n` + `- Best Match Length: ${bestMatchContent ? bestMatchContent.length : 0} characters\n` + `- Tried both standard and aggressive line number stripping${malformedMarkerHint}\n` + `- Tip: Use the read_file tool to get the latest content of the file before attempting ` + `to use the apply_diff tool again, as the file content may have changed\n` + `- Tip: The SEARCH block must contain complete, whole lines — partial-line (substring) matching is not supported. ` + - `If the target line is very long, copy the entire line into the SEARCH block\n\n` + - `Search Content:\n${searchChunk}${bestMatchSection}${originalContentSection}`, + `If the target line is very long, copy the entire line into the SEARCH block`, }) continue } diff --git a/src/core/tools/ApplyDiffTool.ts b/src/core/tools/ApplyDiffTool.ts index bdc594a9a5..f793620c0a 100644 --- a/src/core/tools/ApplyDiffTool.ts +++ b/src/core/tools/ApplyDiffTool.ts @@ -21,10 +21,7 @@ interface ApplyDiffParams { diff: string } -// Failures embed file content; cap both detail size and the number of summaries. const DETAILED_APPLY_DIFF_FAILURES = 5 -// Strategy errors put file excerpts after this heading; main exposed at most one such excerpt. -const FILE_CONTENT_SECTION_HEADING = "\n\nSearch Content:\n" const MAX_DETAILED_FAILURE_CHARACTERS = 4000 const MAX_SUMMARY_FAILURE_CHARACTERS = 200 const MAX_ABBREVIATED_FAILURES = 50 @@ -102,13 +99,13 @@ export class ApplyDiffTool extends BaseTool<"apply_diff"> { ? `${failedParts.length} diff blocks failed to apply. Fix the failures below before retrying.\n\n` : "" - formattedError = `Unable to apply diff to file: ${absolutePath}\n\n\n${summary}${this.formatFailedParts(failedParts, 1)}\n` + formattedError = `Unable to apply diff to file: ${absolutePath}\n\n\n${summary}${this.formatFailedParts(failedParts)}\n` } else { - const errorDetails = diffResult.details ? JSON.stringify(diffResult.details, null, 2) : "" - - formattedError = `Unable to apply diff to file: ${absolutePath}\n\n\n${ - diffResult.error - }${errorDetails ? `\n\nDetails:\n${errorDetails}` : ""}\n` + // `details` may carry search/best-match file text, so it is never forwarded. + formattedError = `Unable to apply diff to file: ${absolutePath}\n\n\n${this.truncate( + diffResult.error ?? "Unknown apply_diff failure", + MAX_DETAILED_FAILURE_CHARACTERS, + )}\n` } if (currentCount >= 2) { @@ -249,7 +246,7 @@ export class ApplyDiffTool extends BaseTool<"apply_diff"> { `The other ${unappliedParts.length} did NOT apply. ` + `Inspect the saved file before retrying, since an earlier block may already contain the intended change. ` + `Then resend ONLY the failed blocks that are still needed; the blocks that already applied will no longer match.\n\n` + - `\n${this.formatFailedParts(unappliedParts, 0)}\n\n\n` + `\n${this.formatFailedParts(unappliedParts)}\n\n\n` } // Get the formatted response message @@ -283,11 +280,8 @@ export class ApplyDiffTool extends BaseTool<"apply_diff"> { } } - /** Only the first `fileContentParts` failures may include file excerpts and details. */ - private formatFailedParts( - failedParts: Extract[], - fileContentParts: number, - ): string { + /** Strategy errors are file-content-free diagnostics; `details` may hold file text and is never rendered. */ + private formatFailedParts(failedParts: Extract[]): string { const renderedCount = Math.min(failedParts.length, MAX_ABBREVIATED_FAILURES) const sections = failedParts.slice(0, renderedCount).map((failPart, index) => { const label = failedParts.length > 1 ? `--- Diff block ${index + 1} of ${failedParts.length} ---\n` : "" @@ -298,15 +292,7 @@ export class ApplyDiffTool extends BaseTool<"apply_diff"> { return `${label}${this.truncate(summaryLine, MAX_SUMMARY_FAILURE_CHARACTERS)}` } - if (index >= fileContentParts) { - const diagnostics = error.split(FILE_CONTENT_SECTION_HEADING)[0] - return this.truncate(`${label}${diagnostics}`, MAX_DETAILED_FAILURE_CHARACTERS) - } - - const errorDetails = failPart.details ? JSON.stringify(failPart.details, null, 2) : "" - const detailedSection = `${label}${error}${errorDetails ? `\n\nDetails:\n${errorDetails}` : ""}` - - return this.truncate(detailedSection, MAX_DETAILED_FAILURE_CHARACTERS) + return this.truncate(`${label}${error}`, MAX_DETAILED_FAILURE_CHARACTERS) }) const trailers: string[] = [] diff --git a/src/core/tools/__tests__/applyDiffTool.failureReporting.spec.ts b/src/core/tools/__tests__/applyDiffTool.failureReporting.spec.ts index 8055f58fa1..eb06a79b8c 100644 --- a/src/core/tools/__tests__/applyDiffTool.failureReporting.spec.ts +++ b/src/core/tools/__tests__/applyDiffTool.failureReporting.spec.ts @@ -4,6 +4,7 @@ import path from "path" import type { DiffResult, ToolResponse } from "../../../shared/tools" import type { Task } from "../../task/Task" import { fileExistsAtPath } from "../../../utils/fs" +import { MultiSearchReplaceDiffStrategy } from "../../diff/strategies/multi-search-replace" import type { ToolCallbacks } from "../BaseTool" import { applyDiffTool } from "../ApplyDiffTool" @@ -213,15 +214,15 @@ describe("ApplyDiffTool failure reporting", () => { expect(output).toContain("Unknown apply_diff failure") }) - it("appends Details for detailed failures and omits them for abbreviated ones", async () => { + it("never renders the details object of any failure", async () => { const failures = makeFailureSet(6) failures[0] = makeFailure("FAILURE-MARKER-1\nsecond line of failure 1", { bestMatch: "DETAIL-FIRST" }) failures[5] = makeFailure("FAILURE-MARKER-6\nsecond line of failure 6", { bestMatch: "DETAIL-SIXTH" }) const output = await runApplyDiff({ success: false, failParts: failures }, 6) - expect(output).toContain("Details:") - expect(output).toContain("DETAIL-FIRST") + expect(output).not.toContain("Details:") + expect(output).not.toContain("DETAIL-FIRST") expect(output).not.toContain("DETAIL-SIXTH") }) @@ -306,70 +307,93 @@ describe("ApplyDiffTool failure reporting", () => { }) }) - describe("file content exposure", () => { - const SECRET = "SECRET_TOKEN=abc123" + describe("file content exposure (real strategy)", () => { + const FILE_SECRETS = ["FILE-SECRET-ORIGINAL", "FILE-SECRET-BESTMATCH"] + const SEARCH_SECRET = "SEARCH-SECRET" + const DETAIL_SECRET = "DETAIL-SECRET" + const fileText = [ + "const header = 1", + `token = "${FILE_SECRETS[0]}"`, + `apiKey = "${FILE_SECRETS[1]}"`, + "const footer = 2", + ].join("\n") + + function failingBlock(index: number): string { + return `<<<<<<< SEARCH\n:start_line:2\n-------\napiKey = "${SEARCH_SECRET}-${index}"\n=======\nx${index}\n>>>>>>> REPLACE` + } - function makeContentFailure(index: number): FailPart { - return makeFailure( - `No sufficiently similar match found ${index}\n\nDebug Info:\n- Similarity Score: 10%` + - `\n\nSearch Content:\nsearch-${index}\n\nBest Match Found:\n${SECRET}\n\nOriginal Content:\n${SECRET}`, - { similarity: 0.1 }, - ) + const applyingBlock = "<<<<<<< SEARCH\nconst header = 1\n=======\nconst header = 3\n>>>>>>> REPLACE" + + async function runRealStrategy(diff: string): Promise { + vi.mocked(fs.readFile).mockResolvedValue(fileText) + const strategy = new MultiSearchReplaceDiffStrategy() + const realResult = await strategy.applyDiff(fileText, diff) + // Synthetic details exercise the tool's own guard against forwarding strategy detail objects. + const withDetails = (part: DiffResult): DiffResult => + part.success ? part : { ...part, details: { searchContent: DETAIL_SECRET, bestMatch: DETAIL_SECRET } } + const result: DiffResult = realResult.success + ? { ...realResult, failParts: realResult.failParts?.map(withDetails) } + : { + ...realResult, + failParts: realResult.failParts?.map(withDetails) ?? [], + details: { bestMatch: DETAIL_SECRET }, + } + return runApplyDiff(result, 0, diff) } - it("keeps file content for only the first failed block on the failure path", async () => { - const output = await runApplyDiff( - { success: false, failParts: [1, 2, 3].map((index) => makeContentFailure(index)) }, - 3, - ) + function collectUiAndModelOutputs(output: string): string { + const sayText = vi.mocked(mockTask.say).mock.calls.map((call) => String(call[1] ?? "")) + const recordedErrors = vi.mocked(mockTask.recordToolError).mock.calls.map((call) => String(call[1] ?? "")) + return [output, ...sayText, ...recordedErrors].join("\n") + } - expect(output.split("Original Content:").length - 1).toBe(1) - expect(output.split(SECRET).length - 1).toBe(2) - expect(output.split("Details:").length - 1).toBe(1) - expect(output).toContain("No sufficiently similar match found 2\n\nDebug Info:\n- Similarity Score: 10%") - expect(output).toContain("No sufficiently similar match found 3\n\nDebug Info:\n- Similarity Score: 10%") - }) + function expectNoSecrets(text: string): void { + for (const secret of [...FILE_SECRETS, SEARCH_SECRET, DETAIL_SECRET]) { + expect(text).not.toContain(secret) + } + expect(text).not.toMatch(/Search Content:|Best Match Found:|Original Content:|Details:/) + } - it("keeps the full content of a single failed block", async () => { - const output = await runApplyDiff({ success: false, failParts: [makeContentFailure(1)] }, 1) + it("excludes file, search and detail text from a single failure in model and UI output", async () => { + await runRealStrategy(failingBlock(1)) + const output = await runRealStrategy(failingBlock(1)) - expect(output).toContain(`Original Content:\n${SECRET}`) + expect(output).toContain("No sufficiently similar match found at line: 2") + expect(output).toContain("- Similarity Score:") + expect(mockTask.say).toHaveBeenCalledWith("diff_error", output) + expectNoSecrets(collectUiAndModelOutputs(output)) }) - it("cuts at the strategy heading even when the search text repeats it", async () => { - const repeated = makeFailure( - `Diagnostics kept\n\nSearch Content:\nx\n\nSearch Content:\ny\n\nOriginal Content:\n${SECRET}`, - ) - const output = await runApplyDiff({ success: false, failParts: [makeContentFailure(1), repeated] }, 2) - const secondSection = output.split("--- Diff block 2 of 2 ---\n")[1] + it("excludes sensitive text from every block when multiple blocks fail", async () => { + const diff = [1, 2, 3].map(failingBlock).join("\n") + await runRealStrategy(diff) + const output = await runRealStrategy(diff) - expect(secondSection).toContain("Diagnostics kept") - expect(secondSection).not.toContain("Search Content:") - expect(secondSection).not.toContain(SECRET) + expect(output).toContain("3 diff blocks failed to apply") + expect(output).toContain("--- Diff block 3 of 3 ---") + expectNoSecrets(collectUiAndModelOutputs(output)) }) - it("shows the same content-limited text in the diff_error message", async () => { - const result: DiffResult = { success: false, failParts: [1, 2].map((index) => makeContentFailure(index)) } - await runApplyDiff(result, 2) - const secondOutput = await runApplyDiff(result, 2) + it("excludes sensitive text from the partial-apply result", async () => { + const output = await runRealStrategy(`${applyingBlock}\n${failingBlock(1)}`) - expect(mockTask.say).toHaveBeenCalledWith("diff_error", secondOutput) - expect(secondOutput.split(SECRET).length - 1).toBe(2) + expect(output).toContain("Applied 1 of 2 diff blocks") + expect(output).toContain("No sufficiently similar match found") + expectNoSecrets(collectUiAndModelOutputs(output)) + // The approval payload intentionally echoes the model's own diff, so only detail text is checked. + for (const call of vi.mocked(mockAskApproval).mock.calls) { + expect(String(call[1])).not.toContain(DETAIL_SECRET) + } }) - it("excludes all file content from the partial-apply result", async () => { + it("drops top-level details on the fallback path without failParts", async () => { const output = await runApplyDiff( - { success: true, content: "x", blockCount: 3, failParts: [makeContentFailure(1)] }, - 3, + { success: false, error: "Invalid diff format", details: { bestMatch: DETAIL_SECRET } }, + 1, ) - expect(output).toContain("Applied 2 of 3 diff blocks") - expect(output).toContain("No sufficiently similar match found 1\n\nDebug Info:\n- Similarity Score: 10%") - expect(output).not.toContain("Search Content:") - expect(output).not.toContain("Best Match Found:") - expect(output).not.toContain("Original Content:") - expect(output).not.toContain("Details:") - expect(output).not.toContain(SECRET) + expect(output).toContain("Invalid diff format") + expectNoSecrets(collectUiAndModelOutputs(output)) }) }) From 20dcb7c698b12d08e8ec6d67160c14805f828d48 Mon Sep 17 00:00:00 2001 From: Bertan Ari Date: Mon, 19 Oct 2026 13:17:01 -0700 Subject: [PATCH 11/15] fix(apply_diff): stop echoing malformed marker text; align e2e error fixture The marker hint no longer interpolates the first search line. The e2e mock fixture matched on search text that the failure output no longer contains, so the error smoke test timed out. --- apps/vscode-e2e/src/fixtures/apply-diff.ts | 2 +- .../__tests__/multi-search-replace-marker-hint.spec.ts | 4 +++- src/core/diff/strategies/multi-search-replace.ts | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/apps/vscode-e2e/src/fixtures/apply-diff.ts b/apps/vscode-e2e/src/fixtures/apply-diff.ts index adc604391b..35991d851a 100644 --- a/apps/vscode-e2e/src/fixtures/apply-diff.ts +++ b/apps/vscode-e2e/src/fixtures/apply-diff.ts @@ -31,7 +31,7 @@ export function addApplyDiffResultFixtures(mock: InstanceType) { }, { toolCallId: "call_apply_diff_error_001", - expected: ["No sufficiently similar match found at line: 1", "This content does not exist"], + expected: ["No sufficiently similar match found at line: 1", "- Best Match Location: "], result: "The apply_diff operation on `apply-diff-tool-fixture/error-handling.txt` was rejected - the search content did not match any content in the file, so it was not modified.", id: "call_apply_diff_error_002", }, diff --git a/src/core/diff/strategies/__tests__/multi-search-replace-marker-hint.spec.ts b/src/core/diff/strategies/__tests__/multi-search-replace-marker-hint.spec.ts index 2fc441113b..e89bc32283 100644 --- a/src/core/diff/strategies/__tests__/multi-search-replace-marker-hint.spec.ts +++ b/src/core/diff/strategies/__tests__/multi-search-replace-marker-hint.spec.ts @@ -50,7 +50,9 @@ describe("MultiSearchReplaceDiffStrategy malformed start-line marker hint", () = const diffContent = buildDiff(`${markerVariant}\nnot-in-the-file`, "replacement") const result = await strategy.applyDiff(nonMatchingFile, diffContent) - expect(firstFailureError(result)).toContain(`first line of your search content is "${markerVariant}"`) + const errorText = firstFailureError(result) + expect(errorText).toContain("first line of your search content looks like a malformed start-line marker") + expect(errorText).not.toContain(markerVariant) }, ) diff --git a/src/core/diff/strategies/multi-search-replace.ts b/src/core/diff/strategies/multi-search-replace.ts index 06be958736..805edbde58 100644 --- a/src/core/diff/strategies/multi-search-replace.ts +++ b/src/core/diff/strategies/multi-search-replace.ts @@ -554,7 +554,7 @@ export class MultiSearchReplaceDiffStrategy implements DiffStrategy { const malformedMarkerHint = /^(?::\s*\d{1,9}:?|:?\s*start[_-]?line\s*:?\s*\d{1,9}:?)$/i.test( firstSearchLine, ) - ? `\n- LIKELY CAUSE: the first line of your search content is "${firstSearchLine}", which looks like a malformed start-line marker and was therefore searched for as literal file text${separatorSwallowed ? ', together with the "-------" line after it' : ""}. The only recognized form is ":start_line:" on the line immediately after "<<<<<<< SEARCH", optionally followed by a "-------" line. Remove or correct that line and retry.` + ? `\n- LIKELY CAUSE: the first line of your search content looks like a malformed start-line marker and was therefore searched for as literal file text${separatorSwallowed ? ', together with the "-------" line after it' : ""}. The only recognized form is ":start_line:" on the line immediately after "<<<<<<< SEARCH", optionally followed by a "-------" line. Remove or correct that line and retry.` : "" diffResults.push({ From 4da8b6419b5de755923b5f58a8589f3be03c1f2b Mon Sep 17 00:00:00 2001 From: Bertan Ari Date: Mon, 19 Oct 2026 20:43:46 -0700 Subject: [PATCH 12/15] fix(apply_diff): guard marker-hint regex length, label failures by ordinal, cover zero-applied clamp --- .../multi-search-replace-marker-hint.spec.ts | 17 +++++++++++++ .../diff/strategies/multi-search-replace.ts | 9 ++++--- src/core/tools/ApplyDiffTool.ts | 2 +- .../applyDiffTool.failureReporting.spec.ts | 25 +++++++++++++++---- 4 files changed, 44 insertions(+), 9 deletions(-) diff --git a/src/core/diff/strategies/__tests__/multi-search-replace-marker-hint.spec.ts b/src/core/diff/strategies/__tests__/multi-search-replace-marker-hint.spec.ts index e89bc32283..be5517bb9b 100644 --- a/src/core/diff/strategies/__tests__/multi-search-replace-marker-hint.spec.ts +++ b/src/core/diff/strategies/__tests__/multi-search-replace-marker-hint.spec.ts @@ -56,6 +56,23 @@ describe("MultiSearchReplaceDiffStrategy malformed start-line marker hint", () = }, ) + it("skips the marker hint for an overlong whitespace-padded first line", async () => { + const paddedLine = `startline${" ".repeat(40_000)}` + const diffContent = buildDiff(`${paddedLine}\nnot-in-the-file`, "replacement") + + const startedAt = performance.now() + const errorText = firstFailureError(await strategy.applyDiff(nonMatchingFile, diffContent)) + + expect(errorText).not.toContain("LIKELY CAUSE") + expect(performance.now() - startedAt).toBeLessThan(2_000) + }) + + it("still hints for a padded marker within the length guard", async () => { + const diffContent = buildDiff(`start_line:${" ".repeat(40)}3\nnot-in-the-file`, "replacement") + + expect(firstFailureError(await strategy.applyDiff(nonMatchingFile, diffContent))).toContain("LIKELY CAUSE") + }) + it.each(["value:3", ":3 trailing", "x start_line:3"])( "omits LIKELY CAUSE when %j only contains a marker-like fragment", async (firstLine) => { diff --git a/src/core/diff/strategies/multi-search-replace.ts b/src/core/diff/strategies/multi-search-replace.ts index 805edbde58..6cb8233cb3 100644 --- a/src/core/diff/strategies/multi-search-replace.ts +++ b/src/core/diff/strategies/multi-search-replace.ts @@ -7,6 +7,8 @@ import { ToolUse, DiffStrategy, DiffResult } from "../../../shared/tools" import { normalizeString } from "../../../utils/text-normalization" const BUFFER_LINES = 40 // Number of extra context lines to show before and after matches +// Real malformed markers are short; longer lines skip the hint regex entirely. +const MAX_MARKER_HINT_LINE_LENGTH = 64 function getSimilarity(original: string, search: string): number { // Empty searches are no longer supported @@ -551,9 +553,10 @@ export class MultiSearchReplaceDiffStrategy implements DiffStrategy { const hintLines = searchChunk.split("\n") const firstSearchLine = hintLines[0]?.trim() ?? "" const separatorSwallowed = hintLines[1]?.trim() === "-------" - const malformedMarkerHint = /^(?::\s*\d{1,9}:?|:?\s*start[_-]?line\s*:?\s*\d{1,9}:?)$/i.test( - firstSearchLine, - ) + // Length guard: adjacent `\s*` groups backtrack quadratically on long whitespace runs. + const malformedMarkerHint = + firstSearchLine.length <= MAX_MARKER_HINT_LINE_LENGTH && + /^(?::\s*\d{1,9}:?|:?\s*start[_-]?line\s*:?\s*\d{1,9}:?)$/i.test(firstSearchLine) ? `\n- LIKELY CAUSE: the first line of your search content looks like a malformed start-line marker and was therefore searched for as literal file text${separatorSwallowed ? ', together with the "-------" line after it' : ""}. The only recognized form is ":start_line:" on the line immediately after "<<<<<<< SEARCH", optionally followed by a "-------" line. Remove or correct that line and retry.` : "" diff --git a/src/core/tools/ApplyDiffTool.ts b/src/core/tools/ApplyDiffTool.ts index f793620c0a..4011f802e9 100644 --- a/src/core/tools/ApplyDiffTool.ts +++ b/src/core/tools/ApplyDiffTool.ts @@ -284,7 +284,7 @@ export class ApplyDiffTool extends BaseTool<"apply_diff"> { private formatFailedParts(failedParts: Extract[]): string { const renderedCount = Math.min(failedParts.length, MAX_ABBREVIATED_FAILURES) const sections = failedParts.slice(0, renderedCount).map((failPart, index) => { - const label = failedParts.length > 1 ? `--- Diff block ${index + 1} of ${failedParts.length} ---\n` : "" + const label = failedParts.length > 1 ? `--- Failure ${index + 1} of ${failedParts.length} ---\n` : "" const error = failPart.error ?? "Unknown apply_diff failure" if (index >= DETAILED_APPLY_DIFF_FAILURES) { diff --git a/src/core/tools/__tests__/applyDiffTool.failureReporting.spec.ts b/src/core/tools/__tests__/applyDiffTool.failureReporting.spec.ts index eb06a79b8c..bf7b3f5351 100644 --- a/src/core/tools/__tests__/applyDiffTool.failureReporting.spec.ts +++ b/src/core/tools/__tests__/applyDiffTool.failureReporting.spec.ts @@ -152,8 +152,9 @@ describe("ApplyDiffTool failure reporting", () => { expect(output).toContain("FAILURE-MARKER-1") expect(output).toContain("FAILURE-MARKER-2") - expect(output).toContain("--- Diff block 1 of 2 ---") - expect(output).toContain("--- Diff block 2 of 2 ---") + expect(output).toContain("--- Failure 1 of 2 ---") + expect(output).toContain("--- Failure 2 of 2 ---") + expect(output).not.toContain("Diff block") }) it("names the target file before the multi-block failure details", async () => { @@ -265,9 +266,9 @@ describe("ApplyDiffTool failure reporting", () => { const output = await runApplyDiff({ success: false, failParts: mixedParts }, 3) expect(output).toContain( - "\n2 diff blocks failed to apply. Fix the failures below before retrying.\n\n--- Diff block 1 of 2 ---\n", + "\n2 diff blocks failed to apply. Fix the failures below before retrying.\n\n--- Failure 1 of 2 ---\n", ) - expect(output).toContain("second line of failure 1\n\n--- Diff block 2 of 2 ---\nFAILURE-MARKER-2") + expect(output).toContain("second line of failure 1\n\n--- Failure 2 of 2 ---\nFAILURE-MARKER-2") }) it("renders a single failure verbatim with no summary, label, details or trailer", async () => { @@ -370,7 +371,7 @@ describe("ApplyDiffTool failure reporting", () => { const output = await runRealStrategy(diff) expect(output).toContain("3 diff blocks failed to apply") - expect(output).toContain("--- Diff block 3 of 3 ---") + expect(output).toContain("--- Failure 3 of 3 ---") expectNoSecrets(collectUiAndModelOutputs(output)) }) @@ -458,6 +459,20 @@ describe("ApplyDiffTool failure reporting", () => { expect(output).not.toContain("NOT in the file") }) + it.each([ + { name: "every block fails", failures: 2, blockCount: 2 }, + { name: "failures outnumber parsed blocks", failures: 3, blockCount: 2 }, + ])("reports zero applied blocks when $name", async ({ failures, blockCount }) => { + const output = await runApplyDiff( + { success: true, content: "updated content", failParts: makeFailureSet(failures), blockCount }, + blockCount, + ) + + expect(output).toContain(`Applied 0 of ${blockCount} diff blocks`) + expect(output).toContain(`The other ${failures} did NOT apply.`) + expect(output).not.toMatch(/Applied -\d/) + }) + it("tolerates a successful result without failParts or SEARCH markers", async () => { const output = await runApplyDiff( { success: true, content: "updated content" }, From e75fd92d623180cc0456c1f9ce72ef81b2545ca6 Mon Sep 17 00:00:00 2001 From: Bertan Ari Date: Mon, 19 Oct 2026 20:48:55 -0700 Subject: [PATCH 13/15] style(apply_diff): split marker-hint guard to satisfy prettier --- src/core/diff/strategies/multi-search-replace.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/core/diff/strategies/multi-search-replace.ts b/src/core/diff/strategies/multi-search-replace.ts index 6cb8233cb3..babe60938d 100644 --- a/src/core/diff/strategies/multi-search-replace.ts +++ b/src/core/diff/strategies/multi-search-replace.ts @@ -554,9 +554,10 @@ export class MultiSearchReplaceDiffStrategy implements DiffStrategy { const firstSearchLine = hintLines[0]?.trim() ?? "" const separatorSwallowed = hintLines[1]?.trim() === "-------" // Length guard: adjacent `\s*` groups backtrack quadratically on long whitespace runs. - const malformedMarkerHint = + const looksLikeMalformedMarker = firstSearchLine.length <= MAX_MARKER_HINT_LINE_LENGTH && /^(?::\s*\d{1,9}:?|:?\s*start[_-]?line\s*:?\s*\d{1,9}:?)$/i.test(firstSearchLine) + const malformedMarkerHint = looksLikeMalformedMarker ? `\n- LIKELY CAUSE: the first line of your search content looks like a malformed start-line marker and was therefore searched for as literal file text${separatorSwallowed ? ', together with the "-------" line after it' : ""}. The only recognized form is ":start_line:" on the line immediately after "<<<<<<< SEARCH", optionally followed by a "-------" line. Remove or correct that line and retry.` : "" From 09b67e566f4bf5a80d376aa0c58f3e494121e5c3 Mon Sep 17 00:00:00 2001 From: Bertan Ari Date: Wed, 21 Oct 2026 07:41:43 -0700 Subject: [PATCH 14/15] test(apply-diff): make overlong marker fixture exercise the length guard --- .../__tests__/multi-search-replace-marker-hint.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/diff/strategies/__tests__/multi-search-replace-marker-hint.spec.ts b/src/core/diff/strategies/__tests__/multi-search-replace-marker-hint.spec.ts index be5517bb9b..deb8acc928 100644 --- a/src/core/diff/strategies/__tests__/multi-search-replace-marker-hint.spec.ts +++ b/src/core/diff/strategies/__tests__/multi-search-replace-marker-hint.spec.ts @@ -57,7 +57,7 @@ describe("MultiSearchReplaceDiffStrategy malformed start-line marker hint", () = ) it("skips the marker hint for an overlong whitespace-padded first line", async () => { - const paddedLine = `startline${" ".repeat(40_000)}` + const paddedLine = `startline${" ".repeat(40_000)}3` const diffContent = buildDiff(`${paddedLine}\nnot-in-the-file`, "replacement") const startedAt = performance.now() From 7d30818e30dbece06605e2d242bb32a0b0b2af24 Mon Sep 17 00:00:00 2001 From: Bertan Ari Date: Thu, 22 Oct 2026 18:14:00 -0700 Subject: [PATCH 15/15] test: pin 64/65-char boundary of marker hint length guard --- .../multi-search-replace-marker-hint.spec.ts | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/core/diff/strategies/__tests__/multi-search-replace-marker-hint.spec.ts b/src/core/diff/strategies/__tests__/multi-search-replace-marker-hint.spec.ts index deb8acc928..c38b2e24f2 100644 --- a/src/core/diff/strategies/__tests__/multi-search-replace-marker-hint.spec.ts +++ b/src/core/diff/strategies/__tests__/multi-search-replace-marker-hint.spec.ts @@ -67,10 +67,16 @@ describe("MultiSearchReplaceDiffStrategy malformed start-line marker hint", () = expect(performance.now() - startedAt).toBeLessThan(2_000) }) - it("still hints for a padded marker within the length guard", async () => { - const diffContent = buildDiff(`start_line:${" ".repeat(40)}3\nnot-in-the-file`, "replacement") + it.each([ + { length: 64, hinted: true }, + { length: 65, hinted: false }, + ])("hints only while the first line fits the length guard ($length chars)", async ({ length, hinted }) => { + const firstLine = `startline${" ".repeat(length - 10)}3` + expect(firstLine).toHaveLength(length) + const diffContent = buildDiff(`${firstLine}\nnot-in-the-file`, "replacement") + const errorText = firstFailureError(await strategy.applyDiff(nonMatchingFile, diffContent)) - expect(firstFailureError(await strategy.applyDiff(nonMatchingFile, diffContent))).toContain("LIKELY CAUSE") + expect(errorText.includes("LIKELY CAUSE")).toBe(hinted) }) it.each(["value:3", ":3 trailing", "x start_line:3"])(