fix(session-persistence): address preparation review feedback

packages/session-persistence/session-persistence/README.i18n.yaml

@@ -2,5 +2,5 @@
 # side as of the last confirmed-consistent state. Both languages carry equal authority;
 # after editing either side, bring the other along and re-record with:
 #   pnpm run verify-translation-pairing --write packages/session-persistence/session-persistence/README.md
-README.md: 4c00c28184c0046d5507270980822529bfe3bc08
-README.zh.md: 32461cec5efd279a86bf3af40b23e7e54d33a89e
+README.md: 675f59cebe338c7bbf9bed4436db26b4593ee473
+README.zh.md: ff6589c0f29e1a634b27a4118878dc7b503c9e98

packages/session-persistence/session-persistence/README.md

@@ -15,7 +15,7 @@ The persisted unit IS the existing `SessionEvent` (event-sourced model — the l
 | `append(id, events): Promise<void>` | Durably persist a batch. Append-only; first event `seq` == stored next-seq after any repair; rejects non-JSON-serializable data naming the offending type. |
 | `prepare(id, signal?): Promise<SessionPreparation>` | Reserve the exact unpublished Session used by resume. A coordinator reuses an earlier inspection when available, commits pending recovery, and releases an unpublished reservation back to its bounded cache on disposal. |
 | `load(id): Promise<{ meta; events }>` | Return an immutable balanced logical log after supported same-version shape upgrades and commit cold recovery. A live load first flushes its snapshot and rejects while its turn is open; a cold load preserves an interrupted final turn and durably closes it with synthetic `tool/result`/`step/end?`/`turn/end {interrupted}` events. Only a torn tail fragment is dropped; committed corruption, malformed shapes, and unknown `version` reject. |
-| `inspect(id, signal?): Promise<{ meta; events }>` | Return the same upgraded, validated, deeply frozen logical view without committing recovery or publishing a Session. Synthetic recovery closers exist only in memory, the physical torn tail remains untouched, and coordinator-backed implementations retain the exact unpublished Session in a bounded LRU for later `prepare`. Same-id inspections share an in-flight read. |
+| `inspect(id, signal?): Promise<{ meta; events }>` | Return an upgraded, validated, deeply frozen logical view without committing recovery or publishing a Session. A cold view receives in-memory synthetic recovery closers while its physical torn tail remains untouched; an already-live view is its current immutable snapshot and may contain an open turn. Coordinator-backed implementations retain the exact cold unpublished Session in a bounded LRU for later `prepare`. Same-id inspections share an in-flight read. |
 | `readFrom(id, fromSeq, signal?): Promise<{ meta; events }>` | The detached physical-suffix primitive: return valid stored events with `seq >= fromSeq` without preparation caching, truncation, closers, or coordinator state. A `fromSeq` at or past the stored end returns an empty event list; a negative or non-safe-integer `fromSeq` rejects. Seek-capable backends (SQLite) read only the suffix unless a supported old shape requires prefix context for normalization; sequential backends (JSONL) parse the whole artifact and skip forward. Intended for checkpoint consumers that fold only the tail past a watermark. |
 | `list(signal?): Promise<SessionHeader[]>` | Lightweight listing from metadata, no full-log parse. The optional signal cancels backend listing work. A zero-event lazily-materialized session is absent from `list`. |
 | `listSnapshots(signal?): Promise<SessionPersistenceSnapshot[]>` | Lightweight metadata plus an opaque branded per-log revision, without loading event logs. A revision stays equal while that log and its backing store are unchanged, changes after append or mutating load repair, and cannot collide solely because two stores use the same local counter. The optional signal requests cancellation of backend discovery work; first-party backends settle any started listing work before rejecting so an awaited call is quiescent. |

packages/session-persistence/session-persistence/README.zh.md

@@ -15,7 +15,7 @@
 | `append(id, events): Promise<void>` | 持久保存一个批次。仅追加；任何修复后，第一个事件 `seq` == 已存储 next-seq；非 JSON 可序列化数据会被拒绝，并命名违规类型。 |
 | `prepare(id, signal?): Promise<SessionPreparation>` | 预留恢复使用的精确未发布 Session。协调器会尽可能复用之前的检查结果、提交待处理恢复，并在 dispose 时将未发布 reservation 释放回有界缓存。 |
 | `load(id): Promise<{ meta; events }>` | 在升级受支持的同版本形状后返回不可变、平衡的逻辑日志，并提交冷恢复。实时 load 先 flush 其快照，并在轮次开放时拒绝；冷 load 保留中断的最终轮次，并用合成 `tool/result`/`step/end?`/`turn/end {interrupted}` 事件持久关闭它。只丢弃撕裂尾部碎片；已提交损坏、格式错误的形状和未知 `version` 会被拒绝。 |
-| `inspect(id, signal?): Promise<{ meta; events }>` | 返回同一份已经升级、验证和深度冻结的逻辑视图，但不提交恢复或发布 Session。合成恢复 closer 只存在于内存，物理撕裂尾部保持不变，基于协调器的实现会在有界 LRU 中保留精确的未发布 Session，供后续 `prepare` 使用。同 id 检查共享进行中的读取。 |
+| `inspect(id, signal?): Promise<{ meta; events }>` | 返回已经升级、验证和深度冻结的逻辑视图，但不提交恢复或发布 Session。冷视图会获得仅存在于内存的合成恢复 closer，物理撕裂尾部保持不变；已经实时存在的视图则是当前不可变快照，可能包含打开的 turn。基于协调器的实现会在有界 LRU 中保留精确的冷未发布 Session，供后续 `prepare` 使用。同 id 检查共享进行中的读取。 |
 | `readFrom(id, fromSeq, signal?): Promise<{ meta; events }>` | 脱离的物理后缀原语：返回 `seq >= fromSeq` 的有效已存储事件，不进入 preparation 缓存、不截断、不合成 closer，也不发布协调器状态。`fromSeq` 达到或超过已存储末尾时返回空事件列表；负数或非安全整数 `fromSeq` 会被拒绝。可寻址后端（SQLite）只读后缀，除非受支持的旧形状需要前缀上下文才能完成规范化；顺序后端（JSONL）解析整个产物并向前跳过。用于只续折水位之后尾部的 checkpoint 消费方。 |
 | `list(signal?): Promise<SessionHeader[]>` | 从元数据轻量列出，不解析完整日志。可选信号取消后端列表工作。零事件延迟实体化会话不在 `list` 中。 |
 | `listSnapshots(signal?): Promise<SessionPersistenceSnapshot[]>` | 返回轻量元数据和每份日志一个不透明、带品牌类型的修订值，不加载事件日志。日志及其后端存储不变时，修订保持相等；append 或变更性 load 修复后会改变；不会仅因两个存储使用相同本地计数器而冲突。可选信号请求取消后端发现工作；第一方后端会先等待所有已启动的列出工作结束，再予以拒绝，因此调用返回拒绝时，相关工作已完全停稳。 |

packages/session-persistence/session-persistence/src/coordinator.ts

@@ -22,6 +22,18 @@ import type { SessionPreparationReservation } from './preparations.ts'
 /** Default number of detached session preparations retained by a coordinator. */
 export const DEFAULT_PREPARED_SESSION_CACHE_SIZE = 5
 
+/** Durable session contents failed validation after a successful backend read. */
+export class SessionPersistenceCorruptionError extends Error {
+  /**
+   * @param message - stable corruption context.
+   * @param options - original validation failure.
+   */
+  constructor(message: string, options: ErrorOptions) {
+    super(message, options)
+    this.name = 'SessionPersistenceCorruptionError'
+  }
+}
+
 /** Coordinator policy supplied by a concrete persistence backend. */
 export interface PersistenceCoordinatorOptions {
   /** Maximum completed unpublished preparations retained for reuse. */
@@ -623,7 +635,7 @@ export class PersistenceCoordinator<TornMarker = unknown> {
       }
       const reservation = await this.preparations.reserve(
         id,
-        () => this.serialize(id, () => this.prepareCore(id, signal), signal),
+        () => this.serialize(id, () => this.prepareCore(id)),
         source => this.serialize(id, () => this.commitPrepared(source), signal),
         signal,
       )
@@ -674,16 +686,17 @@ export class PersistenceCoordinator<TornMarker = unknown> {
    * Inspect a logical session without publishing it or committing recovery.
    * @param id - persisted session to inspect.
    * @param signal - optional cancellation for preparation work.
-   * @returns immutable prepared metadata and balanced events.
+   * @returns immutable prepared metadata and events; a live view may have an open turn.
    */
   async inspect(id: SessionId, signal?: AbortSignal): Promise<SessionInspection> {
-    await this.waitForRetirement(id, signal)
+    signal?.throwIfAborted()
+    if (this.retirements.has(id)) await this.waitForRetirement(id, signal)
     const live = this.ctx.sessions.get(id)
     if (live !== undefined) return this.inspectLive(live)
     try {
       const source = await this.preparations.inspect(
         id,
-        () => this.serialize(id, () => this.prepareCore(id, signal), signal),
+        () => this.serialize(id, () => this.prepareCore(id)),
         signal,
       )
       const attached = this.ctx.sessions.get(id)
@@ -776,29 +789,36 @@ export class PersistenceCoordinator<TornMarker = unknown> {
     }
     signal?.throwIfAborted()
     if (stored === undefined) throw new Error(`session "${id}" not found`)
-    const { meta, events, tornMarker } = stored
-    this.assertStoredId(id, meta)
-    this.assertVersion(meta)
-    const storedEvents = adoptStoredEvents(events, id)
+    try {
+      const { meta, events, tornMarker } = stored
+      this.assertStoredId(id, meta)
+      this.assertVersion(meta)
+      const storedEvents = adoptStoredEvents(events, id)
 
-    // Preserve complete interrupted events and synthesize only missing closers.
-    const closers = interruptedTurnClosers(storedEvents).map(adoptSessionEvent)
-    const balanced = [...storedEvents, ...closers]
-    const session = this.ctx.sessions.prepare(id, {
-      seed: balanced,
-      meta,
-      seedSource: 'persistence',
-    })
-    const inspection: SessionInspection = Object.freeze({
-      meta: session.header,
-      events: Object.freeze(balanced),
-    })
-    return {
-      inspection,
-      session,
-      sessionLength: session.events.length,
-      tornMarker,
-      closers,
+      // Preserve complete interrupted events and synthesize only missing closers.
+      const closers = interruptedTurnClosers(storedEvents).map(adoptSessionEvent)
+      const balanced = [...storedEvents, ...closers]
+      const session = this.ctx.sessions.prepare(id, {
+        seed: balanced,
+        meta,
+        seedSource: 'persistence',
+      })
+      const inspection: SessionInspection = Object.freeze({
+        meta: session.header,
+        events: Object.freeze(balanced),
+      })
+      return {
+        inspection,
+        session,
+        sessionLength: session.events.length,
+        tornMarker,
+        closers,
+      }
+    } catch (error: unknown) {
+      throw new SessionPersistenceCorruptionError(
+        `stored session "${id}" failed validation: ${String(error)}`,
+        { cause: error },
+      )
     }
   }
 

packages/session-persistence/session-persistence/src/index.ts

@@ -31,7 +31,11 @@ export interface SessionInspection {
 }
 
 // The backend-agnostic write-path orchestration first-party backends compose.
-export { DEFAULT_PREPARED_SESSION_CACHE_SIZE, PersistenceCoordinator } from './coordinator.ts'
+export {
+  DEFAULT_PREPARED_SESSION_CACHE_SIZE,
+  PersistenceCoordinator,
+  SessionPersistenceCorruptionError,
+} from './coordinator.ts'
 export type {
   PersistenceBackend,
   PersistenceCoordinatorOptions,
@@ -134,14 +138,17 @@ export abstract class SessionPersistence extends Service {
   abstract load(id: SessionId): Promise<SessionInspection>
 
   /**
-   * Inspect an immutable balanced logical session without committing recovery
-   * or publishing it. A complete interrupted turn receives synthetic closers
-   * in memory and a torn physical tail remains untouched. Coordinator-backed
-   * implementations retain the exact unpublished Session for bounded reuse by
-   * a later {@link prepare}; callers borrow only its immutable header and log.
+   * Inspect an immutable logical session without committing recovery or
+   * publishing it. A cold complete interrupted turn receives synthetic closers
+   * in memory and a torn physical tail remains untouched. An already-live
+   * Session instead yields its current immutable snapshot, which may contain an
+   * open turn and its `session/end-seed` boundary. Coordinator-backed
+   * implementations retain the exact cold unpublished Session for bounded
+   * reuse by a later {@link prepare}; callers borrow only its immutable header
+   * and log.
    * @param id - the persisted session to inspect.
    * @param signal - optional cancellation for queued and backend read work.
-   * @returns the validated header and balanced logical event log.
+   * @returns the validated header and current logical event log.
    */
   abstract inspect(id: SessionId, signal?: AbortSignal): Promise<SessionInspection>
 

packages/session-persistence/session-persistence/src/preparations.ts

@@ -55,8 +55,8 @@ export class SessionPreparations<Source extends PreparedSource, CommitState> {
     load: () => Promise<Source>,
     signal?: AbortSignal,
   ): Promise<Source> {
-    const { entry, created } = this.entryFor(id, load)
-    const loaded = signal === undefined || created
+    const entry = this.entryFor(id, load)
+    const loaded = signal === undefined
       ? await entry.result
       : await observeQueuedAbort(entry.result, signal)
     const source = entry.source ?? loaded
@@ -78,10 +78,8 @@ export class SessionPreparations<Source extends PreparedSource, CommitState> {
     commit: (source: Source) => Promise<{ source: Source; state: CommitState }>,
     signal?: AbortSignal,
   ): Promise<SessionPreparationReservation<Source, CommitState> | undefined> {
-    const { entry, created } = this.entryFor(id, load)
-    await (signal === undefined || created
-      ? entry.result
-      : observeQueuedAbort(entry.result, signal))
+    const entry = this.entryFor(id, load)
+    await (signal === undefined ? entry.result : observeQueuedAbort(entry.result, signal))
     while (this.entries.get(id) === entry && entry.phase !== 'ready') {
       const settled = entry.reservationSettled
       /* v8 ignore next -- committing/reserved transitions install this waiter synchronously. */
@@ -132,7 +130,7 @@ export class SessionPreparations<Source extends PreparedSource, CommitState> {
       && entry.reservation !== undefined) {
       return entry.reservation
     }
-    throw new Error(`cannot publish session "${session.id}" while a persisted preparation exists`)
+    throw new Error(`cannot publish session "${session.id}": persisted state already owns this identity`)
   }
 
   /**
@@ -216,20 +214,37 @@ export class SessionPreparations<Source extends PreparedSource, CommitState> {
   private entryFor(
     id: SessionId,
     load: () => Promise<Source>,
-  ): { entry: PreparationEntry<Source, CommitState>; created: boolean } {
+  ): PreparationEntry<Source, CommitState> {
     const existing = this.entries.get(id)
-    if (existing !== undefined) return { entry: existing, created: false }
-    const result = Promise.resolve().then(load)
-    const entry: PreparationEntry<Source, CommitState> = { id, result, phase: 'loading' }
+    if (existing !== undefined) return existing
+    const deferred = Promise.withResolvers<Source>()
+    const entry: PreparationEntry<Source, CommitState> = {
+      id,
+      result: deferred.promise,
+      phase: 'loading',
+    }
     this.entries.set(id, entry)
-    void result.then((source) => {
-      if (this.entries.get(id) !== entry) return
-      entry.source = source
-      entry.phase = 'ready'
-    }, () => {
+    let loading: Promise<Source>
+    try {
+      // Start immediately so a same-tick serialized append queues behind this
+      // read. The deferred result settles only after the entry becomes ready.
+      loading = load()
+    } catch (error: unknown) {
       this.remove(entry)
+      deferred.reject(error)
+      return entry
+    }
+    void loading.then((source) => {
+      if (this.entries.get(id) === entry) {
+        entry.source = source
+        entry.phase = 'ready'
+      }
+      deferred.resolve(source)
+    }, (error: unknown) => {
+      this.remove(entry)
+      deferred.reject(error)
     })
-    return { entry, created: true }
+    return entry
   }
 
   private makeReady(entry: PreparationEntry<Source, CommitState>): void {

packages/session-persistence/session-persistence/tests/persistence.spec.ts

@@ -353,7 +353,7 @@ describe('PersistenceCoordinator stored identity', () => {
 
       await expect(ctx.plugin(Object.assign((inner: Context) => {
         inner.sessions.create(id, { seed: [start], meta: header })
-      }, { inject: ['sessions'] }))).rejects.toThrow(/persisted preparation exists/)
+      }, { inject: ['sessions'] }))).rejects.toThrow(/persisted state already owns this identity/)
       expect(ctx.sessions.get(id)).toBeUndefined()
 
       loadGate.resolve(true)
@@ -590,6 +590,59 @@ describe('PersistenceCoordinator session preparations', () => {
     }
   })
 
+  it('queues a same-tick cold append behind preparation readiness', async () => {
+    const ctx = new Context()
+    await ctx.plugin(SessionStore)
+    const backend = new ControlledBackend()
+    const id = SessionId('inspect-cold-append-race')
+    backend.store.set(id, { meta: meta(id), events: oneTurnLog() })
+    let coordinator!: PersistenceCoordinator<never>
+    const fiber = await ctx.plugin(Object.assign((inner: Context) => {
+      coordinator = new PersistenceCoordinator(inner, backend)
+    }, { inject: ['sessions'] }))
+
+    try {
+      const inspection = coordinator.inspect(id)
+      const append = coordinator.append(id, [{
+        type: 'turn/start',
+        seq: oneTurnLog().length,
+        time: 7,
+        data: { turn: 2 },
+      }])
+
+      await expect(inspection).resolves.toMatchObject({ meta: { id } })
+      await expect(append).resolves.toBeUndefined()
+      expect(backend.loadAttempts).toBe(1)
+      expect(backend.store.get(id)?.events).toHaveLength(oneTurnLog().length + 1)
+    } finally {
+      await fiber.dispose()
+      await ctx.fiber.dispose()
+    }
+  })
+
+  it('inspects an open live turn without balancing it', async () => {
+    const ctx = new Context()
+    await ctx.plugin(SessionStore)
+    const backend = new ControlledBackend()
+    let coordinator!: PersistenceCoordinator<never>
+    const fiber = await ctx.plugin(Object.assign((inner: Context) => {
+      coordinator = new PersistenceCoordinator(inner, backend)
+    }, { inject: ['sessions'] }))
+
+    try {
+      const session = ctx.sessions.create(SessionId('inspect-live-open-turn'))
+      session.append('turn/start', { turn: 1 })
+
+      const inspected = await coordinator.inspect(session.id)
+      expect(inspected.events).toBe(session.events)
+      expect(inspected.events.map(event => event.type)).toEqual(['turn/start'])
+      await expect(coordinator.load(session.id)).rejects.toThrow(/live turn is open/)
+    } finally {
+      await fiber.dispose()
+      await ctx.fiber.dispose()
+    }
+  })
+
   it('keeps synthetic recovery in memory during inspect and commits it only once on prepare', async () => {
     const ctx = new Context()
     await ctx.plugin(SessionStore)
@@ -770,54 +823,36 @@ describe('PersistenceCoordinator observation cancellation', () => {
     }
   })
 
-  it('waits for active cooperative inspection cleanup before rejecting cancellation', async () => {
+  it('keeps a shared cold read alive when its creating inspect is cancelled', async () => {
     const ctx = new Context()
     await ctx.plugin(SessionStore)
     const backend = new ControlledBackend()
-    const id = SessionId('active-inspect-cancellation')
+    const id = SessionId('creating-inspect-cancellation')
     backend.store.set(id, { meta: meta(id), events: oneTurnLog() })
-    const cleanupGate = Promise.withResolvers<boolean>()
-    let cleanupComplete = false
-    backend.beforeLoadStored = async (_attempt, signal) => {
-      await new Promise<void>((resolve) => {
-        signal?.addEventListener('abort', () => {
-          void cleanupGate.promise.then(() => {
-            cleanupComplete = true
-            resolve()
-          })
-        }, { once: true })
-      })
-      throw new Error('backend cancellation after cleanup')
-    }
+    const loadGate = Promise.withResolvers<boolean>()
+    backend.beforeLoadStored = () => loadGate.promise.then(() => undefined)
     let coordinator!: PersistenceCoordinator<never>
     const fiber = await ctx.plugin(Object.assign((inner: Context) => {
       coordinator = new PersistenceCoordinator(inner, backend)
     }, { inject: ['sessions'] }))
+    let prepared: Awaited<ReturnType<typeof coordinator.prepare>> | undefined
 
     try {
       const controller = new AbortController()
-      const reason = new Error('active inspect cancelled')
-      const pending = coordinator.inspect(id, controller.signal)
-      let observedReason: unknown
-      const observed = pending.catch((error: unknown) => {
-        observedReason = error
-      })
+      const reason = new Error('creating inspect cancelled')
+      const inspection = coordinator.inspect(id, controller.signal)
       await vi.waitFor(() => { expect(backend.loadAttempts).toBe(1) })
+      const reservation = coordinator.prepare(id)
 
       controller.abort(reason)
-      await Promise.resolve()
-
-      expect(observedReason).toBeUndefined()
-      expect(cleanupComplete).toBe(false)
-      cleanupGate.resolve(true)
-      await observed
-      expect(cleanupComplete).toBe(true)
-      expect(observedReason).toBe(reason)
-      const backendFailure = new Error('later inspection failure')
-      backend.beforeLoadStored = () => Promise.reject(backendFailure)
-      await expect(coordinator.inspect(id)).rejects.toBe(backendFailure)
+      await expect(inspection).rejects.toBe(reason)
+      loadGate.resolve(true)
+      prepared = await reservation
+      expect(prepared.session.id).toBe(id)
+      expect(backend.loadAttempts).toBe(1)
     } finally {
-      cleanupGate.resolve(true)
+      loadGate.resolve(true)
+      prepared?.[Symbol.dispose]()
       await fiber.dispose()
       await ctx.fiber.dispose()
     }
@@ -1092,7 +1127,7 @@ describe('PersistenceCoordinator retirement', () => {
 
       await expect(ctx.plugin(Object.assign((inner: Context) => {
         inner.sessions.create(id)
-      }, { inject: ['sessions'] }))).rejects.toThrow(/persisted preparation exists/)
+      }, { inject: ['sessions'] }))).rejects.toThrow(/persisted state already owns this identity/)
 
       loadGate.resolve(true)
       await expect(coldLoad).resolves.toMatchObject({

packages/session-persistence/session-persistence/tests/preparations.spec.ts

@@ -39,6 +39,25 @@ describe('SessionPreparations inspection', () => {
     expect(preparations.has(id)).toBe(false)
   })
 
+  it('keeps a shared load alive when its first observer cancels', async () => {
+    const preparations = new SessionPreparations<PreparedSource, string>(1)
+    const id = SessionId('cancelled-first-observer')
+    const gate = Promise.withResolvers<PreparedSource>()
+    const load = vi.fn(() => gate.promise)
+    const controller = new AbortController()
+    const reason = new Error('first observer cancelled')
+    const first = preparations.inspect(id, load, controller.signal)
+    const joined = preparations.inspect(id, load)
+
+    controller.abort(reason)
+    await expect(first).rejects.toBe(reason)
+    const source = prepared(id)
+    gate.resolve(source)
+    await expect(joined).resolves.toBe(source)
+    await expect(preparations.inspect(id, load)).resolves.toBe(source)
+    expect(load).toHaveBeenCalledOnce()
+  })
+
   it('removes failed and invalidated in-flight loads without changing their observers', async () => {
     const preparations = new SessionPreparations<PreparedSource, string>(1)
     const failedId = SessionId('failed-inspection')
@@ -63,6 +82,15 @@ describe('SessionPreparations inspection', () => {
     await expect(rejected).rejects.toBe(failure)
   })
 
+  it('removes a load that throws before returning its promise', async () => {
+    const preparations = new SessionPreparations<PreparedSource, string>(1)
+    const id = SessionId('synchronous-load-failure')
+    const failure = new Error('synchronous load failure')
+
+    await expect(preparations.inspect(id, () => { throw failure })).rejects.toBe(failure)
+    expect(preparations.has(id)).toBe(false)
+  })
+
   it('evicts ready entries while leaving reserved entries alone', async () => {
     const preparations = new SessionPreparations<PreparedSource, string>(1)
     const reservedA = await preparations.reserve(