refactor(gui): react-doctor cleanup for Codex accounts#475
Conversation
|
Warning Review limit reached
Next review available in: 40 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: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (28)
📝 WalkthroughWalkthroughThe PR decomposes Codex account OAuth and pool management into reducer-driven state, dedicated OAuth and modal components, reusable account-pool presentation components, and centralized reset-credit handling. ChangesCodex account OAuth flow
Codex account pool
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant AddCodexAccountModal
participant useAddCodexAccountOAuth
participant CodexAuthAPI
AddCodexAccountModal->>useAddCodexAccountOAuth: startOAuth(requestedId)
useAddCodexAccountOAuth->>CodexAuthAPI: POST start login
CodexAuthAPI-->>useAddCodexAccountOAuth: flowId and authUrl
useAddCodexAccountOAuth->>CodexAuthAPI: poll login status
CodexAuthAPI-->>useAddCodexAccountOAuth: login status
useAddCodexAccountOAuth-->>AddCodexAccountModal: update reducer UI state
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![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: 6d35e51525
ℹ️ 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".
|
|
||
| useEffect(() => () => { | ||
| clearManualCode(); | ||
| aliveRef.current = false; |
There was a problem hiding this comment.
In the default development render, gui/src/main.tsx wraps the app in React.StrictMode, so React runs this effect's cleanup and setup again on mount. The first cleanup permanently sets aliveRef.current to false and aborts the initial reauthentication request; the replayed auto-start is then skipped because startedReauthRef still contains the account ID, while manually started add flows discard their responses at the subsequent liveness check. Consequently, adding or reauthenticating an account never progresses in the development app. Restore liveness during effect setup and reset or otherwise make the reauthentication guard safe to replay.
AGENTS.md reference: AGENTS.md:L65-L68
Useful? React with 👍 / 👎.
| }); | ||
| type LoginResponse = { url?: string; flowId?: string; error?: string; status?: string }; | ||
| let resp = await requestLogin(); | ||
| let data = await readJsonIfOk<LoginResponse>(resp); |
There was a problem hiding this comment.
readJsonIfOk returns null without consuming the body for every non-2xx response, so the structured errors returned by src/codex/auth-api.ts for invalid IDs, duplicate IDs, unknown reauthentication accounts, and server-side OAuth failures are now replaced with the misleading modal.networkError message. These are expected 400/404/500 API outcomes rather than connectivity failures; check the status while still parsing and displaying the response's error field.
Useful? React with 👍 / 👎.
| <> | ||
| <div style={{ textAlign: "center", padding: "12px 0" }}> | ||
| <div className="confirm-icon"><IconAlert width={22} /></div> | ||
| <h3>{t("codexAuth.confirmResetTitle")}</h3> |
There was a problem hiding this comment.
When resetConfirm becomes true, the element carrying id="codex-reset-title" is removed, but the native dialog continues to use aria-labelledby="codex-reset-title". The destructive confirmation step therefore has no accessible dialog name for screen-reader users. Give the confirmation heading the referenced ID as well, or switch aria-labelledby to an ID that remains mounted across both steps.
Useful? React with 👍 / 👎.
Codex review follow-up (6b0ca63)Addressed all three findings after rebase onto
Verification: related Codex GUI tests 41/41 pass; |
6d35e51 to
6b0ca63
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Addressed open CodeRabbit findings on Fixed
Skipped
Verified
|
Remaining blockers addressed (
|
3853728 to
236ba2c
Compare
Make aliveRef StrictMode-replay-safe, preserve structured login errors on non-2xx, and keep the reset dialog labelled during confirmation.
Guard undefined login JSON bodies and point source-contract tests at the extracted OAuth/card modules.
Avoid inventing reset-credit counts, clear toast error tone on redeem, guard OAuth status polling, and localize leftover English fallbacks.
Clear startedReauthRef on cleanup so StrictMode remount can restart OAuth, abort in-flight login-status polls on cancel, and add focused regression tests for reauth, single-flight polling, already_redeemed, and toast tone.
Refresh WHAM quota after reset/already_redeemed and return remaining from stored quota. Drop GUI knownCredits-1 and modal-snapshot arithmetic so toasts never invent a decrement.
After reset/already_redeemed, return remaining only from a freshly parsed rate_limit_reset_credits.available_count so stale cached credits never leak.
236ba2c to
3fcdd96
Compare
1 participant
Summary
pollErrorStreakRef), fetch status checks, native dialogs,type="button".Stack
Test plan
bun x tsc -bingui/Summary by CodeRabbit