refactor: extract storage file path helpers

[ndycode]

Summary

• extract storage file-path helper functions out of lib/storage.ts into a dedicated helper module
• keep the existing backup, wal, reset-marker, and flagged-path behavior intact while continuing the storage split in small slices

What Changed

• added lib/storage/file-paths.ts with the backup-path, wal-path, reset-marker, and flagged-storage path helpers
• updated lib/storage.ts to import the extracted helpers and keep the public getFlaggedAccountsPath() surface intact
• preserved the existing storage behavior and coverage through the storage test suite

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 21aaa89 to restore the inline storage file-path helper implementation

Additional Notes

• this continues the storage split after the error-hint and identity-helper extractions 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

this pr extracts the storage file-path helper functions (getAccountsBackupPath, getAccountsWalPath, getIntentionalResetMarkerPath, getAccountsBackupRecoveryCandidates, getFlaggedAccountsPath) and the three suffix constants (ACCOUNTS_BACKUP_SUFFIX, ACCOUNTS_WAL_SUFFIX, RESET_MARKER_SUFFIX) out of lib/storage.ts into the new lib/storage/file-paths.ts module, continuing the incremental storage split. the previously flagged duplicate-constants risk is resolved — storage.ts now imports from file-paths.ts and the local copies are deleted, making file-paths.ts the single source of truth.

• lib/storage/file-paths.ts: new dedicated helper module; all path and suffix constants exported from here; ACCOUNTS_BACKUP_HISTORY_DEPTH correctly kept private
• lib/storage.ts: local constant and function definitions removed; public getFlaggedAccountsPath() surface preserved via a thin wrapper that delegates to the extracted helper
• test/storage-file-paths.test.ts: new vitest suite covering posix paths, backup candidate ordering, and flagged-path construction; windows-path test is tautological on linux ci (both implementation and assertion use the same node:path call) — consider using path.win32 for a meaningful cross-platform check

Confidence Score: 5/5

• safe to merge — clean refactor with no behavior change and the previous duplicate-constants concern fully resolved
• the duplicate-constants issue from the prior review is fully addressed; file-paths.ts is the sole source of truth for suffixes and path helpers; storage.ts public api is intact; the only remaining item is a non-blocking p2 on a tautological windows-path test that doesn't affect runtime correctness
• no files require special attention

Important Files Changed

Filename	Overview
lib/storage/file-paths.ts	new helper module extracting backup-path, wal-path, reset-marker, and flagged-storage path helpers; constants and logic are now the single source of truth; thin getFlaggedAccountsPath wrapper is slightly redundant but harmless.
lib/storage.ts	imports extracted helpers and removes the now-duplicate constants and inline functions; public api surface (getFlaggedAccountsPath) is preserved; all usages of ACCOUNTS_BACKUP_SUFFIX, ACCOUNTS_WAL_SUFFIX, and RESET_MARKER_SUFFIX now draw from the single source in file-paths.ts.
test/storage-file-paths.test.ts	good coverage of posix paths and backup candidate ordering; the windows-path test is tautological on linux ci — both the implementation and the assertion compute join(dirname(...), fileName) with the same node:path module, so it passes vacuously without verifying actual windows sibling-path resolution.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["lib/storage.ts\n(public API)"]
    B["lib/storage/file-paths.ts\n(new helper module)"]
    C["ACCOUNTS_BACKUP_SUFFIX\nACCOUNTS_WAL_SUFFIX\nRESET_MARKER_SUFFIX\n(single source of truth)"]
    D["getAccountsBackupPath()\ngetAccountsWalPath()\ngetIntentionalResetMarkerPath()\ngetAccountsBackupRecoveryCandidates()\ngetFlaggedAccountsPath()"]
    E["storage.ts callers:\ngetAccountsBackupRecoveryCandidatesWithDiscovery()\nisRotatingBackupTempArtifact()\ngetFlaggedAccountsPath()\ngetLegacyFlaggedAccountsPath()"]

    A -->|"imports (named re-export)"| B
    B --> C
    B --> D
    A --> E
    E -->|"uses imported helpers + constants"| B

Loading

Prompt To Fix All With AI

This is a comment left during a code review.
Path: test/storage-file-paths.test.ts
Line: 48-56

Comment:
**tautological windows-path test**

the assertion mirrors the implementation identically — both sides call `join(dirname(storagePath), fileName)` via the same `node:path` import. on linux ci, `dirname('C:\\Users\\user\\.codex\\openai-codex-accounts.json')` returns `'.'` (backslashes aren't posix separators), so both sides produce `'openai-codex-blocked-accounts.json'` — a relative path — and the test passes vacuously without verifying correct windows sibling-path resolution.

to test windows semantics cross-platform, assert against a hard-coded expected value computed with `path.win32`:

```ts
import { join as joinWin32, dirname as dirnameWin32 } from "node:path/win32";

it("builds sibling paths for windows-style storage paths", () => {
  const storagePath = String.raw`C:\Users\user\.codex\openai-codex-accounts.json`;
  const fileName = "openai-codex-blocked-accounts.json";

  // use path.win32 explicitly so the assertion is meaningful on posix ci too
  expect(getFlaggedAccountsPath(storagePath, fileName)).toBe(
    joinWin32(dirnameWin32(storagePath), fileName),
  );
});
```

note: this also means the implementation in `file-paths.ts` should use `path.win32` (or `path.normalize`) for windows paths if cross-platform correctness is required on the caller side.

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

_{Reviews (3): Last reviewed commit: "test: cover storage file path helpers" | 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 25 minutes and 25 seconds before requesting another review.

⌛ 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: 81a438db-6787-408d-9807-fceaa3728245

📥 Commits

Reviewing files that changed from the base of the PR and between 39bf72a and b191492.

📒 Files selected for processing (3)

• lib/storage.ts
• lib/storage/file-paths.ts
• test/storage-file-paths.test.ts

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch refactor/pr3-storage-path-state-helpers

✨ Simplify code

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

---

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.