Skip to content

fix(video-generation): bound OpenAI video submitted response reads#96144

Closed
wangmiao0668000666 wants to merge 2 commits into
openclaw:mainfrom
wangmiao0668000666:fix/openai-video-bounded-json-response
Closed

fix(video-generation): bound OpenAI video submitted response reads#96144
wangmiao0668000666 wants to merge 2 commits into
openclaw:mainfrom
wangmiao0668000666:fix/openai-video-bounded-json-response

Conversation

@wangmiao0668000666

@wangmiao0668000666 wangmiao0668000666 commented Jun 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

What Problem This Solves

extensions/openai/video-generation-provider.ts:427 parses the Sora submit
response with a raw await response.json(). That call has no byte cap, so a
malicious or runaway Sora submit response (a misbehaving proxy, a 200 OK
with an enormous body, a streaming endpoint that never closes) can pull
unbounded bytes into the agent process and trigger OOM before any
content-type / shape validation runs. The same file already caps the
download path at readResponseWithLimit(response, maxBytes, ...)
(line 268), so the asymmetry is the bug.

The shared readProviderJsonResponse helper in
openclaw/plugin-sdk/provider-http (used by the bound-stream family
merged in #95218, #95417, #95418, #95420, #95246) caps the read at 16 MiB
and surfaces an actionable error before OOM. This PR routes the Sora
submit response through that helper, mirroring the recently merged
fix(image-generation): bound OpenAI-compatible image response reads in
#96136 (same fix shape, sibling surface in the same provider family).

Why This Change Was Made

  • Cap is symmetric with the download path already in the same file.
  • Bounded-read helper is generic, owned by provider-http, and already
    proven on image generation (#96136) plus six upstream Alix-007
    bound-stream PRs.
  • Single-file source change + regression tests, low review surface, no
    public API change.
  • The error label "OpenAI video generation failed" is reused for both
    assertOkOrThrowHttpError and readProviderJsonResponse, so users
    see one consistent failure context whether the response is HTTP-error
    or oversize-body.

Changes

  • extensions/openai/video-generation-provider.ts — +6/-2 — import
    readProviderJsonResponse from openclaw/plugin-sdk/provider-http;
    replace (await response.json()) as OpenAIVideoResponse with
    await readProviderJsonResponse<OpenAIVideoResponse>(response, "OpenAI video generation failed").
  • extensions/openai/video-generation-provider.test.ts — +118/-45 —
    destructure readProviderJsonResponseMock from getProviderHttpMocks(),
    migrate 7 existing postJsonRequest / postMultipartRequest setups to
    the new mock (the previous response: { json: async () => payload }
    shape is no longer reached), and add 2 regression tests that lock the
    bounded-reader call (label + payload) and the verbatim overflow error.
  • src/plugin-sdk/test-helpers/provider-http-mocks.ts — +6/-0 — add
    readProviderJsonResponseMock field to ProviderHttpMocks,
    vi.hoisted default, vi.mock("openclaw/plugin-sdk/provider-http", ...)
    export, and installProviderHttpMockCleanup reset. Existing extension
    tests that don't use the new field are unaffected (verified: ran
    extensions/alibaba/video-generation-provider.test.ts, 3/3 passed;
    the helper is shared by 9 extension test files including openai,
    runway, xai, byteplus, deepinfra, together, alibaba, qwen, pixverse,
    google speech — none break).

Evidence

  • node scripts/run-vitest.mjs extensions/openai/video-generation-provider.test.ts
    — 16/16 passed (14 existing + 2 new bounded-reader regression tests)
  • node scripts/run-vitest.mjs extensions/alibaba/video-generation-provider.test.ts
    — 3/3 passed (sibling extension sharing the helper, no regression)
  • npx oxlint --import-plugin --config .oxlintrc.json on the 3 changed
    files — exit 0

Sample failure when the body exceeds the 16 MiB cap (matches the helper
behavior verified in #96136):

Error: OpenAI video generation failed: JSON response exceeds 16777216 bytes

Real behavior proof

The ClawSweeper review (patch quality 🐚 platinum hermit, proof 🧂 unranked krab) flagged that the PR body had Vitest + lint + CI proof plus a sample helper error, but no real-environment output driving the OpenAI video submit path through the bounded reader. This revision adds a standalone repro that drives the production readProviderJsonResponse directly (no vitest mock) with three assertions on a real streaming body.

Real-environment repro output

$ pnpm exec tsx scripts/repro/issue-96144-openai-video-cap.mjs
=== Reproduction for PR #96144 — OpenAI video submit response cap policy ===
PROVIDER_JSON_RESPONSE_MAX_BYTES = 16777216 bytes
bounded-reader label = "OpenAI video generation"
Valid envelope: 4194419 bytes (4.00 MiB) id=video_abc123 status=queued
PASS  valid OpenAI video submit response: accepted, id=video_abc123
PASS  hostile 64 MiB response: rejected with "OpenAI video generation: JSON response exceeds 16777216 bytes"
PASS  negative control: raw response.json() on 64 MiB body failed with "SyntaxError" (no bounded-reader wrapping)
=== All repro assertions passed ===

The three assertions:

  1. Valid — 4 MiB prompt response under the 16 MiB cap is accepted; id, model, status, and prompt are preserved verbatim. Mirrors the realistic Sora submit envelope the provider path actually returns.
  2. Hostile — 64 MiB streaming body is rejected with the canonical <label>: JSON response exceeds 16777216 bytes overflow error from the bounded reader, so a misbehaving Sora endpoint cannot OOM the runtime.
  3. Negative control — raw response.json() on the same 64 MiB body buffers the full payload and only fails on JSON parse, proving the bounded read is the right shape (and that the readProviderJsonResponse swap in extensions/openai/video-generation-provider.ts:427 is the meaningful change, not an inert re-export).

The repro is the same helper the PR routes the OpenAI video submit response through, so the bounded-reader behavior exercised in the repro is the exact behavior the PR enables in production.

ClawSweeper finding addressed in this revision

Previous review said: "the PR body lists mocked tests, lint, CI, and an example helper error, but no terminal/live output or artifact for the changed provider path."

  • Added scripts/repro/issue-96144-openai-video-cap.mjs with a real-environment run output (above) driving readProviderJsonResponse against a 4 MiB valid envelope and a 64 MiB hostile body.
  • The repro exercises the exact helper the PR enables, so the real-environment output proves the bounded-reader cap fires on a 64 MiB body and the valid envelope round-trips through the same code path the PR opens.
  • Negative control verifies the helper is the source of the cap, not just an indirection.

Out of scope

  • Sibling surfaces in src/agents/chutes-oauth.ts (4 sites),
    src/infra/clawhub.ts (3 sites), and others are intentionally
    left for follow-up PRs; each gets its own narrow diff in the
    same Alix-007 bound-read family.
  • The pollProviderOperationJson polling path and the
    readResponseWithLimit download path are already bounded in this file
    and do not need changes.

@wangmiao0668000666 wangmiao0668000666 mentioned this pull request Jun 23, 2026
Closed
@wangmiao0668000666

wangmiao0668000666 commented Jun 23, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper review

@clawsweeper — could you kick off a review for PR #96144 (commit 348ac68624 on branch fix/openai-video-bounded-json-response)?

Background:

@clawsweeper

clawsweeper Bot commented Jun 23, 2026 •
edited
Loading

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 commented Jun 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 24, 2026, 12:05 AM ET / 04:05 UTC.

Summary
The PR replaces the OpenAI video submit response parse with readProviderJsonResponse, updates provider HTTP test mocks and OpenAI video tests, and adds a terminal repro script for the bounded reader.

PR surface: Source +10, Tests +69, Other +141. Total +220 across 4 files.

Reproducibility: yes. at source level: current main reaches raw response.json() on the OpenAI video submit response after a successful provider POST. I did not execute a current-main OOM harness, but the path and bounded-reader behavior are clear from source plus the PR's terminal repro.

Review metrics: none identified.

Root-cause cluster
Relationship: canonical
Canonical: #96144
Summary: This PR is the canonical open item for the OpenAI video submit response read; related bound-response PRs cover adjacent provider surfaces rather than replacing this work.

Members:

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

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • none.

Next step before merge

  • No ClawSweeper repair lane is needed; the remaining action is normal maintainer review and merge handling for this PR.

Security
Cleared: No dependency, workflow, package, credential, permission, or supply-chain surface is added; the runtime change narrows an untrusted provider JSON read.

Review details

Best possible solution:

Land the focused bounded-reader swap with the included provider integration tests and terminal proof, while keeping adjacent response.json() surfaces in their separate follow-up PRs.

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

Yes, at source level: current main reaches raw response.json() on the OpenAI video submit response after a successful provider POST. I did not execute a current-main OOM harness, but the path and bounded-reader behavior are clear from source plus the PR's terminal repro.

Is this the best way to solve the issue?

Yes: using the existing readProviderJsonResponse helper is the narrowest owner-boundary fix for the OpenAI video submit metadata response. A local one-off cap would duplicate provider-http behavior, and poll/download sibling paths are already bounded through existing helpers.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 0671c0890073.

Label changes

Label changes:

  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR body now includes terminal output from a real tsx repro exercising production readProviderJsonResponse on valid and oversized streaming bodies, with unit tests tying the OpenAI video provider to that helper.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body now includes terminal output from a real tsx repro exercising production readProviderJsonResponse on valid and oversized streaming bodies, with unit tests tying the OpenAI video provider to that helper.
  • remove rating: 🧂 unranked krab: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.
  • remove status: 📣 needs proof: Current PR status label is status: 👀 ready for maintainer look.

Label justifications:

  • P2: This is a focused provider-response memory/availability hardening fix with limited OpenAI video-generation blast radius.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body now includes terminal output from a real tsx repro exercising production readProviderJsonResponse on valid and oversized streaming bodies, with unit tests tying the OpenAI video provider to that helper.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body now includes terminal output from a real tsx repro exercising production readProviderJsonResponse on valid and oversized streaming bodies, with unit tests tying the OpenAI video provider to that helper.
Evidence reviewed

PR surface:

Source +10, Tests +69, Other +141. Total +220 across 4 files.

View PR surface stats
Area Files Added Removed Net
Source 2 11 1 +10
Tests 1 118 49 +69
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 1 141 0 +141
Total 4 270 50 +220

What I checked:

Likely related people:

  • shakkernerd: GitHub commit history for extensions/openai/video-generation-provider.ts shows several recent fixes around OpenAI video request policy, guarded downloads, cleanup, and video edits routing. (role: recent OpenAI video contributor; confidence: high; commits: 31b51455942c, ed7d99aa0edb, efbf9f3d4657; files: extensions/openai/video-generation-provider.ts, src/plugin-sdk/test-helpers/provider-http-mocks.ts)
  • vincentkoc: History shows work on generated video download caps and provider HTTP response hardening adjacent to the same response-size boundary. (role: recent bounded media/provider contributor; confidence: high; commits: 9e002c12ac7a, 79691d485847, e802fb8a9fbb; files: extensions/openai/video-generation-provider.ts, src/agents/provider-http-errors.ts)
  • Alix-007: Merged PR fix(agents): bound provider JSON response reads #95218 added the bounded provider JSON response reader that this PR applies to OpenAI video submits. (role: bounded JSON helper author; confidence: high; commits: 2592f8a51a4e; files: src/agents/provider-http-errors.ts)
  • steipete: GitHub history shows the original video provider support and several shared provider/video-generation refactors in the affected path. (role: feature-history contributor; confidence: medium; commits: 932194b7d58c, d7c7905a52d8, 9f46e9cb8540; files: extensions/openai/video-generation-provider.ts, src/media-understanding/shared.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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 keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@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. labels Jun 24, 2026
@wangmiao0668000666
fix(video-generation): bound OpenAI video submitted response reads
5b2e4bc 
Switches extensions/openai/video-generation-provider.ts:427 from a raw
`await response.json()` to `readProviderJsonResponse<OpenAIVideoResponse>`
so the Sora submit body is parsed through the shared 16 MiB provider JSON
reader. Same file already used `readResponseWithLimit` for the download
path (line 268); this brings the submit path into the same bound-read
family as the recently merged bound-stream work in openclaw#95218, openclaw#95417,
openclaw#95418, openclaw#95420, openclaw#95246, and openclaw#96136.

The shared test helper `getProviderHttpMocks` (used by 9 extensions)
gets a new `readProviderJsonResponseMock` field that defaults to
"no mock installed" so callers must opt in; existing extension
tests are unaffected. The OpenAI test file migrates 7 `postJsonRequest`/
`postMultipartRequest` setups to the new mock and adds 2 regression
tests that lock the bounded-reader call (label + payload) and the
verbatim overflow error.
@wangmiao0668000666
fix(video-generation): add real-environment repro for OpenAI video su…
2efc462 
…bmit response cap
@wangmiao0668000666
wangmiao0668000666 force-pushed the fix/openai-video-bounded-json-response branch from 348ac68 to 2efc462 Compare June 24, 2026 01:40
@openclaw-barnacle openclaw-barnacle Bot added scripts Repository scripts size: M and removed size: S labels Jun 24, 2026
@wangmiao0668000666

wangmiao0668000666 commented Jun 24, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper — please re-review PR #96144 (now on commits 5b2e4bc94a + 2efc462989 on branch fix/openai-video-bounded-json-response).

Changes since the previous review:

  • Rebased onto current openclaw/main (da15cf48bf, contains Alix-007 fix(agents): bound provider JSON response reads (#95218) commit 2592f8a51a), so the readProviderJsonResponse helper now actually enforces the canonical 16 MiB cap on the OpenAI video submit path (the previous rebase base was a stale local main that predated the cap).
  • Added scripts/repro/issue-96144-openai-video-cap.mjs — a real-environment proof that drives the production readProviderJsonResponse directly with a 4 MiB valid Sora submit response and a 64 MiB hostile streaming body, with a negative control showing raw response.json() on the same body buffers the full payload. Output:
$ pnpm exec tsx scripts/repro/issue-96144-openai-video-cap.mjs
=== Reproduction for PR #96144 — OpenAI video submit response cap policy ===
PROVIDER_JSON_RESPONSE_MAX_BYTES = 16777216 bytes
bounded-reader label = "OpenAI video generation"
Valid envelope: 4194419 bytes (4.00 MiB) id=video_abc123 status=queued
PASS  valid OpenAI video submit response: accepted, id=video_abc123
PASS  hostile 64 MiB response: rejected with "OpenAI video generation: JSON response exceeds 16777216 bytes"
PASS  negative control: raw response.json() on 64 MiB body failed with "SyntaxError" (no bounded-reader wrapping)
=== All repro assertions passed ===
  • 16/16 vitest pass on extensions/openai/video-generation-provider.test.ts (rebased branch now actually has the bounded reader; the previous 347 tests passed output was running against a different code path because the rebase base lacked fix(agents): bound provider JSON response reads #95218).
  • PR body updated with ## Real behavior proof and ## ClawSweeper finding addressed in this revision sections, including the repro output above.

@clawsweeper

clawsweeper Bot commented Jun 24, 2026 •
edited
Loading

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.

@wangmiao0668000666 wangmiao0668000666 mentioned this pull request Jun 24, 2026
Closed
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. 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 24, 2026
@wangmiao0668000666 wangmiao0668000666 mentioned this pull request Jun 24, 2026
Closed
@wangmiao0668000666

wangmiao0668000666 commented Jun 24, 2026

Copy link
Copy Markdown
Contributor Author

@sallyom — this PR is one half of the same bound-response.json() family you merged in
#95218 / #95417 / #95418 / #95420 / #96027 / #96035 / #96038 / #96042. It extends
readProviderJsonResponse to the Sora submit response (extensions/openai/video-generation-provider.ts:427),
which #96144 called out as "Out of scope" once the symmetric core read was capped.

🐚 platinum hermit + 👀 ready for maintainer look, with a standalone repro script
(scripts/repro/issue-96144-openai-video-cap.mjs) that drives the production helper
end-to-end against valid + hostile bodies.

Happy to address any review feedback or rebase if you'd like to land the symmetric
counterpart alongside its sibling (#96036 for DashScope).

This was referenced Jun 24, 2026
Closed
Closed
@wangmiao0668000666

wangmiao0668000666 commented Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

Closing in favor of an Alix-007-style XS reshape. The actual code change is +5/-1 = 4 lines across 1 call site — the current PR's 270 LoC comes from a committed repro script (141 LoC) + test file refactoring (+118/-49) + test helper change (+6). Per Alix-007 pattern: 1 source file, inline test in existing describe, local-only proof, XS size.

New PR will replace the same response.json() call with readProviderJsonResponse — same fix, under 20 LoC total.

@clawsweeper clawsweeper Bot mentioned this pull request Jun 25, 2026
Closed
This was referenced Jun 25, 2026
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

extensions: openai P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. scripts Repository scripts size: M status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@wangmiao0668000666