From 31af23b4fe22d4288067dcf8eba12aa2c93a1c90 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Sat, 20 Jun 2026 01:30:33 +0800 Subject: [PATCH] docs(session): fix stale sidecar/migration references (Codex review) Codex's converge pass on PR A flagged three now-false references the deletion left behind: - the proposed write-coordinator RFC still listed an "update summary" backend hook and "sidecar behavior" in its test focus; - the JSONL README's format-version note still said a format change needs a "version bump + migration" (contradicting the no-migration pre-release stance); - a stale "sidecar pathing" comment in findLog's cwd-recovery branch. All three corrected to current truth. --- .../2026-06-18-shared-persistence-write-coordinator.md | 4 ++-- packages/session-persistence-jsonl/README.md | 2 +- packages/session-persistence-jsonl/src/index.ts | 3 ++- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/docs/rfc/proposed/2026-06-18-shared-persistence-write-coordinator.md b/docs/rfc/proposed/2026-06-18-shared-persistence-write-coordinator.md index 06bbe1a93d..fbb7defcd0 100644 --- a/docs/rfc/proposed/2026-06-18-shared-persistence-write-coordinator.md +++ b/docs/rfc/proposed/2026-06-18-shared-persistence-write-coordinator.md @@ -8,7 +8,7 @@ Status: proposed ## 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, update summary, delete, and list. +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. @@ -16,7 +16,7 @@ The public `SessionPersistence` service shape can stay the same. The coordinator - 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/sidecar behavior and SQLite schema/WAL/transaction behavior. +- 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 diff --git a/packages/session-persistence-jsonl/README.md b/packages/session-persistence-jsonl/README.md index c012886099..640f132e78 100644 --- a/packages/session-persistence-jsonl/README.md +++ b/packages/session-persistence-jsonl/README.md @@ -25,7 +25,7 @@ The JSONL durable session-persistence backend — a concrete `SessionPersistence - **Append-only.** Committed events (at or below a flushed `turn/end`) are never rewritten. Subsequent appends are line appends at EOF + `fsync`. - **Crash recovery — close, don't truncate.** A crash can leave a log whose final turn never closed (real events after the last `turn/end`). `load` PRESERVES those events (a turn can be huge — they are real work) and closes the orphaned turn by durably appending synthetic boundary events: an error `tool/result` for every `tool-call` the crash left unanswered (the loop logs the assistant message before running the tools, so a mid-tool crash leaves dangling calls — and `deriveMessages()` would replay an assistant tool-call with no result, which providers reject), then a `step/end` if a step was open, then `turn/end {kind:'interrupted'}`, returning a balanced log. Only a never-fully-written **torn tail fragment** (a final line with no newline / unparseable) is `ftruncate`d away before the closers are written. See [session persistence](../../docs/rfc/implemented/2026-06-14-session-persistence.md). - **Contiguous-seq.** `load` rejects a mid-log parse error or `seq` gap (unloadable); `append` rejects a batch whose first `seq` does not continue the stored log, and rejects non-JSON-serializable `event.data` naming the offending event type. -- **Format version.** Only v1 is supported; `load` rejects an unknown version. A future format change requires a version bump + migration. +- **Format version.** Only v1 is supported; `load` rejects an unknown version. While the harness is unreleased a format change bumps the version and rejects non-current logs — there is no migration (no persisted user data to preserve). ## Write path diff --git a/packages/session-persistence-jsonl/src/index.ts b/packages/session-persistence-jsonl/src/index.ts index 110569d3e3..ef921f2b8f 100644 --- a/packages/session-persistence-jsonl/src/index.ts +++ b/packages/session-persistence-jsonl/src/index.ts @@ -490,7 +490,8 @@ export class SessionPersistenceJsonl extends SessionPersistence { for (const dir of await this.listCwdDirs()) { const path = `${dir}/${target}` if (await this.exists(path)) { - // Recover cwd from the header for accurate sidecar pathing. + // Recover the cwd from the header so the caller has the session's + // bucket location (which `findLog` was given an unknown cwd for). const { meta } = scanLog(await readFile(path)) return { path, cwd: meta.cwd } }