Skip to content

fix(session): serialize state read-modify-write to prevent lost updates#55

Merged
i8ramin merged 1 commit into
mainfrom
fix/serialize-concurrent-state
Jul 14, 2026
Merged

fix(session): serialize state read-modify-write to prevent lost updates#55
i8ramin merged 1 commit into
mainfrom
fix/serialize-concurrent-state

Conversation

@i8ramin

@i8ramin i8ramin commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Addresses item 2 of #52: concurrent evaluate() / ingest() calls could lose-update the session store.

The race

session.ingest#withState does an async load → mutate → save (src/session.ts). The content adapter's own hooks coalesce via a pending flag, so through them evaluations never overlap. But #51 now recommends adopters drive controller.evaluate() from their own navigation signal, and those calls are not coalesced. Two overlapping evaluations both load the pre-write snapshot; the second save clobbers the first, dropping a just-recorded stand-down record. Worst case that flips a host from standDown: true back to false — activation on an already-attributed page, the exact thing standdown exists to prevent.

Fix

A small FIFO mutex on the session:

  • #serialize(task) chains onto a #stateLock promise so each state operation runs to completion before the next starts.
  • #withState, #failClosedWithAudit, and exportAuditLog all run inside it.

Safe against deadlock: every fn callback passed to #withState is synchronous and self-contained, and no serialized method calls another. Bonus: read-after-write consistency — a shouldStandDown queued after an ingest now observes that ingest's committed state instead of possibly loading a snapshot from before the save landed.

Test

New test fires two overlapping ingests (different hosts) through a store that yields on every load/save to force interleaving, then asserts both records persist. Verified it fails without the lock (one host's record is dropped: expected undefined to be defined) and passes with it.

Verification

typecheck + lint clean, 163 tests pass (was 162 + the new one), policies-cite-check passes, build ok. No API change; behavior change is limited to ordering.

Leaves item 3 of #52 (session-exemption TTL / value-scoping) as the remaining open design question.

session.ingest -> #withState does an async load -> mutate -> save. The adapter's
own hooks coalesce via a `pending` flag, but once callers drive evaluate() /
ingest() from their own navigation signal (as the content docs now recommend),
two evaluations can overlap: both load the pre-write snapshot and the second
save clobbers the first, dropping a just-recorded stand-down. That can flip a
host from standDown:true back to false — an activation on an already-attributed
page.

Add a FIFO mutex (#serialize) and route #withState, #failClosedWithAudit, and
exportAuditLog through it, so state operations run to completion in order. Also
gives read-after-write consistency: a shouldStandDown queued after an ingest now
observes that ingest's committed state.

Adds a concurrency test (verified to fail without the lock: one host's record is
lost; passes with it).

Addresses item 2 of #52.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PXZmuMFkq8dE2e2KLXvitx
@i8ramin
i8ramin merged commit 2b35d89 into main Jul 14, 2026
2 checks passed
@i8ramin
i8ramin deleted the fix/serialize-concurrent-state branch July 14, 2026 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant