refactor(contracts): extract the broker stores' shared persistence, not their model (#497)

[pofallon]

Refs #497 — deliberately not closing it. See "What this leaves open" below.

Why this isn't what #497 asked for

#497 sets the trigger for generalising the broker-state model at a third store. There are two, and since I filed it they have diverged further (#502 added retention pruning, #509/#510 made a second service depend on the restore store's locking). They are genuinely different models:

	BackupBrokerStateStore	RestoreBrokerStateStore
shape	one record per host	keyed by request id
why	a capture has one open operation at a time	several restores can be open at once
update()	bare read-modify-write	serialising lock + retention pruning
also has	—	transition (compare-and-set), get, pendingFor, retainWhileWaiting

Forcing a shared base on two data points would be guessing at the boundary, so this doesn't.

What it does instead

read() and write() are character-for-character identical between the two, apart from a log string and the type. That is extracted into a composed JsonRecordStore<T> — composition, not inheritance, so each store keeps its own update(), locking, expiry and queries, and a third store can take the mechanism without inheriting a shape it may not want.

The two policies are why this is worth centralising, and both are choices about what must not happen:

• A read fails open to {}. A missing file is the first-run case; an unreadable one must not wedge the broker. The record is bookkeeping, and refusing to proceed without it would turn a corrupt file into an outage. Each caller's expiry rule then decides what empty means — isPrepareExpired treats an unparseable timestamp as expired precisely so failing open here cannot leave a dump on disk forever.
• A write warns rather than throws. Losing the record costs bookkeeping, never the correctness of the operation a provider is mid-way through announcing.

Those were previously asserted only indirectly, via two stores that happened to implement them the same way. They now have direct tests — including the corrupt-file and failing-storage paths, which the indirect coverage never reached.

Honest accounting

This removes ~40 lines of duplicated logic and adds a 75-line module, most of it the explanation above. Raw line count goes up. The gain is that the two policies have one home and one test, not that there is less code. If that trade isn't worth it to you, this is a clean revert — the model generalisation in #497 is unaffected either way.

What this leaves open

#497 stays open for the model question, with its trigger intact: a third brokered store is still the signal to look at whether the single-record and keyed-by-id shapes want a common abstraction. This change makes that cheaper to do (the mechanism is already shared) without pre-judging it.

Verification

Server 1392 → 1397 (+5 direct tests for the extracted policies). Both stores' own suites unchanged and green (restore-broker.test.ts 22/22). Full gate green: typecheck ×2, lint, test (1397 server / 377 web / 286 CLI), build.

Also removes an orphaned private logger field the extraction left in the restore store — unused private fields don't trip the linter, so it would have sat there.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Vck5KSX2CLxhohx14nb5Sh