fix(storage): flush pending debounced writes on unmount instead of discarding them

[Matthew-Selvam]

WARNING CORE_CAPABILITIES

Summary

The useStorageSync unmount cleanup cleared the debounce timer without flushing, and pendingCollectionsRef — a per-mount useRef — was dropped with the component. An edit made within the 500ms debounce window of navigating away existed only in localStorage; the next mount's pullFromServer() then unconditionally overwrote localStorage with server data, destroying the write permanently and silently.

Fix

The cleanup now flushes whatever is pending before tearing down:

• Guarded on serverModeRef — in local mode the queue is always empty and the PUT would be pointless work.
• Guarded on a non-empty pending set — no spurious push when nothing is queued.
• A failed post-teardown flush still re-queues its collections (existing behavior), and the existing !mountedRef.current guard already keeps it from arming stray retry timers — so the previous fix for the dead-hook retry loop is unaffected.

Test plan

• New: a pending debounced write is flushed on unmount, not discarded — edit dispatched inside the debounce window, unmount, assert the PUT lands
• New: unmount with nothing pending does not push — asserts the flush is conditional
• Existing a push that fails after unmount does not leave a timer running still passes (in-flight push ≠ pending push; no double-send)

Found during a broader code review of the storage sync layer; no issue existed yet for it.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[cevheri]

Reproduced the loss end to end in a real browser against live PostgreSQL, and it is as bad as you describe. With a history row already on the server, adding an entry and clicking Monitoring inside the debounce window leaves main with the old server row and, after the remount's pull, the new entry is gone from localStorage too. Permanent and silent. With this patch the entry lands. Reverting only the hook reddens the flush test, so that one is pinning real behaviour. The ordering is right as well: the flush starts its fetch synchronously before mountedRef goes false, so the failure branch still sees an unmounted hook and arms no stray timer.

One thing to fix: the second test cannot fail. I replaced the whole condition with if (true), removing both guards, and all 32 tests stayed green. Both properties are structural rather than guarded: flushPending returns before any fetch when the queue is empty, and in local mode the queue is never non-empty because the listener effect returns early on !isServerMode. So "unmount with nothing pending does not push" asserts something the code cannot violate, which reads as protection that is not there. Please drop it. Keeping the guards themselves is fine, they document the intent cheaply.

One limit worth stating in the description: this covers navigation inside the app, where React unmounts the hook and the process keeps running. It does not cover leaving the page itself. If the user closes the tab, hits reload, or types a different address within the debounce window, the browser tears the document down without running the React cleanup, no PUT is sent, and the next load's pull overwrites the edit exactly as before. I measured that case and it still loses the write. Covering it needs keepalive or sendBeacon, which is a separate change, so nothing to do here. But "navigating away" in the description reads as if both cases were fixed, and only one is.