Skip to content

fix(db): prevent singleton poisoning when migrate() throws#111

Merged
BYK merged 1 commit intomainfrom
fix/db-singleton-poisoning
Apr 29, 2026
Merged

fix(db): prevent singleton poisoning when migrate() throws#111
BYK merged 1 commit intomainfrom
fix/db-singleton-poisoning

Conversation

@BYK
Copy link
Copy Markdown
Owner

@BYK BYK commented Apr 29, 2026

Summary

  • Fix db() singleton poisoning: the module-level instance was assigned before migrate() ran, so if migration threw (SQLITE_BUSY from lock contention, partial prior run with duplicate ALTER TABLE, disk error), every subsequent db() call returned the un-migrated handle — causing "no such table: kv_meta" errors for the rest of the process lifetime.
  • The fix uses a local variable and only promotes it to the cached singleton after migrate() succeeds, so a failed init is retried on the next call.
  • Added tests for kv_meta table existence and db() re-initialization after close().

All 552 existing tests pass.

If migrate() threw (SQLITE_BUSY, partial prior run, disk error), the
module-level `instance` was already assigned to the un-migrated Database
handle. Every subsequent db() call returned this stale handle via the
`if (instance) return instance` short-circuit, causing 'no such table'
errors (e.g. kv_meta) for the rest of the process lifetime.

Fix: use a local variable for the Database handle and only promote it to
the module-level singleton after migrate() succeeds. If migrate() throws,
`instance` remains undefined and the next db() call retries from scratch.
@BYK BYK enabled auto-merge (squash) April 29, 2026 23:23
@BYK BYK merged commit e445d87 into main Apr 29, 2026
1 check passed
@BYK BYK deleted the fix/db-singleton-poisoning branch April 29, 2026 23:24
@BYK BYK mentioned this pull request Apr 29, 2026
5 tasks
@craft-deployer craft-deployer Bot mentioned this pull request Apr 29, 2026
5 tasks
BYK added a commit that referenced this pull request May 2, 2026
## Summary

- Fix "no such table: kv_meta" on DBs where the schema version is
already at latest (11) but `kv_meta` is missing. Root cause: migration 7
is a multi-statement string (`ALTER TABLE knowledge ADD COLUMN embedding
BLOB; CREATE TABLE IF NOT EXISTS kv_meta ...`). If the ALTER TABLE hits
a duplicate-column error from a prior partial run, `database.exec()`
aborts before `CREATE TABLE kv_meta` runs.
- Catch duplicate-column errors in the migration loop, strip the
already-applied ALTER statements, and re-exec the remaining SQL.
- Add `recoverMissingObjects()` that idempotently creates `kv_meta` —
runs both after fresh migrations and on already-at-latest DBs to heal
existing broken databases.
- Regression test: drops `kv_meta`, close/reopen DB, verifies recovery.

Follows up on #111 (singleton poisoning prevention). That fix prevents
the cached handle from being stale, but doesn't help DBs that are
already at version 11 with the table missing — this PR heals those.

All 583 tests pass.
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.

1 participant