diff --git a/.changeset/harden-strict-provider-wire-compliance.md b/.changeset/harden-strict-provider-wire-compliance.md new file mode 100644 index 000000000..b98307895 --- /dev/null +++ b/.changeset/harden-strict-provider-wire-compliance.md @@ -0,0 +1,5 @@ +--- +"@moonshot-ai/kimi-code": patch +--- + +Stop a malformed message history from permanently bricking a session on strict providers (Anthropic). The request is repaired before sending — orphaned tool calls are closed and empty/whitespace-only text blocks dropped — and if the provider still rejects its structure, it is resent once with a wire-compliant rebuild. diff --git a/packages/agent-core/src/agent/context/index.ts b/packages/agent-core/src/agent/context/index.ts index 15c2c64d3..b9192ded0 100644 --- a/packages/agent-core/src/agent/context/index.ts +++ b/packages/agent-core/src/agent/context/index.ts @@ -13,7 +13,12 @@ import { type CompactionInput, type CompactionResult, } from '../compaction'; -import { project, type ProjectOptions, trimTrailingOpenToolExchange } from './projector'; +import { + project, + type ProjectionAnomaly, + type ProjectOptions, + trimTrailingOpenToolExchange, +} from './projector'; import { USER_PROMPT_ORIGIN, type AgentContextData, @@ -43,6 +48,9 @@ export class ContextMemory { private pendingToolResultIds = new Set(); private deferredMessages: ContextMessage[] = []; private _lastAssistantAt: number | null = null; + // Signature of the last logged set of projection repairs, so a repair that + // recurs identically on every send is logged once rather than per step. + private lastProjectionRepairSignature: string | null = null; constructor(protected readonly agent: Agent) {} @@ -314,13 +322,95 @@ export class ContextMemory { } project(messages: readonly ContextMessage[], options?: ProjectOptions): Message[] { - return project(this.agent.microCompaction.compact(messages), options); + const anomalies: ProjectionAnomaly[] = []; + const result = project(this.agent.microCompaction.compact(messages), { + ...options, + onAnomaly: (anomaly) => { + anomalies.push(anomaly); + options?.onAnomaly?.(anomaly); + }, + }); + this.reportProjectionRepairs(anomalies); + return result; + } + + // Surface the projector's wire-repairs so a silently-mangled history leaves a + // trace instead of being papered over. Deduped by signature: a repair that + // recurs identically every send (e.g. a persistently lost result re-synthesized + // each turn) logs once, not per step. Trailing-tail synthesis is excluded — it + // is the expected close of an in-flight call under `synthesizeMissing` + // (compaction / strict resend), not a defect. + private reportProjectionRepairs(anomalies: readonly ProjectionAnomaly[]): void { + const notable = anomalies.filter( + (anomaly) => !(anomaly.kind === 'tool_result_synthesized' && anomaly.trailing), + ); + if (notable.length === 0) { + this.lastProjectionRepairSignature = null; + return; + } + const signature = notable + .map((anomaly) => ('toolCallId' in anomaly ? `${anomaly.kind}:${anomaly.toolCallId}` : anomaly.kind)) + .toSorted() + .join('|'); + if (signature === this.lastProjectionRepairSignature) return; + this.lastProjectionRepairSignature = signature; + + let reordered = 0; + let synthesized = 0; + let droppedOrphan = 0; + let leadingDropped = 0; + let assistantsMerged = 0; + let whitespaceDropped = 0; + for (const anomaly of notable) { + if (anomaly.kind === 'tool_result_reordered') reordered += 1; + else if (anomaly.kind === 'tool_result_synthesized') synthesized += 1; + else if (anomaly.kind === 'orphan_tool_result_dropped') droppedOrphan += 1; + else if (anomaly.kind === 'leading_non_user_dropped') leadingDropped += 1; + else if (anomaly.kind === 'consecutive_assistants_merged') assistantsMerged += 1; + else whitespaceDropped += 1; + } + const toolCallIds = [ + ...new Set( + notable.flatMap((anomaly) => ('toolCallId' in anomaly ? [anomaly.toolCallId] : [])), + ), + ].slice(0, 5); + this.agent.log.warn('repaired the request to keep it wire-valid', { + reordered, + synthesized, + droppedOrphan, + leadingDropped, + assistantsMerged, + whitespaceDropped, + toolCallIds, + }); + this.agent.telemetry.track('context_projection_repaired', { + reordered, + synthesized, + dropped_orphan: droppedOrphan, + leading_dropped: leadingDropped, + assistants_merged: assistantsMerged, + whitespace_dropped: whitespaceDropped, + }); } get messages(): Message[] { return this.project(this.history); } + // Last-resort projection for the post-400 strict resend: close every open tool + // call (including a trailing in-flight one) and drop any stray tool result with + // no matching call, so the request is wire-compliant for strict providers no + // matter how the history was mangled. Only used when the provider has already + // rejected the normal projection — see the adjacency fallback in `turn-step`. + get strictMessages(): Message[] { + return this.project(this.history, { + synthesizeMissing: true, + dropOrphanResults: true, + dropLeadingNonUser: true, + mergeConsecutiveAssistants: true, + }); + } + useProjectedHistoryFrom(source: ContextMemory): void { this.clear(); this.pushHistory(...trimTrailingOpenToolExchange(source.project(source.history))); @@ -500,7 +590,12 @@ function toolResultOutputForModel(result: ExecutableToolResult): string | Conten return isEmptyOutputText(output) ? TOOL_EMPTY_STATUS : output; } - if (output.length === 0) { + // Treat an array output with no sendable content (empty, or only empty/ + // whitespace-only text blocks) the same as an empty string output: emit the + // placeholder. Otherwise projection would strip the blank text blocks, leave + // the tool message empty, and throw on every send — bricking the session. A + // non-text part (image/etc.) or any non-whitespace text keeps the real output. + if (isEmptyEquivalentContentArray(output)) { return [ { type: 'text', @@ -514,8 +609,12 @@ function toolResultOutputForModel(result: ExecutableToolResult): string | Conten return output; } +function isEmptyEquivalentContentArray(output: readonly ContentPart[]): boolean { + return output.every((part) => part.type === 'text' && part.text.trim().length === 0); +} + function isEmptyOutputText(output: string): boolean { - return output.length === 0 || output.trim() === TOOL_OUTPUT_EMPTY_TEXT; + return output.trim().length === 0 || output.trim() === TOOL_OUTPUT_EMPTY_TEXT; } function formatUndoUnavailableMessage( diff --git a/packages/agent-core/src/agent/context/projector.ts b/packages/agent-core/src/agent/context/projector.ts index c10de2f9a..9471b2ead 100644 --- a/packages/agent-core/src/agent/context/projector.ts +++ b/packages/agent-core/src/agent/context/projector.ts @@ -5,19 +5,94 @@ import type { ContextMessage } from './types'; export interface ProjectOptions { /** - * When `true`, emit a synthetic `tool_result` for any assistant `tool_use` - * whose result is not present in the provided messages. Used by full - * compaction, where the compacted prefix is a slice that may exclude a - * delayed result preserved in the retained tail; the synthetic result keeps - * the exchange closed so the summary request is not rejected. Leave `false` - * for normal turns, where a missing result means the call is still in-flight - * and must not be closed prematurely. + * When `true`, emit a synthetic `tool_result` for *every* assistant `tool_use` + * whose result is not present in the provided messages — including a trailing, + * still-in-flight call. Used by full compaction, where the compacted prefix is + * a slice that may exclude a delayed result preserved in the retained tail; the + * synthetic result keeps the exchange closed so the summary request is not + * rejected. Leave `false` for normal turns: a *trailing* missing result there + * means the call is still in-flight and must not be closed prematurely. (A + * *non-trailing* missing result is always closed regardless of this flag — see + * `repairToolExchangeAdjacency` — because a later turn proves it is not + * in-flight.) */ readonly synthesizeMissing?: boolean; + /** + * When `true`, drop any `tool_result` whose `toolCallId` matches no assistant + * `tool_use` anywhere in the provided messages. Strict providers reject such a + * stray result as an "unexpected `tool_result`". Off by default so the normal + * path never silently discards recorded output; the post-400 strict-resend + * fallback enables it (together with `synthesizeMissing`) as a last resort to + * force a wire-compliant request out of an otherwise-bricked session. + */ + readonly dropOrphanResults?: boolean; + /** + * When `true`, drop leading messages until the first one is a user turn. Strict + * providers require the first message to be `user`; a history that (after + * dropping/compaction) starts with an assistant or tool message is rejected. + * Strict-resend only — the normal path keeps the original opening. + */ + readonly dropLeadingNonUser?: boolean; + /** + * When `true`, merge back-to-back assistant messages into one. Strict providers + * reject consecutive same-role turns ("roles must alternate"); consecutive user + * turns are already merged at the provider boundary, but consecutive assistant + * turns are not. Strict-resend only. Content is concatenated verbatim — callers + * must not rely on this when extended-thinking ordering matters, but two + * consecutive assistant turns do not arise in well-formed transcripts. + */ + readonly mergeConsecutiveAssistants?: boolean; + /** + * Optional sink invoked for every repair the projector applies to keep the + * outgoing wire valid: a displaced result moved back next to its call, a + * synthetic result invented for a missing one, a stray result dropped, a + * leading non-user message dropped, or consecutive assistants merged. The + * projection itself stays a pure transform; the caller decides whether/how to + * surface these (the context logs them so a silently-mangled history is never + * papered over without a trace). Not called when the history is already + * well-formed. + */ + readonly onAnomaly?: (anomaly: ProjectionAnomaly) => void; } +/** + * A repair the projector applied to make the history wire-valid. Each one means + * the stored history was not directly sendable to a strict provider. + */ +export type ProjectionAnomaly = + /** A recorded result was not adjacent to its call and had to be moved up. */ + | { readonly kind: 'tool_result_reordered'; readonly toolCallId: string } + /** + * No result existed for a call, so a placeholder was synthesized. `trailing` + * is true when it closed a still-open tail call (expected under + * `synthesizeMissing`), false when it closed a mid-history orphan whose result + * was lost (a genuine defect worth investigating). + */ + | { readonly kind: 'tool_result_synthesized'; readonly toolCallId: string; readonly trailing: boolean } + /** A result with no matching call anywhere was dropped (strict resend only). */ + | { readonly kind: 'orphan_tool_result_dropped'; readonly toolCallId: string } + /** A leading non-user message was dropped so the first turn is user (strict). */ + | { readonly kind: 'leading_non_user_dropped'; readonly role: string } + /** Two adjacent assistant turns were merged into one (strict). */ + | { readonly kind: 'consecutive_assistants_merged' } + /** A non-empty but all-whitespace text block was dropped (always). */ + | { readonly kind: 'whitespace_text_dropped'; readonly role: string }; + export function project(history: readonly ContextMessage[], options?: ProjectOptions): Message[] { - return repairToolExchangeAdjacency(mergeAdjacentUserMessages(history), options); + let result = repairToolExchangeAdjacency( + mergeAdjacentUserMessages(history, options?.onAnomaly), + options, + ); + if (options?.mergeConsecutiveAssistants === true) { + result = mergeConsecutiveAssistantMessages(result, options.onAnomaly); + } + if (options?.dropOrphanResults === true) { + result = dropOrphanToolResults(result, options.onAnomaly); + } + if (options?.dropLeadingNonUser === true) { + result = dropLeadingNonUserMessages(result, options.onAnomaly); + } + return result; } // Strict providers (Anthropic) require every assistant `tool_use` to be answered @@ -32,16 +107,21 @@ export function project(history: readonly ContextMessage[], options?: ProjectOpt // Repair the adjacency so every assistant `tool_use` is immediately followed by // its matching `tool_result` message(s). Matching results are moved up from // wherever they appear later in the history; any intervening messages keep their -// relative order and simply follow the repaired exchange. A tool call with no -// recorded result anywhere later in the history is left untouched by default — -// it is still in-flight (pending) rather than orphaned, and the -// trailing-open-exchange trim plus the interrupted-result synthesis during replay -// own those cases. With `synthesizeMissing`, a synthetic `tool_result` is emitted -// for such calls instead; full compaction uses this to keep a sliced prefix -// closed when a delayed result lives in the retained tail. This is purely a -// projection-time fix: the underlying history is left untouched, so replay and -// transcripts keep their original order, while the model always sees a -// well-formed tool exchange. +// relative order and simply follow the repaired exchange. +// +// A tool call with no recorded result anywhere is closed with a synthetic +// `tool_result` UNLESS it belongs to the trailing exchange (no later +// user/assistant message follows it). A non-trailing missing result can never be +// in-flight — a subsequent turn proves the model already moved on — so leaving it +// open would strand the whole session behind a 400 on every send; it is closed +// here instead. The trailing exchange is left untouched by default: there a +// missing result genuinely means the call is still pending, and the +// trailing-open-exchange trim plus replay's interrupted-result synthesis own that +// case. With `synthesizeMissing`, even the trailing call is closed; full +// compaction uses this to keep a sliced prefix closed when a delayed result lives +// in the retained tail. This is purely a projection-time fix: the underlying +// history is left untouched, so replay and transcripts keep their original order, +// while the model always sees a well-formed tool exchange. const SYNTHETIC_TOOL_RESULT_TEXT = 'Tool result is not available in the current context. Do not assume the tool completed successfully.'; @@ -49,6 +129,16 @@ function repairToolExchangeAdjacency( messages: readonly Message[], options?: ProjectOptions, ): Message[] { + // The trailing exchange is the only one whose missing result may still be + // in-flight: any assistant `tool_use` that precedes a later user/assistant + // message has been overtaken by a new turn and cannot be pending. Find the last + // non-tool message so an orphan can be classified as trailing (index >= it) or + // mid-history (index < it). + let lastNonToolIndex = messages.length - 1; + while (lastNonToolIndex >= 0 && messages[lastNonToolIndex]?.role === 'tool') { + lastNonToolIndex -= 1; + } + const out: Message[] = []; const consumed = new Set(); for (let i = 0; i < messages.length; i++) { @@ -61,6 +151,10 @@ function repairToolExchangeAdjacency( out.push(message); const pending = new Set(message.toolCalls.map((toolCall) => toolCall.id)); + // Tracks whether a foreign message (anything that is not one of this + // exchange's own results) sits between the call and a later matching result; + // if so, that result was displaced and pulling it up is a real repair. + let foreignBetween = false; for (let j = i + 1; j < messages.length && pending.size > 0; j++) { if (consumed.has(j)) continue; const next = messages[j]!; @@ -69,23 +163,94 @@ function repairToolExchangeAdjacency( out.push(next); consumed.add(j); pending.delete(toolCallId); + if (foreignBetween) options?.onAnomaly?.({ kind: 'tool_result_reordered', toolCallId }); + } else { + foreignBetween = true; } } - if (options?.synthesizeMissing === true) { - // Close any tool call whose result is absent from the provided messages. - // Only used by full compaction, where the prefix is a slice that may - // exclude a delayed result preserved in the retained tail. For normal - // turns a missing result means the call is still in-flight, so it is left - // for the trailing-open-exchange trim and replay's interrupted-result - // synthesis instead of being closed here. + // Close any tool call whose result is absent. A mid-history orphan (a later + // user/assistant message follows) is always closed — it cannot be in-flight. + // The trailing exchange is closed only when `synthesizeMissing` is set, so a + // genuinely pending call is left for the trim / replay synthesis otherwise. + const isMidHistory = i < lastNonToolIndex; + if (options?.synthesizeMissing === true || isMidHistory) { for (const missingId of pending) { out.push(makeSyntheticToolResult(missingId)); + options?.onAnomaly?.({ + kind: 'tool_result_synthesized', + toolCallId: missingId, + trailing: !isMidHistory, + }); } } } return out; } +// Remove any `tool_result` whose `toolCallId` matches no assistant `tool_use` +// anywhere in the projected messages. Strict providers reject such a stray +// result; the post-400 strict-resend fallback drops them as a last resort. Kept +// separate from the adjacency repair so the normal path never discards output. +function dropOrphanToolResults( + messages: readonly Message[], + onAnomaly?: (anomaly: ProjectionAnomaly) => void, +): Message[] { + const toolUseIds = new Set(); + for (const message of messages) { + if (message.role === 'assistant') { + for (const toolCall of message.toolCalls) toolUseIds.add(toolCall.id); + } + } + return messages.filter((message) => { + if (message.role !== 'tool' || message.toolCallId === undefined) return true; + if (toolUseIds.has(message.toolCallId)) return true; + onAnomaly?.({ kind: 'orphan_tool_result_dropped', toolCallId: message.toolCallId }); + return false; + }); +} + +// Merge back-to-back assistant messages into one. Strict providers reject +// consecutive same-role turns; the provider boundary already merges consecutive +// user turns, but not assistant turns. Strict-resend only. Content is +// concatenated verbatim (no reordering), so this is safe for the well-formed +// transcripts where it never fires, and a best-effort last resort otherwise. +function mergeConsecutiveAssistantMessages( + messages: readonly Message[], + onAnomaly?: (anomaly: ProjectionAnomaly) => void, +): Message[] { + const out: Message[] = []; + for (const message of messages) { + const previous = out.at(-1); + if (previous !== undefined && previous.role === 'assistant' && message.role === 'assistant') { + out[out.length - 1] = { + ...previous, + content: [...previous.content, ...message.content], + toolCalls: [...previous.toolCalls, ...message.toolCalls], + }; + onAnomaly?.({ kind: 'consecutive_assistants_merged' }); + continue; + } + out.push(message); + } + return out; +} + +// Drop leading messages until the first one is a user turn. Strict providers +// require the first message to be `user`; a history that starts with an +// assistant or tool message (after dropping/compaction edge cases) is rejected. +// Strict-resend only. +function dropLeadingNonUserMessages( + messages: readonly Message[], + onAnomaly?: (anomaly: ProjectionAnomaly) => void, +): Message[] { + let start = 0; + while (start < messages.length && messages[start]!.role !== 'user') { + onAnomaly?.({ kind: 'leading_non_user_dropped', role: messages[start]!.role }); + start += 1; + } + return start === 0 ? [...messages] : messages.slice(start); +} + function makeSyntheticToolResult(toolCallId: string): Message { return { role: 'tool', @@ -95,10 +260,13 @@ function makeSyntheticToolResult(toolCallId: string): Message { }; } -function mergeAdjacentUserMessages(history: readonly ContextMessage[]): Message[] { +function mergeAdjacentUserMessages( + history: readonly ContextMessage[], + onAnomaly?: (anomaly: ProjectionAnomaly) => void, +): Message[] { const out: ContextMessage[] = []; for (const source of history) { - const message = prepareMessageForProjection(source); + const message = prepareMessageForProjection(source, onAnomaly); if (message === null) continue; const previous = out.at(-1); @@ -115,13 +283,26 @@ function mergeAdjacentUserMessages(history: readonly ContextMessage[]): Message[ return out.map(stripContextMetadata); } -function prepareMessageForProjection(message: ContextMessage): ContextMessage | null { +function prepareMessageForProjection( + message: ContextMessage, + onAnomaly?: (anomaly: ProjectionAnomaly) => void, +): ContextMessage | null { if (message.partial === true) return null; let content: ContentPart[] | undefined; for (const [index, part] of message.content.entries()) { - if (part.type === 'text' && part.text.length === 0) { + // Strict providers reject a text block that is empty OR whitespace-only + // ("text content blocks must contain non-whitespace text"). Drop both; a + // block with surrounding whitespace but real content is kept verbatim. + if (part.type === 'text' && part.text.trim().length === 0) { content ??= message.content.slice(0, index); + // Report only whitespace-only (non-empty) blocks: a truly empty `''` block + // is routine cleanup (e.g. a trailing empty text part after a tool call), + // whereas a block that is non-empty yet all-whitespace signals something + // upstream fed blank content and is worth surfacing for debugging. + if (part.text.length > 0) { + onAnomaly?.({ kind: 'whitespace_text_dropped', role: message.role }); + } continue; } content?.push(part); diff --git a/packages/agent-core/src/agent/turn/index.ts b/packages/agent-core/src/agent/turn/index.ts index 08f2178d0..847794aaa 100644 --- a/packages/agent-core/src/agent/turn/index.ts +++ b/packages/agent-core/src/agent/turn/index.ts @@ -682,6 +682,7 @@ export class TurnFlow { signal, llm: this.agent.llm, buildMessages: () => this.agent.context.messages, + buildMessagesStrict: () => this.agent.context.strictMessages, dispatchEvent: this.buildDispatchEvent(turnId), tools: this.agent.tools.loopTools, log: this.agent.log, diff --git a/packages/agent-core/src/loop/run-turn.ts b/packages/agent-core/src/loop/run-turn.ts index 326dba854..3ee74cbcd 100644 --- a/packages/agent-core/src/loop/run-turn.ts +++ b/packages/agent-core/src/loop/run-turn.ts @@ -34,6 +34,12 @@ export interface RunTurnInput { readonly signal: AbortSignal; readonly llm: LLM; readonly buildMessages: LoopMessageBuilder; + /** + * Optional strict, guaranteed wire-compliant rebuild of the request messages. + * Used only to resend once after a provider rejects the normal projection with + * a tool_use/tool_result adjacency 400 (see `executeLoopStep`). + */ + readonly buildMessagesStrict?: LoopMessageBuilder | undefined; readonly dispatchEvent: LoopEventDispatcher; readonly tools?: readonly ExecutableTool[] | undefined; readonly hooks?: LoopHooks | undefined; @@ -51,6 +57,7 @@ export async function runTurn(input: RunTurnInput): Promise { signal, llm, buildMessages, + buildMessagesStrict, dispatchEvent, tools, hooks, @@ -85,6 +92,7 @@ export async function runTurn(input: RunTurnInput): Promise { turnId, signal, buildMessages, + buildMessagesStrict, dispatchEvent, llm, tools, diff --git a/packages/agent-core/src/loop/turn-step.ts b/packages/agent-core/src/loop/turn-step.ts index 2a0fc6132..8d72cad5d 100644 --- a/packages/agent-core/src/loop/turn-step.ts +++ b/packages/agent-core/src/loop/turn-step.ts @@ -9,10 +9,11 @@ import { randomUUID } from 'node:crypto'; -import type { TokenUsage } from '@moonshot-ai/kosong'; +import { isRecoverableRequestStructureError, type TokenUsage } from '@moonshot-ai/kosong'; import type { Logger } from '#/logging/types'; import type { LoopEventDispatcher } from './events'; +import { errorMessage } from './errors'; import type { LLM, LLMChatParams, LLMChatResponse } from './llm'; import { chatWithRetry } from './retry'; import { runToolCallBatch, type ToolCallStepContext } from './tool-call'; @@ -33,6 +34,7 @@ export interface ExecuteLoopStepDeps { readonly turnId: string; readonly signal: AbortSignal; readonly buildMessages: LoopMessageBuilder; + readonly buildMessagesStrict?: LoopMessageBuilder | undefined; readonly dispatchEvent: LoopEventDispatcher; readonly llm: LLM; readonly tools?: readonly ExecutableTool[] | undefined; @@ -51,6 +53,7 @@ export async function executeLoopStep(deps: ExecuteLoopStepDeps): Promise<{ turnId, signal, buildMessages, + buildMessagesStrict, dispatchEvent, llm, tools, @@ -110,16 +113,56 @@ export async function executeLoopStep(deps: ExecuteLoopStepDeps): Promise<{ stepUuid, }), }; - const response: LLMChatResponse = await chatWithRetry({ + const retryInput = { llm, - params: chatParams, dispatchEvent, turnId, currentStep, stepUuid, maxAttempts: maxRetryAttempts, log, - }); + } as const; + let response: LLMChatResponse; + try { + response = await chatWithRetry({ ...retryInput, params: chatParams }); + } catch (error) { + // A structural request rejection (tool_use/tool_result pairing, empty or + // whitespace-only text, non-user first message, non-alternating roles) means + // the projected history is not wire-compliant for a strict provider — and + // since the same history is re-sent every turn, the session would stay stuck + // on this error forever. Resend ONCE with a strict, guaranteed-compliant + // rebuild (every open call closed, stray results dropped, leading non-user + // trimmed, consecutive assistants merged) as a last resort. Any other error, + // or a host that supplied no strict builder, propagates unchanged. + if (buildMessagesStrict === undefined || !isRecoverableRequestStructureError(error)) throw error; + signal.throwIfAborted(); + log?.warn('provider rejected request structure; resending with strict projection', { + turnStep: `${turnId}.${String(currentStep)}`, + model: llm.modelName, + }); + const strictMessages = await buildMessagesStrict(); + signal.throwIfAborted(); + try { + response = await chatWithRetry({ + ...retryInput, + params: { ...chatParams, messages: strictMessages }, + }); + } catch (strictError) { + // The strictly-sanitized rebuild was still rejected — our wire-compliance + // repair did not cover this case. Surface it loudly: the session is stuck + // and this is the signal we need to diagnose the gap. + log?.error('strict resend still rejected by provider; request remains wire-invalid', { + turnStep: `${turnId}.${String(currentStep)}`, + model: llm.modelName, + originalError: errorMessage(error), + strictError: errorMessage(strictError), + }); + throw strictError; + } + log?.info('recovered after strict resend', { + turnStep: `${turnId}.${String(currentStep)}`, + }); + } const usage = response.usage; const usageResult = await recordUsage(usage); const stopTurnAfterUsage = usageResult?.stopTurn === true; diff --git a/packages/agent-core/test/agent/compaction/anthropic-compliance.test.ts b/packages/agent-core/test/agent/compaction/anthropic-compliance.test.ts index 521f0cbd3..fa2315858 100644 --- a/packages/agent-core/test/agent/compaction/anthropic-compliance.test.ts +++ b/packages/agent-core/test/agent/compaction/anthropic-compliance.test.ts @@ -221,6 +221,64 @@ describe('compaction — Anthropic wire compliance', () => { assertValidAnthropic(wire); }); + it('closes a mid-history tool call whose result is missing on the normal send path', async () => { + const ctx = testAgent(); + ctx.configure({ provider: PROVIDER, modelCapabilities: CAPS }); + // 'call_1' was issued but its result was never recorded; a later real turn + // ('call_2' + result) proves it is not in-flight. On a strict provider this + // bricks the session — every normal send re-rejects. The projector closes the + // mid-history orphan WITHOUT synthesizeMissing (the normal send path). + const orphaned: ContextMessage[] = [ + { role: 'user', content: [{ type: 'text', text: 'run it' }], toolCalls: [], origin: { kind: 'user' } }, + { + role: 'assistant', + content: [{ type: 'text', text: 'first call' }], + toolCalls: [{ type: 'function', id: 'call_1', name: 'Bash', arguments: '{}' }], + }, + { role: 'user', content: [{ type: 'text', text: 'next thing' }], toolCalls: [], origin: { kind: 'user' } }, + { + role: 'assistant', + content: [{ type: 'text', text: 'second call' }], + toolCalls: [{ type: 'function', id: 'call_2', name: 'Bash', arguments: '{}' }], + }, + { role: 'tool', content: [{ type: 'text', text: 'done' }], toolCalls: [], toolCallId: 'call_2' }, + ]; + + // Normal send path: no synthesizeMissing, no dropOrphanResults. + const projected = ctx.agent.context.project(orphaned); + const wire = await toAnthropicWire(projected, [BASH_TOOL]); + assertValidAnthropic(wire); + // The mid-history orphan 'call_1' is closed by a synthetic tool_result. + const call1Index = wire.findIndex((m) => + m.content.some((b) => b.type === 'tool_use' && b.id === 'call_1'), + ); + expect(call1Index).toBeGreaterThanOrEqual(0); + expect( + wire[call1Index + 1]!.content.some( + (b) => b.type === 'tool_result' && b.tool_use_id === 'call_1', + ), + ).toBe(true); + }); + + it('drops a stray tool result with no matching call on the strict resend path', async () => { + const ctx = testAgent(); + ctx.configure({ provider: PROVIDER, modelCapabilities: CAPS }); + // A tool_result whose tool_use is gone (e.g. an undo removed the assistant). + // The normal path leaves it (it has no anchor); the strict resend drops it. + const stray: ContextMessage[] = [ + { role: 'user', content: [{ type: 'text', text: 'hello' }], toolCalls: [], origin: { kind: 'user' } }, + { role: 'tool', content: [{ type: 'text', text: 'orphan output' }], toolCalls: [], toolCallId: 'gone' }, + ]; + + const projected = ctx.agent.context.project(stray, { + synthesizeMissing: true, + dropOrphanResults: true, + }); + expect(projected.some((m) => m.role === 'tool')).toBe(false); + const wire = await toAnthropicWire(projected); + assertValidAnthropic(wire); + }); + it('closes a still-open tool call in the summarizer request with a synthetic result', async () => { const ctx = testAgent(); ctx.configure({ provider: PROVIDER, modelCapabilities: CAPS }); diff --git a/packages/agent-core/test/agent/context.test.ts b/packages/agent-core/test/agent/context.test.ts index eaec4923d..9ebe197ab 100644 --- a/packages/agent-core/test/agent/context.test.ts +++ b/packages/agent-core/test/agent/context.test.ts @@ -168,6 +168,52 @@ describe('Agent context', () => { expect(textOf(output)).toContain('exit code'); }); + it('normalizes a whitespace-only array tool result to the empty-output placeholder', () => { + const ctx = testAgent(); + ctx.configure(); + + ctx.dispatch({ + type: 'context.append_loop_event', + event: { type: 'step.begin', uuid: 's1', turnId: 't', step: 1 }, + }); + ctx.dispatch({ + type: 'context.append_loop_event', + event: { + type: 'tool.call', + uuid: 'call_ws', + turnId: 't', + step: 1, + stepUuid: 's1', + toolCallId: 'call_ws', + name: 'Run', + args: {}, + }, + }); + ctx.dispatch({ + type: 'context.append_loop_event', + event: { + type: 'tool.result', + parentUuid: 'call_ws', + toolCallId: 'call_ws', + // Array (ContentPart[]) output whose only block is whitespace. The tool + // contract allows arbitrary content arrays (e.g. MCP tools), so this must + // be normalized to the empty placeholder rather than left to be stripped + // empty by projection (which would throw on every send). + result: { output: [{ type: 'text', text: ' \n' }] }, + }, + }); + + expect(() => ctx.agent.context.messages).not.toThrow(); + expect(ctx.agent.context.messages).toMatchObject([ + { role: 'assistant', toolCalls: [{ id: 'call_ws' }] }, + { + role: 'tool', + content: [{ type: 'text', text: 'Tool output is empty.' }], + toolCallId: 'call_ws', + }, + ]); + }); + it('renders tool error and empty-output status as model-visible text', () => { const ctx = testAgent(); ctx.configure(); @@ -227,7 +273,7 @@ describe('Agent context', () => { ]); }); - it('drops empty text parts only in LLM projection', () => { + it('drops empty and whitespace-only text parts in LLM projection', () => { const history: ContextMessage[] = [ { role: 'user', @@ -247,12 +293,20 @@ describe('Agent context', () => { content: [{ type: 'text', text: '' }], toolCalls: [{ type: 'function', id: 'call_empty', name: 'empty', arguments: '{}' }], }, + { + role: 'tool', + content: [{ type: 'text', text: 'result' }], + toolCalls: [], + toolCallId: 'call_empty', + }, { role: 'assistant', content: [{ type: 'think', think: '', encrypted: 'enc_empty_thinking' }], toolCalls: [], }, { + // Whitespace-only message: strict providers reject the block, so the + // whole message is dropped from the projection. role: 'user', content: [{ type: 'text', text: ' ' }], toolCalls: [], @@ -271,13 +325,14 @@ describe('Agent context', () => { toolCalls: [{ type: 'function', id: 'call_empty', name: 'empty', arguments: '{}' }], }, { - role: 'assistant', - content: [{ type: 'think', think: '', encrypted: 'enc_empty_thinking' }], + role: 'tool', + content: [{ type: 'text', text: 'result' }], toolCalls: [], + toolCallId: 'call_empty', }, { - role: 'user', - content: [{ type: 'text', text: ' ' }], + role: 'assistant', + content: [{ type: 'think', think: '', encrypted: 'enc_empty_thinking' }], toolCalls: [], }, ]); diff --git a/packages/agent-core/test/agent/context/projector.test.ts b/packages/agent-core/test/agent/context/projector.test.ts index 6dc2a3ab6..8ddcad859 100644 --- a/packages/agent-core/test/agent/context/projector.test.ts +++ b/packages/agent-core/test/agent/context/projector.test.ts @@ -1,7 +1,7 @@ import type { ContentPart, Message, ToolCall } from '@moonshot-ai/kosong'; import { describe, expect, it } from 'vitest'; -import { project } from '../../../src/agent/context/projector'; +import { project, type ProjectionAnomaly } from '../../../src/agent/context/projector'; import type { ContextMessage } from '../../../src/agent/context/types'; // --------------------------------------------------------------------------- @@ -14,8 +14,10 @@ import type { ContextMessage } from '../../../src/agent/context/types'; // result exists anywhere in the projected history, that result sits in the // consecutive tool messages immediately following the assistant message. // -// A tool call with no recorded result anywhere is considered still in-flight -// (pending) and is intentionally left untouched — it is not an orphan. +// A tool call with no recorded result anywhere is closed with a synthetic +// `tool_result` when a later turn follows it (it cannot be in-flight). Only the +// trailing exchange's missing result is left untouched — there the call is +// genuinely still pending. interface MisplacedToolUse { readonly assistantIndex: number; @@ -63,6 +65,30 @@ function findMisplacedToolUses(messages: readonly Message[]): MisplacedToolUse[] return violations; } +/** + * Strict wire-compliance check: every assistant `tool_use` must be answered by a + * `tool_result` in the consecutive tool messages immediately following it. Use + * only where no trailing in-flight call is expected — an in-flight call has no + * result by design and would (correctly) fail this check. + */ +function everyToolUseImmediatelyAnswered(messages: readonly Message[]): boolean { + for (let i = 0; i < messages.length; i++) { + const message = messages[i]!; + if (message.role !== 'assistant' || message.toolCalls.length === 0) continue; + const adjacentResultIds = new Set(); + let j = i + 1; + while (j < messages.length && messages[j]!.role === 'tool') { + const id = messages[j]!.toolCallId; + if (id !== undefined) adjacentResultIds.add(id); + j++; + } + if (message.toolCalls.some((toolCall) => !adjacentResultIds.has(toolCall.id))) { + return false; + } + } + return true; +} + // --------------------------------------------------------------------------- // Builders // --------------------------------------------------------------------------- @@ -223,7 +249,7 @@ describe('project tool_use/tool_result adjacency', () => { it('leaves a pending (in-flight) tool call without a recorded result untouched', () => { const history: ContextMessage[] = [user('u1'), assistant(['a', 'b']), tool('a')]; - // b has no recorded result — it is still pending, not orphaned. + // b has no recorded result — it is the trailing exchange, still in-flight. const projected = project(history); expect(projected.map((m) => [m.role, m.toolCallId])).toEqual([ ['user', undefined], @@ -234,6 +260,42 @@ describe('project tool_use/tool_result adjacency', () => { expect(projected.some((m) => m.toolCallId === 'b')).toBe(false); }); + it('synthesizes a result for a mid-history tool call whose result is missing entirely', () => { + // 'a' has no recorded result anywhere, but a later turn (u2 / assistant b) + // proves the model already moved on — the call cannot be in-flight, so it is + // a genuine orphan that strict providers reject. It must be closed in place. + const history: ContextMessage[] = [ + user('u1'), + assistant(['a']), + user('u2'), + assistant(['b']), + tool('b'), + ]; + const projected = project(history); + const aIndex = projected.findIndex((m) => m.toolCalls.some((tc) => tc.id === 'a')); + expect(projected[aIndex + 1]).toMatchObject({ role: 'tool', toolCallId: 'a' }); + expect(textOf(projected[aIndex + 1])).toContain('not available'); + expect(findMisplacedToolUses(projected)).toEqual([]); + // No assistant tool_use is left unanswered for a strict provider. + expect(everyToolUseImmediatelyAnswered(projected)).toBe(true); + }); + + it('closes a mid-history orphan while leaving the trailing in-flight call untouched', () => { + // 'a' is a mid-history orphan (a later turn follows); 'b' is the trailing + // exchange whose result is genuinely still pending. + const history: ContextMessage[] = [ + user('u1'), + assistant(['a']), + user('u2'), + assistant(['b']), + ]; + const projected = project(history); + const aIndex = projected.findIndex((m) => m.toolCalls.some((tc) => tc.id === 'a')); + expect(projected[aIndex + 1]).toMatchObject({ role: 'tool', toolCallId: 'a' }); + // The trailing in-flight call 'b' is not closed with a synthetic result. + expect(projected.some((m) => m.toolCallId === 'b')).toBe(false); + }); + it('synthesizes a tool result for a missing tool call when synthesizeMissing is set', () => { const history: ContextMessage[] = [user('u1'), assistant(['a', 'b']), tool('a')]; const projected = project(history, { synthesizeMissing: true }); @@ -313,6 +375,166 @@ describe('project tool_use/tool_result adjacency', () => { }); }); +// --------------------------------------------------------------------------- +// Repair reporting (onAnomaly) +// --------------------------------------------------------------------------- + +describe('project repair reporting', () => { + it('reports nothing for an already well-formed history', () => { + const anomalies: ProjectionAnomaly[] = []; + project([user('u1'), assistant(['a']), tool('a'), user('u2')], { + onAnomaly: (a) => anomalies.push(a), + }); + expect(anomalies).toEqual([]); + }); + + it('reports a displaced result that had to be moved up', () => { + const anomalies: ProjectionAnomaly[] = []; + project([user('u1'), assistant(['a']), notification('ping'), tool('a')], { + onAnomaly: (a) => anomalies.push(a), + }); + expect(anomalies).toEqual([{ kind: 'tool_result_reordered', toolCallId: 'a' }]); + }); + + it('does not report adjacent parallel results that are merely out of order', () => { + const anomalies: ProjectionAnomaly[] = []; + project([user('u1'), assistant(['a', 'b', 'c']), tool('c'), tool('a'), tool('b')], { + onAnomaly: (a) => anomalies.push(a), + }); + expect(anomalies).toEqual([]); + }); + + it('reports a mid-history synthesis as a non-trailing (defect) repair', () => { + const anomalies: ProjectionAnomaly[] = []; + project([user('u1'), assistant(['a']), user('u2'), assistant(['b']), tool('b')], { + onAnomaly: (a) => anomalies.push(a), + }); + expect(anomalies).toEqual([ + { kind: 'tool_result_synthesized', toolCallId: 'a', trailing: false }, + ]); + }); + + it('marks a forced trailing synthesis as trailing (expected, not a defect)', () => { + const anomalies: ProjectionAnomaly[] = []; + project([user('u1'), assistant(['a', 'b']), tool('a')], { + synthesizeMissing: true, + onAnomaly: (a) => anomalies.push(a), + }); + expect(anomalies).toEqual([ + { kind: 'tool_result_synthesized', toolCallId: 'b', trailing: true }, + ]); + }); + + it('reports a dropped orphan result only on the strict path', () => { + const history: ContextMessage[] = [user('u1'), assistant(['a']), tool('a'), tool('stray')]; + const normal: ProjectionAnomaly[] = []; + project(history, { onAnomaly: (a) => normal.push(a) }); + expect(normal).toEqual([]); // normal path leaves the stray result in place + + const strict: ProjectionAnomaly[] = []; + project(history, { dropOrphanResults: true, onAnomaly: (a) => strict.push(a) }); + expect(strict).toEqual([{ kind: 'orphan_tool_result_dropped', toolCallId: 'stray' }]); + }); + + it('reports a whitespace-only text drop but not a truly-empty one', () => { + const anomalies: ProjectionAnomaly[] = []; + project( + [ + // empty '' block (routine) followed by real text — not reported + { role: 'user', content: [textPart(''), textPart('hi')], toolCalls: [] }, + // whitespace-only block dropped — reported + { role: 'assistant', content: [textPart(' \n'), textPart('ok')], toolCalls: [] }, + ], + { onAnomaly: (a) => anomalies.push(a) }, + ); + expect(anomalies).toEqual([{ kind: 'whitespace_text_dropped', role: 'assistant' }]); + }); + + it('reports leading-non-user drops and consecutive-assistant merges (strict)', () => { + const anomalies: ProjectionAnomaly[] = []; + project( + [ + { role: 'assistant', content: [textPart('opener')], toolCalls: [] }, + user('hi'), + { role: 'assistant', content: [textPart('one')], toolCalls: [] }, + { role: 'assistant', content: [textPart('two')], toolCalls: [] }, + ], + { dropLeadingNonUser: true, mergeConsecutiveAssistants: true, onAnomaly: (a) => anomalies.push(a) }, + ); + expect(anomalies).toContainEqual({ kind: 'consecutive_assistants_merged' }); + expect(anomalies).toContainEqual({ kind: 'leading_non_user_dropped', role: 'assistant' }); + }); +}); + +// --------------------------------------------------------------------------- +// Whitespace-only text + strict-provider sanitizers +// --------------------------------------------------------------------------- + +function ws(text: string): ContextMessage { + return { role: 'user', content: [textPart(text)], toolCalls: [] }; +} + +function assistantText(text: string): ContextMessage { + return { role: 'assistant', content: [textPart(text)], toolCalls: [] }; +} + +describe('project drops whitespace-only text', () => { + it('drops a text block that is only whitespace (Anthropic rejects it)', () => { + const projected = project([ + user('real'), + { + role: 'assistant', + content: [textPart(' '), textPart('answer')], + toolCalls: [], + }, + ]); + const assistantMsg = projected.find((m) => m.role === 'assistant'); + expect(assistantMsg?.content).toEqual([{ type: 'text', text: 'answer' }]); + }); + + it('drops a message whose only text block is whitespace', () => { + const projected = project([user('real'), ws(' \n\t ')]); + expect(projected.map((m) => textOf(m))).toEqual(['real']); + }); + + it('keeps surrounding whitespace inside a non-empty block', () => { + const projected = project([user(' hello ')]); + expect(textOf(projected[0])).toBe(' hello '); + }); +}); + +describe('project strict-provider sanitizers', () => { + it('drops leading non-user messages so the first message is a user turn', () => { + // History that (pathologically) starts with an assistant turn. + const projected = project( + [assistantText('stray opener'), user('hi'), assistant(['a']), tool('a')], + { dropLeadingNonUser: true }, + ); + expect(projected[0]?.role).toBe('user'); + expect(textOf(projected[0])).toBe('hi'); + }); + + it('only drops leading non-user under the strict flag (normal path keeps them)', () => { + const history: ContextMessage[] = [assistantText('stray opener'), user('hi')]; + expect(project(history)[0]?.role).toBe('assistant'); + expect(project(history, { dropLeadingNonUser: true })[0]?.role).toBe('user'); + }); + + it('merges consecutive assistant messages under the strict flag', () => { + const projected = project([user('hi'), assistantText('part one'), assistantText('part two')], { + mergeConsecutiveAssistants: true, + }); + expect(projected.map((m) => m.role)).toEqual(['user', 'assistant']); + expect(textOf(projected[1])).toContain('part one'); + expect(textOf(projected[1])).toContain('part two'); + }); + + it('leaves consecutive assistant messages untouched on the normal path', () => { + const projected = project([user('hi'), assistantText('part one'), assistantText('part two')]); + expect(projected.map((m) => m.role)).toEqual(['user', 'assistant', 'assistant']); + }); +}); + // --------------------------------------------------------------------------- // Property-based fuzz test // --------------------------------------------------------------------------- diff --git a/packages/agent-core/test/loop/tool-exchange-fallback.e2e.test.ts b/packages/agent-core/test/loop/tool-exchange-fallback.e2e.test.ts new file mode 100644 index 000000000..6357cb424 --- /dev/null +++ b/packages/agent-core/test/loop/tool-exchange-fallback.e2e.test.ts @@ -0,0 +1,124 @@ +/** + * Post-400 strict-resend fallback. + * + * When a strict provider rejects a step with a tool_use/tool_result adjacency + * 400, the same history would be re-sent every turn and the session would stay + * stuck forever. `executeLoopStep` resends ONCE with a strict, guaranteed + * wire-compliant rebuild (`buildMessagesStrict`). Any other error propagates + * unchanged and the strict builder is never consulted. + */ + +import { APIStatusError, type Message } from '@moonshot-ai/kosong'; +import { describe, expect, it } from 'vitest'; + +import { + createLoopEventDispatcher, + runTurn, + type LoopMessageBuilder, + type RunTurnInput, +} from '../../src/loop/index'; +import { CollectingSink } from './fixtures/collecting-sink'; +import { FakeLLM, makeEndTurnResponse } from './fixtures/fake-llm'; +import { RecordingContext } from './fixtures/recording-context'; + +const ADJACENCY_400 = new APIStatusError( + 400, + 'messages.142: `tool_use` ids were found without `tool_result` blocks immediately after: ' + + 'toolu_01MWFhDRqdbB4nzCJNuWYiun. Each `tool_use` block must have a corresponding ' + + '`tool_result` block in the next message.', +); + +function userMessage(text: string): Message { + return { role: 'user', content: [{ type: 'text', text }], toolCalls: [] }; +} + +interface Harness { + readonly input: RunTurnInput; + readonly llm: FakeLLM; + readonly strictCalls: { count: number }; + readonly strictMessages: Message[]; +} + +function makeHarness(error: unknown): Harness { + const llm = new FakeLLM({ + responses: [makeEndTurnResponse('unused'), makeEndTurnResponse('recovered')], + throwOnIndex: { index: 0, error }, + }); + const context = new RecordingContext({ messages: [userMessage('normal projection')] }); + const sink = new CollectingSink({}); + const strictMessages: Message[] = [userMessage('strict projection')]; + const strictCalls = { count: 0 }; + const buildMessagesStrict: LoopMessageBuilder = () => { + strictCalls.count += 1; + return strictMessages; + }; + const input: RunTurnInput = { + turnId: 'turn-1', + signal: new AbortController().signal, + llm, + buildMessages: context.buildMessages, + buildMessagesStrict, + dispatchEvent: createLoopEventDispatcher({ + appendTranscriptRecord: context.appendTranscriptRecord, + emitLiveEvent: sink.emit, + }), + }; + return { input, llm, strictCalls, strictMessages }; +} + +describe('executeLoopStep — tool exchange adjacency fallback', () => { + it('resends once with strict messages after an adjacency 400 and recovers', async () => { + const { input, llm, strictCalls, strictMessages } = makeHarness(ADJACENCY_400); + + const result = await runTurn(input); + + expect(result.stopReason).toBe('end_turn'); + // Exactly two provider calls: the rejected one and the strict resend. + expect(llm.callCount).toBe(2); + expect(strictCalls.count).toBe(1); + // The first attempt used the normal projection; the resend used the strict one. + expect(llm.calls[0]?.messages).toEqual([userMessage('normal projection')]); + expect(llm.calls[1]?.messages).toBe(strictMessages); + }); + + it('does not resend for an unrelated 400 — the error propagates and strict is untouched', async () => { + const { input, llm, strictCalls } = makeHarness(new APIStatusError(400, 'Bad request')); + + await expect(runTurn(input)).rejects.toThrow('Bad request'); + + expect(llm.callCount).toBe(1); + expect(strictCalls.count).toBe(0); + }); + + it('resends only once: if the strict rebuild is also rejected, it gives up (no loop)', async () => { + // Throw a recoverable structural 400 on every attempt; the loop must stop + // after exactly two provider calls (first attempt + one strict resend). + const llm = new FakeLLM({ responses: [] }); + let calls = 0; + llm.chat = async () => { + calls += 1; + throw ADJACENCY_400; + }; + const context = new RecordingContext({ messages: [userMessage('normal')] }); + const sink = new CollectingSink({}); + let strictCount = 0; + const input: RunTurnInput = { + turnId: 'turn-1', + signal: new AbortController().signal, + llm, + buildMessages: context.buildMessages, + buildMessagesStrict: () => { + strictCount += 1; + return [userMessage('strict')]; + }, + dispatchEvent: createLoopEventDispatcher({ + appendTranscriptRecord: context.appendTranscriptRecord, + emitLiveEvent: sink.emit, + }), + }; + + await expect(runTurn(input)).rejects.toBe(ADJACENCY_400); + expect(calls).toBe(2); // first attempt + one strict resend, then give up + expect(strictCount).toBe(1); + }); +}); diff --git a/packages/kosong/src/errors.ts b/packages/kosong/src/errors.ts index 989f02249..a2a8cdc4a 100644 --- a/packages/kosong/src/errors.ts +++ b/packages/kosong/src/errors.ts @@ -143,6 +143,51 @@ export function isContextOverflowStatusError(statusCode: number, message: string return CONTEXT_OVERFLOW_MESSAGE_PATTERNS.some((pattern) => pattern.test(lowerMessage)); } +// Strict providers (Anthropic) reject a request whose assistant `tool_use` and +// `tool_result` blocks are not correctly paired and adjacent — a missing result, +// a stray result with no matching call, or a result that does not immediately +// follow its call. The validation runs before any generation, so the error is a +// non-retryable 4xx. A caller can react by resending a re-projected, strictly +// wire-compliant request rather than leaving the session permanently stuck. +const TOOL_EXCHANGE_ADJACENCY_MESSAGE_PATTERNS = [ + /tool_use[\s\S]*tool_result/, + /tool_result[\s\S]*tool_use/, + /unexpected\s+`?tool_result/, +] as const; + +export function isToolExchangeAdjacencyError(error: unknown): boolean { + if (!(error instanceof APIStatusError)) return false; + if (error instanceof APIContextOverflowError) return false; + if (error.statusCode !== 400 && error.statusCode !== 422) return false; + const lowerMessage = error.message.toLowerCase(); + return TOOL_EXCHANGE_ADJACENCY_MESSAGE_PATTERNS.some((pattern) => pattern.test(lowerMessage)); +} + +// The broader family of structural request rejections a strict provider returns +// when the message array itself is malformed — tool_use/tool_result pairing, +// empty or whitespace-only text blocks, a non-user first message, or +// non-alternating roles. All are deterministic 4xx validation failures (no +// generation happened) on a history that is re-sent every turn, so the only +// recovery is to resend a re-projected, strictly wire-compliant request rather +// than leave the session permanently stuck. Context-overflow 400s are excluded — +// they are handled by compaction, not by re-projection. +const STRUCTURAL_REQUEST_MESSAGE_PATTERNS = [ + /text content blocks must be non-empty/, + /text content blocks must contain non-whitespace/, + /first message must use the .*user.* role/, + /roles must alternate/, + /multiple .*(?:user|assistant).* roles in a row/, +] as const; + +export function isRecoverableRequestStructureError(error: unknown): boolean { + if (isToolExchangeAdjacencyError(error)) return true; + if (!(error instanceof APIStatusError)) return false; + if (error instanceof APIContextOverflowError) return false; + if (error.statusCode !== 400 && error.statusCode !== 422) return false; + const lowerMessage = error.message.toLowerCase(); + return STRUCTURAL_REQUEST_MESSAGE_PATTERNS.some((pattern) => pattern.test(lowerMessage)); +} + export function isProviderRateLimitError(error: unknown): boolean { if (error instanceof APIProviderRateLimitError) return true; diff --git a/packages/kosong/src/index.ts b/packages/kosong/src/index.ts index b8bd9bdcb..01662bcbc 100644 --- a/packages/kosong/src/index.ts +++ b/packages/kosong/src/index.ts @@ -68,7 +68,9 @@ export { ChatProviderError, isContextOverflowStatusError, isProviderRateLimitError, + isRecoverableRequestStructureError, isRetryableGenerateError, + isToolExchangeAdjacencyError, } from './errors'; /** diff --git a/packages/kosong/test/errors.test.ts b/packages/kosong/test/errors.test.ts index 0317f405a..ad1f82431 100644 --- a/packages/kosong/test/errors.test.ts +++ b/packages/kosong/test/errors.test.ts @@ -7,7 +7,9 @@ import { APITimeoutError, ChatProviderError, isProviderRateLimitError, + isRecoverableRequestStructureError, isRetryableGenerateError, + isToolExchangeAdjacencyError, normalizeAPIStatusError, } from '#/errors'; import { describe, expect, it } from 'vitest'; @@ -209,6 +211,102 @@ describe('normalizeAPIStatusError', () => { }); }); +describe('isToolExchangeAdjacencyError', () => { + // The exact Anthropic message observed in the field when a tool_use was not + // immediately followed by its tool_result. + const ANTHROPIC_MISSING_RESULT = + 'messages.142: `tool_use` ids were found without `tool_result` blocks immediately after: ' + + 'toolu_01MWFhDRqdbB4nzCJNuWYiun. Each `tool_use` block must have a corresponding ' + + '`tool_result` block in the next message.'; + + it('matches the missing-tool_result 400', () => { + expect(isToolExchangeAdjacencyError(new APIStatusError(400, ANTHROPIC_MISSING_RESULT))).toBe( + true, + ); + }); + + it('matches the reverse unexpected-tool_result 400', () => { + expect( + isToolExchangeAdjacencyError( + new APIStatusError( + 400, + 'messages.5: `tool_result` block(s) provided when previous message does not ' + + 'contain any `tool_use` blocks', + ), + ), + ).toBe(true); + expect( + isToolExchangeAdjacencyError(new APIStatusError(400, 'unexpected `tool_result` block')), + ).toBe(true); + }); + + it('also matches a 422 with the same shape', () => { + expect(isToolExchangeAdjacencyError(new APIStatusError(422, ANTHROPIC_MISSING_RESULT))).toBe( + true, + ); + }); + + it('does not match a context-overflow 400 or unrelated errors', () => { + expect( + isToolExchangeAdjacencyError(new APIContextOverflowError(400, 'context length exceeded')), + ).toBe(false); + expect(isToolExchangeAdjacencyError(new APIStatusError(400, 'Bad request'))).toBe(false); + expect(isToolExchangeAdjacencyError(new APIStatusError(500, ANTHROPIC_MISSING_RESULT))).toBe( + false, + ); + expect(isToolExchangeAdjacencyError(new Error(ANTHROPIC_MISSING_RESULT))).toBe(false); + expect(isToolExchangeAdjacencyError('boom')).toBe(false); + }); +}); + +describe('isRecoverableRequestStructureError', () => { + it('matches the whole tool_use/tool_result adjacency family', () => { + expect( + isRecoverableRequestStructureError( + new APIStatusError(400, '`tool_use` ids were found without `tool_result` blocks'), + ), + ).toBe(true); + }); + + it('matches empty / whitespace-only text content rejections', () => { + expect( + isRecoverableRequestStructureError( + new APIStatusError(400, 'messages: text content blocks must be non-empty'), + ), + ).toBe(true); + expect( + isRecoverableRequestStructureError( + new APIStatusError(400, 'text content blocks must contain non-whitespace text'), + ), + ).toBe(true); + }); + + it('matches first-message-must-be-user and role-alternation rejections', () => { + expect( + isRecoverableRequestStructureError( + new APIStatusError(400, 'messages: first message must use the "user" role'), + ), + ).toBe(true); + expect( + isRecoverableRequestStructureError( + new APIStatusError( + 400, + 'messages: roles must alternate between "user" and "assistant", but found multiple "user" roles in a row', + ), + ), + ).toBe(true); + }); + + it('does not match context overflow, auth, or non-status errors', () => { + expect( + isRecoverableRequestStructureError(new APIContextOverflowError(400, 'context length exceeded')), + ).toBe(false); + expect(isRecoverableRequestStructureError(new APIStatusError(401, 'unauthorized'))).toBe(false); + expect(isRecoverableRequestStructureError(new APIStatusError(400, 'Bad request'))).toBe(false); + expect(isRecoverableRequestStructureError(new Error('roles must alternate'))).toBe(false); + }); +}); + describe('isProviderRateLimitError', () => { it('matches explicit HTTP 429 status errors', () => { expect(isProviderRateLimitError(new APIProviderRateLimitError('rate limited'))).toBe(true);