fix(windows): resolve DB path isolation, WebSocket fatal classification, and OCR backfill lag

[thesohamdatta]

Summary

• DB Path Isolation: Resolved path resolution in getReadonly() to correctly honor OMI_DB_PATH. Added public interface integration tests verifying the path.
• WebSocket Fatal Flag: Classifies WebSocket error fatal status correctly by tracking session connection completion state, preventing mid-session connection drops.
• OCR Backfill Optimization: Ported macOS backfill pacing policy including async readFile, powerMonitor battery gating, and 100ms inter-frame throttling.
• Shortcut Test Fix: Mocked electron in shortcut.test.ts to resolve pre-existing vitest test runner loading issues.

Test Plan

• Run
  pm test\ inside \desktop/windows\ to verify that all 517 tests pass green.
• Run \db.test.ts\ to verify that setting/unsetting OMI_DB_PATH correctly redirects SQLite files at runtime.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

[cubic-dev-ai]

No issues found across 6 files

Confidence score: 5/5

• Automated review surfaced no issues in the provided summaries.
• No files require special attention.

_{Re-trigger cubic}

[Git-on-my-level]

Thanks @thesohamdatta — nice set of Windows bug fixes here. Static review of all five changed files looks solid. A few notes before this lands:

What looks good

• getReadonly() now correctly honors OMI_DB_PATH, matching the writable get() path. Previously it was hardcoded to userData/omi.db, so the bench harness's throwaway DB was only half-respected (writes went to the temp DB, reads went to the real user DB). Good catch, and the new db.test.ts covers both branches.
• ocrService.ts porting the macOS pacing policy (async readFile, powerMonitor battery gating, 100ms inter-frame throttle, batch size 10) is a faithful, well-commented port. The readFileSync → await readFile swap is a real event-loop improvement.
• The shortcut.test.ts electron mock fixes a pre-existing test-runner load failure cleanly.

Wants a closer look before merge

• WebSocket fatal reclassification is an untested behavior change. Switching fatal from ws.readyState !== WebSocket.OPEN to !session.connected changes what the renderer treats as recoverable vs. fatal. The new connected flag is only ever set true in ws.on('open'), so any error after the handshake that fires while readyState is CLOSING/CLOSED would now be classified non-fatal. That's plausibly correct, but there's no test covering the error path, and the change spans connection-lifecycle semantics. A test (even a unit test with a mocked ws) for the fatal/non-fatal split would help a lot here.
• No CI beyond the cubic bot runs on this repo path, so the "517 tests pass" claim in the PR body can't be independently verified from checks. If you can confirm which test command was run (and that it includes db.test.ts and shortcut.test.ts), that'd help.
• Scope / changelog noise. The desktop/macos/CHANGELOG.json reformatting is ~1200 of the ~1370 changed lines and isn't functionally related to the Windows fixes. It bloats the diff and makes review/rollback harder. Consider dropping the CHANGELOG churn (or splitting it) so the real signal — the four Windows fixes — stands out.

Labels added: needs-maintainer-review, needs-tests, needs-scope-reduction.

Not approving formally yet — this is a first-time contribution touching WebSocket connection lifecycle and DB path resolution, and the bundled CHANGELOG reformatting inflates the diff. Leaving for a maintainer to make the final call. Appreciate the work on these.

[thesohamdatta]

Addressed all three points.

WebSocket fatal test. Pulled classifyWsFatal(connected: boolean) out as an exported function in omiListen.ts so it's testable without standing up a WebSocket. New omiListen.test.ts covers both cases: error before open is fatal, error after open is not. Behavior is unchanged from the inline !session.connected — just named and verifiable now.

Test command. npm test runs vitest run across src/**/*.test.ts. On this machine: 519 passed, 0 failed, 3 skipped. db.test.ts and shortcut.test.ts are both in that run and pass.

Worth flagging: the postinstall step compiles better-sqlite3 against Electron's Node. On a fresh clone, npm test will fail on db.test.ts with a native module version mismatch until you run npm rebuild better-sqlite3 first. This predates the PR, but it'll catch contributors who don't know to do that step.

CHANGELOG. Reverted entirely. Diff is now 4 files, all under desktop/windows/.

LMK if anything broke

[Git-on-my-level]

Thanks @thesohamdatta — all three points are cleanly addressed:

• WebSocket fatal test: Exporting classifyWsFatal and covering both branches (pre-connect fatal, post-connect non-fatal) is exactly right — behavior preserved and now verifiable without standing up a WebSocket.
• Scope: Dropping the CHANGELOG churn makes the real signal stand out; all changed files are now under desktop/windows/.
• Test command / native rebuild: Appreciate the honest note on the better-sqlite3 rebuild step — that's genuine contributor friction worth tracking separately.

Static review of the changed files looks solid: getReadonly() now honors OMI_DB_PATH consistently with the writable path, the OCR backfill pacing faithfully mirrors macOS (async readFile, battery gating, 100ms throttle, batch 10), and the shortcut test mock resolves the pre-existing load failure.

Leaving this for a maintainer to give the final approval — it's a first-time contribution touching WebSocket connection lifecycle and DB path resolution, and the only check on this path is the cubic bot, so I'd want a human sign-off before merge. The work here is good.