Merge pull request #46 from deepseek-ai/split/turn-enclosure

feat(agent-loop): turn-enclosure invariant + post-turn error model (split 2/5, re-land)

docs/adr/0017-turn-enclosure-invariant.md

@@ -0,0 +1,38 @@
+# ADR 0017: Every session event is enclosed in a turn
+
+Status: accepted (2026-06-15)
+
+## Context
+
+A durable session-persistence backend (added in a companion change) uses the **turn** as its crash-recovery boundary: `load` returns events only up to the last complete `turn/end`, and the first post-load `append` truncates whatever follows as a never-committed crash tail. This is safe only if nothing *legitimately* durable can sit after the last `turn/end`.
+
+That assumption did not hold. Two paths recorded events outside any turn:
+
+1. **Queued user messages.** The loop drained queued messages and appended `user/message` *before* `turn/start` — so a turn's own prompt sat in the gap between the previous `turn/end` and the next `turn/start`.
+2. **Idle context injection.** `agent.inject()` appends a `context/message` directly. Its real production caller is `dsh-tool-bash`, which injects a background-task completion notice from `ctx.bash.onTaskDone` — a callback that fires whenever a background bash task finishes, frequently while the agent is **idle** (between turns).
+
+In case 2, if the injected `context/message` is the last event before a flush/dispose (no later turn appends a `turn/end`), `scanLog` treats it as crash debris and **drops it on resume** — the injected context is durably on disk but silently lost on reload. Case 1 was benign in isolation (a `user/message` is always followed by the turn it triggered) but made the "what may appear outside a turn" rule fuzzy.
+
+Two ways to fix it: relax the *reader* (let `scanLog` commit events that sit outside an open turn), or constrain the *producer* (make every event turn-enclosed so the reader's simple "last `turn/end`" rule is both correct and complete). We chose the producer-side invariant: a single, checkable rule beats a more permissive boundary scan that has to reason about partial turns *and* loose between-turn events.
+
+## Decision
+
+**Every session event lives inside a turn** — between a `turn/start` and its matching `turn/end`. Concretely:
+
+- The loop appends queued `user/message` events **after** `turn/start` (inside the turn), not before it. `turn/end` is therefore owed the moment those messages are recorded, and the existing finalizer guarantees it.
+- An `agent.inject()` made while the agent is **running** appends its `context/message` into the already-open turn (unchanged).
+- An `agent.inject()` made while **idle** wraps its `context/message` in a one-shot turn: `turn/start{trigger:{kind:'injection'}}` → `context/message` → `turn/end{completed}`. A new `injection` variant joins the merge-extensible `TurnTriggerMap`.
+- The loop derives the next turn number from the log each iteration (`lastTurnNumber(session) + 1`) instead of keeping a private counter, so an idle injection's one-shot turn cannot collide with the next real turn's number.
+- The `dsh-invariants` plugin **enforces** the invariant in dev: a `user/message` / `context/message` / `steering/message` appended while no turn is open throws an `InvariantError`.
+
+The serializability invariant is enforced at the same source boundary (`Session.append` throws on non-JSON-serializable data), so "what may enter the log" is now governed in one place rather than discovered downstream by whichever backend happens to be watching.
+
+## Consequences
+
+The turn is now the *single* durability/replay boundary, so a persistence backend's "last `turn/end` = commit point" rule is complete, not merely sufficient: a backend can discard everything after the last `turn/end` with zero risk of losing between-turn context, because there is no between-turn context. `scanLog` stays simple (no partial-turn boundary walk), and an idle background-task notice survives persist + resume.
+
+Costs: `agent.inject()` while idle now writes three log lines instead of one, and the derived history gains a turn that carries only injected context (no assistant output) — `deriveMessages()` already derives purely by event type, so this renders identically. The `injection` trigger is a new on-disk vocabulary value; like every `SessionEventMap`/`TurnTriggerMap` addition it is part of the frozen format. Event ordering within a turn changed (`turn/start` now precedes `user/message`), which is observable to anything that asserted the old order — the loop's own tests were the only such consumers.
+
+The rule is intentionally producer-enforced and dev-checked rather than reader-tolerated: a future backend (SQLite/WAL) inherits the same clean boundary for free, and a plugin that records an event outside a turn fails loudly in dev instead of silently losing data on the next reload.
+
+The invariant also constrains where the loop may record an `error` event. A failure detected while a turn is open is appended INSIDE the turn (before `turn/end`); but a failure that surfaces once the turn is already closed — a rejecting `session/flush` (which runs as the post-`turn/end` durability checkpoint) or a throwing `agent/turn-end` listener (after `closeTurn` already appended `turn/end`) — has no in-turn position left. Appending an `error` there would land it past the last `turn/end`, exactly the crash-tail position a backend discards. So those post-turn failures are reported via the `agent/error` event and the logger only, never as a `SessionEvent`; the turn stays balanced and persistence keeps its buffered events for the next checkpoint. If durable operational diagnostics are ever needed, they belong on a separate telemetry channel, not the replayable session log.

docs/adr/README.md

@@ -28,3 +28,4 @@ Do NOT write an ADR for: a mechanical or local choice (a variable name, a one-fi
 | [0014](0014-doc-sync-enforcement.md) | Doc-sync enforcement (doc code blocks + event taxonomy) | accepted |
 | [0015](0015-structured-error-taxonomy.md) | Structured error taxonomy (HarnessError base) | accepted |
 | [0016](0016-pnpm-over-yarn.md) | pnpm as the package manager instead of Yarn 4 | accepted |
+| [0017](0017-turn-enclosure-invariant.md) | Every session event is enclosed in a turn | accepted |

docs/architecture.md

@@ -90,7 +90,7 @@ A `Session` is an append-only log of typed `SessionEvent`s — the single source
 - `tool/result` → user message carrying a `tool-result` block
 - `context/message`, `steering/message` → user-role messages wrapped in a tagged envelope (`<context source="…">…</context>`) at their chronological position — the "system-reminder" pattern; models distinguish them from real user prompts by the envelope. **TODO(review)**: the real adapters now exist (the original precondition); the envelope still wants a deliberate review against live model behavior (`TODO(review)` in dsh-session).
 
-Replay/fork = `ctx.sessions.create(id, seedEvents)`. Trace/telemetry = listen to `session/event`.
+Replay/fork = `ctx.sessions.create(id, { seed: seedEvents })`. Trace/telemetry = listen to `session/event`.
 
 **Durability seam**: `session/event` is a synchronous notification; persistence plugins buffer (write-behind) and drain at the awaited `session/flush` checkpoint the loop fires at every turn end (see `examples/echo-agent/src/session-jsonl.ts` for the pattern). **TODO**: real persistence backends (JSONL per session dir, sqlite) are a future phase.
 
@@ -114,7 +114,7 @@ Tool schemas are deliberately **part of the assembly**: "what the model is told
 
 - `send(content)` — queued message; starts a turn when idle, else next turn
 - `steer(content)` — mid-turn injection, drained **between steps**; behaves like `send` when idle
-- `inject(content)` — in-session context (`context/message` event) without triggering a turn; the next request sees it (Claude Code attachment / system-reminder analog)
+- `inject(content)` — in-session context (`context/message` event); the next request sees it (Claude Code attachment / system-reminder analog). An inject made while the agent is *running* joins the open turn; an inject while *idle* is wrapped in a one-shot turn (`turn/start{trigger:injection}` → `context/message` → `turn/end`) so every event stays turn-enclosed (see ADR 0017).
 - `abort(reason)` — aborts the in-flight step via `AbortSignal`
 - `session`, `status`, `options`
 
@@ -131,7 +131,7 @@ forever:
   wait for queued messages (idle)
   emit agent/status(running)
   TURN (error-contained — a throwing plugin ends the turn, never the loop):
-    drain queued → session('user/message'…) → 'turn/start' → emit agent/turn-start
+    drain queued → 'turn/start' → session('user/message'…) → emit agent/turn-start
     STEP loop:
       drain steering (late steering from previous step's listeners)
       session('step/start'); emit agent/step-start
@@ -160,7 +160,11 @@ forever:
   emit agent/status(idle) unless more queued
 ```
 
-Error containment: a throwing `agent/turn-continuation` listener or a rejecting `session/flush` ends the **turn** with an `error` event — never the driver loop. An adapter that ends its stream with a `finish {kind:'error'}` or `{kind:'aborted'}` chunk (the in-band error path, for adapters that can't throw mid-stream) is likewise translated into a step error, so the turn ends `error`/`aborted` instead of logging a normal `completed` assistant message. `abort()` is honored mid-stream **and** between tool calls; disposal mid-turn ends the turn with reason `disposed` and emits `agent/status('disposed')`.
+Error containment: a throwing `agent/turn-continuation` listener or a broken step ends the **turn** with an `error` event (appended INSIDE the turn, before `turn/end`) — never the driver loop. An adapter that ends its stream with a `finish {kind:'error'}` or `{kind:'aborted'}` chunk (the in-band error path, for adapters that can't throw mid-stream) is likewise translated into a step error, so the turn ends `error`/`aborted` instead of logging a normal `completed` assistant message. `abort()` is honored mid-stream **and** between tool calls; disposal mid-turn ends the turn with reason `disposed` and emits `agent/status('disposed')`.
+
+A failure that happens once the turn is already closed has no in-turn position for a session `error` event (appending one after `turn/end` would put it past a persistence backend's commit boundary, where it is dropped as a crash tail — ADR 0017). So a rejecting `session/flush` (the post-`turn/end` durability checkpoint) and a throwing `agent/turn-end` listener are reported via `agent/error` + the logger only, NOT as a session event; the turn stays balanced and the backend keeps its buffered events for the next flush.
+
+**Turn-enclosure invariant**: every session event lives inside a turn (between a `turn/start` and its `turn/end`). The loop appends queued `user/message` events *after* `turn/start`, and an idle `agent.inject()` wraps its `context/message` in a one-shot `injection` turn. This makes the turn the single durability/replay boundary: a persistence backend can treat anything after the last `turn/end` as an interrupted-crash tail without risking the loss of legitimately-recorded between-turn context. The `dsh-invariants` plugin enforces it in dev (a message event outside an open turn throws). See ADR 0017.
 
 ### Event taxonomy
 
@@ -221,7 +225,7 @@ Every MVP feature (including the TODO-marked ones), with the mechanism that impl
 | Memory | section provider + tool |
 | Scheduled tasks (cron) | plugin registers model-callable scheduling tools; timer fires → `send(…, {source: {kind: 'cron', …}})` when idle / `inject()` notification when busy |
 | UI (GUI; CLI emits JSONL) | listen `agent/stream-chunk` + `session/event`; input → `send()` |
-| Telemetry / replayable trace | `session/event` → JSONL; replay = `sessions.create(id, seed)` |
+| Telemetry / replayable trace | `session/event` → JSONL; replay = `sessions.create(id, { seed })` |
 | DeepSeek V4 (and other) models | `LlmAdapter` subclass via `registerAdapter`. **Implemented twice**: `dsh-llm-deepseek` (hand-rolled) and `dsh-llm-pi-ai` (pi-ai-backed) |
 | Plugin hot-reload | every registration is a `ctx.effect` → vendored HMR just works |