Scope Codex OAuth cancellation to the originating flowId

[luvs01]

Summary

POST /api/codex-auth/login/cancel is keyed only by provider: cancelLoginFlow("chatgpt") aborts whichever AbortController is currently registered for the provider, and expireCodexAuthFlow marks the supplied (or every pending) flow row as cancelled. A stale login modal can therefore POST its old flowId and silently abort a newer login flow that the user just started — the replacement attempt dies behind the fresh modal.

This binds each login attempt to the flowId that owns it and validates cancellation against the pending-flow registry:

• loginAbort entries now record { controller, flowId }; startLoginFlow propagates the caller''s flowId through LoginFlowLifecycle.
• cancelLoginFlow(provider, flowId?) refuses to abort when the active attempt''s flowId does not match. Callers that omit flowId (e.g. the generic /api/oauth/login/cancel route) keep the existing provider-wide semantics.
• handleCodexAuthLoginCancel now requires flowId, rejects unknown/expired/non-pending flows with 400 (matching /login/code validation), and only then cancels and expires that exact flow — it can no longer wipe every pending flow when flowId is absent.
• Regression test: a stale modal''s flowId is rejected without touching cancelLoginFlow; the owning flow''s cancel passes ("chatgpt", flowId) through.

Note: ocx account cancel chatgpt without --flow now receives a clear 400 flowId required instead of cancelling the active attempt; pass --flow <flow-id> (printed by ocx login) to target the attempt.

Verification

• Head: 410146c3993aa918d142c0490f0d042881cbdd84 (tree bb28ceb6cb908815cde53936046c21d76c6c3b7e), base origin/dev 2f025814f3b85a027e8c76f913b1e10f342aba5e.
• bun x tsc --noEmit — clean.
• bun run structure:check — passed.
• bun run privacy:scan — passed.
• bun scripts/file-size-ratchet.ts — passed (kept src/oauth/index.ts under the 2000-line threshold).
• bun test tests/codex-integration/codex-auth-api.test.ts — 340 pass / 0 fail.
• bun test tests/oauth/oauth-manual-code.test.ts tests/oauth/oauth-public-surface.test.ts — 32 pass / 0 fail.
• Fork CI dispatched: run 35261279433

Remaining gates

• Full fork CI matrix green at the posted head except the known-capped lane: the rerun passed every lane including the previously flaky windows 6/9 shard; only macos control was cancelled at the known 30-minute timeout-minutes cap tracked in [Bug]: macOS control is cancelled near its 30-minute limit in full dispatch CI #4905, which is not caused by this change.
• Maintainer review.

Summary by CodeRabbit

• Bug Fixes
  • OAuth login cancellation now targets only the specific pending login flow identified by its flow ID.
  • Invalid, missing, or stale flow IDs return a clear HTTP 400 error instead of cancelling another flow.
  • Prevented older login flows from affecting newer replacement flows during cancellation or cleanup.

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

• All CI tests are green on my local testing.

• I pushed my PR to the latest dev commit.

• I resolved all correct Codex and CodeRabbit findings.

• My PR is ready for review.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: bfd3ba43-c473-41e0-9e5c-faf3d7bdae1e

📥 Commits

Reviewing files that changed from the base of the PR and between 43cd1ad and 410146c.

📒 Files selected for processing (3)

• src/codex/auth-api/login-flow.ts
• src/oauth/index.ts
• tests/codex-integration/codex-auth-api.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review.

---

📝 Walkthrough

Walkthrough

Changes

OAuth cancellation flow

Layer / File(s)	Summary
OAuth lifecycle flow ownership src/oauth/index.ts	LoginFlowLifecycle and abort state now store an optional flowId. Cancellation and ownership checks compare the requested flow ID with the active flow.
Login API cancellation binding src/codex/auth-api/login-flow.ts	Login startup passes the created flowId to startLoginFlow. Cancellation now requires a pending flowId, then passes it to cancelLoginFlow and expireCodexAuthFlow.
Cancellation behavior validation tests/codex-integration/codex-auth-api.test.ts	Tests verify that stale flow IDs return HTTP 400 without OAuth cancellation, while the owning flow returns HTTP 200 and passes its ID to cancelLoginFlow.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant CodexAuthAPI
  participant OAuthLifecycle
  participant LoginFlowState
  CodexAuthAPI->>OAuthLifecycle: startLoginFlow({ flowId })
  OAuthLifecycle->>LoginFlowState: store abort controller and flowId
  CodexAuthAPI->>OAuthLifecycle: cancelLoginFlow("chatgpt", flowId)
  OAuthLifecycle->>LoginFlowState: validate active flowId
  CodexAuthAPI->>LoginFlowState: expireCodexAuthFlow(flowId)

Loading

Merge Risk: ⚪ Minimal · up to 41014

Cancellation now targets only the owning pending login flow and rejects stale or missing flow IDs. The change is ready to merge after normal checks.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 3 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: Codex OAuth cancellation is scoped to the originating flowId. This matches the updates in src/codex/auth-api/login-flow.ts, src/oauth/index.t…

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Create a new PR

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

⚠️ Deterministic hygiene checks failed.

• unsponsored_surface — This changes an authentication, workflow, release-automation, or dependency surface. MAINTAINERS.md requires security review for these; ask a maintainer to apply maintainer-sponsored once they have reviewed it. Paths: src/oauth/index.ts.

[lidge-jun]

리뷰 · 우선순위 76 / 80

이 PR은 지금 dev HEAD 43cd1ade1 (#4623 offload notes에서 remote runner 접근 정보 제거, package 2.59.0) 위에 올린 Codex OAuth 취소가 잘못된 로그인을 죽이는 버그 수정입니다. 지금 dev의 POST /api/codex-auth/login/cancel은 cancelLoginFlow("chatgpt")만 호출합니다. 키가 provider 하나라서, 예전에 연 로그인 모달이 늦게 취소를 보내면 방금 새로 시작한 로그인 AbortController까지 같이 끊을 수 있습니다. expireCodexAuthFlow(body.flowId ?? null)도 flowId가 없으면 pending 행을 전부 error로 바꿔 버립니다. 사용자는 새 모달을 보고 있는데 뒤에서는 새 시도가 이미 죽은 상태가 됩니다.

고치는 방법은 간단합니다. LoginFlowLifecycle에 flowId?를 넣고, startLoginFlow가 loginAbort에 { controller, flowId }를 저장합니다. handleCodexAuthLoginStart는 이미 만든 flowId를 lifecycle로 넘깁니다. cancelLoginFlow(provider, flowId?)는 flowId가 왔고 활성 시도의 flowId와 다르면 abort하지 않습니다. flowId를 안 넘기는 호출(일반 /api/oauth/login/cancel)은 예전처럼 provider 단위 취소를 유지합니다.

handleCodexAuthLoginCancel은 이제 flowId 문자열을 요구합니다. 없거나 pending이 아니면 /login/code와 같이 400입니다. pending인 그 flowId만 cancelLoginFlow("chatgpt", flowId)와 expireCodexAuthFlow(flowId)로 닫습니다. dynamic import 뒤에 registry를 다시 보는 이유도 PR 본문에 적혀 있습니다. import yield 동안 모달이 바뀌어도, 모르는 flowId는 cancel을 부르지 않게 하려는 장치입니다.

테스트는 예전 「취소하면 pending flow가 error」 한 줄을 빼고, startPendingFlow 안에서 stale flowId는 400이고 cancelLoginFlow를 안 부르는지, 소유 flowId는 ("chatgpt", flowId)로 부르는지 spy로 고정합니다. 로컬 게이트(tsc / structure / privacy / file-size-ratchet / 타깃 테스트)는 PR 본문 기준 통과입니다. types.ts/config.ts 대분할과 무관한 좁은 auth 수정이고, 중복 PR로 보이지 않습니다.

한 가지 제품 동작이 바뀝니다. ocx account cancel chatgpt처럼 Codex 경로에서 --flow 없이 취소하면 이제 서버가 400 flowId required를 줍니다. ocx login이 찍어 준 --flow <flow-id>를 붙여야 합니다. 일반 OAuth 취소 라우트는 그대로 provider 단위입니다.

라인 531-540 - src/codex/auth-api/login-flow.ts / handleCodexAuthLoginCancel — pending 확인 후 cancelLoginFlow가 false여도 expireCodexAuthFlow는 계속 돌고 { ok:true, cancelled:false }를 돌려줍니다. 대부분 맞지만, 클라이언트가 cancelled=false를 성공으로만 보면 혼동할 수 있습니다.
라인 1903-1910 - src/oauth/index.ts / cancelLoginFlow — flowId를 넘겼는데 active.flowId가 undefined면(구 lifecycle) mismatch로 거절합니다. Codex start는 이제 flowId를 넣으니 새 경로는 괜찮고, 다른 호출자가 lifecycle 없이 start한 뒤 flowId로 cancel하면 막힙니다.
경로 - src/cli/account-auth.ts cancel — Codex면 body에 flowId만 넣습니다. --flow 생략 시 undefined가 JSON에 실리고 서버 400이 납니다. 의도된 breaking change인데, CLI 사용 문구/도움말에 한 줄 더 있으면 덜 놀랍니다.
경로 - /api/oauth/login/cancel vs /api/codex-auth/login/cancel — 전자는 여전히 provider-wide, 후자만 flowId 강제. 설계상 맞지만 리뷰어가 「둘 다 scoped」로 읽으면 오해합니다.
라인 - CI — fork CI 진행 중, macos control 30분 timeout(#4905)은 이 diff 원인이 아니라고 본문에 적혀 있습니다. exact-head 초록 증거는 아직 대기입니다.

메인테이너의 판단이 필요한 지점

• ocx account cancel chatgpt 무옵션 취소를 영구히 막고 --flow를 강제할지, 아니면 CLI만 활성 시도 조회 후 flowId를 채우는 편의 경로를 둘지.
• generic OAuth cancel을 나중에 flowId scoped로 맞출지, Codex만 특수 취급을 유지할지.
• exact-head Cross-platform/fork CI 초록을 랜딩 게이트로 둘지(본문 Remaining gates와 동일).

너의 추천
exact-head CI가 초록이면 dev에 랜딩하세요. 실제 사용자 버그(오래된 모달이 새 로그인을 죽임)를 작은 diff로 막고, stale reject + owning cancel 인자가 테스트로 고정돼 있습니다. CLI 무옵션 Codex cancel이 깨지는 점은 릴리즈 노트/도움말에 한 줄만 남기면 됩니다. CI 전 merge는 하지 마세요. 라벨은 건드리지 않았습니다.

이 댓글은 grok-bot이 작성했습니다

[github-actions]

⏳ DRAFT

• hygiene: unsponsored_surface.

What to do

• Fix unsponsored_surface — This changes an authentication, workflow, release-automation, or dependency surface. MAINTAINERS.md requires security review for these; ask a maintainer to apply maintainer-sponsored once they have reviewed it. Paths: src/cli/account-auth.ts, src/oauth/index.ts.

Review readiness checklist

• ✅ All CI tests are green on my local testing.
• ✅ I pushed my PR to the latest dev commit.
• ✅ I resolved all correct Codex and CodeRabbit findings.
• ✅ My PR is ready for review.

✅ 4/4 boxes ticked.

This pull request was already a draft. Its draft status will be preserved after every issue above is resolved.

[luvs01]

Exact-head CI is green on the posted head 410146c: the Cross-platform run (ci, gates, macos 1/2, macos 2/2, keyring x3, docker smoke, api usage, changes, npm-global, label) passed, and the only failing checks are hygiene/enforce-target reporting unsponsored_surface on src/oauth/index.ts — the security-review gate this PR is expected to wait on.

On the inline findings:

• cancelled:false with { ok: true }: intentional — the request was well-formed but nothing owned by that flowId was still active, so the client can distinguish "accepted, nothing to cancel" from an error. Happy to rename or document if you read it differently.
• Legacy lifecycle without flowId: refusing is the intended scoping — a caller that never went through the new startLoginFlow cannot cancel by flowId, which is what prevents a stale modal from killing a fresh attempt.
• CLI --flow: agreed the bare ocx account cancel chatgpt failure is surprising. I can push a one-line usage/help note (e.g. "Codex cancel requires --flow from ocx login") if you'd like it in this PR, or leave it to the release notes as you suggested.
• Route scope: /api/oauth/login/cancel staying provider-wide while only the Codex route requires flowId is deliberate; I can add a short comment on the generic route pointing at the scoped one if that helps future readers.

On your judgment points: keeping the un-optioned Codex cancel blocked (rather than adding a convenience lookup) is the safer default in my view — a wrong-flowId refusal is exactly the protection this PR adds. Generic OAuth cancel can stay provider-wide until a second provider needs scoping.

Requesting maintainer-sponsored when you're ready — the diff is unchanged since your review and CI at this head is green.

[luvs01]

Addressed the review findings in 96974b52f:

• CLI --flow (line-comment 3): ocx account cancel chatgpt without --flow now fails fast as a usage error naming the flag ocx account login prints, instead of posting { flowId: undefined } and surfacing a bare server 400. The usage line marks --flow as required for codex. Covered by a new cli-account case asserting exit 2 with no request sent (154 tests pass).

• cancelled:false semantics (line-comment 1): kept as-is deliberately. expireCodexAuthFlow(flowId) runs unconditionally once the pending flow is validated, so the flow is closed either way; cancelled only reports whether a live AbortController fired. ok:true, cancelled:false therefore means "closed, nothing left to abort" — changing the shape would break GUI clients that read cancelled.

• Legacy lifecycle without flowId (line-comment 2): the mismatch refusal is the intended direction. A flowId-scoped cancel must only abort the flow it can prove is that flow; an active lifecycle entry with no recorded flowId cannot be proven, so refusing is the fail-closed choice. All current Codex starts record flowId, so the new path is unaffected.

• Two cancel routes (line-comment 4): correct — /api/oauth/login/cancel stays provider-wide for non-Codex providers; only /api/codex-auth/login/cancel is flow-scoped. The split is intentional and the CLI now routes to each accordingly.

The PR remains in the unsponsored_surface lane (src/oauth/, src/codex/auth-*, src/cli/account-auth.ts), so maintainer-sponsored is still the pending gate.