fix: stop empty-payload images from poisoning sessions - #2249
fix: stop empty-payload images from poisoning sessions#2249nothankyouzzz wants to merge 6 commits into
Conversation
🦋 Changeset detectedLatest commit: a7c94d2 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 |
![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: 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.
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.
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.
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 and add a media_stripped telemetry event when the strip recovery fires
A zero-byte uploaded image was inlined as `data:image/png;base64,` and poisoned every later request in the session. Replace empty uploads (and empty inline payloads) with a notice naming the file instead.
A failed clipboard read can hand the browser a named, typed, zero-byte File. Uploading it attached an empty image that the provider then rejected on every request. Show a failed chip with the reason instead, and log clipboard diagnostics when a paste yields zero bytes.
Keep the legacy engine in sync with agent-core-v2: empty data-URL payloads are rejected at the format gate at ingestion and projection time, and the stripped/degraded media placeholders state the removal explicitly.
- keep the stripped-media notice cause-neutral: the same projection also recovers a request that stayed too large after media degradation, where the images are valid but oversized - move comments added beside functions/statements into the module headers, per the package comment conventions
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
1 participant
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.