preserve storage path context for deferred saves

[ndycode]

Summary

• preserve the original storage-root context for deferred account saves so cwd changes do not redirect writes into the wrong project scope

What Changed

• captured storage path state when the account manager is created and reused that state for debounced disk writes
• taught resolvePath() to honor the stored project root when it differs from the current process cwd
• added focused storage-path regression coverage and deflaked a time-sensitive tracker assertion that blocked a fresh validation run

Validation

• npm run lint
• npm run typecheck
• npm test
• npm test -- test/documentation.test.ts
• npm run build
• Focused tests: npx vitest run test/accounts.test.ts test/paths.test.ts

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: Low
• Rollback plan: Revert 72a115e and cef0db1

Additional Notes

• The test-only follow-up stays on this branch because it was required to make the storage-path PR validate cleanly.

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 fixes a correctness bug where deferred (debounced) account saves could write to the wrong project's storage file after a cwd change. the fix captures the storage path context at AccountManager construction time and wraps saveToDisk in AsyncLocalStorage.run() so each save executes against its own scoped project root — without touching the global currentStorageState and without the save-interleave race flagged in the prior review.

key changes:

• runWithStoragePathState uses AsyncLocalStorage.run() (not enterWith), correctly scoping context per-call and restoring the ambient state when the callback resolves — this directly fixes the concurrent-save race from the prior review
• AccountManager captures storagePathState at construction via a shallow spread; safe because all StoragePathState fields are string | null primitives
• resolvePath now reads currentProjectRoot from the active ALS store (or global fallback), keeping the security gate aligned with the correct project scope during saves
• toBeCloseTo replaces toBe on floating-point health scores to deflake two time-sensitive tracker assertions
• beforeEach/afterEach state resets added in both test files to prevent cross-test state bleed
• npm test (full suite) is unchecked in the validation checklist — recommend confirming before merge given the 80% coverage threshold and the security-gate change in resolvePath

Confidence Score: 5/5

safe to merge — the concurrency race is correctly resolved using ALS scoping, all remaining findings are P2 quality suggestions

the prior P1 race (concurrent saves clobbering currentStorageState) is properly addressed by switching to AsyncLocalStorage.run(). the shallow copy of StoragePathState is safe for all-primitive fields. the resolvePath change is correct and scoped. remaining findings are a missing full test run (process gap, not a code defect) and a missing rejection test (coverage gap, not a broken gate). neither blocks merge.

test/paths.test.ts — worth adding a rejection-path test; test/accounts.test.ts — full npm test run should be confirmed

Important Files Changed

Filename	Overview
lib/storage/path-state.ts	adds runWithStoragePathState using AsyncLocalStorage.run() — correctly scopes context per-call without touching the global currentStorageState, resolving the prior save-interleave race
lib/accounts.ts	captures storage path state at construction time via shallow spread (safe: all fields are `string
lib/storage/paths.ts	replaces hardcoded process.cwd() with getStoragePathState().currentProjectRoot ?? process.cwd() in the resolvePath security gate — correctly reads the ALS-scoped root inside a run() context and falls back to process.cwd() outside one
test/accounts.test.ts	adds beforeEach/afterEach state resets, two new focused save-context tests (deferred and concurrent), and toBeCloseTo fix for flaky health-score assertions; full npm test run not confirmed
test/paths.test.ts	adds beforeEach/afterEach state resets and an acceptance test for resolvePath under an active currentProjectRoot; missing corresponding rejection test for paths outside the stored root

Sequence Diagram

sequenceDiagram
    participant App
    participant AM_A as AccountManager(repo-a)
    participant AM_B as AccountManager(repo-b)
    participant ALS as AsyncLocalStorage
    participant Disk as withAccountStorageTransaction

    App->>AM_A: new AccountManager() [cwd=repo-a]
    Note over AM_A: storagePathState = {root: repo-a}
    App->>AM_B: new AccountManager() [cwd=repo-b]
    Note over AM_B: storagePathState = {root: repo-b}

    App->>AM_A: saveToDisk()
    AM_A->>ALS: run(stateA, fn)
    Note over ALS: context = repo-a (scoped)
    ALS->>Disk: withAccountStorageTransaction(...)
    Note over Disk: getStoragePathState() → repo-a ✓

    App->>AM_B: saveToDisk() [concurrent]
    AM_B->>ALS: run(stateB, fn)
    Note over ALS: context = repo-b (separate scope)
    ALS->>Disk: withAccountStorageTransaction(...)
    Note over Disk: getStoragePathState() → repo-b ✓

    Note over ALS: both run() scopes resolve independently,\nglobal currentStorageState never mutated

Loading

Prompt To Fix All With AI

This is a comment left during a code review.
Path: test/accounts.test.ts
Line: 1808

Comment:
**full test suite not run before merge**

the validation checklist shows `npm test` unchecked. the project has an 80% vitest coverage threshold across 87 files / 2071 tests. the focused run (`npx vitest run test/accounts.test.ts test/paths.test.ts`) validates the changed paths but won't catch regressions in callers of `resolvePath` or `getStoragePathState` in other files (e.g. `storage.test.ts`, `storage-async.test.ts`, `chaos/fault-injection.test.ts`). recommend running the full suite before merging, especially given the security-gate change in `resolvePath`.

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/paths.test.ts
Line: 798-828

Comment:
**missing rejection coverage for `resolvePath` under active storage state**

the new test only asserts acceptance (path inside `currentProjectRoot` is allowed). there's no test asserting that a path from a *different* project root is still rejected when a storage state is active. consider adding:

```typescript
it("rejects paths outside the storage state's project root when cwd would have allowed them", () => {
  const cwd = process.cwd();
  const otherRoot = path.join(path.dirname(cwd), "other-project");
  const target = path.join(cwd, "file.txt"); // allowed by cwd, not by stored root

  setStoragePathState({
    currentStoragePath: path.join(otherRoot, "accounts.json"),
    currentLegacyProjectStoragePath: null,
    currentLegacyWorktreeStoragePath: null,
    currentProjectRoot: otherRoot,
  });

  // target is within process.cwd() but the stored root is otherRoot — should throw
  // (assuming target is not inside home or tmp)
  expect(() => resolvePath(target)).toThrow(/Access denied/);
});
```

this closes the gap on the security gate: if the `isWithinDirectory(projectRoot, ...)` branch were accidentally broadened, only an acceptance test would pass silently.

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

_{Reviews (2): Last reviewed commit: "Fix storage path state save race" | Re-trigger Greptile}

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

Warning

Rate limit exceeded

@ndycode has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 11 minutes and 10 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 11 minutes and 10 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 47931848-f949-428b-933c-239c927fc225

📥 Commits

Reviewing files that changed from the base of the PR and between b9c9273 and b9d7c2f.

📒 Files selected for processing (5)

• lib/accounts.ts
• lib/storage/path-state.ts
• lib/storage/paths.ts
• test/accounts.test.ts
• test/paths.test.ts

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch audit/pr3-storage-path-correctness

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch audit/pr3-storage-path-correctness

---

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