fix(server): Codex network waits no longer show a stale retry limit#29224

open1

main

GitHub fork: fix/codex-retry-stale-max-attempts

opened8d agobyAafrojun

View on GitHubTwo-way sync enabledPR last imported 10h ago

Problem

When a Codex turn loses its network connection, the retry row climbs past its own limit. I hit Retrying provider (272/5) in a delegated Codex thread that sat offline for about 17 hours.

The provider event log for that thread shows the cause. Codex sent error notifications with willRetry: true and the messages Reconnecting... 2/5 through Reconnecting... 5/5. It then switched to Reconnecting... waiting for network, which has no count and no limit, and sent that 270 times until the turn was cancelled.

Change

CodexAdapterV2 reads N/M from the retry message. For a message without a count it adds one to the previous attempt, which is still right, but it also kept the previous maximum. It now sets maxAttempts to null in that case. The existing web label in providerErrorPresentation already renders a null maximum as a bare count, so the row reads Retrying provider (10) rather than (11/5).

Messages with an explicit N/M behave as before. The capacity-retry path enforces its own capacityRetryAttempt limit and is unaffected.

Related: #29208 adds a deadline to the same open-ended network wait. It doesn't touch maxAttempts, so without this fix its failed row would still read like Provider error after 14/5 retries. The two changes are independent and don't conflict.

Scope and approval

A very small, focused fix for an obvious display bug: a retry counter shown above its own maximum. It changes one line of adapter logic plus its regression test. Retry behaviour, defaults and other providers are unchanged.

Verification

Regression test. I extended the existing resolves retryable app-server errors on resumed provider activity case in CodexAdapterV2.test.ts with a Reconnecting... waiting for network frame copied from the real log.

On main it fails with expected { attempt: 3, maxAttempts: 5 } to deeply equal { attempt: 3, maxAttempts: null }.
With the fix, all 133 tests in the file pass, and vp run check:changed passes (format, lint, knip, typecheck, tests).

Real Codex, before and after. I ran codex app-server 0.166.0 under vp run dev from this branch, then again with main's adapter line restored. Both runs used an empty dev database, no credentials, and a local model_providers endpoint that drops the first streams mid-response and then stops listening. In both runs Codex emitted the same sequence as the original incident: 1/5 to 5/5 (stream disconnected before completion), then waiting for network (Connection failed: error sending request).

Before (`main`)	After (this PR)
Before: Retrying provider (11/5)	After: Retrying provider (10)

The before capture was taken a little later in its run, which is why the counts differ (11 vs 10).

Other surfaces. Desktop uses the same web label. apps/mobile doesn't render the retry count, so mobile has nothing to change.

Code by Claude Opus 5.5 in Claude Code (running in T3 Code), reviewed by gpt-6.1-sol in Codex.

🤖 Generated with Claude Code

Fixes pingdotgg/t3code#30791

Checks

21 successful · 8 skipped

All checks passed

MmacroscopeappBotapproved these changes8d ago

MmacroscopeappBot8d ago

Approvability

Verdict: Approved at 384c605

Macroscope's review found this PR approvable — Codex network-wait status now drops a stale retry limit instead of continuing to display it. The change affects retry progress reporting, not retry execution or product defaults.

^{You can add or adjust custom eligibility rules. Learn more.}

CcoderabbitaiBot8d ago

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

Codex retry events now use a null maximum attempt count when the current message has no parsed retry limit. The replay test covers a subsequent retry with a null HTTP status and the recovery update.

Changes: Codex retry attempt limits

Layer / File(s)	Summary
Retry limit handling and test coverage `apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.ts`, `apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.test.ts`	Retry events use the parsed retry limit or null. The test checks the subsequent retry update and completion of the original item as “Provider recovered.”

Estimated code review effort: 2 (Simple) | ~8 minutes

Possibly related issues

pingdotgg/t3code#30791: The change clears the stale maxAttempts value during open-ended “waiting for network” retries.
pingdotgg/t3code#28936: The change updates the retry label when a Codex retry message has no explicit count, but does not implement the requested Limited state or timeout.
pingdotgg/t3code#31446: The change clears the stale maximum during network retries, but does not address stalled turns or missing activity events.

Possibly related PRs

pingdotgg/t3code#30814: Both changes set maxAttempts to null when a Codex retry notification has no parsed maximum and extend the replay test for network-wait notifications.
pingdotgg/t3code#32725: Both changes clear the stale maxAttempts value for Codex retry notifications without a parsed maximum and test network-wait and recovery updates.

Suggested reviewers: juliusmarminge

Merge Risk: External image Minimal · up to 137fe

No concrete merge-blocking risk is identified in the supplied evidence.

Pre-merge checks: ^Passed 4

Check name	Status	Explanation
Title check	Passed	The title clearly summarizes the main change: Codex network waits no longer retain a stale retry limit.
Linked Issues check	Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	Passed	Check skipped because no linked issues were found for this pull request.
Description check	Passed	The description covers the problem, change, scope rationale, and focused verification results. It also identifies the regression test, reported checks, and limitations.

Finishing Touches

Generate unit tests (beta)

Create a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

External image Recent review info

External image Run configuration

Source mode: Auto
Repository: pingdotgg/t3code
Branch: main
File: .coderabbit.config.ts
Commit: ff7aa5a
Layers: Repository TS config
Review profile: CHILL
Plan: Advanced
Run ID: `766c7b36-4ce1-4de1-ae2a-bf4e1b0dbea3`

External image Commits

Reviewing files that changed from the base of the PR and between 384c6054806bb679c1ab9add4c58a93861e84693 and 137feb2de915c225408dd613f05b304bae05a721.

External image Files selected for processing: 2

`apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.test.ts`
`apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.ts`

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

Mmaria8d ago

Note

Written by gpt-6.1-sol on behalf of Maria

Verified in a real dev environment

verified head 384c6054806bb679c1ab9add4c58a93861e84693 as-is on linux. reproduced the bug first on github/main at 55e25385619c7680dad0b0f34f5185187806c57a, then stopped that server and ran the exact pr head with isolated state.

used repository playwright and /usr/bin/google-chrome to add a real git project, select codex gpt-6.1-sol, and submit real turns through the web composer. codex app-server was 0.166.0. a task-owned transport forwarded actual responses from the local provider proxy, cut streams, then closed its listener to induce network waiting. no mock events, fixtures, or fabricated model responses.

main showed Retrying provider (6/5) and continued above five. its persisted network-wait retry retained maxAttempts: 5.
the pr preserved numbered retries, then showed Retrying provider (6) during network waiting. its persisted retry had maxAttempts: null.
restoring connectivity completed the same turn, showed Provider recovered (7 retries), and returned network recovery ok..
stopping another network-wait turn showed Provider retry stopped (6) and Stopped by user. subsequent real turns returned after cancellation ok..
blacksmith: all 133 adapter tests passed. vp run check:changed HEAD^ passed format, lint, knip, graphql, three package typechecks, and 134 tests. the explicit base was needed because the worker snapshot initially lacked the merge base.

before, main:

main shows a stale retry limit at 6/5

after, exact pr head with the same project and recovery prompt:

pr clears the stale maximum during network waiting

real codex numbered retries, network waiting, and recovery

cancellation stops network waiting and a subsequent turn succeeds

runtime command transcript

Not verified

native desktop, ios, and android clients were not launched on this linux vm. other providers, capacity retries, and a 17-hour network outage were not exercised. the tested outage was brief and used real transport failures against the local proxy. no code changes or pushes were needed.

Added 1 commit1d ago

fix(server): Codex network waits no longer show a stale retry limit137feb2