Skip to content

fix(responses): let retryOnReset replace chained native turns - #8486

Merged
lidge-jun merged 3 commits into
devfrom
codex/issues-streaming-1111
Nov 10, 2026
Merged

lidge-jun merged 3 commits into
devfrom
codex/issues-streaming-1111

Conversation

@lidge-jun

@lidge-jun lidge-jun commented Nov 10, 2026 •

Copy link
Copy Markdown
Owner

Summary

An operator who opts a native openai-responses provider into retryOnReset saw the replacement never spent on Codex turns. Every pre-header reset answered the synthetic 429 upstream_reset_replay_refused after one send, as reported for two key-auth providers in #8453 (78 of 78 resets).

Cause: a Codex turn chained with previous_response_id is expanded from the proxy's own replay state, and the native adapter strips the id before every send (stripPreviousResponseId in src/adapters/openai-responses/passthrough.ts). The ambiguous-reset judgment still read the inbound _rawBody, which keeps the id (expandPreviousResponseInput leaves it there for Kiro and Cursor). As a result, selfContainedResponsesBody refused every chained turn even though the wire body is complete and stateless.

  • nativePassthroughReplayJudgmentBody (src/server/responses/reset-replay.ts) judges the inbound body minus a previous_response_id that was expanded locally (_previousResponseInputExpanded). An unexpanded id is forwarded to the origin and still refuses.
  • Every ambiguous row on the native lane (pre-header, post-header SSE, Codex WebSocket create death) now goes through one authorizer that logs the gate's refusal reason: [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".
  • To the issue's scope question: yes, retryOnReset applies to key-auth openai-responses rows; the provider gate (resetReplayPolicyFor) has no auth-mode condition.
  • Docs: the retryOnReset row in reference/configuration/providers.md (English plus 7 locales) and structure/transports/responses-failover.md.
  • responses-reset-replay.test.ts moves from scripts/test-layout/layout.json into the layout-responses.json shard next to the new test. On dev, layout.json is exactly 2000 lines, which already fails file-size ratchet: repository (NEW_OVERSIZED). The move takes it to 1999.

Closes #8453

Verification

  • Failing first: bun test tests/responses/responses-reset-replay-chained.test.ts on 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.
  • Focused: the new file plus 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, and ci-workflows/file-size-ratchet: 0 failures.
  • bun run typecheck: pass. bun run privacy:scan: pass.
  • bun run structure:check: the only failure is structure/providers/anthropic-account-pool.md is 603 lines, which is identical on origin/dev and not touched here. responses-failover.md stays at its 600-line budget.
  • bun run test:changed (805 files): 20392 pass, 18 fail, 3 errors. 17 failures plus the 3 errors are tests/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 log socket 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 on origin/dev.

NOT RUN: the full bun run test (test:changed already 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 throws ECONNRESET at the provider fetch seam).

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.

Maintainer judgment

  • Value: without this, an opted-in operator gets no reset protection on chained Codex turns, which is most Codex App traffic. The new log line also gives [Bug]: Intermittent canonical OpenAI synthetic 429 upstream_reset_replay_refused on v2.91.0; request safe transport mitigation #7720-style reports a direct answer about why a reset was not replaced.
  • UX change scope: only providers that already opted into retryOnReset change 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 one console.warn line per refused ambiguous reset.
  • Regression evidence for existing users: an unexpanded previous_response_id still refuses, with one send, in the new test. All existing reset, replay, combo-parity, and WebSocket ambiguous-resend suites pass unchanged.
  • Windows impact: none specific; pure request-judgment logic and a log line.
  • Overlap: Fix native Responses recovery after prelude-only EOF #8292 also edits the self-contained judgment in passthrough-dispatch.ts and 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

    • Chained Responses requests can now be retried after a connection reset when the proxy supplied the prior response ID from its replay state. That ID is removed before resending; requests with an unrecognized ID remain ineligible.
  • Documentation

    • Clarified retry eligibility and documented reasons a reset may not be retried, including when no retry policy applies, the request cannot be replayed, or the retry allowance is exhausted.
    • Updated the relevant guidance across supported languages.

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
@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner November 10, 2026 18:06
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 10, 2026 •

Copy link
Copy Markdown
Codex Review Summary: ✅ Completed

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-10T18:07:59.332943Z 09acdca PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

github-actions Bot commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Nov 10, 2026
@coderabbitai

coderabbitai Bot commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

Native Responses passthrough now evaluates replayability after removing a locally expanded previous_response_id. Resend paths use a shared ambiguous-resend authorizer that logs refusal stage and reason. Tests cover replacement and refusal cases. Provider and failover documentation describe these rules.

Changes: Native Responses replay

Layer / File(s) Summary
Replay eligibility and resend authorization
src/server/responses/reset-replay.ts, src/server/responses/passthrough-dispatch.ts
Replay judgment omits previous_response_id when the proxy marked it as locally expanded. Passthrough resend callbacks use a shared authorizer for the applicable failure stage. The authorizer logs the stage and reason for ambiguous refusals.
Replay coverage and documentation
tests/responses/responses-reset-replay-chained.test.ts, tests/fixtures/test-layout-expected.json, scripts/test-layout/layout-responses.json, structure/transports/responses-failover.md, docs-site/src/content/docs/reference/configuration/providers.md, docs-site/src/content/docs/{fr,ja,ko,ru,tr,zh-cn,zh-tw}/reference/configuration/providers.md
Tests verify replacement when a chained request uses a locally expanded ID, refusal when the ID is unexpanded, and refusal without retry policy. They check refusal status and warning contents. Test-layout mappings and documentation include the chained replay behavior and refusal reasons.

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

Change: Bug fix · Severity of issue fixed: Medium

Possibly related issues

  • Issue 7720 — The PR adds logging of ambiguous resend refusal stages and reasons, which overlaps with the issue’s request for diagnosis of locally generated replay-refusal errors.

Possibly related PRs

  • lidge-jun/opencodex#7376 — That PR adds local previous_response_id expansion and replay-state tracking. This PR uses the local-expansion marker when judging native passthrough replayability.
Merge Risk: Minimal · up to ff7c9

This change lets retryOnReset replace chained native Responses turns whose previous_response_id the proxy expanded locally, and it logs the reason when an ambiguous reset is not replaced. No actionable merge-blocking risk was found. The author reports a live provider reset was not run, which is normal residual validation.

Pre-merge checks: Passed 5

Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely identifies the main change: allowing retryOnReset to replace chained native Responses turns.
Linked Issues check Passed Issue #8453 asks for refusal diagnostics, clarification that retryOnReset applies to key-auth openai-responses providers, and consideration of replay judgment against the outbound body. `src/serve…
Out of Scope Changes check Passed The code, tests, and documentation changes support #8453. The documentation updates describe chained-turn eligibility and refusal reasons. The test-layout changes retain replay-test coverage in the re…
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Full details: Linked Issues check

Explanation

Issue #8453 asks for refusal diagnostics, clarification that retryOnReset applies to key-auth openai-responses providers, and consideration of replay judgment against the outbound body. src/server/responses/reset-replay.ts adds ambiguousResendAuthorizer and logs the host label, stage, and gate refusal reason without logging body content or credentials. src/server/responses/passthrough-dispatch.ts uses this authorizer for pre-header and later ambiguous reset decisions. The same file now judges replayability through nativePassthroughReplayJudgmentBody: it removes previous_response_id only when the proxy expanded it locally, while an unexpanded ID remains non-replayable. The chained-turn test covers replacement and wire-body omission for the expanded ID, refusal for an unexpanded ID, and a refusal log that omits request text and the API key. The prior review established that resetReplayPolicyFor has no auth-mode condition and that a key-auth fixture opts in; this change does not alter that policy. The outbound-body wording in #8453 is a consideration, not a required general rewrite; the implementation addresses the identified locally expanded ID that is absent from the wire body.

Full details: Out of Scope Changes check

Explanation

The code, tests, and documentation changes support #8453. The documentation updates describe chained-turn eligibility and refusal reasons. The test-layout changes retain replay-test coverage in the responses shard and add the chained-turn test. The reviewed changes show no unrelated behavior or cleanup.

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 3 functions across 3 files. (1 skipped: 1 unsupported.)

Finishing Touches

Generate unit tests (beta)

  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

Recent review info

Run configuration

Commits

Reviewing files that changed from the base of the PR and between 09acdca and ff7c95b.

Files selected for processing: 1

  • scripts/test-layout/layout-responses.json
Files with no reviewable changes: 1

  • scripts/test-layout/layout-responses.json

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.

@lidge-jun
lidge-jun merged commit c225873 into dev Nov 10, 2026
35 of 37 checks passed
@lidge-jun
lidge-jun deleted the codex/issues-streaming-1111 branch November 10, 2026 18:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: retryOnReset replacements never spent for key-auth openai-responses providers — every pre-header reset answers synthetic 429 (v2.99.0)

1 participant