fix: stop empty-payload images from poisoning sessions - #2249
fix: stop empty-payload images from poisoning sessions#2249nothankyouzzz wants to merge 3 commits into
Conversation
🦋 Changeset detectedLatest commit: aeacd8d 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aa88cd68ef
ℹ️ 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".
| // The image format gate runs at projection time too (not only at ingestion): | ||
| // a malformed/undeliverable image already persisted in an old session's | ||
| // history is replaced by a text notice on the wire, so it can no longer | ||
| // poison every later request. Read-side only — the history keeps its parts. |
There was a problem hiding this comment.
Move implementation narration to the module header
This newly added statement-level comment violates the scoped agent-core-v2 convention that comments live solely in the top-of-file block and never beside functions, methods, or statements. Remove it or fold only the module’s external responsibility into the header; the new inline block in llmRequesterService.ts and function-level JSDoc in image-format-policy.ts require the same cleanup.
AGENTS.md reference: packages/agent-core-v2/AGENTS.md:L11-L15
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in ab920e7 — the inline comments added beside functions/statements were removed and their external-role content folded into the module headers, per the package comment conventions.
| image_url: | ||
| '[image omitted for provider compatibility; re-read the file to view it or get conversion guidance]', | ||
| '[An image attached to this message was removed before sending because the provider could not accept it (unsupported or unreadable image data). You have NOT seen this image — do not describe or guess its contents. Tell the user the image failed to reach you and suggest re-sending it as PNG or JPEG.]', |
There was a problem hiding this comment.
Keep the stripped-media notice cause-neutral
When a request remains too large after the media-degraded retry, AgentLLMRequesterService.requestWithRecovery switches to this same media-stripped projection. In that path the images can be valid PNG/JPEG files, but this notice tells the model they were unsupported or unreadable and directs the user to resend them as PNG/JPEG, which misdiagnoses the failure and offers a recovery that may fail again. Use a cause-neutral notice or select distinct placeholders for format rejection versus request-size recovery.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in ab920e7 — the stripped-media notice is now cause-neutral ("the provider rejected the request (unsupported or unreadable image data, or the request exceeded its size limit)") with a size-appropriate recovery hint, mirrored in both engines and their tests.
ab920e7 to
a7c94d2
Compare
…xtProjector/contextProjectorService.ts
…quester/llmRequesterService.ts
…/image-compress.ts
…/image-format-policy.ts
…extProjector/projector-tool-exchanges.test.ts
…a/image-compress.test.ts
…image-compress.ts
…image-format-policy.ts
…/projector.test.ts
d815bc8 to
e30bd58
Compare
e30bd58 to
d7b404e
Compare
An empty image payload (`data:image/png;base64,`) — produced when a clipboard/upload failure captures zero bytes — passed every ingestion check, entered the session history, and made the provider reject every later request. Each rejection then stripped ALL media in context, so pasted images kept vanishing for the rest of the session. - reject empty payloads at the image format gate (moved to image-format-policy) and replace them with an explicit notice - run the gate at projection time too, so histories already carrying a bad image heal on the next request (read-side only) - rewrite the stripped/degraded media placeholders to state the removal, its reason, and that the model must not guess the contents - log the rejection's status/message when the strip recovery fires
A zero-byte image — a clipboard/upload failure handing the client an empty payload — submitted inline would enter the session history as `data:image/...;base64,` and poison every later request; submitting the prompt without the image the user meant to attach also silently wastes the turn. Reject both shapes in the fail-fast file-reference check before anything session-scoped happens (agent creation, control overrides, prompt enqueue): inline base64 that decodes to nothing, and zero-byte uploaded image files. The client keeps the draft and can re-paste.
d7b404e to
aeacd8d
Compare
Related Issue
Resolve #2209
Problem
See linked issue. In short: a zero-byte image from a failed clipboard read entered the session history as
data:image/png;base64,; the provider rejected every later request containing it, and the recovery path stripped ALL media in context — so pasted images kept vanishing for the rest of the session, and the ambiguous placeholder let the model hallucinate having seen them.What changed
Layered fix so an undeliverable image can neither enter nor keep poisoning a session:
media_strippedtelemetry event.Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.