feat(sync): add manual rollback

[ndycode]

Summary

• Reopens the preserved sync-rollback slice from archived PR #88
• Stacked PR on fresh/14-auto-snapshots

What Changed

• Adds manual rollback planning and restore support for sync-center operations.
• Keeps the branch focused to manual rollback behavior, checkpoint selection, and the matching sync/CLI coverage.

Validation

• npm run lint
• npm run typecheck
• npm test
• npm test -- test/documentation.test.ts
• npm run build

Docs and Governance Checklist

• README updated (if user-visible behavior changed)
• docs/getting-started.md updated (if onboarding flow changed)
• docs/features.md updated (if capability surface changed)
• relevant docs/reference/* pages updated (if commands/settings/paths changed)
• docs/upgrade.md updated (if migration behavior changed)
• SECURITY.md and CONTRIBUTING.md reviewed for alignment

Risk and Rollback

• Risk level: medium
• Rollback plan: revert d262177

Additional Notes

• Preserved branch validation from the fresh-restack run passed lint, typecheck, build, and full tests.

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 reopens the archived sync-rollback slice from #88 and lands manual rollback planning and restore support for the sync center, alongside a new reset destructive action, a restore-backup login mode, and a sync-history.ts ndjson journal.

key changes:

• lib/codex-cli/sync.ts — adds previewCodexCliSync, applyCodexCliSyncToStorage, getLatestCodexCliSyncRollbackPlan, rollbackLatestCodexCliSync, and the pending/commit revision machinery; two concurrency issues identified (see inline comments)
• lib/sync-history.ts — new ndjson history file with a promise-chain mutex, tail-read optimization, and a synchronous fast-path read (readFileSync) on first access
• lib/destructive-actions.ts — correctly centralizes deleteAccountAtIndex, deleteSavedAccounts, and resetLocalState; removes ad-hoc storage calls from index.ts and codex-manager.ts
• lib/codex-manager.ts / lib/cli.ts — wires reset and restore-backup into the login flow with destructiveActionInFlight guard and typed-confirm prompts

two p1 concurrency issues to address before merge:

1. commitPendingCodexCliSyncRun allocates a fresh publish revision at commit time rather than at pending-allocation time — if an older run commits after a newer one it gets a higher revision and overwrites lastCodexCliSyncRun, corrupting the rollback plan target
2. rollbackLatestCodexCliSync has no in-flight guard — concurrent calls can both write to storage and both publish history entries; on windows the second write can race the first on the .bak file and exhaust the retry budget

additional notes:

• readLatestSyncHistorySync uses readFileSync and blocks the event loop on first getLastCodexCliSyncRun() access
• EACCES is absent from RETRYABLE_ROLLBACK_SAVE_CODES and is a common windows access-denied error code; the win32 EPERM-only branch may leave legit retryable errors unhandled
• test coverage is thorough except for the two concurrency scenarios above

Confidence Score: 2/5

• not safe to merge until the revision-allocation race and the double-rollback guard are resolved
• two p1 concurrency bugs in the rollback critical path: (1) commitPendingCodexCliSyncRun allocates a new revision at commit time, so an older in-flight run can overwrite a newer manual apply as lastCodexCliSyncRun and corrupt the rollback snapshot reference; (2) rollbackLatestCodexCliSync has no in-flight guard, allowing concurrent calls to write storage twice and log the rollback twice; both issues are untested; rest of the pr is well-structured and the destructive-actions refactor is clean
• lib/codex-cli/sync.ts (lines 339-359 and 574-630) needs the concurrency fixes before merge

Important Files Changed

Filename	Overview
lib/codex-cli/sync.ts	major new file — adds rollback planning, preview, and apply; has two concurrency issues: revision-allocation-at-commit-time causes stale-overwrite when runs complete out of order, and rollbackLatestCodexCliSync has no in-flight guard against double execution
lib/sync-history.ts	new file — ndjson sync history with promise-chain mutex for writes, tail-read optimization for limit queries, and latest-snapshot fast path; readLatestSyncHistorySync uses readFileSync which blocks the event loop on first access
lib/destructive-actions.ts	new file — correctly centralizes deleteAccountAtIndex, deleteSavedAccounts, and resetLocalState; stale-storage fallback when current is null in the transaction is a minor edge-case concern; otherwise well-tested
lib/codex-manager.ts	wires in new reset/restore-backup modes and destructiveActionInFlight guard; code looks correct; suppressRecoveryPrompt flow after fresh/reset is correctly set
test/codex-cli-sync.test.ts	extensive new coverage for preview, rollback, selection timestamp retry, and concurrent applies; missing test for commit-order race (older run overwriting newer) and double-rollback concurrent execution
test/destructive-actions.test.ts	new test file — good coverage of deleteAccountAtIndex index rebasing, deleteSavedAccounts isolation, resetLocalState partial-failure (EPERM), and Codex CLI state cache invalidation
test/sync-history.test.ts	new test file — covers tail-read optimization, async history fallback for getLastCodexCliSyncRun, and mutex write-ordering; well-structured with proper isolation

Sequence Diagram

sequenceDiagram
    participant UI as codex-manager / index.ts
    participant Sync as codex-cli/sync.ts
    participant History as sync-history.ts
    participant Storage as storage.ts

    UI->>Sync: applyCodexCliSyncToStorage(current, {trigger:"manual"})
    Sync->>Storage: captureRollbackSnapshot(current, targetPath)
    Storage-->>Sync: rollbackSnapshot {name, path}
    Sync-->>UI: {storage, changed:true, pendingRun:{revision:N, run:{rollbackSnapshot}}}

    UI->>Storage: saveAccounts(storage)
    Storage-->>UI: ok

    UI->>Sync: commitPendingCodexCliSyncRun(pendingRun)
    Note over Sync: allocates NEW publish revision at commit time ⚠️
    Sync->>History: appendSyncHistoryEntry({kind:"codex-cli-sync", run})
    History-->>Sync: ok
    Note over Sync: lastCodexCliSyncRun = run

    UI->>Sync: getLatestCodexCliSyncRollbackPlan()
    Sync->>Sync: findLatestManualRollbackRun()
    Sync->>History: readSyncHistory({kind:"codex-cli-sync", limit:200})
    History-->>Sync: entries[]
    Sync-->>UI: {status:"ready", snapshot, storage}

    UI->>Sync: rollbackLatestCodexCliSync(plan)
    Note over Sync: no in-flight guard ⚠️
    Sync->>Storage: saveRollbackStorageWithRetry(plan.storage)
    Storage-->>Sync: ok
    Sync->>History: appendSyncHistoryEntry(rollback run)
    Sync-->>UI: {status:"restored", snapshot, accountCount}

Loading

Comments Outside Diff (10)

1. lib/codex-manager/settings-hub.ts, line 322-400 (link)

   Unnecessary typeof runtime guards on statically-imported module

   codexCliSyncModule is imported via import * as codexCliSyncModule from "../codex-cli/sync.js" — a static ES import. TypeScript already enforces that getLatestCodexCliSyncRollbackPlan and rollbackLatestCodexCliSync exist on that module at compile time. The typeof ... === "function" fallback branches are dead code in production and silently suppress future refactor errors (e.g., if either export is renamed, the guard would route to the fallback instead of surfacing a type error).

   The correct pattern here is to call the imports directly, the same way every other sync function is called in this file (e.g., applyCodexCliSyncToStorage, previewCodexCliSync). Remove the wrapper functions entirely and call codexCliSyncModule.getLatestCodexCliSyncRollbackPlan() and codexCliSyncModule.rollbackLatestCodexCliSync(plan) inline, or just import them by name at the top of the file like the other exports.

2. lib/codex-cli/sync.ts, line 266-278 (link)

   Rollback plan stays "ready" after a successful restore

   rollbackLatestCodexCliSync calls saveAccounts(resolvedPlan.storage) but does not append any SyncHistoryEntry to the history. This means findLatestManualRollbackRun() will always return the same manual "changed" run, so getLatestCodexCliSyncRollbackPlan() will remain "ready" indefinitely after a successful rollback.

   In the sync center loop, rollbackPlan is refreshed right after the restore succeeds — and the user immediately sees the rollback option enabled again pointing at the same checkpoint. Clicking it again silently re-saves the same storage (idempotent but confusing). On Windows, repeated saves from the same stale in-memory snapshot also bypass the lock-check that snapshotAccountStorage would otherwise trigger.

   Consider recording a lightweight rollback history entry (outcome "noop" or a new "rollback" outcome) after a successful restore, or marking the used snapshot as consumed so the plan transitions to "unavailable" after the first restore.

3. lib/codex-cli/sync.ts, line 444-450 (link)

   Token-sensitive path exposed in error message

   snapshot.path is the location of a JSON file that contains refreshToken and accessToken in plaintext. embedding the full filesystem path in the reason string means any caller that logs, displays, or surfaces this error will leak the credential store location.

   on windows desktop environments (the primary target for this app) this path typically sits inside %APPDATA% and the full path can reveal the username.

   the ENOENT message should redact or omit the path:

4. lib/codex-cli/sync.ts, line 471-501 (link)

   No write-retry for Windows file locking on the exported rollback path

   rollbackLatestCodexCliSync calls saveAccounts directly, with no retry or queuing mechanism. the ui entry-point in settings-hub.ts wraps runSyncCenterRollback in withQueuedRetry, so it is protected. but this function is also exported and called by rollbackLastCodexCliSync (which is what CLI tooling and tests use) with zero file-lock protection.

   on windows, antivirus software routinely holds an exclusive lock on json credential files for hundreds of milliseconds after a write, which is exactly when a user might trigger a rollback (right after a sync apply). the result is a status: "error" with an opaque system error message, with no retry, and no windows-specific guidance.

   this is the concurrency risk the custom project policy requires to be explicitly addressed. at minimum, document whether the caller is expected to provide retry, or add retry internally. also, there's no test covering EBUSY / EPERM failure during rollback.

5. lib/codex-cli/sync.ts, line 306-311 (link)

   write-error path leaks filesystem paths (token/path leakage risk)

   the read path (loadRollbackSnapshot) correctly wraps errors with getRedactedFilesystemErrorLabel, but the write path here passes error.message raw into reason. on windows, saveAccounts can throw errors whose messages include the full file path (e.g. EPERM: operation not permitted, open 'C:\Users\john\AppData\Roaming\...'), which leaks the local username. this is inconsistent with the rest of the file and violates our redaction strategy.

   Rule Used: What: Every code change must explain how it defend... (source)

6. lib/codex-cli/sync.ts, line 540-552 (link)

   rollbackLastCodexCliSync bypasses write queue - windows concurrency risk

   this exported function calls rollbackLatestCodexCliSync() with no pre-fetched plan and no withQueuedRetry wrapper. every other write path in promptSyncCenter (apply, save, rollback from the UI) goes through withQueuedRetry(preview.targetPath, ...) to serialize filesystem writes. a programmatic caller of rollbackLastCodexCliSync() running concurrently with the UI loop's rollback or apply action will race to call saveAccounts on windows, which is exactly the EBUSY/EPERM class of bug the retry logic is defending against.

   the UI path correctly uses:

   await withQueuedRetry(preview.targetPath, async () =>
       runSyncCenterRollback(rollbackPlan),
   )

   but a direct call to rollbackLastCodexCliSync() in a CLI command or background task gets no such protection. consider either removing this export (forcing callers through rollbackLatestCodexCliSync with a pre-fetched plan), or documenting that callers are responsible for serializing writes.

7. lib/codex-cli/sync.ts, line 555-588 (link)

   Rollback success is not recorded in sync history and doesn't reset in-memory state

   after a successful saveRollbackStorageWithRetry, the function returns without:

   1. calling appendSyncHistoryEntry — rollback events are completely invisible in the history ndjson
   2. clearing or updating lastCodexCliSyncRun — it still points to the pre-rollback manual "changed" run with its snapshot

   the consequence: getLastCodexCliSyncRun() returns stale state post-rollback (ui shows wrong "last sync"), and calling rollbackLatestCodexCliSync() again immediately finds the same in-memory plan without any async history read, restoring from the same checkpoint again silently. this is idempotent but produces zero audit trail for either rollback event.

   at minimum, clear lastCodexCliSyncHistoryLoadAttempted and null out lastCodexCliSyncRun so the next caller re-reads from history, and add an appendSyncHistoryEntry call for the rollback event:

   	try {
   		await saveRollbackStorageWithRetry(resolvedPlan.storage);
   		// reset in-memory run so callers re-read from history (avoids silent re-rollback)
   		lastCodexCliSyncRun = null;
   		lastCodexCliSyncHistoryLoadAttempted = false;
   		void appendSyncHistoryEntry({
   			kind: "codex-cli-sync",
   			recordedAt: Date.now(),
   			run: createSyncRun({
   				outcome: "changed",
   				sourcePath: null,
   				targetPath: getStoragePath(),
   				summary: createEmptySyncSummary(),
   				trigger: "manual",
   				rollbackSnapshot: null,
   				message: `Rolled back to snapshot: ${resolvedPlan.snapshot?.name ?? "unknown"}`,
   			}),
   		}).catch(() => undefined);
   		return {
   			status: "restored",
   			reason: resolvedPlan.reason,
   			snapshot: resolvedPlan.snapshot,
   			accountCount:
   				resolvedPlan.accountCount ?? resolvedPlan.storage.accounts.length,
   		};

8. lib/sync-history.ts, line 195-221 (link)

   pendingHistoryWrites snapshot race with waitForPendingHistoryWrites

   waitForPendingHistoryWrites snapshots pendingHistoryWrites with Array.from(...) at call time. but pendingHistoryWrites.add(writePromise) happens after withHistoryLock(...) returns (synchronously), and pendingHistoryWrites.delete(writePromise) runs in the outer finally block after the promise settles.

   the gap: between waitForPendingHistoryWrites capturing the set snapshot and resolving, a concurrent caller can add a new write to pendingHistoryWrites that isn't waited on. readSyncHistory could then read the file before the new entry is flushed.

   this is currently masked because readSyncHistory also uses the history lock via the ndjson read path, but the lock is not held during the full read flow — waitForPendingHistoryWrites only drains, it doesn't block new arrivals:

   async function waitForPendingHistoryWrites(): Promise<void> {
   	if (pendingHistoryWrites.size === 0) return;
   	// snapshot is taken here — new writes arriving after this are missed
   	await Promise.allSettled(Array.from(pendingHistoryWrites));
   }

   worth adding a note in the jsdoc that callers should not assume this is a complete drain barrier against concurrent producers.

9. lib/codex-cli/sync.ts, line 339-359 (link)

   stale-overwrite concurrency in revision allocation

   allocateCodexCliSyncRunRevision() is called at commit time, not at pending-allocation time. consider this sequence:

   1. run A starts → allocatePendingCodexCliSyncRunRevision() → pending rev 1
   2. run B starts → pending rev 2
   3. B commits first → calls allocateCodexCliSyncRunRevision() → publish rev 3 → lastCodexCliSyncRun = B
   4. A commits after → calls allocateCodexCliSyncRunRevision() → publish rev 4 → 4 > 3 → lastCodexCliSyncRun = A, overwriting B

   A is the older run but wins the revision race because it committed last. for manual+changed runs this directly corrupts the rollback plan: findLatestManualRollbackRun reads getLastCodexCliSyncRun() first, so it would find A's pre-A snapshot instead of B's pre-B snapshot even though B ran after A.

   fix: carry the commit revision inside PendingCodexCliSyncRun by allocating it at pending time and using that value in commitPendingCodexCliSyncRun instead of calling allocateCodexCliSyncRunRevision() again.

   export interface PendingCodexCliSyncRun {
     revision: number;
     commitRevision: number; // allocated at pending time
     run: CodexCliSyncRun;
   }
   

   this is also missing a vitest for the commit-ordering case (both pending runs committed in reverse order).

10. lib/codex-cli/sync.ts, line 574-630 (link)

    no guard against concurrent rollback calls

    rollbackLatestCodexCliSync has no mutex or in-flight flag. if called twice concurrently with the same plan object (both pass the plan.status === "ready" check before either write begins), both calls proceed through saveRollbackStorageWithRetry and both reach publishCodexCliSyncRun. because publishCodexCliSyncRun allocates a fresh revision for each call, both publishes succeed — the rollback gets written to storage twice and logged twice in sync history.

    on windows, the second write races the first and can hit EPERM / EBUSY on the .bak file created by saveAccounts during the first write, which the retry loop would then retry five times before failing.

    the minimal fix is a module-level let rollbackInFlight = false guard:

    if (rollbackInFlight) {
      return { status: "unavailable", reason: "A rollback is already in progress.", snapshot: null };
    }
    rollbackInFlight = true;
    try { ... } finally { rollbackInFlight = false; }
    

    no test covers this concurrent-rollback path.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: lib/codex-cli/sync.ts
Line: 339-359

Comment:
**stale-overwrite concurrency in revision allocation**

`allocateCodexCliSyncRunRevision()` is called at **commit time**, not at pending-allocation time. consider this sequence:

1. run A starts → `allocatePendingCodexCliSyncRunRevision()` → pending rev 1
2. run B starts → pending rev 2
3. B commits first → calls `allocateCodexCliSyncRunRevision()` → **publish rev 3** → `lastCodexCliSyncRun = B`
4. A commits after → calls `allocateCodexCliSyncRunRevision()` → **publish rev 4** → 4 > 3 → **`lastCodexCliSyncRun = A`**, overwriting B

A is the older run but wins the revision race because it committed last. for manual+changed runs this directly corrupts the rollback plan: `findLatestManualRollbackRun` reads `getLastCodexCliSyncRun()` first, so it would find A's pre-A snapshot instead of B's pre-B snapshot even though B ran after A.

fix: carry the commit revision inside `PendingCodexCliSyncRun` by allocating it at pending time and using that value in `commitPendingCodexCliSyncRun` instead of calling `allocateCodexCliSyncRunRevision()` again.

```
export interface PendingCodexCliSyncRun {
  revision: number;
  commitRevision: number; // allocated at pending time
  run: CodexCliSyncRun;
}
```

this is also missing a vitest for the commit-ordering case (both pending runs committed in reverse order).

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-cli/sync.ts
Line: 574-630

Comment:
**no guard against concurrent rollback calls**

`rollbackLatestCodexCliSync` has no mutex or in-flight flag. if called twice concurrently with the same `plan` object (both pass the `plan.status === "ready"` check before either write begins), both calls proceed through `saveRollbackStorageWithRetry` and both reach `publishCodexCliSyncRun`. because `publishCodexCliSyncRun` allocates a fresh revision for each call, both publishes succeed — the rollback gets written to storage twice and logged twice in sync history.

on windows, the second write races the first and can hit `EPERM` / `EBUSY` on the `.bak` file created by `saveAccounts` during the first write, which the retry loop would then retry five times before failing.

the minimal fix is a module-level `let rollbackInFlight = false` guard:

```
if (rollbackInFlight) {
  return { status: "unavailable", reason: "A rollback is already in progress.", snapshot: null };
}
rollbackInFlight = true;
try { ... } finally { rollbackInFlight = false; }
```

no test covers this concurrent-rollback path.

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/sync-history.ts
Line: 274-284

Comment:
**`readLatestSyncHistorySync` blocks the event loop**

`readFileSync` is called from `readLatestSyncHistorySync`, which is invoked from `getLastCodexCliSyncRun()` on the very first call. for slow or remote filesystems (common when the codex log dir is on a network mount or on windows with defender scanning), this can block the event loop for tens of ms during dashboard startup.

consider making the first-load path fully async (returning a promise), or at minimum add a size cap so the blocking call is bounded:

```ts
export function readLatestSyncHistorySync(): SyncHistoryEntry | null {
  try {
    // guard: skip if file is unreasonably large to keep the blocking window bounded
    const stat = statSync(getSyncHistoryPaths().latestPath);
    if (stat.size > 64 * 1024) return null;
    ...
  }
}
```

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-cli/sync.ts
Line: 1529-1538

Comment:
**`EACCES` missing from windows rollback retry set**

`RETRYABLE_ROLLBACK_SAVE_CODES` includes `EBUSY` and `EAGAIN`, and `EPERM` is added only for `win32`. on windows, access-denied errors frequently surface as `EACCES` (node maps `ERROR_ACCESS_DENIED` to `EACCES` on older runtimes and via some antivirus/defender holds). if the rollback file write is blocked by a scanner, `EACCES` would not be retried and the rollback would fail immediately.

```ts
const RETRYABLE_ROLLBACK_SAVE_CODES = new Set(["EBUSY", "EAGAIN", "EACCES"]);
```

and remove the `win32`-only `EPERM` branch (or keep both). token safety note: if the retry exhausts and throws, the caller should log a redacted error — which it does via `getRedactedFilesystemErrorLabel` — so tokens are not leaked in the error message.

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/destructive-actions.ts
Line: 108-121

Comment:
**stale-storage fallback when `current` is null**

inside the `withAccountAndFlaggedStorageTransaction` callback:
```ts
const sourceStorage = current ?? options.storage;
```

if `current` is `null` (no accounts file on disk at transaction start), the function silently falls back to `options.storage`, which is the caller's in-memory state that may have been loaded earlier. if another process deleted the accounts file between the caller loading storage and invoking `deleteAccountAtIndex`, this fallback means the deletion proceeds on stale data rather than surfacing a clear error.

consider returning `null` early when `current === null` to make the intent explicit, since deleting from a storage that doesn't exist on disk is a no-op anyway:
```ts
const sourceStorage = current;
if (!sourceStorage) return null;
```

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

_{Last reviewed commit: "fix(sync): record ro..."}

[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.

[coderabbitai]

📝 Walkthrough

Walkthrough

introduces a rollback capability for codex cli sync operations. adds new types and public functions to capture snapshots on manual syncs, retrieve the latest rollback plan, and execute rollback restoration. wires rollback plan fetching and execution into the sync center ui with keyboard shortcuts. extends storage and ui copy.

Changes

Cohort / File(s)	Summary
Rollback Core Infrastructure lib/codex-cli/sync.ts	introduces CodexCliSyncTrigger ("manual" | "automatic"), CodexCliSyncRollbackSnapshot, CodexCliSyncRollbackPlan, and CodexCliSyncRollbackResult types. adds public functions: getLatestCodexCliSyncRollbackPlan, rollbackLatestCodexCliSync, rollbackLastCodexCliSync. expands CodexCliSyncRun with trigger and rollbackSnapshot fields. implements capture, load, and save helpers for rollback snapshots with retry logic (saveRollbackStorageWithRetry, isRetryableRollbackSaveError). modifies applyCodexCliSyncToStorage to accept trigger and conditionally capture rollback snapshots on manual changes.
Sync Center UI Integration lib/codex-manager/settings-hub.ts	adds rollback action type to SyncCenterAction. implements getSyncCenterRollbackPlan and runSyncCenterRollback functions. wires rollback plan fetching at initialization, after refresh, and after rollback execution. enables rollback keyboard shortcut ("L") and displays rollback status in ui based on plan readiness. passes trigger: "manual" to sync operations.
Type & Text Updates lib/storage.ts, lib/ui/copy.ts	adds "codex-cli-sync" to AccountSnapshotReason union. adds syncCenterRollback ui text entry and updates syncCenterHelp to include "L Rollback" option between Apply and Refresh.
Test Coverage test/codex-cli-sync.test.ts, test/codex-manager-cli.test.ts	introduces comprehensive rollback tests: snapshot capture on manual applies, skipping when no changes occur, restoration from latest manual apply, error handling for missing/invalid checkpoints, retry logging on transient filesystem errors. exports CodexCliSyncRun, getLatestCodexCliSyncRollbackPlan, rollbackLastCodexCliSync, rollbackLatestCodexCliSync, appendSyncHistoryEntry. adds mocks for rollback functions in cli test setup.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant SyncCenter as Sync Center UI
    participant CLI as Codex CLI Sync
    participant Storage as Account Storage
    participant Filesystem as Filesystem

    User->>SyncCenter: Apply Changes (manual trigger)
    activate SyncCenter
    SyncCenter->>CLI: applyCodexCliSyncToStorage(trigger: "manual")
    activate CLI
    CLI->>Storage: apply changes
    activate Storage
    Storage->>Filesystem: write normalized accounts
    Filesystem-->>Storage: success
    deactivate Storage
    CLI->>CLI: detectChangesOccurred()
    alt changes detected
        CLI->>Filesystem: captureRollbackSnapshot()
        Filesystem-->>CLI: snapshot metadata (name, path)
        CLI->>Filesystem: saveRollbackStorageWithRetry()
        Filesystem-->>CLI: snapshot saved
    end
    CLI-->>SyncCenter: CodexCliSyncRun {trigger, rollbackSnapshot}
    deactivate CLI
    SyncCenter->>SyncCenter: recordSync
    deactivate SyncCenter

    User->>SyncCenter: Request Rollback
    activate SyncCenter
    SyncCenter->>CLI: getLatestCodexCliSyncRollbackPlan()
    activate CLI
    CLI->>Filesystem: findLatestManualRollbackRun()
    Filesystem-->>CLI: manual run with snapshot
    CLI->>Filesystem: loadRollbackSnapshot()
    Filesystem-->>CLI: CodexCliSyncRollbackPlan {ready, snapshot, storage}
    deactivate CLI
    CLI-->>SyncCenter: plan ready
    SyncCenter->>User: display rollback available
    User->>SyncCenter: Confirm Rollback
    SyncCenter->>CLI: rollbackLatestCodexCliSync()
    activate CLI
    CLI->>Storage: restoreFromSnapshot(plan)
    activate Storage
    Storage->>Filesystem: write restored accounts
    Filesystem-->>Storage: success
    deactivate Storage
    CLI-->>SyncCenter: CodexCliSyncRollbackResult {restored, snapshot}
    deactivate CLI
    SyncCenter->>SyncCenter: refresh plan & state
    deactivate SyncCenter

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

concerns worth flagging:

• concurrency risk in rollback snapshot capture: lib/codex-cli/sync.ts doesn't appear to guard against concurrent manual syncs overwriting the same rollback snapshot. if two manual applies land near-simultaneously, the second could corrupt or overwrite the first's snapshot metadata. check that sync runs serialize or use atomic file operations.

• windows path handling in rollback snapshots: rollback snapshots store snapshot.path as strings. verify that path normalization in lib/codex-cli/sync.ts handles windows backslashes vs forward slashes correctly when loading/comparing snapshot locations cross-platform.

• missing regression test for failed rollback recovery: test/codex-cli-sync.test.ts covers retry logic and transient errors, but lacks end-to-end test verifying that a rollback failure leaves accounts in a safe, readable state and doesn't corrupt the snapshot manifest for future attempts.

• no test coverage for partial snapshot save failures: if saveRollbackStorageWithRetry succeeds in saving snapshot metadata but fails partway through account storage writes, the rollback snapshot could be marked as "ready" when it's actually incomplete. test that a failed snapshot save doesn't register a usable rollback plan.

• trigger metadata doesn't persist in sync history for ui replay: lib/codex-manager/settings-hub.ts passes trigger: "manual" to sync, but verify that older sync history entries missing the trigger field gracefully default or are handled by the rollback plan lookup logic.

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 3.85% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	⚠️ Warning	PR description follows the template structure and includes summary, what changed, validation checklist (all passing), docs checklist (mostly unchecked), risk assessment, and notes. However, critical P1 concurrency issues are mentioned in greptile review but not documented in the PR description itself.	Explicitly document the two known P1 concurrency issues in the PR description or risk section: (1) revision-allocation-at-commit-time race in lib/codex-cli/sync.ts:339-359, and (2) missing rollback in-flight guard in lib/codex-cli/sync.ts:574-630. These must be resolved before merge.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	Title follows conventional commits format with type 'feat', scope 'sync', and concise lowercase summary (31 chars, well under 72 limit) that accurately describes the main feature addition.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fresh/15-sync-rollback

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch fresh/15-sync-rollback

📝 Coding Plan

• Generate coding plan for human review comments

---

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 and usage tips.}

[Copilot]

Pull request overview

Adds manual rollback support for Codex CLI sync “sync-center” operations by capturing pre-apply snapshots, exposing rollback planning/restoration APIs, and wiring a rollback action into the settings UI/CLI with corresponding tests.

Changes:

• Capture and persist rollback checkpoint metadata for manual Codex CLI sync applies.
• Introduce rollback planning + restore APIs (getLatestCodexCliSyncRollbackPlan, rollbackLatestCodexCliSync, rollbackLastCodexCliSync).
• Add a rollback action (L) to the sync-center UI and expand test coverage around checkpoint selection and error cases.

Reviewed changes

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

Show a summary per file

File	Description
test/codex-manager-cli.test.ts	Extends CLI mocks/reset paths to include new rollback APIs.
test/codex-cli-sync.test.ts	Adds tests for snapshot capture, rollback planning/restore, and error handling.
lib/ui/copy.ts	Updates sync-center help text and adds rollback action label.
lib/storage.ts	Adds a new snapshot reason for codex-cli sync checkpoints.
lib/codex-manager/settings-hub.ts	Wires rollback into sync-center menu + key handling and triggers “manual” apply runs.
lib/codex-cli/sync.ts	Implements rollback snapshot capture, plan resolution, and restore logic; augments sync run metadata.

---

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

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@lib/codex-cli/sync.ts`:
- Around line 494-503: getLatestCodexCliSyncRollbackPlan currently relies only
on readSyncHistory() and can miss a just-appended checkpoint or throw; update it
to first consult getLastCodexCliSyncRun() (to pick up the most recent
in-memory/just-published run from publishCodexCliSyncRun()), and only then
attempt the history read; wrap the
readSyncHistory()/findLatestManualRollbackRun() logic in a try/catch so that any
history-read exception is swallowed and the function returns a closed failure {
status: "unavailable", reason: "...", snapshot: null } instead of letting the
rejection escape; keep using loadRollbackSnapshot() when a valid manual run is
found.
- Around line 362-372: captureRollbackSnapshot currently re-reads disk via
snapshotAccountStorage which races with concurrent saveAccounts; change
captureRollbackSnapshot to accept and use the in-memory baseline/accounts passed
from the reconciliation flow (e.g., add a parameter like currentBaseline or
accounts and update the caller that invokes captureRollbackSnapshot after
reconcile/save) and either (a) extend snapshotAccountStorage to accept an
accounts object to create metadata from memory, or (b) add a new
snapshotFromBaseline(accounts) helper used by captureRollbackSnapshot so the
rollback metadata reflects the exact manual apply state; ensure snapshot
creation is performed while holding the same storage lock used by saveAccounts
or implement a tight lock/atomic handoff to avoid races with saveAccounts, add
retries/backoff for filesystem EBUSY/HTTP 429 during snapshot write, and update
tests (test/codex-cli-sync.test.ts) to assert metadata and retry behavior; keep
logs free of tokens/emails.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: b4aeef97-8092-4039-8bb8-94c562b86e01

📥 Commits

Reviewing files that changed from the base of the PR and between 97a02c2 and 1b45f34.

📒 Files selected for processing (6)

• lib/codex-cli/sync.ts
• lib/codex-manager/settings-hub.ts
• lib/storage.ts
• lib/ui/copy.ts
• test/codex-cli-sync.test.ts
• test/codex-manager-cli.test.ts

📜 Review details

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Greptile Review

🧰 Additional context used

📓 Path-based instructions (2)

lib/**

⚙️ CodeRabbit configuration file

focus on auth rotation, windows filesystem IO, and concurrency. verify every change cites affected tests (vitest) and that new queues handle EBUSY/429 scenarios. check for logging that leaks tokens or emails.

Files:

• lib/codex-manager/settings-hub.ts
• lib/storage.ts
• lib/ui/copy.ts
• lib/codex-cli/sync.ts

test/**

⚙️ CodeRabbit configuration file

tests must stay deterministic and use vitest. demand regression cases that reproduce concurrency bugs, token refresh races, and windows filesystem behavior. reject changes that mock real secrets or skip assertions.

Files:

• test/codex-manager-cli.test.ts
• test/codex-cli-sync.test.ts

🔇 Additional comments (6)

lib/storage.ts (1)

212-213: test coverage for the new snapshot reason already exists.

The "codex-cli-sync" snapshot reason is tested in lib/storage.ts:213. test/codex-cli-sync.test.ts includes explicit regression tests: snapshot metadata recording (test/codex-cli-sync.test.ts:1349), rollback retry on windows transient EBUSY (test/codex-cli-sync.test.ts:1840), and rollback retry on EPERM with logging validation (test/codex-cli-sync.test.ts:1912). No additional tests needed.

lib/ui/copy.ts (1)

131-136: rollback keybinding already has regression test coverage.

the "l" hotkey for sync center rollback is already wired (lib/codex-manager/settings-hub.ts:2855) and tested. rollback functions have vitest coverage in test/codex-cli-sync.test.ts:1349,1670,1760,1840,1912, and the cli-level keybinding is tested via test/codex-manager-cli.test.ts:5662 (triggerSettingsHotkey("l")). the rollback action is wrapped with withQueuedRetry (lib/codex-manager/settings-hub.ts:2872) which serializes via enqueueSettingsWrite, protecting against concurrent dispatch. retry handling covers 429 and filesystem errors (EBUSY) as required.

			> Likely an incorrect or invalid review comment.

test/codex-manager-cli.test.ts (4)

35-36: mock declarations look good.

properly declared as vi.fn() and follow naming conventions.

---

162-165: mock wiring is correct.

both getLatestCodexCliSyncRollbackPlan and rollbackLatestCodexCliSync are properly exposed in the vi.mock for ../lib/codex-cli/sync.js.

---

579-580: reset calls properly added to beforeEach.

ensures clean state between tests.

---

765-774: rollback tests already exist in the test suite—no action required.

comprehensive rollback tests are present in test/codex-cli-sync.test.ts, not in test/codex-manager-cli.test.ts. the mocks at test/codex-manager-cli.test.ts:765-774 correctly stub unavailable responses for test isolation. the actual unit tests cover the scenarios you mentioned:

• successful rollback plan retrieval and execution (test/codex-cli-sync.test.ts:1349, test/codex-cli-sync.test.ts:1471)
• transient failures with windows EBUSY retry behavior (test/codex-cli-sync.test.ts:1840)
• error handling and missing checkpoint detection (test/codex-cli-sync.test.ts:1670, test/codex-cli-sync.test.ts:1760)
• validation of rollback metadata (test/codex-cli-sync.test.ts:1717)

the mocking strategy in codex-manager-cli.test.ts is appropriate since integration with the rollback module is tested at the unit level.

			> Likely an incorrect or invalid review comment.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

avoid history-only rollback-plan lookup.

publishCodexCliSyncRun() appends history asynchronously at lib/codex-cli/sync.ts:231-239, but getLatestCodexCliSyncRollbackPlan() only consults readSyncHistory(). lib/codex-manager/settings-hub.ts:2925-2929 asks for the plan immediately after a successful apply, so the new checkpoint can be missed until the append finishes. if readSyncHistory() throws, that rejection also escapes the direct awaits in lib/codex-manager/settings-hub.ts:2791-2792 and lib/codex-manager/settings-hub.ts:2868-2869. check getLastCodexCliSyncRun() first, then fail closed to status: "unavailable" on history-read failures. there is no vitest regression in test/codex-cli-sync.test.ts:1283-1426 or test/codex-cli-sync.test.ts:1471-2032 for immediate post-commit availability or unreadable history.

As per coding guidelines, "focus on auth rotation, windows filesystem IO, and concurrency. verify every change cites affected tests (vitest) and that new queues handle EBUSY/429 scenarios. check for logging that leaks tokens or emails."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@lib/codex-cli/sync.ts` around lines 494 - 503,
getLatestCodexCliSyncRollbackPlan currently relies only on readSyncHistory() and
can miss a just-appended checkpoint or throw; update it to first consult
getLastCodexCliSyncRun() (to pick up the most recent in-memory/just-published
run from publishCodexCliSyncRun()), and only then attempt the history read; wrap
the readSyncHistory()/findLatestManualRollbackRun() logic in a try/catch so that
any history-read exception is swallowed and the function returns a closed
failure { status: "unavailable", reason: "...", snapshot: null } instead of
letting the rejection escape; keep using loadRollbackSnapshot() when a valid
manual run is found.