Skip to content

fix(image-generation): bound OpenAI-compatible image API response reads#96261

Closed
cxbAsDev wants to merge 2 commits into
openclaw:mainfrom
cxbAsDev:fix/bound-image-gen-response
Closed

fix(image-generation): bound OpenAI-compatible image API response reads#96261
cxbAsDev wants to merge 2 commits into
openclaw:mainfrom
cxbAsDev:fix/bound-image-gen-response

Conversation

@cxbAsDev

@cxbAsDev cxbAsDev commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Summary

Replace custom readResponseWithLimit + JSON.parse with the shared readProviderJsonResponse from provider-http-errors.ts in the OpenAI-compatible image generation provider. Update test mock response objects with arrayBuffer so the bounded read path works in tests.

Real behavior proof (required for external PRs)

Behavior addressed: Custom bounded read replaced with shared readProviderJsonResponse. Test mocks updated with arrayBuffer support.

Real environment tested: Linux, Node 24, OpenClaw main @ 56d95b1

Exact steps or command run after fix:

node scripts/run-vitest.mjs run src/image-generation/openai-compatible-image-provider.test.ts

Evidence after fix:

Test Files  1 passed (1)
     Tests  5 passed (5)

All 5 tests pass including:

  • generates images via JSON request body — normal JSON path
  • generates images via multipart request body — multipart path
  • honors default operation timeouts and empty-response errors — edge case
  • wraps malformed successful image responses with provider-owned errors — error wrapping preserved

Mock responses now include arrayBuffer alongside json so readProviderJsonResponse can read the body through readResponseWithLimit.

Observed result after fix: Image API response reads use the shared readProviderJsonResponse with the standard 16 MB cap. Error wrapping is preserved through the provider-owned failure label.

What was not tested: A live image generation API call against a real endpoint was not tested.

Tests and validation

  • openai-compatible-image-provider.test.ts: 5/5 passed ✅

Risk

Low. Uses shared readProviderJsonResponse which is the standard bounded reader for all provider JSON responses. No custom bounded-read implementation remains.

Replace raw response.json() with readResponseWithLimit to prevent
unbounded buffering of image generation/edit API responses.

Co-Authored-By: Claude <[email protected]>
@clawsweeper

clawsweeper Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Thanks for the context here. I did a careful shell check against current main, and this is already implemented.

Current main already contains a stronger implementation from the merged canonical image-generation bounded-read PR, so this conflicting narrower branch is no longer a useful landing candidate.

Root-cause cluster
Relationship: superseded
Canonical: #96495
Summary: The current PR is superseded by a merged canonical PR that implements the same shared provider bounded-read work with broader image-provider coverage and safer cap sizing.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

So I’m closing this as already implemented rather than keeping a duplicate issue open.

Review details

Best possible solution:

Keep the current main implementation from #96495 and close this narrower branch rather than merging duplicate bounded-read changes.

Do we have a high-confidence way to reproduce the issue?

Yes, at source and history level. v2026.6.10 still used raw response.json() for image success bodies, while current main now routes the same path through bounded JSON reads with image-sized cap coverage.

Is this the best way to solve the issue?

No for this branch as a landing path. The current main implementation is the better fix because it uses the shared bounded reader with image-sized caps and broader sibling-provider coverage.

Security review:

Security review cleared: The diff narrows provider-response buffering exposure and does not add dependencies, workflows, lockfiles, permissions, secrets handling, package metadata, or downloaded-code execution paths.

AGENTS.md: found and applied where relevant.

What I checked:

Likely related people:

  • steipete: The shared OpenAI-compatible image provider and its tests were introduced in commit 719ec4f. (role: feature introducer and adjacent image-generation owner; confidence: high; commits: 719ec4f2927a; files: src/image-generation/openai-compatible-image-provider.ts, src/image-generation/openai-compatible-image-provider.test.ts, src/plugin-sdk/image-generation.ts)
  • sallyom: Commit 9490c8a added the image-sized JSON cap follow-up in the canonical merged PR and the live discussion accepted that cap sizing for overlapping PRs. (role: canonical fix contributor and reviewer; confidence: high; commits: 9490c8a8c626; files: src/image-generation/openai-compatible-image-provider.ts, src/image-generation/image-assets.ts, extensions/openai/image-generation-provider.ts)
  • hugenshen: Merged PR fix(image-gen): bound image generation provider JSON response reads #96495 was authored by hugenshen and became the current main implementation for this image-generation response-read hardening. (role: canonical PR author; confidence: high; commits: 527f8f0cbba1; files: src/image-generation/openai-compatible-image-provider.ts, extensions/openai/image-generation-provider.ts, extensions/microsoft-foundry/image-generation-provider.ts)
  • Alix-007: Merged commit 2592f8a introduced readProviderJsonResponse, the helper used by both this PR and the current main implementation. (role: shared bounded-reader contributor; confidence: high; commits: 2592f8a51a4e; files: src/agents/provider-http-errors.ts, src/agents/provider-http-errors.test.ts)
  • vincentkoc: Commit eb7a082 recently hardened image response schema handling in the same parser and shared provider area. (role: recent image response hardening contributor; confidence: medium; commits: eb7a082b777d; files: src/image-generation/image-assets.ts, src/image-generation/image-assets.test.ts, src/image-generation/openai-compatible-image-provider.ts)

Codex review notes: model internal, reasoning high; reviewed against cee2aca40912; fix evidence: commit 527f8f0cbba1, main fix timestamp 2026-06-26T11:08:30Z.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. labels Jun 24, 2026
@cxbAsDev

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@cxbAsDev

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

…fixes

Replace custom readResponseWithLimit + JSON.parse with the shared
readProviderJsonResponse from provider-http-errors, and add
arrayBuffer to all image gen test mock response objects so
the bounded read path works in tests.

Co-Authored-By: Claude <[email protected]>
@cxbAsDev

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 27, 2026
@clawsweeper

clawsweeper Bot commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper applied the proposed close for this PR.

@clawsweeper clawsweeper Bot closed this Jun 27, 2026
@cxbAsDev
cxbAsDev deleted the fix/bound-image-gen-response branch July 15, 2026 03:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. P2 Normal backlog priority with limited blast radius. rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. size: XS status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant