From 62629fa9963dea02d25cfb652f1d0692df2353f7 Mon Sep 17 00:00:00 2001 From: Dustin Date: Tue, 1 Sep 2026 14:15:24 +0000 Subject: [PATCH 1/3] fix(contract): let the reviewed god seed grow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit verify-contract.mjs asserted the SMITE god seed contained exactly 88 rows, with the number hard-coded in the script. The catalog grows as SMITE ships gods, and diese-tech/smite-content-sync proposes those additions as reviewed PRs into this repository — so the first real god addition failed check:contract and stayed blocked until someone edited a verification script. Confirmed by adding a row locally: Error: The reviewed local SMITE god seed must contain exactly 88 rows. What the guard is actually for is a seed that silently loses entries, not one that gains them; smite-content-sync already runs its own catalog-size regression guard before exporting. So the assertion becomes a declared floor in contract.json, where the release is declared, rather than a magic number in a script. An exact count caught duplicates only by accident, through the total, so a duplicate-name check now stands on its own — matching case-insensitively and unescaping doubled quotes, since the seed carries names like Chang''e. Verified all four cases against the real seed: unchanged (88) passes, growth to 89 passes, shrinkage to 87 fails naming both numbers, and a duplicate name at full count fails naming the god. --- contract.json | 3 ++- docs/runbooks/consumer-contract.md | 11 ++++++++- scripts/seed-contract.mjs | 14 ++++++++++++ scripts/verify-contract.mjs | 26 ++++++++++++++++++--- test/verification-scripts.test.mjs | 36 ++++++++++++++++++++++++++++++ 5 files changed, 85 insertions(+), 5 deletions(-) diff --git a/contract.json b/contract.json index 0e601d9..3c6fdf5 100644 --- a/contract.json +++ b/contract.json @@ -2,5 +2,6 @@ "version": "db-v1.21.0", "migrationHead": "20260901120000", "supabaseCliVersion": "2.109.1", - "typesSha256": "sha256:4f4564132dcca1d763ae72e08319bfe90b675cbdb940dfdcc730329b2fa6543d" + "typesSha256": "sha256:4f4564132dcca1d763ae72e08319bfe90b675cbdb940dfdcc730329b2fa6543d", + "smiteGodSeedMinimumRows": 88 } diff --git a/docs/runbooks/consumer-contract.md b/docs/runbooks/consumer-contract.md index a8f7dc9..a3282da 100644 --- a/docs/runbooks/consumer-contract.md +++ b/docs/runbooks/consumer-contract.md @@ -7,10 +7,19 @@ The database release stores this repository-owned `contract.json`: "version": "db-v1.0.0", "migrationHead": "<14-digit migration version>", "supabaseCliVersion": "2.109.1", - "typesSha256": "sha256:" + "typesSha256": "sha256:", + "smiteGodSeedMinimumRows": 88 } ``` +`smiteGodSeedMinimumRows` is the floor the reviewed SMITE god seed must meet. +The catalog grows as SMITE ships gods, and `diese-tech/smite-content-sync` +proposes those additions as reviewed pull requests, so the contract asserts the +seed never *shrinks* rather than pinning an exact count that every real addition +would break. A seed that loses rows, or repeats a name under a second id, still +fails closed. Lowering the floor is a deliberate edit here, in the manifest +where the release is declared. + Each consumer commits a separate `db-contract.lock.json` with this shape: ```json diff --git a/scripts/seed-contract.mjs b/scripts/seed-contract.mjs index b305aab..9a9f1f5 100644 --- a/scripts/seed-contract.mjs +++ b/scripts/seed-contract.mjs @@ -4,3 +4,17 @@ export const countSqlSeedRows = (source) => export const usesIdentityPreservingNameUpsert = (source) => /on\s+conflict\s*\(\s*name\s*\)\s+do\s+update\s+set/i.test(source) && !/\bid\s*=\s*excluded\.id\b/i.test(source); + +// A growing seed must still name each entity once. The old exact-row assertion +// caught a duplicate only by accident, through the count; a floor would not. +export const findDuplicateSeedNames = (source) => { + const names = [...source.matchAll(/^\s*\('[^']*',\s*'((?:[^']|'')*)'/gm)] + .map((match) => match[1].replaceAll("''", "'").trim().toLowerCase()); + const seen = new Set(); + const duplicates = new Set(); + for (const name of names) { + if (seen.has(name)) duplicates.add(name); + seen.add(name); + } + return [...duplicates].sort(); +}; diff --git a/scripts/verify-contract.mjs b/scripts/verify-contract.mjs index 7318cfe..e0e99c2 100644 --- a/scripts/verify-contract.mjs +++ b/scripts/verify-contract.mjs @@ -2,7 +2,11 @@ import { createHash } from 'node:crypto'; import { readFileSync, readdirSync } from 'node:fs'; import { fileURLToPath } from 'node:url'; import { readDatabaseMajorVersion } from './supabase-config.mjs'; -import { countSqlSeedRows, usesIdentityPreservingNameUpsert } from './seed-contract.mjs'; +import { + countSqlSeedRows, + findDuplicateSeedNames, + usesIdentityPreservingNameUpsert, +} from './seed-contract.mjs'; const contract = JSON.parse(readFileSync(new URL('../contract.json', import.meta.url), 'utf8')); const types = readFileSync(new URL('../generated/database.types.ts', import.meta.url)); @@ -73,8 +77,24 @@ if (missingDatabaseTests.length !== 0) { if (contract.typesSha256 !== hash) { throw new Error(`Generated type hash mismatch: expected ${contract.typesSha256}, received ${hash}.`); } -if (countSqlSeedRows(godSeed) !== 88) { - throw new Error('The reviewed local SMITE god seed must contain exactly 88 rows.'); +// The god catalog grows as SMITE ships gods, and diese-tech/smite-content-sync +// proposes those additions as reviewed PRs. An exact row count froze the +// catalog: every real addition failed this check until someone edited this +// script. What the guard is actually for is a seed that silently loses or +// duplicates entries, so it asserts a declared floor instead. Lowering the +// floor is a deliberate edit to contract.json, where the release is declared. +const godSeedRows = countSqlSeedRows(godSeed); +if (!Number.isInteger(contract.smiteGodSeedMinimumRows) || contract.smiteGodSeedMinimumRows < 1) { + throw new Error('contract.smiteGodSeedMinimumRows must be a positive integer.'); +} +if (godSeedRows < contract.smiteGodSeedMinimumRows) { + throw new Error( + `The reviewed local SMITE god seed must contain at least ${contract.smiteGodSeedMinimumRows} rows, received ${godSeedRows}.`, + ); +} +const duplicateGodNames = findDuplicateSeedNames(godSeed); +if (duplicateGodNames.length !== 0) { + throw new Error(`The SMITE god seed repeats a name: ${duplicateGodNames.join(', ')}.`); } if (!usesIdentityPreservingNameUpsert(godSeed)) { throw new Error('The SMITE god seed must reconcile by unique name without replacing historical IDs.'); diff --git a/test/verification-scripts.test.mjs b/test/verification-scripts.test.mjs index 274bc91..4ea2a3c 100644 --- a/test/verification-scripts.test.mjs +++ b/test/verification-scripts.test.mjs @@ -11,6 +11,7 @@ import { normalizeMigrationPlan } from '../scripts/normalize-migration-plan.mjs' import { assertProductionTestSqlIsReadOnly } from '../scripts/production-test-contract.mjs'; import { countSqlSeedRows, + findDuplicateSeedNames, usesIdentityPreservingNameUpsert, } from '../scripts/seed-contract.mjs'; @@ -116,6 +117,41 @@ test('counts reviewed SQL seed tuples without relying on live table contents', ( ); }); +test('reports seed names repeated under different ids', () => { + // The old exact-row assertion caught a duplicate only through the count. A + // growth floor does not, so the duplicate check stands on its own. + assert.deepEqual( + findDuplicateSeedNames( + "insert into public.gods values\n ('zeus', 'Zeus'),\n ('zeus-2', 'Zeus'),\n ('hel', 'Hel');\n", + ), + ['zeus'], + ); +}); + +test('accepts a growing seed with distinct names', () => { + assert.deepEqual( + findDuplicateSeedNames( + "insert into public.gods values\n ('a', 'A'),\n ('b', 'B'),\n ('c', 'C');\n", + ), + [], + ); +}); + +test('compares seed names case-insensitively and unescapes doubled quotes', () => { + assert.deepEqual( + findDuplicateSeedNames( + "insert into public.gods values\n ('ah', 'Ah Muzen Cab'),\n ('ah2', 'ah muzen cab');\n", + ), + ['ah muzen cab'], + ); + assert.deepEqual( + findDuplicateSeedNames( + "insert into public.gods values\n ('x', 'Chang''e'),\n ('y', 'Chang''e');\n", + ), + ["chang'e"], + ); +}); + test('requires god seed reconciliation to preserve historical IDs', () => { assert.equal( usesIdentityPreservingNameUpsert( From ab62a36302c7f086292b5cb31c1a1feacf1e6f80 Mon Sep 17 00:00:00 2001 From: Dustin Date: Tue, 1 Sep 2026 14:20:26 +0000 Subject: [PATCH 2/3] fix(contract): ratchet the seed guard against the base branch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The declared floor does not rise when growth is accepted. Once a sync PR takes the catalog to 89, a later change could drop back to 88 and still pass, quietly losing a god that had already been reviewed — so the floor alone did not deliver the "never shrinks" invariant the runbook claimed. CI now also compares each reviewed seed against the same file on the pull request's base branch and fails if it lost rows. That holds the invariant continuously without a manual bump on every addition, and the floor stays as an absolute backstop. Verified against the real seed: growth 88 to 89 passes, the regression the review described (base 89, PR back to 88) fails naming both counts, an unchanged seed passes, and a seed with no base copy is skipped rather than erroring, so adding a brand-new seed file still works. Also corrects the runbook, which described the floor as if it ratcheted. --- .github/workflows/ci.yml | 16 ++++++++++++++++ docs/runbooks/consumer-contract.md | 19 ++++++++++++++----- scripts/seed-contract.mjs | 10 ++++++++++ scripts/verify-seed-growth.mjs | 27 +++++++++++++++++++++++++++ test/verification-scripts.test.mjs | 17 +++++++++++++++++ 5 files changed, 84 insertions(+), 5 deletions(-) create mode 100644 scripts/verify-seed-growth.mjs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 97cb661..4630c90 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -39,6 +39,22 @@ jobs: - run: npm run typecheck - run: npm test - run: npm run build + - name: Verify reviewed seeds did not lose rows + if: github.event_name == 'pull_request' + shell: bash + run: | + git fetch --no-tags --depth=1 origin "$BASE_SHA" + mkdir -p "$RUNNER_TEMP/base-seeds" + for seed in smite2-gods smite2-items; do + git show "$BASE_SHA:supabase/seeds/$seed.sql" \ + > "$RUNNER_TEMP/base-seeds/$seed.sql" 2>/dev/null || true + node scripts/verify-seed-growth.mjs \ + "$RUNNER_TEMP/base-seeds/$seed.sql" \ + "supabase/seeds/$seed.sql" \ + "$seed" + done + env: + BASE_SHA: ${{ github.event.pull_request.base.sha }} - run: npm audit --audit-level=high secret-scan: diff --git a/docs/runbooks/consumer-contract.md b/docs/runbooks/consumer-contract.md index a3282da..7382ee2 100644 --- a/docs/runbooks/consumer-contract.md +++ b/docs/runbooks/consumer-contract.md @@ -14,11 +14,20 @@ The database release stores this repository-owned `contract.json`: `smiteGodSeedMinimumRows` is the floor the reviewed SMITE god seed must meet. The catalog grows as SMITE ships gods, and `diese-tech/smite-content-sync` -proposes those additions as reviewed pull requests, so the contract asserts the -seed never *shrinks* rather than pinning an exact count that every real addition -would break. A seed that loses rows, or repeats a name under a second id, still -fails closed. Lowering the floor is a deliberate edit here, in the manifest -where the release is declared. +proposes those additions as reviewed pull requests, so the contract asserts a +floor rather than pinning an exact count that every real addition would break. +Lowering the floor is a deliberate edit here, in the manifest where the release +is declared. + +The floor alone is a backstop, not a ratchet: it does not rise when growth is +accepted, so on its own it would let a later change drop back to it and quietly +lose a god that had already been reviewed. Two further guards close that: + +- CI compares each seed against the same file on the pull request's base + branch and fails if it lost rows (`scripts/verify-seed-growth.mjs`), so + "never shrinks" holds continuously without a manual bump on every addition. +- The god seed may not repeat a name under a second id, which the old exact-row + assertion only ever caught through the total. Each consumer commits a separate `db-contract.lock.json` with this shape: diff --git a/scripts/seed-contract.mjs b/scripts/seed-contract.mjs index 9a9f1f5..af28963 100644 --- a/scripts/seed-contract.mjs +++ b/scripts/seed-contract.mjs @@ -18,3 +18,13 @@ export const findDuplicateSeedNames = (source) => { } return [...duplicates].sort(); }; + +// The contract.json floor is an absolute backstop, but it does not move on its +// own: once growth is accepted, a later change could drop back to the floor and +// still pass. This compares a seed against the same seed on the base branch, so +// "never shrinks" holds continuously without a manual bump on every addition. +export const findSeedRowRegression = (previousSource, nextSource) => { + const previousRows = countSqlSeedRows(previousSource); + const nextRows = countSqlSeedRows(nextSource); + return nextRows < previousRows ? { previousRows, nextRows } : null; +}; diff --git a/scripts/verify-seed-growth.mjs b/scripts/verify-seed-growth.mjs new file mode 100644 index 0000000..e69a3e6 --- /dev/null +++ b/scripts/verify-seed-growth.mjs @@ -0,0 +1,27 @@ +import { readFileSync } from 'node:fs'; +import { findSeedRowRegression } from './seed-contract.mjs'; + +const [previousPath, nextPath, label = 'seed'] = process.argv.slice(2); + +if (!previousPath || !nextPath) { + throw new Error('Usage: node scripts/verify-seed-growth.mjs [label]'); +} + +// A base branch with no such seed yet (a brand-new seed file) cannot regress. +let previousSource; +try { + previousSource = readFileSync(previousPath, 'utf8'); +} catch { + console.log(`No base copy of the ${label} seed to compare against; nothing to regress from.`); + process.exit(0); +} + +const regression = findSeedRowRegression(previousSource, readFileSync(nextPath, 'utf8')); +if (regression !== null) { + throw new Error( + `The reviewed ${label} seed lost rows against the base branch: ${regression.previousRows} to ${regression.nextRows}. ` + + 'A previously reviewed entry may only be removed in a change that says so explicitly.', + ); +} + +console.log(`Verified the ${label} seed did not lose rows against the base branch.`); diff --git a/test/verification-scripts.test.mjs b/test/verification-scripts.test.mjs index 4ea2a3c..a83e236 100644 --- a/test/verification-scripts.test.mjs +++ b/test/verification-scripts.test.mjs @@ -12,6 +12,7 @@ import { assertProductionTestSqlIsReadOnly } from '../scripts/production-test-co import { countSqlSeedRows, findDuplicateSeedNames, + findSeedRowRegression, usesIdentityPreservingNameUpsert, } from '../scripts/seed-contract.mjs'; @@ -117,6 +118,22 @@ test('counts reviewed SQL seed tuples without relying on live table contents', ( ); }); +test('reports a seed that lost rows against its base branch', () => { + // The contract floor does not move on its own, so once growth is accepted a + // later change could drop back to it. This is what keeps "never shrinks" + // true continuously rather than only against the declared floor. + const base = "insert into public.gods values\n ('a', 'A'),\n ('b', 'B');\n"; + const shrunk = "insert into public.gods values\n ('a', 'A');\n"; + assert.deepEqual(findSeedRowRegression(base, shrunk), { previousRows: 2, nextRows: 1 }); +}); + +test('accepts a seed that grew or held steady against its base branch', () => { + const base = "insert into public.gods values\n ('a', 'A'),\n ('b', 'B');\n"; + const grown = "insert into public.gods values\n ('a', 'A'),\n ('b', 'B'),\n ('c', 'C');\n"; + assert.equal(findSeedRowRegression(base, grown), null); + assert.equal(findSeedRowRegression(base, base), null); +}); + test('reports seed names repeated under different ids', () => { // The old exact-row assertion caught a duplicate only through the count. A // growth floor does not, so the duplicate check stands on its own. From 2a2069c799c30b2f6085a409a59474e56a1633e1 Mon Sep 17 00:00:00 2001 From: Dustin Date: Tue, 1 Sep 2026 14:23:31 +0000 Subject: [PATCH 3/3] chore(contract): release the seed guard as db-v1.21.1 This change alters contract.json itself, so it needs its own release identity. Leaving it as db-v1.21.0 would let that tag resolve to two different manifests depending on which commit a consumer fetched, and sal-site's verifier compares the manifest at the release tag against the one at the pinned commit. --- contract.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contract.json b/contract.json index 3c6fdf5..dc386ff 100644 --- a/contract.json +++ b/contract.json @@ -1,5 +1,5 @@ { - "version": "db-v1.21.0", + "version": "db-v1.21.1", "migrationHead": "20260901120000", "supabaseCliVersion": "2.109.1", "typesSha256": "sha256:4f4564132dcca1d763ae72e08319bfe90b675cbdb940dfdcc730329b2fa6543d",