fix: harden storage path transactions

[ndycode]

Summary

• pin transactional account writes to the storage path captured at transaction start
• pin flagged-account writes to the matching flagged path so rollback restores the same storage root
• fail fast when exportAccounts is called from an active transaction after the storage path drifts
• allow equivalent Windows and symlink-resolved paths during that guard
• mark transaction snapshots inactive once the handler exits so later exports reload fresh state

Validation

• npm exec vitest run test/storage.test.ts
• npm run typecheck

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 hardens storage path transactions by pinning all load/save operations to the storage path captured at transaction start, rather than re-calling getStoragePath() mid-transaction. it also clears state.active in a finally block so post-transaction exports always reload fresh state, and adds a fast-fail guard in exportAccounts when called inside an active transaction whose storage path has drifted — with correct equivalence handling for windows case/slash normalization and symlink-resolved paths.

changes:

• loadAccountsInternal and saveAccountsUnlocked accept an optional storagePath parameter (defaults to getStoragePath()) so callers can pin the path
• withAccountStorageTransaction and withAccountAndFlaggedStorageTransaction capture storagePath (and flaggedStoragePath) once at transaction start and thread them through all load/save/rollback calls
• withAccountAndFlaggedStorageTransaction now also pins getFlaggedAccountsPath() at transaction start, so rollbacks target the correct flagged storage root
• both transaction wrappers set state.active = false in finally, preventing stale snapshots from leaking into post-transaction exports
• exportAccounts replaces the silent wrong-path fallback with an explicit error on path drift; unchanged-path exports continue to use the in-transaction snapshot
• new normalizeStorageComparisonPath / areEquivalentStoragePaths helpers handle symlinks and windows path equivalence
• one gap: normalizeStorageComparisonPath only catches ENOENT from fs.realpath — windows EPERM/EACCES (antivirus locks) would surface as unexpected errors from exportAccounts; worth catching those and falling back to the normalized input
• test coverage is comprehensive for the new behaviors; missing one case: export-path-drift guard exercised inside withAccountAndFlaggedStorageTransaction

Confidence Score: 4/5

• safe to merge with one targeted follow-up: extend the realpath error catch to cover windows EPERM/EACCES
• the path-pinning logic is correct and well-reasoned; active flag teardown via finally is solid; the fast-fail export guard eliminates a silent wrong-path data hazard; test coverage is extensive across the new behaviors including windows-style paths, symlinks, and rollback under EBUSY. single gap is the narrow ENOENT-only catch in normalizeStorageComparisonPath which is a p2 windows safety concern, not a blocking logic bug.
• lib/storage.ts lines 393–402 — normalizeStorageComparisonPath error catch should also cover EPERM/EACCES for windows safety

Important Files Changed

Filename	Overview
lib/storage.ts	hardened transaction path pinning: storage path captured at transaction start is threaded through all load/save calls; active flag cleared in finally; exportAccounts fast-fails on path drift; new normalizeStorageComparisonPath handles symlinks and windows case/slash normalization correctly — one gap: only ENOENT is caught in the realpath fallback, windows EPERM/EACCES would surface as unexpected errors
test/storage.test.ts	thorough new test coverage for path-drift pinning, windows-style path equivalence, symlink-resolved paths, active-flag reset on throw/success, and rollback under EBUSY; one missing case: export-path-drift guard inside a combined (account + flagged) transaction

Sequence Diagram

sequenceDiagram
    participant C as caller
    participant T as withAccountStorageTransaction
    participant L as loadAccountsInternal
    participant S as saveAccountsUnlocked
    participant E as exportAccounts
    participant ALS as AsyncLocalStorage

    C->>T: call handler
    T->>T: capture storagePath = getStoragePath()
    T->>L: load(persistMigration, storagePath)
    L-->>T: snapshot
    T->>ALS: run(state{snapshot, storagePath, active:true}, handler)
    ALS->>C: handler(current, persist)
    C->>S: persist(storage) → saveAccountsUnlocked(storage, storagePath)
    S-->>C: done
    note over C: path drift may occur here
    C->>E: exportAccounts(path)
    E->>ALS: getStore() → state
    alt paths differ
        E-->>C: throw "Export blocked by storage path mismatch"
    else paths equivalent
        E-->>C: export from state.snapshot
    end
    C-->>ALS: handler returns/throws
    ALS->>ALS: finally: state.active = false
    T-->>C: result or error

Loading

Prompt To Fix All With AI

This is a comment left during a code review.
Path: lib/storage.ts
Line: 393-402

Comment:
**Windows EPERM/EACCES not caught in realpath fallback**

the catch only falls back on `ENOENT`. on windows, `fs.realpath` on a restricted or temporarily locked path (e.g., antivirus mid-scan) can throw `EPERM` or `EACCES`. those bubble straight out of `exportAccounts` as a raw syscall error instead of either the clean path-mismatch message or a graceful fallback. given this project explicitly tracks windows antivirus lock risks, `EPERM` and `EACCES` should also fall back to the normalized input.

```suggestion
	} catch (error: unknown) {
		// Fall back to the normalized input when the path does not exist yet,
		// or when the filesystem denies access (e.g. Windows antivirus lock).
		if (
			!error ||
			typeof error !== "object" ||
			!("code" in error) ||
			(error.code !== "ENOENT" &&
				error.code !== "EPERM" &&
				error.code !== "EACCES")
		) {
			throw error;
		}
	}
```

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/storage.test.ts
Line: 1196-1225

Comment:
**Missing vitest coverage: export-path-drift inside combined transaction**

the "fails fast when export is called from an active transaction after the storage path changes" test only exercises `withAccountStorageTransaction`. there's no parallel test for `withAccountAndFlaggedStorageTransaction`. the path-mismatch guard in `exportAccounts` reads `transactionSnapshotContext.getStore()` regardless of which transaction wrapper set the context, so the combined variant should be covered too — especially since that wrapper is the more complex one (rollback path, two storage files).

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

_{Last reviewed commit: "fix: harden storage ..."}

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

Important

Review skipped

Auto reviews are limited based on label configuration.

🚫 Review skipped — only excluded labels are configured. (1)

• skip-review

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: c80a8be4-231d-4594-9b7c-8cf927dfe3f6

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch git-clean/pr147-storage-hardening

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch git-clean/pr147-storage-hardening

---

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