From 4c32057cb70ee092b546e3a9e00811217a467120 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 25 Jul 2026 08:47:21 +0000 Subject: [PATCH 1/2] store: convert every pre-convention table to SQLite STRICT (v27) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #148. STRICT rejects any value whose storage class can't be losslessly converted to the column's declared type instead of coercing it through type affinity. The schema was already STRICT-compatible — only INTEGER/TEXT/BLOB columns, ns-integer timestamps, INTEGER CHECK (x IN (0,1)) booleans, fixed-length BLOB hashes — so no row in a healthy database changes. What changes is the failure mode of a future mishap (a stray string bound into size_bytes, a concatenated query, a reflection accident): a hard error at the write instead of a wrong-storage-class row. STRICT can't be switched on with ALTER, so migrate_v27.go rebuilds each of the eighteen tables that predate the convention: create a STRICT copy, INSERT…SELECT every row, drop the original, rename the copy into place, recreate its indexes and triggers. The sweep runs in one transaction with foreign keys off — off because the drops and renames would otherwise trip the dense FK graph, and because a rename with FKs on would rewrite other tables' REFERENCES clauses to point at the scratch name — and verifies PRAGMA foreign_key_check before commit, like the v4→v5 and v24→v25 rebuilds. Per-table row counts are compared so a short copy fails the migration instead of losing observations, and a table the chain never materialised is skipped rather than diagnosed here. Tests: TestSchemaIsAllStrict makes the AGENTS.md convention executable (any non-STRICT table at SchemaVersion fails, including in future migrations); TestStrictRebuildsCoverEveryV26Table pins the conversion list against the non-STRICT tables a v26 database actually carries; TestMigrateV26ToV27PreservesEveryRow drives the real chain to v26, seeds every table, and checks every row, id, blob, index and trigger survives with the FK graph clean; TestMigrateV26ToV27KeepsSchemaGuarantees checks the contents append-only triggers and the one-live-row-per-path index still bite, and that STRICT now refuses a mistyped bind. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01HompuyJMt84zb3KGZENkTF --- AGENTS.md | 16 +- cmd/squirrel/db_test.go | 12 +- store/migrate_v27.go | 541 ++++++++++++++++++++++++++++++++++++++ store/migrate_v27_test.go | 453 +++++++++++++++++++++++++++++++ store/migrations.go | 14 +- store/schema.sql | 346 ++++++++++++------------ 6 files changed, 1200 insertions(+), 182 deletions(-) create mode 100644 store/migrate_v27.go create mode 100644 store/migrate_v27_test.go diff --git a/AGENTS.md b/AGENTS.md index 939418f..a5bfcd7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -54,7 +54,7 @@ on drift, so CI catches a stale snapshot. `squirrel db schema` prints the DDL of a database directly (opening it runs migrations first), for inspecting a real index without a repo checkout. -Every **new** `CREATE TABLE` — whether in a migration or a future fresh +Every `CREATE TABLE` — whether in a migration or a future fresh baseline — MUST be declared `STRICT` (`CREATE TABLE … (…) STRICT`). STRICT rejects any value that can't be losslessly converted to the column's declared type instead of silently coercing it via type affinity, so a non-numeric string @@ -70,11 +70,15 @@ This is exactly the discipline the schema already follows — ns-integer timestamps (`…_ns INTEGER`), `INTEGER … CHECK (x IN (0, 1))` booleans, and fixed-length `BLOB` hashes (`CHECK (length(h) = 32)`). -The ~13 tables that predate this convention are **not** STRICT yet: STRICT -can't be added with `ALTER`, so each needs a full rebuild (create STRICT copy -→ `INSERT … SELECT` → drop → rename) recreating every index and trigger. That -bulk conversion is a dedicated, well-tested migration PR of its own — don't -ride it along with a feature, and don't convert tables ad hoc. +Every table is STRICT as of **v27**: the eighteen that predated the +convention were converted in bulk by `store/migrate_v27.go` (STRICT can't be +added with `ALTER`, so each was rebuilt — create STRICT copy → `INSERT … +SELECT` → drop → rename → recreate every index and trigger, all in one +transaction with foreign keys off and a `foreign_key_check` before commit). +`TestSchemaIsAllStrict` fails if any table at `SchemaVersion` is not STRICT, +so the convention is enforced, not just documented. Rebuilding a table for +some other reason must carry the keyword forward — dropping it is a +regression, not a neutral rewrite. # Code quality diff --git a/cmd/squirrel/db_test.go b/cmd/squirrel/db_test.go index 3609527..7d7081c 100644 --- a/cmd/squirrel/db_test.go +++ b/cmd/squirrel/db_test.go @@ -243,15 +243,19 @@ func TestCLIDBRestoreRejectsSchemaMismatch(t *testing.T) { // TestCLIDBSchemaPrintsDDL confirms `squirrel db schema` dumps the // opened database's DDL, including the invariants the schema enforces: -// the foundational volumes table, the content entity table, and the -// one-live-row-per-path partial unique index. +// the foundational volumes table, the content entity table, the +// one-live-row-per-path partial unique index, and STRICT typing. The +// table names come back quoted because the v27 STRICT conversion +// rebuilt them under a scratch name and renamed them into place — +// SQLite quotes the substituted name in the stored DDL. func TestCLIDBSchemaPrintsDDL(t *testing.T) { f := writeSyncFixture(t) out := runCLI(t, "--config", f.configPath, "db", "schema") for _, want := range []string{ - "CREATE TABLE volumes", - "CREATE TABLE contents", + `CREATE TABLE "volumes"`, + `CREATE TABLE "contents"`, "uniq_files_live_per_path", + ") STRICT", } { if !strings.Contains(out, want) { t.Fatalf("db schema output missing %q:\n%s", want, out) diff --git a/store/migrate_v27.go b/store/migrate_v27.go new file mode 100644 index 0000000..eb8afdc --- /dev/null +++ b/store/migrate_v27.go @@ -0,0 +1,541 @@ +package store + +import ( + "context" + "database/sql" + "fmt" + "strings" +) + +// This file holds the v26 → v27 bulk conversion of every pre-convention +// table to SQLite STRICT (issue #148). It lives apart from migrations.go +// because it is a mechanical rewrite of the whole schema rather than a +// feature step: the table bodies below are the authoritative shape of the +// eighteen tables that predate the STRICT convention, transcribed verbatim +// from v26 with `STRICT` appended and nothing else changed. The registry +// entry that runs it stays in migrations.go with every other step. + +// strictRebuildSuffix names the scratch table each rebuild copies into +// before it is renamed over the original. Naming it after the target +// version follows the convention of the earlier rebuilds (files_v3, +// runs_v25) and keeps the name recognisable if one ever shows up in a +// debugger or a `db schema` dump. +const strictRebuildSuffix = "_v27" + +// strictRebuild is one table's conversion. body is the parenthesised +// column/constraint list of the CREATE TABLE, and objects holds the indexes +// and triggers to recreate after the rename — SQLite drops those along with +// the old table, and they are not carried by the copy. +// +// body deliberately omits the table name and the STRICT keyword: +// rebuildTableAsStrict spells the scratch name itself, so no spec can name +// the wrong table or forget the keyword this migration exists to add. +type strictRebuild struct { + table string + body string + objects []string +} + +// migrateV26ToV27 converts every remaining non-STRICT table to STRICT, +// closing the second half of issue #148 (the first half — STRICT for new +// tables — has been convention since v25's destination_alarms). +// +// STRICT makes SQLite reject any value whose storage class can't be +// losslessly converted to the column's declared type, instead of quietly +// coercing it through type affinity. Every column in this schema already +// declares INTEGER, TEXT, or BLOB and every writer already binds matching +// Go types, so no row in a healthy database changes; what changes is the +// failure mode of a future mishap (a stray string bound into size_bytes, a +// concatenated query, a reflection accident): a hard error at the write +// instead of a wrong-storage-class row that reads back as garbage. That is +// the same integrity-first stance as the CHECK / NOT NULL / append-only +// trigger discipline the schema already carries. +// +// STRICT cannot be switched on with ALTER, so each table is rebuilt with +// the standard recipe: create a STRICT copy, INSERT…SELECT every row, drop +// the original, rename the copy into place, recreate its indexes and +// triggers. The whole sweep runs in one transaction with foreign keys off — +// off because the drops and renames would otherwise trip the dense FK graph +// mid-rebuild, and because a rename with FKs enabled would rewrite other +// tables' REFERENCES clauses to point at the scratch name. PRAGMA +// foreign_key_check verifies the result before commit, exactly as the v4→v5 +// and v24→v25 rebuilds do. +// +// Cost: this rewrites the entire database, so it is O(index size) in time +// and needs room for a second copy of the largest table. It is a one-time +// cost at upgrade, and Open's pre-migration snapshot is the rollback +// surface if anything goes wrong. Row counts are compared per table so a +// silently short copy fails the migration rather than losing observations. +func migrateV26ToV27(ctx context.Context, db *sql.DB) error { + conn, restore, err := disableForeignKeys(ctx, db) + if err != nil { + return err + } + defer restore() + + tx, err := conn.BeginTx(ctx, nil) + if err != nil { + return err + } + defer tx.Rollback() + + for _, r := range strictRebuildsV27() { + // A table can legitimately be absent: this sweep converts the + // tables the chain has materialised, and it is not the place to + // diagnose a database whose history skipped one (a partial + // fixture, a hand-repaired index). There is nothing to convert + // and nothing to lose, so move on. + present, err := tableExists(ctx, tx, r.table) + if err != nil { + return err + } + if !present { + continue + } + if err := rebuildTableAsStrict(ctx, tx, r); err != nil { + return err + } + } + if _, err := tx.ExecContext(ctx, `INSERT INTO schema_version (version) VALUES (27)`); err != nil { + return fmt.Errorf("record schema v27: %w", err) + } + if err := verifyForeignKeysClean(ctx, tx, "v26→v27"); err != nil { + return err + } + return tx.Commit() +} + +// rebuildTableAsStrict performs one table's create-copy-drop-rename cycle +// and recreates the objects SQLite dropped with the original table. Each +// cycle leaves the schema whole before the next begins, so the rename — +// which makes SQLite reparse every trigger in the schema — never sees a +// half-converted database. +func rebuildTableAsStrict(ctx context.Context, tx *sql.Tx, r strictRebuild) error { + tmp := r.table + strictRebuildSuffix + before, err := countTableRows(ctx, tx, r.table) + if err != nil { + return err + } + if _, err := tx.ExecContext(ctx, fmt.Sprintf("CREATE TABLE %s %s STRICT", tmp, r.body)); err != nil { + return fmt.Errorf("create %s: %w", tmp, err) + } + cols, err := tableColumnNames(ctx, tx, tmp) + if err != nil { + return err + } + if _, err := tx.ExecContext(ctx, fmt.Sprintf("INSERT INTO %s (%s) SELECT %s FROM %s", tmp, cols, cols, r.table)); err != nil { + return fmt.Errorf("copy %s into its STRICT replacement (a legacy value whose storage class doesn't match its column type surfaces here; nothing is committed, and the pre-migration snapshot under backups/ is the rollback surface): %w", r.table, err) + } + after, err := countTableRows(ctx, tx, tmp) + if err != nil { + return err + } + if before != after { + return fmt.Errorf("copy %s: %d of %d rows carried over; refusing to drop the original", r.table, after, before) + } + stmts := append([]string{ + fmt.Sprintf("DROP TABLE %s", r.table), + fmt.Sprintf("ALTER TABLE %s RENAME TO %s", tmp, r.table), + }, r.objects...) + for _, q := range stmts { + if _, err := tx.ExecContext(ctx, q); err != nil { + return fmt.Errorf("rebuild %s: exec %q: %w", r.table, q, err) + } + } + return nil +} + +// tableExists reports whether the named table is present in the main +// schema. Indexes and triggers are excluded by the type filter. +func tableExists(ctx context.Context, tx *sql.Tx, table string) (bool, error) { + var n int + if err := tx.QueryRowContext(ctx, + `SELECT COUNT(*) FROM sqlite_master WHERE type = 'table' AND name = ?`, table).Scan(&n); err != nil { + return false, fmt.Errorf("look up table %s: %w", table, err) + } + return n > 0, nil +} + +// countTableRows counts a table inside the migration transaction. table comes +// from the spec list in this file, never from user input. +func countTableRows(ctx context.Context, tx *sql.Tx, table string) (int64, error) { + var n int64 + if err := tx.QueryRowContext(ctx, fmt.Sprintf("SELECT COUNT(*) FROM %s", table)).Scan(&n); err != nil { + return 0, fmt.Errorf("count %s: %w", table, err) + } + return n, nil +} + +// tableColumnNames returns the table's columns in declaration order, joined for +// use in an INSERT … SELECT. Reading them back from the freshly created +// STRICT table means the copy always names exactly the columns that exist, +// so a spec edit can't leave a column list behind to drift. +func tableColumnNames(ctx context.Context, tx *sql.Tx, table string) (string, error) { + rows, err := tx.QueryContext(ctx, `SELECT name FROM pragma_table_info(?) ORDER BY cid`, table) + if err != nil { + return "", fmt.Errorf("read columns of %s: %w", table, err) + } + defer rows.Close() + + var cols []string + for rows.Next() { + var name string + if err := rows.Scan(&name); err != nil { + return "", fmt.Errorf("scan column of %s: %w", table, err) + } + cols = append(cols, name) + } + if err := rows.Err(); err != nil { + return "", fmt.Errorf("iterate columns of %s: %w", table, err) + } + if len(cols) == 0 { + return "", fmt.Errorf("table %s has no columns", table) + } + return strings.Join(cols, ", "), nil +} + +// strictRebuildsV27 is the full conversion list, split into one function per +// subsystem — each is a DDL table rather than logic, so the split is for +// readability, not phases. The order between groups is free: every rebuild +// is self-contained, and the generated schema.sql snapshot is sorted by +// table name regardless. +// +// contested_paths and destination_alarms are absent because they were born +// STRICT (v26 and v25). +func strictRebuildsV27() []strictRebuild { + var rs []strictRebuild + rs = append(rs, strictRebuildsBookkeeping()...) + rs = append(rs, strictRebuildsVolumeTree()...) + rs = append(rs, strictRebuildsContentModel()...) + rs = append(rs, strictRebuildsRunHistory()...) + rs = append(rs, strictRebuildsDestinationState()...) + rs = append(rs, strictRebuildsUploadState()...) + rs = append(rs, strictRebuildsPeerState()...) + return rs +} + +// strictRebuildsBookkeeping covers the two tables that aren't part of any +// subsystem: the version marker and the node registry. schema_version is +// rebuilt like any other table — the INSERT recording v27 lands in the +// rebuilt copy, inside the same transaction. +func strictRebuildsBookkeeping() []strictRebuild { + return []strictRebuild{ + { + table: "schema_version", + body: `( + version INTEGER NOT NULL PRIMARY KEY +)`, + }, + { + table: "nodes", + body: `( + id INTEGER PRIMARY KEY, + name TEXT NOT NULL UNIQUE, + endpoint TEXT, + public_key_fingerprint TEXT +)`, + }, + } +} + +// strictRebuildsVolumeTree covers the addressing side of the index: the +// volumes and the folder tree inside them, including folders' self +// reference through parent_id (harmless here — the rebuild runs with +// foreign keys off). +func strictRebuildsVolumeTree() []strictRebuild { + return []strictRebuild{ + { + table: "volumes", + body: `( + id INTEGER PRIMARY KEY, + name TEXT NOT NULL UNIQUE, + path TEXT NOT NULL +)`, + }, + { + table: "folders", + body: `( + id INTEGER PRIMARY KEY, + volume_id INTEGER NOT NULL REFERENCES volumes(id), + parent_id INTEGER REFERENCES folders(id), + path TEXT NOT NULL, + shallow_blake3 BLOB CHECK (shallow_blake3 IS NULL OR length(shallow_blake3) = 32), + deep_blake3 BLOB CHECK (deep_blake3 IS NULL OR length(deep_blake3) = 32), + last_changed_run_id INTEGER REFERENCES runs(id), + file_count INTEGER NOT NULL DEFAULT 0, + cumulative_size INTEGER NOT NULL DEFAULT 0, + UNIQUE (volume_id, path) +)`, + objects: []string{ + `CREATE INDEX idx_folders_parent ON folders(parent_id)`, + }, + }, + } +} + +// strictRebuildsContentModel covers the core principle's two tables: the +// append-only contents entity and the files observations that bind a path +// to a content. contents keeps both append-only triggers and files keeps +// the partial unique index enforcing at most one live row per path — the +// schema-level guarantees AGENTS.md calls out, recreated verbatim after the +// rename because SQLite drops them with the old table. +func strictRebuildsContentModel() []strictRebuild { + return []strictRebuild{ + { + table: "contents", + body: `( + id INTEGER PRIMARY KEY, + blake3 BLOB NOT NULL UNIQUE CHECK (length(blake3) = 32), + size_bytes INTEGER NOT NULL, + origin_node_id INTEGER REFERENCES nodes(id), + origin_run_id INTEGER +)`, + objects: []string{ + `CREATE INDEX idx_contents_origin_node ON contents(origin_node_id) + WHERE origin_node_id IS NOT NULL`, + `CREATE TRIGGER contents_no_delete BEFORE DELETE ON contents + BEGIN + SELECT RAISE(ABORT, 'contents is append-only; a content row is never deleted'); + END`, + `CREATE TRIGGER contents_no_update BEFORE UPDATE ON contents + BEGIN + SELECT RAISE(ABORT, 'contents is append-only; supersede the files row and insert new content instead of updating'); + END`, + }, + }, + { + table: "files", + body: `( + folder_id INTEGER NOT NULL REFERENCES folders(id), + name TEXT NOT NULL, + content_id INTEGER NOT NULL REFERENCES contents(id), + mtime_ns INTEGER NOT NULL, + status TEXT NOT NULL CHECK (status IN ('present','missing','superseded','offloaded')), + first_seen_run_id INTEGER NOT NULL REFERENCES runs(id), + last_seen_run_id INTEGER NOT NULL REFERENCES runs(id), + indexed_at_ns INTEGER NOT NULL, + status_changed_run_id INTEGER REFERENCES runs(id), + PRIMARY KEY (folder_id, name, content_id) +)`, + objects: []string{ + `CREATE INDEX idx_files_content ON files(content_id)`, + `CREATE INDEX idx_files_missing ON files(folder_id, name) WHERE status = 'missing'`, + `CREATE UNIQUE INDEX uniq_files_live_per_path ON files(folder_id, name) WHERE status != 'superseded'`, + }, + }, + } +} + +// strictRebuildsRunHistory covers the audit trail: runs, the transition +// log hanging off it, and the hook invocations it triggers. runs never +// auto-prunes, so this is the largest history in the schema after files — +// its copy carries every id forward because six other tables reference it. +func strictRebuildsRunHistory() []strictRebuild { + return []strictRebuild{ + { + table: "runs", + body: `( + id INTEGER PRIMARY KEY, + kind TEXT NOT NULL CHECK (kind IN ('index','sync','restore','audit','offload')), + volume_id INTEGER REFERENCES volumes(id), + destination TEXT, + started_at_ns INTEGER NOT NULL, + ended_at_ns INTEGER, + status TEXT NOT NULL CHECK (status IN ('running','success','failed','partial','refused','aborted')), + error TEXT, + file_count INTEGER NOT NULL DEFAULT 0, + peer_node_id INTEGER REFERENCES nodes(id), + correlated_run_id INTEGER, + shallow INTEGER CHECK (shallow IS NULL OR shallow IN (0, 1)), + CHECK ( + (kind IN ('index','audit','offload') AND destination IS NULL) OR + (kind IN ('sync','restore') AND destination IS NOT NULL AND destination != '') + ) +)`, + objects: []string{ + `CREATE INDEX idx_runs_volume_started ON runs(volume_id, started_at_ns)`, + `CREATE INDEX idx_runs_destination ON runs(destination) WHERE destination IS NOT NULL`, + }, + }, + { + table: "runs_audit", + body: `( + id INTEGER PRIMARY KEY, + run_id INTEGER NOT NULL REFERENCES runs(id), + transition TEXT NOT NULL, + operator TEXT, + at_ns INTEGER NOT NULL, + note TEXT +)`, + objects: []string{ + `CREATE INDEX idx_runs_audit_run ON runs_audit(run_id)`, + }, + }, + { + table: "hook_runs", + body: `( + id INTEGER PRIMARY KEY, + volume_id INTEGER NOT NULL REFERENCES volumes(id), + trigger TEXT NOT NULL CHECK (trigger IN ('change','interval')), + triggering_run_id INTEGER REFERENCES runs(id), + changed INTEGER NOT NULL CHECK (changed IN (0, 1)), + started_at_ns INTEGER NOT NULL, + ended_at_ns INTEGER, + status TEXT NOT NULL CHECK (status IN ('running','success','failed')), + exit_code INTEGER, + error TEXT, + CHECK ( + (trigger = 'change' AND triggering_run_id IS NOT NULL) OR + (trigger = 'interval' AND triggering_run_id IS NULL) + ) +)`, + objects: []string{ + `CREATE INDEX idx_hook_runs_volume_trigger ON hook_runs(volume_id, trigger, started_at_ns)`, + }, + }, + } +} + +// strictRebuildsDestinationState covers the per-destination watermarks: the +// run-id each origin has landed at a destination, that watermark's history, +// and the push-freshness view. The columns bolted on by earlier ALTER TABLE +// steps (verify_method, source_node_id, verified_at_ns) keep their trailing +// positions, so the copy stays a positional no-op. +func strictRebuildsDestinationState() []strictRebuild { + return []strictRebuild{ + { + table: "destination_run_ids", + body: `( + volume_id INTEGER NOT NULL REFERENCES volumes(id), + destination TEXT NOT NULL, + origin_node_id INTEGER NOT NULL REFERENCES nodes(id), + origin_run_id INTEGER NOT NULL, + updated_at_ns INTEGER NOT NULL, + verify_method TEXT, + source_node_id INTEGER REFERENCES nodes(id), + verified_at_ns INTEGER, + PRIMARY KEY (volume_id, destination, origin_node_id) +)`, + }, + { + table: "destination_run_ids_history", + body: `( + id INTEGER PRIMARY KEY, + volume_id INTEGER NOT NULL, + destination TEXT NOT NULL, + origin_node_id INTEGER NOT NULL, + origin_run_id INTEGER NOT NULL, + at_ns INTEGER NOT NULL, + verify_method TEXT, + source_node_id INTEGER +)`, + objects: []string{ + `CREATE INDEX idx_destination_run_ids_history + ON destination_run_ids_history(volume_id, destination)`, + }, + }, + { + table: "destination_push_freshness", + body: `( + volume_id INTEGER NOT NULL REFERENCES volumes(id), + destination TEXT NOT NULL, + origin_node_id INTEGER NOT NULL REFERENCES nodes(id), + origin_run_id INTEGER NOT NULL, + updated_at_ns INTEGER NOT NULL, + PRIMARY KEY (volume_id, destination, origin_node_id) +)`, + }, + } +} + +// strictRebuildsUploadState covers what has been uploaded where: the +// per-content and per-pack upload fingerprints, and the pack entity with +// its members. remote_objects and remote_packs both carry the +// checksum_algo/checksum pairing CHECK, which the rebuild reproduces +// verbatim — STRICT constrains storage classes, not nullability pairings. +func strictRebuildsUploadState() []strictRebuild { + return []strictRebuild{ + { + table: "remote_objects", + body: `( + content_id INTEGER NOT NULL REFERENCES contents(id), + destination TEXT NOT NULL, + uploaded_run_id INTEGER NOT NULL REFERENCES runs(id), + checksum_algo TEXT, + checksum TEXT, + verified_at_ns INTEGER, + PRIMARY KEY (content_id, destination), + CHECK ((checksum_algo IS NULL) = (checksum IS NULL)) +)`, + }, + { + table: "packs", + body: `( + id INTEGER PRIMARY KEY, + pack_key BLOB NOT NULL UNIQUE CHECK (length(pack_key) = 32), + size_bytes INTEGER NOT NULL, + member_count INTEGER NOT NULL, + created_run_id INTEGER NOT NULL REFERENCES runs(id) +)`, + }, + { + table: "pack_members", + body: `( + content_id INTEGER NOT NULL REFERENCES contents(id), + pack_id INTEGER NOT NULL REFERENCES packs(id), + byte_offset INTEGER NOT NULL, + byte_length INTEGER NOT NULL, + PRIMARY KEY (content_id) +)`, + objects: []string{ + `CREATE INDEX idx_pack_members_pack ON pack_members(pack_id)`, + }, + }, + { + table: "remote_packs", + body: `( + pack_id INTEGER NOT NULL REFERENCES packs(id), + destination TEXT NOT NULL, + uploaded_run_id INTEGER NOT NULL REFERENCES runs(id), + checksum_algo TEXT, + checksum TEXT, + verified_at_ns INTEGER, + PRIMARY KEY (pack_id, destination), + CHECK ((checksum_algo IS NULL) = (checksum IS NULL)) +)`, + }, + } +} + +// strictRebuildsPeerState covers the peer-sync watermarks and their +// history. last_shared_run_id stays free of a FK on purpose: it carries the +// *initiator's* run id, which is meaningless in this node's runs table. +func strictRebuildsPeerState() []strictRebuild { + return []strictRebuild{ + { + table: "peer_sync_state", + body: `( + volume_id INTEGER NOT NULL REFERENCES volumes(id), + peer_node_id INTEGER NOT NULL REFERENCES nodes(id), + last_shared_run_id INTEGER, + last_synced_at INTEGER NOT NULL, + PRIMARY KEY (volume_id, peer_node_id) +)`, + }, + { + table: "peer_sync_state_history", + body: `( + id INTEGER PRIMARY KEY, + volume_id INTEGER NOT NULL REFERENCES volumes(id), + peer_node_id INTEGER NOT NULL REFERENCES nodes(id), + last_shared_run_id INTEGER, + last_synced_at INTEGER NOT NULL, + at_ns INTEGER NOT NULL +)`, + objects: []string{ + `CREATE INDEX idx_peer_sync_history_pair + ON peer_sync_state_history(volume_id, peer_node_id)`, + }, + }, + } +} diff --git a/store/migrate_v27_test.go b/store/migrate_v27_test.go new file mode 100644 index 0000000..04e7173 --- /dev/null +++ b/store/migrate_v27_test.go @@ -0,0 +1,453 @@ +package store + +import ( + "bytes" + "context" + "database/sql" + "path/filepath" + "sort" + "strings" + "testing" +) + +// TestSchemaIsAllStrict makes the AGENTS.md convention executable: every +// table in a database migrated to SchemaVersion must be STRICT. It covers +// the v26→v27 bulk conversion and, from here on, any new migration that +// forgets the keyword on a CREATE TABLE. +func TestSchemaIsAllStrict(t *testing.T) { + s := openTestStore(t) + ctx := context.Background() + + rows, err := s.db.QueryContext(ctx, ` + SELECT name, strict FROM pragma_table_list + WHERE schema = 'main' AND type = 'table' AND name NOT LIKE 'sqlite_%' + ORDER BY name`) + if err != nil { + t.Fatalf("pragma_table_list: %v", err) + } + defer rows.Close() + + var loose []string + tables := 0 + for rows.Next() { + var name string + var strict int + if err := rows.Scan(&name, &strict); err != nil { + t.Fatalf("scan table row: %v", err) + } + tables++ + if strict != 1 { + loose = append(loose, name) + } + } + if err := rows.Err(); err != nil { + t.Fatalf("iterate tables: %v", err) + } + if tables == 0 { + t.Fatal("no tables found in a migrated database") + } + if len(loose) > 0 { + t.Errorf("non-STRICT tables at v%d: %s — declare new tables STRICT (AGENTS.md, Schema & migrations)", + SchemaVersion, strings.Join(loose, ", ")) + } +} + +// TestStrictRebuildsCoverEveryV26Table checks the conversion list against +// the database it converts: the tables it rebuilds must be exactly the +// non-STRICT tables a v26 database carries. A table added before v27 and +// forgotten in the list would show up here as a missing entry, and a stale +// entry for a table that no longer exists as an extra one. +func TestStrictRebuildsCoverEveryV26Table(t *testing.T) { + dsn := filepath.Join(t.TempDir(), "test.db") + db := buildPopulatedV26DB(t, dsn) + defer db.Close() + + rows, err := db.QueryContext(context.Background(), ` + SELECT name FROM pragma_table_list + WHERE schema = 'main' AND type = 'table' AND strict = 0 AND name NOT LIKE 'sqlite_%' + ORDER BY name`) + if err != nil { + t.Fatalf("pragma_table_list: %v", err) + } + defer rows.Close() + var loose []string + for rows.Next() { + var name string + if err := rows.Scan(&name); err != nil { + t.Fatalf("scan table name: %v", err) + } + loose = append(loose, name) + } + if err := rows.Err(); err != nil { + t.Fatalf("iterate tables: %v", err) + } + + var specced []string + for _, r := range strictRebuildsV27() { + specced = append(specced, r.table) + } + sort.Strings(loose) + sort.Strings(specced) + if !equalStrings(specced, loose) { + t.Errorf("conversion list = %v,\nnon-STRICT tables at v26 = %v", specced, loose) + } +} + +// TestMigrateV26ToV27PreservesEveryRow drives the real migration chain to +// v26, seeds every table, then opens with the current binary so v26→v27 +// runs against a fully populated database. It pins what a whole-schema +// rebuild must not lose: every row of every table, the ids rows are keyed +// by, blob values byte-for-byte, and the full set of indexes and triggers. +func TestMigrateV26ToV27PreservesEveryRow(t *testing.T) { + dsn := filepath.Join(t.TempDir(), "test.db") + ctx := context.Background() + db := buildPopulatedV26DB(t, dsn) + countsBefore := tableRowCounts(t, db) + objectsBefore := schemaObjectNames(t, db) + if err := db.Close(); err != nil { + t.Fatalf("close v26 db: %v", err) + } + + s, err := OpenWithOptions(dsn, OpenOptions{DisablePreMigrationBackup: true}) + if err != nil { + t.Fatalf("Open (migrates v26→v27): %v", err) + } + defer s.Close() + if v, _ := s.CurrentSchemaVersion(ctx); v != SchemaVersion { + t.Fatalf("schema_version = %d, want %d", v, SchemaVersion) + } + + countsAfter := tableRowCounts(t, s.db) + for table, before := range countsBefore { + if before == 0 { + t.Errorf("fixture left %s empty — the rebuild of an empty table proves nothing", table) + } + want := before + if table == "schema_version" { + // The migration records v27 in the table it just rebuilt. + want++ + } + if after := countsAfter[table]; after != want { + t.Errorf("%s rows = %d after migration, want %d", table, after, want) + } + } + if got, want := len(countsAfter), len(countsBefore); got != want { + t.Errorf("table count = %d after migration, want %d", got, want) + } + if after := schemaObjectNames(t, s.db); !equalStrings(after, objectsBefore) { + t.Errorf("indexes/triggers after migration = %v, want %v", after, objectsBefore) + } + + assertContentRowIntact(t, s) + assertFilesRowsIntact(t, s) + + // Every FK still resolves: the rebuild ran with enforcement off, so + // this is the check that the id-preserving copies kept the graph whole. + fkRows, err := s.db.QueryContext(ctx, `PRAGMA foreign_key_check`) + if err != nil { + t.Fatalf("foreign_key_check: %v", err) + } + violations := 0 + for fkRows.Next() { + violations++ + } + fkRows.Close() + if violations != 0 { + t.Errorf("foreign_key_check reported %d violations after migration", violations) + } +} + +// assertContentRowIntact checks the content entity survived the rebuild +// byte-exactly: the id↔hash binding, the 32-byte blob's storage class, and +// the size. Losing any of these would break the core principle — a hash +// once observed stays retrievable by the id everything else references. +func assertContentRowIntact(t *testing.T, s *Store) { + t.Helper() + var hash []byte + var size int64 + var typ string + err := s.db.QueryRowContext(context.Background(), + `SELECT blake3, size_bytes, typeof(blake3) FROM contents WHERE id = 1`).Scan(&hash, &size, &typ) + if err != nil { + t.Fatalf("read contents id=1: %v", err) + } + if !bytes.Equal(hash, fixtureHash(0xa1)) { + t.Errorf("contents.blake3 = %x, want %x", hash, fixtureHash(0xa1)) + } + if size != 1000 { + t.Errorf("contents.size_bytes = %d, want 1000", size) + } + if typ != "blob" { + t.Errorf("typeof(contents.blake3) = %q, want blob", typ) + } +} + +// assertFilesRowsIntact checks the path↔content observations came across +// with their statuses: the live row and the superseded predecessor at the +// same path, plus the missing row in the child folder. +func assertFilesRowsIntact(t *testing.T, s *Store) { + t.Helper() + rows, err := s.db.QueryContext(context.Background(), + `SELECT folder_id, name, content_id, status FROM files ORDER BY folder_id, name, content_id`) + if err != nil { + t.Fatalf("read files: %v", err) + } + defer rows.Close() + + var got []string + for rows.Next() { + var folderID, contentID int64 + var name, status string + if err := rows.Scan(&folderID, &name, &contentID, &status); err != nil { + t.Fatalf("scan files row: %v", err) + } + got = append(got, strings.Join([]string{name, status}, "=")) + } + // Ordered by (folder_id, name, content_id): a.txt's superseded + // predecessor (content 1) precedes its live row (content 2). + want := []string{"a.txt=superseded", "a.txt=present", "gone.txt=missing"} + if !equalStrings(got, want) { + t.Errorf("files rows = %v, want %v", got, want) + } +} + +// TestMigrateV26ToV27KeepsSchemaGuarantees verifies the rebuilt tables +// still enforce what they enforced at v26 — the append-only triggers on +// contents and the one-live-row-per-path partial unique index on files — +// and that STRICT now rejects a value whose storage class doesn't match its +// column, which is the whole point of the conversion. +func TestMigrateV26ToV27KeepsSchemaGuarantees(t *testing.T) { + dsn := filepath.Join(t.TempDir(), "test.db") + ctx := context.Background() + db := buildPopulatedV26DB(t, dsn) + if err := db.Close(); err != nil { + t.Fatalf("close v26 db: %v", err) + } + s, err := OpenWithOptions(dsn, OpenOptions{DisablePreMigrationBackup: true}) + if err != nil { + t.Fatalf("Open (migrates v26→v27): %v", err) + } + defer s.Close() + + refusals := []struct { + what string + sql string + args []any + }{ + {"delete from contents", `DELETE FROM contents WHERE id = 1`, nil}, + {"update contents", `UPDATE contents SET size_bytes = 7 WHERE id = 1`, nil}, + { + "second live row at one path", + `INSERT INTO files (folder_id, name, content_id, mtime_ns, status, + first_seen_run_id, last_seen_run_id, indexed_at_ns) + VALUES (1, 'a.txt', 3, 999, 'present', 1, 1, 999)`, + nil, + }, + { + "TEXT bound into an INTEGER column", + `INSERT INTO files (folder_id, name, content_id, mtime_ns, status, + first_seen_run_id, last_seen_run_id, indexed_at_ns) + VALUES (1, 'strict.txt', 3, ?, 'present', 1, 1, 999)`, + []any{"not-a-timestamp"}, + }, + {"TEXT bound into contents.size_bytes", `INSERT INTO contents (blake3, size_bytes) VALUES (?, ?)`, + []any{fixtureHash(0xc3), "huge"}}, + } + for _, r := range refusals { + if _, err := s.db.ExecContext(ctx, r.sql, r.args...); err == nil { + t.Errorf("%s was accepted after migration; want refusal", r.what) + } + } + + // A well-typed insert still works — the conversion tightened types, it + // didn't wall the tables off. + if _, err := s.db.ExecContext(ctx, + `INSERT INTO contents (blake3, size_bytes) VALUES (?, ?)`, fixtureHash(0xd4), 4096); err != nil { + t.Errorf("well-typed insert rejected after migration: %v", err) + } +} + +// buildPopulatedV26DB creates a database at v26 through the real migration +// chain — not a hand-written fixture, so the shape is exactly what a v26 +// binary left behind — and seeds every table with at least one row. It +// returns the open handle; the caller closes it before reopening through +// Open to trigger the v26→v27 migration. +func buildPopulatedV26DB(t *testing.T, dsn string) *sql.DB { + t.Helper() + ctx := context.Background() + db, err := openSQLite(buildDSN(dsn)) + if err != nil { + t.Fatalf("openSQLite: %v", err) + } + // Non-STRICT, the way every pre-v27 binary bootstrapped it, so the + // migration's rebuild of schema_version itself is exercised. + if _, err := db.ExecContext(ctx, `CREATE TABLE schema_version (version INTEGER NOT NULL PRIMARY KEY)`); err != nil { + t.Fatalf("create schema_version: %v", err) + } + if err := applyV5(ctx, db); err != nil { + t.Fatalf("applyV5: %v", err) + } + var through []migration + for _, m := range buildMigrations(migrationCtx{nodeName: "v26-self"}) { + if m.version <= 26 { + through = append(through, m) + } + } + end, err := runMigrations(ctx, db, freshSchemaBaseline, through) + if err != nil { + t.Fatalf("migrate to v26: %v", err) + } + if end != 26 { + t.Fatalf("chain stopped at v%d, want v26", end) + } + for _, stmt := range v26Seed() { + if _, err := db.ExecContext(ctx, stmt.sql, stmt.args...); err != nil { + t.Fatalf("seed %q: %v", stmt.sql, err) + } + } + return db +} + +type seedStmt struct { + sql string + args []any +} + +// v26Seed populates every table a v26 database carries, in FK-safe order: +// two nodes (the self row is already seeded by v5→v6), a volume with a +// folder tree, two contents with three files observations across them, run +// history and audit rows, a pack and its member, remote upload +// fingerprints, the peer and destination watermarks with their history, and +// the two standing-state latches. Each table ends up non-empty so the +// row-count comparison across the migration is meaningful everywhere. +func v26Seed() []seedStmt { + hashA, hashB := fixtureHash(0xa1), fixtureHash(0xb2) + return []seedStmt{ + {sql: `INSERT INTO volumes (id, name, path) VALUES (1, 'photos', '/vol/photos')`}, + {sql: `INSERT INTO nodes (id, name, endpoint, public_key_fingerprint) VALUES (2, 'nas', 'nas:9000', 'fp:abc')`}, + {sql: `INSERT INTO runs (id, kind, volume_id, destination, started_at_ns, ended_at_ns, status, file_count, shallow) + VALUES (1, 'index', 1, NULL, 100, 200, 'success', 3, 0)`}, + {sql: `INSERT INTO runs (id, kind, volume_id, destination, started_at_ns, ended_at_ns, status, error, file_count, peer_node_id, correlated_run_id) + VALUES (2, 'sync', 1, 'nas', 300, 400, 'partial', 'one object failed', 2, 2, 7)`}, + {sql: `INSERT INTO folders (id, volume_id, parent_id, path, shallow_blake3, deep_blake3, last_changed_run_id, file_count, cumulative_size) + VALUES (1, 1, NULL, '', ?, ?, 1, 2, 3000)`, args: []any{hashA, hashB}}, + {sql: `INSERT INTO folders (id, volume_id, parent_id, path, last_changed_run_id, file_count, cumulative_size) + VALUES (2, 1, 1, 'sub', 1, 1, 0)`}, + {sql: `INSERT INTO contents (id, blake3, size_bytes, origin_node_id, origin_run_id) VALUES (1, ?, 1000, NULL, 1)`, args: []any{hashA}}, + {sql: `INSERT INTO contents (id, blake3, size_bytes, origin_node_id, origin_run_id) VALUES (2, ?, 2000, 2, 7)`, args: []any{hashB}}, + {sql: `INSERT INTO files (folder_id, name, content_id, mtime_ns, status, first_seen_run_id, last_seen_run_id, indexed_at_ns, status_changed_run_id) + VALUES (1, 'a.txt', 1, 111, 'superseded', 1, 1, 120, 1)`}, + {sql: `INSERT INTO files (folder_id, name, content_id, mtime_ns, status, first_seen_run_id, last_seen_run_id, indexed_at_ns, status_changed_run_id) + VALUES (1, 'a.txt', 2, 222, 'present', 1, 1, 230, NULL)`}, + {sql: `INSERT INTO files (folder_id, name, content_id, mtime_ns, status, first_seen_run_id, last_seen_run_id, indexed_at_ns, status_changed_run_id) + VALUES (2, 'gone.txt', 1, 333, 'missing', 1, 1, 340, 1)`}, + {sql: `INSERT INTO runs_audit (id, run_id, transition, operator, at_ns, note) VALUES (1, 2, 'running→partial', 'alice', 410, 'retried')`}, + {sql: `INSERT INTO hook_runs (id, volume_id, trigger, triggering_run_id, changed, started_at_ns, ended_at_ns, status, exit_code, error) + VALUES (1, 1, 'change', 1, 1, 250, 260, 'success', 0, NULL)`}, + {sql: `INSERT INTO hook_runs (id, volume_id, trigger, triggering_run_id, changed, started_at_ns, status) + VALUES (2, 1, 'interval', NULL, 0, 270, 'running')`}, + {sql: `INSERT INTO packs (id, pack_key, size_bytes, member_count, created_run_id) VALUES (1, ?, 512, 1, 2)`, args: []any{fixtureHash(0xe5)}}, + {sql: `INSERT INTO pack_members (content_id, pack_id, byte_offset, byte_length) VALUES (1, 1, 0, 1000)`}, + {sql: `INSERT INTO remote_objects (content_id, destination, uploaded_run_id, checksum_algo, checksum, verified_at_ns) + VALUES (2, 'nas', 2, 'sha256', 'deadbeef', 420)`}, + {sql: `INSERT INTO remote_packs (pack_id, destination, uploaded_run_id, checksum_algo, checksum, verified_at_ns) + VALUES (1, 'nas', 2, NULL, NULL, NULL)`}, + {sql: `INSERT INTO peer_sync_state (volume_id, peer_node_id, last_shared_run_id, last_synced_at) VALUES (1, 2, 7, 400)`}, + {sql: `INSERT INTO peer_sync_state_history (id, volume_id, peer_node_id, last_shared_run_id, last_synced_at, at_ns) + VALUES (1, 1, 2, 7, 400, 401)`}, + {sql: `INSERT INTO destination_run_ids (volume_id, destination, origin_node_id, origin_run_id, updated_at_ns, verify_method, source_node_id, verified_at_ns) + VALUES (1, 'nas', 1, 2, 400, 'checksum', 2, 405)`}, + {sql: `INSERT INTO destination_run_ids_history (id, volume_id, destination, origin_node_id, origin_run_id, at_ns, verify_method, source_node_id) + VALUES (1, 1, 'nas', 1, 2, 400, 'checksum', 2)`}, + {sql: `INSERT INTO destination_push_freshness (volume_id, destination, origin_node_id, origin_run_id, updated_at_ns) + VALUES (1, 'nas', 1, 2, 400)`}, + {sql: `INSERT INTO destination_alarms (destination, kind, detail, raised_run_id, raised_at_ns) + VALUES ('nas', 'verify-mismatch', 'checksum differs', 2, 430)`}, + {sql: `INSERT INTO contested_paths (volume_id, path, live_blake3, preserved_blake3, preserved_at_path, peer_node_id, raised_run_id, raised_at_ns) + VALUES (1, 'a.txt', ?, ?, '.squirrel-conflicts/run-2/a.txt', 2, 2, 440)`, args: []any{hashB, hashA}}, + } +} + +// fixtureHash returns a deterministic 32-byte hash-shaped blob, satisfying +// the length CHECK on every hash column. +func fixtureHash(fill byte) []byte { + return bytes.Repeat([]byte{fill}, 32) +} + +// tableRowCounts returns row counts per table for every table in the main +// schema, so a rebuild can be compared table-by-table rather than trusting +// a spot check. +func tableRowCounts(t *testing.T, db *sql.DB) map[string]int64 { + t.Helper() + ctx := context.Background() + rows, err := db.QueryContext(ctx, ` + SELECT name FROM sqlite_master + WHERE type = 'table' AND name NOT LIKE 'sqlite_%' ORDER BY name`) + if err != nil { + t.Fatalf("list tables: %v", err) + } + var tables []string + for rows.Next() { + var name string + if err := rows.Scan(&name); err != nil { + rows.Close() + t.Fatalf("scan table name: %v", err) + } + tables = append(tables, name) + } + rows.Close() + if err := rows.Err(); err != nil { + t.Fatalf("iterate table names: %v", err) + } + + counts := make(map[string]int64, len(tables)) + for _, table := range tables { + var n int64 + // table names come from sqlite_master, not from input. + if err := db.QueryRowContext(ctx, `SELECT COUNT(*) FROM "`+table+`"`).Scan(&n); err != nil { + t.Fatalf("count %s: %v", table, err) + } + counts[table] = n + } + return counts +} + +// schemaObjectNames returns the sorted names of every index and trigger the +// schema declares, excluding the ones SQLite auto-creates for PRIMARY KEY / +// UNIQUE constraints (those carry a NULL sql and follow their table). +func schemaObjectNames(t *testing.T, db *sql.DB) []string { + t.Helper() + rows, err := db.QueryContext(context.Background(), ` + SELECT name FROM sqlite_master + WHERE type IN ('index', 'trigger') AND sql IS NOT NULL + ORDER BY name`) + if err != nil { + t.Fatalf("list schema objects: %v", err) + } + defer rows.Close() + var names []string + for rows.Next() { + var name string + if err := rows.Scan(&name); err != nil { + t.Fatalf("scan object name: %v", err) + } + names = append(names, name) + } + if err := rows.Err(); err != nil { + t.Fatalf("iterate schema objects: %v", err) + } + sort.Strings(names) + return names +} + +func equalStrings(a, b []string) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + return false + } + } + return true +} diff --git a/store/migrations.go b/store/migrations.go index 1526724..81e7dc4 100644 --- a/store/migrations.go +++ b/store/migrations.go @@ -10,7 +10,7 @@ import ( ) // SchemaVersion is the schema version this binary writes and reads. -const SchemaVersion = 26 +const SchemaVersion = 27 // freshSchemaBaseline is the version applied to a brand-new database. The // chain in `migrations` continues from here. v1 is no longer reachable from @@ -38,8 +38,9 @@ type migration struct { // Convention for new migrations: every new CREATE TABLE MUST be declared // STRICT and use only INTEGER/TEXT/BLOB column types (no TIMESTAMP / // BOOLEAN / DATETIME / VARCHAR / REAL). See AGENTS.md "Schema & migrations". -// The existing tables predate this and are not STRICT yet — their bulk -// conversion is a separate migration PR, not something to do ad hoc here. +// Every table is STRICT as of v27 (migrate_v27.go converted the ones that +// predated the convention), so a rebuild that drops the keyword would be a +// regression, not a neutral rewrite. func buildMigrations(mctx migrationCtx) []migration { return []migration{ {version: 3, up: migrateV2ToV3}, @@ -68,6 +69,7 @@ func buildMigrations(mctx migrationCtx) []migration { {version: 24, up: migrateV23ToV24}, {version: 25, up: migrateV24ToV25}, {version: 26, up: migrateV25ToV26}, + {version: 27, up: migrateV26ToV27}, } } @@ -89,8 +91,12 @@ type migrationCtx struct { // would only roll back failed migrations, not buggy-but-successful // ones. Fresh databases (current == 0) skip the snapshot because // there's nothing to lose. +// +// The bootstrap CREATE is STRICT per the schema convention. On a database +// that predates v27 it is a no-op (IF NOT EXISTS doesn't compare +// definitions) and the v26→v27 rebuild converts the existing table instead. func (s *Store) migrate(ctx context.Context, nodeName string, opts OpenOptions) error { - if _, err := s.db.ExecContext(ctx, `CREATE TABLE IF NOT EXISTS schema_version (version INTEGER NOT NULL PRIMARY KEY)`); err != nil { + if _, err := s.db.ExecContext(ctx, `CREATE TABLE IF NOT EXISTS schema_version (version INTEGER NOT NULL PRIMARY KEY) STRICT`); err != nil { return fmt.Errorf("create schema_version: %w", err) } diff --git a/store/schema.sql b/store/schema.sql index 0713a6a..5897ae7 100644 --- a/store/schema.sql +++ b/store/schema.sql @@ -1,31 +1,31 @@ -- Generated by `go test ./store -update-schema` — DO NOT EDIT. -- --- Flattened snapshot of the squirrel index schema at version 26, for humans +-- Flattened snapshot of the squirrel index schema at version 27, for humans -- and agents who want the current shape without replaying the migration -- chain in migrations.go. It is NOT used to create or migrate databases — -- a fresh DB is built by applyV5 plus the migration registry. The golden -- test TestSchemaSnapshot fails if this file drifts from that chain. -CREATE TABLE contents ( - id INTEGER PRIMARY KEY, - blake3 BLOB NOT NULL UNIQUE CHECK (length(blake3) = 32), - size_bytes INTEGER NOT NULL, - origin_node_id INTEGER REFERENCES nodes(id), - origin_run_id INTEGER - ); +CREATE TABLE "contents" ( + id INTEGER PRIMARY KEY, + blake3 BLOB NOT NULL UNIQUE CHECK (length(blake3) = 32), + size_bytes INTEGER NOT NULL, + origin_node_id INTEGER REFERENCES nodes(id), + origin_run_id INTEGER +) STRICT; CREATE INDEX idx_contents_origin_node ON contents(origin_node_id) - WHERE origin_node_id IS NOT NULL; + WHERE origin_node_id IS NOT NULL; CREATE TRIGGER contents_no_delete BEFORE DELETE ON contents - BEGIN - SELECT RAISE(ABORT, 'contents is append-only; a content row is never deleted'); - END; + BEGIN + SELECT RAISE(ABORT, 'contents is append-only; a content row is never deleted'); + END; CREATE TRIGGER contents_no_update BEFORE UPDATE ON contents - BEGIN - SELECT RAISE(ABORT, 'contents is append-only; supersede the files row and insert new content instead of updating'); - END; + BEGIN + SELECT RAISE(ABORT, 'contents is append-only; supersede the files row and insert new content instead of updating'); + END; CREATE TABLE contested_paths ( volume_id INTEGER NOT NULL REFERENCES volumes(id), @@ -47,47 +47,53 @@ CREATE TABLE destination_alarms ( raised_at_ns INTEGER NOT NULL ) STRICT; -CREATE TABLE destination_push_freshness ( - volume_id INTEGER NOT NULL REFERENCES volumes(id), - destination TEXT NOT NULL, - origin_node_id INTEGER NOT NULL REFERENCES nodes(id), - origin_run_id INTEGER NOT NULL, - updated_at_ns INTEGER NOT NULL, - PRIMARY KEY (volume_id, destination, origin_node_id) - ); - -CREATE TABLE destination_run_ids ( - volume_id INTEGER NOT NULL REFERENCES volumes(id), - destination TEXT NOT NULL, - origin_node_id INTEGER NOT NULL REFERENCES nodes(id), - origin_run_id INTEGER NOT NULL, - updated_at_ns INTEGER NOT NULL, verify_method TEXT, source_node_id INTEGER REFERENCES nodes(id), verified_at_ns INTEGER, - PRIMARY KEY (volume_id, destination, origin_node_id) - ); - -CREATE TABLE destination_run_ids_history ( - id INTEGER PRIMARY KEY, - volume_id INTEGER NOT NULL, - destination TEXT NOT NULL, - origin_node_id INTEGER NOT NULL, - origin_run_id INTEGER NOT NULL, - at_ns INTEGER NOT NULL - , verify_method TEXT, source_node_id INTEGER); +CREATE TABLE "destination_push_freshness" ( + volume_id INTEGER NOT NULL REFERENCES volumes(id), + destination TEXT NOT NULL, + origin_node_id INTEGER NOT NULL REFERENCES nodes(id), + origin_run_id INTEGER NOT NULL, + updated_at_ns INTEGER NOT NULL, + PRIMARY KEY (volume_id, destination, origin_node_id) +) STRICT; + +CREATE TABLE "destination_run_ids" ( + volume_id INTEGER NOT NULL REFERENCES volumes(id), + destination TEXT NOT NULL, + origin_node_id INTEGER NOT NULL REFERENCES nodes(id), + origin_run_id INTEGER NOT NULL, + updated_at_ns INTEGER NOT NULL, + verify_method TEXT, + source_node_id INTEGER REFERENCES nodes(id), + verified_at_ns INTEGER, + PRIMARY KEY (volume_id, destination, origin_node_id) +) STRICT; + +CREATE TABLE "destination_run_ids_history" ( + id INTEGER PRIMARY KEY, + volume_id INTEGER NOT NULL, + destination TEXT NOT NULL, + origin_node_id INTEGER NOT NULL, + origin_run_id INTEGER NOT NULL, + at_ns INTEGER NOT NULL, + verify_method TEXT, + source_node_id INTEGER +) STRICT; CREATE INDEX idx_destination_run_ids_history - ON destination_run_ids_history(volume_id, destination); + ON destination_run_ids_history(volume_id, destination); CREATE TABLE "files" ( - folder_id INTEGER NOT NULL REFERENCES folders(id), - name TEXT NOT NULL, - content_id INTEGER NOT NULL REFERENCES contents(id), - mtime_ns INTEGER NOT NULL, - status TEXT NOT NULL CHECK (status IN ('present','missing','superseded','offloaded')), - first_seen_run_id INTEGER NOT NULL REFERENCES runs(id), - last_seen_run_id INTEGER NOT NULL REFERENCES runs(id), - indexed_at_ns INTEGER NOT NULL, status_changed_run_id INTEGER REFERENCES runs(id), - PRIMARY KEY (folder_id, name, content_id) - ); + folder_id INTEGER NOT NULL REFERENCES folders(id), + name TEXT NOT NULL, + content_id INTEGER NOT NULL REFERENCES contents(id), + mtime_ns INTEGER NOT NULL, + status TEXT NOT NULL CHECK (status IN ('present','missing','superseded','offloaded')), + first_seen_run_id INTEGER NOT NULL REFERENCES runs(id), + last_seen_run_id INTEGER NOT NULL REFERENCES runs(id), + indexed_at_ns INTEGER NOT NULL, + status_changed_run_id INTEGER REFERENCES runs(id), + PRIMARY KEY (folder_id, name, content_id) +) STRICT; CREATE INDEX idx_files_content ON files(content_id); @@ -95,144 +101,148 @@ CREATE INDEX idx_files_missing ON files(folder_id, name) WHERE status = 'missing CREATE UNIQUE INDEX uniq_files_live_per_path ON files(folder_id, name) WHERE status != 'superseded'; -CREATE TABLE folders ( - id INTEGER PRIMARY KEY, - volume_id INTEGER NOT NULL REFERENCES volumes(id), - parent_id INTEGER REFERENCES folders(id), - path TEXT NOT NULL, - shallow_blake3 BLOB CHECK (shallow_blake3 IS NULL OR length(shallow_blake3) = 32), - deep_blake3 BLOB CHECK (deep_blake3 IS NULL OR length(deep_blake3) = 32), - last_changed_run_id INTEGER REFERENCES runs(id), file_count INTEGER NOT NULL DEFAULT 0, cumulative_size INTEGER NOT NULL DEFAULT 0, - UNIQUE (volume_id, path) - ); +CREATE TABLE "folders" ( + id INTEGER PRIMARY KEY, + volume_id INTEGER NOT NULL REFERENCES volumes(id), + parent_id INTEGER REFERENCES folders(id), + path TEXT NOT NULL, + shallow_blake3 BLOB CHECK (shallow_blake3 IS NULL OR length(shallow_blake3) = 32), + deep_blake3 BLOB CHECK (deep_blake3 IS NULL OR length(deep_blake3) = 32), + last_changed_run_id INTEGER REFERENCES runs(id), + file_count INTEGER NOT NULL DEFAULT 0, + cumulative_size INTEGER NOT NULL DEFAULT 0, + UNIQUE (volume_id, path) +) STRICT; CREATE INDEX idx_folders_parent ON folders(parent_id); -CREATE TABLE hook_runs ( - id INTEGER PRIMARY KEY, - volume_id INTEGER NOT NULL REFERENCES volumes(id), - trigger TEXT NOT NULL CHECK (trigger IN ('change','interval')), - triggering_run_id INTEGER REFERENCES runs(id), - changed INTEGER NOT NULL CHECK (changed IN (0, 1)), - started_at_ns INTEGER NOT NULL, - ended_at_ns INTEGER, - status TEXT NOT NULL CHECK (status IN ('running','success','failed')), - exit_code INTEGER, - error TEXT, - CHECK ( - (trigger = 'change' AND triggering_run_id IS NOT NULL) OR - (trigger = 'interval' AND triggering_run_id IS NULL) - ) - ); +CREATE TABLE "hook_runs" ( + id INTEGER PRIMARY KEY, + volume_id INTEGER NOT NULL REFERENCES volumes(id), + trigger TEXT NOT NULL CHECK (trigger IN ('change','interval')), + triggering_run_id INTEGER REFERENCES runs(id), + changed INTEGER NOT NULL CHECK (changed IN (0, 1)), + started_at_ns INTEGER NOT NULL, + ended_at_ns INTEGER, + status TEXT NOT NULL CHECK (status IN ('running','success','failed')), + exit_code INTEGER, + error TEXT, + CHECK ( + (trigger = 'change' AND triggering_run_id IS NOT NULL) OR + (trigger = 'interval' AND triggering_run_id IS NULL) + ) +) STRICT; CREATE INDEX idx_hook_runs_volume_trigger ON hook_runs(volume_id, trigger, started_at_ns); -CREATE TABLE nodes ( - id INTEGER PRIMARY KEY, - name TEXT NOT NULL UNIQUE, - endpoint TEXT, - public_key_fingerprint TEXT - ); - -CREATE TABLE pack_members ( - content_id INTEGER NOT NULL REFERENCES contents(id), - pack_id INTEGER NOT NULL REFERENCES packs(id), - byte_offset INTEGER NOT NULL, - byte_length INTEGER NOT NULL, - PRIMARY KEY (content_id) - ); +CREATE TABLE "nodes" ( + id INTEGER PRIMARY KEY, + name TEXT NOT NULL UNIQUE, + endpoint TEXT, + public_key_fingerprint TEXT +) STRICT; + +CREATE TABLE "pack_members" ( + content_id INTEGER NOT NULL REFERENCES contents(id), + pack_id INTEGER NOT NULL REFERENCES packs(id), + byte_offset INTEGER NOT NULL, + byte_length INTEGER NOT NULL, + PRIMARY KEY (content_id) +) STRICT; CREATE INDEX idx_pack_members_pack ON pack_members(pack_id); -CREATE TABLE packs ( - id INTEGER PRIMARY KEY, - pack_key BLOB NOT NULL UNIQUE CHECK (length(pack_key) = 32), - size_bytes INTEGER NOT NULL, - member_count INTEGER NOT NULL, - created_run_id INTEGER NOT NULL REFERENCES runs(id) - ); - -CREATE TABLE peer_sync_state ( - volume_id INTEGER NOT NULL REFERENCES volumes(id), - peer_node_id INTEGER NOT NULL REFERENCES nodes(id), - last_shared_run_id INTEGER, - last_synced_at INTEGER NOT NULL, - PRIMARY KEY (volume_id, peer_node_id) - ); - -CREATE TABLE peer_sync_state_history ( - id INTEGER PRIMARY KEY, - volume_id INTEGER NOT NULL REFERENCES volumes(id), - peer_node_id INTEGER NOT NULL REFERENCES nodes(id), - last_shared_run_id INTEGER, - last_synced_at INTEGER NOT NULL, - at_ns INTEGER NOT NULL - ); +CREATE TABLE "packs" ( + id INTEGER PRIMARY KEY, + pack_key BLOB NOT NULL UNIQUE CHECK (length(pack_key) = 32), + size_bytes INTEGER NOT NULL, + member_count INTEGER NOT NULL, + created_run_id INTEGER NOT NULL REFERENCES runs(id) +) STRICT; + +CREATE TABLE "peer_sync_state" ( + volume_id INTEGER NOT NULL REFERENCES volumes(id), + peer_node_id INTEGER NOT NULL REFERENCES nodes(id), + last_shared_run_id INTEGER, + last_synced_at INTEGER NOT NULL, + PRIMARY KEY (volume_id, peer_node_id) +) STRICT; + +CREATE TABLE "peer_sync_state_history" ( + id INTEGER PRIMARY KEY, + volume_id INTEGER NOT NULL REFERENCES volumes(id), + peer_node_id INTEGER NOT NULL REFERENCES nodes(id), + last_shared_run_id INTEGER, + last_synced_at INTEGER NOT NULL, + at_ns INTEGER NOT NULL +) STRICT; CREATE INDEX idx_peer_sync_history_pair - ON peer_sync_state_history(volume_id, peer_node_id); + ON peer_sync_state_history(volume_id, peer_node_id); CREATE TABLE "remote_objects" ( - content_id INTEGER NOT NULL REFERENCES contents(id), - destination TEXT NOT NULL, - uploaded_run_id INTEGER NOT NULL REFERENCES runs(id), - checksum_algo TEXT, - checksum TEXT, - verified_at_ns INTEGER, - PRIMARY KEY (content_id, destination), - CHECK ((checksum_algo IS NULL) = (checksum IS NULL)) - ); - -CREATE TABLE remote_packs ( - pack_id INTEGER NOT NULL REFERENCES packs(id), - destination TEXT NOT NULL, - uploaded_run_id INTEGER NOT NULL REFERENCES runs(id), - checksum_algo TEXT, - checksum TEXT, - verified_at_ns INTEGER, - PRIMARY KEY (pack_id, destination), - CHECK ((checksum_algo IS NULL) = (checksum IS NULL)) - ); + content_id INTEGER NOT NULL REFERENCES contents(id), + destination TEXT NOT NULL, + uploaded_run_id INTEGER NOT NULL REFERENCES runs(id), + checksum_algo TEXT, + checksum TEXT, + verified_at_ns INTEGER, + PRIMARY KEY (content_id, destination), + CHECK ((checksum_algo IS NULL) = (checksum IS NULL)) +) STRICT; + +CREATE TABLE "remote_packs" ( + pack_id INTEGER NOT NULL REFERENCES packs(id), + destination TEXT NOT NULL, + uploaded_run_id INTEGER NOT NULL REFERENCES runs(id), + checksum_algo TEXT, + checksum TEXT, + verified_at_ns INTEGER, + PRIMARY KEY (pack_id, destination), + CHECK ((checksum_algo IS NULL) = (checksum IS NULL)) +) STRICT; CREATE TABLE "runs" ( - id INTEGER PRIMARY KEY, - kind TEXT NOT NULL CHECK (kind IN ('index','sync','restore','audit','offload')), - volume_id INTEGER REFERENCES volumes(id), - destination TEXT, - started_at_ns INTEGER NOT NULL, - ended_at_ns INTEGER, - status TEXT NOT NULL CHECK (status IN ('running','success','failed','partial','refused','aborted')), - error TEXT, - file_count INTEGER NOT NULL DEFAULT 0, - peer_node_id INTEGER REFERENCES nodes(id), - correlated_run_id INTEGER, - shallow INTEGER CHECK (shallow IS NULL OR shallow IN (0, 1)), - CHECK ( - (kind IN ('index','audit','offload') AND destination IS NULL) OR - (kind IN ('sync','restore') AND destination IS NOT NULL AND destination != '') - ) - ); + id INTEGER PRIMARY KEY, + kind TEXT NOT NULL CHECK (kind IN ('index','sync','restore','audit','offload')), + volume_id INTEGER REFERENCES volumes(id), + destination TEXT, + started_at_ns INTEGER NOT NULL, + ended_at_ns INTEGER, + status TEXT NOT NULL CHECK (status IN ('running','success','failed','partial','refused','aborted')), + error TEXT, + file_count INTEGER NOT NULL DEFAULT 0, + peer_node_id INTEGER REFERENCES nodes(id), + correlated_run_id INTEGER, + shallow INTEGER CHECK (shallow IS NULL OR shallow IN (0, 1)), + CHECK ( + (kind IN ('index','audit','offload') AND destination IS NULL) OR + (kind IN ('sync','restore') AND destination IS NOT NULL AND destination != '') + ) +) STRICT; CREATE INDEX idx_runs_destination ON runs(destination) WHERE destination IS NOT NULL; CREATE INDEX idx_runs_volume_started ON runs(volume_id, started_at_ns); -CREATE TABLE runs_audit ( - id INTEGER PRIMARY KEY, - run_id INTEGER NOT NULL REFERENCES runs(id), - transition TEXT NOT NULL, - operator TEXT, - at_ns INTEGER NOT NULL, - note TEXT - ); +CREATE TABLE "runs_audit" ( + id INTEGER PRIMARY KEY, + run_id INTEGER NOT NULL REFERENCES runs(id), + transition TEXT NOT NULL, + operator TEXT, + at_ns INTEGER NOT NULL, + note TEXT +) STRICT; CREATE INDEX idx_runs_audit_run ON runs_audit(run_id); -CREATE TABLE schema_version (version INTEGER NOT NULL PRIMARY KEY); +CREATE TABLE "schema_version" ( + version INTEGER NOT NULL PRIMARY KEY +) STRICT; -CREATE TABLE volumes ( - id INTEGER PRIMARY KEY, - name TEXT NOT NULL UNIQUE, - path TEXT NOT NULL - ); +CREATE TABLE "volumes" ( + id INTEGER PRIMARY KEY, + name TEXT NOT NULL UNIQUE, + path TEXT NOT NULL +) STRICT; From 07449c72c880c19ddeaf2842c388547ba7d37fdc Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 25 Jul 2026 08:53:02 +0000 Subject: [PATCH 2/2] store: tighten the v27 migration test assertions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two review points from the PR feed, both about the new test file: - assertFilesRowsIntact scanned folder_id and content_id and then compared only name/status, so a swapped content_id would have passed. The binding of a path to a *particular* content is the observation, so compare the full (folder_id, name, content_id, status) tuple. - The foreign_key_check loop counted rows without checking rows.Err() — a swallowed iteration error would have read as "clean graph". Extracted countFKViolations, which defers Close and fails on an iteration error. Also added the missing rows.Err() check to the files read. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01HompuyJMt84zb3KGZENkTF --- store/migrate_v27_test.go | 42 ++++++++++++++++++++++++++++++--------- 1 file changed, 33 insertions(+), 9 deletions(-) diff --git a/store/migrate_v27_test.go b/store/migrate_v27_test.go index 04e7173..856415a 100644 --- a/store/migrate_v27_test.go +++ b/store/migrate_v27_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "database/sql" + "fmt" "path/filepath" "sort" "strings" @@ -143,18 +144,30 @@ func TestMigrateV26ToV27PreservesEveryRow(t *testing.T) { // Every FK still resolves: the rebuild ran with enforcement off, so // this is the check that the id-preserving copies kept the graph whole. - fkRows, err := s.db.QueryContext(ctx, `PRAGMA foreign_key_check`) + if violations := countFKViolations(t, s.db); violations != 0 { + t.Errorf("foreign_key_check reported %d violations after migration", violations) + } +} + +// countFKViolations returns how many rows PRAGMA foreign_key_check reports. +// Zero rows is a clean graph; the row contents don't matter here, only that +// iteration itself succeeded — a swallowed iteration error would read as +// "clean". +func countFKViolations(t *testing.T, db *sql.DB) int { + t.Helper() + rows, err := db.QueryContext(context.Background(), `PRAGMA foreign_key_check`) if err != nil { t.Fatalf("foreign_key_check: %v", err) } + defer rows.Close() violations := 0 - for fkRows.Next() { + for rows.Next() { violations++ } - fkRows.Close() - if violations != 0 { - t.Errorf("foreign_key_check reported %d violations after migration", violations) + if err := rows.Err(); err != nil { + t.Fatalf("iterate foreign_key_check rows: %v", err) } + return violations } // assertContentRowIntact checks the content entity survived the rebuild @@ -183,8 +196,12 @@ func assertContentRowIntact(t *testing.T, s *Store) { } // assertFilesRowsIntact checks the path↔content observations came across -// with their statuses: the live row and the superseded predecessor at the -// same path, plus the missing row in the child folder. +// whole: the live row and the superseded predecessor at the same path in +// the root folder, plus the missing row in the child folder. The full +// (folder_id, name, content_id, status) tuple is compared — the binding of +// a path to a *particular* content is the observation, so a swapped +// content_id would be exactly the kind of loss this migration must not +// cause. func assertFilesRowsIntact(t *testing.T, s *Store) { t.Helper() rows, err := s.db.QueryContext(context.Background(), @@ -201,11 +218,18 @@ func assertFilesRowsIntact(t *testing.T, s *Store) { if err := rows.Scan(&folderID, &name, &contentID, &status); err != nil { t.Fatalf("scan files row: %v", err) } - got = append(got, strings.Join([]string{name, status}, "=")) + got = append(got, fmt.Sprintf("folder=%d name=%s content=%d status=%s", folderID, name, contentID, status)) + } + if err := rows.Err(); err != nil { + t.Fatalf("iterate files rows: %v", err) } // Ordered by (folder_id, name, content_id): a.txt's superseded // predecessor (content 1) precedes its live row (content 2). - want := []string{"a.txt=superseded", "a.txt=present", "gone.txt=missing"} + want := []string{ + "folder=1 name=a.txt content=1 status=superseded", + "folder=1 name=a.txt content=2 status=present", + "folder=2 name=gone.txt content=1 status=missing", + } if !equalStrings(got, want) { t.Errorf("files rows = %v, want %v", got, want) }