fix: bound streamed tool argument memory - #847
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
|
CI follow-up: the first Windows run passed 6,514 tests and timed out only the unrelated existing image-normalization P2 case at 5.39 seconds against its 5-second test limit. Re-running the failed Windows job passed. Ubuntu, macOS, Windows, package-install, target-enforcement, label, and React checks are now green. |
|
Thanks for this — it is a valuable part of the #820 series, especially because it bounds streamed tool-argument memory across the major translation paths. It currently conflicts with dev, so it will need a rebase, followed by an independent review from someone other than the author, before it can land.\n\nFor the broader draft series (#840, #841, #843, #844, #845, and #847), is there a specific landing order we should follow, or would it be better to consolidate any of the overlapping work? We are tracking #820 and would like to get these into the next cycle. |
Summary
Why
Compatible upstreams can omit SSE delimiters or stream unbounded tool argument fragments. Those buffers were protocol-required, but they had no byte ownership limit and could grow for the lifetime of a turn. Truncating and completing a call would produce invalid or unsafe executable input, so overflow now fails truthfully while preserving the existing sequential tool event contract.
Validation
bun teston 15 related bridge, Chat, OpenAI Chat, and SSE suites: 206 passbun run typecheckbun run privacy:scanRelated to #820. Focused alternative to the overlapping portions of #829.