fix(ai): use neutral MCP text for silent terminal_execute success

Codex P2 on #2724: ok:true empty serial/raw PTY results no longer fall
back to "Error: Operation failed". Prefer Catty's "Command completed
(no output)" and keep operational failures / non-zero exits unchanged.
Also prefer non-empty OpenCode tool error/output when formatting tool
results.

electron/bridges/aiBridge/sdk/opencodeDriver.cjs

@@ -287,8 +287,26 @@ function translateOpenCodeEvent(event, emitter, state = {}) {
           emitter.toolResult(callId, part.state.output || "", toolName);
         }
       } else if (part.state?.status === "error") {
-        emitter.emitError(part.state.error || "OpenCode tool failed");
-        return { idle: false, error: true, content: false };
+        // Tool-level failures must not abort the whole OpenCode turn. Other
+        // drivers (Cursor / Codex / Grok) surface tool errors as tool results
+        // so the model can adapt and continue multi-step work (issue #2718).
+        state.toolCalls = state.toolCalls || new Set();
+        if (!state.toolCalls.has(callId)) {
+          state.toolCalls.add(callId);
+          emitter.toolCall(toolName, input, callId);
+        }
+        state.toolResults = state.toolResults || new Set();
+        if (!state.toolResults.has(callId)) {
+          state.toolResults.add(callId);
+          // Prefer non-empty error, then output, then a stable default (blank
+          // string error must not hide a useful output payload).
+          const rawError = part.state.error || part.state.output || "OpenCode tool failed";
+          const errorText = typeof rawError === "string"
+            ? rawError
+            : (extractOpenCodeErrorMessage(rawError) || "OpenCode tool failed");
+          emitter.toolResult(callId, errorText, toolName);
+        }
+        return { idle: false, error: false, content: true };
       }
     }
     return { idle: false, error: false, content: part.type === "tool" };

electron/bridges/aiBridge/sdk/opencodeDriver.test.cjs

@@ -403,6 +403,101 @@ test("translateOpenCodeEvent emits a tool call before a result when only complet
   ]);
 });
 
+test("translateOpenCodeEvent maps tool status error to toolResult without failing the turn (#2718)", () => {
+  const { events, emitter } = collector();
+  const state = {};
+  const result = translateOpenCodeEvent(
+    {
+      type: "message.part.updated",
+      properties: {
+        part: {
+          type: "tool",
+          callID: "tool-err",
+          tool: "netcatty-remote-hosts_terminal_execute",
+          state: {
+            status: "error",
+            input: { command: "du /missing" },
+            error: "Error: Operation failed",
+          },
+        },
+      },
+    },
+    emitter,
+    state,
+  );
+
+  assert.equal(result.error, false);
+  assert.equal(result.content, true);
+  assert.deepEqual(events, [
+    {
+      k: "toolCall",
+      name: "netcatty-remote-hosts_terminal_execute",
+      args: { command: "du /missing" },
+      id: "tool-err",
+    },
+    {
+      k: "toolResult",
+      id: "tool-err",
+      out: "Error: Operation failed",
+      name: "netcatty-remote-hosts_terminal_execute",
+    },
+  ]);
+  assert.equal(events.some((event) => event.k === "error"), false);
+});
+
+test("runOpenCodeTurn continues after a tool error part and reaches session.idle (#2718)", async () => {
+  const { events, emitter } = collector();
+  const abortController = new AbortController();
+  const stream = {
+    async *[Symbol.asyncIterator]() {
+      yield {
+        type: "message.part.updated",
+        properties: {
+          part: {
+            type: "tool",
+            sessionID: "sess-1",
+            callID: "t1",
+            tool: "terminal_execute",
+            state: { status: "error", input: { command: "false" }, error: "Error: Operation failed" },
+          },
+        },
+      };
+      yield {
+        type: "message.part.updated",
+        properties: {
+          part: {
+            type: "text",
+            sessionID: "sess-1",
+            id: "p1",
+            text: "retrying",
+          },
+          delta: "retrying",
+        },
+      };
+      yield { type: "session.idle", properties: { sessionID: "sess-1" } };
+    },
+  };
+  const client = {
+    global: { event: async () => ({ stream }) },
+    session: {
+      create: async () => ({ data: { id: "sess-1" } }),
+      promptAsync: async () => ({ data: true }),
+    },
+  };
+
+  await runOpenCodeTurn({
+    prompt: "run false then continue",
+    emitter,
+    abortController,
+    openCodeFactory: async () => ({ client, server: { close() {} } }),
+  });
+
+  assert.ok(events.some((event) => event.k === "toolResult" && event.out === "Error: Operation failed"));
+  assert.ok(events.some((event) => event.k === "text" && event.t === "retrying"));
+  assert.ok(events.some((event) => event.k === "done"));
+  assert.equal(events.some((event) => event.k === "error"), false);
+});
+
 test("mapOpenCodeModels flattens providers", () => {
   assert.deepEqual(mapOpenCodeModels({
     providers: [

electron/capabilities/codegen/mcpToolRegistry.cjs

@@ -46,19 +46,71 @@ function buildZodSchema(inputShape) {
 }
 
 function formatRpcError(result) {
-  return `Error: ${result?.error || "Operation failed"}`;
+  if (result?.error) return `Error: ${result.error}`;
+  if (result?.code) return `Error: Operation failed (${result.code})`;
+  return "Error: Operation failed";
+}
+
+/**
+ * True when the RPC result carries command-run evidence (output and/or exit
+ * code). Non-zero exits from PTY exec set ok:false without an error string —
+ * that is still a completed command, not an operational failure.
+ */
+function hasTerminalExecuteEvidence(result) {
+  if (!result || typeof result !== "object") return false;
+  if (typeof result.stdout === "string" && result.stdout.length > 0) return true;
+  if (typeof result.stderr === "string" && result.stderr.length > 0) return true;
+  return result.exitCode != null;
 }
 
 function formatTerminalExecuteResult(result) {
   const parts = [];
-  if (result.stdout) parts.push(result.stdout);
-  if (result.stderr) parts.push(`[stderr] ${result.stderr}`);
-  if (result.exitCode != null) {
+  if (result?.stdout) parts.push(result.stdout);
+  if (result?.stderr) parts.push(`[stderr] ${result.stderr}`);
+  if (result?.exitCode != null) {
     parts.push(`[exit code: ${result.exitCode}]`);
   }
+  if (result?.error) parts.push(`[error] ${result.error}`);
   return parts.join("\n");
 }
 
+// Align with Catty executor wording when a command ran but produced no text.
+const TERMINAL_EXECUTE_NO_OUTPUT_TEXT = "Command completed (no output)";
+
+/**
+ * Format terminal_execute for MCP clients.
+ * Match Catty semantics: when the command actually ran, always surface
+ * stdout/stderr/exitCode so the model can judge the failure. Only pure
+ * operational failures (session missing, blocked, etc.) become a bare
+ * "Error: …" payload. See issue #2718.
+ *
+ * Successful empty results (serial/network raw PTY often returns ok:true with
+ * empty stdout/stderr and exitCode:null) must NOT fall back to
+ * "Error: Operation failed" — Codex P2 on PR #2724.
+ */
+function formatTerminalExecuteMcpResponse(result) {
+  if (!result || typeof result !== "object") {
+    return { content: [{ type: "text", text: formatRpcError(result) }], isError: true };
+  }
+
+  // Operational failure with no command evidence (session not found, busy, blocked…).
+  if (result.ok === false && !hasTerminalExecuteEvidence(result)) {
+    return { content: [{ type: "text", text: formatRpcError(result) }], isError: true };
+  }
+
+  // Empty successful payloads (serial/raw PTY): neutral text.
+  // Never fall back to formatRpcError here — that mislabels silent successes (#2724 P2).
+  // When result.error is set, formatTerminalExecuteResult already includes `[error] …`.
+  const text = formatTerminalExecuteResult(result) || TERMINAL_EXECUTE_NO_OUTPUT_TEXT;
+  // Mark isError only for operational/runtime failures that include an error
+  // string (timeout, cancel, stream lost). Non-zero exit alone is success of
+  // the tool call with a failed command — same as Catty ok:true + exitCode.
+  if (result.ok === false && result.error) {
+    return { content: [{ type: "text", text }], isError: true };
+  }
+  return { content: [{ type: "text", text }] };
+}
+
 function createToolHandler(toolDef, deps) {
   const {
     rpcCall,
@@ -76,10 +128,10 @@ function createToolHandler(toolDef, deps) {
         return { content: [{ type: "text", text: `Error: ${guardErr}` }], isError: true };
       }
       const result = await rpcCall(rpcMethod, { ...scopeParams, sessionId, command });
-      if (!result.ok) {
-        return { content: [{ type: "text", text: formatRpcError(result) }], isError: true };
-      }
       if (TERMINAL_START_TOOLS.has(mcpTool)) {
+        if (!result?.ok) {
+          return { content: [{ type: "text", text: formatRpcError(result) }], isError: true };
+        }
         return {
           content: [{
             type: "text",
@@ -94,7 +146,7 @@ function createToolHandler(toolDef, deps) {
           }],
         };
       }
-      return { content: [{ type: "text", text: formatTerminalExecuteResult(result) }] };
+      return formatTerminalExecuteMcpResponse(result);
     }
 
     if (TERMINAL_POLL_TOOLS.has(mcpTool) || TERMINAL_STOP_TOOLS.has(mcpTool)) {
@@ -188,6 +240,10 @@ function registerMcpTools(server, deps) {
 module.exports = {
   buildZodSchema,
   buildZodShapeObject,
+  formatRpcError,
+  formatTerminalExecuteResult,
+  formatTerminalExecuteMcpResponse,
+  hasTerminalExecuteEvidence,
   registerMcpTools,
   SFTP_WRITE_TOOLS,
 };

electron/capabilities/codegen/toolSurfaces.test.cjs

@@ -235,3 +235,129 @@ test("session_close remains available as a cleanup action in observer mode", asy
   assert.equal(result.isError, undefined);
   assert.equal(guardCalls, 0);
 });
+
+test("terminal_execute MCP response preserves stdout/exitCode on non-zero exit (#2718)", async () => {
+  let handler = null;
+  const fakeServer = {
+    tool(name, _description, _shape, candidate) {
+      if (name === "terminal_execute") handler = candidate;
+    },
+  };
+  registerMcpTools(fakeServer, {
+    rpcCall: async () => ({
+      ok: false,
+      stdout: "du: cannot access '/missing': No such file or directory",
+      stderr: "",
+      exitCode: 1,
+    }),
+    scopeParams: { chatSessionId: "chat-1" },
+    guardWriteOperation: () => null,
+    catalogDescription: (_name, fallback) => fallback,
+  });
+
+  assert.ok(handler, "terminal_execute handler registered");
+  const result = await handler({ sessionId: "sess-1", command: "du /missing" });
+  assert.equal(result.isError, undefined);
+  const text = result.content?.[0]?.text || "";
+  assert.match(text, /cannot access '\/missing'/);
+  assert.match(text, /\[exit code: 1\]/);
+  assert.doesNotMatch(text, /Operation failed/);
+});
+
+test("terminal_execute MCP response keeps operational failures as isError", async () => {
+  let handler = null;
+  const fakeServer = {
+    tool(name, _description, _shape, candidate) {
+      if (name === "terminal_execute") handler = candidate;
+    },
+  };
+  registerMcpTools(fakeServer, {
+    rpcCall: async () => ({ ok: false, error: "Session not found" }),
+    scopeParams: { chatSessionId: "chat-1" },
+    guardWriteOperation: () => null,
+    catalogDescription: (_name, fallback) => fallback,
+  });
+
+  const result = await handler({ sessionId: "gone", command: "uptime" });
+  assert.equal(result.isError, true);
+  assert.equal(result.content?.[0]?.text, "Error: Session not found");
+});
+
+test("terminal_execute MCP response includes partial output on timeout", async () => {
+  let handler = null;
+  const fakeServer = {
+    tool(name, _description, _shape, candidate) {
+      if (name === "terminal_execute") handler = candidate;
+    },
+  };
+  registerMcpTools(fakeServer, {
+    rpcCall: async () => ({
+      ok: false,
+      stdout: "partial lines",
+      stderr: "",
+      exitCode: -1,
+      error: "Command timed out (60s)",
+    }),
+    scopeParams: { chatSessionId: "chat-1" },
+    guardWriteOperation: () => null,
+    catalogDescription: (_name, fallback) => fallback,
+  });
+
+  const result = await handler({ sessionId: "sess-1", command: "sleep 999" });
+  assert.equal(result.isError, true);
+  const text = result.content?.[0]?.text || "";
+  assert.match(text, /partial lines/);
+  assert.match(text, /\[exit code: -1\]/);
+  assert.match(text, /\[error\] Command timed out \(60s\)/);
+});
+
+test("terminal_execute MCP response uses neutral text for successful empty output (#2724)", async () => {
+  let handler = null;
+  const fakeServer = {
+    tool(name, _description, _shape, candidate) {
+      if (name === "terminal_execute") handler = candidate;
+    },
+  };
+  // Serial/network-device raw PTY success: ok true, empty streams, exitCode null.
+  registerMcpTools(fakeServer, {
+    rpcCall: async () => ({
+      ok: true,
+      stdout: "",
+      stderr: "",
+      exitCode: null,
+    }),
+    scopeParams: { chatSessionId: "chat-1" },
+    guardWriteOperation: () => null,
+    catalogDescription: (_name, fallback) => fallback,
+  });
+
+  const result = await handler({ sessionId: "sess-1", command: "configure terminal" });
+  assert.equal(result.isError, undefined);
+  assert.equal(result.content?.[0]?.text, "Command completed (no output)");
+  assert.doesNotMatch(result.content?.[0]?.text || "", /Operation failed/);
+});
+
+test("terminal_execute MCP response keeps exit-only non-zero without isError", async () => {
+  let handler = null;
+  const fakeServer = {
+    tool(name, _description, _shape, candidate) {
+      if (name === "terminal_execute") handler = candidate;
+    },
+  };
+  registerMcpTools(fakeServer, {
+    rpcCall: async () => ({
+      ok: false,
+      stdout: "",
+      stderr: "",
+      exitCode: 1,
+    }),
+    scopeParams: { chatSessionId: "chat-1" },
+    guardWriteOperation: () => null,
+    catalogDescription: (_name, fallback) => fallback,
+  });
+
+  const result = await handler({ sessionId: "sess-1", command: "false" });
+  assert.equal(result.isError, undefined);
+  assert.equal(result.content?.[0]?.text, "[exit code: 1]");
+  assert.doesNotMatch(result.content?.[0]?.text || "", /Operation failed/);
+});