fix: sanitize OpenAI compat gateway images and surface streaming errors#62070
fix: sanitize OpenAI compat gateway images and surface streaming errors#62070htplbc wants to merge 5 commits into
Conversation
…ompat endpoint Gateway-provided images (via OpenAI compat /v1/chat/completions) bypassed the sanitization/resize pipeline because detectAndLoadPromptImages() early- returned existingImages unsanitized when no image refs were found in prompt text. Large images (e.g. Mac Retina screenshots) hit Anthropic's 5MB limit. Additionally, in streaming mode, when a lifecycle "error" event fired before the async catch block, the stream was closed with only [DONE] and no error content — silently swallowing the failure. Fixes openclaw#59913 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Greptile SummaryFixes two bugs: gateway-provided images now run through Confidence Score: 5/5Safe to merge — both fixes are correct and the only open item is a minor test-reliability suggestion. No P0 or P1 issues found. The image sanitization fix passes the same limits as the non-early-return path (only maxDimensionPx; maxBytes is also omitted in the existing path). The streaming error fix correctly relies on the existing closed flag and the if (!closed) guard in the finally block to prevent double-writes and double-closes. Single P2 comment is about replacing a wall-clock sleep with Promise.resolve() for more reliable event-loop ordering in the new test. src/gateway/openai-http.test.ts — consider replacing the 50 ms setTimeout with await Promise.resolve() in the new lifecycle-error streaming test. Prompt To Fix All With AIThis is a comment left during a code review.
Path: src/gateway/openai-http.test.ts
Line: 861-862
Comment:
**Timing-dependent test synchronization**
The 50 ms `setTimeout` races against event-loop scheduling — slow CI could let the lifecycle error handler fire *after* the `await`, making this test intermittently miss the fix. `emitAgentEvent` is synchronous and its listeners run in the same tick, so `await Promise.resolve()` is sufficient to yield without a wall-clock delay.
```suggestion
// Yield the microtask queue so the synchronous event handler runs before we return
await Promise.resolve();
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix: sanitize gateway images and surface..." | Re-trigger Greptile |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7e29768fa0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…or path Set sawAssistantDelta before emitting the lifecycle error event in the catch block, so the lifecycle handler skips writing a second error chunk. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
|
This pull request has been automatically marked as stale due to inactivity. |
|
Codex review: needs real behavior proof before merge. Reviewed June 6, 2026, 12:45 AM ET / 04:45 UTC. Summary PR surface: Source +16, Tests +51. Total +67 across 4 files. Reproducibility: unclear. The review failed before ClawSweeper could establish a reproduction path. Review metrics: none identified. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Risk before merge
Maintainer options:
Next step before merge
Review detailsBest possible solution: Retry the Codex review after fixing the execution failure. Do we have a high-confidence way to reproduce the issue? Unclear. The review failed before ClawSweeper could establish a reproduction path. Is this the best way to solve the issue? Unclear. Retry the review first so ClawSweeper can evaluate the actual issue and fix direction. AGENTS.md: unclear because the file could not be read completely. Codex review notes: model gpt-5.5, reasoning high; reviewed against 9313471fa579. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +16, Tests +51. Total +67 across 4 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
|
ClawSweeper PR egg 🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat. Where did the egg go?
|
|
This pull request has been automatically marked as stale due to inactivity. |
Summary
Fixes #59913
/v1/chat/completionsimage_urlparts) bypassed the sanitization/resize pipeline.detectAndLoadPromptImages()early-returnedexistingImagesunsanitized when no image refs were found in the prompt text. Large images (e.g. Mac Retina screenshots >5MB) were forwarded to Anthropic at full resolution, hitting the 5MB per-image limit.stream: true), when a lifecycle"error"event fired before the async catch block could run, the stream was closed with onlydata: [DONE]— the error was completely swallowed and the client received no error content.Changes
src/agents/pi-embedded-runner/run/images.ts: ApplysanitizeImagesWithLog()toexistingImageson the early-return path (no prompt image refs detected)src/gateway/openai-http.ts: Write error content chunk in the lifecycle"error"handler before closing the stream, when no assistant content was already sentTest plan
existingImagesare sanitized even when prompt has no image references (images.test.ts)openai-http.test.ts)AI-assisted: Built with Claude Code. Fully tested locally. I understand what the code does.
🤖 Generated with Claude Code