fix(executor): un error interno de una herramienta dice qué tipo de falla fue y si conviene reintentar

packages/hosts/mcp/src/failure-category.test.ts

@@ -0,0 +1,103 @@
+import { describe, expect, it } from "@effect/vitest";
+import { Cause, Data } from "effect";
+
+import { classifyFailure, describeFailure } from "./failure-category";
+
+class WrappedError extends Data.TaggedError("WrappedError")<{
+  readonly message: string;
+  readonly cause: unknown;
+}> {}
+class OtherError extends Data.TaggedError("OtherError")<{ readonly message: string }> {}
+class RequestError extends Data.TaggedError("RequestError")<{ readonly message: string }> {}
+class ResponseError extends Data.TaggedError("ResponseError")<{
+  readonly message: string;
+  readonly status: number;
+}> {}
+class TimeoutException extends Data.TaggedError("TimeoutException")<{
+  readonly message: string;
+}> {}
+class CredentialResolutionError extends Data.TaggedError("CredentialResolutionError")<{
+  readonly message: string;
+}> {}
+
+describe("classifyFailure", () => {
+  it("classifies a timeout defect as retryable", () => {
+    expect(classifyFailure(Cause.die(new TimeoutException({ message: "x" })))).toEqual({
+      category: "upstream_timeout",
+      retryable: true,
+    });
+  });
+
+  it("classifies an AbortError defect as a timeout", () => {
+    const abort = new DOMException("The operation was aborted", "AbortError");
+    expect(classifyFailure(Cause.die(abort))).toEqual({
+      category: "upstream_timeout",
+      retryable: true,
+    });
+  });
+
+  it("classifies a transport failure as retryable network failure", () => {
+    expect(classifyFailure(Cause.fail(new RequestError({ message: "x" })))).toEqual({
+      category: "upstream_network",
+      retryable: true,
+    });
+  });
+
+  it("finds the category through a wrapped cause chain", () => {
+    const wrapped = new WrappedError({
+      message: "wrapper",
+      cause: new TimeoutException({ message: "inner" }),
+    });
+    expect(classifyFailure(Cause.die(wrapped)).category).toBe("upstream_timeout");
+  });
+
+  it("marks 5xx and 429 as retryable and 4xx as not", () => {
+    expect(classifyFailure(Cause.fail(new ResponseError({ message: "x", status: 503 })))).toEqual({
+      category: "upstream_http",
+      retryable: true,
+      status: 503,
+    });
+    expect(classifyFailure(Cause.fail(new ResponseError({ message: "x", status: 429 })))).toEqual({
+      category: "upstream_http",
+      retryable: true,
+      status: 429,
+    });
+    expect(classifyFailure(Cause.fail(new ResponseError({ message: "x", status: 404 })))).toEqual({
+      category: "upstream_http",
+      retryable: false,
+      status: 404,
+    });
+  });
+
+  it("classifies credential failures as auth and not retryable", () => {
+    expect(classifyFailure(Cause.fail(new CredentialResolutionError({ message: "x" })))).toEqual({
+      category: "auth",
+      retryable: false,
+    });
+  });
+
+  it("falls back to internal for anything else", () => {
+    expect(classifyFailure(Cause.die(new OtherError({ message: "boom" })))).toEqual({
+      category: "internal",
+      retryable: false,
+    });
+  });
+});
+
+describe("describeFailure", () => {
+  it("returns an empty suffix for internal failures", () => {
+    expect(describeFailure({ category: "internal", retryable: false })).toBe("");
+  });
+
+  it("names the tool and signals retry for a timeout", () => {
+    expect(
+      describeFailure({ category: "upstream_timeout", retryable: true }, "slack.org.main.history"),
+    ).toBe(": upstream timeout calling slack.org.main.history. Retry may succeed.");
+  });
+
+  it("includes the status and omits the retry hint when not retryable", () => {
+    expect(describeFailure({ category: "upstream_http", retryable: false, status: 404 })).toBe(
+      ": upstream returned HTTP 404.",
+    );
+  });
+});

packages/hosts/mcp/src/failure-category.ts

@@ -0,0 +1,81 @@
+import { Predicate } from "effect";
+import * as Cause from "effect/Cause";
+
+export type FailureCategory =
+  | "upstream_timeout"
+  | "upstream_network"
+  | "upstream_http"
+  | "auth"
+  | "internal";
+
+export interface FailureClassification {
+  readonly category: FailureCategory;
+  readonly retryable: boolean;
+  readonly status?: number;
+}
+
+const MAX_CAUSE_DEPTH = 6;
+const TIMEOUT_NAMES = new Set(["TimeoutError", "TimeoutException", "AbortError"]);
+const NETWORK_TAGS = new Set(["RequestError", "TransportError", "FetchError"]);
+const RESPONSE_TAGS = new Set(["ResponseError", "StatusCodeError"]);
+const AUTH_TAGS = new Set(["CredentialResolutionError", "OAuthRefreshError"]);
+
+const isRetryableStatus = (status: number): boolean => status === 429 || status >= 500;
+
+const hasName = (value: object, names: ReadonlySet<string>): boolean => {
+  const name = (value as { readonly name?: unknown }).name;
+  return typeof name === "string" && names.has(name);
+};
+
+const isOneOf = (value: object, tags: ReadonlySet<string>): boolean =>
+  hasName(value, tags) || [...tags].some((tag) => Predicate.isTagged(value, tag));
+
+const statusOf = (value: unknown): number | undefined => {
+  if (typeof value !== "object" || value === null) return undefined;
+  const status = (value as { readonly status?: unknown }).status;
+  return typeof status === "number" && status >= 400 && status < 600 ? status : undefined;
+};
+
+const classifyValue = (value: unknown, depth: number): FailureClassification | undefined => {
+  if (depth > MAX_CAUSE_DEPTH || typeof value !== "object" || value === null) return undefined;
+  if (isOneOf(value, TIMEOUT_NAMES)) return { category: "upstream_timeout", retryable: true };
+  if (isOneOf(value, NETWORK_TAGS)) return { category: "upstream_network", retryable: true };
+  if (isOneOf(value, AUTH_TAGS)) return { category: "auth", retryable: false };
+  const status = statusOf(value);
+  if (isOneOf(value, RESPONSE_TAGS) && status !== undefined) {
+    return { category: "upstream_http", retryable: isRetryableStatus(status), status };
+  }
+  return classifyValue((value as { readonly cause?: unknown }).cause, depth + 1);
+};
+
+/** Reads only error tags, names and numeric status codes, never messages, so
+ *  the result is safe to show to the sandbox. */
+export const classifyFailure = (cause: Cause.Cause<unknown>): FailureClassification => {
+  for (const reason of cause.reasons) {
+    const value = Cause.isFailReason(reason)
+      ? reason.error
+      : Cause.isDieReason(reason)
+        ? reason.defect
+        : undefined;
+    const found = classifyValue(value, 0);
+    if (found) return found;
+  }
+  return { category: "internal", retryable: false };
+};
+
+const SUMMARY: Record<Exclude<FailureCategory, "internal" | "upstream_http">, string> = {
+  upstream_timeout: "upstream timeout",
+  upstream_network: "upstream network failure",
+  auth: "authentication failure, reconnect the integration",
+};
+
+/** Text appended after `Internal tool error [id]`; empty when nothing safe is known. */
+export const describeFailure = (failure: FailureClassification, toolPath?: string): string => {
+  if (failure.category === "internal") return "";
+  const target = toolPath === undefined ? "" : ` calling ${toolPath}`;
+  const summary =
+    failure.category === "upstream_http"
+      ? `upstream returned HTTP ${failure.status}${target}`
+      : `${SUMMARY[failure.category]}${target}`;
+  return `: ${summary}.${failure.retryable ? " Retry may succeed." : ""}`;
+};

packages/hosts/mcp/src/tool-server.test.ts

@@ -31,6 +31,10 @@ class TestExecutionError extends Data.TaggedError("TestExecutionError")<{
   readonly message: string;
 }> {}
 
+class TestTimeoutException extends Data.TaggedError("TimeoutException")<{
+  readonly message: string;
+}> {}
+
 const expectString: (value: unknown) => asserts value is string = (value) => {
   expect(typeof value).toBe("string");
 };
@@ -775,6 +779,28 @@ describe("MCP host server — native elicitation mode", () => {
     });
   });
 
+  it("execute tool names a retryable upstream timeout without leaking the message", async () => {
+    const engine = makeStubEngine({
+      // oxlint-disable-next-line executor/no-effect-escape-hatch -- boundary: test injects a timeout defect to verify classification
+      execute: () =>
+        Effect.die(new TestTimeoutException({ message: "https://api.example.com/?token=secret" })),
+    });
+
+    await withNativeClient(engine, ELICITATION_CAPS, async (client) => {
+      const result = await client.callTool({ name: "execute", arguments: { code: "run" } });
+      const text = textOf(result);
+      expect(text).toMatch(
+        /^Error: Internal tool error \[[0-9a-f]{8}\]: upstream timeout\. Retry may succeed\.$/,
+      );
+      expect(text).not.toContain("secret");
+      expect(result.structuredContent).toMatchObject({
+        status: "error",
+        errorCategory: "upstream_timeout",
+        retryable: true,
+      });
+    });
+  });
+
   it("form elicitation is bridged from engine to MCP client and back", async () => {
     const engine = makeElicitingEngine(
       FormElicitation.make({

packages/hosts/mcp/src/tool-server.ts

@@ -90,6 +90,7 @@ import {
 import { MCP_ORG_WRITE_ACCESS_HEADER } from "./seams";
 import { passthroughInstructions, SEARCH_INVOKE_SKILL } from "./passthrough-tools";
 import type { McpToolMode } from "./browser-approval";
+import { classifyFailure, describeFailure } from "./failure-category";
 import { logToolCall, trackEngineRun, type McpCodeRuntime } from "./tool-call-log";
 
 // ---------------------------------------------------------------------------
@@ -862,8 +863,9 @@ const formatResumeApprovalRequired = (input: {
   },
 });
 
-const toMcpFailureResult = (cause: Cause.Cause<unknown>): McpToolResult => {
+const toMcpFailureResult = (cause: Cause.Cause<unknown>, toolPath?: string): McpToolResult => {
   const correlationId = newCorrelationId();
+  const failure = classifyFailure(cause);
   const defect = Cause.findDefect(cause);
   const nativeElicitationFailed =
     Result.isSuccess(defect) &&
@@ -872,19 +874,25 @@ const toMcpFailureResult = (cause: Cause.Cause<unknown>): McpToolResult => {
   try {
     console.error(`[executor:mcp] execute defect correlation_id=${correlationId}`, {
       nativeElicitationFailed,
+      category: failure.category,
+      retryable: failure.retryable,
+      tool: toolPath,
+      cause: Cause.pretty(cause),
     });
   } catch {
     /* ignore logger failures */
   }
   const text = nativeElicitationFailed
     ? `Native elicitation transport failed [${correlationId}]. Reconnect the MCP client and try again.`
-    : `Internal tool error [${correlationId}]`;
+    : `Internal tool error [${correlationId}]${describeFailure(failure, toolPath)}`;
   return {
     content: [{ type: "text", text: `Error: ${text}` }],
     structuredContent: {
       status: "error",
       error: text,
       ...(nativeElicitationFailed ? { errorCode: "native_elicitation_transport_failed" } : {}),
+      ...(failure.category === "internal" ? {} : { errorCategory: failure.category }),
+      ...(failure.category === "internal" ? {} : { retryable: failure.retryable }),
     },
     isError: true,
   };
@@ -1329,14 +1337,15 @@ const registerPassthroughTools = <E extends Cause.YieldableError>(
     const boundary = <A extends McpToolResult, F>(
       effect: Effect.Effect<A, F>,
       extra: McpRequestJoinKeys,
+      toolPath?: string,
     ) =>
       Effect.runPromiseWith(context)(
         effect.pipe(
           Effect.provideService(
             CurrentOrgWriteAccess,
             makeOrgWriteAccessState(requestOrgWriteAccess(extra)),
           ),
-          Effect.catchCause((cause) => Effect.succeed(toMcpFailureResult(cause))),
+          Effect.catchCause((cause) => Effect.succeed(toMcpFailureResult(cause, toolPath))),
         ),
       );
     yield* Effect.sync(() => {
@@ -1574,6 +1583,7 @@ const registerPassthroughTools = <E extends Cause.YieldableError>(
               return yield* run(address, checked.data, extra);
             }),
             extra,
+            id,
           ),
       );
     });