[woodsonl]

Recreates #54396 (closed when its head fork was deleted; branch restored from the preserved PR head commit a121b9d).

Issue for this PR

Part of #47566 — this covers the single-statement half. The transactional writes (Bus.publish, publishAll, replay, the write in session.ts) need whole-transaction replay, which #53704's StorageRetry adds at the caller layer; #47566 should stay open until that lands, so it is deliberately not closed here.

Type of change

• Bug fix

What does this PR do?

Port of #47567 to the dev line: statements that fail with SQLITE_BUSY after busy_timeout retry with jittered backoff instead of killing an unrelated prompt. Inside a transaction the error still surfaces at once (BUSY_SNAPSHOT repeats until rollback), and a BEGIN that loses the race still retries, since the connection is not in a transaction until BEGIN succeeds.

Scope, per review: the retry lives at statement scope only. A statement inside a transaction fails fast because the only recovery there is rolling back and rerunning the whole transaction, and only the caller can decide whether rerunning its body is safe — #53704 adds exactly that caller-side replay (StorageRetry.write) for the durable event paths. The two layers compose without retrying the same write twice: a transaction's body statements never retry here, its BEGIN/COMMIT/ROLLBACK retry in place while it holds the client semaphore, and a caller-side replay only starts after this client's budget is spent.

The review on the first port caught a hole: an attempt that only repeats the statement sleeps without holding the client semaphore, so a transaction another fiber opens while it sleeps swallows the next attempt. The second review caught a smaller version of the same hole: the acquirer releases the permit as soon as it hands back the connection, so even a first attempt, and the flag check that routed control statements, ran outside it. Plain attempts now hold the permit for the statement itself, and control statements are separated structurally instead of by a flag: the transaction acquirer hands out a connection whose service-less statements (BEGIN, COMMIT, ROLLBACK) retry in place because the transaction already holds the permit, while plain statements gate through the acquirer's connection and take the permit per attempt. Nothing reads connection state outside the permit, so a foreign statement can never join a transaction. A test parks a plain write mid-statement, past its acquirer, and opens a transaction there; it fails deterministically on the previous gate and passes with the permit held.

The port also fixed node:sqlite reporting busy as ERR_SQLITE_ERROR with no errno, which the classifier reads, so lock timeouts filed as UnknownError and the retry never fired on that driver. (#53704's StorageRetry carries its own copy of that normalization for its layer; once both land, the driver-level fix is the right home and the copy can fold in.)

This push also rebases onto the latest dev — the branch was over a thousand commits behind; the touched files had not changed there apart from the isRecord import path, which this picks up.

How did you verify your code works?

In packages/core: bun run typecheck passes and bun test test/sqlite-lock-retry.test.ts runs 7 tests, all green across repeated runs, alongside the database/drizzle/bus/kv/session-store suites. With the pre-fix drivers swapped in, 4 of the 7 fail — including the cross-fiber race, which now fails deterministically instead of by timing luck. Locks are held by a second connection and released by the test at wide margins (no setTimeout holders), the interfering transaction in the race test is parked mid-body on a Deferred the test releases, and no test depends on sleeps lining up.

Screenshots / recordings

Not a UI change.

Checklist

• I have tested my changes locally
• I have not included unrelated changes in this PR

---

Added by the opencode bot (AI-generated).

Related: #47566, #53704, #56282

{
  "schema_version": 1,
  "pr": 59521,
  "head": "f70694d47610410e0c711362bbcfb82c76645965",
  "base": "dev",
  "session_kind": "interactive"
}

[opencode-agent]

The retry works for single statements, but I don't think it fixes the prompt failures yet.

In packages/core, the new tests pass and typecheck is clean. With the base sqlite.ts and sqlite.node.ts swapped in, 4 of the 7 tests fail, so the retry and the node:sqlite error mapping do work for statements that run on their own.

The problem is that v2 does most of its writes inside transactions. Bus.publish, publishAll and replay (bus.ts) and the write in session.ts all run in deferred db.transaction blocks, and this PR makes statements inside a transaction fail fast on purpose. I ran a test shaped like Bus.publish (select, then update, inside db.transaction) while another connection held begin immediate for 200ms. On this branch it still failed at once with a query error. So most writes that fail a prompt get no retry here.

Inside a transaction the only safe recovery is to roll back and rerun the whole transaction. That is what #53704 does for durable event commits (StorageRetry.write). Please check how this fits with that PR, so the two don't overlap or retry the same write twice. If this one stays limited to single statements, please don't let it close #47566.

The tests are timing-based: setTimeout lock holders at 100 and 300ms, and the race test depends on 10, 30 and 120ms sleeps lining up. They may be flaky on slow CI. The file is also 361 lines for 7 cases, with a lot of repeated setup and casts. Please cut it down to a few small cases (bun retry, node classification, the cross-fiber race) with shared setup, and avoid timing where you can, for example by releasing the lock from the test instead of on a timer.

Public API: unchanged. These are internal database client changes only.

Related: #53704, #56282, #53879

[opencode-agent]

Because statements inside a transaction fail fast here, any write in db.transaction(...) still fails on a foreign write lock. That covers Bus.publish, publishAll and replay and the write in session.ts, which is most of what v2 writes during a prompt. I checked with a select-then-update transaction while another connection held begin immediate for 200ms: it still failed at once on this branch. Retrying at this layer can only help single statements. The transactional paths need the whole transaction rerun, which is what #53704 does.

Any statement that reaches this point outside a transaction context while some transaction is open runs straight away, without taking the semaphore. That's intended for this transaction's own BEGIN/COMMIT/ROLLBACK, but the client-wide transactionOpen flag can't tell who is calling. It would help to tie this to the transaction's own control statements, or document why nothing else can get here.

[woodsonl]

Documented in a7af455. The invariant: every gated statement runs holding the client semaphore — both acquirers take it before handing out the connection, and the statement executes inside the acquirer's scope — and transactionOpen is true exactly while a transaction holds that semaphore. A foreign statement therefore never observes the flag true: it queues on the semaphore until the transaction closes, by which point the flag has flipped back. The only service-less callers that arrive with the flag set are effect-sql's BEGIN (run before it provides the transaction service) and COMMIT/ROLLBACK (run after that context is withdrawn); nested savepoints run inside the outer transaction's context, so they arrive on the fail-fast path above instead. Structurally tying the bypass to those three statements would mean matching their SQL text, which felt more brittle than stating the invariant — happy to switch if you prefer.

[opencode-agent]

This test depends on timers lining up: the lock holder releases on a 300ms setTimeout, and the fibers use 10, 30 and 120ms sleeps. On a loaded CI runner the order can change and the test can pass or fail for the wrong reason. Could you control the order directly, for example with a Deferred or latch, and release the holder from the test?

[woodsonl]

Rewritten in a7af455. The holder is now a second connection released by the test itself (never setTimeout), and the cross-fiber race is ordered with a Deferred: the retrying statement forks first and gets a full second to enter the retry loop, the transaction then opens and parks mid-body on the Deferred, the test frees the foreign lock, and only then releases the transaction. The remaining sleeps are wide margins rather than aligned windows — effect sleeps never fire early, so the first attempt always precedes a release scheduled a second later. Shared setup (schema/WAL, holder, value reads) took the file from 361 lines / 7 cases to ~250 / 6 focused cases; the two fail-fast cases are unchanged in spirit and the stale-snapshot one has no timing at all. Verified 4 of the 6 fail against the pre-fix drivers, the race test deterministically.

[woodsonl]

a7af455 addresses the review, and the branch is rebased onto the latest dev (it was >1100 commits behind; the touched files were unchanged there apart from the isRecord import path).

On transaction coverage. I checked against #53704 and kept this PR at statement scope on purpose. Rerunning a whole transaction from inside the client is not safe to do generically — the bodies the review lists read and branch between statements (Bus.publish checks the aggregate row and sequence before inserting, session.ts writes carry lifecycle decisions), so whether a rerun is safe is the caller's call, and #53704 already adds exactly that caller-side replay (StorageRetry.write) for the durable event paths. The two layers compose without retrying the same write twice: body statements inside a transaction never retry here (they fail fast, as before), BEGIN/COMMIT/ROLLBACK retry in place while the transaction holds the client semaphore, and a StorageRetry replay only starts after this client's budget is spent — the layers are sequential, never nested per attempt. With #53704's BEGIN IMMEDIATE, the common contention point moves to BEGIN, which this PR already rides out before the body runs. One overlap worth naming: both carry a copy of the node:sqlite ERR_SQLITE_ERROR normalization (#53704's classifier has its own); once both land, the driver-level fix here is the right home and that copy can fold in. Since this PR therefore fixes only the single-statement half of #47566, the description now says Part of #47566 instead of Closes, so the issue stays open until #53704 lands.

On the tests. Rewritten: the holder is a second connection released by the test at wide margins (no setTimeout), the cross-fiber race parks the interfering transaction mid-body on a Deferred the test releases, and no test depends on 10/30/120ms sleeps lining up. 361 lines / 7 cases became ~250 lines / 6 focused cases (bun retry, values path, node classification, stale-snapshot fail-fast, non-lock fail-fast, cross-fiber race) with shared setup. The stale-snapshot test has no timing at all — the foreign write is synchronous between the read and the write. Against the pre-fix drivers 4 of the 6 fail, and the race test now fails deterministically rather than by luck.

On the transactionOpen bypass. Documented in place in gateStatement: every gated statement runs holding the client semaphore (both acquirers take it before handing out the connection and the statement executes inside their scope), and the flag is true exactly while a transaction holds that semaphore, so a foreign statement never observes it true — it queues on the semaphore until the transaction closes, by which point the flag has flipped back. The only service-less arrivals with the flag set are effect-sql's BEGIN (before it provides the transaction service) and COMMIT/ROLLBACK (after that context is withdrawn); nested savepoints run inside the outer transaction's context and fail fast with it. Tying the bypass to the statements structurally would mean matching their SQL text, which seemed worse than stating the invariant; happy to revisit if you'd prefer that.

[opencode-agent]

The retry now matches the narrower scope. On this head the 6 tests in sqlite-lock-retry.test.ts pass, and with the pre-fix sqlite.ts and sqlite.node.ts swapped in, the 4 retry and race tests fail. I couldn't run the package typecheck script here because it calls bun check, which the installed Bun doesn't have. Running tsgo directly shows no errors in the changed files.

Public API: unchanged.

Addressed: the scope is now clear. The PR no longer closes #47566 and leaves the writes inside transactions to #53704. The tests no longer depend on timers lining up: locks are released by the test, and the race test parks its transaction on a Deferred.

Still open: the new comment in gateStatement says why only a transaction's own BEGIN/COMMIT/ROLLBACK can reach the transactionOpen shortcut, but that reasoning doesn't hold for the current acquirer (see the inline comment).

• packages/core/src/database/sqlite.ts:122: This says every gated statement runs while holding the client semaphore. But acquirer is semaphore.withPermits(1)(Effect.succeed(gatedConnection)): it takes the permit and releases it as soon as it has handed back the connection. So a plain statement (whether it goes through effect-sql's acquirer or the per-attempt Effect.andThen(acquirer, attempt)) runs with the permit already released. If that fiber yields after acquirer returns and before gateStatement runs, another fiber's transactionAcquirer can take the semaphore and set transactionOpen. The foreign statement then takes the shortcut and runs inside that transaction, which is the hole this PR set out to close. It's unlikely, but the comment states it as impossible. Either hold the permit for the statement itself (for example semaphore.withPermits(1)(attempt) on the plain path), or reword the comment to describe what is actually guaranteed.

[woodsonl]

Fixed in 3e90cf2. You were right: the comment claimed a guarantee the acquirer didn't provide. It releases the permit as soon as it hands back the connection, so both the first statement and every retry ran with the permit already released.

The gate now holds the permit across the attempt itself (semaphore.withPermits(1)(attempt)), and the flag is gone. Control statements are told apart structurally: transactionAcquirer hands out a connection whose service-less statements (BEGIN, COMMIT, ROLLBACK) retry in place, because the transaction already holds the permit, while plain statements gate through the other connection and take the permit per attempt. Nothing reads outside the permit anymore, so there's no window left to race.

The new test parks a plain write mid-statement, after its acquirer has returned, and opens a transaction right there. On the old gate the write joins that transaction and gets rolled back with it; the test fails deterministically. With the permit held, the transaction can't open until the write commits, and the write survives.

[woodsonl]

3e90cf2 addresses the open item from your last review (the acquirer releasing the permit before the statement runs). Could you re-review the current head?