Merge pull request #3664 from Tibsfox/fix/error-success-masks-real-error

fix(gsd): surface real provider error when errorMessage is 'success'

src/resources/extensions/gsd/bootstrap/agent-end-recovery.ts

@@ -95,12 +95,24 @@ export async function handleAgentEnd(
     return;
   }
   if (lastMsg && "stopReason" in lastMsg && lastMsg.stopReason === "error") {
-    const errorDetail = "errorMessage" in lastMsg && lastMsg.errorMessage ? `: ${lastMsg.errorMessage}` : "";
-    const errorMsg = ("errorMessage" in lastMsg && lastMsg.errorMessage) ? String(lastMsg.errorMessage) : "";
+    // #3588: errorMessage can be useless (e.g. "success") while the real error
+    // is in the assistant message text content. Fall back to content when
+    // errorMessage looks uninformative.
+    const rawErrorMsg = ("errorMessage" in lastMsg && lastMsg.errorMessage) ? String(lastMsg.errorMessage) : "";
+    const isUseless = !rawErrorMsg || /^(success|ok|true|error|unknown)$/i.test(rawErrorMsg.trim());
+    // #3588: When errorMessage is uninformative, extract the real error from
+    // the assistant message text content for display purposes only.
+    // Classification still uses rawErrorMsg to avoid false positives from prose.
+    let displayMsg = rawErrorMsg;
+    if (isUseless && "content" in lastMsg && Array.isArray(lastMsg.content)) {
+      const textBlock = lastMsg.content.find((b: any) => b.type === "text" && b.text);
+      if (textBlock) displayMsg = (textBlock as any).text.slice(0, 300);
+    }
+    const errorDetail = displayMsg ? `: ${displayMsg}` : "";
     const explicitRetryAfterMs = ("retryAfterMs" in lastMsg && typeof lastMsg.retryAfterMs === "number") ? lastMsg.retryAfterMs : undefined;
 
-    // ── 1. Classify ──────────────────────────────────────────────────────
-    const cls = classifyError(errorMsg, explicitRetryAfterMs);
+    // ── 1. Classify using rawErrorMsg to avoid prose false-positives ────
+    const cls = classifyError(rawErrorMsg, explicitRetryAfterMs);
 
     // Cap rate-limit backoff for CLI-style providers (openai-codex, google-gemini-cli)
     // which use per-user quotas with shorter windows (#2922).

src/resources/extensions/gsd/tests/error-success-mask.test.ts

@@ -0,0 +1,37 @@
+/**
+ * error-success-mask.test.ts — #3664
+ *
+ * Verify that the agent-end-recovery error handler detects when errorMessage
+ * is uninformative (e.g. "success", "ok", "unknown") and falls back to
+ * extracting the real error from the assistant message text content.
+ */
+
+import { describe, test } from "node:test";
+import assert from "node:assert/strict";
+import { readFileSync } from "node:fs";
+import { join, dirname } from "node:path";
+import { fileURLToPath } from "node:url";
+
+const __dirname = dirname(fileURLToPath(import.meta.url));
+const sourceFile = join(__dirname, "..", "bootstrap", "agent-end-recovery.ts");
+
+describe("error-success mask detection (#3664)", () => {
+  const source = readFileSync(sourceFile, "utf-8");
+
+  test("detects useless errorMessage values with regex", () => {
+    assert.match(source, /success\|ok\|true\|error\|unknown/i);
+  });
+
+  test("extracts display message from content text block", () => {
+    assert.match(source, /textBlock/);
+    assert.match(source, /\.text\.slice\(0,\s*300\)/);
+  });
+
+  test("classifies using rawErrorMsg, not displayMsg", () => {
+    assert.match(source, /classifyError\(rawErrorMsg/);
+  });
+
+  test("references issue #3588 in comments", () => {
+    assert.match(source, /#3588/);
+  });
+});