Merge remote-tracking branch 'origin/master' into codex/simp-session-dead-surface

# Conflicts:
#	docs/cordis-catalog/services.md
#	packages/core/session/README.md
#	packages/core/session/tests/derived-cache.spec.ts
#	packages/session-persistence/session-persistence-jsonl/tests/jsonl.spec.ts
#	packages/session-persistence/session-persistence/src/coordinator.ts
#	packages/session-persistence/session-persistence/src/index.ts

packages/core/session/tests/derived-cache.spec.ts

@@ -1,10 +1,7 @@
 /**
- * Derived-message cache tests: the session projects each surface node exactly
- * once (O(new nodes) per call), rebuilds on a surface replace (the
- * replaceGeneration signal), returns a fresh array snapshot
- * per call over shared frozen messages, and stays deep-equal to a from-scratch
- * replay derivation at every step — the incremental==scratch property the
- * reconstructability RFC's invariant enforces in dev at request time.
+ * Derived-message cache contract against a scratch oracle: project new nodes
+ * once, rebuild on surface replacements, return fresh arrays over shared
+ * frozen messages, and remain value-equal to replay at every step.
  */
 
 import { describe, expect, it } from 'vitest'
@@ -28,7 +25,6 @@ describe('derived-message cache', () => {
     userText(session, 'two')
     session.append('assistant/message', { turn: 1, step: 1, content: [{ type: 'text', text: 'reply' }] }, { surfaceOp: 'append' })
     expect(session.deriveMessages()).toEqual(scratch(session))
-    // An empty-content assistant/message (usage host) projects to nothing.
     session.append('assistant/message', { turn: 1, step: 2, content: [], usage: { inputTokens: 1, outputTokens: 0 } }, { surfaceOp: 'append' })
     expect(session.deriveMessages()).toEqual(scratch(session))
   })
@@ -48,7 +44,6 @@ describe('derived-message cache', () => {
 
     expect(session.deriveMessages()).toHaveLength(1)
     expect(session.deriveMessages()).toEqual(scratch(session))
-    // The array a caller took before the replace is untouched.
     expect(beforeReplace).toHaveLength(2)
   })
 
@@ -61,7 +56,7 @@ describe('derived-message cache', () => {
     const second = session.deriveMessages()
     expect(first).toHaveLength(1)
     expect(second).toHaveLength(2)
-    // Shared projection objects: the same frozen message instance, once ever.
+    // Array snapshots share their frozen message projections.
     expect(second[0]).toBe(first[0])
     expect(Object.isFrozen(first[0])).toBe(true)
   })
@@ -73,8 +68,7 @@ describe('Session.deriveEventMessage — the per-event projection', () => {
     const session = new Session(SessionId('per-event'))
     session.append('turn/start', { turn: 1, trigger: { kind: 'message', source: { kind: 'user' } } })
     const event = session.append('user/message', { content: [{ type: 'text', text: 'hi' }], source: { kind: 'user' } }, { surfaceOp: 'append' })
-    // The fold path (deriveMessages) and the per-event path share the
-    // projection, so an external reconstructor cannot disagree with the cache.
+    // Full and per-event derivation share one projection.
     expect(session.deriveEventMessage(event)).toEqual(session.deriveMessages().at(-1))
   })
 

packages/core/session/tests/gen-persistence-catalog.spec.ts

@@ -1,16 +1,6 @@
 /**
  * Negative-path tests for the persistence log catalog generator
  * (`scripts/gen-persistence-catalog.ts`).
- *
- * The generated catalog is frozen by a regenerate-and-diff freshness gate, so
- * the freshness half is exercised by `pnpm run verify-persistence-catalog` in
- * CI. What a freshness diff CANNOT prove is that the generator REJECTS
- * malformed source the way it promises to — a member without description
- * prose, a forbidden `@mode` tag, a non-literal member name, a duplicate event
- * declaration, a missing or ambiguous `SurfaceEventType` union, a stale union
- * member. These tests drive the exported collectors against synthetic fixture
- * packages to prove each guard fires (and that well-formed declarations pass),
- * mirroring the gen-cordis-catalog negative tests.
  */
 
 import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from 'node:fs'

packages/core/session/tests/properties.spec.ts

@@ -13,10 +13,8 @@ import { CallId } from '@deepseek-ai/dsh-llm'
 import { Session, SessionId } from '@deepseek-ai/dsh-session'
 import type { SessionEventMap, SessionEventType, SurfaceIntent } from '@deepseek-ai/dsh-session'
 
-// An appendable event: its type/data plus, for surface-eligible types, the
-// explicit surface intent the generator declares (mirroring how a real caller
-// passes it). The intent is part of the generated fixture, NOT synthesized by
-// `build`, so each arbitrary states the marker it produces.
+// Each arbitrary supplies its own surface intent; `build` must not synthesize
+// one or the property would fail to exercise malformed fixture choices.
 type Appendable = {
   [T in SessionEventType]: { type: T; data: SessionEventMap[T]; intent?: SurfaceIntent }
 }[SessionEventType]

packages/core/session/tests/repair.spec.ts

@@ -155,11 +155,8 @@ describe('interruptedTurnClosers', () => {
   })
 
   it('handles tool/call without a matching assistant/message entry gracefully', () => {
-    // A tool/call event exists in the log but no assistant/message registered
-    // the callId in pendingCalls (e.g., a plugin appended it directly, or the
-    // assistant/message from a prior step didn't have this call). The repair
-    // should still close the turn — it just won't synthesize a result for this
-    // call (there's nothing to answer).
+    // A raw tool/call with no assistant-registered pending call has nothing to
+    // answer; repair still closes the step and turn without synthesizing a result.
     const events: SessionEvent[] = [
       userTurnStart(1, 0),
       { type: 'step/start', seq: 1, time: 1, data: { turn: 1, step: 1 } },

packages/core/session/tests/session.spec.ts

@@ -81,9 +81,6 @@ describe('Session', () => {
     const before = structuredClone(session.events)
 
     // A misbehaving consumer tries to mutate the messages it was handed.
-    // Derived messages are frozen shared projections (cloned once off the
-    // log, then deep-frozen): every mutation attempt THROWS in strict mode —
-    // isolation by unrepresentability, not by per-call cloning.
     const messages = session.deriveMessages()
     const userBlock = messages[0]!.content[0]!
     expect(() => { if (userBlock.type === 'text') userBlock.text = 'HACKED' }).toThrow(TypeError)
@@ -132,11 +129,8 @@ describe('Session', () => {
   it('rejects a surface-eligible append with no surfaceOp marker (runtime guard for the union-widening loophole)', () => {
     const session = new Session(SessionId('s5b'))
     session.append('turn/start', { turn: 1, trigger: { kind: 'message', source: { kind: 'user' } } })
-    // The typed overload makes surfaceOp mandatory only when the type argument is
-    // a SPECIFIC SurfaceEventType literal. A caller iterating raw events widens it
-    // to the SessionEventType union, where the conditional rest collapses to
-    // optional — the exact shape `for (const e of log) append(e.type, e.data)`
-    // produces. Reproduce that here and assert the runtime guard rejects it.
+    // A widened SessionEventType bypasses the overload's conditional requirement,
+    // so the runtime guard must still reject the missing surface marker.
     const widenedType = 'user/message' as SessionEventType
     expect(() => session.append(widenedType, { content: [{ type: 'text', text: 'hi' }], source: { kind: 'user' } }))
       .toThrow(/surface-eligible and requires a surfaceOp marker/)
@@ -663,10 +657,8 @@ describe('SessionStore', () => {
   })
 
   it('enter() rejects a stale prepared session whose id is already live (no overwrite)', async () => {
-    // prepare()/enter() are public cross-package primitives that a caller may
-    // separate with arbitrary work. A stale prepared session must NOT overwrite
-    // a live store entry of the same id — its detach disposer would later delete
-    // the REAL session, breaking the store-uniqueness invariant.
+    // A stale prepared object must not replace the live same-id entry; its later
+    // detach would otherwise remove the wrong session.
     const ctx = new Context()
     await ctx.plugin(SessionStore)
     const stale = ctx.sessions.prepare(SessionId('racy'))

packages/core/session/tests/surface.spec.ts

@@ -115,14 +115,11 @@ describe('SurfaceManager', () => {
 
   it('rebuild with replace operation splices out shadowed nodes', () => {
     const s = surfaceSession()
-    // seq: 0=turn/start, 1=user, 2=assistant, 3=turn/end
-    // Surface nodes: seq 1 (user), seq 2 (assistant).
-    // Replace both with a compaction marker. Both 1 and 2 are valid surface seqs.
+    // Replace surface seqs 1 (user) and 2 (assistant) with the summary.
     s.append('assistant/message',
       { turn: 2, step: 1, content: [{ type: 'text', text: 'summary' }] },
       { surfaceOp: { op: 'replace', start: 1, end: 2 }, sourceEventSeqs: [1, 2] },
     )
-    // Now the surface should have just the compaction node.
     expect(s.surface.nodes.length).toBe(1)
     expect(s.surface.nodes[0]!.seq).toBe(4) // seq of the compaction marker
     expect(s.surface.nodes[0]!.prev).toBeNull()

packages/core/session/tests/tool-pairing.spec.ts

@@ -4,24 +4,9 @@ import { Session, SessionId, isToolPairingBalanced } from '../src/index.ts'
 import type { SessionEvent, SurfaceNode } from '../src/index.ts'
 
 /**
- * Unit coverage for the tool-pairing balance check. It decides whether a CUT in
- * the surface (a gap before a given surface node, or the after-tail gap) is a
- * safe edge for a collapsed region (compaction): a region must never split an
- * `assistant/message`'s tool-calls from their `tool/result`s. A cut is balanced
- * when no unanswered tool-call sits before it on the surface. Nodes belonging to
- * no step (pre-step user message, inter-step steering, injection context) are
- * pairing-neutral, so their cuts are free boundaries.
- *
- * The fixtures are built through a real {@link Session} so the surface linked
- * list is derived exactly as production does — including the non-monotonic
- * surface a `replace` op leaves (a compaction checkpoint at a high log seq
- * sitting at the surface head), which is the case the abandoned log-position
- * scan mis-classified.
- *
- * Builders mirror the agent loop's real append order: queued user messages land
- * BEFORE `step/start`; within a step the order is `assistant/message` then
- * `tool/result`(s); injection turns are a bare `turn/start → context/message →
- * turn/end` with no step.
+ * Unit coverage for compaction-cut safety: a cut is balanced only when it
+ * separates no assistant tool call from its result. Non-step nodes are neutral,
+ * and replace operations prove surface order—not raw log order—is authoritative.
  */
 
 const SURFACE = { surfaceOp: 'append' as const }
@@ -182,10 +167,8 @@ describe('isToolPairingBalanced — multiple tool calls in one assistant message
 })
 
 describe('isToolPairingBalanced — a mid-step injection context/message', () => {
-  // A background task-done inject() lands a context/message INSIDE an open step,
-  // between the assistant (with a tool-call) and its tool/result. It is
-  // pairing-neutral, so the cut on EITHER side of it is unbalanced (the call is
-  // still open across it) — it is NOT a free boundary in this position.
+  // The injected context is pairing-neutral, but both adjacent cuts remain
+  // unbalanced because the tool call is still open across them.
   function midStepInjection(): Session {
     const s = new Session(SessionId('mid-inject'))
     s.append('turn/start', { turn: 1, trigger: { kind: 'message', source: { kind: 'user' } } })
@@ -236,11 +219,8 @@ describe('isToolPairingBalanced on an injection turn (no step)', () => {
 })
 
 describe('isToolPairingBalanced — CBR-001: a head checkpoint left by a replace op', () => {
-  // The case the log-position scan got wrong. After a compaction, a replacement
-  // user/message lands at a HIGH log seq but sits at the SURFACE head, beside
-  // the still-open step whose events follow it in the log. It carries no
-  // tool-call/result pair (just summarized prose), so it must be a balanced cut
-  // on BOTH sides regardless of its log neighbours.
+  // A replacement checkpoint has a high log seq but sits at the surface head;
+  // its cuts are balanced regardless of later raw-log neighbors.
   function checkpointHeadedSession(): Session {
     const s = new Session(SessionId('checkpoint'))
     // A closed turn with a tool step → surface [u1, asst(call), result].
@@ -291,10 +271,8 @@ describe('isToolPairingBalanced — CBR-001: a head checkpoint left by a replace
   })
 
   it('end cut after the head checkpoint is balanced (it carries no tool pair)', () => {
-    // This is the exact assertion the log-position scan failed: the forward log
-    // scan from the checkpoint reached the open step's assistant/message and
-    // wrongly reported mid-step. The surface balance sees a neutral node whose
-    // following cut closes no open call.
+    // This is the exact assertion the log-position scan failed: the forward log scan from the
+    // checkpoint reached the open step's assistant/message and wrongly reported mid-step.
     const s = checkpointHeadedSession()
     expect(endBalanced(s, s.surface.nodes[0]!.seq)).toBe(true)
   })