Skip to content

feat(turso): self-heal a stale WAL index on open - #441

Open
jkemp814 wants to merge 4 commits into
tickernelz:mainfrom
jkemp814:feat/turso-open-guard
Open

jkemp814 wants to merge 4 commits into
tickernelz:mainfrom
jkemp814:feat/turso-open-guard

Conversation

@jkemp814

@jkemp814 jkemp814 commented Oct 26, 2026 •

Copy link
Copy Markdown

Summary

Makes a turso store recoverable when its WAL index has outlived its WAL, instead of leaving the whole memory subsystem offline until someone fixes it by hand. Fixes the failure mode in #435; complements #440.

Test plan

  • Exercised the change locally (or N/A for docs/meta-only) — 12 tests across recovery + connection manager, full suite run, and a mutation check (short-circuiting the recovery call fails 3 of 7 new tests)
  • Noted any follow-up or known limitations — see Risks below: recovery/ is never pruned, and a live foreign reader during the move is a judgement call worth a ruling

Checklist

  • Branch is based on main (do not open PRs from commits already on main)
  • bun test passes (or N/A) — 868 pass / 4 skip / 2 pre-existing config failures
  • bun run typecheck passes (or N/A)
  • bun run check passes (format + lint + typecheck; or N/A)
  • Docs / README updated when behavior or contribution process changes — CONFIG_TEMPLATE, opencode-mem.example.jsonc and an English README paragraph for both knobs (translations handled separately)

Makes a turso store recoverable when its WAL index has outlived its WAL, instead of leaving the whole memory subsystem offline until someone fixes it by hand.

Fixes the failure mode reported in #435: <db>-tshm still claims frames the <db>-wal no longer has, so every open fails with either

I/O error: short read on WAL frame at offset N: expected 4096 bytes, got 0
Corrupt database: Invalid page type: 0

and ensureTursoReady() gates memory tools, chat capture and the web server behind it. The main .db is intact in both cases — measured, not assumed — so the store is recoverable by discarding the sidecars.

Behaviour

On either signature, once per database path per process:

  1. Copy the main .db only into a temp dir and run PRAGMA integrity_check on the copy. The live file is never opened by another engine — that is itself a documented way to produce the Invalid page type class.
  2. If the copy is healthy: renameSync the index sidecars -shm/-tshm into <storagePath>/recovery/<dbstem>-<timestamp>/. A -wal is removed only when it is absent or 0 bytes — a non-empty WAL can hold committed transactions and is left in place for the engine to rebuild its index from. Moved, never deleted.
  3. Retry the open once.
  4. Healing is serialised across processes with <storagePath>/recovery/<dbstem>.lock (O_EXCL, stale after 5 min). If the lock cannot be taken, nothing moves and the original error surfaces. Under the lock the open is re-attempted before any file is touched, so a peer that healed first leaves nothing to do.
  5. If the copy is unhealthy, or the retry fails: touch nothing further, log the path and (if any) the quarantine dir, and re-raise the original error. No loops.

Plus a one-time warn per path when -shm appears beside -tshm, i.e. a foreign SQLite process has visited the store. -shm is never deleted — it may belong to a live reader.

Why the warn says "writer" and not "anyone"

Measured against a live multiprocess_wal store (experimental: ["encryption", "multiprocess_wal"] — without those flags stock sqlite cannot even open it):

intruder sidecars after outcome
stock sqlite read-only + -shm consistent, no loss
stock sqlite read-write -tshm only, -wal=0 turso reads a stale snapshot, its next write is silently lost

So read-only inspection is harmless and warning users off it would be wrong; writers are the hazard. Measurement and harness are in #435.

Config

  • tursoAutoHeal (default true) — the guard only runs when the store is already unopenable, and it moves rather than deletes.
  • tursoMultiprocessWal (default true, no behaviour change) — false opens without the multiprocess_wal experimental flag, so a foreign process gets a loud database is locked instead of diverging silently. Reasonable choice for single-user stores; needs a ruling on whether that should become the default.

Documented in CONFIG_TEMPLATE and opencode-mem.example.jsonc. I deliberately left the READMEs alone to avoid i18n churn — happy to add a sentence if you want it.

Verification

  • bun run format:check, bun run lint, bun run typecheck — clean.
  • bun test tests/turso-wal-index-recovery.test.ts tests/turso-connection-manager.test.ts — 12 pass.
  • bun test full suite — 868 pass / 4 skip / 2 fail; both failures are the pre-existing config > CONFIG defaults ones, reproduced on a pristine main in this worktree. (13 other failures in a fresh checkout are only the missing dist/ build; bunx tsc restores them.)
  • The new tests are not vacuous: short-circuiting the recovery call makes 3 of the 7 fail, restoring it makes all 7 pass.
  • All seven cases use the injected ConnectFactory; no native-binary gambling.

Risks / open questions

  • recovery/ is never pruned, and now also holds one small <dbstem>.lock per healed database. Each heal costs one directory of sidecar sizes. A retention sweep is a follow-up, deliberately out of scope here.
  • Keeping a non-empty WAL relies on the engine rebuilding -tshm from it. Measured here (16,512-byte WAL, committed rows recovered after discarding the index) and covered by a regression test, but it is engine behaviour rather than a documented API contract. A torn WAL is the "retry fails, original error surfaces, WAL untouched" case — no silent rollback.
  • If a live foreign reader still maps the -shm being moved, POSIX keeps the inode alive but its path changes. This only triggers on an already-unopenable store, which is why I took the trade — worth a maintainer ruling.
  • The temp-copy cost is one copy plus a full integrity_check of that file on a cold error path, once per path per process. Gating it on sidecar mtimes would weaken the guarantee, so it is unconditional.
  • The copy is main-.db-only by design: if the main file were genuinely mid-checkpoint corrupt, the check fails and the store is left alone.

Relates to #435. Complements #440 (checkpoint before close): prevention on the way out, recovery on the way in.

A turso store whose `-tshm` outlived its `-wal` cannot be opened: either
"I/O error: short read on WAL frame at offset N" or "Corrupt database:
Invalid page type: 0". The main `.db` stays intact in both cases, so the
store is recoverable by discarding the sidecars — but nothing in the
plugin does that, so the whole memory subsystem stays down until someone
intervenes by hand (issue tickernelz#435).

On either signature, verify the main database on a COPY (opening the live
file with another engine is itself a corruption source), move `-shm`,
`-tshm` and `-wal` into `<storage>/recovery/<db>-<timestamp>/` — moved,
never deleted — and retry the open once. Unhealthy copy or failed retry
leaves the store untouched and re-raises the original error. One attempt
per path per process; no loop.

Also warn once per path when a foreign SQLite `-shm` appears beside a
Turso `-tshm`: measured, read-only access is harmless, but a foreign
writer diverges silently and loses its own next write.

Config: `tursoAutoHeal` (default true, only fires when the store is
already unopenable) and `tursoMultiprocessWal` (default true; false makes
foreign access fail loudly with "database is locked" instead of diverging).
Copilot AI balanced review requested due to automatic review settings October 26, 2026 07:27
Both knobs change observable behaviour on shared storage: single-owner mode
turns silent divergence from a foreign writer into a loud 'database is
locked', and auto-heal is what recovers a stale WAL index on open. English
README only; translations are maintained separately.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Recovery can exclude committed WAL transactions and race across processes, while filesystem failures can mask the original open error.

3 open issues
What changed in this PR

Adds automatic recovery for stale Turso WAL indexes that prevent the memory subsystem from starting, addressing #435 and complementing checkpoint-on-close work in #440.

Changes:

  • Verifies a temporary database copy, quarantines sidecars, and retries failed opens once.
  • Adds recovery and multiprocess-WAL configuration options, plus a foreign-SQLite warning.
  • Adds injected-driver tests for recovery, failure handling, and configuration.
File Description
tests/​turso-wal-index-recovery.test.ts Tests recovery paths and configuration switches.
src/​services/​turso/​wal-index-recovery.ts Detects error signatures, checks database copies, and quarantines sidecars.
src/​services/​turso/​connection-manager.ts Integrates recovery, retry limits, and foreign-SQLite warnings.
src/​config.ts Defines and documents the new configuration options.
opencode-mem.example.jsonc Documents recovery and single-owner settings.

Review effort: Balanced

Comment on lines +100 to +103
const quarantineDir = quarantineDirFor(dbPath);
mkdirSync(quarantineDir, { recursive: true });
const moved: string[] = [];
for (const suffix of WAL_SIDECAR_SUFFIXES) {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 1049517: recovery is now serialised across processes with <storagePath>/recovery/<dbstem>.lock, created O_EXCL. If the lock cannot be taken, nothing is moved and the original error is rethrown — a live owner is never raced. Under the lock the open is re-attempted before any file moves, so a peer that healed first leaves nothing to do; lock release runs in finally and cannot throw past the original error. A lock older than five minutes is treated as a crashed healer and taken over (logged). Test: a second healer finds the lock held, moves nothing, and surfaces the original error.

Comment on lines +103 to +106
for (const suffix of WAL_SIDECAR_SUFFIXES) {
const sidecar = `${dbPath}${suffix}`;
if (!existsSync(sidecar)) continue;
renameSync(sidecar, join(quarantineDir, basename(sidecar)));

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 1049517. Recovery no longer touches a non-empty -wal: only the index sidecars -shm/-tshm are quarantined, and -wal is removed only when it is absent or 0 bytes. Committed-but-uncheckpointed rows therefore survive.

The rebuild assumption is measured, not assumed: discarding -tshm and keeping a 16,512-byte WAL with committed rows, the engine reopened and returned both rows. The regression test seeds a real DB, leaves rows only in the WAL, discards the index, reads them back, and asserts a fresh -tshm exists beside the surviving WAL. Documented limit: the evidence covers a fully written final frame; a torn WAL is the 'retry fails, original error surfaces, WAL untouched' path.

}
this.healedPaths.add(dbPath);

const outcome = await quarantineWalSidecars(dbPath, this.connectFactory, opts);

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 1049517: lock acquisition, verification, copy, quarantine and cleanup are all wrapped. Failures are logged with the database path, the quarantine directory and what was already moved, and the original open error is rethrown — a filesystem throw can no longer replace the corruption error or skip its diagnostics. Lock release is in finally with its own guard so it cannot throw past the original error either.

Review findings on tickernelz#441.

Data safety: recovery now quarantines only the index sidecars (-shm, -tshm).
A -wal is removed solely when it is absent or 0 bytes; a non-empty WAL is left
in place, because committed transactions can live there while the main .db
still passes integrity_check. Deleting it would silently roll the store back
to an older snapshot. Measured on this engine: after discarding -tshm and
keeping a 16,512-byte WAL, reopen returns both committed rows — the index is
rebuilt from the surviving WAL. A torn WAL is the documented "retry fails,
original error surfaces, WAL untouched" case.

Cross-process: healing is serialised with <storagePath>/recovery/<dbstem>.lock,
created O_EXCL. A second process that cannot take the lock leaves the store
alone instead of moving files a live owner is writing to; a lock older than
five minutes is treated as a crashed healer and taken over. Under the lock the
open is re-attempted before anything moves, so a peer that healed first leaves
nothing to do.

Error reporting: lock acquisition, verification, copy, quarantine and cleanup
failures are caught and logged with the database path, the quarantine state and
the original error, then the original open error is rethrown — a filesystem
problem can no longer mask the corruption or skip its diagnostics. Lock release
runs in finally and cannot throw past the original error.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants