fix(retries): refuse transient 5xx after an operator-authorized reset replacement

[luvs01]

Motivation

• Prevent a transient upstream 5xx response from escaping after an operator-authorized ambiguous-reset replacement and thereby inviting client-level retries that can duplicate a logical turn.

Description

• In fetchWithResetRetry detect when an operator replacement was spent (spentOperatorReplacement) and the replacement resolved to a transient upstream status, cancel that response body with cancelResponseBodyBestEffort and return the existing non-replayable replayRefusalResponse() instead of returning the transient 5xx.
• Add a regression test in tests/lib/upstream-retry.test.ts that reproduces the ECONNRESET -> replacement 502 sequence and asserts the terminal answer is the non-replayable 429 refusal.
• Preserve existing behavior for thrown errors: thrown transport errors after a spent replacement still settle as the refusal (unchanged).

Testing

• bun test tests/lib/upstream-retry.test.ts: 49 tests pass.

[coderabbitai]

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

• Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

---

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 PR hygiene checks passed.

[github-actions]

✅ READY

• all PR quality gates passed.

Hygiene

✅ Deterministic PR hygiene checks passed.

[lidge-jun]

리뷰 · 우선순위 67 / 80

요청을 보냈는데 연결이 중간에 끊기면, 서버가 그 일을 이미 했는지 알 수 없습니다. 그래서 자동으로 다시 보내지 않습니다. 운영자가 한 번만 다시 보내라고 허락한 경우에만, 한 번 더 보냅니다.

그 한 번이 서버의 잠깐 오류로 끝나면, 예전 코드는 502 같은 답을 그대로 클라이언트에게 줬습니다. 이 길을 쓰는 클라이언트는 그런 5xx를 다시 보내도록 되어 있습니다. 허락한 한 번 위에 또 보내면, 같은 일이 두 번 실행될 수 있습니다.

이 PR은 그 5xx를 클라이언트에게 넘기지 않습니다. 받은 답의 몸체는 버리고, 다시 보내지 말라는 429를 돌려줍니다. 그 429에는 다시 보내지 말라는 표시가 있습니다. 테스트는 끊김 다음에 502가 오면 세 번째 전송이 없는지 확인합니다. base는 dev입니다. 지금 tip은 ac3df52cca3b이고, 패키지는 @bitkyc08/opencodex 2.61.0입니다. 이 커밋은 tip보다 1개 앞이고, 뒤처진 커밋은 없습니다. GitHub에서는 아직 초안입니다. 본문 준비 체크는 네 칸 모두 표시되어 있고, review-ready 라벨이 붙어 있습니다. GitHub에 나온 검사는 hygiene, label, enforce-target, resolve-pr뿐입니다. CodeRabbit은 초안이라 건너뛰었습니다. types.ts와 config.ts를 나누는 변경은 아닙니다.

src/lib/upstream-retry.ts:609 - 거절로 바꿀 때 쓰는 문장은, 응답을 받기 전에 연결이 닫혔다고 말합니다. 이 길은 502를 받은 다음입니다. 429와 다시 보내지 말라는 표시는 맞습니다. 문장만 보면, 답이 없었던 일처럼 읽힙니다.

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

• 교체 전송이 408, 409, 또는 서버의 429로 끝나면, 그 답은 그대로 클라이언트에게 갑니다. 이 프록시를 부르는 흔한 클라이언트는 그 상태도 다시 보냅니다. 5xx만 거절로 바꿀지, 클라이언트가 다시 보내는 상태까지 거절로 바꿀지입니다.
• 교체가 200으로 끝나면 그 성공을 그대로 돌려주는 것이 맞습니다. 바로 위 주석은 "이제는 거절만 한다"고 더 넓게 적혀 있습니다. 주석을 코드에 맞출지는 선택입니다.
• 아직 초안입니다. 테스트 작업은 GitHub 검사 목록에 없습니다. 작성자는 로컬에서 bun test tests/lib/upstream-retry.test.ts 49개가 통과했다고 적었습니다. 이 리뷰는 그 테스트를 다시 돌리지 않았습니다.

너의 추천
방향은 맞습니다. 5xx를 429 거절로 바꾸는 위치도 맞습니다. 초안을 풀고, GitHub 테스트가 초록이 된 뒤에 머지를 판단하세요. 408과 409를 같은 거절에 넣을지는 그 전에 정하면 됩니다. 거절 문장은 머지를 막지 않습니다.

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

[luvs01]

Review feedback applied on c1fd0ccf08:nn- The refusal message now reads "did not complete reliably" instead of "connection closed before a response was received" - accurate for both the reset path and the post-502 path. The narrow classifier substring and the request-log test were updated to match.n- The spentOperatorReplacement comment now says the leg settles as the refusal OR an unambiguous answer (a 200 still passes through), instead of "only the refusal".nnOn 408/409/429 passthrough: left unchanged - those answers carry upstream semantics worth surfacing (a 429 body can name its own reset), and widening the refusal class is a separate decision.nnTests: upstream-retry+request-log` 125 pass.

[luvs01]

Consolidated into #5553 as a single related-function aggregate.

Source head: f770506554a6b0edc895b5e7e3757c43a0156a10. Replacement head: 67c4f579e4fb5201dcd36be225ee54306d78e63e.

Implementation 980c662 and wording/logging follow-up c1fd0cc match carried 35fb727 and 940b318 by stable patch ID with source authors retained. The dev merge adds no remerge-resolution delta. The final aggregate's 604 distinct focused tests include terminal transient-5xx refusal after an operator-granted replacement, preserving one policy invocation and no alternate-account retry. The proposal remains limited to transient 5xx; whether to extend it to 408/409/provider 429 is explicitly carried as an open review question on #5553. Full-suite completion, exact-head hosted CI and security review remain pending.

Closing this duplicate standalone review entry as part of the requested consolidation after verifying coverage. This is not a merge or release claim; remaining integration checks and reviews are tracked on the replacement. Original branches are retained.

[lidge-jun]

Thanks @luvs01. This fence landed on dev in #5575 (b7351ddef3), with your authorship and a co-author trailer.

It was widened on the way in. After a spent replacement, 307, 308, 401, 402, 408, 409, 413, 429 and every 5xx now settle as the refusal, where before 429 and 529 escaped. Other errors keep their real status but are marked non-replayable, and the Codex pool's gated-model retry and combo hops now honor that marker. The reworded refusal message landed too, with the classifier matching the full sentence.