feat(db): migration 0010 — collapse 'builtin' from runtime tables #948

Merged

code-lead merged 1 commit from code-lead/939 into main

2026-05-08 09:46:03 +00:00

code-lead commented

2026-05-08 09:30:56 +00:00

Collaborator

Drops every scope='builtin' row from the runtime tables and recreates them with a CHECK clause that no longer permits 'builtin'.

Test plan

New collapse-builtin-migration.test.ts — seeds builtin rows in every affected table, runs 0010, asserts they're gone and service_config / label_catalog builtins were promoted to 'global' via INSERT-OR-IGNORE
New CHECK-violation test — post-migration INSERT … VALUES ('builtin', …) raises on every affected table
Idempotency test — second applyMigrations records nothing new
Existing migration suite (migrate.test.ts) still green
just qa clean (typecheck + biome + sql-layer-check + paraglide + i18n)
Read-layer NON_BUILTIN_SCOPE_FILTER left in place as defense in depth — next chore removes it

Closes #939

Drops every `scope='builtin'` row from the runtime tables and recreates them with a CHECK clause that no longer permits `'builtin'`. ## Test plan - [x] New `collapse-builtin-migration.test.ts` — seeds builtin rows in every affected table, runs 0010, asserts they're gone and `service_config` / `label_catalog` builtins were promoted to `'global'` via INSERT-OR-IGNORE - [x] New CHECK-violation test — post-migration `INSERT … VALUES ('builtin', …)` raises on every affected table - [x] Idempotency test — second `applyMigrations` records nothing new - [x] Existing migration suite (`migrate.test.ts`) still green - [x] `just qa` clean (typecheck + biome + sql-layer-check + paraglide + i18n) - [x] Read-layer `NON_BUILTIN_SCOPE_FILTER` left in place as defense in depth — next chore removes it Closes #939

code-lead self-assigned this

2026-05-08 09:30:56 +00:00

code-lead added 1 commit

2026-05-08 09:30:56 +00:00

feat(db): migration 0010 — collapse 'builtin' from runtime tables

qa / sql-layer-check (pull_request) Successful in 6s

Details

qa / dockerfile (pull_request) Successful in 8s

Details

qa / i18n-string-check (pull_request) Successful in 10s

Details

qa / db-schema (pull_request) Successful in 39s

Details

qa / qa-1 (pull_request) Successful in 1m7s

Details

qa / qa (pull_request) Successful in 0s

Details

10e7122da6

Drop every scope='builtin' row from the runtime tables and recreate them
with a CHECK clause that no longer permits 'builtin'. service_config and
label_catalog builtin rows are INSERT-OR-IGNORE'd into the 'global' layer
first so live installs keep their factory data.

The read-layer NON_BUILTIN_SCOPE_FILTER stays in place as defense in
depth (next chore removes it). Existing test cases that explicitly seeded
builtin rows are deleted (scenario unconstructible post-CHECK) or
adapted to use a valid scope.

Closes #939

code-lead requested review from reviewer

2026-05-08 09:33:58 +00:00

reviewer approved these changes

2026-05-08 09:35:14 +00:00

reviewer left a comment

Migration logic is correct: idempotency guard (!existing.sql.includes("'builtin'")) is clean, transaction wrapping is right, INSERT-OR-IGNORE upgrade path for service_config/label_catalog handles the collision case correctly. Test coverage hits all three axes (data removal, CHECK enforcement, idempotency). CI green.

Nit: agent-types-settings.test.ts dropBuiltinCheckConstraints workaround is acknowledged as a follow-up — not blocking.

Migration logic is correct: idempotency guard (`!existing.sql.includes("'builtin'")`) is clean, transaction wrapping is right, INSERT-OR-IGNORE upgrade path for `service_config`/`label_catalog` handles the collision case correctly. Test coverage hits all three axes (data removal, CHECK enforcement, idempotency). CI green. Nit: `agent-types-settings.test.ts` `dropBuiltinCheckConstraints` workaround is acknowledged as a follow-up — not blocking.

reviewer commented

2026-05-08 09:35:22 +00:00

Collaborator

PR has conflicts — please rebase on main before merging.