fix(infra): enforce maxBytes in body-less HTTP error snippet path by Pick-cat · Pull Request #99340 · openclaw/openclaw

Pick-cat · 2026-07-03T03:21:04Z

What Problem This Solves

Fixes an issue where readResponseBodySnippet would bypass the maxBytes limit when reading provider error bodies from responses whose body is null or lacks a getReader function. A malformed response entering this fallback path would buffer the entire body via response.text() before applying the maxChars truncation — the maxBytes guard was never enforced. The stream path (normal ReadableStream responses) already enforced maxBytes correctly.

Why This Change Was Made

The body-less path now encodes the text and truncates at maxBytes before decoding and applying maxChars, matching the existing stream path contract. The truncated byte buffer is decoded with { stream: true } so an incomplete multi-byte UTF-8 sequence at the cut point is dropped rather than rendered as a replacement character.

User Impact

Provider error snippets (used in MiniMax VLM and native PDF provider error messages) now respect the configured byte budget in all response shapes. No API or config surface changes.

Evidence

Real behavior proof (fix). The body-less path now enforces maxBytes:

[case 1] body-less path: text under maxBytes returns unmodified
  ok: returns original text
[case 2] body-less path: text exceeding maxBytes is truncated
  ok: result within maxBytes
  bytes returned: 100 / 100 max
[case 3] body-less path: UTF-8 multi-byte not corrupted at byte boundary
  ok: no replacement character
  ok: returns ab
  result: "ab" (bytes=2)
[case 4] stream path: enforces maxBytes (regression guard)
  ok: stream path respects maxBytes
[case 5] maxBytes=0 returns empty string
  ok: maxBytes=0 returns empty

ALL PROOF ASSERTIONS: 6 passed, 0 failed

Negative control (pre-fix main). Same proof script against origin/main — maxBytes is ignored, full text returned:

[case 2] body-less path: text exceeding maxBytes is truncated
  FAIL: result within maxBytes
  bytes returned: 5000 / 100 max
[case 3] body-less path: UTF-8 multi-byte not corrupted at byte boundary
  FAIL: returns ab
  result: "ab😀cd" (bytes=8)
[case 5] maxBytes=0 returns empty string
  FAIL: maxBytes=0 returns empty

ALL PROOF ASSERTIONS: 3 passed, 3 failed
exit=1

Focused tests:

node scripts/run-vitest.mjs src/infra/http-error-body.test.ts
Tests  9 passed (9)

Caller regression:

node scripts/run-vitest.mjs src/agents/minimax-vlm.normalizes-api-key.test.ts
Tests  12 passed (12)

node scripts/run-vitest.mjs src/agents/tools/pdf-native-providers.test.ts
Tests  20 passed (20)

Format: oxfmt --check clean on both changed files.

clawsweeper · 2026-07-03T03:26:05Z

Codex review: needs maintainer review before merge. Reviewed July 2, 2026, 11:30 PM ET / 03:30 UTC.

Summary
The branch updates readResponseBodySnippet so the non-stream fallback clamps returned text by maxBytes before maxChars, and adds focused Vitest coverage for the helper.

PR surface: Source +9, Tests +99. Total +108 across 2 files.

Reproducibility: yes. source-reproducible: current main’s fallback path ignores maxBytes and only slices by maxChars. The PR body also provides a negative-control proof where the body-less path returns 5000 bytes with a 100-byte limit before the patch.

Review metrics: none identified.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🐚 platinum hermit
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:

[P2] Wait for the current CI run to finish and attribute the failing jobs before merge.
If maintainers want non-stream fallback memory-read caps, reconcile that larger direction with fix(boundary-safety): guard response reads and UTF-16 truncation #97774 instead of expanding this focused PR.

Risk before merge

[P1] Current CI was not fully green during review; failed job logs were unavailable while the workflow was still running, so maintainers should wait for attribution before merge.
[P2] The non-stream fallback still materializes response.text() before truncation, so this PR bounds the returned snippet for body-less Response-like objects rather than proving a memory-read cap for that fallback.

Maintainer options:

Decide the mitigation before merge
Land this focused helper fix after CI attribution, or intentionally supersede it with the broader boundary-safety PR if maintainers choose that path.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

[P2] No narrow ClawSweeper repair is identified from the diff; maintainers should wait for CI attribution and decide whether this focused fix lands or the broader boundary-safety PR subsumes it.

Security
Cleared: No supply-chain, workflow, dependency, permission, secret, or credential surface is changed; the diff only changes local error-snippet truncation and tests.

Review details

Best possible solution:

Land this focused helper fix after CI attribution, or intentionally supersede it with the broader boundary-safety PR if maintainers choose that path.

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

Yes, source-reproducible: current main’s fallback path ignores maxBytes and only slices by maxChars. The PR body also provides a negative-control proof where the body-less path returns 5000 bytes with a 100-byte limit before the patch.

Is this the best way to solve the issue?

Yes for the returned diagnostic snippet: the shared helper is the right narrow owner because both MiniMax VLM and native PDF error paths call it. The remaining limitation is that a non-stream fallback cannot avoid response.text() materialization unless maintainers choose a broader boundary redesign.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 2e049f509fad.

Label changes

Label justifications:

P2: This is a focused bug fix for bounded provider error snippets with limited blast radius and no new public API or config surface.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes copied after-fix and negative-control terminal output plus focused test commands for this non-visual helper behavior.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes copied after-fix and negative-control terminal output plus focused test commands for this non-visual helper behavior.

Evidence reviewed

PR surface:

Source +9, Tests +99. Total +108 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	10	1	+9
Tests	1	99	0	+99
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	109	1	+108

What I checked:

Current main fallback ignores maxBytes: readResponseBodySnippet currently returns (await response.text()).slice(0, limits.maxChars) when response.body is absent or lacks getReader, so the fallback path does not apply the byte limit. (src/infra/http-error-body.ts:8, 2e049f509fad)
Current callers rely on the byte limit: MiniMax VLM passes both MINIMAX_VLM_ERROR_BODY_MAX_BYTES and MINIMAX_VLM_ERROR_BODY_MAX_CHARS into the shared snippet helper for provider error bodies. (src/agents/minimax-vlm.ts:133, 2e049f509fad)
Native PDF caller shares the same invariant: Native PDF provider errors use the same helper with an 8 KiB byte cap and 400-character display cap, so fixing the helper covers both cited call sites. (src/agents/tools/pdf-native-providers.ts:59, 2e049f509fad)
PR patch applies the missing fallback clamp: The PR encodes fallback text, slices the byte buffer to limits.maxBytes when needed, decodes it, and then applies limits.maxChars. (src/infra/http-error-body.ts:8, 16f6145c93c8)
Focused tests cover the changed helper: The added test file covers non-stream maxBytes truncation, maxChars truncation, maxBytes-before-maxChars ordering, multibyte UTF-8 boundary behavior, stream-path guards, zero byte caps, and empty bodies. (src/infra/http-error-body.test.ts:28, 16f6145c93c8)
TextDecoder boundary behavior checked: A local Node probe confirmed TextDecoder.decode(..., { stream: true }) drops an incomplete trailing UTF-8 sequence while default decode emits a replacement character.

Likely related people:

RomneyDa: Live PR metadata and local blame show this handle authored and merged the recent main commit carrying the current helper and caller files in the checked-out history. (role: recent area contributor; confidence: medium; commits: aae57adc80ba, 58c92df58837; files: src/infra/http-error-body.ts, src/agents/minimax-vlm.ts, src/agents/tools/pdf-native-providers.ts)
Vincent Koc: The latest release commit containing the same helper and caller files is authored and committed by this person, so they are relevant to shipped-behavior provenance. (role: release-line contributor; confidence: medium; commits: e085fa1a3ffd; files: src/infra/http-error-body.ts, src/agents/minimax-vlm.ts, src/agents/tools/pdf-native-providers.ts)
eleqtrizit: Merged PR fix(pdf): guard native provider requests #97872 recently hardened native PDF provider HTTP handling and touched the PDF caller that uses this helper. (role: recent adjacent owner; confidence: high; commits: 3d639ebbbb1f, 289c193f632a, 6be4158786c0; files: src/agents/tools/pdf-native-providers.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.

Co-Authored-By: Claude Opus 4.7 <[email protected]>

…enclaw#99340) * fix(infra): enforce maxBytes in body-less HTTP error snippet path * chore: rebase to trigger CI after boundaries check fix Co-Authored-By: Claude Opus 4.7 <[email protected]> --------- Co-authored-by: Claude Opus 4.7 <[email protected]>

openclaw-barnacle Bot added the size: S label Jul 3, 2026

Pick-cat force-pushed the fix/http-error-body-bounded-read branch from 16f6145 to 7bf7e25 Compare July 3, 2026 03:28

Pick-cat and others added 2 commits July 3, 2026 12:27

fix(infra): enforce maxBytes in body-less HTTP error snippet path

18a7817

chore: rebase to trigger CI after boundaries check fix

f7ed084

Co-Authored-By: Claude Opus 4.7 <[email protected]>

Pick-cat force-pushed the fix/http-error-body-bounded-read branch from 7bf7e25 to f7ed084 Compare July 3, 2026 04:27

clawsweeper Bot mentioned this pull request Jul 3, 2026

fix(boundary-safety): guard response reads and UTF-16 truncation #97774

Closed

openclaw-clownfish Bot merged commit 7c9c2ef into openclaw:main Jul 6, 2026
96 checks passed

vincentkoc mentioned this pull request Jul 6, 2026

chore(jobs): prune resolved OpenClaw inbox jobs openclaw/clownfish#221

Merged

maweibin mentioned this pull request Jul 14, 2026

fix(agents): guard null response body in proxy stream reader #107325

Merged

This was referenced Jul 16, 2026

Bound HTTP error body snippets to readable streams #109245

Open

fix(provider-error): keep error body truncation UTF-16 safe #109974

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(infra): enforce maxBytes in body-less HTTP error snippet path#99340

fix(infra): enforce maxBytes in body-less HTTP error snippet path#99340
openclaw-clownfish[bot] merged 2 commits into
openclaw:mainfrom
Pick-cat:fix/http-error-body-bounded-read

Pick-cat commented Jul 3, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jul 3, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

Pick-cat commented Jul 3, 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

Uh oh!

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

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Pick-cat commented Jul 3, 2026 •

edited

Loading

clawsweeper Bot commented Jul 3, 2026 •

edited

Loading