From 29101b2365054d84b21fd313e77c46a3a19ae061 Mon Sep 17 00:00:00 2001 From: Vincent-HD Date: Thu, 13 Aug 2026 23:40:11 +0200 Subject: [PATCH] fix(cursor): harden structured-edit apply_patch conversion Cursor models still emit git-style hunks, sequential multi_edit, and empty-old creates after #1017. Convert those to valid Codex apply_patch grammar without fuzzy-applying the filesystem, and keep recoverable rejects as text so the turn stays alive. Addresses #1388. Co-authored-by: Cursor --- src/adapters/cursor/protobuf-events.ts | 582 +++++++++++- src/adapters/cursor/tool-definitions.ts | 8 +- tests/cursor-structured-edit.test.ts | 1085 ++++++++++++++++++++++- tests/cursor-tool-definitions.test.ts | 1 + 4 files changed, 1657 insertions(+), 19 deletions(-) diff --git a/src/adapters/cursor/protobuf-events.ts b/src/adapters/cursor/protobuf-events.ts index 98c8779ca7..f526b6556a 100644 --- a/src/adapters/cursor/protobuf-events.ts +++ b/src/adapters/cursor/protobuf-events.ts @@ -346,6 +346,446 @@ function resolveCompletedArgs(buffered: string, args: McpArgs | undefined, state const PATCH_BEGIN = "*** Begin Patch"; const PATCH_END = "*** End Patch"; +const GIT_HUNK_HEADER = /^@@ -\d+(?:,\d+)? \+\d+(?:,\d+)?(?: @@.*)?$/; +const MARKDOWN_FENCE = /^```[\w+-]*\s*$/; +const PATH_ARG_KEYS = ["file_path", "filePath", "path", "filepath", "filename", "file", "target_file", "targetFile", "target_path", "targetPath"] as const; +const OLD_STRING_KEYS = ["old_string", "oldString", "oldtext", "old_text", "old_content", "oldContent", "before", "search"] as const; +const NEW_STRING_KEYS = ["new_string", "newString", "newtext", "new_text", "contents", "content", "new_contents", "newContents", "after", "replace"] as const; + +export type StructuredEditPair = { old_string: string; new_string: string }; + +/** First index where `needle` appears as consecutive whole lines in `haystack`, or -1. */ +function lineBlockIndex(haystack: string, needle: string): number { + if (needle.length === 0) return -1; + const hay = patchLines(haystack); + const ned = patchLines(needle); + if (ned.length === 0 || ned.length > hay.length) return -1; + for (let i = 0; i <= hay.length - ned.length; i++) { + if (ned.every((line, j) => hay[i + j] === line)) return i; + } + return -1; +} + +function replaceLineBlock(haystack: string, needle: string, replacement: string): string { + const at = lineBlockIndex(haystack, needle); + if (at < 0) return haystack; + const hay = patchLines(haystack); + const ned = patchLines(needle); + const next = [...hay.slice(0, at), ...patchLines(replacement), ...hay.slice(at + ned.length)]; + return next.join("\n"); +} + +/** + * Codex apply_patch matches every hunk against the original file (atomic). Cursor models + * emit sequential multi_edit (later old_string is the text after an earlier replacement). + * Fold only when one side contains the other as whole lines — raw substring includes() + * merged independent edits (B8: `hello world` inside `x = hello world`). + */ +export function foldSequentialStructuredEdits(edits: StructuredEditPair[]): StructuredEditPair[] { + const folded: StructuredEditPair[] = []; + for (const edit of edits) { + let absorbed = false; + for (let i = folded.length - 1; i >= 0; i--) { + const prior = folded[i]; + if (lineBlockIndex(prior.new_string, edit.old_string) >= 0) { + folded[i] = { + old_string: prior.old_string, + new_string: replaceLineBlock(prior.new_string, edit.old_string, edit.new_string), + }; + absorbed = true; + break; + } + if (lineBlockIndex(edit.old_string, prior.new_string) >= 0) { + folded[i] = { + old_string: replaceLineBlock(edit.old_string, prior.new_string, prior.old_string), + new_string: edit.new_string, + }; + absorbed = true; + break; + } + } + if (!absorbed) folded.push({ old_string: edit.old_string, new_string: edit.new_string }); + } + return folded; +} + +const GIT_NO_NEWLINE = /^\\ No newline at end of file\s*$/; +const GIT_META_PREFIX = /^(diff --git |index |new file mode |deleted file mode |old mode |new mode |similarity index |dissimilarity index |rename from |rename to |copy from |copy to )/; +const GIT_FILE_HEADER = /^(---|\+\+\+) (?:\/dev\/null|"[ab]\/|[ab]\/)/; + +function isCodexFileOpLine(line: string): boolean { + return line.startsWith("*** Update File:") + || line.startsWith("*** Add File:") + || line.startsWith("*** Delete File:"); +} + +function canonicalizeCodexLine(line: string): string { + const trimmed = line.replace(/^\uFEFF/, ""); + const lower = trimmed.toLowerCase(); + if (lower === "*** begin patch" || lower.startsWith("*** begin patch ")) return PATCH_BEGIN; + if (lower === "*** end patch" || lower.startsWith("*** end patch ")) return PATCH_END; + const colonOps = [ + ["*** update file:", "*** Update File:"], + ["*** add file:", "*** Add File:"], + ["*** delete file:", "*** Delete File:"], + ["*** move to:", "*** Move to:"], + ] as const; + for (const [needle, canon] of colonOps) { + if (lower.startsWith(needle)) return `${canon}${trimmed.slice(needle.length)}`; + } + const spaceOps = [ + ["*** update file ", "*** Update File: "], + ["*** add file ", "*** Add File: "], + ["*** delete file ", "*** Delete File: "], + ] as const; + for (const [needle, canon] of spaceOps) { + if (lower.startsWith(needle)) return `${canon}${trimmed.slice(needle.length)}`; + } + return trimmed; +} + +function isGitPreambleLine(line: string): boolean { + return line === "---" + || GIT_NO_NEWLINE.test(line) + || GIT_META_PREFIX.test(line) + || GIT_FILE_HEADER.test(line); +} + +function unquoteGitPath(path: string): string { + const trimmed = path.trim(); + if ( + (trimmed.startsWith("\"") && trimmed.endsWith("\"")) + || (trimmed.startsWith("'") && trimmed.endsWith("'")) + ) { + return normalizePatchPath(trimmed.slice(1, -1)); + } + return normalizePatchPath(trimmed); +} + +/** Grammar-only path cleanup: trim, POSIX slashes, drop a leading `./`. */ +function normalizePatchPath(path: string): string { + let next = path.trim().replace(/\\/g, "/"); + while (next.startsWith("./")) next = next.slice(2); + return next; +} + +function parseDiffGitPaths(line: string): { a: string; b: string } | undefined { + const quoted = /^diff --git "a\/(.+)" "b\/(.+)"$/.exec(line); + if (quoted) return { a: unquoteGitPath(quoted[1]), b: unquoteGitPath(quoted[2]) }; + const plain = /^diff --git a\/(.+) b\/(.+)$/.exec(line); + if (plain) return { a: unquoteGitPath(plain[1]), b: unquoteGitPath(plain[2]) }; + return undefined; +} + +function parseGitSidePath(line: string, side: "a" | "b"): string | undefined { + const quoted = new RegExp(`^(?:---|[+][+][+]) "${side}\\/(.+)"$`).exec(line); + if (quoted) return unquoteGitPath(quoted[1]); + const plain = new RegExp(`^(?:---|[+][+][+]) ${side}\\/(.+)$`).exec(line); + if (plain) return unquoteGitPath(plain[1]); + return undefined; +} + +function isDevNull(line: string): boolean { + return /^(---|\+\+\+) \/dev\/null$/.test(line); +} + +function rewriteHunkHeader(line: string): string { + return GIT_HUNK_HEADER.test(line) ? "@@" : line; +} + +function isFenceLine(line: string): boolean { + return MARKDOWN_FENCE.test(line); +} + +function isHunkBodyLine(line: string): boolean { + return line === "@@" + || line.startsWith("@@ ") + || GIT_HUNK_HEADER.test(line) + || line.startsWith("+") + || line.startsWith("-") + || line.startsWith(" ") + || line === ""; +} + +function rewriteCodexFileOpLine(line: string): string { + for (const prefix of ["*** Update File:", "*** Add File:", "*** Delete File:", "*** Move to:"] as const) { + if (!line.startsWith(prefix)) continue; + const path = normalizePatchPath(line.slice(prefix.length).replace(/^\s+/, "")); + return path ? `${prefix} ${path}` : line; + } + return line; +} + +function normalizeAddFileBody(lines: readonly string[]): string[] { + const out: string[] = []; + let inAdd = false; + for (const line of lines) { + if (line.startsWith("*** Add File:")) { + inAdd = true; + out.push(line); + continue; + } + if (isCodexFileOpLine(line)) { + inAdd = false; + out.push(line); + continue; + } + if (inAdd && (line === "@@" || line.startsWith("@@ ") || GIT_HUNK_HEADER.test(line))) continue; + if (inAdd && line.length > 0 && !line.startsWith("+")) { + out.push(`+${line}`); + continue; + } + out.push(line); + } + return out; +} + +function hasNonEmptyCodexOp(lines: readonly string[]): boolean { + let kind: "add" | "update" | "delete" | undefined; + let hunk = false; + let any = false; + const flush = () => { + if (kind === "delete" || ((kind === "add" || kind === "update") && hunk)) any = true; + }; + for (const line of lines) { + if (line.startsWith("*** Update File:")) { + flush(); + kind = "update"; + hunk = false; + continue; + } + if (line.startsWith("*** Add File:")) { + flush(); + kind = "add"; + hunk = false; + continue; + } + if (line.startsWith("*** Delete File:")) { + flush(); + kind = "delete"; + hunk = false; + continue; + } + if (line.startsWith("+") || line.startsWith("-") || line === "@@" || line.startsWith("@@ ")) hunk = true; + } + flush(); + return any; +} + +function trimEmptyEdges(lines: readonly string[]): string[] { + let start = 0; + let end = lines.length; + while (start < end && lines[start] === "") start++; + while (end > start && lines[end - 1] === "") end--; + return lines.slice(start, end); +} + +function cleanHunkLines(lines: readonly string[]): string[] { + return trimEmptyEdges( + lines + .filter(line => + !isGitPreambleLine(line) + && !isFenceLine(line) + && line !== PATCH_BEGIN + && line !== PATCH_END + && !isCodexFileOpLine(line) + && !line.startsWith("*** Move to:") + ) + .map(rewriteHunkHeader) + .filter(line => isHunkBodyLine(line)), + ); +} + +function isGitSectionStart(line: string, splitOnDiffGit: boolean): boolean { + if (splitOnDiffGit) return line.startsWith("diff --git "); + return /^(--- )(?:\/dev\/null|"a\/|a\/)/.test(line); +} + +function splitGitSections(lines: readonly string[]): string[][] { + const splitOnDiffGit = lines.some(line => line.startsWith("diff --git ")); + const sections: string[][] = []; + let current: string[] = []; + for (const line of lines) { + if (isGitSectionStart(line, splitOnDiffGit) && current.length > 0) { + sections.push(current); + current = [line]; + continue; + } + current.push(line); + } + if (current.length > 0) sections.push(current); + return sections; +} + +function isGitBinarySection(lines: readonly string[]): boolean { + return lines.some(line => /^Binary files /.test(line) || line.startsWith("GIT binary patch")); +} + +function isGitCopySection(lines: readonly string[]): boolean { + return lines.some(line => line.startsWith("copy from ") || line.startsWith("copy to ")); +} + +function isGitEmptyRenameSection(lines: readonly string[]): boolean { + const hasRenameMeta = lines.some(line => line.startsWith("rename from ") || line.startsWith("rename to ")); + const diff = lines.map(parseDiffGitPaths).find(path => path !== undefined); + if (!hasRenameMeta && !(diff && diff.a !== diff.b)) return false; + const body = cleanHunkLines(lines); + return !body.some(line => line === "@@" || line.startsWith("@@ ") || line.startsWith("+") || line.startsWith("-")); +} + +function isGitUntranslatableSection(lines: readonly string[]): boolean { + return isGitBinarySection(lines) || isGitCopySection(lines) || isGitEmptyRenameSection(lines); +} + +function convertGitSection(lines: readonly string[]): string[] | undefined { + if (isGitBinarySection(lines)) return undefined; + const existingOp = lines.find(isCodexFileOpLine); + if (existingOp) { + const move = lines.filter(line => line.startsWith("*** Move to:")); + return [existingOp, ...move, ...cleanHunkLines(lines)]; + } + const diffPaths = lines.map(parseDiffGitPaths).find(path => path !== undefined); + const plusPath = lines.map(line => parseGitSidePath(line, "b")).find(path => path !== undefined); + const minusPath = lines.map(line => parseGitSidePath(line, "a")).find(path => path !== undefined); + const renameFrom = lines.find(line => line.startsWith("rename from "))?.slice("rename from ".length); + const renameTo = lines.find(line => line.startsWith("rename to "))?.slice("rename to ".length); + const plusIsNull = lines.some(line => line.startsWith("+++ ") && isDevNull(line)); + const minusIsNull = lines.some(line => line.startsWith("--- ") && isDevNull(line)); + const isNewFile = lines.some(line => line.startsWith("new file mode ")) || minusIsNull; + const isDeleted = lines.some(line => line.startsWith("deleted file mode ")) || plusIsNull; + const pathA = minusPath ?? (renameFrom ? unquoteGitPath(renameFrom) : undefined) ?? diffPaths?.a; + const pathB = plusPath ?? (renameTo ? unquoteGitPath(renameTo) : undefined) ?? diffPaths?.b; + const body = cleanHunkLines(lines); + if (isDeleted) { + const path = pathA ?? pathB; + return path ? [`*** Delete File: ${path}`] : undefined; + } + if (isNewFile) { + const path = pathB ?? pathA; + if (!path) return undefined; + return [`*** Add File: ${path}`, ...body.filter(line => line.startsWith("+"))]; + } + const hasMinus = body.some(line => line.startsWith("-")); + if (plusPath && !minusPath && !diffPaths && !hasMinus) { + return [`*** Add File: ${plusPath}`, ...body.filter(line => line.startsWith("+"))]; + } + const from = (renameFrom ? unquoteGitPath(renameFrom) : undefined) ?? pathA ?? pathB; + const to = (renameTo ? unquoteGitPath(renameTo) : undefined) ?? pathB; + if (!from) return undefined; + const hasHunk = body.some(line => line === "@@" || line.startsWith("@@ ") || line.startsWith("+") || line.startsWith("-")); + // Codex 0.147 rejects "Update file hunk ... is empty" (mode-only diffs, 100% renames). + if (!hasHunk) return undefined; + const header = [`*** Update File: ${from}`]; + if (to && to !== from) header.push(`*** Move to: ${to}`); + return [...header, ...body]; +} + +function hasCodexFileOp(lines: readonly string[]): boolean { + return lines.some(isCodexFileOpLine); +} + +/** Grammar-only repair for Cursor-emitted freeform apply_patch. Not a fuzzy filesystem apply. */ +export function sanitizeCodexApplyPatch(patch: string): string { + const trimmed = patch.replace(/^\uFEFF/, "").replace(/\n+$/, ""); + const rawLines = trimmed.split("\n").map(line => canonicalizeCodexLine(line.endsWith("\r") ? line.slice(0, -1) : line)); + const hasCodex = rawLines.some(isCodexFileOpLine); + const hasGitHeaders = rawLines.some(line => line.startsWith("diff --git ") || GIT_FILE_HEADER.test(line)); + if (hasGitHeaders && !hasCodex) { + const sections = splitGitSections(rawLines); + // A binary hunk cannot be expressed in Codex apply_patch. Leave the original + // text alone rather than wrapping the text files and dropping the binary one. + if (sections.some(isGitUntranslatableSection)) return patch.replace(/^\uFEFF/, ""); + const ops = sections + .map(convertGitSection) + .filter((section): section is string[] => section !== undefined && hasCodexFileOp(section)) + .map(normalizeAddFileBody); + if (ops.length > 0) return [PATCH_BEGIN, ...ops.flat(), PATCH_END].join("\n"); + } + const selected: string[] = []; + let inAdd = false; + for (const raw of rawLines) { + if (isGitPreambleLine(raw) || isFenceLine(raw) || raw === PATCH_BEGIN || raw === PATCH_END) continue; + const line = rewriteCodexFileOpLine(rewriteHunkHeader(raw)); + if (line.startsWith("*** Add File:")) { + inAdd = true; + selected.push(line); + continue; + } + if (isCodexFileOpLine(line)) { + inAdd = false; + selected.push(line); + continue; + } + if (line.startsWith("*** Move to:") || isHunkBodyLine(line) || (inAdd && line.length > 0)) { + selected.push(line); + } + } + const lines = normalizeAddFileBody(trimEmptyEdges(selected)); + const looksLikePatch = lines.some(line => + line === "@@" + || line.startsWith("@@ ") + || isCodexFileOpLine(line) + ); + if (!looksLikePatch) return patch.replace(/^\uFEFF/, ""); + // A hunk with no file op is not a valid Codex patch. Do not invent Begin/End around it. + if (!hasCodexFileOp(lines)) return lines.join("\n"); + if (!hasNonEmptyCodexOp(lines)) return patch.replace(/^\uFEFF/, ""); + return [PATCH_BEGIN, ...lines, PATCH_END].join("\n"); +} + +function coercePatchInput(value: unknown): string | undefined { + if (typeof value === "string") { + const trimmed = value.trim(); + if (trimmed.startsWith("{")) { + try { + const inner: unknown = JSON.parse(trimmed); + if (inner && typeof inner === "object" && !Array.isArray(inner)) { + const record = inner as Record; + if (typeof record.input === "string") return record.input; + if (Array.isArray(record.input) && record.input.every(item => typeof item === "string")) { + return record.input.join("\n"); + } + } + } catch { + // The string is the patch, not nested JSON. + } + } + return value; + } + if (Array.isArray(value) && value.every(item => typeof item === "string")) return value.join("\n"); + return undefined; +} + +export function sanitizeEmittedApplyPatchArgs(argsText: string): string { + try { + const parsed: unknown = JSON.parse(argsText); + if (parsed && typeof parsed === "object" && !Array.isArray(parsed)) { + const record = parsed as Record; + const raw = coercePatchInput(record.input) ?? coercePatchInput(record.patch) ?? coercePatchInput(record.content); + if (raw !== undefined) { + const input = sanitizeCodexApplyPatch(raw); + if (input !== record.input) { + const next: Record = { ...record, input }; + delete next.patch; + delete next.content; + return JSON.stringify(next); + } + } + } + } catch { + if ( + argsText.includes("@@") + || argsText.includes("***") + || argsText.includes("diff --git") + || argsText.includes("--- a/") + || argsText.includes("+++ b/") + || argsText.includes("--- /dev/null") + ) { + return JSON.stringify({ input: sanitizeCodexApplyPatch(argsText) }); + } + } + return argsText; +} function firstStringArg(args: Record, keys: readonly string[]): string | undefined { for (const key of keys) { @@ -355,6 +795,17 @@ function firstStringArg(args: Record, keys: readonly string[]): return undefined; } +function firstStringOrLines(args: Record, keys: readonly string[]): string | undefined { + for (const key of keys) { + const value = args[key]; + if (typeof value === "string") return value; + if (Array.isArray(value) && value.length > 0 && value.every(item => typeof item === "string")) { + return value.join("\n"); + } + } + return undefined; +} + /** Split a replacement into patch lines, ignoring one trailing newline (line-based patch semantics). */ function patchLines(text: string): string[] { const lines = text.split("\n"); @@ -362,6 +813,32 @@ function patchLines(text: string): string[] { return lines; } +/** + * Models often copy indent in old_string and omit it in new_string. Codex trim-matches the + * old line and writes new_string verbatim, which strips indent. Copy old leading whitespace + * onto a flush-left new line of the same line count. Do not change a new line that already + * has indent (intentional dedent stays possible). + */ +function restoreFlushLeftIndent(oldString: string, newString: string): string { + const oldLines = patchLines(oldString); + const newLines = patchLines(newString); + if (oldLines.length !== newLines.length || oldLines.length === 0) return newString; + return newLines.map((line, i) => { + const oldLead = /^[ \t]*/.exec(oldLines[i])?.[0] ?? ""; + const newLead = /^[ \t]*/.exec(line)?.[0] ?? ""; + if (oldLead.length > 0 && newLead.length === 0 && line.length > 0) return oldLead + line; + return line; + }).join("\n"); +} + +function addFilePatch(path: string, newString: string): StructuredEditTranslation { + const newLines = patchLines(newString); + if (newLines.length === 0) { + return { error: "structured edit requires a non-empty old_string; an empty replacement is not a valid edit" }; + } + return { patch: [PATCH_BEGIN, `*** Add File: ${path}`, ...newLines.map(line => `+${line}`), PATCH_END].join("\n") }; +} + /** One `@@` hunk replacing `oldString` with `newString`. */ function replacementHunk(oldString: string, newString: string): { hunk: string } | { error: string } { if (oldString.length === 0) { @@ -370,8 +847,9 @@ function replacementHunk(oldString: string, newString: string): { hunk: string } "structured edit requires a non-empty old_string to locate the replacement; for new files or insertions without existing text, call apply_patch with an `*** Add File` / context hunk or use the shell bridge", }; } + const restoredNew = restoreFlushLeftIndent(oldString, newString); const oldLines = patchLines(oldString); - const newLines = patchLines(newString); + const newLines = patchLines(restoredNew); // Line-based patch semantics cannot express an edit that only adds or removes the file's // final newline, and an old/new pair that normalizes to the same lines is a silent no-op — // reject it rather than emitting an empty hunk that apply_patch would drop. @@ -406,6 +884,7 @@ export function translateStructuredEditCall( let parsed: unknown; try { parsed = JSON.parse(argsText); + if (typeof parsed === "string") parsed = JSON.parse(parsed); } catch { return { error: `${toolName} arguments were not valid JSON; the call was dropped. ${ @@ -419,35 +898,111 @@ export function translateStructuredEditCall( return { error: `${toolName} arguments must be a JSON object; the call was dropped.` }; } const args = parsed as Record; - const path = firstStringArg(args, ["file_path", "filePath", "path", "filepath", "filename"]); - if (!path || path.trim().length === 0) { + const rawPath = firstStringArg(args, PATH_ARG_KEYS); + const path = rawPath ? normalizePatchPath(rawPath) : undefined; + if (!path) { return { error: `${toolName} is missing a non-empty file_path; the call was dropped.` }; } + if (/[\n\r\0]/.test(path)) { + return { error: `${toolName} file_path must not contain a newline, CR, or NUL; the call was dropped.` }; + } + if (args.replace_all === true || args.replaceAll === true) { + return { + error: + `${toolName} replace_all is not supported; Codex apply_patch first-matches only. Split into unique old_string hunks or include more surrounding lines.`, + }; + } + if (args.delete_file === true || args.deleteFile === true) { + return { patch: [PATCH_BEGIN, `*** Delete File: ${path}`, PATCH_END].join("\n") }; + } const hunks: string[] = []; const addReplacement = (record: Record): StructuredEditTranslation => { - const oldString = firstStringArg(record, ["old_string", "oldString", "oldtext", "old_text"]); - const newString = firstStringArg(record, ["new_string", "newString", "newtext", "new_text"]); + const oldString = firstStringOrLines(record, OLD_STRING_KEYS); + const newString = firstStringOrLines(record, NEW_STRING_KEYS); if (oldString === undefined || newString === undefined) { return { error: `${toolName} requires old_string and new_string; the call was dropped.` }; } const hunk = replacementHunk(oldString, newString); if ("error" in hunk) return { error: hunk.error }; - return { patch: hunk.hunk as string }; + return { patch: hunk.hunk }; }; if (toolName === CURSOR_MULTI_EDIT_TOOL) { - const edits = args.edits; + let edits: unknown = args.edits; + if (typeof edits === "string") { + try { + edits = JSON.parse(edits); + } catch { + return { error: "multi_edit edits were not valid JSON; the call was dropped." }; + } + } if (!Array.isArray(edits) || edits.length === 0) { return { error: "multi_edit requires a non-empty edits array; the call was dropped." }; } + const pairs: StructuredEditPair[] = []; for (const edit of edits) { if (!edit || typeof edit !== "object" || Array.isArray(edit)) { return { error: "multi_edit edits entries must be objects with old_string and new_string; the call was dropped." }; } - const editResult = addReplacement(edit as Record); - if (editResult.error !== undefined) return editResult; - hunks.push(editResult.patch); + const record = edit as Record; + if (record.replace_all === true || record.replaceAll === true) { + return { + error: + "multi_edit replace_all is not supported; Codex apply_patch first-matches only. Split into unique old_string hunks or include more surrounding lines.", + }; + } + const oldString = firstStringOrLines(record, OLD_STRING_KEYS); + const newString = firstStringOrLines(record, NEW_STRING_KEYS); + if (oldString === undefined || newString === undefined) { + return { error: `${toolName} requires old_string and new_string; the call was dropped.` }; + } + pairs.push({ old_string: oldString, new_string: newString }); + } + const folded = foldSequentialStructuredEdits(pairs); + const seenOld = new Set(); + for (const edit of folded) { + const oldKey = patchLines(edit.old_string).join("\n"); + if (seenOld.has(oldKey)) { + return { + error: + "multi_edit has two edits with the same old_string after line normalization; Codex apply_patch first-matches, so both hunks would hit the same location. Include more surrounding lines to disambiguate.", + }; + } + seenOld.add(oldKey); + } + for (let i = 0; i < folded.length; i++) { + for (let j = 0; j < folded.length; j++) { + if (i === j || folded[j].old_string.length === 0) continue; + if (lineBlockIndex(folded[i].old_string, folded[j].old_string) >= 0) { + return { + error: + "multi_edit hunks overlap: one old_string is a whole-line subset of another. Codex apply_patch matches every hunk against the original file, so both would first-match the same region. Include more surrounding lines to disambiguate.", + }; + } + } + } + if (folded.length === 1 && folded[0].old_string.length === 0) { + return addFilePatch(path, folded[0].new_string); + } + if (folded.some(edit => edit.old_string.length === 0)) { + return { + error: + "multi_edit cannot mix an Add File (empty old_string) with an independent Update hunk on the same path; put the full new-file contents in one empty-old_string edit, or create the file first.", + }; + } + for (const edit of folded) { + const hunk = replacementHunk(edit.old_string, edit.new_string); + if ("error" in hunk) return { error: hunk.error }; + hunks.push(hunk.hunk); } } else { + const oldString = firstStringOrLines(args, OLD_STRING_KEYS); + const newString = firstStringOrLines(args, NEW_STRING_KEYS); + if (oldString === "") { + if (newString === undefined) { + return { error: `${toolName} requires old_string and new_string; the call was dropped.` }; + } + return addFilePatch(path, newString); + } const editResult = addReplacement(args); if (editResult.error !== undefined) return editResult; hunks.push(editResult.patch); @@ -544,7 +1099,9 @@ function dropStructuredEditCall(state: CursorProtobufEventState, callId: string, state.openToolCalls.delete(callId); state.translatorBudget?.closeCall(callId); state.completedToolCalls.add(callId); - return [{ type: "error", message: `${toolName} call was not converted to apply_patch: ${reason}` }]; + // Recoverable: a fatal adapter error becomes response.failed / upstream_server_error and + // the model never sees the reason. Completing with text keeps the turn alive (#1388). + return [{ type: "text", text: `\n${toolName} call was not converted to apply_patch: ${reason}` }]; } function commitToolCall(state: CursorProtobufEventState, callId: string, finalArgs: string): CursorServerMessage[] { @@ -573,7 +1130,8 @@ function commitToolCall(state: CursorProtobufEventState, callId: string, finalAr state.translatorBudget?.releaseRetained(previousBytes, { kind: "tool_args", callId }); } const emittedName = translation ? CODEX_APPLY_PATCH_TOOL : open.name; - const emittedArgs = translation ? JSON.stringify({ input: translation.patch }) : finalArgs; + const rawArgs = translation ? JSON.stringify({ input: translation.patch }) : finalArgs; + const emittedArgs = emittedName === CODEX_APPLY_PATCH_TOOL ? sanitizeEmittedApplyPatchArgs(rawArgs) : rawArgs; const out: CursorServerMessage[] = [{ type: "tool_call_start", id: callId, name: emittedName }]; if (emittedArgs.length > 0) out.push({ type: "tool_call_delta", arguments: emittedArgs }); out.push(...endToolCall(state, callId)); diff --git a/src/adapters/cursor/tool-definitions.ts b/src/adapters/cursor/tool-definitions.ts index 7bdc544eb2..c0fd95a894 100644 --- a/src/adapters/cursor/tool-definitions.ts +++ b/src/adapters/cursor/tool-definitions.ts @@ -249,14 +249,14 @@ export function cursorStructuredEditTools( name: CURSOR_EDIT_FILE_TOOL, cursorStructuredEdit: true, description: - "Replace one block of exact text in a file. OpenCodex converts the replacement into a Codex apply_patch change, which the Codex client applies with its normal approval and sandbox policy. old_string must match the current file content exactly at exactly one location (apply_patch rejects ambiguous hunks). Matching is line-based, so an edit cannot add or remove only the file's final newline, and old_string/new_string that are identical after line normalization are rejected as a no-op.", + "Replace one block of text in a file. OpenCodex converts the replacement into a Codex apply_patch change. Copy old_string and new_string with their exact leading whitespace — Codex may locate a line after trimming indent, but it writes new_string verbatim, so stripped indent silently corrupts the file. An empty old_string with a non-empty new_string creates a new file (Add File). If the same text appears more than once, the first match is updated. Matching is line-based, so an edit cannot add or remove only the file's final newline, and old_string/new_string that are identical after line normalization are rejected as a no-op.", parameters: { ...CURSOR_EDIT_FILE_INPUT_SCHEMA }, }, { name: CURSOR_MULTI_EDIT_TOOL, cursorStructuredEdit: true, description: - "Apply several exact-text replacements to one file. OpenCodex converts the edits into a single Codex apply_patch change, which the Codex client applies with its normal approval and sandbox policy. Each old_string must match the current file content exactly at exactly one location (apply_patch rejects ambiguous hunks). Edits are independent: every old_string is matched against the ORIGINAL file content, so a later edit must not rely on text introduced by an earlier one. Matching is line-based, so an edit cannot add or remove only the file's final newline, and old_string/new_string that are identical after line normalization are rejected as a no-op.", + "Apply several text replacements to one file. OpenCodex converts them into one Codex apply_patch change. Copy each old_string/new_string with exact leading whitespace. If a later edit's old_string is the text after an earlier replacement, OpenCodex folds those edits into one original-file hunk. Independent edits stay separate hunks. An empty old_string with a non-empty new_string creates a new file (Add File); do not mix that with an independent Update on the same path. If the same text appears more than once, the first match is updated. Matching is line-based, so an edit cannot add or remove only the file's final newline, and identical old/new after line normalization are rejected as a no-op.", parameters: { ...CURSOR_MULTI_EDIT_INPUT_SCHEMA }, }, ]; @@ -558,7 +558,7 @@ export function buildCursorToolGuidanceSystemNote( // Host-shell-neutral: the Codex client executes bridge commands, and may differ from // the OpenCodex proxy OS (LAN/SSH remote-proxy). Always cover PowerShell 5.1 pitfalls. const hostShellNote = hasBareExec - ? "Match shell syntax to the Codex client host that runs the bridge (not only the proxy OS). Windows PowerShell 5.1: no CMD `cd /d`, no bash heredocs (`< 0 - ? `For file edits, prefer the structured edit tools ${quotedNames(structuredEditNames)} — they take exact-match replacements that OpenCodex converts into Codex \`apply_patch\` changes for approval. Use \`apply_patch\` directly only when you can emit its exact freeform syntax (\`*** Begin Patch\` envelope with \`@@\` hunks and \`-\`/\`+\` line prefixes); never emit patch-like plain text as tool arguments.` + ? `For file edits, prefer the structured edit tools ${quotedNames(structuredEditNames)} — they take replacements that OpenCodex converts into Codex \`apply_patch\` changes. Include exact leading whitespace in old_string/new_string. Use \`apply_patch\` directly only with a \`*** Begin Patch\` envelope and bare \`@@\` hunks (never git-style \`@@ -n,m +n,m @@\`); never emit patch-like plain text as tool arguments.` : "For file edits, use the `apply_patch` tool, not built-in file write/delete tools." : undefined, hasBareExec diff --git a/tests/cursor-structured-edit.test.ts b/tests/cursor-structured-edit.test.ts index ab412eb4be..af146387ca 100644 --- a/tests/cursor-structured-edit.test.ts +++ b/tests/cursor-structured-edit.test.ts @@ -14,8 +14,11 @@ import { } from "../src/adapters/cursor/gen/agent_pb"; import { createCursorProtobufEventState, + foldSequentialStructuredEdits, mapCursorProtobufServerMessage, mapSyntheticMcpExecToToolEvents, + sanitizeCodexApplyPatch, + sanitizeEmittedApplyPatchArgs, translateStructuredEditCall, } from "../src/adapters/cursor/protobuf-events"; import { planMcpArgsHandling } from "../src/adapters/cursor/live-transport"; @@ -150,6 +153,9 @@ describe("cursor structured edit tools (#1017)", () => { expect(note).toContain("`edit_file`"); expect(note).toContain("`multi_edit`"); expect(note).toContain("never emit patch-like plain text as tool arguments"); + expect(note).toContain("exact leading whitespace"); + expect(note).toContain("never git-style"); + expect(note).not.toContain("rejects ambiguous hunks"); }); test("guidance note keeps the apply_patch-only guidance without structured tools", () => { @@ -218,6 +224,16 @@ describe("translateStructuredEditCall", () => { }); }); + test("structured edit tool text does not claim unique-hunk rejection", () => { + const tools = cursorStructuredEditTools([applyPatchTool()], "auto"); + for (const tool of tools) { + expect(tool.description).toContain("exact leading whitespace"); + expect(tool.description).toContain("first match"); + expect(tool.description).not.toContain("rejects ambiguous hunks"); + } + expect(tools[0]?.description).toContain("Add File"); + }); + test("converts multi_edit into one apply_patch payload with one hunk per edit", () => { const args = JSON.stringify({ file_path: "src/e.ts", @@ -241,11 +257,1019 @@ describe("translateStructuredEditCall", () => { }); }); + test("folds a dependent multi_edit into one original-file hunk (#1388 L4)", () => { + const args = JSON.stringify({ + file_path: "git.nix", + edits: [ + { old_string: ' editor = "nvim";', new_string: ' editor = "hx";' }, + { + old_string: ' editor = "hx";\n };', + new_string: ' editor = "hx";\n excludesfile = "~/.gitignore";\n };', + }, + ], + }); + expect(translateStructuredEditCall(CURSOR_MULTI_EDIT_TOOL, args)).toEqual({ + patch: [ + "*** Begin Patch", + "*** Update File: git.nix", + "@@", + '- editor = "nvim";', + "- };", + '+ editor = "hx";', + '+ excludesfile = "~/.gitignore";', + "+ };", + "*** End Patch", + ].join("\n"), + }); + }); + + test("foldSequentialStructuredEdits keeps independent pairs", () => { + expect(foldSequentialStructuredEdits([ + { old_string: "a", new_string: "b" }, + { old_string: "c", new_string: "d" }, + ])).toEqual([ + { old_string: "a", new_string: "b" }, + { old_string: "c", new_string: "d" }, + ]); + }); + + test("does not fold a later old_string that is only a substring of an earlier new_string (B8)", () => { + expect(foldSequentialStructuredEdits([ + { old_string: "x = 1", new_string: "x = hello world" }, + { old_string: "hello world", new_string: "hello earth" }, + ])).toEqual([ + { old_string: "x = 1", new_string: "x = hello world" }, + { old_string: "hello world", new_string: "hello earth" }, + ]); + expect(translateStructuredEditCall(CURSOR_MULTI_EDIT_TOOL, JSON.stringify({ + file_path: "f.txt", + edits: [ + { old_string: "x = 1", new_string: "x = hello world" }, + { old_string: "hello world", new_string: "hello earth" }, + ], + }))).toEqual({ + patch: [ + "*** Begin Patch", + "*** Update File: f.txt", + "@@", + "-x = 1", + "+x = hello world", + "@@", + "-hello world", + "+hello earth", + "*** End Patch", + ].join("\n"), + }); + }); + + test("does not fold when an earlier new_string is only a substring of a later old_string", () => { + expect(foldSequentialStructuredEdits([ + { old_string: "alpha", new_string: "a" }, + { old_string: "apple", new_string: "pear" }, + ])).toEqual([ + { old_string: "alpha", new_string: "a" }, + { old_string: "apple", new_string: "pear" }, + ]); + }); + + test("still folds an exact sequential hop one→two→three", () => { + expect(foldSequentialStructuredEdits([ + { old_string: "one", new_string: "two" }, + { old_string: "two", new_string: "three" }, + { old_string: "three", new_string: "four" }, + ])).toEqual([{ old_string: "one", new_string: "four" }]); + }); + + test("still folds a later old_string that contains an earlier new_string as whole lines (L4)", () => { + expect(foldSequentialStructuredEdits([ + { old_string: ' editor = "nvim";', new_string: ' editor = "hx";' }, + { + old_string: ' editor = "hx";\n };', + new_string: ' editor = "hx";\n excludesfile = "~/.gitignore";\n };', + }, + ])).toEqual([{ + old_string: ' editor = "nvim";\n };', + new_string: ' editor = "hx";\n excludesfile = "~/.gitignore";\n };', + }]); + }); + + test("converts edit_file with empty old_string into Add File (R6)", () => { + expect(translateStructuredEditCall( + CURSOR_EDIT_FILE_TOOL, + JSON.stringify({ file_path: "hello.txt", old_string: "", new_string: "hello world\n" }), + )).toEqual({ + patch: [ + "*** Begin Patch", + "*** Add File: hello.txt", + "+hello world", + "*** End Patch", + ].join("\n"), + }); + }); + + test("copies old_string leading whitespace onto a flush-left new_string of the same line count", () => { + expect(translateStructuredEditCall( + CURSOR_EDIT_FILE_TOOL, + JSON.stringify({ + file_path: "math.py", + old_string: " return a - b", + new_string: "return a + b", + }), + )).toEqual({ + patch: [ + "*** Begin Patch", + "*** Update File: math.py", + "@@", + "- return a - b", + "+ return a + b", + "*** End Patch", + ].join("\n"), + }); + }); + + test("folds a later edit that tweaks a whole line inside an earlier replacement", () => { + expect(foldSequentialStructuredEdits([ + { old_string: "foo\nbar", new_string: "foo\nbaz\nqux" }, + { old_string: "baz", new_string: "BAZ" }, + ])).toEqual([{ old_string: "foo\nbar", new_string: "foo\nBAZ\nqux" }]); + }); + + test("copies a leading tab onto a flush-left new_string", () => { + expect(translateStructuredEditCall( + CURSOR_EDIT_FILE_TOOL, + JSON.stringify({ file_path: "t.nix", old_string: "\tname = nvim", new_string: "name = hx" }), + )).toEqual({ + patch: [ + "*** Begin Patch", + "*** Update File: t.nix", + "@@", + "-\tname = nvim", + "+\tname = hx", + "*** End Patch", + ].join("\n"), + }); + }); + + test("does not copy indent when the replacement changes line count", () => { + expect(translateStructuredEditCall( + CURSOR_EDIT_FILE_TOOL, + JSON.stringify({ + file_path: "a.ts", + old_string: " foo", + new_string: "foo\nbar", + }), + )).toEqual({ + patch: [ + "*** Begin Patch", + "*** Update File: a.ts", + "@@", + "- foo", + "+foo", + "+bar", + "*** End Patch", + ].join("\n"), + }); + }); + + test("does not invent indent when new_string already has leading whitespace", () => { + expect(translateStructuredEditCall( + CURSOR_EDIT_FILE_TOOL, + JSON.stringify({ + file_path: "git.nix", + old_string: ' editor = "nvim";', + new_string: ' editor = "hx";', + }), + )).toEqual({ + patch: [ + "*** Begin Patch", + "*** Update File: git.nix", + "@@", + '- editor = "nvim";', + '+ editor = "hx";', + "*** End Patch", + ].join("\n"), + }); + }); + + test("sanitizeCodexApplyPatch rewrites git-style hunk headers and missing envelopes", () => { + expect(sanitizeCodexApplyPatch([ + "*** Begin Patch", + "*** Update File: git.nix", + "@@ -3,7 +3,7 @@", + '- editor = "nvim";', + '+ editor = "hx";', + "*** End Patch", + ].join("\n"))).toContain("\n@@\n"); + expect(sanitizeCodexApplyPatch([ + "*** Update File: git.nix", + "@@", + "-old", + "+new", + "*** End Patch", + ].join("\n")).startsWith("*** Begin Patch")).toBe(true); + const alreadyValid = [ + "*** Begin Patch", + "*** Update File: git.nix", + "@@", + "-old", + "+new", + "*** End Patch", + "", + ].join("\n"); + expect(sanitizeCodexApplyPatch(alreadyValid)).toBe(alreadyValid.replace(/\n+$/, "")); + expect(sanitizeCodexApplyPatch(alreadyValid).split("*** End Patch").length).toBe(2); + }); + + test("does not wrap a hunk that has no file operation (OFF_L8)", () => { + const hunkOnly = sanitizeCodexApplyPatch(["@@", "-old", "+new"].join("\n")); + expect(hunkOnly.startsWith("*** Begin Patch")).toBe(false); + expect(hunkOnly).toBe(["@@", "-old", "+new"].join("\n")); + }); + + test("strips a git unified-diff preamble and infers Update File from +++ b/", () => { + expect(sanitizeCodexApplyPatch([ + "diff --git a/git.nix b/git.nix", + "--- a/git.nix", + "+++ b/git.nix", + "@@ -3,7 +3,7 @@", + '- editor = "nvim";', + '+ editor = "hx";', + ].join("\n"))).toBe([ + "*** Begin Patch", + "*** Update File: git.nix", + "@@", + '- editor = "nvim";', + '+ editor = "hx";', + "*** End Patch", + ].join("\n")); + }); + + test("keeps a CR that is already in old_string and does not invent one on new_string", () => { + // Codex 0.147 strips CR from patch lines on apply, so copying CR onto new_string is a no-op. + expect(translateStructuredEditCall( + CURSOR_EDIT_FILE_TOOL, + JSON.stringify({ + file_path: "crlf.txt", + old_string: "beta\r\n", + new_string: "BETA\n", + }), + )).toEqual({ + patch: [ + "*** Begin Patch", + "*** Update File: crlf.txt", + "@@", + "-beta\r", + "+BETA", + "*** End Patch", + ].join("\n"), + }); + }); + + test("rejects multi_edit edits that share the same old_string after line normalization", () => { + expect(translateStructuredEditCall(CURSOR_MULTI_EDIT_TOOL, JSON.stringify({ + file_path: "a.txt", + edits: [ + { old_string: "editor = nvim", new_string: "editor = hx" }, + { old_string: "editor = nvim\n", new_string: "editor = vim" }, + ], + }))?.error).toContain("same old_string"); + }); + + test("rejects multi_edit hunks whose old_string is a whole-line subset of another (overlapping first-match)", () => { + expect(translateStructuredEditCall(CURSOR_MULTI_EDIT_TOOL, JSON.stringify({ + file_path: "a.txt", + edits: [ + { old_string: "line1\nline2", new_string: "LINE1\nLINE2" }, + { old_string: "line2", new_string: "x" }, + ], + }))?.error).toContain("overlap"); + }); + + test("converts multi_edit empty old_string into Add File", () => { + expect(translateStructuredEditCall(CURSOR_MULTI_EDIT_TOOL, JSON.stringify({ + file_path: "hello.txt", + edits: [{ old_string: "", new_string: "hello world\n" }], + }))).toEqual({ + patch: [ + "*** Begin Patch", + "*** Add File: hello.txt", + "+hello world", + "*** End Patch", + ].join("\n"), + }); + }); + + test("folds a later tweak into a multi_edit Add File", () => { + expect(translateStructuredEditCall(CURSOR_MULTI_EDIT_TOOL, JSON.stringify({ + file_path: "hello.txt", + edits: [ + { old_string: "", new_string: "hello" }, + { old_string: "hello", new_string: "hello world" }, + ], + }))).toEqual({ + patch: [ + "*** Begin Patch", + "*** Add File: hello.txt", + "+hello world", + "*** End Patch", + ].join("\n"), + }); + }); + + test("rejects multi_edit that mixes Add File with an independent Update hunk", () => { + expect(translateStructuredEditCall(CURSOR_MULTI_EDIT_TOOL, JSON.stringify({ + file_path: "hello.txt", + edits: [ + { old_string: "", new_string: "hello" }, + { old_string: "other", new_string: "OTHER" }, + ], + }))?.error).toContain("Add File"); + }); + + test("normalizes file_path whitespace, ./ prefix, and Windows slashes", () => { + expect(translateStructuredEditCall( + CURSOR_EDIT_FILE_TOOL, + JSON.stringify({ file_path: " ./src\\foo.ts ", old_string: "a", new_string: "b" }), + )).toEqual({ + patch: [ + "*** Begin Patch", + "*** Update File: src/foo.ts", + "@@", + "-a", + "+b", + "*** End Patch", + ].join("\n"), + }); + }); + + test("infers Add File from a git new-file preamble", () => { + expect(sanitizeCodexApplyPatch([ + "diff --git a/hello.txt b/hello.txt", + "new file mode 100644", + "index 0000000..3b18e51", + "--- /dev/null", + "+++ b/hello.txt", + "@@ -0,0 +1 @@", + "+hello world", + ].join("\n"))).toBe([ + "*** Begin Patch", + "*** Add File: hello.txt", + "+hello world", + "*** End Patch", + ].join("\n")); + }); + + test("infers Delete File from a git deleted-file preamble", () => { + expect(sanitizeCodexApplyPatch([ + "diff --git a/gone.txt b/gone.txt", + "deleted file mode 100644", + "index 3b18e51..0000000", + "--- a/gone.txt", + "+++ /dev/null", + "@@ -1 +0,0 @@", + "-delete me", + ].join("\n"))).toBe([ + "*** Begin Patch", + "*** Delete File: gone.txt", + "*** End Patch", + ].join("\n")); + }); + + test("infers Update File + Move to from a git rename", () => { + expect(sanitizeCodexApplyPatch([ + "diff --git a/old.txt b/new.txt", + "similarity index 80%", + "rename from old.txt", + "rename to new.txt", + "--- a/old.txt", + "+++ b/new.txt", + "@@ -1 +1 @@", + "-old", + "+new", + ].join("\n"))).toBe([ + "*** Begin Patch", + "*** Update File: old.txt", + "*** Move to: new.txt", + "@@", + "-old", + "+new", + "*** End Patch", + ].join("\n")); + }); + + test("splits a multi-file git diff into one Codex file operation per path", () => { + expect(sanitizeCodexApplyPatch([ + "diff --git a/a.txt b/a.txt", + "--- a/a.txt", + "+++ b/a.txt", + "@@ -1 +1 @@", + "-old", + "+new", + "diff --git a/b.txt b/b.txt", + "--- a/b.txt", + "+++ b/b.txt", + "@@ -1 +1 @@", + "-x", + "+y", + ].join("\n"))).toBe([ + "*** Begin Patch", + "*** Update File: a.txt", + "@@", + "-old", + "+new", + "*** Update File: b.txt", + "@@", + "-x", + "+y", + "*** End Patch", + ].join("\n")); + }); + + test("strips git no-newline markers and old/new mode lines", () => { + expect(sanitizeCodexApplyPatch([ + "diff --git a/a.txt b/a.txt", + "old mode 100644", + "new mode 100755", + "--- a/a.txt", + "+++ b/a.txt", + "@@ -1 +1 @@", + "-old", + "\\ No newline at end of file", + "+new", + "\\ No newline at end of file", + ].join("\n"))).toBe([ + "*** Begin Patch", + "*** Update File: a.txt", + "@@", + "-old", + "+new", + "*** End Patch", + ].join("\n")); + }); + + test("unquotes a git path with spaces", () => { + expect(sanitizeCodexApplyPatch([ + 'diff --git "a/my file.txt" "b/my file.txt"', + '--- "a/my file.txt"', + '+++ "b/my file.txt"', + "@@ -1 +1 @@", + "-hello", + "+HELLO", + ].join("\n"))).toBe([ + "*** Begin Patch", + "*** Update File: my file.txt", + "@@", + "-hello", + "+HELLO", + "*** End Patch", + ].join("\n")); + }); + + test("strips a trailing CR from CRLF-encoded git patch lines", () => { + const lf = [ + "diff --git a/a.txt b/a.txt", + "--- a/a.txt", + "+++ b/a.txt", + "@@ -1 +1 @@", + "-old", + "+new", + ].join("\n"); + expect(sanitizeCodexApplyPatch(["diff --git a/a.txt b/a.txt", "--- a/a.txt", "+++ b/a.txt", "@@ -1 +1 @@", "-old", "+new"].join("\r\n"))).toBe(sanitizeCodexApplyPatch(lf)); + }); + + test("does not invent an Update File for a git binary diff", () => { + const binary = [ + "diff --git a/x.bin b/x.bin", + "index 111..222", + "Binary files a/x.bin and b/x.bin differ", + ].join("\n"); + expect(sanitizeCodexApplyPatch(binary)).toBe(binary); + expect(sanitizeCodexApplyPatch(binary).startsWith("*** Begin Patch")).toBe(false); + }); + + test("does not drop a binary file from a mixed git diff", () => { + const mixed = [ + "diff --git a/a.txt b/a.txt", + "--- a/a.txt", + "+++ b/a.txt", + "@@ -1 +1 @@", + "-old", + "+new", + "diff --git a/x.bin b/x.bin", + "Binary files a/x.bin and b/x.bin differ", + ].join("\n"); + expect(sanitizeCodexApplyPatch(mixed)).toBe(mixed); + }); + + test("does not treat a git copy as a Move (source must stay)", () => { + const copy = [ + "diff --git a/old.txt b/new.txt", + "similarity index 100%", + "copy from old.txt", + "copy to new.txt", + ].join("\n"); + // A 100% copy has no hunk bytes. Inventing Move would delete the source; + // inventing an empty Add File would not copy contents. Leave the original. + expect(sanitizeCodexApplyPatch(copy)).toBe(copy); + }); + + test("keeps unified-diff context lines on an Update File", () => { + expect(sanitizeCodexApplyPatch([ + "diff --git a/a.txt b/a.txt", + "--- a/a.txt", + "+++ b/a.txt", + "@@ -1,3 +1,3 @@", + " keep", + "-old", + "+new", + " also", + ].join("\n"))).toBe([ + "*** Begin Patch", + "*** Update File: a.txt", + "@@", + " keep", + "-old", + "+new", + " also", + "*** End Patch", + ].join("\n")); + }); + + test("does not invent an empty Update File for a mode-only git diff", () => { + const modeOnly = [ + "diff --git a/a.txt b/a.txt", + "old mode 100644", + "new mode 100755", + ].join("\n"); + expect(sanitizeCodexApplyPatch(modeOnly)).toBe(modeOnly); + }); + + test("does not invent an empty Update hunk for a 100% git rename", () => { + const rename = [ + "diff --git a/old.txt b/new.txt", + "similarity index 100%", + "rename from old.txt", + "rename to new.txt", + ].join("\n"); + // Codex 0.147 rejects "Update file hunk ... is empty". A 100% rename has no + // bytes we can put in a hunk, so leave the original git text alone. + expect(sanitizeCodexApplyPatch(rename)).toBe(rename); + }); + + test("rejects a file_path that normalizes to empty", () => { + expect(translateStructuredEditCall( + CURSOR_EDIT_FILE_TOOL, + JSON.stringify({ file_path: "./", old_string: "a", new_string: "b" }), + )?.error).toContain("file_path"); + }); + + test("rejects a file_path that contains a newline or NUL", () => { + expect(translateStructuredEditCall( + CURSOR_EDIT_FILE_TOOL, + JSON.stringify({ file_path: "foo\nbar.ts", old_string: "a", new_string: "b" }), + )?.error).toContain("file_path"); + expect(translateStructuredEditCall( + CURSOR_EDIT_FILE_TOOL, + JSON.stringify({ file_path: "foo\u0000bar.ts", old_string: "a", new_string: "b" }), + )?.error).toContain("file_path"); + }); + + test("infers Update File from a unified diff that has no diff --git line", () => { + expect(sanitizeCodexApplyPatch([ + "--- a/git.nix", + "+++ b/git.nix", + "@@ -3,7 +3,7 @@", + '- editor = "nvim";', + '+ editor = "hx";', + ].join("\n"))).toBe([ + "*** Begin Patch", + "*** Update File: git.nix", + "@@", + '- editor = "nvim";', + '+ editor = "hx";', + "*** End Patch", + ].join("\n")); + }); + + test("infers Add File from --- /dev/null without diff --git", () => { + expect(sanitizeCodexApplyPatch([ + "--- /dev/null", + "+++ b/hello.txt", + "@@ -0,0 +1 @@", + "+hello", + ].join("\n"))).toBe([ + "*** Begin Patch", + "*** Add File: hello.txt", + "+hello", + "*** End Patch", + ].join("\n")); + }); + + test("splits two unified diffs that have no diff --git lines", () => { + expect(sanitizeCodexApplyPatch([ + "--- a/a.txt", + "+++ b/a.txt", + "@@ -1 +1 @@", + "-old", + "+new", + "--- a/b.txt", + "+++ b/b.txt", + "@@ -1 +1 @@", + "-x", + "+y", + ].join("\n"))).toBe([ + "*** Begin Patch", + "*** Update File: a.txt", + "@@", + "-old", + "+new", + "*** Update File: b.txt", + "@@", + "-x", + "+y", + "*** End Patch", + ].join("\n")); + }); + + test("strips markdown fences and leading/trailing prose from a Codex patch", () => { + expect(sanitizeCodexApplyPatch([ + "Sure, here is the patch:", + "```diff", + "*** Begin Patch", + "*** Update File: git.nix", + "@@", + "-old", + "+new", + "*** End Patch", + "```", + "Hope that helps!", + ].join("\n"))).toBe([ + "*** Begin Patch", + "*** Update File: git.nix", + "@@", + "-old", + "+new", + "*** End Patch", + ].join("\n")); + }); + + test("rewrites a hunk header that omits the trailing @@", () => { + expect(sanitizeCodexApplyPatch([ + "*** Begin Patch", + "*** Update File: git.nix", + "@@ -1,3 +1,3", + "-old", + "+new", + "*** End Patch", + ].join("\n"))).toContain("\n@@\n"); + }); + + test("does not re-wrap an empty Update File (Codex rejects empty hunks)", () => { + const empty = ["*** Begin Patch", "*** Update File: foo.txt", "*** End Patch"].join("\n"); + expect(sanitizeCodexApplyPatch(empty)).toBe(empty); + }); + + test("drops @@ after Add File (every Add File line must be a + line)", () => { + expect(sanitizeCodexApplyPatch([ + "*** Begin Patch", + "*** Add File: hello.txt", + "@@", + "+hello", + "*** End Patch", + ].join("\n"))).toBe([ + "*** Begin Patch", + "*** Add File: hello.txt", + "+hello", + "*** End Patch", + ].join("\n")); + }); + + test("normalizes ./ and Windows slashes on an existing Codex file header", () => { + expect(sanitizeCodexApplyPatch([ + "*** Begin Patch", + "*** Update File: ./src\\foo.ts", + "@@", + "-a", + "+b", + "*** End Patch", + ].join("\n"))).toBe([ + "*** Begin Patch", + "*** Update File: src/foo.ts", + "@@", + "-a", + "+b", + "*** End Patch", + ].join("\n")); + }); + + test("accepts file and contents aliases on edit_file", () => { + expect(translateStructuredEditCall( + CURSOR_EDIT_FILE_TOOL, + JSON.stringify({ file: "a.ts", old_string: "a", new_string: "b" }), + )).toEqual({ + patch: [ + "*** Begin Patch", + "*** Update File: a.ts", + "@@", + "-a", + "+b", + "*** End Patch", + ].join("\n"), + }); + expect(translateStructuredEditCall( + CURSOR_EDIT_FILE_TOOL, + JSON.stringify({ file_path: "hello.txt", old_string: "", contents: "hello" }), + )).toEqual({ + patch: [ + "*** Begin Patch", + "*** Add File: hello.txt", + "+hello", + "*** End Patch", + ].join("\n"), + }); + }); + + test("rejects a file_path that contains a CR (header injection)", () => { + expect(translateStructuredEditCall( + CURSOR_EDIT_FILE_TOOL, + JSON.stringify({ file_path: "foo\r*** Delete File: secrets.env", old_string: "a", new_string: "b" }), + )?.error).toContain("file_path"); + }); + + test("rejects replace_all because apply_patch first-matches only", () => { + expect(translateStructuredEditCall( + CURSOR_EDIT_FILE_TOOL, + JSON.stringify({ file_path: "a.ts", old_string: "x", new_string: "y", replace_all: true }), + )?.error).toContain("replace_all"); + expect(translateStructuredEditCall(CURSOR_MULTI_EDIT_TOOL, JSON.stringify({ + file_path: "a.ts", + edits: [{ old_string: "x", new_string: "y", replace_all: true }], + }))?.error).toContain("replace_all"); + }); + + test("sanitizeEmittedApplyPatchArgs rewrites a patch key onto input", () => { + const raw = JSON.stringify({ + patch: [ + "*** Begin Patch", + "*** Update File: a.txt", + "@@ -1 +1 @@", + "-old", + "+new", + "*** End Patch", + ].join("\n"), + }); + expect(JSON.parse(sanitizeEmittedApplyPatchArgs(raw))).toEqual({ + input: [ + "*** Begin Patch", + "*** Update File: a.txt", + "@@", + "-old", + "+new", + "*** End Patch", + ].join("\n"), + }); + }); + + test("canonicalizes lowercase Codex file-op headers", () => { + expect(sanitizeCodexApplyPatch([ + "*** begin patch", + "*** update file: git.nix", + "@@", + "-old", + "+new", + "*** end patch", + ].join("\n"))).toBe([ + "*** Begin Patch", + "*** Update File: git.nix", + "@@", + "-old", + "+new", + "*** End Patch", + ].join("\n")); + }); + + test("accepts *** Update File path without a colon", () => { + expect(sanitizeCodexApplyPatch([ + "*** Update File a.txt", + "@@", + "-old", + "+new", + ].join("\n"))).toBe([ + "*** Begin Patch", + "*** Update File: a.txt", + "@@", + "-old", + "+new", + "*** End Patch", + ].join("\n")); + }); + + test("prefixes unprefixed Add File body lines with +", () => { + expect(sanitizeCodexApplyPatch([ + "*** Begin Patch", + "*** Add File: hello.txt", + "hello world", + "second line", + "*** End Patch", + ].join("\n"))).toBe([ + "*** Begin Patch", + "*** Add File: hello.txt", + "+hello world", + "+second line", + "*** End Patch", + ].join("\n")); + }); + + test("infers Add File from +++ b/ with only added lines and no --- a/", () => { + expect(sanitizeCodexApplyPatch([ + "+++ b/hello.txt", + "@@ -0,0 +1 @@", + "+hello", + ].join("\n"))).toBe([ + "*** Begin Patch", + "*** Add File: hello.txt", + "+hello", + "*** End Patch", + ].join("\n")); + }); + + test("joins array old_string/new_string into a replacement", () => { + expect(translateStructuredEditCall( + CURSOR_EDIT_FILE_TOOL, + JSON.stringify({ file_path: "a.ts", old_string: ["line1", "line2"], new_string: ["line1", "changed"] }), + )).toEqual({ + patch: [ + "*** Begin Patch", + "*** Update File: a.ts", + "@@", + "-line1", + "-line2", + "+line1", + "+changed", + "*** End Patch", + ].join("\n"), + }); + }); + + test("parses a JSON-string edits array on multi_edit", () => { + expect(translateStructuredEditCall(CURSOR_MULTI_EDIT_TOOL, JSON.stringify({ + file_path: "a.ts", + edits: JSON.stringify([{ old_string: "a", new_string: "b" }]), + }))).toEqual({ + patch: [ + "*** Begin Patch", + "*** Update File: a.ts", + "@@", + "-a", + "+b", + "*** End Patch", + ].join("\n"), + }); + }); + + test("parses double-encoded structured edit arguments", () => { + expect(translateStructuredEditCall( + CURSOR_EDIT_FILE_TOOL, + JSON.stringify(JSON.stringify({ file_path: "a.ts", old_string: "a", new_string: "b" })), + )).toEqual({ + patch: [ + "*** Begin Patch", + "*** Update File: a.ts", + "@@", + "-a", + "+b", + "*** End Patch", + ].join("\n"), + }); + }); + + test("accepts before/after and search/replace aliases", () => { + expect(translateStructuredEditCall( + CURSOR_EDIT_FILE_TOOL, + JSON.stringify({ file_path: "a.ts", before: "a", after: "b" }), + )).toEqual(expect.objectContaining({ patch: expect.stringContaining("+b") })); + expect(translateStructuredEditCall( + CURSOR_EDIT_FILE_TOOL, + JSON.stringify({ file_path: "a.ts", search: "a", replace: "b" }), + )).toEqual(expect.objectContaining({ patch: expect.stringContaining("+b") })); + }); + + test("does not treat from/to as a text replacement (rename-shaped args)", () => { + expect(translateStructuredEditCall( + CURSOR_EDIT_FILE_TOOL, + JSON.stringify({ file_path: "a.ts", from: "old.ts", to: "new.ts" }), + )?.error).toContain("old_string"); + }); + + test("does not treat a bare delete flag as Delete File", () => { + expect(translateStructuredEditCall( + CURSOR_EDIT_FILE_TOOL, + JSON.stringify({ file_path: "gone.txt", delete: true }), + )?.error).toBeTruthy(); + }); + + test("converts delete_file: true into Delete File", () => { + expect(translateStructuredEditCall( + CURSOR_EDIT_FILE_TOOL, + JSON.stringify({ file_path: "gone.txt", delete_file: true }), + )).toEqual({ + patch: [ + "*** Begin Patch", + "*** Delete File: gone.txt", + "*** End Patch", + ].join("\n"), + }); + }); + + test("sanitizes apply_patch input when it is an array of lines", () => { + expect(JSON.parse(sanitizeEmittedApplyPatchArgs(JSON.stringify({ + input: ["*** Begin Patch", "*** Update File: a.txt", "@@ -1 +1 @@", "-old", "+new", "*** End Patch"], + })))).toEqual({ + input: [ + "*** Begin Patch", + "*** Update File: a.txt", + "@@", + "-old", + "+new", + "*** End Patch", + ].join("\n"), + }); + }); + + test("sanitizes a raw unified diff that has no @@ or Begin Patch", () => { + expect(JSON.parse(sanitizeEmittedApplyPatchArgs([ + "--- a/a.txt", + "+++ b/a.txt", + "-old", + "+new", + ].join("\n")))).toEqual({ + input: [ + "*** Begin Patch", + "*** Update File: a.txt", + "-old", + "+new", + "*** End Patch", + ].join("\n"), + }); + }); + + test("unwraps a nested JSON input string", () => { + expect(JSON.parse(sanitizeEmittedApplyPatchArgs(JSON.stringify({ + input: JSON.stringify({ + input: ["*** Begin Patch", "*** Update File: a.txt", "@@", "-old", "+new", "*** End Patch"].join("\n"), + }), + })))).toEqual({ + input: [ + "*** Begin Patch", + "*** Update File: a.txt", + "@@", + "-old", + "+new", + "*** End Patch", + ].join("\n"), + }); + }); + + test("rejects overlapping multi_edit when the shorter old_string comes first", () => { + expect(translateStructuredEditCall(CURSOR_MULTI_EDIT_TOOL, JSON.stringify({ + file_path: "a.txt", + edits: [ + { old_string: "line2", new_string: "x" }, + { old_string: "line1\nline2", new_string: "LINE1\nLINE2" }, + ], + }))?.error).toContain("overlap"); + }); + + test("sanitizeEmittedApplyPatchArgs rewrites a JSON git new-file payload", () => { + const raw = JSON.stringify({ + input: [ + "diff --git a/hello.txt b/hello.txt", + "new file mode 100644", + "--- /dev/null", + "+++ b/hello.txt", + "@@ -0,0 +1 @@", + "+hello world", + ].join("\n"), + }); + expect(JSON.parse(sanitizeEmittedApplyPatchArgs(raw))).toEqual({ + input: [ + "*** Begin Patch", + "*** Add File: hello.txt", + "+hello world", + "*** End Patch", + ].join("\n"), + }); + }); + test("rejects malformed structured edit calls instead of relaying invalid patch text", () => { expect(translateStructuredEditCall(CURSOR_EDIT_FILE_TOOL, "not json")?.error).toBeTruthy(); expect(translateStructuredEditCall(CURSOR_EDIT_FILE_TOOL, JSON.stringify({ file_path: "src/f.ts" }))?.error).toBeTruthy(); expect(translateStructuredEditCall(CURSOR_EDIT_FILE_TOOL, JSON.stringify({ file_path: "", old_string: "a", new_string: "b" }))?.error).toBeTruthy(); - expect(translateStructuredEditCall(CURSOR_EDIT_FILE_TOOL, JSON.stringify({ file_path: "src/f.ts", old_string: "", new_string: "b" }))?.error).toBeTruthy(); + expect(translateStructuredEditCall(CURSOR_EDIT_FILE_TOOL, JSON.stringify({ file_path: "src/f.ts", old_string: "", new_string: "" }))?.error).toBeTruthy(); + expect(translateStructuredEditCall(CURSOR_MULTI_EDIT_TOOL, JSON.stringify({ + file_path: "src/f.ts", + edits: [{ old_string: "", new_string: "b" }], + }))).toEqual(expect.objectContaining({ patch: expect.stringContaining("*** Add File: src/f.ts") })); expect(translateStructuredEditCall(CURSOR_MULTI_EDIT_TOOL, JSON.stringify({ file_path: "src/f.ts", edits: [] }))?.error).toBeTruthy(); expect(translateStructuredEditCall(CURSOR_MULTI_EDIT_TOOL, JSON.stringify({ file_path: "src/f.ts", edits: [{ old_string: "a" }] }))?.error).toBeTruthy(); expect(translateStructuredEditCall("exec_command", JSON.stringify({ cmd: "echo hi" }))).toBeUndefined(); @@ -308,7 +1332,62 @@ describe("cursor protobuf event translation", () => { ]); }); - test("drops a malformed structured edit call with a clear error instead of relaying it", () => { + test("sanitizes a freeform apply_patch git-style header before Codex sees it (#1388 L3)", () => { + const state = createCursorProtobufEventState({ + clientToolNames: ["apply_patch"], + toolSchemas: new Map([["apply_patch", { type: "object", properties: { input: { type: "string" } } }]]), + cursorToolNameMap: new Map([["apply_patch", "apply_patch"]]), + }); + const toolCall = mcpToolCall("apply_patch", { + input: [ + "*** Begin Patch", + "*** Update File: git.nix", + "@@ -3,7 +3,7 @@", + '- editor = "nvim";', + '+ editor = "hx";', + "*** End Patch", + ].join("\n"), + }); + const events = mapCursorProtobufServerMessage(interaction({ + case: "toolCallCompleted", + value: create(ToolCallCompletedUpdateSchema, { callId: "call_p", modelCallId: "model_p", toolCall }), + }), state); + expect(events[0]).toEqual({ type: "tool_call_start", id: "call_p", name: "apply_patch" }); + expect(events[1]).toEqual({ + type: "tool_call_delta", + arguments: expect.stringContaining("\\n@@\\n"), + }); + expect(JSON.stringify(events)).not.toContain("@@ -3,7 +3,7 @@"); + }); + + test("emits empty-old_string edit_file as apply_patch Add File", () => { + const state = createCursorProtobufEventState({ + clientToolNames: [CURSOR_EDIT_FILE_TOOL, "apply_patch"], + syntheticStructuredEditToolNames: [CURSOR_EDIT_FILE_TOOL], + toolSchemas: new Map([[CURSOR_EDIT_FILE_TOOL, CURSOR_EDIT_FILE_INPUT_SCHEMA]]), + cursorToolNameMap: new Map([[CURSOR_EDIT_FILE_TOOL, CURSOR_EDIT_FILE_TOOL]]), + }); + const toolCall = mcpToolCall(CURSOR_EDIT_FILE_TOOL, { + file_path: "hello.txt", + old_string: "", + new_string: "hello world\n", + }); + expect(mapCursorProtobufServerMessage(interaction({ + case: "toolCallCompleted", + value: create(ToolCallCompletedUpdateSchema, { callId: "call_add", modelCallId: "model_add", toolCall }), + }), state)).toEqual([ + { type: "tool_call_start", id: "call_add", name: "apply_patch" }, + { + type: "tool_call_delta", + arguments: JSON.stringify({ + input: ["*** Begin Patch", "*** Add File: hello.txt", "+hello world", "*** End Patch"].join("\n"), + }), + }, + { type: "tool_call_end", id: "call_add" }, + ]); + }); + + test("surfaces a malformed structured edit as recoverable text instead of failing the turn (#1388 L6/L7)", () => { const state = createCursorProtobufEventState({ clientToolNames: [CURSOR_EDIT_FILE_TOOL], syntheticStructuredEditToolNames: [CURSOR_EDIT_FILE_TOOL], @@ -320,7 +1399,7 @@ describe("cursor protobuf event translation", () => { case: "toolCallCompleted", value: create(ToolCallCompletedUpdateSchema, { callId: "call_1", modelCallId: "model_1", toolCall }), }), state))?.toEqual([ - { type: "error", message: expect.stringContaining("was not converted to apply_patch") }, + { type: "text", text: expect.stringContaining("was not converted to apply_patch") }, ]); }); diff --git a/tests/cursor-tool-definitions.test.ts b/tests/cursor-tool-definitions.test.ts index d418b304b2..3b90b9e2ea 100644 --- a/tests/cursor-tool-definitions.test.ts +++ b/tests/cursor-tool-definitions.test.ts @@ -330,6 +330,7 @@ describe("Cursor tool definitions", () => { if (!note) throw new Error("Expected Cursor tool guidance note"); expect(note).toContain("Windows PowerShell 5.1"); + expect(note).toContain("never emit Get-Content or Get-ChildItem unless the host shell is PowerShell"); expect(note).toContain("cd /d"); expect(note).toContain("<