Skip to content

fix(anthropic): bound streaming 200 success-body SSE reads via shared plugin-sdk guard#96632

Closed
wangmiao0668000666 wants to merge 1 commit into
openclaw:mainfrom
wangmiao0668000666:fix/anthropic-stream-bound-read
Closed

fix(anthropic): bound streaming 200 success-body SSE reads via shared plugin-sdk guard#96632
wangmiao0668000666 wants to merge 1 commit into
openclaw:mainfrom
wangmiao0668000666:fix/anthropic-stream-bound-read

Conversation

@wangmiao0668000666

@wangmiao0668000666 wangmiao0668000666 commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

src/agents/anthropic-transport-stream.ts and src/llm/providers/anthropic.ts accumulate an unbounded SSE byte stream into a string buffer while waiting for \n\n frame delimiters. The companion error-body path (readAnthropicMessagesErrorBodySnippet, 8 KiB cap + 10s idle timeout) was bounded by #95108, but the symmetric 200 success-body path was left open. A hostile or malfunctioning Anthropic-compatible endpoint (including an MITM-able proxy, a cloud-hosted mirror like Anthropic-Vertex, or a hijacked self-hosted provider) can return a Content-Length-less SSE body that streams forever, exhausting process memory on a code path that runs for every /v1/messages call.

This is the streaming-body analogue of the bounded-read sweep Alix-007 finished for non-streaming response.json() / response.text() (#95218, #95223, #96027, #96035, #96038, #96042): same OOM family, same helper-driven design, same 16 MiB cap, but on the chunk-by-chunk SSE path that Alix-007's sweep did not cover.

ClawSweeper review follow-up (r2)

The first review (🦐 gold shrimp) flagged the helper as a new public plugin-SDK subpath needing maintainer API-boundary acceptance plus regenerated SDK metadata. Following Maintainer Option 2 ("Keep The Guard Internal For Now"), the helper now lives at src/agents/streaming-byte-guard.ts as a core-internal helper mirroring the existing readResponseWithLimit / readProviderJsonResponse family in src/agents/provider-http-errors.ts. Both Anthropic SSE parsers consume it via relative imports. The public SDK subpath, the package.json export, the scripts/lib/plugin-sdk-entrypoints.json entry, and the docs row are all removed. SDK API baseline hash (docs/.generated/plugin-sdk-api-baseline.sha256) is unchanged from upstream main. When an extension (e.g. extensions/google, extensions/ollama) genuinely needs the helper, it will be promoted back through the SDK seam in its own follow-up PR.

Changes

  • src/agents/streaming-byte-guard.ts (new, ~108 lines) — createSseByteGuard helper. Internal core helper (NOT a plugin-SDK subpath). Wraps a ReadableStreamDefaultReader<Uint8Array>, tracks accumulated bytes across the caller's existing chunk loop, and cancels the underlying reader + throws a canonical overflow error once maxBytes is exceeded. Exposes read() / cancel() / totalBytes() / overflowed() so the same guard drives both production and AbortSignal-handling code paths.
  • src/agents/anthropic-transport-stream.ts:691parseAnthropicSseBodyForTest now wires the guard with ANTHROPIC_MESSAGES_SUCCESS_BODY_MAX_BYTES = 16 * 1024 * 1024. The existing readAnthropicSseChunk type was widened to accept any { read, cancel } so the same helper drives the guarded reader.
  • src/llm/providers/anthropic.ts:355iterateSseMessages (the older provider's SSE parser) gets the same bound, with its own constant to keep the two call sites independently readable.
  • src/agents/anthropic-transport-stream.test.ts — two new tests:
    1. bounds streamed Anthropic success responses without content-length drives parseAnthropicSseBodyForTest against a hostile 1 MiB-per-pull streaming body, asserts the canonical overflow message (Anthropic Messages success body exceeded 16777216 bytes), confirms cancel(reason) was invoked, and verifies pullCount is bounded well below the unconstrained ceiling.
    2. parses normal Anthropic success SSE responses end-to-end ensures small valid SSE frames still parse through the bounded reader unchanged.

Real behavior proof

  • Behavior addressed: unbounded buffering of the Anthropic Messages streaming 200 success-body SSE read; after the fix the read is capped at 16 MiB and the stream is cancelled on overflow.
  • Real environment tested: a real node:http server bound to 127.0.0.1 returning a Content-Length-less 64 MiB body (4× the cap) chunk-by-chunk with backpressure, plus a small (8 MiB) negative-control body, plus a happy-path body. Drives the production createSseByteGuard from src/agents/streaming-byte-guard.ts against both Anthropic SSE parsers. Run with pnpm exec tsx scripts/repro/issue-anthropic-sse-bound-read.mjs.
  • Exact steps:
    • node scripts/run-vitest.mjs anthropic-transport-stream anthropic.test --run — 115/115 passed (2 files, includes 2 new tests)
    • pnpm exec tsx scripts/repro/issue-anthropic-sse-bound-read.mjs — 5/5 PASS
    • node scripts/run-tsgo.mjs — clean
    • node scripts/run-oxlint.mjs — clean
    • node scripts/sync-plugin-sdk-exports.mjs --checkplugin-sdk exports synced.
    • node scripts/generate-plugin-sdk-api-baseline.ts --check — exit 0 (baseline hash unchanged from upstream main)
  • Evidence after fix:
    PASS  Anthropic success body bounded: rejected with "Anthropic Messages success body exceeded 16777216 bytes (received 16834166)..."; bytesSent=17825792 (< 67108864); server.aborted=true
    PASS  cap-trace: bounded reader cancelled at ~18874368 bytes (full body = 67108864); server.aborted=true
    PASS  negative control: small body fully drained (8388608 bytes >= 8388608); bounded reader saw 8388608 bytes; cap did not trigger; aborted=false
    PASS  happy path: small valid Anthropic SSE response drained end-to-end (360 bytes <= 16777216)
    PASS  legacy-path parity: anthropic.ts iterateSseMessages overflow surfaces "legacy path: Anthropic Messages success body exceeded 16777216 bytes (received 1..."; bytesSent=18874368; server.aborted=true
    
  • Observed result after fix: createSseByteGuard cancelled the upstream ReadableStreamDefaultReader after ~17-19 MiB (≪ 64 MiB). Server-side bytesSent observed at 17,825,792 — bounded, not drained. cancel(reason) received an Error instance as the cancel reason. Negative control confirms the cap is the cause of the overflow, not body structure.
  • What was not tested: live Anthropic API call (this proof exercises the same production helper against the same shape of attack the live endpoint could carry); cross-platform Node 18/20 differences (Node 22 only, matches CI).

Out of scope (separate PRs)

  • The same parseAnthropicSseBody pattern recurs in 5 other streaming code paths (openai-chatgpt-responses.ts, runtime/proxy.ts, extensions/google/transport-stream.ts, extensions/ollama/src/stream.ts, plus a 6th in the WhatsApp stalled-read family) — each will get its own PR. The core PRs (openai-chatgpt-responses.ts + runtime/proxy.ts) will reuse the same internal helper via relative import. The extension PRs (extensions/google + extensions/ollama) will promote the helper back to a public plugin-SDK subpath with the full SDK metadata synchronization, since extensions cannot import src/agents/** per the boundary rule in src/plugin-sdk/CLAUDE.md.

Risk

  • Low: same 16 MiB cap already used by readProviderJsonResponse for non-streaming Anthropic success bodies. The SSE parser's behavior for normal-sized responses is unchanged — only the unbounded case now throws a clear overflow error instead of buffering without limit.
  • Cancel propagation: createSseByteGuard.cancel(reason) propagates the overflow error to the underlying reader; verified by expect(cancelReason).toBeInstanceOf(Error) in the test.
  • SDK surface: ZERO public SDK surface change in this PR. The helper is core-internal; SDK API baseline hash is unchanged from upstream main. The follow-up extension PRs will own the SDK promotion when it becomes necessary.

@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation scripts Repository scripts agents Agent runtime and tooling size: L labels Jun 25, 2026
@clawsweeper

clawsweeper Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Codex review: found issues before merge. Reviewed June 25, 2026, 12:13 AM ET / 04:13 UTC.

Summary
The PR adds an internal streaming byte guard, applies a 16 MiB cap to both Anthropic Messages SSE success-body parser paths, and adds focused tests plus a local repro script.

PR surface: Source +138, Tests +76, Other +415. Total +629 across 5 files.

Reproducibility: yes. Source inspection shows current main appends delimiter-free Anthropic SSE success chunks into unbounded buffers in both parser paths, and the PR body includes after-fix local-server proof with overflow and negative-control cases.

Review metrics: 2 noteworthy metrics.

  • Anthropic SSE success-body caps: 2 added. Both Anthropic success-body parser paths now fail closed at 16 MiB, which is the compatibility boundary maintainers need to notice before merge.
  • Public plugin-SDK surface: 0 added, 0 changed. The r2 branch keeps the guard internal and avoids the public plugin API expansion that the earlier review flagged.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
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] Fix or remove the misleading overflowed()/post-overflow read contract in the internal guard.
  • Update the repro script comment so it no longer claims a plugin-SDK surface.
  • Have a maintainer accept or tune the 16 MiB Anthropic SSE success-body boundary.

Risk before merge

  • [P1] The 16 MiB guard intentionally changes unusually large Anthropic-compatible streaming success responses from continuing to throwing and cancelling, so maintainers should explicitly accept or tune that compatibility boundary before merge.

Maintainer options:

  1. Accept The 16 MiB Anthropic Cap (recommended)
    Maintainers can accept the new fail-closed boundary because it matches the established provider JSON cap and the PR includes live local-server proof for overflow cancellation and normal small responses.
  2. Tune The Cap Before Merge
    If legitimate Anthropic-compatible SSE success streams can exceed 16 MiB, adjust the constant and update the focused tests plus repro output before landing.
  3. Pause For Shared Streaming Policy
    If core, proxy, Google, Ollama, and other SSE surfaces should share one maintainer-owned policy first, pause this PR until that boundary is decided.

Next step before merge

  • [P1] The remaining blocker is maintainer acceptance or tuning of the 16 MiB fail-closed compatibility boundary; the P3 cleanup is small but not the main decision.

Security
Cleared: The diff is security hardening and does not add dependencies, lockfile changes, secret handling, CI permission changes, or downloaded third-party execution paths.

Review findings

  • [P3] Align overflow state with the helper contract — src/agents/streaming-byte-guard.ts:74-75
  • [P3] Update the repro script SDK-surface comment — scripts/repro/issue-anthropic-sse-bound-read.mjs:12-13
Review details

Best possible solution:

Land the internal Anthropic SSE guard after maintainers accept or adjust the 16 MiB fail-closed boundary and the helper/repro cleanup is addressed; leave SDK promotion to a separate plugin-owned follow-up.

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

Yes. Source inspection shows current main appends delimiter-free Anthropic SSE success chunks into unbounded buffers in both parser paths, and the PR body includes after-fix local-server proof with overflow and negative-control cases.

Is this the best way to solve the issue?

Yes, with small cleanup. Keeping the guard internal is the narrow owner-boundary fix for the two core Anthropic paths, while the exact 16 MiB fail-closed boundary needs maintainer acceptance or tuning.

Full review comments:

  • [P3] Align overflow state with the helper contract — src/agents/streaming-byte-guard.ts:74-75
    overflowed() currently returns the generic cancelled flag, which is also set by explicit cancel(), and subsequent reads after overflow return done even though the comment says they throw. Either remove this unused state surface or track overflow separately so the helper does not grow a misleading internal contract.
    Confidence: 0.86
  • [P3] Update the repro script SDK-surface comment — scripts/repro/issue-anthropic-sse-bound-read.mjs:12-13
    This comment still says the helper comes from a new plugin-SDK surface, but r2 moved it to src/agents/streaming-byte-guard.ts and removed the SDK export. Leaving the stale claim conflicts with the PR's boundary proof.
    Confidence: 0.93

Overall correctness: patch is correct
Overall confidence: 0.89

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P2: This is normal-priority provider memory hardening with limited blast radius and a clear maintainer compatibility decision before merge.
  • merge-risk: 🚨 compatibility: The PR changes existing oversized Anthropic streaming success responses from continuing to throwing and cancelling at 16 MiB.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster 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 after-fix live output from a real local node:http streaming server showing overflow cancellation, negative control, happy path, and legacy-path parity.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix live output from a real local node:http streaming server showing overflow cancellation, negative control, happy path, and legacy-path parity.
Evidence reviewed

PR surface:

Source +138, Tests +76, Other +415. Total +629 across 5 files.

View PR surface stats
Area Files Added Removed Net
Source 3 148 10 +138
Tests 1 76 0 +76
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 1 415 0 +415
Total 5 639 10 +629

What I checked:

  • Repository policy read: Root AGENTS.md and scoped src/agents/AGENTS.md plus scripts/AGENTS.md were read; the review applied the policy that provider/runtime fail-closed changes are compatibility-sensitive and that scoped script wrappers and comments should stay aligned. (AGENTS.md:21, e7f2b125f628)
  • Current main transport parser is unbounded: Current main reads raw Anthropic transport SSE chunks and appends decoded data into an in-memory buffer before delimiter parsing without a success-body byte cap. (src/agents/anthropic-transport-stream.ts:673, e7f2b125f628)
  • Current main legacy parser is unbounded: The legacy Anthropic provider path also reads raw chunks and appends into a buffer while consuming SSE lines, with no success-body byte cap. (src/llm/providers/anthropic.ts:345, e7f2b125f628)
  • PR caps the transport Anthropic SSE path: The PR wraps the transport SSE reader with createSseByteGuard using ANTHROPIC_MESSAGES_SUCCESS_BODY_MAX_BYTES and a canonical overflow error. (src/agents/anthropic-transport-stream.ts:687, 73693b5abbef)
  • PR caps the legacy Anthropic provider path: The PR applies the same internal guard and 16 MiB overflow message to iterateSseMessages in the older provider implementation. (src/llm/providers/anthropic.ts:351, 73693b5abbef)
  • Existing 16 MiB cap precedent: Current main already uses a 16 MiB cap for provider JSON and binary success-body reads through readResponseWithLimit, which supports the cap choice while still leaving the SSE fail-closed compatibility decision for maintainers. (src/agents/provider-http-errors.ts:15, e7f2b125f628)

Likely related people:

  • Shakker: Current-main blame for both Anthropic SSE parser loops points to fa6a950 in this checkout. (role: recent area contributor; confidence: medium; commits: fa6a9509bc95; files: src/agents/anthropic-transport-stream.ts, src/llm/providers/anthropic.ts)
  • vincentkoc: Authored and merged the sibling Anthropic error-body bounded-read PR that hardened the non-OK stream path. (role: related Anthropic stream hardening contributor; confidence: medium; commits: d6cefe26f499, bc68fbf70cc6; files: src/agents/anthropic-transport-stream.ts, src/agents/anthropic-transport-stream.test.ts)
  • Alix-007: Authored the merged provider JSON bounded-read PR that established the 16 MiB success-body cap pattern this PR mirrors. (role: adjacent bounded-read campaign contributor; confidence: medium; commits: 2592f8a51a4e, a15f8e3aaac5; files: src/agents/provider-http-errors.ts, src/agents/provider-http-errors.test.ts)
  • joshavant: Recent main history includes successful provider response read bounding in the shared provider HTTP helper area. (role: recent bounded-read maintainer; confidence: low; commits: 0a14444924e3; files: src/agents/provider-http-errors.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 proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. labels Jun 25, 2026
@wangmiao0668000666
wangmiao0668000666 force-pushed the fix/anthropic-stream-bound-read branch from 539054c to 395cbdf Compare June 25, 2026 03:01
@openclaw-barnacle openclaw-barnacle Bot removed the docs Improvements or additions to documentation label Jun 25, 2026
@wangmiao0668000666

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Following your review verdict (🦐 gold shrimp) and the three maintainer options you surfaced, I picked Option 2 — Keep The Guard Internal For Now:

What changed in r2:

  • src/plugin-sdk/stream-reader-with-limit.ts is deleted.
  • The same helper now lives at src/agents/streaming-byte-guard.ts as a core-internal helper, mirroring the existing readResponseWithLimit / readProviderJsonResponse family in src/agents/provider-http-errors.ts.
  • Both Anthropic SSE parsers (src/agents/anthropic-transport-stream.ts:691 and src/llm/providers/anthropic.ts:355) consume the helper via relative imports — no plugin-SDK boundary touched.
  • The package.json "./plugin-sdk/stream-reader-with-limit" export is removed.
  • The scripts/lib/plugin-sdk-entrypoints.json:314 entry is removed.
  • The docs/plugins/sdk-subpaths.md row is removed.

Closes the P1 blockers from your previous review:

  1. "This PR adds one new public plugin-SDK subpath; that becomes third-party plugin API surface and needs maintainer acceptance" — Resolved: zero new SDK surface in this PR.
  2. "The generated plugin-SDK API baseline hash and package-export sync state are not shown as updated" — Resolved: docs/.generated/plugin-sdk-api-baseline.sha256 is unchanged from upstream main; plugin-sdk:check-exports reports plugin-sdk exports synced.

Should also resolve the three CI failures (check-lint, check-test-types, checks-node-compact-small-whole-2) which cascaded from the SDK metadata drift.

Re-verification (all green locally):

  • node scripts/run-vitest.mjs anthropic-transport-stream anthropic.test — 115/115 passed
  • pnpm exec tsx scripts/repro/issue-anthropic-sse-bound-read.mjs — 5/5 PASS (real node:http server, 64 MiB body)
  • node scripts/run-tsgo.mjs — clean
  • node scripts/run-oxlint.mjs — clean
  • node scripts/sync-plugin-sdk-exports.mjs --check — synced
  • node scripts/generate-plugin-sdk-api-baseline.ts --check — exit 0

When does the helper get promoted to a public SDK subpath? Only when an external plugin (planned: extensions/google + extensions/ollama) actually needs it. Extensions can't import src/agents/** per src/plugin-sdk/CLAUDE.md, so they will own their own SDK-promotion PR with full metadata sync — keeping this PR a clean bounded-refactor with no API surface change.

@clawsweeper

clawsweeper Bot commented Jun 25, 2026

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 force-pushed the fix/anthropic-stream-bound-read branch from 395cbdf to ec7cb9f Compare June 25, 2026 03:12
@wangmiao0668000666

Copy link
Copy Markdown
Contributor Author

Additional fix in r2 (force-pushed as ec7cb9f6e8):

  • check-lint was still failing on src/agents/anthropic-transport-stream.test.ts:378 (typescript(no-unnecessary-type-assertion)). After widening the parseAnthropicSseBodyForTest reader type to accept the duck-typed {read, cancel} interface, the event as Record<string, unknown> cast was no longer needed. Removed it.

Re-verification after the additional lint fix:

  • node scripts/run-oxlint.mjs --tsconfig tsconfig.json <modified files> — exit 0
  • node scripts/run-vitest.mjs anthropic-transport-stream anthropic.test — 115/115 passed
  • pnpm exec tsx scripts/repro/issue-anthropic-sse-bound-read.mjs — 5/5 PASS

CI for check-lint was the last remaining hard failure after the SDK-surface revert. All other originally-failing checks (check-test-types, checks-node-compact-small-whole-2) were already passing on r2's first commit. This additional commit should clear check-lint and leave the r2 PR fully green for the reviewer pass.

@clawsweeper clawsweeper Bot added 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: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Jun 25, 2026
… internal guard

ClawSweeper review follow-up: revert stream-reader-with-limit from a new
public plugin-SDK subpath to an internal helper under src/agents/. The new
subpath required maintainer API-boundary acceptance plus regenerated SDK
metadata; deferring that to the extension follow-up PRs (extensions/google
+ extensions/ollama) keeps this PR a clean bounded-refactor with no public
SDK surface change.

The helper now lives at src/agents/streaming-byte-guard.ts, mirroring the
existing readResponseWithLimit / readProviderJsonResponse family in
src/agents/provider-http-errors.ts. Both Anthropic SSE parsers (the new
src/agents/anthropic-transport-stream.ts parser and the legacy
src/llm/providers/anthropic.ts iterateSseMessages) consume the helper via
relative imports, no plugin-sdk boundary involved.

Removal summary:
- DELETE src/plugin-sdk/stream-reader-with-limit.ts
- CREATE src/agents/streaming-byte-guard.ts (rename with comment update)
- UPDATE src/agents/anthropic-transport-stream.ts (relative import)
- UPDATE src/llm/providers/anthropic.ts (relative import)
- UPDATE scripts/repro/issue-anthropic-sse-bound-read.mjs (relative path)
- REMOVE entry from scripts/lib/plugin-sdk-entrypoints.json (line 314)
- REMOVE ./plugin-sdk/stream-reader-with-limit export from package.json
- REMOVE row from docs/plugins/sdk-subpaths.md
- REMOVE two now-unused @ts-expect-error directives in anthropic-transport-stream.test.ts

Re-verification:
- vitest: 115/115 (src/agents/anthropic-transport-stream.test.ts + src/llm/providers/anthropic.test.ts)
- repro script: 5/5 PASS (real node:http server, 64 MiB content-length-less body)
- tsgo: clean
- oxlint: clean
- plugin-sdk:check-exports: synced
- docs/.generated/plugin-sdk-api-baseline.sha256: unchanged (SDK surface restored)

Closes ClawSweeper P1 blockers from review on this PR: new public SDK subpath
requires maintainer acceptance (deferred), SDK metadata drift (resolved by
removal).
@wangmiao0668000666
wangmiao0668000666 force-pushed the fix/anthropic-stream-bound-read branch from ec7cb9f to 73693b5 Compare June 25, 2026 04:03
wangmiao0668000666 added a commit to wangmiao0668000666/openclaw that referenced this pull request Jun 25, 2026
…red internal guard

Same OOM family as PR openclaw#96632 (Anthropic SSE success-body bound), applied to the
two remaining core streaming-body surfaces that read response.body.getReader()
without an accumulation cap:

  - src/llm/providers/openai-chatgpt-responses.ts (parseSSE, ChatGPT Responses)
  - src/agents/runtime/proxy.ts (streamProxy, internal proxy route)

Both surfaces accumulate decoder-decoded bytes into a string buffer while
waiting for the next frame delimiter, with no upper bound. A hostile or
malfunctioning endpoint (ChatGPT Responses, custom proxy backend) can return
a Content-Length-less SSE body that streams forever, exhausting process memory
on every /v1/messages or proxied-stream call.

Fix: wrap the existing response.body.getReader() in the shared
src/agents/streaming-byte-guard.ts createSseByteGuard helper (introduced in
openclaw#96632, now reused). 16 MiB cap, matching readProviderJsonResponse. The
existing chunk-by-chunk parsing loop, frame delimiter handling, and signal
abort checks are unchanged.

For proxy.ts the abort handler also routes through guard.cancel() so the
bounded cancel propagates immediately on signal abort (with a fallback to
reader.cancel() for the brief window before the guard is constructed).

Tests:
  - src/llm/providers/openai-chatgpt-responses.test.ts: new describe block
    drives parseSSEForTest (renamed from parseSSE for direct testing) against
    a hostile 1 MiB-per-pull streaming body, asserts canonical overflow message
    and bounded pullCount (17-20 pulls), plus a happy-path end-to-end parse.
  - src/agents/runtime/proxy.test.ts: new integration test stubs fetch with
    a hostile ReadableStream body, asserts the stream ends with the bounded
    error and pullCount is bounded.

Real-environment proof: scripts/repro/issue-openai-proxy-sse-bound-read.mjs
runs against a real node:http server bound to 127.0.0.1 that streams a
Content-Length-less 64 MiB body chunk-by-chunk. Five PASS lines:
  1. bounded rejection: bytesSent ~19 MiB (<< 64 MiB), server.aborted=true
  2. cap-trace: bounded reader cancelled at ~16 MiB
  3. negative control: small body fully drained, no cap
  4. happy path: small valid OpenAI SSE drained end-to-end
  5. legacy-path parity: parseSSEForTest overflow surfaces canonical error

Stacked on fix/anthropic-stream-bound-read (openclaw#96632); no new public SDK
subpath, no SDK metadata drift, no plugin boundary touched.

Stacked-on: fix/anthropic-stream-bound-read
@wangmiao0668000666

Copy link
Copy Markdown
Contributor Author

Closing in favor of the Alix-007-style XS split.

Why: This PR's diff (L size, 5 files, 639 LoC) is bigger than the per-surface pattern Alix-007 uses for the bounded-read family (XS, 2 files, 50–135 LoC — see #95218, #95417, #96027, #96035, #96038, #96042, #95244, #96607). ClawSweeper's gold-shrimp verdict reflects patch-quality concerns about diff size, not proof quality.

What happens to this work:

  • The helper (src/agents/streaming-byte-guard.ts) and 2 anthropic call sites are preserved in branch fix/anthropic-stream-bound-read.
  • They will land as part of a new Foundation PR (promote the helper to plugin-SDK via openclaw/plugin-sdk/stream-reader-with-limit, with the 4 core consumers updated to the SDK import path) followed by per-surface application PRs in extensions/.

Diff size analysis:

Alix-007 #96607 My #96632
46 LoC, XS, 2 files 639 LoC, L, 5 files
Reuses existing SDK helper Creates new helper
Inline test (~40 LoC) Separate describe block (~76 LoC)
No committed repro 415-line committed repro script

Following the per-surface split will resolve both the ClawSweeper patch-quality flag and the staging-order concern.

wangmiao0668000666 added a commit to wangmiao0668000666/openclaw that referenced this pull request Jun 25, 2026
…(provider path)

Apply createSseByteGuard to src/llm/providers/anthropic.ts (legacy provider
iterateSseMessages) so the Anthropic Messages provider SSE parser cannot
be exhausted by a hostile or malfunctioning Anthropic-compatible endpoint
that streams an unbounded SSE body. The 16 MiB cap matches the
non-streaming readProviderJsonResponse cap.

What changed:
- src/llm/providers/anthropic.ts: wrap body.getReader() in
  createSseByteGuard before the existing iterateSseMessages loop. The
  function is also re-exported as iterateSseMessagesForTest for direct
  test access.
- Inline test added to anthropic.test.ts: hostile 1 MiB pull stream,
  asserts canonical overflow message, cancel(reason) was an Error
  instance, pullCount bounded to 17-20.

Reuses the createSseByteGuard helper from src/agents/streaming-byte-guard.ts
(introduced in openclaw#96701 for the anthropic-transport-stream path; same
helper, same 16 MiB cap, same overflow-error shape). This PR is the
second of the planned per-surface rescue series for the previously
closed PR openclaw#96632, after openclaw#96701.

No SDK surface change. No repro script committed (proof in this body).
The companion error-body path is already bounded (readAnthropicMessages
ErrorBodySnippet, 8 KiB + 10s idle timeout, openclaw#95108); this PR closes the
legacy-provider success-body gap.
wangmiao0668000666 added a commit to wangmiao0668000666/openclaw that referenced this pull request Jun 27, 2026
…(provider path)

Apply createSseByteGuard to src/llm/providers/anthropic.ts (legacy provider
iterateSseMessages) so the Anthropic Messages provider SSE parser cannot
be exhausted by a hostile or malfunctioning Anthropic-compatible endpoint
that streams an unbounded SSE body. The 16 MiB cap matches the
non-streaming readProviderJsonResponse cap.

What changed:
- src/llm/providers/anthropic.ts: wrap body.getReader() in
  createSseByteGuard before the existing iterateSseMessages loop. The
  function is also re-exported as iterateSseMessagesForTest for direct
  test access.
- Inline test added to anthropic.test.ts: hostile 1 MiB pull stream,
  asserts canonical overflow message, cancel(reason) was an Error
  instance, pullCount bounded to 17-20.

Reuses the createSseByteGuard helper from src/agents/streaming-byte-guard.ts
(introduced in openclaw#96701 for the anthropic-transport-stream path; same
helper, same 16 MiB cap, same overflow-error shape). This PR is the
second of the planned per-surface rescue series for the previously
closed PR openclaw#96632, after openclaw#96701.

No SDK surface change. No repro script committed (proof in this body).
The companion error-body path is already bounded (readAnthropicMessages
ErrorBodySnippet, 8 KiB + 10s idle timeout, openclaw#95108); this PR closes the
legacy-provider success-body gap.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. 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: L status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant