From bead2eb4961a673df504e95ca510f6033a28fe3d Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Mon, 14 Sep 2026 20:43:06 -0400 Subject: [PATCH 1/2] feat(db): guarantee public.uuidv7() exists after nself start Requested by ummeco/ummat (inbox msg-2026-09-14-uuidv7-extension, high). Their migration 20260511104612 runs `CREATE EXTENSION IF NOT EXISTS pg_uuidv7` and SOFT-FAILS by design: the base image does not ship it, so the migration warns and is still recorded as applied. Its own header then tells later DDL to use `id UUID PRIMARY KEY DEFAULT public.uuidv7()`. Two later migrations did exactly that and died with "function public.uuidv7() does not exist". A soft-failing install that later DDL trusts is worse than a hard failure. The ask was to add pg_uuidv7 to the base PG16 image. Not doing that, for two concrete reasons: - It would mean nSelf building, publishing and maintaining its own Postgres image instead of using pgvector/pgvector:pg16. That is a distribution change (Docker Hub is one of the eight bus-factor accounts) and an owner decision, not a code fix. - It would not work at all on the embedded pglite runtime, which cannot load a C extension. That path would still lack the function. Instead, guarantee the POST-CONDITION the consumers actually need: after InitializeDatabase, public.uuidv7() exists. ensureUUIDv7 tries CREATE EXTENSION first, because a C implementation is better when present, tolerates its absence, and then applies a guarded SQL definition that no-ops if anything already provides the function. An image that does ship pg_uuidv7 keeps the C version. The SQL body is the standard community implementation, taken from ummat's own verified fallback rather than written fresh: overlay a v4 UUID's first 6 bytes with the big-endian millisecond timestamp, then set bits 52 and 53 to turn the version nibble from 0100 into 0111. gen_random_uuid() has already set the RFC 9562 variant bits and the overlay does not touch them. Verified live against postgres:16-alpine, which has no pg_uuidv7, so these exercise the fallback path and not a C implementation: --- PASS: TestEnsureUUIDv7_Integration_FallbackDefinesAConformingFunction --- PASS: TestEnsureUUIDv7_Integration_IsIdempotent --- PASS: TestEnsureUUIDv7_Integration_DoesNotReplaceAnExistingDefinition ok github.com/nself-org/cli/internal/database 6.379s The first pins what consumers depend on rather than merely that a function exists: version nibble 7, variant bits in 8/9/a/b, the leading 48 bits decoding to within 60s of now, strict ordering across milliseconds, and 200 distinct values in one batch. A wrong implementation still returns a uuid, so "it exists" would not have been a test. --- internal/database/init.go | 85 ++++++++++ internal/database/uuidv7_integration_test.go | 160 +++++++++++++++++++ 2 files changed, 245 insertions(+) create mode 100644 internal/database/uuidv7_integration_test.go diff --git a/internal/database/init.go b/internal/database/init.go index 427b8437..dc424343 100644 --- a/internal/database/init.go +++ b/internal/database/init.go @@ -206,6 +206,86 @@ func createExtensions(ctx context.Context, cfg *config.Config) error { return nil } +// uuidV7FallbackSQL defines public.uuidv7() only when nothing already provides +// it, so an image that ships the pg_uuidv7 extension keeps the C implementation +// and this is a no-op. +// +// The body is the standard community implementation, not a bespoke one: take a +// v4 UUID, overlay the first 6 bytes with the big-endian millisecond timestamp, +// then set bits 52 and 53 to turn the version nibble 0100 (v4) into 0111 (v7). +// gen_random_uuid() has already set the RFC 9562 variant bits, which the overlay +// does not touch. +const uuidV7FallbackSQL = ` +DO $$ +BEGIN + IF NOT EXISTS ( + SELECT 1 FROM pg_proc p + JOIN pg_namespace n ON n.oid = p.pronamespace + WHERE p.proname = 'uuidv7' AND n.nspname = 'public' + ) THEN + EXECUTE $fn$ + CREATE FUNCTION public.uuidv7() RETURNS uuid + LANGUAGE sql VOLATILE + AS $body$ + SELECT encode( + set_bit( + set_bit( + overlay( + uuid_send(gen_random_uuid()) + PLACING substring( + int8send(floor(extract(epoch FROM clock_timestamp()) * 1000)::bigint) + FROM 3 + ) + FROM 1 FOR 6 + ), + 52, 1 + ), + 53, 1 + ), + 'hex' + )::uuid + $body$; + $fn$; + END IF; +END $$;` + +// ensureUUIDv7 guarantees that public.uuidv7() exists after initialization. +// +// Why this is not just another entry in createExtensions' list: the base image +// (pgvector/pgvector:pg16) does not ship pg_uuidv7, so `CREATE EXTENSION +// pg_uuidv7` errors and would abort startup for every user. The embedded pglite +// runtime cannot load a C extension at all. +// +// So: try the extension first, because a C implementation is better when it is +// there, and tolerate its absence. Then install the SQL fallback, which no-ops +// when the extension already defined the function. Either way the post-condition +// is the same and is strictly stronger than before — public.uuidv7() exists. +// +// This closes a real trap reported by ummeco/ummat: a migration ran +// `CREATE EXTENSION IF NOT EXISTS pg_uuidv7`, soft-failed with a WARNING, was +// still recorded as applied, and its own header told later DDL to use +// `DEFAULT public.uuidv7()`. Two later migrations then died with +// "function public.uuidv7() does not exist". +func ensureUUIDv7(ctx context.Context, cfg *config.Config) error { + db := cfg.Postgres.DB + if db == "" { + db = "nself" + } + + // Best effort: present in a custom image, absent in the stock one. + if err := runSQLOnDB(ctx, cfg, db, "CREATE EXTENSION IF NOT EXISTS pg_uuidv7"); err != nil { + slog.Debug("pg_uuidv7 extension unavailable, using SQL fallback", "err", err) + } + + // Not best effort: after this, the function must exist. + if err := runSQLOnDB(ctx, cfg, db, uuidV7FallbackSQL); err != nil { + return fmt.Errorf("ensure public.uuidv7(): %w", err) + } + + slog.Info("public.uuidv7() available") + return nil +} + // InitializeDatabase waits for PostgreSQL to become ready, then creates the // database, schemas, grants, and required extensions. This is Phase 3 of the // nself startup sequence. @@ -217,6 +297,7 @@ func createExtensions(ctx context.Context, cfg *config.Config) error { // 3. CREATE SCHEMA IF NOT EXISTS auth, storage, public // 4. GRANT ALL ON SCHEMA auth, storage, public TO user // 5. CREATE EXTENSION IF NOT EXISTS pgcrypto, citext +// 6. Guarantee public.uuidv7() exists (extension when available, SQL otherwise) func InitializeDatabase(ctx context.Context, cfg *config.Config) error { slog.Info("initializing database") @@ -251,6 +332,10 @@ func InitializeDatabase(ctx context.Context, cfg *config.Config) error { return err } + if err := ensureUUIDv7(ctx, cfg); err != nil { + return err + } + slog.Info("database initialization complete") return nil } diff --git a/internal/database/uuidv7_integration_test.go b/internal/database/uuidv7_integration_test.go new file mode 100644 index 00000000..644eb983 --- /dev/null +++ b/internal/database/uuidv7_integration_test.go @@ -0,0 +1,160 @@ +//go:build integration + +// Live verification that ensureUUIDv7 leaves public.uuidv7() usable against a +// REAL Postgres that does not ship the pg_uuidv7 extension. +// +// postgres:16-alpine (what startTestPostgres boots) has no pg_uuidv7, which is +// exactly the case this exists to cover: the stock nSelf image +// (pgvector/pgvector:pg16) does not have it either. So the CREATE EXTENSION +// attempt inside ensureUUIDv7 fails here and the SQL fallback has to carry it. +// +// Asserting "the function exists" would be far too weak. A wrong implementation +// still defines a function and still returns a uuid. These tests pin the +// properties consumers actually depend on: RFC 9562 version 7, the variant bits, +// a timestamp that decodes to now, and strict ordering across milliseconds — +// the ordering is the whole reason callers chose uuidv7 over uuid_generate_v4 +// for keyset-pagination cursor indexes. +package database + +import ( + "bytes" + "context" + "os/exec" + "strconv" + "strings" + "testing" + "time" +) + +// queryScalar runs a single-value query inside the test container. +func queryScalar(t *testing.T, cfg *configForQuery, sql string) string { + t.Helper() + var stdout, stderr bytes.Buffer + cmd := exec.Command("docker", "exec", cfg.container, + "psql", "-U", "postgres", "-d", "nself", "-tAc", sql) + cmd.Stdout = &stdout + cmd.Stderr = &stderr + if err := cmd.Run(); err != nil { + t.Fatalf("query %q failed: %v\n%s", sql, err, stderr.String()) + } + return strings.TrimSpace(stdout.String()) +} + +type configForQuery struct{ container string } + +func TestEnsureUUIDv7_Integration_FallbackDefinesAConformingFunction(t *testing.T) { + skipUnlessIntegration(t) + cfg := startTestPostgres(t) + q := &configForQuery{container: cfg.ProjectName + "_postgres"} + + // Precondition: the extension really is absent here, so we are proving the + // fallback path and not accidentally testing a C implementation. + if got := queryScalar(t, q, + "SELECT count(*) FROM pg_available_extensions WHERE name = 'pg_uuidv7'"); got != "0" { + t.Fatalf("precondition: expected pg_uuidv7 to be unavailable in postgres:16-alpine, got count=%s", got) + } + + if err := ensureUUIDv7(context.Background(), cfg); err != nil { + t.Fatalf("ensureUUIDv7: %v", err) + } + + if got := queryScalar(t, q, `SELECT count(*) FROM pg_proc p + JOIN pg_namespace n ON n.oid = p.pronamespace + WHERE p.proname = 'uuidv7' AND n.nspname = 'public'`); got != "1" { + t.Fatalf("expected exactly one public.uuidv7(), got %s", got) + } + + // Version nibble must be 7 (RFC 9562 §5.7). Character 15 of the canonical + // text form, 1-indexed, is the version. + if got := queryScalar(t, q, "SELECT substring(public.uuidv7()::text FROM 15 FOR 1)"); got != "7" { + t.Errorf("version nibble = %q, want \"7\"", got) + } + + // Variant bits must be 10xx, i.e. the first character of the 4th group is + // one of 8, 9, a, b. gen_random_uuid() sets these and the overlay must not + // have disturbed them. + if got := queryScalar(t, q, "SELECT substring(public.uuidv7()::text FROM 20 FOR 1)"); !strings.Contains("89ab", got) { + t.Errorf("variant nibble = %q, want one of 8/9/a/b", got) + } + + // The leading 48 bits must decode to a timestamp at about now, which is what + // makes these sortable. Allow a generous window; we are catching a wrong + // field order or unit, not clock skew. + skewRaw := queryScalar(t, q, ` + SELECT abs(extract(epoch FROM now()) - + (('x' || substring(public.uuidv7()::text FROM 1 FOR 8))::bit(32)::bigint * 65536 + + ('x' || substring(public.uuidv7()::text FROM 10 FOR 4))::bit(16)::bigint) / 1000.0)::int`) + if skewRaw == "" { + t.Fatal("timestamp decode returned nothing") + } + skewSec, err := strconv.Atoi(skewRaw) + if err != nil { + t.Fatalf("timestamp decode returned %q, not a number", skewRaw) + } + if skewSec > 60 { + t.Errorf("decoded timestamp is %ds away from now — field order or unit is wrong", skewSec) + } + + // Strict ordering across milliseconds. Sleeping between calls guarantees a + // different millisecond, so a correct implementation is strictly increasing. + first := queryScalar(t, q, "SELECT public.uuidv7()") + time.Sleep(5 * time.Millisecond) + second := queryScalar(t, q, "SELECT public.uuidv7()") + if !(first < second) { + t.Errorf("not time-ordered: %q >= %q", first, second) + } + + // Distinctness within the same millisecond — the random tail must still vary. + if got := queryScalar(t, q, + "SELECT count(DISTINCT public.uuidv7()) FROM generate_series(1, 200)"); got != "200" { + t.Errorf("expected 200 distinct values in one batch, got %s", got) + } +} + +func TestEnsureUUIDv7_Integration_IsIdempotent(t *testing.T) { + skipUnlessIntegration(t) + cfg := startTestPostgres(t) + q := &configForQuery{container: cfg.ProjectName + "_postgres"} + + ctx := context.Background() + for i := 0; i < 3; i++ { + if err := ensureUUIDv7(ctx, cfg); err != nil { + t.Fatalf("ensureUUIDv7 call %d: %v", i+1, err) + } + } + + // Still exactly one definition, and still working. Re-running nself start + // must not accumulate overloads or error on the second boot. + if got := queryScalar(t, q, `SELECT count(*) FROM pg_proc p + JOIN pg_namespace n ON n.oid = p.pronamespace + WHERE p.proname = 'uuidv7' AND n.nspname = 'public'`); got != "1" { + t.Fatalf("after 3 calls expected exactly one public.uuidv7(), got %s", got) + } + if got := queryScalar(t, q, "SELECT substring(public.uuidv7()::text FROM 15 FOR 1)"); got != "7" { + t.Errorf("version nibble after repeat calls = %q, want \"7\"", got) + } +} + +func TestEnsureUUIDv7_Integration_DoesNotReplaceAnExistingDefinition(t *testing.T) { + skipUnlessIntegration(t) + cfg := startTestPostgres(t) + q := &configForQuery{container: cfg.ProjectName + "_postgres"} + + // Stand in for the C extension: a pre-existing public.uuidv7() that returns a + // recognisable constant. The guard must leave it alone, otherwise an image + // that really does ship pg_uuidv7 would silently lose its implementation. + sentinel := "11111111-1111-7111-8111-111111111111" + if out, err := exec.Command("docker", "exec", q.container, "psql", "-U", "postgres", "-d", "nself", + "-c", "CREATE FUNCTION public.uuidv7() RETURNS uuid LANGUAGE sql VOLATILE AS $$ SELECT '"+sentinel+"'::uuid $$", + ).CombinedOutput(); err != nil { + t.Fatalf("seed sentinel uuidv7: %v\n%s", err, out) + } + + if err := ensureUUIDv7(context.Background(), cfg); err != nil { + t.Fatalf("ensureUUIDv7: %v", err) + } + + if got := queryScalar(t, q, "SELECT public.uuidv7()::text"); got != sentinel { + t.Errorf("existing definition was replaced: got %q, want the sentinel %q", got, sentinel) + } +} From bb23aef60c23ea4fe02427da1dab346905744dd9 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Mon, 14 Sep 2026 21:00:49 -0400 Subject: [PATCH 2/2] refactor(db): move the uuidv7 bootstrap into its own file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI caught this, correctly: --- FAIL: TestFileSizeBudgetNotExceeded file_size_test.go:120: 1 files exceed the 300-line cap but the budget is 0. Adding ensureUUIDv7 took internal/database/init.go from 275 to 360 lines, past the 300-line cap the repo enforces. Raising the budget was not an option: the cap is the rule, and a gate that gets relaxed whenever it fires is not a gate. Split instead, which is the better shape anyway — the uuidv7 bootstrap is its own concern, not part of the create-database/schema/extension sequence. init.go 360 -> 280 uuidv7.go 89 (new) Verified after the split: go test ./internal/repoqa/ -run TestFileSizeBudget ok INTEGRATION=1 go test -tags integration -run TestEnsureUUIDv7 ok (6.604s) --- internal/database/init.go | 80 --------------------------------- internal/database/uuidv7.go | 89 +++++++++++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+), 80 deletions(-) create mode 100644 internal/database/uuidv7.go diff --git a/internal/database/init.go b/internal/database/init.go index dc424343..222ecf16 100644 --- a/internal/database/init.go +++ b/internal/database/init.go @@ -206,86 +206,6 @@ func createExtensions(ctx context.Context, cfg *config.Config) error { return nil } -// uuidV7FallbackSQL defines public.uuidv7() only when nothing already provides -// it, so an image that ships the pg_uuidv7 extension keeps the C implementation -// and this is a no-op. -// -// The body is the standard community implementation, not a bespoke one: take a -// v4 UUID, overlay the first 6 bytes with the big-endian millisecond timestamp, -// then set bits 52 and 53 to turn the version nibble 0100 (v4) into 0111 (v7). -// gen_random_uuid() has already set the RFC 9562 variant bits, which the overlay -// does not touch. -const uuidV7FallbackSQL = ` -DO $$ -BEGIN - IF NOT EXISTS ( - SELECT 1 FROM pg_proc p - JOIN pg_namespace n ON n.oid = p.pronamespace - WHERE p.proname = 'uuidv7' AND n.nspname = 'public' - ) THEN - EXECUTE $fn$ - CREATE FUNCTION public.uuidv7() RETURNS uuid - LANGUAGE sql VOLATILE - AS $body$ - SELECT encode( - set_bit( - set_bit( - overlay( - uuid_send(gen_random_uuid()) - PLACING substring( - int8send(floor(extract(epoch FROM clock_timestamp()) * 1000)::bigint) - FROM 3 - ) - FROM 1 FOR 6 - ), - 52, 1 - ), - 53, 1 - ), - 'hex' - )::uuid - $body$; - $fn$; - END IF; -END $$;` - -// ensureUUIDv7 guarantees that public.uuidv7() exists after initialization. -// -// Why this is not just another entry in createExtensions' list: the base image -// (pgvector/pgvector:pg16) does not ship pg_uuidv7, so `CREATE EXTENSION -// pg_uuidv7` errors and would abort startup for every user. The embedded pglite -// runtime cannot load a C extension at all. -// -// So: try the extension first, because a C implementation is better when it is -// there, and tolerate its absence. Then install the SQL fallback, which no-ops -// when the extension already defined the function. Either way the post-condition -// is the same and is strictly stronger than before — public.uuidv7() exists. -// -// This closes a real trap reported by ummeco/ummat: a migration ran -// `CREATE EXTENSION IF NOT EXISTS pg_uuidv7`, soft-failed with a WARNING, was -// still recorded as applied, and its own header told later DDL to use -// `DEFAULT public.uuidv7()`. Two later migrations then died with -// "function public.uuidv7() does not exist". -func ensureUUIDv7(ctx context.Context, cfg *config.Config) error { - db := cfg.Postgres.DB - if db == "" { - db = "nself" - } - - // Best effort: present in a custom image, absent in the stock one. - if err := runSQLOnDB(ctx, cfg, db, "CREATE EXTENSION IF NOT EXISTS pg_uuidv7"); err != nil { - slog.Debug("pg_uuidv7 extension unavailable, using SQL fallback", "err", err) - } - - // Not best effort: after this, the function must exist. - if err := runSQLOnDB(ctx, cfg, db, uuidV7FallbackSQL); err != nil { - return fmt.Errorf("ensure public.uuidv7(): %w", err) - } - - slog.Info("public.uuidv7() available") - return nil -} - // InitializeDatabase waits for PostgreSQL to become ready, then creates the // database, schemas, grants, and required extensions. This is Phase 3 of the // nself startup sequence. diff --git a/internal/database/uuidv7.go b/internal/database/uuidv7.go new file mode 100644 index 00000000..02e63739 --- /dev/null +++ b/internal/database/uuidv7.go @@ -0,0 +1,89 @@ +package database + +import ( + "context" + "fmt" + "log/slog" + + "github.com/nself-org/cli/internal/config" +) + +// uuidV7FallbackSQL defines public.uuidv7() only when nothing already provides +// it, so an image that ships the pg_uuidv7 extension keeps the C implementation +// and this is a no-op. +// +// The body is the standard community implementation, not a bespoke one: take a +// v4 UUID, overlay the first 6 bytes with the big-endian millisecond timestamp, +// then set bits 52 and 53 to turn the version nibble 0100 (v4) into 0111 (v7). +// gen_random_uuid() has already set the RFC 9562 variant bits, which the overlay +// does not touch. +const uuidV7FallbackSQL = ` +DO $$ +BEGIN + IF NOT EXISTS ( + SELECT 1 FROM pg_proc p + JOIN pg_namespace n ON n.oid = p.pronamespace + WHERE p.proname = 'uuidv7' AND n.nspname = 'public' + ) THEN + EXECUTE $fn$ + CREATE FUNCTION public.uuidv7() RETURNS uuid + LANGUAGE sql VOLATILE + AS $body$ + SELECT encode( + set_bit( + set_bit( + overlay( + uuid_send(gen_random_uuid()) + PLACING substring( + int8send(floor(extract(epoch FROM clock_timestamp()) * 1000)::bigint) + FROM 3 + ) + FROM 1 FOR 6 + ), + 52, 1 + ), + 53, 1 + ), + 'hex' + )::uuid + $body$; + $fn$; + END IF; +END $$;` + +// ensureUUIDv7 guarantees that public.uuidv7() exists after initialization. +// +// Why this is not just another entry in createExtensions' list: the base image +// (pgvector/pgvector:pg16) does not ship pg_uuidv7, so `CREATE EXTENSION +// pg_uuidv7` errors and would abort startup for every user. The embedded pglite +// runtime cannot load a C extension at all. +// +// So: try the extension first, because a C implementation is better when it is +// there, and tolerate its absence. Then install the SQL fallback, which no-ops +// when the extension already defined the function. Either way the post-condition +// is the same and is strictly stronger than before — public.uuidv7() exists. +// +// This closes a real trap reported by ummeco/ummat: a migration ran +// `CREATE EXTENSION IF NOT EXISTS pg_uuidv7`, soft-failed with a WARNING, was +// still recorded as applied, and its own header told later DDL to use +// `DEFAULT public.uuidv7()`. Two later migrations then died with +// "function public.uuidv7() does not exist". +func ensureUUIDv7(ctx context.Context, cfg *config.Config) error { + db := cfg.Postgres.DB + if db == "" { + db = "nself" + } + + // Best effort: present in a custom image, absent in the stock one. + if err := runSQLOnDB(ctx, cfg, db, "CREATE EXTENSION IF NOT EXISTS pg_uuidv7"); err != nil { + slog.Debug("pg_uuidv7 extension unavailable, using SQL fallback", "err", err) + } + + // Not best effort: after this, the function must exist. + if err := runSQLOnDB(ctx, cfg, db, uuidV7FallbackSQL); err != nil { + return fmt.Errorf("ensure public.uuidv7(): %w", err) + } + + slog.Info("public.uuidv7() available") + return nil +}