amadan
idear: unexport Models(), pin Pending's expiry boundary (fix round 1)
Review found a Critical: exporting Models() for an app to add to its own Models list makes rastrillo migration check permanently red, since migrate/dump.Compute diffs an app's own Schema against that same app's own Models as a matched pair — idear's Schema only ever belongs in BootSchema, never the app's Schema, so idear's models must never join the app's Models either. No core subsystem (sessions, auth, blobs, passkey) exports one, for the same reason. Unexported it (models()), moved its test into an internal test file since external tests can no longer reach it, and rewrote Schema's doc comment to state the rule positively. Also pins Pending's exact expiry boundary (ExpiresAt == now is not pending, matching the CAS SQL's "expires_at > ?" Task 3 will write) with its own independent test, alongside the existing accepted/revoked/expired-in-the-past/pending cases. gofmt -l ., go vet ./..., go test ./... -count=1 all clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
4 files changed,
+59
−24
member_test.go+17 −0schema.go+22 −9schema_internal_test.go+20 −0schema_test.go+0 −15
diff --git a/member_test.go b/member_test.go| index 83091aa..ceb4453 100644 |
| --- a/member_test.go |
| +++ b/member_test.go |
| @@ -80,6 +80,23 @@ func TestInvitation_Pending_Expired(t *testing.T) { |
| } |
| } |
| +// TestInvitation_Pending_ExpiresAtNow pins the exact boundary: |
| +// ExpiresAt == now is not pending. Pending uses ExpiresAt.After(now), |
| +// so an invitation expiring at exactly now must read as already |
| +// expired — matching the CAS SQL Task 3 writes, "expires_at > ?", |
| +// which also excludes the boundary. Left unpinned, a future edit to |
| +// use !now.After(ExpiresAt) (or similar) would flip this one instant |
| +// without any other test noticing. |
| +func TestInvitation_Pending_ExpiresAtNow(t *testing.T) { |
| + now := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC) |
| + inv := &Invitation{ |
| + ExpiresAt: now, |
| + } |
| + if inv.Pending(now) { |
| + t.Fatalf("Pending(now) = true when ExpiresAt == now exactly, want false") |
| + } |
| +} |
| + |
| // TestInvitation_Pending_True checks the remaining case: unaccepted, |
| // unrevoked, unexpired is pending. |
| func TestInvitation_Pending_True(t *testing.T) { |
diff --git a/schema.go b/schema.go| index 4e94992..7d0f99e 100644 |
| --- a/schema.go |
| +++ b/schema.go |
| @@ -10,16 +10,29 @@ import ( |
| var migrationFS embed.FS |
| // Schema is idear's migration set, following the sessions / auth / |
| -// blobs convention exactly: an app merges it into its own BootSchema |
| -// — migrate.Merge(sessions.Schema, idear.Schema, Schema) — never into |
| -// its own Schema (see docs/superpowers/specs/2026-08-23-idear-design.md |
| -// §4), so `rastrillo migration check` never proposes dropping tables |
| -// Models does not know about. |
| +// blobs convention exactly. |
| +// |
| +// Merge it into your app's BootSchema — |
| +// migrate.Merge(sessions.Schema, idear.Schema, Schema) — and apply |
| +// that merged set at boot. Never merge it into your app's own Schema, |
| +// and never add idear's model structs to your app's own Models list: |
| +// rastrillo's `migration check`/`generate` diff one app's Schema |
| +// against that same app's Models as a matched pair, so mixing idear's |
| +// models into your app's list compares them against a Schema that has |
| +// no idear migrations in it. That makes `migration check` permanently |
| +// red and makes `generate` write a second, GORM-flavoured |
| +// `CREATE TABLE idear_members` into your app's own migration file — |
| +// one that collides at boot with idear's own |
| +// `CREATE TABLE IF NOT EXISTS idear_members` from BootSchema. idear's |
| +// Schema and its model structs are a pair scoped to idear alone; no |
| +// core subsystem (sessions, auth, blobs, passkey) exports its models |
| +// for the same reason. |
| var Schema = migrate.MustFromFS(migrationFS, "idear") |
| -// Models is what an app hands rastrillo's migration checker alongside |
| -// its own models, so `rastrillo migration generate`/`check` can see |
| -// idear's tables too. |
| -func Models() []any { |
| +// models is idear's own model list, used only by idear's tests to |
| +// check Schema and the struct tags agree. It is deliberately |
| +// unexported — see Schema's doc comment for why an app must never be |
| +// handed this list. |
| +func models() []any { |
| return []any{&Member{}, &Invitation{}} |
| } |
diff --git a/schema_internal_test.go b/schema_internal_test.go| new file mode 100644 |
| index 0000000..dd201bc |
| --- /dev/null |
| +++ b/schema_internal_test.go |
| @@ -0,0 +1,20 @@ |
| +package idear |
| + |
| +import "testing" |
| + |
| +// TestModels checks that models (unexported — see Schema's doc |
| +// comment for why an app must never be handed this list) returns |
| +// pointers to both idear model types, in the shape idear's own tests |
| +// use to check Schema and the struct tags agree. |
| +func TestModels(t *testing.T) { |
| + got := models() |
| + if len(got) != 2 { |
| + t.Fatalf("len(models()) = %d, want 2", len(got)) |
| + } |
| + if _, ok := got[0].(*Member); !ok { |
| + t.Errorf("models()[0] = %T, want *Member", got[0]) |
| + } |
| + if _, ok := got[1].(*Invitation); !ok { |
| + t.Errorf("models()[1] = %T, want *Invitation", got[1]) |
| + } |
| +} |
diff --git a/schema_test.go b/schema_test.go| index 67df2c2..6772ca6 100644 |
| --- a/schema_test.go |
| +++ b/schema_test.go |
| @@ -84,21 +84,6 @@ func TestSchema_Apply_Twice(t *testing.T) { |
| } |
| } |
| -// TestModels checks that Models returns pointers to both idear model |
| -// types, in the shape an app hands to rastrillo's migration checker. |
| -func TestModels(t *testing.T) { |
| - models := idear.Models() |
| - if len(models) != 2 { |
| - t.Fatalf("len(Models()) = %d, want 2", len(models)) |
| - } |
| - if _, ok := models[0].(*idear.Member); !ok { |
| - t.Errorf("Models()[0] = %T, want *idear.Member", models[0]) |
| - } |
| - if _, ok := models[1].(*idear.Invitation); !ok { |
| - t.Errorf("Models()[1] = %T, want *idear.Invitation", models[1]) |
| - } |
| -} |
| - |
| // frozenIdearChecksum is migrate.Checksum of idear's shipped |
| // migrations/0001_init.sql, recorded the day it shipped. |
| // |