Coder/deepseek-harness
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

refactor(session-persistence): extract a shared write coordinator

Browse Source 
The JSONL and SQLite backends were byte-identical (or same-algorithm) for ALL
of their write-path orchestration — the four maps (states/buffers/chains/inits),
installWritePath, initFor, onCreated's four adoption cases, flush, drain,
serialize, adopt/adoptLivePrefix, assertVersion, and the create/append/load/
has/delete skeletons. Only the storage primitives (write bytes vs INSERT rows)
differed, so every fix landed twice.

Extract that orchestration into a PersistenceCoordinator in the seam package.
Each backend composes one (new PersistenceCoordinator(ctx, this)), implements a
small PersistenceBackend hook interface (loadStored, loadLive, appendBatch,
commitRepair, deleteStored, list, optional close), and delegates its six public
service methods to it. Composition, not inheritance — a backend exposes only the
hooks, can't reach the coordinator's private state, and the public
SessionPersistence API is unchanged so a third-party backend may still implement
it directly.

The crash-repair torn-tail token is OPAQUE: the coordinator computes the
synthetic closers (it owns interruptedTurnClosers) but only tests
`tornMarker !== undefined` and round-trips it to commitRepair, never inspecting
it (JSONL = byte offset, SQLite = seq). loadStored vs loadLive stay distinct so
HMR adoption is cwd-scoped (a same-id log at a different cwd is a collision, not
a resume). appendBatch carries meta so lazy-materialize + first-batch commit
atomically (no separate materialize hook).

Tests: the duplicated orchestration tests (adoption, HMR, collision,
dispose-drain, crash-tail) move into one runCoordinatorContract suite run once
per backend (memory + jsonl + sqlite) via hook fixtures; per-backend specs keep
only storage mechanics. A through-coordinator torn-tail test per real backend
keeps the commitRepair-with-marker branch covered under the 100% gate.

Net -112 lines (the dedup outweighs the new coordinator + shared suite); 100%
coverage; backends shrank ~1200 lines of duplicated churn. Migrates the
write-coordinator RFC proposed -> implemented.
This commit is contained in:
Tianyi Cui
2026-06-20 03:47:28 +08:00
parent 31af23b4fe
commit ab02e9acec
13 changed files with 1879 additions and 1991 deletions
Download Patch File Download Diff File Expand all files Collapse all files

24
docs/rfc/proposed/2026-06-18-shared-persistence-write-coordinator.md
View File

@@ -1,24 +0,0 @@
# RFC: Shared persistence write coordinator
Status: proposed
## Problem
`dsh-session-persistence-jsonl` and `dsh-session-persistence-sqlite` intentionally prove the same `SessionPersistence` contract over different storage media, but their write-path orchestration is now duplicated: per-session state, `session/created` adoption, backend-specific prefix reads, write-behind buffers, serialized flush chains, HMR seeding, and dispose drains. The pure seed-prefix collision and serializability guards have already moved into the seam package; the remaining orchestration is still correctness-heavy and already receives the same fixes twice.
## Proposal
Extract a backend-agnostic coordinator into `dsh-session-persistence`. The coordinator owns live-session adoption, buffering, cursor filtering, per-id serialization, and disposal quiescence. Concrete backends provide small hooks for durable operations: create lazy state, find/load stored prefix, append a contiguous batch, delete, and list.
The public `SessionPersistence` service shape can stay the same. The coordinator can be an internal exported helper or protected base class used by first-party backends; third-party backends may still implement the abstract service directly if their write path is different.
## Acceptance Criteria
- JSONL and SQLite keep passing the existing shared `runPersistenceContract`.
- HMR/adoption/collision tests move to a shared coordinator test suite and run once for each backend through hook-driven fixtures.
- Backend-specific tests focus on storage mechanics only: JSONL path safety/fsync behavior and SQLite schema/WAL/transaction behavior.
- A future backend does not need to copy the current `session/event` → buffer → flush orchestration.
## Risks
The current duplication is verbose but explicit. A coordinator must not hide storage-specific durability semantics or make unusual backends fight an inheritance hierarchy. Prefer narrow hooks and contract tests over a large framework.