fix(web-search): bound withheld tool events and close cells on read failure

[luvs01]

Summary

The hosted web-search bridge can retain an unbounded number of client-tool events while waiting to decide a leg's outcome. If the upstream reader then fails, search cells already shown to the client can remain open. This change bounds the retained events and closes those cells before emitting the failed terminal.

• Limit withheld client-tool events per leg to 1,000 events and 8,388,608 UTF-16 code units of cumulative SSE data payload. This is a payload accounting limit, not a byte or total-heap limit.
• Release held events lazily without constructing a second complete array of serialized frames. Reset payload accounting and identity sets on release, discard, and leg initialization.
• On upstream read/limit failure or exhausted continuation budget, discard withheld tools and close every search cell opened by the leg as failed before one response.failed and one [DONE]. No search execution or continuation follows that failure.
• Preserve the current mixed-leg behavior, including the distinction between failed and incomplete terminals. Routing, credentials, workflows, and dependencies are unchanged.
• Add nine stream-lifecycle regression cases and update the owning runtime contract.

Coverage includes index-only and item-ID-only delta floods, repeated added-item events, aggregate character overflow with small frames, closure of two opened search cells, a throwing upstream reader, exact acceptance of 1,000 held events with lossless arguments and monotonic numbering, and failed/incomplete terminal controls. Pull-driven fixtures assert upstream cancellation without a live provider.

Verification

Validated after merging upstream dev at 5e3029e6fdcc85e3ed3c6963b74c554df6bc9bd3; candidate head f7204e0dad4cdceb8dd5d0d9d4e04c048ec1d7a7.

• PASS: Bun 1.4.2 on Windows, bun test tests/web-search/web-search-progress-stream.test.ts tests/web-search/web-search-passthrough-bridge.test.ts --timeout 20000 — 98 tests, 441 assertions.
• PASS: Static review of admission bounds, lazy release/finally cleanup, failure ordering, identity matching, and mixed-terminal paths.
• Cross-platform CI run 35055882562 completed successfully on the exact head, with all 26 jobs green across the ordinary and Windows matrices and the aggregate gate. That run covers the repository-wide typecheck, structure and privacy checks and the full suite.

Checklist

• Scope stays focused and avoids unrelated source cleanup.
• Docs or release notes were updated when needed.
• Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults (static review; no independent approval claimed).

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

• All CI tests are green on my local testing. Exact-head run 35082021781 on 0abc196 passed all 26 jobs; all jobs passed on the first attempt.

• I pushed my PR to the latest dev commit.

• I resolved all correct Codex and CodeRabbit findings. No unresolved current review threads remain.

• My PR is ready for review.

Summary by CodeRabbit

• Bug Fixes

  • Prevented excessive buffering of client-executed tool-call events.
  • Improved failure handling when event limits are exceeded, upstream reads fail, or continuation budgets are exhausted.
  • Ensured search cells close correctly and failed turns emit consistent terminal events.
  • Improved cancellation behavior and event streaming reliability.

• Documentation

  • Documented buffering limits and cleanup behavior for hosted-search passthrough flows.

• Tests

  • Added coverage for buffering limits, failure scenarios, cancellation, event ordering, and successful boundary cases.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 9996068d-febb-4ec6-9d92-7db0e6b1bad2

📥 Commits

Reviewing files that changed from the base of the PR and between 5d922e8 and 0abc196.

📒 Files selected for processing (3)

• src/web-search/passthrough-bridge.ts
• structure/runtime.md
• tests/web-search/web-search-progress-stream.test.ts

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

---

📝 Walkthrough

Walkthrough

The web-search bridge now bounds held client-tool events per leg, releases them lazily, and uses shared cleanup for read failures and exhausted continuation budgets. Tests cover limits, cancellation, ordering, terminal handling, and search-cell closure.

Changes

Passthrough bridge lifecycle

Layer / File(s)	Summary
Held event bounds and per-leg state src/web-search/passthrough-bridge.ts	The bridge limits held events to 1,000 and held payloads to 8 MiB of UTF-16 data. It tracks payload size, resets held state per leg, and applies the limits to both withholding paths.
Lazy release and failure cleanup src/web-search/passthrough-bridge.ts	Held events release through a lazy generator. Shared failure handling drops held calls, closes opened search cells, and emits the failure terminal for read errors and exhausted continuation budgets.
Lifecycle documentation and integration coverage structure/runtime.md, tests/web-search/web-search-progress-stream.test.ts	The documentation records the limits and cleanup behavior. Tests cover overflow, cancellation, exact-boundary release, ordering, terminal handling, and search-cell closure.

Priority: ⬇️ Low

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

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 0abc1

No actionable merge risk is identified from the available evidence; proceed with normal checks.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. (1 skipped: 1 …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately and concisely describes the main changes: bounding withheld web-search tool events and closing search cells when an upstream read fails.
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.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. (1 skipped: 1 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

Warning

Git: CodeRabbit could not clone the repository, so clone-backed analysis was skipped and this review may be incomplete. Verify repository clone access, such as SSH credentials, before requesting another full review. If clone access is intentionally unavailable, use path_filters to narrow the review scope.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[github-actions]

⚠️ Deterministic hygiene checks failed.

• missing_coauthor_credit — This pull request says it reimplements, supersedes, carries, or rebases another author's pull request, but no Co-authored-by trailer names that author. Prose in a commit body is not read by anything; the trailer is what GitHub counts. Add it to the description or a commit, or obtain attribution-approved. Paths: #481.

[github-actions]

✅ READY

• all PR quality gates passed; the review readiness checklist is complete.

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.

✅ 4/4 boxes ticked.

Automatic ready-for-review conversion failed; please mark the pull request ready manually if it is still a draft.
The review-ready label marks this PR as ready; review automation runs independently.
Maintainers notified: @lidge-jun @Ingwannu

[luvs01]

Publication verification at head 922d498095031a80a948418383da85dea25eda1c:

• GitHub reports this PR open, draft, and mergeable, targeting dev at 3070d64d8822c6d8c62989665f82ab665e4d164c.
• Re-read the published patch: three changed files; the runtime changes are the held-event limits, lazy release/state reset, and shared read/limit-failure cleanup. The owning runtime document adds the accounting/cleanup contract.
• Cross-platform CI run https://github.com/lidge-jun/opencodex/actions/runs/35046608041 reports action_required; its jobs list is empty. React Doctor run https://github.com/lidge-jun/opencodex/actions/runs/35046607882 also reports action_required. These are not passing executable checks; repository-side approval/action is still needed before the tests can establish readiness.
• Keep this PR in draft. No local Bun test, typecheck, full suite, structure check, or privacy scan result is being claimed for this head.

[lidge-jun]

리뷰 · 우선순위 73 / 80

이 PR은 web-search 패스스루 브리지가 leg 운명이 정해지기 전에 보류(withheld)하는 클라이언트 툴 이벤트를 개수(1,000)와 SSE data 페이로드 UTF-16 코드유닛(8,388,608)으로 묶고, upstream read/limit 실패·continuation 예산 소진 시 이미 보여 준 search cell을 닫는 공통 failLegFrames로 모읍니다. 지금 dev의 src/web-search/passthrough-bridge.ts는 heldCalls 배열에 상한이 없고, flushHeldCalls가 직렬화 프레임 배열을 한 번에 만들며, read 실패 경로와 decide(fail) 경로의 cell 정리 코드가 갈라져 있습니다. 이 변경은 holdCall에서 한도를 넘기면 throw하고, flush를 generator로 바꿔 두 번째 전체 배열 할당을 피하며, dropHeldCalls가 chars/index/id Set까지 리셋합니다. 라벨에 intake: hygiene-blocked가 붙어 있어 intake 게이트상 그대로 머지할 수 없는 상태입니다. draft·base dev·파일 3개로 범위는 좁습니다.

UTF-16 code unit 카운트는 바이트/힙 총량이 아니라고 주석·structure에 명시돼 있어, 운영자가 “8MiB RSS 캡”으로 오해하지 않게 한 점은 좋습니다. 한도 초과 throw가 bridgeStreamBlocks catch → failLegFrames로 이어져 cell을 닫는지도 테스트(web-search-progress-stream.test.ts)로 고정돼 있어야 합니다. pre-split 모놀리스 재편집은 없습니다.

passthrough-bridge.ts holdCall - 이벤트 수와 heldCallChars 합산. data.length는 문자열 코드유닛. 바이너리/비BMP에서 “바이트”와 어긋날 수 있으나 계약상 의도.
flushHeldCalls generator - finally에서 dropHeldCalls. 중간에 consumer break해도 보류 집합이 남지 않음.
failLegFrames - dropHeldCalls 후 searches에 searchEndFrames(error) + failureFrames. read 실패와 decide(fail)가 같은 정리 경로.
라벨 intake: hygiene-blocked - 내용 리뷰와 별개로 머지 차단. 하이진 사유(포맷/시크릿/레이아웃 등)를 해소한 뒤에만 랜딩.

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

• hygiene-blocked 해제에 필요한 구체 수정(작성자 luvs01 vs 메인테이너).
• held 한도 throw 메시지를 클라이언트에 얼마나 노출할지(지금은 bridge error로 래핑).
• draft 유지 기간.

너의 추천
코드 방향은 KEEP이지만 라벨 intake: hygiene-blocked가 해제되기 전에는 머지하지 마세요. 하이진 통과 → draft 해제 → tip CI 후 머지. 지금은 WAIT(하이진 해제 후 KEEP).

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

[chatgpt-codex-connector]

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]

Carried in #4801 on the current dev head with your commit and a Co-authored-by trailer preserved. Two changes on top: the 1,000-event bound counts every held function_call_arguments delta, so a sizeable client-executed apply_patch streamed as fine-grained deltas could cross it and the leg would discard a legitimate tool call — the count is re-derived from the 8 MiB code-unit budget that is the real memory guard, with the derivation stated; and overflow now reports as a proxy-side bound rather than through the upstream-read-failure message. Cancelling the run on this head so the carry gets the macOS runners.

[lidge-jun]

Landed via #4801 at c272309

[lidge-jun]

Landed through #4801 with your commit and attribution preserved, with the event bound re-derived and the overflow message corrected.