Skip to content

fix(storage): bump SCHEMA_VERSION so pre-#305 indexes get the valid-time columns - #309

Merged
mavaali merged 1 commit into
mainfrom
claude/schema-version-valid-from-fix
Jul 28, 2026
Merged

fix(storage): bump SCHEMA_VERSION so pre-#305 indexes get the valid-time columns#309
mavaali merged 1 commit into
mainfrom
claude/schema-version-valid-from-fix

Conversation

@mavaali

@mavaali mavaali commented Jul 28, 2026

Copy link
Copy Markdown
Owner

Summary

An index built before #305 cannot be opened after upgrading past it. The vault refuses to serve with:

cannot open index db: no such column: valid_from

#305 added valid_from/valid_until to the documents DDL and to the upsert, but left SCHEMA_VERSION at "10" — the value #303 had already introduced — while editing the comment above it to claim the bump covered them. CREATE TABLE IF NOT EXISTS is a no-op against an existing table, so any index built between #303 and #305 stored version 10 without the columns, matched on the version check, skipped the rebuild, and died on its first upsert.

Recovery today is manual: delete .daftari/index.db. That works — the index is ephemeral by design — but the failure is a hard error with a cryptic message rather than the automatic rebuild SCHEMA_VERSION exists to trigger.

CI cannot catch this class of bug. index.db is gitignored, so every CI run builds a fresh index and never reaches the upgrade path. It only appears when upgrading in place. It surfaced here from a stale local database and reproduced on three separate ones — the sample-vault fixture and both packages/router fixtures, all reporting schema_version: 10 with no valid_from.

Why a bump rather than an ALTER

The bump is cheap now, and #305 is the reason. That same commit removed embeddings from the drop list, on the grounds that it is content-addressed on (content_hash, model, dim) and so cannot be invalidated by a documents column change — test/storage/schema-bump-embeddings.test.ts pins it. A version change therefore rebuilds derived tables from the markdown and leaves the durable vector cache intact; embeddings_vec is a mirror, repopulated at zero provider cost.

So the expensive part of a bump was removed in the same commit that needed one. An idempotent ALTER TABLE migration would be strictly more code for a cost that no longer exists.

Test plan

test/storage/schema-valid-from-migration.test.ts constructs the stale state explicitly — writing a documents table exactly as it stood before #305 plus a schema_version of 10 — because that is the only way to cover a path a fresh checkout cannot reach. Four cases:

  • the database opens instead of returning no such column: valid_from
  • it gains both valid-time columns
  • the freshness manifest is cleared, so the rebuilt tables actually repopulate rather than existing empty
  • the new version is recorded, so the rebuild runs once and not on every open

Two existing assertions in test/storage/index-db.test.ts that hardcoded "10" are updated to "11".

  • npm run lint — pass
  • npm run build — pass (tsc, clean)
  • test/storage/ — 93/93 pass
  • New behavior has tests — yes, the migration test above

Invariants

Touches only the SQLite index, which CLAUDE.md declares ephemeral and rebuildable from the markdown at any time — this change restores that property for the upgrade path rather than bending it. No frontmatter, no new metadata format, no versioning mechanism, no src/tools/ handler.

Note on origin

Found while working on #306 (read-path fence) and originally committed there. Split out at the repo owner's request so it can merge independently — users upgrading past #305 stay broken until it lands, and that shouldn't wait on unrelated design work. #306 no longer carries it.


Generated by Claude Code

…ime columns

#305 added valid_from/valid_until to the documents DDL and to the upsert but
left SCHEMA_VERSION at "10", the value #303 had already introduced, while
editing the comment to claim the bump covered them. CREATE TABLE IF NOT
EXISTS does not alter an existing table, so every index built between the two
stored version 10 with no valid_from, matched on the version check, skipped
the rebuild, and failed its first upsert with `no such column: valid_from`.
The vault then refuses to serve until the operator deletes .daftari/index.db.

CI cannot see this. index.db is gitignored, so every run builds a fresh index
and never reaches the upgrade path — the failure only appears when upgrading
in place. It surfaced from a stale local database and reproduced on three.

The bump is the right fix rather than an ALTER, and cheaply so: #305 also
removed `embeddings` from the drop list, so a version change now rebuilds
derived tables from the markdown and leaves the durable vector cache intact.
The expensive part of a bump was removed in the same commit that needed one.

Adds test/storage/schema-valid-from-migration.test.ts, which constructs the
stale state explicitly — the only way to cover a path CI's fresh checkout
cannot reach — and asserts the database opens, gains both columns, clears the
freshness manifest, and records the new version so the rebuild runs once.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GFCxbrpCHmV1uEeCXNwsiY
mavaali pushed a commit that referenced this pull request Jul 28, 2026
The valid_from upgrade break is a bug on main, unrelated to the read-path
fence. Users upgrading past #305 stay broken until it lands, so it should
not wait on design work under review here. It now lives in #309, branched
from main so it can merge on its own.

The fence design's Decision 5 correction stays. It is about this design —
the ALTER-plus-backfill machinery it specified is dropped because #305 made
a version bump cheap — and it holds whether or not the fix has landed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GFCxbrpCHmV1uEeCXNwsiY
@mavaali
mavaali marked this pull request as ready for review July 28, 2026 17:14
@mavaali
mavaali merged commit 2fce607 into main Jul 28, 2026
16 checks passed
mavaali added a commit that referenced this pull request Jul 28, 2026
Both lineages claimed SCHEMA_VERSION 11 (#309's valid-time missed-bump
fix on main; contextual chunking on this branch). The wave chain is
renumbered 12-14 so no value is claimed twice and every pre-merge
index, either lineage, rebuilds.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
mavaali added a commit that referenced this pull request Jul 28, 2026
… migration test

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
mavaali pushed a commit that referenced this pull request Jul 28, 2026
…ve main

Merging main brought in the MCP 2026-07-28 stateless revision (#312), which
rewrote src/server.ts and grew src/tools/staged-actions.ts. Rather than fold a
silent drift into the merge, every [DATA] citation in the read-path fence
design was re-checked line by line.

Nine had drifted and are corrected: VaultStatusResult, vaultStatus and
scanVaultDocs in read.ts; both checkTierGuard call sites, the sameBody early
return, the vaultMerge targetRaw spread and the domain_warnings shape in
write.ts; the vault_ratify dispatch range in staged-actions.ts; and the
tool-handler-annotated hit fields in hybrid.ts. The index-db drop list gains a
line anchor and the current SCHEMA_VERSION. The resolveAccess call-site list
gains the serve/sleep line numbers it was asserting without.

Two load-bearing claims were re-checked and survive unchanged: the three-channel
result bridge the design depends on came through the v2-SDK migration intact,
and vault_read still declares no `summarize`, so one change still covers both
its channels.

One substantive gap. Decision 5's surface list predates the ratify elicitation
prompt, which #312 added. describeRatifyElicitation interpolates the staged
action's `rationale` verbatim into the message src/server.ts hands the client as
an inputRequired form. `rationale` is required caller-supplied free text on
vault_stage_action, and a propose-only role can stage — so the least-trusted
writer in the RBAC model authors the sentence a human reads at the moment of
approving a write. It ships no document body, so it does not breach the literal
rule, but leaving it off the list would have shipped exactly the failure mode
the section names: a channel carrying foreign text unlabelled. The interpolated
span is fenced with fenceSpan, not the whole message; the surrounding prose is
daftari's own and fencing the container would mislabel it.

Also records that #309 fixed the valid_from SCHEMA_VERSION break, and why CI
structurally could not have caught it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GFCxbrpCHmV1uEeCXNwsiY
@claude claude Bot mentioned this pull request Jul 29, 2026
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