Skip to content

fix(tui): retry Codex responses requests#3344

Closed
cyq1017 wants to merge 3 commits into
Hmbown:mainfrom
cyq1017:codex/codewhale-3019-responses-retry
Closed

fix(tui): retry Codex responses requests#3344
cyq1017 wants to merge 3 commits into
Hmbown:mainfrom
cyq1017:codex/codewhale-3019-responses-retry

Conversation

@cyq1017

@cyq1017 cyq1017 commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Fixes #3019.

Problem:

  • The Codex Responses streaming path still sent the /codex/responses request once and returned immediately on retryable transport/status failures.

Change:

  • Route the Responses request through send_with_retry while rebuilding the request body and headers per attempt.
  • Add a focused regression test that returns a retryable 429 before a successful SSE stream.

Verification:

  • cargo test -p codewhale-tui client::responses
  • cargo fmt --all -- --check
  • git diff --check

Known gate:

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the Responses API request in DeepSeekClient to use a retry mechanism (send_with_retry) and adds an integration test to verify retries on rate-limited requests. The review feedback points out that calling crate::oauth::codex_account_id() inside the retry closure causes redundant synchronous disk I/O on every retry attempt, and suggests resolving the account ID once outside the closure.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +90 to +103
let response = self
.send_with_retry(|| {
let mut builder = self
.http_client
.post(&url)
.header("Content-Type", "application/json")
.header("Accept", "text/event-stream")
.header("OpenAI-Beta", "responses=experimental")
.header("originator", "codex_cli_rs");
if let Some(account_id) = crate::oauth::codex_account_id() {
builder = builder.header("chatgpt-account-id", account_id);
}
builder.json(&body)
})

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Calling crate::oauth::codex_account_id() inside the retry closure causes a synchronous disk read of the Codex credentials file (auth.json) on every retry attempt. To avoid redundant synchronous I/O in an asynchronous context, resolve the account ID once outside the closure and capture it by reference.

Suggested change
let response = self
.send_with_retry(|| {
let mut builder = self
.http_client
.post(&url)
.header("Content-Type", "application/json")
.header("Accept", "text/event-stream")
.header("OpenAI-Beta", "responses=experimental")
.header("originator", "codex_cli_rs");
if let Some(account_id) = crate::oauth::codex_account_id() {
builder = builder.header("chatgpt-account-id", account_id);
}
builder.json(&body)
})
let account_id = crate::oauth::codex_account_id();
let response = self
.send_with_retry(|| {
let mut builder = self
.http_client
.post(&url)
.header("Content-Type", "application/json")
.header("Accept", "text/event-stream")
.header("OpenAI-Beta", "responses=experimental")
.header("originator", "codex_cli_rs");
if let Some(account_id) = &account_id {
builder = builder.header("chatgpt-account-id", account_id);
}
builder.json(&body)
})

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

@github-actions

Copy link
Copy Markdown

Thanks @cyq1017 — your contribution landed in 9730e3d0ff96 on main:

fix(tui): retry Codex responses requests

Closing this PR now that the code is on main. Credit lives in the commit message and (where applicable) the CHANGELOG.md entry for the next release. Apologies for not closing this at the time of the merge — the auto-close workflow is new in v0.8.31.

If you want to land more work and would prefer your future PRs merge cleanly without a harvest step, the CONTRIBUTING.md doc has a short note on what makes a contribution mergeable as-is.

@github-actions github-actions Bot closed this Jun 21, 2026
hongqitai pushed a commit to hongqitai/CodeWhale that referenced this pull request Jun 21, 2026
Route the Codex Responses stream request through the shared retry stack so retryable transport and HTTP failures get the same backoff, retry-status banner, and request health accounting as the chat path.

Add a wiremock regression that returns a retryable 429 before a successful SSE stream, and resolve the optional Codex account id once before retry attempts.

Harvested from PR Hmbown#3344 by @cyq1017.

Refs Hmbown#3019, Hmbown#2487.

Verified with: cargo test -p codewhale-tui client::responses --locked

Verified with: cargo fmt --all -- --check

Co-authored-by: cyq1017 <61975706+cyq1017@users.noreply.github.com>
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.

v0.8.58: Codex/Responses client reliability — retry/backoff parity, land function_call_output + xhigh effort fixes

1 participant