oficina / sheets Public

Wire migrate.Apply into boot and every test helper; add the CI gate

cmd/sheets/main.go now runs migrate.Apply(ctx, d, sheets.BootSchema)
right after opening the database, before anything else touches it —
a real instance migrates through the ledgered path at boot instead of
never migrating anything at all.

Every AutoMigrate call in the test suite (internal/sheets/store's
helpers_test.go, handle_test.go, handle_grid_test.go,
handle_more_test.go, handle_gate_test.go, handle_blank_cap_test.go,
handle_blank_crash_test.go; internal/sheets/xlsx's helpers_test.go and
plaintext_test.go) is replaced with the same migrate.Apply(ctx, d,
sheets.BootSchema) call production boots with. This was the actual
point of the exercise: a test schema built by AutoMigrate and a
production schema built by migrate.Apply are two definitions of one
schema, and tests built on AutoMigrate would never have exercised what
production actually runs — the additive-only constraint AGENTS.md
states in prose had no mechanism behind it, and no test would have
caught a migration that silently diverged from the models. Several
call sites had their own redundant per-test AutoMigrate calls layered
on top of the shared helper's; those are just deleted, since
openTestDB's migrate.Apply(BootSchema) already creates every table.

Added the CI gate: a test-migrations Makefile target running
`rastrillo migration check` (no database, just Models vs Schema),
positioned right after test-build in both the aggregate `test` target
and .amadan/ci.d/35-migrations (three lines, exec make
test-migrations, matching the other ci.d steps' shape) — early, since
it needs nothing but a compiling app and is cheap next to test-go's
32s or test-race's 65s.

make test is green end to end, gate included, in the new six-step
order.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01APtNStXDZ5H2fAc1ZuGqtS

Paul Campbell pushed by paul@keymail.dev 1 month ago 972839cfca399b1775fa87e7b4534a04d9933220 parent d49ed01

14 files changed, +77 −68

.amadan/ci.d/35-migrations +3 −0
Makefile +20 −7
cmd/sheets/main.go +18 −3
internal/sheets/store/events.go +1 −1
internal/sheets/store/handle_blank_cap_test.go +3 −4
internal/sheets/store/handle_blank_crash_test.go +3 −4
internal/sheets/store/handle_gate_test.go +0 −6
internal/sheets/store/handle_grid_test.go +0 −6
internal/sheets/store/handle_more_test.go +0 −5
internal/sheets/store/handle_test.go +6 −14
internal/sheets/store/helpers_test.go +13 −6
internal/sheets/store/wal_gate_test.go +1 −1
internal/sheets/xlsx/helpers_test.go +6 −6
internal/sheets/xlsx/plaintext_test.go +3 −5

Intent

full ledger: core

diff --git a/.amadan/ci.d/35-migrations b/.amadan/ci.d/35-migrations

new file mode 100755

index 0000000..44565b6

--- /dev/null

+++ b/.amadan/ci.d/35-migrations

@@ -0,0 +1,3 @@

+#!/bin/sh

+set -eu

+exec make test-migrations

diff --git a/Makefile b/Makefile

index b23b5ee..b695aad 100644

--- a/Makefile

+++ b/Makefile

@@ -1,4 +1,4 @@

-.PHONY: build test test-vet test-fmt test-build test-go test-race

+.PHONY: build test test-vet test-fmt test-build test-migrations test-go test-race

# One binary, CGO_ENABLED=0, cgo-free SQLite (AGENTS.md / Deploys). Set

# per-target below rather than exported globally, because test-race is

@@ -8,15 +8,16 @@ build:

# The gate, split into named targets so CI can report them individually

# (.amadan/ci.d calls these one at a time, each its own step). `make

-# test` runs exactly the same five things in the same order, so the

+# test` runs exactly the same six things in the same order, so the

# local gate and CI stay ONE definition rather than two lists that

# drift apart. Debugging CI is running `make test`.

# Order is cheapest-and-fastest first: vet and gofmt are near-instant,

-# build catches compile errors before minutes are spent on tests, plain

-# `go test` (tens of seconds, dominated by the store package) comes

-# next, and -race — by far the slowest step — runs last.

-test: test-vet test-fmt test-build test-go test-race

+# build catches compile errors before minutes are spent on tests,

+# test-migrations needs the app to compile but no database and no test

+# suite, plain `go test` (tens of seconds, dominated by the store

+# package) comes next, and -race — by far the slowest step — runs last.

+test: test-vet test-fmt test-build test-migrations test-go test-race

test-vet:

CGO_ENABLED=0 go vet ./...

@@ -34,7 +35,19 @@ test-fmt:

test-build:

CGO_ENABLED=0 go build ./...

-# Includes internal/guards: the mechanical guards for the line cap,

+# CI gate for AGENTS.md's additive-only migrations rule: models

+# (internal/sheets/migrations.go's Models) and migrations

+# (internal/sheets/migrations/*.sql, via Schema) must already agree.

+# Touches no database — it diffs structs against committed SQL, the

+# same computation `rastrillo migration generate` does before deciding

+# there is nothing to write — so it belongs this early, right after the

+# app is known to compile and well before test-go pays for real SQLite

+# files. A model changed without `rastrillo migration generate` run

+# after it fails here, not in production at the next boot.

+test-migrations:

+ rastrillo migration check

+# Includes internal/guardstest: the mechanical guards for the line cap,

# forbidden imports, excelize confinement, calc isolation and sealtest

# containment all run here as ordinary tests, plus internal/sheets/seal

# and internal/sheets/xlsx's plaintext-leak checks (AGENTS.md / "The one

diff --git a/cmd/sheets/main.go b/cmd/sheets/main.go

index 696ace1..0637c5d 100644

--- a/cmd/sheets/main.go

+++ b/cmd/sheets/main.go

@@ -10,23 +10,27 @@

// SIGTERM drain, baseline security headers. This file only wires the

// app's own *gorm.DB handle (via rastrillo's db package) into

// sheets.New, because the app needs GORM, not the raw *sql.DB

-// rastrillo.Options.Router would hand back.

+// rastrillo.Options.Router would hand back. It also runs migrate.Apply

+// against sheets.BootSchema before anything else touches the database —

+// see internal/sheets/migrations.go.

-// rastrillo.Resolve/Run's shape at v0.20.0 differs from a zero-argument

+// rastrillo.Resolve/Run's shape at v0.21.0 differs from a zero-argument

// Resolve()/Run(handler) pair: Resolve takes and returns an Options, and

// an app that opens its own database handle (as this one does, for

// gorm) uses Resolve+Serve directly rather than Run — see

-// $(go env GOMODCACHE)/github.com/carlosframework/rastrillo@v0.20.0/SKILL.md

+// $(go env GOMODCACHE)/github.com/carlosframework/rastrillo@v0.21.0/SKILL.md

// and run.go's doc comment on Resolve.

package main

import (

+ "context"

"log/slog"

"net/http"

"os"

"github.com/carlosframework/rastrillo"

rdb "github.com/carlosframework/rastrillo/db"

+ "github.com/carlosframework/rastrillo/migrate"

"amadan.net/oficina/sheets/internal/sheets"

)

@@ -47,6 +51,17 @@ func main() {

}

defer d.Close()

+ // Ledgered, once-at-boot, never re-run or reversed — see

+ // internal/sheets/migrations.go's Schema/BootSchema/Models and

+ // AGENTS.md's additive-only constraint. A boot that can't reconcile

+ // its schema (a structural diff migrate.Apply doesn't recognise as

+ // its own pending migrations) refuses rather than guessing, which

+ // is why this is a hard exit, same as a failed db.Open above.

+ if _, err := migrate.Apply(context.Background(), d, sheets.BootSchema); err != nil {

+ logger.Error("migrate", "err", err)

+ os.Exit(1)

+ }

app := sheets.New(d.G)

// rastrillo.Options.Mux wants a *http.ServeMux; the app's own

diff --git a/internal/sheets/store/events.go b/internal/sheets/store/events.go

index bce4af5..39ac41b 100644

--- a/internal/sheets/store/events.go

+++ b/internal/sheets/store/events.go

@@ -25,7 +25,7 @@ const (

// type's doc comment in models.go for why (migrations.go's Models list

// has to see it, and package sheets cannot import this package back).

// This alias means every existing call site — store.Event{...},

-// &store.Event{}, AutoMigrate(&store.Event{}) in tests — keeps compiling

+// &store.Event{}, db.Where(...).First(&store.Event{}) — keeps compiling

// unchanged; a type alias makes store.Event and sheets.Event the exact

// same type, not merely convertible ones.

type Event = sheets.Event

diff --git a/internal/sheets/store/handle_blank_cap_test.go b/internal/sheets/store/handle_blank_cap_test.go

index bec5998..faf837c 100644

--- a/internal/sheets/store/handle_blank_cap_test.go

+++ b/internal/sheets/store/handle_blank_cap_test.go

@@ -1,10 +1,12 @@

package store

import (

+ "context"

"path/filepath"

"testing"

rdb "github.com/carlosframework/rastrillo/db"

+ "github.com/carlosframework/rastrillo/migrate"

"amadan.net/oficina/sheets/internal/sheets"

"amadan.net/oficina/sheets/internal/sheets/calc"

@@ -45,10 +47,7 @@ func TestBlankFeederOverCapDependentsAreRecoverable(t *testing.T) {

}

})

db := d.G

- if err := db.AutoMigrate(

- &sheets.Sheet{}, &sheets.Tab{}, &sheets.Row{}, &sheets.Col{},

- &sheets.Cell{}, &sheets.Format{}, &Event{},

- ); err != nil {

+ if _, err := migrate.Apply(context.Background(), d, sheets.BootSchema); err != nil {

t.Fatalf("migrate: %v", err)

}

s, err := seal.New(make([]byte, 32))

diff --git a/internal/sheets/store/handle_blank_crash_test.go b/internal/sheets/store/handle_blank_crash_test.go

index dbbe519..47218a9 100644

--- a/internal/sheets/store/handle_blank_crash_test.go

+++ b/internal/sheets/store/handle_blank_crash_test.go

@@ -1,11 +1,13 @@

package store

import (

+ "context"

"errors"

"path/filepath"

"testing"

rdb "github.com/carlosframework/rastrillo/db"

+ "github.com/carlosframework/rastrillo/migrate"

"amadan.net/oficina/sheets/internal/sheets"

"amadan.net/oficina/sheets/internal/sheets/calc"

@@ -40,10 +42,7 @@ func TestBlankCrashLeavesDependentsRecoverable(t *testing.T) {

}

})

db := d.G

- if err := db.AutoMigrate(

- &sheets.Sheet{}, &sheets.Tab{}, &sheets.Row{}, &sheets.Col{},

- &sheets.Cell{}, &sheets.Format{}, &Event{},

- ); err != nil {

+ if _, err := migrate.Apply(context.Background(), d, sheets.BootSchema); err != nil {

t.Fatalf("migrate: %v", err)

}

s, err := seal.New(make([]byte, 32))

diff --git a/internal/sheets/store/handle_gate_test.go b/internal/sheets/store/handle_gate_test.go

index 03fbca2..86355ba 100644

--- a/internal/sheets/store/handle_gate_test.go

+++ b/internal/sheets/store/handle_gate_test.go

@@ -24,12 +24,6 @@ func TestSetDoesNotLeakPlaintext(t *testing.T) {

dir := t.TempDir()

dbPath := filepath.Join(dir, "test.db")

db := openTestDB(t, dbPath)

- if err := db.AutoMigrate(

- &sheets.Sheet{}, &sheets.Tab{}, &sheets.Row{}, &sheets.Col{},

- &sheets.Cell{}, &sheets.Format{},

- ); err != nil {

- t.Fatalf("migrate: %v", err)

- }

s, err := seal.New(make32())

if err != nil {

t.Fatal(err)

diff --git a/internal/sheets/store/handle_grid_test.go b/internal/sheets/store/handle_grid_test.go

index 2462243..81a119e 100644

--- a/internal/sheets/store/handle_grid_test.go

+++ b/internal/sheets/store/handle_grid_test.go

@@ -20,12 +20,6 @@ func emptyTab(t *testing.T) (*gorm.DB, *seal.Sealer, uint64) {

t.Helper()

dir := t.TempDir()

db := openTestDB(t, filepath.Join(dir, "test.db"))

- if err := db.AutoMigrate(

- &sheets.Sheet{}, &sheets.Tab{}, &sheets.Row{}, &sheets.Col{},

- &sheets.Cell{}, &sheets.Format{},

- ); err != nil {

- t.Fatalf("migrate sheets models: %v", err)

- }

s, err := seal.New(make32())

if err != nil {

t.Fatalf("seal.New: %v", err)

diff --git a/internal/sheets/store/handle_more_test.go b/internal/sheets/store/handle_more_test.go

index 1f663d9..f7d9cf6 100644

--- a/internal/sheets/store/handle_more_test.go

+++ b/internal/sheets/store/handle_more_test.go

@@ -144,11 +144,6 @@ func TestReleaseEvictsAndReopenSeesFreshState(t *testing.T) {

func TestRecoveryDoesNotRegressAConsistentDependentsVersion(t *testing.T) {

dir := t.TempDir()

db := openTestDB(t, filepath.Join(dir, "test.db"))

- if err := db.AutoMigrate(

- &sheets.Sheet{}, &sheets.Tab{}, &sheets.Row{}, &sheets.Col{}, &sheets.Cell{},

- ); err != nil {

- t.Fatalf("migrate: %v", err)

- }

s, err := seal.New(make32())

if err != nil {

t.Fatal(err)

diff --git a/internal/sheets/store/handle_test.go b/internal/sheets/store/handle_test.go

index 42a0217..7fa8c94 100644

--- a/internal/sheets/store/handle_test.go

+++ b/internal/sheets/store/handle_test.go

@@ -24,21 +24,16 @@ const (

)

// seeded builds a real database (through rastrillo's driver, WAL mode —

-// the same reasoning as store/helpers_test.go's openTestDB), migrates

-// every sheets model a handle touches, and seeds one tab with two rows,

-// one column, and a single literal cell at (rowA, colA) holding "5". It

-// returns the db, the sealer, and the tab id; rowA, rowB, colA and actor

-// above are the ids to address it with.

+// the same reasoning as store/helpers_test.go's openTestDB, whose

+// migrate.Apply(sheets.BootSchema) already covers every sheets model a

+// handle touches), and seeds one tab with two rows, one column, and a

+// single literal cell at (rowA, colA) holding "5". It returns the db,

+// the sealer, and the tab id; rowA, rowB, colA and actor above are the

+// ids to address it with.

func seeded(t *testing.T) (*gorm.DB, *seal.Sealer, uint64) {

t.Helper()

dir := t.TempDir()

db := openTestDB(t, filepath.Join(dir, "test.db"))

- if err := db.AutoMigrate(

- &sheets.Sheet{}, &sheets.Tab{}, &sheets.Row{}, &sheets.Col{},

- &sheets.Cell{}, &sheets.Format{},

- ); err != nil {

- t.Fatalf("migrate sheets models: %v", err)

- }

s, err := seal.New(make32())

if err != nil {

@@ -126,9 +121,6 @@ func TestStaleCacheIsRecomputedOnOpen(t *testing.T) {

func TestCellCapIsAnHonestError(t *testing.T) {

dir := t.TempDir()

db := openTestDB(t, filepath.Join(dir, "test.db"))

- if err := db.AutoMigrate(&sheets.Cell{}); err != nil {

- t.Fatalf("migrate: %v", err)

- }

s, err := seal.New(make32())

if err != nil {

t.Fatal(err)

diff --git a/internal/sheets/store/helpers_test.go b/internal/sheets/store/helpers_test.go

index e09d5cb..410d445 100644

--- a/internal/sheets/store/helpers_test.go

+++ b/internal/sheets/store/helpers_test.go

@@ -1,20 +1,27 @@

package store_test

import (

+ "context"

"testing"

rdb "github.com/carlosframework/rastrillo/db"

+ "github.com/carlosframework/rastrillo/migrate"

"gorm.io/gorm"

- "amadan.net/oficina/sheets/internal/sheets/store"

+ "amadan.net/oficina/sheets/internal/sheets"

)

// openTestDB opens a real SQLite database through rastrillo's db package —

// the same cgo-free modernc driver, WAL journal mode and pragma order the

-// app uses at runtime — and migrates the event log table onto it. This is

-// deliberately not sqlite's :memory: mode and not a hand-rolled driver

-// setup: the plaintext gate has to inspect the actual file bytes an actor

-// with bucket access would see, so the test path has to be the real one.

+// app uses at runtime — and applies sheets.BootSchema onto it through the

+// same migrate.Apply call production boots with (cmd/sheets/main.go),

+// rather than GORM's AutoMigrate: a test schema built one way and a

+// production schema built another are two definitions of one schema, and

+// the whole point of BootSchema existing is that there is exactly one.

+// This is deliberately not sqlite's :memory: mode and not a hand-rolled

+// driver setup: the plaintext gate has to inspect the actual file bytes an

+// actor with bucket access would see, so the test path has to be the real

+// one.

func openTestDB(t *testing.T, path string) *gorm.DB {

t.Helper()

d, err := rdb.Open(path, nil)

@@ -26,7 +33,7 @@ func openTestDB(t *testing.T, path string) *gorm.DB {

t.Errorf("close db: %v", err)

}

})

- if err := d.G.AutoMigrate(&store.Event{}); err != nil {

+ if _, err := migrate.Apply(context.Background(), d, sheets.BootSchema); err != nil {

t.Fatalf("migrate: %v", err)

}

return d.G

diff --git a/internal/sheets/store/wal_gate_test.go b/internal/sheets/store/wal_gate_test.go

index e55b95d..78ecae0 100644

--- a/internal/sheets/store/wal_gate_test.go

+++ b/internal/sheets/store/wal_gate_test.go

@@ -160,7 +160,7 @@ func TestSealedWriteIsUnflushedBeforeExplicitCheckpoint(t *testing.T) {

// nonce, so this is not a coincidental match). If any of that is not

// so, the insert above was already folded into the main file by

// something other than an explicit checkpoint — a connection-close

- // checkpoint, pool churn, AutoMigrate side effects, or journal_mode

+ // checkpoint, pool churn, migrate.Apply side effects, or journal_mode

// defaulting to something other than WAL — and the sealed-path test's

// checkpoint() call would be a no-op.

info, err := os.Stat(dbPath + "-wal")

diff --git a/internal/sheets/xlsx/helpers_test.go b/internal/sheets/xlsx/helpers_test.go

index 16d1c88..bd78d6d 100644

--- a/internal/sheets/xlsx/helpers_test.go

+++ b/internal/sheets/xlsx/helpers_test.go

@@ -2,6 +2,7 @@ package xlsx_test

import (

"bytes"

+ "context"

"io"

"os"

"path/filepath"

@@ -9,6 +10,7 @@ import (

"testing"

rdb "github.com/carlosframework/rastrillo/db"

+ "github.com/carlosframework/rastrillo/migrate"

"github.com/xuri/excelize/v2"

"gorm.io/gorm"

@@ -52,8 +54,9 @@ func fixtureNames(t *testing.T) []string {

// openTestDB mirrors internal/sheets/store's own helper of the same

// name (helpers_test.go there) — the real cgo-free modernc driver, WAL

-// mode, the same runtime path — duplicated rather than imported because

-// it lives in that package's _test.go file and is not exported.

+// mode, the same runtime path, and the same migrate.Apply(sheets.

+// BootSchema) production boots with — duplicated rather than imported

+// because it lives in that package's _test.go file and is not exported.

func openTestDB(t *testing.T) *gorm.DB {

t.Helper()

dir := t.TempDir()

@@ -66,10 +69,7 @@ func openTestDB(t *testing.T) *gorm.DB {

t.Errorf("close db: %v", err)

}

})

- if err := d.G.AutoMigrate(

- &sheets.Sheet{}, &sheets.Tab{}, &sheets.Row{}, &sheets.Col{},

- &sheets.Cell{}, &sheets.Format{}, &store.Event{},

- ); err != nil {

+ if _, err := migrate.Apply(context.Background(), d, sheets.BootSchema); err != nil {

t.Fatalf("migrate: %v", err)

}

return d.G

diff --git a/internal/sheets/xlsx/plaintext_test.go b/internal/sheets/xlsx/plaintext_test.go

index 192ba75..33560bf 100644

--- a/internal/sheets/xlsx/plaintext_test.go

+++ b/internal/sheets/xlsx/plaintext_test.go

@@ -2,14 +2,15 @@ package xlsx_test

import (

"bytes"

+ "context"

"path/filepath"

"testing"

rdb "github.com/carlosframework/rastrillo/db"

+ "github.com/carlosframework/rastrillo/migrate"

"amadan.net/oficina/sheets/internal/sheets"

"amadan.net/oficina/sheets/internal/sheets/sealtest"

- "amadan.net/oficina/sheets/internal/sheets/store"

"amadan.net/oficina/sheets/internal/sheets/xlsx"

)

@@ -30,10 +31,7 @@ func TestImportDoesNotLeakPlaintext(t *testing.T) {

t.Fatalf("open db: %v", err)

}

defer d.Close()

- if err := d.G.AutoMigrate(

- &sheets.Sheet{}, &sheets.Tab{}, &sheets.Row{}, &sheets.Col{},

- &sheets.Cell{}, &sheets.Format{}, &store.Event{},

- ); err != nil {

+ if _, err := migrate.Apply(context.Background(), d, sheets.BootSchema); err != nil {

t.Fatalf("migrate: %v", err)

}

sealer := testSealer(t)