fix(agent-core): harden strict-provider wire compliance so malformed history can't brick a session - #1241
Merged
Merged
Conversation
Rework the full-compaction summary to read as the agent's own continuing notes instead of a third-party report: - compaction-instruction.md: free-form first-person continuation that preserves exact commands, paths and outcomes, states the precise next action, and flags claimed-but-unverified work rather than trusting it. - compaction-summary-prefix.md: skeptical "your own working notes" framing; drop the collaborative third-party prefix. - system.md: add compaction-awareness guidance so the model continues naturally from a summary and re-checks any reported "done". - Rename the compaction helpers module to handoff.ts. Update tests and regenerate snapshots for the new prompt text, and fill in contextSummary in the restored-compaction replay expectations.
estimateTokensForContentPart returned 0 for image_url/audio_url/video_url, so auto-compaction triggers, the overflow-shrink budget, the kept-user budget, and the reported context size all went blind to media — a media-heavy session could overflow the model window while the estimate reported a near-empty context. Media parts now carry a fixed estimate (MEDIA_TOKEN_ESTIMATE), and the content-part switch is exhaustive so a new ContentPart kind must declare its estimate rather than silently count as zero.
Folding the live context to [recent user prompts, summary] drops the messages that started background tasks and their status updates, so the model could forget a task is still running and spawn a duplicate. injectAfterCompaction now appends a system-reminder listing active background tasks (with guidance to use TaskOutput/TaskList/TaskStop instead of re-spawning). It runs only post-compaction and carries an injection origin, so the next compaction drops and rebuilds it rather than stacking copies; the all-user-role post-compaction shape is preserved (no tool-pairing reintroduced).
Adds compaction-scenarios.test.ts driving the real Agent/ContextMemory/ FullCompaction machinery: - A guard test locking in that repeated compaction folds the prior summary into the new one instead of stacking two summaries. - Seven `it.fails` probes that executably reproduce known, currently-accepted edge-case defects so the suite stays green while documenting each one precisely; any of them will flip red (forcing removal of `.fails`) the day the behavior is fixed. They cover: assistant/tool appended during an in-flight summarizer call being dropped; unbounded shrink on empty summaries; the fixed 20k kept-user budget overflowing a small model window; a tool result orphaned when compaction starts mid-exchange; legacy compaction records dropping their verbatim tail on replay; micro-compaction clearing recent tool results in an overflow-shrunk suffix; and media being discarded when the oldest kept user message is truncated.
…ontext A tool call and its result can end up non-adjacent in history — a background-task notification or flushed steer lands between them, or an interrupted/nested step delays the result — which strict providers reject with HTTP 400. The projector now moves each tool_use's result up to immediately follow it (projection-time only; the stored history is untouched), and full compaction projects its summarizer input with a synthetic result for any still-open call so the summary request stays well-formed. Micro-compaction only surfaced this latent ordering by busting the prompt cache, so it now defaults off. Includes projector adjacency regression tests, a context-level integration test, and a compaction synthesize-missing guard; the prior "keeps an unresolved tool exchange out of the compaction prompt" test is updated to the now-well-formed (synthetic-result) behavior.
…pactions A pre-rework `context.apply_compaction` record used `[summary, ...history.slice(compactedCount)]` semantics and kept a verbatim recent tail, but it has no `keptUserMessageCount`. The reworked applyCompaction re-folded such records into the all-user shape, dropping the recent assistant/tool tail — so resuming a session compacted by an older version silently lost its most recent context. On restore of such a record (gated on records.restoring, no keptUserMessageCount, and compactedCount < history length) reproduce the old shape instead. The forward/live path is unchanged; the projector's tool-adjacency repair keeps the restored tail well-formed, and compaction only runs at clean step boundaries so the tail has no open exchange. The legacy-tail probe now passes as a regression guard via the real restore path.
The transcript reducer re-derived foldedLength for pre-rework context.apply_compaction records (no keptUserMessageCount) using the new kept-user+summary rule, but ContextMemory's restore now reproduces the legacy [summary, ...history.slice(compactedCount)] shape for those records. The two diverged for legacy sessions, so MessageService's foldedLength-vs-live-history comparison could mis-handle GET /messages (miss or misorder recent output). The reducer now mirrors the live legacy fold: when compactedCount is below the pre-compaction length it computes 1 + (length - compactedCount); otherwise it falls back to the kept-user derivation. The MessageService transcript test's fixture is corrected to a new-format record, matching its all-user live mock.
The Anthropic message merge keyed on isToolResultOnly(last) === isToolResultOnly(converted), which left a tool_result-only user turn followed by a plain-text user turn unmerged. After tool-exchange repair this shape (assistant tool_use -> tool_result -> injected notification) produces two adjacent user messages, which strict Anthropic-compatible backends reject with HTTP 400. Switch to the asymmetric predicate isToolResultOnly(last) || !isToolResultOnly(converted): a tool-result-only running message absorbs whatever user turn follows (parallel tool_results or a trailing text), yielding a valid [tool_result, ..., text] message; a plain-text running message still only absorbs plain text. [tool_result, text] is valid for both native Anthropic (which concatenates anyway) and strict backends.
The 'does not clear recent tool results when projecting a shrunk suffix' probe is an it.fails that only documents a real defect while micro-compaction is active. It inherited the ambient KIMI_CODE_EXPERIMENTAL master switch, so its pass/fail flipped with the runner: green locally (master switch on) but a hard failure in CI, where the flag defaults off and MicroCompaction.compact() is a no-op that leaves the tool result intact. Enable KIMI_CODE_EXPERIMENTAL_MICRO_COMPACTION explicitly for this probe so it deterministically exercises the micro-compaction path regardless of the environment.
…unded shrink, and media loss Three compaction-path fixes surfaced by review, each flipping its documenting it.fails probe to a passing it: - Append race (CMP-02): after the summarizer returns, the post-summary history check only compared the compacted prefix. A live step appending to the tail while a manual/SDK compaction was in flight slipped through — an appended assistant/tool turn is neither summarized (the summary covers only the snapshot) nor kept (the rebuild keeps user input), so it vanished. Now cancel when the appended tail contains a non-user message; an appended user message is still kept (rebuild picks it up), preserving the existing 'keeps messages appended while compacting an unchanged prefix' behavior. - Unbounded empty/truncated shrink: an empty or truncated summary dropped the oldest message and reset retryCount, so a model that kept returning empty could issue ~one request per history entry. Bound the shrink attempts by MAX_COMPACTION_RETRY_ATTEMPTS, mirroring the overflow-shrink counter. - Media dropped on truncation (CMP-07): truncating the oldest kept user message replaced its whole content with one text block, discarding any image/audio/video. Keep the non-text parts and spend the remaining budget (maxTokens minus their cost) on truncated text.
For a pre-rework context.apply_compaction record (no keptUserMessageCount), agent-core's ContextMemory restore and the transcript reducer keep the old [summary, ...history.slice(compactedCount)] tail — a verbatim recent tail including assistant/tool. The vis model-mode projector always applied the new kept-user selection, so opening an older compacted session in model mode hid the assistant/tool tail the resumed agent still holds (and surfaced a pre-compaction user message the agent dropped). Branch on a missing keptUserMessageCount with compactedCount < history length and reproduce the legacy shape, matching the agent-core restore.
Resolve conflicts from main adding the plugin_command PromptOrigin kind alongside this branch's compaction rework, which centralized real-user detection into isRealUserInput / compactionUserMessageDisposition: - Drop the per-file isRealUserPrompt helpers main re-added in sessionService, context, transcript, and the vis projector — every caller on this branch already routes through isRealUserInput, so they were dead duplicates. Refreshed two stale comments that still named isRealUserPrompt in the vis projector. - Teach compactionUserMessageDisposition the new plugin_command kind (user-slash => keep, like skill_activation), restoring the exhaustive switch, and add it to the handoff disposition test's origin matrices.
The in-flight append guard cancelled only when the tail grew with a non-user role. A user-role message that compaction would still drop — a background-task notification, hook/cron reminder, or shell-command output — slipped through: appended after the summary snapshot (so absent from the summary) and dropped by the all-user rebuild (which keeps only real user input), vanishing silently. Key the guard on the same predicate applyCompaction uses (!isRealUserInput) so it cancels whenever the appended tail holds anything compaction would drop. A real user message is still kept, so a live user turn racing a manual/SDK compaction continues to complete.
The transcript reducer's legacy fallback (records predating keptUserMessageCount, compacted with no verbatim tail) re-derived the kept-user count from the whole transcript, including messages before the last context.clear. Live ContextMemory rebuilds _history from post-clear messages only, so counting pre-clear prompts overstated foldedLength; MessageService then saw context.history.length <= foldedLength and skipped appending unflushed live tail messages, dropping recent output from the messages endpoint for old sessions compacted after a clear. Derive only from entries at or after clearFloor to match the live context.
Revert the media-preserving truncation: keeping non-text parts on the truncated boundary message overshot the kept-user budget when the media alone exceeded it, and reordered interleaved text/media parts. Both codex (no media-aware truncation) and Claude Code (strips media at compaction) decline to preserve media on a truncated message, since media cannot be partially truncated and keeping it whole breaks the budget. truncateUserMessage now keeps only the truncated text. Recent messages that fit the budget are still kept verbatim with their media; only the oldest, partially-overflowing boundary message loses its attachments.
A manual/SDK compaction could start while a turn was streaming, or a new
turn could launch while a compaction was in flight. Either way the turn
mutates the shared context (streaming content into an existing assistant
message, or appending new messages) during the summarizer await, and that
output is neither summarized nor preserved by the all-user rebuild —
silent loss that object-identity checks can't detect (the streamed message
is mutated in place).
Guard both directions so the agent does one of {turn, compaction} at a
time: begin() refuses a manual compaction while a turn is active, and
launch() refuses a new turn while a compaction is in progress. Auto
compaction is exempt — it runs from within the turn at a step boundary,
which blocks the turn for its duration.
…ction Drive real compaction output and the compaction summarizer projection through the real Anthropic provider conversion and assert the wire request is well-formed: strict user/assistant alternation and every tool_use answered by an adjacent tool_result. Locks in the cross-layer guarantee (projector merge + Anthropic consecutive-user merge + adjacency repair + synthesizeMissing) that compacted sessions stay valid for strict Anthropic-compatible backends.
# Conflicts: # packages/agent-core/src/agent/compaction/full.ts # packages/agent-core/test/agent/compaction/full.test.ts
…tead of rejecting Manual/SDK compaction runs outside a turn, so the earlier guard rejected prompts/steers that arrived while it held the context. That broke three things: a REST/web prompt got stuck 'running' (no terminal turn event), a background-task/cron steer was silently lost (null was read as 'buffered' but nothing was), and a follow-up prompt could land in the window after isCompacting cleared but before reminders were reinjected. Reuse the existing defer-and-replay model instead of rejecting: - steer() and launch() buffer into steerBuffer while a compaction is in progress (returning null = buffered), mirroring how an active turn defers input. - FullCompaction.compactionWorker keeps isCompacting true through refreshSystemPrompt + injectAfterCompaction (moving markCompleted and the completed event after reinjection), then replays the buffer via TurnFlow.onCompactionFinished — on success, on an A1 prefix/tail cancel, and on failure/abort. - onCompactionFinished flushes into an active turn if one exists, else launches a fresh turn from the deferred input. No PromptService change: a deferred prompt's eventual turn.started lets it associate the pending prompt and clear it on turn.ended.
Add isToolExchangeAdjacencyError to classify the strict-provider 400 raised when an assistant tool_use is not correctly paired with its tool_result (missing, stray, or non-adjacent), excluding context-overflow 400s. Lets the agent loop recognize the error and resend a wire-compliant request instead of leaving the session stuck.
…compliant after a strict 400 Strict providers (Anthropic) reject a request whose assistant tool_use is not answered by an adjacent tool_result, and the same malformed history is re-sent every turn, permanently bricking the session. - Projector now closes a mid-history tool call whose result is missing entirely (a later turn proves it is not in-flight) with a synthetic result; the trailing in-flight call is still left untouched. - Add a strict projection (synthesize every open call, drop stray results) and, on a tool_use/tool_result adjacency 400, resend the request once with it. - Report every projection repair (reorder / synthesize / drop) via log and telemetry, deduped by signature, so a silently-mangled history leaves a trace. Trailing-tail synthesis (expected under compaction) is not flagged.
Gemini/Vertex require strictly alternating user/model turns and reject consecutive user turns with HTTP 400. They arise after compaction (kept prompts + user-role summary + injected reminders) and when a turn is steered in right after a tool result. Anthropic already merged them inline; the Google converter did not, so post-compaction requests failed. Extract the asymmetric merge into a shared mergeConsecutiveUserMessages helper applied at each strict provider's conversion boundary: refactor Anthropic to use it (behavior unchanged) and apply it at the Google converter's exit. A conformance suite drives every strict provider with the post-compaction shape and a steer-after-tool-result shape, asserting no consecutive same-role turns reach the wire, so a new strict provider cannot silently omit the merge. The provider-agnostic projector stays structure-preserving: lenient providers (OpenAI/Kimi) keep distinct turns for clearer message boundaries; only strict providers normalize, where the requirement lives.
# Conflicts: # .changeset/rework-compaction-strategy.md
Add isRecoverableRequestStructureError, covering the strict-provider 400s that stem from a malformed message array re-sent every turn: tool_use/tool_result pairing, empty/whitespace-only text blocks, a non-user first message, and non-alternating roles. Context-overflow 400s are excluded (handled by compaction). Lets the loop trigger one strict, wire-compliant resend for the whole family rather than only tool-pairing errors.
…s, with diagnostics - Drop empty AND whitespace-only text blocks in projection (Anthropic rejects whitespace-only with "text content blocks must contain non-whitespace text", which otherwise sticks a session); treat whitespace-only tool output as empty. - Broaden the post-400 strict resend to the whole structural family and add two strict-only passes to the strict projection: drop leading non-user messages (first message must be user) and merge consecutive assistant turns. - Log + telemetry for every wire repair the projector applies (reorder, synthesize, drop orphan, drop leading, merge assistants, drop whitespace), deduped by signature; log the strict resend outcome (recovered or still rejected) so a stuck session always leaves a trace.
…e-compliance # Conflicts: # .changeset/rework-compaction-strategy.md # packages/agent-core/src/agent/context/index.ts # packages/agent-core/src/agent/context/projector.ts # packages/agent-core/test/agent/compaction/anthropic-compliance.test.ts # packages/agent-core/test/agent/context/projector.test.ts
🦋 Changeset detectedLatest commit: 7d8081b The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2b137fd650
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| return messages.filter((message) => { | ||
| if (message.role !== 'tool' || message.toolCallId === undefined) return true; | ||
| if (toolUseIds.has(message.toolCallId)) return true; |
There was a problem hiding this comment.
In the strict resend path, this keeps any tool result whose ID appears anywhere in the request. If a malformed/restored history has a result before its matching assistant call, e.g. user -> tool(a) -> assistant(tool_use a), the result survives even though it is not immediately after a preceding assistant; Anthropic still sees an unexpected tool_result, so the fallback sends another invalid request and gives up. The strict filter needs to drop results that were not consumed adjacent to their call, not just IDs with no call globally.
Useful? React with 👍 / 👎.
| // 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) { |
There was a problem hiding this comment.
When a tool returns a ContentPart[] whose only text is whitespace, this branch strips the only block and the following tool-role empty check throws Tool result message content cannot be empty.... String tool outputs with the same whitespace are normalized to the empty-tool status in toolResultOutputForModel, but array outputs are still allowed by the tool contract, so the next turn can fail locally instead of sending the safe empty-result placeholder.
Useful? React with 👍 / 👎.
… empty placeholder A tool result whose ContentPart[] output has no sendable content (an empty array, or only empty/whitespace-only text blocks) was returned verbatim, so projection stripped the blank blocks, left the tool message empty, and threw on every send — bricking the session locally. String outputs were already normalized; do the same for arrays. A non-text part or any non-whitespace text still keeps the real output.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When a strict provider (Anthropic) rejects a request because the message history is structurally malformed — e.g. an assistant `tool_use` not immediately followed by its `tool_result` — the same history is re-projected and re-sent on every turn, so the session is permanently stuck on the 400. Observed in the field:
Replaying the reported session confirmed the cause is pure ordering (a message landed between a call and its result; all results were present), which the released version had no repair for.
Approach
Two layers of defense plus diagnostics, all at the single projection chokepoint every send passes through.
1. Pre-send projection repairs (always on, safe):
2. Post-400 strict resend (last resort): if the provider still rejects the request structure — tool_use/tool_result pairing, empty/whitespace text, non-user first message, or non-alternating roles — resend once with a strictly wire-compliant rebuild: synthesize every open call, drop stray results, trim leading non-user messages, merge consecutive assistant turns. Any other error propagates unchanged; no retry loop.
3. Diagnostics: every repair the projector applies (reorder / synthesize / drop orphan / drop leading / merge assistants / drop whitespace) and the strict-resend outcome (recovered or still rejected) are logged and tracked via telemetry, deduped by signature so a persistently-mangled history logs once rather than per step.
Key safety properties
Testing