Repository navigation
Replies: 1 comment
|
Hi! I would suggest making validation atomic for the entire append operation. When multiple events are appended together, validating each event independently against the original state may miss transitions that become invalid after earlier events in the same batch. A safer approach would be to validate the complete proposed sequence against a temporary state representation before committing any durable changes. If validation fails, the entire append should be rejected without advancing the persistent cursor. I would add regression tests for invalid transitions within a single batch, transitions spanning two batches, and recovery after a rejected append. This would help ensure that the append API cannot partially commit a logically invalid session history. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
1/3 - Session append path does not validate turn/step relationships, so plugins can silently write an unloadable session log
Version: 0.2.1-rc.5 (Windows desktop)
Area: session persistence (
@deepseek-ai/dsh-session-persistence-jsonl)Summary
The durable append path validates only seq contiguity, not the turn/step relationship that the loader later enforces. A plugin that appends a message event while no step is open writes a perfectly "valid-looking" log that fails on the next load, and nothing reports it at write time.
Evidence
app.asar/dsh/node_modules/@deepseek-ai/dsh-session-persistence-jsonl/lib/index.js:118append(events, options)->materializeAppendBatch->persistContiguous:224persistContiguous(batch)- the single choke point for every durable write (also used by the live-buffer drain at:206):228assertContiguous(this.id, batch, this.state.cursor)- seq only:237persistBatch(...)- writesMeanwhile the loader enforces the relationship:
Relationships.requireStepthrowsSessionFormatError: <type> does not match an open turn and stepwhen
turn/stepdo not match the currently open pair.validateStoredEvents(in@deepseek-ai/dsh-session-persistence) only runs on the read side.Reproduction
agent/pre-step) appendsdeveloper/messagewith{turn, step}before that step'sstep/startevent exists.stored session "<id>" is corrupt: stored log is corrupt: SessionFormatError: developer/message does not match an open turn and stepThe bad write happened ~2 hours before the failure was noticed; there is no log line, no console warning, and no UI indication at write time.
Impact
Any third-party plugin can permanently brick a session with a single misplaced
appendcall, and the user only finds out on the next open - by which point the turn is already corrupted in the persisted log.Suggested fix
Validate the relationship in
persistContiguousbeforepersistBatch(the open turn/step state is already tracked by the adopting relationship object), and either:SessionFormatError(fail fast, plugin author sees it immediately), orwarnand keep the session loadable.Either is strictly better than persisting an event the loader will refuse.
Appendix - full evidence
The failing load
The exact six events (decoded log, seq 331-336, user text redacted)
{"type": "turn/start", "seq": 331, "time": 1793697442528, "data": {"turn": 3}} {"type": "agent/inbox/spliced", "seq": 332, "time": 1793697442529, "data": {"target": "next-turn", "start": 0, "removedCount": 1, "inserted": []}} {"type": "plugin:working-activity/status", "seq": 333, "time": 1793697442531, "data": {"phase": "waiting", "line": "模型说等一下 · 总0s", "toolCount": 0, "turnElapsedMs": 1, "phaseStartedAt": 1793697442528, "phrase": "模型说等一下"}, "ignorable": true} {"type": "developer/message", "seq": 334, "time": 1793697442683, "data": {"turn": 3, "step": 1, "message": {"id": "6709c7bd-6343-4fd8-a835-a84d85911f6a", "role": "developer", "content": [{"type": "text", "text": "[Router] 判据命中:small_read|只读信息类,直接作答并说明不需要派"}], "source": {"kind": "dsh-lead-router", "form": "instructions"}}}, "surfaceOp": "append"} {"type": "step/start", "seq": 335, "time": 1793697442684, "data": {"turn": 3, "step": 1}} {"type": "user/message", "seq": 336, "time": 1793697442686, "data": {"content": [{"type": "text", "text": "<redacted 24 chars>"}], "source": {"kind": "user", "rpcId": "d0ce729e-6d4d-49ad-a1a8-7c089b0890e9", "clientTimeZone": "Asia/Taipei"}, "role": "user", "id": "c45f7662-4fbc-43a2-98e2-5e5dd2fe08a0"}, "surfaceOp": "append"}seq 334is thedeveloper/message(turn=3, step=1);seq 335is thestep/startfor that same turn/step. The message was persisted one event before the step existed.seq 331(turn/start) opened turn 3 but no step, and nothing between 331 and 334 opened step 1 - sopersistContiguoushad no way to know the append was illegal, and nothing rejected it.Log shape
A sanitized copy of all 712 events (message text replaced with
<redacted N chars>, types/turn/step/structure preserved) is available on request.What a reader needs to know
0.2.1-rc.5, Windows 11. Session store:%USERPROFILE%\.dsh\sessions\<workspace>\<id>\session.v4.jsonl.zstd.agent/pre-stephook calledsession.append("developer/message", { turn, step, ... })afterturn/start(turn 3) had been persisted but beforestep/start(turn 3 / step 1) was. The append was accepted,persistContiguouschecked seq contiguity only,persistBatchwrote it.validateStoredEvents/Relationships.requireStepdo exist - but only on the load path. The write path hasassertContiguousand nothing else.validateStoredEvents-> 158/158 pass. So this is not general flakiness; it only appears after an illegal append. It is an API-hardening gap, not a corruption bug.persistContiguous(lib/index.js:224), right afterassertContiguous(:228), validate the batch against the open turn/step tracker. Reject with aSessionFormatError(preferred - the plugin author sees it immediately) or drop +warn. ~10 lines.All reactions