From f7a0d2fc29698ba8c4d9f6ee41af3d065b07cc32 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 22 Aug 2026 10:50:52 +0000 Subject: [PATCH] feat(cli): `os migrate duplicates` reports the rows blocking the kernel:ready index tightenings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: `ensureMetadataOverlayIndexes` (`sys_metadata`, one index per overlay state), `ensureViewDefinitionActiveIndex` (`sys_view_definition`) and `ensureSysSettingIdentityIndex` (`sys_setting`). Each is a tightening, so rows an installation already holds can block it; the migration then refuses under ADR-0120 D4 — previous index kept, no row touched, boot continues — and reports at `error` on the boot channel. That channel was the only one, and not by omission. These indexes are invisible to the drift differ 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), and 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. A prior round measured it with a matched control — one database carrying the same duplicate damage under a declared organization-unique index and under `sys_view_definition`'s runtime one — and `os migrate plan` named the declared one in full while saying nothing whatsoever about the runtime one. Maintainer ruling, 2026-08-22: the reporting path is `os migrate duplicates`, which already boots read-only and owns the "inventory, never repair" contract, 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. What lands: * `@objectstack/metadata-protocol` gains `runtime-index-preflight.ts` — `runtimeIndexProbes()` and `collectRuntimeIndexPreflight()`. The descriptors read each migration's OWN exported builders rather than restating the keys, so the pre-flight and the boot report cannot describe different duplicates, and the `sys_setting` probe uses the migration's MySQL spelling on MySQL, where the bare form is ERROR 1064 on the reserved word `key`. * The report gains `runtimeIndexPreflight` (one entry per index: blocked/clear/table-absent/unreadable, a blocked one naming every colliding key group and its row count) plus `summary.runtimeIndexesBlocked` and `summary.runtimeIndexBlockingRows`. `reportVersion` moves 1 → 2: every version-1 field keeps its name, shape and meaning, and the bump says there is more in the document for a consumer that validates it strictly. * `collectDuplicateIdentifierReport`'s new option is REQUIRED rather than optional. An optional section defaults to `[]`, and `[]` is also what a clean database produces, so a caller that forgot to wire it would ship a clean bill of health from a probe that never ran — the #10677 failure, one section over. Required moves that mistake to a compile error. * Liveness is keyed on whether the seam returns a RESULT SET, reusing the sibling migration's `isResultSet` rather than copying it. A no-op seam would otherwise report all four tightenings as `table-absent`, which is that same failure again wearing a different status. * Nine referral sites repointed, not deleted (the ruling is explicit): three conflict-error strings that told the operator to "run `os migrate plan`" — an instruction the measurement proved false — and the six doc comments that state the same referral as part of the D4 disposition. Their pins now assert both that the new command is named and that the false one is gone. Nothing about a migration's behaviour changes: no tightening is armed, deferred or altered, and `plan` is untouched. The pre-flight only makes the refusal's evidence readable one command before the restart. Read-only is pinned LOGICALLY — schema plus every row, ordered — in both the new unit suite and the CLI integration test, never 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. Fixes #8725 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019bmVFqoQPq63zhKrxdYG1r --- ...grate-duplicates-kernel-ready-preflight.md | 62 +++ content/docs/deployment/cli.mdx | 53 ++- docs/qa/platform-checklist/areas/cli.json | 13 +- .../migrate/duplicates.contract.test.ts | 48 ++- .../migrate/duplicates.integration.test.ts | 105 +++++- .../migrate/duplicates.null-seam.test.ts | 14 +- .../migrate/duplicates.pre-repair.test.ts | 7 +- .../cli/src/commands/migrate/duplicates.ts | 101 ++++- packages/metadata-protocol/src/index.ts | 16 + .../src/migrations/overlay-index.test.ts | 8 +- .../src/migrations/overlay-index.ts | 23 +- .../runtime-index-preflight.test.ts | 252 +++++++++++++ .../src/migrations/runtime-index-preflight.ts | 353 ++++++++++++++++++ .../sys-setting-identity-index.test.ts | 4 +- .../migrations/sys-setting-identity-index.ts | 21 +- .../view-definition-active-index.test.ts | 8 +- .../view-definition-active-index.ts | 25 +- 17 files changed, 1071 insertions(+), 42 deletions(-) create mode 100644 .changeset/migrate-duplicates-kernel-ready-preflight.md create mode 100644 packages/metadata-protocol/src/migrations/runtime-index-preflight.test.ts create mode 100644 packages/metadata-protocol/src/migrations/runtime-index-preflight.ts 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 b2db97142e..add7f83e79 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;