[WRONG BRANCH] fix(web-search): bound withheld tool events - #481
Conversation
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. |
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 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. |
⏳ DRAFT
What to do
Its title has been prefixed with |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1efcfadfcb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (this.heldCalls.length >= MAX_HELD_CALL_EVENTS | ||
| || dataChars > MAX_HELD_CALL_CHARS - this.heldCallChars) { | ||
| throw new Error("upstream client tool events exceeded the web-search bridge buffer bound"); |
There was a problem hiding this comment.
Close opened search cells when the hold limit trips
When a leg emits a web_search item before another client tool exceeds this new limit, holdCall throws into the generic bridgeStreamBlocks read-error catch, which emits only response.failed. The already streamed web_search_call therefore remains in_progress, unlike the normal mixed-tool failure path that calls searchEndFrames, leaving Codex with a stuck search spinner. Close every search opened by the current leg before emitting this limit failure, or route the exception through the existing leg-failure cleanup.
AGENTS.md reference: src/AGENTS.md:L17-L19
Useful? React with 👍 / 👎.
Motivation
Description
MAX_HELD_CALL_EVENTS(1,000 events) andMAX_HELD_CALL_CHARS(8 MiB) and track held payload bytes withheldCallCharsto bound retained client-tool events.holdCall(...)helper that appends toheldCallsand throws a clear error when either event count or byte budget is exceeded.flushHeldCalls()into a generator that yields serialized frames incrementally and clears the held state in afinallyblock to avoid materializing a second full in-memory array.emitto accept any iterable of frames and route client-executed call accumulation through the bounded helper inconsume(...).function_call/delta frames and asserts the bridge fails with the expected error without releasing the withheld call.Testing
node_modules/.bin/bun test tests/web-search/web-search-passthrough-bridge.test.ts, which passed (22 tests passed).bun run typecheck, which succeeded, andbun run privacy:scan, which passed.git diff --check; no issues found.bun run testencountered unrelated environment/worker panics in the container (parallel test worker panic and serial retry hit container-specific process-group/zombie behavior); these failures are infrastructure-specific and not caused by this change. The focused bridge tests used the repo-local Bun and passed.Codex Task