Skip to content

fix(inbox): stop rendering a body-less notification into an empty user text part - #1961

Closed
wqymi wants to merge 1 commit into
mainfrom
fix/inbox-empty-notification-part
Closed

fix(inbox): stop rendering a body-less notification into an empty user text part#1961
wqymi wants to merge 1 commit into
mainfrom
fix/inbox-empty-notification-part

Conversation

@wqymi

@wqymi wqymi commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

The producer of the empty user content

A live Bedrock 400 (messages.<N>: user messages must have non-empty content) has been traced through the SDK before: ai@6.0.168's convertToLanguageModelMessage user branch drops empty text parts with no backfill (dist/index.mjs:1424, .filter((part) => part.type !== "text" || part.text !== "")), so a user message whose content was [{type:"text",text:""}] reaches the provider as content: []. What was never identified is who produces that empty text part. This is one such producer, end to end and reachable:

  1. src/inbox/render.ts:8return content.text ?? "(no notification body)". The ?? only catches null/undefined; an empty body passes straight through as "". The placeholder was clearly meant to cover exactly this case.
  2. src/inbox/inbox.ts:275drain() persists text: renderInboxRow(row) verbatim, one part per queued row, via sessions.updateMessage + sessions.updatePart. This bypasses createUserMessage and its hasSubstantiveContent guard entirely, and Session.updatePart validates nothing.
  3. With a single queued row the message's parts are exactly [{type:"text",text:""}]length 1, so every parts.length === 0 guard misses it, including message-v2.ts:922.
  4. src/inbox/inbox.ts:162Inbox.send stores content: { text: input.content } with no emptiness check, and the reachable trigger is actor send main "" --type actor_notification: the JSON path declares content: z.string().min(1) (src/tool/actor.ts:454) but the shell path takes content: rest[1] verbatim (:185) and shell-wrap.ts:121 calls def.execute(parsed, ...) without re-validating against parameters.

This also explains why a post-hoc DB sweep for "a user message with an exactly-empty text part" found nothing useful: the row that carries it is a synthetic drain part, and the queued inbox row that caused it is DELETEd in the same drain.

Fix

  • src/inbox/render.ts|| instead of ?? for both row types, so a body-less row always renders its placeholder. This makes "the text persisted by drain() is never empty" an invariant of the renderer.
  • src/tool/actor.ts — reject an empty content token in the shell send verb, restoring parity with the JSON path's min(1) that shell-wrap bypasses. The model gets a loud, self-correctable error instead of silently delivering a placeholder notification.

Tests

  • test/inbox/empty-notification-part.test.ts (new) — renderInboxRow never returns an empty string for any row type x body combination, and an end-to-end Inbox.send("") -> drain() writes a non-empty synthetic part (plus a mixed empty/real batch leaving no empty part behind).
  • test/session/message-v2.test.ts — mechanism pin: an empty-text-only user message survives our layer at parts.length === 1 (content: [{type:"text",text:""}]) and only collapses to content: [] inside the SDK step that runs between us and the provider (convertToLanguageModelPrompt from ai/internal). Sits next to the existing "filters out messages with no parts" test, which is the complementary proof that a zero-part user message is dropped by us and can never reach a provider.
  • test/tool/actor.shell.test.tsactor send main "" is rejected.

Every test above was revert-probed: with render.ts restored to ??, all 5 tests in the new file fail, the drain one with Expected: "(no notification body)" / Received: ""; with the actor.ts guard removed the shell test fails Expected: "Failure" / Received: "Success".

Note

This is a producer fix, not a replacement for a pre-send invariant. The ensureNonEmptyContent backstop proposed in #1948 is still wanted as defence in depth — this PR does not touch it, and other producers of the same shape may exist (unvalidated PATCH .../part/:partID, the import/migration paths, Session.fork, and compaction replay all copy part text without an emptiness check).

…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.
@wqymi wqymi changed the title probe fix(inbox): stop rendering a body-less notification into an empty user text part Jul 28, 2026
@wqymi

wqymi commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator Author

Folded into #1948 (fix/no-empty-content-invariant) and closing here.

Rationale: this PR identified the producer of the empty user text part; #1948 adds the pre-send invariant that backstops it. They are the root cause and the backstop of one defect (Bedrock 400 messages.<N>: user messages must have non-empty content), so they belong in one review — and #1948's body previously claimed the producer "was never identified", which only this PR's finding could correct.

Commit 7c2a31b50 was cherry-picked verbatim onto #1948's branch (now e080a8394d); nothing was dropped — src/inbox/render.ts, src/tool/actor.ts, test/inbox/empty-notification-part.test.ts, and the test/session/message-v2.test.ts / test/tool/actor.shell.test.ts additions are all present there. #1948's body now presents both layers, and ensureNonEmptyContent is deliberately kept as defence in depth.

Post-fold on #1948: test/provider/ + test/inbox/ + test/session/message-v2 + test/tool/actor.shell = 511 pass / 0 fail (pristine #1948 baseline: 504 pass / 0 fail), bun typecheck exit 0. This PR's revert probe was re-run after the fold and still bites: restoring ?? in render.ts gives 0 pass / 5 fail with Expected: "(no notification body)" / Received: "".

Branch fix/inbox-empty-notification-part is left in place (not deleted).

@wqymi wqymi closed this Jul 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant