fix(web-search): bound search replay ownership, reset replacement answers, and sidecar sends

[lidge-jun]

Summary

Three replay and retry paths could send or expose more than they should. This change carries @luvs01's replay-isolation and reset-replacement work and closes the gaps review found in it.

• Bridged web-search replay is scoped to its owner. A recorded search result is restored only for the same caller principal, conversation, provider, adapter, model, destination and physical credential. The scope is rebound before a dispatch-time key switch rebuilds the request. A caller with no resolvable principal (a keyless loopback client) now gets no scope at all: nothing is recorded or restored for it, and its hosted cells reach the destination unchanged. Before, every such caller shared one "loopback" principal.
• A spent reset replacement cannot cause a third send. After an ambiguous reset spends the operator-authorized replacement, the replacement's answer is sorted. 2xx passes through unchanged. 307, 308, 401, 402, 408, 409, 413, 429 and every 5xx settle as the non-replayable refusal with the body released; before, only the gateway 5xx subset did, so 429 and 529 escaped. Any other error keeps its real status and body but is marked non-replayable. The Codex pool's gated-model 400 check now honors that marker as the other recovery ladders already did, and a combo carries it through its rebuilt failure and stops instead of sending the turn to its next target.
• The web-search sidecar makes at most three physical sends per search. Reset recovery and 429 replays share one budget, where before each quota replay opened a fresh three-send reset allowance (up to nine sends). The budget is checked before a 429 body is released, so an exhausted budget or a deadline that cannot fit the backoff ends with the observed 429 as the recorded outcome. A caller abort is recorded as neutral.

Known costs, also recorded in structure/: keyless loopback callers lose cross-turn restoration of bridged searches (the pre-#4587 behavior); a real 401, 402 or 429 on a replacement send is not recorded against its credential for that request; a reset after an observed 429 that exhausts the sidecar budget is still recorded as a connection failure, and a sidecar replay can still be sent close to the deadline. A marked context-overflow 400 outside a combo keeps its status, and a routing-profile policy fallback can still hop on it; the replacement carried the same body as the first send, so such an answer is evidence the first send was refused on size too. Carrying the marker into policy fallback is left as a follow-up.

Carries the replay and retry commits from #5480 and #5423 (both closed by their author and folded into #5562 and #5553). The unrelated inline-document fixture change from the carried commit is left as dev has it.

Co-authored-by: luvs01 27862058+luvs01@users.noreply.github.com

Verification

• Local checks: NOT RUN by lane policy (no local test, typecheck or build); hosted CI on the exact head is the verifier.
• New regressions: keyed-caller restore (positive control), keyless no-restore and no shared bucket, key-switch rebind under a real key-derived principal, unit scope refusal without a principal (server-key-failover-e2e, web-search-bridge-replay); a refusal for every resend-inducing status with body release, real status plus marker for other errors, unchanged 2xx, and no marking without a spent replacement (upstream-retry); a marked gated-model 400 opens no alternate-account retry (codex-model-denial-evidence); a two-target combo whose first target resets and then answers a context overflow or a 413 sends nothing to the second target and nothing is resent by the client (replay-refusal-parity); shared three-send budget across reset and 429, 429 preserved at exhaustion, caller abort recorded as neutral (web-search-sidecar-429).
• An independent review of the design and an independent security review of this diff were completed before publication; the security verdict is posted as a PR comment.

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.

Summary by CodeRabbit

• Enhancements

  • Bridged web-search results now replay only for the same authenticated caller, conversation, provider, model, and credential.
  • Keyless callers do not receive or share replayed search results.
  • Search retries now respect a shared three-send limit and remaining request deadline.

• Bug Fixes

  • Prevented ambiguous upstream failures from being resent to alternate providers.
  • Improved handling of non-replayable responses and reset-related retry outcomes.
  • Updated upstream retry-refusal messaging.

• Documentation

  • Added guidance on search replay scope, retry limits, deadlines, and connection-reset behavior.

[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]

Independent security review

An independent reviewer, separate from the author of this change, reviewed the concrete diff by reading source only. Local checks were not run under this lane's policy; hosted CI is the verifier.

Verdict: PASS, no blocking issues (lane commits through e7f0820441; the later dev merge touches no file in this change's scope).

• Replay ownership: the scope key requires a caller principal, the conversation id and a bound serving identity, and fails closed if any is missing. A keyless caller never falls into a shared bucket. The principal is a process-local digest and is never logged. The scope is rebound before a credential switch rebuilds the request.
• Replacement answers: every consumer that could resend the same turn was traced. The refusal set plus the non-replayable marker stop each of them, including the Codex pool gated-model retry and combo hops, where the marker now survives the rebuilt failure.
• Sidecar budget: at most three physical sends per search across reset and 429 recovery, and an observed 429 is preserved at exhaustion or deadline.
• Regressions: each new test was checked statically to fail when its fix is reverted.
• Hygiene: no secrets, private hosts or absolute paths in tracked files.

A first review round found one blocking gap: a non-replayable answer could lose its marker inside a combo. It was fixed in 26ed40aeb4 and re-verified.

Accepted residuals (also in the description): the sidecar deadline check does not reserve time for the replay send; a marked context-overflow answer outside a combo keeps its status and can still be hopped by routing-profile policy fallback. That last one is a follow-up.

[github-actions]

✅ Deterministic PR hygiene checks passed.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: f03ebcf0-276f-4050-add5-5de60fb546d4

📥 Commits

Reviewing files that changed from the base of the PR and between 798c07c and 8a78611.

📒 Files selected for processing (25)

• docs-site/src/content/docs/reference/configuration/providers.md
• src/adapters/openai-responses/passthrough.ts
• src/adapters/openai-responses/tool-output-recovery.ts
• src/lib/errors.ts
• src/lib/upstream-retry.ts
• src/responses/bridge-search-replay-cache.ts
• src/server/responses/core-codex-account.ts
• src/server/responses/core-combo-failure.ts
• src/server/responses/core-combo.ts
• src/server/responses/core-options.ts
• src/server/responses/passthrough-delivery.ts
• src/server/responses/request-prepare.ts
• src/server/responses/request-transport.ts
• src/types/request.ts
• src/web-search/executor.ts
• structure/providers-and-adapters.md
• structure/runtime.md
• structure/transports/responses.md
• tests/codex-integration/codex-model-denial-evidence.test.ts
• tests/lib/upstream-retry.test.ts
• tests/server/replay-refusal-parity.test.ts
• tests/server/server-key-failover-e2e.test.ts
• tests/usage/request-log.test.ts
• tests/web-search/web-search-bridge-replay.test.ts
• tests/web-search/web-search-sidecar-429.test.ts

 ___________________________________________________
< Vibe code cleanup in progress... Expect swearing. >
 ---------------------------------------------------
  \
   \   (\__/)
       (•ㅅ•)
       / 　 づ

✨ Finishing Touches

📝 Generate docstrings

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

---

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.}

[lidge-jun]

리뷰 · 우선순위 61 / 80

이 PR은 웹 검색 다시 쓰기(replay)와 연결 끊김 뒤 한 번 더 보내기(reset replacement), 그리고 검색 사이드카의 재시도가 같은 요청을 너무 많이 보내거나 다른 사람 결과를 꺼내 쓰던 구멍을 막습니다. 검색 결과는 이제 같은 호출자·대화·제공자·어댑터·모델·목적지·실제 자격증명일 때만 되살립니다. 키가 없는 로컬(loopback) 호출자는 공통 "loopback" 바구니에 넣지 않고, 범위가 없으면 기록도 복원도 하지 않습니다. 운영자가 허용한 교체 전송을 이미 쓴 뒤에는, 클라이언트가 또 보내게 만드는 상태코드(307/308/401/402/408/409/413/429와 모든 5xx)는 거절로 바꾸고, 그 밖의 오류는 원래 상태코드를 유지하되 “다시 보내면 안 됨” 표시를 붙입니다. 콤보와 Codex 풀의 다른 계정 재시도도 그 표시를 보고 멈춥니다. 검색 한 번에 실제 전송은 최대 세 번이고, 리셋 복구와 429 재시도가 그 예산을 나눕니다. base는 dev입니다. types/config 분할·프리뷰 배포와는 무관합니다. 같은 주제로 열린 #5562(검색 replay 묶음)와 #5553(재시도·교체 묶음)과 파일이 크게 겹치고, 이 PR이 더 엄격합니다(#5562는 키 없는 loopback을 아직 공유 주인으로 두고, #5553은 교체 뒤 5xx 위주).

라인 - src/server/responses/policy-fallback.ts — 교체 뒤 “다시 보내면 안 됨” 표시를 콤보·Codex 계정 재시도는 읽지만, 라우팅 프로필 policy fallback 길은 이 표시를 보지 않습니다. PR 본문도 후속으로 남겨 두었습니다. 표시만 있는 context overflow 400이 policy 쪽으로 넘어가면, 이미 돌았을 수 있는 턴이 다른 정책 경로로 한 번 더 나갈 수 있습니다.

라인 - 열린 PR #5562 · #5553 — bridge-search-replay-cache.ts, executor.ts, upstream-retry.ts 등 같은 파일을 건드리는 스택 PR이 아직 열려 있습니다. #5562는 키 없는 loopback 공유 주인을 유지하고, #5553은 교체 거절 범위를 더 좁게 둡니다. 이 PR을 먼저 넣으면 두 스택은 재기반도 충돌도 납니다.

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

policy fallback에도 같은 “다시 보내면 안 됨” 표시를 이번 PR에 넣을지, 후속 이슈로 둘지. 키 없는 loopback이 턴을 넘기며 검색 결과를 되살리지 못하는 비용을 제품으로 받아들일지(문서·structure에는 이미 적혀 있음). #5562·#5553을 이 PR에 맞춰 닫을지, 남은 부분만 남길지.

너의 추천

보안·재전송 경계는 맞고 합쳐도 됩니다. 독립 보안 리뷰도 PASS입니다. 머지 전에 policy fallback에 표시 전파를 넣거나, 넣지 않을 거면 후속 이슈 번호만 고정하세요. #5562와 #5553은 이 변경에 흡수·무효화된 부분부터 닫는 쪽을 권합니다. 호스트 CI test 샤드가 아직 돌고 있으니, 그 통과를 머지 조건으로 두면 됩니다.

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