refactor: extract backup metadata helpers

[ndycode]

Summary

• extract backup-metadata helper functions out of lib/storage.ts into a dedicated helper module
• keep the existing backup metadata behavior intact while continuing the storage split in small slices

What Changed

• added lib/storage/backup-metadata.ts with latestValidSnapshot(...), buildMetadataSection(...), and the snapshot metadata types used by restore diagnostics
• updated lib/storage.ts to consume the extracted helpers and types while preserving current behavior
• kept the backup metadata behavior covered by 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 eb10a1f to restore the inline backup-metadata helpers

Additional Notes

• this continues the storage split after named-backup discovery 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 continues the incremental lib/storage.ts split by extracting two cohesive groups of helpers into dedicated modules: lib/storage/backup-metadata.ts (snapshot types + latestValidSnapshot / buildMetadataSection) and lib/runtime/metrics.ts (RuntimeMetrics, createRuntimeMetrics, header-parsing, and env helpers). index.ts gains consistently tab-indented, alphabetically-sorted imports in the same commit. no logic changes — this is a pure structural refactor.

• lib/storage/backup-metadata.ts exports the three snapshot types and two functions that were previously private to lib/storage.ts; storage.ts re-imports them with no behavioral change
• lib/runtime/metrics.ts additionally promotes clampRetryHintMs to an export and adds an injectable now parameter to parseRetryAfterHintMs, enabling the deterministic header-parsing tests in test/runtime-metrics.test.ts
• test/backup-metadata.test.ts covers latestValidSnapshot well (undefined return, tie-breaking, mixed-valid) but is missing a buildMetadataSection happy-path case where latestValidPath should be populated — the all-invalid path is the only buildMetadataSection test
• no windows filesystem or token safety risks introduced; no concurrency surfaces touched

Confidence Score: 5/5

• safe to merge — pure structural refactor with no logic changes and passing test/lint/typecheck/build gates
• all extracted code is logically identical to the inlined originals; the only delta is parseRetryAfterHintMs gaining an injectable now param (purely additive), and clampRetryHintMs becoming an export. the two p2 test gaps (missing buildMetadataSection happy-path, imprecise mtime-zero assertion) don't affect correctness and are follow-up improvements rather than merge blockers.
• test/backup-metadata.test.ts — missing buildMetadataSection happy-path test where latestValidPath is set

Important Files Changed

Filename	Overview
lib/storage/backup-metadata.ts	clean extraction of BackupSnapshotKind / BackupSnapshotMetadata / BackupMetadataSection types and latestValidSnapshot / buildMetadataSection helpers from lib/storage.ts; logic is unchanged, all types are now exported
lib/runtime/metrics.ts	clean extraction of RuntimeMetrics, createRuntimeMetrics, parseFailoverMode, parseEnvInt, clampRetryHintMs, parseRetryAfterHintMs, and sanitizeResponseHeadersForLog from index.ts; parseRetryAfterHintMs now accepts an injectable now param for deterministic testing
lib/storage.ts	removed inline type/function definitions and re-imports them from the new backup-metadata module; behavior is identical, public BackupMetadata export is unchanged
index.ts	import reordering (alphabetical, consistent tab indentation) and extraction of RuntimeMetrics helpers to lib/runtime/metrics.ts; no logic change, formatting is now consistent throughout
test/backup-metadata.test.ts	new dedicated unit tests for latestValidSnapshot and buildMetadataSection; covers undefined-return, tie-break, and all-invalid cases, but missing the happy-path for buildMetadataSection where latestValidPath is populated
test/runtime-metrics.test.ts	comprehensive unit tests for all extracted metrics helpers; deterministic via injected now param; covers all header formats (ms, seconds, date string, epoch seconds, epoch ms) and the sanitize allowlist

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[lib/storage.ts] -->|imports types + helpers| B[lib/storage/backup-metadata.ts]
    B -->|BackupSnapshotKind\nBackupSnapshotMetadata\nBackupMetadataSection| A
    B -->|latestValidSnapshot\nbuildMetadataSection| A

    C[index.ts] -->|imports helpers| D[lib/runtime/metrics.ts]
    D -->|RuntimeMetrics\ncreateRuntimeMetrics\nparseFailoverMode\nparseEnvInt\nparseRetryAfterHintMs\nsanitizeResponseHeadersForLog| C

    E[test/backup-metadata.test.ts] -->|unit tests| B
    F[test/runtime-metrics.test.ts] -->|unit tests| D

    style B fill:#d4edda,stroke:#28a745
    style D fill:#d4edda,stroke:#28a745
    style E fill:#cce5ff,stroke:#004085
    style F fill:#cce5ff,stroke:#004085

Loading

Prompt To Fix All With AI

This is a comment left during a code review.
Path: test/backup-metadata.test.ts
Line: 51-70

Comment:
**missing happy-path coverage for `buildMetadataSection`**

the only `buildMetadataSection` test covers the all-invalid case (`latestValidPath: undefined`). the branch where at least one valid snapshot exists — and `latestValidPath` is populated from `latestValidSnapshot` — is never exercised directly. given the project's 80% coverage threshold and the fact that `storage.test.ts` doesn't call `getBackupMetadata` or `buildMetadataSection` directly, this branch will likely be missed.

suggest adding:

```typescript
it("sets latestValidPath to the most recent valid snapshot", () => {
	const older = createSnapshot({ path: "/tmp/older.bak", mtimeMs: 100 });
	const newer = createSnapshot({
		path: "/tmp/newer.bak",
		kind: "accounts-backup-history",
		mtimeMs: 200,
	});
	const invalid = createSnapshot({ path: "/tmp/bad.bak", valid: false, mtimeMs: 9999 });

	expect(
		buildMetadataSection("/tmp/openai-codex-accounts.json", [older, newer, invalid]),
	).toEqual({
		storagePath: "/tmp/openai-codex-accounts.json",
		latestValidPath: "/tmp/newer.bak",
		snapshotCount: 3,
		validSnapshotCount: 2,
		snapshots: [older, newer, invalid],
	});
});
```

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/backup-metadata.test.ts
Line: 41-49

Comment:
**tie-breaking test doesn't prove the "zero" semantic**

the test name says "treats missing mtimes as zero when choosing the latest valid snapshot", but both snapshots have `undefined` `mtimeMs` so the sort key is `0` for both — the assertion just confirms stable-sort order, not that zero-mtime snapshots lose to a snapshot with `mtimeMs: 1`.

a more meaningful version would include a third snapshot with a positive `mtimeMs` and assert it wins:

```typescript
it("treats missing mtimes as zero when choosing the latest valid snapshot", () => {
	const noMtime = createSnapshot({ path: "/tmp/no-mtime.bak" });
	const hasMtime = createSnapshot({
		path: "/tmp/has-mtime.bak",
		kind: "accounts-discovered-backup",
		mtimeMs: 1,
	});
	expect(latestValidSnapshot([noMtime, hasMtime])).toEqual(hasMtime);
});
```

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

_{Reviews (2): Last reviewed commit: "test: cover extracted backup metadata he..." | 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 20 minutes and 16 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: f65a1184-f5c5-41d9-88f7-b1f74dba3038

📥 Commits

Reviewing files that changed from the base of the PR and between 45cda79 and 8181ecc.

📒 Files selected for processing (6)

• index.ts
• lib/runtime/metrics.ts
• lib/storage.ts
• lib/storage/backup-metadata.ts
• test/backup-metadata.test.ts
• test/runtime-metrics.test.ts

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch refactor/pr3-storage-backup-metadata-helpers

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch refactor/pr3-storage-backup-metadata-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.