diff --git a/src/adapters/cursor/protobuf-request.ts b/src/adapters/cursor/protobuf-request.ts index a869634a30..2a55cdb179 100644 --- a/src/adapters/cursor/protobuf-request.ts +++ b/src/adapters/cursor/protobuf-request.ts @@ -5,6 +5,7 @@ import type { OcxAssistantContentPart, OcxMessage, OcxToolResultMessage } from " import { namespacedToolName } from "../../types"; import type { CursorRunRequest } from "./types"; import { isCursorExternalWireModel } from "./discovery"; +import { normalizeCursorToolResultText } from "./tool-result-normalize"; import { debugProviderDiagnostic } from "../../lib/debug"; import { createCursorBlobRequestScope, @@ -240,7 +241,9 @@ function rootPromptMessages(request: CursorRunRequest, requestScope: CursorBlobR } // Assistant tool CALLS are intentionally NOT replayed as visible "[Tool Call]" text here. } else if (message.role === "toolResult") { - const prefix = message.isError ? "[Tool Error]" : "[Tool Result]"; + // #1920: the prefix must reflect the NORMALIZED error state (an empty + // node_repl result is an error even when the runtime said isError=false). + const prefix = normalizedToolResult(message, contentToText(message.content)).isError ? "[Tool Error]" : "[Tool Result]"; const text = `${prefix}\n${toolResultToText(message)}`; entries.push(rootBlobCandidate( toolResultRootPayload(text), @@ -427,7 +430,11 @@ function toolResultContentItems( ) { const parts = decoded ?? decodeResultParts(message); if (!parts) { - const text = typeof message.content === "string" ? message.content : ""; + const raw = typeof message.content === "string" ? message.content : ""; + // #1920/#1866: empty or failure-state Computer Use / node_repl results are + // normalized before they reach the native wire (isError is applied in + // toolResultPart via normalizedToolResult below). + const { text } = normalizedToolResult(message, raw); return [create(McpToolResultContentItemSchema, { content: { case: "text" as const, value: create(McpTextContentSchema, { text }) }, })]; @@ -483,16 +490,30 @@ function toolResultContentItems( } function toolResultToText(message: OcxToolResultMessage): string { + const normalized = normalizedToolResult(message, contentToText(message.content)); return [ "[tool_result]", `call_id: ${message.toolCallId}`, `name: ${namespacedToolName(message.toolNamespace, message.toolName)}`, - `is_error: ${message.isError}`, + `is_error: ${normalized.isError}`, "output:", - contentToText(message.content), + normalized.text, ].join("\n"); } +/** + * Shared #1920 normalization entry: pure-text results only. Image-bearing or + * encrypted results pass through untouched (their content is not plain text). + */ +function normalizedToolResult(message: OcxToolResultMessage, text: string): { text: string; isError: boolean } { + if (message.containsEncryptedContent) return { text, isError: message.isError }; + return normalizeCursorToolResultText(text, { + toolName: message.toolName, + toolNamespace: message.toolNamespace, + isError: message.isError, + }); +} + function argBytes(value: unknown): Uint8Array { try { return toBinary(ValueSchema, fromJson(ValueSchema, value as JsonValue)); @@ -546,11 +567,15 @@ function toolCallStep( } function toolResultPart(message: OcxToolResultMessage, decoded?: DecodedResultPart[], maxImages?: number) { + const parts = decoded ?? decodeResultParts(message); + const normalizedIsError = parts + ? message.isError + : normalizedToolResult(message, typeof message.content === "string" ? message.content : "").isError; return create(McpToolResultSchema, { result: { case: "success", value: create(McpSuccessSchema, { - isError: message.isError, + isError: normalizedIsError, content: toolResultContentItems(message, decoded, maxImages), }), }, @@ -643,11 +668,15 @@ function conversationTurns( if (message.role === "toolResult") { if (!current) continue; if (externalModel) { - const prefix = message.isError ? "[Tool Error]" : "[Tool Result]"; + // #1920/#1866: this external-replay site bypasses toolResultToText, so it + // must consume the normalizer directly — cursor/grok-4.6 is the exact + // reported repro path for empty Computer Use results. + const normalized = normalizedToolResult(message, contentToText(message.content)); + const prefix = normalized.isError ? "[Tool Error]" : "[Tool Result]"; current.steps.push(storeCursorBlob(toBinary(ConversationStepSchema, create(ConversationStepSchema, { message: { case: "assistantMessage", - value: create(AssistantMessageSchema, { text: `${prefix}\n${contentToText(message.content)}` }), + value: create(AssistantMessageSchema, { text: `${prefix}\n${normalized.text}` }), }, })), requestScope)); continue; diff --git a/src/adapters/cursor/tool-result-normalize.ts b/src/adapters/cursor/tool-result-normalize.ts new file mode 100644 index 0000000000..b87ef29854 --- /dev/null +++ b/src/adapters/cursor/tool-result-normalize.ts @@ -0,0 +1,92 @@ +/** + * Cursor tool-result normalization for Computer Use / node_repl surfaces (#1920/#1866). + * + * Scoped re-implementation of PR #1920 per the 260818 campaign disposition + * (REDESIGN-SMALL: "apply formatted.text at native toolResultPart + decode test"). + * Only empty-output and known-failure-state normalization ships here; screenshot + * stripping and AXTree text compaction from the original PR are deliberately out + * of scope (the native path already bounds step size by real serialized bytes, + * dropping images oldest-first — see toolCallStep in protobuf-request.ts). + */ + +const COMPUTER_USE_TOOL_NAMES = new Set([ + "node_repl", + "node_repl__js", + "mcp__node_repl__js", + "get_app_state", + "list_apps", + "screenshot", + "computer_use", +]); + +function isNodeReplOrComputerUseTool(toolName?: string, toolNamespace?: string): boolean { + if (toolNamespace && (toolNamespace === "mcp__node_repl" || toolNamespace.includes("node_repl") || toolNamespace.includes("computer_use"))) { + return true; + } + if (!toolName) return false; + const lower = toolName.toLowerCase(); + if (COMPUTER_USE_TOOL_NAMES.has(lower)) return true; + return lower.startsWith("mcp__node_repl") || lower.startsWith("mcp__computer_use"); +} + +/** Failure states the Computer Use / node_repl runtime reports as PLAIN TEXT inside a non-error result. */ +const RUNTIME_FAILURE_GUIDANCE: ReadonlyArray<{ marker: string; guidance: string }> = [ + { + marker: "SkyComputerUseError", + guidance: "The Computer Use runtime rejected this action. Re-check application state with get_app_state before retrying.", + }, + { + marker: "sky is not defined", + guidance: "The sky binding is unavailable in this context; Computer Use calls only work inside the privileged node_repl session.", + }, + { + marker: "has already been declared", + guidance: "The node_repl session keeps earlier declarations; rename the variable or use var/reassignment instead of redeclaring.", + }, + { + marker: "unsupported import in exec", + guidance: "Imports are not available in this exec context; use the injected globals instead.", + }, +]; + +/** Matches exec wrappers whose only payload is an empty-output marker. */ +const EMPTY_EXEC_OUTPUT_REGEX = /^(?:(?:Script completed|Script failed|Command finished|Execution finished)[^\n]*\n+)?(?:Wall time[^\n]*\n+)?(?:Output:\s*)?(?:)?\s*$/; + +export interface NormalizedToolResultText { + text: string; + isError: boolean; + /** True when normalization changed either field (lets callers skip work on the common path). */ + changed: boolean; +} + +/** + * Normalize a Cursor-bound tool-result TEXT payload: + * - blank / empty-exec-wrapper output on Computer Use or node_repl tools becomes an + * actionable error instead of an empty string the model silently accepts; + * - known runtime failure states reported as plain text are marked isError with a + * one-line recovery hint appended. + * Everything else passes through byte-identical. + */ +export function normalizeCursorToolResultText( + text: string, + options: { toolName?: string; toolNamespace?: string; isError?: boolean } = {}, +): NormalizedToolResultText { + const isError = options.isError === true; + const computerUse = isNodeReplOrComputerUseTool(options.toolName, options.toolNamespace); + if (computerUse && EMPTY_EXEC_OUTPUT_REGEX.test(text.trim())) { + return { + text: "[empty output: the tool ran but produced no stdout or return value. Verify application state with get_app_state, or make the script emit output.]", + isError: true, + changed: true, + }; + } + if (!isError) { + for (const { marker, guidance } of RUNTIME_FAILURE_GUIDANCE) { + if (text.includes(marker)) { + return { text: `${text}\n[recovery: ${guidance}]`, isError: true, changed: true }; + } + } + } + return { text, isError, changed: false }; +} + diff --git a/tests/cursor-toolresult-normalize.test.ts b/tests/cursor-toolresult-normalize.test.ts new file mode 100644 index 0000000000..7b94e0ad3b --- /dev/null +++ b/tests/cursor-toolresult-normalize.test.ts @@ -0,0 +1,145 @@ +import { describe, expect, test } from "bun:test"; +import { create, fromBinary } from "@bufbuild/protobuf"; +import { handleCursorNativeKv } from "../src/adapters/cursor/native-exec"; +import { encodeCursorRunRequest } from "../src/adapters/cursor/protobuf-request"; +import { normalizeCursorToolResultText } from "../src/adapters/cursor/tool-result-normalize"; +import { + AgentClientMessageSchema, + ConversationTurnStructureSchema, + ConversationStepSchema, + GetBlobArgsSchema, + KvServerMessageSchema, +} from "../src/adapters/cursor/gen/agent_pb"; +import type { OcxMessage } from "../src/types"; + +function blobData(blobId: Uint8Array): Uint8Array { + const reply = fromBinary(AgentClientMessageSchema, handleCursorNativeKv(create(KvServerMessageSchema, { + id: 1, + message: { case: "getBlobArgs", value: create(GetBlobArgsSchema, { blobId }) }, + }))); + if (reply.message.case !== "kvClientMessage") throw new Error("not kv"); + const kv = reply.message.value; + if (kv.message.case !== "getBlobResult") throw new Error("not blob result"); + return kv.message.value.blobData; +} + +/** Decode the native-wire McpToolResult attached to the first tool call step. */ +function decodedToolResult(bytes: Uint8Array) { + const msg = fromBinary(AgentClientMessageSchema, bytes); + const run = msg.message.case === "runRequest" ? msg.message.value : undefined; + const turnIds = run?.conversationState?.turns ?? []; + for (const turnId of turnIds) { + const turn = fromBinary(ConversationTurnStructureSchema, blobData(turnId)); + if (turn.turn.case !== "agentConversationTurn") continue; + for (const stepId of turn.turn.value.steps ?? []) { + const step = fromBinary(ConversationStepSchema, blobData(stepId)); + if (step.message.case !== "toolCall") continue; + const tool = step.message.value.tool; + if (tool.case !== "mcpToolCall") continue; + const result = tool.value.result; + if (result?.result.case !== "success") continue; + return result.result.value; + } + } + return undefined; +} + +function requestWith(resultContent: string, toolOverrides: Partial<{ toolName: string; toolNamespace?: string; isError: boolean }> = {}) { + const rawMessages: OcxMessage[] = [ + { role: "user", content: "run it", timestamp: 1 }, + { + role: "assistant", + model: "cursor/auto", + timestamp: 2, + content: [{ type: "toolCall", id: "call_1", name: toolOverrides.toolName ?? "js", namespace: toolOverrides.toolNamespace ?? "mcp__node_repl", arguments: {} }], + }, + { + role: "toolResult", + toolCallId: "call_1", + toolName: toolOverrides.toolName ?? "js", + toolNamespace: "toolNamespace" in toolOverrides ? toolOverrides.toolNamespace : "mcp__node_repl", + content: resultContent, + isError: toolOverrides.isError ?? false, + timestamp: 3, + }, + ]; + return encodeCursorRunRequest({ + modelId: "composer-2.5", + conversationId: "cursor_normalize_test", + system: ["You are helpful."], + messages: [{ role: "tool", content: "[tool_result]" }], + rawMessages, + }); +} + +describe("normalizeCursorToolResultText (#1920/#1866 unit rows)", () => { + test("blank node_repl output becomes an actionable error", () => { + const out = normalizeCursorToolResultText("", { toolName: "js", toolNamespace: "mcp__node_repl" }); + expect(out.isError).toBe(true); + expect(out.text).toContain("get_app_state"); + }); + + test("empty exec wrapper (Script completed + ) normalizes for node_repl", () => { + const out = normalizeCursorToolResultText("Script completed\nOutput:\n", { toolName: "node_repl" }); + expect(out.isError).toBe(true); + expect(out.text).toContain("[empty output"); + }); + + test.each([ + ["SkyComputerUseError: focus lost", "get_app_state"], + ["ReferenceError: sky is not defined", "privileged node_repl"], + ["SyntaxError: Identifier 'x' has already been declared", "redeclaring"], + ["unsupported import in exec", "injected globals"], + ])("runtime failure %p is marked as error with guidance", (payload, hint) => { + const out = normalizeCursorToolResultText(payload, { toolName: "js", toolNamespace: "mcp__node_repl" }); + expect(out.isError).toBe(true); + expect(out.text).toContain(payload); + expect(out.text).toContain(hint); + }); + + test("a non-computer-use tool with empty output stays byte-identical", () => { + const out = normalizeCursorToolResultText("", { toolName: "read_file" }); + expect(out.changed).toBe(false); + expect(out.text).toBe(""); + expect(out.isError).toBe(false); + }); + + test("ordinary non-empty output on node_repl stays byte-identical", () => { + const out = normalizeCursorToolResultText("42", { toolName: "js", toolNamespace: "mcp__node_repl" }); + expect(out.changed).toBe(false); + expect(out.text).toBe("42"); + }); + + test("an already-error result is not double-annotated", () => { + const out = normalizeCursorToolResultText("SkyComputerUseError: x", { toolName: "js", toolNamespace: "mcp__node_repl", isError: true }); + expect(out.changed).toBe(false); + expect(out.isError).toBe(true); + }); +}); + +describe("native wire decode (#1920 disposition: formatted text at toolResultPart)", () => { + test("an empty node_repl result decodes as normalized error text with isError=true on the wire", () => { + const result = decodedToolResult(requestWith("")); + expect(result).toBeDefined(); + expect(result!.isError).toBe(true); + const first = result!.content[0]; + expect(first.content.case).toBe("text"); + expect(first.content.case === "text" ? first.content.value.text : "").toContain("[empty output"); + }); + + test("a failure-state node_repl result decodes with recovery guidance and isError=true", () => { + const result = decodedToolResult(requestWith("ReferenceError: sky is not defined")); + expect(result).toBeDefined(); + expect(result!.isError).toBe(true); + const first = result!.content[0]; + expect(first.content.case === "text" ? first.content.value.text : "").toContain("recovery"); + }); + + test("a normal tool result decodes byte-identical (no normalization side effects)", () => { + const result = decodedToolResult(requestWith("plain output", { toolName: "read_file", toolNamespace: undefined })); + expect(result).toBeDefined(); + expect(result!.isError).toBe(false); + const first = result!.content[0]; + expect(first.content.case === "text" ? first.content.value.text : "").toBe("plain output"); + }); +});