Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/openai-tool-exchange-400-recovery.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@moonshot-ai/kosong": patch
---

Recognize the OpenAI-compatible `role 'tool' must be a response to a preceding message with 'tool_calls'` and `assistant message with 'tool_calls' must be followed by tool messages` 400s (OpenAI / DeepSeek / vLLM / Qwen phrasings) as recoverable tool-exchange structural errors, so the post-400 strict-resend fallback fires and un-bricks the session instead of failing every subsequent turn — including after switching providers or models.
5 changes: 5 additions & 0 deletions .changeset/projector-drops-orphan-tool-results.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@moonshot-ai/agent-core": patch
---

Drop orphan tool results at the projection boundary so a malformed history cannot brick a session. A `tool` result whose assistant `tool_call` is nowhere in the history (e.g. an older session whose compaction cut fell inside a tool exchange, restored via the legacy path) is now removed from every projected request, not only on the post-400 strict resend. The stored history is left faithful to the wire records — so consumers that model it, like the transcript fold length, stay in sync — while a strict provider (OpenAI / DeepSeek) always receives a valid request. The drop is surfaced via the projection-repair log rather than done silently.
9 changes: 8 additions & 1 deletion packages/agent-core/src/agent/compaction/full.ts
Original file line number Diff line number Diff line change
Expand Up @@ -411,8 +411,15 @@ export class FullCompaction {
let overflowShrinkCount = 0;
let emptyOrTruncatedShrinkCount = 0;
while (true) {
// A request-building projection: close still-open calls in the sliced
// prefix (synthesizeMissing) and drop stray results with no call anywhere
// (dropOrphanResults), so the summarizer request cannot be rejected by a
// strict provider even when the history carries a legacy-restore orphan.
const messages = [
...this.agent.context.project(historyForModel, { synthesizeMissing: true }),
...this.agent.context.project(historyForModel, {
synthesizeMissing: true,
dropOrphanResults: true,
}),
createUserMessage(instruction),
];
const estimatedCompactionRequestTokens = this.estimateRequestTokens(messages);
Expand Down
37 changes: 24 additions & 13 deletions packages/agent-core/src/agent/context/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -273,14 +273,18 @@ export class ContextMemory {
};
// Wire backward-compat: a pre-rework `context.apply_compaction` record (which
// has no `keptUserMessageCount`) used `[summary, ...history.slice(compactedCount)]`
// semantics and kept a verbatim recent tail. Reproduce that exact shape on
// restore so resuming a session compacted by an older version does not
// silently drop the recent assistant/tool tail beyond `compactedCount`. Gated
// on `records.restoring`, so the live/forward path — which always sets
// `contextSummary` and `keptUserMessageCount` — is unaffected. The projector's
// tool-adjacency repair keeps the restored tail well-formed for strict
// providers; compaction only runs at a clean step boundary, so the tail has no
// open tool exchange to track.
// semantics and kept a verbatim recent tail. Reproduce that shape on restore
// so resuming a session compacted by an older version does not silently drop
// the recent assistant/tool tail beyond `compactedCount`. Gated on
// `records.restoring`, so the live/forward path — which always sets
// `contextSummary` and `keptUserMessageCount` — is unaffected.
//
// The cut can land inside a tool exchange, leaving the tail starting with an
// orphan `tool` result whose assistant is now in the summarized prefix. The
// history is kept faithful to the wire records (so the transcript reducer's
// fold length stays in sync); the projector drops the orphan at the wire
// boundary — see `dropOrphanToolResults` — so a strict provider still gets a
// valid request without mutating the stored history here.
const isLegacyRestore =
this.agent.records.restoring !== null &&
input.keptUserMessageCount === undefined &&
Expand Down Expand Up @@ -394,14 +398,21 @@ export class ContextMemory {
}

get messages(): Message[] {
return this.project(this.history);
// The normal wire projection. `dropOrphanResults` is on for every
// request-building projection (here, `strictMessages`, and the compaction
// summarizer): a stray result with no matching call anywhere is wire-invalid
// on strict providers and useless to the model, so it never reaches the
// provider — while fragment projections (e.g. token estimation of a history
// slice) leave it alone.
return this.project(this.history, { dropOrphanResults: true });
}

// 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`.
// call (including a trailing in-flight one), drop stray tool results, drop a
// leading non-user message, and merge consecutive assistant turns, 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,
Expand Down
22 changes: 14 additions & 8 deletions packages/agent-core/src/agent/context/projector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,13 @@ export interface ProjectOptions {
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.
* `tool_use` anywhere in the provided messages. Such an orphan is wire-invalid
* on every strict provider and useless to the model (it has no record of the
* call the result answers). Enabled on every request-building projection — the
* normal wire, the strict resend, and the compaction summarizer — so a stray
* result never reaches a provider. Left OFF for non-request projections (e.g.
* token-estimating a history slice), where a result's matching call may
* legitimately sit outside the slice and must not be mistaken for an orphan.
*/
readonly dropOrphanResults?: boolean;
/**
Expand Down Expand Up @@ -69,7 +71,7 @@ export type ProjectionAnomaly =
* 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). */
/** A result with no matching call anywhere was dropped (wire exits 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 }
Expand Down Expand Up @@ -189,8 +191,12 @@ function repairToolExchangeAdjacency(

// 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.
// result, and it is useless to the model regardless (it has no record of the
// call the result answers), so every request-building projection drops it (via
// `dropOrphanResults`). Kept separate from the adjacency repair, which only
// reorders results that DO have a matching call; this removes the ones that do
// not. Reported via `onAnomaly` so the drop leaves a trace instead of silently
// discarding a recorded result.
function dropOrphanToolResults(
messages: readonly Message[],
onAnomaly?: (anomaly: ProjectionAnomaly) => void,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ describe('compaction — Anthropic wire compliance', () => {
{ role: 'tool', content: [{ type: 'text', text: 'done' }], toolCalls: [], toolCallId: 'call_2' },
];

// Normal send path: no synthesizeMissing, no dropOrphanResults.
// Normal send path: no synthesizeMissing.
const projected = ctx.agent.context.project(orphaned);
const wire = await toAnthropicWire(projected, [BASH_TOOL]);
assertValidAnthropic(wire);
Expand All @@ -260,20 +260,19 @@ describe('compaction — Anthropic wire compliance', () => {
).toBe(true);
});

it('drops a stray tool result with no matching call on the strict resend path', async () => {
it('drops a stray tool result with no matching call from request projections', 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.
// A tool_result whose tool_use is gone (e.g. an undo removed the assistant,
// or a legacy-restore compaction cut mid-exchange). Every request-building
// projection (`messages`, `strictMessages`, the summarizer) enables
// dropOrphanResults — it has no anchor and is useless to the model.
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,
});
const projected = ctx.agent.context.project(stray, { dropOrphanResults: true });
expect(projected.some((m) => m.role === 'tool')).toBe(false);
const wire = await toAnthropicWire(projected);
assertValidAnthropic(wire);
Expand Down
26 changes: 17 additions & 9 deletions packages/agent-core/test/agent/context/projector.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -341,15 +341,20 @@ describe('project tool_use/tool_result adjacency', () => {
tool('orphan-result'),
user('u2'),
];
// Without dropOrphanResults (fragment projections, e.g. token estimation)
// the stray result stays where it was; nothing references it.
const projected = project(history);
// The stray result stays where it was; nothing references it.
expect(projected.map((m) => [m.role, m.toolCallId])).toEqual([
['user', undefined],
['assistant', undefined],
['tool', 'a'],
['tool', 'orphan-result'],
['user', undefined],
]);
// Request-building projections enable dropOrphanResults and remove it.
const wire = project(history, { dropOrphanResults: true });
expect(wire.some((m) => m.toolCallId === 'orphan-result')).toBe(false);
expect(wire.some((m) => m.toolCallId === 'a')).toBe(true);
});

it('does not crash when a tool result appears before its tool_use', () => {
Expand Down Expand Up @@ -425,15 +430,18 @@ describe('project repair reporting', () => {
]);
});

it('reports a dropped orphan result only on the strict path', () => {
it('reports a dropped orphan result when dropOrphanResults is set', () => {
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' }]);
// Fragment projections (no flag) leave the stray in place and report nothing.
const fragment: ProjectionAnomaly[] = [];
project(history, { onAnomaly: (a) => fragment.push(a) });
expect(fragment).toEqual([]);

// Request-building projections (normal wire, strict resend, summarizer)
// enable the flag, drop the stray, and surface the repair.
const wire: ProjectionAnomaly[] = [];
project(history, { dropOrphanResults: true, onAnomaly: (a) => wire.push(a) });
expect(wire).toEqual([{ kind: 'orphan_tool_result_dropped', toolCallId: 'stray' }]);
});

it('reports a whitespace-only text drop but not a truly-empty one', () => {
Expand Down
89 changes: 89 additions & 0 deletions packages/agent-core/test/agent/resume.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,95 @@ describe('Agent resume', () => {
]);
});

it('keeps a legacy mid-tool-exchange cut faithful but projects it wire-valid', async () => {
// A pre-rework compaction record (no `keptUserMessageCount`) restores via the
// legacy path, which keeps a verbatim tail `history.slice(compactedCount)`.
// Here the cut (compactedCount=2) lands *between* the assistant `tool_call`
// and its result, so the retained tail starts with a `tool` message whose
// assistant was summarized away — a wire-invalid orphan a strict provider
// (OpenAI / DeepSeek) rejects with "role 'tool' must be a response to a
// preceding message with 'tool_calls'". The restore keeps the history
// faithful (so the transcript reducer's fold length stays in sync); the
// projector drops the orphan at the wire boundary.
const persistence = new RecordingAgentPersistence([
{
type: 'context.append_message',
message: {
role: 'user',
content: [{ type: 'text', text: 'first prompt' }],
toolCalls: [],
origin: { kind: 'user' },
},
},
{
type: 'context.append_loop_event',
event: { type: 'step.begin', uuid: 'orphan-step', turnId: '0', step: 1 },
},
{
type: 'context.append_loop_event',
event: {
type: 'tool.call',
uuid: 'orphan-call',
turnId: '0',
step: 1,
stepUuid: 'orphan-step',
toolCallId: 'call_orphaned',
name: 'Bash',
args: { command: 'pwd' },
},
},
{
type: 'context.append_loop_event',
event: {
type: 'tool.result',
parentUuid: 'orphan-call',
toolCallId: 'call_orphaned',
result: { output: 'ok', isError: false },
},
},
{
type: 'context.append_message',
message: {
role: 'user',
content: [{ type: 'text', text: 'second prompt' }],
toolCalls: [],
origin: { kind: 'user' },
},
},
{
type: 'context.apply_compaction',
summary: 'Compacted the first exchange.',
compactedCount: 2,
tokensBefore: 120,
tokensAfter: 24,
},
]);
const ctx = testAgent({ persistence });

await ctx.agent.resume();

// The stored history stays faithful to the wire records: the orphan `tool`
// result is kept verbatim (not mutated away at restore), so downstream
// consumers that model the history from the records — e.g. the transcript
// reducer's fold length — stay in sync.
expect(ctx.agent.context.history.some((message) => message.role === 'tool')).toBe(true);

// But the projected wire the provider actually sees has no orphan: every
// `tool` result is answered by a preceding assistant `tool_calls`.
const projected = ctx.agent.context.messages;
const toolCallIds = new Set(
projected.flatMap((message) =>
message.role === 'assistant' ? message.toolCalls.map((toolCall) => toolCall.id) : [],
),
);
const orphanToolResults = projected.filter(
(message) =>
message.role === 'tool' &&
(message.toolCallId === undefined || !toolCallIds.has(message.toolCallId)),
);
expect(orphanToolResults).toEqual([]);
});

it('projects restored cancelled compactions into replay records', async () => {
const persistence = new RecordingAgentPersistence([
{
Expand Down
21 changes: 21 additions & 0 deletions packages/agent-core/test/loop/tool-exchange-fallback.e2e.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,14 @@ const ADJACENCY_400 = new APIStatusError(
// structural rejection. Verbatim from the field, doubled space included.
const MOONSHOT_TOOL_CALL_ID_400 = new APIStatusError(400, '400 tool_call_id is not found');

// The OpenAI / DeepSeek phrasing of an orphan `tool` result — a `tool` message
// with no preceding assistant `tool_calls`. This is what a DeepSeek / OpenAI-
// compatible provider returns for a history bricked by a stray tool result.
const OPENAI_ROLE_TOOL_400 = new APIStatusError(
400,
"Messages with role 'tool' must be a response to a preceding message with 'tool_calls'",
);

function userMessage(text: string): Message {
return { role: 'user', content: [{ type: 'text', text }], toolCalls: [] };
}
Expand Down Expand Up @@ -98,6 +106,19 @@ describe('executeLoopStep — tool exchange adjacency fallback', () => {
expect(llm.calls[1]?.messages).toBe(strictMessages);
});

it('resends once and recovers after an OpenAI/DeepSeek role-tool 400', async () => {
const { input, llm, strictCalls, strictMessages } = makeHarness(OPENAI_ROLE_TOOL_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);
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'));

Expand Down
Loading
Loading