From 362f1b9fb8b71163ba2e021db09e57ba546c1666 Mon Sep 17 00:00:00 2001 From: Agent59353 Date: Wed, 5 Aug 2026 08:50:09 +0800 Subject: [PATCH 1/7] fix(cursor): expose structured edit tools that convert to valid apply_patch calls (#1017) Co-authored-by: NexusCore <22769595+ZachDreamZ@users.noreply.github.com> --- src/adapters/cursor/native-exec-fs.ts | 2 +- src/adapters/cursor/protobuf-events.ts | 134 +++++++++- src/adapters/cursor/request-builder.ts | 15 +- src/adapters/cursor/tool-definitions.ts | 93 ++++++- tests/cursor-structured-edit.test.ts | 310 ++++++++++++++++++++++++ 5 files changed, 545 insertions(+), 9 deletions(-) create mode 100644 tests/cursor-structured-edit.test.ts diff --git a/src/adapters/cursor/native-exec-fs.ts b/src/adapters/cursor/native-exec-fs.ts index e9c22a2c4e..bd5f29f6fc 100644 --- a/src/adapters/cursor/native-exec-fs.ts +++ b/src/adapters/cursor/native-exec-fs.ts @@ -40,7 +40,7 @@ const MAX_GREP_RESULTS = 200; const MAX_FILE_BYTES = 1_000_000; function codexNativeMutationRefusal(operation: "write" | "delete"): string { - return `Cursor-native ${operation} is disabled for this Codex request because apply_patch is available. Use the apply_patch tool for file edits so Codex can approve the change, enforce sandbox policy, show diffs, and record rollout. No file was changed.`; + return `Cursor-native ${operation} is disabled for this Codex request because apply_patch is available. Use the structured edit tools (\`edit_file\` / \`multi_edit\`) or the \`apply_patch\` tool for file edits so Codex can approve the change, enforce sandbox policy, show diffs, and record rollout. No file was changed.`; } const NATIVE_LOCAL_EXEC_DISABLED = diff --git a/src/adapters/cursor/protobuf-events.ts b/src/adapters/cursor/protobuf-events.ts index 2f9531cf7f..0c556a4cc1 100644 --- a/src/adapters/cursor/protobuf-events.ts +++ b/src/adapters/cursor/protobuf-events.ts @@ -3,10 +3,13 @@ import type { AgentServerMessage, McpArgs, ToolCall } from "./gen/agent_pb"; import { decodeCursorArgsMap } from "./arg-codec"; import { normalizeArgKeys } from "./arg-normalize"; import { + CODEX_APPLY_PATCH_TOOL, + CURSOR_MULTI_EDIT_TOOL, cursorShellBridgeArgsValid, cursorShellBridgeDropError, defaultShellBridgeArgNormalizeSchema, isCodexShellBridgeToolName, + isCursorStructuredEditToolName, normalizeCursorWireName, OCX_RESPONSES_TOOL_PROVIDER, resolveShellBridgeAliasKey, @@ -311,6 +314,106 @@ function resolveCompletedArgs(buffered: string, args: McpArgs | undefined, state return ""; } +const PATCH_BEGIN = "*** Begin Patch"; +const PATCH_END = "*** End Patch"; + +function firstStringArg(args: Record, keys: readonly string[]): string | undefined { + for (const key of keys) { + const value = args[key]; + if (typeof value === "string") return value; + } + 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"); + if (lines.length > 0 && lines[lines.length - 1] === "") lines.pop(); + return lines; +} + +/** One `@@` hunk replacing `oldString` with `newString`. */ +function replacementHunk(oldString: string, newString: string): { hunk: string } | { error: string } { + if (oldString.length === 0) { + return { + error: + "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 removed = patchLines(oldString).map(line => `-${line}`); + const added = patchLines(newString).map(line => `+${line}`); + return { hunk: ["@@", ...removed, ...added].join("\n") }; +} + +/** + * Convert a completed Cursor structured edit call (`edit_file` / `multi_edit`) into a valid Codex + * apply_patch freeform payload (#1017). Cursor-trained models cannot emit Codex's freeform patch + * grammar, so the adapter advertises exact-match replacement tools and performs the grammar here. + * Returns `{ patch }` for a valid conversion, `{ error }` for a malformed call (which must never be + * relayed verbatim: Codex would reject it locally after the HTTP 200, the reported failure mode), + * and `undefined` for tools that are not structured edits. + */ +export type StructuredEditTranslation = + | { patch: string; error?: undefined } + | { error: string; patch?: undefined }; + +export function translateStructuredEditCall( + toolName: string, + argsText: string, +): StructuredEditTranslation | undefined { + if (!isCursorStructuredEditToolName(toolName)) return undefined; + let parsed: unknown; + try { + parsed = JSON.parse(argsText); + } catch { + return { + error: `${toolName} arguments were not valid JSON; the call was dropped. ${ + toolName === CURSOR_MULTI_EDIT_TOOL + ? "Use file_path and edits[] (each edit with old_string and new_string)." + : "Use file_path, old_string and new_string." + }`, + }; + } + if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) { + 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) { + return { error: `${toolName} is missing a non-empty file_path; the call was dropped.` }; + } + 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"]); + 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 }; + }; + if (toolName === CURSOR_MULTI_EDIT_TOOL) { + const edits = args.edits; + if (!Array.isArray(edits) || edits.length === 0) { + return { error: "multi_edit requires a non-empty edits array; the call was dropped." }; + } + 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); + } + } else { + const editResult = addReplacement(args); + if (editResult.error !== undefined) return editResult; + hunks.push(editResult.patch); + } + return { patch: [PATCH_BEGIN, `*** Update File: ${path}`, ...hunks, PATCH_END].join("\n") }; +} + export function mapSyntheticMcpExecToToolEvents( args: McpArgs, fallbackCallId = "cursor_mcp_exec", @@ -340,10 +443,18 @@ export function mapSyntheticMcpExecToToolEvents( return [{ type: "error", message: cursorShellBridgeDropError(responsesName) }]; } } + const translation = translateStructuredEditCall(responsesName, normalizedArgs); + if (translation?.error !== undefined) { + return [{ type: "error", message: `${responsesName} call was not converted to apply_patch: ${translation.error}` }]; + } + const emittedName = translation ? CODEX_APPLY_PATCH_TOOL : responsesName; + const emittedArgs = translation ? JSON.stringify({ input: translation.patch }) : normalizedArgs; // Stateless fallback (no shared event state): emit a complete, self-contained tool call. return [ - { type: "tool_call_start", id: callId, name: responsesName }, - ...(normalizedArgs.length > 2 ? [{ type: "tool_call_delta" as const, arguments: normalizedArgs }] : []), + { type: "tool_call_start", id: callId, name: emittedName }, + ...(emittedArgs.length > 2 + ? [{ type: "tool_call_delta" as const, arguments: emittedArgs }] + : []), { type: "tool_call_end", id: callId }, ]; } @@ -385,6 +496,13 @@ function dropShellBridgeCall(state: CursorProtobufEventState, callId: string, to return [{ type: "error", message: cursorShellBridgeDropError(toolName) }]; } +function dropStructuredEditCall(state: CursorProtobufEventState, callId: string, toolName: string, reason: string): CursorServerMessage[] { + 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}` }]; +} + function commitToolCall(state: CursorProtobufEventState, callId: string, finalArgs: string): CursorServerMessage[] { const open = state.openToolCalls.get(callId); if (!open) return []; @@ -392,6 +510,12 @@ function commitToolCall(state: CursorProtobufEventState, callId: string, finalAr if (!cursorShellBridgeArgsValid(finalArgs, open.name, schema)) { if (isCodexShellBridgeToolName(open.name)) return dropShellBridgeCall(state, callId, open.name); } + // Structured edit calls are converted to apply_patch here so both the interactionUpdate and the + // native-exec mcpArgs paths emit the same valid freeform payload (#1017). + const translation = translateStructuredEditCall(open.name, finalArgs); + if (translation?.error !== undefined) { + return dropStructuredEditCall(state, callId, open.name, translation.error); + } if (finalArgs !== open.args) { const previousBytes = Buffer.byteLength(open.args); const reservation = state.translatorBudget?.reserveTransient( @@ -402,8 +526,10 @@ function commitToolCall(state: CursorProtobufEventState, callId: string, finalAr reservation?.commitRetained(); state.translatorBudget?.releaseRetained(previousBytes, { kind: "tool_args", callId }); } - const out: CursorServerMessage[] = [{ type: "tool_call_start", id: callId, name: open.name }]; - if (finalArgs.length > 0) out.push({ type: "tool_call_delta", arguments: finalArgs }); + const emittedName = translation ? CODEX_APPLY_PATCH_TOOL : open.name; + const emittedArgs = translation ? JSON.stringify({ input: translation.patch }) : finalArgs; + 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)); return out; } diff --git a/src/adapters/cursor/request-builder.ts b/src/adapters/cursor/request-builder.ts index e550c6910c..a28a32acb6 100644 --- a/src/adapters/cursor/request-builder.ts +++ b/src/adapters/cursor/request-builder.ts @@ -16,8 +16,10 @@ import { cursorMcpToolsEncodedSize, cursorToolAllowedByChoice, cursorToolChoiceAliases, + cursorStructuredEditTools, cursorToolWireName, cursorToolsForActivePrompt, + isCursorStructuredEditToolName, isBareCodexShellBridgeTool, } from "./tool-definitions"; import { lookupCursorThreadConversation } from "./thread-continuity"; @@ -41,6 +43,9 @@ function toolPriority(tool: OcxTool, selectedNames: ReadonlySet): number // selected filler cannot starve the Codex execution path during truncation (#399). if (isBareCodexShellBridgeTool(tool)) return 0; if (!tool.namespace && tool.name === "apply_patch") return 1; + // Structured edit tools convert to apply_patch on the return path, so they must survive the + // same byte/count truncation as the freeform tool they stand in for (#1017). + if (!tool.namespace && isCursorStructuredEditToolName(tool.name)) return 1; if (cursorToolChoiceAliases(tool).some(name => selectedNames.has(name))) return 2; if (tool.loadedFromToolSearch) return 3; if (!tool.namespace) return 4; @@ -61,7 +66,11 @@ export function applyCursorToolBudget( toolChoice: OcxToolChoice | undefined, ): CursorToolBudgetResult { const catalog = tools ?? []; - const eligible = catalog.filter(tool => cursorToolAllowedByChoice(tool, toolChoice, catalog)); + const baseEligible = catalog.filter(tool => cursorToolAllowedByChoice(tool, toolChoice, catalog)); + // Synthetic structured edit tools ride along with the freeform apply_patch tool (#1017). They are + // part of the advertised catalog, so their serialized size counts toward the byte ceiling here. + const synthetic = cursorStructuredEditTools(catalog, toolChoice); + const eligible = [...baseEligible, ...synthetic]; if ( eligible.length <= CURSOR_TOOL_COUNT_LIMIT && cursorMcpToolsEncodedSize(eligible, toolChoice) <= CURSOR_TOOL_BYTES_LIMIT @@ -101,7 +110,9 @@ export function applyCursorToolBudget( return { tools: eligible.filter(tool => keptSet.has(tool)), - omitted: eligible.filter(tool => !keptSet.has(tool)), + // Synthetic tools are pinned in phase 1 and never reported as omitted; the note counts only + // tools the client itself requested. + omitted: baseEligible.filter(tool => !keptSet.has(tool)), }; } diff --git a/src/adapters/cursor/tool-definitions.ts b/src/adapters/cursor/tool-definitions.ts index 7688f209a1..9a9a3de8cc 100644 --- a/src/adapters/cursor/tool-definitions.ts +++ b/src/adapters/cursor/tool-definitions.ts @@ -8,6 +8,9 @@ export const OCX_RESPONSES_TOOL_PROVIDER = "opencodex-responses"; export const CODEX_EXEC_COMMAND_TOOL = "exec_command"; export const CODEX_SHELL_COMMAND_TOOL = "shell_command"; export const CODEX_APPLY_PATCH_TOOL = "apply_patch"; +export const CURSOR_EDIT_FILE_TOOL = "edit_file"; +export const CURSOR_MULTI_EDIT_TOOL = "multi_edit"; +export const CURSOR_STRUCTURED_EDIT_TOOLS = [CURSOR_EDIT_FILE_TOOL, CURSOR_MULTI_EDIT_TOOL] as const; export const CURSOR_EXEC_COMMAND_TOOL = CODEX_EXEC_COMMAND_TOOL; export const CODEX_SHELL_BRIDGE_TOOL_NAMES = [CODEX_EXEC_COMMAND_TOOL, CODEX_SHELL_COMMAND_TOOL] as const; export const CURSOR_SHELL_ALIAS_SYSTEM_NOTE = @@ -41,6 +44,47 @@ export const CURSOR_EXEC_COMMAND_INPUT_SCHEMA = { additionalProperties: false, } as const; +/** + * Structured single-replacement schema advertised to Cursor models in addition to the freeform + * `apply_patch` tool. Cursor-trained models reliably emit exact-match replacements (the native + * Edit shape) but cannot produce Codex's freeform patch grammar, so every file edit attempt on the + * Cursor route produced malformed `apply_patch` payloads that the Codex client rejected locally + * (#1017). Calls to this tool are converted server-side into a valid apply_patch payload. + */ +export const CURSOR_EDIT_FILE_INPUT_SCHEMA = { + type: "object", + properties: { + file_path: { type: "string", description: "Path of the file to edit, relative to the workspace root." }, + old_string: { type: "string", description: "Exact text to replace. Must match the current file content, including line breaks." }, + new_string: { type: "string", description: "Replacement text. Empty removes the matched text." }, + }, + required: ["file_path", "old_string", "new_string"], + additionalProperties: false, +} as const; + +/** Structured multi-replacement schema; mirrors Cursor's native MultiEdit shape. */ +export const CURSOR_MULTI_EDIT_INPUT_SCHEMA = { + type: "object", + properties: { + file_path: { type: "string", description: "Path of the file to edit, relative to the workspace root." }, + edits: { + type: "array", + items: { + type: "object", + properties: { + old_string: { type: "string", description: "Exact text to replace. Must match the current file content, including line breaks." }, + new_string: { type: "string", description: "Replacement text. Empty removes the matched text." }, + }, + required: ["old_string", "new_string"], + additionalProperties: false, + }, + description: "Ordered replacement edits for this file. Each old_string must match the current file content.", + }, + }, + required: ["file_path", "edits"], + additionalProperties: false, +} as const; + /** * Responses/Codex-side schema used ONLY for arg-key normalization after Cursor returns a call. * Cursor models are trained to emit `cmd`; Codex `shell_command` / `exec_command` validate @@ -140,6 +184,46 @@ export function cursorRequestAdvertisesApplyPatch( return catalog.some(tool => !tool.namespace && tool.name === CODEX_APPLY_PATCH_TOOL && tool.freeform === true && cursorToolAllowedByChoice(tool, toolChoice, catalog)); } +export function isCursorStructuredEditToolName(name: string): boolean { + return (CURSOR_STRUCTURED_EDIT_TOOLS as readonly string[]).includes(name); +} + +/** + * Synthetic structured edit tools for the Cursor route (#1017). + * + * Codex exposes `apply_patch` as a freeform custom tool whose body must be the exact Codex patch + * grammar (`*** Begin Patch` envelope, `@@` hunks, `-`/`+` prefixes). Cursor-trained models are + * trained on exact-match edit tools instead and emit malformed patch text on every attempt, which + * the Codex client then rejects locally ("invalid hunk"). When the request advertises the freeform + * `apply_patch` tool, also advertise Cursor-native-shaped `edit_file` / `multi_edit` tools; the + * adapter converts their exact-match replacements into a valid apply_patch payload (see + * protobuf-events.translateStructuredEditCall). + * + * Never widened when the caller pinned an explicit tool choice: a forced `apply_patch` selection + * must not gain sibling tools the client did not ask for. + */ +export function cursorStructuredEditTools( + tools: readonly Pick[] | undefined, + toolChoice?: OcxRequestOptions["toolChoice"], +): OcxTool[] { + if (!cursorRequestAdvertisesApplyPatch(tools, toolChoice)) return []; + if (toolChoice && toolChoice !== "auto" && toolChoice !== "required") return []; + return [ + { + name: CURSOR_EDIT_FILE_TOOL, + 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.", + parameters: { ...CURSOR_EDIT_FILE_INPUT_SCHEMA }, + }, + { + name: CURSOR_MULTI_EDIT_TOOL, + description: + "Apply several ordered 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.", + parameters: { ...CURSOR_MULTI_EDIT_INPUT_SCHEMA }, + }, + ]; +} + export function cursorToolWireName(tool: Pick): string { return namespacedToolName(tool.namespace, tool.name); } @@ -415,6 +499,9 @@ export function buildCursorToolGuidanceSystemNote( const hasBareExec = shellBridgeNames.length > 0; const shellBridgeLabel = quotedNames(shellBridgeNames.length > 0 ? shellBridgeNames : [...CODEX_SHELL_BRIDGE_TOOL_NAMES]); const hasApplyPatch = cursorRequestAdvertisesApplyPatch(tools, toolChoice); + const structuredEditNames = tools + ?.filter(tool => !tool.namespace && isCursorStructuredEditToolName(tool.name)) + .map(tool => tool.name) ?? []; const discoveryTools = discoveryToolLabel(wireNames); const unavailableNeighborNames = unavailableNeighborAgentToolNames(wireNames); // Host-shell-neutral: the Codex client executes bridge commands, and may differ from @@ -443,7 +530,9 @@ export function buildCursorToolGuidanceSystemNote( ? `For file read/search/listing, use ${shellBridgeLabel} when no more specific listed tool is available.` : undefined, hasApplyPatch - ? "For file edits, use the `apply_patch` tool, not built-in file write/delete tools." + ? structuredEditNames.length > 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, use the `apply_patch` tool, not built-in file write/delete tools." : undefined, hasBareExec ? "For tool-count demos, each counted tool must be a separate Codex shell-bridge invocation/result; do not collapse several requested tools into one chained shell command." @@ -457,7 +546,7 @@ export function buildCursorToolGuidanceSystemNote( : undefined, "Do not count or report a tool call unless a tool result was actually returned.", hasBareExec - ? `If a Cursor-native file read, directory listing, grep, or shell operation is rejected by the runtime, silently use ${shellBridgeLabel} with an equivalent host-shell-safe command (POSIX: \`cat\`/\`ls\`/\`rg\`; Windows PowerShell: \`Get-Content\`/\`Get-ChildItem\`/\`Select-String\`). Do not tell the user access is blocked. For file edits, use \`apply_patch\` when available.` + ? `If a Cursor-native file read, directory listing, grep, or shell operation is rejected by the runtime, silently use ${shellBridgeLabel} with an equivalent host-shell-safe command (POSIX: \`cat\`/\`ls\`/\`rg\`; Windows PowerShell: \`Get-Content\`/\`Get-ChildItem\`/\`Select-String\`). Do not tell the user access is blocked. For file edits, use ${structuredEditNames.length > 0 ? `the structured edit tools (${quotedNames(structuredEditNames)}) or ` : ""}\`apply_patch\` when available.` : undefined, ].filter((note): note is string => typeof note === "string"); return notes.join(" "); diff --git a/tests/cursor-structured-edit.test.ts b/tests/cursor-structured-edit.test.ts new file mode 100644 index 0000000000..473e0b29c9 --- /dev/null +++ b/tests/cursor-structured-edit.test.ts @@ -0,0 +1,310 @@ +import { create } from "@bufbuild/protobuf"; +import { describe, expect, test } from "bun:test"; +import type { OcxTool } from "../src/types"; +import { + AgentServerMessageSchema, + InteractionUpdateSchema, + McpArgsSchema, + McpToolCallSchema, + PartialToolCallUpdateSchema, + ToolCallCompletedUpdateSchema, + ToolCallSchema, + ToolCallStartedUpdateSchema, +} from "../src/adapters/cursor/gen/agent_pb"; +import { + createCursorProtobufEventState, + mapCursorProtobufServerMessage, + mapSyntheticMcpExecToToolEvents, + translateStructuredEditCall, +} from "../src/adapters/cursor/protobuf-events"; +import { + applyCursorToolBudget, +} from "../src/adapters/cursor/request-builder"; +import { + buildCursorToolGuidanceSystemNote, + CURSOR_EDIT_FILE_INPUT_SCHEMA, + CURSOR_EDIT_FILE_TOOL, + CURSOR_MULTI_EDIT_INPUT_SCHEMA, + CURSOR_MULTI_EDIT_TOOL, + cursorStructuredEditTools, +} from "../src/adapters/cursor/tool-definitions"; + +const encoder = new TextEncoder(); + +function applyPatchTool(): OcxTool { + return { + name: "apply_patch", + description: "Edit files with a freeform patch.", + parameters: { + type: "object", + properties: { input: { type: "string" } }, + required: ["input"], + }, + freeform: true, + }; +} + +function execCommandTool(): OcxTool { + return { + name: "exec_command", + description: "Run a shell command.", + parameters: { + type: "object", + properties: { cmd: { type: "string" } }, + required: ["cmd"], + }, + }; +} + +function interaction(message: Parameters>[1]["message"]) { + return create(AgentServerMessageSchema, { + message: { + case: "interactionUpdate", + value: create(InteractionUpdateSchema, { message }), + }, + }); +} + +function mcpToolCall(toolName: string, args: Record) { + const encoded: Record = {}; + for (const [key, value] of Object.entries(args)) encoded[key] = encoder.encode(JSON.stringify(value)); + return create(ToolCallSchema, { + tool: { + case: "mcpToolCall", + value: create(McpToolCallSchema, { + args: create(McpArgsSchema, { + name: toolName, + toolName, + toolCallId: "call_1", + providerIdentifier: "opencodex-responses", + args: encoded, + }), + }), + }, + }); +} + +describe("cursor structured edit tools (#1017)", () => { + test("advertises edit_file and multi_edit alongside a bare freeform apply_patch", () => { + const tools = cursorStructuredEditTools([applyPatchTool()], "auto"); + expect(tools.map(tool => tool.name)).toEqual([CURSOR_EDIT_FILE_TOOL, CURSOR_MULTI_EDIT_TOOL]); + expect(tools[0]?.parameters).toEqual(CURSOR_EDIT_FILE_INPUT_SCHEMA); + expect(tools[1]?.parameters).toEqual(CURSOR_MULTI_EDIT_INPUT_SCHEMA); + }); + + test("does not widen a forced or allow-listed tool choice", () => { + const catalog = [applyPatchTool()]; + expect(cursorStructuredEditTools(catalog, { name: "apply_patch" })).toEqual([]); + expect(cursorStructuredEditTools(catalog, { allowedTools: ["apply_patch"] })).toEqual([]); + }); + + test("does not advertise structured edit tools without an advertised freeform apply_patch", () => { + expect(cursorStructuredEditTools([execCommandTool()], "auto")).toEqual([]); + expect(cursorStructuredEditTools(undefined, "auto")).toEqual([]); + // Namespaced apply_patch is a remote MCP tool, not the Codex freeform tool. + expect(cursorStructuredEditTools([{ ...applyPatchTool(), namespace: "mcp__fs" }], "auto")).toEqual([]); + }); + + test("cursor tool budget keeps the structured edit tools with apply_patch", () => { + const result = applyCursorToolBudget([applyPatchTool(), execCommandTool()], "auto"); + const names = result.tools.map(tool => tool.name); + expect(names).toContain("apply_patch"); + expect(names).toContain(CURSOR_EDIT_FILE_TOOL); + expect(names).toContain(CURSOR_MULTI_EDIT_TOOL); + expect(result.omitted).toEqual([]); + }); + + test("cursor tool budget omits structured edit tools when apply_patch is forced", () => { + const result = applyCursorToolBudget([applyPatchTool()], { name: "apply_patch" }); + expect(result.tools.map(tool => tool.name)).toEqual(["apply_patch"]); + }); + + test("guidance note tells the model to prefer the structured edit tools", () => { + const note = buildCursorToolGuidanceSystemNote([applyPatchTool(), ...cursorStructuredEditTools([applyPatchTool()])], "auto"); + expect(note).toContain("prefer the structured edit tools"); + expect(note).toContain("`edit_file`"); + expect(note).toContain("`multi_edit`"); + expect(note).toContain("never emit patch-like plain text as tool arguments"); + }); + + test("guidance note keeps the apply_patch-only guidance without structured tools", () => { + const note = buildCursorToolGuidanceSystemNote([applyPatchTool()], "auto"); + expect(note).toContain("For file edits, use the `apply_patch` tool"); + }); +}); + +describe("translateStructuredEditCall", () => { + test("converts a single edit_file replacement into a valid apply_patch payload", () => { + const args = JSON.stringify({ file_path: "src/a.ts", old_string: "old", new_string: "new" }); + expect(translateStructuredEditCall(CURSOR_EDIT_FILE_TOOL, args)).toEqual({ + patch: [ + "*** Begin Patch", + "*** Update File: src/a.ts", + "@@", + "-old", + "+new", + "*** End Patch", + ].join("\n"), + }); + }); + + test("converts multi-line replacements into one hunk with -/+ prefixed lines", () => { + const args = JSON.stringify({ + file_path: "src/b.ts", + old_string: "line1\nline2", + new_string: "line1\nchanged\nline2", + }); + expect(translateStructuredEditCall(CURSOR_EDIT_FILE_TOOL, args))?.toMatchObject({ + patch: [ + "*** Begin Patch", + "*** Update File: src/b.ts", + "@@", + "-line1", + "-line2", + "+line1", + "+changed", + "+line2", + "*** End Patch", + ].join("\n"), + }); + }); + + test("accepts Cursor-style argument aliases", () => { + const args = JSON.stringify({ path: "src/c.ts", oldtext: "a", newtext: "b" }); + expect(translateStructuredEditCall(CURSOR_EDIT_FILE_TOOL, args))?.toMatchObject({ patch: expect.stringContaining("*** Update File: src/c.ts") }); + const camel = JSON.stringify({ filePath: "src/c.ts", oldString: "a", newString: "b" }); + expect(translateStructuredEditCall(CURSOR_EDIT_FILE_TOOL, camel))?.toMatchObject({ patch: expect.stringContaining("*** Update File: src/c.ts") }); + }); + + test("converts an empty new_string into a deletion hunk", () => { + const args = JSON.stringify({ file_path: "src/d.ts", old_string: "dead", new_string: "" }); + expect(translateStructuredEditCall(CURSOR_EDIT_FILE_TOOL, args))?.toMatchObject({ + patch: [ + "*** Begin Patch", + "*** Update File: src/d.ts", + "@@", + "-dead", + "*** End Patch", + ].join("\n"), + }); + }); + + test("converts multi_edit into one apply_patch payload with one hunk per edit", () => { + const args = JSON.stringify({ + file_path: "src/e.ts", + edits: [ + { old_string: "a", new_string: "b" }, + { old_string: "c", new_string: "d" }, + ], + }); + expect(translateStructuredEditCall(CURSOR_MULTI_EDIT_TOOL, args))?.toMatchObject({ + patch: [ + "*** Begin Patch", + "*** Update File: src/e.ts", + "@@", + "-a", + "+b", + "@@", + "-c", + "+d", + "*** 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_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(); + }); +}); + +describe("cursor protobuf event translation", () => { + test("emits a structured edit_file call as an apply_patch custom tool call", () => { + const state = createCursorProtobufEventState({ + clientToolNames: [CURSOR_EDIT_FILE_TOOL, "apply_patch"], + 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: "src/a.ts", old_string: "old", new_string: "new" }); + + expect(mapCursorProtobufServerMessage(interaction({ + case: "toolCallStarted", + value: create(ToolCallStartedUpdateSchema, { callId: "call_1", modelCallId: "model_1", toolCall }), + }), state)).toEqual([]); + expect(mapCursorProtobufServerMessage(interaction({ + case: "partialToolCall", + value: create(PartialToolCallUpdateSchema, { callId: "call_1", modelCallId: "model_1", toolCall, argsTextDelta: "{\"file_path\":\"src/a.ts\",\"old_string\":\"old\",\"new_string\":\"new\"}" }), + }), state)).toEqual([]); + expect(mapCursorProtobufServerMessage(interaction({ + case: "toolCallCompleted", + value: create(ToolCallCompletedUpdateSchema, { callId: "call_1", modelCallId: "model_1", toolCall }), + }), state)).toEqual([ + { type: "tool_call_start", id: "call_1", name: "apply_patch" }, + { + type: "tool_call_delta", + arguments: JSON.stringify({ + input: [ + "*** Begin Patch", + "*** Update File: src/a.ts", + "@@", + "-old", + "+new", + "*** End Patch", + ].join("\n"), + }), + }, + { type: "tool_call_end", id: "call_1" }, + ]); + }); + + test("drops a malformed structured edit call with a clear error instead of relaying it", () => { + const state = createCursorProtobufEventState({ + clientToolNames: [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: "src/a.ts" }); + expect(mapCursorProtobufServerMessage(interaction({ + 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") }, + ]); + }); + + test("stateless native-exec path converts edit_file the same way", () => { + const args = create(McpArgsSchema, { + name: CURSOR_EDIT_FILE_TOOL, + toolName: CURSOR_EDIT_FILE_TOOL, + toolCallId: "call_2", + providerIdentifier: "opencodex-responses", + args: { + file_path: encoder.encode(JSON.stringify("src/g.ts")), + old_string: encoder.encode(JSON.stringify("x")), + new_string: encoder.encode(JSON.stringify("y")), + }, + }); + expect(mapSyntheticMcpExecToToolEvents(args, "fallback")).toEqual([ + { type: "tool_call_start", id: "call_2", name: "apply_patch" }, + { + type: "tool_call_delta", + arguments: JSON.stringify({ + input: [ + "*** Begin Patch", + "*** Update File: src/g.ts", + "@@", + "-x", + "+y", + "*** End Patch", + ].join("\n"), + }), + }, + { type: "tool_call_end", id: "call_2" }, + ]); + }); +}); From 5442d5649c578fab462433397b5a7106df1ea581 Mon Sep 17 00:00:00 2001 From: Agent59353 Date: Wed, 5 Aug 2026 10:18:16 +0800 Subject: [PATCH 2/7] fix(cursor): never shadow an existing bare edit tool with the synthetic structured tools Co-authored-by: NexusCore <22769595+ZachDreamZ@users.noreply.github.com> --- src/adapters/cursor/tool-definitions.ts | 8 +++++++- tests/cursor-structured-edit.test.ts | 6 ++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/adapters/cursor/tool-definitions.ts b/src/adapters/cursor/tool-definitions.ts index 9a9a3de8cc..b124942c0b 100644 --- a/src/adapters/cursor/tool-definitions.ts +++ b/src/adapters/cursor/tool-definitions.ts @@ -208,7 +208,12 @@ export function cursorStructuredEditTools( ): OcxTool[] { if (!cursorRequestAdvertisesApplyPatch(tools, toolChoice)) return []; if (toolChoice && toolChoice !== "auto" && toolChoice !== "required") return []; - return [ + // Never shadow an already-advertised bare tool with the same name (a client catalog could + // legitimately expose its own `edit_file` / `multi_edit` MCP-style tools). + const existingBareNames = new Set( + (tools ?? []).filter(tool => !tool.namespace).map(tool => tool.name), + ); + const candidates: OcxTool[] = [ { name: CURSOR_EDIT_FILE_TOOL, description: @@ -222,6 +227,7 @@ export function cursorStructuredEditTools( parameters: { ...CURSOR_MULTI_EDIT_INPUT_SCHEMA }, }, ]; + return candidates.filter(tool => !existingBareNames.has(tool.name)); } export function cursorToolWireName(tool: Pick): string { diff --git a/tests/cursor-structured-edit.test.ts b/tests/cursor-structured-edit.test.ts index 473e0b29c9..05c65409b3 100644 --- a/tests/cursor-structured-edit.test.ts +++ b/tests/cursor-structured-edit.test.ts @@ -105,6 +105,12 @@ describe("cursor structured edit tools (#1017)", () => { expect(cursorStructuredEditTools([{ ...applyPatchTool(), namespace: "mcp__fs" }], "auto")).toEqual([]); }); + test("does not shadow a bare client tool that already uses a structured edit name", () => { + const catalog = [applyPatchTool(), { ...applyPatchTool(), name: CURSOR_EDIT_FILE_TOOL, freeform: undefined }]; + const tools = cursorStructuredEditTools(catalog, "auto"); + expect(tools.map(tool => tool.name)).toEqual([CURSOR_MULTI_EDIT_TOOL]); + }); + test("cursor tool budget keeps the structured edit tools with apply_patch", () => { const result = applyCursorToolBudget([applyPatchTool(), execCommandTool()], "auto"); const names = result.tools.map(tool => tool.name); From 50b9ad846549ec7ecae55d0ddc2bd613dcc6c6ef Mon Sep 17 00:00:00 2001 From: Agent59353 Date: Wed, 5 Aug 2026 10:37:23 +0800 Subject: [PATCH 3/7] test(cursor): cover native-exec mcpArgs structured edit translation Co-authored-by: NexusCore <22769595+ZachDreamZ@users.noreply.github.com> --- tests/cursor-structured-edit.test.ts | 48 ++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/tests/cursor-structured-edit.test.ts b/tests/cursor-structured-edit.test.ts index 05c65409b3..86733b4745 100644 --- a/tests/cursor-structured-edit.test.ts +++ b/tests/cursor-structured-edit.test.ts @@ -3,6 +3,7 @@ import { describe, expect, test } from "bun:test"; import type { OcxTool } from "../src/types"; import { AgentServerMessageSchema, + ExecServerMessageSchema, InteractionUpdateSchema, McpArgsSchema, McpToolCallSchema, @@ -17,6 +18,7 @@ import { mapSyntheticMcpExecToToolEvents, translateStructuredEditCall, } from "../src/adapters/cursor/protobuf-events"; +import { planMcpArgsHandling } from "../src/adapters/cursor/live-transport"; import { applyCursorToolBudget, } from "../src/adapters/cursor/request-builder"; @@ -313,4 +315,50 @@ describe("cursor protobuf event translation", () => { { type: "tool_call_end", id: "call_2" }, ]); }); + + test("native-exec mcpArgs path (planMcpArgsHandling) emits the translated apply_patch call", () => { + const state = createCursorProtobufEventState({ + clientToolNames: [CURSOR_EDIT_FILE_TOOL, "apply_patch"], + toolSchemas: new Map([[CURSOR_EDIT_FILE_TOOL, CURSOR_EDIT_FILE_INPUT_SCHEMA]]), + cursorToolNameMap: new Map([[CURSOR_EDIT_FILE_TOOL, CURSOR_EDIT_FILE_TOOL]]), + }); + const execMsg = create(ExecServerMessageSchema, { + id: 7, + execId: "exec_7", + message: { + case: "mcpArgs", + value: create(McpArgsSchema, { + name: CURSOR_EDIT_FILE_TOOL, + toolName: CURSOR_EDIT_FILE_TOOL, + toolCallId: "call_3", + providerIdentifier: "opencodex-responses", + args: { + file_path: encoder.encode(JSON.stringify("src/h.ts")), + old_string: encoder.encode(JSON.stringify("before")), + new_string: encoder.encode(JSON.stringify("after")), + }, + }), + }, + }); + const plan = planMcpArgsHandling(execMsg, state); + expect(plan.handledByResponsesBridge).toBe(true); + expect(plan.cancelCursorRun).toBe(false); + expect(plan.events).toEqual([ + { type: "tool_call_start", id: "call_3", name: "apply_patch" }, + { + type: "tool_call_delta", + arguments: JSON.stringify({ + input: [ + "*** Begin Patch", + "*** Update File: src/h.ts", + "@@", + "-before", + "+after", + "*** End Patch", + ].join("\n"), + }), + }, + { type: "tool_call_end", id: "call_3" }, + ]); + }); }); From d5d96a677154b68cefddb3f38049356495371152 Mon Sep 17 00:00:00 2001 From: Agent59353 Date: Wed, 5 Aug 2026 15:18:36 +0800 Subject: [PATCH 4/7] fix(cursor): address CodeRabbit + Codex review feedback on #1017 Gate the native-mutation refusal hint on whether the synthetic structured edit tools are actually advertised (not every apply_patch request widens). Reject trailing-newline-only and identical old/new replacements as a silent no-op instead of emitting an empty hunk that apply_patch would drop. Give addReplacement a single StructuredEditTranslation return type so the patch field does not overload two different meanings. Document the line-based matching limitations (single-location, edits matched against ORIGINAL content, no final-newline-only edits) in the tool descriptions. Test hardening: remove optional-chaining after expect(), use toEqual for full-shape assertions, add no-op rejection cases and a stateful non-identity wire-name multi_edit translation case. Verification: 21 focused tests pass, typecheck/lint/privacy clean. Co-authored-by: NexusCore <22769595+ZachDreamZ@users.noreply.github.com> --- src/adapters/cursor/live-transport.ts | 2 + src/adapters/cursor/native-exec-fs.ts | 15 +++--- src/adapters/cursor/native-exec.ts | 6 ++- src/adapters/cursor/protobuf-events.ts | 17 ++++-- src/adapters/cursor/tool-definitions.ts | 16 +++++- tests/cursor-structured-edit.test.ts | 69 +++++++++++++++++++++++-- 6 files changed, 107 insertions(+), 18 deletions(-) diff --git a/src/adapters/cursor/live-transport.ts b/src/adapters/cursor/live-transport.ts index eb7ebdcc60..f94650ad30 100644 --- a/src/adapters/cursor/live-transport.ts +++ b/src/adapters/cursor/live-transport.ts @@ -66,6 +66,7 @@ import { desktopDepsFromConfig } from "./native-exec-desktop"; import { buildCursorToolDefinitions, cursorRequestAdvertisesApplyPatch, + cursorRequestAdvertisesStructuredEdits, cursorRequestHasShellAlias, cursorToolArgNormalizeSchema, cursorToolWireName, @@ -544,6 +545,7 @@ class LiveCursorTransport implements CursorTransport { ...this.execContext, clientToolDefs, rejectNativeFileMutations: cursorRequestAdvertisesApplyPatch(request.tools, request.toolChoice), + structuredEditAvailable: cursorRequestAdvertisesStructuredEdits(request.tools, request.toolChoice), }; const toolSchemas = new Map(); const cursorToolNameMap = new Map(); diff --git a/src/adapters/cursor/native-exec-fs.ts b/src/adapters/cursor/native-exec-fs.ts index bd5f29f6fc..fb22826912 100644 --- a/src/adapters/cursor/native-exec-fs.ts +++ b/src/adapters/cursor/native-exec-fs.ts @@ -39,8 +39,11 @@ const MAX_GREP_FILES = 500; const MAX_GREP_RESULTS = 200; const MAX_FILE_BYTES = 1_000_000; -function codexNativeMutationRefusal(operation: "write" | "delete"): string { - return `Cursor-native ${operation} is disabled for this Codex request because apply_patch is available. Use the structured edit tools (\`edit_file\` / \`multi_edit\`) or the \`apply_patch\` tool for file edits so Codex can approve the change, enforce sandbox policy, show diffs, and record rollout. No file was changed.`; +function codexNativeMutationRefusal(operation: "write" | "delete", structuredEditAvailable: boolean): string { + const structuredHint = structuredEditAvailable + ? " Use the structured edit tools (`edit_file` / `multi_edit`) or the `apply_patch` tool for file edits so Codex can approve the change, enforce sandbox policy, show diffs, and record rollout." + : " Use the `apply_patch` tool for file edits so Codex can approve the change, enforce sandbox policy, show diffs, and record rollout."; + return `Cursor-native ${operation} is disabled for this Codex request because apply_patch is available.${structuredHint} No file was changed.`; } const NATIVE_LOCAL_EXEC_DISABLED = @@ -84,13 +87,13 @@ export function readExec(execMsg: ExecServerMessage): Uint8Array { } } -export function rejectWriteExecForApplyPatch(execMsg: ExecServerMessage): Uint8Array { +export function rejectWriteExecForApplyPatch(execMsg: ExecServerMessage, structuredEditAvailable = false): Uint8Array { if (execMsg.message.case !== "writeArgs") throw new Error("invalid write exec"); const path = resolve(execMsg.message.value.path); return execBytes(execMsg, "writeResult", create(WriteResultSchema, { result: { case: "rejected", - value: create(WriteRejectedSchema, { path, reason: codexNativeMutationRefusal("write") }), + value: create(WriteRejectedSchema, { path, reason: codexNativeMutationRefusal("write", structuredEditAvailable) }), }, })); } @@ -133,13 +136,13 @@ export function writeExec(execMsg: ExecServerMessage): Uint8Array { } } -export function rejectDeleteExecForApplyPatch(execMsg: ExecServerMessage): Uint8Array { +export function rejectDeleteExecForApplyPatch(execMsg: ExecServerMessage, structuredEditAvailable = false): Uint8Array { if (execMsg.message.case !== "deleteArgs") throw new Error("invalid delete exec"); const path = resolve(execMsg.message.value.path); return execBytes(execMsg, "deleteResult", create(DeleteResultSchema, { result: { case: "rejected", - value: create(DeleteRejectedSchema, { path, reason: codexNativeMutationRefusal("delete") }), + value: create(DeleteRejectedSchema, { path, reason: codexNativeMutationRefusal("delete", structuredEditAvailable) }), }, })); } diff --git a/src/adapters/cursor/native-exec.ts b/src/adapters/cursor/native-exec.ts index 8cfb2eae67..52856b79e1 100644 --- a/src/adapters/cursor/native-exec.ts +++ b/src/adapters/cursor/native-exec.ts @@ -70,6 +70,8 @@ export interface CursorNativeExecContext extends CursorNativeExecDeps { unsafeAllowNativeLocalExec?: boolean; /** apply_patch is visible for this request; Cursor-native write/delete must not bypass Codex. */ rejectNativeFileMutations?: boolean; + /** The synthetic exact-match edit tools (edit_file / multi_edit) are advertised this request. */ + structuredEditAvailable?: boolean; } export function cursorUnsafeNativeLocalExecEnabled(input: Pick = {}): boolean { @@ -514,8 +516,8 @@ export async function handleCursorNativeExec(execMsg: ExecServerMessage, deps: C if (execCase === "fetchArgs") return [rejectFetchExecForPolicy(execMsg)]; } if (execCase === "readArgs") return [readExec(execMsg)]; - if (execCase === "writeArgs") return [deps.rejectNativeFileMutations ? rejectWriteExecForApplyPatch(execMsg) : writeExec(execMsg)]; - if (execCase === "deleteArgs") return [deps.rejectNativeFileMutations ? rejectDeleteExecForApplyPatch(execMsg) : deleteExec(execMsg)]; + if (execCase === "writeArgs") return [deps.rejectNativeFileMutations ? rejectWriteExecForApplyPatch(execMsg, deps.structuredEditAvailable === true) : writeExec(execMsg)]; + if (execCase === "deleteArgs") return [deps.rejectNativeFileMutations ? rejectDeleteExecForApplyPatch(execMsg, deps.structuredEditAvailable === true) : deleteExec(execMsg)]; if (execCase === "lsArgs") return [lsExec(execMsg)]; if (execCase === "grepArgs") return [grepExec(execMsg)]; if (execCase === "shellArgs") return [shellExec(execMsg)]; diff --git a/src/adapters/cursor/protobuf-events.ts b/src/adapters/cursor/protobuf-events.ts index 0c556a4cc1..e7686f7c79 100644 --- a/src/adapters/cursor/protobuf-events.ts +++ b/src/adapters/cursor/protobuf-events.ts @@ -340,8 +340,19 @@ 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 removed = patchLines(oldString).map(line => `-${line}`); - const added = patchLines(newString).map(line => `+${line}`); + const oldLines = patchLines(oldString); + const newLines = patchLines(newString); + // 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. + if (oldLines.length === 0 && newLines.length === 0) { + return { error: "structured edit requires a non-empty old_string; an empty replacement is not a valid edit" }; + } + if (oldLines.length === newLines.length && oldLines.every((line, i) => line === newLines[i])) { + return { error: "structured edit old_string and new_string are identical after line normalization; the replacement is a no-op and was dropped" }; + } + const removed = oldLines.map(line => `-${line}`); + const added = newLines.map(line => `+${line}`); return { hunk: ["@@", ...removed, ...added].join("\n") }; } @@ -391,7 +402,7 @@ export function translateStructuredEditCall( } const hunk = replacementHunk(oldString, newString); if ("error" in hunk) return { error: hunk.error }; - return { patch: hunk.hunk }; + return { patch: hunk.hunk as string }; }; if (toolName === CURSOR_MULTI_EDIT_TOOL) { const edits = args.edits; diff --git a/src/adapters/cursor/tool-definitions.ts b/src/adapters/cursor/tool-definitions.ts index b124942c0b..1531165f38 100644 --- a/src/adapters/cursor/tool-definitions.ts +++ b/src/adapters/cursor/tool-definitions.ts @@ -217,19 +217,31 @@ export function cursorStructuredEditTools( { name: CURSOR_EDIT_FILE_TOOL, 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.", + "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.", parameters: { ...CURSOR_EDIT_FILE_INPUT_SCHEMA }, }, { name: CURSOR_MULTI_EDIT_TOOL, description: - "Apply several ordered 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.", + "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.", parameters: { ...CURSOR_MULTI_EDIT_INPUT_SCHEMA }, }, ]; return candidates.filter(tool => !existingBareNames.has(tool.name)); } +/** + * True when this request actually advertises the synthetic structured edit tools (`edit_file` / + * `multi_edit`) — i.e. a freeform `apply_patch` is advertised, no tool-choice pin blocks widening, + * and neither name is shadowed by an existing bare tool in the client catalog. + */ +export function cursorRequestAdvertisesStructuredEdits( + tools: readonly Pick[] | undefined, + toolChoice?: OcxRequestOptions["toolChoice"], +): boolean { + return cursorStructuredEditTools(tools, toolChoice).length > 0; +} + export function cursorToolWireName(tool: Pick): string { return namespacedToolName(tool.namespace, tool.name); } diff --git a/tests/cursor-structured-edit.test.ts b/tests/cursor-structured-edit.test.ts index 86733b4745..17ee47a766 100644 --- a/tests/cursor-structured-edit.test.ts +++ b/tests/cursor-structured-edit.test.ts @@ -162,7 +162,7 @@ describe("translateStructuredEditCall", () => { old_string: "line1\nline2", new_string: "line1\nchanged\nline2", }); - expect(translateStructuredEditCall(CURSOR_EDIT_FILE_TOOL, args))?.toMatchObject({ + expect(translateStructuredEditCall(CURSOR_EDIT_FILE_TOOL, args)).toEqual({ patch: [ "*** Begin Patch", "*** Update File: src/b.ts", @@ -179,14 +179,18 @@ describe("translateStructuredEditCall", () => { test("accepts Cursor-style argument aliases", () => { const args = JSON.stringify({ path: "src/c.ts", oldtext: "a", newtext: "b" }); - expect(translateStructuredEditCall(CURSOR_EDIT_FILE_TOOL, args))?.toMatchObject({ patch: expect.stringContaining("*** Update File: src/c.ts") }); + expect(translateStructuredEditCall(CURSOR_EDIT_FILE_TOOL, args)).toEqual( + expect.objectContaining({ patch: expect.stringContaining("*** Update File: src/c.ts") }), + ); const camel = JSON.stringify({ filePath: "src/c.ts", oldString: "a", newString: "b" }); - expect(translateStructuredEditCall(CURSOR_EDIT_FILE_TOOL, camel))?.toMatchObject({ patch: expect.stringContaining("*** Update File: src/c.ts") }); + expect(translateStructuredEditCall(CURSOR_EDIT_FILE_TOOL, camel)).toEqual( + expect.objectContaining({ patch: expect.stringContaining("*** Update File: src/c.ts") }), + ); }); test("converts an empty new_string into a deletion hunk", () => { const args = JSON.stringify({ file_path: "src/d.ts", old_string: "dead", new_string: "" }); - expect(translateStructuredEditCall(CURSOR_EDIT_FILE_TOOL, args))?.toMatchObject({ + expect(translateStructuredEditCall(CURSOR_EDIT_FILE_TOOL, args)).toEqual({ patch: [ "*** Begin Patch", "*** Update File: src/d.ts", @@ -205,7 +209,7 @@ describe("translateStructuredEditCall", () => { { old_string: "c", new_string: "d" }, ], }); - expect(translateStructuredEditCall(CURSOR_MULTI_EDIT_TOOL, args))?.toMatchObject({ + expect(translateStructuredEditCall(CURSOR_MULTI_EDIT_TOOL, args)).toEqual({ patch: [ "*** Begin Patch", "*** Update File: src/e.ts", @@ -229,6 +233,21 @@ describe("translateStructuredEditCall", () => { 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(); }); + + test("rejects a trailing-newline-only edit as a silent no-op", () => { + // old_string normalizes to the same lines as new_string; line-based patch cannot express "add a final newline". + const args = JSON.stringify({ file_path: "src/nl.ts", old_string: "export {};\n", new_string: "export {};" }); + expect(translateStructuredEditCall(CURSOR_EDIT_FILE_TOOL, args)).toEqual({ + error: "structured edit old_string and new_string are identical after line normalization; the replacement is a no-op and was dropped", + }); + }); + + test("rejects identical old/new as a no-op", () => { + const args = JSON.stringify({ file_path: "src/same.ts", old_string: "x", new_string: "x" }); + expect(translateStructuredEditCall(CURSOR_EDIT_FILE_TOOL, args)).toEqual({ + error: "structured edit old_string and new_string are identical after line normalization; the replacement is a no-op and was dropped", + }); + }); }); describe("cursor protobuf event translation", () => { @@ -361,4 +380,44 @@ describe("cursor protobuf event translation", () => { { type: "tool_call_end", id: "call_3" }, ]); }); + + test("translates a non-identity wire-name mapping (Cursor display name -> Codex tool name) for multi_edit", () => { + // Cursor advertises the Responses tool as `mcp_opencodex-responses_multi_edit`; the adapter must + // map that display name back to the advertised `multi_edit` before translating (#399 pattern). + const state = createCursorProtobufEventState({ + clientToolNames: [CURSOR_MULTI_EDIT_TOOL, "apply_patch"], + toolSchemas: new Map([[CURSOR_MULTI_EDIT_TOOL, CURSOR_MULTI_EDIT_INPUT_SCHEMA]]), + cursorToolNameMap: new Map([[CURSOR_MULTI_EDIT_TOOL, CURSOR_MULTI_EDIT_TOOL]]), + }); + const toolCall = mcpToolCall(`mcp_opencodex-responses_${CURSOR_MULTI_EDIT_TOOL}`, { + file_path: "src/multi.ts", + edits: [ + { old_string: "a", new_string: "b" }, + { old_string: "c", new_string: "d" }, + ], + }); + expect(mapCursorProtobufServerMessage(interaction({ + case: "toolCallCompleted", + value: create(ToolCallCompletedUpdateSchema, { callId: "call_4", modelCallId: "model_4", toolCall }), + }), state)).toEqual([ + { type: "tool_call_start", id: "call_4", name: "apply_patch" }, + { + type: "tool_call_delta", + arguments: JSON.stringify({ + input: [ + "*** Begin Patch", + "*** Update File: src/multi.ts", + "@@", + "-a", + "+b", + "@@", + "-c", + "+d", + "*** End Patch", + ].join("\n"), + }), + }, + { type: "tool_call_end", id: "call_4" }, + ]); + }); }); From d42722a7e288571f84c40036ebae97a35e472c9a Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Thu, 6 Aug 2026 21:19:27 +0900 Subject: [PATCH 5/7] fix(cursor): gate structured-edit conversion on provenance, not tool name (#1017) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Builds on Agent59353's structured-edit work in the commits below. The conversion itself is unchanged and is the hard part: validated JSON, several argument spellings, line-based hunks, no-op and final-newline rejection, and a drop-with-explanation instead of a best-effort patch. The gap was at the other end. `translateStructuredEditCall` decided a call was ours from the tool NAME alone, and both call sites pass a name that came off the wire. `cursorStructuredEditTools` already refuses to shadow a client tool called `edit_file` or `multi_edit` — so the collision was understood at injection — but that knowledge never reached the translation. A user running an MCP server that exposes `edit_file` would have their call silently re-emitted as `apply_patch`, or dropped with an error naming a conversion they never requested. This threads the answer through instead: live-transport records the bare names we actually advertised on this request, derived from `cursorStructuredEditTools` rather than from the name, and the event state carries them. Both call sites convert only when the name is in that set. The stateless fallback now passes through rather than converting. It has no request state, so it cannot know whether we advertised anything, and the safe direction is obvious: an unconverted structured call is a visible, recoverable failure; a wrongly converted one edits a file. Live traffic always carries state, so real conversions are unaffected. Its test previously pinned the old contract ("stateless native-exec path converts edit_file the same way"), so it now pins the new one and says why. Ablation on the new collision test — restoring the name-only gate: (fail) a client tool named edit_file is not hijacked when we advertised nothing (#1036 review) 21 pass, 1 fail Exactly one test goes red, and the twenty-one conversion tests stay green, which is what shows the gate narrows behavior without breaking the feature. tests/cursor-structured-edit.test.ts 22 pass / 0 fail; cursor-tool-budget and cursor-protobuf-events 33 pass / 0 fail; typecheck clean. Co-authored-by: NexusCore <22769595+ZachDreamZ@users.noreply.github.com> --- src/adapters/cursor/live-transport.ts | 13 ++++++ src/adapters/cursor/protobuf-events.ts | 49 ++++++++++++++++++--- tests/cursor-structured-edit.test.ts | 60 ++++++++++++++++++-------- 3 files changed, 98 insertions(+), 24 deletions(-) diff --git a/src/adapters/cursor/live-transport.ts b/src/adapters/cursor/live-transport.ts index f94650ad30..1a9cb6500d 100644 --- a/src/adapters/cursor/live-transport.ts +++ b/src/adapters/cursor/live-transport.ts @@ -67,6 +67,7 @@ import { buildCursorToolDefinitions, cursorRequestAdvertisesApplyPatch, cursorRequestAdvertisesStructuredEdits, + cursorStructuredEditTools, cursorRequestHasShellAlias, cursorToolArgNormalizeSchema, cursorToolWireName, @@ -547,6 +548,17 @@ class LiveCursorTransport implements CursorTransport { rejectNativeFileMutations: cursorRequestAdvertisesApplyPatch(request.tools, request.toolChoice), structuredEditAvailable: cursorRequestAdvertisesStructuredEdits(request.tools, request.toolChoice), }; + // Provenance for the synthetic structured-edit tools (#1036 review): record the bare names WE + // actually advertised on THIS request, taken from the definitions that were really sent rather + // than from the name alone. A client or MCP tool legitimately called `edit_file` is in + // clientToolDefs too, so the discriminator is `cursorStructuredEditTools` having produced it — + // which is precisely what `structuredEditAvailable` already reflects. + const syntheticStructuredEditToolNames = new Set( + (this.execContext.structuredEditAvailable + ? cursorStructuredEditTools(request.tools, request.toolChoice) + : [] + ).map(tool => tool.name), + ); const toolSchemas = new Map(); const cursorToolNameMap = new Map(); for (const tool of cursorVisibleTools ?? []) { @@ -573,6 +585,7 @@ class LiveCursorTransport implements CursorTransport { parallelToolCalls: request.parallelToolCalls, toolSchemas, cursorToolNameMap, + syntheticStructuredEditToolNames, translatorBudget: this.translatorBudget, contextUsage, ...(prepared.estimatedInputTokens !== undefined diff --git a/src/adapters/cursor/protobuf-events.ts b/src/adapters/cursor/protobuf-events.ts index e7686f7c79..98c8779ca7 100644 --- a/src/adapters/cursor/protobuf-events.ts +++ b/src/adapters/cursor/protobuf-events.ts @@ -164,14 +164,41 @@ export interface CursorProtobufEventState { toolSchemas?: Map; /** Cursor wire-name → original Responses/Codex tool name for this request. */ cursorToolNameMap?: Map; + /** + * Bare names WE advertised as synthetic structured-edit tools on this request. + * See structuredEditCallIsOurs: conversion is gated on provenance, not on the name. + */ + syntheticStructuredEditToolNames?: ReadonlySet; translatorBudget?: TranslatorBudget; } + +/** + * Did WE advertise this bare tool name as a synthetic structured-edit tool on this request? + * + * Provenance, not a name test. `edit_file` / `multi_edit` are ordinary names a client or MCP + * server may legitimately expose, and `cursorStructuredEditTools` already refuses to shadow one + * that exists. Converting on the name alone would undo that refusal at the other end of the + * request: the client's own call would be silently re-emitted as `apply_patch`, or dropped with + * an error naming a conversion the user never asked for. + * + * Absent set = we advertised nothing, so nothing converts. Fail-closed in the safe direction: + * an unconverted structured call is a visible, recoverable failure; a wrongly converted one + * edits a file. + */ +function structuredEditCallIsOurs( + advertised: ReadonlySet | undefined, + toolName: string, +): boolean { + return advertised?.has(toolName) === true; +} + export function createCursorProtobufEventState(options: { clientToolNames?: Iterable; parallelToolCalls?: boolean; toolSchemas?: Map; cursorToolNameMap?: Map; + syntheticStructuredEditToolNames?: Iterable; contextUsage?: CursorContextUsageControls; /** * Request-local input estimate derived from the payload actually sent. Used only @@ -188,6 +215,9 @@ export function createCursorProtobufEventState(options: { openToolCalls: new Map(), completedToolCalls: new Set(), ...(options.clientToolNames ? { clientToolNames: new Set(options.clientToolNames) } : {}), + ...(options.syntheticStructuredEditToolNames + ? { syntheticStructuredEditToolNames: new Set(options.syntheticStructuredEditToolNames) } + : {}), ...(options.parallelToolCalls !== undefined ? { parallelToolCalls: options.parallelToolCalls } : {}), startedClientToolCalls: 0, ...(options.toolSchemas ? { toolSchemas: options.toolSchemas } : {}), @@ -454,13 +484,16 @@ export function mapSyntheticMcpExecToToolEvents( return [{ type: "error", message: cursorShellBridgeDropError(responsesName) }]; } } - const translation = translateStructuredEditCall(responsesName, normalizedArgs); - if (translation?.error !== undefined) { - return [{ type: "error", message: `${responsesName} call was not converted to apply_patch: ${translation.error}` }]; - } - const emittedName = translation ? CODEX_APPLY_PATCH_TOOL : responsesName; - const emittedArgs = translation ? JSON.stringify({ input: translation.patch }) : normalizedArgs; // Stateless fallback (no shared event state): emit a complete, self-contained tool call. + // + // No conversion happens here by design (#1036 review). Structured-edit translation is gated on + // provenance — did WE advertise this bare name on THIS request — and that record lives on the + // request state, which this branch does not have. Converting anyway would reinstate the exact + // hazard the gate exists to close: a client or MCP tool legitimately named `edit_file` would be + // rewritten into an apply_patch it never asked for. The live path always carries state + // (live-transport seeds it), so this only affects direct/unit callers. + const emittedName = responsesName; + const emittedArgs = normalizedArgs; return [ { type: "tool_call_start", id: callId, name: emittedName }, ...(emittedArgs.length > 2 @@ -523,7 +556,9 @@ function commitToolCall(state: CursorProtobufEventState, callId: string, finalAr } // Structured edit calls are converted to apply_patch here so both the interactionUpdate and the // native-exec mcpArgs paths emit the same valid freeform payload (#1017). - const translation = translateStructuredEditCall(open.name, finalArgs); + const translation = structuredEditCallIsOurs(state.syntheticStructuredEditToolNames, open.name) + ? translateStructuredEditCall(open.name, finalArgs) + : undefined; if (translation?.error !== undefined) { return dropStructuredEditCall(state, callId, open.name, translation.error); } diff --git a/tests/cursor-structured-edit.test.ts b/tests/cursor-structured-edit.test.ts index 17ee47a766..088b989ae3 100644 --- a/tests/cursor-structured-edit.test.ts +++ b/tests/cursor-structured-edit.test.ts @@ -254,6 +254,8 @@ describe("cursor protobuf event translation", () => { test("emits a structured edit_file call as an apply_patch custom tool call", () => { const state = createCursorProtobufEventState({ clientToolNames: [CURSOR_EDIT_FILE_TOOL, "apply_patch"], + // We advertised the synthetic edit tool on this request, so conversion is ours to do. + 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]]), }); @@ -292,6 +294,7 @@ describe("cursor protobuf event translation", () => { test("drops a malformed structured edit call with a clear error instead of relaying it", () => { const state = createCursorProtobufEventState({ clientToolNames: [CURSOR_EDIT_FILE_TOOL], + 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]]), }); @@ -304,7 +307,7 @@ describe("cursor protobuf event translation", () => { ]); }); - test("stateless native-exec path converts edit_file the same way", () => { + test("stateless native-exec path passes edit_file through untranslated (no provenance)", () => { const args = create(McpArgsSchema, { name: CURSOR_EDIT_FILE_TOOL, toolName: CURSOR_EDIT_FILE_TOOL, @@ -316,28 +319,50 @@ describe("cursor protobuf event translation", () => { new_string: encoder.encode(JSON.stringify("y")), }, }); - expect(mapSyntheticMcpExecToToolEvents(args, "fallback")).toEqual([ - { type: "tool_call_start", id: "call_2", name: "apply_patch" }, - { - type: "tool_call_delta", - arguments: JSON.stringify({ - input: [ - "*** Begin Patch", - "*** Update File: src/g.ts", - "@@", - "-x", - "+y", - "*** End Patch", - ].join("\n"), - }), + // The stateless branch carries no request state, so it has no record of whether WE + // advertised `edit_file` on this request. Converting on the name alone would rewrite a + // client or MCP tool of the same name into an apply_patch it never asked for (#1036 + // review), so this path relays the call untouched. The live transport always seeds + // state, so real traffic still converts — see the stateful tests above. + const events = mapSyntheticMcpExecToToolEvents(args, "fallback"); + expect(events[0]).toEqual({ type: "tool_call_start", id: "call_2", name: CURSOR_EDIT_FILE_TOOL }); + expect(JSON.stringify(events)).not.toContain("*** Begin Patch"); + expect(events.at(-1)).toEqual({ type: "tool_call_end", id: "call_2" }); + }); + + test("a client tool named edit_file is not hijacked when we advertised nothing (#1036 review)", () => { + // The collision the name-only gate allowed: an MCP server exposing `edit_file`. State exists + // (so this is the live shape), but syntheticStructuredEditToolNames is absent because we + // advertised no synthetic tools on this request. + const state = createCursorProtobufEventState({ + clientToolNames: [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 args = create(McpArgsSchema, { + name: CURSOR_EDIT_FILE_TOOL, + toolName: CURSOR_EDIT_FILE_TOOL, + toolCallId: "call_collision", + providerIdentifier: "opencodex-responses", + args: { + file_path: encoder.encode(JSON.stringify("src/client-owned.ts")), + old_string: encoder.encode(JSON.stringify("x")), + new_string: encoder.encode(JSON.stringify("y")), }, - { type: "tool_call_end", id: "call_2" }, - ]); + }); + + const events = mapSyntheticMcpExecToToolEvents(args, "call_collision", { state }); + + expect(JSON.stringify(events)).not.toContain("*** Begin Patch"); + expect(JSON.stringify(events)).not.toContain("was not converted to apply_patch"); + expect(JSON.stringify(events)).toContain(CURSOR_EDIT_FILE_TOOL); }); test("native-exec mcpArgs path (planMcpArgsHandling) emits the translated apply_patch call", () => { const state = createCursorProtobufEventState({ clientToolNames: [CURSOR_EDIT_FILE_TOOL, "apply_patch"], + // We advertised the synthetic edit tool on this request, so conversion is ours to do. + 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]]), }); @@ -386,6 +411,7 @@ describe("cursor protobuf event translation", () => { // map that display name back to the advertised `multi_edit` before translating (#399 pattern). const state = createCursorProtobufEventState({ clientToolNames: [CURSOR_MULTI_EDIT_TOOL, "apply_patch"], + syntheticStructuredEditToolNames: [CURSOR_MULTI_EDIT_TOOL], toolSchemas: new Map([[CURSOR_MULTI_EDIT_TOOL, CURSOR_MULTI_EDIT_INPUT_SCHEMA]]), cursorToolNameMap: new Map([[CURSOR_MULTI_EDIT_TOOL, CURSOR_MULTI_EDIT_TOOL]]), }); From ca5665d529d3245a317f3e6cb6c5249de0ba8cea Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Fri, 7 Aug 2026 00:33:03 +0900 Subject: [PATCH 6/7] fix(cursor): derive structured edits from final catalog Adapt PR #1036 for issue #1017 by tagging injected edit tools with internal provenance and deriving conversion eligibility only after the request-builder catalog has been filtered and budgeted. Co-authored-by: bitkyc08-arch Co-authored-by: NexusCore <22769595+ZachDreamZ@users.noreply.github.com> --- src/adapters/cursor/live-transport.ts | 24 ++++++++++-------------- src/adapters/cursor/tool-definitions.ts | 9 +++++++++ src/types.ts | 2 ++ tests/cursor-structured-edit.test.ts | 17 +++++++++++++++++ 4 files changed, 38 insertions(+), 14 deletions(-) diff --git a/src/adapters/cursor/live-transport.ts b/src/adapters/cursor/live-transport.ts index 1a9cb6500d..eafb2c9b4b 100644 --- a/src/adapters/cursor/live-transport.ts +++ b/src/adapters/cursor/live-transport.ts @@ -66,12 +66,11 @@ import { desktopDepsFromConfig } from "./native-exec-desktop"; import { buildCursorToolDefinitions, cursorRequestAdvertisesApplyPatch, - cursorRequestAdvertisesStructuredEdits, - cursorStructuredEditTools, cursorRequestHasShellAlias, cursorToolArgNormalizeSchema, cursorToolWireName, cursorToolsForActivePrompt, + isCursorSyntheticStructuredEditTool, isGenericToolUseCountDemoPrompt, requestedCursorToolUseCount, } from "./tool-definitions"; @@ -542,23 +541,20 @@ class LiveCursorTransport implements CursorTransport { this.activeClientToolFinalizeGraceMs = clientToolFinalizeGraceMsForRequest(request, this.clientToolFinalizeGraceMs); const cursorVisibleTools = cursorToolsForActivePrompt(request.tools, activeText, request.toolChoice); const clientToolDefs = buildCursorToolDefinitions(cursorVisibleTools, request.toolChoice); + // `request.tools` is the catalog already filtered and budgeted by request-builder. Derive + // conversion provenance only from tagged synthetic tools that also survive this final prompt + // filter; a client tool with the same wire name can never opt into conversion by collision. + const syntheticStructuredEditToolNames = new Set( + (cursorVisibleTools ?? []) + .filter(isCursorSyntheticStructuredEditTool) + .map(cursorToolWireName), + ); this.execContext = { ...this.execContext, clientToolDefs, rejectNativeFileMutations: cursorRequestAdvertisesApplyPatch(request.tools, request.toolChoice), - structuredEditAvailable: cursorRequestAdvertisesStructuredEdits(request.tools, request.toolChoice), + structuredEditAvailable: syntheticStructuredEditToolNames.size > 0, }; - // Provenance for the synthetic structured-edit tools (#1036 review): record the bare names WE - // actually advertised on THIS request, taken from the definitions that were really sent rather - // than from the name alone. A client or MCP tool legitimately called `edit_file` is in - // clientToolDefs too, so the discriminator is `cursorStructuredEditTools` having produced it — - // which is precisely what `structuredEditAvailable` already reflects. - const syntheticStructuredEditToolNames = new Set( - (this.execContext.structuredEditAvailable - ? cursorStructuredEditTools(request.tools, request.toolChoice) - : [] - ).map(tool => tool.name), - ); const toolSchemas = new Map(); const cursorToolNameMap = new Map(); for (const tool of cursorVisibleTools ?? []) { diff --git a/src/adapters/cursor/tool-definitions.ts b/src/adapters/cursor/tool-definitions.ts index 1531165f38..9702aa28fe 100644 --- a/src/adapters/cursor/tool-definitions.ts +++ b/src/adapters/cursor/tool-definitions.ts @@ -188,6 +188,13 @@ export function isCursorStructuredEditToolName(name: string): boolean { return (CURSOR_STRUCTURED_EDIT_TOOLS as readonly string[]).includes(name); } +/** Internal provenance gate for synthetic edits after prompt filtering and catalog budgeting. */ +export function isCursorSyntheticStructuredEditTool( + tool: Pick, +): boolean { + return !tool.namespace && tool.cursorStructuredEdit === true && isCursorStructuredEditToolName(tool.name); +} + /** * Synthetic structured edit tools for the Cursor route (#1017). * @@ -216,12 +223,14 @@ export function cursorStructuredEditTools( const candidates: OcxTool[] = [ { 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.", 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.", parameters: { ...CURSOR_MULTI_EDIT_INPUT_SCHEMA }, diff --git a/src/types.ts b/src/types.ts index 91280a5c62..f8b4cda203 100644 --- a/src/types.ts +++ b/src/types.ts @@ -166,6 +166,8 @@ export interface OcxTool { toolSearch?: boolean; /** Tool definition restored from a prior tool_search output; transports may prioritize it when catalogs are bounded. */ loadedFromToolSearch?: boolean; + /** Cursor-only synthetic exact-match edit tool; never inferred from the wire name. */ + cursorStructuredEdit?: true; /** Synthetic web_search tool: the model's call is executed by the gpt-5.4-mini sidecar, not relayed to Codex. */ webSearch?: boolean; /** Synthetic image_gen tool: the model's call is executed by the xAI image bridge sidecar, not relayed to Codex. */ diff --git a/tests/cursor-structured-edit.test.ts b/tests/cursor-structured-edit.test.ts index 088b989ae3..ab412eb4be 100644 --- a/tests/cursor-structured-edit.test.ts +++ b/tests/cursor-structured-edit.test.ts @@ -29,6 +29,8 @@ import { CURSOR_MULTI_EDIT_INPUT_SCHEMA, CURSOR_MULTI_EDIT_TOOL, cursorStructuredEditTools, + cursorToolsForActivePrompt, + isCursorSyntheticStructuredEditTool, } from "../src/adapters/cursor/tool-definitions"; const encoder = new TextEncoder(); @@ -90,6 +92,7 @@ describe("cursor structured edit tools (#1017)", () => { test("advertises edit_file and multi_edit alongside a bare freeform apply_patch", () => { const tools = cursorStructuredEditTools([applyPatchTool()], "auto"); expect(tools.map(tool => tool.name)).toEqual([CURSOR_EDIT_FILE_TOOL, CURSOR_MULTI_EDIT_TOOL]); + expect(tools.every(isCursorSyntheticStructuredEditTool)).toBe(true); expect(tools[0]?.parameters).toEqual(CURSOR_EDIT_FILE_INPUT_SCHEMA); expect(tools[1]?.parameters).toEqual(CURSOR_MULTI_EDIT_INPUT_SCHEMA); }); @@ -127,6 +130,20 @@ describe("cursor structured edit tools (#1017)", () => { expect(result.tools.map(tool => tool.name)).toEqual(["apply_patch"]); }); + test("derives structured-edit provenance after the final prompt filter", () => { + const catalog = [ + execCommandTool(), + ...cursorStructuredEditTools([applyPatchTool()], "auto"), + ]; + const filtered = cursorToolsForActivePrompt(catalog, "Use exactly 2 tools for this demo", "auto"); + const names = (filtered ?? []) + .filter(isCursorSyntheticStructuredEditTool) + .map(tool => tool.name); + + expect(filtered?.map(tool => tool.name)).toEqual(["exec_command"]); + expect(names).toEqual([]); + }); + test("guidance note tells the model to prefer the structured edit tools", () => { const note = buildCursorToolGuidanceSystemNote([applyPatchTool(), ...cursorStructuredEditTools([applyPatchTool()])], "auto"); expect(note).toContain("prefer the structured edit tools"); From 80e3b9a33d032a3d6856993f270ac3ba9c1b0c9b Mon Sep 17 00:00:00 2001 From: Agent59353 Date: Thu, 6 Aug 2026 21:30:17 +0800 Subject: [PATCH 7/7] fix(responses): preserve replay across empty deltas Adapt PR #1126 by preserving reasoning replay candidates across empty text_delta and thinking_delta events in streaming and batch builders. Keep the cache memory-only; omit disk persistence, exit hooks, counters, config plumbing, and openai-chat diagnostics. Co-authored-by: Agent59353 Co-authored-by: NexusCore <22769595+ZachDreamZ@users.noreply.github.com> --- src/bridge.ts | 14 ++- src/responses/reasoning-replay-cache.ts | 1 + tests/bridge-reasoning-replay-batch.test.ts | 92 +++++++++++++++ tests/reasoning-replay-robustness.test.ts | 120 ++++++++++++++++++++ 4 files changed, 222 insertions(+), 5 deletions(-) create mode 100644 tests/bridge-reasoning-replay-batch.test.ts create mode 100644 tests/reasoning-replay-robustness.test.ts diff --git a/src/bridge.ts b/src/bridge.ts index 45f046f424..699f5c83e3 100644 --- a/src/bridge.ts +++ b/src/bridge.ts @@ -827,8 +827,10 @@ export function bridgeToResponsesSSE( if (currentReasoning) closeCurrentReasoning(); if (currentRawReasoning) closeCurrentRawReasoning(); flushHiddenRawReasoning(); - // Reasoning consumed by a text turn, not a tool call: no cache target. - rawReasoningForNextToolCall = ""; + // Reasoning consumed by a REAL text turn, not a tool call: no cache target. + // Empty text deltas must not wipe reasoning that precedes a tool call + // (chat-completions providers emit empty content deltas mid-tool-turn). + if (event.text.length > 0) rawReasoningForNextToolCall = ""; if (currentToolCall) closeCurrentToolCall(); // Only flush on an explicit phase change. A later delta that omits `phase` must // keep appending to the current message rather than wiping the earlier phase. @@ -880,7 +882,7 @@ export function bridgeToResponsesSSE( if (currentMsg) closeCurrentMessage("commentary"); if (currentRawReasoning) closeCurrentRawReasoning(); flushHiddenRawReasoning(); - rawReasoningForNextToolCall = ""; + if (event.thinking.length > 0) rawReasoningForNextToolCall = ""; if (currentToolCall) closeCurrentToolCall(); if (!currentReasoning) { const itemId = `rs_${uuid()}`; @@ -1561,7 +1563,9 @@ function buildResponseJSONWithBudget( if (currentText && e.phase !== undefined && currentTextPhase !== e.phase) flushText("commentary"); if (currentSummaryReasoning) flushSummaryReasoning(); if (currentRawReasoning) flushRawReasoning(); - rawReasoningForNextToolCall = ""; + // Empty text deltas (batch chat responses always carry content, often "") must + // not wipe reasoning that precedes a tool call (#950 non-streaming path). + if (e.text.length > 0) rawReasoningForNextToolCall = ""; if (currentToolCallId) flushToolCall(); // Compaction turns keep the summary out of normal message output (replay dedup — see // bridgeToResponsesSSE); it ships only inside the synthetic compaction item below. @@ -1580,7 +1584,7 @@ function buildResponseJSONWithBudget( case "thinking_delta": if (currentText) flushText("commentary"); if (currentRawReasoning) flushRawReasoning(); - rawReasoningForNextToolCall = ""; + if (e.thinking.length > 0) rawReasoningForNextToolCall = ""; if (currentToolCallId) flushToolCall(); { ({ value: currentSummaryReasoning, bytes: currentSummaryReasoningBytes } = appendBatchString( diff --git a/src/responses/reasoning-replay-cache.ts b/src/responses/reasoning-replay-cache.ts index 12c585f2cb..5e1eceb7f7 100644 --- a/src/responses/reasoning-replay-cache.ts +++ b/src/responses/reasoning-replay-cache.ts @@ -44,6 +44,7 @@ const keyFor = (callId: string, scope: string | undefined): string => * id is never read again. */ export function rememberReasoningForCall(callId: string, text: string, scope?: string): void { + // Empty provider deltas are absence of new reasoning, not a request to erase a candidate. if (!callId || typeof text !== "string" || text.length === 0) return; const bytes = Buffer.byteLength(text, "utf8"); // A single entry larger than the whole budget would immediately evict itself. diff --git a/tests/bridge-reasoning-replay-batch.test.ts b/tests/bridge-reasoning-replay-batch.test.ts new file mode 100644 index 0000000000..de617128be --- /dev/null +++ b/tests/bridge-reasoning-replay-batch.test.ts @@ -0,0 +1,92 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { bridgeToResponsesSSE, buildResponseJSON } from "../src/bridge"; +import { + clearReasoningReplayCacheForTests, + peekReasoningForCall, +} from "../src/responses/reasoning-replay-cache"; +import type { AdapterEvent } from "../src/types"; + +/** + * Regression for issue #950's non-streaming path: chat-completions batch + * responses always carry `content` (often an empty string), and the adapter + * emits that as a text_delta BEFORE tool_call_start. The bridge used to wipe + * the pending reasoning handoff on every text_delta, so batch-born tool rounds + * never entered the replay cache and any later continuation serialized bare → + * upstream 400 ("The `reasoning_content` in the thinking mode must be passed + * back to the API."). All real-world 400s on this proxy were closeReason + * non_stream. + */ + +const REASONING = "I need to inspect files before answering."; +const SCOPE = "thread-batch"; + +function batchOutput(events: AdapterEvent[]): Record { + return buildResponseJSON(events, "opencode-free/deepseek-v4-flash-free", { + replayCacheScope: SCOPE, + }); +} + +async function streamFrames(events: AdapterEvent[]): Promise { + async function* replay(list: AdapterEvent[]): AsyncGenerator { + for (const event of list) yield event; + } + const reader = bridgeToResponsesSSE( + replay(events), + "opencode-free/deepseek-v4-flash-free", + undefined, + undefined, + undefined, + undefined, + undefined, + { replayCacheScope: SCOPE }, + ).getReader(); + const decoder = new TextDecoder(); + while (true) { + const { done, value } = await reader.read(); + if (done) break; + decoder.decode(value, { stream: true }); + } +} + +const toolRoundEvents = (): AdapterEvent[] => [ + { type: "reasoning_raw_delta", text: REASONING }, + { type: "text_delta", text: "" }, + { type: "tool_call_start", id: "call_batch_1", name: "read_file" }, + { type: "tool_call_delta", arguments: '{"path":"README.md"}' }, + { type: "tool_call_end" }, + { type: "done" }, +]; + +describe("reasoning replay survives empty text deltas (both wire modes)", () => { + beforeEach(() => { + clearReasoningReplayCacheForTests(); + }); + afterEach(() => { + clearReasoningReplayCacheForTests(); + }); + + test("batch: reasoning + empty content + tool call is cached for the call id", () => { + const response = batchOutput(toolRoundEvents()); + expect(response.output.some(o => (o as Record).type === "function_call")).toBe(true); + expect(peekReasoningForCall("call_batch_1", SCOPE)).toBe(REASONING); + }); + + test("batch: real text between reasoning and the tool call clears the cache target", () => { + const events = toolRoundEvents(); + events[1] = { type: "text_delta", text: "Let me look at the repo first." }; + batchOutput(events); + expect(peekReasoningForCall("call_batch_1", SCOPE)).toBeUndefined(); + }); + + test("stream: reasoning + empty content delta + tool call is cached for the call id", async () => { + await streamFrames(toolRoundEvents()); + expect(peekReasoningForCall("call_batch_1", SCOPE)).toBe(REASONING); + }); + + test("stream: real text between reasoning and the tool call clears the cache target", async () => { + const events = toolRoundEvents(); + events[1] = { type: "text_delta", text: "Let me look at the repo first." }; + await streamFrames(events); + expect(peekReasoningForCall("call_batch_1", SCOPE)).toBeUndefined(); + }); +}); diff --git a/tests/reasoning-replay-robustness.test.ts b/tests/reasoning-replay-robustness.test.ts new file mode 100644 index 0000000000..c8212476bb --- /dev/null +++ b/tests/reasoning-replay-robustness.test.ts @@ -0,0 +1,120 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { existsSync, mkdtempSync, readdirSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { pathToFileURL } from "node:url"; +import { bridgeToResponsesSSE, buildResponseJSON } from "../src/bridge"; +import { + clearReasoningReplayCacheForTests, + peekReasoningForCall, + rememberReasoningForCall, +} from "../src/responses/reasoning-replay-cache"; +import type { AdapterEvent } from "../src/types"; + +const REASONING = "I need to inspect files before answering."; +const SCOPE = "thread-empty-delta"; + +function reasoningToolRound(intervening: AdapterEvent): AdapterEvent[] { + return [ + { type: "reasoning_raw_delta", text: REASONING }, + intervening, + { type: "tool_call_start", id: "call_empty_delta", name: "read_file" }, + { type: "tool_call_delta", arguments: '{"path":"README.md"}' }, + { type: "tool_call_end" }, + { type: "done" }, + ]; +} + +async function consumeStreaming(events: AdapterEvent[]): Promise { + async function* replay(): AsyncGenerator { + yield* events; + } + const reader = bridgeToResponsesSSE( + replay(), + "opencode-free/deepseek-v4-flash-free", + undefined, + undefined, + undefined, + undefined, + undefined, + { replayCacheScope: SCOPE }, + ).getReader(); + while (!(await reader.read()).done) { + // Drain the response so the bridge closes and records the replay candidate. + } +} + +describe("reasoning replay empty-delta robustness", () => { + beforeEach(() => { + clearReasoningReplayCacheForTests(); + }); + + afterEach(() => { + clearReasoningReplayCacheForTests(); + }); + + test("an empty cache update does not replace an existing replay candidate", () => { + rememberReasoningForCall("call_cache", REASONING, SCOPE); + rememberReasoningForCall("call_cache", "", SCOPE); + + expect(peekReasoningForCall("call_cache", SCOPE)).toBe(REASONING); + }); + + test("batch: an empty thinking delta preserves reasoning for the following tool call", () => { + buildResponseJSON( + reasoningToolRound({ type: "thinking_delta", thinking: "" }), + "opencode-free/deepseek-v4-flash-free", + { replayCacheScope: SCOPE }, + ); + + expect(peekReasoningForCall("call_empty_delta", SCOPE)).toBe(REASONING); + }); + + test("stream: an empty thinking delta preserves reasoning for the following tool call", async () => { + await consumeStreaming(reasoningToolRound({ type: "thinking_delta", thinking: "" })); + + expect(peekReasoningForCall("call_empty_delta", SCOPE)).toBe(REASONING); + }); + + test("non-empty thinking still consumes the pending raw-reasoning candidate", () => { + buildResponseJSON( + reasoningToolRound({ type: "thinking_delta", thinking: "A visible summary." }), + "opencode-free/deepseek-v4-flash-free", + { replayCacheScope: SCOPE }, + ); + + expect(peekReasoningForCall("call_empty_delta", SCOPE)).toBeUndefined(); + }); + + test("the memory-only cache writes no reasoning state to disk even when persistence env vars are set", async () => { + const scratch = mkdtempSync(join(tmpdir(), "ocx-reasoning-memory-only-")); + const spill = join(scratch, "must-not-exist.json"); + const moduleUrl = pathToFileURL(join(import.meta.dir, "../src/responses/reasoning-replay-cache.ts")).href; + const script = [ + `const cache = await import(${JSON.stringify(moduleUrl)});`, + `cache.rememberReasoningForCall("call_disk_guard", ${JSON.stringify(REASONING)}, "disk-guard");`, + ].join("\n"); + + try { + const child = Bun.spawn([process.execPath, "-e", script], { + cwd: scratch, + env: { + ...process.env, + OPENCODEX_HOME: scratch, + OPENCODEX_REASONING_REPLAY_PERSIST: "1", + OPENCODEX_REASONING_REPLAY_FILE: spill, + }, + stdout: "pipe", + stderr: "pipe", + }); + const exitCode = await child.exited; + const stderr = await new Response(child.stderr).text(); + + expect(exitCode, stderr).toBe(0); + expect(existsSync(spill)).toBe(false); + expect(readdirSync(scratch)).toEqual([]); + } finally { + rmSync(scratch, { recursive: true, force: true }); + } + }); +});