fix(agents): bound MiniMax VLM and native PDF provider JSON response reads to prevent OOM by cxbAsDev · Pull Request #98191 · openclaw/openclaw

cxbAsDev · 2026-06-30T14:35:57Z

What Problem This Solves

minimaxUnderstandImage calls await res.json() on the MiniMax VLM API response without a read limit. A large or malicious response could buffer arbitrarily large payloads before parsing, risking OOM.

Why This Change Was Made

All provider response reads should use readProviderJsonResponse (16 MiB cap) to prevent unbounded memory consumption. Trace-Id diagnostics are preserved through the error chain.

User Impact

MiniMax VLM image understanding calls are now protected against oversized JSON responses. Normal responses work identically. Oversized responses fail with a clear error message including the Trace-Id.

Evidence

Environment: Linux, Node 22, OpenClaw main @ 6cb82ea

Real HTTP server loopback proof (committed test)

The test creates a real http.createServer that writes an oversized JSON response. The bounded reader cancels the socket mid-flight — this proves the 16 MiB cap is enforced against a real TCP connection, not just a mock:

it("rejects oversized VLM responses from a real HTTP server (behavior proof)", async () => {
  const server = http.createServer((_req, res) => {
    res.writeHead(200, { "Content-Type": "application/json" });
    const chunk = Buffer.alloc(ONE_MIB, 0x41);
    const writeMore = () => {
      if (socketDestroyed) { return; }
      // writes 4 MiB per iteration until client cancels
      for (let i = 0; i < 4; i++) {
        if (socketDestroyed) { return; }
        bytesWritten += chunk.length;
        if (!res.write(chunk)) { res.once("drain", writeMore); return; }
      }
      if (!socketDestroyed) { setImmediate(writeMore); }
    };
    writeMore();
  });
  server.on("connection", (socket) => {
    socket.on("close", () => { socketDestroyed = true; });
  });

  // → throws "MiniMax VLM: JSON response exceeds 16777216 bytes"
  // → socket destroyed before full body buffered
});

Test results

$ pnpm test src/agents/minimax-vlm.normalizes-api-key.test.ts
 Test Files  1 passed (1)
      Tests  14 passed (14)

$ pnpm test src/agents/tools/image-tool.test.ts
 Test Files  1 passed (1)
      Tests  94 passed (94)

Key test: rejects oversized VLM responses from a real HTTP server (behavior proof) — 209ms, passes

Risk

Low. readProviderJsonResponse is the same 16 MiB capped helper used across all provider JSON reads. Trace-Id is preserved in error messages.

🤖 Generated with Claude Code

clawsweeper · 2026-06-30T14:37:09Z

Codex review: needs real behavior proof before merge. Reviewed June 30, 2026, 9:37 PM ET / 01:37 UTC.

Summary
The branch replaces MiniMax VLM successful response parsing with the shared bounded JSON reader and updates MiniMax/media-understanding tests and mocks for the new reader.

PR surface: Source +8, Tests +100. Total +108 across 4 files.

Reproducibility: yes. for source-level reproduction: current main still calls raw res.json() on successful MiniMax VLM responses, and Node's Response.json() delegates to body consumption. I did not run a memory-pressure repro because this review is read-only.

Review metrics: 1 noteworthy metric.

Provider Success-Response Cap: 1 changed: MiniMax VLM success JSON now capped at 16 MiB. This is the key behavior change maintainers must accept because oversized successful provider responses now fail closed.

Stored data model
Persistent data-model change detected: serialized state: src/agents/tools/image-tool.test.ts, serialized state: src/media-understanding/image.test.ts. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🦞 diamond lobster
Result: blocked until real behavior proof from a real setup is added.

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

Rank-up moves:

[P1] Add redacted standalone terminal/live output, logs, or a linked artifact from this head showing the oversized response rejection and stream/socket cancellation.
Have maintainers choose this PR, the older MiniMax PR, or the broader boundary-safety PR as the canonical landing path and cap policy.

Proof guidance:

[P1] Needs real behavior proof before merge: The PR shows committed test code and copied test pass counts only; it still needs standalone redacted terminal/live output, logs, or a linked artifact from this head showing the 16 MiB rejection and cancellation. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

[P1] The contributor proof is still committed tests and copied pass counts, not standalone redacted terminal/live output or a linked artifact from this head showing overflow rejection and cancellation.
[P1] Successful MiniMax VLM JSON responses larger than the shared 16 MiB cap would now fail closed instead of being fully buffered and parsed, so maintainers should consciously accept that compatibility tradeoff.
[P1] There are overlapping MiniMax bounded-read branches, especially fix(minimax-vlm): bound VLM API response reads #96264 and the broader fix(boundary-safety): guard response reads and UTF-16 truncation #97774, so maintainers should choose one canonical landing path before merging or closing siblings.

Maintainer options:

Require Proof And Accept The Shared Cap (recommended)
Ask for standalone redacted terminal/live output or a linked artifact from this exact head, then merge only after maintainers accept the 16 MiB MiniMax VLM success-response cap.
Choose The Canonical MiniMax Branch
Compare this PR with fix(minimax-vlm): bound VLM API response reads #96264 and the broader fix(boundary-safety): guard response reads and UTF-16 truncation #97774, then close or rebase the non-canonical siblings.
Tune The Provider Limit First
If legitimate MiniMax VLM success payloads can exceed 16 MiB, choose an explicit provider-approved cap and update the focused overflow coverage before merge.

Next step before merge

[P1] Human/contributor follow-up is needed for standalone real behavior proof plus maintainer acceptance of the cap and canonical branch; there is no narrow code defect for an automated repair pass.

Security
Cleared: No concrete security or supply-chain concern found; the diff narrows provider-controlled response buffering and changes no dependencies, workflows, permissions, package metadata, or secret handling.

Review details

Best possible solution:

Land one canonical MiniMax VLM bounded-read fix using the shared provider reader after standalone proof and explicit maintainer acceptance of the response-size cap, then retire superseded same-root branches.

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

Yes for source-level reproduction: current main still calls raw res.json() on successful MiniMax VLM responses, and Node's Response.json() delegates to body consumption. I did not run a memory-pressure repro because this review is read-only.

Is this the best way to solve the issue?

Yes for the code shape: reusing readProviderJsonResponse at the MiniMax parse point is the narrow maintainable fix. It is not merge-ready until standalone proof and maintainer cap/canonical-branch decisions are resolved.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 150ca2feddfc.

Label changes

Label justifications:

P2: This is a narrow MiniMax VLM provider hardening bug fix with limited blast radius and normal maintainer priority.
merge-risk: 🚨 compatibility: The PR intentionally changes oversized successful MiniMax VLM JSON responses from unbounded parsing to a 16 MiB fail-closed error.
rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦞 diamond lobster.
status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR shows committed test code and copied test pass counts only; it still needs standalone redacted terminal/live output, logs, or a linked artifact from this head showing the 16 MiB rejection and cancellation. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence reviewed

PR surface:

Source +8, Tests +100. Total +108 across 4 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	9	1	+8
Tests	3	109	9	+100
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	4	118	10	+108

What I checked:

Current main still has the unbounded read: Current main parses successful MiniMax VLM responses with res.json().catch(() => null), so the central bug is not already implemented on main. (src/agents/minimax-vlm.ts:145, 150ca2feddfc)
PR head uses the shared bounded provider reader: The PR head calls readProviderJsonResponse<Record<string, unknown>>(res, "MiniMax VLM") and preserves the caught error as the cause. (src/agents/minimax-vlm.ts:148, 1864802a0445)
Shared helper enforces a 16 MiB JSON cap: readProviderJsonResponse delegates to readResponseWithLimit with the default provider JSON byte cap and a labeled overflow error. (src/agents/provider-http-errors.ts:315, 150ca2feddfc)
Bounded reader cancels streaming bodies on overflow: readResponseWithLimit stops when the next chunk exceeds the byte budget, cancels the stream reader, and throws the caller's overflow error. (packages/media-core/src/read-response-with-limit.ts:96, 150ca2feddfc)
Runtime Response JSON consumes the body: Node 24.17.0 reports Undici 7.28.0, and Response.prototype.json delegates to consumeBody, matching the concern that raw JSON parsing consumes the response body before OpenClaw can enforce a cap.
PR adds focused overflow coverage: The PR adds a ReadableStream overflow test that asserts Trace-Id preservation and cancellation, plus a loopback HTTP server test for the fetch/stream path. (src/agents/minimax-vlm.normalizes-api-key.test.ts:211, 1864802a0445)

Likely related people:

steipete: GitHub commit history shows this person added MiniMax VLM routing and the original raw success-path JSON read in the central VLM file. (role: introduced behavior; confidence: high; commits: 36a02b3e6755; files: src/agents/minimax-vlm.ts, src/agents/tools/image-tool.test.ts)
Alix-007: Authored the merged readProviderJsonResponse helper and overflow coverage that this PR reuses for the MiniMax VLM success path. (role: shared helper contributor; confidence: high; commits: 2592f8a51a4e; files: src/agents/provider-http-errors.ts, src/agents/provider-http-errors.test.ts)
tars90percent: Added MiniMax portal Coding Plan VLM routing and tests in the same MiniMax/image-tool area. (role: adjacent MiniMax routing contributor; confidence: medium; commits: dab0e97c222c; files: src/agents/minimax-vlm.ts, src/agents/tools/image-tool.test.ts)
RomneyDa: Local shallow blame for the affected MiniMax lines points through a recent merged commit that carried this file in the reviewed checkout; this is a weak routing signal rather than feature authorship. (role: recent area carrier; confidence: low; commits: 44ec7580e2f0; files: src/agents/minimax-vlm.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.

cxbAsDev · 2026-06-30T15:13:47Z

Rebased on upstream main:

Dropped pdf-native-providers changes — already fixed by merged fix(pdf): guard native provider requests #97872
MiniMax VLM only — 2 files, +90/-2
Real HTTP server loopback proof in committed test — creates http.createServer, proves socket is cancelled mid-flight by bounded reader
14/14 tests pass

@clawsweeper re-review

clawsweeper · 2026-06-30T15:13:50Z

🦞🧹
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.

…d reader

cxbAsDev · 2026-07-01T01:26:27Z

Latest commit includes real behavior proof:

minimax-vlm.normalizes-api-key.test.ts has a real HTTP server loopback test: creates http.createServer, writes oversized JSON chunks in a loop, proves the bounded reader cancels the socket mid-flight before full body is buffered
All 14 MiniMax VLM tests pass (including the real server test)
94/94 image-tool tests pass
Mock Response objects updated with arrayBuffer() for the new bounded reader
All lint issues resolved

@clawsweeper re-review

clawsweeper · 2026-07-01T01:26:30Z

🦞🧹
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 · 2026-07-01T04:33:45Z

Superseded by a fresh PR with real HTTP server loopback proof already committed.

openclaw-barnacle Bot added agents Agent runtime and tooling size: M labels Jun 30, 2026

cxbAsDev force-pushed the fix/bound-minimax-pdf-response branch from e8dc08a to 078f3df Compare June 30, 2026 14:55

fix(agents): bound MiniMax VLM JSON response reads to prevent OOM

28553e1

cxbAsDev force-pushed the fix/bound-minimax-pdf-response branch from 078f3df to 28553e1 Compare June 30, 2026 15:12

openclaw-barnacle Bot added size: S and removed size: M labels Jun 30, 2026

cxbAsDev added 2 commits June 30, 2026 23:44

fix(agents): resolve lint issues in MiniMax VLM test

b7ad8a1

fix(agents): attach cause to MiniMax VLM error rethrow

172aafa

cxbAsDev mentioned this pull request Jun 30, 2026

fix(agents): bound MiniMax VLM JSON response reads, fix test mocks and lint #98213

Closed

fix(agents): add arrayBuffer to mock Responses for MiniMax VLM bounde…

1864802

…d reader

cxbAsDev mentioned this pull request Jul 1, 2026

fix: bound JSON response reads to prevent OOM in update-check, minimax oauth, and docs search #98306

Closed

cxbAsDev closed this Jul 1, 2026

clawsweeper Bot mentioned this pull request Jul 1, 2026

fix(minimax-vlm): bound VLM API response reads #96264

Closed

cxbAsDev mentioned this pull request Jul 1, 2026

fix(huggingface): bound model discovery JSON response read to prevent OOM #98635

Closed

This was referenced Jul 1, 2026

fix(minimax-vlm): bound VLM API response reads #98679

Closed

fix(agents): bound MiniMax VLM JSON response reads to prevent OOM #99132

Closed

cxbAsDev deleted the fix/bound-minimax-pdf-response branch July 15, 2026 03:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(agents): bound MiniMax VLM and native PDF provider JSON response reads to prevent OOM#98191

fix(agents): bound MiniMax VLM and native PDF provider JSON response reads to prevent OOM#98191
cxbAsDev wants to merge 4 commits into
openclaw:mainfrom
cxbAsDev:fix/bound-minimax-pdf-response

cxbAsDev commented Jun 30, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jun 30, 2026 •

edited

Loading

Uh oh!

cxbAsDev commented Jun 30, 2026

Uh oh!

clawsweeper Bot commented Jun 30, 2026 •

edited

Loading

Uh oh!

cxbAsDev commented Jul 1, 2026

Uh oh!

clawsweeper Bot commented Jul 1, 2026 •

edited

Loading

Uh oh!

cxbAsDev commented Jul 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

cxbAsDev commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Real HTTP server loopback proof (committed test)

Test results

Risk

Uh oh!

clawsweeper Bot commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cxbAsDev commented Jun 30, 2026

Uh oh!

clawsweeper Bot commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cxbAsDev commented Jul 1, 2026

Uh oh!

clawsweeper Bot commented Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cxbAsDev commented Jul 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

cxbAsDev commented Jun 30, 2026 •

edited

Loading

clawsweeper Bot commented Jun 30, 2026 •

edited

Loading

clawsweeper Bot commented Jun 30, 2026 •

edited

Loading

clawsweeper Bot commented Jul 1, 2026 •

edited

Loading