fix: serialize same-session state initialization and transforms (#404)

devlog/2026-09-16_serialize-session-init-transforms/DESIGN.md

@@ -0,0 +1,52 @@
+# DESIGN - Serialize same-session state initialization and transforms
+
+- Task ID: `2026-09-16_serialize-session-init-transforms`
+- Home Repo: `opencode-acp`
+- Created: 2026-09-16
+- Status: Accepted
+
+## 1. Problem Statement
+
+- **What problem are we solving?** Issue #404: `SessionState` is mutable per-session state shared across many async entry points (message transform, compress/decompress tools, event hook saves, system hook limit writes). Nothing serialized them across `await`s, so (a) concurrent initializations could observe partially loaded state, and (b) a stale in-flight transaction could persist over a newer committed one.
+- **Why now?**: Reproduced deterministically with a slow-host mock; same failure shape as the baseline-reset class of bugs (#5.7.3) — silent cross-turn state corruption invisible to single-turn tests.
+
+## 2. Goals & Non-Goals
+
+- **Goals**: One-writer-at-a-time per session across ALL mutation paths; init coalescing so N racers pay for 1 initialization; zero steady-state overhead; no cross-session blocking; no persisted-format changes.
+- **Non-Goals**: Cross-process locking; redesigning the debounced save queue (#384); v2 runtime support (#395 → billion-context#735).
+
+## 3. Current Architecture
+
+- **How it works today**: `SessionStateRegistry.states: Map<sessionId, SessionState>`. The transform hook calls `getOrCreate` → `ensureSessionInitialized` (idempotency via `state.sessionId === sessionId`, which was assigned synchronously pre-first-await), then a long mutate pipeline, then `saveContext`/`saveSessionState`. Tools call `prepareSession` → `ensureSessionInitialized` directly on the registry state. Event/system hooks grab state by id and write+save.
+- **Pain points**: Every step above can interleave with another same-session trigger at any await boundary.
+
+## 4. Proposed Architecture
+
+- **Overview**:
+  ```
+  trigger (transform | tool | event | system)
+        │
+        ▼
+  registry.withSessionGuard(sessionId, fn)   ← FIFO promise-chain mutex
+        │  (await previous chain entry, then run fn exclusively)
+        ▼
+  fn: getOrCreate/prepareSession → mutate → save   ← single writer
+  ```
+- **Key components**:
+  - `createSessionGuard()` (`lib/state/state.ts`): returns `{ run(sessionId, fn) }`; internal `Map<sessionId, Promise>` chain; each task enqueues before awaiting its predecessor; map entry deleted when the tail task completes → empty when idle. Rejections propagate to the caller but never poison the chain (predecessor awaited via `.catch(()=>{})`).
+  - `ensureSessionInitialized`: now a coalescing wrapper over private `runSessionInitialization`. Module-level `WeakMap<SessionState, Promise<void>>` keyed by the STATE OBJECT (not session id) so soft-cap eviction + recreation starts fresh rather than awaiting a stale promise.
+- **Data flow**: unchanged apart from ordering guarantees. Snapshot/restore (`restoreCompressionState`) already preserves `compressionTiming` identity — locked in by test 7.
+
+## 5. Critical Subtleties (load-bearing)
+
+1. **Inflight check precedes fast path.** `runSessionInitialization` assigns `state.sessionId` synchronously before its first await. If the `state.sessionId === sessionId` early-return ran first, racing callers would skip coalescing and return mid-init — the original bug. Order: inflight check → fast path → start+track.
+2. **Guard granularity = whole transaction, not individual mutations.** A read-modify-write must be atomic end-to-end; wrapping only `saveSessionState` would still allow stale reads during the pipeline. Hence the transform body became `runPipeline(state)` invoked inside one guard acquisition.
+3. **Ephemeral transform branch unguarded.** When no user message exists, the handler builds a throwaway `createSessionState()` per request — independent objects, nothing shared, no lock needed (locking on a synthetic key would only add overhead).
+4. **Tools hold the guard across `prepareSession` (incl. permission `ask`).** In OpenCode v1 a tool runs while the session turn is paused, so no same-session transform is concurrently in flight — the lock is free in practice; if a trigger did interleave, serialization is exactly the desired behavior. No deadlock possible: guards are non-reentrant and no code path acquires two locks.
+5. **Event handler skips states without sessionId** inside the guarded section (defensive; such states have no persistence identity).
+
+## 6. Alternatives Considered
+
+- **Lock only around save**: insufficient — stale READS during the pipeline already corrupted the snapshot being saved.
+- **Module-level lock instead of registry method**: rejected — tools receive `registry` via `ToolFactoryContext`; a module singleton would duplicate ownership and complicate test stubs. Test stubs compose the real factory (`tests/registry-stub.ts`) to avoid drift.
+- **Reentrant guard / ref-counting**: rejected — no nesting exists today; reentrancy invites future misuse.

devlog/2026-09-16_serialize-session-init-transforms/REQ.md

@@ -0,0 +1,50 @@
+# REQ - Serialize same-session state initialization and transforms
+
+- Task ID: `2026-09-16_serialize-session-init-transforms`
+- Home Repo: `opencode-acp`
+- Created: 2026-09-16
+- Status: Done
+- Priority: P1
+- Owner: ranxianglei
+- References: https://github.com/ranxianglei/opencode-acp/issues/404
+
+## 1. Background & Problem Statement
+
+- **Context**: ACP keeps mutable per-session `SessionState` in a process-wide registry. Every LLM request runs the `messages.transform` hook (init + mutate + persist), and the `compress`/`decompress` tools, the `event` hook (duration attach + save) and the `system.transform` hook (model-limit write + save) also read/mutate/persist the same state.
+- **Current behavior (symptom)**: Node is single-threaded but async work interleaves at every `await`. Two triggers:
+  1. **Init race** — `ensureSessionInitialized` assigned `state.sessionId = sessionId` synchronously before its first await. A concurrent caller for the same session then hit the idempotency fast path and returned *partially initialized* state (persisted blocks/messageIds not yet loaded).
+  2. **Stale transaction** — a transform that awaits mid-pipeline can resume after a newer transform already committed; last-write-wins persistence then stores the stale snapshot over committed state (lost updates / corrupted prune state).
+- **Expected behavior**: All same-session state work is serialized; concurrent initializations coalesce into one; no caller ever observes partially initialized state; committed state is never overwritten by a stale snapshot.
+- **Impact**: Intermittent data corruption in sessions under concurrency (subagent fan-out, retries, fast successive requests): lost compression blocks, wrong nudge baselines, duplicated message refs.
+
+## 2. Reproduction (if applicable)
+
+- **Environment**: Node 22/24, any OS — the race is in-process, timing-dependent.
+- **Minimal reproduction steps**:
+  1) Seed persisted state for session S (e.g. `modelContextLimit`).
+  2) Fire two `getOrCreate(client, "S", ...)` calls concurrently with a slow host (`client.session.get` delayed ~25 ms).
+  3) The second caller returns before `loadSessionState` completes → reads `modelContextLimit === undefined` instead of the persisted value.
+- **Relevant configuration**: none — default config exercises the path.
+
+## 3. Constraints & Non-Goals
+
+- **Constraints**:
+  - Backward compatibility: persisted state format unchanged; exported API only grows (`createSessionGuard`, `SessionGuard`, `registry.withSessionGuard`); internal `dcp` naming untouched.
+  - Performance requirements: O(1) constant overhead on the steady-state path (one Map get/set/delete plus a microtask hop per guarded request when nothing else is in flight); different sessions must never block each other.
+  - Resource limits: lock map must be empty when idle (no per-session leak); init-coalescing map keyed by state object (WeakMap) so eviction + recreation starts fresh.
+- **Non-Goals** (explicitly out of scope):
+  - Cross-process locking (out of scope for an in-process plugin).
+  - Changing `saveSessionState`'s existing debounced queue semantics (#384).
+  - OpenCode v2 runtime support (v2 moved to billion-context#735 per #395).
+
+## 4. Acceptance Criteria (must be testable)
+
+- **Correctness**:
+  - [x] Concurrent `getOrCreate` calls for one session coalesce into a single initialization; every waiter observes fully loaded state at return time.
+  - [x] Same-session tasks run FIFO through the guard; other sessions are unaffected.
+  - [x] Guard releases on rejection; subsequent tasks proceed.
+  - [x] Serialized read-modify-write: a stale transaction cannot persist over a newer committed value.
+  - [x] Regression test FAILS when coalescing is disabled (verified by temporarily disabling it).
+- **Performance / Stability**:
+  - [x] Full suite green: 1270 tests, 0 failures (was 1263 before this change added 7).
+  - [x] `tsc --noEmit` clean; `npm run build` clean.

devlog/2026-09-16_serialize-session-init-transforms/WORKLOG.md

@@ -0,0 +1,65 @@
+# WORKLOG - Serialize same-session state initialization and transforms
+
+- Task ID: `2026-09-16_serialize-session-init-transforms`
+- Home Repo: `opencode-acp`
+- Status: Done
+- Updated: 2026-09-16 (dual-review fix round)
+
+## 1. Summary
+
+- **What was done** (1–3 sentences): Added a per-session FIFO mutex (`createSessionGuard` / `registry.withSessionGuard`) and in-flight init coalescing (`inflightInits` WeakMap) to `lib/state/state.ts`; wrapped every same-session mutation path — message transform pipeline, compress/decompress tool execution, event-hook duration attach+save, system-hook model-limit write+save — in the guard.
+- **Why** (1–3 sentences): Same-session async work interleaved at awaits: racing initializations handed out partially loaded state, and stale transactions could persist over newer committed state (issue #404). Serialization makes the single-writer invariant hold across awaits.
+- **Behavior / compatibility changes**: No persisted-format or API breakage. New exported symbols only. Steady-state fast paths unchanged (guard map empty when idle; init fast path preserved behind the inflight check). One deliberate ordering change: the init inflight check now precedes the `sessionId` fast path (required for correctness — see DESIGN §5).
+- **Risk level**: Medium (touches the core request pipeline; mitigated by full-suite green + new regression tests + FIFO guard semantics that cannot deadlock across sessions).
+
+## 2. Change Log
+
+### Commits
+
+| Commit | Description |
+|--------|-------------|
+| `571ccf0` | fix: serialize same-session state initialization and transforms (#404) |
+| `see-branch` | docs: fill devlog commit SHAs (self-referential SHA omitted) |
+| `see-branch` | review fixes: transform wiring regression test, event-handler cross-session parallelism, command-handler guard, prettier width wraps, REQ wording |
+
+### Key Files
+
+- `lib/state/state.ts` — `createSessionGuard()`/`SessionGuard` factory (FIFO chain mutex); `ensureSessionInitialized` split into coalescing wrapper + `runSessionInitialization`; registry exposes `withSessionGuard` (field intentionally not `readonly` so the wiring test can wrap it as an observation seam).
+- `lib/hooks.ts` — system hook limit write wrapped in guard; message transform restructured into `runPipeline(state)` closure invoked inside `registry.withSessionGuard(sessionID, ...)` (session branch only; ephemeral branch unguarded by design); event handler per-state apply+save guarded per session AND run concurrently across sessions (`Promise.allSettled`) so one long-held guard cannot head-of-line-block other sessions' duration attachment; `/acp` command handler `getOrCreate`+permission-sync kept under the guard (getOrCreate can initialize + persist).
+- `lib/compress/range.ts` — entire `execute` body from `prepareSession` onward wrapped in `factoryCtx.registry.withSessionGuard(toolCtx.sessionID, ...)`.
+- `lib/compress/decompress.ts` — same wrap from `prepareDecompressSession` onward.
+- `tests/registry-stub.ts` — both stubs compose the real `createSessionGuard()` (no drift).
+- `tests/session-guard.test.ts` — 8 new tests (FIFO order, cross-session independence, release-on-reject, coalescing regression, failed-init semantics, stale read-modify-write, timing-identity preservation, **end-to-end transform wiring**: two concurrent same-session requests through the real `createChatMessageTransformHandler` must produce strictly nested guard enter/exit pairs).
+
+## 3. Design & Implementation Notes
+
+- **Entry point / key function**: `createSessionGuard()` in `lib/state/state.ts` — chain-based promise queue keyed by sessionId; entry deleted when the tail task finishes (no idle leak).
+- **Key logic explanation**: See DESIGN.md. Critical subtlety: the init inflight check must run BEFORE the `state.sessionId === sessionId` fast path, because `runSessionInitialization` assigns `sessionId` synchronously before its first await — otherwise racing callers early-return mid-init (the original bug shape).
+
+## 4. Testing & Verification
+
+### Build & Test Commands
+
+```sh
+npx tsc --noEmit          # clean
+npm run build             # clean (tsup + d.ts)
+npm run test              # 1271 pass / 0 fail
+node --import tsx --test tests/session-guard.test.ts   # 8/8
+```
+
+### Regression verification (per AGENTS.md §5.7.3)
+
+Temporarily disabled the inflight check in `ensureSessionInitialized` → `tests/session-guard.test.ts` test 4 ("concurrent getOrCreate coalesces…") FAILED with `bLimitAtReturn === undefined` (the pre-fix partial-snapshot observation). Re-enabled → all green.
+
+Wiring test (test 8): temporarily bypassed the transform-branch guard in `lib/hooks.ts` (body ran unguarded) → test 8 FAILED (`events === []`, no serialization observed). Restored → green. Reviewer 2 independently repeated the coalescing red/green check.
+
+### Dual-agent review (AGENTS.md §5.3 / §5.6)
+
+Both reviewers: **APPROVE-WITH-NITS**, zero blockers/majors; each ran tsc + full suite independently (1271/1271). Fix round applied directly to the branch:
+
+- MAJOR (tests): missing end-to-end wiring coverage → added test 8 (above).
+- MINOR: event handler awaited each session's guard sequentially (head-of-line blocking) → `Promise.allSettled` across sessions, per-session containment preserved.
+- MINOR: command handler called `getOrCreate` outside any guard → wrapped with permission-sync under one acquisition.
+- MINOR: two new-code prettier width violations (`state.ts`, `hooks.ts`) → wrapped.
+- NIT: REQ.md "zero overhead" claim imprecise → "O(1) constant overhead".
+- Follow-ups filed separately (source-marked per §5.1.3): guard leak if a host abandons (without rejecting) tool execution while the guard spans a permission ask; sticky failed-init (a transient first-request init failure permanently suppresses persisted-state load for that session until process restart — pre-existing behavior, preserved).

lib/compress/decompress.ts

@@ -275,6 +275,10 @@ export function createDecompressTool(factoryCtx: ToolFactoryContext): ReturnType
         args: buildSchema(),
         async execute(args, toolCtx) {
             const ctx = resolveToolContext(factoryCtx, toolCtx.sessionID)
+            // [Issue #404] Serialize the full prepare→mutate→finalize transaction under the
+            // per-session guard (see compress/range.ts for rationale). The body intentionally
+            // keeps its original indentation inside `run` to keep this change additive-only.
+            const run = async () => {
             const { rawMessages } = await prepareDecompressSession(ctx, toolCtx)
 
             const effectiveLimitBefore = resolveEffectiveContextLimit(ctx.state, ctx.config)
@@ -407,6 +411,8 @@ export function createDecompressTool(factoryCtx: ToolFactoryContext): ReturnType
             })
 
             return lines.join("\n")
+            }
+            return factoryCtx.registry.withSessionGuard(toolCtx.sessionID, () => run())
         },
     })
 }

lib/compress/range.ts

@@ -119,6 +119,11 @@ export function createCompressRangeTool(factoryCtx: ToolFactoryContext): ReturnT
                     ? (toolCtx as unknown as { callID: string }).callID
                     : undefined
 
+            // [Issue #404] Serialize the full prepare→mutate→finalize transaction under the
+            // per-session guard so it cannot interleave with concurrent same-session work
+            // (message transforms, event-hook saves). The body intentionally keeps its
+            // original indentation inside `run` to keep this change additive-only in diff.
+            const run = async () => {
             const { rawMessages, searchContext } = await prepareSession(
                 ctx0,
                 toolCtx,
@@ -366,6 +371,8 @@ export function createCompressRangeTool(factoryCtx: ToolFactoryContext): ReturnT
                 ? `\n⚠️ acknowledgeRisk was ignored: no quality gate rejection was pending, so quality checks ran normally. Only pass it when retrying immediately after a quality gate rejection.\n`
                 : ""
             return `Compressed ${totalCompressedMessages} messages into ${COMPRESSED_BLOCK_HEADER}.${skippedNote}${ackNote}\nIMPORTANT: This was an automatic context compression. You MUST continue your previous task exactly where you left off. Do NOT ask the user what to do next.\n💡 Tip: Use search_context('keyword') to find compressed content when you need it later.`
+            }
+            return factoryCtx.registry.withSessionGuard(toolCtx.sessionID, () => run())
         },
     })
 }