fix(provider): retry transient errors on first stream item before ending turn - #10968
Conversation
# Conflicts: # crates/goose/src/agents/reply_parts.rs
|
Thanks for starting this, @Abhijay007! I updated your branch with current |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4d7cdb4a24
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if !skip_backoff { | ||
| tokio::time::sleep(delay).await; |
There was a problem hiding this comment.
When the first stream item is a transient error, this sleep prevents the legacy agent loop from responding to cancellation until the entire backoff finishes, because crates/goose/src/agents/agent.rs:2466-2468 awaits stream.next() before checking its cancellation token. A provider-supplied retry_delay can make Stop appear hung for an arbitrarily long time, while the state-machine path remains cancellable through the tokio::select! in ops_llm.rs:513-518; make this retry wait cancellation-aware in both agent-loop paths.
AGENTS.md reference: AGENTS.md:L19-L23
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fc66ca562f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if goose_providers::retry::should_retry(&error, &retry_config) | ||
| && attempts < retry_config.max_retries => |
There was a problem hiding this comment.
When an OpenAI-compatible gateway sends a deterministic client/validation failure as the first SSE frame, this retry check treats it as transient because the existing parser maps choice-less status/statusCode/code >= 400 and detail error frames to ProviderError::ServerError (crates/goose-provider-types/src/formats/openai.rs:1113-1137). That means a bad model/payload/auth-shaped in-stream 4xx now replays the same invalid request until max_retries and delays surfacing the real error; discriminate 4xx/validation frames before applying the transient retry policy.
Useful? React with 👍 / 👎.
* origin/main: fix(mcp): prune dead notification subscribers (#11032) chore: remove the extension and tool count suggestion (#10869) feat: compaction in the GDK (#11042) fix(provider): retry transient errors on first stream item before ending turn (#10968) feat(cli): add /new to start a fresh session without restarting (#10767) feat(acp): title new sessions from _meta.sessionTitle (#10712) fix: adjust rmcp::model::Meta ref (#11107) Skip hook loading and lifecycle events for subagents (#10596) Sanitize Unicode tags in Responses output (#10745) fix(conversation): sanitize nested tool responses (#10609) fix(hints): bound recursive file expansion (#10546) fix(providers): drop stale signed thinking blocks after a mid-conversation model switch (#10007) fix(desktop): clarify compact cost display (#11093) Index messages by (session_id, created_timestamp, id) to stop on-disk sort storms (#10874) docs: add tool shim guide covering when to enable, backends, and troubleshooting (#10858) fix(deep-link): route extension/session deep links to regular windows not standalone app windows (#10908) fix(ui): raise chat input z-index so slash menu appears above loading indicator (#11015) fix(ui): support remote working directory for external backend (#10827)
* main: fix(mcp): prune dead notification subscribers (#11032) chore: remove the extension and tool count suggestion (#10869) feat: compaction in the GDK (#11042) fix(provider): retry transient errors on first stream item before ending turn (#10968) feat(cli): add /new to start a fresh session without restarting (#10767) feat(acp): title new sessions from _meta.sessionTitle (#10712) fix: adjust rmcp::model::Meta ref (#11107) Skip hook loading and lifecycle events for subagents (#10596) Sanitize Unicode tags in Responses output (#10745) fix(conversation): sanitize nested tool responses (#10609) fix(hints): bound recursive file expansion (#10546) fix(providers): drop stale signed thinking blocks after a mid-conversation model switch (#10007) fix(desktop): clarify compact cost display (#11093) Index messages by (session_id, created_timestamp, id) to stop on-disk sort storms (#10874) docs: add tool shim guide covering when to enable, backends, and troubleshooting (#10858) fix(deep-link): route extension/session deep links to regular windows not standalone app windows (#10908) fix(ui): raise chat input z-index so slash menu appears above loading indicator (#11015) fix(ui): support remote working directory for external backend (#10827)
3 participants
Fixes: #10887
Summary
Some providers on OpenRouter (poolside being a common one) frequently hit 429s that show up as the first item in the response stream, after the connection itself succeeds. The existing retry logic only covers the HTTP call, so goose was treating these as hard failures and ending the turn.
Fix peeks the first stream item before passing the stream back. Transient errors get retried with backoff;
non-transient errors and anything after the first successful chunk pass through as-is. Providers that manage their own
context are skipped to avoid replaying state.
Testing
Verification:
cargo test -p goose --lib -- agents::reply_parts::tests::first_itemagents::
reply_parts::tests::empty_stream agents::reply_parts::tests::error_after— 7 tests, all passing.