## Summary - rebuild the `codex-multi-auth` sync flow on top of current `main` with a narrowed review scope - keep only the sync-specific runtime, storage, and dashboard wiring needed for preview, import, prune, and synced-overlap cleanup - include the in-scope safety fixes for `idToken` backup redaction and case-sensitive refresh-token matching ## Validation - `npm run typecheck` - `npm run lint` - `npm run build` - `npx vitest run test/sync-prune-backup.test.ts test/codex-multi-auth-sync.test.ts test/cli.test.ts test/index.test.ts --reporter=dot` ## Notes - this PR replaces the sync core from `#74` with a materially smaller review surface - deferred follow-up work remains on local branch `clean/pr74-supporting-hardening` - `#74` is superseded by this PR --- ## Summary by cubic Adds an experimental `codex-multi-auth` sync flow (preview, import, prune, and synced-overlap cleanup) wired into the auth menu with a persisted toggle and safer storage. Closes remaining cleanup gaps by locking apply/cleanup together and atomically writing backups while tightening identity handling. - **New Features** - Experimental `codex-multi-auth` sync: preview, import, prune, and overlap cleanup. - Auth menu “Sync tools”: toggle, sync-now, and overlap cleanup; config flag `experimental.syncFromCodexMultiAuth.enabled`. - Import safety: dedup by account/email, discovery of `openai-codex-accounts.json`/`codex-accounts.json`, capacity limits via `CODEX_AUTH_SYNC_MAX_ACCOUNTS`, and prune backups with token redaction. - **Bug Fixes** - Storage safety: atomic flagged-account writes; config mutations serialized with temp-file replacement and strict validation; atomic temp-rename for prune and overlap-cleanup backups. - Cleanup consistency: overlap-cleanup backup and apply now run under one storage lock; operations use a locked snapshot; exact removal identities preserved with composite flagged filtering. - Token safety and tests: redact `idToken` and other tokens in backups; case-sensitive refresh-token matching; prune orphaned flagged entries only when an accounts snapshot exists; restored and added regression tests with Windows fixture alignment. ^{Written for commit ddb1a0f56fabc89f55b623f27e11eb8ddea00049. Summary will update on new commits.} ## Summary by CodeRabbit * **New Features** * Added Codex multi-account sync tools: toggle sync, run sync now, preview & prune removals, and cleanup overlapping accounts with safe backups and previews. * UI/menu exposes sync controls and prompts for capacity-aware imports and retry flows. * **Chores** * Improved storage robustness, atomic config updates, transactional backups, and clearer capacity/error messaging. * **Tests** * Added comprehensive tests covering sync, backup, preview, deduplication, and overlap cleanup scenarios. **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 rebuilds the `codex-multi-auth` sync flow on top of `main` with a narrowed review surface, adding preview, import, prune, and overlap-cleanup operations wired into the auth menu. it closes the most critical safety gaps from `#74`: `saveFlaggedAccountsUnlocked` and `savePluginConfigMutation` now use atomic temp→rename writes, the prune backup in `createSyncPruneBackup` uses temp→rename, the overlap-cleanup backup runs inside the storage transaction (closing the separate-lock window), and `removeOrphanedFlaggedAccounts` auto-heals the two-phase commit gap on every flagged-account load. key remaining concerns: - the backup write inside `cleanupCodexMultiAuthSyncedOverlaps` still uses `fs.writeFile` with `flag: "wx"` directly — not the temp→rename pattern — so a mid-write av lock or oom kill on windows can truncate the only rollback point before `persist` runs - `loadFlaggedAccountsUnlocked` now silently writes to disk on every call (orphan pruning); no vitest covers the false-positive case where the accounts file is empty during a concurrent add, which would incorrectly purge valid flagged metadata - `test/sync-prune-backup.test.ts` still does not exercise the `idToken` deletion branch in `cloneWithoutTokens` (no test account has an `idToken` field), leaving the stated in-scope safety fix without a regression test

Confidence Score: 3/5

- safe to merge with low risk, but the non-atomic backup write in cleanup and the write-on-read orphan pruning pattern should be addressed before the feature is enabled by default - the pr addresses most previous review thread issues correctly (atomic flagged-account writes, config mutation serialization, backup under lock, temp→rename for prune backup); the remaining non-atomic write in `cleanupCodexMultiAuthSyncedOverlaps` is a real windows safety risk on the rollback backup, and the write-on-read orphan pruning lacks coverage — but both are contained to the experimental sync path which is off by default - `lib/codex-multi-auth-sync.ts` (non-atomic backup write at lines 1228–1234) and `lib/storage.ts` (write-on-read orphan pruning in `loadFlaggedAccountsUnlocked`)

Important Files Changed

| Filename | Overview | |----------|----------| | lib/codex-multi-auth-sync.ts | new file implementing the full codex-multi-auth sync flow; most safety patterns are correct (prefixed identity keys, temp-dir token handling, storage lock integration) but `cleanupCodexMultiAuthSyncedOverlaps` writes the rollback backup non-atomically with `flag:"wx"` instead of temp→rename, leaving the backup vulnerable to truncation on windows av locks or oom kills. | | lib/storage.ts | adds `saveFlaggedAccountsUnlocked` (now atomic via temp→rename), `withFlaggedAccountsTransaction`, `loadAccountAndFlaggedStorageSnapshot`, and `backupRawAccountsFile`; `loadFlaggedAccountsUnlocked` introduces a write-on-read side effect via `removeOrphanedFlaggedAccounts` that silently prunes flagged state on every load — no vitest covers the false-positive orphan purge path. | | lib/config.ts | adds `savePluginConfigMutation` with a module-level promise mutex (`configMutationMutex`) and temp→rename pattern; closes the concurrent toggle race and the sync `readFileSync` blocking issue; `isRecord` correctly updated to exclude arrays. | | index.ts | wires sync toggle, sync-now, and overlap-cleanup into the auth menu loop; `createSyncPruneBackup` now correctly uses temp→rename for the prune backup; `removeAccountsForSync` commits accounts and flagged in separate transactions (inherent two-phase gap auto-healed on next load by `removeOrphanedFlaggedAccounts`). | | lib/sync-prune-backup.ts | minimal new file; `createSyncPruneBackupPayload` correctly redacts `accessToken`, `refreshToken`, and `idToken` via `cloneWithoutTokens` — the type-level `TokenRedacted` enforces this at compile time. | | test/codex-multi-auth-sync.test.ts | 1798-line test suite covering source discovery, preview, sync, capacity errors, and overlap cleanup; preview correctly asserts no write-lock acquisition; windows UNC path validation is exercised; good breadth but no test for two synced accounts sharing an organizationId in cleanup. | | test/sync-prune-backup.test.ts | covers token redaction in backup payloads but test accounts lack an `idToken` field, so the `delete clone.idToken` branch in `cloneWithoutTokens` is never exercised; this was flagged in a previous thread and remains unaddressed. |

Sequence Diagram

```mermaid sequenceDiagram participant UI as auth-menu / cli participant IDX as index.ts participant SYNC as codex-multi-auth-sync participant STOR as storage.ts participant FS as filesystem note over UI,FS: Sync-now flow UI->>IDX: experimental-sync-now IDX->>SYNC: loadCodexMultiAuthSourceStorage() SYNC->>FS: readFile(accountsPath) FS-->>SYNC: raw JSON IDX->>SYNC: previewSyncFromCodexMultiAuth() SYNC->>STOR: previewImportAccountsWithExistingStorage() STOR-->>SYNC: { imported, skipped, total } IDX->>IDX: confirm import? IDX->>SYNC: syncFromCodexMultiAuth() SYNC->>STOR: importAccounts(tempFile, options, prepare) STOR->>STOR: withAccountStorageTransaction → persist STOR-->>IDX: ImportAccountsResult note over UI,FS: Prune-then-retry flow (capacity error) IDX->>IDX: createSyncPruneBackup() IDX->>STOR: loadAccountAndFlaggedStorageSnapshot() STOR->>STOR: withStorageLock → loadAccounts + loadFlaggedAccounts STOR-->>IDX: snapshot IDX->>FS: writeFile(tempBackupPath) → rename(backupPath) [atomic] IDX->>IDX: removeAccountsForSync(targets) IDX->>STOR: withAccountStorageTransaction → persist IDX->>STOR: withFlaggedAccountsTransaction → persist IDX->>SYNC: syncFromCodexMultiAuth() [retry] note over UI,FS: Overlap-cleanup flow UI->>IDX: experimental-cleanup-overlaps IDX->>SYNC: previewCodexMultiAuthSyncedOverlapCleanup() SYNC->>STOR: withAccountStorageTransaction(readonly) STOR-->>SYNC: CleanupResult preview IDX->>IDX: confirm cleanup? IDX->>SYNC: cleanupCodexMultiAuthSyncedOverlaps(backupPath) SYNC->>STOR: withAccountStorageTransaction STOR->>FS: writeFile(backupPath) [NON-ATOMIC ⚠] STOR->>STOR: buildCodexMultiAuthOverlapCleanupPlan → persist ```

Comments Outside Diff (1)

1. `lib/codex-multi-auth-sync.ts`, line 1932-1939 ([link](https://github.com/ndycode/oc-chatgpt-multi-auth/blob/080ca634b2bd83065965e8757a7ea104a38cd27e/lib/codex-multi-auth-sync.ts#L1932-L1939)) **Non-atomic backup write inside the transaction** the backup is written with `fs.writeFile` + `flag: "wx"` — no temp→rename. on windows, if av scans or oom-kills the process mid-write, the backup file is left truncated. the `flag: "wx"` means the corrupted file will block any future retry (EEXIST at the same timestamp path is impossible since timestamps differ, but the truncated file will silently appear valid until a restore is attempted). this is the same atomicity gap that `saveAccountsUnlocked` was already fixed to use `renameWithWindowsRetry` for. apply the same pattern here: write to a `${backupPath}.${Date.now()}.tmp`, rename on success, unlink on error. note: the backup does contain live `refreshToken` values (intentional for rollback). a truncated backup with a partial token array is worse than no backup at all. no vitest covers a backup mid-write interruption for the overlap cleanup path.

Prompt To Fix With AI

`````markdown This is a comment left during a code review. Path: lib/codex-multi-auth-sync.ts Line: 1932-1939 Comment: **Non-atomic backup write inside the transaction** the backup is written with `fs.writeFile` + `flag: "wx"` — no temp→rename. on windows, if av scans or oom-kills the process mid-write, the backup file is left truncated. the `flag: "wx"` means the corrupted file will block any future retry (EEXIST at the same timestamp path is impossible since timestamps differ, but the truncated file will silently appear valid until a restore is attempted). this is the same atomicity gap that `saveAccountsUnlocked` was already fixed to use `renameWithWindowsRetry` for. apply the same pattern here: write to a `${backupPath}.${Date.now()}.tmp`, rename on success, unlink on error. note: the backup does contain live `refreshToken` values (intentional for rollback). a truncated backup with a partial token array is worse than no backup at all. no vitest covers a backup mid-write interruption for the overlap cleanup path. How can I resolve this? If you propose a fix, please make it concise. `````

_{Last reviewed commit: ddb1a0f} > Greptile also left **1 inline comment** on this PR. **Context used:** - Rule used - What: Every code change must explain how it defend... ([source](https://app.greptile.com/review/custom-context?memory=637a42e6-7a78-40d6-9ef8-6a45e02e73b6))