Skip to content

fix(core): restore the SQLite lock-wait window - #155

Merged
brentyates-swx merged 9 commits into
swxtchfrom
fm/opencode-busy-window-153
Oct 10, 2026
Merged

brentyates-swx merged 9 commits into
swxtchfrom
fm/opencode-busy-window-153

Conversation

@brentyates-swx

Copy link
Copy Markdown

Closes #153
Closes #154

Problem

#88 replaced the 5s native busy_timeout with a 5ms native wait plus four async backoffs, so every SQLite statement gave up after about 250ms. On the shared fleet store (7+ writer processes), any write lock held longer than that failed accepted prompts with Database is locked (SQLITE_BUSY) from SessionQueue.consume.

Fix

  • Restored window (Lock wait cut from 5s to ~250ms: fleet prompts fail with SQLITE_BUSY on the shared store #153). sqlite.bun.ts, sqlite.node.ts and effect-sqlite-node keep the short native attempt, so bun:sqlite never blocks the event loop. A statement now retries asynchronously with capped, jittered exponential backoff, bounded by Schedule.during to 5s of elapsed contention rather than an attempt count. The retry still runs inside the acquired connection permit. Exhaustion still returns the classified LockTimeoutError with its native cause, and nonretryable errors still fail at once.
  • Stale snapshots (fix(core): don't retry SQLITE_BUSY_SNAPSHOT inside a transaction for the whole lock window #154). SQLITE_BUSY_SNAPSHOT (bun code, node:sqlite errcode 517) can never succeed on a statement retry inside a transaction. All three clients now stop retrying on it, so it no longer holds the connection for the whole window. It still surfaces as LockTimeoutError with its cause. In a contention experiment, about 1,500 autocommit writes against three competing writers produced only plain SQLITE_BUSY, never the snapshot code.

Tests (packages/core/test/sqlite-busy.test.ts)

  • Extended hold: a real write lock in a second process is held past the old window after the contender has met it, on the Bun production layer, core Node and standalone Node, through run and values. During the hold, an unrelated HTTP request must be served and an event-loop stall monitor must stay under its bound. After release, the statement must succeed once with its returned rows.
  • Exhaustion: asserts the elapsed window instead of the old attempt count. A backstop above the window catches a retry that never stops, and independent work must succeed after release. The Bun responsiveness case no longer depends on node:sqlite.
  • Stale snapshot: one real two-connection WAL test per client requires a fast failure, a single execution and no write.
  • Red proofs: the old schedule fails all 11 rewritten tests within their bounds. A 5s synchronous native wait fails the hold tests on the stall bound. Removing the snapshot exclusion fails the snapshot tests on time (~5s) and on execution count (53 instead of 1).
  • opencode consumers: the lock-fixture backstops and the affected bounds are raised so the real driver exhausts first. The held-lock terminal-recovery test now takes about 72s, because each refusal waits out a full driver window.

Results: focused core 23/23, processor-effect 29/29, httpapi-error-middleware 7/7, typecheck clean in core, effect-sqlite-node and opencode.

🤖 Generated with Claude Code

https://claude.ai/code/session_01BQzKnpxtC4SMFP6UwY4C1t

Refs #153.

1. Keep the 5ms native busy_timeout on every client so bun:sqlite never blocks
   the event loop for long. Replace the four-attempt 25-75ms schedule in
   sqlite.bun.ts, sqlite.node.ts and effect-sqlite-node with a capped, jittered
   exponential backoff bounded by Schedule.during: each statement retries
   asynchronously for 5 seconds of elapsed contention, the old native window,
   then returns its classified LockTimeoutError. Retries stay inside the
   acquired connection permit, and nonretryable errors still fail immediately.

2. In packages/core/test/sqlite-busy.test.ts, rewrite the release-after-
   contention tests to hold a real write lock for over a second after the
   contender's first attempt, on the Bun production layer, core Node and
   standalone Node, for run and values. Probe an unrelated HTTP request during
   that hold, then require success, the returned rows and one persisted write.
   Move the exhaustion tests from the attempt count and the 1s bound to the
   elapsed window, raise their backstops above it, and check that independent
   work succeeds after release.

3. Raise the backstops in packages/opencode/test/fixture/sqlite-lock.ts and the
   affected consumer test bounds so the real driver exhausts first.

4. Prove each rewritten regression goes red on the old schedule, then run the
   focused core and opencode tests, typechecks, lint and the identity gate. The
   CHANGESET entry needs the PR number, so it lands at publication.

Claude-Session: https://claude.ai/code/session_01BQzKnpxtC4SMFP6UwY4C1t
Every runtime client gave up on a busy statement after four backoffs, about
250ms, so a competing writer on the shared store failed accepted prompts. Each
statement now retries asynchronously between 5ms native attempts for 5 seconds
of elapsed contention, with capped, jittered exponential backoff, before it
returns the classified lock failure.

The release-after-contention tests now hold a real write lock for 1.5s after the
contender meets it, probe an unrelated request during the hold, and cover the
Bun production layer, core Node and standalone Node through run and values. The
exhaustion tests assert the elapsed window instead of the old attempt count and
1s bound, and Bun's responsiveness case no longer depends on node:sqlite.

Refs #153.

Claude-Session: https://claude.ai/code/session_01BQzKnpxtC4SMFP6UwY4C1t
The success-path probe could slip through a backoff gap between two long
synchronous native waits. Track the longest gap between 5ms ticks across the
contention, counting the gap still open when it is read, so any multi-second
synchronous wait fails the extended-hold and persistent-lock tests.

Claude-Session: https://claude.ai/code/session_01BQzKnpxtC4SMFP6UwY4C1t
The produced-lock fixtures and the SQLITE_LOCKED middleware case now give the
real driver room to exhaust its 5s retry window before their own backstop. The
refused terminal-write recovery test takes a full driver window per refusal, so
its wait for the second progress report and its test bound grow to match.

Refs #153.

Claude-Session: https://claude.ai/code/session_01BQzKnpxtC4SMFP6UwY4C1t
The constants own the retry window, backoff and backstop values, so the
comments now state only the goal: restore the old busy_timeout tolerance while
waiting asynchronously between short native attempts, and catch a retry that
never stops.

Claude-Session: https://claude.ai/code/session_01BQzKnpxtC4SMFP6UwY4C1t
A write inside a transaction whose read snapshot went stale fails with
SQLITE_BUSY_SNAPSHOT on every retry; only restarting the transaction recovers
it. With the restored lock window, the statement retry spun on it for the whole
window while holding the connection. All three clients now stop retrying on
that code and still return the classified LockTimeoutError with its cause.

A real two-connection WAL test per client reads inside a transaction, commits
from another connection, then writes, and requires a fast failure with a single
execution and no write. The shared attempt counter now counts executions of
the prepared statement, since clients retry the execution, not the prepare.

Closes #154.

Claude-Session: https://claude.ai/code/session_01BQzKnpxtC4SMFP6UwY4C1t
Effect.retry builds its schedule's step function before the first attempt, so
every statement, including the many that succeed at once, paid for the deeper
elapsed-time schedule. That moved where fibers reach Effect's operation budget
and yield. Two SessionRunnerLLM tests that check their results after a single
Effect.yieldNow then saw the follow-up drain unfinished: "runs steering input
accepted while the active provider turn fails" and "durably closes partial tool
input when the provider stream is interrupted". With no SQLite error at all,
they fail deterministically on the restored window and pass on the base. With
the budget lifted they pass, and on the base they fail at a budget of 64.

Each client now runs the statement once and enters the retry only on a
retryable failure. The retry re-attempts at once, then backs off, so the
window runs from the first failed retry. Successful statements build no
schedule.

Refs #153.

Claude-Session: https://claude.ai/code/session_01BQzKnpxtC4SMFP6UwY4C1t
@brentyates-swx

Copy link
Copy Markdown
Author

Cause of the unit (linux) failure (run 38005391141): the retry window change, not the snapshot change, and not a flake.

  • Reproduced: both SessionRunnerLLM failures reproduce 3/3 locally on the previous head (dec90796ec). They pass 88/88 with the base sqlite.bun.ts. With the snapshot exclusion removed but the window kept, they still fail. Of the window's parts, removing Schedule.during makes them pass and removing the jitter does not.
  • No SQLite error involved: an instrumented retry predicate showed no statement failing in those tests. Effect.retry builds its schedule's step function before the first attempt, so every successful statement paid for the deeper elapsed-time schedule. That moved where fibers reach Effect's operation budget and yield.
  • Budget evidence: both tests check their result after a single Effect.yieldNow. With MaxOpsBeforeYield lifted, the old head passes 88/88. The base itself fails 3 of them at a budget of 64.
  • Fix (8bc0fde500): each client runs the statement once and enters the retry only on a retryable failure. That retry re-attempts at once, then backs off within the same window. Successful statements now build no schedule, which is also less work on the hot path than base.
  • Verified: core session-runner 88/88 three runs in a row, full core suite 1165/1165, sqlite-busy 23/23, opencode processor-effect and httpapi-error-middleware 36/36, typecheck clean.
  • Follow-up: the yield-budget dependence of those tests is filed as test(core): SessionRunnerLLM assertions depend on the fiber yield budget #156.

@brentyates-swx
brentyates-swx merged commit 8cff350 into swxtch Oct 10, 2026
3 checks passed
@brentyates-swx
brentyates-swx deleted the fm/opencode-busy-window-153 branch October 10, 2026 00:41
@brentyates-swx

Copy link
Copy Markdown
Author

Claimed at dispatch by firstmate. Work is under way locally; a PR appears once the review converges, so the absence of one here is expected.

  • task: opencode-revert-155
  • branch: fm/opencode-revert-155
  • session: 92cf41b0-348a-47a5-a603-7dc4edc59574
  • launch cwd: /home/byates/fm-llmrouter (firstmate home; the dispatching session's launch cwd was not discoverable)

brentyates-swx added a commit that referenced this pull request Oct 10, 2026
* Revert "fix(core): restore the SQLite lock-wait window (#155)"

This reverts commit 8cff350.

Reason: the multi-writer failure in data/opencode-fixed-binary-155/report.md. Refs #162

Claude-Session: https://claude.ai/code/session_019PSxF9tfYzQesA13oxDTHe

* docs: record the #155 revert in CHANGESET.md

Refs #163

Claude-Session: https://claude.ai/code/session_019PSxF9tfYzQesA13oxDTHe
brentyates-swx added a commit that referenced this pull request Oct 10, 2026
Brings in the #155 revert (#163) before this PR merges. The reviewed
head 394ec55 stays on the first parent.

Claude-Session: https://claude.ai/code/session_015hTsruVJZ4rc4nVuPj1KgU
brentyates-swx added a commit that referenced this pull request Oct 10, 2026
#194)

* docs(core): plan the SQLite snapshot-loss fix for #162

Plan for fixing forward accepted-prompt loss under concurrent writers on top
of the reverted base (60f779b). Evidence: data/opencode-fixed-binary-155/
report.md sections 5 and 6, and data/opencode-revert-155/report.md section 7.

Mechanism, from the code (not yet measured):

- The admission transaction is SessionQueue writable() in
  packages/opencode/src/session/queue.ts. It reads first (EventV2.assertWritableIn)
  and then writes (sequence upsert and queue insert). It begins as a deferred
  transaction, so its read snapshot exists before its write lock is taken.
- A plain-busy wait inside a deferred transaction can only end in
  SQLITE_BUSY_SNAPSHOT when another connection commits during the wait: the
  snapshot goes stale, and the driver does not restart the transaction. This is
  the mechanism that predicts the report's section 6.3 observation that the
  snapshot class grew when the wait was added. It is not measured yet.
- The admission and promotion writes are the deferred read-then-write
  transactions found on the accepted-prompt path by grepping its call sites.
  The EventV2 append that persists the user message already begins with
  behavior immediate (event.ts:467). The proof decides whether that is complete.

Correction to #162. Its option 2 names Client.make beginTransaction, which the
production path never reaches. opencode runs transactions through
effect-drizzle-sqlite, whose withTransaction issues "begin <behavior>" itself
(packages/effect-drizzle-sqlite/src/effect-sqlite/session.ts), default deferred.
Effect's SqlClient.withTransaction, which reads beginTransaction, has no caller
in opencode. The mechanism that works is the existing per-transaction option,
behavior immediate, which the codebase already uses in event.ts and project.ts.

Choice: (a) BEGIN IMMEDIATE on write-capable transactions, not (b) restart.
- (a) takes the write lock before any read snapshot exists, so the failing
  class cannot occur for that transaction. The lock wait happens at BEGIN,
  where a retry is safe because nothing has been read yet.
- (b) restarts a body that has already run. The admission body is pure SQL,
  but restart needs a per-site audit of every transaction body for duplicated
  side effects, and it leaves the stale-snapshot race in place.

Steps:

1. Restore the #155 plain-busy wait and the #154 fail-fast on
   SQLITE_BUSY_SNAPSHOT in packages/core/src/database/sqlite.bun.ts,
   sqlite.node.ts and packages/effect-sqlite-node/src/index.ts. The three
   copies stay identical, as #162 requires. Dedupe stays with #165.
   sqlite.node.ts is outside the brief's named scope; it is named here
   because it is the third copy of the same rule.
2. Begin writable() with behavior immediate in queue.ts. This covers every
   queue write: the callers are admit, reserve, withdraw, restore, and update.
3. Restore the #155 test versions that cover the wait and the fail-fast:
   packages/core/test/sqlite-busy.test.ts, and the opencode lock fixture
   and consumer bounds, which #155 raised for the 5 s window.
4. Add a regression at the admission path. A second connection holds the
   write lock across the admission read, then commits. The admission must
   succeed and persist its row. Red proof: revert writable() to deferred.
   Also run the restored core tests against the reverted driver for red.
5. Proofs, in the brief's order: focused suites with CI commands; the
   candidate against the reverted parent, concurrently on separate DBs, same
   load, several pairs, raw counts; fresh-store proof. The parent is the
   reverted build (tree equal to bf5180a).

Not in this change: session queue consumption (#167); the CHANGESET entry
(needs the PR number at publication); the snapshot-tracking PATH issue (#164).

Claude-Session: https://claude.ai/code/session_01LrS7BTRTnc6eYrSxBFoWAo

* fix(core): restore the SQLite lock-wait window for busy statements

Restores the plain-busy wait that #155 added. A statement that meets a lock
retries asynchronously for 5 seconds of elapsed contention. A stale-snapshot
failure (SQLITE_BUSY_SNAPSHOT) is still not retried (#154). The three retry
copies stay identical in sqlite.bun.ts, sqlite.node.ts and effect-sqlite-node.

Red on the reverted driver: the restored sqlite-busy suite fails 14 of 23.
The 250 ms window cannot absorb a 1.5 s hold, and a stale-snapshot statement
is executed more than once. Green with the window: 23 of 23. The opencode
lock-consumer suites pass 36 of 36 with their #155 bounds.

Refs #162

Claude-Session: https://claude.ai/code/session_01LrS7BTRTnc6eYrSxBFoWAo

* fix(opencode): admit queue writes under BEGIN IMMEDIATE

SessionQueue writable() runs its read (EventV2.assertWritableIn) and then its
write in one transaction. Begun deferred, that transaction pins a read
snapshot before it takes the write lock. If another writer commits while the
write waits on the lock, the snapshot goes stale, and the write fails with
SQLITE_BUSY_SNAPSHOT, which is not retried. The prompt was already accepted
(204), so it was lost with no user message.

With behavior immediate the write lock is taken at BEGIN, before any read.
The lock wait then happens where a retry is safe, and no snapshot can go
stale inside the transaction. The restored lock-wait window absorbs the wait.

The regression holds the write lock from a second connection across the
admission read, then commits. Red with the deferred writable: the sequence
upsert fails with SQLITE_BUSY_SNAPSHOT after the commit. Green with immediate.

Refs #162

Claude-Session: https://claude.ai/code/session_01LrS7BTRTnc6eYrSxBFoWAo

* refactor(core): give the SQLite busy-retry gate one owner

The busy-retry schedule and the stale-snapshot gate were copied into three
statement paths: sqlite.bun.ts, sqlite.node.ts and effect-sqlite-node. #155
changed all three, so a rule change had to land three times, and nothing said
which copy was authoritative.

effect-sqlite-node/src/retry.ts now owns both. It exports retryLocked, which
every driver routes its statements through, and it recognizes the stale-snapshot
class in both native error shapes: bun:sqlite reports it by code and node:sqlite
by errcode. The module imports only effect, so the Bun binary can consume it
without loading node:sqlite. The drivers no longer carry a schedule or a gate.

Behavior is unchanged. The core busy suite is 23 of 23 across the three
clients. Removing the snapshot exclusion from the gate turns the three
stale-snapshot tests red. Dropping the node error shape turns two of them
red (core-node, standalone-node), and dropping the bun shape turns one red
(bun). Removing the retry turns the six lock-wait tests red.

statementError, the native error classification, is still copied in the same
three files. It is not part of this change, and #87 tracks it.

Refs #162
Refs #165

Claude-Session: https://claude.ai/code/session_01LrS7BTRTnc6eYrSxBFoWAo

* test(opencode): wait on the admission's busy attempt, not a sleep

Review round 1. The admission test held the write lock for a fixed 1500 ms,
then committed. An admission that started late ran after the commit and
passed even with a deferred write, so the test did not prove the fix.

The test now counts the admission's statement attempts at bun:sqlite's query
path, as the core sqlite-busy test does, and records the native codes of the
attempts that throw SQLITE_BUSY. It commits only after the admission has met
the held lock, asserts the admission is still pending, then checks the
persisted row. The timeout is a backstop, not the signal. On failure, the
message lists the admission's attempts and busy codes.

Checked locally:
- writable() immediate: pass, with and without a 1600 ms delay before the fork
- writable() reverted to deferred: fail with SQLITE_BUSY_SNAPSHOT after the
  commit, with and without the 1600 ms delay

No product code changes in this commit.

Claude-Session: https://claude.ai/code/session_01LrS7BTRTnc6eYrSxBFoWAo

* test(opencode): read the admission probe's query method through its descriptor

Referencing prototype.query unbound tripped the unbound-method lint rule. Reading
the descriptor's value keeps the method unbound and adds no behavior change.

Claude-Session: https://claude.ai/code/session_01LrS7BTRTnc6eYrSxBFoWAo

* docs(opencode): require queue writes to go through writable()

The #162 fix makes writable() open every queue write with BEGIN IMMEDIATE. The
rule lived only in a code comment, so a future queue write could bypass it. The
V1 Prompt Queue section now states it where contributors and agents read queue
rules.

Claude-Session: https://claude.ai/code/session_01LrS7BTRTnc6eYrSxBFoWAo

* test(core): one statement-attempt counter for the busy tests

The core sqlite-busy test and the opencode admission test each patched bun:sqlite's
query method to count statement attempts. That job now has one owner, the
fixture-only module packages/core/src/database/sqlite-attempts.ts. Both tests import
it, and the core test's node branches count through its countStatement.

No runtime module imports the new file. It sits in core's src because opencode tests
reach core through its wildcard export, and no shared test-support package exists.

Checked: core sqlite-busy (23 pass); admission green with and without a 1600 ms delay
before the fork; admission red with writable() reverted to deferred, with and without
the delay (busy codes end in SQLITE_BUSY_SNAPSHOT).

Claude-Session: https://claude.ai/code/session_01LrS7BTRTnc6eYrSxBFoWAo

* docs: record the #162 lock fix in CHANGESET.md

Adds the fork changeset entry for PR #194 (#162 and #165)
as the first item under Runtime fixes. Nothing else changes.

Claude-Session: https://claude.ai/code/session_01LrS7BTRTnc6eYrSxBFoWAo
brentyates-swx added a commit that referenced this pull request Oct 10, 2026
* docs(core): plan the SQLite snapshot-loss fix for #162

Plan for fixing forward accepted-prompt loss under concurrent writers on top
of the reverted base (60f779b). Evidence: data/opencode-fixed-binary-155/
report.md sections 5 and 6, and data/opencode-revert-155/report.md section 7.

Mechanism, from the code (not yet measured):

- The admission transaction is SessionQueue writable() in
  packages/opencode/src/session/queue.ts. It reads first (EventV2.assertWritableIn)
  and then writes (sequence upsert and queue insert). It begins as a deferred
  transaction, so its read snapshot exists before its write lock is taken.
- A plain-busy wait inside a deferred transaction can only end in
  SQLITE_BUSY_SNAPSHOT when another connection commits during the wait: the
  snapshot goes stale, and the driver does not restart the transaction. This is
  the mechanism that predicts the report's section 6.3 observation that the
  snapshot class grew when the wait was added. It is not measured yet.
- The admission and promotion writes are the deferred read-then-write
  transactions found on the accepted-prompt path by grepping its call sites.
  The EventV2 append that persists the user message already begins with
  behavior immediate (event.ts:467). The proof decides whether that is complete.

Correction to #162. Its option 2 names Client.make beginTransaction, which the
production path never reaches. opencode runs transactions through
effect-drizzle-sqlite, whose withTransaction issues "begin <behavior>" itself
(packages/effect-drizzle-sqlite/src/effect-sqlite/session.ts), default deferred.
Effect's SqlClient.withTransaction, which reads beginTransaction, has no caller
in opencode. The mechanism that works is the existing per-transaction option,
behavior immediate, which the codebase already uses in event.ts and project.ts.

Choice: (a) BEGIN IMMEDIATE on write-capable transactions, not (b) restart.
- (a) takes the write lock before any read snapshot exists, so the failing
  class cannot occur for that transaction. The lock wait happens at BEGIN,
  where a retry is safe because nothing has been read yet.
- (b) restarts a body that has already run. The admission body is pure SQL,
  but restart needs a per-site audit of every transaction body for duplicated
  side effects, and it leaves the stale-snapshot race in place.

Steps:

1. Restore the #155 plain-busy wait and the #154 fail-fast on
   SQLITE_BUSY_SNAPSHOT in packages/core/src/database/sqlite.bun.ts,
   sqlite.node.ts and packages/effect-sqlite-node/src/index.ts. The three
   copies stay identical, as #162 requires. Dedupe stays with #165.
   sqlite.node.ts is outside the brief's named scope; it is named here
   because it is the third copy of the same rule.
2. Begin writable() with behavior immediate in queue.ts. This covers every
   queue write: the callers are admit, reserve, withdraw, restore, and update.
3. Restore the #155 test versions that cover the wait and the fail-fast:
   packages/core/test/sqlite-busy.test.ts, and the opencode lock fixture
   and consumer bounds, which #155 raised for the 5 s window.
4. Add a regression at the admission path. A second connection holds the
   write lock across the admission read, then commits. The admission must
   succeed and persist its row. Red proof: revert writable() to deferred.
   Also run the restored core tests against the reverted driver for red.
5. Proofs, in the brief's order: focused suites with CI commands; the
   candidate against the reverted parent, concurrently on separate DBs, same
   load, several pairs, raw counts; fresh-store proof. The parent is the
   reverted build (tree equal to bf5180a).

Not in this change: session queue consumption (#167); the CHANGESET entry
(needs the PR number at publication); the snapshot-tracking PATH issue (#164).

Claude-Session: https://claude.ai/code/session_01LrS7BTRTnc6eYrSxBFoWAo

* fix(core): restore the SQLite lock-wait window for busy statements

Restores the plain-busy wait that #155 added. A statement that meets a lock
retries asynchronously for 5 seconds of elapsed contention. A stale-snapshot
failure (SQLITE_BUSY_SNAPSHOT) is still not retried (#154). The three retry
copies stay identical in sqlite.bun.ts, sqlite.node.ts and effect-sqlite-node.

Red on the reverted driver: the restored sqlite-busy suite fails 14 of 23.
The 250 ms window cannot absorb a 1.5 s hold, and a stale-snapshot statement
is executed more than once. Green with the window: 23 of 23. The opencode
lock-consumer suites pass 36 of 36 with their #155 bounds.

Refs #162

Claude-Session: https://claude.ai/code/session_01LrS7BTRTnc6eYrSxBFoWAo

* fix(opencode): admit queue writes under BEGIN IMMEDIATE

SessionQueue writable() runs its read (EventV2.assertWritableIn) and then its
write in one transaction. Begun deferred, that transaction pins a read
snapshot before it takes the write lock. If another writer commits while the
write waits on the lock, the snapshot goes stale, and the write fails with
SQLITE_BUSY_SNAPSHOT, which is not retried. The prompt was already accepted
(204), so it was lost with no user message.

With behavior immediate the write lock is taken at BEGIN, before any read.
The lock wait then happens where a retry is safe, and no snapshot can go
stale inside the transaction. The restored lock-wait window absorbs the wait.

The regression holds the write lock from a second connection across the
admission read, then commits. Red with the deferred writable: the sequence
upsert fails with SQLITE_BUSY_SNAPSHOT after the commit. Green with immediate.

Refs #162

Claude-Session: https://claude.ai/code/session_01LrS7BTRTnc6eYrSxBFoWAo

* refactor(core): give the SQLite busy-retry gate one owner

The busy-retry schedule and the stale-snapshot gate were copied into three
statement paths: sqlite.bun.ts, sqlite.node.ts and effect-sqlite-node. #155
changed all three, so a rule change had to land three times, and nothing said
which copy was authoritative.

effect-sqlite-node/src/retry.ts now owns both. It exports retryLocked, which
every driver routes its statements through, and it recognizes the stale-snapshot
class in both native error shapes: bun:sqlite reports it by code and node:sqlite
by errcode. The module imports only effect, so the Bun binary can consume it
without loading node:sqlite. The drivers no longer carry a schedule or a gate.

Behavior is unchanged. The core busy suite is 23 of 23 across the three
clients. Removing the snapshot exclusion from the gate turns the three
stale-snapshot tests red. Dropping the node error shape turns two of them
red (core-node, standalone-node), and dropping the bun shape turns one red
(bun). Removing the retry turns the six lock-wait tests red.

statementError, the native error classification, is still copied in the same
three files. It is not part of this change, and #87 tracks it.

Refs #162
Refs #165

Claude-Session: https://claude.ai/code/session_01LrS7BTRTnc6eYrSxBFoWAo

* test(opencode): wait on the admission's busy attempt, not a sleep

Review round 1. The admission test held the write lock for a fixed 1500 ms,
then committed. An admission that started late ran after the commit and
passed even with a deferred write, so the test did not prove the fix.

The test now counts the admission's statement attempts at bun:sqlite's query
path, as the core sqlite-busy test does, and records the native codes of the
attempts that throw SQLITE_BUSY. It commits only after the admission has met
the held lock, asserts the admission is still pending, then checks the
persisted row. The timeout is a backstop, not the signal. On failure, the
message lists the admission's attempts and busy codes.

Checked locally:
- writable() immediate: pass, with and without a 1600 ms delay before the fork
- writable() reverted to deferred: fail with SQLITE_BUSY_SNAPSHOT after the
  commit, with and without the 1600 ms delay

No product code changes in this commit.

Claude-Session: https://claude.ai/code/session_01LrS7BTRTnc6eYrSxBFoWAo

* test(opencode): read the admission probe's query method through its descriptor

Referencing prototype.query unbound tripped the unbound-method lint rule. Reading
the descriptor's value keeps the method unbound and adds no behavior change.

Claude-Session: https://claude.ai/code/session_01LrS7BTRTnc6eYrSxBFoWAo

* docs(opencode): require queue writes to go through writable()

The #162 fix makes writable() open every queue write with BEGIN IMMEDIATE. The
rule lived only in a code comment, so a future queue write could bypass it. The
V1 Prompt Queue section now states it where contributors and agents read queue
rules.

Claude-Session: https://claude.ai/code/session_01LrS7BTRTnc6eYrSxBFoWAo

* test(core): one statement-attempt counter for the busy tests

The core sqlite-busy test and the opencode admission test each patched bun:sqlite's
query method to count statement attempts. That job now has one owner, the
fixture-only module packages/core/src/database/sqlite-attempts.ts. Both tests import
it, and the core test's node branches count through its countStatement.

No runtime module imports the new file. It sits in core's src because opencode tests
reach core through its wildcard export, and no shared test-support package exists.

Checked: core sqlite-busy (23 pass); admission green with and without a 1600 ms delay
before the fork; admission red with writable() reverted to deferred, with and without
the delay (busy codes end in SQLITE_BUSY_SNAPSHOT).

Claude-Session: https://claude.ai/code/session_01LrS7BTRTnc6eYrSxBFoWAo

* docs: record the #162 lock fix in CHANGESET.md

Adds the fork changeset entry for PR #194 (#162 and #165)
as the first item under Runtime fixes. Nothing else changes.

Claude-Session: https://claude.ai/code/session_01LrS7BTRTnc6eYrSxBFoWAo

* docs(core): plan SQLite lock-hold and stop-site diagnostics (#180)

Diagnostic-only logging on candidate 7b0ea4a (fm/opencode-lock-fix-162) so
the next SQLITE_BUSY session stop names its lock holder and its call site.
Every new line goes through the existing Effect logger; no value, cause, SQL,
schedule, timeout or transaction behaviour changes.

1. Write-lock hold timing (effect-drizzle-sqlite session.withTransaction).
   For a top-level BEGIN IMMEDIATE/EXCLUSIVE that succeeds, time from BEGIN
   returning to a successful COMMIT or ROLLBACK (including the rollback after
   a failed commit). Over 250 ms, log "sqlite write lock held" with pid,
   transaction purpose, current span and duration. Savepoints, deferred
   transactions and failed BEGINs are not holds. Purpose comes from a new
   TransactionPurpose context reference, falling back to the current span.
   event.ts commitDurableEvent provides the event definition type.

2. Retry exhaustion (effect-sqlite-node retryLocked, given the statement SQL
   by all three drivers). Per execution, count attempts and elapsed time;
   when a lock failure leaves the gate, log once: statement head, attempts,
   elapsed ms, and the native cause code and full errcode (item 4), with
   separate messages for an exhausted window and a stale snapshot that
   fails without retry. Recovering statements log nothing.

3. Stop call site. A core side table keyed by the failure object records
   where a failure came from without changing it: commitDurableEvent records
   the event type and the span chain at the failed commit; the processor
   records the stream case (handler) or the provider stream (stream).
   halt's existing "process" error line adds those fields.

Tests (focused, producer-driven, removal-to-red proven):
- packages/core/test/sqlite-lock-diagnostics.test.ts: long vs short holds,
  commit and rollback, wait-then-short-hold, nested savepoints; one
  exhaustion summary with observed attempt count; none on recovery;
  BUSY vs BUSY_SNAPSHOT native codes on bun, core-node, standalone-node.
- packages/opencode/test/session/processor-effect.test.ts: halt line names
  event, case and call site for reasoning-end (finishReasoning helper) and
  step-finish (message.updated); provider-stream failure has no event.
Preservation: core sqlite-busy and opencode queue-admission suites unchanged.
Live proof: env -i scratch store run whose file log shows the hold line.

Claude-Session: https://claude.ai/code/session_01QuyVZfg7zCF6ZJmDUf8BnB

* feat(core): log long SQLite write-lock holds and lock retry give-ups

Diagnostic logging only (#180); every value, cause, SQL statement, retry
schedule and transaction step is unchanged.

- effect-drizzle-sqlite withTransaction times a top-level immediate or
  exclusive BEGIN from its success to the successful COMMIT or ROLLBACK
  (including the rollback after a failed commit) and logs "sqlite write lock
  held" with pid, purpose, span, outcome and duration over 250 ms. A new
  TransactionPurpose reference names the purpose, else the caller's span.
- retryLocked takes the statement SQL from all three drivers and, per
  execution, logs one summary when a lock failure leaves it: statement head,
  attempts, elapsed ms, and the native code and full errcode, so SQLITE_BUSY
  and SQLITE_BUSY_SNAPSHOT stay distinguishable.
- A core FailureOrigin side table records a failure's event type and span
  chain at the durable-event commit without changing the failure.
- commitDurableEvent supplies its definition type as the purpose.
- SessionProcessor records the stream case (or the provider stream) on a
  failure, and halt's "process" line adds the recorded event, case and
  call site.

Claude-Session: https://claude.ai/code/session_01QuyVZfg7zCF6ZJmDUf8BnB

* test(opencode): prove halt names the failed write's event, case and call site

Adds focused processor tests driving real lock failures into halt (#180):
- reasoning-end: the finishReasoning helper's message.part.updated commit
  meets a held lock; halt names the event, the case and a call-site chain
  running Session.updatePart < SessionProcessor.finishReasoning.
- step-finish: the message.updated write meets the lock instead, so the
  same case path reports a different event identity.
- a provider stream that dies after its handled cases succeed is attributed
  to the stream, with no invented event or case.

The held-lock fixture gains a trigger choosing which write takes the lock;
its default keeps the existing tests' reasoning-end behaviour.

The core nested-savepoint hold test now nests an immediate transaction long
enough to log on its own, and withTransaction drops a guard that was dead:
only the top-level COMMIT and ROLLBACK report a release.

Claude-Session: https://claude.ai/code/session_01QuyVZfg7zCF6ZJmDUf8BnB

* fix(core): omit an absent span from the lock-hold line

A hold with no caller span printed span=undefined in the file log. The bun
stale-snapshot and BEGIN exhaustion tests now also pin the extended errno
bun reports (517 and 5), alongside its code.

Claude-Session: https://claude.ai/code/session_01QuyVZfg7zCF6ZJmDUf8BnB

* docs: record the stop-instrument diagnostics in CHANGESET.md

Adds the fork entry under Runtime fixes now, so review covers its text. The PR
number does not exist until publication; script/changeset-check.ts keys the
entry on it and fails the PR until `#PR` is replaced with that number.

Claude-Session: https://claude.ai/code/session_01QuyVZfg7zCF6ZJmDUf8BnB

* docs: say the stop-instrument entry follows #194, which has landed

Claude-Session: https://claude.ai/code/session_01QuyVZfg7zCF6ZJmDUf8BnB

* fix(core): name an exhausted statement by its operation only

Review round 1, required 1: the retry gate's summary logged the first three
SQL tokens, so a leading comment such as /* secret */ or a quoted identifier
reached the log and the operation itself could be lost. It now logs only the
leading keyword (and a BEGIN's deferred/immediate/exclusive), after skipping
leading comments and whitespace; nothing after the operation is read.
Returned errors and parameters are unchanged.

The new focused test runs real statements through the retry gate on held
production connections (leading block and line comments, a string literal,
a quoted identifier, mixed case and whitespace, a multi-statement string and
a bound parameter) and asserts each summary names its operation and none
contains the payload. The first hold test gets an explicit 20 s timeout like
its siblings.

Claude-Session: https://claude.ai/code/session_01QuyVZfg7zCF6ZJmDUf8BnB

* fix(core): take the stop call site from where the failure was raised

Review round 1, required 2: FailureOrigin read the observing fiber's span
after the failing scopes had unwound, so a provider-stream failure recorded
only SessionProcessor.process. It now reads the stack-frame chain the Effect
runtime annotates onto the cause where the failure is raised
(Cause.StackTrace), leaving out Effect.fn's definition-site frames, so the
call site names the failing callable across the stream and helper boundary.

The provider-stream halt test now fails inside a named callable and asserts
the call site starts with it; the held-lock tests pin their innermost frame
to Session.updatePart and Session.updateMessage.

Claude-Session: https://claude.ai/code/session_01QuyVZfg7zCF6ZJmDUf8BnB

* docs: record the stop-instrument entry as #201 in CHANGESET.md

Claude-Session: https://claude.ai/code/session_01QuyVZfg7zCF6ZJmDUf8BnB
brentyates-swx added a commit that referenced this pull request Oct 11, 2026
Plan for fixing forward accepted-prompt loss under concurrent writers on top
of the reverted base (60f779b). Evidence: data/opencode-fixed-binary-155/
report.md sections 5 and 6, and data/opencode-revert-155/report.md section 7.

Mechanism, from the code (not yet measured):

- The admission transaction is SessionQueue writable() in
  packages/opencode/src/session/queue.ts. It reads first (EventV2.assertWritableIn)
  and then writes (sequence upsert and queue insert). It begins as a deferred
  transaction, so its read snapshot exists before its write lock is taken.
- A plain-busy wait inside a deferred transaction can only end in
  SQLITE_BUSY_SNAPSHOT when another connection commits during the wait: the
  snapshot goes stale, and the driver does not restart the transaction. This is
  the mechanism that predicts the report's section 6.3 observation that the
  snapshot class grew when the wait was added. It is not measured yet.
- The admission and promotion writes are the deferred read-then-write
  transactions found on the accepted-prompt path by grepping its call sites.
  The EventV2 append that persists the user message already begins with
  behavior immediate (event.ts:467). The proof decides whether that is complete.

Correction to #162. Its option 2 names Client.make beginTransaction, which the
production path never reaches. opencode runs transactions through
effect-drizzle-sqlite, whose withTransaction issues "begin <behavior>" itself
(packages/effect-drizzle-sqlite/src/effect-sqlite/session.ts), default deferred.
Effect's SqlClient.withTransaction, which reads beginTransaction, has no caller
in opencode. The mechanism that works is the existing per-transaction option,
behavior immediate, which the codebase already uses in event.ts and project.ts.

Choice: (a) BEGIN IMMEDIATE on write-capable transactions, not (b) restart.
- (a) takes the write lock before any read snapshot exists, so the failing
  class cannot occur for that transaction. The lock wait happens at BEGIN,
  where a retry is safe because nothing has been read yet.
- (b) restarts a body that has already run. The admission body is pure SQL,
  but restart needs a per-site audit of every transaction body for duplicated
  side effects, and it leaves the stale-snapshot race in place.

Steps:

1. Restore the #155 plain-busy wait and the #154 fail-fast on
   SQLITE_BUSY_SNAPSHOT in packages/core/src/database/sqlite.bun.ts,
   sqlite.node.ts and packages/effect-sqlite-node/src/index.ts. The three
   copies stay identical, as #162 requires. Dedupe stays with #165.
   sqlite.node.ts is outside the brief's named scope; it is named here
   because it is the third copy of the same rule.
2. Begin writable() with behavior immediate in queue.ts. This covers every
   queue write: the callers are admit, reserve, withdraw, restore, and update.
3. Restore the #155 test versions that cover the wait and the fail-fast:
   packages/core/test/sqlite-busy.test.ts, and the opencode lock fixture
   and consumer bounds, which #155 raised for the 5 s window.
4. Add a regression at the admission path. A second connection holds the
   write lock across the admission read, then commits. The admission must
   succeed and persist its row. Red proof: revert writable() to deferred.
   Also run the restored core tests against the reverted driver for red.
5. Proofs, in the brief's order: focused suites with CI commands; the
   candidate against the reverted parent, concurrently on separate DBs, same
   load, several pairs, raw counts; fresh-store proof. The parent is the
   reverted build (tree equal to bf5180a).

Not in this change: session queue consumption (#167); the CHANGESET entry
(needs the PR number at publication); the snapshot-tracking PATH issue (#164).

Claude-Session: https://claude.ai/code/session_01LrS7BTRTnc6eYrSxBFoWAo
brentyates-swx added a commit that referenced this pull request Oct 11, 2026
Restores the plain-busy wait that #155 added. A statement that meets a lock
retries asynchronously for 5 seconds of elapsed contention. A stale-snapshot
failure (SQLITE_BUSY_SNAPSHOT) is still not retried (#154). The three retry
copies stay identical in sqlite.bun.ts, sqlite.node.ts and effect-sqlite-node.

Red on the reverted driver: the restored sqlite-busy suite fails 14 of 23.
The 250 ms window cannot absorb a 1.5 s hold, and a stale-snapshot statement
is executed more than once. Green with the window: 23 of 23. The opencode
lock-consumer suites pass 36 of 36 with their #155 bounds.

Refs #162

Claude-Session: https://claude.ai/code/session_01LrS7BTRTnc6eYrSxBFoWAo
brentyates-swx added a commit that referenced this pull request Oct 11, 2026
The busy-retry schedule and the stale-snapshot gate were copied into three
statement paths: sqlite.bun.ts, sqlite.node.ts and effect-sqlite-node. #155
changed all three, so a rule change had to land three times, and nothing said
which copy was authoritative.

effect-sqlite-node/src/retry.ts now owns both. It exports retryLocked, which
every driver routes its statements through, and it recognizes the stale-snapshot
class in both native error shapes: bun:sqlite reports it by code and node:sqlite
by errcode. The module imports only effect, so the Bun binary can consume it
without loading node:sqlite. The drivers no longer carry a schedule or a gate.

Behavior is unchanged. The core busy suite is 23 of 23 across the three
clients. Removing the snapshot exclusion from the gate turns the three
stale-snapshot tests red. Dropping the node error shape turns two of them
red (core-node, standalone-node), and dropping the bun shape turns one red
(bun). Removing the retry turns the six lock-wait tests red.

statementError, the native error classification, is still copied in the same
three files. It is not part of this change, and #87 tracks it.

Refs #162
Refs #165

Claude-Session: https://claude.ai/code/session_01LrS7BTRTnc6eYrSxBFoWAo
brentyates-swx added a commit that referenced this pull request Oct 11, 2026
* docs(core): plan the SQLite snapshot-loss fix for #162

Plan for fixing forward accepted-prompt loss under concurrent writers on top
of the reverted base (60f779b). Evidence: data/opencode-fixed-binary-155/
report.md sections 5 and 6, and data/opencode-revert-155/report.md section 7.

Mechanism, from the code (not yet measured):

- The admission transaction is SessionQueue writable() in
  packages/opencode/src/session/queue.ts. It reads first (EventV2.assertWritableIn)
  and then writes (sequence upsert and queue insert). It begins as a deferred
  transaction, so its read snapshot exists before its write lock is taken.
- A plain-busy wait inside a deferred transaction can only end in
  SQLITE_BUSY_SNAPSHOT when another connection commits during the wait: the
  snapshot goes stale, and the driver does not restart the transaction. This is
  the mechanism that predicts the report's section 6.3 observation that the
  snapshot class grew when the wait was added. It is not measured yet.
- The admission and promotion writes are the deferred read-then-write
  transactions found on the accepted-prompt path by grepping its call sites.
  The EventV2 append that persists the user message already begins with
  behavior immediate (event.ts:467). The proof decides whether that is complete.

Correction to #162. Its option 2 names Client.make beginTransaction, which the
production path never reaches. opencode runs transactions through
effect-drizzle-sqlite, whose withTransaction issues "begin <behavior>" itself
(packages/effect-drizzle-sqlite/src/effect-sqlite/session.ts), default deferred.
Effect's SqlClient.withTransaction, which reads beginTransaction, has no caller
in opencode. The mechanism that works is the existing per-transaction option,
behavior immediate, which the codebase already uses in event.ts and project.ts.

Choice: (a) BEGIN IMMEDIATE on write-capable transactions, not (b) restart.
- (a) takes the write lock before any read snapshot exists, so the failing
  class cannot occur for that transaction. The lock wait happens at BEGIN,
  where a retry is safe because nothing has been read yet.
- (b) restarts a body that has already run. The admission body is pure SQL,
  but restart needs a per-site audit of every transaction body for duplicated
  side effects, and it leaves the stale-snapshot race in place.

Steps:

1. Restore the #155 plain-busy wait and the #154 fail-fast on
   SQLITE_BUSY_SNAPSHOT in packages/core/src/database/sqlite.bun.ts,
   sqlite.node.ts and packages/effect-sqlite-node/src/index.ts. The three
   copies stay identical, as #162 requires. Dedupe stays with #165.
   sqlite.node.ts is outside the brief's named scope; it is named here
   because it is the third copy of the same rule.
2. Begin writable() with behavior immediate in queue.ts. This covers every
   queue write: the callers are admit, reserve, withdraw, restore, and update.
3. Restore the #155 test versions that cover the wait and the fail-fast:
   packages/core/test/sqlite-busy.test.ts, and the opencode lock fixture
   and consumer bounds, which #155 raised for the 5 s window.
4. Add a regression at the admission path. A second connection holds the
   write lock across the admission read, then commits. The admission must
   succeed and persist its row. Red proof: revert writable() to deferred.
   Also run the restored core tests against the reverted driver for red.
5. Proofs, in the brief's order: focused suites with CI commands; the
   candidate against the reverted parent, concurrently on separate DBs, same
   load, several pairs, raw counts; fresh-store proof. The parent is the
   reverted build (tree equal to bf5180a).

Not in this change: session queue consumption (#167); the CHANGESET entry
(needs the PR number at publication); the snapshot-tracking PATH issue (#164).

Claude-Session: https://claude.ai/code/session_01LrS7BTRTnc6eYrSxBFoWAo

* fix(core): restore the SQLite lock-wait window for busy statements

Restores the plain-busy wait that #155 added. A statement that meets a lock
retries asynchronously for 5 seconds of elapsed contention. A stale-snapshot
failure (SQLITE_BUSY_SNAPSHOT) is still not retried (#154). The three retry
copies stay identical in sqlite.bun.ts, sqlite.node.ts and effect-sqlite-node.

Red on the reverted driver: the restored sqlite-busy suite fails 14 of 23.
The 250 ms window cannot absorb a 1.5 s hold, and a stale-snapshot statement
is executed more than once. Green with the window: 23 of 23. The opencode
lock-consumer suites pass 36 of 36 with their #155 bounds.

Refs #162

Claude-Session: https://claude.ai/code/session_01LrS7BTRTnc6eYrSxBFoWAo

* fix(opencode): admit queue writes under BEGIN IMMEDIATE

SessionQueue writable() runs its read (EventV2.assertWritableIn) and then its
write in one transaction. Begun deferred, that transaction pins a read
snapshot before it takes the write lock. If another writer commits while the
write waits on the lock, the snapshot goes stale, and the write fails with
SQLITE_BUSY_SNAPSHOT, which is not retried. The prompt was already accepted
(204), so it was lost with no user message.

With behavior immediate the write lock is taken at BEGIN, before any read.
The lock wait then happens where a retry is safe, and no snapshot can go
stale inside the transaction. The restored lock-wait window absorbs the wait.

The regression holds the write lock from a second connection across the
admission read, then commits. Red with the deferred writable: the sequence
upsert fails with SQLITE_BUSY_SNAPSHOT after the commit. Green with immediate.

Refs #162

Claude-Session: https://claude.ai/code/session_01LrS7BTRTnc6eYrSxBFoWAo

* refactor(core): give the SQLite busy-retry gate one owner

The busy-retry schedule and the stale-snapshot gate were copied into three
statement paths: sqlite.bun.ts, sqlite.node.ts and effect-sqlite-node. #155
changed all three, so a rule change had to land three times, and nothing said
which copy was authoritative.

effect-sqlite-node/src/retry.ts now owns both. It exports retryLocked, which
every driver routes its statements through, and it recognizes the stale-snapshot
class in both native error shapes: bun:sqlite reports it by code and node:sqlite
by errcode. The module imports only effect, so the Bun binary can consume it
without loading node:sqlite. The drivers no longer carry a schedule or a gate.

Behavior is unchanged. The core busy suite is 23 of 23 across the three
clients. Removing the snapshot exclusion from the gate turns the three
stale-snapshot tests red. Dropping the node error shape turns two of them
red (core-node, standalone-node), and dropping the bun shape turns one red
(bun). Removing the retry turns the six lock-wait tests red.

statementError, the native error classification, is still copied in the same
three files. It is not part of this change, and #87 tracks it.

Refs #162
Refs #165

Claude-Session: https://claude.ai/code/session_01LrS7BTRTnc6eYrSxBFoWAo

* test(opencode): wait on the admission's busy attempt, not a sleep

Review round 1. The admission test held the write lock for a fixed 1500 ms,
then committed. An admission that started late ran after the commit and
passed even with a deferred write, so the test did not prove the fix.

The test now counts the admission's statement attempts at bun:sqlite's query
path, as the core sqlite-busy test does, and records the native codes of the
attempts that throw SQLITE_BUSY. It commits only after the admission has met
the held lock, asserts the admission is still pending, then checks the
persisted row. The timeout is a backstop, not the signal. On failure, the
message lists the admission's attempts and busy codes.

Checked locally:
- writable() immediate: pass, with and without a 1600 ms delay before the fork
- writable() reverted to deferred: fail with SQLITE_BUSY_SNAPSHOT after the
  commit, with and without the 1600 ms delay

No product code changes in this commit.

Claude-Session: https://claude.ai/code/session_01LrS7BTRTnc6eYrSxBFoWAo

* test(opencode): read the admission probe's query method through its descriptor

Referencing prototype.query unbound tripped the unbound-method lint rule. Reading
the descriptor's value keeps the method unbound and adds no behavior change.

Claude-Session: https://claude.ai/code/session_01LrS7BTRTnc6eYrSxBFoWAo

* docs(opencode): require queue writes to go through writable()

The #162 fix makes writable() open every queue write with BEGIN IMMEDIATE. The
rule lived only in a code comment, so a future queue write could bypass it. The
V1 Prompt Queue section now states it where contributors and agents read queue
rules.

Claude-Session: https://claude.ai/code/session_01LrS7BTRTnc6eYrSxBFoWAo

* test(core): one statement-attempt counter for the busy tests

The core sqlite-busy test and the opencode admission test each patched bun:sqlite's
query method to count statement attempts. That job now has one owner, the
fixture-only module packages/core/src/database/sqlite-attempts.ts. Both tests import
it, and the core test's node branches count through its countStatement.

No runtime module imports the new file. It sits in core's src because opencode tests
reach core through its wildcard export, and no shared test-support package exists.

Checked: core sqlite-busy (23 pass); admission green with and without a 1600 ms delay
before the fork; admission red with writable() reverted to deferred, with and without
the delay (busy codes end in SQLITE_BUSY_SNAPSHOT).

Claude-Session: https://claude.ai/code/session_01LrS7BTRTnc6eYrSxBFoWAo

* docs: record the #162 lock fix in CHANGESET.md

Adds the fork changeset entry for PR #194 (#162 and #165)
as the first item under Runtime fixes. Nothing else changes.

Claude-Session: https://claude.ai/code/session_01LrS7BTRTnc6eYrSxBFoWAo

* docs(opencode): plan bounded consume recovery for #167 and #180

Plan for option A (captain decision, 2026-10-10): recover retained queued
intent after a consume that met a held write lock, through the existing queue
owner, with a bounded terminal refusal. Starting head 7b0ea4a (the #162 fix
branch); its runtime source equals candidate 9e088df: queue writable() opens
BEGIN IMMEDIATE, and every statement goes through retryLocked/retrySchedule in
packages/effect-sqlite-node/src/retry.ts (5 s window). Neither changes here.

Failure boundary, from the code:

- SessionPrompt.run (runLoop in prompt.ts) calls queue.consume at every loop
  top, including the one after a tool step. consume is
  exclusive(writable(DELETE promoted rows)).pipe(Effect.orDie).
- With a second connection holding the write lock, BEGIN IMMEDIATE fails and is
  retried by retryLocked until its window ends; the LockTimeoutError becomes a
  defect. The transaction body never ran.
- The defect leaves runLoop before `park`, the runner's finishRun completes
  every joiner's Deferred with it, and drain dies at state.ensureRunning, so
  its awaitingDrain re-check never runs. The pending queue row, the completed
  tool part and the unanswered tool-calls turn stay in the database, and
  nothing drives them until another prompt wakes the session.

Change (queue.ts only; prompt.ts keeps its one consume call):

- consume re-attempts its own transaction when the failure is a lock failure
  and the transaction body never began (tracked by the body, so an executed
  body is never replayed). Each attempt takes the session permit afresh and
  waits on the lock through the unchanged statement retry policy.
- Recovery condition: a consume attempt commits, which is a held write lock
  taken by this connection and the promoted rows deleted. The loop then reads
  history and goes on to the provider turn, so the turn and the queued prompt
  continue in the same run, answered once. Time never decides success; each
  attempt is bounded by the existing window.
- Bound: 3 attempts in total. Every failed attempt logs a warning naming the
  stage and the attempt number.
- Refusal: when the last attempt fails the same way, consume dies with
  SessionQueue.ConsumeLockedError, naming the consume stage, the session and
  the attempts. The run ends as a run defect does today: joiners receive it,
  prompt_async publishes it as session.error, the runner goes idle, and the
  drain re-check is not reached, so nothing re-drains it automatically. The
  queue row, the tool part and the turn stay as they were, for a later wake.
- No new re-drain path. The one re-check in drain stays the only one.

Proof:

- prompt.test.ts, on a scratch file database: a tool turn with a queued
  continuation, the write lock taken from a second connection at the loop-top
  busy status after the tool step. Recovery: release once the first failed
  attempt is logged; the turn and the continuation are each answered once,
  the bash tool ran once, one continuation user message, queue empty.
  Refusal: keep the lock; the run dies with ConsumeLockedError after 3
  attempts, the row stays pending, the tool ran once, the runner is idle; a
  later prompt delivers the continuation once.
- Red/green by reverting the recovery in queue.ts.
- The archived continuation harness, copied, against a built binary: below
  2 s unchanged, above 8 s continues exactly once, and a hold past the bound
  ends in the refusal.

Out of scope: #175, #176, #166, #179; no running server is touched.

Refs #167, #180, #181

Claude-Session: https://claude.ai/code/session_01KdxgEDScVEkwpbHyhm1EBq

* test(opencode): reproduce the stranded continuation after a locked consume

A bash tool turn with a queued continuation, on a scratch file database. A
second connection takes the write lock at the loop-top busy status after the
tool step, so the run's own SessionQueue.consume meets it.

On this head both cases fail: consume dies with LockTimeoutError at
SessionPrompt.run after the statement retry window, and the continuation is
left pending with no recovery and no named refusal.

Refs #167, #180

Claude-Session: https://claude.ai/code/session_01KdxgEDScVEkwpbHyhm1EBq

* fix(opencode): recover a consume that meets a held write lock

SessionQueue.consume runs at every loop top, including the one after a tool
step. When another connection held the write lock past the statement retry
window, its BEGIN IMMEDIATE failed, the defect ended the run before `park`
and before drain's awaitingDrain re-check, and the completed tool turn and its
queued continuation waited for an unrelated prompt.

consume now makes the attempt again while the failure is a lock failure and
its transaction never began, so no executed body is replayed. Each attempt
takes the session permit afresh and waits on the lock under the unchanged
statement retry policy; success is the attempt's own commit, after which the
loop reads history and goes on to the provider turn in the same run. The bound
is three attempts, each logged as "queue consume met a held write lock". A
lock that outlasts all three ends the run with SessionQueue.ConsumeLockedError,
which names the consume stage, the session and the attempts, and leaves every
queued row, the tool result and the unanswered turn as they were. Nothing
re-drains it automatically: the run fails as a run defect does, so drain's
re-check is not reached, and the next prompt delivers the kept work.

Refs #167, #180

Claude-Session: https://claude.ai/code/session_01KdxgEDScVEkwpbHyhm1EBq

* fix(opencode): bound each consume attempt's acquisition with a backstop

Review round 1. A consume attempt could wait without bound before its
transaction began: for the session permit, or for the shared native connection,
which the driver acquires and retries BEGIN on uninterruptibly. Attempt 3 and
ConsumeLockedError were then never reached.

Each attempt now runs in its own fiber, and the caller waits at most
consumeAcquireBackstop (10 s: the attempt's own statement window plus one other
statement's ahead of it on the connection) for the attempt's body to begin. An
attempt that has not begun by then is abandoned and counted as a failed
attempt, logged as "queue consume waited past its acquisition backstop". Its
fiber is interrupted without waiting, and if it still reaches its body it rolls
back without deleting anything. A body that began is waited for untimed. The
attempt count (3) and the statement retry windows are unchanged, and recovery
is still the same re-attempt inside the existing owner.

Tests (prompt.test.ts): the fixture takes a pluggable hold at the loop top and
reads records through its own read-only connection. Two cases are new:
- the session permit held by a writer that never finishes: three backstop
  attempts, then ConsumeLockedError, the continuation kept, and a later prompt
  delivers it once;
- a SQLITE_BUSY failure of the consume's delete, after its body began: the run
  dies with that lock error, and no further attempt is made (replay guard).
The refusal test binds SessionStatus and SessionRunState to named variables.

Refs #167, #180

Claude-Session: https://claude.ai/code/session_01KdxgEDScVEkwpbHyhm1EBq

* fix(opencode): give a consume five attempts before it refuses

Captain decision in review round 2: keep the count design and the per-attempt
acquisition backstop, and raise the attempt count from 3 to 5. Each attempt
still waits on the lock within its existing statement window. The refusal's
attempt count and message now come from consumeAttempts rather than the loop
counter.

The refusal and permit tests expect five attempts and five logs, and their own
bounds grow with the budget. The replay-guard test's bound grows too, so its
red, five replayed attempts, still ends within it.

Refs #167, #180

Claude-Session: https://claude.ai/code/session_01KdxgEDScVEkwpbHyhm1EBq

* test(opencode): cover a consume whose attempts wait on a held connection

Review round 2. A new prompt.test.ts regression drives the real prompt and
runner. The run's first consume attempt meets a native SQLITE_BUSY on its
BEGIN IMMEDIATE. Inside that attempt's log, before the next attempt, an
unrelated deferred transaction takes the shared connection and keeps it, and
the external lock is then released. The remaining attempts wait on that
connection and are abandoned at the backstop. The run must refuse with
ConsumeLockedError, and the runner must stop, while the transaction still
holds the connection. Once the transaction ends, the abandoned attempt that
kept the session permit begins its transaction, deletes nothing, and frees
the permit. The records are unchanged, and a later wake delivers the
continuation once.

Replacing the detached interrupt of an abandoned attempt with an awaited one
makes it red, because the run cannot stop until the connection is free. The
state.abandoned guard is not reached while the interrupt is in place: the
body is interrupted at its interruptible boundary first. Removing the guard
alone stays green, and no coverage is claimed for it.

Also, abandon now reports whether it won. An abandon that loses to a body that
already began waits for that body, so a committed consume is never reported
as a backstop.

Refs #167, #180

Claude-Session: https://claude.ai/code/session_01KdxgEDScVEkwpbHyhm1EBq

* docs: record the #167 consume recovery in CHANGESET.md

Claude-Session: https://claude.ai/code/session_019DxEchLHHdPjd4GUE1EJAj
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant