fix(server): surface real errors and child-process output in diagnostics

Provider diagnostics were stringifying Error objects through JSON.stringify, which yields {} and hides the cause. Spawned children logged stderr to the daemon log but never attached it to thrown errors, so the diagnostic showed "Server: Unavailable ({})" with no clue why. Generalize toDiagnosticErrorMessage to extract message, code, signal, stderr, stdout, and recursive cause from any Error (including execFile-style errors), and never return {} or empty. Buffer stderr/stdout in OpenCode startServer and include them in startup-failure rejections. Stop swallowing errors in resolveBinaryVersion. Drop the duplicate stringifyUnknownError helpers in opencode-agent and pi-direct-agent and route every callsite through the single canonical helper.
2026-04-28 11:58:19 +07:00
parent 221b76bf52
commit 05d279cc65
4 changed files with 216 additions and 54 deletions
@@ -0,0 +1,78 @@
+import { describe, expect, test } from "vitest";
+
+import { toDiagnosticErrorMessage } from "./diagnostic-utils.js";
+
+describe("toDiagnosticErrorMessage", () => {
+  test("returns message for plain Error", () => {
+    expect(toDiagnosticErrorMessage(new Error("boom"))).toBe("boom");
+  });
+
+  test("includes stderr, stdout, code, and signal from execFile-style Error", () => {
+    const error = new Error("Command failed: opencode --version") as Error & {
+      stderr: string;
+      stdout: string;
+      code: number;
+      signal: string;
+    };
+    error.stderr = "permission denied\n";
+    error.stdout = "partial output";
+    error.code = 127;
+    error.signal = "SIGTERM";
+
+    const message = toDiagnosticErrorMessage(error);
+    expect(message).toContain("Command failed: opencode --version");
+    expect(message).toContain("exit code: 127");
+    expect(message).toContain("signal: SIGTERM");
+    expect(message).toContain("stderr: permission denied");
+    expect(message).toContain("stdout: partial output");
+  });
+
+  test("preserves multi-line stderr on a real Error", () => {
+    const error = new Error("Command failed") as Error & { stderr: string };
+    error.stderr = "line one\nline two\nline three";
+    const message = toDiagnosticErrorMessage(error);
+    expect(message).toContain("stderr: line one\nline two\nline three");
+  });
+
+  test("recursively formats Error cause", () => {
+    const inner = new Error("inner failure") as Error & { stderr: string };
+    inner.stderr = "inner stderr";
+    const outer = new Error("outer failure", { cause: inner });
+    const message = toDiagnosticErrorMessage(outer);
+    expect(message).toContain("outer failure");
+    expect(message).toContain("caused by: inner failure");
+    expect(message).toContain("stderr: inner stderr");
+  });
+
+  test("serializes plain objects rather than returning {}", () => {
+    expect(toDiagnosticErrorMessage({ foo: "bar" })).toBe('{"foo":"bar"}');
+  });
+
+  test("returns trimmed string when given a string", () => {
+    expect(toDiagnosticErrorMessage("oops")).toBe("oops");
+    expect(toDiagnosticErrorMessage("  spaced  ")).toBe("spaced");
+  });
+
+  test("returns Unknown error for null and undefined", () => {
+    expect(toDiagnosticErrorMessage(null)).toBe("Unknown error");
+    expect(toDiagnosticErrorMessage(undefined)).toBe("Unknown error");
+  });
+
+  test("returns Unknown error for an Error with no message and no extras", () => {
+    const error = new Error("");
+    expect(toDiagnosticErrorMessage(error)).toBe("Unknown error");
+  });
+
+  test("truncates very long stderr", () => {
+    const long = "x".repeat(5000);
+    const error = new Error("big") as Error & { stderr: string };
+    error.stderr = long;
+    const message = toDiagnosticErrorMessage(error);
+    expect(message).toContain("…(truncated)");
+    expect(message.length).toBeLessThan(long.length + 200);
+  });
+
+  test("returns Unknown error for empty plain object after serialization fallback", () => {
+    expect(toDiagnosticErrorMessage({})).toBe("Unknown error");
+  });
+});
@@ -14,7 +14,7 @@ export function formatProviderDiagnosticError(providerName: string, error: unkno
  return formatProviderDiagnostic(providerName, [
    {
      label: "Error",
-      value: error instanceof Error ? error.message : String(error),
+      value: toDiagnosticErrorMessage(error),
    },
  ]);
 }
@@ -33,16 +33,97 @@ export function formatDiagnosticStatus(
  return formatAvailabilityStatus(available);
 }

-export function toDiagnosticErrorMessage(error: unknown): string {
-  if (error instanceof Error && error.message) {
-    return error.message;
+const DIAGNOSTIC_OUTPUT_CAP = 4096;
+
+function truncateForDiagnostic(value: string): string {
+  const trimmed = value.trim();
+  if (trimmed.length <= DIAGNOSTIC_OUTPUT_CAP) {
+    return trimmed;
  }
-  if (typeof error === "string" && error.trim().length > 0) {
-    return error;
+  return `${trimmed.slice(0, DIAGNOSTIC_OUTPUT_CAP)}…(truncated)`;
+}
+
+function readStringProperty(error: object, key: string): string | undefined {
+  const value = (error as Record<string, unknown>)[key];
+  if (typeof value === "string") {
+    return value;
+  }
+  if (typeof value === "number" || typeof value === "bigint") {
+    return String(value);
+  }
+  return undefined;
+}
+
+function readUnknownProperty(error: object, key: string): unknown {
+  return (error as Record<string, unknown>)[key];
+}
+
+function pushIfNonEmpty(sections: string[], label: string, value: string | undefined): void {
+  if (value && value.trim().length > 0) {
+    sections.push(`${label}: ${value.trim()}`);
+  }
+}
+
+function pushTruncatedIfNonEmpty(
+  sections: string[],
+  label: string,
+  value: string | undefined,
+): void {
+  if (value && value.trim().length > 0) {
+    sections.push(`${label}: ${truncateForDiagnostic(value)}`);
+  }
+}
+
+function formatErrorDiagnostic(error: Error): string {
+  const sections: string[] = [];
+  if (error.message && error.message.trim().length > 0) {
+    sections.push(error.message.trim());
+  }
+  pushIfNonEmpty(sections, "exit code", readStringProperty(error, "code"));
+  pushIfNonEmpty(sections, "signal", readStringProperty(error, "signal"));
+  pushTruncatedIfNonEmpty(sections, "stderr", readStringProperty(error, "stderr"));
+  pushTruncatedIfNonEmpty(sections, "stdout", readStringProperty(error, "stdout"));
+  const cause = readUnknownProperty(error, "cause");
+  if (cause !== undefined && cause !== null) {
+    const causeMessage = toDiagnosticErrorMessage(cause);
+    if (causeMessage && causeMessage !== "Unknown error") {
+      sections.push(`caused by: ${causeMessage}`);
+    }
+  }
+  return sections.length > 0 ? sections.join("\n") : "Unknown error";
+}
+
+function formatNonErrorDiagnostic(error: unknown): string {
+  try {
+    const serialized = JSON.stringify(error);
+    if (serialized && serialized !== "{}" && serialized !== '""') {
+      return serialized;
+    }
+  } catch {
+    // fall through to String() below
+  }
+
+  const stringified = String(error);
+  if (stringified.length > 0 && stringified !== "[object Object]") {
+    return stringified;
  }
  return "Unknown error";
 }

+export function toDiagnosticErrorMessage(error: unknown): string {
+  if (error instanceof Error) {
+    return formatErrorDiagnostic(error);
+  }
+  if (typeof error === "string") {
+    const trimmed = error.trim();
+    return trimmed.length > 0 ? trimmed : "Unknown error";
+  }
+  if (error === null || error === undefined) {
+    return "Unknown error";
+  }
+  return formatNonErrorDiagnostic(error);
+}
+
 export async function resolveBinaryVersion(binaryPath: string): Promise<string> {
  try {
    const { stdout } = await execCommand(binaryPath, ["--version"], {
@@ -50,8 +131,8 @@ export async function resolveBinaryVersion(binaryPath: string): Promise<string>
      timeout: 5_000,
    });
    return stdout.trim() || "unknown";
-  } catch {
-    return "unknown";
+  } catch (error) {
+    return `error: ${toDiagnosticErrorMessage(error)}`;
  }
 }

@@ -225,22 +225,6 @@ function toOpenCodeMcpConfig(config: McpServerConfig): OpenCodeMcpConfig {
  };
 }

-function stringifyUnknownError(error: unknown): string {
-  if (typeof error === "string") {
-    return error;
-  }
-  try {
-    return JSON.stringify(error);
-  } catch {
-    return String(error);
-  }
-}
-
-function normalizeTurnFailureError(error: unknown): string {
-  const normalized = stringifyUnknownError(error).trim();
-  return normalized.length > 0 ? normalized : "Unknown error";
-}
-
 type TerminalTurnEvent = Extract<
  AgentStreamEvent,
  { type: "turn_completed" | "turn_failed" | "turn_canceled" }
@@ -251,7 +235,7 @@ function toTerminalTurnEvent(event: AgentStreamEvent): TerminalTurnEvent | null
    return {
      type: "turn_failed",
      provider: "opencode",
-      error: normalizeTurnFailureError(event.error),
+      error: toDiagnosticErrorMessage(event.error),
    };
  }
  if (event.type === "turn_completed" || event.type === "turn_canceled") {
@@ -286,7 +270,7 @@ async function reconcileOpenCodeSessionClose(params: {
      logger.warn(
        {
          sessionId,
-          error: normalizeTurnFailureError(response.error),
+          error: toDiagnosticErrorMessage(response.error),
        },
        "Failed to abort OpenCode session during close",
      );
@@ -295,7 +279,7 @@ async function reconcileOpenCodeSessionClose(params: {
    logger.warn(
      {
        sessionId,
-        error: normalizeTurnFailureError(error),
+        error: toDiagnosticErrorMessage(error),
      },
      "Failed to abort OpenCode session during close",
    );
@@ -311,7 +295,7 @@ async function reconcileOpenCodeSessionClose(params: {
      logger.warn(
        {
          sessionId,
-          error: normalizeTurnFailureError(response.error),
+          error: toDiagnosticErrorMessage(response.error),
        },
        "Failed to archive OpenCode session during close",
      );
@@ -320,7 +304,7 @@ async function reconcileOpenCodeSessionClose(params: {
    logger.warn(
      {
        sessionId,
-        error: normalizeTurnFailureError(error),
+        error: toDiagnosticErrorMessage(error),
      },
      "Failed to archive OpenCode session during close",
    );
@@ -345,7 +329,7 @@ function isOpenCodeHeadersTimeoutFailure(error: unknown): boolean {
      continue;
    }

-    const normalized = stringifyUnknownError(current).trim().toLowerCase();
+    const normalized = toDiagnosticErrorMessage(current).trim().toLowerCase();
    if (normalized) {
      diagnostics.add(normalized);
    }
@@ -380,7 +364,7 @@ function isOpenCodeHeadersTimeoutFailure(error: unknown): boolean {
 }

 function isAlreadyPresentMcpError(error: unknown): boolean {
-  const normalized = stringifyUnknownError(error).toLowerCase();
+  const normalized = toDiagnosticErrorMessage(error).toLowerCase();
  return MCP_ALREADY_PRESENT_ERROR_TOKENS.some((token) => normalized.includes(token));
 }

@@ -873,14 +857,37 @@ export class OpenCodeServerManager {
      );

      let started = false;
+      let stderrBuffer = "";
+      let stdoutBuffer = "";
+      const STARTUP_BUFFER_CAP = 8192;
+      const appendCapped = (current: string, chunk: string): string => {
+        if (current.length >= STARTUP_BUFFER_CAP) {
+          return current;
+        }
+        const remaining = STARTUP_BUFFER_CAP - current.length;
+        return current + chunk.slice(0, remaining);
+      };
+      const buildStartupErrorMessage = (headline: string): string => {
+        const sections = [headline];
+        const stderrTrimmed = stderrBuffer.trim();
+        if (stderrTrimmed.length > 0) {
+          sections.push(`stderr: ${stderrTrimmed}`);
+        }
+        const stdoutTrimmed = stdoutBuffer.trim();
+        if (stdoutTrimmed.length > 0) {
+          sections.push(`stdout: ${stdoutTrimmed}`);
+        }
+        return sections.join("\n");
+      };
      const timeout = setTimeout(() => {
        if (!started) {
-          reject(new Error("OpenCode server startup timeout"));
+          reject(new Error(buildStartupErrorMessage("OpenCode server startup timeout")));
        }
      }, 30_000);

      serverProcess.stdout?.on("data", (data: Buffer) => {
        const output = data.toString();
+        stdoutBuffer = appendCapped(stdoutBuffer, output);
        if (output.includes("listening on") && !started) {
          started = true;
          clearTimeout(timeout);
@@ -895,18 +902,21 @@ export class OpenCodeServerManager {
      });

      serverProcess.stderr?.on("data", (data: Buffer) => {
-        this.logger.error({ stderr: data.toString().trim() }, "OpenCode server stderr");
+        const output = data.toString();
+        stderrBuffer = appendCapped(stderrBuffer, output);
+        this.logger.error({ stderr: output.trim() }, "OpenCode server stderr");
      });

      serverProcess.on("error", (error) => {
        clearTimeout(timeout);
-        reject(error);
+        const headline = error instanceof Error ? error.message : String(error);
+        reject(new Error(buildStartupErrorMessage(headline)));
      });

      serverProcess.on("exit", (code) => {
        if (!started) {
          clearTimeout(timeout);
-          reject(new Error(`OpenCode server exited with code ${code}`));
+          reject(new Error(buildStartupErrorMessage(`OpenCode server exited with code ${code}`)));
        }
        if (this.currentServer?.process === serverProcess) {
          this.currentServer = null;
@@ -1195,7 +1205,7 @@ export class OpenCodeAgentClient implements AgentClient {
        const { url } = await this.serverManager.ensureRunning();
        serverStatus = `Running (${url})`;
      } catch (error) {
-        serverStatus = `Unavailable (${normalizeTurnFailureError(error)})`;
+        serverStatus = `Unavailable (${toDiagnosticErrorMessage(error)})`;
      }

      if (available) {
@@ -1504,7 +1514,7 @@ export function translateOpenCodeEvent(
        events.push({
          type: "turn_failed",
          provider: "opencode",
-          error: normalizeTurnFailureError(event.properties.error),
+          error: toDiagnosticErrorMessage(event.properties.error),
        });
      }
      break;
@@ -1821,7 +1831,7 @@ function appendOpenCodeSessionStatus(
    events.push({
      type: "turn_failed",
      provider: "opencode",
-      error: normalizeTurnFailureError(status.message),
+      error: toDiagnosticErrorMessage(status.message),
    });
  }
  // "retry" and "busy" are transient — no terminal event.
@@ -2054,7 +2064,7 @@ class OpenCodeAgentSession implements AgentSession {
              );
              return;
            }
-            const errorMsg = normalizeTurnFailureError(response.error);
+            const errorMsg = toDiagnosticErrorMessage(response.error);
            this.finishForegroundTurn(
              { type: "turn_failed", provider: "opencode", error: errorMsg },
              turnId,
@@ -2080,7 +2090,7 @@ class OpenCodeAgentSession implements AgentSession {
            return;
          }
          this.finishForegroundTurn(
-            { type: "turn_failed", provider: "opencode", error: normalizeTurnFailureError(err) },
+            { type: "turn_failed", provider: "opencode", error: toDiagnosticErrorMessage(err) },
            turnId,
          );
        });
@@ -2109,7 +2119,7 @@ class OpenCodeAgentSession implements AgentSession {
              {
                type: "turn_failed",
                provider: "opencode",
-                error: normalizeTurnFailureError(promptResponse.error),
+                error: toDiagnosticErrorMessage(promptResponse.error),
              },
              turnId,
            );
@@ -2121,7 +2131,7 @@ class OpenCodeAgentSession implements AgentSession {
            {
              type: "turn_failed",
              provider: "opencode",
-              error: normalizeTurnFailureError(error),
+              error: toDiagnosticErrorMessage(error),
            },
            turnId,
          );
@@ -2186,7 +2196,7 @@ class OpenCodeAgentSession implements AgentSession {
          {
            type: "turn_failed",
            provider: "opencode",
-            error: normalizeTurnFailureError(error),
+            error: toDiagnosticErrorMessage(error),
          },
          turnId,
        );
@@ -2585,7 +2595,7 @@ class OpenCodeAgentSession implements AgentSession {
    }

    throw new Error(
-      `Failed to ${operation} OpenCode MCP server '${name}': ${stringifyUnknownError(error)}`,
+      `Failed to ${operation} OpenCode MCP server '${name}': ${toDiagnosticErrorMessage(error)}`,
    );
  }

@@ -624,13 +624,6 @@ function mapToolDetail(toolCall: PiTrackedToolCall, result?: PiToolResult): Tool
  }
 }

-function stringifyUnknownError(error: unknown): string {
-  if (error instanceof Error) {
-    return error.message;
-  }
-  return typeof error === "string" ? error : "Unknown Pi error";
-}
-
 function parseModelReference(modelId: string | null): PiModelReference | null {
  if (!modelId) {
    return null;
@@ -756,7 +749,7 @@ function isPiRequestAbortError(error: unknown): boolean {
    return true;
  }

-  return /\brequest was aborted\b/i.test(stringifyUnknownError(error));
+  return /\brequest was aborted\b/i.test(toDiagnosticErrorMessage(error));
 }

 function resolveThinkingOptionId(
@@ -1113,7 +1106,7 @@ export class PiDirectAgentSession implements AgentSession {
            type: "turn_canceled",
            provider: PI_PROVIDER,
            turnId: failedTurnId,
-            reason: stringifyUnknownError(error),
+            reason: toDiagnosticErrorMessage(error),
          });
          return;
        }
@@ -1121,7 +1114,7 @@ export class PiDirectAgentSession implements AgentSession {
          type: "turn_failed",
          provider: PI_PROVIDER,
          turnId: failedTurnId,
-          error: stringifyUnknownError(error),
+          error: toDiagnosticErrorMessage(error),
        });
      });