fix(runtime): mitigate SQLite contention with bounded recovery

docs/SQLITE_CONCURRENCY.md

@@ -1,8 +1,64 @@
-# SQLite contention regression (#162 / #163)
+# SQLite contention recovery (#162 / #163)
+
+## Ordinary writes and maintenance
+
+The local compatibility bridge installs recovery on the native session store
+after successful migration. The upstream store remains the owner of SQL,
+transactions, IDs and session state. No schema changes or whole-turn retries are
+introduced.
+
+- Audited input, message, session, permission and todo operations may retry
+  `SQLITE_BUSY` (including its extended codes). An operation retries only after
+  its transaction has rolled back, or when its existing ID-based writes are
+  idempotent. `SQLITE_LOCKED`, constraint, filesystem and other errors propagate.
+- Writes through the bridge serialize by database path within one process.
+  Conversation writes precede queued usage maintenance; ordering within each
+  class is FIFO. Existing externally owned transactions retain native behavior
+  and are never replayed by the bridge. Counter/goal and legacy workflow writes
+  are serialized but not replayed because a partial commit is not idempotent.
+- Retryable operations use a 25 ms native busy wait, scoped to each synchronous
+  SQLite call. The original connection timeout is restored before returning
+  from that call, including failures. Async backoff uses jitter and a maximum
+  delay of 200 ms. The total budget is 30 seconds, including time spent in the
+  local queue. Busy usage writes serve pending conversation writes between
+  attempts while later usage writes remain ordered behind them.
+  A synchronous SQL statement already executing cannot be preempted.
+- Closing a store cancels its queued writes and backoff. Exhaustion preserves
+  the SQLite cause and reports the operation, attempts and elapsed time. Model
+  calls, shell commands and completed tools are outside the retry boundary.
+- Automatic usage cleanup runs at most once per connection per five minutes,
+  after checking whether any rows are expired. A busy cleanup is deferred for
+  one second and does not fail an already committed usage fact. Explicit
+  `beforeTime` cleanup requests retain the upstream behavior and error handling.
+  Retention remains 30 days, and successful usage writes are never buffered.
+
+The synchronous dynamic-workflow journal and synchronous permission-mode API
+retain their native contract and timeout. They do not acquire an async retry
+capability through this bridge. Cross-process contention is coordinated by
+SQLite and bounded backoff, not by the process-local queue.
+
+The operation allowlist and maintenance SQL were reviewed against upstream
+`zai-org/ZCode` commit `29628c9acdb81b703bbd4080c207a0e7ce5e276e`. The required
+`sqlite-write-recovery` patch validates both post-migration hooks and the native
+cleanup transaction. Missing, ambiguous or partially patched anchors stop
+synchronization. The implementation is in `src/runtime-sqlite-recovery.ts`,
+with scoped native waits and cleanup admission in its two SQLite helpers;
+`vendor/cli-config.cjs` is their existing distribution boundary.
+
+Required regression coverage includes both store-open paths, recovered and
+exhausted locks, responsive timers during backoff, same-process connections,
+input-promotion rollback, ordered updates, close during recovery, preserved
+non-BUSY errors, and sustained writes from 10–15 independent processes. Cleanup
+tests must verify retention and that repeated usage writes do not each open a
+cleanup transaction. All storage regressions run on real Node SQLite against
+the extracted release artifact.
+
+## Original timeout mitigation
 
 The CLI's shared session database uses SQLite WAL. Readers can overlap a writer,
-but different CLI processes still serialize writes to this file. After successful
-store initialization, each connection now waits up to 10 seconds for a write lock.
+but different CLI processes still serialize writes to this file. The original
+#163 mitigation sets a 10-second connection timeout after initialization. This
+remains the default outside the scoped retryable calls described above.
 Both the synchronous constructor and asynchronous `openStartup()` path apply this
 setting **after** migrations finish. Startup migration lock budgets, short busy
 waits, backoff, rollback, and failure cleanup are unchanged.
@@ -29,28 +85,40 @@ requests. They cover:
 - the effective `PRAGMA busy_timeout` after sync open, async startup, and reopen;
 - a native session write blocked by another process for 6.5 seconds, longer than
   the previous 5-second timeout, then succeeding exactly once;
-- a deliberately shortened timeout that reports `SQLITE_BUSY`, writes no session,
-  and permits a later write after the lock is released;
+- recovery after a 10.5-second lock, with timers progressing during backoff;
+- a deliberately shortened recovery budget that reports `SQLITE_BUSY`, writes
+  no session, and permits a later write after the lock is released;
 - four independent processes migrating the same fresh database and persisting
   distinct sessions without duplicate migrations or integrity errors;
 - killing a writer with uncommitted changes, then reopening without those changes
-  and successfully writing another session.
+  and successfully writing another session;
+- same-process connections completing an asynchronous native transaction without
+  starving its commit, plus caller-owned transactions retaining rollback ownership;
+- a failure after message/part insertion that rolls back before input promotion
+  retries, and an idempotent partial part write that cannot overtake a newer update;
+- non-BUSY errors and partially committed goal counters never being replayed;
+- closing a store cancelling both backoff and pending writes;
+- 100 usage facts requiring one cleanup transaction when expired rows exist,
+  with recent records retained and explicit cleanup still honored;
+- retrying usage writes yielding priority to conversation writes;
+- twelve independent processes persisting 300 input promotions and matching
+  messages, parts and usage records, followed by integrity and foreign-key checks.
 
-The hold-and-release test needs a separate process: `DatabaseSync` blocks the
-calling event loop, so a timer in that same process cannot release the lock.
-Each test creates its own database and holder rather than reusing the remainder
-of a lock window from a previous measurement.
+Lock holders use independent processes. One test schedules their release from
+the writer's event loop to verify that the new recovery yields instead of
+blocking that timer. Each test creates its own database and holder.
 
 CI builds and packs once, then tests that exact artifact on Node 22.19.0, 24, and
 26 on Linux, plus Node 24 on macOS. Bun's `node:sqlite` compatibility implementation
 is not used as a substitute for Node's SQLite driver.
 
 ## Follow-up boundary
 
-If real workloads still exceed the wait budget, investigate long transactions and
-add bounded retries only at persistence boundaries that can be safely rolled back
-and replayed. Never retry a whole agent turn and repeat completed external tools.
-Per-session databases or a shared writer service are separate architectural changes
-requiring discovery, lifecycle, and migration design. The contention regression
-tests also pass against the locked Desktop 3.14.0 runtime. They do not establish
-that sustained high-concurrency workloads are free of contention.
+The regressions use the locked Desktop 3.14.3 / runtime 0.16.9 artifact. They do
+not establish that every production workload is free of contention. Keep #162
+open for validation on the reported workload. On recovery exhaustion, the error
+includes `operation`, `attempts`, `elapsedMs` and the original SQLite cause;
+use those fields to identify the next persistence boundary needing investigation.
+Synchronous journal writes, non-idempotent counter operations, larger transactions
+and cross-process fairness remain separate follow-up areas. Per-session databases
+or a shared writer service require lifecycle and migration design.

scripts/check-runtime.ts

@@ -18,13 +18,15 @@ import {
   hasRuntimeHttpNoContentGuard,
   hasRuntimeNetworkRetryGuard,
   hasRuntimeSqliteBusyTimeout,
+  hasRuntimeSqliteWriteRecovery,
   hasRuntimeStreamEofFinishGuard,
   patchRuntimeGoalFailurePause,
   patchRuntimeHttpNoContent,
   patchRuntimeLoginModelDefaults,
   patchRuntimeNetworkRetryClassification,
   patchRuntimeOfficialMcpAvailability,
   patchRuntimeSqliteBusyTimeout,
+  patchRuntimeSqliteWriteRecovery,
   patchRuntimeStreamEofFinishGuard,
   parseRuntimePatchReports,
   runtimePatchPlan,
@@ -85,6 +87,8 @@ if (patchRuntimeLoginModelDefaults(runtimeSource) !== runtimeSource
   || !hasRuntimeNetworkRetryGuard(runtimeSource)
   || patchRuntimeSqliteBusyTimeout(runtimeSource) !== runtimeSource
   || !hasRuntimeSqliteBusyTimeout(runtimeSource)
+  || patchRuntimeSqliteWriteRecovery(runtimeSource) !== runtimeSource
+  || !hasRuntimeSqliteWriteRecovery(runtimeSource)
   || patchRuntimeStreamEofFinishGuard(runtimeSource) !== runtimeSource
   || !hasRuntimeStreamEofFinishGuard(runtimeSource)
   || (patchEnabled("cli-help-contract") && !hasRuntimeCliHelpContract(runtimeSource))

scripts/runtime-sqlite-patches.ts

@@ -0,0 +1,88 @@
+export const sqliteBusyTimeoutMs = 10_000;
+
+const pragma = `pragma busy_timeout = ${sqliteBusyTimeoutMs}`;
+const helper = 'require(require("node:path").join(__dirname,"cli-config.cjs"))';
+const identifier = "[A-Za-z_$][\\w$]*";
+const boundedIdentifier = "[A-Za-z_$][\\w$]{0,80}";
+
+function escape(value: string): string {
+  return value.replace(/[.*+?^${}()|[\]\\]/gu, "\\$&");
+}
+
+function count(source: string, pattern: RegExp): number {
+  return [...source.matchAll(pattern)].length;
+}
+
+/** Both open paths must retain their steady-state timeout after migration. */
+export function hasRuntimeSqliteBusyTimeout(runtime: string): boolean {
+  const recovery = escape(`,${helper}.installSqliteWriteRecovery(`);
+  const sync = new RegExp(`try\\{${identifier}!==${identifier}&&\\(${identifier}\\(this\\.db,this\\.dbPath,${identifier}\\),this\\.db\\.exec\\("${pragma}"\\)(?:${recovery}this\\))?\\)\\}catch`, "gu");
+  const startup = new RegExp(`try\\{return await ${identifier}\\((${identifier})\\.db,\\1\\.dbPath,${identifier}\\),\\1\\.db\\.exec\\("${pragma}"\\)(?:${recovery}\\1\\))?,\\1\\}catch`, "gu");
+  return count(runtime, sync) === 1 && count(runtime, startup) === 1;
+}
+
+export function patchRuntimeSqliteBusyTimeout(runtime: string): string {
+  if (hasRuntimeSqliteBusyTimeout(runtime)) return runtime;
+  const sync = /try\{([A-Za-z_$][\w$]*)!==([A-Za-z_$][\w$]*)&&([A-Za-z_$][\w$]*)\(this\.db,this\.dbPath,([A-Za-z_$][\w$]*)\)\}catch/gu;
+  const startup = /try\{return await ([A-Za-z_$][\w$]*)\(([A-Za-z_$][\w$]*)\.db,\2\.dbPath,([A-Za-z_$][\w$]*)\),\2\}catch/gu;
+  if (count(runtime, sync) !== 1 || count(runtime, startup) !== 1) {
+    throw new Error("ZCode runtime is incompatible with the SQLite busy-timeout patch (store migration anchors missing or ambiguous).");
+  }
+  const patched = runtime.replace(sync, (_match, mode, deferred, migrate, timeout) =>
+    `try{${mode}!==${deferred}&&(${migrate}(this.db,this.dbPath,${timeout}),this.db.exec("${pragma}"))}catch`
+  ).replace(startup, (_match, migrate, store, options) =>
+    `try{return await ${migrate}(${store}.db,${store}.dbPath,${options}),${store}.db.exec("${pragma}"),${store}}catch`
+  );
+  if (!hasRuntimeSqliteBusyTimeout(patched)) throw new Error("SQLite busy-timeout patch failed postcondition verification.");
+  return patched;
+}
+
+export function hasRuntimeSqliteWriteRecovery(runtime: string): boolean {
+  const sync = `${helper}.installSqliteWriteRecovery(this)`;
+  const startup = new RegExp(`${escape(`.db.exec("${pragma}"),${helper}.installSqliteWriteRecovery(`)}(${identifier})\\),\\1\\}catch`, "gu");
+  return hasRuntimeSqliteBusyTimeout(runtime)
+    && runtime.split(sync).length === 2
+    && count(runtime, startup) === 1
+    && runtime.split(`${helper}.pruneSqliteUsage(`).length === 2;
+}
+
+/** Keep the native SQL body and transaction; only the maintenance admission moves into our helper. */
+function patchUsageCleanup(runtime: string): string {
+  const labels = [...runtime.matchAll(new RegExp(`${boundedIdentifier}\\((${boundedIdentifier}),"pruneUsage"\\)`, "gu"))];
+  if (labels.length !== 1) throw new Error("SQLite recovery pruneUsage symbol is missing or ambiguous.");
+  const name = labels[0]![1]!;
+  const prefix = new RegExp(`async function ${escape(name)}\\((${boundedIdentifier}),(${boundedIdentifier})=\\{\\}\\)\\{(?:let|const) (${boundedIdentifier})=\\2\\.beforeTime\\?\\?Date\\.now\\(\\)-[^;]{1,100};`, "gu");
+  const starts = [...runtime.matchAll(prefix)];
+  if (starts.length !== 1) throw new Error("SQLite recovery cleanup boundary is incompatible.");
+  const start = starts[0]!;
+  const [, db, options, cutoff] = start;
+  const bodyStart = start.index! + start[0].length;
+  const tail = new RegExp(`\\}catch\\((${boundedIdentifier})\\)\\{throw ${escape(db!)}\\.exec\\("rollback"\\),\\1\\}\\}`, "u");
+  const end = tail.exec(runtime.slice(bodyStart, bodyStart + 4_000));
+  if (!end) throw new Error("SQLite recovery cleanup transaction is incompatible.");
+  const bodyEnd = bodyStart + end.index + end[0].length - 1;
+  const body = runtime.slice(bodyStart, bodyEnd);
+  if (/\bawait\b|\byield\b/u.test(body)
+    || !body.startsWith(`${db}.exec("begin immediate");try{`)
+    || !["model_usage", "turn_usage", "tool_usage"].every(table =>
+      body.includes(`${db}.prepare("delete from ${table} where started_at < ?").run(${cutoff})`))
+    || !body.includes(`${db}.exec("commit")`)) {
+    throw new Error("SQLite recovery cleanup SQL changed; review the native transaction before patching.");
+  }
+  const replacement = `${start[0]}return ${helper}.pruneSqliteUsage(${db},${options},${cutoff},()=>{${body}})}`;
+  return runtime.slice(0, start.index) + replacement + runtime.slice(bodyEnd + 1);
+}
+
+export function patchRuntimeSqliteWriteRecovery(runtime: string): string {
+  if (hasRuntimeSqliteWriteRecovery(runtime)) return runtime;
+  if (runtime.includes(".installSqliteWriteRecovery(") || runtime.includes(".pruneSqliteUsage(")) {
+    throw new Error("SQLite recovery patch is partial or ambiguous.");
+  }
+  if (!hasRuntimeSqliteBusyTimeout(runtime)) throw new Error("SQLite recovery requires the post-migration timeout patch.");
+  const timeout = new RegExp(`(${identifier})\\.db\\.exec\\("${pragma}"\\)`, "gu");
+  if (count(runtime, timeout) !== 2) throw new Error("SQLite recovery open boundaries are ambiguous.");
+  let patched = runtime.replace(timeout, (call, store) => `${call},${helper}.installSqliteWriteRecovery(${store})`);
+  patched = patchUsageCleanup(patched);
+  if (!hasRuntimeSqliteWriteRecovery(patched)) throw new Error("SQLite recovery patch failed postcondition verification.");
+  return patched;
+}

scripts/sync-runtime.ts

@@ -14,6 +14,14 @@ import {
 } from "../src/runtime-capabilities.ts";
 import { parseReleaseVersion, syncedReleaseVersion } from "./release-version.ts";
 import { markRuntimeModified } from "./runtime-attribution.ts";
+import {
+  hasRuntimeSqliteBusyTimeout, hasRuntimeSqliteWriteRecovery,
+  patchRuntimeSqliteBusyTimeout, patchRuntimeSqliteWriteRecovery
+} from "./runtime-sqlite-patches.ts";
+export {
+  hasRuntimeSqliteBusyTimeout, hasRuntimeSqliteWriteRecovery,
+  patchRuntimeSqliteBusyTimeout, patchRuntimeSqliteWriteRecovery, sqliteBusyTimeoutMs
+} from "./runtime-sqlite-patches.ts";
 
 const root = resolve(dirname(fileURLToPath(import.meta.url)), "..");
 const cdnRoot = "https://cdn-zcode.z.ai/zcode/electron/releases";
@@ -1105,44 +1113,6 @@ export function patchRuntimeHttpNoContent(runtime: string): string {
   return changed ? patched : runtime;
 }
 
-export const sqliteBusyTimeoutMs = 10_000;
-
-const runtimeSqliteBusyTimeoutPragma = `pragma busy_timeout = ${sqliteBusyTimeoutMs}`;
-
-/** Verify both successful store-open paths, not an unrelated pragma string. */
-export function hasRuntimeSqliteBusyTimeout(runtime: string): boolean {
-  const pragma = escapeRegExpName(runtimeSqliteBusyTimeoutPragma);
-  const sync = new RegExp(`try\\{[A-Za-z_$][\\w$]*!==[A-Za-z_$][\\w$]*&&\\([A-Za-z_$][\\w$]*\\(this\\.db,this\\.dbPath,[A-Za-z_$][\\w$]*\\),this\\.db\\.exec\\("${pragma}"\\)\\)\\}catch`, "gu");
-  const startup = new RegExp(`try\\{return await [A-Za-z_$][\\w$]*\\(([A-Za-z_$][\\w$]*)\\.db,\\1\\.dbPath,[A-Za-z_$][\\w$]*\\),\\1\\.db\\.exec\\("${pragma}"\\),\\1\\}catch`, "gu");
-  return countRegExpMatches(runtime, sync) === 1 && countRegExpMatches(runtime, startup) === 1;
-}
-
-/**
- * Concurrent zcode processes share ~/.zcode/cli/db/db.sqlite. The runtime opens
- * it with a 5s timeout. Async startup temporarily uses a short timeout for
- * migration retries and resets it in finally. Apply the steady-state timeout
- * AFTER either migration path succeeds, preserving startup's lock budget and
- * cleanup. This bounds ordinary write contention; it is not a transaction or
- * whole-turn retry, and cannot guarantee success under sustained contention.
- */
-export function patchRuntimeSqliteBusyTimeout(runtime: string): string {
-  if (hasRuntimeSqliteBusyTimeout(runtime)) return runtime;
-  const sync = /try\{([A-Za-z_$][\w$]*)!==([A-Za-z_$][\w$]*)&&([A-Za-z_$][\w$]*)\(this\.db,this\.dbPath,([A-Za-z_$][\w$]*)\)\}catch/gu;
-  const startup = /try\{return await ([A-Za-z_$][\w$]*)\(([A-Za-z_$][\w$]*)\.db,\2\.dbPath,([A-Za-z_$][\w$]*)\),\2\}catch/gu;
-  if (countRegExpMatches(runtime, sync) !== 1 || countRegExpMatches(runtime, startup) !== 1) {
-    throw new Error("ZCode runtime is incompatible with the SQLite busy-timeout patch (store migration anchors missing or ambiguous).");
-  }
-  const patched = runtime.replace(
-    sync,
-    (_match, mode: string, deferred: string, migrate: string, timeout: string) => `try{${mode}!==${deferred}&&(${migrate}(this.db,this.dbPath,${timeout}),this.db.exec("${runtimeSqliteBusyTimeoutPragma}"))}catch`
-  ).replace(
-    startup,
-    (_match, migrate: string, store: string, options: string) => `try{return await ${migrate}(${store}.db,${store}.dbPath,${options}),${store}.db.exec("${runtimeSqliteBusyTimeoutPragma}"),${store}}catch`
-  );
-  if (!hasRuntimeSqliteBusyTimeout(patched)) throw new Error("SQLite busy-timeout patch failed postcondition verification.");
-  return patched;
-}
-
 function escapeRegExpName(value: string): string {
   return value.replace(/[.*+?^${}()|[\]\\]/gu, "\\$&");
 }
@@ -1629,6 +1599,12 @@ export const runtimePatchPlan: readonly RuntimePatchDefinition[] = [
     apply: patchRuntimeSqliteBusyTimeout,
     verify: hasRuntimeSqliteBusyTimeout
   },
+  {
+    id: "sqlite-write-recovery",
+    requirement: "required",
+    apply: patchRuntimeSqliteWriteRecovery,
+    verify: hasRuntimeSqliteWriteRecovery
+  },
   {
     id: "oauth-http-errors",
     requirement: "optional",

src/runtime-config-bridge.ts

@@ -6,6 +6,7 @@ import { cliSettingsPath, legacyCliConfigPath, providerConfigPath, providerMigra
 export { assertSessionModelReady, readSessionModelState } from "./session-model-recovery.ts";
 export { readTuiRuntimeProjection, sendTuiBackgroundTaskMessage } from "./runtime-tui-bridge.ts";
 export { restoreTuiBackgroundTasks } from "./runtime-background-restore.ts";
+export { installSqliteWriteRecovery, pruneSqliteUsage, sqliteRecoveryStats } from "./runtime-sqlite-recovery.ts";
 
 function record(value: unknown): Record<string, unknown> | undefined {
   return value && typeof value === "object" && !Array.isArray(value) ? value as Record<string, unknown> : undefined;