Skip to content

fix: classify unexpected transient HTTP statuses for retry - #5273

Open
Afloat16 wants to merge 1 commit into
milind-soni:mainfrom
Afloat16:fix/retry-unexpected-transient-status
Open

Afloat16 wants to merge 1 commit into
milind-soni:mainfrom
Afloat16:fix/retry-unexpected-transient-status

Conversation

@Afloat16

@Afloat16 Afloat16 commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

What changed

Remove the blanket unexpected status match from terminal request-error classification. Explicit authentication, quota, invalid-request, interruption, and safety-block checks retain their priority; 429 and 5xx errors now reach their existing transient classifiers.

Why

Codex reports transport failures with messages such as unexpected status 503. The shared retry classifier treated that prefix as an invalid request before checking the HTTP status. A temporary outage or short rate limit therefore stopped an otherwise retryable turn. The same prefix also misclassified a 404 as an invalid request instead of not found.

How it was verified

  • Node 24.22.1 and Vitest 4.1.11: 33 tests passed across server/drivers/retry.test.ts and server/drivers/retry-delay.test.ts using a temporary Node-only config with one worker.
  • Ten added status cases cover 429/500/502/503/504 across Error, text, and CLI-exit inputs, and preserve terminal 400/401/403/404/422 decisions. Six cases fail on the upstream base before the source fix.
  • Existing safety-block, quota, interruption, replay-limit, backoff and cancellation regressions pass.
  • git diff --check passes.
  • Full project installation was blocked by the active dependency release-age policy for the newly published CUA driver. Full typecheck, the repository-wide suite and isolated full-server flow have not been run; no policy override was applied.

Related history

Checked the original retry implementation (#388), current user contributions and searches for unexpected status retry handling. No existing fix for this prefix masking 429/5xx was found. The existing replay caps and guards against replaying answered or streamed turns remain in force.

Checklist

  • Full pnpm typecheck and pnpm test (dependency installation blocked as described above)
  • Server behavior change has regression tests
  • No dist-server/ edits
  • No platform-specific process launching or shell string-building
  • No secrets added to logs, responses, events, or argv

Summary by CodeRabbit

  • Bug Fixes
    • Temporary rate-limit and server errors are now recognized as retryable, helping requests recover from transient service issues.
    • Client, authentication, and not-found errors remain non-retryable.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@vercel

vercel Bot commented Nov 10, 2026

Copy link
Copy Markdown

@Afloat16 is attempting to deploy a commit to the SupaMaus Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented Nov 10, 2026 •

Copy link
Copy Markdown

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

The change removes “unexpected status” from the terminal error pattern. Tests verify that unexpected 429 and 5xx statuses are transient, while unexpected 400, 401, 403, 404, and 422 statuses remain terminal.

Changes: Retry classification

Layer / File(s) Summary
Unexpected-status classification
server/drivers/retry.ts, server/drivers/retry.test.ts
The terminal pattern no longer matches “unexpected status.” Tests cover transient classification for 429 and 5xx statuses and terminal classification for 400, 401, 403, 404, and 422 statuses.

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

Change: Bug fix

Possibly related issues

  • milind-soni/OpenMausBot#5012: The issue describes missing retries for transient 429 and 5xx errors. This change allows unexpected-status messages for those statuses to reach transient classification.

Possibly related PRs

Suggested reviewers: willsigmon

Merge Risk: Minimal · up to ba4a9

Unexpected transient statuses can now be retried. No merge-blocking risk was identified.

Pre-merge checks: Passed 5

Check name Status Explanation
Title check Passed The title clearly summarizes the main change: allowing unexpected transient HTTP statuses to reach retry classification.
Description check Passed The description covers what changed, why, verification, and the checklist. It reports the targeted tests and clearly identifies the full typecheck and repository-wide tests that could not be run becau…
Docstring Coverage Passed No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
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.
Full details: Description check

Explanation

The description covers what changed, why, verification, and the checklist. It reports the targeted tests and clearly identifies the full typecheck and repository-wide tests that could not be run because dependency installation was blocked. The screenshots section is not needed for this server-only change.

Full details: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. 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 0 functions across 2 files.

Finishing Touches

Generate unit tests (beta)

  • 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

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: f9e3903f-000e-40dd-90cd-4263b316566e
Commits

Reviewing files that changed from the base of the PR and between 9d6678a and ba4a929.

Files selected for processing: 2

  • server/drivers/retry.test.ts
  • server/drivers/retry.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.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant