refactor: extract storage path state

[ndycode]

Summary

• extract storage path-state bookkeeping out of lib/storage.ts into a dedicated helper module
• keep the existing storage path resolution behavior intact while continuing the storage split in small slices

What Changed

• added lib/storage/path-state.ts with the storage path state shape and its get / set helpers
• updated lib/storage.ts to consume the extracted path-state helper module while preserving current behavior

Validation

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

Risk and Rollback

• Risk level: low
• Rollback plan: revert ca9f9a6 to restore the inline storage path-state implementation

Additional Notes

• this continues the storage split after the file-path helper extraction in the same isolated worktree and stacked review flow

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

clean mechanical extraction — moves the StoragePathState type, module-level currentStorageState singleton, AsyncLocalStorage context, and getStoragePathState/setStoragePathState helpers verbatim from lib/storage.ts into a new lib/storage/path-state.ts module. behavior in lib/storage.ts is fully preserved; all call sites are unchanged.

• the comment added to getStoragePathState() correctly documents the ALS/fallback trade-off and addresses the prior review thread on concurrency
• no test file test/storage-path-state.test.ts was added despite the project's 80% coverage threshold and the convention of pairing every extracted lib/storage/ helper with a test suite — the fallback branch and ALS isolation are unexercised
• currentStorageState is a module-level singleton with no exported reset helper, creating test-isolation risk for future vitest suites that call setStoragePathState directly
• lib/AGENTS.md structure listing for lib/storage/ still shows only migrations.ts and paths.ts; path-state.ts should be added to keep the knowledge base accurate for future agents working in this repo

Confidence Score: 4/5

• safe to merge — pure extraction with no behavioral change; missing tests are a follow-up concern, not a blocker
• the diff is a pure mechanical move: all logic is identical, import/export wiring is correct, and the prior concurrency concern is now commented. the only gap is missing dedicated vitest coverage for the new module and no reset export for test isolation — both are style-level concerns that won't cause runtime regressions given existing storage.test.ts indirect coverage
• lib/storage/path-state.ts — new module with no dedicated test file and no reset export for test isolation

Important Files Changed

Filename	Overview
lib/storage/path-state.ts	new module extracting StoragePathState type, module-level singleton, ALS context, and get/set helpers from lib/storage.ts — logic is identical to what was removed; comment added to document the ALS/fallback trade-off; no dedicated test file exists for this module
lib/storage.ts	removes inline StoragePathState block and wires in the new path-state.js import; all call sites unchanged; net deletion only, no behavioral change

Sequence Diagram

sequenceDiagram
    participant caller as lib/storage.ts callers
    participant storage as lib/storage.ts
    participant ps as lib/storage/path-state.ts
    participant als as AsyncLocalStorage

    caller->>storage: setStoragePath(projectPath)
    storage->>ps: setStoragePathState(state)
    ps->>ps: currentStorageState = state
    ps->>als: enterWith(state)

    caller->>storage: getStoragePath()
    storage->>ps: getStoragePathState()
    ps->>als: getStore()
    alt ALS store exists (async chain propagated)
        als-->>ps: StoragePathState
    else ALS store undefined (new/un-propagated context)
        ps->>ps: fallback: currentStorageState
    end
    ps-->>storage: StoragePathState
    storage-->>caller: resolved path string

Loading

Prompt To Fix All With AI

This is a comment left during a code review.
Path: lib/storage/path-state.ts
Line: 1-30

Comment:
**no dedicated vitest coverage for the new module**

`lib/storage/path-state.ts` is a new isolated module with exported public API (`getStoragePathState`, `setStoragePathState`, `StoragePathState`) but there is no corresponding `test/storage-path-state.test.ts`. the project runs 2071 tests across 87 files with an 80%+ coverage threshold, and every other extracted helper under `lib/storage/` (`migrations.ts`, `paths.ts`, `file-paths.ts`, `error-hints.ts`, `identity.ts`) has a paired test file.

the most interesting behavior here — the `AsyncLocalStorage.getStore() ?? currentStorageState` fallback path — is never directly exercised. specifically, these branches lack coverage:
- `getStoragePathState()` when `getStore()` returns `undefined` (fallback to `currentStorageState`)
- `getStoragePathState()` after `setStoragePathState()` (ALS path)
- isolation between concurrent async chains via `enterWith`

because `currentStorageState` is module-level, there is also a test-isolation concern: state set in one test leaks into subsequent tests unless explicitly reset. a `resetStoragePathState()` export (or `beforeEach` reset helper) should be considered for test hygiene.

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/storage/path-state.ts
Line: 27-30

Comment:
**no reset/clear export for test isolation**

the module-level `currentStorageState` singleton persists across the entire process lifetime. callers in `lib/storage.ts` do reset it to all-nulls via `setStoragePathState({...nulls})`, but there is no exported helper to restore that initial state from tests. once `setStoragePathState` is called in a test, the fallback value is poisoned for every subsequent test that runs in the same module context. the other stateful helpers in this codebase (e.g. unified-settings write queue, rate-limit tracking) expose reset helpers or use `beforeEach` teardown.

consider exporting a reset helper:

```typescript
export function resetStoragePathState(): void {
	currentStorageState = {
		currentStoragePath: null,
		currentLegacyProjectStoragePath: null,
		currentLegacyWorktreeStoragePath: null,
		currentProjectRoot: null,
	};
}
```

this keeps test suites hermetic without depending on callers to zero out the state.

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

_{Last reviewed commit: "docs: clarify storag..."}

[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: 5ba88ad5-84bb-48f6-9a1f-9f4139cdae64

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 refactor/pr3-storage-path-state-module

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch refactor/pr3-storage-path-state-module

---

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

[ndycode]

Closing because this work is now included in main via #318 and #319.