fix(core): retry sqlite statements on lock timeout - #47567

The following comment was made by an LLM, it may be inaccurate:

Based on my search, I found one potentially related PR:

PR #44558: fix(db): serialize database init and migrations across processes

• fix(db): serialize database init and migrations across processes #44558
• This PR also addresses multi-process database contention issues, though it focuses on initialization and migrations rather than statement-level locking with retry logic.

This is loosely related to the current PR since both handle concurrency in shared SQLite databases, but they solve different problems (migration serialization vs. statement lock retry). The current PR (47567) is the primary fix for runtime statement lock contention with exponential backoff retry logic.

No other duplicate PRs found addressing the same sqlite lock timeout retry mechanism.

Thanks for updating your PR! It now meets our contributing guidelines. 👍

Why this matters

Multi-process / multi-client OpenCode hits SQLite database is locked and dies the prompt instead of waiting out a short lock — operators see a hard failure where a retry would have succeeded (#47566).

Evidence

Baseline on current dev (e03db9b) — no retryLocked on the effect SQL path:

1. Second connection holds BEGIN IMMEDIATE
2. Client sets PRAGMA busy_timeout = 0
3. UPDATE through the effect SQL client

Live result: LockTimeoutError / SQLiteError: database is locked (wrapped in EffectDrizzleQueryError) — no retry. Matches #47566.

PR tip 57f1bda leaf (right scope):

• packages/core/src/database/sqlite.ts — retryLocked (jittered retry on LockTimeoutError)
• packages/core/src/database/sqlite.bun.ts / sqlite.node.ts — statement effects .pipe(retryLocked)
• packages/core/test/sqlite-lock-retry.test.ts — lock-timeout retry coverage
• call sites that Effect.orDie on statement errors still kill a prompt on dev without this

Confirms this PR is the leaf. Happy to help land as-is.

Architecture (proposal)

Keep merge scope = lock-timeout retry only. Durable follow-on: one shared SQLite busy policy (busy_timeout + which errors retry vs orDie) documented at the effect-SQL client boundary so prompt-path writers don’t each invent kill-vs-retry. Not blocking #47567.

Thanks for verifying the repro on dev and confirming the leaf scope. I'm glad the retry path covers the failure you hit.

I agree with your follow-up proposal. A single shared busy policy at the effect-SQL client boundary, with a busy_timeout default and an explicit list of which statement errors retry versus orDie, is the right durable shape. It is deliberately out of scope here so this PR stays a focused lock-timeout fix. If you open an issue for it, I'm happy to weigh in on the error classification list. That is the trickiest part, since it has to distinguish transient lock timeouts from genuine constraint or conflict errors that should keep failing fast.

@woodsonl one case the retry can't recover from, since you mentioned the error classification is the tricky part.

Effect's matchesSqliteCode (effect/unstable/sql/SqlError.ts) turns any SQLITE_BUSY_* or SQLITE_LOCKED_* code into LockTimeoutError, so retryLocked also retries SQLITE_BUSY_SNAPSHOT. SQLite returns that when a deferred WAL transaction has already read and another connection commits before it writes. It comes back at once, without waiting on busy_timeout, and it repeats on every attempt of the same statement until the transaction is rolled back. So inside a transaction the retry spends its whole 30 s budget and then fails with the same error.

Repro with bun:sqlite on Bun 1.3.14, two connections to one WAL file, busy_timeout = 5000 on both:

a.run("BEGIN") // deferred
a.query("SELECT v FROM t WHERE id = 1").get() // takes a read snapshot
b.run("UPDATE t SET v = v + 1 WHERE id = 1") // b commits
a.run("UPDATE t SET v = v + 10 WHERE id = 1") // SQLITE_BUSY_SNAPSHOT (517), 0 ms
a.run("UPDATE t SET v = v + 10 WHERE id = 1") // SQLITE_BUSY_SNAPSHOT again, 0 ms
a.run("ROLLBACK")
a.run("BEGIN")
a.run("UPDATE t SET v = v + 10 WHERE id = 1") // ok

I'm carrying a fix for this on my fork: 54f5929. retryLocked takes an inTransaction predicate (native.inTransaction on Bun, native.isTransaction on node:sqlite) and only retries when it's false. Inside a transaction the error surfaces at once, so the caller can roll back and start over. A BEGIN IMMEDIATE that loses to another writer is still retried, since the connection isn't in a transaction until BEGIN succeeds (inTransaction reads false after the failed BEGIN IMMEDIATE). The commit adds a test that a SQLITE_BUSY_SNAPSHOT inside a deferred transaction fails in under 2 s. Feel free to fold it into this PR.

Thanks for the detailed repro. I checked matchesSqliteCode in the effect version we pin and confirmed the prefix match, so SQLITE_BUSY_SNAPSHOT does land in LockTimeoutError. And since run and runValues also serve statements inside drizzle transactions, the retry would have looped the snapshot error for the full 30 seconds and then failed the same way. Failing fast inside a transaction is strictly better.

I also agree with surfacing plain SQLITE_BUSY inside a transaction immediately. busy_timeout already waits within the statement, and the caller needs the error so it can roll back and restart the transaction.

I cherry-picked 54f5929 (4291616) with your authorship intact. The new test passes, and the core suite is green with typecheck clean. Thanks for including the test as well; the deferred-transaction setup is clean.

This branch has been upgraded to V2. We're no longer taking PRs for V1. If you think this is still relevant, please port it over and open it for V2.

— from 𝕺𝖕𝖊𝖓𝕮𝖔𝖉𝖊