diff --git a/.changeset/migrate-duplicates-kernel-ready-preflight.md b/.changeset/migrate-duplicates-kernel-ready-preflight.md new file mode 100644 index 0000000000..9cc2e79f5c --- /dev/null +++ b/.changeset/migrate-duplicates-kernel-ready-preflight.md @@ -0,0 +1,62 @@ +--- +"@objectstack/metadata-protocol": minor +"@objectstack/cli": minor +--- + +`os migrate duplicates` now reports the rows blocking the three `kernel:ready` +NULL-safe index tightenings, and the three migrations' conflict messages point +there instead of at `os migrate plan` (#8725). + +**The gap.** Three migrations replace a declared UNIQUE index with the NULL-safe +— and sometimes active-rows-only — form it was always meant to have, at +`kernel:ready` on a serving boot: + +| table | index(es) | migration | +| --- | --- | --- | +| `sys_metadata` | overlay `active` + `draft` | `ensureMetadataOverlayIndexes` | +| `sys_view_definition` | `idx_sys_view_def_active` | `ensureViewDefinitionActiveIndex` | +| `sys_setting` | the declared row identity | `ensureSysSettingIdentityIndex` | + +Each is a tightening, so rows an installation already holds can block it. The +migration then refuses — previous index kept, no row touched, boot continues — +and reports at `error` on the boot channel. That channel was the only one: +these indexes are invisible to `os migrate plan` **by construction**, twice +over. After the tightening, `isRuntimeManagedIndex` excludes the index (without +that exclusion a boot would propose rebuilding away the guarantee it had just +created); before it, each migration deliberately reuses the *declared* index's +name, so the reconciler's name-matched slot reads as filled whichever physical +form is really there. Measured with a matched control — one database carrying +the same duplicate damage under a declared index and under +`sys_view_definition`'s runtime one — `plan` named the declared one in full and +said nothing whatsoever about the runtime one. + +**What is new.** The report gains a `runtimeIndexPreflight` section, one entry +per index, each `blocked` (with every colliding key group and its row count), +`clear`, `table-absent` (`sys_setting` arrives with the optional settings +service) or `unreadable` (with the driver's own message), plus +`summary.runtimeIndexesBlocked` and `summary.runtimeIndexBlockingRows`. +`reportVersion` moves `1` → `2`. Every `1` field keeps its name, shape and +meaning; the bump says there is more in the document, for consumers that +validate it strictly. + +The probes are the migrations' own duplicate-listing statements — +`@objectstack/metadata-protocol` exports `collectRuntimeIndexPreflight` and +`runtimeIndexProbes`, which read those builders rather than restating the keys, +so the pre-flight and the boot report cannot describe different duplicates. On +MySQL the `sys_setting` probe uses the migration's MySQL spelling, where the +bare form is `ERROR 1064` on the reserved word `key`. + +**The referral, repointed rather than deleted** (maintainer ruling, 2026-08-22). +All three conflict messages told the operator to "run `os migrate plan`" as an +alternative way to list the blocking rows, and that instruction was false: they +now name `os migrate duplicates`, which answers it. The six doc comments that +state the same referral as part of the ADR-0120 D4 disposition are updated with +them. + +**Nothing about a migration's behaviour changes.** No tightening is armed, +deferred or altered, and `os migrate plan`'s drift contract is untouched. The +pre-flight only makes the refusal's evidence readable one command before the +restart — from a command that boots read-only and writes nothing, which is +pinned logically (schema plus every row, ordered) rather than by a file hash: a +raw hash over a SQLite file moves on any read-write open and would accuse this +command of mutating the install it exists to describe. diff --git a/content/docs/deployment/cli.mdx b/content/docs/deployment/cli.mdx index 8e12d67573..e45f98c8df 100644 --- a/content/docs/deployment/cli.mdx +++ b/content/docs/deployment/cli.mdx @@ -677,7 +677,7 @@ where the data lives. | `os migrate value-shapes` | Scan stored reference and structured-JSON field values against the platform's value contract, and record the deployment's migration flag when clean | | `os migrate summary-nulls` | Backfill roll-up `count` / `sum` columns still stored as `NULL` on parent rows created before the insert-time seed. Repairs values; no flag, nothing depends on it having run | | `os migrate meta --stored` | Replay the metadata conversion chain over this deployment's `sys_metadata` rows and rewrite the ones still carrying a pre-protocol shape. Hygiene, not a gate — nothing depends on it having run | -| `os migrate duplicates` | Report business identifiers already minted twice across the organization partitions — a read-only inventory as JSON on stdout. Renumbers nothing and writes nothing at all; run it before the boot-time tenancy repair, which overwrites part of the evidence | +| `os migrate duplicates` | Report business identifiers already minted twice across the organization partitions, and the rows blocking the boot-time NULL-safe index tightenings — a read-only inventory as JSON on stdout. Renumbers nothing and writes nothing at all; run it before the boot-time tenancy repair, which overwrites part of the evidence | ```bash os migrate files-to-references # Dry run: full report, writes nothing @@ -1011,6 +1011,57 @@ that could not be probed is listed under `skipped` with its reason, because a driver with no raw SQL seam (memory, MongoDB) fails the whole run with `error: "no_sql_seam"` rather than returning an empty inventory. +##### The `kernel:ready` index pre-flight + +The report carries a second section, `runtimeIndexPreflight`, answering a +different question: **will the next server start be able to finish tightening +the platform's own unique indexes?** + +Three migrations run at `kernel:ready` on a serving boot (`os dev`, `os serve`, +`os start`) and replace a declared UNIQUE index with the NULL-safe — and +sometimes active-rows-only — form it was always meant to have: + +| Table | Index | What the tightening adds | +| :--- | :--- | :--- | +| `sys_metadata` | overlay `active` and `draft` | package-less overlays stop being NULL-distinct | +| `sys_view_definition` | `idx_sys_view_def_active` | shared and environment-level views stop being NULL-distinct, and only active rows are constrained | +| `sys_setting` | the declared row identity | tenant- and global-scope rows stop being NULL-distinct on `user_id` | + +Each is a **tightening**, so rows an installation already holds can block it. +When that happens the migration refuses — the previous index stays in place, no +row is touched, and the server keeps running — and reports it at `error` in the +boot log. Until this section existed that log line was the only channel: these +indexes are invisible to `os migrate plan` by construction, because the drift +reconciler deliberately excludes runtime-managed indexes (otherwise the next +boot would propose rebuilding away the guarantee it just created), and because +each migration reuses the *declared* index's name, so the reconciler's slot for +it reads as correctly filled whichever form is physically there. + +So the pre-flight lives here instead, on the command that already boots +read-only and repairs nothing. It runs the migrations' own duplicate-listing +queries — the exact statements the boot log prints — and reports one entry per +index: + +| `status` | Meaning | +| :--- | :--- | +| `blocked` | Rows collide under the tightened key. `groups` lists each colliding key and how many rows hold it. The next serving boot will refuse this index | +| `clear` | The probe ran and nothing collides | +| `table-absent` | The table is not installed here. `sys_setting`, for instance, arrives with the optional settings service | +| `unreadable` | The probe could not run; `detail` carries the driver's message | + +`summary.runtimeIndexesBlocked` and `summary.runtimeIndexBlockingRows` are the +same finding counted at the head of the document. + +Read `blocked` as **work to do before the restart, not damage**: nothing is +lost while an index stays untightened, but the guarantee it carries is not in +force until the listed rows are resolved — and only an operator can decide which +of two colliding rows survives, which is why the platform refuses rather than +picking one. + +`--object` does not narrow this section. It is a fixed set of platform indexes +rather than a slice of your registry, and `filter` describes the object scan +only. + ### Scaffolding | Command | Alias | Description | diff --git a/docs/qa/platform-checklist/areas/cli.json b/docs/qa/platform-checklist/areas/cli.json index 4e90f418a1..8513dffb8a 100644 --- a/docs/qa/platform-checklist/areas/cli.json +++ b/docs/qa/platform-checklist/areas/cli.json @@ -1047,7 +1047,7 @@ "title": "os migrate duplicates: a read-only JSON inventory of identifiers minted across partitions — within-partition repeats excluded, nothing written, the live two-counter condition reported, and runnable BEFORE the #8686 repair destroys the evidence", "since": "v17", "status": "active", - "revision": 2, + "revision": 3, "priority": "P1", "surface": "cli", "personas": ["operator (local shell, pre-repair audit)"], @@ -1066,7 +1066,8 @@ "boot the scratch app once with `os dev -d file:/tmp//dup.db` so the base schema exists; stop it", "seed via direct SQL per the fixture recipe: the cross-partition duplicate, the within-partition repeat, an organizations row for '', and the paired sequence counters", "md5sum the DB file; run `os migrate duplicates > report.json; echo $?`; md5sum again and byte-compare", - "jq the report: .report/.reportVersion/.generatedAt/.database/.globalPartition/.filter/.counters/.scanned/.skipped/.duplicates/.liveConditions/.summary", + "jq the report: .report/.reportVersion/.generatedAt/.database/.globalPartition/.filter/.counters/.scanned/.skipped/.duplicates/.liveConditions/.runtimeIndexPreflight/.summary", + "seed the kernel:ready blocker too (#8725): two ACTIVE sys_view_definition rows with the SAME name and organization_id/owner both NULL — then jq .runtimeIndexPreflight and .summary.runtimeIndexesBlocked", "run `os migrate duplicates --object ` and `--object ` — capture .filter in both payloads", "run `os migrate duplicates --database-url file:/tmp//dup.db` and confirm it reaches the same DB (the flag also honors OS_DATABASE_URL)", "negative: from the memory-driver scratch config run `os migrate duplicates; echo $?` and capture the refusal payload", @@ -1074,7 +1075,7 @@ ], "acceptance": [ { - "clause": "the report is the declared machine-readable contract on stdout: report 'duplicate-identifiers', reportVersion 1, generatedAt, database, globalPartition, filter, counters {table, status read|absent}, scanned[], skipped[], duplicates[], liveConditions[], summary — and each duplicate carries object/field/value/holderCount/partitions plus per-holder id/organization/partition/createdAt (createdAt null when the object has no such column, never a failed probe)", + "clause": "the report is the declared machine-readable contract on stdout: report 'duplicate-identifiers', reportVersion 2, generatedAt, database, globalPartition, filter, counters {table, status read|absent}, scanned[], skipped[], duplicates[], liveConditions[], runtimeIndexPreflight[], summary {…, runtimeIndexesBlocked, runtimeIndexBlockingRows} — and each duplicate carries object/field/value/holderCount/partitions plus per-holder id/organization/partition/createdAt (createdAt null when the object has no such column, never a failed probe)", "oracle": "log", "verify": "jq walks every declared key of the seeded run's payload; shape pinned by duplicates.contract.test.ts — cite its pass for the full-shape guarantee, drive the CLI for the instance", "evidence": "report.json + the jq walk" @@ -1091,6 +1092,12 @@ "verify": "the two md5sums match; duplicates.pre-repair.test.ts pins the same invariant down to _objectstack_sequences", "evidence": "the md5 pair" }, + { + "clause": "the kernel:ready pre-flight (#8725) reports the RUNTIME-migration class os migrate plan cannot see: one entry per index the three kernel:ready migrations tighten (four — the overlay migration owns two), each blocked|clear|table-absent|unreadable, a blocked one naming every colliding key group and its row count", + "oracle": "log", + "verify": ".runtimeIndexPreflight names idx_sys_view_def_active as blocked with the seeded view name, organization_id_key '__global__' and owner_key '' — and the SAME database run through `os migrate plan` mentions neither the index nor the view name (the matched control: the declared-index duplicate above IS reported by plan, this one is not)", + "evidence": "the pre-flight section beside the plan output for one database" + }, { "clause": "the live condition (#8928 point 4) fires exactly when a __global__ counter sits beside an org-scoped counter for the same object/field — reported as a prediction with globalLastValue and the per-org counters, and counters.status says where it was read from ('absent' still yields a complete duplicates inventory)", "oracle": "log", diff --git a/packages/cli/src/commands/migrate/duplicates.contract.test.ts b/packages/cli/src/commands/migrate/duplicates.contract.test.ts index 3d62b6e544..5851e7efba 100644 --- a/packages/cli/src/commands/migrate/duplicates.contract.test.ts +++ b/packages/cli/src/commands/migrate/duplicates.contract.test.ts @@ -21,7 +21,9 @@ import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { SqlDriver } from '@objectstack/driver-sql'; import { + collectRuntimeIndexPreflight, normalizeRows, + runtimeIndexProbes, GLOBAL_TENANT, ORGANIZATION_FIELD, SEQUENCES_TABLE, @@ -120,7 +122,7 @@ afterAll(async () => { try { rmSync(dir, { recursive: true, force: true }); } catch { /* ignore */ } }); -const collect = (objectFilter?: string) => +const collect = async (objectFilter?: string) => collectDuplicateIdentifierReport({ exec, normalize: normalizeRows, @@ -131,14 +133,31 @@ const collect = (objectFilter?: string) => sequencesTable: SEQUENCES_TABLE, client: 'better-sqlite3', now: () => new Date('2026-08-17T12:00:00.000Z'), + // The real pre-flight against the real fixture — never a stand-in. This + // database has none of the four platform tables, so every entry is + // `table-absent`, and the `blocked` shape is pinned in its own test below + // over a database that really carries the damage. + runtimeIndexPreflight: await collectRuntimeIndexPreflight(exec, { client: 'better-sqlite3' }), ...(objectFilter ? { objectFilter } : {}), }); +/** + * The four probes, as `@objectstack/metadata-protocol` declares them. + * + * Read from the producer rather than restated here: the descriptor (table, + * index name, key parts, row scope, the two statements) is the migration's own + * definition of its key, and a second copy in this file would be a second + * definition to keep in step. What this file pins is that the report carries + * that descriptor through UNCHANGED, plus a status and its groups. + */ +const PROBES = runtimeIndexProbes({ client: 'better-sqlite3' }); + describe('#8928 os migrate duplicates — the report document', () => { it('is exactly this shape, whole', async () => { expect(await collect()).toEqual({ report: 'duplicate-identifiers', - reportVersion: 1, + // #8725 added `runtimeIndexPreflight` and its two summary counters. + reportVersion: 2, generatedAt: '2026-08-17T12:00:00.000Z', database: 'better-sqlite3 (fixture)', globalPartition: '__global__', @@ -212,16 +231,40 @@ describe('#8928 os migrate duplicates — the report document', () => { organizationCounters: [{ organization: 'org_x', lastValue: 4 }], }, ], + // One entry per index the three `kernel:ready` migrations tighten — FOUR, + // because the overlay migration builds one per state and either can be + // blocked on its own. Present whatever the outcome: an index left out + // would make "nothing blocks it" and "it was never probed" the same + // absence, which is the rule this command already applies to `skipped`. + runtimeIndexPreflight: PROBES.map((probe) => ({ + ...probe, + status: 'table-absent', + groups: [], + })), summary: { objectsScanned: 2, fieldsScanned: 3, duplicateValues: 3, duplicateRows: 6, liveConditions: 1, + runtimeIndexesBlocked: 0, + runtimeIndexBlockingRows: 0, }, }); }); + it('names the four kernel:ready indexes, and the migration behind each', async () => { + const report = await collect(); + expect( + report.runtimeIndexPreflight.map((p) => `${p.migration}:${p.table}:${p.index}`), + ).toEqual([ + 'ensureMetadataOverlayIndexes:sys_metadata:idx_sys_metadata_overlay_active', + 'ensureMetadataOverlayIndexes:sys_metadata:idx_sys_metadata_overlay_draft', + 'ensureViewDefinitionActiveIndex:sys_view_definition:idx_sys_view_def_active', + 'ensureSysSettingIdentityIndex:sys_setting:uniq_sys_setting_organization_id_namespace_key_scope_user_id', + ]); + }); + it('does NOT report a value repeated inside ONE partition — the narrow ruled definition', async () => { // `REF-1` is held twice, both times by `org_x`. That is a repeat the // partitioned unique index already refuses; reporting it would be the wider @@ -248,6 +291,7 @@ describe('#8928 os migrate duplicates — the report document', () => { organizationField: ORGANIZATION_FIELD, sequencesTable: SEQUENCES_TABLE, client: 'better-sqlite3', + runtimeIndexPreflight: [], }); expect(report.skipped).toEqual([ expect.objectContaining({ diff --git a/packages/cli/src/commands/migrate/duplicates.integration.test.ts b/packages/cli/src/commands/migrate/duplicates.integration.test.ts index c8c44bff22..e2011dfed2 100644 --- a/packages/cli/src/commands/migrate/duplicates.integration.test.ts +++ b/packages/cli/src/commands/migrate/duplicates.integration.test.ts @@ -16,6 +16,26 @@ * is the part of the run with the most write paths behind it (schema sync, * the artifact seed, the `kernel:ready` migrations), so it is the part worth * measuring rather than reasoning about. + * + * ## The matched control (#8725) + * + * Since #8725 the fixture carries the SAME duplicate damage twice, in one + * database, under two different vocabularies: + * + * - `crm_case.case_number` — a DECLARED identifier, one value held on both + * sides of the organization partition. Reported by the `duplicates` scan, + * and reported before this card existed. + * - `sys_view_definition` — two ACTIVE shared views under one name, which is + * exactly what blocks `ensureViewDefinitionActiveIndex`'s NULL-safe + * tightening. Invisible to the drift differ by construction, and therefore + * to `os migrate plan`: `isRuntimeManagedIndex` excludes the index once the + * partial form exists, and before that the migration reuses the DECLARED + * index's name so the name-matched slot reads as filled either way. + * + * The control is what makes the second assertion mean something. A pre-flight + * that quietly reported only the declared class — or a fixture that failed to + * carry damage at all — would still satisfy "the report names some duplicate". + * Both classes are asserted, separately, over one run. */ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; @@ -26,6 +46,7 @@ import { SqlDriver } from '@objectstack/driver-sql'; import { resolveSeedTenancyExec, normalizeRows, + collectRuntimeIndexPreflight, GLOBAL_TENANT, ORGANIZATION_FIELD, ORGANIZATION_TABLE, @@ -38,7 +59,17 @@ let dir: string; let dbFile: string; const savedEnv: Record = {}; -/** Read the fixture with a connection of our own — never the booted stack's. */ +/** + * The fixture's LOGICAL state — the schema plus every row of every table, + * ordered — read with a connection of our own, never the booted stack's. + * + * ⚠️ Deliberately not a hash of the database FILE. SQLite rewrites header + * bytes (the change counter, the version-valid-for cookie) on any read-write + * open, so a file hash reports a difference after a run that only SELECTed and + * would accuse this command of mutating the install it exists to describe — + * measured, and very nearly filed as a defect. What must not change is the + * schema and the rows. + */ async function readState(): Promise { const probe = new SqlDriver({ client: 'better-sqlite3', @@ -47,10 +78,15 @@ async function readState(): Promise { }); try { const k = (probe as any).knex; - return { - cases: await k('crm_case').select('*').orderBy('id'), - sequences: await k(SEQUENCES_TABLE).select('*').orderBy(['object', 'tenant_id']), - }; + const schema = await k + .raw("SELECT type, name, sql FROM sqlite_master ORDER BY type, name") + .then((r: any) => r); + const rows: Record = {}; + for (const entry of schema as Array<{ type: string; name: string }>) { + if (entry.type !== 'table' || entry.name.startsWith('sqlite_')) continue; + rows[entry.name] = await k.raw(`SELECT * FROM "${entry.name}" ORDER BY rowid`); + } + return { schema, rows }; } finally { await probe.disconnect(); } @@ -98,6 +134,27 @@ beforeAll(async () => { { id: 's1', created_at: '2026-01-01T00:00:00.000Z', organization_id: null, subject: 'seeded', case_number: 'CASE-00001' }, { id: 'a1', created_at: '2026-02-01T00:00:00.000Z', organization_id: 'org_x', subject: 'api', case_number: 'CASE-00001' }, ]); + // ── The runtime-migration half of the matched control (#8725) ────────── + // Two ACTIVE shared views under one name: `owner` NULL and `organization_id` + // NULL both fold into their sentinel buckets, so these two rows collide under + // `idx_sys_view_def_active`'s NULL-safe key while the declared, NULL-distinct + // index admits them. This is what blocks the tightening on the next serving + // boot — and what the drift differ cannot report. + await k.schema.createTable('sys_view_definition', (t: any) => { + t.string('id').primary(); + t.string('name'); + t.string('organization_id'); + t.string('owner'); + t.string('state'); + }); + await k('sys_view_definition').insert([ + { id: 'v1', name: 'crm_case.all_open', organization_id: null, owner: null, state: 'active' }, + { id: 'v2', name: 'crm_case.all_open', organization_id: null, owner: null, state: 'active' }, + // Outside the partial index's row scope: the same collision among archived + // rows is legal and must not be reported. + { id: 'v3', name: 'crm_case.retired', organization_id: null, owner: null, state: 'archived' }, + { id: 'v4', name: 'crm_case.retired', organization_id: null, owner: null, state: 'archived' }, + ]); await k.schema.createTable(ORGANIZATION_TABLE, (t: any) => { t.string('id').primary(); t.string('name'); @@ -151,6 +208,7 @@ describe('#8928 os migrate duplicates — against a really booted stack', () => // a package is scanned exactly like one from this project's config. expect((stack.allObjects() as Array<{ name?: string }>).map((o) => o?.name)).toContain('crm_case'); + const client = String((stack.driver?.config as { client?: unknown })?.client ?? ''); produced = await collectDuplicateIdentifierReport({ exec: exec!, normalize: normalizeRows, @@ -159,7 +217,10 @@ describe('#8928 os migrate duplicates — against a really booted stack', () => globalTenant: GLOBAL_TENANT, organizationField: ORGANIZATION_FIELD, sequencesTable: SEQUENCES_TABLE, - client: String((stack.driver?.config as { client?: unknown })?.client ?? ''), + client, + // The real pre-flight over the real booted seam — the same call + // `MigrateDuplicates.run()` makes. + runtimeIndexPreflight: await collectRuntimeIndexPreflight(exec!, { client }), }); } finally { await stack.shutdown(); @@ -184,6 +245,38 @@ describe('#8928 os migrate duplicates — against a really booted stack', () => }, ]); + // ── The other half of the control: the RUNTIME-migration class ──────── + // ⭐ This is the assertion the card exists for. The declared-vocabulary + // duplicate above was already reported before #8725; a probe that surfaced + // only that class would satisfy "the report names some duplicate" and still + // leave the operator with nothing at the moment they are blocked. + const viewIndex = produced.runtimeIndexPreflight.find( + (entry) => entry.index === 'idx_sys_view_def_active', + ); + expect(viewIndex, 'the pre-flight must cover sys_view_definition').toBeDefined(); + expect(viewIndex).toMatchObject({ + migration: 'ensureViewDefinitionActiveIndex', + table: 'sys_view_definition', + rowScope: "state = 'active'", + status: 'blocked', + groups: [ + { + key: { name: 'crm_case.all_open', organization_id_key: '__global__', owner_key: '' }, + rowCount: 2, + }, + ], + }); + // The archived pair is outside the partial index and is NOT reported. + expect(JSON.stringify(viewIndex!.groups)).not.toContain('crm_case.retired'); + // The summary counts it, so an operator scanning the head of the document + // sees that something is blocked without reading every entry. + expect(produced.summary.runtimeIndexesBlocked).toBe(1); + expect(produced.summary.runtimeIndexBlockingRows).toBe(2); + // `sys_metadata` exists on this fixture only if the boot made it; whatever + // its status, the four indexes are all accounted for. + expect(produced.runtimeIndexPreflight).toHaveLength(4); + expect(produced.reportVersion).toBe(2); + // The whole run — boot included — wrote nothing. If a future change arms a // repair on this boot path, THIS is the assertion that says so, before an // operator finds out by losing their evidence. diff --git a/packages/cli/src/commands/migrate/duplicates.null-seam.test.ts b/packages/cli/src/commands/migrate/duplicates.null-seam.test.ts index 40c98f33e1..b9d7b7e1e0 100644 --- a/packages/cli/src/commands/migrate/duplicates.null-seam.test.ts +++ b/packages/cli/src/commands/migrate/duplicates.null-seam.test.ts @@ -36,7 +36,7 @@ import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { dirname, join, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; -import { resolveSeedTenancyExec, normalizeRows, GLOBAL_TENANT, ORGANIZATION_FIELD, SEQUENCES_TABLE } from '@objectstack/metadata-protocol'; +import { resolveSeedTenancyExec, normalizeRows, collectRuntimeIndexPreflight, GLOBAL_TENANT, ORGANIZATION_FIELD, SEQUENCES_TABLE } from '@objectstack/metadata-protocol'; import { bootSchemaStack } from '../../utils/schema-migrate.js'; import MigrateDuplicates, { answeringSeam, @@ -204,6 +204,7 @@ describe('#10677 the memory driver, booted for real', () => { globalTenant: GLOBAL_TENANT, organizationField: ORGANIZATION_FIELD, sequencesTable: SEQUENCES_TABLE, + runtimeIndexPreflight: [], }; // The population is real, so "nothing was scanned" cannot explain the @@ -224,6 +225,17 @@ describe('#10677 the memory driver, booted for real', () => { for (const entry of after.skipped) expect(entry.reason).toMatch(/no result set/); // The counter table is no longer claimed as read, either. expect(after.counters.status).toBe('absent'); + + // ── The same separation, on the #8725 pre-flight ──────────────────── + // Its per-index presence question reads a refusal as "this table is not + // installed", which is right on a working seam and catastrophic on this + // one — four tightenings reported as absent by a probe that never ran. + // So the pre-flight proves the SEAM live first, against a statement whose + // failure cannot mean "absent", and reports every index as `unreadable`. + const preflight = await collectRuntimeIndexPreflight(answeringSeam(exec)); + expect(preflight.length).toBeGreaterThan(0); + expect(new Set(preflight.map((p) => p.status))).toEqual(new Set(['unreadable'])); + for (const entry of preflight) expect(String(entry.detail)).toMatch(/no result set/); } finally { await stack.shutdown(); } diff --git a/packages/cli/src/commands/migrate/duplicates.pre-repair.test.ts b/packages/cli/src/commands/migrate/duplicates.pre-repair.test.ts index 2a892f1139..22e7865f70 100644 --- a/packages/cli/src/commands/migrate/duplicates.pre-repair.test.ts +++ b/packages/cli/src/commands/migrate/duplicates.pre-repair.test.ts @@ -22,6 +22,7 @@ import { join } from 'node:path'; import { SqlDriver } from '@objectstack/driver-sql'; import { backfillSeedTenancy, + collectRuntimeIndexPreflight, normalizeRows, GLOBAL_TENANT, ORGANIZATION_FIELD, @@ -47,7 +48,7 @@ async function snapshot(): Promise { }; } -const report = () => +const report = async () => collectDuplicateIdentifierReport({ exec, normalize: normalizeRows, @@ -57,6 +58,10 @@ const report = () => organizationField: ORGANIZATION_FIELD, sequencesTable: SEQUENCES_TABLE, client: 'better-sqlite3', + // The real pre-flight (#8725), so the byte-identity assertions below cover + // its four SELECTs too — a section that writes would destroy the same + // evidence the ruling's timing note is about. + runtimeIndexPreflight: await collectRuntimeIndexPreflight(exec, { client: 'better-sqlite3' }), }); beforeEach(async () => { diff --git a/packages/cli/src/commands/migrate/duplicates.ts b/packages/cli/src/commands/migrate/duplicates.ts index 3f9fc12110..f61218f058 100644 --- a/packages/cli/src/commands/migrate/duplicates.ts +++ b/packages/cli/src/commands/migrate/duplicates.ts @@ -12,7 +12,7 @@ import type { IObjectQLEngine } from '@objectstack/spec/contracts'; // `import()`s every command module on every CLI invocation, so a heavy static // import here is charged to whatever command the operator actually ran (#5726 // makes the same point about driver packages). -import type { SeedTenancyExec } from '@objectstack/metadata-protocol'; +import type { RuntimeIndexPreflight, SeedTenancyExec } from '@objectstack/metadata-protocol'; /** * `os migrate duplicates` — the operator-facing inventory of business @@ -77,6 +77,29 @@ import type { SeedTenancyExec } from '@objectstack/metadata-protocol'; * wrong for a report, which must not silently omit a real duplicate. Anything * that could not be probed is listed in `skipped` with the reason — a target * this command could not read is never reported as a target with no findings. + * + * ## The second population: the `kernel:ready` index pre-flight (#8725) + * + * `runtimeIndexPreflight` is a SEPARATE section, and the separation is the + * point. `duplicates` above answers the #8928 ruling's narrow question — one + * business identifier held across more than one organization partition — and + * folding a second definition of "duplicate" into that array would quietly + * widen a list the ruling deliberately narrowed. + * + * The second question is: do rows already in this database block one of the + * three `kernel:ready` NULL-safe index tightenings? Those migrations refuse + * when they do (ADR-0120 D4 — the previous index stays, no row is touched) and + * report on the boot channel only, because the drift differ cannot see the + * class at all: after the tightening `isRuntimeManagedIndex` excludes the + * index, and before it there is nothing to see either, since each migration + * reuses the DECLARED index's name so the name-matched slot reads as filled in + * both states. Maintainer ruling, 2026-08-22: that report lands here, on the + * command that already boots read-only and owns the "inventory, never repair" + * contract, and `os migrate plan`'s drift contract stays untouched. + * + * Fitting: this command's boot disarms the platform migrations, so the + * pre-flight describes what the next SERVING boot will find — before the + * operator restarts, which is the moment the information is worth anything. */ /** One `(object, field)` this run probed. */ @@ -145,7 +168,13 @@ export interface DuplicateLiveCondition { */ export interface DuplicateIdentifierReport { report: 'duplicate-identifiers'; - reportVersion: 1; + /** + * `2` since #8725 added the `runtimeIndexPreflight` section and its two + * summary counters. Additive — every `1` field is unchanged in name, shape and + * meaning — but a consumer that validates the document strictly would reject + * the new key, so the version says plainly that there is more here now. + */ + reportVersion: 2; /** ISO-8601, so archived reports sort and diff. */ generatedAt: string; /** The database this describes, in the same words the migrate family prints. */ @@ -160,12 +189,30 @@ export interface DuplicateIdentifierReport { skipped: DuplicateSkippedTarget[]; duplicates: DuplicateIdentifier[]; liveConditions: DuplicateLiveCondition[]; + /** + * The `kernel:ready` index tightenings and what currently blocks each — the + * second population described in the module header. Always present, one entry + * per index probed, whatever the outcome: an index reported as `clear` is a + * measurement, and leaving it out would make "nothing blocks it" and "it was + * never probed" the same absence. + * + * ⚠️ `--object` does NOT narrow this section, and `filter` above describes the + * object scan only. This population is four fixed platform indexes rather than + * a slice of the registry, and narrowing it would replace a complete four-row + * measurement with an empty array on exactly the runs where an operator is + * looking at one object — an absence that reads as "nothing blocks anything". + */ + runtimeIndexPreflight: RuntimeIndexPreflight[]; summary: { objectsScanned: number; fieldsScanned: number; duplicateValues: number; duplicateRows: number; liveConditions: number; + /** Indexes whose tightening is blocked by rows already stored. */ + runtimeIndexesBlocked: number; + /** Rows holding a blocking key, summed across those indexes. */ + runtimeIndexBlockingRows: number; }; } @@ -539,6 +586,19 @@ export interface CollectDuplicateReportOptions { objectFilter?: string; /** Injected so the contract test can pin a stable document. */ now?: () => Date; + /** + * The `kernel:ready` index pre-flight, already collected — from + * `collectRuntimeIndexPreflight` in `@objectstack/metadata-protocol`, which + * owns the probes because it owns the migrations that build those indexes. + * + * ⚠️ REQUIRED rather than optional, deliberately. An optional section defaults + * to `[]`, and `[]` is also what a database with nothing blocking produces — + * so a caller that simply forgot to wire it would ship a clean bill of health + * from a probe that never ran, which is the exact failure `#10677` closed on + * the seam a few lines below. Making it required moves that mistake from + * runtime silence to a compile error. + */ + runtimeIndexPreflight: RuntimeIndexPreflight[]; } /** @@ -701,9 +761,10 @@ export async function collectDuplicateIdentifierReport( } const objectsScanned = new Set(targets.map((t) => t.object)).size; + const blocked = opts.runtimeIndexPreflight.filter((p) => p.status === 'blocked'); return { report: 'duplicate-identifiers', - reportVersion: 1, + reportVersion: 2, generatedAt: (opts.now?.() ?? new Date()).toISOString(), database: opts.database, globalPartition: globalTenant, @@ -713,19 +774,26 @@ export async function collectDuplicateIdentifierReport( skipped, duplicates, liveConditions, + runtimeIndexPreflight: opts.runtimeIndexPreflight, summary: { objectsScanned, fieldsScanned: targets.length, duplicateValues: duplicates.length, duplicateRows: duplicates.reduce((total, d) => total + d.holderCount, 0), liveConditions: liveConditions.length, + runtimeIndexesBlocked: blocked.length, + runtimeIndexBlockingRows: blocked.reduce( + (total, p) => total + p.groups.reduce((rows, g) => rows + g.rowCount, 0), + 0, + ), }, }; } export default class MigrateDuplicates extends Command { static override description = - 'Report business identifiers already minted twice across the organization partitions (#8928). ' + + 'Report business identifiers already minted twice across the organization partitions (#8928), ' + + 'and the rows blocking the kernel:ready NULL-safe index tightenings (#8725). ' + 'Read-only inventory as JSON on stdout — never renumbers, deduplicates or rewrites anything. ' + 'Run it BEFORE the #8686 tenancy backfill: the repair overwrites the evidence.'; @@ -777,6 +845,7 @@ export default class MigrateDuplicates extends Command { const { resolveSeedTenancyExec, normalizeRows, + collectRuntimeIndexPreflight, GLOBAL_TENANT, ORGANIZATION_FIELD, SEQUENCES_TABLE, @@ -811,21 +880,29 @@ export default class MigrateDuplicates extends Command { return; } + const client = (stack.driver?.config as { client?: unknown } | undefined)?.client; + const clientName = client ? String(client) : undefined; + // Wrapped, not bare: the preflight cleared the seam as a whole, and this + // keeps every individual probe held to the same standard — one that + // returns no result set becomes a `skipped` entry (or an `unreadable` + // index) rather than zero findings. + const answering = answeringSeam(exec); + const report = await collectDuplicateIdentifierReport({ - // Wrapped, not bare: the preflight cleared the seam as a whole, and - // this keeps every individual probe held to the same standard — one - // that returns no result set becomes a `skipped` entry rather than - // zero findings. - exec: answeringSeam(exec), + exec: answering, + // The `kernel:ready` pre-flight (#8725), through the same wrapped seam + // and the same dialect: a probe that answers nothing lands as + // `unreadable` rather than as an index with nothing blocking it. + runtimeIndexPreflight: await collectRuntimeIndexPreflight(answering, { + ...(clientName ? { client: clientName } : {}), + }), normalize: normalizeRows, objects: stack.allObjects(), database: stack.dbLabel, globalTenant: GLOBAL_TENANT, organizationField: ORGANIZATION_FIELD, sequencesTable: SEQUENCES_TABLE, - ...(((stack.driver?.config as { client?: unknown } | undefined)?.client) - ? { client: String((stack.driver?.config as { client?: unknown }).client) } - : {}), + ...(clientName ? { client: clientName } : {}), ...(flags.object ? { objectFilter: flags.object } : {}), }); diff --git a/packages/metadata-protocol/src/index.ts b/packages/metadata-protocol/src/index.ts index 8767a697ee..246ea6b10e 100644 --- a/packages/metadata-protocol/src/index.ts +++ b/packages/metadata-protocol/src/index.ts @@ -104,6 +104,22 @@ export { // asserted, which is also what an operator copies out of the docs page. } from './migrations/seed-tenancy-backfill.js'; export type { SeedTenancySeam } from './migrations/seed-tenancy-backfill.js'; +// [#8725] The read-only duplicate PRE-FLIGHT for the three `kernel:ready` index +// tightenings above. Exported because it has a real consumer outside this +// package — `os migrate duplicates`, the reporting path the maintainer ruled +// (2026-08-22) for a class the drift differ cannot see by construction, and +// which the three migrations' conflict reports now name. Nothing about when a +// migration runs or what it does changes here; only its evidence becomes +// readable one command before the restart. +export { + collectRuntimeIndexPreflight, + runtimeIndexProbes, +} from './migrations/runtime-index-preflight.js'; +export type { + RuntimeIndexProbe, + RuntimeIndexPreflight, + RuntimeIndexDuplicateGroup, +} from './migrations/runtime-index-preflight.js'; export type { SeedTenancyExec, SeedTenancyLogger, diff --git a/packages/metadata-protocol/src/migrations/overlay-index.test.ts b/packages/metadata-protocol/src/migrations/overlay-index.test.ts index 475bb91910..00f136b428 100644 --- a/packages/metadata-protocol/src/migrations/overlay-index.test.ts +++ b/packages/metadata-protocol/src/migrations/overlay-index.test.ts @@ -252,7 +252,10 @@ describe('sys_metadata overlay uniqueness (#6418)', () => { const msg = String(logger.error.mock.calls[0]![0]); expect(msg).toContain("COALESCE(package_id, '')"); expect(msg).toContain('The previous index is left in place'); - expect(msg).toContain('os migrate plan'); + expect(msg).toContain('os migrate duplicates'); + // The repointing (#8725) is only done if the FALSE referral is gone: + // `os migrate plan` cannot report this index, by construction. + expect(msg).not.toContain('os migrate plan'); expect(msg).toContain(buildOverlayDuplicateProbeSql('active')); // …and that shipped query really does name the offenders, on this very @@ -374,7 +377,8 @@ describe('sys_metadata overlay uniqueness (#6418)', () => { expect(result.status).toBe('conflict'); expect(indexDdl(OVERLAY_INDEX_NAMES.active)).toEqual(DECLARED_ACTIVE_INDEX_DDL); expect(logger.error).toHaveBeenCalledTimes(1); - expect(String(logger.error.mock.calls[0]![0])).toContain('os migrate plan'); + expect(String(logger.error.mock.calls[0]![0])).toContain('os migrate duplicates'); + expect(String(logger.error.mock.calls[0]![0])).not.toContain('os migrate plan'); }); it('an unclassifiable failure is reported at error and leaves the index alone', async () => { diff --git a/packages/metadata-protocol/src/migrations/overlay-index.ts b/packages/metadata-protocol/src/migrations/overlay-index.ts index ede45717d8..3bfdfe75a9 100644 --- a/packages/metadata-protocol/src/migrations/overlay-index.ts +++ b/packages/metadata-protocol/src/migrations/overlay-index.ts @@ -58,11 +58,21 @@ * and only once that has demonstrably succeeded drop the real name and rebuild * it — and reports every failure the way ADR-0120 D4 requires: keep the * previous index, name the key that is NOT enforced, ship the exact query that - * lists the offending rows, point at `os migrate plan`, never block the boot. - * `SqlDriver.createNullSafeUniqueIndex` is the in-repo precedent for that + * lists the offending rows, point at `os migrate duplicates`, never block the + * boot. `SqlDriver.createNullSafeUniqueIndex` is the in-repo precedent for that * disposition; the sibling `view-definition-active-index.ts` is the precedent * for the order. * + * ⚠️ The referral named `os migrate plan` until #8725, and it was FALSE for + * this class: the differ never sees these indexes — `isRuntimeManagedIndex` + * excludes the partial form once it is built, and before that the migration + * reuses the DECLARED index's name, so the name-matched slot reads as filled + * either way — and `PendingSchemaWork` carries no index kind at all. Maintainer + * ruling 2026-08-22 routes the class to `os migrate duplicates`, which boots + * read-only and owns the "inventory, never repair" contract, keeping `os + * migrate plan`'s drift contract untouched. Both overlay states are probed + * there, because either can be blocked independently. + * * ## Why the fallback stays NON-unique * * On a dialect that cannot build the partial form the degradation target is a @@ -183,7 +193,12 @@ export function buildOverlayFallbackIndexSql(indexName: string): string { /** * The query that lists the rows blocking the tightening — ADR-0120 D4's "name * the offending rows", shipped inside the report so an operator has it without - * waiting for `os migrate plan`. + * waiting for `os migrate duplicates`. + * + * The same statement is what `os migrate duplicates` issues for this index + * (#8725), once per state: the command reads this builder rather than restating + * the key, so the pre-flight and the boot report cannot describe different + * duplicates. * * It GROUPs by exactly the index's own key parts, so what it reports and what * the index rejects cannot diverge. The nullable column is projected through @@ -359,7 +374,7 @@ function reportDegradation( `(${columns}) is enforced only as far as it was before; ADR-0005 overlay uniqueness is NOT ` + `enforced until the duplicates are resolved, and getMetaItem has no defined answer for ` + `which of the colliding rows wins. List them with: ${duplicateQuery} — or run ` + - `"os migrate plan" — then restart (ADR-0120 D4, #6418).`, + `"os migrate duplicates" — then restart (ADR-0120 D4, #6418, #8725).`, detail, ); return; diff --git a/packages/metadata-protocol/src/migrations/runtime-index-preflight.test.ts b/packages/metadata-protocol/src/migrations/runtime-index-preflight.test.ts new file mode 100644 index 0000000000..62880292e8 --- /dev/null +++ b/packages/metadata-protocol/src/migrations/runtime-index-preflight.test.ts @@ -0,0 +1,252 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { DatabaseSync } from 'node:sqlite'; + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; + +import { + collectRuntimeIndexPreflight, + runtimeIndexProbes, + type RuntimeIndexPreflight, +} from './runtime-index-preflight.js'; +import { buildOverlayDuplicateProbeSql } from './overlay-index.js'; +import { + buildSysSettingDuplicateProbeSql, + buildSysSettingDuplicateProbeSqlMysql, + buildSysSettingPresenceSql, +} from './sys-setting-identity-index.js'; +import { buildDuplicateProbeSql as buildViewActiveDuplicateProbeSql } from './view-definition-active-index.js'; +import type { IndexExec } from './partial-index-probe.js'; + +/** + * The `kernel:ready` duplicate pre-flight (#8725), against a REAL SQLite + * database. + * + * Real, because the whole point of the section is that it reads rows the drift + * differ cannot see: an exec double asked to return rows would testify only + * that this file's own fixture was typed correctly. `node:sqlite` rather than + * `better-sqlite3` for the same reason the sibling migration tests use it — + * this package needs no SQL dependency of its own, and the built-in gives real + * partial indexes and real grouping semantics for free. + * + * ⚠️ What is NOT pinned here: anything about a migration ARMING or RUNNING. + * This module reads; the migrations beside it write. Their own suites own that. + */ +describe('kernel:ready index pre-flight (#8725)', () => { + let db: DatabaseSync; + let exec: IndexExec; + + /** + * Every row of every table plus the schema, ordered — the LOGICAL state. + * + * ⚠️ Deliberately not a file hash. A raw hash over a SQLite file reports a + * difference after any read-write open (header churn: the change counter and + * the version-valid-for cookie move) and would accuse this pre-flight of + * mutating a database it only SELECTed from. What must not change is the + * schema and the rows, so that is what is compared. + */ + const logicalState = (): string => { + const schema = db + .prepare('SELECT type, name, sql FROM sqlite_master ORDER BY type, name') + .all() as Array<{ type: string; name: string; sql: string | null }>; + const tables = schema + .filter((entry) => entry.type === 'table') + .map((entry) => entry.name); + const rows: Record = {}; + for (const table of tables) { + rows[table] = db.prepare(`SELECT * FROM ${table} ORDER BY id`).all(); + } + return JSON.stringify({ schema, rows }); + }; + + beforeEach(() => { + db = new DatabaseSync(':memory:'); + exec = async (sql: string) => db.prepare(sql).all(); + + // ── sys_view_definition: two ACTIVE shared views under one name ──── + // The #5839/#6417 tightening's live conflict — `owner` NULL and + // `organization_id` NULL both fold into their sentinel buckets, so the + // two rows collide under the NULL-safe key while the declared, + // NULL-distinct index admits them. + db.exec(`CREATE TABLE sys_view_definition ( + id TEXT PRIMARY KEY, name TEXT, organization_id TEXT, owner TEXT, state TEXT + );`); + const view = db.prepare( + 'INSERT INTO sys_view_definition (id, name, organization_id, owner, state) VALUES (?,?,?,?,?)', + ); + view.run('v1', 'crm_case.all_open', null, null, 'active'); + view.run('v2', 'crm_case.all_open', null, null, 'active'); + // The control for the ROW SCOPE: the same collision among ARCHIVED rows + // is legal — the index is partial — and must not be reported. + view.run('v3', 'crm_case.retired', null, null, 'archived'); + view.run('v4', 'crm_case.retired', null, null, 'archived'); + // A personal view that collides with nothing. + view.run('v5', 'crm_case.mine', 'org_x', 'usr_1', 'active'); + + // ── sys_metadata: two ACTIVE package-less overlays for one key ───── + db.exec(`CREATE TABLE sys_metadata ( + id TEXT PRIMARY KEY, type TEXT, name TEXT, organization_id TEXT, + package_id TEXT, state TEXT + );`); + const meta = db.prepare( + 'INSERT INTO sys_metadata (id, type, name, organization_id, package_id, state) VALUES (?,?,?,?,?,?)', + ); + meta.run('m1', 'view', 'crm_case.board', null, null, 'active'); + meta.run('m2', 'view', 'crm_case.board', null, null, 'active'); + // One DRAFT row, so the draft index is probed over real rows and comes + // back clear — the two states are independent indexes and a run that + // conflated them would report this as blocked too. + meta.run('m3', 'view', 'crm_case.board', null, null, 'draft'); + + // ⛔ `sys_setting` is deliberately NOT created: it is registered by the + // OPTIONAL `service-settings`, so an ordinary kernel reaches + // `kernel:ready` without it and the migration treats that as a silent + // no-op. The pre-flight has to say `table-absent`, never `clear`. + }); + + afterEach(() => { + db.close(); + }); + + const by = (index: string, results: RuntimeIndexPreflight[]): RuntimeIndexPreflight => { + const found = results.find((entry) => entry.index === index); + if (!found) throw new Error(`no pre-flight entry for '${index}'`); + return found; + }; + + it('reports the rows blocking each tightening, and says nothing about the ones that do not', async () => { + const results = await collectRuntimeIndexPreflight(exec, { client: 'better-sqlite3' }); + + expect(results.map((entry) => `${entry.index}:${entry.status}`)).toEqual([ + 'idx_sys_metadata_overlay_active:blocked', + 'idx_sys_metadata_overlay_draft:clear', + 'idx_sys_view_def_active:blocked', + 'uniq_sys_setting_organization_id_namespace_key_scope_user_id:table-absent', + ]); + + // The view-definition conflict, named row-for-row. Both NULL columns are + // reported through their own sentinel bucket, which is what the index + // actually keys on: `organization_id_key = '__global__'` reads as + // "organization_id IS NULL", `owner_key = ''` as "owner IS NULL". + expect(by('idx_sys_view_def_active', results).groups).toEqual([ + { + key: { name: 'crm_case.all_open', organization_id_key: '__global__', owner_key: '' }, + rowCount: 2, + }, + ]); + + // The overlay conflict — and here `organization_id` is BARE, because + // #6418 deliberately did not fold it. A NULL stays a NULL in the key. + expect(by('idx_sys_metadata_overlay_active', results).groups).toEqual([ + { + key: { + type: 'view', + name: 'crm_case.board', + organization_id: null, + package_id_key: '', + }, + rowCount: 2, + }, + ]); + + // ⭐ The archived pair is NOT reported. The index is partial, those rows + // are outside it, and a pre-flight that flagged them would send an + // operator to delete data nothing is refusing. + const viewKeys = JSON.stringify(by('idx_sys_view_def_active', results).groups); + expect(viewKeys).not.toContain('crm_case.retired'); + + expect(by('idx_sys_metadata_overlay_draft', results).groups).toEqual([]); + expect(by('uniq_sys_setting_organization_id_namespace_key_scope_user_id', results).groups).toEqual([]); + }); + + it('carries the row scope and key parts each migration actually builds', async () => { + const results = await collectRuntimeIndexPreflight(exec); + + expect(by('idx_sys_view_def_active', results)).toMatchObject({ + migration: 'ensureViewDefinitionActiveIndex', + table: 'sys_view_definition', + rowScope: "state = 'active'", + keyParts: ['name', "COALESCE(organization_id, '__global__')", "COALESCE(owner, '')"], + }); + expect(by('idx_sys_metadata_overlay_draft', results)).toMatchObject({ + migration: 'ensureMetadataOverlayIndexes', + table: 'sys_metadata', + rowScope: "state = 'draft'", + keyParts: ['type', 'name', 'organization_id', "COALESCE(package_id, '')"], + }); + // `sys_setting` has no lifecycle column: the declaration means every row. + expect(by('uniq_sys_setting_organization_id_namespace_key_scope_user_id', results).rowScope).toBeNull(); + }); + + it('issues the OWNING migration\'s own statements, never a second spelling of the key', () => { + const probes = runtimeIndexProbes(); + const sql = Object.fromEntries(probes.map((probe) => [probe.index, probe.duplicateSql])); + + expect(sql['idx_sys_metadata_overlay_active']).toBe(buildOverlayDuplicateProbeSql('active')); + expect(sql['idx_sys_metadata_overlay_draft']).toBe(buildOverlayDuplicateProbeSql('draft')); + expect(sql['idx_sys_view_def_active']).toBe(buildViewActiveDuplicateProbeSql()); + expect(sql['uniq_sys_setting_organization_id_namespace_key_scope_user_id']).toBe( + buildSysSettingDuplicateProbeSql(), + ); + // The presence question is the migration's own too, so this cannot come + // to a different verdict about the table than the migration does. + expect( + probes.find((p) => p.table === 'sys_setting')!.presenceSql, + ).toBe(buildSysSettingPresenceSql()); + }); + + it('spells the sys_setting probe for MySQL, where the bare form is a parse error', () => { + const mysql = runtimeIndexProbes({ client: 'mysql2' }); + const setting = mysql.find((probe) => probe.table === 'sys_setting')!; + // `key` is RESERVED on MySQL — the bare statement is ERROR 1064 there + // (#9434), which is why the migration ships a second spelling and this + // picks it up rather than compiling a third. + expect(setting.duplicateSql).toBe(buildSysSettingDuplicateProbeSqlMysql()); + expect(setting.duplicateSql).toContain('`key`'); + // …and the platform's own spelling everywhere else. + expect(runtimeIndexProbes({ client: 'mysql' }).find((p) => p.table === 'sys_setting')!.duplicateSql).toBe( + buildSysSettingDuplicateProbeSqlMysql(), + ); + expect(runtimeIndexProbes({ client: 'pg' }).find((p) => p.table === 'sys_setting')!.duplicateSql).toBe( + buildSysSettingDuplicateProbeSql(), + ); + }); + + it('writes NOTHING — schema and every row byte-identical across a full run', async () => { + const before = logicalState(); + const results = await collectRuntimeIndexPreflight(exec, { client: 'better-sqlite3' }); + // The run really did something, so the comparison below is not vacuous. + expect(results.some((entry) => entry.status === 'blocked')).toBe(true); + expect(logicalState()).toEqual(before); + }); + + it('a seam that accepts every statement and answers none is unreadable, never absent', async () => { + // `InMemoryDriver.execute()` shape (#10677): it neither throws nor is + // missing, it just returns `null`. Read through the per-index presence + // question alone that would be four `table-absent` entries — a clean + // bill of health from a probe that never ran. + const noop: IndexExec = async () => null; + const results = await collectRuntimeIndexPreflight(noop); + + expect(results.length).toBeGreaterThan(0); + expect(new Set(results.map((entry) => entry.status))).toEqual(new Set(['unreadable'])); + }); + + it('reports a probe that throws as unreadable, with the driver\'s own message', async () => { + const failing: IndexExec = async (sql: string) => { + if (sql.includes('sys_view_definition') && sql.includes('GROUP BY')) { + throw new Error('database is locked'); + } + return db.prepare(sql).all(); + }; + const results = await collectRuntimeIndexPreflight(failing); + + expect(by('idx_sys_view_def_active', results)).toMatchObject({ + status: 'unreadable', + detail: 'database is locked', + groups: [], + }); + // …and one unreadable index never aborts the rest of the inventory. + expect(by('idx_sys_metadata_overlay_active', results).status).toBe('blocked'); + }); +}); diff --git a/packages/metadata-protocol/src/migrations/runtime-index-preflight.ts b/packages/metadata-protocol/src/migrations/runtime-index-preflight.ts new file mode 100644 index 0000000000..23c46a7367 --- /dev/null +++ b/packages/metadata-protocol/src/migrations/runtime-index-preflight.ts @@ -0,0 +1,353 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The `kernel:ready` index migrations' duplicate pre-flight, read-only (#8725). + * + * ## The gap this closes + * + * Three migrations in this directory tighten an existing UNIQUE index into a + * NULL-safe (and sometimes row-scoped) form at `kernel:ready`: + * + * | migration | table | index(es) | + * |---|---|---| + * | `ensureMetadataOverlayIndexes` | `sys_metadata` | active + draft | + * | `ensureViewDefinitionActiveIndex` | `sys_view_definition` | active | + * | `ensureSysSettingIdentityIndex` | `sys_setting` | row identity | + * + * Each is a **tightening**, so rows the previous index admitted can block the + * build. When that happens the migration refuses (ADR-0120 D4: the previous + * index stays, no row is touched) and reports at `error` on the boot channel. + * + * That report was the ONLY channel. `os migrate plan` cannot carry it, and not + * by omission — by construction, twice over: + * + * - **after** the tightening runs, `isRuntimeManagedIndex` in + * `driver-sql`'s `schema-drift.ts` excludes the index, because + * `isSyncReproducibleIndex` is false for a partial index and for any key + * part that is a `COALESCE` over a NON-tenant column. That exclusion is + * correct — without it a boot would propose rebuilding away the guarantee it + * had just created; + * - **before** it runs, there is no drift to see either: each migration + * deliberately REUSES the declared index's name (see each module's "Why the + * index REUSES the declared name"), so the differ's name-matched slot reads + * as filled whichever physical form is actually there. + * + * Measured end to end before this module existed: a database carrying the same + * duplicate damage twice — once under a DECLARED organization-unique index and + * once under `sys_view_definition`'s runtime one — produced an `os migrate + * plan` that named the declared one in full and said nothing whatsoever about + * the runtime one. The control is what makes that evidence rather than a + * reading: a fixture that simply failed to carry damage would have been silent + * on both. + * + * ## Why the reporting path is `os migrate duplicates` + * + * Maintainer ruling, 2026-08-22 — a probe on `os migrate duplicates`, "which + * already boots read-only and owns the 'inventory, never repair' contract", + * surfaces the blocking rows before the operator restarts the server, **keeping + * `os migrate plan`'s drift contract untouched**. `plan` describes work + * `os migrate apply` will do; this work is applied by the next SERVING boot, by + * a different applier, and a `plan` line an operator reads as "apply will + * handle it" would be a promise `apply` cannot keep. + * + * ## What this module is, and what it deliberately is not + * + * It **describes and reads**. Every statement it issues is a `SELECT`; it + * creates nothing, drops nothing and repairs nothing — the same "inventory, + * never repair" contract the command it feeds already carries, and the reason + * the whole run is safe to point at production. + * + * ⚠️ It also changes **nothing** about when a migration runs or what it does. + * A pre-flight that armed, deferred or altered a tightening would be a + * different decision with a different ceremony; this one only makes the + * refusal's evidence readable one command earlier. + * + * Every probe statement comes from the migration that owns the index — never a + * copy. That is the whole reason the descriptors below are assembled here + * rather than in the CLI: a second spelling of a key is a second definition of + * what "duplicate" means for that index, and the two would drift apart on the + * first re-keying. + */ + +import { isResultSet, normalizeRows } from './seed-tenancy-backfill.js'; +import type { IndexExec } from './partial-index-probe.js'; +import { + OVERLAY_INDEX_NAMES, + OVERLAY_TABLE, + buildOverlayDuplicateProbeSql, + overlayIndexKeyParts, + type OverlayIndexState, +} from './overlay-index.js'; +import { + SYS_SETTING_IDENTITY_INDEX_NAME, + SYS_SETTING_TABLE, + buildSysSettingDuplicateProbeSql, + buildSysSettingDuplicateProbeSqlMysql, + buildSysSettingPresenceSql, + sysSettingIdentityKeyParts, +} from './sys-setting-identity-index.js'; +import { + VIEW_ACTIVE_INDEX_NAME, + VIEW_DEFINITION_TABLE, + buildDuplicateProbeSql as buildViewActiveDuplicateProbeSql, + viewActiveIndexKeyParts, +} from './view-definition-active-index.js'; + +/** The column every one of the three duplicate-listing queries counts into. */ +const DUPLICATE_ROWS_COLUMN = 'duplicate_rows'; + +/** + * One index a `kernel:ready` migration tightens, and how to ask a database + * whether anything currently blocks it. + * + * Everything here is derived from the owning migration's own exported builders, + * so this descriptor cannot describe a key the migration does not build. + */ +export interface RuntimeIndexProbe { + /** The exported migration function that builds this index. */ + migration: string; + table: string; + /** The index name — the DECLARED one, which each migration reuses on purpose. */ + index: string; + /** The index's key parts, in key order, in the NULL-safe spelling it is built with. */ + keyParts: string[]; + /** The row subset the index covers, or `null` when it covers every row. */ + rowScope: string | null; + /** Cheapest statement that answers "is this table here?" without reading a row. */ + presenceSql: string; + /** The migration's own `GROUP BY … HAVING COUNT(*) > 1` listing. */ + duplicateSql: string; +} + +/** + * What a pre-flight found for one index. + * + * `status` separates the four outcomes that must never read the same: + * + * - `blocked` — rows collide under the key. The migration will refuse on the + * next serving boot and leave the previous index in place. + * - `clear` — the probe ran and nothing collides. + * - `table-absent` — the table is not on this install. A no-op, never a + * finding: `sys_setting` is registered by the OPTIONAL `service-settings`, + * so an ordinary kernel reaches `kernel:ready` with no such table and the + * migration itself treats that as a silent no-op. + * - `unreadable` — the probe could not run, and `detail` says why. Reported + * because "found nothing" and "never looked" must not read the same — the + * rule `os migrate duplicates` already applies to its own `skipped` list. + */ +export interface RuntimeIndexPreflight extends RuntimeIndexProbe { + status: 'blocked' | 'clear' | 'table-absent' | 'unreadable'; + /** One entry per colliding key group; empty unless `status` is `blocked`. */ + groups: RuntimeIndexDuplicateGroup[]; + /** The driver's own message, when the probe could not run. */ + detail?: string; +} + +/** One key value held by more rows than the tightened index would admit. */ +export interface RuntimeIndexDuplicateGroup { + /** + * The key, exactly as the migration's query projects it: a bare column + * name, or `_key` for a column whose NULLs the index folds into a + * sentinel bucket. Neither sentinel can occur in real data, so + * `organization_id_key = '__global__'` reads as "organization_id IS NULL". + */ + key: Record; + /** How many rows hold it. */ + rowCount: number; +} + +/** `SELECT 1 FROM WHERE 1 = 0` — reads no row, writes nothing. */ +function buildPresenceSql(table: string): string { + return `SELECT 1 FROM ${table} WHERE 1 = 0`; +} + +/** MySQL and MariaDB, the one dialect that needs a differently-spelled probe. */ +function isMysqlClient(client?: string): boolean { + const c = String(client ?? '').toLowerCase(); + return c === 'mysql' || c === 'mysql2'; +} + +/** + * Every index the three `kernel:ready` migrations tighten — FOUR, from three + * migrations, because `ensureMetadataOverlayIndexes` builds one index per + * overlay state and either can be blocked independently. + * + * ## Why one arm takes a dialect and three do not + * + * `sys_setting`'s listing query is the only one whose bare spelling is not + * merely unidiomatic on MySQL but a parse error: `key` is a RESERVED word + * there, measured as `ERROR 1064` on MySQL 8.0.46 (#9434), which is why the + * migration already ships a MySQL-spelled variant. The other three queries name + * no MySQL-reserved identifier, so the platform's own spelling — the one the + * migration prints in its boot report — runs on all three dialects, and + * compiling a second variant of them would buy nothing and add a second + * spelling of the same key. + */ +export function runtimeIndexProbes(opts: { client?: string } = {}): RuntimeIndexProbe[] { + const overlay = (state: OverlayIndexState): RuntimeIndexProbe => ({ + migration: 'ensureMetadataOverlayIndexes', + table: OVERLAY_TABLE, + index: OVERLAY_INDEX_NAMES[state], + keyParts: overlayIndexKeyParts(), + rowScope: `state = '${state}'`, + presenceSql: buildPresenceSql(OVERLAY_TABLE), + duplicateSql: buildOverlayDuplicateProbeSql(state), + }); + return [ + overlay('active'), + overlay('draft'), + { + migration: 'ensureViewDefinitionActiveIndex', + table: VIEW_DEFINITION_TABLE, + index: VIEW_ACTIVE_INDEX_NAME, + keyParts: viewActiveIndexKeyParts(), + rowScope: "state = 'active'", + presenceSql: buildPresenceSql(VIEW_DEFINITION_TABLE), + duplicateSql: buildViewActiveDuplicateProbeSql(), + }, + { + migration: 'ensureSysSettingIdentityIndex', + table: SYS_SETTING_TABLE, + index: SYS_SETTING_IDENTITY_INDEX_NAME, + keyParts: sysSettingIdentityKeyParts(), + // No `WHERE`: `sys_setting` has no lifecycle column and the + // declaration means every row. + rowScope: null, + // The migration's OWN presence statement, not a local rebuild of the + // same shape — it is the question `ensureSysSettingIdentityIndex` + // asks before it does anything, and asking a different one here + // could classify a table it can see as absent, or the reverse. + presenceSql: buildSysSettingPresenceSql(), + duplicateSql: isMysqlClient(opts.client) + ? buildSysSettingDuplicateProbeSqlMysql() + : buildSysSettingDuplicateProbeSql(), + }, + ]; +} + +/** + * Sort key for one group, so an archived report diffs against the next one. + * + * `JSON.stringify` over the entry pairs rather than a joined string with a + * separator literal: the key values are user data, and any separator character + * chosen here could occur inside one of them and reorder two groups that differ. + */ +function groupSortKey(group: RuntimeIndexDuplicateGroup): string { + return JSON.stringify(Object.entries(group.key)); +} + +/** + * The one statement every working SQL seam answers, on every dialect this + * platform supports — no table, no row, nothing written. + * + * It separates the two failures the per-probe presence question below cannot + * tell apart. Reading "the presence SELECT did not answer" as "the table is not + * here" is right when the seam works, and catastrophic when it does not: a seam + * that accepts every statement and answers none of them would report all four + * tightenings as `table-absent` — a clean bill of health from a probe that never + * ran, which is the #10677 defect `os migrate duplicates` already closed on its + * own scan. So liveness is established once, first, against a statement whose + * failure cannot mean "absent". + */ +const SEAM_LIVENESS_SQL = 'SELECT 1 AS os_preflight_probe'; + +/** + * The #10677 discriminator, applied here: a seam that ANSWERS returns a result + * set, and a seam that cannot answer returns no result set at all — it need not + * throw, and the one measured in this repo does not (`InMemoryDriver.execute()` + * logs and returns `null`). {@link isResultSet} is the sibling migration's own + * test for that, imported rather than copied: this is its third caller inside + * this package and a second spelling of "did the driver answer" is exactly what + * lets one of them drift into reading `null` as zero rows. + */ +const SEAM_NO_ANSWER_DETAIL = + 'the raw-SQL seam returned no result set — a seam that cannot answer is not a seam that answered "no rows"'; + +/** + * Run one probe. + * + * The presence question is asked FIRST and its refusal is read as absence — the + * same width `ensureSysSettingIdentityIndex` uses, and for the same reason: on a + * host where the framework cannot even `SELECT` from the table it certainly + * cannot rebuild that table's index, and one unactionable finding per run is how + * the actionable ones stop being read. The seam itself has already been proved + * live by {@link SEAM_LIVENESS_SQL}, so "absent" here really is about the table. + */ +async function runProbe(exec: IndexExec, probe: RuntimeIndexProbe): Promise { + const unreadable = (detail: string): RuntimeIndexPreflight => ({ + ...probe, + status: 'unreadable', + groups: [], + detail, + }); + + try { + // A seam already proved live that now answers nothing for THIS statement + // is a seam that stopped answering, not a table that is not there. + if (!isResultSet(await exec(probe.presenceSql))) return unreadable(SEAM_NO_ANSWER_DETAIL); + } catch { + return { ...probe, status: 'table-absent', groups: [] }; + } + + let rows: Array>; + try { + const result = await exec(probe.duplicateSql); + if (!isResultSet(result)) return unreadable(SEAM_NO_ANSWER_DETAIL); + rows = normalizeRows(result); + } catch (error) { + return unreadable(error instanceof Error ? error.message : String(error)); + } + + const groups: RuntimeIndexDuplicateGroup[] = rows.map((row) => { + const key: Record = {}; + for (const [column, value] of Object.entries(row)) { + if (column === DUPLICATE_ROWS_COLUMN) continue; + key[column] = value == null ? null : String(value); + } + const count = Number(row[DUPLICATE_ROWS_COLUMN]); + return { key, rowCount: Number.isFinite(count) ? count : 0 }; + }); + groups.sort((a, b) => groupSortKey(a).localeCompare(groupSortKey(b))); + + return { ...probe, status: groups.length > 0 ? 'blocked' : 'clear', groups }; +} + +/** + * Pre-flight every `kernel:ready` index tightening against a live database. + * + * Read-only from end to end, and sequential on purpose: this runs inside a + * command an operator may point at production, and four `SELECT`s in a row cost + * nothing worth parallelising a shared seam for. + * + * A probe that fails never aborts the run — an inventory that stopped at the + * first unreadable table would be one the operator cannot trust to be complete, + * which is the same rule `collectDuplicateIdentifierReport` applies to its own + * targets. + */ +export async function collectRuntimeIndexPreflight( + exec: IndexExec, + opts: { client?: string } = {}, +): Promise { + const probes = runtimeIndexProbes(opts); + + let seamFailure: string | undefined; + try { + if (!isResultSet(await exec(SEAM_LIVENESS_SQL))) seamFailure = SEAM_NO_ANSWER_DETAIL; + } catch (error) { + seamFailure = error instanceof Error ? error.message : String(error); + } + if (seamFailure !== undefined) { + return probes.map((probe) => ({ + ...probe, + status: 'unreadable' as const, + groups: [], + detail: seamFailure, + })); + } + + const results: RuntimeIndexPreflight[] = []; + for (const probe of probes) { + results.push(await runProbe(exec, probe)); + } + return results; +} diff --git a/packages/metadata-protocol/src/migrations/sys-setting-identity-index.test.ts b/packages/metadata-protocol/src/migrations/sys-setting-identity-index.test.ts index 03cef6204e..612c7edac3 100644 --- a/packages/metadata-protocol/src/migrations/sys-setting-identity-index.test.ts +++ b/packages/metadata-protocol/src/migrations/sys-setting-identity-index.test.ts @@ -292,7 +292,9 @@ describe('sys_setting row-identity uniqueness (#8629)', () => { expect(message).toContain('The previous index is left in place'); expect(message).toContain('no row is discarded automatically'); expect(message).toContain(buildSysSettingDuplicateProbeSql()); - expect(message).toContain('os migrate plan'); + expect(message).toContain('os migrate duplicates'); + // The repointing (#8725) is only done if the FALSE referral is gone. + expect(message).not.toContain('os migrate plan'); // #9434 stays out of THIS arm: a conflict means real rows blocked a // build the server was willing to attempt, which only SQLite and // PostgreSQL ever are. MySQL refuses the statement outright and diff --git a/packages/metadata-protocol/src/migrations/sys-setting-identity-index.ts b/packages/metadata-protocol/src/migrations/sys-setting-identity-index.ts index f7dd9d0cfc..d87258f141 100644 --- a/packages/metadata-protocol/src/migrations/sys-setting-identity-index.ts +++ b/packages/metadata-protocol/src/migrations/sys-setting-identity-index.ts @@ -125,7 +125,17 @@ * ADR-0120 D4's full disposition in one `error` line: the key that is not * enforced, the consequence, and the exact query that lists the offending rows * ({@link buildSysSettingDuplicateProbeSql}) so the list does not depend on - * reaching for `os migrate plan` first. + * reaching for `os migrate duplicates` first. + * + * ⚠️ That referral named `os migrate plan` until #8725, and it was FALSE. As + * the section above records, this index is invisible to the drift differ by + * construction — `isSyncReproducibleIndex` rejects its non-tenant `COALESCE` + * key part, so `isRuntimeManagedIndex` is true and the differ skips it — which + * is exactly why `detectManagedDrift()` returning `[]` is the measured, correct + * behaviour. A command that reports drift can therefore never report these + * rows. Maintainer ruling 2026-08-22 routes the class to `os migrate + * duplicates`, which boots read-only and owns the "inventory, never repair" + * contract, and leaves `os migrate plan`'s drift contract untouched. * * ## Why it probes for the TABLE first * @@ -294,7 +304,12 @@ export function buildSysSettingPresenceSql(): string { /** * The query that lists the rows blocking the tightening — ADR-0120 D4's "name * the offending rows", shipped inside the conflict report so an operator has the - * list from the boot log, without waiting for `os migrate plan`. + * list from the boot log, without waiting for `os migrate duplicates`. + * + * The same statement is what `os migrate duplicates` issues for this index + * (#8725) — the command reads this builder rather than restating the key, so + * the pre-flight and the boot report cannot describe different duplicates. The + * MySQL variant below travels with it for the same reason. * * It GROUPs by exactly the index's own key parts, so what it reports and what * the index rejects cannot diverge — the projection and the `GROUP BY` are built @@ -538,7 +553,7 @@ function reportDegradation( `place, so (${columns}) is enforced only as far as it was before, and the NULL-safe key is NOT ` + `enforced until the duplicates are resolved: settings rows are admin-authored configuration, so ` + `no row is discarded automatically and this migration will keep refusing until an operator ` + - `decides which row survives. List them with: ${duplicateQuery} — or run "os migrate plan" — ` + + `decides which row survives. List them with: ${duplicateQuery} — or run "os migrate duplicates" — ` + `then restart (ADR-0120 D4, #8629).`, detail, ); diff --git a/packages/metadata-protocol/src/migrations/view-definition-active-index.test.ts b/packages/metadata-protocol/src/migrations/view-definition-active-index.test.ts index af90d0400c..1db243327d 100644 --- a/packages/metadata-protocol/src/migrations/view-definition-active-index.test.ts +++ b/packages/metadata-protocol/src/migrations/view-definition-active-index.test.ts @@ -379,7 +379,9 @@ describe('sys_view_definition active-row uniqueness (#5839) on a NULL-safe key ( expect(logger.error).toHaveBeenCalledTimes(1); const msg = String(logger.error.mock.calls[0]![0]); expect(msg).toContain("COALESCE(owner, '')"); - expect(msg).toContain('os migrate plan'); + expect(msg).toContain('os migrate duplicates'); + // The repointing (#8725) is only done if the FALSE referral is gone. + expect(msg).not.toContain('os migrate plan'); expect(msg).toContain(buildDuplicateProbeSql()); // …and that shipped query really does name the offending rows, on this @@ -425,7 +427,9 @@ describe('sys_view_definition active-row uniqueness (#5839) on a NULL-safe key ( expect(logger.error).toHaveBeenCalledTimes(1); const msg = String(logger.error.mock.calls[0]![0]); expect(msg).toContain("COALESCE(organization_id, '__global__')"); - expect(msg).toContain('os migrate plan'); + expect(msg).toContain('os migrate duplicates'); + // The repointing (#8725) is only done if the FALSE referral is gone. + expect(msg).not.toContain('os migrate plan'); }); /** diff --git a/packages/metadata-protocol/src/migrations/view-definition-active-index.ts b/packages/metadata-protocol/src/migrations/view-definition-active-index.ts index f1a17abe28..51002613bd 100644 --- a/packages/metadata-protocol/src/migrations/view-definition-active-index.ts +++ b/packages/metadata-protocol/src/migrations/view-definition-active-index.ts @@ -114,7 +114,18 @@ * It is handled the way ADR-0120 D4 requires: the PREVIOUS index stays in * place (never a table with no unique index at all), the report names the key * that is not enforced, ships the exact query that lists the offending rows, - * points at `os migrate plan`, and the boot continues. + * points at `os migrate duplicates`, and the boot continues. + * + * ⚠️ The referral used to name `os migrate plan`, and it was FALSE (#8725): + * `plan` reports drift, and this index is invisible to the differ by + * construction — excluded by `isRuntimeManagedIndex` once the partial form is + * built, and indistinguishable from its own declaration before that, because + * the migration reuses the declared NAME on purpose (see above). Measured with + * a matched control: one database carrying the same duplicate damage under a + * declared index and under this one, `plan` named the declared one in full and + * said nothing about this one. Maintainer ruling 2026-08-22 routes this class + * to `os migrate duplicates`, which boots read-only and owns the "inventory, + * never repair" contract, and leaves `plan`'s drift contract untouched. */ import { @@ -223,7 +234,11 @@ export function buildActiveIndexSql(indexName: string): string { /** * The query that lists the rows blocking the tightening — ADR-0120 D4's * "name the offending rows", shipped inside the conflict report so an operator - * has it without waiting for `os migrate plan`. + * has it without waiting for `os migrate duplicates`. + * + * The same statement is what `os migrate duplicates` issues for this index + * (#8725): the command reads this builder rather than restating the key, so the + * pre-flight and the boot report can never describe different duplicates. * * It GROUPs by exactly the index's own key parts, so what it reports and what * the index rejects cannot diverge — the projection and the `GROUP BY` are @@ -378,14 +393,16 @@ function reportDegradation( // — now block the build. ADR-0120 D4's disposition, in full: keep the // previous index (never an unconstrained table), name the key that is // not enforced, hand over the exact query that lists the offending - // rows, point at `os migrate plan`, and let the boot continue. + // rows, point at `os migrate duplicates`, and let the boot continue. + // The referral is repointed rather than dropped (#8725): the rows ARE + // reportable before a restart, just not by the drift differ. logProblem( logger, `[metadata-protocol] cannot tighten '${VIEW_ACTIVE_INDEX_NAME}' on "${VIEW_DEFINITION_TABLE}" — ` + `existing rows violate (${keyParts}) among state='active'. The previous index is left in ` + `place, so (${columns}) is enforced only as far as it was before; the NULL-safe key is NOT ` + `enforced until the duplicates are resolved. List them with: ${buildDuplicateProbeSql()} — or ` + - `run "os migrate plan" — then restart (ADR-0120 D4, #6417).`, + `run "os migrate duplicates" — then restart (ADR-0120 D4, #6417, #8725).`, detail, ); return;