Session logs corrupted by concurrent writers: append path trusts its in-memory cursor

[yylonly]

Symptom

Two of my sessions became permanently unloadable after two live lifecycles wrote the same session log. The loader correctly refused them:

corrupt session log: seq gap in committed region …

One log had a duplicate session/end-seed / agent/inbox/spliced pair at seq 3; the other had a closing batch two seqs behind the committed prefix. The data was already destroyed at write time — the load refusal was correct, but late.

Root cause

PersistenceCoordinator.appendCore validates a batch's seqs against its in-memory cursor only, and the backend appendBatch writes without comparing anything to what it last observed. A second process sharing the same sessions root (its own coordinator state) can commit events the first process never sees; the first process's next batch then writes seqs that overlap the committed region.

The prepare/load path already handles concurrent writers — commitPrepared verifies isPreparedSourceCurrent and retries. The append path had no equivalent guard; that asymmetry is what made the corruption possible.

Fix (branch on my fork)

appendCore now revalidates the durable revision before trusting the cursor:

1. SessionState records the durable revision (state.revision) its cursor was established against — set at every state-establishment site (commitPrepared, HMR adoptLivePrefix, and after every successful append).
2. Before a batch, appendCore re-reads readStoredRevision; when a different writer changed the log, it reconciles the cursor from a fresh loadStored and lets the existing contiguity check decide:
   • the batch continues the reconciled cursor → append proceeds (the other writer only left a torn tail);
   • the batch starts below it → loud refusal: session "<id>" durable log changed under this process (stored N events, this batch starts at seq M); reload the session before appending;
   • the log is gone → refuse with the missing-log direction instead of re-materializing a header-less artifact.
3. Cost: one stat-based revision read before and after each append — negligible next to the fsync the append already performs.

Residual (documented in the note): the check-then-act window between the revision read and the file write still exists; closing it needs per-log locking the format deliberately avoids. The window is now bounded per batch instead of always-open.

Branch: fix/session-persistence-append-revision-guard ·
Compare against master

Rebased onto origin/master (dd6322d604). Verified locally: full build:lib:host (types), session-persistence suite 403/403, session-persistence-jsonl suite 248/248 (new revision-guard cases in the shared coordinator contract), tool-cordis 11/11, doc-sync 32/32. The commit carries a full bilingual Agent Note.

Workaround until fixed

Avoid two processes sharing one sessions root with live sessions — the corruption needs a second writer committing into a log this process still has open.

---

中文版

症状

两个活动生命周期写同一份会话日志后，我的两个会话永久无法加载。加载器正确地拒绝了它们：

corrupt session log: seq gap in committed region …

其中一份日志在 seq 3 处出现重复的 session/end-seed / agent/inbox/spliced；另一份的关闭批次比已提交前缀落后两个 seq。数据在写入时就已经被毁——加载器的拒绝是对的，但为时已晚。

根因

PersistenceCoordinator.appendCore 只用内存游标校验批次 seq，后端 appendBatch 写盘时也不与它上次观察到的状态做任何比对。第二个进程共享同一会话根目录（持有自己的协调器状态）时，可以提交本进程从未见过的事件；本进程的下一个批次随后写入与已提交区域重叠的 seq。

prepare/load 路径已经处理了并发写入者——commitPrepared 校验 isPreparedSourceCurrent 并重试。append 路径没有等价防护；正是这个不对称让损坏成为可能。

修复（我 fork 上的分支）

appendCore 现在在信任游标之前重验持久化修订号：

1. SessionState 记录其游标建立时所对应的持久化修订（state.revision）——在每个状态建立点设置（commitPrepared、HMR adoptLivePrefix、以及每次成功 append 之后）。
2. 批次写入前，appendCore 重读 readStoredRevision；发现别的写入者改过日志时，先从新的 loadStored 对齐游标，再交给既有的连续性检查裁决：
   • 批次延续对齐后的游标 → 继续追加（对方只留下了撕裂尾部）；
   • 批次起点低于它 → 大声拒绝：session "<id>" durable log changed under this process (stored N events, this batch starts at seq M); reload the session before appending；
   • 日志已消失 → 按缺失日志的方向拒绝，而不是重新物化一个没有 header 的产物。
3. 代价：每次 append 前后各一次基于 stat 的修订读取——相对 append 本身已有的 fsync 可忽略。

残留问题（note 里有记录）：修订读取与文件写入之间的检查-再行动窗口仍然存在；彻底关闭它需要格式刻意回避的 per-log 锁。该窗口从"永远敞开"收窄为"每批次有界"。

分支： fix/session-persistence-append-revision-guard ·
与 master 对比

已 rebase 到 origin/master（dd6322d604）。本地验证：完整 build:lib:host（类型）、session-persistence 套件 403/403、session-persistence-jsonl 套件 248/248（共享协调器契约里新增了修订守卫用例）、tool-cordis 11/11、doc-sync 32/32。提交带有完整的双语 Agent Note。

修复之前的规避方法

避免两个进程带着活动会话共享同一会话根目录——损坏需要第二个写入者提交进本进程仍打开着的日志。

相关报告

#5243 报告了同类损坏症状（seq gap in committed region），但根因不同（中断轮次恢复时回卷 seq）；两者都会让加载器永久拒绝该日志。

[JakeLi168]

Independent reproduction on DSH 0.1.1-rc.2 (macOS, web via launchd + Desktop shell sharing ~/.dsh/sessions) — same corruption signature, second confirmation of your root-cause analysis:

Damage fingerprint (both of our affected sessions, 100% identical)

turn/end(N)          ← instance A restart recovery window
session/end-seed(N)  ← written by the recovering process (restore seed)
agent/inbox/spliced(N)  ← DUPLICATE seq, written ~2-3 min later by the OTHER instance
turn/start(N+1)

• The end-seed lands on disk ~2-3 min before the inbox/spliced of the next user message — exactly what you predicted: one process still holds the session open (its in-memory firstLiveSeq/cursor says N), a second process restores the same session from disk (sees last committed seq N-1, appends end-seed at N), then the first process's live write also lands at N.
• Every file had exactly one duplicate-seq pair; the remaining ~239k / ~59k events were perfectly contiguous — only the seq = log.length in-memory cursor contract is at fault, matching your appendCore diagnosis.

User-visible impact note

The loader's refusal is technically correct but permanent: once a session log has one duplicate seq, the session can never be opened again via the UI ("history read failed … seq gap in committed region"), which users perceive as silently losing a conversation after any restart. We did a survey-style check: with two instances (web + shell) both enabled, the corruption recurs whenever a restart happens while the other instance holds a recently-used session.

Recovery (we recovered both sessions with zero event loss — may be worth including in a future test fixture)

Using the same published package's own codecs (no foreign rewrite of the zstd frames):

1. readRaw(id) to get the decoded JSONL text;
2. decode every line with decodeStorageRecord (note: packed chunk rows (text-chunks etc.) carry seq0 and expand to several events — checking seq by line number produces hundreds of false gaps; must compare against the expanded event stream);
3. find the duplicate pair, then renumber seq/seq0 +1 from that row onward;
4. rewrite with packChunks: false + official materialize/appendBatch (header frame + event batch frames, so it stays byte-format-valid);
5. verify: official readPrefix passes, seq 0..N contiguous.

Original corrupted files kept as SHA-256-pinned backups before the fix.

Suggestion for the maintainers: since a duplicate (or short) seq inside the committed region is exactly the deterministic artifact of this concurrency bug, the loader could detect the "exactly-one-duplicate-seq here + rest contiguous" pattern during load and either self-heal (with a warning) or at least show a clear "this session was affected by the concurrent-writer bug; run " message instead of a generic permanent refusal.

Supporting the upstream fix: the fix/session-persistence-append-revision-guard branch addresses the real asymmetry (prepare path revalidates via isPreparedSourceCurrent, append path doesn't) — worth being considered for merge; until then, "don't run two processes over one sessions root" is the only mitigation, which matches our experience.

Reference: local recovery record + this discussion's dual-language note. Happy to provide the full recovery script or more log fingerprints on request.

[argszero]

Excellent analysis, and the concurrent-writer asymmetry is real. I independently re-read the alpha.2 source (0a53fb55be) of packages/session/session-persistence/src/coordinator.ts and confirmed exactly what you describe — the append path trusts an in-memory cursor while the prepare path revalidates against the durable revision.

Append path — no concurrent-writer guard (coordinator.ts:appendCore)

The appendCore method checks seq contiguity against state.cursor only:

let state = this.states.get(id)
if (state === undefined) state = await this.adopt(id)
for (const [i, event] of events.entries()) {
  if (event.seq !== state.cursor + i) {
    throw new Error(`append seq mismatch for "${id}": ...`)
  }
}
await this.backend.appendBatch(state.meta, events, state.materialized)

state.cursor is the in-memory cursor established by adopt(). Critically, adopt() here branches on this.states.get(id) — if the process already has a live state for this id (its own coordinator's cursor), it uses that without re-reading the durable revision. Then appendBatch writes without comparing anything to what it last observed. I confirmed the JSONL backend appendBatch is a plain appendLines(meta, events) / materialize(meta, events) — no revision recheck.

Prepare path — guarded (coordinator.ts:commitPrepared / isPreparedSourceCurrent)

By contrast, the resume path verifies the durable log before committing:

if (!await this.isPreparedSourceCurrent(source)) return undefined
// ...
async isPreparedSourceCurrent(source, signal?) {
  return await this.backend.readStoredRevision(source.inspection.meta.id, signal) === source.revision
}

So the asymmetry is structural and exactly where you place it: the load side has a revision guard, the append side does not. Two processes sharing a sessions root each advance their own cursor; a second process's committed batch is invisible to the first until the next adopt(), so the first's subsequent batch can overlap the committed region.

On the fix direction — revalidating readStoredRevision before appendCore trusts the cursor is the right seam, and the reconciliation you describe (re-load via loadStored, then let the existing contiguity check decide) is sound. One thing worth pinning down for the test suite: a two-process interleaving where process A's batch and process B's batch both target the same id, interleaved at the seq boundary — that's the minimal deterministic repro that exercises the guard without relying on timing. JakeLi168's independent reproduction on rc.2 adds confidence that this isn't alpha.2-specific.

This plugs into a known family (#1333/#1452/#1497/#1473/#1586/#2167/#2342/#2839/#2900/#3198/#3450/#4274/#4811/#5059/#5103/#5160) where several "corrupt session log" reports trace back to cross-process or replay-ordering writers. A durable-revision guard on append is the missing invariant those reports point at.