diff --git a/frontend/src/lib/components/agentTrace.test.ts b/frontend/src/lib/components/agentTrace.test.ts index 2cca3177ee..d2df80107a 100644 --- a/frontend/src/lib/components/agentTrace.test.ts +++ b/frontend/src/lib/components/agentTrace.test.ts @@ -144,17 +144,3 @@ describe('the max-iterations path', () => { ]) }) }) - -// A result is whatever a script returned, so every nested field is arbitrary even -// once the messages have roles. A throw in here takes down the whole result -// viewer, including the ordinary error such a payload usually rides on. -describe('a malformed message that still has a role', () => { - it.each([ - ['tool_calls that are not a list', { role: 'assistant', tool_calls: {} }], - ['annotations that are not a list', { role: 'assistant', content: 'hi', annotations: 'abc' }], - ['a content object', { role: 'assistant', content: { text: 'hi' } }], - ['an agent_action that is not an object', { role: 'tool', agent_action: 'tool_call' }] - ])('survives %s', (_label, message) => { - expect(() => buildAgentTrace([message as never])).not.toThrow() - }) -}) diff --git a/frontend/src/lib/components/agentTrace.ts b/frontend/src/lib/components/agentTrace.ts index a30bb1c4cf..e76b68415b 100644 --- a/frontend/src/lib/components/agentTrace.ts +++ b/frontend/src/lib/components/agentTrace.ts @@ -46,13 +46,10 @@ function contentText(content: unknown): string { function sourcesOf(message: AgentMessage): WebSearchSource[] | undefined { const annotations = message.annotations - if (!Array.isArray(annotations) || annotations.length === 0) { + if (!annotations?.length) { return undefined } - const sources = annotations - .filter((a) => typeof a?.url === 'string') - .map((a) => ({ url: a.url, title: a.title })) - return sources.length > 0 ? sources : undefined + return annotations.map((a) => ({ url: a.url, title: a.title })) } export function buildAgentTrace(messages: AgentMessage[]): AgentTraceEntry[] { @@ -61,14 +58,7 @@ export function buildAgentTrace(messages: AgentMessage[]): AgentTraceEntry[] { // the two are joined by `tool_call_id`. const argsByCallId = new Map() for (const message of messages) { - // A job result is whatever its script returned, and the shape check that got - // us here only proves each message has a `role`. Anything nested is still - // arbitrary, and a throw here would take the whole result viewer down with - // it — including the plain error a lookalike payload is usually attached to. - if (!Array.isArray(message.tool_calls)) { - continue - } - for (const call of message.tool_calls) { + for (const call of message.tool_calls ?? []) { if (call.id && typeof call.function?.arguments === 'string') { argsByCallId.set(call.id, call.function.arguments) } diff --git a/frontend/src/lib/components/aiAgentResult.test.ts b/frontend/src/lib/components/aiAgentResult.test.ts index 75875e8c30..a055e4d26b 100644 --- a/frontend/src/lib/components/aiAgentResult.test.ts +++ b/frontend/src/lib/components/aiAgentResult.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it } from 'vitest' +import { buildAgentTrace } from './agentTrace' import { advanceAgentStream, emptyAgentStreamProgress, @@ -170,3 +171,38 @@ describe('a stream that narrates before calling a tool', () => { ) }) }) + +// A result is whatever a script returned, so a message that has a `role` still has +// arbitrary anything underneath. Coercing once here is what lets every reader +// treat `AgentMessage` as true; a bad value reaching them throws mid-render and +// takes the result viewer down, including the plain error it usually rides on. +describe('coercing messages at the boundary', () => { + function messagesOf(raw: unknown) { + return parseAgentResult({ output: '', messages: raw })?.messages + } + + it.each([ + ['tool_calls that are not a list', { role: 'assistant', tool_calls: {} }], + ['a null entry inside tool_calls', { role: 'assistant', tool_calls: [null] }], + ['a tool call whose function is a string', { role: 'assistant', tool_calls: [{ id: 'c1', function: 'q' }] }], + ['non-string arguments', { role: 'assistant', tool_calls: [{ id: 'c1', function: { arguments: 3 } }] }], + ['annotations that are not a list', { role: 'assistant', annotations: 'abc' }], + ['a null entry inside annotations', { role: 'assistant', annotations: [null] }], + ['an annotation with no url', { role: 'assistant', annotations: [{ title: 'x' }] }], + ['an agent_action that is not an object', { role: 'tool', agent_action: 'tool_call' }], + ['an agent_action with no type', { role: 'tool', agent_action: {} }] + ])('survives %s', (_label, message) => { + const parsed = messagesOf([message]) + expect(parsed).toHaveLength(1) + expect(() => buildAgentTrace(parsed!)).not.toThrow() + }) + + it('keeps a well-formed call intact', () => { + const parsed = messagesOf([ + { role: 'assistant', tool_calls: [{ id: 'c1', type: 'function', function: { name: 'q', arguments: '{}' } }] } + ]) + expect(parsed?.[0].tool_calls).toEqual([ + { id: 'c1', type: 'function', function: { name: 'q', arguments: '{}' } } + ]) + }) +}) diff --git a/frontend/src/lib/components/aiAgentResult.ts b/frontend/src/lib/components/aiAgentResult.ts index c954ac3964..19ea2419e0 100644 --- a/frontend/src/lib/components/aiAgentResult.ts +++ b/frontend/src/lib/components/aiAgentResult.ts @@ -43,6 +43,51 @@ function hasRole(message: unknown): boolean { return isRecord(message) && typeof message.role === 'string' } +/** + * A job result is whatever its script returned, so a message that passed the + * shape check still has arbitrary anything underneath. Everything downstream + * reads these as `AgentMessage`, and a value of the wrong type there throws in + * the middle of rendering, taking the whole result viewer with it. + * + * So the coercion happens once, here: past this point the declared type is the + * real one, and no reader needs a guard of its own. + */ +function toAgentMessage(raw: Record): AgentMessage { + const toolCalls = Array.isArray(raw.tool_calls) + ? raw.tool_calls.filter(isRecord).map((call) => ({ + id: typeof call.id === 'string' ? call.id : undefined, + type: typeof call.type === 'string' ? call.type : undefined, + function: isRecord(call.function) + ? { + name: typeof call.function.name === 'string' ? call.function.name : undefined, + arguments: + typeof call.function.arguments === 'string' ? call.function.arguments : undefined + } + : undefined + })) + : undefined + const annotations = Array.isArray(raw.annotations) + ? raw.annotations.filter((a): a is Record => isRecord(a) && typeof a.url === 'string') + : undefined + return { + role: raw.role as string, + content: raw.content, + tool_calls: toolCalls, + tool_call_id: typeof raw.tool_call_id === 'string' ? raw.tool_call_id : undefined, + // The union is discriminated on `type`; an action without a string one + // matches no branch and is treated as untagged. + agent_action: + isRecord(raw.agent_action) && typeof raw.agent_action.type === 'string' + ? (raw.agent_action as unknown as AgentAction) + : undefined, + annotations: annotations as AgentMessage['annotations'] + } +} + +function toAgentMessages(raw: unknown[]): AgentMessage[] { + return raw.map((message) => toAgentMessage(message as Record)) +} + /** * Recognise the envelope by its shape rather than by a marker key the worker * would have to add: sniffing works on runs that already completed, and the @@ -79,7 +124,7 @@ export function parseAgentResult(result: unknown): AgentResult | undefined { } return { output: result.output, - messages: result.messages as AgentMessage[], + messages: toAgentMessages(result.messages), usage: isRecord(result.usage) ? (result.usage as AgentTokenUsage) : undefined, wm_stream: typeof result.wm_stream === 'string' ? result.wm_stream : undefined } @@ -101,7 +146,7 @@ export function parseAgentErrorMessages(result: unknown): AgentMessage[] | undef if (inner.messages.length === 0 || !inner.messages.every(hasRole)) { return undefined } - return inner.messages as AgentMessage[] + return toAgentMessages(inner.messages) } export type AgentResultSummary = {