Skip to content

fix(oauth): allow dismissing a settled failed login flow - #7379

Closed
Ingwannu wants to merge 2 commits into
lidge-jun:devfrom
Ingwannu:fix/oauth-dismiss-failed-flow-7378
Closed

Ingwannu wants to merge 2 commits into
lidge-jun:devfrom
Ingwannu:fix/oauth-dismiss-failed-flow-7378

Conversation

@Ingwannu

@Ingwannu Ingwannu commented Oct 19, 2026 •

Copy link
Copy Markdown
Owner

Summary

Closes #7378.

A failed add-account attempt can remain the newest OAuth flow even when a persisted account still works. The cancel endpoint returned cancelled:false after that attempt's controller was gone, leaving its error visible until another login or restart.

Extend the existing provider/flow-scoped cancellation owner with an explicit public-endpoint opt-in to dismiss exactly one settled failed flow. Internal callers retain the boolean proof of live-controller cancellation. Credential state and successful terminal outcomes remain untouched; pending cancellation still records Login cancelled, and a repeated dismissal is a no-op. Keep flow errors observable until explicitly dismissed rather than hiding every failure whenever a credential exists. Update the owning contract, ADR and public API reference.

Verification

  • Base: dev 79fa97be6e6ae974ccd31d9aeedc2da96a0baa73.
  • Linux, Bun 1.4.3, fresh HOME/CODEX_HOME/OPENCODEX_HOME/TMPDIR under sequential cgroup caps (75% CPU, 1536 MiB maximum, zero swap, 300s deadline).
  • Before the source fix: two new failed-flow dismissal regressions fail; the pending/success control passes.
  • bun test --isolate tests/oauth/oauth-status-privacy.test.ts tests/oauth/oauth-code-login.test.ts tests/oauth/oauth-manual-code.test.ts tests/codex-integration/codex-auth-cancel-ownership.test.ts: 71 pass, 0 fail. Includes actual management cancel-handler response, unchanged auth-store bytes, provider/flow isolation, successful-result retention, pending cancellation, code paste and reaper controls. No live OAuth exchange.
  • Initial hosted test 4/4 exposed the shared Codex stale-controller boolean contract; the public endpoint now opts in explicitly, and the existing Codex ownership regression passes unchanged. No assertion was weakened.
  • bun scripts/structure-ssot.ts, bun scripts/privacy-scan.ts, git diff --check: pass.
  • Full local suite and documentation build were deferred to hosted CI to preserve the fixed local resource envelope. Root typecheck was not rerun locally after the prior intake's 300s timeout; current-head hosted typecheck/CI now pass as recorded below. No skipped platform or live installed-dashboard flow is certified.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Keep Draft pending current-head hosted validation and independent authentication/flow-boundary review. No sponsorship, merge, release or live restart is requested by this patch.

Hosted validation

Current head bb9cc0e61292a81b4af0fc126790d3974e56567a: Cross-platform CI run 39257981928 completed successfully, including Typecheck, Privacy scan, all four selected Ubuntu test shards, structure and docs build. This supplements the focused local receipts; skipped macOS/Windows remain unverified. Keep Draft for independent boundary review.

@github-actions

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@coderabbitai

coderabbitai Bot commented Oct 19, 2026

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

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

lidge-jun added a commit that referenced this pull request Oct 19, 2026
…ate paths and shim token handling (B5) (#7397)

* fix(oauth): allow dismissing a settled failed login flow (carry #7379)

Co-authored-by: Ingwannu <186453546+Ingwannu@users.noreply.github.com>

* fix(providers): preserve explicit API-key mode when keys are unavailable (carry #7304)

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* fix(dsh): bound account profile response reads (carry #7333)

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* test(dsh): drain server fixtures before removing homes (carry #7305)

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* fix(claude-cli): keep local errors out of subscription cooldown (carry #7343)

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* fix(claude): bound Desktop library reads and preflight writes (carry #7301)

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* fix(service): stop scheduler wrappers before Windows reinstall (carry #7313)

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* fix(startup): require RunAtLoad before Desktop relaunch (carry #7277)

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* fix(codex): strip OpenSSL startup variables from updater child env (carry #7342)

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* fix(codex): keep the service token out of bypassed shim launches

All three Codex shims loaded the service API token into OPENCODEX_API_AUTH_TOKEN before checking OCX_SHIM_BYPASS, so a bypassed native passthrough launch handed the proxy data-plane token to the real Codex and its descendants. The POSIX, CMD and PowerShell shims now skip the token load under bypass; the Unix revision marker moves to 4 so installed wrappers are refreshed.

loadServiceTokenFromFile reads the inherited OCX_API_TOKEN_FILE through a no-follow, nonblocking descriptor, accepts only a regular file of at most 4 KiB and returns only a single-line printable token.

* test(claude): wait for a real ctime transition before requiring rejection

Re-applying the same utimes moves ctime only after the kernel clock ticks, so the real ctime case from #7301 often observed no change and the reader correctly returned {}. Repeat the call until ctime has advanced (bounded), assert that precondition, then require desktop_config_not_regular.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* fix(google-antigravity): admit a probed project_id fallback in account import (carry #7385)

An Antigravity account whose Cloud Code Assist project is not auto-discovered was rejected with missing_project even when the operator knows its project. A Cockpit record may now carry a bounded project_id (Google Cloud project grammar, 6-30 chars, exact match). Discovery keeps precedence; the fallback is admitted only after the account quota probe succeeds with that account own access token against that project. Probe refusal, probe errors and cancellation write nothing. No shared default project.

Co-authored-by: yobushka <51312852+yobushka@users.noreply.github.com>

* fix(quota): name a closed reason when an OAuth account quota probe fails

Anthropic per-account quota rows reported quotaUnavailable with no reason, and Antigravity reported account_unavailable for any credential-side throw. The Anthropic usage read now reports a closed code for an HTTP status class, an unusable body or a thrown dispatch, and a probe that cannot obtain a credential reports account_unavailable without dispatching. Antigravity distinguishes a credential refresh deadline (timeout). Codes reuse the existing closed QUOTA_FAILURE_CODES set, carry no upstream text and never change account health.

* test(claude): match the symlink-resolved rollback target in the picker restore case

The atomic writer renames onto the symlink-resolved target, so on macOS (/var -> /private/var) the rollback rename never matched the fixture path and the restore-failure injection did not fire. Compare both spellings.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* fix(google-antigravity): cancel the import project probe with the import request

probeAntigravityUsageQuota accepts an optional caller signal that cancels every request of the probe and stops before the models fallback; the import verifier forwards the import signal and never admits after cancellation.

* fix(codex): upgrade installed Windows shims that load the token before bypass

CMD and PowerShell wrappers now carry a Windows revision marker. A marked wrapper without it (installed before the bypass-token fix) is obsolete: installCodexShim and autoRestoreCodexShim rewrite it in place over the same backing launcher instead of reporting it healthy or already installed, and diagnostics report it unhealthy. On Windows the service token reader also binds the opened descriptor to the path by identity, so a link swapped in after lstat is refused.

* test(codex): mark the in-place Windows diagnostics fixture with the current revision

* fix(codex): refresh obsolete Windows shims without clobbering a concurrent replacement

The obsolete-wrapper refresh truncated whatever occupied the wrapper path. It now moves the observed wrapper aside and proves it is the probed file, stages the current wrapper beside it, and publishes with a hard link that refuses an occupied path. A concurrent replacement is restored or kept untouched and the refresh reports deferred.

* docs(structure): record the Windows shim refresh transaction

* fix(codex): create the Windows shim refresh stage exclusively and resume partial upgrades

The refresh stage is now created with O_CREAT|O_EXCL (and O_NOFOLLOW on POSIX) under an independent random name, and publication, withdrawal and cleanup are bound to the identity of the file this refresh created, so a planted link is never written through. Auto-restore resumes a Windows revision-only sibling mix (current plus obsolete wrappers) instead of deferring forever after one transient publish failure.

* fix(codex): bind Windows shim refresh rollback and recovery to observed identities

Publication keeps a linked entry only if it is the created stage, and withdraws a link to a swapped stage (file or symlink) while leaving independent replacements. Restoring the moved wrapper requires the aside to still be the moved file. Automatic revision recovery now refreshes only the admitted obsolete wrappers and first re-verifies every current sibling, so a replacement during the stability wait is never adopted.

* fix(codex): never displace a Windows shim where its replacement cannot be published

The refresh proves hard-link support in the wrapper directory with a throwaway pair before moving the wrapper aside, and if a restore link still fails it falls back to an identity-checked rename onto the empty path, so the launcher never stays missing.

---------

Co-authored-by: Ingwannu <186453546+Ingwannu@users.noreply.github.com>
Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>
Co-authored-by: yobushka <51312852+yobushka@users.noreply.github.com>
@lidge-jun

Copy link
Copy Markdown
Owner

Carried into #7397 (squash dbb908f) on dev with a Co-authored-by trailer, as part of the B5 OAuth/accounts/service hardening bundle. It ships in the next release (2.90.0). Thanks!

@lidge-jun lidge-jun closed this Oct 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants