feat(gui): main-card Re-login with device code (#3898) - #4441
Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks 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 |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
✅ Deterministic PR hygiene checks passed. |
⏳ DRAFT
What to do
This pull request was already a draft. Its draft status will be preserved after every issue above is resolved. |
리뷰 · 우선순위 72 / 80이 PR은 이슈 #3898의 L3입니다. L2(#4433)가 만든 전용 표면 현재 훅 설계는 L2 폴링 계약을 잘 따릅니다. 시작 POST는 빈 body(키가 있으면 400), 시작 응답에는 URL/코드가 비어 있을 수 있어서 바로 GET 폴링으로 채웁니다. 베이스는 라인 136 - 라인 68 - 라인 155-156 - verification URL을 라인 248-249 - 경로 메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
![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: d111f8d63b
ℹ️ 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(() => { | ||
| return () => { | ||
| unmountedRef.current = true; |
There was a problem hiding this comment.
The dashboard mounts under React.StrictMode (gui/src/main.tsx), whose development effect replay runs this cleanup and then sets the effect up again; because setup never restores unmountedRef.current to false, every subsequent start exits at the first polling guard after the POST and leaves the card stuck in starting while the server flow continues. The same permanent disablement occurs when apiBase changes in production, so set the flag to false in the effect setup before returning this cleanup.
Useful? React with 👍 / 👎.
| try { | ||
| await fetch(`${apiBase}/api/codex-auth/main/reauth-device?flowId=${encodeURIComponent(flowId)}`, { method: "DELETE" }); | ||
| } catch { /* best-effort: the flow expires on its own */ } | ||
| setState({ phase: "cancelled" }); |
There was a problem hiding this comment.
When the DELETE fails or races with credential publication, this code still unconditionally reports cancelled. The management API can explicitly return succeeded after publication (cancelMainDeviceReauth documents that publication wins), while a transport failure can leave the server flow active, so the operator may be told an authentication operation stopped even though it continues or already updated auth.json; inspect the HTTP/DTO result and refresh on success or expose a cancellation failure instead.
AGENTS.md reference: gui/AGENTS.md:L10-L10
Useful? React with 👍 / 👎.
| {mainReauth.state.verificationUrl && ( | ||
| <span>{t("codexAuth.mainReauthOpen")}: {mainReauth.state.verificationUrl}</span> | ||
| )} |
There was a problem hiding this comment.
When the verification URL arrives, the UI labels it “Open” but renders it as plain text, so neither pointer nor keyboard users can activate it and there is no copy control. This makes the new device-login flow depend on manually selecting a long URL; render an accessible link/button or reuse the existing LoginUrlBlock pattern.
AGENTS.md reference: gui/AGENTS.md:L33-L33
Useful? React with 👍 / 👎.
…ng flow [skip ci] Both device-reauth GUI tests failed on this branch before the merge. The first clicked button.codex-auth-action-btn, but the always-rendered pause control ships the same class and renders first, so the click hit pause and starts stayed 0; select by label the way the cancel test already does. The second awaited start() to completion while the mock always answers pending, and start() owns the flow until a terminal status, so it timed out at 5s; drive it and wait for the first poll to land instead. Product code is unchanged.
… ci] oxlint's react-compiler pass lowers a for statement and asserts its init is a variable declaration, so the empty init in for (;;) raised "Invariant: Expected a variable declaration" and blamed the enclosing hook. GUI lint runs in hosted CI, so this failed the gate. The loop body already returns on abort, unmount or a replaced flow id, and nothing follows the loop, so keying the loop on the same abort signal preserves behavior exactly.
1 participant
Summary
__main__grant from the dashboard.use-main-device-reauthhook drives ONLY the dedicated/api/codex-auth/main/reauth-devicenamespace (empty-body start, immediate poll until the URL/code arrive, flowId-owned polling, late responses ignored, unmount cancel). The poolAddCodexAccountModal/openReauthpath is deliberately untouched:/api/codex-auth/loginrejects__main__and would write the wrong credential store.https://auth.openai.com/codex/device; the human code must match a short alphanumeric shape; nothing persists to browser storage.codexAuth.*keys plus a revisedmainTokenExpired(no longer claims App login is the only path) in all 9 shipped locales. Structure claim in structure/gui-and-management-api.md.Stack (merge bottom-up):
Depends on #4433. Review this PR's diff only.
Verification
Checklist