Merge pull request #733 from rowboatlabs/fix/builtin-error-envelope

fix(x): record builtin error envelopes as isError in durable results

apps/x/packages/core/src/runtime/tools/types.ts

@@ -1,6 +1,12 @@
 // The builtin-tool catalog schema: every entry is {description, inputSchema,
 // execute, permission, isAvailable?}. Shared typing for the domain modules
 // and the merged catalog.
+//
+// Failure convention: builtins return an error envelope instead of throwing —
+// `{ error: "…" }`, `{ success: false, error|message: "…" }`, or composio's
+// `{ successful: false }`. The turns tool-registry bridge maps those to the
+// durable result's isError flag, so don't put a top-level `error` string on
+// a SUCCESS return.
 
 import { z, ZodType } from "zod";
 

apps/x/packages/core/src/runtime/turns/bridges/real-tool-registry.test.ts

@@ -85,6 +85,64 @@ function makeRegistry(execImpl: (call: ExecCall) => Promise<unknown>) {
     return { registry, calls, abortRegistry };
 }
 
+describe("error-envelope mapping", () => {
+    async function resultFor(value: unknown) {
+        const { registry } = makeRegistry(async () => value);
+        const tool = (await registry.resolve(descriptor())) as SyncRuntimeTool;
+        return tool.execute({}, makeCtx());
+    }
+
+    it("maps a bare error envelope to isError", async () => {
+        const result = await resultFor({ error: "File not found: /x" });
+        expect(result.isError).toBe(true);
+        expect(result.output).toEqual({ error: "File not found: /x" });
+    });
+
+    it("maps success:false with a message to isError", async () => {
+        const result = await resultFor({
+            success: false,
+            message: "Failed to analyze agent: boom",
+        });
+        expect(result.isError).toBe(true);
+    });
+
+    it("maps composio's successful:false envelope to isError", async () => {
+        const result = await resultFor({
+            successful: false,
+            data: null,
+            error: null,
+        });
+        expect(result.isError).toBe(true);
+    });
+
+    it("a non-zero exit code is a result, not a tool failure", async () => {
+        const result = await resultFor({
+            success: false,
+            stdout: "",
+            stderr: "no matches",
+            exitCode: 1,
+            command: "grep needle haystack",
+        });
+        expect(result.isError).toBe(false);
+    });
+
+    it("success shapes stay non-error, including composio passthrough", async () => {
+        for (const value of [
+            { ok: 1 },
+            { success: true, analysis: "fine" },
+            { successful: true, data: { id: 1 }, error: null },
+            "plain text",
+            ["a", "b"],
+            null,
+            { error: "" },
+            { error: null },
+        ]) {
+            const result = await resultFor(value);
+            expect(result.isError).toBe(false);
+        }
+    });
+});
+
 describe("RealToolRegistry", () => {
     it("executes builtins through execTool with a turn-scoped ToolContext", async () => {
         const { registry, calls, abortRegistry } = makeRegistry(async () => ({ ok: 1 }));

apps/x/packages/core/src/runtime/turns/bridges/real-tool-registry.ts

@@ -220,7 +220,7 @@ export class RealToolRegistry implements IToolRegistry {
                     );
                     return {
                         output,
-                        isError: false,
+                        isError: isErrorEnvelope(output),
                         ...(additions === undefined
                             ? {}
                             : { metadata: { toolAdditions: additions } }),
@@ -234,6 +234,33 @@ export class RealToolRegistry implements IToolRegistry {
     }
 }
 
+// Builtins signal failure by returning an envelope rather than throwing:
+//   { error: "…" }                          (files, web, mcp, models, …)
+//   { success: false, error|message: "…" }  (app, browser, loadSkill, …)
+//   { successful: false, … }                (composio's API envelope)
+// Map those to isError so the durable log records failures as failures —
+// the model already sees the error text either way (encoding ignores
+// isError), but the reducer, renderer, telemetry, and the tools_extended
+// gate all key off the flag. Deliberately NOT an error: executeCommand's
+// success:false for a plain non-zero exit (stdout/stderr/exitCode, no
+// error/message) — a failing grep is a result, not a tool failure.
+function isErrorEnvelope(output: JsonValue): boolean {
+    if (output === null || typeof output !== "object" || Array.isArray(output)) {
+        return false;
+    }
+    const record = output as { [key: string]: JsonValue };
+    if (typeof record.error === "string" && record.error.length > 0) {
+        return true;
+    }
+    if (record.successful === false) {
+        return true;
+    }
+    if (record.success === false) {
+        return typeof record.message === "string" && record.message.length > 0;
+    }
+    return false;
+}
+
 // Lift a builtin's reserved tool-additions key out of the model-visible
 // output into result metadata: the model gets the added tools as native
 // definitions on its next call, so repeating their schemas inside the tool