fix: cap uncapped string accumulators (RangeError: Invalid string length)#307
Merged
Conversation
…string length Two uncapped in-memory string accumulators in the aimock proxy/record path could build a string past V8's max string length (2^29-1, ~512MiB), throwing `RangeError: Invalid string length` (~1/sec in prod, caught at server.ts:1230). Amplified by the real-key fixture-miss passthrough, which routes large real upstream responses into these accumulators. Two sites, both fixed to mirror the existing PROXY_BUFFER_HARD_CEILING pattern (bound then mark truncated, never fail the client — recording is degraded to a clean skip while the full response is still relayed): 1. agui-recorder.ts — `Buffer.concat(chunks).toString()` on stream end had NO byte cap. Add a configurable record-buffer cap (AGUIRecordConfig. maxRecordBufferBytes, default 64MiB, hard ceiling 256MiB): stop accumulating on trip, free the buffer, skip fixture construction, and keep the live client relay untouched. 2. stream-collapse.ts — the `content`/`reasoning`/tool `arguments`/`audioB64`/ `argsStr` accumulators (and orderAtoms coalescing) were unbounded. Guard each collapser's input under MAX_COLLAPSE_STRING_LENGTH (256Mi code units) before accumulation begins — the sum of any one accumulator can never exceed the input, so bounding the input bounds every accumulator. Over-ceiling input is truncated and the result stamped `truncated: true`. Also covers the exported direct-call collapse entry points where no byte cap ran. Also fixes the false recorder.ts comment that claimed stream-collapse "never throws Invalid string length". Test-only ceiling overrides (setAGUIRecordBufferCeilingForTests / setCollapseStringLimitForTests) let the cap suites exercise the truncation path without allocating hundreds of MB.
commit: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
RangeError: Invalid string lengthwas firing ~1/sec in prod (caught atserver.ts:1230). V8's max string length on 64-bit is2^29-1(~512MiB of UTF-16 code units); appending past it throws.Two uncapped in-memory string accumulators in the aimock binary build a JS string past that limit:
src/agui-recorder.ts— on stream end the recorder doesBuffer.concat(chunks).toString()over every buffered upstream SSE chunk with no byte cap (unlike the generic proxy path inrecorder.ts, which is capped at 64MiBmaxProxyBufferBytes/ 256MiBPROXY_BUFFER_HARD_CEILING).src/stream-collapse.ts— thecontent += delta/reasoning += delta/entry.arguments += delta/audioB64 += …/argsStr += …accumulators (plusorderAtomscoalescing) grow unbounded across every collapser.Amplified 07-13 by the real-key fixture-miss passthrough (
b2ef945): large real upstream responses now flow into these accumulators. Therecorder.tscomment claiming "stream-collapse never throws Invalid string length" was false for these paths and is fixed here.Fix
Mirrors the existing
PROXY_BUFFER_HARD_CEILING/maxProxyBufferBytesdiscipline — bound then mark truncated, never fail the client. The client always receives its full response; only the recording is degraded to a clean skip.AGUIRecordConfig.maxRecordBufferBytes, default 64MiB, clamped to a 256MiBAGUI_RECORD_BUFFER_HARD_CEILING). On trip: stop accumulating, free the buffer, skip fixture construction, keep the live client relay untouched — neverBuffer.concat().toString()the oversized buffer.MAX_COLLAPSE_STRING_LENGTH(256Mi code units) before accumulation. The sum of any single accumulator can never exceed the total input, so bounding the input bounds every accumulator at one chokepoint (no per-+=whack-a-mole). Over-ceiling input is truncated and the result stampedtruncated: trueso the recorder skips journaling a partial fixture. Also covers the exported direct-call collapse entry points where no byte cap ran.recorder.ts:31comment.setAGUIRecordBufferCeilingForTests/setCollapseStringLimitForTests) let the cap suites exercise truncation without allocating hundreds of MB.Local red-green proof (real failure surface)
RED — AG-UI recorder (streaming ~535MiB of real SSE through
proxyAndRecordAGUIon the unmodified build):(
0x1fffffe8=2^29-1— the exact V8 max-string-length boundary, i.e. the prodInvalid string length.)GREEN — same repro, post-fix:
Client received all 562,041,225 bytes — relay unaffected, only recording skipped.
stream-collapse mutation-guard (real
collapseOpenAISSE, input just over the 256Mi ceiling):content.length=276824064, truncated=undefined(unbounded, no clamp)content.length=260046848, truncated=true(clamped at ceiling, flagged)Tests
src/__tests__/agui-record-buffer-cap.test.ts(2) — full relay + no crash + recording skipped over cap; normal recording under cap.src/__tests__/stream-collapse-string-cap.test.ts(6) — every collapser clamps + flagstruncated, never throws; no truncation under cap; Bedrock byte-cap.