Fix account support for mutable workspace after login

[Codex]

Thanks for assigning this issue to me. I'm starting to work on it and will keep this PR's description up to date as I form a plan and make progress.

Original prompt

---

This section details on the original issue you should resolve

<issue_title>[feature] account with mutible workspace</issue_title>
<issue_description>after i login i find out seem like this dont support acount with mutible workspace</issue_description>

Comments on the Issue (you are @codex[agent] in this section)

• Fixes [feature] account with mutible workspace #100

note: greptile review for oc-chatgpt-multi-auth. cite files like lib/foo.ts:123. confirm regression tests + windows concurrency/token redaction coverage.

Greptile Summary

this pr adds proper multi-workspace binding support by introducing resolveStoredAccountIdentity and applyTokenAccountIdentity, which preserve org- and manual-source workspace bindings across all token refresh paths (health-check, switch, autoSync, verify-flagged, doctor). previously, every refresh blindly overwrote accountId with whatever the new token carried, silently downgrading an explicit org selection back to a personal token id.

key changes:

• resolveStoredAccountIdentity delegates to resolveRequestAccountId + shouldUpdateAccountIdFromToken (from lib/auth/token-utils.ts) to produce the correct id/source pair, keeping org/manual bindings stable
• applyTokenAccountIdentity wraps the above with an early-return contract and is called in five previously-inline sites across codex-manager.ts
• storage.ts: active: true added to withAccountAndFlaggedStorageTransaction state so exportAccounts called within that transaction reuses the snapshot; also fixes a T | null → T | undefined type mismatch in findCompatibleRefreshTokenMatchIndex
• five new/updated vitest cases cover org-binding preservation for verify-flagged, health-check, switch, and autoSync

outstanding gaps:

• no test covering runDoctor org-workspace preservation path
• no Windows filesystem concurrency or token-leakage reasoning per project policy
• minor guard asymmetry (!== undefined vs !) between upsertRecoveredFlaggedAccount and applyTokenAccountIdentity

Confidence Score: 3/5

• the core binding logic is sound but incomplete test coverage and missing concurrency documentation warrant caution before merging
• the approach via resolveStoredAccountIdentity/applyTokenAccountIdentity is architecturally correct and the previous thread's undefined-overwrite concerns are addressed. however, the runDoctor refresh path has no org-binding test, the project-required Windows concurrency and token-leakage documentation is absent, and a guard style inconsistency (!== undefined vs !) in upsertRecoveredFlaggedAccount is a latent footgun if resolveRequestAccountId ever returns an empty string.
• lib/codex-manager.ts — guard asymmetry at line 2550 and missing Windows/token-safety documentation for the new identity-resolution functions

Important Files Changed

Filename	Overview
lib/codex-manager.ts	introduces resolveStoredAccountIdentity and applyTokenAccountIdentity to preserve org/manual workspace bindings across token refreshes; replaces five inline accountId-overwrite patterns. guard style inconsistency in upsertRecoveredFlaggedAccount (!== undefined vs !) and no Windows concurrency/token-leakage documentation.
lib/storage.ts	two targeted fixes: adds active: true to withAccountAndFlaggedStorageTransaction so exportAccounts called from within that transaction correctly reuses the snapshot instead of deadlocking; fixes a `T
test/codex-manager-cli.test.ts	adds two new flagged-recovery tests (org binding preserved, no-accountId token doesn't clear binding) and upgrades three existing tests to assert workspace binding stability. mock for shouldUpdateAccountIdFromToken added. runDoctor org-binding path has no coverage.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[token refresh completes] --> B[extractAccountId from new token]
    B --> C[resolveStoredAccountIdentity\nstoredId, storedSource, tokenId]
    C --> D{resolveRequestAccountId\nreturns value?}
    D -- no --> E[return empty\nnextAccountId = undefined]
    D -- yes accountId --> F{shouldUpdateAccountIdFromToken?\nsource = org or manual}
    F -- no keep stored --> G[return stored accountId\n+ stored source]
    F -- yes token/id_token --> H{accountId === tokenAccountId?}
    H -- yes --> I[return accountId\nsource = token]
    H -- no --> J[return accountId\nsource = storedSource]
    E --> K[applyTokenAccountIdentity:\nguard !nextAccountId → skip write]
    G --> L[applyTokenAccountIdentity:\nguard same values → skip write]
    I --> M[applyTokenAccountIdentity:\nwrite new accountId + source]
    J --> M
    K --> N[account unchanged ✓]
    L --> N
    M --> O[account.accountId updated\nchanged = true]

Loading

Comments Outside Diff (1)

1. lib/codex-manager.ts, line 3678-3685 (link)

   redundant storageChangedFromDoctorSync = true inside conditional

   applyTokenAccountIdentity returns true here and you set storageChangedFromDoctorSync = true at line 3679 — but line 3685 sets it unconditionally in the same success block. the conditional at 3678-3680 is dead code: the flag is always set to true regardless of the identity-change result, which defeats the purpose of checking the return value. compare with runHealthCheck and autoSyncActiveAccountToCodex, which use the return value as the sole trigger for marking storage dirty.

   if the intent is to mark storage changed whenever a successful refresh happens (not just on identity change), then the if (applyTokenAccountIdentity(...)) block is misleading noise. either drop the inner if block and just call applyTokenAccountIdentity without capturing its return, or remove the unconditional line 3685 and add separate tracking for non-identity changes.

   → should become one of these alternatives, not both:

   // option a — always persist on any successful refresh:
   applyTokenAccountIdentity(activeAccount, refreshedAccountId);
   storageChangedFromDoctorSync = true;
   
   // option b — only persist when identity actually changed:
   if (applyTokenAccountIdentity(activeAccount, refreshedAccountId)) {
       storageChangedFromDoctorSync = true;
   }
   

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: lib/codex-manager.ts
   Line: 3678-3685
   
   Comment:
   **redundant `storageChangedFromDoctorSync = true` inside conditional**
   
   `applyTokenAccountIdentity` returns `true` here and you set `storageChangedFromDoctorSync = true` at line 3679 — but line 3685 sets it unconditionally in the same success block. the conditional at 3678-3680 is dead code: the flag is always set to `true` regardless of the identity-change result, which defeats the purpose of checking the return value. compare with `runHealthCheck` and `autoSyncActiveAccountToCodex`, which use the return value as the sole trigger for marking storage dirty.
   
   if the intent is to mark storage changed whenever a successful refresh happens (not just on identity change), then the `if (applyTokenAccountIdentity(...))` block is misleading noise. either drop the inner `if` block and just call `applyTokenAccountIdentity` without capturing its return, or remove the unconditional line 3685 and add separate tracking for non-identity changes.
   
   
   
   → should become one of these alternatives, not both:
   
   ```
   // option a — always persist on any successful refresh:
   applyTokenAccountIdentity(activeAccount, refreshedAccountId);
   storageChangedFromDoctorSync = true;
   
   // option b — only persist when identity actually changed:
   if (applyTokenAccountIdentity(activeAccount, refreshedAccountId)) {
       storageChangedFromDoctorSync = true;
   }
   ```
   
   How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: lib/codex-manager.ts
Line: 2550-2556

Comment:
**guard asymmetry between `upsertRecoveredFlaggedAccount` and `applyTokenAccountIdentity`**

`applyTokenAccountIdentity` uses a falsy guard (`!nextIdentity.accountId`), which catches `""` in addition to `undefined`. the guard here uses strict `!== undefined`, so an empty string `nextAccountId` would still pass through and overwrite `existing.accountId`.

`resolveRequestAccountId` is unlikely to return `""` today, but the asymmetry is a latent footgun. prefer matching the pattern used in `applyTokenAccountIdentity`:

```suggestion
		if (
			nextAccountId &&
			(
				(nextAccountId !== existing.accountId)
				|| (nextAccountIdSource !== existing.accountIdSource)
			)
		) {
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: test/codex-manager-cli.test.ts
Line: 760

Comment:
**missing vitest coverage for `runDoctor` org-workspace preservation**

three new tests cover `verify-flagged`, `health-check`, `autoSync`, and `switch` paths. `runDoctor` at `lib/codex-manager.ts:3681` also calls `applyTokenAccountIdentity` after a forced refresh — but there is no test confirming it preserves an `"org"`-source binding when the refreshed token returns a different id. if a regression is introduced in that call site, the test suite won't catch it.

add a test for `auth doctor` (or the `runDoctor` export) that sets up an account with `accountIdSource: "org"`, stubs `extractAccountId` to return a different value for the new token, and asserts the org binding is unchanged after the doctor run.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: lib/codex-manager.ts
Line: 909-934

Comment:
**no Windows filesystem concurrency or token-leakage reasoning**

per project policy, every code change must document how it defends against Windows filesystem concurrency bugs and token leakage. `resolveStoredAccountIdentity` and `applyTokenAccountIdentity` are now the single choke-point that decides which workspace binding gets persisted across all token refresh paths.

two risks not addressed:
- on Windows an antivirus lock during `saveAccountsUnlocked` can leave a half-written file even though `withStorageLock` guards against concurrent Node.js writes; a refresh that retries immediately could read stale data and re-enter this function with an inconsistent `storedAccountIdSource`.
- if a CSRF-crafted token carrying a different `accountId` reaches `applyTokenAccountIdentity` on an account whose source is `"token"`, `shouldUpdateAccountIdFromToken` returns `true` and the binding is silently overwritten; there is no validation that the incoming token's workspace claim is consistent with prior history.

please add a comment block describing the concurrency strategy (e.g. "atomic rename + post-write verification on Windows") and note the token-validation boundary, then reference the regression test that exercises the race.

How can I resolve this? If you propose a fix, please make it concise.

_{Last reviewed commit: 4973a20}

Context used:

• Rule used - What: Every code change must explain how it defend... (source)

[chatgpt-codex-connector]

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

[Copilot]

Pull request overview

This PR addresses accounts that are bound to an org-selected workspace so that token refresh/verification flows don’t overwrite the stored workspace binding after login.

Changes:

• Introduces helper logic to resolve whether stored accountId/accountIdSource should be updated from token-derived identity.
• Updates multiple flows (health check, doctor, switch, flagged recovery/verify) to use the shared identity-resolution helper.
• Expands CLI test coverage to assert org-selected workspace bindings are preserved across refresh/recovery flows.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
test/codex-manager-cli.test.ts	Adds/updates tests and mocks to validate preserving org-selected workspace binding during refresh/recovery/switch flows.
lib/codex-manager.ts	Centralizes account identity resolution and applies it across token refresh, flagged recovery, and sync paths to avoid overwriting org bindings.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.