fix(discord,telegram,github-copilot,google-meet): bound JSON response reads to prevent OOM by lsr911 · Pull Request #96618 · openclaw/openclaw

lsr911 · 2026-06-25T01:32:26Z

What Problem This Solves

Replace unbounded await response.json() with readResponseWithLimit (1 MiB cap)
across bound JSON response reads to prevent OOM:4 extensions, preventing OOM from oversized upstream HTTP responses.

Evidence

Real HTTP bounded-read proof

node --import tsx test/_proof_bounded_json.mts

[proof] real server on :58033, cap=1048576 bytes, would-stream≈67108864 bytes
[proof] small server on :58034

PASS  oversized body: throws bounded cap error :: threw=true msg="Error: JSON response exceeds 1048576 bytes"
PASS  stream cancelled at cap: server sent ≈ cap bytes, not ~64 MiB :: bytesSent=1048576 (cap=1048576, would-stream=67108864)
PASS  happy path: small JSON parsed correctly :: status=ok id=batch-1
PASS  negative control: unbounded read buffers PAST cap :: buffered=33554457 bytes (> 1048576)

[proof] 4 PASS, 0 FAIL

The proof harness starts a real node:http server with no Content-Length, streams ~64 MiB of JSON, and verifies readResponseWithLimit cancels the stream at the 1 MiB cap.

AI-assisted.

clawsweeper · 2026-06-25T01:33:39Z

Codex review: needs real behavior proof before merge. Reviewed June 27, 2026, 2:22 AM ET / 06:22 UTC.

Summary
The branch replaces three Discord success-path native JSON reads in PluralKit, webhook send-result parsing, and voice upload URL parsing with readProviderJsonResponse.

PR surface: Source +11. Total +11 across 3 files.

Reproducibility: yes. by source inspection: current main still has direct success-path response.json() calls in the three touched Discord paths, and the PluralKit test-double incompatibility is visible from current source. I did not run a live OOM reproduction in this read-only review.

Review metrics: 1 noteworthy metric.

Capped Discord success JSON reads: 3 changed. Each replacement changes an existing success-response parsing path, so representative changed-path proof matters before merge.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🦐 gold shrimp
Result: blocked until stronger real behavior proof is added.

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

Rank-up moves:

Update the PluralKit success fixture to return a real Response or equivalent body/arrayBuffer-capable object.
[P1] Add redacted terminal output, logs, or a recording from PluralKit lookup, webhook send-result parsing, or voice upload URL parsing through the new reader.

Proof guidance:

[P1] Needs stronger real behavior proof before merge: The PR body includes terminal output from a standalone 1 MiB bounded-read harness, not after-fix proof through one patched Discord runtime path at the shared 16 MiB cap. 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] Existing successful Discord responses over the shared 16 MiB provider JSON cap will now fail instead of being parsed by native response.json(), which is intentional hardening but still a compatibility change.
[P1] The posted proof does not exercise any changed Discord runtime path, so it does not show PluralKit lookup, webhook result parsing, or voice upload URL parsing still works after the patch.
[P1] The PluralKit success fixture is still a json()-only mock, so focused regression coverage for normal PluralKit parsing will break or remain untrustworthy until the test double is updated.

Maintainer options:

Fix fixture and prove one Discord path (recommended)
Update the PluralKit success fixture, then add redacted terminal output, logs, or a recording that drives PluralKit lookup, webhook response parsing, or voice upload URL parsing through the new reader.
Accept helper-level proof explicitly
Maintainers may choose to accept the standalone helper proof, but that owns the compatibility and availability uncertainty for the changed Discord success paths.
Pause until changed-path proof exists
Keep the PR open but unmerged if the contributor cannot yet provide representative proof from one of the patched Discord runtime paths.

Next step before merge

[P1] The remaining blockers are contributor-supplied changed-path proof plus a focused test-fixture repair, so this should stay in human review rather than ClawSweeper repair automation.

Security
Cleared: No workflow, dependency, package-resolution, permission, or secret-handling surface changed; the diff only bounds existing Discord HTTP response reads.

Review findings

[P2] Update the PluralKit success fixture — extensions/discord/src/pluralkit.ts:65

Review details

Best possible solution:

Keep the shared bounded-reader approach, update the PluralKit success fixture to use a real Response or equivalent body/arrayBuffer-capable double, and add redacted proof from one patched Discord path before merge.

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

Yes, by source inspection: current main still has direct success-path response.json() calls in the three touched Discord paths, and the PluralKit test-double incompatibility is visible from current source. I did not run a live OOM reproduction in this read-only review.

Is this the best way to solve the issue?

No as submitted. Reusing readProviderJsonResponse is the right layer, but the branch still needs the PluralKit fixture repair and changed-path real behavior proof before it is the best mergeable fix.

Full review comments:

[P2] Update the PluralKit success fixture — extensions/discord/src/pluralkit.ts:65
fetchPluralKitMessageInfo now routes successful responses through readProviderJsonResponse; the current success fixture returns a lightweight object with json() but no stream body or arrayBuffer(). That mock will throw in the bounded-reader path instead of proving normal PluralKit parsing, so return a real Response or an equivalent body/arrayBuffer-capable test double.
Confidence: 0.88

Overall correctness: patch is incorrect
Overall confidence: 0.86

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

P2: This is normal-priority Discord response-size hardening with limited plugin-local blast radius and no active outage evidence.
merge-risk: 🚨 compatibility: Existing oversized successful Discord JSON responses would change from native parsing to the shared bounded-reader failure behavior.
merge-risk: 🚨 availability: PluralKit lookup, webhook result parsing, or voice upload URL parsing could fail after merge if the new bounded-reader path or response assumptions are wrong.
rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR body includes terminal output from a standalone 1 MiB bounded-read harness, not after-fix proof through one patched Discord runtime path at the shared 16 MiB cap. 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 +11. Total +11 across 3 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	3	17	6	+11
Tests	0	0	0	0
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	3	17	6	+11

What I checked:

Root policy applied: Root and extension-scoped policy require review beyond the diff, plugin SDK boundary checks, compatibility-risk review, and real changed-behavior proof for PRs touching bundled plugins. (AGENTS.md:43, fbfadbd806e1)
Current main still has unbounded Discord success reads: Current main still calls native res.json() or response.json() on the three touched Discord success paths, so the central hardening request is not already implemented on main. (extensions/discord/src/pluralkit.ts:62, fbfadbd806e1)
Shared bounded-reader contract: readProviderJsonResponse uses the shared 16 MiB provider JSON cap and delegates body reading to readResponseWithLimit, which cancels stream readers on overflow. (src/agents/provider-http-errors.ts:312, fbfadbd806e1)
PR diff uses the shared helper: The PR head swaps the three Discord success JSON reads to readProviderJsonResponse, reusing the plugin SDK facade rather than adding a new response reader. (extensions/discord/src/pluralkit.ts:65, 0acda0c2555d)
PluralKit fixture mismatch introduced by the patch: The existing PluralKit success fixture returns status, ok, text(), and json() only; the new reader reaches the bounded-reader fallback when there is no stream body and expects arrayBuffer(), so the focused success test double must be updated. (extensions/discord/src/pluralkit.test.ts:12, fbfadbd806e1)
Proof is helper-level, not changed-path proof: The PR body reports terminal output from test/_proof_bounded_json.mts using a standalone 1 MiB bounded-read harness; it does not drive PluralKit lookup, webhook response parsing, or voice upload URL parsing through the patched 16 MiB helper path. (0acda0c2555d)

Likely related people:

vincentkoc: Recent current-main history bounded Discord PluralKit, webhook, and voice-upload error bodies in the same three files that this PR now changes on success paths. (role: recent adjacent contributor; confidence: high; commits: aea208f0ac51, 4c3b15bae671, ce7f89916522; files: extensions/discord/src/pluralkit.ts, extensions/discord/src/send.webhook.ts, extensions/discord/src/voice-message.ts)
steipete: GitHub file history shows repeated recent Discord plugin and plugin SDK boundary work around webhook sending, voice upload, and related response handling. (role: recent area contributor; confidence: medium; commits: bd20f8e07e92, cd9b2c0af4f5, 827b0de0ce74; files: extensions/discord/src/send.webhook.ts, extensions/discord/src/voice-message.ts, extensions/discord/src/pluralkit.ts)
Alix-007: Authored the shared provider JSON bounded-reader helper and the merged GitHub Copilot bounded success-read precedent that this PR follows. (role: shared bounded-reader contributor; confidence: high; commits: 2592f8a51a4e, 646e54ae3578; files: src/agents/provider-http-errors.ts, src/plugin-sdk/provider-http.ts, extensions/github-copilot/usage.ts)
scoootscooob: Moved the Discord implementation into extensions/discord, establishing the current bundled plugin boundary for the touched files. (role: introduced extension boundary; confidence: low; commits: 5682ec37fada; files: extensions/discord/src/pluralkit.ts, extensions/discord/src/send.webhook.ts, extensions/discord/src/voice-message.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.

lsr911 · 2026-06-26T07:48:49Z

@clawsweeper re-review

clawsweeper · 2026-06-26T07:48:52Z

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

lsr911 · 2026-06-26T09:11:48Z

@clawsweeper re-review

clawsweeper · 2026-06-26T09:11: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.

lsr911 · 2026-06-26T14:33:54Z

@clawsweeper re-review

clawsweeper · 2026-06-26T14:33:58Z

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

lsr911 · 2026-06-26T14:51:30Z

@clawsweeper re-review

- pluralkit: use readProviderJsonResponse for message info lookup - send.webhook: use readProviderJsonResponse for webhook responses - voice-message: use readProviderJsonResponse for upload URL responses Co-Authored-By: Claude <[email protected]> Signed-off-by: lsr911 <[email protected]>

lsr911 · 2026-06-27T01:24:21Z

@clawsweeper re-review

clawsweeper · 2026-06-27T01:24:24Z

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

lsr911 · 2026-06-27T05:21:27Z

@clawsweeper re-review

lsr911 · 2026-06-27T06:06:03Z

@clawsweeper re-review

lsr911 · 2026-06-30T06:13:18Z

Closed: merge conflicts with upstream. Superseded by cleaner single-module PRs covering the same modules.

openclaw-barnacle Bot added channel: discord Channel integration: discord channel: telegram Channel integration: telegram plugin: google-meet extensions: github-copilot size: S labels Jun 25, 2026

lsr911 mentioned this pull request Jun 25, 2026

fix(feishu,browser,msteams,azure-speech,bedrock-mantle,googlechat,huggingface,perplexity): bound JSON response reads #96620

Closed

clawsweeper Bot mentioned this pull request Jun 25, 2026

fix(github-copilot): bound usage response reads #96607

Merged

lsr911 closed this Jun 25, 2026

lsr911 reopened this Jun 26, 2026

openclaw-barnacle Bot added the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jun 26, 2026

mushuiyu886 mentioned this pull request Jun 26, 2026

fix(google-media): bound JSON response reads #96920

Merged

lsr911 force-pushed the fix/batch2-bounded-json branch from 1e680de to 0c250fe Compare June 26, 2026 09:02

lsr911 force-pushed the fix/batch2-bounded-json branch from 0c250fe to 6deeea6 Compare June 26, 2026 14:31

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

clawsweeper Bot mentioned this pull request Jun 27, 2026

Fix/discord bound probe getme json reads #97278

Merged

This was referenced Jul 8, 2026

fix(google-meet): bound Meet v2 + Drive files.export request deadlines #102149

Merged

fix(google-meet): bound Calendar v3 events.list request deadline #102157

Merged

Uh oh!

Uh oh!

Conversation

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

Uh oh!

What Problem This Solves

Evidence

Real HTTP bounded-read proof

Uh oh!

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

Uh oh!

Uh oh!

lsr911 commented Jun 26, 2026

Uh oh!

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

Uh oh!

Uh oh!

lsr911 commented Jun 26, 2026

Uh oh!

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

Uh oh!

Uh oh!

lsr911 commented Jun 26, 2026

Uh oh!

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

Uh oh!

Uh oh!

lsr911 commented Jun 26, 2026

Uh oh!

lsr911 commented Jun 27, 2026

Uh oh!

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

Uh oh!

Uh oh!

lsr911 commented Jun 27, 2026

Uh oh!

lsr911 commented Jun 27, 2026

Uh oh!

lsr911 commented Jun 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

lsr911 commented Jun 25, 2026 •

edited

Loading

clawsweeper Bot commented Jun 25, 2026 •

edited

Loading

clawsweeper Bot commented Jun 26, 2026 •

edited

Loading

clawsweeper Bot commented Jun 26, 2026 •

edited

Loading

clawsweeper Bot commented Jun 26, 2026 •

edited

Loading

clawsweeper Bot commented Jun 27, 2026 •

edited

Loading