Skip to content

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

Open

Description

@drexb-ops

Problem

SessionStateRegistry.getOrCreate() inserts a newly created state into the registry before ensureSessionInitialized() finishes. A concurrent caller for the same session can find that entry, observe the session ID, and return it before persisted state, fork translation, message refs, and derived indexes are initialized. Concurrent context transforms and compression tools can also clone or persist overlapping versions of the same session state.

Impact

A same-session race can expose empty or partially initialized state, lose a newer transform through last-writer overwrite, enqueue speculative persistence before a request is validated, or make ACP state diverge from the context actually sent to the model. The OpenCode V2 adapter requires speculative transform/patch validation, which makes this race especially important.

Evidence

  • lib/state/state.ts: SessionStateRegistry.getOrCreate() stores the entry before awaiting initialization.
  • ensureSessionInitialized() can return early when another caller sees the session ID already assigned.
  • lib/state/persistence.ts serializes file writes but cannot retract an invalid snapshot already enqueued.

Proposed fix

Add per-session initialization and mutation serialization shared by context transforms and compression tool execution. Run V2 speculative transforms on a complete cloned runtime state, defer persistence/notifications, validate the outgoing patch, and commit only while holding the same session guard. Preserve the shared compression-timing identity rather than replacing it during rollback.

Verification

Add concurrent initialization and transform/tool tests proving callers cannot observe partial state and an older speculative transaction cannot overwrite a newer committed state.

Related migration: #395.

Activity

  1. added a commit that references this issue on Sep 16, 2026
  2. ranxianglei commented on Sep 16, 2026

    @ranxianglei
    Owner

    🤖 Powered by ework · qwen3.8-27b

    [bot] ✅ Done — fix is in PR #408: #408 (Fixes #404, CI fully green).

    What was implemented

    1. Per-session FIFO mutex — createSessionGuard() in lib/state/state.ts
    A promise-chain mutex keyed by session ID with O(1) constant overhead when uncontended and auto-cleanup of idle entries. It serializes every same-session mutation site that was racing:

    • lib/hooks.ts message-transform handler (the whole pipeline incl. getOrCreate init)
    • lib/hooks.ts system-prompt handler (model context-limit write + save)
    • lib/hooks.ts event handler (per-state duration attach + save, now Promise.allSettled across sessions so one long-held guard can't head-of-line-block others)
    • lib/hooks.ts command handler (/acp stats|context|export getOrCreate path)
    • lib/compress/range.ts + lib/compress/decompress.ts full tool execution (additive-only wrapping, +7/+6 lines)

    2. Init coalescing — ensureSessionInitialized in lib/state/state.ts
    Concurrent callers now share a single in-flight init promise (module-level WeakMap<SessionState, Promise<void>>, checked before the idempotency fast path), so no caller can observe partially-initialized state. This kills the stale-read-modify-write race from the issue: an old transaction can no longer save its stale snapshot over a newer commit, because same-session operations are serialized end-to-end.

    Verification

    • New tests/session-guard.test.ts — 8 tests: FIFO ordering, cross-session independence, release-on-rejection, coalescing regression (asserts state is fully loaded at return time, not just after settle), failed-init coalescing, the exact stale-RMW scenario from the issue report, compression-timing identity through snapshot/restore, and an end-to-end wiring regression running two concurrent transforms through the real createChatMessageTransformHandler asserting strict [enter, exit, enter, exit] lock order plus persisted-state survival.
    • Red/green proven per AGENTS.md §5.7.3: disabling the coalescing makes the regression test fail (bLimitAtReturn === undefined); bypassing the transform guard makes the wiring test fail (events === []). Both restored → green.
    • Full suite: 1271 pass / 0 fail; tsc --noEmit clean; build clean.
    • CI on branch: pr-validation, build, test (Node 22 & 24), e2e — all passing.

    Dual-agent review

    Two independent reviewers (code + tests) both returned APPROVE-WITH-NITS, zero blockers/majors. All actionable findings were fixed in follow-up commit 8340afa: cross-session HOL blocking in the event handler, the unguarded command-handler init path, two line-width violations, doc wording, and the missing concurrent-pipeline wiring test (added as above).

    Follow-ups filed (source-marked)

    Per AGENTS.md §5.1.1.2 I cannot merge PRs — please merge #408 yourself once you're satisfied.


    中文摘要:修复了同一会话的状态初始化与消息转换并发竞争(旧事务可用过期快照覆盖新提交),通过每会话 FIFO 互斥锁 + 初始化去重(inflight 合并)实现,新增 8 个测试并做了红绿验证,双人 review 均通过、CI 全绿,可以合并;另发现两个遗留问题已分别立为 #410、#411。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions