fix(vision): bound Anthropic sidecar SSE and error reads - #3874
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
✅ Deterministic PR hygiene checks passed. |
리뷰 · 우선순위 69 / 80이 PR은 #3841을 Lane C 2층( 지금 테스트는 partial-description 보존·discarded tail·non-settling cancel을 잠급니다. astra-high PASS. types/config 분할과 무관하고, #3873 위에만 올리면 됩니다. Lane C top CI는
메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
c07456e to
47803f1
Compare
788ed57 to
71ec4c7
Compare
…ip ci] `parseAnthropicSidecarSSE` folded frames only when it found a `\n\n` separator, and appended every decoded chunk to `buffer` until then. An upstream that streams without ever emitting a separator therefore grew the buffer for the whole response. The non-OK branch had the same shape: it read the untrusted error body with an unbounded `res.text()` just to build one auth-failure message. Reuse the existing `MAX_SIDECAR_RESPONSE_BYTES` cap and the reader-cancel helper that `parseSidecarSSE` already applies in the same folder, so both paths stop reading at the same 64 KiB bound. Frames folded before the cap are kept; only the unterminated tail is dropped. `cancelReaderWithoutWaiting` is exported for this, unchanged. The regression drives a stream that never terminates a frame and asserts the read is cancelled well before the producer finishes. It fails without the cap and passes with it. (cherry picked from commit c25bdab) Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>
…ion [skip ci] Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>
…[skip ci] (cherry picked from commit 55b009b) Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>
…cel [skip ci] Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>
47803f1 to
51b4437
Compare
71ec4c7 to
8eaa564
Compare
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
2 participants
Summary
Carry #3841 with the matching 64 KiB accepted-byte bound for Anthropic vision SSE and HTTP error bodies. Pin partial-description preservation and incomplete-tail rejection at the cap, including non-settling cancellation. Safe public error projection and downstream description clamps remain intact.
Verification
f1b8f2d2f1edee55b68202469f86a5334710688d: Linux 4, macOS 2, gates, storage, API, all keyring/npm smokes and Docker. Overall run status iscompleted/success; this is not an assertion that every workflow job passed.macos controlto one run on the final release-train head. They are excluded only from this per-chain LANDABLE decision. Windows keyring/npm smoke jobs remain required. Lower product CI is represented by the cumulative top-head result.git diff --check: passed.Checklist
Manual dependent PR chain
Native stack:
null. Review only this layer’s diff. Main integrates bottom-up after refreshing dev ancestry, live heads/bases, review threads and required gates. This task does not merge. The CI-tested cumulative top tree is the integration target; advancing dev requires another cascade and top-head verification.51b44371860b2ec933d66de1b68ca790be3b81fb8eaa5641902b84bec6c97a94de6c2fe6d810c31d91b54b79bc780011031b00354621cc9fcaef360eabbdb227f385443228656c8440282187cb5dec75f1b8f2d2f1edee55b68202469f86a5334710688dMaintainer integration decision (MAINTAINERS.md, dev-only admin integration): @lidge-jun integrates lane C's manual chain (#3873 → #3874 → #3875 → #3876 → #3878) into
devbottom-up. Chain-top evidence at headf1b8f2d2f: Cross-platform CI run 34116228181, aggregateci= success (attempt 2: macos 1/2 timed out at 20 min in attempt 1 aftercodex-inject-write-lock.test.tswith no output; independent diagnosis found no causal link to this chain and the same head's macos control passed those tests; the failed job alone was rerun and passed — no code change). Windows 6/6 and macos control also green on this head. Prospective merge tree oforigin/dev@522ce5f8c+ top =e0b0e5886= tested tree. Independent source/security/GUI reviews PASS per PR bodies; screenshots from this run's dashboard-preview artifact. Local suites NOT RUN. Maintainer integration, not self-approval.