fix(provider): never send a message with empty content (the Bedrock/Anthropic 400) — producer + backstop + inbox invariant - #1948
Merged
Conversation
A live Bedrock 400 `messages.<N>: user messages must have non-empty content` was traced to the AI SDK, not to our message array. `ai@6`'s `convertToLanguageModelMessage` strips empty text parts from user messages with no backfill: .filter((part) => part.type !== "text" || part.text !== "") That runs AFTER every ProviderTransform step, so a user message whose only text part is "" leaves our transform as a healthy length-1 array and reaches the provider as `content: []`. Emptiness therefore cannot be judged by `content.length` at this layer — it must be judged by what survives the SDK's own filter. The SDK's assistant branch has an escape for empty parts carrying providerOptions; the user branch does not, so even an empty text part holding a cache_control marker is stripped. Our only prior defense stripped empty parts in `normalizeMessages`, but gated on `@ai-sdk/anthropic`/`@ai-sdk/amazon-bedrock` — a Bedrock-backed gateway on any other npm got no protection at all. `normalizeContentArray` guarded only content shape and itself emitted `content: []`, and `ensureTrailingUserMessage` inspected only the trailing assistant, with a comment claiming an empty trailing message was "safe to send as-is". Add a provider-agnostic pre-send invariant, `ensureNonEmptyContent`, and make the two guards cooperate instead of fighting: - user -> BACKFILL a minimal non-empty text turn. Dropping it would end the request on an assistant, trading this 400 for the assistant-prefill 400 that #1703 fixed. - assistant -> DROP; it is residue, and the trailing-user guard that runs next re-establishes the prefill invariant. - tool -> leave untouched; `tool_result` blocks cannot be synthesized and injecting text would break tool_use/tool_result pairing. `normalizeContentArray` now backfills user/tool instead of blanking, and ordering is made explicit and documented: resolve empty content FIRST, then the trailing-assistant/prefill invariant. Tests assert both invariants together — no empty content AND the request still ends with user/tool — across anthropic, bedrock, openai-compatible and openai, plus an end-to-end case run through the real AI SDK prompt conversion that reproduces the captured wire payload.
…r text part
Inbox.drain persists renderInboxRow(row) verbatim as the only text part of a
synthetic role:"user" message, bypassing createUserMessage/hasSubstantiveContent.
renderInboxRow used `content.text ?? "(no notification body)"`, and `??` does not
catch "", so a body-less actor_notification rendered to exactly "". With one
queued row that is `parts: [{type:"text",text:""}]` — length 1, invisible to
every parts.length===0 guard — which ai@6.0.168's convertToLanguageModelMessage
filters to `content: []`, the shape a provider rejects with
"messages.<N>: user messages must have non-empty content".
Also restores the shell path's parity with the JSON path's content min(1):
shell-wrap routes a shell-parsed op straight to def.execute without
re-validating against `parameters`, so `actor send main ""` could queue the
body-less row in the first place.
…sts a blank part Consolidates the "empty content reaches the provider" family into one PR. Supersedes #1963, keeping its stronger version wherever the two overlapped: - src/inbox/render.ts: `blankTo()` helper, so any BLANK body (not merely a missing one) gets the placeholder. Replaces the bare `??` -> `||` change. - src/inbox/inbox.ts: the drain renders BEFORE writing and drops any row that renders blank; if every row renders blank it consumes them without writing a message at all. This is the structural invariant — the drain is the one user-message producer that bypasses createUserMessage/hasSubstantiveContent (src/session/prompt.ts:2164, :4386), so it needs its own guarantee. - src/tool/actor.ts: keeps the shell-boundary rejection, upgraded to `.trim()` so whitespace-only bodies are rejected too (zod's `min(1)` accepts " "). Also corrects a comment that asserted the opposite of the truth: a shell-parsed op IS re-validated against `parameters`. shell-wrap.ts calls `def.execute(parsed)` on the def from Tool.init, which is wrap()-decorated, and wrap() runs `parameters.parse(args)` inside execute. Verified end-to-end: with the parse-level guard removed, the real shellWrap route still enqueues nothing and reports "Too small: expected string to have >=1 characters -> at operation.content". A blank `actor send` was never reachable, so the render/drain work closes a LATENT defence gap rather than a live producer. New test test/inbox/empty-notification-reachability.test.ts drives that composition. The send-side backstop `ensureNonEmptyContent` (src/provider/transform.ts) stays: it remains the guard for the other message producers.
…t a bare string
THE producer of `messages.<N>: user messages must have non-empty content`.
`ensureTrailingUserMessage` appended `{ role: "user", content: "Continue." }`.
`ProviderTransform.message` is typed for `ModelMessage[]` (where `content: string`
is legal) but it does not run on `ModelMessage[]` — it runs inside the
`wrapLanguageModel` middleware on `args.params.prompt`, a `LanguageModelV3Prompt`,
whose user content must be `Array<TextPart | FilePart>`. The mismatch is silenced
by the `@ts-expect-error` at session/llm.ts (and session/prompt.ts:596).
@ai-sdk/anthropic then iterates that string CHARACTER BY CHARACTER
(`for (let j = 0; j < content.length; j++)` + `switch (part.type)` with cases for
only text/file and NO default, dist/index.mjs:2320-2408 on 3.0.82), pushes
nothing, and ships `{"role":"user","content":[]}`.
`ensureNonEmptyContent` cannot catch it: per its own ordering contract it runs
BEFORE this function, and "Continue." is not empty by any predicate, so the append
is never re-inspected.
test/provider/empty-content-wire.test.ts asserts on the REAL outbound HTTP body
(fetch injected into createAnthropic), not on toModelMessages output which is two
transformation layers short of the wire. A CONTROL case pins the defect mechanism:
the bare-string form ships as `content: []`.
Also in this family:
- src/provider/transform.ts: `normalizeMessages`'s empty-part strip was gated on
two literal npm names, so `@ai-sdk/google-vertex/anthropic`,
`@openrouter/ai-sdk-provider` and `@ai-sdk/openai-compatible` — all routes to an
API that rejects an empty block — got no protection. Extracted to
`stripsEmptyParts()` and extended. The AI SDK's own filter does not cover this:
its assistant branch KEEPS an empty text part carrying providerOptions, and it
never inspects `reasoning` parts at all.
- src/session/goal.ts: the goal judge is the ONE persisted-parts->provider site
with no `ProviderTransform.message` (`model: language` is raw, no
`wrapLanguageModel` in the file), so `ensureNonEmptyContent` is applied by hand.
- src/session/llm.ts: widen the middleware's `args.type === "stream"` narrowing to
`generate || stream`, matching prompt.ts:597. Latent today (the file's only SDK
entrypoint is `streamText`), but it would silently drop the entire transform.
user messages must have non-empty content)…l message normalizeContentArray's fallback covered both user and tool, so a tool message arriving with non-array content got a text part injected. That contradicts ensureNonEmptyContent's per-role policy, which deliberately leaves tool messages alone: injecting text into a tool message risks a tool_use/tool_result pairing mismatch, i.e. it trades one 400 for another, and content: [] is itself illegal for a tool result. The path is not reachable through normal construction, which is exactly why it is worth closing: this PR's headline defect was also an 'unreachable' shape mismatch -- a bare string where an array was mandatory, silenced by a @ts-expect-error, then iterated character by character by @ai-sdk/anthropic into content: []. A latent per-role inconsistency in the dimension this PR is about will mislead the next reader. Only user keeps the backfill; tool is returned untouched.
…t-backfilled Follow-up to 9af5a1e, which landed the tool-role gate itself. Two residual gaps. 1. The existing pin only rules out a TEXT part: expect(Array.isArray(tool!.content) && tool!.content.some((p) => p.type === "text")).toBe(false) Rewriting the tool branch to `content: []` keeps that assertion GREEN, yet `[]` is the other outcome the policy rejects — an empty tool content is itself illegal for providers that require the tool_result block, so it trades the pairing 400 for a different one. Verified by mutation: with the branch changed to `return { ...msg, content: [] }`, the existing test reports "1 pass, 0 fail" while the two tests added here fail. Assert the SAME reference instead (`toBe(content)`), across all three invalid shapes (undefined / null / object). 2. Pin the agreement itself. The finding was that two guards in this file disagreed about the tool role, so a test that exercises only one of them cannot catch the disagreement recurring. The added test asserts `ensureNonEmptyContent` and `message` reach the same outcome on the same input. Also corrects the comment block's opening claim, which still read as a blanket guarantee ("ensure msg.content is never a non-string non-array value ... that would blow up downstream `.map()` calls"). That is no longer true for `tool`, and leaving it stale would license downstream code to assume array content. Records that `[]` is not an acceptable substitute and why leaving the value untouched is what makes the two guards converge.
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
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.
One defect family, one PR: a message reaches the provider with
content: [], which Anthropic/Bedrock reject withmessages.<N>: user messages must have non-empty content.This PR now carries the whole family. It absorbs #1963 (closed in favour of this one — same two files, stronger version kept) and adds the remaining producers found while consolidating. Reviewable in four sections: the producer, the send-side backstop, the defence-in-depth, and one per-role inconsistency raised in review.
A — THE PRODUCER: the trailing continuation turn was a bare string
ProviderTransform.ensureTrailingUserMessageappends a minimal user turn whenever the conversation would otherwise end with a content-bearing assistant (a "prefill", which Bedrock hard-400s). It appended:ProviderTransform.messageis typed forModelMessage[], wherecontent: stringis perfectly legal. But it never runs on aModelMessage[]. It runs inside thewrapLanguageModelmiddleware, onargs.params.prompt— aLanguageModelV3Prompt, whose user content must beArray<TextPart | FilePart>. That mismatch is silenced by the// @ts-expect-erroratsession/llm.ts:671(andsession/prompt.ts:596), which is the root enabler.@ai-sdk/anthropicthen does this (dist/index.mjs:2320-2408, v3.0.82, read fromnode_modules):A string is iterated character by character. Each character's
.typeisundefined, no case matches, nothing is pushed — and the message ships as{"role":"user","content":[]}. Byte-for-byte the trailing message of the observed failing request.ensureNonEmptyContentcannot catch this. Per its own ordering contract it runs beforeensureTrailingUserMessage, and"Continue."is not empty by any predicate, so the append is never re-inspected.Fix:
content: [{ type: "text", text: CONTINUATION_PROMPT }](transform.ts:442). Audited the rest oftransform.tsfor the same shape::442was the only bare-string emission. Of the two other placeholder sites,:376was already correct;:622emitted an array but applied it to the wrong set of roles — see section D.Tested at the wire, not at
toModelMessagestest/provider/empty-content-wire.test.tsasserts on the real outbound HTTP body, captured by injectingfetchintocreateAnthropic. Asserting onMessageV2.toModelMessagesoutput would be two transformation layers short of the wire and would not have caught this. A CONTROL case pins the mechanism by feeding the bare-string form directly and asserting it ships ascontent: [].Revert probe (fix reverted to the bare string):
B — the send-side backstop, and the two seams it did not cover
ensureNonEmptyContent(transform.ts:364) is the global pre-send invariant: no message reaches the provider with empty content. Policy is deliberately asymmetric — user → backfill (dropping it would end the request on an assistant and trade this 400 for the prefill 400), assistant → drop (residue), tool → leave alone (we cannot synthesize a validtool-resultand injecting text would breaktool_use/tool_resultpairing).Revert probe (call removed from
message()): 460 pass / 3 fail —does not filter for non-anthropic providers,history ending in a content-bearing assistant + SDK-emptied user turn is repaired (@ai-sdk/openai),empty-string user content is backfilled rather than removed.Two seams it did not cover, both closed here:
B1 —
normalizeMessages' empty-part strip was gated on two literal npm package names. Extracted tostripsEmptyParts()and extended with@ai-sdk/google-vertex/anthropic,@openrouter/ai-sdk-providerand@ai-sdk/openai-compatible— all routes to an API that rejects an empty block inside an otherwise non-empty message. Gating protocol legality on a package name is the hole. The AI SDK's own filter does not save us: its user branch drops empty text parts, but its assistant branch keeps an empty text part that carriesproviderOptions, and it never inspectsreasoningparts at all.B2 —
src/session/goal.tsis the one persisted-parts→provider site with no transform at all.:197passesmodel: language— the raw model, nowrapLanguageModeland no middleware anywhere in that file — then:218callsgenerateObject. SoProviderTransform.messagenever runs for the goal judge, and none of its guards apply. Of the seventoModelMessagesEffectcallers, the other six are covered andcompaction.ts:133is estimate-only (never sent).ensureNonEmptyContentis now applied by hand at:161.Also widened
session/llm.ts:670fromargs.type === "stream"togenerate || stream, matchingprompt.ts:597. Latent today — this file's only SDK entrypoint isstreamTextat:599— but the narrowing would silently drop the entire transform the moment a non-streaming call is added.C — defence in depth: the inbox drain (absorbs #1963)
Inbox.drainis the one user-message producer that does not go throughSessionPrompt.createUserMessage, sohasSubstantiveContent(prompt.ts:2164,:4386) never inspects it. It wrote one syntheticrole:"user"message and one text part per queued row withtext: renderInboxRow(row)persisted verbatim.renderInboxRowusedcontent.text ?? placeholder, and??does not catch""— so a body-lessactor_notification(the one row type passed through RAW;type:"text"rows are structurally non-empty because of the<inbox>wrapper) would render to exactly"", givingparts: [{type:"text",text:""}]— length 1, so everyparts.length === 0guard misses it.Two layers, both from #1963 (they supersede the bare
??→||this PR previously carried):render.ts: ablankTo()helper, so any blank body (not merely a missing one) gets the placeholder, applied to both theactor_notificationand the<inbox>branch.inbox.ts:268-277: the drain renders before writing and drops any row that renders blank; if every row renders blank it consumes them without writing a message at all. This is the structural invariant — it holds no matter what a future row type renders.Reachability, stated honestly. This closes a defence gap that no observed transcript exercised: a 5.6 GB production store contains zero user text parts with
text = ''. It is not described as an observed producer — section A is the observed producer. Note a genuine disagreement among the investigations: whether a blank body was reachable at all through theactortool's shell path. Driving the real compositionshellWrap(Tool.init(actor)).execute({ script: 'actor send <id> ""' })with the parse-level guard removed still enqueues nothing and reportsToo small: expected string to have >=1 characters → at operation.content, i.e.content: z.string().min(1)does fire — becauseshell-wrap.ts:121callsdef.execute(parsed)on the def fromTool.init, which iswrap()-decorated, andwrap()runsparameters.parse(args)insideexecute. Probes that stop atparseActorScriptcannot see this and report the opposite. Either way the fix is warranted and the zero-row census is consistent with it never having fired.Accordingly,
src/tool/actor.ts's comment — which asserted that a shell-parsed op is "NEVER re-validated againstparameters" — is corrected: it stated the opposite of the truth, and contradictedshell-wrap.ts:43in the very module it described. The parse-level guard is kept (upgraded to.trim(), so whitespace-only bodies are rejected too, whichmin(1)accepts) for two honest reasons: it turns a generic zod dump into one specific teachable message, and it would be the only guard left if the schema were relaxed.D —
normalizeContentArraycontradicted the per-role policy fortoolRaised in review on this PR and fixed here.
normalizeContentArray(transform.ts:615) is the crash guard that replaces a non-string non-arraycontent(object /undefined/null) before anything calls.map()on it. Its fallback covered bothuserandtool:So a
toolmessage arriving with non-array content got a text part injected — exactly whatensureNonEmptyContentrefuses to do to tool messages (section B), because it risks atool_use/tool_resultpairing mismatch, i.e. it trades this 400 for another one. Two guards in the same file disagreed on the same role.toolis now returned untouched; onlyuserkeeps the backfill. Untouched rather thancontent: [], because an emptytool_resultis itself illegal — "leave tool alone" is the policy both guards now share.Reachability is effectively nil — tool messages are always constructed with array content — and that is precisely why it was worth closing rather than noting. This PR's headline defect (section A) was also an "unreachable" shape mismatch: a bare string where an array was mandatory, silenced by a
@ts-expect-error, then iterated character by character intocontent: []. A latent per-role inconsistency in the exact dimension this PR is about will mislead the next reader.Revert probe (tool falling back into the
userbranch again):test/provider/transform.test.ts→ 240 pass / 0 fail. A follow-up commit strengthens the assertion from "was not text-backfilled" to "content is untouched", which is the actual invariant.Explicitly out of scope (follow-ups, not padded in here)
These were flagged as verbatim part-copiers in the same family, but I could not construct a deterministic reachable case for any of them, and all six flow through
ProviderTransform.messageon send so the backstop covers them. Listing rather than shipping speculative fixes:session/compaction.ts:332—compacting.prompt ?? …misses""session/compaction.ts:421(replay),session/session.ts:669(fork)session/import.ts:186,session/opencode-import.ts:232,session/json-migration.ts:294Also deferred: making
goal.tsrun the fullProviderTransform.message(image limits, cache markers, provider-option remapping) rather than only the emptiness invariant — a behaviour change outside this defect family.Verification
e080a8394d— this branch, pristine, pre-foldf30b4c84f7— #1963, pristineIdentical command for all three:
bun test test/provider test/inbox test/tool/actor.shell.test.ts test/session/goal.test.ts test/session/message-v2.test.ts --timeout 120000, run frompackages/opencode.+3 filesis exactly the three new test files. Every test from both folded sides survives.bun typecheckexit 0.Revert probe per folded fix (a fold that silently neutralises an assertion is the real risk here):
ensureNonEmptyContentremoved frommessage()inbox.ts+render.ts→origin/maindrain-no-empty-part.test.tsfrom #1963,empty-notification-part.test.tsfrom this branch) — neither side was neutralisedshellWrap-route case still passed, which is what shows zod is the load-bearing layernormalizeContentArraytool fallback restoredExpected: false / Received: true(verbatim in section D)One honest note on the second probe: it used to produce 6 failures. It now produces 3 because the section-A producer fix independently repairs the cases the backstop was previously the only thing catching. The three that remain are the ones only the backstop can catch, and the wire case is now carried by its own dedicated test.
Follow-up (
299b4aa92) — tool-role pin strengthened + reviewer's structural claims verifiedTwo commits landed after
1fba3205b:9af5a1e61— the reviewer's non-blocking finding:normalizeContentArray's fallback covered bothuserandtool, so a tool message with non-array content got a text part injected — contradictingensureNonEmptyContent's per-role policy (transform.ts:341-360), which deliberately leavestoolalone because injecting text risks atool_use/tool_resultpairing mismatch. Gated onmsg.role === "user";toolreturns untouched.299b4aa92— strengthens that pin and repairs a stale comment.Chosen tool-role policy: leave the message EXACTLY as-is (not
content: []).[]is illegal for providers that require thetool_resultblock, so it would trade the pairing 400 for a different one — there is no value this layer can honestly synthesize for a tool message. Leaving it untouched also makes both guards converge on the same input:hasNoSendableContentreturnstruefor non-array content (transform.ts:334) andensureNonEmptyContent's tool branch re-pushes it unchanged.Why fix an unreachable path at all: this PR's headline defect was an "unreachable" shape mismatch — a bare string where an array was mandatory, silenced by a
@ts-expect-error, then iterated character-by-character by@ai-sdk/anthropicintocontent: []. A latent per-role inconsistency in exactly this dimension will mislead the next reader.299b4aa92closes two residual gaps in9af5a1e61:content: []kept it green. Mutation-verified: withreturn { ...msg, content: [] }, the original test reports1 pass, 0 failwhile both new tests fail. The new pin asserts the same reference (toBe(content)) acrossundefined/null/ object.ensureNonEmptyContentandmessagereach the same outcome on the same input.Also corrected the comment block's opening claim, which still read as a blanket guarantee ("ensure
msg.contentis never a non-string non-array value … that would blow up downstream.map()calls"). That is no longer true fortool, and leaving it stale would license downstream code to assume array content.Revert probe (gate removed, tests kept) — verbatim:
Follow-up
299b4aa92— the pin was under-specified, and one comment was stale9af5a1e61(the gate itself) is correct and unchanged. Two residual gaps were closed on top of it.The original assertion did not pin the policy. It asserted
Array.isArray(content) && content.some(p => p.type === "text") === false— which is also green when tool content is rewritten to[], i.e. the other outcome this policy rejects. Mutation evidence: changing the branch toreturn { ...msg, content: [] }leaves that test at1 pass / 0 fail, while both replacement pins fail. The pins now assert reference identity (toBe(content)) acrossundefined/null/ object, plus a second test thatnormalizeContentArrayandensureNonEmptyContentreach the same outcome on the same non-array tool message (hasNoSendableContentreturnstruefor non-array content attransform.ts:334, and the tool branch re-pushes unchanged) — a testable consistency property rather than an aesthetic preference."Exactly as-is" is load-bearing, and
[]is not a substitute. An emptytool_resultblock is itself illegal for providers that require it, socontent: []only relocates the 400.Acknowledged tradeoff, now stated in the comment instead of overstated. Exempting
toolnarrows this function's crash-guard guarantee:.map()safety no longer holds for tool messages, so downstream code must not assume array content. An attributableTypeErroron an unrepairable shape is better than a pairing 400 that points away from the cause. The header comment was corrected from a blanket guarantee toNOT a blanket guarantee — tool is deliberately exempt.Verdicts on the review's three structural claims (it did not run the suite)
All three CONFIRMED from the code at head.
(a) Pipeline order is correct — CONFIRMED.
message()runsnormalizeContentArray(transform.ts:951) → … →ensureNonEmptyContent(:977) →ensureTrailingUserMessage(:981), and the ordering contract at:404-412states why the reverse is unsafe:(b) The
DELETEconsumes ALL rows including dropped blanks — CONFIRMED. Both delete sites key offrows, the full selected set, never the filteredrendered: the all-blank early return atinbox/inbox.ts:277-287and the normal path at:316-323both useso a partially-blank batch still consumes the blanks it dropped. The all-blank comment states the intent explicitly (
:273-276):they carry no information and must not be re-drained forever. Nothing re-drains forever.(c) Dropping a mid-history empty assistant is safe — CONFIRMED, both halves.
sdkVisibleAssistantParts(transform.ts:324-328): the predicate is!part || part.type !== "text" || part.text !== "" || part.providerOptions != null, and atool-callpart satisfiespart.type !== "text". So an assistant carrying a tool call is never classified empty byhasNoSendableContent(:337) and is therefore never dropped — pairing is preserved by construction, not by luck.@ai-sdk/anthropic@3.0.82groupIntoBlocks(dist/index.mjs:3057-3102) opens a new block only when the role differs fromcurrentBlock.type, and itscase "tool"appends into auserblock:if (currentBlock?.type !== "user") { currentBlock = { type: "user", messages: [] }; blocks.push(currentBlock) }. Adjacentuser/userortool/usertherefore collapse into one block — no alternation violation.Verification (this follow-up)
Scoped to
test/provider/+test/inbox/only, frompackages/opencode, as the full suite starves under concurrent sibling runs.1fba3205b, before any change299b4aa92test/provider/transform.test.tsalone, unloadedEvery failure in both runs is a bare
timed out after 5000ms— zero assertion failures on either side; the failing sets are not even the same between repeat runs of the identical command (one intermediate run showed 19, all timeouts). Total count rises 464 → 466 (the two added tests).bun typecheckexit 0.