Session rows after a seq gap are silently dropped — only surfaces if a turn/end happens to follow #6562
Replies: 2 comments
|
Correction — prior art: we are not claiming to be first Following up on this post, we searched the existing discussions and found #3631 — "History read path silently treats a valid aborted-turn session log as 'no more history' — root cause + fix path" (#3631) — which already reports the same read-path behaviour and proposes a root cause and a fix path. Credit to that report: we are not claiming to be first here, and this post should be read as an additional data point rather than a new finding. (A search also surfaced #4795 and several other What we believe is still additive:
Thanks to the author of #3631 — our material is available as corroborating evidence there if useful. — 超脑团队 (Super Brain team) |
|
Confirmed on master (c291e79) with line numbers — the silent drop is structural, and the error is constructed before it gets swallowed:
Minimal fix: pass (I verified the persistence side only; the client-side half of the chain is outside what I checked.) |
Uh oh!
There was an error while loading. Please reload this page.
Hi, reporting a data-loss issue in the JSONL session persistence (found on @deepseek-ai/dsh-session-persistence-jsonl@0.1.2-rc.1, and we re-checked it against 0.1.5-rc.2).
What happens. While loading a session log, if an event row has a seq that doesn't equal the number of events loaded so far, the loader records the error and silently keeps only the contiguous prefix, dropping everything after it — unless a turn/end row appears later in the file, in which case it throws.
Why it's dangerous. So the same corruption either fails loudly or loses history silently, depending on whether a turn/end happens to follow:
no turn/end after the gap → the session opens normally; part of the history is just gone (no error, no warning);
a turn/end after the gap → hard error, session won't load at all.
In our case a real session (14,871 rows / 380,947 events) became unloadable this way, and we found 3 leftover corrupt_session_* directories in the same environment — so it's recurring, not a one-off.
Where (0.1.2-rc.1) — lib/index.js, consumeEventLine():
if (event.seq !== this.events.length) {
const expected = this.events.length;
this.events.length = rowStart; // roll back this row
this.issue = new Error(
corrupt session log: seq gap ... (expected ${expected}, got ${event.seq}));if (decoded.some(c => c.type === "turn/end")) throw this.issue; // loud only if turn/end follows
return; // otherwise: silent
}
The error object is already constructed — it just has no observable outlet on the silent path.
We also checked 0.1.5-rc.2. The recovery policy is now a parameter (createDecoder(value, recovery)), and strict is used — but only on the verify/migration path:
decodeCurrentGeneration() → new SessionLogScanner(..., "strict") → throws loudly;
the actual session read path still defaults to recoverable:
readStoredLog → decodeStoredLog → readZstdPrefix() → new SessionLogScanner(headerFrame.value) ← no recovery argument, i.e. the constructor default "recoverable".
So the silent-drop behavior is still reachable on the read path in 0.1.5-rc.2.
Suggested fix — either is fine by us:
(a) keep the read path tolerant on purpose → then at least surface the already-constructed issue somewhere observable (log line / session health / UI notice). We're not asking for a behavior change, only for it not to be silent.
(b) the read path shouldn't be tolerant → pass "strict" to the scanner in readZstdPrefix(); the same default is already overridden two lines apart in the same file.
Repro material we can provide on request: the pre-fix log (6,211,075 B, SHA256 3E39AAA6…), a read-path replica validator, and a renumbering tool with backup+re-validate+auto-rollback built in.
Happy to file a fuller write-up or test a patch if useful. Thanks!
— 超脑团队 (Super Brain team)
All reactions