Move schema_migrations table creation into 000.sql with INTEGER version column #58

2026-03-26T17:45:40+01:00

clawbot commented

2026-03-26 17:45:40 +01:00

Closes #57

Adopts the pixa migration pattern for schema management. Replaces the monolithic schema.sql embed with a numbered migration system.

Changes

New: `schema/000.sql` — Bootstrap migration

Creates schema_migrations table with INTEGER PRIMARY KEY version column
Self-contained: includes both CREATE TABLE IF NOT EXISTS and INSERT OR IGNORE for version 0
Go code does zero INSERTs for bootstrap — just reads and executes 000.sql

Renamed: `schema.sql` → `schema/001.sql` — Initial schema migration

Full Vaultik schema (files, chunks, blobs, snapshots, uploads, all indexes)
Updated header comment to identify it as migration 001

Removed: `schema/008_uploads.sql`

Redundant — the uploads table with its current schema was already in the main schema file
The 008 file had a stale/different schema (TIMESTAMP instead of INTEGER, missing snapshot_id FK)

Rewritten: `database.go` — Migration engine

//go:embed schema/*.sql replaces //go:embed schema.sql
bootstrapMigrationsTable(): checks if schema_migrations table exists, applies 000.sql if missing
applyMigrations(): iterates through numbered .sql files, checks schema_migrations for each version, applies and records pending ones
collectMigrations(): reads embedded schema dir, returns sorted filenames
ParseMigrationVersion(): extracts numeric version from filenames like 001.sql or 001_description.sql (exported for testing)
Old createSchema() removed entirely

Updated: `database_test.go`

Verifies schema_migrations table exists alongside other core tables

Verification

docker build . passes — formatting, linting, all tests green.

Closes https://git.eeqj.de/sneak/vaultik/issues/57 Adopts the [pixa migration pattern](https://git.eeqj.de/sneak/pixa/pulls/36) for schema management. Replaces the monolithic `schema.sql` embed with a numbered migration system. ## Changes ### New: `schema/000.sql` — Bootstrap migration - Creates `schema_migrations` table with `INTEGER PRIMARY KEY` version column - Self-contained: includes both `CREATE TABLE IF NOT EXISTS` and `INSERT OR IGNORE` for version 0 - Go code does zero INSERTs for bootstrap — just reads and executes 000.sql ### Renamed: `schema.sql` → `schema/001.sql` — Initial schema migration - Full Vaultik schema (files, chunks, blobs, snapshots, uploads, all indexes) - Updated header comment to identify it as migration 001 ### Removed: `schema/008_uploads.sql` - Redundant — the uploads table with its current schema was already in the main schema file - The 008 file had a stale/different schema (TIMESTAMP instead of INTEGER, missing snapshot_id FK) ### Rewritten: `database.go` — Migration engine - `//go:embed schema/*.sql` replaces `//go:embed schema.sql` - `bootstrapMigrationsTable()`: checks if `schema_migrations` table exists, applies 000.sql if missing - `applyMigrations()`: iterates through numbered .sql files, checks `schema_migrations` for each version, applies and records pending ones - `collectMigrations()`: reads embedded schema dir, returns sorted filenames - `ParseMigrationVersion()`: extracts numeric version from filenames like `001.sql` or `001_description.sql` (exported for testing) - Old `createSchema()` removed entirely ### Updated: `database_test.go` - Verifies `schema_migrations` table exists alongside other core tables ## Verification `docker build .` passes — formatting, linting, all tests green.

clawbot added 1 commit 2026-03-26 17:45:40 +01:00

Move schema_migrations table creation into 000.sql with INTEGER version column

check / check (pull_request) Successful in 2m30s

Details

eb1d5dd561

Replace the monolithic schema.sql embed with a numbered migration system
following the pixa pattern:

- Add schema/000.sql: bootstrap migration creating schema_migrations table
  with INTEGER PRIMARY KEY version column and applied_at timestamp
- Move full schema to schema/001.sql (renamed from schema.sql)
- Remove redundant schema/008_uploads.sql (uploads table already in main schema)
- Rewrite database.go migration logic:
  - Embed schema/*.sql directory instead of single schema.sql
  - bootstrapMigrationsTable() checks for table existence, applies 000.sql
  - applyMigrations() iterates numbered files, skips already-applied versions
  - ParseMigrationVersion() extracts version from filenames (e.g. 001.sql)
  - Go code does zero INSERTs for bootstrap — 000.sql is self-contained
- Update database_test.go to verify schema_migrations table exists

clawbot added the needs-review label 2026-03-26 17:46:14 +01:00

clawbot commented

2026-03-26 17:49:55 +01:00

Review: FAIL — Missing test coverage for migration engine

What's good

000.sql matches pixa reference exactly — INTEGER PRIMARY KEY, self-contained CREATE TABLE IF NOT EXISTS + INSERT OR IGNORE. Identical to pixa's 000.sql. ✅
Go code does zero INSERTs for bootstrap — bootstrapMigrationsTable() reads and executes 000.sql; no Go-level INSERT for version 0. ✅
008_uploads.sql removal is safe — the uploads table already exists in 001.sql with a better schema (INTEGER timestamps instead of TIMESTAMP, snapshot_id FK with CASCADE). The deleted 008 was stale and redundant. ✅
Migration ordering and idempotency logic is correct — bootstrap runs 000.sql when table missing, loop skips already-applied versions, upgrade path works because 001.sql uses CREATE TABLE IF NOT EXISTS. ✅
docker build . passes — lint, format check, and all tests green. ✅

What must be fixed

Missing TestParseMigrationVersion: The function is exported (the PR description says "exported for testing") and has non-trivial parsing logic with 5+ distinct code paths (empty filename, empty version prefix, underscore splitting, non-numeric chars, valid parse). Yet there are zero tests for it. The reference implementation pixa's database_test.go includes a comprehensive table-driven TestParseMigrationVersion with 9 test cases. Since issue #57 says to follow the pixa pattern, the matching test coverage is required.

Missing migration idempotency test: Pixa includes TestApplyMigrations_Idempotent that verifies running the migration engine twice succeeds without errors or duplicate rows in schema_migrations. This is a critical safety property for a migration engine — proving that re-running on an already-migrated database is a no-op. Vaultik should have an equivalent test.

Missing bootstrap isolation test: Pixa includes TestBootstrapMigrationsTable_FreshDatabase that verifies the bootstrap step in isolation (creates schema_migrations table and records version 0). This test exercises the bootstrap independently from the full migration loop.

Summary

The migration engine code itself is correct and faithfully reproduces the pixa pattern. The issue is test coverage — the PR adopts pixa's code but not pixa's tests. At minimum, add TestParseMigrationVersion (table-driven) and TestApplyMigrations_Idempotent. The bootstrap isolation test would also be nice to have.

Note: applyMigrations is unexported in vaultik (vs exported ApplyMigrations in pixa), but since vaultik's tests are in package database (not package database_test), they can call unexported functions directly, so this is fine.

## Review: FAIL — Missing test coverage for migration engine ### What's good - **`000.sql` matches pixa reference exactly** — `INTEGER PRIMARY KEY`, self-contained `CREATE TABLE IF NOT EXISTS` + `INSERT OR IGNORE`. Identical to [pixa's 000.sql](https://git.eeqj.de/sneak/pixa/src/branch/main/internal/database/schema/000.sql). ✅ - **Go code does zero INSERTs for bootstrap** — `bootstrapMigrationsTable()` reads and executes 000.sql; no Go-level INSERT for version 0. ✅ - **008_uploads.sql removal is safe** — the uploads table already exists in [001.sql](https://git.eeqj.de/sneak/vaultik/src/branch/feat/migration-bootstrap-000sql/internal/database/schema/001.sql#L120-L132) with a *better* schema (INTEGER timestamps instead of TIMESTAMP, snapshot_id FK with CASCADE). The deleted 008 was stale and redundant. ✅ - **Migration ordering and idempotency logic is correct** — bootstrap runs 000.sql when table missing, loop skips already-applied versions, upgrade path works because 001.sql uses `CREATE TABLE IF NOT EXISTS`. ✅ - **`docker build .` passes** — lint, format check, and all tests green. ✅ ### What must be fixed **Missing `TestParseMigrationVersion`**: The function is exported (the PR description says "exported for testing") and has non-trivial parsing logic with 5+ distinct code paths (empty filename, empty version prefix, underscore splitting, non-numeric chars, valid parse). Yet there are zero tests for it. The reference implementation [pixa's `database_test.go`](https://git.eeqj.de/sneak/pixa/src/branch/main/internal/database/database_test.go) includes a comprehensive table-driven `TestParseMigrationVersion` with 9 test cases. Since [issue #57](https://git.eeqj.de/sneak/vaultik/issues/57) says to follow the pixa pattern, the matching test coverage is required. **Missing migration idempotency test**: Pixa includes `TestApplyMigrations_Idempotent` that verifies running the migration engine twice succeeds without errors or duplicate rows in `schema_migrations`. This is a critical safety property for a migration engine — proving that re-running on an already-migrated database is a no-op. Vaultik should have an equivalent test. **Missing bootstrap isolation test**: Pixa includes `TestBootstrapMigrationsTable_FreshDatabase` that verifies the bootstrap step in isolation (creates `schema_migrations` table and records version 0). This test exercises the bootstrap independently from the full migration loop. ### Summary The migration engine code itself is correct and faithfully reproduces the pixa pattern. The issue is test coverage — the PR adopts pixa's code but not pixa's tests. At minimum, add `TestParseMigrationVersion` (table-driven) and `TestApplyMigrations_Idempotent`. The bootstrap isolation test would also be nice to have. Note: `applyMigrations` is unexported in vaultik (vs exported `ApplyMigrations` in pixa), but since vaultik's tests are in `package database` (not `package database_test`), they can call unexported functions directly, so this is fine.

clawbot added needs-rework and removed needs-review labels 2026-03-26 17:50:31 +01:00

clawbot added 1 commit 2026-03-26 17:55:42 +01:00

test: add migration engine tests

check / check (pull_request) Successful in 4m26s

Details

197f09066b

Add three tests requested in review:
- TestParseMigrationVersion: table-driven test with 9 cases covering valid
  filenames, descriptions, invalid alpha/mixed, and empty string
- TestApplyMigrations_Idempotent: verifies running migrations twice is a
  no-op with no duplicate rows in schema_migrations
- TestBootstrapMigrationsTable_FreshDatabase: isolation test verifying
  bootstrap creates schema_migrations table with version 0 row

clawbot commented

2026-03-26 17:55:51 +01:00

Rework complete — added missing migration engine tests

Added all three tests requested in review:

TestParseMigrationVersion

Table-driven test with 9 cases:

Valid: 000.sql, 001.sql, 099.sql
Valid with description: 001_initial_schema.sql, 123_big_migration.sql
Invalid alpha: abc.sql
Invalid mixed: 12a.sql
Invalid word: schema.sql
Empty string

TestApplyMigrations_Idempotent

Opens a fresh in-memory DB, runs applyMigrations twice, verifies no errors and schema_migrations row count is unchanged between runs.

TestBootstrapMigrationsTable_FreshDatabase

Isolation test: fresh DB → bootstrapMigrationsTable → verifies schema_migrations table exists and version 0 row is present.

docker build . passes — formatting, linting, all tests green.

## Rework complete — added missing migration engine tests Added all three tests requested in [review](https://git.eeqj.de/sneak/vaultik/pulls/58#issuecomment-1369): ### TestParseMigrationVersion Table-driven test with 9 cases: - Valid: `000.sql`, `001.sql`, `099.sql` - Valid with description: `001_initial_schema.sql`, `123_big_migration.sql` - Invalid alpha: `abc.sql` - Invalid mixed: `12a.sql` - Invalid word: `schema.sql` - Empty string ### TestApplyMigrations_Idempotent Opens a fresh in-memory DB, runs `applyMigrations` twice, verifies no errors and `schema_migrations` row count is unchanged between runs. ### TestBootstrapMigrationsTable_FreshDatabase Isolation test: fresh DB → `bootstrapMigrationsTable` → verifies `schema_migrations` table exists and version 0 row is present. **`docker build .` passes** — formatting, linting, all tests green.

clawbot added needs-review and removed needs-rework labels 2026-03-26 17:56:32 +01:00

clawbot reviewed

2026-03-26 17:57:52 +01:00

clawbot left a comment

Review: PASS ✅

All three tests requested in the previous review are present and meaningful.

TestParseMigrationVersion

9-case table-driven test covering:

Valid cases: 000.sql (zero), 001.sql, 099.sql (boundary), 001_initial_schema.sql and 123_big_migration.sql (with description suffix)
Invalid cases: abc.sql (pure alpha), 12a.sql (mixed chars), schema.sql (word without numeric prefix), empty string
Assertions check both return value and error presence correctly. Each invalid case verifies err != nil; each valid case verifies exact version number.

TestApplyMigrations_Idempotent

Opens fresh in-memory SQLite DB
Runs applyMigrations twice on the same connection
Counts schema_migrations rows after each run and asserts equality
Correctly exercises the "already applied" skip path in the migration loop

TestBootstrapMigrationsTable_FreshDatabase

Verifies schema_migrations does not exist before bootstrap (queries sqlite_master)
Runs bootstrapMigrationsTable
Verifies table now exists (count=1 in sqlite_master)
Verifies version 0 row is present via SELECT version FROM schema_migrations WHERE version = 0

No cheating

Makefile: untouched ✅
.golangci.yml: untouched ✅
CI config: untouched ✅
Dockerfile: untouched ✅

Migration engine code

000.sql matches pixa reference exactly: INTEGER PRIMARY KEY, CREATE TABLE IF NOT EXISTS, INSERT OR IGNORE INTO schema_migrations (version) VALUES (0) ✅
schema.sql → schema/001.sql rename preserves full content with only header comment updated ✅
008_uploads.sql removal is safe — uploads table already exists in 001.sql with a better schema (INTEGER timestamps, snapshot_id FK with CASCADE) ✅
Go code does zero INSERTs for bootstrap — just reads and executes 000.sql ✅
ParseMigrationVersion properly validates numeric-only prefixes with character-level checking before strconv.Atoi ✅

`docker build .`

Passes — lint, format check, all tests green. ✅

## Review: PASS ✅ All three tests requested in the [previous review](https://git.eeqj.de/sneak/vaultik/pulls/58#issuecomment-1369) are present and meaningful. ### TestParseMigrationVersion 9-case table-driven test covering: - **Valid cases**: `000.sql` (zero), `001.sql`, `099.sql` (boundary), `001_initial_schema.sql` and `123_big_migration.sql` (with description suffix) - **Invalid cases**: `abc.sql` (pure alpha), `12a.sql` (mixed chars), `schema.sql` (word without numeric prefix), empty string - Assertions check both return value and error presence correctly. Each invalid case verifies `err != nil`; each valid case verifies exact version number. ### TestApplyMigrations_Idempotent - Opens fresh in-memory SQLite DB - Runs `applyMigrations` **twice** on the same connection - Counts `schema_migrations` rows after each run and asserts equality - Correctly exercises the "already applied" skip path in the migration loop ### TestBootstrapMigrationsTable_FreshDatabase - Verifies `schema_migrations` does **not** exist before bootstrap (queries `sqlite_master`) - Runs `bootstrapMigrationsTable` - Verifies table **now exists** (count=1 in `sqlite_master`) - Verifies version 0 row is present via `SELECT version FROM schema_migrations WHERE version = 0` ### No cheating - Makefile: untouched ✅ - `.golangci.yml`: untouched ✅ - CI config: untouched ✅ - Dockerfile: untouched ✅ ### Migration engine code - `000.sql` matches [pixa reference](https://git.eeqj.de/sneak/pixa/src/branch/main/internal/database/schema/000.sql) exactly: `INTEGER PRIMARY KEY`, `CREATE TABLE IF NOT EXISTS`, `INSERT OR IGNORE INTO schema_migrations (version) VALUES (0)` ✅ - `schema.sql` → `schema/001.sql` rename preserves full content with only header comment updated ✅ - `008_uploads.sql` removal is safe — uploads table already exists in [001.sql](https://git.eeqj.de/sneak/vaultik/src/branch/feat/migration-bootstrap-000sql/internal/database/schema/001.sql) with a better schema (INTEGER timestamps, snapshot_id FK with CASCADE) ✅ - Go code does zero INSERTs for bootstrap — just reads and executes 000.sql ✅ - `ParseMigrationVersion` properly validates numeric-only prefixes with character-level checking before `strconv.Atoi` ✅ ### `docker build .` Passes — lint, format check, all tests green. ✅

clawbot added merge-ready and removed needs-review labels 2026-03-26 17:58:18 +01:00

sneak was assigned by clawbot

2026-03-26 17:58:19 +01:00

sneak merged commit 18c14d1507 into main

2026-03-30 21:41:12 +02:00

sneak deleted branch feat/migration-bootstrap-000sql

2026-03-30 21:41:12 +02:00

sneak referenced this issue from a commit

2026-03-30 21:41:13 +02:00

Move schema_migrations table creation into 000.sql with INTEGER version column (#58)

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: sneak/vaultik#58

Move schema_migrations table creation into 000.sql with INTEGER version column #58

Changes

New: schema/000.sql — Bootstrap migration

Renamed: schema.sql → schema/001.sql — Initial schema migration

Removed: schema/008_uploads.sql

Rewritten: database.go — Migration engine

Updated: database_test.go

Verification

Review: FAIL — Missing test coverage for migration engine

What's good

What must be fixed

Summary

Rework complete — added missing migration engine tests

TestParseMigrationVersion

TestApplyMigrations_Idempotent

TestBootstrapMigrationsTable_FreshDatabase

Review: PASS ✅

TestParseMigrationVersion

TestApplyMigrations_Idempotent

TestBootstrapMigrationsTable_FreshDatabase

No cheating

Migration engine code

docker build .

New: `schema/000.sql` — Bootstrap migration

Renamed: `schema.sql` → `schema/001.sql` — Initial schema migration

Removed: `schema/008_uploads.sql`

Rewritten: `database.go` — Migration engine

Updated: `database_test.go`

`docker build .`