diff --git a/.changeset/fix-vacuous-assistant-wedge.md b/.changeset/fix-vacuous-assistant-wedge.md new file mode 100644 index 0000000000..457c30cdf6 --- /dev/null +++ b/.changeset/fix-vacuous-assistant-wedge.md @@ -0,0 +1,8 @@ +--- +"@moonshot-ai/agent-core": patch +"@moonshot-ai/agent-core-v2": patch +"@moonshot-ai/kosong": patch +"@moonshot-ai/kimi-code": patch +--- + +Fix sessions getting stuck on every turn with a provider "message must not be empty" error after a content-filtered response. diff --git a/packages/agent-core-v2/src/agent/contextMemory/contextTranscript.ts b/packages/agent-core-v2/src/agent/contextMemory/contextTranscript.ts index 481dd5865f..ebb902b27e 100644 --- a/packages/agent-core-v2/src/agent/contextMemory/contextTranscript.ts +++ b/packages/agent-core-v2/src/agent/contextMemory/contextTranscript.ts @@ -18,7 +18,9 @@ * - `context.append_message` → append (deferred while a tool exchange is open) * - `context.append_loop_event` → step.begin/content.part/tool.call mutate the * open assistant; tool.result appends a tool - * message with the raw output + * message with the raw output; settling a step + * (at its end or at the next begin) drops an + * output-free assistant, mirroring the live fold * - `context.apply_compaction` → keep the full history, append the user-role * summary marker, recover `foldedLength` from * the recorded kept-count fields @@ -39,6 +41,7 @@ import { } from './compactionHandoff'; import type { LoopRecordedEvent } from './loopEventFold'; import type { ContextMessage } from './types'; +import { isVacuousContentPart } from './vacuousContent'; const TOOL_INTERRUPTED_ON_RESUME_OUTPUT = 'Tool execution was interrupted before its result was recorded. Do not assume the tool completed successfully.'; @@ -82,6 +85,7 @@ export function createContextTranscriptReducer(): ContextTranscriptReducer { const openSteps = new Map(); const pendingToolResultIds = new Set(); let deferred: MutableEntry[] = []; + let lastOpenStepUuid: string | undefined; const push = (...entries: MutableEntry[]): void => { transcript.push(...entries); @@ -114,22 +118,37 @@ export function createContextTranscriptReducer(): ContextTranscriptReducer { openSteps.clear(); pendingToolResultIds.clear(); deferred = []; + lastOpenStepUuid = undefined; + }; + const settleStep = (uuid: string): void => { + const entry = openSteps.get(uuid); + if (entry === undefined) return; + openSteps.delete(uuid); + if (entry.message.toolCalls.length > 0) return; + if (!entry.message.content.every(isVacuousContentPart)) return; + const index = transcript.indexOf(entry); + if (index === -1) return; + transcript.splice(index, 1); + foldedLength = Math.max(0, foldedLength - 1); }; const applyLoopEvent = (event: LoopRecordedEvent, time: number | undefined): void => { switch (event.type) { case 'step.begin': { closePendingToolResults(time); + if (lastOpenStepUuid !== undefined) settleStep(lastOpenStepUuid); const entry: MutableEntry = { message: { role: 'assistant', content: [], toolCalls: [] }, time, }; push(entry); openSteps.set(event.uuid, entry); + lastOpenStepUuid = event.uuid; return; } case 'step.end': { - openSteps.delete(event.uuid); + settleStep(event.uuid); + if (lastOpenStepUuid === event.uuid) lastOpenStepUuid = undefined; flushDeferredIfToolExchangeClosed(); return; } diff --git a/packages/agent-core-v2/src/agent/contextMemory/loopEventFold.ts b/packages/agent-core-v2/src/agent/contextMemory/loopEventFold.ts index 546efab460..cf32c5f588 100644 --- a/packages/agent-core-v2/src/agent/contextMemory/loopEventFold.ts +++ b/packages/agent-core-v2/src/agent/contextMemory/loopEventFold.ts @@ -25,9 +25,10 @@ * wrapping), clear its pending id * - `step.end` → settle the assistant * "Settle" closes any tool exchange left open (interrupted result messages), - * then drops the partial assistant when it is empty (no content, no tool - * calls — an empty assistant only trips provider message validation) and - * seals it (`partial: undefined`) when it carries output. v1 never produced + * then drops the partial assistant when nothing sendable was recorded (no + * tool calls; every content part vacuous — an output-free assistant only + * trips provider message validation) and seals it (`partial: undefined`) + * when it carries output. v1 never produced * `step.begin` without `step.end` (its retries stayed inside one request), so * the drop/seal rule is the v2 extension that makes loop-level retries — a * retried attempt is its own `step.begin` — replay to the same history the @@ -47,6 +48,7 @@ import { createToolMessage, type ContentPart, type ToolCall } from '#/app/llmPro import type { TokenUsage } from '#/app/llmProtocol/usage'; import type { ContextMessage } from './types'; +import { isVacuousContentPart } from './vacuousContent'; const TOOL_INTERRUPTED_ON_RESUME_OUTPUT = 'Tool execution was interrupted before its result was recorded. Do not assume the tool completed successfully.'; @@ -215,7 +217,7 @@ function settleOpenStep( const index = findOpenAssistantIndex(closed); if (index === -1) return closed; const open = closed[index]!; - if (open.content.length === 0 && open.toolCalls.length === 0) { + if (open.toolCalls.length === 0 && open.content.every(isVacuousContentPart)) { return [...closed.slice(0, index), ...closed.slice(index + 1)]; } const next = closed.slice(); diff --git a/packages/agent-core-v2/src/agent/contextMemory/vacuousContent.ts b/packages/agent-core-v2/src/agent/contextMemory/vacuousContent.ts new file mode 100644 index 0000000000..7f108a5e5b --- /dev/null +++ b/packages/agent-core-v2/src/agent/contextMemory/vacuousContent.ts @@ -0,0 +1,18 @@ +/** + * `contextMemory` vacuous-content predicate — shared test for content parts + * that carry nothing the provider wire can represent, used by the loop-event + * fold (settle-time drop of output-free steps) and the context projector + * (wire-time drop of wholly-vacuous messages). Vacuous means an empty or + * whitespace-only text block, or an empty thinking block with no provider + * signature; a signed thinking block (`encrypted`) is never vacuous — + * reasoning providers require it back verbatim — and media parts always + * carry content. + */ + +import type { ContentPart } from '#/app/llmProtocol/message'; + +export function isVacuousContentPart(part: ContentPart): boolean { + if (part.type === 'text') return part.text.trim().length === 0; + if (part.type === 'think') return part.encrypted === undefined && part.think.trim().length === 0; + return false; +} diff --git a/packages/agent-core-v2/src/agent/contextProjector/contextProjectorService.ts b/packages/agent-core-v2/src/agent/contextProjector/contextProjectorService.ts index af425efa2f..d8f1521056 100644 --- a/packages/agent-core-v2/src/agent/contextProjector/contextProjectorService.ts +++ b/packages/agent-core-v2/src/agent/contextProjector/contextProjectorService.ts @@ -7,7 +7,9 @@ * outgoing wire valid (a displaced result moved back to its call, a synthetic * result invented for a lost one, an orphan/duplicate dropped, leading * non-user messages dropped, consecutive assistants merged, blank text - * dropped) are reported through an optional sink and surfaced once here as a + * dropped, wholly-vacuous messages — nothing sendable was recorded, e.g. an + * assistant step that kept only an empty thinking part — dropped whole) are + * reported through an optional sink and surfaced once here as a * single deduped warning plus a `context_projection_repaired` telemetry event, * so a silently-mangled history always leaves a trace. * @@ -27,6 +29,7 @@ import { LifecycleScope, registerScopedService } from '#/_base/di/scope'; import { ILogService } from '#/_base/log/log'; import { renderToolResultForModel } from '#/agent/contextMemory/toolResultRender'; import type { ContextMessage } from '#/agent/contextMemory/types'; +import { isVacuousContentPart } from '#/agent/contextMemory/vacuousContent'; import { ErrorCodes, Error2 } from '#/errors'; import type { ContentPart, Message } from '#/app/llmProtocol/message'; import { ITelemetryService } from '#/app/telemetry/telemetry'; @@ -108,6 +111,7 @@ export class AgentContextProjectorService implements IAgentContextProjectorServi let leadingDropped = 0; let assistantsMerged = 0; let whitespaceDropped = 0; + let vacuousDropped = 0; for (const anomaly of notable) { if (anomaly.kind === 'tool_result_reordered') reordered += 1; else if (anomaly.kind === 'tool_result_synthesized') synthesized += 1; @@ -116,6 +120,7 @@ export class AgentContextProjectorService implements IAgentContextProjectorServi else if (anomaly.kind === 'duplicate_tool_result_dropped') duplicateResultsDropped += 1; else if (anomaly.kind === 'leading_non_user_dropped') leadingDropped += 1; else if (anomaly.kind === 'consecutive_assistants_merged') assistantsMerged += 1; + else if (anomaly.kind === 'vacuous_message_dropped') vacuousDropped += 1; else whitespaceDropped += 1; } const toolCallIds = [ @@ -132,6 +137,7 @@ export class AgentContextProjectorService implements IAgentContextProjectorServi leadingDropped, assistantsMerged, whitespaceDropped, + vacuousDropped, toolCallIds, }); this.telemetry.track2('context_projection_repaired', { @@ -143,6 +149,7 @@ export class AgentContextProjectorService implements IAgentContextProjectorServi leading_dropped: leadingDropped, assistants_merged: assistantsMerged, whitespace_dropped: whitespaceDropped, + vacuous_dropped: vacuousDropped, }); } } @@ -155,7 +162,8 @@ type ProjectionAnomaly = | { readonly kind: 'duplicate_tool_result_dropped'; readonly toolCallId: string } | { readonly kind: 'leading_non_user_dropped'; readonly role: string } | { readonly kind: 'consecutive_assistants_merged' } - | { readonly kind: 'whitespace_text_dropped'; readonly role: string }; + | { readonly kind: 'whitespace_text_dropped'; readonly role: string } + | { readonly kind: 'vacuous_message_dropped'; readonly role: string }; type OnAnomaly = (anomaly: ProjectionAnomaly) => void; @@ -314,8 +322,10 @@ function dedupeDuplicateToolCalls(messages: readonly Message[], onAnomaly?: OnAn }); if (kept.length === message.toolCalls.length) { out.push(message); - } else if (kept.length > 0 || message.content.length > 0) { + } else if (kept.length > 0 || !message.content.every(isVacuousContentPart)) { out.push({ ...message, toolCalls: kept }); + } else if (message.content.length > 0) { + onAnomaly?.({ kind: 'vacuous_message_dropped', role: message.role }); } continue; } @@ -407,7 +417,13 @@ function project(history: readonly ContextMessage[], onAnomaly?: OnAnomaly): Mes const emit = (source: ContextMessage): void => { const content = projectedContent(source, onAnomaly); - if (content.length === 0 && source.toolCalls.length === 0 && !hasDeclaredTools(source)) return; + if (source.toolCalls.length === 0 && !hasDeclaredTools(source)) { + if (content.length === 0) return; + if (content.every(isVacuousContentPart)) { + onAnomaly?.({ kind: 'vacuous_message_dropped', role: source.role }); + return; + } + } if (openSlots.size > 0) markForeignBetween(); diff --git a/packages/agent-core-v2/src/app/llmProtocol/errors.ts b/packages/agent-core-v2/src/app/llmProtocol/errors.ts index 7a4d5d70cd..b9da080ac7 100644 --- a/packages/agent-core-v2/src/app/llmProtocol/errors.ts +++ b/packages/agent-core-v2/src/app/llmProtocol/errors.ts @@ -333,6 +333,7 @@ const STRUCTURAL_REQUEST_MESSAGE_PATTERNS = [ /roles must alternate/, /multiple .*(?:user|assistant).* roles in a row/, /tool_use[\s\S]*ids must be unique/, + /message at position \d+ with role ['"`]?[a-z]+['"`]? must not be empty/, ] as const; export function isRecoverableRequestStructureError(error: unknown): boolean { diff --git a/packages/agent-core-v2/src/app/telemetry/events.ts b/packages/agent-core-v2/src/app/telemetry/events.ts index 0323b4f451..99dc47fa23 100644 --- a/packages/agent-core-v2/src/app/telemetry/events.ts +++ b/packages/agent-core-v2/src/app/telemetry/events.ts @@ -209,6 +209,7 @@ export interface ContextProjectionRepairedEvent { leading_dropped: number; assistants_merged: number; whitespace_dropped: number; + vacuous_dropped: number; } export interface BackgroundTaskCreatedEvent { @@ -615,6 +616,7 @@ export const telemetryEventDefinitions = { leading_dropped: 'Leading non-user messages dropped', assistants_merged: 'Consecutive assistant messages merged', whitespace_dropped: 'Whitespace-only text blocks dropped', + vacuous_dropped: 'Messages dropped because every recorded part serialized to nothing', }, }), background_task_created: defineTelemetryEvent({ diff --git a/packages/agent-core-v2/test/agent/contextMemory/contextTranscript.test.ts b/packages/agent-core-v2/test/agent/contextMemory/contextTranscript.test.ts index d6e2170d7a..710c136b28 100644 --- a/packages/agent-core-v2/test/agent/contextMemory/contextTranscript.test.ts +++ b/packages/agent-core-v2/test/agent/contextMemory/contextTranscript.test.ts @@ -227,4 +227,50 @@ describe('reduceContextTranscript', () => { expect(result.entries[2]!.toolCallId).toBe('call_1'); expect(result.foldedLength).toBe(3); }); + + it('drops an output-free assistant at step.end, mirroring the live fold', () => { + const result = reduceContextTranscript([ + appendMessage(userMessage('q')), + loopEvent({ type: 'step.begin', uuid: 's1' }), + loopEvent({ type: 'content.part', stepUuid: 's1', part: { type: 'think', think: '' } }), + loopEvent({ type: 'step.end', uuid: 's1' }), + ]); + expect(result.entries.map((m) => m.role)).toEqual(['user']); + expect(result.foldedLength).toBe(1); + }); + + it('drops a failed attempt left open when the retry begins', () => { + const result = reduceContextTranscript([ + appendMessage(userMessage('q')), + loopEvent({ type: 'step.begin', uuid: 's1' }), + loopEvent({ type: 'step.begin', uuid: 's2' }), + loopEvent({ type: 'content.part', stepUuid: 's2', part: { type: 'text', text: 'recovered' } }), + loopEvent({ type: 'step.end', uuid: 's2' }), + ]); + expect(result.entries.map((m) => m.role)).toEqual(['user', 'assistant']); + expect(texts(result)).toEqual(['q', 'recovered']); + expect(result.foldedLength).toBe(2); + }); + + it('keeps settled steps that carry any sendable output', () => { + const result = reduceContextTranscript([ + appendMessage(userMessage('q')), + loopEvent({ type: 'step.begin', uuid: 's1' }), + loopEvent({ type: 'content.part', stepUuid: 's1', part: { type: 'think', think: 'real' } }), + loopEvent({ type: 'step.end', uuid: 's1' }), + loopEvent({ type: 'step.begin', uuid: 's2' }), + loopEvent({ + type: 'content.part', + stepUuid: 's2', + part: { type: 'think', think: '', encrypted: 'sig' }, + }), + loopEvent({ type: 'step.end', uuid: 's2' }), + loopEvent({ type: 'step.begin', uuid: 's3' }), + loopEvent({ type: 'content.part', stepUuid: 's3', part: { type: 'think', think: '' } }), + loopEvent({ type: 'content.part', stepUuid: 's3', part: { type: 'text', text: 'answer' } }), + loopEvent({ type: 'step.end', uuid: 's3' }), + ]); + expect(result.entries.map((m) => m.role)).toEqual(['user', 'assistant', 'assistant', 'assistant']); + expect(result.foldedLength).toBe(4); + }); }); diff --git a/packages/agent-core-v2/test/agent/contextMemory/loopEventFold.test.ts b/packages/agent-core-v2/test/agent/contextMemory/loopEventFold.test.ts index 50267db277..25f9f73b39 100644 --- a/packages/agent-core-v2/test/agent/contextMemory/loopEventFold.test.ts +++ b/packages/agent-core-v2/test/agent/contextMemory/loopEventFold.test.ts @@ -192,6 +192,125 @@ describe('loop-event fold parity', () => { expect(context.get()).toEqual([]); }); + it('drops an assistant whose only recorded part is an empty thinking block at step.end', () => { + context.appendLoopEvent({ type: 'step.begin', uuid: 's1' }); + context.appendLoopEvent({ + type: 'content.part', + stepUuid: 's1', + part: { type: 'think', think: '' }, + }); + context.appendLoopEvent({ type: 'step.end', uuid: 's1' }); + + expect(context.get()).toEqual([]); + }); + + it('drops a vacuous partial assistant left by a failed attempt when the retry begins', () => { + context.appendLoopEvent({ type: 'step.begin', uuid: 's1' }); + context.appendLoopEvent({ + type: 'content.part', + stepUuid: 's1', + part: { type: 'think', think: ' ' }, + }); + context.appendLoopEvent({ type: 'step.begin', uuid: 's2' }); + context.appendLoopEvent({ + type: 'content.part', + stepUuid: 's2', + part: { type: 'text', text: 'recovered' }, + }); + context.appendLoopEvent({ type: 'step.end', uuid: 's2' }); + + expect(shapes(context.get())).toEqual([ + { + role: 'assistant', + content: [{ type: 'text', text: 'recovered' }], + toolCalls: [], + toolCallId: undefined, + isError: undefined, + partial: undefined, + }, + ]); + }); + + it('seals a step whose thinking block has real content', () => { + context.appendLoopEvent({ type: 'step.begin', uuid: 's1' }); + context.appendLoopEvent({ + type: 'content.part', + stepUuid: 's1', + part: { type: 'think', think: 'real reasoning' }, + }); + context.appendLoopEvent({ type: 'step.end', uuid: 's1' }); + + expect(context.get().at(-1)?.content).toEqual([{ type: 'think', think: 'real reasoning' }]); + }); + + it('seals a step whose empty thinking block carries a provider signature', () => { + context.appendLoopEvent({ type: 'step.begin', uuid: 's1' }); + context.appendLoopEvent({ + type: 'content.part', + stepUuid: 's1', + part: { type: 'think', think: '', encrypted: 'sig' }, + }); + context.appendLoopEvent({ type: 'step.end', uuid: 's1' }); + + expect(context.get().at(-1)?.content).toEqual([{ type: 'think', think: '', encrypted: 'sig' }]); + }); + + it('seals a step that pairs an empty thinking block with real text', () => { + context.appendLoopEvent({ type: 'step.begin', uuid: 's1' }); + context.appendLoopEvent({ + type: 'content.part', + stepUuid: 's1', + part: { type: 'think', think: '' }, + }); + context.appendLoopEvent({ + type: 'content.part', + stepUuid: 's1', + part: { type: 'text', text: 'answer' }, + }); + context.appendLoopEvent({ type: 'step.end', uuid: 's1' }); + + expect(context.get().at(-1)?.content).toEqual([ + { type: 'think', think: '' }, + { type: 'text', text: 'answer' }, + ]); + }); + + it('seals an assistant with tool calls even when its thinking block is empty', () => { + context.appendLoopEvent({ type: 'step.begin', uuid: 's1' }); + context.appendLoopEvent({ + type: 'content.part', + stepUuid: 's1', + part: { type: 'think', think: '' }, + }); + context.appendLoopEvent({ + type: 'tool.call', + stepUuid: 's1', + toolCallId: 'c1', + name: 'Lookup', + args: {}, + }); + context.appendLoopEvent({ type: 'step.end', uuid: 's1' }); + + expect(shapes(context.get())).toEqual([ + { + role: 'assistant', + content: [{ type: 'think', think: '' }], + toolCalls: [{ type: 'function', id: 'c1', name: 'Lookup', arguments: '{}' }], + toolCallId: undefined, + isError: undefined, + partial: undefined, + }, + { + role: 'tool', + content: expect.any(Array), + toolCalls: [], + toolCallId: 'c1', + isError: true, + partial: undefined, + }, + ]); + }); + it('folds a tool-result note as structured model-only metadata', () => { context.append( { diff --git a/packages/agent-core-v2/test/agent/contextProjector/projector-tool-exchanges.test.ts b/packages/agent-core-v2/test/agent/contextProjector/projector-tool-exchanges.test.ts index 49363e0cea..be0ce624fc 100644 --- a/packages/agent-core-v2/test/agent/contextProjector/projector-tool-exchanges.test.ts +++ b/packages/agent-core-v2/test/agent/contextProjector/projector-tool-exchanges.test.ts @@ -412,6 +412,66 @@ describe('projector tool-exchange normalization', () => { expect((projected[2]?.content[0] as { text: string }).text).toBe('late result'); }); + it('strict mode drops an assistant left with only vacuous content after deduping', () => { + const history = [ + user('go'), + assistant('first', ['dup']), + toolResult('dup', 'one'), + { + role: 'assistant' as const, + content: [{ type: 'think' as const, think: '' }], + toolCalls: [{ type: 'function' as const, id: 'dup', name: 'Lookup', arguments: '{}' }], + }, + toolResult('dup', 'two'), + user('next'), + ]; + + const projected = projectStrict(history); + + expect( + projected.map((message) => + message.role === 'tool' ? `tool:${message.toolCallId}` : message.role, + ), + ).toEqual(['user', 'assistant', 'tool:dup', 'user']); + expect(repairPayloads(warnings)).toEqual([ + expect.objectContaining({ duplicateCallsDropped: 1, vacuousDropped: 1 }), + ]); + }); + + it('strict mode keeps a deduped assistant whose remaining content is sendable', () => { + const history = [ + user('go'), + assistant('first', ['dup']), + toolResult('dup', 'one'), + { + role: 'assistant' as const, + content: [ + { type: 'think' as const, think: '' }, + { type: 'text' as const, text: 'second' }, + ], + toolCalls: [{ type: 'function' as const, id: 'dup', name: 'Lookup', arguments: '{}' }], + }, + toolResult('dup', 'two'), + user('next'), + ]; + + const projected = projectStrict(history); + + expect( + projected.map((message) => + message.role === 'tool' ? `tool:${message.toolCallId}` : message.role, + ), + ).toEqual(['user', 'assistant', 'tool:dup', 'assistant', 'user']); + expect(projected[3]?.toolCalls).toEqual([]); + expect(projected[3]?.content).toEqual([ + { type: 'think', think: '' }, + { type: 'text', text: 'second' }, + ]); + expect(repairPayloads(warnings)).toEqual([ + expect.objectContaining({ duplicateCallsDropped: 1, vacuousDropped: 0 }), + ]); + }); + it('strict mode drops leading non-user messages', () => { const projected = projectStrict([assistant('stale'), toolResult('ghost', 'orphaned'), user('hi')]); @@ -512,6 +572,7 @@ describe('projector tool-exchange normalization', () => { leading_dropped: 0, assistants_merged: 0, whitespace_dropped: 0, + vacuous_dropped: 0, }, }, ]); @@ -524,6 +585,92 @@ describe('projector tool-exchange normalization', () => { }); }); + describe('vacuous (thinking-only) messages', () => { + function thinkingAssistant(content: ContextMessage['content']): ContextMessage { + return { role: 'assistant', content: [...content], toolCalls: [] }; + } + + it('drops an assistant message whose only part is an empty think block', () => { + const history = [ + user('u1'), + thinkingAssistant([{ type: 'think', think: '' }]), + reminder('ping'), + ]; + expect(shape(history)).toEqual(['user', 'user']); + expect(repairPayloads(warnings)).toEqual([expect.objectContaining({ vacuousDropped: 1 })]); + expect(telemetryRecords).toEqual([ + { + event: 'context_projection_repaired', + properties: expect.objectContaining({ vacuous_dropped: 1 }), + }, + ]); + }); + + it('un-wedges a history poisoned by a filtered step (session regression)', () => { + const history = [ + user('u1'), + assistant('', ['c1']), + toolResult('c1', 'one'), + thinkingAssistant([{ type: 'think', think: '' }]), + reminder('ping'), + ]; + expect(shape(history)).toEqual(['user', 'assistant', 'tool:c1', 'user']); + expect(repairPayloads(warnings)).toEqual([expect.objectContaining({ vacuousDropped: 1 })]); + }); + + it('keeps a message with real text intact — including its empty think part', () => { + const history = [ + user('u1'), + thinkingAssistant([{ type: 'think', think: '' }, { type: 'text', text: 'answer' }]), + ]; + expect(project(history)[1]?.content).toEqual([ + { type: 'think', think: '' }, + { type: 'text', text: 'answer' }, + ]); + expect(repairPayloads(warnings)).toEqual([]); + }); + + it('keeps a message whose think block has real content', () => { + const history = [user('u1'), thinkingAssistant([{ type: 'think', think: 'real reasoning' }])]; + expect(shape(history)).toEqual(['user', 'assistant']); + expect(repairPayloads(warnings)).toEqual([]); + }); + + it('keeps a signed think block even when its text is empty', () => { + const history = [ + user('u1'), + thinkingAssistant([{ type: 'think', think: '', encrypted: 'sig' }]), + ]; + expect(shape(history)).toEqual(['user', 'assistant']); + expect(project(history)[1]?.content).toEqual([{ type: 'think', think: '', encrypted: 'sig' }]); + }); + + it('drops a message whose think block is whitespace-only', () => { + const history = [ + user('u1'), + thinkingAssistant([{ type: 'think', think: ' ' }]), + reminder('ping'), + ]; + expect(shape(history)).toEqual(['user', 'user']); + expect(repairPayloads(warnings)).toEqual([expect.objectContaining({ vacuousDropped: 1 })]); + }); + + it('keeps an assistant message with tool calls even when its think part is empty', () => { + const history = [ + user('u1'), + { + role: 'assistant' as const, + content: [{ type: 'think' as const, think: '' }], + toolCalls: [{ type: 'function' as const, id: 'c1', name: 'Lookup', arguments: '{}' }], + }, + toolResult('c1', 'one'), + ]; + expect(shape(history)).toEqual(['user', 'assistant', 'tool:c1']); + expect(project(history)[1]?.content).toEqual([{ type: 'think', think: '' }]); + expect(repairPayloads(warnings)).toEqual([]); + }); + }); + describe('projectMediaDegraded', () => { function imageMessage(url: string): ContextMessage { return { diff --git a/packages/agent-core-v2/test/app/llmProtocol/errors.test.ts b/packages/agent-core-v2/test/app/llmProtocol/errors.test.ts index 5de9f5b1aa..b37a1b3edd 100644 --- a/packages/agent-core-v2/test/app/llmProtocol/errors.test.ts +++ b/packages/agent-core-v2/test/app/llmProtocol/errors.test.ts @@ -656,6 +656,17 @@ describe('isRecoverableRequestStructureError', () => { ).toBe(true); }); + it('matches the Moonshot/Kimi vacuous-message rejection', () => { + expect( + isRecoverableRequestStructureError( + new APIStatusError( + 400, + "400 the message at position 105 with role 'assistant' must not be empty", + ), + ), + ).toBe(true); + }); + it('does not match context overflow, auth, or non-status errors', () => { expect( isRecoverableRequestStructureError(new APIContextOverflowError(400, 'context length exceeded')), diff --git a/packages/agent-core/src/agent/context/index.ts b/packages/agent-core/src/agent/context/index.ts index 3934ac3b05..f02ad31292 100644 --- a/packages/agent-core/src/agent/context/index.ts +++ b/packages/agent-core/src/agent/context/index.ts @@ -498,6 +498,7 @@ export class ContextMemory { let leadingDropped = 0; let assistantsMerged = 0; let whitespaceDropped = 0; + let vacuousDropped = 0; for (const anomaly of notable) { if (anomaly.kind === 'tool_result_reordered') reordered += 1; else if (anomaly.kind === 'tool_result_synthesized') synthesized += 1; @@ -506,6 +507,7 @@ export class ContextMemory { else if (anomaly.kind === 'duplicate_tool_result_dropped') duplicateResultsDropped += 1; else if (anomaly.kind === 'leading_non_user_dropped') leadingDropped += 1; else if (anomaly.kind === 'consecutive_assistants_merged') assistantsMerged += 1; + else if (anomaly.kind === 'vacuous_message_dropped') vacuousDropped += 1; else whitespaceDropped += 1; } const toolCallIds = [ @@ -522,6 +524,7 @@ export class ContextMemory { leadingDropped, assistantsMerged, whitespaceDropped, + vacuousDropped, toolCallIds, }); this.agent.telemetry.track('context_projection_repaired', { @@ -533,6 +536,7 @@ export class ContextMemory { leading_dropped: leadingDropped, assistants_merged: assistantsMerged, whitespace_dropped: whitespaceDropped, + vacuous_dropped: vacuousDropped, }); } diff --git a/packages/agent-core/src/agent/context/projector.ts b/packages/agent-core/src/agent/context/projector.ts index f607788e68..14d3970752 100644 --- a/packages/agent-core/src/agent/context/projector.ts +++ b/packages/agent-core/src/agent/context/projector.ts @@ -49,9 +49,10 @@ export interface ProjectOptions { readonly mergeConsecutiveAssistants?: boolean; /** * When `true`, drop assistant tool calls whose id already appeared earlier - * (first occurrence wins; a message left with no content and no calls is - * dropped), and drop every tool result after the first for a given id so the - * kept call keeps exactly one answer. Duplicate ids are wire-invalid on + * (first occurrence wins; a message left with no calls and no sendable + * content is dropped), and drop every tool result after the first for a + * given id so the kept call keeps exactly one answer. Duplicate ids are + * wire-invalid on * strict providers ("`tool_use` ids must be unique") and no other pass can * repair them. Strict-resend only: a provider that accepted the duplicates * when it produced them (e.g. per-response counter ids like `call_0`) must @@ -97,7 +98,15 @@ export type ProjectionAnomaly = /** 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 }; + | { readonly kind: 'whitespace_text_dropped'; readonly role: string } + /** + * Every recorded part serialized to nothing on the wire (e.g. an assistant + * step that recorded only an empty thinking part from a provider-filtered + * response), so the whole message was dropped. Distinct from the silent + * empty-content drop: parts were recorded, yet none of them was sendable — + * a genuine defect signal, not routine cleanup. + */ + | { readonly kind: 'vacuous_message_dropped'; readonly role: string }; export function project(history: readonly ContextMessage[], options?: ProjectOptions): Message[] { let result = mergeAdjacentUserMessages(history, options?.onAnomaly); @@ -212,7 +221,11 @@ function repairToolExchangeAdjacency( // Strict providers reject a request whose assistant messages carry two // `tool_use` blocks with the same id ("tool_use ids must be unique"). Keep the // first occurrence of each call id, drop the rest, and drop an assistant -// message entirely when duplicates were all it carried. Every result after the +// message entirely when duplicates were all it carried — or when removing +// them leaves only vacuous content (e.g. a single empty thinking block), +// which would serialize as an empty assistant and be rejected again on the +// strict retry ("the message ... with role 'assistant' must not be empty"). +// Every result after the // first for a given id is dropped with its call, so no dangling tool message // survives the dedupe; when the kept call has no result of its own, the later // duplicate's surviving result is reattached by the adjacency repair. Runs @@ -239,8 +252,14 @@ function dedupeDuplicateToolCalls( }); if (kept.length === message.toolCalls.length) { out.push(message); - } else if (kept.length > 0 || message.content.length > 0) { + } else if (kept.length > 0 || !message.content.every(isVacuousContentPart)) { out.push({ ...message, toolCalls: kept }); + } else if (message.content.length > 0) { + // Every call this message carried was a later duplicate, and only + // vacuous content remains: kept, it would serialize as an empty + // assistant and be rejected again on the strict retry. Drop it with + // a trace (a truly content-free message stays a silent drop). + onAnomaly?.({ kind: 'vacuous_message_dropped', role: message.role }); } continue; } @@ -405,7 +424,34 @@ function prepareMessageForProjection( // content-free — it must survive the empty-message cleanup or the loaded // schemas silently vanish from every outgoing request. if (next.tools !== undefined && next.tools.length > 0) return next; - return next.content.length === 0 && next.toolCalls.length === 0 ? null : next; + if (next.toolCalls.length > 0) return next; + if (next.content.length === 0) return null; + // Every remaining part serializes to nothing on the wire — e.g. an + // assistant step that recorded only an empty thinking part from a + // provider-filtered response. Sent as-is it becomes an assistant message + // with no content and no tool calls, which strict providers reject ("the + // message ... with role 'assistant' must not be empty") on every resend, + // permanently wedging the session. Drop the whole message here. A message + // that carries any real content keeps every part verbatim — including + // empty thinking blocks, which preserved-thinking providers require back. + if (next.content.every(isVacuousContentPart)) { + onAnomaly?.({ kind: 'vacuous_message_dropped', role: next.role }); + return null; + } + return next; +} + +/** + * True when a content part carries nothing the provider wire can represent: + * an empty or whitespace-only text block, or an empty thinking block with no + * provider signature. A signed thinking block (`encrypted`) is never vacuous + * — reasoning providers require it back verbatim — and media parts always + * carry content. + */ +function isVacuousContentPart(part: ContentPart): boolean { + if (part.type === 'text') return part.text.trim().length === 0; + if (part.type === 'think') return part.encrypted === undefined && part.think.trim().length === 0; + return false; } function canMergeUserMessage(message: ContextMessage): boolean { diff --git a/packages/agent-core/test/agent/context/projector.test.ts b/packages/agent-core/test/agent/context/projector.test.ts index 35889f9b2d..09b0500e67 100644 --- a/packages/agent-core/test/agent/context/projector.test.ts +++ b/packages/agent-core/test/agent/context/projector.test.ts @@ -535,6 +535,102 @@ describe('project drops whitespace-only text', () => { }); }); +// --------------------------------------------------------------------------- +// Vacuous (thinking-only) messages +// --------------------------------------------------------------------------- + +function thinkPart(think: string, encrypted?: string): ContentPart { + return encrypted === undefined + ? { type: 'think', think } + : { type: 'think', think, encrypted }; +} + +function thinkingAssistant(parts: readonly ContentPart[]): ContextMessage { + return { role: 'assistant', content: [...parts], toolCalls: [] }; +} + +describe('project drops vacuous (thinking-only) messages', () => { + it('drops an assistant message whose only part is an empty think block', () => { + // A provider-filtered response records exactly this shape; sending it + // back is rejected as "message ... with role 'assistant' must not be + // empty" on every resend, permanently wedging the session. + const anomalies: ProjectionAnomaly[] = []; + const projected = project( + [user('u1'), thinkingAssistant([thinkPart('')]), user('u2')], + { onAnomaly: (a) => anomalies.push(a) }, + ); + expect(projected.map((m) => m.role)).toEqual(['user', 'user']); + expect(anomalies).toEqual([{ kind: 'vacuous_message_dropped', role: 'assistant' }]); + }); + + it('un-wedges a history poisoned by a filtered step (session regression)', () => { + const anomalies: ProjectionAnomaly[] = []; + const projected = project( + [ + user('u1'), + assistant(['a']), + tool('a'), + // sealed by the filtered step: no text, no tool calls, one empty think + thinkingAssistant([thinkPart('')]), + notification('ping'), + user('u2'), + ], + { onAnomaly: (a) => anomalies.push(a) }, + ); + expect(findMisplacedToolUses(projected)).toEqual([]); + expect(projected.map((m) => m.role)).toEqual(['user', 'assistant', 'tool', 'user', 'user']); + expect(anomalies).toEqual([{ kind: 'vacuous_message_dropped', role: 'assistant' }]); + }); + + it('keeps a message with real text intact — including its empty think part', () => { + // Preserved-thinking providers require even empty reasoning back; only + // wholly-vacuous messages may be dropped, never parts of a real message. + const projected = project([user('u1'), thinkingAssistant([thinkPart(''), textPart('answer')])]); + expect(projected[1]?.content).toEqual([thinkPart(''), textPart('answer')]); + }); + + it('keeps a message whose think block has real content', () => { + const projected = project([user('u1'), thinkingAssistant([thinkPart('real reasoning')])]); + expect(projected.map((m) => m.role)).toEqual(['user', 'assistant']); + expect(projected[1]?.content).toEqual([thinkPart('real reasoning')]); + }); + + it('keeps a signed think block even when its text is empty', () => { + const projected = project([user('u1'), thinkingAssistant([thinkPart('', 'sig')])]); + expect(projected.map((m) => m.role)).toEqual(['user', 'assistant']); + expect(projected[1]?.content).toEqual([thinkPart('', 'sig')]); + }); + + it('drops a message whose think block is whitespace-only', () => { + const anomalies: ProjectionAnomaly[] = []; + const projected = project([user('u1'), thinkingAssistant([thinkPart(' ')]), user('u2')], { + onAnomaly: (a) => anomalies.push(a), + }); + expect(projected.map((m) => m.role)).toEqual(['user', 'user']); + expect(anomalies).toEqual([{ kind: 'vacuous_message_dropped', role: 'assistant' }]); + }); + + it('keeps an assistant message with tool calls even when its think part is empty', () => { + const projected = project([ + user('u1'), + { + role: 'assistant', + content: [thinkPart('')], + toolCalls: [{ type: 'function', id: 'a', name: 'Run', arguments: '{}' }], + }, + tool('a'), + ]); + expect(projected.map((m) => m.role)).toEqual(['user', 'assistant', 'tool']); + expect(projected[1]?.content).toEqual([thinkPart('')]); + }); + + it('reports no anomaly for a silently dropped empty-content message', () => { + const anomalies: ProjectionAnomaly[] = []; + project([user('u1'), emptyAssistant(), user('u2')], { onAnomaly: (a) => anomalies.push(a) }); + expect(anomalies).toEqual([]); + }); +}); + 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. @@ -666,6 +762,73 @@ describe('project duplicate tool_use ids', () => { ); expect(projected.map((message) => message.role)).toEqual(['user', 'assistant', 'tool', 'user']); }); + + it('under the strict flag, drops an assistant left with only vacuous content after removing duplicates', () => { + // A tool step with an empty thinking block whose call id is a later + // duplicate: dedupe strips its only call, and keeping the vacuous + // remainder would serialize as an empty assistant and be rejected again + // ("the message ... with role 'assistant' must not be empty"). + const anomalies: ProjectionAnomaly[] = []; + const projected = project( + [ + user('u1'), + assistant(['call_a'], 'first'), + tool('call_a', 'first result'), + { + role: 'assistant', + content: [thinkPart('')], + toolCalls: [{ type: 'function', id: 'call_a', name: 'Run', arguments: '{}' }], + }, + tool('call_a', 'second result'), + user('u2'), + ], + { + dedupeDuplicateToolCalls: true, + dropOrphanResults: true, + onAnomaly: (a) => anomalies.push(a), + }, + ); + expect(projected.map((message) => message.role)).toEqual(['user', 'assistant', 'tool', 'user']); + expect(anomalies).toContainEqual({ kind: 'duplicate_tool_call_dropped', toolCallId: 'call_a' }); + expect(anomalies).toContainEqual({ kind: 'duplicate_tool_result_dropped', toolCallId: 'call_a' }); + expect(anomalies).toContainEqual({ kind: 'vacuous_message_dropped', role: 'assistant' }); + expect(everyToolUseImmediatelyAnswered(projected)).toBe(true); + }); + + it('under the strict flag, keeps a duplicate-stripped assistant whose remaining content is sendable', () => { + const anomalies: ProjectionAnomaly[] = []; + const projected = project( + [ + user('u1'), + assistant(['call_a'], 'first'), + tool('call_a', 'first result'), + { + role: 'assistant', + content: [thinkPart(''), textPart('second')], + toolCalls: [{ type: 'function', id: 'call_a', name: 'Run', arguments: '{}' }], + }, + tool('call_a', 'second result'), + user('u2'), + ], + { + dedupeDuplicateToolCalls: true, + dropOrphanResults: true, + onAnomaly: (a) => anomalies.push(a), + }, + ); + expect(projected.map((message) => message.role)).toEqual([ + 'user', + 'assistant', + 'tool', + 'assistant', + 'user', + ]); + const stripped = projected[3]!; + expect(stripped.toolCalls).toEqual([]); + expect(stripped.content).toEqual([thinkPart(''), textPart('second')]); + expect(anomalies).not.toContainEqual({ kind: 'vacuous_message_dropped', role: 'assistant' }); + expect(everyToolUseImmediatelyAnswered(projected)).toBe(true); + }); }); // --------------------------------------------------------------------------- diff --git a/packages/kosong/src/errors.ts b/packages/kosong/src/errors.ts index 9f0862e753..544268657f 100644 --- a/packages/kosong/src/errors.ts +++ b/packages/kosong/src/errors.ts @@ -490,6 +490,12 @@ const STRUCTURAL_REQUEST_MESSAGE_PATTERNS = [ // when a provider reused a call id (e.g. per-response counter ids) earlier // in the session; the strict resend dedupes the ids. /tool_use[\s\S]*ids must be unique/, + // Moonshot / Kimi rejects a message whose serialized form carries nothing — + // no content, no tool_calls, an empty reasoning_content: "the message at + // position N with role 'assistant' must not be empty". Seen when a filtered + // response left an assistant message holding only an empty thinking part in + // the history; the strict resend's projection drops such vacuous messages. + /message at position \d+ with role ['"`]?[a-z]+['"`]? must not be empty/, ] as const; export function isRecoverableRequestStructureError(error: unknown): boolean { diff --git a/packages/kosong/test/errors.test.ts b/packages/kosong/test/errors.test.ts index 1eb62efdbf..2e39e971ed 100644 --- a/packages/kosong/test/errors.test.ts +++ b/packages/kosong/test/errors.test.ts @@ -506,6 +506,17 @@ describe('isRecoverableRequestStructureError', () => { ).toBe(true); }); + it('matches the Moonshot/Kimi vacuous-message rejection', () => { + expect( + isRecoverableRequestStructureError( + new APIStatusError( + 400, + "400 the message at position 105 with role 'assistant' must not be empty", + ), + ), + ).toBe(true); + }); + it('does not match context overflow, auth, or non-status errors', () => { expect( isRecoverableRequestStructureError(new APIContextOverflowError(400, 'context length exceeded')),