diff --git a/.changeset/25134-migrate-plan-nullable-tenant-columns.md b/.changeset/25134-migrate-plan-nullable-tenant-columns.md new file mode 100644 index 00000000000..a2e2a1e4404 --- /dev/null +++ b/.changeset/25134-migrate-plan-nullable-tenant-columns.md @@ -0,0 +1,17 @@ +--- +'@objectstack/cli': patch +--- + +feat(cli): `os migrate plan` lists the tenant columns still nullable and names the voluntary organization-ownership ceremony; the ceremony reports a rotation shard already NOT NULL as such (#25134) + +Clause-②: yes (widening) — `os migrate plan --json` gains the `nullableTenantColumns` field, and the ceremony's `not-null` step gains the `already-not-null` outcome and the `alreadyNotNull` summary count. No exit code, drift entry, op, refusal or accepted input moves. + +Metadata-protocol: none + +A database the published 18.0.0 created is certified at birth with its tenant columns (`organization_id`) nullable, and `os migrate organization-ownership`, which tightens them to `NOT NULL`, is voluntary on it. Nothing told the operator that. + +- **`os migrate plan`: a new advisory section.** It lists every tenant column the database still holds nullable (table and column) and names `os migrate organization-ownership` as the remedy. Each column says whether the ceremony tightens it or leaves it, and why. The ceremony leaves a rotation shard created before shards were created `NOT NULL`, `sys_email_template` (its rows are mirrors the ceremony deletes, and the table retires), a table its inventory exempts, and every table on a database whose transport cannot rebuild a table (the Turso remote face). When the ceremony would tighten none of the listed columns, the section says so and does not offer it. A fresh install lists nothing. A column named `organization_id` on an object with tenancy disabled (`sys_sso_provider`) is not a tenant column and is not listed. +- **Advisory only.** It is not drift: it proposes no change, adds nothing to `total` or `changes`, and the exit code does not change. Under `--json` it is the separate field `nullableTenantColumns`: `{ status: 'read', adr, remedy, columns: [{ table, column, database?, ceremony: 'tightens' | 'leaves', reason? }] }`, or `{ status: 'unreadable', adr, detail }` when the plan could not read it. +- **The ceremony's `not-null` step.** A rotation shard that the driver created `NOT NULL` used to be reported as `unsupported`, with a note saying each new shard is created nullable. That has not been true since the driver began creating shards `NOT NULL`. Such a shard is now `already-not-null` (counted in `alreadyNotNull`) and gets no note. A shard rotated in before then stays `unsupported`, and its note says it is dropped when its period leaves the base table's retention. + +**What to do.** Nothing is required: the server starts either way. To tighten the columns the plan lists, run `os migrate organization-ownership --out PLAN`, review it, then run `os migrate organization-ownership --apply --plan-file PLAN --backup-taken`. diff --git a/content/docs/deployment/cli.mdx b/content/docs/deployment/cli.mdx index edc6603c569..81a1be5fad4 100644 --- a/content/docs/deployment/cli.mdx +++ b/content/docs/deployment/cli.mdx @@ -986,6 +986,37 @@ without `NODE_ENV=development` describes a production `os serve`, which keeps th objects in the primary unless `OS_TELEMETRY_DB` names a file. A `plan` against a sibling that does not exist yet lists its tables to create and leaves no file behind. +#### Tenant columns still nullable (advisory) + +`os migrate plan` lists every tenant column the database still holds nullable, in a +section of its own, and names `os migrate organization-ownership` as the remedy. A +database the published 18.0.0 created is certified at birth with its tenant columns +nullable, and the [organization-ownership ceremony](#os-migrate-organization-ownership) +that tightens them is voluntary on it: the server starts either way. A database the +ceremony upgraded keeps only the few the ceremony leaves, and each of those is listed +with the reason it stays nullable: + +```text +ℹ 2 tenant column(s) are still nullable (ADR-0131 D1). This is advisory: it is not drift, and nothing in this plan or in "os migrate apply" changes it. + • sys_activity__r20261020.organization_id — the ceremony leaves it nullable: a rotation shard of sys_activity, created before shards were created NOT NULL: the ceremony rebuilds no shard, and it is dropped when its period leaves sys_activity's retention + • sys_email_template.organization_id — the ceremony leaves it nullable: its rows are mirrors the ceremony deletes, and the table retires (ADR-0131 D10 fate 2, D13), so the ceremony does not tighten it + None of them is one the organization-ownership ceremony tightens; each line says why it stays nullable. +``` + +When the ceremony would tighten any listed column, the section's last line names its +two commands instead. On a Turso remote database every listed column is one the +ceremony leaves, because that transport cannot rebuild a table. A fresh install creates +every tenant column `NOT NULL` and the section prints nothing. A column named +`organization_id` on an object with tenancy disabled (`sys_sso_provider`) is not a +tenant column and is not listed. + +The section is advisory only. It is not drift: it proposes no change, adds nothing to +the change count, and does not change the exit code. Under `--json` it is the separate +`nullableTenantColumns` field: `{ status: "read", adr, remedy, columns }`, where each +column is `{ table, column, database?, ceremony: "tightens" | "leaves", reason? }`. It is +`{ status: "unreadable", detail }` when the plan could not read it; `columns: []` means +it read and found none. + #### Nothing is applied before you confirm Both commands boot your app to read its metadata. That boot writes no row and no diff --git a/content/docs/upgrading.mdx b/content/docs/upgrading.mdx index c8e044a8b0a..a770667ae3c 100644 --- a/content/docs/upgrading.mdx +++ b/content/docs/upgrading.mdx @@ -354,6 +354,11 @@ ceremony, under any tenancy posture. A v18 server serves a database it created itself or one the ceremony certified, and refuses any other at boot, naming the command. +**A database the published 18.0.0 created** carries a birth marker and is certified +with its tenant columns (`organization_id`) still nullable. `os migrate plan` lists +them, and the ceremony, which such a database does not require, tightens them to +`NOT NULL` when you run it. + **The sequence:** 1. Before the first 18.0 boot, run `os migrate security-catalog-overlays`. It diff --git a/packages/cli/src/commands/migrate/organization-ownership.ts b/packages/cli/src/commands/migrate/organization-ownership.ts index ea223b9e5d7..c281ce101b2 100644 --- a/packages/cli/src/commands/migrate/organization-ownership.ts +++ b/packages/cli/src/commands/migrate/organization-ownership.ts @@ -865,6 +865,14 @@ export default class OrganizationOwnershipCeremony extends Command { const { StorageNameMapping } = await import('@objectstack/spec/system'); const { rotationBaseOf } = await import('../../utils/unmanaged-tables.js'); const { applyNotNullThroughDrift } = await import('../../utils/organization-ownership-not-null.js'); + const { readColumnNullability } = await import('../../utils/nullable-tenant-columns.js'); + const { normalizeRows } = await import('@objectstack/metadata-protocol'); + const driverFor = (database: string | undefined): unknown => + database === undefined + ? liveStack.driver + : database === TELEMETRY_DATASOURCE && liveStack.telemetryDatabase + ? liveStack.drivers[1] + : undefined; const declared = new Set( (liveStack.allObjects() as Array<{ name?: unknown } | null>) .map((o) => o?.name) @@ -875,12 +883,13 @@ export default class OrganizationOwnershipCeremony extends Command { { isDeclared: (table) => declared.has(rotationBaseOf(table) ?? table), shardBaseOf: (table) => rotationBaseOf(table) ?? null, - driverFor: (database) => - (database === undefined - ? liveStack.driver - : database === TELEMETRY_DATASOURCE && liveStack.telemetryDatabase - ? liveStack.drivers[1] - : undefined) as never, + // [ADR-0131 D1] A shard the driver created NOT NULL is reported + // so, never as one the ceremony could not tighten. + isNotNull: async (table, column, database) => { + const read = await readColumnNullability(driverFor(database), [column], normalizeRows); + return 'detail' in read ? null : read.tables.get(table)?.get(column) ?? null; + }, + driverFor: (database) => driverFor(database) as never, }, columns, ); diff --git a/packages/cli/src/commands/migrate/plan.apply-limit.test.ts b/packages/cli/src/commands/migrate/plan.apply-limit.test.ts index b8bc9ce7f60..4c4326661f1 100644 --- a/packages/cli/src/commands/migrate/plan.apply-limit.test.ts +++ b/packages/cli/src/commands/migrate/plan.apply-limit.test.ts @@ -24,8 +24,9 @@ * 2. The footer offers `--allow-destructive` only for a destructive entry the * database can apply, and names how many listed entries it cannot. * 3. CONTROL: a driver that declares nothing reads exactly as before. - * 4. The `--json` document is the report, unchanged: the same keys, and each - * entry's message as the differ wrote it. + * 4. The `--json` document is the report, unchanged: the same keys (the + * ADR-0131 D1 `nullableTenantColumns` advisory among them, which these + * limits do not move), and each entry's message as the differ wrote it. * * `bootSchemaStack` is stubbed, so this runs without a database; the same * cases run against a real `sqld` in `apply.remote-face.integration.test.ts`. @@ -245,7 +246,7 @@ describe('a driver that declares what its apply door cannot perform', () => { const { payload, exitCode } = await run(['--json']); expect(exitCode).toBe(0); expect(Object.keys(payload).sort()).toEqual( - ['changes', 'database', 'databaseSource', 'duration', 'managedTables', 'pending', 'schemaSync', 'total', 'unmanagedTables'], + ['changes', 'database', 'databaseSource', 'duration', 'managedTables', 'nullableTenantColumns', 'pending', 'schemaSync', 'total', 'unmanagedTables'], ); expect(payload.changes.map((c: Entry) => c.message)).toEqual([DROP.message, TIGHTEN.message, RELAX.message]); }, CASE_TIMEOUT_MS); @@ -430,7 +431,7 @@ describe('#24925 — a database the organization-ownership ceremony has not veri const { payload, exitCode } = await run(['--json']); expect(exitCode).toBe(0); expect(Object.keys(payload).sort()).toEqual( - ['changes', 'database', 'databaseSource', 'duration', 'managedTables', 'pending', 'total', 'unmanagedTables'], + ['changes', 'database', 'databaseSource', 'duration', 'managedTables', 'nullableTenantColumns', 'pending', 'total', 'unmanagedTables'], ); expect(payload.changes.map((c: Entry) => c.message)).toEqual([INDEX.message, ORG_JOB.message, LEGACY.message]); expect(getService).not.toHaveBeenCalled(); diff --git a/packages/cli/src/commands/migrate/plan.nullable-tenant-columns.integration.test.ts b/packages/cli/src/commands/migrate/plan.nullable-tenant-columns.integration.test.ts new file mode 100644 index 00000000000..f8ad44276b8 --- /dev/null +++ b/packages/cli/src/commands/migrate/plan.nullable-tenant-columns.integration.test.ts @@ -0,0 +1,191 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect, afterAll } from 'vitest'; +import { spawnSync } from 'node:child_process'; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { dirname, join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import Database from 'better-sqlite3'; + +/** + * [ADR-0131 D1; ruling record 6328195425, letter A, item 2] `os migrate plan` + * lists the tenant columns a database still holds nullable, in an advisory + * section of its own that names `os migrate organization-ownership`. + * + * A database the published 18.0.0 created is certified with its tenant columns + * nullable; this one is brought to that shape by rebuilding a table a fresh + * boot created with its `organization_id` nullable — the 18.0.0 DDL — keeping + * its rows and indexes. + * + * What is pinned, through the public door (`serve`'s provision-and-exit, then + * the command, from the source entry): + * + * - a fresh store lists nothing (`columns: []`, read — not unreadable); + * - the 18.0.0-shaped store lists the nullable tenant column as one the + * ceremony tightens, and a rotation shard created nullable as one it leaves; + * - ⭐ it is advisory: the plan's `changes` and `total` are the fresh store's, + * byte for byte — no drift entry and no op is added — and the exit code is + * 0 in both faces; + * - the human face prints the section and the ceremony's two commands. + */ + +const HERE = dirname(fileURLToPath(import.meta.url)); +const CLI_ROOT = resolve(HERE, '..', '..', '..'); +const RUN_DEV = resolve(CLI_ROOT, 'bin', 'run-dev.js'); +const TSX = resolve(CLI_ROOT, '..', '..', 'node_modules', '.bin', 'tsx'); +const CASE_TIMEOUT_MS = 300_000; + +const APP_OBJECT = 'os25134_deal'; +const SHARD = `${APP_OBJECT}__r20261019`; + +const CONFIG = [ + 'export default {', + " manifest: { id: 'com.example.os25134advisory', name: 'advisory', version: '0.0.0', type: 'app' },", + " requires: ['auth'],", + ` objects: [{ name: '${APP_OBJECT}', fields: { title: { type: 'text' } } }],`, + '};', + '', +].join('\n'); + +const dirs: string[] = []; +afterAll(() => { + for (const d of dirs) { + try { rmSync(d, { recursive: true, force: true }); } catch { /* best-effort */ } + } +}); + +interface Fixture { + dir: string; + db: string; +} + +function fixture(): Fixture { + const dir = mkdtempSync(join(tmpdir(), 'os-25134-advisory-')); + dirs.push(dir); + mkdirSync(join(dir, 'dist'), { recursive: true }); + writeFileSync(join(dir, 'objectstack.config.ts'), CONFIG); + return { dir, db: join(dir, 'boot.db') }; +} + +/** The child's environment: the fixture's database, one file, nothing inherited that retargets it. */ +function childEnv(f: Fixture, extra: NodeJS.ProcessEnv = {}): NodeJS.ProcessEnv { + const env: NodeJS.ProcessEnv = { ...process.env }; + for (const key of [ + 'OS_DATABASE_URL', 'DATABASE_URL', 'TURSO_DATABASE_URL', 'OS_DATABASE_DRIVER', 'OS_HOME', + 'OS_TELEMETRY_DB', 'AUTH_SECRET', 'BETTER_AUTH_SECRET', 'NO_COLOR', + ]) { + delete env[key]; + } + return { + ...env, + OS_DATABASE_URL: `file:${f.db}`, + OS_ARTIFACT_PATH: join(f.dir, 'dist', 'objectstack.json'), + OS_CLOUD_URL: 'off', + OS_TELEMETRY_DISABLED: '1', + OS_TELEMETRY_DB: '0', + OS_AUTH_SECRET: 'os25134-advisory-secret-at-least-32-characters', + OS_LOG_LEVEL: 'warn', + NO_COLOR: '1', + ...extra, + }; +} + +/** `os serve`'s own provision-and-exit: a fresh store, the full composition schema-synced into it. */ +function serveBoot(f: Fixture): void { + const boot = spawnSync(TSX, [RUN_DEV, 'serve', '--no-server', '--no-ui', '--no-console', '--port', '0'], { + cwd: f.dir, env: childEnv(f, { OS_MIGRATE_AND_EXIT: '1' }), encoding: 'utf8', timeout: 240_000, + }); + expect(boot.status, `the serving boot did not provision the database:\n${(boot.stderr ?? '').slice(-6000)}`).toBe(0); +} + +function plan(f: Fixture, json: boolean): { exit: number | null; payload: any; text: string } { + const run = spawnSync(TSX, [RUN_DEV, 'migrate', 'plan', ...(json ? ['--json'] : [])], { + cwd: f.dir, env: childEnv(f), encoding: 'utf8', timeout: 240_000, + }); + let payload: any; + if (json) { + try { payload = JSON.parse(run.stdout); } catch { payload = { unparsed: `${run.stdout}\n${run.stderr}`.slice(-800) }; } + } + return { exit: run.status, payload, text: `${run.stdout}\n${run.stderr}` }; +} + +/** + * Rebuild `table` with its `organization_id` nullable — the shape the 18.0.0 + * driver created — keeping its rows and its indexes. + */ +function relaxTenantColumn(file: string, table: string): void { + const db = new Database(file); + try { + const { sql } = db.prepare("SELECT sql FROM sqlite_master WHERE type = 'table' AND name = ?").get(table) as { sql: string }; + const relaxed = sql.replace(/([`"]organization_id[`"][^,]*?)\s+not null/i, '$1'); + if (relaxed === sql) throw new Error(`${table}.organization_id is not NOT NULL in ${sql}`); + const indexes = (db.prepare("SELECT sql FROM sqlite_master WHERE type = 'index' AND tbl_name = ? AND sql IS NOT NULL").all(table) as Array<{ sql: string }>) + .map((i) => i.sql); + db.exec('PRAGMA foreign_keys = OFF'); + db.transaction(() => { + db.exec(`ALTER TABLE "${table}" RENAME TO "${table}__pre"`); + db.exec(relaxed); + db.exec(`INSERT INTO "${table}" SELECT * FROM "${table}__pre"`); + db.exec(`DROP TABLE "${table}__pre"`); + for (const index of indexes) db.exec(index); + })(); + } finally { + db.close(); + } +} + +function notNull(file: string, table: string): boolean { + const db = new Database(file, { readonly: true }); + try { + const col = (db.prepare(`SELECT "notnull" AS nn FROM pragma_table_info(?) WHERE name = 'organization_id'`).get(table)) as { nn: number } | undefined; + if (!col) throw new Error(`${table} carries no organization_id`); + return col.nn === 1; + } finally { + db.close(); + } +} + +describe('os migrate plan lists the nullable tenant columns as an advisory (ADR-0131 D1)', () => { + it('a fresh store lists nothing; an 18.0.0-shaped one lists them, and neither the drift nor the exit code moves', () => { + const f = fixture(); + serveBoot(f); + expect(notNull(f.db, APP_OBJECT), 'a table this release creates carries its tenant column NOT NULL').toBe(true); + + const fresh = plan(f, true); + expect(fresh.exit, JSON.stringify(fresh.payload).slice(0, 800)).toBe(0); + expect(fresh.payload.nullableTenantColumns).toEqual({ + status: 'read', adr: 'ADR-0131 D1', remedy: 'os migrate organization-ownership', columns: [], + }); + + // The 18.0.0 shape: the app's tenant column nullable, and a shard rotated in nullable. + relaxTenantColumn(f.db, APP_OBJECT); + expect(notNull(f.db, APP_OBJECT)).toBe(false); + const db = new Database(f.db); + try { db.exec(`CREATE TABLE "${SHARD}" ("id" varchar(255) PRIMARY KEY, "organization_id" varchar(255))`); } finally { db.close(); } + + const shaped = plan(f, true); + expect(shaped.exit, JSON.stringify(shaped.payload).slice(0, 800)).toBe(0); + expect(shaped.payload.nullableTenantColumns).toEqual({ + status: 'read', + adr: 'ADR-0131 D1', + remedy: 'os migrate organization-ownership', + columns: [ + { table: APP_OBJECT, column: 'organization_id', ceremony: 'tightens' }, + { table: SHARD, column: 'organization_id', ceremony: 'leaves', reason: expect.stringMatching(/^a rotation shard of os25134_deal/) }, + ], + }); + // ⭐ Advisory, not drift: the plan's changes are the fresh store's, and no + // entry names a tenant column. + expect(shaped.payload.changes).toEqual(fresh.payload.changes); + expect(shaped.payload.total).toBe(fresh.payload.total); + expect(shaped.payload.changes.filter((c: any) => c.column === 'organization_id')).toEqual([]); + + const human = plan(f, false); + expect(human.exit, human.text.slice(-2000)).toBe(0); + expect(human.text).toContain('2 tenant column(s) are still nullable (ADR-0131 D1). This is advisory: it is not drift'); + expect(human.text).toContain(`• ${APP_OBJECT}.organization_id\n`); + expect(human.text).toContain(`• ${SHARD}.organization_id — the ceremony leaves it nullable: a rotation shard of ${APP_OBJECT}`); + expect(human.text).toContain('tightens 1 of them: run "os migrate organization-ownership --out PLAN"'); + }, CASE_TIMEOUT_MS); +}); diff --git a/packages/cli/src/commands/migrate/plan.ts b/packages/cli/src/commands/migrate/plan.ts index 5d66675a324..f7367d2ac74 100644 --- a/packages/cli/src/commands/migrate/plan.ts +++ b/packages/cli/src/commands/migrate/plan.ts @@ -36,6 +36,11 @@ import { renderUnmanagedTables, type UnmanagedTablesReport, } from '../../utils/unmanaged-tables.js'; +import { + collectNullableTenantColumns, + renderNullableTenantColumns, + type NullableTenantColumnsReport, +} from '../../utils/nullable-tenant-columns.js'; import { describeOccupancy, JOURNAL_MODE_SWITCH_NOTE } from '../../utils/sqlite-occupancy.js'; import { describeDriverSqliteFile } from '../../utils/connection-display.js'; import { @@ -107,6 +112,15 @@ import { * `os migrate comment-reactions --apply` is recorded. Each gate's predicate * and read live in `utils/apply-drop-gates.ts`, which `apply` asks too; a held * drop names what it waits for, and the footer names each gate's command. + * + * [ADR-0131 D1; ruling record 6328195425, letter A] Every tenant column the + * database still holds nullable is listed in its own advisory section, with + * `os migrate organization-ownership` as the remedy: a database the published + * 18.0.0 created is certified with them nullable, and the ceremony that + * tightens them is voluntary there. Each column says whether the ceremony + * tightens it or leaves it, and why. It is not drift: it proposes no op, adds + * nothing to the change count, and changes no exit code; under `--json` it is + * the separate `nullableTenantColumns` field (`utils/nullable-tenant-columns.ts`). */ export default class MigratePlan extends Command { static override description = @@ -263,6 +277,18 @@ export default class MigratePlan extends Command { normalize: normalizeRows, }); + // [ADR-0131 D1] The tenant columns still nullable — advisory only, beside + // the drift and never in it. Computed before the "in sync" return below: + // an 18.0.0-born database has no drift and every such column. + const nullableTenantColumns: NullableTenantColumnsReport = await collectNullableTenantColumns({ + databases: distinctDatabaseDrivers(stack.drivers).map((driver) => ( + driver === stack.drivers[0] ? { driver } : { driver, database: stack.databaseOf(driver) ?? 'a planned database' } + )), + tenantDrivers: stack.drivers, + mirrored: stack.composition.servedBoot.mirrored, + normalize: normalizeRows, + }); + // [ADR-0120 D5e, advisory form] Installation-wide uniques on app objects // are a decision point under the `isolated` posture — organizations there // are separate CUSTOMERS. The HARD gate runs at app install; this covers @@ -302,6 +328,10 @@ export default class MigratePlan extends Command { // key on the clean case would make those two byte-identical, which // is the blind spot this section exists to remove. unmanagedTables, + // [ADR-0131 D1] Advisory, not drift: never counted in `total` and + // never in `changes`. Present whenever a SQL driver was found, so + // `columns: []` (read, none nullable) is not `unreadable`. + nullableTenantColumns, ...(uniqueScopeAdvisory.length > 0 ? { uniqueScopeAdvisory: { @@ -397,6 +427,16 @@ export default class MigratePlan extends Command { console.log(''); } + // [ADR-0131 D1] Its own block, above the drift verdict: an advisory about + // what the voluntary ceremony would tighten, never a change this plan or + // `os migrate apply` makes. Silent when every tenant column is NOT NULL. + const nullableLines = renderNullableTenantColumns(nullableTenantColumns); + if (nullableLines.length > 0) { + printInfo(nullableLines[0]!); + for (const line of nullableLines.slice(1)) console.log(chalk.dim(` ${line}`)); + console.log(''); + } + if (drift.length === 0 && pending.length === 0) { // [#13028] "In sync" is a claim about the objects this plan EXAMINED. // On a composed host that examined a strict subset — a control plane diff --git a/packages/cli/src/utils/nullable-tenant-columns.test.ts b/packages/cli/src/utils/nullable-tenant-columns.test.ts new file mode 100644 index 00000000000..9deb8ce77f2 --- /dev/null +++ b/packages/cli/src/utils/nullable-tenant-columns.test.ts @@ -0,0 +1,163 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The nullable-tenant-column advisory of `os migrate plan` over a REAL SQLite + * driver (ADR-0131 D1; ruling record 6328195425, letter A, item 2): + * + * - a tenant column the database holds nullable is listed, as one the + * ceremony tightens; one it holds NOT NULL (every table this release + * creates) is not; + * - a column named `organization_id` on an object that opts out of tenancy + * (`sys_sso_provider`) is not a tenant column, and is not listed; + * - the residue the ceremony leaves is listed apart, with why: a rotation + * shard created nullable (a shard created NOT NULL is not listed), a table + * whose rows are mirrors the ceremony deletes (`sys_email_template`), and a + * database whose apply door cannot tighten a column; + * - "could not look" is `unreadable`, never an empty list. + */ + +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { normalizeRows } from '@objectstack/metadata-protocol'; +import { + NULLABLE_TENANT_COLUMNS_REMEDY, + collectNullableTenantColumns, + renderNullableTenantColumns, + type NullableTenantColumnsReport, +} from './nullable-tenant-columns.js'; + +let dir: string; +let driver: SqlDriver; +let knex: any; + +const ORG = { type: 'lookup', reference: 'sys_organization', required: false }; + +/** Return a table's tenant column to the 18.0.0 shape — nullable — through the driver's own apply door. */ +async function relax(table: string): Promise { + const { applied } = await driver.applyMigrationEntries([ + { + kind: 'nullability_mismatch', remoteName: table, table, column: 'organization_id', category: 'needs_confirm', + op: { type: 'relax_not_null', table, column: 'organization_id' }, + } as never, + ]); + if (applied.length !== 1) throw new Error(`${table} was not returned to its 18.0.0 shape`); +} + +const nullable = async (table: string): Promise => (await knex(table).columnInfo()).organization_id.nullable; + +beforeEach(async () => { + dir = mkdtempSync(join(tmpdir(), 'os-d1-nullable-tenant-')); + driver = new SqlDriver({ client: 'better-sqlite3', connection: { filename: join(dir, 'db.sqlite') }, useNullAsDefault: true }); + (driver as any).logger = { warn: () => {}, info: () => {}, error: () => {}, debug: () => {} }; + knex = (driver as any).knex; + await driver.initObjects([ + { name: 'crm_deal', fields: { name: { type: 'text' }, organization_id: ORG } }, + { name: 'crm_note', fields: { body: { type: 'text' }, organization_id: ORG } }, + { name: 'sys_email_template', fields: { name: { type: 'text' }, organization_id: ORG } }, + { name: 'sys_sso_provider', tenancy: { enabled: false }, fields: { issuer: { type: 'text' }, organization_id: ORG } }, + ] as never); +}); + +afterEach(async () => { + await driver.disconnect?.(); + rmSync(dir, { recursive: true, force: true }); +}); + +const collect = (over: Partial[0]> = {}): Promise => + collectNullableTenantColumns({ databases: [{ driver }], tenantDrivers: [driver], mirrored: true, normalize: normalizeRows, ...over }); + +describe('the nullable-tenant-column advisory', () => { + it('a fresh store lists nothing: every tenant column is NOT NULL from create, and the opt-out is no tenant column', async () => { + expect(await nullable('crm_deal')).toBe(false); + expect(await nullable('sys_sso_provider')).toBe(true); + const report = await collect(); + expect(report).toEqual({ status: 'read', adr: 'ADR-0131 D1', remedy: NULLABLE_TENANT_COLUMNS_REMEDY, columns: [] }); + expect(renderNullableTenantColumns(report)).toEqual([]); + }); + + it('⭐ an 18.0.0-shaped store lists each nullable tenant column, as one the ceremony tightens, and names the ceremony', async () => { + await relax('crm_deal'); + await relax('crm_note'); + const report = await collect(); + expect(report).toEqual({ + status: 'read', + adr: 'ADR-0131 D1', + remedy: 'os migrate organization-ownership', + columns: [ + { table: 'crm_deal', column: 'organization_id', ceremony: 'tightens' }, + { table: 'crm_note', column: 'organization_id', ceremony: 'tightens' }, + ], + }); + const lines = renderNullableTenantColumns(report); + expect(lines[0]).toMatch(/^2 tenant column\(s\) are still nullable \(ADR-0131 D1\)\. This is advisory: it is not drift/); + expect(lines.slice(1, 3)).toEqual([' • crm_deal.organization_id', ' • crm_note.organization_id']); + expect(lines[3]).toContain('"os migrate organization-ownership --out PLAN"'); + }); + + it('the advisory proposes nothing: the same store\'s drift holds no entry for a tenant column', async () => { + await relax('crm_deal'); + expect((await collect()).status).toBe('read'); + const drift = (await driver.detectManagedDrift()).filter((d: any) => d.column === 'organization_id'); + expect(drift).toEqual([]); + }); + + it('the residue the ceremony leaves is listed apart, each with why, and the ceremony is not offered for it', async () => { + await relax('sys_email_template'); + await knex.schema.createTable('crm_deal__r20261019', (t: any) => { + t.string('id').primary(); + t.string('organization_id'); + }); + await knex.schema.createTable('crm_deal__r20261020', (t: any) => { + t.string('id').primary(); + t.string('organization_id').notNullable(); + }); + const report = await collect(); + if (report.status !== 'read') throw new Error(report.detail); + expect(report.columns).toEqual([ + { + table: 'crm_deal__r20261019', + column: 'organization_id', + ceremony: 'leaves', + reason: expect.stringMatching(/^a rotation shard of crm_deal, created before shards were created NOT NULL/), + }, + { + table: 'sys_email_template', + column: 'organization_id', + ceremony: 'leaves', + reason: expect.stringMatching(/mirrors the ceremony deletes, and the table retires/), + }, + ]); + const lines = renderNullableTenantColumns(report); + expect(lines.at(-1)).toBe('None of them is one the organization-ownership ceremony tightens; each line says why it stays nullable.'); + expect(lines.join('\n')).not.toContain('--apply'); + }); + + it('a database whose apply door cannot tighten a column leaves every one, saying so', async () => { + await relax('crm_deal'); + const limited = Object.assign(Object.create(driver), { + driftApplyLimit: (op: string) => (op === 'tighten_not_null' ? 'the remote face runs no SQLite table rebuild' : null), + }); + const report = await collect({ databases: [{ driver: limited, database: 'remote' }] }); + expect(report).toMatchObject({ + status: 'read', + columns: [{ + table: 'crm_deal', database: 'remote', ceremony: 'leaves', + reason: 'this database\'s apply door cannot tighten a column: the remote face runs no SQLite table rebuild', + }], + }); + }); + + it('"could not look" is unreadable, never an empty list', async () => { + await relax('crm_deal'); + expect(await collect({ mirrored: false })).toMatchObject({ status: 'unreadable', detail: expect.stringMatching(/did not compose/) }); + expect(await collect({ tenantDrivers: [{}] })).toMatchObject({ status: 'unreadable', detail: expect.stringMatching(/tenant-column map/) }); + expect(await collect({ databases: [{ driver: {} }] })).toMatchObject({ status: 'unreadable', detail: expect.stringMatching(/raw SQL seam/) }); + const report = await collect({ databases: [] }); + expect(renderNullableTenantColumns(report)).toEqual([ + 'Nullable tenant-column advisory did not run: the plan holds no SQL driver, so no column was read.', + ]); + }); +}); diff --git a/packages/cli/src/utils/nullable-tenant-columns.ts b/packages/cli/src/utils/nullable-tenant-columns.ts new file mode 100644 index 00000000000..a83cb538577 --- /dev/null +++ b/packages/cli/src/utils/nullable-tenant-columns.ts @@ -0,0 +1,261 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The nullable-tenant-column advisory of `os migrate plan` (ADR-0131 D1; + * ruling record 6328195425, letter A, item 2). + * + * A database the published 18.0.0 created is certified at birth with its + * tenant columns nullable, and the organization-ownership ceremony that + * tightens them is voluntary on it (D10: a manual, operator-run migration, + * never a boot step; the shape generation stays 1). A ceremonied database + * keeps the few the ceremony leaves. Nothing else tells the operator that, so + * the plan lists every tenant column still nullable and names the ceremony as + * the remedy. + * + * ⛔ Advisory only. It is not drift — the tenant column's nullability is the + * ceremony's, which is why the differ reads none for it + * (`ceremonyOwnedNullability` in `@objectstack/driver-sql`'s `schema-drift.ts`) + * — so it proposes no op, adds nothing to the plan's change count, and moves + * no exit code. + * + * The population is the column that differ excludes, read from the same + * place: each planned driver's tenant column per table (`tenantFieldByTable`, + * what `resolveTenantField` answers). An object that opts out of tenancy + * (`sys_sso_provider`, whose `organization_id` is better-auth's provider + * relation, not the tenancy anchor) has none, so it is not listed; nor is a + * table no object declares, which the unmanaged-table section reports. A + * rotation shard is read through its base. Nullability is one catalog query + * per database. + * + * Each column says whether the ceremony tightens it. The residue it leaves is + * marked with the reason, so a ceremonied database is not sent back to the + * ceremony for a column it will never tighten: a rotation shard created before + * shards were created NOT NULL, a table its inventory row exempts or whose rows + * are mirrors the ceremony deletes (`sys_email_template`), and every table on a + * transport whose apply door cannot tighten a column (the Turso remote face). + * A table still holding a NULL row the ceremony cannot attribute is not told + * apart here — the ceremony's own plan reports it — so this read stays one + * query. + */ + +import { dialectOfDriver } from '@objectstack/metadata-core'; +import { isResultSet } from '../commands/migrate/duplicates.js'; +import { inventoryEntryFor } from './organization-ownership-inventory.js'; +import { resolvePlannedDriverExec, rotationBaseOf } from './unmanaged-tables.js'; + +/** The command that tightens them: the ceremony's read-only plan, then its `--apply`. */ +export const NULLABLE_TENANT_COLUMNS_REMEDY = 'os migrate organization-ownership'; + +/** One tenant column the database holds nullable. */ +export interface NullableTenantColumn { + table: string; + column: string; + /** The database beside the primary that holds it, as the plan names it (`SchemaStack.databaseOf`); absent for the primary. */ + database?: string; + /** `tightens`: the ceremony applies NOT NULL to it. `leaves`: it does not, and {@link reason} says why. */ + ceremony: 'tightens' | 'leaves'; + reason?: string; +} + +export interface NullableTenantColumnsRead { + status: 'read'; + adr: 'ADR-0131 D1'; + remedy: typeof NULLABLE_TENANT_COLUMNS_REMEDY; + columns: NullableTenantColumn[]; +} + +export interface NullableTenantColumnsUnreadable { + status: 'unreadable'; + adr: 'ADR-0131 D1'; + detail: string; +} + +/** + * Present in the `--json` document whenever a SQL driver was found: + * `columns: []` is "read, none nullable" and `unreadable` is "never read" — + * the two must not be one shape. + */ +export type NullableTenantColumnsReport = NullableTenantColumnsRead | NullableTenantColumnsUnreadable; + +/** One database the plan covers. */ +export interface PlannedDatabase { + driver: unknown; + /** How the plan names it, beside the primary (`SchemaStack.databaseOf`); absent for the primary. */ + database?: string; +} + +/** A name the catalog query may carry as a string literal. */ +const SAFE_IDENTIFIER = /^[A-Za-z_][A-Za-z0-9_]*$/; + +/** + * The planned driver's tenant column per table — `SqlDriver.tenantFieldByTable`, + * the map `resolveTenantField` reads — or `null` when it cannot be read. The + * field is `protected` and reached structurally (as `readManagedTableNames` + * reaches `managedObjectFields`), so a rename hands this an `undefined`, which + * is "could not read", never "no tenant table". + */ +export function readTenantFields(driver: unknown): ReadonlyMap | null { + const map = (driver as { tenantFieldByTable?: unknown } | null | undefined)?.tenantFieldByTable; + if (!map || typeof map !== 'object') return null; + const out = new Map(); + for (const [table, field] of Object.entries(map as Record)) { + if (typeof field === 'string' && field) out.set(table, field); + } + return out; +} + +/** + * The statement that answers, for every base table holding one of `columns`, + * whether that column is NOT NULL — or `null` for a dialect it is not written + * for. One round trip per database. + */ +export function columnNullabilitySql(dialect: string, columns: readonly string[]): string | null { + const names = columns.filter((c) => SAFE_IDENTIFIER.test(c)).map((c) => `'${c}'`).join(', '); + if (!names) return null; + if (dialect === 'sqlite') { + return 'SELECT m.name AS table_name, p.name AS column_name, p."notnull" AS not_null ' + + "FROM sqlite_master m JOIN pragma_table_info(m.name) p WHERE m.type = 'table' " + + `AND m.name NOT LIKE 'sqlite_%' AND p.name IN (${names})`; + } + const schema = dialect === 'postgres' ? 'ANY (current_schemas(false))' : dialect === 'mysql' ? 'DATABASE()' : null; + if (schema === null) return null; + return 'SELECT c.table_name AS table_name, c.column_name AS column_name, ' + + "CASE WHEN c.is_nullable = 'NO' THEN 1 ELSE 0 END AS not_null " + + 'FROM information_schema.columns c JOIN information_schema.tables t ' + + 'ON t.table_schema = c.table_schema AND t.table_name = c.table_name ' + + `WHERE t.table_type = 'BASE TABLE' AND c.table_schema = ${schema} AND c.column_name IN (${names})`; +} + +/** + * Whether each of `columns` is NOT NULL on every base table of one database + * that holds it: `table → column → notNull`. Reads only. A failure is a + * `detail`, never an empty map — "could not look" is not "nothing nullable". + */ +export async function readColumnNullability( + driver: unknown, + columns: readonly string[], + normalize: (result: unknown) => Record[], +): Promise<{ tables: Map> } | { detail: string }> { + const exec = resolvePlannedDriverExec(driver); + if (exec === null) return { detail: 'the planned driver exposes no raw SQL seam, so its column catalog could not be read' }; + const dialect = dialectOfDriver(driver as never); + const sql = columnNullabilitySql(dialect, columns); + if (sql === null) return { detail: `the connected dialect (${dialect}) is not one this read is written for (sqlite / postgres / mysql)` }; + let result: unknown; + try { + result = await exec(sql, []); + } catch (error: unknown) { + return { detail: `the column-catalog read failed: ${error instanceof Error ? error.message : String(error)}` }; + } + if (!isResultSet(result)) return { detail: 'the raw SQL seam returned no result set, so no column was read' }; + const tables = new Map>(); + for (const row of normalize(result)) { + const table = row.table_name ?? row.TABLE_NAME; + const column = row.column_name ?? row.COLUMN_NAME; + if (typeof table !== 'string' || typeof column !== 'string') continue; + const byColumn = tables.get(table) ?? new Map(); + byColumn.set(column, Number(row.not_null ?? row.NOT_NULL) === 1); + tables.set(table, byColumn); + } + return { tables }; +} + +/** + * Why the ceremony leaves `table`'s tenant column nullable, or `null` when it + * tightens it. `shardOf` is the declared base a rotation shard belongs to. + */ +function ceremonyLeaves(table: string, shardOf: string | null, driver: unknown): string | null { + if (shardOf !== null) { + return `a rotation shard of ${shardOf}, created before shards were created NOT NULL: the ceremony rebuilds no ` + + `shard, and it is dropped when its period leaves ${shardOf}'s retention`; + } + const limit = (driver as { driftApplyLimit?: (op: string) => string | null } | null | undefined)?.driftApplyLimit?.('tighten_not_null') ?? null; + if (limit) return `this database's apply door cannot tighten a column: ${limit}`; + const entry = inventoryEntryFor(table); + if (entry?.notNullExempt) return entry.notNullExempt; + if (entry?.fate === 'mirror-deletion') { + return 'its rows are mirrors the ceremony deletes, and the table retires (ADR-0131 D10 fate 2, D13), so the ceremony does not tighten it'; + } + return null; +} + +/** + * Every tenant column the databases the plan covers hold nullable. Reads only, + * and never throws: one database that cannot be read makes the report + * `unreadable`, since a `read` answer is a claim about every database. + * + * @param databases the distinct databases the plan read, the primary first. + * @param tenantDrivers every driver the plan composed: a table's tenant column + * is the one any of them declares, since two drivers on one file each manage + * their own objects. + * @param mirrored whether the plan composed the object set the deployment + * serves; when it did not, which columns are tenant columns is not known. + * @param normalize `normalizeRows` — flattens the three dialect result shapes. + */ +export async function collectNullableTenantColumns(opts: { + databases: readonly PlannedDatabase[]; + tenantDrivers: readonly unknown[]; + mirrored: boolean; + normalize: (result: unknown) => Record[]; +}): Promise { + const unreadable = (detail: string): NullableTenantColumnsUnreadable => ({ status: 'unreadable', adr: 'ADR-0131 D1', detail }); + if (!opts.mirrored) { + return unreadable('this plan did not compose the object set the deployment serves, so which columns are tenant columns is not known'); + } + if (opts.databases.length === 0) return unreadable('the plan holds no SQL driver, so no column was read'); + const tenantField = new Map(); + for (const driver of opts.tenantDrivers) { + const fields = readTenantFields(driver); + if (fields === null) return unreadable('a planned driver exposes no readable tenant-column map, so the tenant columns are not known'); + for (const [table, field] of fields) if (!tenantField.has(table)) tenantField.set(table, field); + } + const fieldNames = [...new Set(tenantField.values())]; + const columns: NullableTenantColumn[] = []; + if (fieldNames.length > 0) { + for (const { driver, database } of opts.databases) { + const read = await readColumnNullability(driver, fieldNames, opts.normalize); + if ('detail' in read) return unreadable(database ? `${database}: ${read.detail}` : read.detail); + for (const [table, byColumn] of read.tables) { + const base = tenantField.has(table) ? null : rotationBaseOf(table); + const field = tenantField.get(table) ?? (base !== null ? tenantField.get(base) : undefined); + if (field === undefined || byColumn.get(field) !== false) continue; + const reason = ceremonyLeaves(table, base, driver); + columns.push({ + table, + column: field, + ...(database ? { database } : {}), + ceremony: reason === null ? 'tightens' : 'leaves', + ...(reason === null ? {} : { reason }), + }); + } + } + } + columns.sort((a, b) => (a.database ?? '').localeCompare(b.database ?? '') || a.table.localeCompare(b.table)); + return { status: 'read', adr: 'ADR-0131 D1', remedy: NULLABLE_TENANT_COLUMNS_REMEDY, columns }; +} + +/** A column as an operator-facing line (no bullet, no colour). */ +export function describeNullableTenantColumn(column: NullableTenantColumn): string { + const name = `${column.database ? `${column.database}: ` : ''}${column.table}.${column.column}`; + return column.ceremony === 'leaves' ? `${name} — the ceremony leaves it nullable: ${column.reason}` : name; +} + +/** + * The advisory section as `os migrate plan` prints it, or `[]` when every + * tenant column is NOT NULL. Loud when it could not read, because "did not + * look" must never read as "none nullable". + */ +export function renderNullableTenantColumns(report: NullableTenantColumnsReport): string[] { + if (report.status === 'unreadable') return [`Nullable tenant-column advisory did not run: ${report.detail}.`]; + if (report.columns.length === 0) return []; + const tightens = report.columns.filter((c) => c.ceremony === 'tightens').length; + return [ + `${report.columns.length} tenant column(s) are still nullable (ADR-0131 D1). This is advisory: it is not drift, ` + + 'and nothing in this plan or in "os migrate apply" changes it.', + ...report.columns.map((c) => ` • ${describeNullableTenantColumn(c)}`), + tightens > 0 + ? `The voluntary organization-ownership ceremony tightens ${tightens} of them: run "${report.remedy} --out PLAN", ` + + `review it, then "${report.remedy} --apply --plan-file PLAN --backup-taken". The server starts either way.` + : 'None of them is one the organization-ownership ceremony tightens; each line says why it stays nullable.', + ]; +} diff --git a/packages/cli/src/utils/organization-ownership-apply.generation-1.test.ts b/packages/cli/src/utils/organization-ownership-apply.generation-1.test.ts index a58296de0e4..bfc1d7b91d0 100644 --- a/packages/cli/src/utils/organization-ownership-apply.generation-1.test.ts +++ b/packages/cli/src/utils/organization-ownership-apply.generation-1.test.ts @@ -111,7 +111,7 @@ const RULES = ['share_custom', 'share_collides']; async function apply( l: ReturnType = ledger(), - opts: { posture?: string; webhooks?: boolean; rules?: boolean; notNull?: boolean } = {}, + opts: { posture?: string; webhooks?: boolean; rules?: boolean; notNull?: boolean; notNullPort?: NotNullPort } = {}, ): Promise { const posture = opts.posture ?? 'single'; const database = join(dir, 'fixture.db'); @@ -133,7 +133,7 @@ async function apply( ...(opts.rules === false ? {} : { declaredSharingRules: { async rules() { return RULES.map((name) => ({ name, refused: null, packageId: 'com.example.app' })); } } }), - ...(opts.notNull === false ? {} : { notNull }), + ...(opts.notNull === false ? {} : { notNull: opts.notNullPort ?? notNull }), door: { async save(type, name) { throw new Error(`no environment save is expected here (${type} ${name})`); @@ -418,6 +418,22 @@ describe('ownership shape generation 1 — the ceremony steps an 18.0 database o expect(tightened.map((c) => c.table)).toContain('sys_email'); }); + it('5d a table the door answers already NOT NULL (a shard the driver created so) is counted so, never as one left nullable', async () => { + const report = await apply(ledger(), { + notNullPort: { + async apply(columns) { + return columns.map((c) => ({ ...c, outcome: c.table === 'sys_activity' ? ('already-not-null' as const) : ('applied' as const) })); + }, + }, + }); + const notNullStep = step(report, 'not-null'); + expect(notNullStep.status).toBe('done'); + expect(notNullStep.summary).toMatchObject({ alreadyNotNull: 1, unsupported: 0, notDeclared: 0 }); + expect(notNullStep.summary.applied).toBe(notNullStep.summary.due! - 1); + expect(notNullStep.notes.filter((n) => n.includes('sys_activity'))).toEqual([]); + expect(notNullStep.blockers).toEqual([]); + }); + it('5d a run with no schema door applies nothing and says so; it does not block', async () => { const report = await apply(ledger(), { notNull: false }); expect(step(report, 'not-null')).toMatchObject({ status: 'done', notes: [expect.stringMatching(/no schema door to apply it: nothing was applied/)] }); diff --git a/packages/cli/src/utils/organization-ownership-apply.ts b/packages/cli/src/utils/organization-ownership-apply.ts index 380ef3b0ae2..602aeb7c482 100644 --- a/packages/cli/src/utils/organization-ownership-apply.ts +++ b/packages/cli/src/utils/organization-ownership-apply.ts @@ -3688,12 +3688,14 @@ export interface NotNullColumn { /** One NOT NULL, as the schema door answered it. */ export interface NotNullOutcome extends NotNullColumn { /** - * `applied`; `not-declared` — the composition this run booted declares no - * object for the table, so nothing rebuilds it against a declaration; - * `unsupported` — the datasource's transport cannot apply it (the Turso - * remote face cannot rebuild a table); `failed`. + * `applied`; `already-not-null` — the column is NOT NULL already (a + * rotation shard the driver created so), nothing to do; `not-declared` — the + * composition this run booted declares no object for the table, so nothing + * rebuilds it against a declaration; `unsupported` — the datasource's + * transport cannot apply it (the Turso remote face cannot rebuild a table), + * or the table is a rotation shard created nullable; `failed`. */ - outcome: 'applied' | 'not-declared' | 'unsupported' | 'failed'; + outcome: 'applied' | 'already-not-null' | 'not-declared' | 'unsupported' | 'failed'; detail?: string; } @@ -3723,7 +3725,7 @@ export interface NotNullPort { */ async function runNotNull(run: Run): Promise { const step: CeremonyStep = 'not-null'; - const summary: Record = { applied: 0, due: 0, notDeclared: 0, unsupported: 0 }; + const summary: Record = { applied: 0, alreadyNotNull: 0, due: 0, notDeclared: 0, unsupported: 0 }; const blockers: string[] = []; const notes: string[] = []; const plan = await planFor(run); @@ -3742,6 +3744,7 @@ async function runNotNull(run: Run): Promise { } for (const outcome of await run.opts.notNull.apply(due)) { if (outcome.outcome === 'applied') summary.applied++; + else if (outcome.outcome === 'already-not-null') summary.alreadyNotNull++; else if (outcome.outcome === 'not-declared') { summary.notDeclared++; notes.push( diff --git a/packages/cli/src/utils/organization-ownership-not-null.test.ts b/packages/cli/src/utils/organization-ownership-not-null.test.ts index 14d0612313d..f6bda742c0c 100644 --- a/packages/cli/src/utils/organization-ownership-not-null.test.ts +++ b/packages/cli/src/utils/organization-ownership-not-null.test.ts @@ -11,7 +11,11 @@ * the same constraint on any other undeclared-NOT-NULL column still is; * - a table the composition does not declare is `not-declared`, untouched; * - a transport whose apply door declares it cannot perform the op is - * `unsupported`, never attempted. + * `unsupported`, never attempted; + * - a rotation shard is never rebuilt: one the driver created NOT NULL is + * `already-not-null`, read from the database, and one created nullable + * before the driver did so is `unsupported`, saying it leaves with the + * base's retention. */ import { afterEach, beforeEach, describe, expect, it } from 'vitest'; @@ -107,19 +111,50 @@ describe('organization_id NOT NULL through the drift apply door', () => { expect(attempted).toBe(0); }); - it('a rotation shard is unsupported, never rebuilt: it has no declaration of its own', async () => { + it('a rotation shard created nullable is unsupported, never rebuilt: it has no declaration of its own', async () => { let attempted = 0; const outcomes = await applyNotNullThroughDrift( { isDeclared: () => true, shardBaseOf: (t) => (t.startsWith('sys_activity__') ? 'sys_activity' : null), + isNotNull: async () => false, driverFor: () => ({ applyMigrationEntries: async () => { attempted++; return { applied: [], skipped: [] }; } }) as never, }, [{ table: 'sys_activity__r20261019', column: 'organization_id' }], ); expect(outcomes).toEqual([ - { table: 'sys_activity__r20261019', column: 'organization_id', outcome: 'unsupported', detail: expect.stringMatching(/rotation shard of sys_activity/) }, + { + table: 'sys_activity__r20261019', + column: 'organization_id', + outcome: 'unsupported', + detail: expect.stringMatching(/rotation shard of sys_activity, rotated in before shards were created with organization_id NOT NULL/), + }, ]); + expect(outcomes[0]!.detail).not.toMatch(/nullable column/); + expect(attempted).toBe(0); + }); + + it('⭐ a rotation shard the driver created NOT NULL is already-not-null, never reported unsupported or rebuilt', async () => { + await knex.schema.createTable('crm_deal__r20261021', (t: any) => { + t.string('id').primary(); + t.string('organization_id').notNullable(); + }); + let attempted = 0; + const asked: unknown[][] = []; + const outcomes = await applyNotNullThroughDrift( + { + isDeclared: () => true, + shardBaseOf: (t) => (t.startsWith('crm_deal__') ? 'crm_deal' : null), + isNotNull: async (table, column, database) => { + asked.push([table, column, database]); + return !(await nullable(table, column)); + }, + driverFor: () => ({ applyMigrationEntries: async () => { attempted++; return { applied: [], skipped: [] }; } }) as never, + }, + [{ table: 'crm_deal__r20261021', column: 'organization_id' }], + ); + expect(outcomes).toEqual([{ table: 'crm_deal__r20261021', column: 'organization_id', outcome: 'already-not-null' }]); + expect(asked).toEqual([['crm_deal__r20261021', 'organization_id', undefined]]); expect(attempted).toBe(0); }); diff --git a/packages/cli/src/utils/organization-ownership-not-null.ts b/packages/cli/src/utils/organization-ownership-not-null.ts index 056dc2262be..f029c21620d 100644 --- a/packages/cli/src/utils/organization-ownership-not-null.ts +++ b/packages/cli/src/utils/organization-ownership-not-null.ts @@ -12,6 +12,12 @@ export interface NotNullTarget { isDeclared(table: string): boolean; /** The declared table a rotation shard belongs to (`sys_activity__r20261019` → `sys_activity`), or `null`. */ shardBaseOf?(table: string): string | null; + /** + * Whether the table's column is NOT NULL already, read from the database the + * plan placed it in; `null` when it cannot be read. Asked of a rotation shard + * only: a shard the driver created NOT NULL has nothing left to do. + */ + isNotNull?(table: string, column: string, database: string | undefined): Promise; } /** @@ -25,10 +31,14 @@ export interface NotNullTarget { * `tighten_not_null`, handed to the one driver that holds the table. A table * the composition does not declare is not rebuilt (`not-declared`); a * transport whose apply door cannot perform the op (the Turso remote face, - * `driftApplyLimit`) is `unsupported`, never attempted; so is a rotation - * shard: the apply door rebuilds a table against its own declaration, which a - * shard does not have, and every shard rotated in later is created from the - * base's declaration, nullable — the shard family holds NOT NULL nowhere. + * `driftApplyLimit`) is `unsupported`, never attempted. + * + * A rotation shard is never rebuilt: the apply door rebuilds a table against + * its own declaration, which a shard does not have. The driver creates every + * shard it rotates in with the tenant column NOT NULL (`ensureShardTable`), so + * a shard rotated in by this release is `already-not-null`, nothing to do. A + * shard rotated in before the driver did that stays nullable and is + * `unsupported`; it is dropped when its period leaves the base's retention. */ export async function applyNotNullThroughDrift(target: NotNullTarget, columns: readonly NotNullColumn[]): Promise { const outcomes: NotNullOutcome[] = []; @@ -39,11 +49,17 @@ export async function applyNotNullThroughDrift(target: NotNullTarget, columns: r } const base = target.shardBaseOf?.(table) ?? null; if (base !== null && base !== table) { + if ((await target.isNotNull?.(table, column, database)) === true) { + outcomes.push({ table, column, outcome: 'already-not-null' }); + continue; + } outcomes.push({ table, column, outcome: 'unsupported', - detail: `${table} is a rotation shard of ${base}: the drift apply door rebuilds only a declared table, and each new shard is created with ${base}'s nullable column`, + detail: + `${table} is a rotation shard of ${base}, rotated in before shards were created with ${column} NOT NULL: the ` + + `drift apply door rebuilds only a declared table, and the shard is dropped when its period leaves ${base}'s retention`, }); continue; } diff --git a/packages/qa/dogfood/test/upgrade-18x-harness.dogfood.test.ts b/packages/qa/dogfood/test/upgrade-18x-harness.dogfood.test.ts index 71db0536a86..0e4e95dd6c9 100644 --- a/packages/qa/dogfood/test/upgrade-18x-harness.dogfood.test.ts +++ b/packages/qa/dogfood/test/upgrade-18x-harness.dogfood.test.ts @@ -55,7 +55,7 @@ import type { VerifyStack } from '@objectstack/verify'; import { columnsOf, type FixtureManifest } from './upgrade-harness/fixture-manifest.js'; import { FIXTURE_NAMES, FIXTURE_PEOPLE, bootFixture } from './upgrade-harness/populate-fixture.js'; import { loadSqlIntoSqlite } from './upgrade-harness/sqlite-text-dump.js'; -import { OWNERSHIP_MARKER, runOwnershipCeremony, runSchemaPlan } from './upgrade-harness/ceremony.js'; +import { OWNERSHIP_MARKER, SERVED_ENV, runOwnershipCeremony, runSchemaPlan } from './upgrade-harness/ceremony.js'; const FIXTURE = new URL('./upgrade-harness/fixture-18.0/', import.meta.url); const MARKER = OWNERSHIP_MARKER; @@ -303,7 +303,37 @@ describe(`[#24522] an 18.0 database (fixture from ${manifest.ref} @ ${manifest.s // columns. 18.0.0's own shape is the baseline: a tenant column still nullable, // and the COALESCE form of a unique index over it. A later step that reshapes // either owes this database a migration, and this leg is where it shows. +// +// [ADR-0131 D1; ruling record 6328195425, letter A] The ceremony stays +// voluntary on it, and `os migrate plan` says so: its advisory lists every +// tenant column still nullable, as one the ceremony tightens, apart from the +// residue it leaves. After the ceremony only that residue is listed, and the +// shard this release rotated in NOT NULL is reported so by the ceremony. A +// fresh store of this release lists nothing. const GA_FIXTURE = new URL('./upgrade-harness/fixture-18.0.0/', import.meta.url); + +/** The physical tables of `file` whose `organization_id` is nullable. */ +function nullableOrganizationTables(file: string): string[] { + const db = new DatabaseSync(file, { readOnly: true }); + try { + return (db.prepare( + `SELECT m.name AS name FROM sqlite_master m JOIN pragma_table_info(m.name) p + WHERE m.type = 'table' AND p.name = 'organization_id' AND p."notnull" = 0 ORDER BY m.name`, + ).all() as Array<{ name: string }>).map((r) => r.name); + } finally { + db.close(); + } +} + +/** The tables carrying a column named `organization_id` that are no tenant column of what HEAD declares. */ +const NOT_A_TENANT_COLUMN = [ + // Retired, its table left in place by ruling: no object declares it (the unmanaged-table section lists it). + 'sys_notification_subscription', + // tenancy.enabled: false: better-auth's provider relation, not the tenancy anchor. + 'sys_sso_provider', +]; + +const isShard = (table: string): boolean => /__r\d{6,8}$/.test(table); const gaManifest = JSON.parse(readFileSync(new URL('manifest.json', GA_FIXTURE), 'utf8')) as FixtureManifest; const gaFixtureSql = readFileSync(new URL('database.sql', GA_FIXTURE), 'utf8'); @@ -337,6 +367,26 @@ describe(`[#24884] an 18.0.0 database (fixture from ${gaManifest.ref} @ ${gaMani expect(plan.changes.filter((c) => c.category === 'destructive'), 'destructive drift').toEqual([]); }, 320_000); + it('[ADR-0131 D1] HEAD\'s schema plan lists its nullable tenant columns as an advisory naming the ceremony, and as no drift', () => { + const plan = runSchemaPlan(file, SERVED_ENV); + const advisory = plan.nullableTenantColumns; + if (advisory.status !== 'read') throw new Error(`the advisory did not read: ${advisory.detail}`); + expect(advisory.remedy).toBe('os migrate organization-ownership'); + const nullable = nullableOrganizationTables(file); + const listed = advisory.columns.map((c) => c.table); + // Every nullable tenant column, and only those: the two that are no tenant column are the difference. + expect(nullable.filter((t) => !listed.includes(t))).toEqual(NOT_A_TENANT_COLUMN); + expect(listed.filter((t) => !nullable.includes(t))).toEqual([]); + expect(advisory.columns.every((c) => c.column === 'organization_id')).toBe(true); + // The ceremony tightens all of them but its residue, each marked with why. + const leaves = advisory.columns.filter((c) => c.ceremony === 'leaves'); + expect(leaves.map((c) => c.table).filter((t) => !isShard(t))).toEqual(['sys_email_template']); + expect(leaves.every((c) => typeof c.reason === 'string' && c.reason.length > 0)).toBe(true); + expect(advisory.columns.filter((c) => c.ceremony === 'tightens').length).toBeGreaterThan(50); + // Advisory, not drift: no change names a tenant column's nullability. + expect(plan.changes.filter((c) => c.column === 'organization_id' && c.kind === 'nullability_mismatch')).toEqual([]); + }, 320_000); + it('HEAD boots it with no refusal, and serves the fixture\'s people and task', async () => { const booted = await tryBoot(file); stack = booted.stack; @@ -369,4 +419,38 @@ describe(`[#24884] an 18.0.0 database (fixture from ${gaManifest.ref} @ ${gaMani db.close(); } }); + + it('[ADR-0131 D1] after the voluntary ceremony the advisory lists only the residue it leaves, and the ceremony reports HEAD\'s shard NOT NULL', () => { + expect(stack, 'HEAD has stopped before the ceremony reads the file').toBeUndefined(); + const ceremonied = join(dir, 'ga.ceremonied.db'); + copyFileSync(file, ceremonied); + const report = runOwnershipCeremony(dir, ceremonied, 'ga-voluntary', SERVED_ENV) as { + complete?: boolean; + steps?: Array<{ step: string; summary: Record; notes: string[] }>; + }; + expect(report.complete).toBe(true); + const notNull = report.steps!.find((s) => s.step === 'not-null')!; + // The shard HEAD's boot rotated in was created NOT NULL: nothing to do, never "unsupported". + expect(notNull.summary.alreadyNotNull, JSON.stringify(notNull)).toBeGreaterThanOrEqual(1); + expect(notNull.notes.filter((n) => /created with \S+ nullable column/.test(n))).toEqual([]); + + const plan = runSchemaPlan(ceremonied, SERVED_ENV); + const advisory = plan.nullableTenantColumns; + if (advisory.status !== 'read') throw new Error(`the advisory did not read: ${advisory.detail}`); + expect(advisory.columns.filter((c) => c.ceremony === 'tightens'), 'a column the ceremony would still tighten').toEqual([]); + const residue = advisory.columns.map((c) => c.table); + expect(residue.filter((t) => !isShard(t))).toEqual(['sys_email_template']); + expect(residue.filter(isShard).length, residue.join(', ')).toBeGreaterThanOrEqual(1); + expect(residue.sort()).toEqual(nullableOrganizationTables(ceremonied).filter((t) => !NOT_A_TENANT_COLUMN.includes(t))); + }, 900_000); + + it('[ADR-0131 D1] a fresh store of HEAD lists no nullable tenant column', async () => { + const fresh = join(dir, 'fresh.db'); + const booted = await bootFixture(fresh); + await booted.stop(); + const plan = runSchemaPlan(fresh, SERVED_ENV); + expect(plan.nullableTenantColumns).toEqual({ status: 'read', adr: 'ADR-0131 D1', remedy: 'os migrate organization-ownership', columns: [] }); + // Its only nullable `organization_id` is the opt-out, which is no tenant column. + expect(nullableOrganizationTables(fresh)).toEqual(['sys_sso_provider']); + }, 320_000); }); diff --git a/packages/qa/dogfood/test/upgrade-harness/ceremony.ts b/packages/qa/dogfood/test/upgrade-harness/ceremony.ts index 4fbeb12b888..1a1620bd917 100644 --- a/packages/qa/dogfood/test/upgrade-harness/ceremony.ts +++ b/packages/qa/dogfood/test/upgrade-harness/ceremony.ts @@ -23,15 +23,22 @@ const CLI_BIN = join(CLI_ROOT, 'bin', 'run.js'); /** The ceremony's sys_migration row: its marker. */ export const OWNERSHIP_MARKER = 'adr-0131-organization-ownership'; +/** + * The environment a production `os serve` of the showcase is started with: an + * auth secret, without which it is refused, and its auth family (the tenant + * tables among them) is not composed. + */ +export const SERVED_ENV: NodeJS.ProcessEnv = { OS_AUTH_SECRET: 'dogfood-upgrade-harness-auth-secret-0123456789' }; + /** * Run the ceremony on `file`: the plan into `dir/-plan.json`, then * `--apply --plan-file … --backup-taken`. The apply report. Throws, with both - * runs' output, when either exits non-zero. + * runs' output, when either exits non-zero. `env` is laid over this process's. */ -export function runOwnershipCeremony(dir: string, file: string, tag: string): Record { +export function runOwnershipCeremony(dir: string, file: string, tag: string, env: NodeJS.ProcessEnv = {}): Record { const run = (args: string[]) => spawnSync(process.execPath, [ CLI_BIN, 'migrate', 'organization-ownership', '--database-url', `file:${file}`, ...args, - ], { cwd: SHOWCASE_DIR, encoding: 'utf8', timeout: 300_000, env: { ...process.env, NO_COLOR: '1' } }); + ], { cwd: SHOWCASE_DIR, encoding: 'utf8', timeout: 300_000, env: { ...process.env, ...env, NO_COLOR: '1' } }); const planFile = join(dir, `${tag}-plan.json`); const plan = run(['--out', planFile]); if (plan.status !== 0) throw new Error(`the ceremony's plan exited ${plan.status}:\n${plan.stdout}\n${plan.stderr}`); @@ -52,14 +59,28 @@ export interface SchemaPlanChange { * `os migrate plan --json` on `file`, the real binary in the showcase's * directory, as an operator reads the drift before serving a database. Writes * no row or table. The parsed document; throws, with the run's output, when it - * exits non-zero. + * exits non-zero. `env` is laid over this process's. */ -export function runSchemaPlan(file: string): { total: number; changes: SchemaPlanChange[]; pending: unknown[] } { +/** One tenant column `os migrate plan` lists as still nullable: its advisory, never drift (ADR-0131 D1). */ +export interface NullableTenantColumn { + table: string; + column: string; + database?: string; + ceremony: 'tightens' | 'leaves'; + reason?: string; +} + +export function runSchemaPlan(file: string, env: NodeJS.ProcessEnv = {}): { + total: number; + changes: SchemaPlanChange[]; + pending: unknown[]; + nullableTenantColumns: { status: 'read'; remedy: string; columns: NullableTenantColumn[] } | { status: 'unreadable'; detail: string }; +} { const plan = spawnSync(process.execPath, [CLI_BIN, 'migrate', 'plan', '--json', '--database-url', `file:${file}`], { cwd: SHOWCASE_DIR, encoding: 'utf8', timeout: 300_000, - env: { ...process.env, NO_COLOR: '1' }, + env: { ...process.env, ...env, NO_COLOR: '1' }, }); if (plan.status !== 0) throw new Error(`os migrate plan exited ${plan.status}:\n${plan.stdout}\n${plan.stderr}`); return JSON.parse(plan.stdout);