fix(cursor): normalize empty and failure-state Computer Use tool results (#1920)

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;

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*)?(?:<empty>)?\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 };
+}
+

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 + <empty>) normalizes for node_repl", () => {
+    const out = normalizeCursorToolResultText("Script completed\nOutput:\n<empty>", { 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");
+  });
+});