diff --git a/packages/claude-agents/agents.md b/packages/claude-agents/agents.md index eb884e4e..e07ef62d 100644 --- a/packages/claude-agents/agents.md +++ b/packages/claude-agents/agents.md @@ -103,16 +103,25 @@ If `config.tools` is absent, `toolMCP` and `allowedTools` are both `undefined`. ### 3. Agentic Loop -The claude-agent-sdk handles the loop internally. The handler iterates the `query()` async generator and waits for a message with a `result` key — that is the final text output. Token usage is read from `message.usage`. +The claude-agent-sdk handles the loop internally. The handler iterates the `query()` async generator and waits for the message whose `type` is `'result'` — its `result` field is the final text output. Token usage is read from `message.usage`. ```ts for await (const message of query({ prompt, options: { ... } })) { - if ('result' in message) { + if (message.type === 'result') { + assertRunSucceeded(message); // throws on a failed run // done — extract output and usage } } ``` +Gate on `type`, never on `'result' in message`: an error-subtype result (`error_max_turns`, +`error_during_execution`, ...) has no `result` field, so a key-presence gate lets a failed run +through. A `success` subtype is not enough either. When an Anthropic API call fails (bad key, no +credits, rate limit after retries), the CLI ends the run with `subtype: 'success'`, `is_error: true`, +`api_error_status` set, and the error text in `result`. `assertRunSucceeded` throws on either shape, +so the caller tracks `$ld:ai:generation:error` instead of being handed the error text as the +model's answer. The native graph adapter applies the same check to every node's run. + ### 4. Telemetry Each invocation emits the same three-span vocabulary as the other five handlers: @@ -137,6 +146,13 @@ measured against Agent SDK 0.3.220 rather than assumed: `user` message — carrying the tool results the next call is sent — and a `system` `init` mark local work finishing. That is a whitelist, so a subtype added by a future CLI is inert rather than silently collapsing the timings. +4. **Some `assistant` messages are not responses at all.** The CLI writes its own + message, with `model` set to the literal ``, when an API call fails + and after a response stops at `max_output_tokens`. Its content is the CLI's + error text and its usage is zero. It gets no `chat` span and never joins the + conversation a later span reports. It can share the real response's + `request_id`, so grouping on that alone would append the error text to the real + response's output. A response's window therefore runs from the last local work to that response's **first** message. The API had already returned every block by then, so extending diff --git a/packages/claude-agents/src/__tests__/native-graph.test.ts b/packages/claude-agents/src/__tests__/native-graph.test.ts index 4c2f421a..49ae8e8d 100644 --- a/packages/claude-agents/src/__tests__/native-graph.test.ts +++ b/packages/claude-agents/src/__tests__/native-graph.test.ts @@ -109,7 +109,7 @@ function makeGraphDef(overrides: Partial = {}): GraphDefinition function makeResultMessage(result = 'done', usage = { input_tokens: 1, output_tokens: 2 }) { return async function* () { - yield { result, usage }; + yield { type: 'result', subtype: 'success', result, usage }; }; } @@ -347,6 +347,59 @@ describe('toClaudeAgents', () => { ); }); + // Measured against Agent SDK 0.3.220: an Anthropic API failure arrives as a `success` result with + // `is_error` set and the error text in `result`. The adapter used to return that text as the answer. + const apiErrorRun = async function* () { + yield { + type: 'result', + subtype: 'success', + is_error: true, + api_error_status: 400, + result: 'Credit balance is too low', + usage: { input_tokens: 0, output_tokens: 0 }, + }; + }; + + it('fails the graph when the root result reports an API error', async () => { + const def = makeGraphDef(); + mockQuery.mockImplementation(apiErrorRun); + + await expect( + toClaudeAgents(Promise.resolve(def), { context: { kind: 'user', key: 'user-1' } }).invoke('hello'), + ).rejects.toThrow(/Credit balance is too low/); + + const events = mockTrack.mock.calls.map((c: unknown[]) => c[0]); + expect(events).toContain('$ld:ai:graph:invocation_failure'); + expect(events).not.toContain('$ld:ai:graph:invocation_success'); + }); + + it('fails the graph when the root result has an error subtype', async () => { + const def = makeGraphDef(); + mockQuery.mockImplementation(async function* () { + yield { + type: 'result', + subtype: 'error_max_turns', + is_error: true, + errors: ['turn limit reached'], + usage: { input_tokens: 5, output_tokens: 1 }, + }; + }); + + await expect(toClaudeAgents(Promise.resolve(def)).invoke('hello')).rejects.toThrow( + /error_max_turns.*turn limit reached/, + ); + }); + + it('fails a sub-agent tool call whose run reports an API error', async () => { + const def = makeGraphDef(); + await toClaudeAgents(Promise.resolve(def)).invoke('hello'); + + mockQuery.mockImplementation(apiErrorRun); + const toolExecutor = mockTool.mock.calls[0][3] as (args: { input: string }) => Promise; + + await expect(toolExecutor({ input: 'subtask' })).rejects.toThrow(/Credit balance is too low/); + }); + // T10: system prompt forwarding from node instructions it('forwards config.instructions as systemPrompt to root query', async () => { const def = makeGraphDef(); @@ -398,7 +451,7 @@ describe('toClaudeAgents', () => { let capturedHooks: { PreToolUse?: Array<{ hooks: Array<(input: unknown) => Promise> }> } | undefined; mockQuery.mockImplementation(async function* (queryArgs: { options?: { hooks?: typeof capturedHooks } }) { capturedHooks = queryArgs.options?.hooks; - yield { result: 'ok', usage: { input_tokens: 1, output_tokens: 1 } }; + yield { type: 'result', subtype: 'success', result: 'ok', usage: { input_tokens: 1, output_tokens: 1 } }; }); await toClaudeAgents(Promise.resolve(def), { diff --git a/packages/claude-agents/src/__tests__/spans.test.ts b/packages/claude-agents/src/__tests__/spans.test.ts index 19ea0af4..353b48ad 100644 --- a/packages/claude-agents/src/__tests__/spans.test.ts +++ b/packages/claude-agents/src/__tests__/spans.test.ts @@ -90,6 +90,35 @@ const errorResultMessage = () => ({ usage: { input_tokens: 40000, output_tokens: 500 }, }); +// Measured against Agent SDK 0.3.220 with a rejected key, and the same for a key without credits: an +// API failure does not fail the run. The CLI emits a synthetic assistant message whose `model` is the +// literal `` and whose content is the error text, then a result that says `success` but +// sets `is_error`. The CLI emits the same kind of synthetic message after a response stops at +// `max_output_tokens`, and that run can still succeed. +const zeroUsage = { input_tokens: 0, output_tokens: 0, cache_creation_input_tokens: 0, cache_read_input_tokens: 0 }; +const syntheticMessage = (text = 'Credit balance is too low', requestId = 'req_synthetic') => ({ + type: 'assistant', + session_id: 'sess-1', + request_id: requestId, + parent_tool_use_id: null, + error: 'billing_error', + message: { + model: '', + role: 'assistant', + stop_reason: 'stop_sequence', + usage: zeroUsage, + content: [{ type: 'text', text }], + }, +}); +const apiErrorResultMessage = (text = 'Credit balance is too low') => ({ + type: 'result' as const, + subtype: 'success' as const, + is_error: true, + api_error_status: 400, + result: text, + usage: zeroUsage, +}); + /** * Replays the SDK's PreToolUse → PostToolUse sequence for one tool call. * @@ -798,6 +827,81 @@ describe('claude-agents span tree against a real tracer', () => { expect(attrs['gen_ai.usage.output_tokens']).toBe(500); }); + it('fails the run when the result reports an API error under the success subtype', async () => { + mockQuery.mockImplementation(async function* () { + yield initMessage(); + yield syntheticMessage(); + yield apiErrorResultMessage(); + }); + + await expect(createClaudeAgentsHandler({ captureContent: true })(baseConfig as never, 'q')).rejects.toThrow( + /400.*Credit balance is too low/, + ); + + // The synthetic message is not a model response, so it gets no `chat` span. + expect(spans().map((s) => s.name)).toEqual(['invoke_agent']); + expect(root()?.status.code).toBe(2); // ERROR, not OK + expect(root()?.events.find((e) => e.name === 'exception')?.attributes?.['exception.message']).toMatch( + /Credit balance is too low/, + ); + // The error text is not model output. + expect(root()?.attributes['gen_ai.output.messages']).toBeUndefined(); + }); + + it('fails a streamed run when the result reports an API error, yielding no done event', async () => { + mockQuery.mockImplementation(async function* () { + yield initMessage(); + yield syntheticMessage(); + yield apiErrorResultMessage(); + }); + + const events: unknown[] = []; + const run = async () => { + const gen = createClaudeAgentsHandler({ captureContent: true }).stream?.(baseConfig as never, 'q', {}, {}); + for await (const event of gen as AsyncGenerator) events.push(event); + }; + + await expect(run()).rejects.toThrow(/400.*Credit balance is too low/); + expect(events).toEqual([]); + expect(spans().map((s) => s.name)).toEqual(['invoke_agent']); + expect(root()?.status.code).toBe(2); + expect(root()?.attributes['gen_ai.output.messages']).toBeUndefined(); + }); + + it('treats a success result with is_error false as a successful run', async () => { + mockQuery.mockImplementation(async function* () { + yield assistantMessage(); + yield { ...resultMessage('fine'), is_error: false }; + }); + + const result = await createClaudeAgentsHandler()(baseConfig as never, 'q'); + + expect(result.output).toBe('fine'); + expect(root()?.status.code).toBe(1); // OK + }); + + // A synthetic message can share the real response's `request_id`, so grouping on it would append + // the error text to that response's output. + it('leaves a synthetic message out of every chat span when the run recovers from it', async () => { + mockQuery.mockImplementation(async function* () { + yield assistantMessage(10, 2, 'req_1', [{ type: 'text', text: 'first half' }]); + yield syntheticMessage('API Error: output token maximum', 'req_1'); + yield toolResultMessage(); + yield assistantMessage(12, 3, 'req_2', [{ type: 'text', text: 'second half' }]); + yield resultMessage('second half'); + }); + + const result = await createClaudeAgentsHandler({ captureContent: true })(baseConfig as never, 'q'); + + expect(result.output).toBe('second half'); + const chats = named('chat '); + expect(chats.map((s) => s.name)).toEqual(['chat claude-opus-4-5', 'chat claude-opus-4-5']); + for (const chat of chats) { + expect(String(chat.attributes['gen_ai.output.messages'])).not.toContain('output token maximum'); + expect(String(chat.attributes['gen_ai.input.messages'])).not.toContain('output token maximum'); + } + }); + it('carries the run-level token totals on the invoke_agent root, not only on the children', async () => { mockQuery.mockImplementation(async function* () { yield assistantMessage(); diff --git a/packages/claude-agents/src/handler.ts b/packages/claude-agents/src/handler.ts index 5d4cd746..07918d75 100644 --- a/packages/claude-agents/src/handler.ts +++ b/packages/claude-agents/src/handler.ts @@ -1,4 +1,12 @@ -import { createSdkMcpServer, type HookInput, query, type SDKUserMessage, tool } from '@anthropic-ai/claude-agent-sdk'; +import { + createSdkMcpServer, + type HookInput, + query, + type SDKResultMessage, + type SDKResultSuccess, + type SDKUserMessage, + tool, +} from '@anthropic-ai/claude-agent-sdk'; import { type AiConfigRep, addCachedTokensToInput, @@ -337,7 +345,10 @@ class InferenceSpans { /** Feed every message of the run, in order. */ record(message: { type: string }): void { if (message.type === 'assistant') { - this.absorb(message as AssistantResponse); + // Not a model response, so no span, no turn in the conversation and no usage. It can share + // the real response's `request_id`, which would otherwise append the error text to that + // response's output. + if (!isSynthetic(message as AssistantResponse)) this.absorb(message as AssistantResponse); return; } if (message.type === 'user') this.absorbUserTurn(message as UserTurn); @@ -533,16 +544,41 @@ function failSpan(span: Span, error: unknown, endedSpans?: Set): void { } /** - * Builds the error for a non-success result message. + * Throws when a `result` message reports a failed run, and narrows it to a success otherwise. + * + * Two shapes fail. A non-`success` subtype is `SDKResultError`, which has no `result` field, so the + * old `'result' in message` gate never matched one: the loop simply ended, the run was reported OK + * with zeroed usage, and the real token spend and `errors` were discarded. A run that hit + * `error_max_turns` or `error_max_budget_usd` genuinely failed and has to surface as a failure. + * + * The other is a `success` subtype with `is_error` set, which is how the CLI reports a failed + * Anthropic API call. Measured against Agent SDK 0.3.220 with a key that had no credits: the run + * ends `success`, `is_error: true`, `api_error_status` set, and the error text in `result` — so a + * caller was handed "Credit balance is too low" as the model's answer. `is_error` is the test the + * agent SDK applies itself; it raises after the result, but this handler stops reading first. + */ +export function assertRunSucceeded(message: SDKResultMessage): asserts message is SDKResultSuccess { + if (message.subtype !== 'success') { + const detail = message.errors?.length ? `: ${message.errors.join('; ')}` : ''; + throw new Error(`Claude agent run ended with ${message.subtype}${detail}`); + } + if (message.is_error !== true) return; + const status = typeof message.api_error_status === 'number' ? ` (HTTP ${message.api_error_status})` : ''; + const detail = message.result ? `: ${message.result}` : ''; + throw new Error(`Claude agent run ended with an error result${status}${detail}`); +} + +/** + * Whether an assistant message is one the CLI wrote itself rather than one the model returned. * - * `SDKResultError` has no `result` field, so the old `'result' in message` gate never matched one: - * the loop simply ended, the run was reported OK with zeroed usage, and the real token spend and - * `errors` were discarded. A run that hit `error_max_turns` or `error_max_budget_usd` genuinely - * failed and has to surface as a failure. + * Measured against Agent SDK 0.3.220: a failed API call surfaces as an assistant message whose + * `model` is the literal ``, whose content is the error text and whose usage is zero, and + * the CLI emits the same after a response stops at `max_output_tokens`. Keyed on `model` rather than + * on the message's `error` field because the latter is set on both, and only the run's `result` + * says whether the run failed. */ -function resultError(subtype: string, errors: ReadonlyArray | undefined): Error { - const detail = errors?.length ? `: ${errors.join('; ')}` : ''; - return new Error(`Claude agent run ended with ${subtype}${detail}`); +function isSynthetic(message: AssistantResponse): boolean { + return message.message?.model === ''; } /** @@ -1018,9 +1054,7 @@ export function createClaudeAgentsHandlerInternal({ // The CLI's own run-level total is authoritative, so the catch below must not // overwrite it with the summed-per-response figure on the error-result path. rootUsageWritten = true; - if (message.subtype !== 'success') { - throw resultError(message.subtype, message.errors); - } + assertRunSucceeded(message); setOutputContentAttributes(span, captureContent, [ { role: 'assistant', @@ -1159,9 +1193,7 @@ export function createClaudeAgentsHandlerInternal({ finishRootSpan(span, config, rawUsage); // Authoritative: the paths below must not overwrite it with the per-response sum. rootUsageWritten = true; - if (message.subtype !== 'success') { - throw resultError(message.subtype, message.errors); - } + assertRunSucceeded(message); setOutputContentAttributes(span, captureContent, [ // biome-ignore lint/suspicious/noExplicitAny: Claude SDK result field is not typed publicly { role: 'assistant', parts: [{ type: 'text', content: (message as any).result ?? fullOutput }] }, diff --git a/packages/claude-agents/src/native-graph.ts b/packages/claude-agents/src/native-graph.ts index cd1052b6..b7dd1d1b 100644 --- a/packages/claude-agents/src/native-graph.ts +++ b/packages/claude-agents/src/native-graph.ts @@ -17,7 +17,7 @@ import { } from '@launchdarkly/ai-server'; import { SpanStatusCode, trace } from '@opentelemetry/api'; import { z } from 'zod'; -import { buildPrompt, buildQueryPrompt, buildToolMCP, partitionTools } from './handler.js'; +import { assertRunSucceeded, buildPrompt, buildQueryPrompt, buildToolMCP, partitionTools } from './handler.js'; import { buildModelParameterQueryOptions } from './model-parameters.js'; import { LD_AI_PACKAGE_NAME, LD_AI_PACKAGE_VERSION } from './version.js'; @@ -126,7 +126,11 @@ const runQuery = async ( ...(systemPrompt ? { systemPrompt } : {}), }, })) { - if ('result' in message) { + // On `type`, not `'result' in message`: an error-subtype result has no `result` field, so that + // gate let a failed node answer with an empty string, and an API failure answered with its + // error text. Throwing fails the graph at the root, and fails the tool call at a sub-agent. + if (message.type === 'result') { + assertRunSucceeded(message); output = message.result; rawUsage = (message.usage as Record) ?? {}; }