fix(sqlite): stamp the last writer's version and refuse older binaries on newer schemas (refs #3609, slice A)

[akushonkamen]

Refs #3609 (slice A — plan-21 step 2, the writer-version stamp only). This is a standalone, self-contained slice per the plan's PR consolidation pass; the other steps are untouched.

Root cause

Nothing records which binary version last wrote the SQLite database, so an older binary can open a newer schema and run its DDL over it. The live case in the #3609 timeline (@husniadil): a 13.4.2 copy inside a Claude Desktop local-agent sandbox reopened a schema-49 DB, ran its v7 rebuild, and dropped discovery_tokens, ON UPDATE CASCADE, and the three session_summaries FTS triggers — 15 days of silent summary loss. The plan-16 stale-dir refusal can't see sandbox paths; step 2's writer-version stamp is what catches this one.

Demonstrated on current main (eccb15e): opening a DB stamped user_version = 99000000 runs the full migration chain and rebuilds the schema — no error, no record.

Fix

• PRAGMA user_version now stamps the writing binary's version: major*1_000_000 + minor*1_000 + patch (13.34.2 → 13034002, int32-safe), prerelease suffixes compared by their base — same semantics as parseBase in compareVersionsDescending. (If you'd rather keep user_version free, the same five functions can read/write a one-row meta table instead — happy to switch.)
• Both DDL entry points — the SessionStore and SessionSearch constructors — read the stamp before any DDL, pragmas included, and a stamp newer than the running binary throws a typed SchemaNewerThanBinaryError (reason: 'schema_newer_than_binary') naming both versions and the upgrade action, mirroring assertChromaStoreCompatible on the chroma data dir. A refused open touches nothing (user_version and integrity_check verified unchanged).
• Stamp-less databases (every existing install, user_version = 0) are adopted on first boot and stamped — never refused.
• SessionStore stamps before the first schema write, not after the chain: the chain's steps commit separately, so a mid-chain failure must still leave the stamp naming the binary that wrote the partial DDL, keeping the database refused to older binaries (fix(sqlite): stamp the last writer's version and refuse older binaries on newer schemas (refs #3609, slice A) #4602 review). The guard runs before any write, so a refused open still touches nothing. SessionSearch stamps only when its FTS DDL actually created anything: a read-only fallback connection (the shipped search-readonly-fallback contract) writes nothing and records no writer.
• Exit path: the constructor throw flows through the existing worker boot failure (initPhaseTracker → GET /api/ready); no new wiring.

Red → green

• Red (test file against unpatched main): bun test tests/sqlite/session-store-writer-version.test.ts → 0 pass / 1 fail — no stamp guard exists; on main the "refuses a newer-stamped DB" case opens the DB, runs the migration chain, and rewrites the schema (FK rebuilt with ON UPDATE CASCADE, user_version left stale).
• Green (with the fix): same file → 7 pass / 0 fail; full suite bun test tests/sqlite/ → 251 pass / 0 fail / 3.0s (baseline 244/244 + 7 new); bun run typecheck clean.

Tests (tests/sqlite/session-store-writer-version.test.ts)

1. Newer-stamped DB refused at the SessionStore constructor, file untouched (user_version + integrity_check + table set equal before/after).
2. Same at the SessionSearch constructor.
3. Stamp-less legacy DB adopted: migration chain runs, stamp written.
4. Equally-stamped DB reopens without refusing.
5. Older stamp migrates up to the current version (exported-guard level; under bun test the build define is absent so the runtime version is 0.0.0-dev).
6. Prerelease suffix compares as its base.
7. SessionSearch stamps after recreating missing FTS tables (the [plan-21] SQLite Schema Evolution & Queue State Integrity — one DDL path guarded by introspection, immutable session keys, and queue/session states that can actually transition #3609 damage scenario).

Merge rubric self-check

• §1 fixes a bug that exists today — silent schema destruction by an older binary, reproduced on eccb15e and reported with a full timeline in [plan-21] SQLite Schema Evolution & Queue State Integrity — one DDL path guarded by introspection, immutable session keys, and queue/session states that can actually transition #3609; the plan master specs this exact behavior as step 2.
• §2 no failure-tolerance machinery — this is the rubric's encouraged case: a fail-fast conversion (silent tolerance → loud typed error at the DDL boundary). No try/catch, fallback, retry, or degrade path added; the two new conditionals (stamp <= current, "did this connection write DDL") are the correct logic itself.
• §3 no second system — lives inside the existing connection/DDL path in src/services/sqlite/; uses SQLite's built-in PRAGMA user_version. No new processes, state files, or env-var modes.
• §4 scale — +83 src lines across 3 files (encode/decode/read/guard/stamp + a 4-line mount in each constructor) and 223 test lines. Nothing else moved.

Disclosure

Authored by @akushonkamen with AI assistance (Claude); Co-Authored-By: Claude trailer on the commit, matching recent merged history.

---

Diffstat

 src/services/sqlite/SessionSearch.ts              |  16 +-
 src/services/sqlite/SessionStore.ts               |   9 +-
 src/services/sqlite/connection.ts                 |  60 +++++++
 tests/sqlite/session-store-writer-version.test.ts | 223 ++++++++++++++++++++++
 4 files changed, 306 insertions(+), 2 deletions(-)

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	⚠️ Failed 2026-10-09T02:34:36.489183Z	1fb0278	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[greptile-apps]

Confidence Score: 5/5

[Critical impact] The changes address all three previously reported issues. No outstanding finding blocks merging.

Summary

This PR records the SQLite writer's version and refuses older binaries before they change a newer database.

• SessionStore now records the writer stamp before starting migrations, protecting databases even when an upgrade stops partway through.
• The worker checks compatibility before applying persistent connection pragmas, so refusing a newer database does not change its journal mode.
• The tests use a nonzero binary version and reset the stamp before checking SessionSearch, making each constructor's stamp write independently observable.
• All three previous findings are addressed. No outstanding issue blocks merging.

_{Reviews (2) · Last reviewed commit: "fix(sqlite): address review findings on ..." · Reviewed by Greptile}

[akushonkamen]

@thedotmack Slice A is ready for review. Greptile's three findings are addressed in 9efd65b — migration-failure stamp semantics, a guarded open path, and mutation-verified tests — and Greptile's re-review of this commit reports no outstanding issues blocking merge. Happy to adjust anything.

[thedotmack]

Thanks @akushonkamen, this is careful work. Guarding before the pragmas, stamping before the chain, and the mutation-checked tests are all right, and they should carry over unchanged. Before we land it, though, I'd like to change one thing: what the stamp is compared against.

The concern. The stamp is the plugin semver, so any binary older than the last writer refuses to open the database, even when no schema changed between the two versions. Most claude-mem releases don't touch the schema. That makes these everyday situations full outages, where the worker never becomes ready and every host loses memory:

• rolling back one patch after a bad release;
• two hosts sharing ~/.claude-mem with different bundled versions (Claude Code plugin plus Codex/OpenCode/npx copies);
• the plan-16 stale-plugin-dir spawn, which today is mostly harmless.

The destructive case in #3609 is narrower than that: an old binary re-running a rebuild (v7/v9) on a database that already has migrations it doesn't know about.

What I'd suggest:

1. Stamp PRAGMA user_version with the highest SessionStore migration version this binary knows (a LATEST_SCHEMA_VERSION constant next to the chain), not the semver encoding.
2. assertSchemaWriterCompatible refuses only when user_version > LATEST_SCHEMA_VERSION. The message can say "this database has schema vN; this claude-mem knows up to vM; upgrade claude-mem". 0 is still adopted.
3. Add a test that pins LATEST_SCHEMA_VERSION === MAX(version) FROM schema_versions after a fresh open, so a new migration can't forget to bump it. Keep the rest (stamp-before-chain, the guarded shared open, SessionSearch stamping) as is.
4. Your existing tests map across directly. Swap the version strings for schema numbers.

That keeps exactly the protection #3609 needs, without turning harmless downgrades into outages. Happy to help land it once it's in.