fix(extensions/huggingface): bound model discovery JSON response read to prevent OOM by cxbAsDev · Pull Request #101079 · openclaw/openclaw

cxbAsDev · 2026-07-06T17:49:25Z

What Problem This Solves

discoverHuggingfaceModels parsed the successful Hugging Face /v1/models response with response.json(). Node's Fetch body mixin consumes the complete response before parsing, so a runaway provider response could grow memory without a bound during provider-catalog discovery.

Why This Change Was Made

The discovery owner now uses OpenClaw's shared readProviderJsonResponse, which limits provider JSON bodies to 16 MiB, cancels the response stream on overflow, and labels malformed JSON errors. The surrounding discovery flow already catches provider failures and returns the bundled static model catalog, so oversized and malformed responses follow the established fail-open catalog behavior.

The shared default is used directly instead of adding a second Hugging Face-specific copy of the same response-size policy.

User Impact

Hugging Face model discovery remains dynamic for normal responses and falls back to the bundled catalog if the provider sends malformed or unexpectedly large JSON. Authentication, routing, and the model-normalization path are unchanged.

Evidence

Hugging Face documents https://router.huggingface.co/v1/models as the OpenAI-compatible model-list endpoint.
Live read on July 6, 2026: HTTP 200, application/json, 81,363 bytes, 119 models; the production 16 MiB cap leaves substantial headroom.
The overflow regression reuses 1 MiB chunks until the seventeenth read, then asserts static-catalog fallback, stream cancellation, and reader lock release without constructing a giant JSON string.
A small valid streamed JSON control still discovers a dynamic model and completes without cancellation.
Fresh structured autoreview on exact head dda99df922200659c5126d1e453da70fba7b1aec: no accepted/actionable findings.
Final exact-head CI evidence will be posted in a maintainer comment before merge.

clawsweeper · 2026-07-06T18:51:28Z

Codex review: needs real behavior proof before merge. Reviewed July 6, 2026, 6:24 PM ET / 22:24 UTC.

Summary
This PR replaces Hugging Face model discovery's successful response.json() read with readProviderJsonResponse and adds overflow and bounded happy-path tests.

PR surface: Source +4, Tests +62. Total +66 across 2 files.

Reproducibility: yes. source-level: current main and latest release still call native response.json() on the successful Hugging Face discovery response. I did not run an OOM repro, but the unbounded read and bounded replacement path are direct source evidence.

Review metrics: 2 noteworthy metrics.

Success JSON Reader Changed: 1 changed. The single Hugging Face discovery success-response parser now has a 16 MiB fallback boundary that maintainers should accept before merge.
Open Hugging Face Overlaps: 3 open overlaps found. Multiple open PRs touch the same bounded Hugging Face response-read problem, so maintainers should choose one canonical branch.

Stored data model
Persistent data-model change detected: serialized state: extensions/huggingface/models.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 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:

[P1] Add terminal or log proof from a local streamed /models server through discoverHuggingfaceModels, with private details redacted.
Ask maintainers to choose whether this PR or fix(huggingface): bound model-discovery JSON response via shared provider reader #97743 is the canonical landing path.

Proof guidance:

[P1] Needs stronger real behavior proof before merge: The PR body includes live endpoint-size context and mocked stream-reader tests, but no after-fix real run through discoverHuggingfaceModels against an oversized streamed response; terminal output, copied live output, or redacted logs would close the proof gap. 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] A successful Hugging Face discovery response over the shared 16 MiB JSON cap will now fall back to the bundled static catalog instead of being parsed dynamically; that is intended hardening but still a compatibility boundary.
[P1] This PR overlaps multiple open Hugging Face bounded-reader PRs, especially fix(huggingface): bound model-discovery JSON response via shared provider reader #97743, so maintainers should choose one canonical landing path before merging.
[P1] The PR body has live endpoint-size context and mocked stream-reader tests, but it still does not show an after-fix real run through discoverHuggingfaceModels against a streamed oversized response.

Maintainer options:

Require Changed-Path Proof (recommended)
Before merge, require terminal or log output from discoverHuggingfaceModels running against a local streamed oversized /models response, or an explicit maintainer proof override.
Accept The Shared Cap
Maintainers can intentionally accept the 16 MiB fallback boundary because normal live payloads are far smaller and the existing discovery path already falls back to the static catalog on provider failures.
Pause For Canonical PR Choice
If the older proof-sufficient PR remains the preferred landing path, pause or close this branch to avoid duplicate implementations.

Next step before merge

[P1] Manual review should choose the canonical overlapping branch and either require changed-path proof or explicitly override the proof gate before merge.

Maintainer decision needed

Question: Should maintainers adopt this maintainer-touched PR as the canonical Hugging Face bounded-reader fix, or close it in favor of the existing proof-sufficient fix(huggingface): bound model-discovery JSON response via shared provider reader #97743?
Rationale: Both branches implement the same production replacement; this branch is smaller and has fresh maintainer commits, while the overlapping PR already has stronger real HTTP proof.
Likely owner: steipete — They are assigned, authored the latest test commits on this PR, and have the strongest recent Hugging Face discovery history.
Options:
- Adopt This Branch After Proof (recommended): Keep this PR as canonical if maintainers add or accept direct changed-path proof for discoverHuggingfaceModels, then close the overlapping branches.
- Land The Proven Older Branch: Use fix(huggingface): bound model-discovery JSON response via shared provider reader #97743 as the canonical PR if maintainers prefer its already-posted real HTTP proof and broader test coverage.
- Fold Tests Into One Branch: Move the preferred stream cleanup tests to whichever branch maintainers choose and close the duplicate implementation attempts.

Security
Cleared: No concrete supply-chain regression was found; the diff narrows an untrusted provider-response memory boundary and does not change dependencies, workflows, permissions, secrets, or package metadata.

Review details

Best possible solution:

Land exactly one Hugging Face bounded-reader PR that reuses the plugin SDK helper, preserves static-catalog fallback, and has direct changed-path proof or an explicit maintainer proof override.

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

Yes, source-level: current main and latest release still call native response.json() on the successful Hugging Face discovery response. I did not run an OOM repro, but the unbounded read and bounded replacement path are direct source evidence.

Is this the best way to solve the issue?

Yes for the code shape: reusing the plugin SDK bounded JSON reader is the narrow owner-boundary fix. The best landing path remains unsettled until maintainers choose between this branch and the proof-sufficient overlapping PR and resolve proof expectations.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

P2: This is normal-priority bundled-provider hardening for a bounded Hugging Face discovery OOM risk with limited blast radius.
merge-risk: 🚨 compatibility: The PR changes successful Hugging Face model-discovery responses over the shared cap from dynamic parsing to static-catalog fallback.
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 stronger real behavior proof before merge: The PR body includes live endpoint-size context and mocked stream-reader tests, but no after-fix real run through discoverHuggingfaceModels against an oversized streamed response; terminal output, copied live output, or redacted logs would close the proof gap. 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 +4, Tests +62. Total +66 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	5	1	+4
Tests	1	62	0	+62
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	67	1	+66

What I checked:

Repository policy read: Read full root AGENTS.md and scoped extensions/AGENTS.md; the extension boundary requires plugin production code to use openclaw/plugin-sdk/*, which this PR does. (AGENTS.md:1, 17d3c2eb0a80)
Current main behavior: Current main still reads the successful Hugging Face model-discovery response with native response.json(), so the central OOM hardening is not implemented on main. (extensions/huggingface/models.ts:168, 17d3c2eb0a80)
PR head behavior: The PR head imports the public plugin SDK helper and uses readProviderJsonResponse<OpenAIListModelsResponse>(response, "huggingface.model-discovery") on the successful discovery path. (extensions/huggingface/models.ts:169, dda99df92220)
Shared reader contract: readProviderJsonResponse defaults to the shared provider JSON byte cap and throws labeled overflow or malformed JSON errors after bounded body reading. (src/agents/provider-http-errors.ts:315, 17d3c2eb0a80)
Stream cleanup contract: readResponseWithLimit uses readResponsePrefix, which cancels the stream on overflow and releases the reader lock before throwing the overflow error. (src/infra/http-body.ts:229, 17d3c2eb0a80)
PR regression tests: The PR adds tests that drive discoverHuggingfaceModels with a reader exceeding the cap, assert static-catalog fallback plus cancel/release, and assert a small streamed JSON response is parsed. (extensions/huggingface/models.test.ts:120, dda99df92220)

Likely related people:

steipete: They are assigned to this PR, authored the latest test cleanup commits on the branch, and recent main history shows Hugging Face discovery timeout/ref and provider-catalog work by them. (role: recent Hugging Face discovery contributor and current branch adopter; confidence: high; commits: dda99df92220, 58a611a64bbf, 5ac07b8ef086; files: extensions/huggingface/models.ts, extensions/huggingface/models.test.ts, extensions/huggingface/provider-catalog.ts)
vincentkoc: Current checked-out blame around discoverHuggingfaceModels points to a broad plugin/discovery cleanup commit, and GitHub history shows adjacent model-discovery environment guard work. (role: current main line provenance and adjacent discovery contributor; confidence: medium; commits: 4ae9ae12b62d, 52cd76a9e2d0; files: extensions/huggingface/models.ts)
RomneyDa: Recent main history for src/agents/provider-http-errors.ts shows they consolidated the bounded HTTP body reader that this PR now reuses through the plugin SDK. (role: recent shared bounded-reader contributor; confidence: medium; commits: 6c53dfa1df99; files: src/agents/provider-http-errors.ts, src/infra/http-body.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.

Review history (1 earlier review cycle)

reviewed 2026-07-06T21:52:36.496Z sha 58a611a :: needs real behavior proof before merge. :: none

… to prevent OOM

steipete · 2026-07-06T22:25:50Z

Maintainer land-ready proof

Reviewed exact head: dda99df922200659c5126d1e453da70fba7b1aec

The model-discovery success body is now consumed at the Hugging Face plugin's HTTP boundary through the shared 16 MiB provider JSON reader. Overflow cancels the response stream and enters the existing static-catalog fallback; a valid small streamed response still discovers dynamic models.

Evidence:

Hugging Face documents the OpenAI-compatible model-list endpoint. A live request on July 6, 2026 returned HTTP 200, application/json, 81,363 bytes, and 119 models.
The focused regression reuses 1 MiB chunks, observes the seventeenth read crossing the shared cap, and asserts stream cancellation, reader-lock release, and static-catalog fallback. The small-response control asserts normal discovery without cancellation.
Exact-SHA release gate: https://github.com/openclaw/openclaw/actions/runs/28826762727 — success, with no failed jobs.
Fresh structured autoreview on the exact head: no accepted/actionable findings; overall correctness confidence 0.86.

Local review commands:

git diff --check origin/main...dda99df922200659c5126d1e453da70fba7b1aec
.agents/skills/autoreview/scripts/autoreview --mode branch --base origin/main --engine codex --model gpt-5.5 --thinking high
scripts/pr review-validate-artifacts 101079

Sanitized direct AWS Crabbox was attempted twice, but the broker rejected lease creation with HTTP 500 code 1101. No contributor code ran on a credential-hydrated Testbox; the exact-SHA release gate is the execution proof backend.

Best-fix judgment: this is the narrow owner-boundary fix. A downstream model-list guard would buffer too late, and a plugin-local response helper or cap would duplicate the shared provider HTTP contract. Direct duplicates #97743 and #96480 can close after this lands; #98941 retains distinct Discord and thread-ownership changes.

No screenshot applies: this is a provider response-stream boundary change.

steipete · 2026-07-06T22:26:31Z

Merged via squash.

Prepared head SHA: dda99df922200659c5126d1e453da70fba7b1aec
Landed commit: a464620141f6aad45da8cbcc45d49d68d27fa1af

… to prevent OOM (openclaw#101079) * fix(extensions/huggingface): bound model discovery JSON response read to prevent OOM * test(huggingface): prove bounded discovery cleanup * test(huggingface): avoid unbound reader assertions --------- Co-authored-by: Peter Steinberger <[email protected]>

…mantics The thread-ownership 409 conflict path still used unbounded resp.json(), inconsistent with the bounded readers shipped across the other channel/provider success paths. A compromised or buggy forwarder returning a multi-GB 409 body would buffer the entire body before JSON.parse, OOMing the agent. Bound the 409 body via readResponseTextLimited (8 KiB cap; a 409 owner field is ~30 bytes). Wrap the parse in try/catch so a truncated or malformed 409 body still cancels the send (owner logged as "unknown") — the 409 status itself means another agent owns this thread, independent of body parseability. The discord webhook and huggingface /v1/models success reads from the original PR were superseded upstream by openclaw#98098 and openclaw#101079 (readProviderJsonResponse), so this branch now carries only the thread-ownership fix. Co-Authored-By: Claude <[email protected]>

… to prevent OOM (openclaw#101079) * fix(extensions/huggingface): bound model discovery JSON response read to prevent OOM * test(huggingface): prove bounded discovery cleanup * test(huggingface): avoid unbound reader assertions --------- Co-authored-by: Peter Steinberger <[email protected]>

…mantics The thread-ownership 409 conflict path still used unbounded resp.json(), inconsistent with the bounded readers shipped across the other channel/provider success paths. A compromised or buggy forwarder returning a multi-GB 409 body would buffer the entire body before JSON.parse, OOMing the agent. Bound the 409 body via readResponseTextLimited (8 KiB cap; a 409 owner field is ~30 bytes). Wrap the parse in try/catch so a truncated or malformed 409 body still cancels the send (owner logged as "unknown") — the 409 status itself means another agent owns this thread, independent of body parseability. The discord webhook and huggingface /v1/models success reads from the original PR were superseded upstream by openclaw#98098 and openclaw#101079 (readProviderJsonResponse), so this branch now carries only the thread-ownership fix. Co-Authored-By: Claude <[email protected]>

… to prevent OOM (openclaw#101079) * fix(extensions/huggingface): bound model discovery JSON response read to prevent OOM * test(huggingface): prove bounded discovery cleanup * test(huggingface): avoid unbound reader assertions --------- Co-authored-by: Peter Steinberger <[email protected]> (cherry picked from commit a464620)

…mantics (#98941) * fix(thread-ownership): bound 409 response read and preserve cancel semantics The thread-ownership 409 conflict path still used unbounded resp.json(), inconsistent with the bounded readers shipped across the other channel/provider success paths. A compromised or buggy forwarder returning a multi-GB 409 body would buffer the entire body before JSON.parse, OOMing the agent. Bound the 409 body via readResponseTextLimited (8 KiB cap; a 409 owner field is ~30 bytes). Wrap the parse in try/catch so a truncated or malformed 409 body still cancels the send (owner logged as "unknown") — the 409 status itself means another agent owns this thread, independent of body parseability. The discord webhook and huggingface /v1/models success reads from the original PR were superseded upstream by #98098 and #101079 (readProviderJsonResponse), so this branch now carries only the thread-ownership fix. Co-Authored-By: Claude <[email protected]> * fix(thread-ownership): bound forwarder conflict handling --------- Co-authored-by: Claude <[email protected]>

…mantics (openclaw#98941) * fix(thread-ownership): bound 409 response read and preserve cancel semantics The thread-ownership 409 conflict path still used unbounded resp.json(), inconsistent with the bounded readers shipped across the other channel/provider success paths. A compromised or buggy forwarder returning a multi-GB 409 body would buffer the entire body before JSON.parse, OOMing the agent. Bound the 409 body via readResponseTextLimited (8 KiB cap; a 409 owner field is ~30 bytes). Wrap the parse in try/catch so a truncated or malformed 409 body still cancels the send (owner logged as "unknown") — the 409 status itself means another agent owns this thread, independent of body parseability. The discord webhook and huggingface /v1/models success reads from the original PR were superseded upstream by openclaw#98098 and openclaw#101079 (readProviderJsonResponse), so this branch now carries only the thread-ownership fix. Co-Authored-By: Claude <[email protected]> * fix(thread-ownership): bound forwarder conflict handling --------- Co-authored-by: Claude <[email protected]>

openclaw-barnacle Bot added extensions: huggingface size: XS labels Jul 6, 2026

cxbAsDev force-pushed the fix/huggingface-models-json-response-bound branch from 0f93d27 to fd7bed5 Compare July 6, 2026 17:59

steipete self-assigned this Jul 6, 2026

openclaw-barnacle Bot added size: S and removed size: XS labels Jul 6, 2026

cxbAsDev and others added 2 commits July 6, 2026 17:34

fix(extensions/huggingface): bound model discovery JSON response read…

9f73e24

… to prevent OOM

test(huggingface): prove bounded discovery cleanup

58a611a

steipete force-pushed the fix/huggingface-models-json-response-bound branch from 9b3d800 to 58a611a Compare July 6, 2026 21:34

test(huggingface): avoid unbound reader assertions

dda99df

steipete merged commit a464620 into openclaw:main Jul 6, 2026
205 checks passed

This was referenced Jul 6, 2026

fix(huggingface): bound model-discovery JSON response via shared provider reader #97743

Closed

fix(agents): bound model catalog JSON response reads for bedrock-mantle and huggingface #96480

Closed

github-actions Bot mentioned this pull request Jul 6, 2026

📡 Upstream Digest — 2026-07-06 22:59 UTC curtismercier/openclaw-mods#1226

Open

miorbnli mentioned this pull request Jul 8, 2026

fix(thread-ownership): bound 409 response read and preserve cancel semantics #98941

Merged

cxbAsDev deleted the fix/huggingface-models-json-response-bound branch July 15, 2026 03:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(extensions/huggingface): bound model discovery JSON response read to prevent OOM#101079

fix(extensions/huggingface): bound model discovery JSON response read to prevent OOM#101079
steipete merged 3 commits into
openclaw:mainfrom
cxbAsDev:fix/huggingface-models-json-response-bound

cxbAsDev commented Jul 6, 2026 •

edited by steipete

Loading

Uh oh!

clawsweeper Bot commented Jul 6, 2026 •

edited

Loading

Uh oh!

steipete commented Jul 6, 2026

Uh oh!

Uh oh!

steipete commented Jul 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

cxbAsDev commented Jul 6, 2026 • edited by steipete 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 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

steipete commented Jul 6, 2026

Maintainer land-ready proof

Uh oh!

Uh oh!

steipete commented Jul 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

cxbAsDev commented Jul 6, 2026 •

edited by steipete

Loading

clawsweeper Bot commented Jul 6, 2026 •

edited

Loading