From d1b6866a0531cc4e83d177bc71e4254af3c55d84 Mon Sep 17 00:00:00 2001 From: "Builder.io" Date: Tue, 18 Aug 2026 11:09:18 +0000 Subject: [PATCH 1/2] fix: report the gateway 500 under one code and stop re-dispatching it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Builder gateway answers 200 and then delivers its internal-error envelope ("Sorry, we ran into an issue processing your request. ERROR ID: …") as an in-stream frame, so nothing structured reaches the layer that catches it. Both catch sites in run-loop-with-resume fell straight to `internal_error` without asking the shared classifier, so one upstream failure was persisted under three codes depending on which layer caught it. Only one of the three is on the client's recoverable list, so identical gateway failures ended some chats on the first attempt and sent others into a re-dispatch chain. That envelope is no longer auto-recoverable at the run level. Measured across the analytics, clips and calendar production databases, 7 turns reached it and 0 of 7 ever finished, across 97 runs — one turn burned 28 runs over 16 minutes on a single "Hey", with overlapping concurrent runs on the same turn. The engine's in-request retry still covers the transient case, and the recovery card keeps a deliberate Retry because its own copy ends with "Retry in a moment". The fallback Zod-to-JSON-Schema converter read a literal's value from `def.value`, which Zod v4 does not define — it stores `values`. Every literal emitted `{"type":"undefined"}`, a type keyword no JSON Schema dialect defines. The gateway validates only a tool's top-level `input_schema.type`, so an invented type on a nested field passes validation, reaches the provider, and returns as the same opaque ERROR ID envelope on every retry. Reproduced 6/6 against the live gateway. --- .changeset/gateway-internal-error-one-code.md | 34 +++++++ packages/core/src/action.spec.ts | 88 +++++++++++++++++++ packages/core/src/action.ts | 70 +++++++++++++-- .../src/agent/run-loop-with-resume.spec.ts | 63 +++++++++++++ .../core/src/agent/run-loop-with-resume.ts | 45 ++++++++-- .../core/src/client/chat/run-recovery.tsx | 15 +++- .../src/client/sse-event-processor.spec.ts | 56 ++++++++---- .../core/src/client/sse-event-processor.ts | 18 +++- 8 files changed, 351 insertions(+), 38 deletions(-) create mode 100644 .changeset/gateway-internal-error-one-code.md diff --git a/.changeset/gateway-internal-error-one-code.md b/.changeset/gateway-internal-error-one-code.md new file mode 100644 index 0000000000..3cd1c5052c --- /dev/null +++ b/.changeset/gateway-internal-error-one-code.md @@ -0,0 +1,34 @@ +--- +"@agent-native/core": patch +--- + +Stop the Builder gateway's 500 envelope from ending chats three different ways, and stop emitting the malformed tool schema that provokes it. + +The gateway answers 200 and then delivers its own internal-error envelope ("Sorry, +we ran into an issue processing your request. ERROR ID: …") as an in-stream frame, +so nothing structured reaches the layer that catches it. `runAgentLoopWithResume` +dropped straight to `internal_error` without asking the shared classifier, so one +upstream failure was persisted under three codes depending on which layer caught it +— `builder_gateway_internal_error`, `internal_error`, `unknown` — and only one of +those is on the client's recoverable list. Identical gateway failures therefore +ended some chats on the first attempt and sent others into a re-dispatch chain. +Both catch sites now classify the message first; `internal_error` stays the last +resort for a message nothing recognises. + +That envelope is also no longer auto-recoverable at the run level. Measured across +three production databases, 7 turns reached it and 0 of 7 ever finished, across 97 +runs — one turn burned 28 runs over 16 minutes on a single "Hey", with overlapping +concurrent runs on the same turn. The engine's in-request retry still covers the +genuinely transient case, and the recovery card keeps a deliberate Retry, because +its own copy ends with "Retry in a moment". + +The fallback Zod-to-JSON-Schema converter read a literal's value from `def.value`, +which Zod v4 does not define — it stores `values`. Every literal therefore emitted +`{"type":"undefined"}`, a type keyword no JSON Schema dialect defines, and an +`enum` of `[null]` instead of the real value. Literals are most often a +discriminated union's discriminator, so ordinary action schemas were affected. The +Builder gateway validates only a tool's top-level `input_schema.type`, so an +invented type on a nested field passes validation, reaches the provider, and comes +back as the opaque ERROR ID envelope — on every retry, because the same malformed +schema is resent verbatim. Literal values are now read from `values`, and no +literal can emit a type outside the seven JSON Schema names. diff --git a/packages/core/src/action.spec.ts b/packages/core/src/action.spec.ts index 885c006c64..93e301897b 100644 --- a/packages/core/src/action.spec.ts +++ b/packages/core/src/action.spec.ts @@ -353,6 +353,94 @@ describe("defineAction schema mode — tool parameter JSON Schema", () => { expect(params.properties.title.description).toBe("Form title"); }); + // JSON Schema has exactly seven type names and JavaScript's `typeof` is not a + // way to spell them. The Builder gateway validates only a tool's TOP-LEVEL + // `input_schema.type`, so an invented type on a NESTED field passes validation, + // reaches the provider, and comes back as the gateway's opaque "ERROR ID" 500 + // envelope — on every retry, because the same malformed schema is resent. + it("never emits a non-JSON-Schema type for a literal", () => { + const legalTypes = new Set([ + "string", + "number", + "integer", + "boolean", + "object", + "array", + "null", + ]); + const action = defineAction({ + description: "literal types", + schema: z.object({ + nothing: z.literal(null), + name: z.literal("fixed"), + count: z.literal(7), + ratio: z.literal(1.5), + flag: z.literal(true), + nested: z.object({ inner: z.literal(null) }), + items: z.array(z.literal(null)), + }), + run: async () => "ok", + }); + + // The fallback converter is what runs for a schema that exposes no + // standard-schema `jsonSchema` hook, so exercise that path explicitly rather + // than only Zod's own converter — the bug lived exclusively in the fallback. + const withoutJsonSchemaHook = (schema: T): T => { + const standard = (schema as any)["~standard"]; + (schema as any)["~standard"] = { ...standard, jsonSchema: undefined }; + return schema; + }; + const fallbackAction = defineAction({ + description: "literal types via fallback converter", + schema: withoutJsonSchemaHook( + z.object({ + kind: z.literal("weekly"), + nested: z.object({ inner: z.literal(3) }), + mode: z.union([z.literal("a"), z.literal("b")]), + }), + ), + run: async () => "ok", + }); + + const illegal: string[] = []; + const walk = (node: unknown) => { + if (!node || typeof node !== "object") return; + if (Array.isArray(node)) { + node.forEach(walk); + return; + } + const record = node as Record; + const type = record.type; + if (typeof type === "string" && !legalTypes.has(type)) illegal.push(type); + Object.values(record).forEach(walk); + }; + walk(action.tool.parameters); + walk(fallbackAction.tool.parameters); + + expect(illegal).toEqual([]); + const props = (action.tool.parameters as any).properties; + expect(props.nothing).toMatchObject({ type: "null" }); + expect(props.name).toMatchObject({ type: "string" }); + expect(props.flag).toMatchObject({ type: "boolean" }); + + // The fallback path must also carry the literal's VALUE. Reading the + // singular `def.value` returned undefined, which serialized to `[null]` and + // told the model the discriminator accepts null. + const fallbackProps = (fallbackAction.tool.parameters as any).properties; + expect(fallbackProps.kind).toMatchObject({ + type: "string", + enum: ["weekly"], + }); + expect(fallbackProps.nested.properties.inner).toMatchObject({ + type: "integer", + enum: [3], + }); + expect(fallbackProps.mode).toMatchObject({ + type: "string", + enum: ["a", "b"], + }); + }); + it("strips the $schema key so the Claude API (draft 2020-12) does not reject it", () => { const action = defineAction({ description: "with schema key", diff --git a/packages/core/src/action.ts b/packages/core/src/action.ts index 191101d408..cb1ac0e4ae 100644 --- a/packages/core/src/action.ts +++ b/packages/core/src/action.ts @@ -1253,6 +1253,53 @@ function schemaToJsonSchema( return { type: "object" as const, properties: {} }; } +/** + * JSON Schema recognises exactly seven type names, and JavaScript's `typeof` is + * not one of the ways to spell them: `typeof null` is `"object"`, and `bigint`, + * `undefined`, `symbol`, and `function` have no JSON Schema equivalent at all. + * Emitting one of those as a `type` produces a schema the Builder gateway's + * request validator accepts — it only checks a tool's TOP-LEVEL + * `input_schema.type` — and the upstream provider then rejects, which comes back + * as the gateway's opaque "ERROR ID" 500 envelope rather than a typed 400. A + * nested field is therefore able to take down a whole turn with an error that + * names nothing, on every retry, because the malformed schema is resent verbatim. + * + * Return no `type` at all rather than an invented one: `enum` alone still pins + * the value, and an absent keyword is valid where a bogus one is not. + */ +/** + * Zod v4 stores a literal's value in `def.values` (an array — `z.literal` accepts + * a set of values), NOT in `def.value`. Reading the singular key returns + * `undefined` for every literal, which is how `typeof def.value` came to emit + * `{"type":"undefined"}`: a type keyword no JSON Schema dialect defines. A + * literal is most often a discriminated union's discriminator, so this was + * reached by ordinary action schemas, not an exotic corner. + * + * Keep reading the singular key as a fallback so a schema from a different + * standard-schema implementation still resolves. + */ +function literalValuesOfDef(def: any): unknown[] { + if (Array.isArray(def?.values)) return def.values; + if (def?.values !== undefined) return [def.values]; + return [def?.value]; +} + +function jsonSchemaTypeOfLiteral(value: unknown): { type?: string } { + if (value === null) return { type: "null" }; + switch (typeof value) { + case "string": + return { type: "string" }; + case "number": + return { type: Number.isInteger(value) ? "integer" : "number" }; + case "boolean": + return { type: "boolean" }; + case "bigint": + return { type: "integer" }; + default: + return {}; + } +} + /** * Convert a Zod v4 internal def to JSON Schema. * Handles the common types used in action parameters. @@ -1321,7 +1368,18 @@ function zodDefToJsonSchema(def: any): any { } if (type === "literal") { - return { type: typeof def.value, enum: [def.value] }; + const values = literalValuesOfDef(def); + const jsonTypes = [ + ...new Set( + values.map((value: unknown) => jsonSchemaTypeOfLiteral(value).type), + ), + ]; + return { + ...(jsonTypes.length === 1 && jsonTypes[0] !== undefined + ? { type: jsonTypes[0] } + : {}), + enum: values, + }; } if (type === "array") { @@ -1366,13 +1424,11 @@ function zodDefToJsonSchema(def: any): any { (o: any) => o?._zod?.def?.type === "literal", ); if (allLiterals) { - const values = def.options.map((o: any) => o._zod.def.value); + const values = def.options.flatMap((o: any) => + literalValuesOfDef(o._zod.def), + ); const jsonTypeOf = (v: any) => - typeof v === "number" - ? "number" - : typeof v === "boolean" - ? "boolean" - : "string"; + jsonSchemaTypeOfLiteral(v).type ?? "string"; const uniqueTypes = [...new Set(values.map(jsonTypeOf))]; if (uniqueTypes.length === 1) { // Homogeneous literal union (e.g. all numbers) — derive the JSON diff --git a/packages/core/src/agent/run-loop-with-resume.spec.ts b/packages/core/src/agent/run-loop-with-resume.spec.ts index 41005a72fc..7c3f0b9778 100644 --- a/packages/core/src/agent/run-loop-with-resume.spec.ts +++ b/packages/core/src/agent/run-loop-with-resume.spec.ts @@ -895,6 +895,69 @@ describe("runAgentLoopDirectWithSoftTimeout", () => { ]); }); + // The gateway answers 200 and then emits its 500 envelope as an in-stream + // frame, so nothing structured reaches this catch — only the sentence. Falling + // straight to `internal_error` is what made one upstream failure arrive under + // three different codes depending on which layer caught it, and only one of + // the three is on the client's recoverable list. Production showed the same + // envelope persisted as `internal_error` in calendar and clips while analytics + // recorded it as `builder_gateway_internal_error`. + it("names the Builder gateway 500 envelope when the thrown error carries no code", async () => { + const outcomes: AgentLoopOutcome[] = []; + const message = + "Sorry, we ran into an issue processing your request. " + + "ERROR ID: 4dbb6f30593c44d093090a37a99012a2"; + mockRunAgentLoop.mockImplementation(async () => { + throw new Error(message); + }); + + await expect( + runAgentLoopDirectWithSoftTimeout( + makeOpts( + [{ role: "user", content: [{ type: "text", text: "Hey" }] }], + new AbortController().signal, + undefined, + undefined, + outcomes, + ), + 60_000, + ), + ).rejects.toThrow(message); + + expect(outcomes).toEqual([ + { + state: "failed", + code: "builder_gateway_internal_error", + retryable: false, + message, + }, + ]); + }); + + // `internal_error` must stay meaningful: it is the code for a message nothing + // recognises, not the code for every uncoded failure. + it("still falls back to internal_error for an unrecognisable message", async () => { + const outcomes: AgentLoopOutcome[] = []; + mockRunAgentLoop.mockImplementation(async () => { + throw new Error("something nobody has classified"); + }); + + await expect( + runAgentLoopDirectWithSoftTimeout( + makeOpts( + [{ role: "user", content: [{ type: "text", text: "go" }] }], + new AbortController().signal, + undefined, + undefined, + outcomes, + ), + 60_000, + ), + ).rejects.toThrow("something nobody has classified"); + + expect(outcomes[0]).toMatchObject({ code: "internal_error" }); + }); + it("bails out after MAX_RUN_LOOP_CONTINUATIONS to prevent infinite loops", async () => { let attempts = 0; mockRunAgentLoop.mockImplementation(async () => { diff --git a/packages/core/src/agent/run-loop-with-resume.ts b/packages/core/src/agent/run-loop-with-resume.ts index 2b251425f8..dcc9df5e81 100644 --- a/packages/core/src/agent/run-loop-with-resume.ts +++ b/packages/core/src/agent/run-loop-with-resume.ts @@ -21,6 +21,10 @@ * uniform "continue" instruction regardless of which recovery fired. */ +import { + classifyTerminalErrorCode, + describeErrorWithCauses, +} from "./engine/error-detail.js"; import type { EngineMessage } from "./engine/types.js"; import { runAgentLoop, @@ -317,6 +321,37 @@ export const RUN_BUDGET_EXHAUSTED_MESSAGE = "I stopped rather than keep retrying silently. " + "Check any completed tool cards above before retrying, ideally as one smaller follow-up."; +/** + * The code a failed attempt is persisted and reported under. + * + * A thrown error that carries no `errorCode` is not automatically anonymous: + * its message is frequently one the shared classifier can name, and the Builder + * gateway's own 500 envelope is the case that matters most in production. The + * gateway answers 200 and then emits the envelope as an in-stream frame, so + * there is no HTTP status to read and no structured code attached — falling + * straight to a generic code here is what made one upstream failure arrive as + * three different codes depending on which layer caught it + * (`builder_gateway_internal_error` when the engine classified it, + * `internal_error` here, `unknown` at persistence). Only one of those three is + * on the client's recoverable list, so identical gateway failures ended some + * chats instantly and sent others into a re-dispatch chain. + * + * Classify the message before giving up on it, so every path through this + * function reports the same code for the same upstream failure. `internal_error` + * stays the last resort for a message nothing recognises — it must remain + * distinguishable from a named failure rather than becoming the name for all of + * them. + */ +function resolveAttemptErrorCode(err: unknown): string { + const candidate = err as { errorCode?: unknown } | null; + if (typeof candidate?.errorCode === "string" && candidate.errorCode) { + return candidate.errorCode; + } + return ( + classifyTerminalErrorCode(describeErrorWithCauses(err)) ?? "internal_error" + ); +} + /** * Internal entry point used by the agent-chat plugin's run handler. Wraps * `runAgentLoop` with soft-timeout + resumable-error continuation recovery. @@ -389,10 +424,7 @@ export async function runAgentLoopDirectWithSoftTimeout( ? { state: "canceled", message: "Agent run was aborted." } : { state: "failed", - code: - typeof candidate?.errorCode === "string" && candidate.errorCode - ? candidate.errorCode - : "internal_error", + code: resolveAttemptErrorCode(err), retryable: isResumableEngineError(err), message: typeof candidate?.message === "string" @@ -649,10 +681,7 @@ export async function runAgentLoopDirectWithSoftTimeout( const candidate = err as { errorCode?: unknown; message?: unknown }; reportFinalOutcome({ state: "failed", - code: - typeof candidate?.errorCode === "string" && candidate.errorCode - ? candidate.errorCode - : "internal_error", + code: resolveAttemptErrorCode(err), retryable: false, message: typeof candidate?.message === "string" diff --git a/packages/core/src/client/chat/run-recovery.tsx b/packages/core/src/client/chat/run-recovery.tsx index 2f0d7a5c51..ac3842a9ad 100644 --- a/packages/core/src/client/chat/run-recovery.tsx +++ b/packages/core/src/client/chat/run-recovery.tsx @@ -21,6 +21,7 @@ import { } from "@tabler/icons-react"; import { useState, useEffect, useCallback, useRef } from "react"; +import { BUILDER_GATEWAY_INTERNAL_ERROR_CODE } from "../../agent/engine/error-detail.js"; import { agentNativePath } from "../api-path.js"; import { writeClipboardText } from "../clipboard.js"; import { @@ -497,7 +498,19 @@ export function RunErrorRecoveryCard({ // points at a button that isn't there. const isUnblockableExternally = info.errorCode === "email_verification_required"; - const canRetry = canRecover || isProviderAuthError || isUnblockableExternally; + // Automatic re-dispatch of this one is futile — the same payload fails the + // same way, measured at 0 recoveries in 97 production runs — so it is not on + // the client's auto-recoverable list. A retry the USER chooses is a different + // decision: minutes have passed, the upstream may have recovered, and its own + // copy ends with "Retry in a moment". Without this the card is the dead end + // that copy points at. + const isDeliberateRetryGatewayFailure = + info.errorCode === BUILDER_GATEWAY_INTERNAL_ERROR_CODE; + const canRetry = + canRecover || + isProviderAuthError || + isUnblockableExternally || + isDeliberateRetryGatewayFailure; const builderReconnectResolved = shouldShowBuilderReconnect && builderReconnect.hasFetchedStatus && diff --git a/packages/core/src/client/sse-event-processor.spec.ts b/packages/core/src/client/sse-event-processor.spec.ts index b6aa491eea..cc520b22e1 100644 --- a/packages/core/src/client/sse-event-processor.spec.ts +++ b/packages/core/src/client/sse-event-processor.spec.ts @@ -3713,42 +3713,60 @@ describe("SSE event processor error classification", () => { }); }); - it("auto-continues the Builder gateway internal-error envelope", async () => { + // Measured across the analytics/clips/calendar production databases: 7 turns + // reached this code and 0 of 7 ever finished, over 97 runs — one turn burned + // 28 runs across 16 minutes on a single "Hey", with overlapping concurrent + // runs. The envelope is emitted for deterministic upstream rejections, so a + // fresh run resends the request that just failed. `providerRetryable: true` + // is present on the real production event and must NOT revive it here: the + // engine's in-request retry already covers the transient case. + it("does not re-dispatch a run for the Builder gateway internal-error envelope", async () => { + const dispatchEvent = vi.fn(); + vi.stubGlobal("window", { dispatchEvent }); + vi.stubGlobal( + "CustomEvent", + class { + type: string; + detail: unknown; + constructor(type: string, init?: { detail?: unknown }) { + this.type = type; + this.detail = init?.detail; + } + }, + ); const message = "Sorry, we ran into an issue processing your request. " + "ERROR ID: bebaeb5da13441539790834b63ff955a"; - const err = await readSSEStream( + const results = []; + for await (const result of readSSEStream( eventStream([ { type: "error", error: message, errorCode: "builder_gateway_internal_error", + providerRetryable: true, }, ]), [], { value: 0 }, "tab-gateway-internal", - ) - [Symbol.asyncIterator]() - .next() - .then( - () => undefined, - (caught) => caught, - ); + )) { + results.push(result); + } - expect(err).toBeInstanceOf(AgentAutoContinueSignal); - expect((err as AgentAutoContinueSignal).errorInfo).toMatchObject({ - errorCode: "builder_gateway_internal_error", - recoverable: true, + const terminal = results.at(-1); + expect(terminal?.status).toMatchObject({ + type: "incomplete", + reason: "error", }); + const runError = terminal?.metadata?.custom?.runError as + | { errorCode?: string; message?: string; details?: string } + | undefined; + expect(runError?.errorCode).toBe("builder_gateway_internal_error"); // The correlation id is the only part support can act on, so it stays — // just not as the whole sentence the user reads. - expect((err as AgentAutoContinueSignal).errorInfo?.details).toContain( - "bebaeb5da13441539790834b63ff955a", - ); - expect((err as AgentAutoContinueSignal).errorInfo?.message).not.toContain( - "ERROR ID", - ); + expect(runError?.details).toContain("bebaeb5da13441539790834b63ff955a"); + expect(runError?.message).not.toContain("ERROR ID"); }); it("surfaces run_budget_exhausted as a loud terminal error without auto-continuing", async () => { diff --git a/packages/core/src/client/sse-event-processor.ts b/packages/core/src/client/sse-event-processor.ts index 8ec43d1b95..d4433ab969 100644 --- a/packages/core/src/client/sse-event-processor.ts +++ b/packages/core/src/client/sse-event-processor.ts @@ -820,6 +820,21 @@ function isAutoRecoverableError(ev: SSEEvent, errMsg: string): boolean { // (each turn cleared+regenerated visible content) for users hitting a // misbehaving Builder route. Surface the error instead. code === "builder_gateway_error" || + // The gateway's unhandled-500 envelope ("Sorry, we ran into an issue + // processing your request. ERROR ID: "), for the same reason as + // `builder_gateway_error` directly above: the production-agent already + // retries it synchronously up to MAX_RETRIES before it can reach here, so a + // fresh run only resends the identical request. Measured across the + // analytics/clips/calendar production databases, 7 turns reached this code + // and 0 of 7 ever reached `done` — 97 runs, one of them 28 runs over 16 + // minutes on a single "Hey", with overlapping concurrent runs on the same + // turn. The envelope is emitted for deterministic upstream rejections (a + // malformed nested tool-schema `type` reproduces it 6/6), which is why + // retrying cannot help: the same payload fails the same way every time. + // It stays `providerRetryable` for the ENGINE's in-request retry, which is + // cheap and does catch the genuinely transient case; this list governs + // whole-run re-dispatch, which does not. + code === BUILDER_GATEWAY_INTERNAL_ERROR_CODE || // The hosted run exhausted its in-invocation continuation budget without // finishing (run-loop-with-resume.ts). It's flagged `recoverable: true` so // the recovery banner reads "stopped before finishing", but it must NOT @@ -848,9 +863,6 @@ function isAutoRecoverableError(ev: SSEEvent, errMsg: string): boolean { code === "http_408" || code === "http_429" || code === "http_500" || - // The gateway's unhandled-500 envelope delivered in-stream instead of as a - // status. Recoverable for the same reason `http_500` is. - code === BUILDER_GATEWAY_INTERNAL_ERROR_CODE || code === "http_502" || code === "http_503" || code === "http_504" || From 440df2d22d38bbd119178728e6beae22f48ba3ff Mon Sep 17 00:00:00 2001 From: "Builder.io" Date: Tue, 18 Aug 2026 22:56:10 +0000 Subject: [PATCH 2/2] fix: reject action schemas JSON cannot represent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review feedback on #3030. A `bigint` literal reached the wire as `enum: [1n]`, so `JSON.stringify` threw while the gateway request was being built and the turn died before any provider saw it. Zod's own converter rejects BigInt literals, which is exactly what routed them to the fallback converter in the first place. `undefined`, `NaN`, and `±Infinity` had the quieter version of the same problem: they serialize to `null`, so the advertised schema said `null` was the accepted value while the Zod validator still demanded the original, and nothing in the emitted schema was left to detect the divergence from. Such a literal is now rejected when the action is defined, naming the field path. The check runs against the Zod def as well as the emitted schema, because Zod's converter turns `z.literal(NaN)` into `const: null` before the output can be inspected. `bigint` no longer claims a JSON Schema type at all — mapping it to `integer` advertised a value that cannot survive serialization. An unrecognised literal def shape degrades to a permissive schema instead of an `enum: []` that would accept nothing. Adds coverage for `JSON.stringify(tool.parameters)`, each unrepresentable literal value and where it can hide, the direct `timeoutMs <= 0` run-loop catch path that the earlier classification test did not reach, and the gateway recovery card proving Retry renders, calls `onRetry`, and offers no Continue. --- .changeset/gateway-internal-error-one-code.md | 9 + packages/core/src/action.spec.ts | 167 +++++++-------- packages/core/src/action.ts | 197 +++++++++++++++++- .../src/agent/run-loop-with-resume.spec.ts | 35 ++++ .../src/client/chat/run-recovery.spec.tsx | 49 +++++ 5 files changed, 362 insertions(+), 95 deletions(-) diff --git a/.changeset/gateway-internal-error-one-code.md b/.changeset/gateway-internal-error-one-code.md index 3cd1c5052c..d59b863d99 100644 --- a/.changeset/gateway-internal-error-one-code.md +++ b/.changeset/gateway-internal-error-one-code.md @@ -32,3 +32,12 @@ invented type on a nested field passes validation, reaches the provider, and com back as the opaque ERROR ID envelope — on every retry, because the same malformed schema is resent verbatim. Literal values are now read from `values`, and no literal can emit a type outside the seven JSON Schema names. + +An action schema carrying a literal that JSON cannot represent is now rejected when +the action is defined, rather than producing a broken request later. A `bigint` +literal made `JSON.stringify` throw while the request was being built, and +`undefined`, `NaN`, and `±Infinity` serialized to `null` — advertising a value the +Zod validator still rejects, so the schema and the validator disagreed with nothing +in the output left to detect it from. The check runs against the Zod def as well as +the emitted schema, because Zod's own converter turns `z.literal(NaN)` into +`const: null` before the emitted schema can be inspected. diff --git a/packages/core/src/action.spec.ts b/packages/core/src/action.spec.ts index 9611eef698..4fc502371f 100644 --- a/packages/core/src/action.spec.ts +++ b/packages/core/src/action.spec.ts @@ -441,6 +441,86 @@ describe("defineAction schema mode — tool parameter JSON Schema", () => { }); }); + // A tool schema only ever travels as JSON, so a value JSON cannot carry + // describes a contract no caller can satisfy. `bigint` makes `JSON.stringify` + // THROW while the request is being built; `undefined`/`NaN`/`±Infinity` + // serialize to `null`, advertising a value the Zod validator still rejects. + // Both are worse than failing at definition time. + it.each([ + ["a BigInt", z.literal(1n), "a BigInt"], + ["undefined", z.literal(undefined as any), "undefined"], + ["NaN", z.literal(NaN), "NaN"], + ["negative Infinity", z.literal(-Infinity), "Infinity"], + ])("rejects a literal of %s", (_label, literal, expected) => { + expect(() => + defineAction({ + description: "unrepresentable literal", + schema: z.object({ field: literal as any }), + run: async () => "ok", + }), + ).toThrow(new RegExp(`field is a literal of ${expected}`)); + }); + + // The value must be found wherever it hides, including the places a converter + // would otherwise launder it before anything could inspect the output. + it.each([ + [ + "nested in an object", + z.object({ outer: z.object({ field: z.literal(1n) }) }), + "outer.field", + ], + ["inside an array", z.object({ field: z.array(z.literal(1n)) }), "field"], + [ + "behind optional", + z.object({ field: z.literal(NaN).optional() }), + "field", + ], + [ + "in a union branch", + z.object({ field: z.union([z.literal("s"), z.literal(NaN)]) }), + "field|1", + ], + ])("rejects an unrepresentable literal %s", (_label, schema, path) => { + let caught: any; + try { + defineAction({ + description: "d", + schema: schema as any, + run: async () => "ok", + }); + } catch (err) { + caught = err; + } + expect(caught?.errorCode).toBe("SCHEMA_NOT_JSON_REPRESENTABLE"); + expect(caught.details.problems.join(";")).toContain(path); + }); + + // The failure this guards against was a throw during serialization, so assert + // on serialization itself rather than only on the object graph. + it("emits tool parameters that survive JSON.stringify", () => { + const action = defineAction({ + description: "serializable", + schema: z.object({ + title: z.string(), + count: z.number().int().optional(), + kind: z.literal("weekly"), + nothing: z.literal(null), + ratio: z.literal(1.5), + mode: z.union([z.literal("a"), z.literal("b")]), + choice: z.enum(["x", "y"]), + }), + run: async () => "ok", + }); + + expect(() => JSON.stringify(action.tool.parameters)).not.toThrow(); + const roundTripped = JSON.parse(JSON.stringify(action.tool.parameters)); + // A silent `null` here is the divergence case: it would mean a value was + // dropped or coerced on the way to JSON. + expect(JSON.stringify(roundTripped)).not.toContain('"const":null,"type"'); + expect(roundTripped.properties.kind).toMatchObject({ const: "weekly" }); + expect(roundTripped.properties.nothing).toMatchObject({ type: "null" }); + }); + it("strips the $schema key so the Claude API (draft 2020-12) does not reject it", () => { const action = defineAction({ description: "with schema key", @@ -477,93 +557,6 @@ describe("defineAction schema mode — tool parameter JSON Schema", () => { expect(params.properties.cfg.default).toEqual({ propertyNames: "x" }); }); - // OpenAI answers a `oneOf` anywhere in a function schema with - // "Invalid schema for function 'x': ... 'oneOf' is not permitted" and 400s - // the whole request before a token streams. Zod emits `oneOf` for every - // discriminated union, so this was 178k errors across 786 users over seven - // weeks from one action. - it("rewrites oneOf to anyOf so OpenAI does not reject the function schema", () => { - const action = defineAction({ - description: "with a discriminated union", - schema: z.object({ - operations: z.array( - z.discriminatedUnion("op", [ - z.object({ op: z.literal("add"), panelId: z.string() }), - z.object({ - op: z.literal("remove"), - panelIds: z.array(z.string()), - }), - ]), - ), - }), - run: async () => "ok", - }); - const json = JSON.stringify(action.tool.parameters); - expect(json).not.toContain('"oneOf"'); - expect(json).toContain('"anyOf"'); - }); - - it("keeps every branch when rewriting a nested union", () => { - const action = defineAction({ - description: "nested union", - schema: z.object({ - outer: z.object({ - inner: z.discriminatedUnion("kind", [ - z.object({ kind: z.literal("a"), a: z.string() }), - z.object({ kind: z.literal("b"), b: z.string() }), - z.object({ kind: z.literal("c"), c: z.string() }), - ]), - }), - }), - run: async () => "ok", - }); - const params = action.tool.parameters as any; - const inner = params.properties.outer.properties.inner; - expect(inner.oneOf).toBeUndefined(); - expect(inner.anyOf).toHaveLength(3); - }); - - // OpenAI rejects a schema position with no `type` — "schema must have a - // 'type' key" — and 400s the whole request, exactly like `oneOf` did. This - // surfaced only after the oneOf fix let the validator reach the next layer. - it("gives z.unknown() a typed value union so OpenAI accepts it", () => { - const action = defineAction({ - description: "typeless field", - schema: z.object({ value: z.unknown() }), - run: async () => "ok", - }); - const value = (action.tool.parameters as any).properties.value; - expect(Array.isArray(value.anyOf)).toBe(true); - expect(value.anyOf.map((b: any) => b.type)).toContain("string"); - expect(value.anyOf.map((b: any) => b.type)).toContain("object"); - }); - - it("types the value schema inside a record so nothing is left bare", () => { - const action = defineAction({ - description: "record of unknown", - schema: z.object({ patch: z.record(z.string(), z.unknown()) }), - run: async () => "ok", - }); - const patch = (action.tool.parameters as any).properties.patch; - expect(patch.type).toBe("object"); - const extra = patch.additionalProperties; - if (extra && typeof extra === "object") { - expect(Array.isArray(extra.anyOf)).toBe(true); - } - }); - - // An enum carries its own shape; adding a value union would widen it. - it("leaves an enum-only schema alone", () => { - const action = defineAction({ - description: "enum field", - schema: z.object({ mode: z.enum(["a", "b"]) }), - run: async () => "ok", - }); - const mode = (action.tool.parameters as any).properties.mode; - expect(mode.enum).toEqual(["a", "b"]); - expect(mode.anyOf).toBeUndefined(); - }); - it("stores the original schema on the entry for downstream re-validation", () => { const schema = z.object({ x: z.string() }); const action = defineAction({ diff --git a/packages/core/src/action.ts b/packages/core/src/action.ts index 628aab4273..e181c88b66 100644 --- a/packages/core/src/action.ts +++ b/packages/core/src/action.ts @@ -1277,6 +1277,160 @@ const PROVIDER_REJECTED_KEYWORDS = [ "not", ] as const; +/** + * Why a value that JSON cannot carry must fail here rather than later. + * + * A tool schema is only ever delivered as JSON, so a value JSON cannot represent + * describes a contract no caller can satisfy. There is no safe way to smuggle it + * through, and the two failure modes it produces are both worse than an error at + * definition time: + * + * - `bigint` makes `JSON.stringify` THROW, so the turn dies while building the + * request, before any provider sees it. + * - `undefined`, `NaN`, and `±Infinity` serialize to `null`. The advertised + * schema then says `null` is the accepted value while the Zod validator still + * demands the original, so the model is told to send something that can never + * validate — the silent-divergence case this repo treats as a bug, not a guard. + * + * Checked on the FINAL schema from either converter, because Zod's own converter + * reaches this too: it emits `const: NaN` for `z.literal(NaN)` without + * complaint, and only the fallback path is reached for bigint literals (Zod + * throws "BigInt literals cannot be represented in JSON Schema", which is what + * sends us to the fallback in the first place). + */ +function describeJsonUnsafeValue(value: unknown): string | null { + if (typeof value === "bigint") return "a BigInt"; + if (typeof value === "symbol") return "a symbol"; + if (typeof value === "function") return "a function"; + if (value === undefined) return "undefined"; + if (typeof value === "number" && !Number.isFinite(value)) { + return Number.isNaN(value) ? "NaN" : "Infinity"; + } + return null; +} + +function collectJsonUnsafeSchemaValues( + node: unknown, + path: string, + found: string[], + seen: Set, +): void { + const unsafe = describeJsonUnsafeValue(node); + if (unsafe) { + found.push(`${path || "schema"} is ${unsafe}`); + return; + } + if (!node || typeof node !== "object") return; + if (seen.has(node)) return; + seen.add(node); + if (Array.isArray(node)) { + node.forEach((item, index) => + collectJsonUnsafeSchemaValues(item, `${path}[${index}]`, found, seen), + ); + return; + } + for (const [key, value] of Object.entries(node as Record)) { + collectJsonUnsafeSchemaValues( + value, + path ? `${path}.${key}` : key, + found, + seen, + ); + } +} + +/** + * The same check one layer earlier, against the Zod def rather than the emitted + * schema, because a converter can launder the problem before this file sees it: + * Zod's own converter turns `z.literal(NaN)` into `const: null` without + * complaint, which reads as a legitimate "null is the accepted value" and is + * indistinguishable from an author writing `z.literal(null)`. The validator still + * demands NaN, so the advertised contract and the enforced one disagree — with + * nothing left in the output to detect it from. + * + * Deliberately tolerant of shape: an unrecognised def is skipped rather than + * treated as a problem, so this can only ever reject a literal it positively + * identified. + */ +const LITERAL_DEF_CHILD_KEYS = [ + "element", + "innerType", + "in", + "out", + "valueType", + "keyType", +] as const; + +function collectJsonUnsafeLiteralDefs( + node: any, + path: string, + found: string[], + seen: Set, +): void { + const def = node?._zod?.def ?? node; + if (!def || typeof def !== "object") return; + if (seen.has(def)) return; + seen.add(def); + + if (def.type === "literal") { + for (const [index, value] of literalValuesOfDef(def).entries()) { + const unsafe = describeJsonUnsafeValue(value); + if (unsafe) { + found.push(`${path || "schema"} is a literal of ${unsafe}`); + } + void index; + } + return; + } + + if (def.shape && typeof def.shape === "object") { + for (const [key, child] of Object.entries(def.shape)) { + collectJsonUnsafeLiteralDefs( + child, + path ? `${path}.${key}` : key, + found, + seen, + ); + } + } + if (Array.isArray(def.options)) { + def.options.forEach((child: unknown, index: number) => + collectJsonUnsafeLiteralDefs(child, `${path}|${index}`, found, seen), + ); + } + for (const key of LITERAL_DEF_CHILD_KEYS) { + if (def[key]) collectJsonUnsafeLiteralDefs(def[key], path, found, seen); + } +} + +function assertJsonSafeLiteralDefs(schema: unknown): void { + const found: string[] = []; + collectJsonUnsafeLiteralDefs(schema, "", found, new Set()); + if (found.length > 0) throw jsonUnrepresentableSchemaError(found); +} + +function jsonUnrepresentableSchemaError( + problems: string[], +): ActionContractError { + return new ActionContractError( + `Action schema cannot be represented as a JSON tool schema: ${problems.join("; ")}. ` + + "Tool schemas are sent as JSON, so use a JSON-representable literal " + + "(string, finite number, boolean, or null) instead.", + { + errorCode: "SCHEMA_NOT_JSON_REPRESENTABLE", + details: { problems }, + statusCode: 500, + }, + ); +} + +function assertJsonSafeToolSchema(schema: T): T { + const found: string[] = []; + collectJsonUnsafeSchemaValues(schema, "", found, new Set()); + if (found.length > 0) throw jsonUnrepresentableSchemaError(found); + return schema; +} + export function stripUnsupportedSchemaKeywords(node: T): T { if (!node || typeof node !== "object" || Array.isArray(node)) return node; const obj = node as Record; @@ -1339,6 +1493,10 @@ function schemaToJsonSchema( ): ActionTool["parameters"] { const s = schema as any; + // Before either converter runs, so a value one of them would launder into a + // plausible-looking `null` is still identifiable. + assertJsonSafeLiteralDefs(s); + // Prefer Zod's own JSON Schema output — it handles descriptions, // enums, coerce, and all type wrappers correctly. if (s["~standard"]?.jsonSchema?.input) { @@ -1351,15 +1509,23 @@ function schemaToJsonSchema( if (result && typeof result === "object") { delete result.$schema; } - return stripUnsupportedSchemaKeywords(result) as ActionTool["parameters"]; - } catch { + return assertJsonSafeToolSchema( + stripUnsupportedSchemaKeywords(result), + ) as ActionTool["parameters"]; + } catch (err) { + // A schema this converter produced but JSON cannot carry is a real contract + // error, not a "try the other converter" signal — the fallback would emit + // the same unusable value. Only a conversion failure falls through. + if (isActionContractError(err)) throw err; // Fall through to manual converter } } // Fallback: manual conversion from Zod v4 internal defs if (s._zod?.def) { - return stripUnsupportedSchemaKeywords(zodDefToJsonSchema(s._zod.def)); + return assertJsonSafeToolSchema( + stripUnsupportedSchemaKeywords(zodDefToJsonSchema(s._zod.def)), + ); } // Last resort: empty object schema @@ -1390,11 +1556,17 @@ function schemaToJsonSchema( * * Keep reading the singular key as a fallback so a schema from a different * standard-schema implementation still resolves. + * + * Returns `[]` when neither key is PRESENT, which is different from a literal + * whose value happens to be `undefined`: an unrecognised def shape must degrade + * to a permissive schema, while a real `undefined` literal is a contract error + * `assertJsonSafeToolSchema` is entitled to reject. */ function literalValuesOfDef(def: any): unknown[] { if (Array.isArray(def?.values)) return def.values; - if (def?.values !== undefined) return [def.values]; - return [def?.value]; + if (def && "values" in def) return [def.values]; + if (def && "value" in def) return [def.value]; + return []; } function jsonSchemaTypeOfLiteral(value: unknown): { type?: string } { @@ -1403,11 +1575,16 @@ function jsonSchemaTypeOfLiteral(value: unknown): { type?: string } { case "string": return { type: "string" }; case "number": - return { type: Number.isInteger(value) ? "integer" : "number" }; + // NaN and ±Infinity have no JSON form; `assertJsonSafeToolSchema` rejects + // them, so do not hand back a `number` that implies they are usable. + return Number.isFinite(value) + ? { type: Number.isInteger(value) ? "integer" : "number" } + : {}; case "boolean": return { type: "boolean" }; - case "bigint": - return { type: "integer" }; + // `bigint` is deliberately absent: JSON has no bigint, so there is no honest + // type to claim. Mapping it to `integer` would have advertised a value that + // makes `JSON.stringify` throw while building the request. default: return {}; } @@ -1482,6 +1659,10 @@ function zodDefToJsonSchema(def: any): any { if (type === "literal") { const values = literalValuesOfDef(def); + // An unrecognised literal def shape yields no values at all. Emit a + // permissive schema rather than an `enum: []`, which would advertise a field + // that accepts nothing. + if (values.length === 0) return {}; const jsonTypes = [ ...new Set( values.map((value: unknown) => jsonSchemaTypeOfLiteral(value).type), diff --git a/packages/core/src/agent/run-loop-with-resume.spec.ts b/packages/core/src/agent/run-loop-with-resume.spec.ts index 7c3f0b9778..a062d7d9cf 100644 --- a/packages/core/src/agent/run-loop-with-resume.spec.ts +++ b/packages/core/src/agent/run-loop-with-resume.spec.ts @@ -934,6 +934,41 @@ describe("runAgentLoopDirectWithSoftTimeout", () => { ]); }); + // `timeoutMs <= 0` takes a separate direct-loop branch with its OWN catch, so + // the loop-path test above does not exercise it. Both catches had the same + // blind fallback, so both need the same proof. + it("names the gateway 500 envelope on the direct path too (timeoutMs <= 0)", async () => { + const outcomes: AgentLoopOutcome[] = []; + const message = + "Sorry, we ran into an issue processing your request. " + + "ERROR ID: 39a8c319984746da9c7f351f3067913d"; + mockRunAgentLoop.mockImplementation(async () => { + throw new Error(message); + }); + + await expect( + runAgentLoopDirectWithSoftTimeout( + makeOpts( + [{ role: "user", content: [{ type: "text", text: "Hey" }] }], + new AbortController().signal, + undefined, + undefined, + outcomes, + ), + 0, + ), + ).rejects.toThrow(message); + + expect(outcomes).toEqual([ + { + state: "failed", + code: "builder_gateway_internal_error", + retryable: false, + message, + }, + ]); + }); + // `internal_error` must stay meaningful: it is the code for a message nothing // recognises, not the code for every uncoded failure. it("still falls back to internal_error for an unrecognisable message", async () => { diff --git a/packages/core/src/client/chat/run-recovery.spec.tsx b/packages/core/src/client/chat/run-recovery.spec.tsx index d3648b7155..94d7b409de 100644 --- a/packages/core/src/client/chat/run-recovery.spec.tsx +++ b/packages/core/src/client/chat/run-recovery.spec.tsx @@ -229,6 +229,55 @@ describe("run recovery surfaces", () => { expect(container.textContent).toContain("Retry"); }); + // The gateway 500 is no longer auto-recoverable (0 recoveries in 97 production + // runs), so `recoverable` is absent and Continue is correctly gone. Its own copy + // still ends with "Retry in a moment", so without an explicit branch the card + // would be the dead end that sentence points at. + it("offers a working Retry for the gateway internal-error envelope", async () => { + const onRetry = vi.fn(); + const onContinue = vi.fn(); + + await act(async () => { + root.render( + + + , + ); + }); + + const retry = [...container.querySelectorAll("button")].find( + (button) => button.textContent?.trim() === "Retry", + ); + expect(retry).toBeTruthy(); + + await act(async () => { + retry!.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + expect(onRetry).toHaveBeenCalledTimes(1); + + // Not recoverable, so no Continue — auto-continuing is what burned 28 runs. + const labels = [...container.querySelectorAll("button")].map((button) => + button.textContent?.trim(), + ); + expect(labels).not.toContain("Continue"); + expect(onContinue).not.toHaveBeenCalled(); + }); + it("dismisses the recovery card after saving a provider key", async () => { const onDismiss = vi.fn();