fix(desktop): show the Codex device sign-in code on connection-detail re-login

[sosyz]

Summary

The connection detail sheet's re-login notice (OAuthReloginNotice) drives the same browser-assisted OAuth flow as the provider catalog panel, but never rendered the flow's stateHint. For Codex that hint is the 9-character device user code the authorization page requires — the verification URL is the static device page and does not embed the code — so a re-login started from the notice could never be completed: the browser asked for a code the app never showed.

• OAuthLoginService gains a showsDeviceCode flag: true for Codex, false for xAI, whose authorization page needs no manual code (mirroring the catalog panel's existing !isXai guard). The mapping stays in use-connection-detail.ts so the legacy AppShell closure gains no file and no dependency under the renderer architecture ratchet.
• OAuthReloginNotice appends the sign-in code to the banner description while authorization is pending, reusing the catalog panel's deviceCode copy so both entry points speak the same words.

The second commit re-applies #3381 (normalize locked npm registry sources). That fix was reverted on main by #3148's lockfile regeneration six hours after it merged, so npm ci under allow-remote=none fails again on main; it is included here so the branch installs cleanly. Happy to drop it if the maintainers prefer a separate PR.

Fixes #3357

Verification

• npm run lint, npm run format:check, npm run check:asf-headers — clean
• npm run typecheck (desktop) — clean
• npm run check:architecture -- --base <main> — passed
• npm --workspace @maka/desktop test — 1695 passed, 0 failed
• npx knip --workspace apps/desktop — clean
• Manual: launched the dev app, opened the Codex connection detail, clicked re-login — the 9-character sign-in code now appears in the banner while the browser shows the device page.

Before (en) — no code while authorization is pending

After (en) — Sign-in code: … shown in the banner

After (zh) — 登录码：… shown in the banner

AI use

Select exactly one:

• No generative tool made a substantive contribution
• Generative tooling made a substantive contribution

Tool(s) and scope: Claude Code (Fable 5) performed the diagnosis, implementation, and verification. The human contributor reviewed and manually verified the result and authorized submission. Commits carry a Generated-by trailer.

Checklist

• Tests cover the change and fail without it — see Verification
• Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

• Yes — described under Summary above
• No

🤖 Generated with Claude Code

[jackwener]

Automated Codex review on exact head dfe2323d669b2af8513c99d27a4060ea1be21334.

The bug still exists on current main: OAuthReloginNotice starts the shared OAuth flow but drops flow.stateHint, while the catalog path renders it for Codex. The JSX change correctly restores the required device code and excludes xAI.

P3 / simplification: the new 68-line test does not assert the repaired behavior. It only pins showsDeviceCode and bridge routing; deleting the <code>{deviceCode}</code> rendering would leave all four tests green. Moving the existing service mapping into a 44-line leaf module was done mainly to make that indirect test loadable, so the test creates an extra production seam without protecting the user-visible regression.

Prefer either:

• replace it with coverage that asserts the notice description actually contains Codex's stateHint and not xAI's; or
• if the repository has no honest component harness for this surface, delete the indirect test and keep the mapping in its existing owner, relying on focused human/UI verification rather than a misleading pin.

Required conclusions:

1. Optimal for the actual problem: functionally yes, structurally a little overbuilt.
2. Production code to delete: the extracted leaf module can be avoided if the indirect test is removed.
3. Tests to delete/replace: replace or delete oauth-relogin-device-code.test.ts as above.
4. Deeper refactor: no; the final structure should keep one service mapping and directly render stateHint at the notice.
5. Ready to merge: product logic is sound and required test is green, but independent human review is still missing; the test-quality cleanup is non-blocking.
6. Residual risk/gaps: no regression assertion currently fails when the rendered device code is removed.

This changes user-visible authentication behavior, so independent human review is required under CONTRIBUTING.md. No additional security, licensing, release, governance, or public-contract effect was identified.

[YayoiNanoka]

PR Assessment

What problem does this PR solve?

OpenAI Codex re-login from the connection-detail sheet starts device authorization but does not display the stateHint containing the required 9-digit user code. The browser therefore asks for information that this entry point never exposes, making the flow impossible to complete.

How does this PR solve the problem?

The patch keeps useOAuthLoginFlow as the single authorization lifecycle owner. It:

• marks Codex as requiring its device code to be displayed while keeping xAI excluded;
• renders the existing flow.stateHint in OAuthReloginNotice;
• reuses the catalog panel's localized device-code copy;
• separately restores six canonical npm registry resolutions without changing package versions.

No new authorization state machine, persistence, or parallel source of truth is introduced.

Is the problem correctly defined?

Correct.

The linked issue, current base implementation, and existing catalog flow agree on the root cause: the re-login path receives the required code but drops it at the rendering boundary.

Principle-based assessment

• First principles: while Codex device authorization is pending, the exact stateHint must remain visible until completion; providers that do not require manual entry must not expose it. The patch preserves both invariants.
• Occam's razor: the product fix is small and stays in the existing service mapping and notice renderer. The previous test-only production seam is no longer present.
• Minimal sufficient solution: the authentication change is minimal and sufficient. The lockfile restoration is correct but independent of the UI defect and slightly broadens the PR's scope.

Required conclusions

1. Is the solution optimal for the actual problem? The authentication fix is; the PR as a unit is slightly broader because of the independent lockfile restoration.
2. Production code that can be deleted: none identified.
3. Low-quality tests that can be deleted or replaced: none remain in the current diff. No automated test protects the rendered regression.
4. Deeper refactor required: no. The final structure should keep the OAuth lifecycle in useOAuthLoginFlow, provider-specific presentation metadata in the existing service mapping, and rendering in the notice.
5. Ready to merge: conditionally, after the UI evidence requirement below is satisfied and an independent human committer approves it.
6. Residual risks or gaps: the actual Desktop/device-authorization flow and visual presentation were not independently reproduced during this review.

Review Findings

P1 Blocker

None.

P2 Should Fix

[P2 Should Fix] Add the required visual evidence for the untested UI change

• Location: CONTRIBUTING.md:87; apps/desktop/src/renderer/settings/provider-connection-detail.tsx:1136
• Trigger: a reviewer evaluates the new Banner content without component/E2E coverage and without a screenshot or recording → the only evidence that the device code is visible and usable in the real Desktop layout is the author's manual assertion.
• Root cause: the PR changes user-visible UI and explicitly states that the Banner render has no automated coverage, but its description contains no before/after screenshots or recording as required by the repository's contribution guide.
• Evidence: the current diff conditionally inserts the localized label and <code>{deviceCode}</code>; the PR checklist leaves regression coverage unchecked and reports only manual verification.
• Required direction: attach before/after screenshots or a short recording of the Codex connection-detail re-login flow showing the code while authorization is pending. If practical, include both supported locales or explain which locale was exercised.

Suggestion

Keep the lockfile restoration independently reviewable

• Location: package-lock.json
• The restoration matches npm's published tarball URLs and SHA-512 integrity values and changes no package versions, so no correctness defect was found.
• It is nevertheless independent of the Codex UI bug. If maintainers restore chore(deps): normalize locked npm registry sources #3381 through a dedicated change, drop commit 881be43746f726c25e3e9c2de165fd635fa719e6 here; otherwise ensure the human review explicitly covers this dependency-provenance change.

Verification

• Base: 9ff267510d381aba89f255b0d77bf6d277601de2
• Head: 881be43746f726c25e3e9c2de165fd635fa719e6
• Executed: fetched refs/pull/3358/head into a push-disabled isolated checkout; verified base/head and clean merge; inspected the linked issue, complete diff, OAuth controller, catalog rendering, localization, contribution rules, and commit provenance; ran renderer TypeScript checking, changed-source lint, renderer architecture tests/check, ASF header audit, and git diff --check; compared all six changed lockfile records with npm registry metadata.
• Key results: clean merge; renderer typecheck passed; 62 architecture tests passed; architecture check passed; lint and ASF header audit passed; all six registry URLs and integrity values matched; the current hosted test and other surfaced checks are successful.
• Temporary tests: not created; isolated checkout removed, never committed or pushed.
• Original workspace: confirmed unchanged.
• Not verified: a real Desktop Codex authorization attempt, visual layout in either locale, a fresh local dependency installation, or the full Desktop suite. Existing dependencies were reused read-only for targeted local checks.

Conclusion

Merge conditionally.

Add the required UI evidence, then obtain the independent human approving review required by CONTRIBUTING.md. If the lockfile commit remains, that review should explicitly include it. No reachable authentication correctness, security, or public-contract blocker was identified in the reviewed revision.

点击展开中文

PR 判断

这个 PR 解决了什么问题？

从连接详情页重新登录 OpenAI Codex 时，应用会启动设备授权，却不显示包含必需 9 位用户码的 stateHint。因此浏览器要求用户输入一个该入口从未展示的值，导致流程无法完成。

这个 PR 如何解决这个问题？

该补丁继续让 useOAuthLoginFlow 作为唯一的授权生命周期所有者。它：

• 标记 Codex 需要展示设备码，同时继续排除无需手动输入的 xAI；
• 在 OAuthReloginNotice 中渲染现有的 flow.stateHint；
• 复用 provider 目录面板已有的本地化设备码文案；
• 另外恢复六个 canonical npm registry resolution，不改变任何包版本。

它没有新增授权状态机、持久化状态或并行事实源。

这个问题定义得对吗？

Correct（正确）。

关联 Issue、当前 base 实现以及已有目录登录流程对根因的描述一致：重新登录路径已经拿到必需的设备码，却在渲染边界丢弃了它。

原则性判断

• 第一性原理：Codex 设备授权等待期间，准确的 stateHint 必须持续可见；不需要手动输入的 provider 不应展示它。补丁同时维持了这两个不变量。
• 奥卡姆剃刀：产品修复很小，仍位于既有 service mapping 和 notice renderer 中。旧版本中为测试新增的生产代码接缝已经不存在。
• 最小充分解：认证改动是最小且充分的。lockfile 恢复是正确的，但与 UI 缺陷相互独立，略微扩大了 PR 范围。

必答结论

1. 当前方案对实际问题是否最优？认证修复是；但整个 PR 因包含独立 lockfile 恢复而略宽。
2. 可删除的生产代码：未发现。
3. 可删除或替换的低质量测试：当前 diff 中已不存在。现在没有自动化测试保护实际渲染回归。
4. 是否需要更深层重构：不需要。最终结构应继续由 useOAuthLoginFlow 管理 OAuth 生命周期，由现有 service mapping 保存 provider 特定展示信息，并由 notice 负责渲染。
5. 当前 revision 是否可以合入：有条件可以；需要先满足下面的 UI 证据要求，并由独立的人类 committer 批准。
6. 剩余风险或验证缺口：本次审查没有独立复现真实 Desktop 设备授权流程及视觉呈现。

Review Findings

P1 阻塞

无。

P2 应该改

[P2 应该改] 为没有自动化覆盖的 UI 改动补充仓库要求的视觉证据

• 位置：CONTRIBUTING.md:87；apps/desktop/src/renderer/settings/provider-connection-detail.tsx:1136
• 触发条件：reviewer 在没有组件/E2E 覆盖、也没有截图或录像的情况下审查新增 Banner 内容 → 设备码在真实 Desktop 布局中是否可见、可用，只剩作者的手动验证陈述作为证据。
• 根因：该 PR 修改了用户可见 UI，并明确说明 Banner 渲染没有自动化覆盖，但 PR 描述没有包含仓库贡献指南要求的前后截图或录像。
• 证据：当前 diff 条件性插入本地化标签和 <code>{deviceCode}</code>；PR checklist 没有勾选回归测试覆盖，验证部分只报告了手动验证。
• 必需方向：附上 Codex 连接详情重新登录流程的前后截图或短录像，展示授权等待期间出现设备码。如果实际可行，请覆盖两种受支持语言；否则说明验证了哪种语言。

建议

保持 lockfile 恢复可被独立审查

• 位置：package-lock.json
• 恢复内容与 npm 发布的 tarball URL 和 SHA-512 integrity 一致，也没有改变包版本，因此没有发现正确性缺陷。
• 但它与 Codex UI 缺陷相互独立。如果维护者通过单独改动恢复 chore(deps): normalize locked npm registry sources #3381，应从本 PR 删除提交 881be43746f726c25e3e9c2de165fd635fa719e6；否则应确保人类 review 明确覆盖这项依赖来源变更。

验证

• Base：9ff267510d381aba89f255b0d77bf6d277601de2
• Head：881be43746f726c25e3e9c2de165fd635fa719e6
• 已执行：在禁止 push 的隔离 checkout 中获取 refs/pull/3358/head；验证 base/head 和干净合并；检查关联 Issue、完整 diff、OAuth controller、目录面板渲染、本地化、贡献规则和提交来源；运行 renderer TypeScript 检查、变更源码 lint、renderer architecture 测试及检查、ASF header audit 和 git diff --check；将六条 lockfile 变更与 npm registry 元数据逐项对比。
• 关键结果：干净合并；renderer typecheck 通过；62 个 architecture 测试通过；architecture check、lint 和 ASF header audit 通过；六条 registry URL 与 integrity 全部匹配；当前托管 test 及其他展示出的 checks 均成功。
• 临时测试：未创建；隔离 checkout 已删除，从未 commit 或 push。
• 原工作区：确认未改变。
• 未验证：真实 Desktop Codex 授权、两种语言下的视觉布局、全新本地依赖安装或完整 Desktop 测试套件。定向本地检查只读复用了已有依赖。

结论

有条件合入。

补充要求的 UI 证据，然后获得 CONTRIBUTING.md 要求的独立人类 approving review。如果保留 lockfile 提交，该 review 应明确覆盖它。本次审查没有在当前 revision 中发现可达的认证正确性、安全或公共契约阻塞问题。

[sosyz]

@YayoiNanoka thanks for the review. Added before/after screenshots to the description: before (en) shows the notice with no code while authorization is pending; after shows the code in both locales — Sign-in code: … (en) and 登录码：… (zh). Each was taken on a real Codex device-authorization attempt from the dev app; the codes shown are short-lived and already expired.

On the lockfile commit: agreed it is independent. I kept it because main currently fails npm ci under allow-remote=none (#3381 was reverted by #3148's lockfile regeneration), so the branch would not install cleanly without it. Happy to drop 881be43 if a maintainer prefers to restore #3381 separately — just say the word.