Fix Codex auth not being applied to the session/token after login.#14552
Fix Codex auth not being applied to the session/token after login.#14552zhiluo20 wants to merge 1 commit intoopenclaw:mainfrom
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| } catch (err) { | ||
| spin.stop("OpenAI OAuth failed"); | ||
| runtime.error(String(err)); | ||
| await prompter.note("Trouble with OAuth? See https://docs.openclaw.ai/start/faq", "OAuth help"); | ||
| throw err; | ||
| } |
There was a problem hiding this comment.
Duplicate error reporting
This helper both reports the error (runtime.error(...) + FAQ note) and rethrows (throw err). Callers now tend to either catch-and-ignore (auth-choice.apply.openai.ts:158-160) or not catch (e.g. models/auth.ts), leading to inconsistent UX: either silent continuation, or double-reporting depending on higher-level error handling.
If this helper is meant to fully handle user-facing errors, it should return null (or a typed failure) instead of throwing; otherwise, remove the user-facing side effects and let callers handle reporting consistently.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/commands/openai-codex-oauth.ts
Line: 49:54
Comment:
**Duplicate error reporting**
This helper both reports the error (`runtime.error(...)` + FAQ note) *and* rethrows (`throw err`). Callers now tend to either catch-and-ignore (`auth-choice.apply.openai.ts:158-160`) or not catch (e.g. `models/auth.ts`), leading to inconsistent UX: either silent continuation, or double-reporting depending on higher-level error handling.
If this helper is meant to fully handle user-facing errors, it should return `null` (or a typed failure) instead of throwing; otherwise, remove the user-facing side effects and let callers handle reporting consistently.
How can I resolve this? If you propose a fix, please make it concise.
Additional Comments (1)
Consider either not catching here, or returning early after the helper handles UX (note+runtime.error) so the command exits with a failure state rather than silently continuing. Prompt To Fix With AIThis is a comment left during a code review.
Path: src/commands/auth-choice.apply.openai.ts
Line: 154:160
Comment:
**Swallowed OAuth errors**
`loginOpenAICodexOAuth` rethrows on failure (`src/commands/openai-codex-oauth.ts:49-54`), but this caller catches and ignores all errors. That means the auth-choice flow can continue and return `{ config: nextConfig, agentModelOverride }` without surfacing that login failed (and without any error exit), which is likely to confuse users and may leave them thinking OAuth was applied.
Consider either not catching here, or returning early after the helper handles UX (note+runtime.error) so the command exits with a failure state rather than silently continuing.
How can I resolve this? If you propose a fix, please make it concise. |
|
Thanks @zhiluo20 for the original report and patch direction here.\n\nThis is now superseded by #15406, which carries the same core fix path on top of current architecture:\n- shared Codex OAuth flow helper\n- support\n- surfaced OAuth failures (no silent swallow)\n\nKeeping traceability by linking this PR directly. |
|
Thanks @zhiluo20 for the original report and patch direction here. This was superseded by #15406, which carries the same core fix path on top of current main architecture:
Linking both PRs to keep traceability explicit. |
2 participants
In the current version, Codex authentication is not applied correctly after login. This patch fixes the issue.
Greptile Overview
Greptile Summary
This PR refactors the OpenAI Codex OAuth login flow into a shared helper (
src/commands/openai-codex-oauth.ts) and updates both the interactive onboarding flow (src/commands/auth-choice.apply.openai.ts) and themodels auth loginCLI command (src/commands/models/auth.ts) to use it. It also changes Codex profiles to be keyed by the OAuth email (openai-codex:<email>) rather than alwaysopenai-codex:default, which should allow the correct profile to be selected/applied after login.Main issue to address before merge: error handling is currently inconsistent—
loginOpenAICodexOAuthboth emits user-facing errors and rethrows, while at least one caller catches and ignores the exception. This can lead to either silent failure or confusing duplicate reporting depending on the entrypoint.Confidence Score: 3/5
(2/5) Greptile learns from your feedback when you react with thumbs up/down!