Files

T

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

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.

2026-06-20 03:47:28 +08:00

4.7 KiB

Raw Blame History

RFC: Drop the mutable session summary

Status: implemented (proposed and accepted 2026-06-19)

Context

The session-persistence seam split a session's out-of-log metadata into two types owned by dsh-session: an immutable SessionHeader (version, id, createdAt, cwd?, parentSession?) written once at creation, and a mutable SessionSummary (updatedAt, title?, firstPrompt?) "updateable without touching the append-only log". Their union was SessionMeta = SessionHeader & SessionSummary, and the abstract SessionPersistence service carried a seventh method — update(id, summary) — for rewriting the summary. Each backend implemented the mutable store its own way: JSONL wrote a separate atomic .summary.json sidecar beside the log (temp-write + rename, best-effort), SQLite kept updated_at/title/first_prompt columns bumped inside the append transaction.

The summary was designed for a future session picker (recency ordering via updatedAt, a title/firstPrompt preview). That picker was never built. An audit of the whole repo found the entire SessionSummary surface is dead state:

SessionPersistence.update() has zero production callers (every .update( hit is createHash().update() or a test).
firstPrompt is never read anywhere in production.
title is read in the ACP bridge — but from a tool-call presenter (present.title), never from stored session metadata.
updatedAt has no consumer: the only production caller of list() reads meta.cwd (a SessionHeader field) to validate a workspace on session/load; resume reads createdAt/cwd/parentSession — all header fields.
Decisively: the live Session.header was already typed SessionHeader, not SessionMeta — the summary never existed on the live session object; it lived only in the persistence layer, written and read by nothing but its own contract test.

Decision

Delete the mutable session summary entirely. SessionSummary and the SessionMeta name are removed; the metadata a backend stores and returns is just SessionHeader. SessionPersistence.update() is removed from the abstract service and every backend. JSONL loses the whole sidecar machinery (writeSidecar/readSidecar/touchSummary/removeSidecars/sidecarPath and the load/list overlays); SQLite drops the updated_at/title/first_prompt columns and the per-append updated_at bump, and its SCHEMA_VERSION goes 1 → 2.

Anything the summary was meant to provide is derivable from the append-only log when a consumer actually needs it (firstPrompt = first user/message; recency = the last event's time or the file mtime) or already lives in the immutable header (createdAt, cwd). The one thing not derivable — a user-edited title — had no implementation and is pure YAGNI; it can return as its own log event or header field if a real feature ever needs it.

This is recorded as a decision because it is durable (it narrows a public service contract and an on-disk format across two backends), contested (the summary was a deliberate forward-looking design, not an accident), and surprising (a future reader finding SessionHeader where the original RFC describes SessionMeta would otherwise ask why the summary vanished). It also unblocks the shared persistence write coordinator: with no mutable summary, the coordinator's hook interface needs no updateSummary hook and the JSONL-sidecar-vs-SQLite-column durability divergence disappears, so the two backends' write paths converge.

No migration

This is unreleased software (see root AGENTS.md § "Pre-release stance: foundation over blast radius"), so there are no on-disk databases or logs to preserve. SQLite does not migrate a v1 database: the openDatabase guard now rejects any non-current on-disk user_version (onDisk !== 0 && onDisk !== SCHEMA_VERSION) — older or newer — so a stale v1 DB is cleanly rejected rather than half-read against the new column set. A fresh database stamps the current version; that is the only path that needs to work.

What we gave up

A future session picker now has to derive its preview/ordering from the log (or reintroduce a typed field) rather than reading a ready-made summary row. That is the correct cost: a cache for a feature that does not exist is dead weight that every backend pays to maintain and every contract test pays to assert. The principle — a passing test pins current behavior, not necessarily correct behavior; behavior can be an artifact of a past compromise — is now recorded as a standalone convention in root AGENTS.md, with this change as its worked example.

4.7 KiB Raw Blame History