Repository navigation
fix(responses): let retryOnReset replace chained native turns - #8486
Conversation
A Codex turn chained with previous_response_id is expanded from the proxy's own replay state, and the native Responses adapter strips the id before every send. The ambiguous-reset judgment still read the inbound body, which keeps the id, so every chained turn was refused and an operator's retryOnReset opt-in never applied to it. A pre-header reset answered the synthetic 429 upstream_reset_replay_refused after one send. Judge the native passthrough body with a locally expanded previous_response_id removed; an unexpanded id still reaches the wire and still refuses. Log the gate's refusal reason (ambiguous-no-policy, ambiguous-request-not-replayable, ambiguous-allowance-spent) with host and stage only, so an operator can tell why an opt-in did not apply. Move responses-reset-replay.test.ts into the responses layout shard so layout.json drops back under the 2000-line ratchet threshold. Closes #8453
Codex Review Summary: ✅ CompletedThis 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. |
|
✅ Deterministic PR hygiene checks passed. |
|
No actionable comments were generated in the recent review. Walkthrough
Merge Risk:
|
Summary
An operator who opts a native
openai-responsesprovider intoretryOnResetsaw the replacement never spent on Codex turns. Every pre-header reset answered the synthetic429 upstream_reset_replay_refusedafter one send, as reported for two key-auth providers in #8453 (78 of 78 resets).Cause: a Codex turn chained with
previous_response_idis expanded from the proxy's own replay state, and the native adapter strips the id before every send (stripPreviousResponseIdinsrc/adapters/openai-responses/passthrough.ts). The ambiguous-reset judgment still read the inbound_rawBody, which keeps the id (expandPreviousResponseInputleaves it there for Kiro and Cursor). As a result,selfContainedResponsesBodyrefused every chained turn even though the wire body is complete and stateless.nativePassthroughReplayJudgmentBody(src/server/responses/reset-replay.ts) judges the inbound body minus aprevious_response_idthat was expanded locally (_previousResponseInputExpanded). An unexpanded id is forwarded to the origin and still refuses.[upstream-retry] ambiguous reset not replaced (<host>, <stage>): ambiguous-no-policy | ambiguous-request-not-replayable | ambiguous-allowance-spent. The line carries only the host label and stage, never body content or credentials. This answers the issue's first ask: an operator can now tell "not opted in" apart from "judged not replayable" and "already spent".retryOnResetapplies to key-authopenai-responsesrows; the provider gate (resetReplayPolicyFor) has no auth-mode condition.retryOnResetrow inreference/configuration/providers.md(English plus 7 locales) andstructure/transports/responses-failover.md.responses-reset-replay.test.tsmoves fromscripts/test-layout/layout.jsoninto thelayout-responses.jsonshard next to the new test. Ondev,layout.jsonis exactly 2000 lines, which already failsfile-size ratchet: repository(NEW_OVERSIZED). The move takes it to 1999.Closes #8453
Verification
bun test tests/responses/responses-reset-replay-chained.test.tson the unmodified tree failed all 3 cases: the chained turn returned 429 instead of 200, and neither refusal reason was logged. With the fix: 3 pass.responses-reset-replay,responses-translated-reset,ws-ambiguous-resend,server/replay-refusal-parity,empty-completion-core,responses-canonical-nonstream,lib/ambiguous-resend-composition,test-layout,test-layout-tooling,lab/core-lab-boundary, andci-workflows/file-size-ratchet: 0 failures.bun run typecheck: pass.bun run privacy:scan: pass.bun run structure:check: the only failure isstructure/providers/anthropic-account-pool.md is 603 lines, which is identical onorigin/devand not touched here.responses-failover.mdstays at its 600-line budget.bun run test:changed(805 files): 20392 pass, 18 fail, 3 errors. 17 failures plus the 3 errors aretests/server/local-control-socket.test.ts/ the [Bug]: link status returns 403 on non-loopback hub management listener #8184 hub tests in this checkout, which logsocket path is too long(the worktree path exceeds the Unix socket limit). The remaining failure,catalog-omit-disabled-native.test.ts"retained sync and serve-options omit the same rows", fails identically onorigin/dev.NOT RUN: the full
bun run test(test:changedalready covered 805 files, and the only failures are the environment and pre-existing cases above), macOS/Windows lanes, and a live provider reset (the fixture throwsECONNRESETat the provider fetch seam).Checklist
Maintainer judgment
retryOnResetchange behavior, and only for chained turns whose id was expanded locally. Those turns can now be replaced once, as the operator asked. Providers without the opt-in are unchanged apart from oneconsole.warnline per refused ambiguous reset.previous_response_idstill refuses, with one send, in the new test. All existing reset, replay, combo-parity, and WebSocket ambiguous-resend suites pass unchanged.passthrough-dispatch.tsand will need a small rebase. Its outbound-body guard ANDs with the inbound judgment, so this fix is still needed after it lands. I checked: Fix native Responses recovery after prelude-only EOF #8292 alone still refuses the chained-turn case.Summary by CodeRabbit
Bug Fixes
Documentation