Moves the schema_migrations table definition from inline Go code into internal/database/schema/000.sql, so the migration tracking table schema lives alongside all other schema files.
Contains the CREATE TABLE IF NOT EXISTS schema_migrations DDL
This is applied as a bootstrap step before the normal migration loop
Refactored: internal/database/database.go
Removed the inline CREATE TABLE IF NOT EXISTS schema_migrations SQL from both runMigrations and ApplyMigrations
Added bootstrapMigrationsTable() which:
Checks sqlite_master to see if the table already exists
If missing: reads and executes 000.sql to create it, then records version 000
If present (backwards compat with existing DBs created by old inline code): back-fills version 000 so the normal loop skips the bootstrap file
Deduplicated: both Database.runMigrations() and the exported ApplyMigrations() now delegate to a single applyMigrations() helper
Added logInfo/logDebug helpers to handle the optional logger (nil when called from ApplyMigrations in tests)
New file: internal/database/database_test.go
TestApplyMigrations_CreatesSchemaAndTables — verifies all migrations apply and all expected tables exist
TestApplyMigrations_Idempotent — verifies running migrations twice produces no errors or duplicates
TestBootstrapMigrationsTable_FreshDatabase — verifies bootstrap creates the table and records version 000
TestBootstrapMigrationsTable_ExistingTableBackwardsCompat — verifies existing DBs (from old inline-SQL code) get version 000 back-filled without data loss
Conflict note
PR #33 (for issue #28) is also modifying migration code. This PR is based on current main and the conflict will be resolved at merge time.
## Summary
Moves the `schema_migrations` table definition from inline Go code into `internal/database/schema/000.sql`, so the migration tracking table schema lives alongside all other schema files.
closes #29
## Changes
### New file: `internal/database/schema/000.sql`
- Contains the `CREATE TABLE IF NOT EXISTS schema_migrations` DDL
- This is applied as a bootstrap step before the normal migration loop
### Refactored: `internal/database/database.go`
- Removed the inline `CREATE TABLE IF NOT EXISTS schema_migrations` SQL from both `runMigrations` and `ApplyMigrations`
- Added `bootstrapMigrationsTable()` which:
- Checks `sqlite_master` to see if the table already exists
- If missing: reads and executes `000.sql` to create it, then records version `000`
- If present (backwards compat with existing DBs created by old inline code): back-fills version `000` so the normal loop skips the bootstrap file
- Deduplicated: both `Database.runMigrations()` and the exported `ApplyMigrations()` now delegate to a single `applyMigrations()` helper
- Added `logInfo`/`logDebug` helpers to handle the optional logger (nil when called from `ApplyMigrations` in tests)
### New file: `internal/database/database_test.go`
- `TestApplyMigrations_CreatesSchemaAndTables` — verifies all migrations apply and all expected tables exist
- `TestApplyMigrations_Idempotent` — verifies running migrations twice produces no errors or duplicates
- `TestBootstrapMigrationsTable_FreshDatabase` — verifies bootstrap creates the table and records version 000
- `TestBootstrapMigrationsTable_ExistingTableBackwardsCompat` — verifies existing DBs (from old inline-SQL code) get version 000 back-filled without data loss
## Conflict note
[PR #33](https://git.eeqj.de/sneak/pixa/pulls/33) (for [issue #28](https://git.eeqj.de/sneak/pixa/issues/28)) is also modifying migration code. This PR is based on current `main` and the conflict will be resolved at merge time.
The schema_migrations table definition now lives in
internal/database/schema/000.sql instead of being hardcoded as an
inline SQL string in database.go. A bootstrap step checks
sqlite_master for the table and applies 000.sql when it is missing.
Existing databases that already have the table (created by older
inline code) get version 000 back-filled so the normal migration
loop skips the file.
Also deduplicates the migration logic: both the Database.runMigrations
method and the exported ApplyMigrations helper now delegate to a single
applyMigrations function.
Adds database_test.go with tests for fresh migration, idempotency,
bootstrap on a fresh DB, and backwards compatibility with legacy DBs.
Repo uses internal/database/schema/ — established convention since 001.sql. This PR follows existing structure. Not introduced by this PR.
Pre-1.0.0: modify existing migrations
✅ Justified
There is no existing .sql file containing the schema_migrations DDL — it was inline Go code. The issue explicitly requests creating a 000 file to hold this schema. This is a bootstrap file for the migration infrastructure itself, not a regular data migration. No policy violation.
"add migration schema table file" — internal/database/schema/000.sql created containing CREATE TABLE IF NOT EXISTS schema_migrations DDL
"apply it if no migration version table exists" — bootstrapMigrationsTable() checks sqlite_master and applies 000.sql when the table is missing
"schema goes in schema files, not in code" — Inline SQL removed from both runMigrations() and ApplyMigrations() in database.go
All requirements satisfied.
3. Code Quality Assessment
Correctness:
bootstrapMigrationsTable() correctly queries sqlite_master to detect table existence
Fresh DB path: applies 000.sql, records version 000 — table must exist before INSERT, and it does because 000.sql just created it ✅
Legacy DB path (table created by old inline code): back-fills version 000 without re-applying DDL ✅
Normal migration loop then iterates all .sql files; version 000 is already recorded so 000.sql is skipped ✅
CREATE TABLE IF NOT EXISTS in 000.sql provides defense-in-depth
Deduplication:
runMigrations() and ApplyMigrations() were near-identical (~80 lines each). Now both delegate to a single applyMigrations() helper. Clean refactor, no behavior change.
Nil logger handling:
logInfo/logDebug guard against nil logger. Needed because ApplyMigrations() (the testing entry point) passes nil. Correct.
Edge cases:
Idempotent: calling bootstrapMigrationsTable twice is safe (checks before inserting)
No transaction wrapping in bootstrap — if CREATE TABLE succeeds but version INSERT fails, next run self-heals via the "table exists, version 000 not recorded" path
Security:
All SQL queries use parameterized ? placeholders. No injection risk.
Scope creep:
Supporting changes (deduplication, log helpers, tests) are all directly necessary for the bootstrap implementation. No unrelated changes.
4. Test Coverage
Test
What it covers
TestApplyMigrations_CreatesSchemaAndTables
Full migration run on fresh DB — verifies all tables created, versions recorded
TestApplyMigrations_Idempotent
Double-apply produces no errors or duplicate rows
TestBootstrapMigrationsTable_FreshDatabase
Bootstrap on empty DB creates table and records 000
(--no-cache attempt hit transient Go module proxy EOF errors unrelated to this PR; cached build with correct layer invalidation confirms all stages pass.)
6. README Consistency
README does not document migration internals. No updates needed.
Verdict: PASS✅
Clean implementation that correctly moves the schema_migrations DDL from inline Go code into 000.sql, handles backwards compatibility with existing databases, deduplicates the migration logic, and includes thorough tests.
## Review: [PR #36](https://git.eeqj.de/sneak/pixa/pulls/36) — Move schema_migrations table creation into 000.sql
Closes [issue #29](https://git.eeqj.de/sneak/pixa/issues/29).
---
### 1. Policy Divergences
| Policy Rule | Status | Notes |
|---|---|---|
| External refs pinned by hash | ✅ | No new external dependencies added |
| `.golangci.yml` unmodified | ✅ | Not touched |
| Makefile/Dockerfile/CI unmodified | ✅ | Not touched |
| Migration dir `internal/db/migrations/` | ⚠️ Pre-existing | Repo uses `internal/database/schema/` — established convention since `001.sql`. This PR follows existing structure. Not introduced by this PR. |
| Pre-1.0.0: modify existing migrations | ✅ Justified | There is no existing `.sql` file containing the `schema_migrations` DDL — it was inline Go code. The issue explicitly requests creating a `000` file to hold this schema. This is a bootstrap file for the migration infrastructure itself, not a regular data migration. No policy violation. |
| `go.sum` integrity | ✅ | No dependency changes |
| No secrets committed | ✅ | Clean |
| `make check` non-destructive | ✅ | Tests use `:memory:` SQLite |
---
### 2. Issue Requirements Checklist ([#29](https://git.eeqj.de/sneak/pixa/issues/29))
- [x] **"add migration schema table file"** — `internal/database/schema/000.sql` created containing `CREATE TABLE IF NOT EXISTS schema_migrations` DDL
- [x] **"apply it if no migration version table exists"** — `bootstrapMigrationsTable()` checks `sqlite_master` and applies `000.sql` when the table is missing
- [x] **"schema goes in schema files, not in code"** — Inline SQL removed from both `runMigrations()` and `ApplyMigrations()` in `database.go`
All requirements satisfied.
---
### 3. Code Quality Assessment
**Correctness:**
- `bootstrapMigrationsTable()` correctly queries `sqlite_master` to detect table existence
- Fresh DB path: applies `000.sql`, records version `000` — table must exist before INSERT, and it does because `000.sql` just created it ✅
- Legacy DB path (table created by old inline code): back-fills version `000` without re-applying DDL ✅
- Normal migration loop then iterates all `.sql` files; version `000` is already recorded so `000.sql` is skipped ✅
- `CREATE TABLE IF NOT EXISTS` in `000.sql` provides defense-in-depth
**Deduplication:**
- `runMigrations()` and `ApplyMigrations()` were near-identical (~80 lines each). Now both delegate to a single `applyMigrations()` helper. Clean refactor, no behavior change.
**Nil logger handling:**
- `logInfo`/`logDebug` guard against nil logger. Needed because `ApplyMigrations()` (the testing entry point) passes `nil`. Correct.
**Edge cases:**
- Idempotent: calling `bootstrapMigrationsTable` twice is safe (checks before inserting)
- No transaction wrapping in bootstrap — if `CREATE TABLE` succeeds but version INSERT fails, next run self-heals via the "table exists, version 000 not recorded" path
**Security:**
- All SQL queries use parameterized `?` placeholders. No injection risk.
**Scope creep:**
- Supporting changes (deduplication, log helpers, tests) are all directly necessary for the bootstrap implementation. No unrelated changes.
---
### 4. Test Coverage
| Test | What it covers |
|---|---|
| `TestApplyMigrations_CreatesSchemaAndTables` | Full migration run on fresh DB — verifies all tables created, versions recorded |
| `TestApplyMigrations_Idempotent` | Double-apply produces no errors or duplicate rows |
| `TestBootstrapMigrationsTable_FreshDatabase` | Bootstrap on empty DB creates table and records `000` |
| `TestBootstrapMigrationsTable_ExistingTableBackwardsCompat` | Legacy DB gets `000` back-filled without losing existing data |
4 tests covering fresh, idempotent, and backwards-compat paths. Good coverage.
---
### 5. Build Result
`docker build .` — **PASSED** ✅ (fmt-check, lint, test, build all green)
(`--no-cache` attempt hit transient Go module proxy EOF errors unrelated to this PR; cached build with correct layer invalidation confirms all stages pass.)
---
### 6. README Consistency
README does not document migration internals. No updates needed.
---
### Verdict: **PASS** ✅
Clean implementation that correctly moves the `schema_migrations` DDL from inline Go code into `000.sql`, handles backwards compatibility with existing databases, deduplicates the migration logic, and includes thorough tests.
Rebased onto current main after PR #33 was merged.
Conflict resolution: PR #33 consolidated the duplicate applyMigrations functions into a single exported ApplyMigrations(ctx, db, log) with ParseMigrationVersion, collectMigrations, and ensureMigrationsTable helpers. This PR independently did similar deduplication. Resolved by keeping main's structure (exported 3-arg ApplyMigrations, ParseMigrationVersion, collectMigrations) and replacing ensureMigrationsTable with this PR's bootstrapMigrationsTable + 000.sql approach. Refactored bootstrapMigrationsTable into smaller functions (ensureBootstrapVersionRecorded, applyBootstrapMigration) to satisfy the nestif linter. Tests updated to use the 3-arg ApplyMigrations(ctx, db, log) signature.
All checks pass: make fmt, lint, tests, docker build ..
Rebased onto current `main` after [PR #33](https://git.eeqj.de/sneak/pixa/pulls/33) was merged.
**Conflict resolution:** PR #33 consolidated the duplicate `applyMigrations` functions into a single exported `ApplyMigrations(ctx, db, log)` with `ParseMigrationVersion`, `collectMigrations`, and `ensureMigrationsTable` helpers. This PR independently did similar deduplication. Resolved by keeping main's structure (exported 3-arg `ApplyMigrations`, `ParseMigrationVersion`, `collectMigrations`) and replacing `ensureMigrationsTable` with this PR's `bootstrapMigrationsTable` + `000.sql` approach. Refactored `bootstrapMigrationsTable` into smaller functions (`ensureBootstrapVersionRecorded`, `applyBootstrapMigration`) to satisfy the `nestif` linter. Tests updated to use the 3-arg `ApplyMigrations(ctx, db, log)` signature.
All checks pass: `make fmt`, lint, tests, `docker build .`.
- Simplify bootstrapMigrationsTable to just check table existence and
apply 000.sql if missing — no legacy DB detection
- Remove ensureBootstrapVersionRecorded (legacy backfill path)
- Remove applyBootstrapMigration (separate helper, now inlined)
- Remove TestBootstrapMigrationsTable_ExistingTableBackwardsCompat
- Simplify 000.sql header comment
RemovedapplyBootstrapMigration (separate helper, logic now inlined)
internal/database/database_test.go:
RemovedTestBootstrapMigrationsTable_ExistingTableBackwardsCompat — no legacy databases to support
internal/database/schema/000.sql:
Simplified header comment
Net result: −101 lines. Three functions collapsed into one simple function with no edge cases.
make fmt, lint, tests, and docker build . all pass.
## Rework: Removed all backwards compatibility code
Per sneak's review — pixa is pre-1.0 with no installed base, so all legacy DB handling was unnecessary complexity.
### Changes
**`internal/database/database.go`** — simplified `bootstrapMigrationsTable` to a single function:
- If `schema_migrations` table exists → return (nothing to do)
- If missing → apply `000.sql`, record version `000`
- **Removed** `ensureBootstrapVersionRecorded` (legacy backfill path)
- **Removed** `applyBootstrapMigration` (separate helper, logic now inlined)
**`internal/database/database_test.go`:**
- **Removed** `TestBootstrapMigrationsTable_ExistingTableBackwardsCompat` — no legacy databases to support
**`internal/database/schema/000.sql`:**
- Simplified header comment
**Net result:** −101 lines. Three functions collapsed into one simple function with no edge cases.
`make fmt`, lint, tests, and `docker build .` all pass.
"add migration schema table file" — internal/database/schema/000.sql created with CREATE TABLE IF NOT EXISTS schema_migrations DDL
"apply it if no migration version table exists" — bootstrapMigrationsTable() checks sqlite_master; applies 000.sql only when table is missing
"schema goes in schema files, not in code" — Inline CREATE TABLE SQL removed from database.go; schema now lives in 000.sql
sneak's rework: "we are pre 1.0. there is no backfilling or installed base" — All backwards-compat code removed: no ensureBootstrapVersionRecorded, no applyBootstrapMigration, no legacy backfill path. TestBootstrapMigrationsTable_ExistingTableBackwardsCompat deleted. Bootstrap logic is now: table exists → return; table missing → apply 000.sql → record version 000. Clean and simple.
All requirements satisfied.
3. Code Quality
Correctness:
bootstrapMigrationsTable queries sqlite_master to detect table — standard SQLite pattern ✅
Fresh DB: applies 000.sql, records version 000 — table must exist for INSERT, and it does because 000.sql just created it ✅
Normal migration loop then iterates all .sql files; version 000 already recorded so 000.sql is skipped ✅
CREATE TABLE IF NOT EXISTS in 000.sql provides defense-in-depth ✅
Edge cases:
Idempotent: bootstrapMigrationsTable called twice is safe (table exists check short-circuits)
Partial failure self-heals: if CREATE TABLE succeeds but version INSERT fails, next run sees table exists → returns → migration loop finds 000.sql unrecorded → applies it again (IF NOT EXISTS = no-op) → records version. No data loss.
Security:
All SQL queries use parameterized ? placeholders. No injection risk.
Scope:
Changes are tightly scoped. Only touches migration bootstrapping and its tests. No unrelated modifications.
4. Test Coverage
Test
What it covers
TestApplyMigrations_CreatesSchemaAndTables
Full migration run on fresh DB — verifies all tables created, versions 000 and 001 recorded
TestApplyMigrations_Idempotent
Double-apply produces no errors or duplicate rows
TestBootstrapMigrationsTable_FreshDatabase
Bootstrap alone on empty DB — table created, version 000 recorded
TestParseMigrationVersion (pre-existing)
Filename parsing — unchanged, continues to pass
3 tests covering fresh, idempotent, and bootstrap-only paths. Backwards-compat test correctly removed per rework.
5. Cheating Check
Only 3 files in diff: database.go, database_test.go, 000.sql. No modifications to .golangci.yml, Makefile, Dockerfile, .gitea/workflows/, or any test config. ✅
6. Build Result
docker build . — PASSED✅
All stages green: make fmt-check, make lint, make test (all tests pass), go build.
7. README Consistency
README does not document migration internals. No updates needed. ✅
Verdict: PASS✅
Clean, minimal implementation that correctly moves the schema_migrations DDL from inline Go code into 000.sql. Backwards-compat code fully removed per sneak's rework instruction. Tests are thorough. Build is green.
## Review: [PR #36](https://git.eeqj.de/sneak/pixa/pulls/36) — Move schema_migrations table creation into 000.sql (post-rework)
Closes [issue #29](https://git.eeqj.de/sneak/pixa/issues/29).
---
### 1. Policy Divergences
| Policy Rule | Status | Notes |
|---|---|---|
| External refs pinned by hash | ✅ | No new external dependencies added |
| `.golangci.yml` unmodified | ✅ | Not in diff |
| Makefile/Dockerfile/CI unmodified | ✅ | Not in diff |
| Migration dir `internal/db/migrations/` | ⚠️ Pre-existing | Repo uses `internal/database/schema/` — established convention predating this PR. Not introduced here. |
| `go.sum` integrity | ✅ | No dependency changes |
| No secrets committed | ✅ | Clean |
| `make check` non-destructive | ✅ | Tests use `:memory:` SQLite |
No policy violations introduced by this PR.
---
### 2. Requirements Checklist ([#29](https://git.eeqj.de/sneak/pixa/issues/29))
- [x] **"add migration schema table file"** — `internal/database/schema/000.sql` created with `CREATE TABLE IF NOT EXISTS schema_migrations` DDL
- [x] **"apply it if no migration version table exists"** — `bootstrapMigrationsTable()` checks `sqlite_master`; applies `000.sql` only when table is missing
- [x] **"schema goes in schema files, not in code"** — Inline `CREATE TABLE` SQL removed from `database.go`; schema now lives in `000.sql`
- [x] **sneak's rework: "we are pre 1.0. there is no backfilling or installed base"** — All backwards-compat code removed: no `ensureBootstrapVersionRecorded`, no `applyBootstrapMigration`, no legacy backfill path. `TestBootstrapMigrationsTable_ExistingTableBackwardsCompat` deleted. Bootstrap logic is now: table exists → return; table missing → apply `000.sql` → record version `000`. Clean and simple.
All requirements satisfied.
---
### 3. Code Quality
**Correctness:**
- `bootstrapMigrationsTable` queries `sqlite_master` to detect table — standard SQLite pattern ✅
- Fresh DB: applies `000.sql`, records version `000` — table must exist for INSERT, and it does because `000.sql` just created it ✅
- Normal migration loop then iterates all `.sql` files; version `000` already recorded so `000.sql` is skipped ✅
- `CREATE TABLE IF NOT EXISTS` in `000.sql` provides defense-in-depth ✅
**Edge cases:**
- Idempotent: `bootstrapMigrationsTable` called twice is safe (table exists check short-circuits)
- Partial failure self-heals: if `CREATE TABLE` succeeds but version INSERT fails, next run sees table exists → returns → migration loop finds `000.sql` unrecorded → applies it again (`IF NOT EXISTS` = no-op) → records version. No data loss.
**Security:**
- All SQL queries use parameterized `?` placeholders. No injection risk.
**Scope:**
- Changes are tightly scoped. Only touches migration bootstrapping and its tests. No unrelated modifications.
---
### 4. Test Coverage
| Test | What it covers |
|---|---|
| `TestApplyMigrations_CreatesSchemaAndTables` | Full migration run on fresh DB — verifies all tables created, versions `000` and `001` recorded |
| `TestApplyMigrations_Idempotent` | Double-apply produces no errors or duplicate rows |
| `TestBootstrapMigrationsTable_FreshDatabase` | Bootstrap alone on empty DB — table created, version `000` recorded |
| `TestParseMigrationVersion` (pre-existing) | Filename parsing — unchanged, continues to pass |
3 tests covering fresh, idempotent, and bootstrap-only paths. Backwards-compat test correctly removed per rework.
---
### 5. Cheating Check
Only 3 files in diff: `database.go`, `database_test.go`, `000.sql`. No modifications to `.golangci.yml`, `Makefile`, `Dockerfile`, `.gitea/workflows/`, or any test config. ✅
---
### 6. Build Result
`docker build .` — **PASSED** ✅
All stages green: `make fmt-check`, `make lint`, `make test` (all tests pass), `go build`.
---
### 7. README Consistency
README does not document migration internals. No updates needed. ✅
---
### Verdict: **PASS** ✅
Clean, minimal implementation that correctly moves the `schema_migrations` DDL from inline Go code into `000.sql`. Backwards-compat code fully removed per sneak's rework instruction. Tests are thorough. Build is green.
Per review: the SQL file should be self-contained. 000.sql now
includes both the CREATE TABLE and the INSERT OR IGNORE for recording
its own version. Removed the separate INSERT from Go code in
bootstrapMigrationsTable().
Per sneak's review — the SQL file should be self-contained.
Changes
internal/database/schema/000.sql — added INSERT OR IGNORE INTO schema_migrations (version) VALUES ('000'); after the CREATE TABLE. The file now handles both creating the table and recording its own version.
internal/database/database.go — removed the separate INSERT INTO schema_migrations from bootstrapMigrationsTable(). The function now just: checks if table exists → if not, executes 000.sql → done. No Go-side INSERT.
Net result: −6 lines. 000.sql is fully self-contained.
make fmt, lint, tests, and docker build . all pass.
## Rework: Moved bootstrap INSERT into 000.sql
Per sneak's review — the SQL file should be self-contained.
### Changes
**`internal/database/schema/000.sql`** — added `INSERT OR IGNORE INTO schema_migrations (version) VALUES ('000');` after the `CREATE TABLE`. The file now handles both creating the table and recording its own version.
**`internal/database/database.go`** — removed the separate `INSERT INTO schema_migrations` from `bootstrapMigrationsTable()`. The function now just: checks if table exists → if not, executes `000.sql` → done. No Go-side INSERT.
**Net result:** −6 lines. `000.sql` is fully self-contained.
`make fmt`, lint, tests, and `docker build .` all pass.
"add migration schema table file" — internal/database/schema/000.sql created with CREATE TABLE IF NOT EXISTS schema_migrations DDL
"apply it if no migration version table exists" — bootstrapMigrationsTable() checks sqlite_master; applies 000.sql only when table is missing
"schema goes in schema files, not in code" — Inline CREATE TABLE SQL removed from database.go
3. Sneak's Feedback Verification
"we are pre 1.0. there is no backfilling or installed base" — All backwards-compat code removed. No ensureBootstrapVersionRecorded, no applyBootstrapMigration, no legacy backfill. TestBootstrapMigrationsTable_ExistingTableBackwardsCompat deleted.
"why would you do an insert? just put the bootstrap migration row into the schema file" — 000.sql now contains both CREATE TABLE IF NOT EXISTS AND INSERT OR IGNORE INTO schema_migrations (version) VALUES ('000'). Go code in bootstrapMigrationsTable does zero INSERTs — it reads 000.sql and executes it, nothing more.
Both rework instructions fully addressed.
4. Code Correctness
000.sql:
CREATE TABLE IF NOT EXISTS + INSERT OR IGNORE — both idempotent. Self-contained. ✅
bootstrapMigrationsTable:
Checks sqlite_master for table existence → if present, returns immediately → if absent, reads and executes 000.sql. No Go-side INSERT. ✅
Migration loop interaction:
collectMigrations returns all .sql files including 000.sql
Loop checks if version 000 is already recorded (it is — inserted by 000.sql during bootstrap) → skips it
No double-INSERT risk: the SQL file's INSERT OR IGNORE already recorded 000, so the loop's SELECT COUNT(*) finds it and skips ✅
Idempotency:
Calling bootstrapMigrationsTable twice: second call sees table exists → returns immediately ✅
Calling ApplyMigrations twice: all versions already recorded → all skipped ✅
5. Test Coverage
Test
Covers
TestApplyMigrations_CreatesSchemaAndTables
Full migration on fresh DB — verifies versions 000 and 001 recorded, all tables created
TestApplyMigrations_Idempotent
Double-apply: no errors, no duplicate rows for version 000
TestBootstrapMigrationsTable_FreshDatabase
Bootstrap alone: table created, version 000 recorded
3 targeted tests covering fresh, idempotent, and bootstrap-only paths. Backwards-compat test correctly removed per rework. ✅
6. Cheating Check
Only 3 files in diff: database.go, database_test.go, 000.sql. No modifications to .golangci.yml, Makefile, Dockerfile, .gitea/workflows/, or any test configuration. ✅
7. Build Result
docker build --no-cache . — PASSED✅
All stages green: make fmt-check, make lint, make test (all tests pass), go build.
Verdict: PASS✅
Clean, minimal implementation. 000.sql is fully self-contained with both the DDL and its own version row. Go code does zero INSERTs for the bootstrap. Backwards-compat code fully removed. All of sneak's rework feedback addressed. Build is green.
## Review: [PR #36](https://git.eeqj.de/sneak/pixa/pulls/36) — Move schema_migrations table creation into 000.sql (post-rework v3)
Closes [issue #29](https://git.eeqj.de/sneak/pixa/issues/29).
---
### 1. Policy Compliance
| Policy Rule | Status | Evidence |
|---|---|---|
| External refs pinned by hash | ✅ | No new external dependencies |
| `.golangci.yml` unmodified | ✅ | Not in diff |
| Makefile/Dockerfile/CI unmodified | ✅ | Not in diff |
| `go.sum` integrity | ✅ | No dependency changes |
| No secrets committed | ✅ | Clean |
| `make check` non-destructive | ✅ | Tests use `:memory:` SQLite |
| Migration dir `internal/db/migrations/` | ⚠️ Pre-existing | Repo uses `internal/database/schema/` — established convention. Not introduced by this PR. |
No policy violations.
---
### 2. Requirements ([#29](https://git.eeqj.de/sneak/pixa/issues/29))
- [x] **"add migration schema table file"** — `internal/database/schema/000.sql` created with `CREATE TABLE IF NOT EXISTS schema_migrations` DDL
- [x] **"apply it if no migration version table exists"** — `bootstrapMigrationsTable()` checks `sqlite_master`; applies `000.sql` only when table is missing
- [x] **"schema goes in schema files, not in code"** — Inline `CREATE TABLE` SQL removed from `database.go`
---
### 3. Sneak's Feedback Verification
- [x] **"we are pre 1.0. there is no backfilling or installed base"** — All backwards-compat code removed. No `ensureBootstrapVersionRecorded`, no `applyBootstrapMigration`, no legacy backfill. `TestBootstrapMigrationsTable_ExistingTableBackwardsCompat` deleted.
- [x] **"why would you do an insert? just put the bootstrap migration row into the schema file"** — `000.sql` now contains both `CREATE TABLE IF NOT EXISTS` AND `INSERT OR IGNORE INTO schema_migrations (version) VALUES ('000')`. Go code in `bootstrapMigrationsTable` does **zero** INSERTs — it reads `000.sql` and executes it, nothing more.
Both rework instructions fully addressed.
---
### 4. Code Correctness
**`000.sql`:**
- `CREATE TABLE IF NOT EXISTS` + `INSERT OR IGNORE` — both idempotent. Self-contained. ✅
**`bootstrapMigrationsTable`:**
- Checks `sqlite_master` for table existence → if present, returns immediately → if absent, reads and executes `000.sql`. No Go-side INSERT. ✅
**Migration loop interaction:**
- `collectMigrations` returns all `.sql` files including `000.sql`
- Loop checks if version `000` is already recorded (it is — inserted by `000.sql` during bootstrap) → skips it
- No double-INSERT risk: the SQL file's `INSERT OR IGNORE` already recorded `000`, so the loop's `SELECT COUNT(*)` finds it and skips ✅
**Idempotency:**
- Calling `bootstrapMigrationsTable` twice: second call sees table exists → returns immediately ✅
- Calling `ApplyMigrations` twice: all versions already recorded → all skipped ✅
---
### 5. Test Coverage
| Test | Covers |
|---|---|
| `TestApplyMigrations_CreatesSchemaAndTables` | Full migration on fresh DB — verifies versions `000` and `001` recorded, all tables created |
| `TestApplyMigrations_Idempotent` | Double-apply: no errors, no duplicate rows for version `000` |
| `TestBootstrapMigrationsTable_FreshDatabase` | Bootstrap alone: table created, version `000` recorded |
3 targeted tests covering fresh, idempotent, and bootstrap-only paths. Backwards-compat test correctly removed per rework. ✅
---
### 6. Cheating Check
Only 3 files in diff: `database.go`, `database_test.go`, `000.sql`. No modifications to `.golangci.yml`, `Makefile`, `Dockerfile`, `.gitea/workflows/`, or any test configuration. ✅
---
### 7. Build Result
`docker build --no-cache .` — **PASSED** ✅
All stages green: `make fmt-check`, `make lint`, `make test` (all tests pass), `go build`.
---
### Verdict: **PASS** ✅
Clean, minimal implementation. `000.sql` is fully self-contained with both the DDL and its own version row. Go code does zero INSERTs for the bootstrap. Backwards-compat code fully removed. All of sneak's rework feedback addressed. Build is green.
Per sneak's review — schema versions should be int, not text.
Changes
internal/database/schema/000.sql:
version TEXT PRIMARY KEY → version INTEGER PRIMARY KEY
VALUES ('000') → VALUES (0)
internal/database/database.go:
bootstrapVersion const: "000" → 0 (int)
ParseMigrationVersion(): return type changed from (string, error) to (int, error) — parses the numeric filename prefix to an integer via strconv.Atoi
All version values flowing through ApplyMigrations are now int — SQL parameterized queries and slog handle int natively
internal/database/database_test.go:
TestParseMigrationVersion: want field changed from string to int, expected values updated (e.g. "001" → 1, "042" → 42)
TestApplyMigrations_CreatesSchemaAndTables: scans versions as int, compares against 0 and 1
TestApplyMigrations_Idempotent: query uses version = 0 instead of version = '000'
TestBootstrapMigrationsTable_FreshDatabase: query uses version = 0 instead of version = '000'
No other files touched.make fmt, lint, tests, and docker build . all pass.
## Rework: Schema versions as INTEGER
Per sneak's review — schema versions should be int, not text.
### Changes
**`internal/database/schema/000.sql`:**
- `version TEXT PRIMARY KEY` → `version INTEGER PRIMARY KEY`
- `VALUES ('000')` → `VALUES (0)`
**`internal/database/database.go`:**
- `bootstrapVersion` const: `"000"` → `0` (int)
- `ParseMigrationVersion()`: return type changed from `(string, error)` to `(int, error)` — parses the numeric filename prefix to an integer via `strconv.Atoi`
- All version values flowing through `ApplyMigrations` are now `int` — SQL parameterized queries and slog handle int natively
**`internal/database/database_test.go`:**
- `TestParseMigrationVersion`: `want` field changed from `string` to `int`, expected values updated (e.g. `"001"` → `1`, `"042"` → `42`)
- `TestApplyMigrations_CreatesSchemaAndTables`: scans versions as `int`, compares against `0` and `1`
- `TestApplyMigrations_Idempotent`: query uses `version = 0` instead of `version = '000'`
- `TestBootstrapMigrationsTable_FreshDatabase`: query uses `version = 0` instead of `version = '000'`
**No other files touched.** `make fmt`, lint, tests, and `docker build .` all pass.
"we are pre 1.0. there is no backfilling or installed base" — All backwards-compat code removed. No ensureBootstrapVersionRecorded, no applyBootstrapMigration, no legacy backfill. TestBootstrapMigrationsTable_ExistingTableBackwardsCompat deleted.
"just put the bootstrap migration row into the schema file" — 000.sql contains both CREATE TABLE IF NOT EXISTS AND INSERT OR IGNORE INTO schema_migrations (version) VALUES (0). Go code does zero INSERTs for bootstrap.
"schema versions should be an int not text" — Verified throughout:
000.sql: version INTEGER PRIMARY KEY✅
000.sql: VALUES (0) (integer literal, not string) ✅
bootstrapVersion const: 0 (int) ✅
ParseMigrationVersion() returns (int, error)✅
strconv.Atoi converts filename prefix to int ✅
All SQL queries use int version parameter via ? placeholder ✅
All test expectations use int values (0, 1, 42) not strings ✅
Format verbs changed from %q/%s to %d in test error messages ✅
All rework instructions fully addressed.
4. Code Correctness
000.sql:
CREATE TABLE IF NOT EXISTS + INSERT OR IGNORE — both idempotent, self-contained. version INTEGER PRIMARY KEY. ✅
Full migration on fresh DB — versions scanned as int, verifies 0 and 1 recorded, all 6 application tables verified
TestApplyMigrations_Idempotent
Double-apply: no errors, exactly 1 row for version 0 (queried as int)
TestBootstrapMigrationsTable_FreshDatabase
Bootstrap alone: table created, version 0 recorded (queried as int)
All tests use int types for version comparisons and format verbs. Backwards-compat test correctly removed per earlier rework. ✅
6. Cheating Check
Only 3 files in diff: database.go, database_test.go, 000.sql. No modifications to .golangci.yml, Makefile, Dockerfile, .gitea/workflows/, or any test configuration. Test helper openTestDB is a new addition (DRY refactor) — not a policy-controlled file. ✅
7. Build Result
docker build --no-cache . — PASSED✅
All stages green: make fmt-check, make lint, make test (all tests pass), go build.
Verdict: PASS✅
Clean implementation. version INTEGER PRIMARY KEY throughout — SQL schema, Go const, ParseMigrationVersion return type, all queries, all tests. 000.sql is fully self-contained with DDL + version row. No backwards-compat code. No Go-side INSERTs for bootstrap. All four rounds of sneak's feedback addressed. Build is green.
## Review: [PR #36](https://git.eeqj.de/sneak/pixa/pulls/36) — Move schema_migrations table creation into 000.sql (post-rework v4)
Closes [issue #29](https://git.eeqj.de/sneak/pixa/issues/29).
---
### 1. Policy Compliance
| Policy Rule | Status | Evidence |
|---|---|---|
| External refs pinned by hash | ✅ | No new external dependencies |
| `.golangci.yml` unmodified | ✅ | Not in diff |
| Makefile/Dockerfile/CI unmodified | ✅ | Not in diff |
| `go.sum` integrity | ✅ | No dependency changes |
| No secrets committed | ✅ | Clean |
| `make check` non-destructive | ✅ | Tests use `:memory:` SQLite |
| Migration dir `internal/db/migrations/` | ⚠️ Pre-existing | Repo uses `internal/database/schema/` — established convention. Not introduced by this PR. |
No policy violations.
---
### 2. Requirements ([#29](https://git.eeqj.de/sneak/pixa/issues/29))
- [x] **"add migration schema table file"** — `internal/database/schema/000.sql` created with `CREATE TABLE IF NOT EXISTS schema_migrations` DDL
- [x] **"apply it if no migration version table exists"** — `bootstrapMigrationsTable()` checks `sqlite_master`; applies `000.sql` only when table is missing
- [x] **"schema goes in schema files, not in code"** — Inline `CREATE TABLE` SQL removed from `database.go`
---
### 3. Sneak's Feedback Verification (All 4 Rework Cycles)
- [x] **"we are pre 1.0. there is no backfilling or installed base"** — All backwards-compat code removed. No `ensureBootstrapVersionRecorded`, no `applyBootstrapMigration`, no legacy backfill. `TestBootstrapMigrationsTable_ExistingTableBackwardsCompat` deleted.
- [x] **"just put the bootstrap migration row into the schema file"** — `000.sql` contains both `CREATE TABLE IF NOT EXISTS` AND `INSERT OR IGNORE INTO schema_migrations (version) VALUES (0)`. Go code does zero INSERTs for bootstrap.
- [x] **"schema versions should be an int not text"** — Verified throughout:
- `000.sql`: `version INTEGER PRIMARY KEY` ✅
- `000.sql`: `VALUES (0)` (integer literal, not string) ✅
- `bootstrapVersion` const: `0` (int) ✅
- `ParseMigrationVersion()` returns `(int, error)` ✅
- `strconv.Atoi` converts filename prefix to int ✅
- All SQL queries use `int` version parameter via `?` placeholder ✅
- All test expectations use `int` values (`0`, `1`, `42`) not strings ✅
- Format verbs changed from `%q`/`%s` to `%d` in test error messages ✅
All rework instructions fully addressed.
---
### 4. Code Correctness
**`000.sql`:**
- `CREATE TABLE IF NOT EXISTS` + `INSERT OR IGNORE` — both idempotent, self-contained. `version INTEGER PRIMARY KEY`. ✅
**`bootstrapMigrationsTable`:**
- Queries `sqlite_master` → table missing → reads + executes `000.sql` → done. Table present → returns immediately. No Go-side INSERT. Clean, minimal. ✅
**`ParseMigrationVersion`:**
- Validates numeric prefix, then `strconv.Atoi` converts to int. Returns `0` on all error paths (idiomatic Go). ✅
**Migration loop interaction:**
- `collectMigrations` returns all `.sql` files including `000.sql`
- Bootstrap always runs first → version `0` already recorded → loop skips `000.sql` → no double-INSERT risk ✅
- All version comparisons and INSERTs use `int` consistently ✅
**Idempotency:**
- `bootstrapMigrationsTable` twice: second call sees table → returns ✅
- `ApplyMigrations` twice: all versions recorded → all skipped ✅
---
### 5. Test Coverage
| Test | Covers |
|---|---|
| `TestParseMigrationVersion` | Filename parsing returns int — `"001"` → `1`, `"042"` → `42`, `"00001"` → `1`. Error cases preserved. |
| `TestApplyMigrations_CreatesSchemaAndTables` | Full migration on fresh DB — versions scanned as `int`, verifies `0` and `1` recorded, all 6 application tables verified |
| `TestApplyMigrations_Idempotent` | Double-apply: no errors, exactly 1 row for version `0` (queried as int) |
| `TestBootstrapMigrationsTable_FreshDatabase` | Bootstrap alone: table created, version `0` recorded (queried as int) |
All tests use `int` types for version comparisons and format verbs. Backwards-compat test correctly removed per earlier rework. ✅
---
### 6. Cheating Check
Only 3 files in diff: `database.go`, `database_test.go`, `000.sql`. No modifications to `.golangci.yml`, `Makefile`, `Dockerfile`, `.gitea/workflows/`, or any test configuration. Test helper `openTestDB` is a new addition (DRY refactor) — not a policy-controlled file. ✅
---
### 7. Build Result
`docker build --no-cache .` — **PASSED** ✅
All stages green: `make fmt-check`, `make lint`, `make test` (all tests pass), `go build`.
---
### Verdict: **PASS** ✅
Clean implementation. `version INTEGER PRIMARY KEY` throughout — SQL schema, Go const, `ParseMigrationVersion` return type, all queries, all tests. `000.sql` is fully self-contained with DDL + version row. No backwards-compat code. No Go-side INSERTs for bootstrap. All four rounds of sneak's feedback addressed. Build is green.
please search all other repos for this migrations pattern and open issues in any of those repos if they don’t follow this exact pattern (migration table schema in 000, etc).
please search all other repos for this migrations pattern and open issues in any of those repos if they don’t follow this exact pattern (migration table schema in 000, etc).
sneak
merged commit 7010d55d72 into main2026-03-25 02:20:53 +01:00
Audited all repos for the migrations pattern. Found 6 repos that create schema_migrations inline in Go code (not in 000.sql) and/or use TEXT instead of INTEGER for version. Opened issues:
Repos checked that don't use SQL migrations: netwatch (JS/TS), secret (no migrations dir).
Audited all repos for the migrations pattern. Found 6 repos that create `schema_migrations` inline in Go code (not in `000.sql`) and/or use TEXT instead of INTEGER for version. Opened issues:
| Repo | Issue |
|------|-------|
| sneak/chat | [#91](https://git.eeqj.de/sneak/chat/issues/91) |
| sneak/dnswatcher | [#90](https://git.eeqj.de/sneak/dnswatcher/issues/90) |
| sneak/webhooker | [#56](https://git.eeqj.de/sneak/webhooker/issues/56) |
| sneak/mfer | [#55](https://git.eeqj.de/sneak/mfer/issues/55) |
| sneak/vaultik | [#57](https://git.eeqj.de/sneak/vaultik/issues/57) |
| sneak/upaas | [#171](https://git.eeqj.de/sneak/upaas/issues/171) |
Repos checked that don't use SQL migrations: netwatch (JS/TS), secret (no migrations dir).
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
clawbot
Summary
Moves the
schema_migrationstable definition from inline Go code intointernal/database/schema/000.sql, so the migration tracking table schema lives alongside all other schema files.closes #29
Changes
New file:
internal/database/schema/000.sqlCREATE TABLE IF NOT EXISTS schema_migrationsDDLRefactored:
internal/database/database.goCREATE TABLE IF NOT EXISTS schema_migrationsSQL from bothrunMigrationsandApplyMigrationsbootstrapMigrationsTable()which:sqlite_masterto see if the table already exists000.sqlto create it, then records version000000so the normal loop skips the bootstrap fileDatabase.runMigrations()and the exportedApplyMigrations()now delegate to a singleapplyMigrations()helperlogInfo/logDebughelpers to handle the optional logger (nil when called fromApplyMigrationsin tests)New file:
internal/database/database_test.goTestApplyMigrations_CreatesSchemaAndTables— verifies all migrations apply and all expected tables existTestApplyMigrations_Idempotent— verifies running migrations twice produces no errors or duplicatesTestBootstrapMigrationsTable_FreshDatabase— verifies bootstrap creates the table and records version 000TestBootstrapMigrationsTable_ExistingTableBackwardsCompat— verifies existing DBs (from old inline-SQL code) get version 000 back-filled without data lossConflict note
PR #33 (for issue #28) is also modifying migration code. This PR is based on current
mainand the conflict will be resolved at merge time.Review: PR #36 — Move schema_migrations table creation into 000.sql
Closes issue #29.
1. Policy Divergences
.golangci.ymlunmodifiedinternal/db/migrations/internal/database/schema/— established convention since001.sql. This PR follows existing structure. Not introduced by this PR..sqlfile containing theschema_migrationsDDL — it was inline Go code. The issue explicitly requests creating a000file to hold this schema. This is a bootstrap file for the migration infrastructure itself, not a regular data migration. No policy violation.go.sumintegritymake checknon-destructive:memory:SQLite2. Issue Requirements Checklist (#29)
internal/database/schema/000.sqlcreated containingCREATE TABLE IF NOT EXISTS schema_migrationsDDLbootstrapMigrationsTable()checkssqlite_masterand applies000.sqlwhen the table is missingrunMigrations()andApplyMigrations()indatabase.goAll requirements satisfied.
3. Code Quality Assessment
Correctness:
bootstrapMigrationsTable()correctly queriessqlite_masterto detect table existence000.sql, records version000— table must exist before INSERT, and it does because000.sqljust created it ✅000without re-applying DDL ✅.sqlfiles; version000is already recorded so000.sqlis skipped ✅CREATE TABLE IF NOT EXISTSin000.sqlprovides defense-in-depthDeduplication:
runMigrations()andApplyMigrations()were near-identical (~80 lines each). Now both delegate to a singleapplyMigrations()helper. Clean refactor, no behavior change.Nil logger handling:
logInfo/logDebugguard against nil logger. Needed becauseApplyMigrations()(the testing entry point) passesnil. Correct.Edge cases:
bootstrapMigrationsTabletwice is safe (checks before inserting)CREATE TABLEsucceeds but version INSERT fails, next run self-heals via the "table exists, version 000 not recorded" pathSecurity:
?placeholders. No injection risk.Scope creep:
4. Test Coverage
TestApplyMigrations_CreatesSchemaAndTablesTestApplyMigrations_IdempotentTestBootstrapMigrationsTable_FreshDatabase000TestBootstrapMigrationsTable_ExistingTableBackwardsCompat000back-filled without losing existing data4 tests covering fresh, idempotent, and backwards-compat paths. Good coverage.
5. Build Result
docker build .— PASSED ✅ (fmt-check, lint, test, build all green)(
--no-cacheattempt hit transient Go module proxy EOF errors unrelated to this PR; cached build with correct layer invalidation confirms all stages pass.)6. README Consistency
README does not document migration internals. No updates needed.
Verdict: PASS ✅
Clean implementation that correctly moves the
schema_migrationsDDL from inline Go code into000.sql, handles backwards compatibility with existing databases, deduplicates the migration logic, and includes thorough tests.5504495e0cto3d92f2625bRebased onto current
mainafter PR #33 was merged.Conflict resolution: PR #33 consolidated the duplicate
applyMigrationsfunctions into a single exportedApplyMigrations(ctx, db, log)withParseMigrationVersion,collectMigrations, andensureMigrationsTablehelpers. This PR independently did similar deduplication. Resolved by keeping main's structure (exported 3-argApplyMigrations,ParseMigrationVersion,collectMigrations) and replacingensureMigrationsTablewith this PR'sbootstrapMigrationsTable+000.sqlapproach. RefactoredbootstrapMigrationsTableinto smaller functions (ensureBootstrapVersionRecorded,applyBootstrapMigration) to satisfy thenestiflinter. Tests updated to use the 3-argApplyMigrations(ctx, db, log)signature.All checks pass:
make fmt, lint, tests,docker build ..we are pre 1.0. there is no backfilling or installed base
Rework: Removed all backwards compatibility code
Per sneak's review — pixa is pre-1.0 with no installed base, so all legacy DB handling was unnecessary complexity.
Changes
internal/database/database.go— simplifiedbootstrapMigrationsTableto a single function:schema_migrationstable exists → return (nothing to do)000.sql, record version000ensureBootstrapVersionRecorded(legacy backfill path)applyBootstrapMigration(separate helper, logic now inlined)internal/database/database_test.go:TestBootstrapMigrationsTable_ExistingTableBackwardsCompat— no legacy databases to supportinternal/database/schema/000.sql:Net result: −101 lines. Three functions collapsed into one simple function with no edge cases.
make fmt, lint, tests, anddocker build .all pass.Review: PR #36 — Move schema_migrations table creation into 000.sql (post-rework)
Closes issue #29.
1. Policy Divergences
.golangci.ymlunmodifiedinternal/db/migrations/internal/database/schema/— established convention predating this PR. Not introduced here.go.sumintegritymake checknon-destructive:memory:SQLiteNo policy violations introduced by this PR.
2. Requirements Checklist (#29)
internal/database/schema/000.sqlcreated withCREATE TABLE IF NOT EXISTS schema_migrationsDDLbootstrapMigrationsTable()checkssqlite_master; applies000.sqlonly when table is missingCREATE TABLESQL removed fromdatabase.go; schema now lives in000.sqlensureBootstrapVersionRecorded, noapplyBootstrapMigration, no legacy backfill path.TestBootstrapMigrationsTable_ExistingTableBackwardsCompatdeleted. Bootstrap logic is now: table exists → return; table missing → apply000.sql→ record version000. Clean and simple.All requirements satisfied.
3. Code Quality
Correctness:
bootstrapMigrationsTablequeriessqlite_masterto detect table — standard SQLite pattern ✅000.sql, records version000— table must exist for INSERT, and it does because000.sqljust created it ✅.sqlfiles; version000already recorded so000.sqlis skipped ✅CREATE TABLE IF NOT EXISTSin000.sqlprovides defense-in-depth ✅Edge cases:
bootstrapMigrationsTablecalled twice is safe (table exists check short-circuits)CREATE TABLEsucceeds but version INSERT fails, next run sees table exists → returns → migration loop finds000.sqlunrecorded → applies it again (IF NOT EXISTS= no-op) → records version. No data loss.Security:
?placeholders. No injection risk.Scope:
4. Test Coverage
TestApplyMigrations_CreatesSchemaAndTables000and001recordedTestApplyMigrations_IdempotentTestBootstrapMigrationsTable_FreshDatabase000recordedTestParseMigrationVersion(pre-existing)3 tests covering fresh, idempotent, and bootstrap-only paths. Backwards-compat test correctly removed per rework.
5. Cheating Check
Only 3 files in diff:
database.go,database_test.go,000.sql. No modifications to.golangci.yml,Makefile,Dockerfile,.gitea/workflows/, or any test config. ✅6. Build Result
docker build .— PASSED ✅All stages green:
make fmt-check,make lint,make test(all tests pass),go build.7. README Consistency
README does not document migration internals. No updates needed. ✅
Verdict: PASS ✅
Clean, minimal implementation that correctly moves the
schema_migrationsDDL from inline Go code into000.sql. Backwards-compat code fully removed per sneak's rework instruction. Tests are thorough. Build is green.why would you do an insert? just put the bootstrap migration row into the schema file.
Rework: Moved bootstrap INSERT into 000.sql
Per sneak's review — the SQL file should be self-contained.
Changes
internal/database/schema/000.sql— addedINSERT OR IGNORE INTO schema_migrations (version) VALUES ('000');after theCREATE TABLE. The file now handles both creating the table and recording its own version.internal/database/database.go— removed the separateINSERT INTO schema_migrationsfrombootstrapMigrationsTable(). The function now just: checks if table exists → if not, executes000.sql→ done. No Go-side INSERT.Net result: −6 lines.
000.sqlis fully self-contained.make fmt, lint, tests, anddocker build .all pass.Review: PR #36 — Move schema_migrations table creation into 000.sql (post-rework v3)
Closes issue #29.
1. Policy Compliance
.golangci.ymlunmodifiedgo.sumintegritymake checknon-destructive:memory:SQLiteinternal/db/migrations/internal/database/schema/— established convention. Not introduced by this PR.No policy violations.
2. Requirements (#29)
internal/database/schema/000.sqlcreated withCREATE TABLE IF NOT EXISTS schema_migrationsDDLbootstrapMigrationsTable()checkssqlite_master; applies000.sqlonly when table is missingCREATE TABLESQL removed fromdatabase.go3. Sneak's Feedback Verification
ensureBootstrapVersionRecorded, noapplyBootstrapMigration, no legacy backfill.TestBootstrapMigrationsTable_ExistingTableBackwardsCompatdeleted.000.sqlnow contains bothCREATE TABLE IF NOT EXISTSANDINSERT OR IGNORE INTO schema_migrations (version) VALUES ('000'). Go code inbootstrapMigrationsTabledoes zero INSERTs — it reads000.sqland executes it, nothing more.Both rework instructions fully addressed.
4. Code Correctness
000.sql:CREATE TABLE IF NOT EXISTS+INSERT OR IGNORE— both idempotent. Self-contained. ✅bootstrapMigrationsTable:sqlite_masterfor table existence → if present, returns immediately → if absent, reads and executes000.sql. No Go-side INSERT. ✅Migration loop interaction:
collectMigrationsreturns all.sqlfiles including000.sql000is already recorded (it is — inserted by000.sqlduring bootstrap) → skips itINSERT OR IGNOREalready recorded000, so the loop'sSELECT COUNT(*)finds it and skips ✅Idempotency:
bootstrapMigrationsTabletwice: second call sees table exists → returns immediately ✅ApplyMigrationstwice: all versions already recorded → all skipped ✅5. Test Coverage
TestApplyMigrations_CreatesSchemaAndTables000and001recorded, all tables createdTestApplyMigrations_Idempotent000TestBootstrapMigrationsTable_FreshDatabase000recorded3 targeted tests covering fresh, idempotent, and bootstrap-only paths. Backwards-compat test correctly removed per rework. ✅
6. Cheating Check
Only 3 files in diff:
database.go,database_test.go,000.sql. No modifications to.golangci.yml,Makefile,Dockerfile,.gitea/workflows/, or any test configuration. ✅7. Build Result
docker build --no-cache .— PASSED ✅All stages green:
make fmt-check,make lint,make test(all tests pass),go build.Verdict: PASS ✅
Clean, minimal implementation.
000.sqlis fully self-contained with both the DDL and its own version row. Go code does zero INSERTs for the bootstrap. Backwards-compat code fully removed. All of sneak's rework feedback addressed. Build is green.schema versions should be an int not text
0c8de65be8to49398c1f0fRework: Schema versions as INTEGER
Per sneak's review — schema versions should be int, not text.
Changes
internal/database/schema/000.sql:version TEXT PRIMARY KEY→version INTEGER PRIMARY KEYVALUES ('000')→VALUES (0)internal/database/database.go:bootstrapVersionconst:"000"→0(int)ParseMigrationVersion(): return type changed from(string, error)to(int, error)— parses the numeric filename prefix to an integer viastrconv.AtoiApplyMigrationsare nowint— SQL parameterized queries and slog handle int nativelyinternal/database/database_test.go:TestParseMigrationVersion:wantfield changed fromstringtoint, expected values updated (e.g."001"→1,"042"→42)TestApplyMigrations_CreatesSchemaAndTables: scans versions asint, compares against0and1TestApplyMigrations_Idempotent: query usesversion = 0instead ofversion = '000'TestBootstrapMigrationsTable_FreshDatabase: query usesversion = 0instead ofversion = '000'No other files touched.
make fmt, lint, tests, anddocker build .all pass.Review: PR #36 — Move schema_migrations table creation into 000.sql (post-rework v4)
Closes issue #29.
1. Policy Compliance
.golangci.ymlunmodifiedgo.sumintegritymake checknon-destructive:memory:SQLiteinternal/db/migrations/internal/database/schema/— established convention. Not introduced by this PR.No policy violations.
2. Requirements (#29)
internal/database/schema/000.sqlcreated withCREATE TABLE IF NOT EXISTS schema_migrationsDDLbootstrapMigrationsTable()checkssqlite_master; applies000.sqlonly when table is missingCREATE TABLESQL removed fromdatabase.go3. Sneak's Feedback Verification (All 4 Rework Cycles)
ensureBootstrapVersionRecorded, noapplyBootstrapMigration, no legacy backfill.TestBootstrapMigrationsTable_ExistingTableBackwardsCompatdeleted.000.sqlcontains bothCREATE TABLE IF NOT EXISTSANDINSERT OR IGNORE INTO schema_migrations (version) VALUES (0). Go code does zero INSERTs for bootstrap.000.sql:version INTEGER PRIMARY KEY✅000.sql:VALUES (0)(integer literal, not string) ✅bootstrapVersionconst:0(int) ✅ParseMigrationVersion()returns(int, error)✅strconv.Atoiconverts filename prefix to int ✅intversion parameter via?placeholder ✅intvalues (0,1,42) not strings ✅%q/%sto%din test error messages ✅All rework instructions fully addressed.
4. Code Correctness
000.sql:CREATE TABLE IF NOT EXISTS+INSERT OR IGNORE— both idempotent, self-contained.version INTEGER PRIMARY KEY. ✅bootstrapMigrationsTable:sqlite_master→ table missing → reads + executes000.sql→ done. Table present → returns immediately. No Go-side INSERT. Clean, minimal. ✅ParseMigrationVersion:strconv.Atoiconverts to int. Returns0on all error paths (idiomatic Go). ✅Migration loop interaction:
collectMigrationsreturns all.sqlfiles including000.sql0already recorded → loop skips000.sql→ no double-INSERT risk ✅intconsistently ✅Idempotency:
bootstrapMigrationsTabletwice: second call sees table → returns ✅ApplyMigrationstwice: all versions recorded → all skipped ✅5. Test Coverage
TestParseMigrationVersion"001"→1,"042"→42,"00001"→1. Error cases preserved.TestApplyMigrations_CreatesSchemaAndTablesint, verifies0and1recorded, all 6 application tables verifiedTestApplyMigrations_Idempotent0(queried as int)TestBootstrapMigrationsTable_FreshDatabase0recorded (queried as int)All tests use
inttypes for version comparisons and format verbs. Backwards-compat test correctly removed per earlier rework. ✅6. Cheating Check
Only 3 files in diff:
database.go,database_test.go,000.sql. No modifications to.golangci.yml,Makefile,Dockerfile,.gitea/workflows/, or any test configuration. Test helperopenTestDBis a new addition (DRY refactor) — not a policy-controlled file. ✅7. Build Result
docker build --no-cache .— PASSED ✅All stages green:
make fmt-check,make lint,make test(all tests pass),go build.Verdict: PASS ✅
Clean implementation.
version INTEGER PRIMARY KEYthroughout — SQL schema, Go const,ParseMigrationVersionreturn type, all queries, all tests.000.sqlis fully self-contained with DDL + version row. No backwards-compat code. No Go-side INSERTs for bootstrap. All four rounds of sneak's feedback addressed. Build is green.please search all other repos for this migrations pattern and open issues in any of those repos if they don’t follow this exact pattern (migration table schema in 000, etc).
Audited all repos for the migrations pattern. Found 6 repos that create
schema_migrationsinline in Go code (not in000.sql) and/or use TEXT instead of INTEGER for version. Opened issues:Repos checked that don't use SQL migrations: netwatch (JS/TS), secret (no migrations dir).