From 054f807925cbe8eb73ac8cbb6a7226ab2f366f73 Mon Sep 17 00:00:00 2001 From: Ramnique Singh <30795890+ramnique@users.noreply.github.com> Date: Sat, 11 Jul 2026 12:31:47 +0530 Subject: [PATCH] fix(x): record builtin error envelopes as isError in durable results MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Builtins signal failure by returning an envelope ({error}, {success: false, error|message}, composio's {successful: false}) instead of throwing, and the tool-registry bridge marked every non-throwing result isError: false — so the durable log recorded most builtin failures as successes, misleading the reducer's record, the renderer's error state, telemetry, and the tools_extended gate. The spec's ToolResultData.isError channel was effectively populated only by throws. The bridge now detects the envelope at its single chokepoint and sets isError accordingly. Model-facing bytes are unchanged (encoding ignores isError). Deliberately not an error: executeCommand's success:false for a plain non-zero exit with no error/message — a failing grep is a result, not a tool failure. Composio success passthrough (error: null) stays non-error via the truthy-string check. The authoring convention is documented on the catalog schema. Co-Authored-By: Claude Fable 5 --- .../packages/core/src/runtime/tools/types.ts | 6 ++ .../turns/bridges/real-tool-registry.test.ts | 58 +++++++++++++++++++ .../turns/bridges/real-tool-registry.ts | 29 +++++++++- 3 files changed, 92 insertions(+), 1 deletion(-) diff --git a/apps/x/packages/core/src/runtime/tools/types.ts b/apps/x/packages/core/src/runtime/tools/types.ts index 0f8cebb1..4c30f06a 100644 --- a/apps/x/packages/core/src/runtime/tools/types.ts +++ b/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"; diff --git a/apps/x/packages/core/src/runtime/turns/bridges/real-tool-registry.test.ts b/apps/x/packages/core/src/runtime/turns/bridges/real-tool-registry.test.ts index 59a44fbb..892769bc 100644 --- a/apps/x/packages/core/src/runtime/turns/bridges/real-tool-registry.test.ts +++ b/apps/x/packages/core/src/runtime/turns/bridges/real-tool-registry.test.ts @@ -85,6 +85,64 @@ function makeRegistry(execImpl: (call: ExecCall) => Promise) { 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 })); diff --git a/apps/x/packages/core/src/runtime/turns/bridges/real-tool-registry.ts b/apps/x/packages/core/src/runtime/turns/bridges/real-tool-registry.ts index 679b4329..fef09f46 100644 --- a/apps/x/packages/core/src/runtime/turns/bridges/real-tool-registry.ts +++ b/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