Skip to content

fix(web-search): bound the Anthropic sidecar SSE and error bodies - #3839

Closed
luvs01 wants to merge 1 commit into
lidge-jun:devfrom
luvs01:agent/websearch-anthropic-sse-bound-20260907
Closed

fix(web-search): bound the Anthropic sidecar SSE and error bodies#3839
luvs01 wants to merge 1 commit into
lidge-jun:devfrom
luvs01:agent/websearch-anthropic-sse-bound-20260907

Conversation

@luvs01

@luvs01 luvs01 commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Bound the Anthropic web-search sidecar's SSE and HTTP error-body reads to the existing 64 KiB MAX_SIDECAR_RESPONSE_BYTES limit. A response without a frame separator previously kept growing the parser buffer, and the HTTP failure path read the entire body with res.text().

The parser retains complete frames received before the limit and discards the unfinished tail. It reuses the Responses sidecar's limit and non-blocking reader-cancellation helper without changing that helper's behavior.

Verification

Head 5d490f7c053f8eabdd2815b72b2ea5cdcf9cfa73, rebased onto dev d00615d5696a9c6b7f1a3006181e57569b012920.

  • Project-pinned Bun 1.4.0 was used for the parent process and child commands.
  • bun run test ./tests/web-search/ --timeout 60000: 176 passed, 0 failed across 10 files.
  • bun run typecheck, bun run privacy:scan, and git diff --check: passed.
  • The bounded-reader regression was previously demonstrated to fail without the production change and pass with it.

Full-suite validation remains outstanding. The prior review-ready attestation has been withdrawn; this PR stays draft until the remaining validation is complete. There is no UI change.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Review readiness checklist

  • All CI tests are green on my local testing.
  • I pushed my PR to the latest dev commit.
  • I resolved all correct Codex and CodeRabbit findings.
  • My PR is ready for review.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Sep 7, 2026
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

⏳ DRAFT

  • review readiness checklist open (1/4 boxes ticked).

What to do

  • Tick all four boxes in the PR description once you're done (currently 1/4).

Review readiness checklist

  • ⬜ All CI tests are green on my local testing.
  • ⬜ I pushed my PR to the latest dev commit.
  • ✅ I resolved all correct Codex and CodeRabbit findings.
  • ⬜ My PR is ready for review.

1/4 boxes ticked.

This PR stays in draft until every box above is ticked.

Hygiene

Deterministic PR hygiene checks passed.

@github-actions
github-actions Bot marked this pull request as draft September 7, 2026 01:35
@coderabbitai

coderabbitai Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Team

Run ID: 044a5608-ec02-4a84-9e0f-e33058ea3343

📥 Commits

Reviewing files that changed from the base of the PR and between bf85e67 and c25bdab.

📒 Files selected for processing (3)
  • src/web-search/anthropic-executor.ts
  • src/web-search/parse.ts
  • tests/web-search/web-search-anthropic.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.


📝 Walkthrough

Walkthrough

The Anthropic sidecar executor now bounds HTTP error-body reads and SSE input. It tracks accepted bytes, truncates oversized chunks, cancels readers without waiting, and drops unterminated tails. A regression test verifies bounded cancellation behavior.

Changes

Sidecar response bounds

Layer / File(s) Summary
Bounded response handling
src/web-search/anthropic-executor.ts, src/web-search/parse.ts, tests/web-search/web-search-anthropic.test.ts
readBoundedText limits HTTP error-body reads to MAX_SIDECAR_RESPONSE_BYTES. parseAnthropicSidecarSSE limits accumulated chunks, cancels the reader at the limit, and drops an unterminated tail. cancelReaderWithoutWaiting is exported. The regression test verifies cancellation and bounded output.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to c25bd

Anthropic web-search sidecar responses now stop reading oversized error and unterminated SSE bodies at the existing limit, reducing memory exposure while retaining complete frames. The bounded behavior and cancellation path are covered, with no current merge-blocking risk identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: bounding Anthropic sidecar SSE and error-body reads.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@luvs01
luvs01 marked this pull request as ready for review September 7, 2026 01:36
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 66 / 80

이 PR은 Anthropic 웹검색 사이드카가 응답을 읽을 때 메모리가 한없이 커질 수 있던 구멍을 막는 수정이다. 지금 dev(HEAD bf85e6754, 2.46.0 open, 최근 팁은 #3836 delivery-note privacy)에는 Responses 사이드카용 parseSidecarSSE(src/web-search/parse.ts)가 이미 MAX_SIDECAR_RESPONSE_BYTES(64 KiB)로 읽기를 끊고, cancelReaderWithoutWaiting으로 리더를 기다리지 않고 취소합니다. 그런데 Anthropic 경로인 parseAnthropicSidecarSSE(src/web-search/anthropic-executor.ts)는 \n\n 구분자가 나올 때까지 디코딩한 청크를 전부 buffer에 붙였고, 실패 분기(!res.ok)는 신뢰할 수 없는 업스트림 에러 본문을 res.text()로 통째로 읽었습니다. 구분자가 안 오는 스트림이나 아주 큰 에러 바디가 오면 프로세스 메모리가 그 크기만큼 불어날 수 있습니다.

이번 변경은 그 두 곳을 같은 정책으로 맞춥니다. cancelReaderWithoutWaitingparse.ts에서 export하고, SSE 루프에 responseBytes 카운터를 넣어 수용 바이트를 자른 뒤 한도에 닿으면 이미 접힌 프레임은 남기고 미종료 꼬리만 버린 채 리더를 취소합니다. 에러 바디는 새 readBoundedText로 같은 한도만 읽습니다. 회귀 테스트는 구분자가 없는 큰 스트림을 밀어 넣고, 생산자가 끝나기 전에 cancel이 걸리고 produced가 1 MiB 미만인지 확인합니다. Gemini/Exa는 이미 같은 상수를 쓰고, xAI만 * 8로 더 느슨합니다. 이 PR은 Anthropic을 Responses와 같은 64 KiB에 맞추는 쪽이라 dev 방향과 잘 맞습니다. types/config 분리 캠페인과도 겹치지 않아 close-don't-rebase 대상이 아닙니다.

라인 75 - responseBytes로 누적 바이트를 세고, 라인 119–121에서 청크를 remaining만큼만 accepted로 자른 뒤 디코드합니다. Responses parseSidecarSSE와 같은 모양이라 정책이 한곳으로 모입니다.
라인 131–136 - 한도에 닿으면 cancelReaderWithoutWaitingbuffer = ""로 미종료 꼬리를 버립니다. 그 아래 라인 139–141 flush는 빈 buffer라서 거대 미종료 프레임을 다시 파싱하지 않습니다. 의도는 맞습니다.
라인 24–48 - readBoundedText가 예전 라인의 무한 res.text()를 대체합니다. 에러를 삼키는 catch는 HTTP 상태를 가리지 않으려는 선택이라 타당합니다.
라인 226 - runAnthropicWebSearch 실패 분기가 readBoundedText(res)를 쓰므로, 인증 실패 메시지용으로만 쓰이던 본문도 64 KiB에서 끊깁니다.
라인 219–221 - 주석이 아직 “failure branch's res.text()”라고 남아 있습니다. 동작은 readBoundedText로 바뀌었으니 주석만 살짝 옛말입니다(동작 버그는 아님).
tests/web-search/web-search-anthropic.test.ts 라인 133–152 - 미종료 프레임 DoS 회귀는 좋습니다. 다만 에러 바디(!res.ok + 거대 body) 경로용 별도 테스트는 없어서, readBoundedText 회귀는 SSE 쪽만큼 단단하지 않습니다.
src/web-search/parse.ts 라인 196 - cancelReaderWithoutWaiting export는 재사용에 맞고, 헬퍼 본문은 그대로입니다.

메인테이너의 판단이 필요한 지점

  • xAI(xai-executor.ts)는 여전히 MAX_SIDECAR_RESPONSE_BYTES * 8입니다. Anthropic/Responses와 숫자를 통일할지, xAI만 느슨히 둘지는 이번 PR 범위 밖이지만 후속 정리 여부는 메인테이너 선택입니다.
  • 에러 바디 회귀 테스트를 이 PR에 더 넣을지, 머지 후 작은 follow-up으로 둘지.

너의 추천
CI가 초록이면 머지하세요. 범위가 작고 dev의 기존 사이드카 바이트 정책과 같고, 보안·안정성 이득이 분명합니다. 머지 전에 가능하면 라인 219–221 주석의 res.text() 표현만 readBoundedText에 맞게 고치거나, 에러 바디 한도 회귀 테스트 한 줄을 추가하면 더 안전합니다. types/config 분리와 무관하니 닫지 말고 착지시키면 됩니다.

이 댓글은 grok-bot이 작성했습니다

`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.
@luvs01
luvs01 marked this pull request as draft September 7, 2026 10:51
@luvs01
luvs01 force-pushed the agent/websearch-anthropic-sse-bound-20260907 branch from c25bdab to 5d490f7 Compare September 7, 2026 10:51
@luvs01

luvs01 commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor
⚠️ Action not completed

No files to review.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

lidge-jun added a commit that referenced this pull request Sep 7, 2026
…carry #3839) [skip ci]

fix(web-search): bound Anthropic sidecar SSE and error reads
@lidge-jun

Copy link
Copy Markdown
Owner

Landed on dev via #3873 (merge f46a7f49c; chain-top Cross-platform CI run 34116228181, aggregate ci green incl. Windows 6/6). carried (cherry-pick -x) with error-body cap and non-settling cancel coverage. Your authorship is preserved with a Co-authored-by: luvs01 trailer on the landed commit. Closing this PR as superseded — thank you @luvs01!

@lidge-jun lidge-jun closed this Sep 7, 2026
@lidge-jun lidge-jun added the landed-via-maintainer Original PR closed after landing via a maintainer merge train label Sep 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working landed-via-maintainer Original PR closed after landing via a maintainer merge train

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants