Fix Codex app-server failure recovery - #784
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
Warning Review limit reached
Next review available in: 7 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThe chat runtime now deduplicates Codex errors and distinguishes retryable failures. The renderer derives turn activity from transcript evidence, displays provider recovery actions, supports retrying failed turns, and can open the model picker programmatically. ChangesProvider failure recovery
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
arul28
changed the title
|
@copilot review but do not make fixes |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review |
![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: 756181db90
ℹ️ 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".
|
Deployment failed with the following error: Learn More: https://vercel.com/arul28s-projects?upgradeToPro=build-rate-limit |
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
03f7e9b to
7e5f12f
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/renderer/components/chat/AgentChatPane.tsx (1)
6706-6763: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winSurface a rejection state for provider-failure retries
onRetryProviderFailurenow callsresendLastUserMessage(...)directly, butCHAT_AUTH_RETRY_REJECTED_EVENTis only handled byAgentCliAuthCard. If the resend is rejected locally, the provider-failure card stays unchanged and the click reads as a no-op. Surface an inline error or rejection state here too.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/components/chat/AgentChatPane.tsx` around lines 6706 - 6763, The provider-failure retry can be rejected without updating this pane, leaving the retry card unchanged. Update resendLastUserMessage and its callers to expose a local rejection state or inline error when submitInFlightRef is already set, no user message is found, or the send is rejected as already active; render that state in the provider-failure UI so the retry click visibly communicates the rejection.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/components/chat/chatTurnState.ts (1)
11-45: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider dedicated unit tests for these pure helpers.
findUserMessageForTurnandtranscriptLatestTurnIsTerminalencode several non-trivial boundary rules (steer exclusion, differing-turn breaks, reopening signals). They're only exercised indirectly today throughAgentChatPane.test.tsxintegration tests. Direct unit tests would pin down edge cases (e.g., interleaved subagent events, multiple back-to-back turns) more cheaply than debugging through the full pane.Also applies to: 48-68, 70-80
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/components/chat/chatTurnState.ts` around lines 11 - 45, Add dedicated unit tests for the pure helpers findUserMessageForTurn and transcriptLatestTurnIsTerminal, covering steer-message exclusion, differing-turn and turn-boundary breaks, interleaved subagent events, reopening signals, and multiple back-to-back turns. Keep these tests focused on helper inputs and outputs rather than relying on AgentChatPane integration tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/desktop/src/renderer/components/chat/AgentChatPane.tsx`:
- Around line 6713-6728: Update the fallback scan in resendLastUserMessage to
exclude user_message events with a steerId, matching findUserMessageForTurn
behavior. Only select non-steer user messages when locating the original prompt
for CHAT_RETRY_AUTH_TURN_EVENT retries.
- Around line 3043-3046: Reset modelPickerOpenRequestKey in the same
session-switch reset effect that clears other transient per-session UI state,
using setModelPickerOpenRequestKey(undefined), so AgentChatComposer cannot carry
the pending request into the next session.
In `@apps/desktop/src/renderer/components/chat/ProviderFailureRecoveryCard.tsx`:
- Around line 9-30: Update classifyProviderFailure to prioritize the structured
event.errorInfo.category before inspecting freeform text: map the "busy"
category to the existing capacity recovery and "rate_limit" to the existing
usage-limit recovery. Preserve the normalized identity matching as a fallback
for cases without recognized structured categories, including existing
server-overloaded, model-at-capacity, usage-limit, and rate-limit phrases.
---
Outside diff comments:
In `@apps/desktop/src/renderer/components/chat/AgentChatPane.tsx`:
- Around line 6706-6763: The provider-failure retry can be rejected without
updating this pane, leaving the retry card unchanged. Update
resendLastUserMessage and its callers to expose a local rejection state or
inline error when submitInFlightRef is already set, no user message is found, or
the send is rejected as already active; render that state in the
provider-failure UI so the retry click visibly communicates the rejection.
---
Nitpick comments:
In `@apps/desktop/src/renderer/components/chat/chatTurnState.ts`:
- Around line 11-45: Add dedicated unit tests for the pure helpers
findUserMessageForTurn and transcriptLatestTurnIsTerminal, covering
steer-message exclusion, differing-turn and turn-boundary breaks, interleaved
subagent events, reopening signals, and multiple back-to-back turns. Keep these
tests focused on helper inputs and outputs rather than relying on AgentChatPane
integration tests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e301e61e-ca45-4185-91eb-7b623cb7d940
⛔ Files ignored due to path filters (3)
docs/features/chat/README.mdis excluded by!docs/**docs/features/chat/composer-and-ui.mdis excluded by!docs/**docs/features/chat/transcript-and-turns.mdis excluded by!docs/**
📒 Files selected for processing (11)
apps/desktop/src/main/services/chat/agentChatService.test.tsapps/desktop/src/main/services/chat/agentChatService.tsapps/desktop/src/renderer/components/chat/AgentChatComposer.tsxapps/desktop/src/renderer/components/chat/AgentChatMessageList.tsxapps/desktop/src/renderer/components/chat/AgentChatPane.test.tsxapps/desktop/src/renderer/components/chat/AgentChatPane.tsxapps/desktop/src/renderer/components/chat/ProviderFailureRecoveryCard.tsxapps/desktop/src/renderer/components/chat/chatTranscriptRows.test.tsapps/desktop/src/renderer/components/chat/chatTranscriptRows.tsapps/desktop/src/renderer/components/chat/chatTurnState.tsapps/desktop/src/renderer/components/shared/ModelPicker/ModelPicker.tsx
|
Resolved the latest CodeRabbit review batch:
I resolved the @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: afb0ded433
ℹ️ 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".
|
@codex review |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
1 participant
Summary
errornotification and failedturn/completed, while preserving distinct failuresProtocol basis
Official Codex app-server semantics allow a top-level error notification before terminal
turn/completed;willRetry: truemeans the app-server is retrying and the turn remains active. ADE now distinguishes that retry notice from a terminal provider failure.Verification
/quality: no remaining Blocker/High findings after two re-review passesSeparate verified follow-ups
These symptoms were investigated and are not caused by the chat lifecycle bug, so this PR does not mix speculative search/CLI changes into the recovery fix:
error/errorInfopayloads, soserverOverloadedcannot be found from the original error eventade chat list --lane … --textandade lanes list --textboth return data; the earlier blank-output report did not reproduce on current main/runtimeSummary by CodeRabbit
New Features
Bug Fixes
Greptile Summary
This PR improves Codex app-server failure recovery in chat. The main changes are:
errorevents and failedturn/completedevents.willRetry: trueprovider errors as non-terminal health notices.Confidence Score: 5/5
Safe to merge with low risk.
The changed paths are focused on chat lifecycle and recovery state, and no current runtime or security bugs were identified.
No files require special attention.
What T-Rex did
Important Files Changed
Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant Codex as Codex app-server participant Main as agentChatService participant Transcript as Transcript history participant Pane as AgentChatPane participant User as User Codex->>Main: error willRetry true Main->>Transcript: provider health system notice Codex->>Main: terminal error willRetry false Main->>Transcript: emit error once Codex->>Main: turn completed failed same payload Main->>Transcript: suppress duplicate and emit status done Transcript->>Pane: hydrate or live refresh events Pane->>Pane: terminal transcript overrides stale active summary Pane->>User: show composer and recovery card User->>Pane: retry turn or choose model Pane->>Main: resend original message or open picker%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant Codex as Codex app-server participant Main as agentChatService participant Transcript as Transcript history participant Pane as AgentChatPane participant User as User Codex->>Main: error willRetry true Main->>Transcript: provider health system notice Codex->>Main: terminal error willRetry false Main->>Transcript: emit error once Codex->>Main: turn completed failed same payload Main->>Transcript: suppress duplicate and emit status done Transcript->>Pane: hydrate or live refresh events Pane->>Pane: terminal transcript overrides stale active summary Pane->>User: show composer and recovery card User->>Pane: retry turn or choose model Pane->>Main: resend original message or open pickerReviews (6): Last reviewed commit: "ship: iteration 3 — consume disabled mod..." | Re-trigger Greptile