diff --git a/src/adapters/cursor/live-transport.ts b/src/adapters/cursor/live-transport.ts index eb7ebdcc60..eafb2c9b4b 100644 --- a/src/adapters/cursor/live-transport.ts +++ b/src/adapters/cursor/live-transport.ts @@ -70,6 +70,7 @@ import { cursorToolArgNormalizeSchema, cursorToolWireName, cursorToolsForActivePrompt, + isCursorSyntheticStructuredEditTool, isGenericToolUseCountDemoPrompt, requestedCursorToolUseCount, } from "./tool-definitions"; @@ -540,10 +541,19 @@ 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: syntheticStructuredEditToolNames.size > 0, }; const toolSchemas = new Map(); const cursorToolNameMap = new Map(); @@ -571,6 +581,7 @@ class LiveCursorTransport implements CursorTransport { parallelToolCalls: request.parallelToolCalls, toolSchemas, cursorToolNameMap, + syntheticStructuredEditToolNames, translatorBudget: this.translatorBudget, contextUsage, ...(prepared.estimatedInputTokens !== undefined diff --git a/src/adapters/cursor/native-exec-fs.ts b/src/adapters/cursor/native-exec-fs.ts index e9c22a2c4e..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 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 2f9531cf7f..98c8779ca7 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, @@ -161,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 @@ -185,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 } : {}), @@ -311,6 +344,117 @@ 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 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") }; +} + +/** + * 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 as string }; + }; + 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", @@ -341,9 +485,20 @@ export function mapSyntheticMcpExecToToolEvents( } } // 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: 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 +540,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 +554,14 @@ 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 = structuredEditCallIsOurs(state.syntheticStructuredEditToolNames, open.name) + ? translateStructuredEditCall(open.name, finalArgs) + : undefined; + 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 +572,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..9702aa28fe 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,73 @@ 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); +} + +/** 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). + * + * 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 []; + // 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, + 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 }, + }, + ]; + 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); } @@ -415,6 +526,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 +557,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 +573,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/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/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/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/cursor-structured-edit.test.ts b/tests/cursor-structured-edit.test.ts new file mode 100644 index 0000000000..ab412eb4be --- /dev/null +++ b/tests/cursor-structured-edit.test.ts @@ -0,0 +1,466 @@ +import { create } from "@bufbuild/protobuf"; +import { describe, expect, test } from "bun:test"; +import type { OcxTool } from "../src/types"; +import { + AgentServerMessageSchema, + ExecServerMessageSchema, + 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 { planMcpArgsHandling } from "../src/adapters/cursor/live-transport"; +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, + cursorToolsForActivePrompt, + isCursorSyntheticStructuredEditTool, +} 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.every(isCursorSyntheticStructuredEditTool)).toBe(true); + 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("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); + 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("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"); + 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)).toEqual({ + 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)).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)).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)).toEqual({ + 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)).toEqual({ + 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(); + }); + + 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", () => { + 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]]), + }); + 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], + 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: "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 passes edit_file through untranslated (no provenance)", () => { + 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")), + }, + }); + // 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")), + }, + }); + + 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]]), + }); + 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" }, + ]); + }); + + 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"], + 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]]), + }); + 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" }, + ]); + }); +}); 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 }); + } + }); +});