diff --git a/.changeset/22580-unmanaged-tables-served-boot.md b/.changeset/22580-unmanaged-tables-served-boot.md new file mode 100644 index 00000000000..186aa4ba0b4 --- /dev/null +++ b/.changeset/22580-unmanaged-tables-served-boot.md @@ -0,0 +1,11 @@ +--- +'@objectstack/cli': patch +--- + +fix(cli): `os migrate plan`'s unmanaged-tables sweep runs whenever the plan's object set mirrors what `os serve` registers, covers every database the plan diffs, and states only a reason that holds when it does not run (#22580) + +On a project with a compiled artifact and no `objectstack.config.ts`, `os migrate plan` composes what `os serve` registers, platform included. The sweep for platform-prefixed tables that no object declares was still withheld there (`unmanagedTables.status: "unreadable"` in `--json`), with a reason claiming the plan's object set was only the artifact plus the platform floor. On a project with neither a config nor a compiled artifact, the same untrue reason was given. + +The plan's composition now records whether its object set mirrors the served boot, and why when it does not. The sweep runs exactly when it does, the compiled-artifact project included. When it does not run, the `detail` names the composition's own reason: the host config that did not load, or a project with neither a config nor a compiled artifact. + +The sweep also reads every database the plan diffs. Where the serving boot keeps lifecycle-classed objects in the `telemetry` sibling database, a table stranded there is now reported too, where before the sweep answered `read` over the primary database alone. A sibling whose catalog cannot be read makes the whole sweep `unreadable`, and the `detail` names that database. The `--json` shape of `unmanagedTables` is unchanged. `physicalTables` counts the tables of every database swept, and the human line says "in the database(s) this plan covers". diff --git a/packages/cli/src/commands/migrate/plan.boot-parity.integration.test.ts b/packages/cli/src/commands/migrate/plan.boot-parity.integration.test.ts index 33c930762e8..cc086148451 100644 --- a/packages/cli/src/commands/migrate/plan.boot-parity.integration.test.ts +++ b/packages/cli/src/commands/migrate/plan.boot-parity.integration.test.ts @@ -38,8 +38,9 @@ import Database from 'better-sqlite3'; * - the boot's tables, a rotation shard read as its object, number exactly * the plan's examined objects plus `sys_packages` (boot ⊆ plan, app tables * included) — with `pending` empty, equal counts are equal sets; - * - where the unmanaged-tables sweep runs (a project with a host config), it - * names nothing but `sys_packages`. + * - the unmanaged-tables sweep RUNS on every shape — the composition says it + * mirrors the served boot, the compiled-artifact project with no config + * included (#22580) — and names nothing but `sys_packages`. * * `sys_packages` is the one table outside the object set, by construction: * `PackageServicePlugin.start()` creates it with raw DDL (`package-table.ts`), @@ -141,12 +142,13 @@ function expectParity(r: Reading): void { const rawDdl = r.tables.filter((t) => NOT_AN_OBJECT.includes(t)); expect(rawDdl).toEqual(NOT_AN_OBJECT); expect(r.tables.length).toBe(r.plan.managedTables + rawDdl.length); - // … and platform tables by name, where the unmanaged sweep runs (it reports - // itself `unreadable` on a project with no host config). - if (r.plan.unmanagedTables?.status === 'read') { - const unmanaged = (r.plan.unmanagedTables.tables ?? []).map((t: any) => (typeof t === 'string' ? t : t.table ?? t.name)); - expect(unmanaged.filter((t: string) => !NOT_AN_OBJECT.includes(t))).toEqual([]); - } + // … and platform tables by name: [#22580] the unmanaged sweep runs on every + // shape, because the composition records that it mirrors the served boot — + // the compiled artifact with no config included, which it used to withhold + // with a reason that no longer held. + expect(r.plan.unmanagedTables?.status, JSON.stringify(r.plan.unmanagedTables)).toBe('read'); + const unmanaged = (r.plan.unmanagedTables.tables ?? []).map((t: any) => t.table); + expect(unmanaged).toEqual(NOT_AN_OBJECT); } describe('os migrate plan examines exactly what os serve registers, per app shape (#22506)', () => { diff --git a/packages/cli/src/commands/migrate/plan.telemetry-sibling.integration.test.ts b/packages/cli/src/commands/migrate/plan.telemetry-sibling.integration.test.ts index 52a66ae05b2..0d9f03c3f25 100644 --- a/packages/cli/src/commands/migrate/plan.telemetry-sibling.integration.test.ts +++ b/packages/cli/src/commands/migrate/plan.telemetry-sibling.integration.test.ts @@ -29,6 +29,9 @@ import Database from 'better-sqlite3'; * - after a development boot, the plan names the sibling, lists no * lifecycle-classed object for the primary, and examines both databases' * objects; `apply` creates nothing, and neither database moves; + * - [#22580] its unmanaged-tables sweep reads the sibling's catalog too: a + * retired lifecycle-classed object strands its table there, and a sweep of + * the primary alone answered `read` without ever looking; * - the CONTROL: a deployment with no sibling (`OS_TELEMETRY_DB=0`) plans as * it always did — no sibling named, nothing pending; * - a plan with the sibling on over a deployment that has none yet plans the @@ -159,6 +162,11 @@ describe('os migrate plan / apply plan each object against the database it lives const siblingTables = tables(f.sibling); expect(siblingTables).toEqual(expect.arrayContaining(['sys_audit_log', 'sys_metadata_audit'])); expect(primaryTables.filter((t) => siblingTables.includes(t)), 'one table in both databases').toEqual([]); + // [#22580] What a retired lifecycle-classed object leaves behind: its table, + // in the sibling, declared by nothing. + const retired = 'sys_os22580_retired_event'; + const db = new Database(f.sibling); + try { db.exec(`CREATE TABLE ${retired} (id TEXT PRIMARY KEY)`); } finally { db.close(); } const before = { primary: state(f.primary), sibling: state(f.sibling) }; const plan = command(f, ['plan', '--json']); @@ -170,6 +178,10 @@ describe('os migrate plan / apply plan each object against the database it lives expect(plan.payload.composition?.coverage?.unexaminedObjects).toBe(0); // Both databases' objects examined: their tables, less the raw-DDL one. expect(plan.payload.managedTables).toBe(primaryTables.length - NOT_AN_OBJECT.length + siblingTables.length); + // [#22580] Both databases' catalogs swept: the sibling's stranded table is + // reported beside the primary's raw-DDL one. + expect(plan.payload.unmanagedTables?.status, JSON.stringify(plan.payload.unmanagedTables)).toBe('read'); + expect(plan.payload.unmanagedTables.tables.map((t: any) => t.table)).toEqual([...NOT_AN_OBJECT, retired].sort()); // A plan writes nothing, to either database. expect({ primary: state(f.primary), sibling: state(f.sibling) }).toEqual(before); diff --git a/packages/cli/src/commands/migrate/plan.ts b/packages/cli/src/commands/migrate/plan.ts index 9b388c4149c..ec15e82ab54 100644 --- a/packages/cli/src/commands/migrate/plan.ts +++ b/packages/cli/src/commands/migrate/plan.ts @@ -195,7 +195,8 @@ export default class MigratePlan extends Command { // indistinguishable from "never looked at". See `../../utils/unmanaged-tables.ts`. const { normalizeRows } = await import('@objectstack/metadata-protocol'); const unmanagedTables: UnmanagedTablesReport = await collectUnmanagedTables({ - driver: stack.driver, + // [#22580] Every database the drift above covers, the sibling included. + drivers: stack.drivers, declaredObjects: stack.allObjects(), composition: stack.composition, normalize: normalizeRows, diff --git a/packages/cli/src/utils/schema-migrate.ts b/packages/cli/src/utils/schema-migrate.ts index 68d5cb73348..c59ed50e908 100644 --- a/packages/cli/src/utils/schema-migrate.ts +++ b/packages/cli/src/utils/schema-migrate.ts @@ -174,7 +174,8 @@ export interface SchemaStack { /** * What the host composition added, and anything it could not (#12938) — the * host config it composed, whether that config actually loaded, the platform - * floor it added. `notes` is always `[]` unless the boot asked for + * floor it added, and whether the set mirrors what `os serve` registers + * (`servedBoot`, #22580). `notes` is always `[]` unless the boot asked for * `composeHostStack`, and `[]` even then when there was nothing to compose, * so a project with neither a config nor a compiled artifact renders * byte-identically to before this existed. @@ -863,6 +864,8 @@ export async function bootSchemaStack( }) : { plugins: [], hostConfigPath: null, hostConfigLoaded: false, hostConfigError: null, + // [#22580] The data stack alone: this boot was not asked for the deployment's set. + servedBoot: { mirrored: false, reason: 'not-composed' }, notes: [], coverage: null, } satisfies SchemaMigrationComposition; for (const plugin of composition.plugins) { diff --git a/packages/cli/src/utils/schema-migration-plugins.test.ts b/packages/cli/src/utils/schema-migration-plugins.test.ts index 64876a7bc34..b3a290e14a5 100644 --- a/packages/cli/src/utils/schema-migration-plugins.test.ts +++ b/packages/cli/src/utils/schema-migration-plugins.test.ts @@ -185,6 +185,8 @@ describe('buildSchemaMigrationPlugins', () => { expect(out.notes).toEqual([]); expect(out.hostConfigPath).toBeNull(); expect(out.hostConfigLoaded).toBe(false); + // [#22580] …and it says so: there is no deployment here to mirror. + expect(out.servedBoot).toEqual({ mirrored: false, reason: 'nothing-to-compose' }); }); it('composes the platform floor once an artifact app is present, and not twice', async () => { @@ -468,7 +470,7 @@ describe('describeUnloadableHostConfig (#12953)', () => { function composition(over: Partial): SchemaMigrationComposition { return { plugins: [], hostConfigPath: null, hostConfigLoaded: false, hostConfigError: null, - notes: [], coverage: null, ...over, + servedBoot: { mirrored: false, reason: 'nothing-to-compose' }, notes: [], coverage: null, ...over, }; } @@ -537,7 +539,7 @@ describe('describeUnloadableHostConfig — the no-DDL notice (#13118)', () => { function composition(over: Partial): SchemaMigrationComposition { return { plugins: [], hostConfigPath: null, hostConfigLoaded: false, hostConfigError: null, - notes: [], coverage: null, ...over, + servedBoot: { mirrored: false, reason: 'nothing-to-compose' }, notes: [], coverage: null, ...over, }; } @@ -917,6 +919,36 @@ describe('servedPlatform (#22506)', () => { 'com.objectstack.platform-objects', ]); }, 60_000); + + /** + * [#22580] The composition records, where it decides it, whether its set + * mirrors the served boot — the fact `os migrate plan`'s unmanaged-tables + * sweep gates on. Every shape a project can be in, each answer its own. + */ + it('[#22580] records whether the set mirrors the served boot — per project shape, never from hostConfigLoaded', async () => { + // A compiled artifact and NO config: `hostConfigLoaded` is false, and the + // set IS the served boot's once the served platform is composed. + const artifactApp = { type: 'app', name: 'plugin.app.demo' }; + const artifact = await buildSchemaMigrationPlugins({ basePlugins: [artifactApp], cwd: tempProject(), servedPlatform: {} }); + expect(artifact.hostConfigLoaded).toBe(false); + expect(artifact.servedBoot).toEqual({ mirrored: true }); + + const config = await buildSchemaMigrationPlugins({ basePlugins: [], cwd: project(APP), servedPlatform: {} }); + expect(config.servedBoot).toEqual({ mirrored: true }); + + // The controls: each set that does NOT mirror the served boot, with its reason. + const stackOnly = await buildSchemaMigrationPlugins({ basePlugins: [], cwd: project(APP) }); + expect(stackOnly.servedBoot).toEqual({ mirrored: false, reason: 'stack-only' }); + + const unloadable = await buildSchemaMigrationPlugins({ + basePlugins: [], cwd: project("throw new Error('fixture: os22580 unloadable');\n"), servedPlatform: {}, + }); + expect(unloadable.hostConfigLoaded).toBe(false); + expect(unloadable.servedBoot).toEqual({ mirrored: false, reason: 'config-unloadable' }); + + const nothing = await buildSchemaMigrationPlugins({ basePlugins: [], cwd: tempProject(), servedPlatform: {} }); + expect(nothing.servedBoot).toEqual({ mirrored: false, reason: 'nothing-to-compose' }); + }, 120_000); }); describe('composeProviderForDeclarations (#22506)', () => { diff --git a/packages/cli/src/utils/schema-migration-plugins.ts b/packages/cli/src/utils/schema-migration-plugins.ts index 6093868754f..d8a6bd2add1 100644 --- a/packages/cli/src/utils/schema-migration-plugins.ts +++ b/packages/cli/src/utils/schema-migration-plugins.ts @@ -1286,6 +1286,36 @@ export interface SchemaMigrationComposition { * `null` on every other shape, including a config that loaded. */ hostConfigError: string | null; + /** + * [#22580] Does the composed object set MIRROR what this deployment's + * `os serve` boot registers? Recorded once, by the code that decides it, so + * a consumer that needs the answer reads it here rather than re-deriving it. + * + * `mirrored: true` exactly when {@link composeServedPlatform} composed what + * `serve` mounts around a stack this boot could read: a host config that + * loaded, or a compiled artifact with no config. That is the set + * `commands/migrate/plan.boot-parity.integration.test.ts` holds equal to a + * real `os serve` boot's, per app shape. Otherwise `reason` says why not: + * + * - `not-composed`: the boot did not ask for the host composition + * (`composeHostStack` off), so the set is the data stack alone; + * - `nothing-to-compose`: neither a host config nor a compiled artifact, so + * there is no deployment here to mirror; + * - `config-unloadable`: the host config exists and did not load (#12953); + * - `stack-only`: the caller did not ask for what `serve` mounts around the + * stack (`servedPlatform`), so its auth family and capability providers + * are not in the set. + * + * ⛔ Not a second reading of {@link hostConfigLoaded}: that is `false` on a + * project with a compiled artifact and no config too, which mirrors the + * served boot once the served platform is composed (#22506). + */ + servedBoot: + | { readonly mirrored: true } + | { + readonly mirrored: false; + readonly reason: 'not-composed' | 'nothing-to-compose' | 'config-unloadable' | 'stack-only'; + }; /** * One line per thing this composition did or could not do, for the command to * print. Empty when nothing was composed, so a project with neither a config @@ -1333,6 +1363,7 @@ const NOTHING_COMPOSED: SchemaMigrationComposition = Object.freeze({ hostConfigPath: null, hostConfigLoaded: false, hostConfigError: null, + servedBoot: { mirrored: false, reason: 'nothing-to-compose' }, notes: [], coverage: null, }) as SchemaMigrationComposition; @@ -1564,6 +1595,12 @@ export async function buildSchemaMigrationPlugins(opts: { const notes: string[] = []; let hostConfigLoaded = false; let hostConfigError: string | null = null; + // [#22580] Whether this set mirrors the served boot — set below where each + // answer is decided: the catch that finds the config unloadable, and the + // branch that composes what `serve` mounts around the stack. + let servedBoot: SchemaMigrationComposition['servedBoot'] = !hostConfigPath && !hasArtifactApp + ? { mirrored: false, reason: 'nothing-to-compose' } + : { mirrored: false, reason: 'stack-only' }; // #21732 — the config's capability declarations, read off the same loaded // config the plugins came from. See {@link resolveRequiredProviders}. let loadedRequires: unknown; @@ -1678,6 +1715,7 @@ export async function buildSchemaMigrationPlugins(opts: { console.warn(`[migrate] ⚠ ${line}`); notes.push(line); hostConfigError = message; + servedBoot = { mirrored: false, reason: 'config-unloadable' }; } } @@ -1713,6 +1751,9 @@ export async function buildSchemaMigrationPlugins(opts: { plugins.splice(1, 0, ...served.authFamily); plugins.push(...served.plugins); notes.push(...served.notes); + // [#22580] The stack this boot read, and what `serve` mounts around it: + // the set the boot-parity pin holds equal to `serve`'s. + if (hostConfigPath || hasArtifactApp) servedBoot = { mirrored: true }; } // #21732 — `serve` step 5, narrowed to what this boot cannot start without: // a provider the config's `requires` supplies, that a composed plugin @@ -1751,7 +1792,8 @@ export async function buildSchemaMigrationPlugins(opts: { } return { - plugins, hostConfigPath, hostConfigLoaded, hostConfigError, notes, coverage: null, writeGuard, lifecycle, + plugins, hostConfigPath, hostConfigLoaded, hostConfigError, servedBoot, + notes, coverage: null, writeGuard, lifecycle, ...(securityPlugin ? { securityPlugin } : {}), }; } diff --git a/packages/cli/src/utils/unmanaged-tables.integration.test.ts b/packages/cli/src/utils/unmanaged-tables.integration.test.ts index a5e710588b4..6df211de61e 100644 --- a/packages/cli/src/utils/unmanaged-tables.integration.test.ts +++ b/packages/cli/src/utils/unmanaged-tables.integration.test.ts @@ -130,12 +130,15 @@ describe('os migrate plan — unmanaged tables, against a real database (#13204) deferSchemaDdl: true, readOnlyProbe: true, composeHostStack: true, + // [#22506] `plan` composes what `os serve` mounts around the stack too — + // and [#22580] the sweep runs only over a set that mirrors the served boot. + composeServedPlatform: true, }); async function sweep(): Promise { const { normalizeRows } = await import('@objectstack/metadata-protocol'); return collectUnmanagedTables({ - driver: stack!.driver, + drivers: stack!.drivers, declaredObjects: stack!.allObjects(), composition: stack!.composition, normalize: normalizeRows, @@ -198,6 +201,7 @@ describe('os migrate plan — unmanaged tables, against a real database (#13204) // assertion below pass for the wrong reason. expect(stack!.driver).not.toBeNull(); expect(stack!.composition.hostConfigLoaded).toBe(true); + expect(stack!.composition.servedBoot).toEqual({ mirrored: true }); expect(stack!.managedTableCount).toBeGreaterThan(0); const declared = (stack!.allObjects() as Array<{ name?: string }>).map((o) => o?.name); expect(declared).toContain('sys_permission_set'); diff --git a/packages/cli/src/utils/unmanaged-tables.test.ts b/packages/cli/src/utils/unmanaged-tables.test.ts index f2a35395114..c1b757a4520 100644 --- a/packages/cli/src/utils/unmanaged-tables.test.ts +++ b/packages/cli/src/utils/unmanaged-tables.test.ts @@ -39,6 +39,7 @@ import { selectUnmanagedTables, type UnmanagedTablesReport, } from './unmanaged-tables.js'; +import type { SchemaMigrationComposition } from './schema-migration-plugins.js'; /** `normalizeRows`' sqlite limb — a bare row array. Kept local so the unit tests hold no driver. */ const normalize = (result: unknown): Record[] => @@ -53,12 +54,19 @@ function fakeDriver(opts: { managed?: string[] | null; client?: string | null; answer?: unknown | (() => unknown); + /** The database file its config names — how a failure on one of several databases is told apart. */ + filename?: string; }): unknown { const driver: Record = {}; if (opts.managed !== null) { driver.managedObjectFields = new Map((opts.managed ?? []).map((t) => [t, {}])); } - if (opts.client !== null) driver.config = { client: opts.client ?? 'better-sqlite3' }; + if (opts.client !== null) { + driver.config = { + client: opts.client ?? 'better-sqlite3', + ...(opts.filename ? { connection: { filename: opts.filename } } : {}), + }; + } if (opts.answer !== undefined) { driver.execute = async () => typeof opts.answer === 'function' ? (opts.answer as () => unknown)() : opts.answer; @@ -69,18 +77,21 @@ function fakeDriver(opts: { /** Catalog rows in the sqlite shape the sweep's own SELECT produces. */ const rows = (...names: string[]) => names.map((name) => ({ table_name: name })); -/** A composition that mirrors the deployment — the premise the sweep requires. */ -const MIRRORED = { hostConfigLoaded: true, hostConfigPath: '/app/objectstack.config.ts' }; +/** What the sweep reads off the composition. */ +type Premise = Pick; + +/** A composition that mirrors the served boot — the premise the sweep requires. */ +const MIRRORED: Premise = { servedBoot: { mirrored: true }, hostConfigPath: '/app/objectstack.config.ts' }; async function sweep(opts: { managed?: string[] | null; declared?: Array<{ name: string }>; client?: string | null; answer?: unknown | (() => unknown); - composition?: { hostConfigLoaded: boolean; hostConfigPath: string | null }; + composition?: Premise; }): Promise { return collectUnmanagedTables({ - driver: fakeDriver(opts), + drivers: [fakeDriver(opts)], declaredObjects: opts.declared ?? [], composition: opts.composition ?? MIRRORED, normalize, @@ -262,29 +273,44 @@ describe('collectUnmanagedTables — every no-answer path is unreadable, not emp expect(report).toMatchObject({ status: 'read', tables: [{ table: 'sys_scim_provider' }] }); }); - it('is unreadable when no host config was loaded — the declaration set is knowingly partial', async () => { - // Measured: with a compiled artifact and NO config, the composed set is the - // artifact plus the platform FLOOR — ten objects, of which the `sys_*` half - // is `sys_metadata` + its four siblings, `sys_migration`, - // `sys_migration_journal`, `sys_metadata_activation`, `sys_secret`. A - // database carrying the other ~40 platform tables would have every one of - // them reported. That is the cry-wolf shape, and it is UNMEASURED, not - // false. - const noConfig = await sweep({ - managed: ['sys_metadata'], - answer: rows('sys_user', 'sys_session', 'sys_account'), - composition: { hostConfigLoaded: false, hostConfigPath: null }, + it('[#22580] runs on a project with a compiled artifact and NO host config once the composition mirrors the served boot', async () => { + // Measured on `main` before this card: such a project composes the served + // platform (75 objects, every one examined), yet the sweep was withheld + // with a reason claiming the set was the artifact plus the platform floor. + // `hostConfigPath: null` is the trap: a gate keyed on the host config, or + // on `hostConfigLoaded` (false here too), turns this case unreadable. + const report = await sweep({ + managed: ['sys_user'], + declared: [{ name: 'sys_secret' }], + answer: rows('sys_user', 'sys_secret', 'sys_scim_provider'), + composition: { servedBoot: { mirrored: true }, hostConfigPath: null }, }); - expect(noConfig.status).toBe('unreadable'); - expect((noConfig as { detail: string }).detail).toContain('no host config'); + expect(report).toMatchObject({ status: 'read', tables: [{ table: 'sys_scim_provider' }] }); + }); - const brokenConfig = await sweep({ - managed: ['sys_metadata'], - answer: rows('sys_user'), - composition: { hostConfigLoaded: false, hostConfigPath: '/app/objectstack.config.ts' }, - }); - expect(brokenConfig.status).toBe('unreadable'); - expect((brokenConfig as { detail: string }).detail).toContain('/app/objectstack.config.ts'); + it('[#22580] is unreadable when the composition does not mirror the served boot, stating the reason it recorded', async () => { + // Every reason the composition can record, against a database that WOULD + // produce findings — so `unreadable` is the gate, not an empty catalog. + const reasons = ['not-composed', 'nothing-to-compose', 'config-unloadable', 'stack-only'] as const; + const details = new Map(); + for (const reason of reasons) { + const report = await sweep({ + managed: ['sys_metadata'], + answer: rows('sys_user', 'sys_session', 'sys_account'), + composition: { + servedBoot: { mirrored: false, reason }, + hostConfigPath: reason === 'config-unloadable' ? '/app/objectstack.config.ts' : null, + }, + }); + expect(report.status, reason).toBe('unreadable'); + details.set(reason, (report as { detail: string }).detail); + } + // One reason, one statement: none of them is another's text. + expect(new Set(details.values()).size).toBe(reasons.length); + // Each names its own subject — the config that did not load, or the + // absence of both a config and an artifact (not an artifact it never had). + expect(details.get('config-unloadable')).toContain('/app/objectstack.config.ts'); + expect(details.get('nothing-to-compose')).toContain('neither a host config nor a compiled artifact'); }); it('is unreadable when the managed map cannot be read', async () => { @@ -333,6 +359,66 @@ describe('collectUnmanagedTables — every no-answer path is unreadable, not emp const report = await sweep({ managed: [], answer: [] }); expect(report).toMatchObject({ status: 'read', physicalTables: 0, tables: [] }); }); + + it('is unreadable when the plan holds no driver at all', async () => { + const report = await collectUnmanagedTables({ drivers: [], declaredObjects: [], composition: MIRRORED, normalize }); + expect(report.status).toBe('unreadable'); + }); +}); + +/** + * [#22580] Every database the plan covers. The plan diffs the primary and, + * where the serving boot keeps one, the `telemetry` sibling; a `read` answer + * over the primary alone was a claim about a database never looked at. + */ +describe('collectUnmanagedTables — every planned database, or an answer about none', () => { + const primary = (answer: unknown) => fakeDriver({ + managed: ['sys_user'], filename: '/srv/app/boot.db', answer, + }); + const sibling = (answer: unknown) => fakeDriver({ + managed: ['sys_audit_log'], filename: '/srv/app/boot.telemetry.db', answer, + }); + + it('reports an orphan stranded in the SECOND database, and counts both catalogs', async () => { + const report = await collectUnmanagedTables({ + drivers: [primary(rows('sys_user', 'contacts')), sibling(rows('sys_audit_log', 'sys_retired_event'))], + declaredObjects: [], + composition: MIRRORED, + normalize, + }); + expect(report).toMatchObject({ status: 'read', physicalTables: 4, tables: [{ table: 'sys_retired_event' }] }); + }); + + it('judges each catalog against every planned managed set — a table is never foreign to its own database', async () => { + // Each database holds the other's managed name too. Judged against its own + // managed set alone, each would report the other's table. + const report = await collectUnmanagedTables({ + drivers: [primary(rows('sys_user', 'sys_audit_log')), sibling(rows('sys_audit_log', 'sys_user'))], + declaredObjects: [], + composition: MIRRORED, + normalize, + }); + expect(report).toMatchObject({ status: 'read', physicalTables: 4, tables: [] }); + }); + + it('one database that cannot be read makes the whole report unreadable, naming that database', async () => { + const report = await collectUnmanagedTables({ + drivers: [primary(rows('sys_user', 'sys_scim_provider')), sibling(null)], + declaredObjects: [], + composition: MIRRORED, + normalize, + }); + // ⛔ Not `read` over the primary's orphan: that answer would claim the + // sibling too, which was never read. + expect(report.status).toBe('unreadable'); + expect((report as { detail: string }).detail).toContain('/srv/app/boot.telemetry.db'); + expect((report as { detail: string }).detail).toContain('no result set'); + }); + + it('a rotation shard present in two databases is one name in the finding', () => { + expect(selectUnmanagedTables(['sys_gone__r20260801', 'sys_gone__r20260801'], new Set())) + .toEqual([{ table: 'sys_gone', rotationShards: ['sys_gone__r20260801'] }]); + }); }); describe('resolvePlannedDriverExec — bound to the planned driver, never to some other one', () => { diff --git a/packages/cli/src/utils/unmanaged-tables.ts b/packages/cli/src/utils/unmanaged-tables.ts index 47b40f229e2..79269b2509d 100644 --- a/packages/cli/src/utils/unmanaged-tables.ts +++ b/packages/cli/src/utils/unmanaged-tables.ts @@ -67,17 +67,32 @@ * * "No declaration accounts for this table" is only answerable when the composed * object set actually MIRRORS what this deployment's `os serve` boot registers. - * On a project with a compiled artifact and no host config, the composed set is - * the artifact plus the platform FLOOR — measured: `sys_metadata` and its four - * siblings, `sys_migration`, `sys_migration_journal`, `sys_metadata_activation`, - * `sys_secret`, and nothing else — so a database carrying the other ~40 - * platform tables would have every one of them reported. That is the cry-wolf - * failure, from a knowingly partial premise rather than from a wrong predicate. - * - * So the sweep runs only when `composition.hostConfigLoaded` is true — the same - * discriminator #12953's ruling kept for consumers, and for the same reason: a - * composition that does not mirror the deployment produces an UNMEASURED - * result, and an unmeasured result must say so rather than render as findings. + * Against a knowingly partial set — a host config that did not load, a project + * with nothing to compose, a stack composed without what `serve` mounts around + * it — a database carrying the rest of the platform's tables would have every + * one of them reported. That is the cry-wolf failure, from a knowingly partial + * premise rather than from a wrong predicate. + * + * So the sweep runs only when the composition says it mirrors the served boot + * (`composition.servedBoot`, #22580): a fact the composition records once, + * where it decides it, and that this module reads rather than re-derives. It + * used to be inferred from `composition.hostConfigLoaded`, which is `false` on + * a project with a compiled artifact and no config too — a project whose + * composition has been the served platform since #22506, so the sweep was + * withheld there for a reason that no longer held. A composition that does not + * mirror the deployment produces an UNMEASURED result, and an unmeasured result + * must say so rather than render as findings — giving the reason the + * composition recorded, so the reason it states is one that holds. + * + * ## Every database the plan covers (#22580) + * + * The plan diffs the primary and, when the serving boot keeps one, the + * `telemetry` sibling lifecycle-classed objects live in (ADR-0057 §3.6, + * #22579). The sweep reads every one of those catalogs: an object retired + * from the sibling strands its table THERE, and a sweep of the primary alone + * answered `read` beside a database it never looked at. The known set is the + * union of every planned driver's managed set and the declared names, so no + * table is judged against a set that leaves out its own database's. * * ## Why this lives in `utils/` and not beside `plan.ts` * @@ -105,6 +120,8 @@ import { PLATFORM_OBJECT_PREFIXES, StorageNameMapping, hasPlatformObjectPrefix } // costs nothing `plan.ts` does not already load — that module's own heavy value // imports are deliberately lazy, for the reason its header gives. import { isResultSet } from '../commands/migrate/duplicates.js'; +import { describeDriverConnection } from './connection-display.js'; +import type { SchemaMigrationComposition } from './schema-migration-plugins.js'; /** How a raw SELECT is issued against the driver the plan diffed. */ export type PlannedDriverExec = (sql: string, params?: unknown[]) => Promise; @@ -129,7 +146,7 @@ export interface UnmanagedTablesRead { status: 'read'; /** Namespace prefixes the sweep considered. */ prefixes: readonly string[]; - /** How many physical base tables the sweep enumerated, of any prefix. */ + /** How many physical base tables the sweep enumerated, of any prefix, over every database the plan covers. */ physicalTables: number; /** Sorted, and empty when everything physical is declared. */ tables: UnmanagedTableFinding[]; @@ -265,7 +282,9 @@ export function selectUnmanagedTables( // base name is ever a `managedObjectFields` key. if (known.has(base)) continue; const existing = findings.get(base) ?? { table: base }; - (existing.rotationShards ??= []).push(table); + const shards = (existing.rotationShards ??= []); + // The same shard in two planned databases is one name to report. + if (!shards.includes(table)) shards.push(table); findings.set(base, existing); continue; } @@ -290,11 +309,11 @@ function tableNameOf(row: Record): string | null { * * ⛔ Deliberately NOT `resolveSeedTenancyExec(engine)`, which walks the engine * for ANY raw-capable driver: on a deployment with more than one datasource - * that can be a different database entirely, and this sweep would then compare - * database A's tables against database B's managed set — every row a false - * positive. The identity that matters is the same one `measureComposedCoverage` - * uses (`driver !== plannedDriver`), so the seam is taken from that driver and - * nowhere else. + * that can be a database the plan never diffed, and this sweep would then + * compare its tables against managed sets that do not include its own — every + * row a false positive. The identity that matters is the same one + * `measureComposedCoverage` uses (the planned drivers), so each seam is taken + * from a planned driver and nowhere else. */ export function resolvePlannedDriverExec(driver: unknown): PlannedDriverExec | null { const d = driver as { @@ -312,89 +331,138 @@ const unreadable = (detail: string): UnmanagedTablesUnreadable => ({ detail, }); +/** Why a composition does not mirror the served boot, as the composition recorded it. */ +type NotMirroredReason = Extract['reason']; + /** - * Sweep the physical database for platform-namespaced tables no declaration - * accounts for. - * - * Reads only — one SELECT against a catalog. Every failure lands as - * {@link UnmanagedTablesUnreadable}; none of them throws, because this section - * is informational and must never turn a working `os migrate plan` into a - * failing one. - * - * @param driver the driver whose managed set the plan diffed. - * @param declaredObjects everything the booted stack registered (`stack.allObjects()`). - * @param composition what the boot composed — read for `hostConfigLoaded`, the - * premise above. - * @param normalize `normalizeRows` — flattens the three dialect result shapes. + * The reason the sweep did not run, one sentence per reason the composition can + * record — so the reason stated is the one that holds, and a reason added there + * fails to compile here until it is given its own sentence. */ -export async function collectUnmanagedTables(opts: { - driver: unknown; - declaredObjects: readonly unknown[]; - composition: { hostConfigLoaded: boolean; hostConfigPath: string | null }; - normalize: (result: unknown) => Record[]; -}): Promise { - if (!opts.composition.hostConfigLoaded) { - return unreadable( - opts.composition.hostConfigPath === null - ? 'this project has no host config, so the composed object set is the compiled artifact plus the ' - + 'platform floor rather than what `os serve` registers — against a knowingly partial declaration ' - + 'set, "declared by nothing" is UNMEASURED rather than false' - : `the host config ${opts.composition.hostConfigPath} did not load, so the composed object set covers ` - + 'only a fraction of this deployment — "declared by nothing" is UNMEASURED against it', - ); - } - - const managed = readManagedTableNames(opts.driver); - if (managed === null) { - return unreadable( - 'the planned driver exposes no readable managed-table map, so "declared by nothing" could not be ' - + 'decided — reporting nothing rather than reporting every platform table as unmanaged', - ); - } +const NOT_MIRRORED: Record string> = { + 'config-unloadable': (hostConfigPath) => + `the host config${hostConfigPath ? ` ${hostConfigPath}` : ''} did not load, so the composed object set ` + + 'covers only a fraction of this deployment — "declared by nothing" is UNMEASURED against it', + 'nothing-to-compose': () => + 'this project has neither a host config nor a compiled artifact, so this plan\'s object set is the data ' + + 'stack alone, not a deployment\'s — "declared by nothing" is UNMEASURED against it', + 'stack-only': () => + 'this boot composed the stack but not what `os serve` mounts around it (its auth family and capability ' + + 'providers), so their tables would read as declared by nothing — "declared by nothing" is UNMEASURED ' + + 'against this object set', + 'not-composed': () => + 'this boot did not compose the deployment\'s object set, only the data stack — "declared by nothing" is ' + + 'UNMEASURED against it', +}; - const exec = resolvePlannedDriverExec(opts.driver); +/** + * Read one planned driver's physical BASE TABLE names — or say why not. + * + * Every failure is a `detail`, never an empty list: see this module's header, + * "Could not look" is never reported as "nothing found". + */ +async function readPhysicalTableNames( + driver: unknown, + normalize: (result: unknown) => Record[], +): Promise<{ names: string[] } | { detail: string }> { + const exec = resolvePlannedDriverExec(driver); if (exec === null) { - return unreadable('the planned driver exposes no raw SQL seam, so its table catalog could not be read'); + return { detail: 'the planned driver exposes no raw SQL seam, so its table catalog could not be read' }; } - const client = (opts.driver as { config?: { client?: unknown } } | null | undefined)?.config?.client; + const client = (driver as { config?: { client?: unknown } } | null | undefined)?.config?.client; const clientName = typeof client === 'string' ? client : ''; const sql = physicalTableListSql(clientName); if (sql === null) { - return unreadable( - `the connected dialect (${clientName || 'unnamed client'}) is not one this sweep enumerates tables on ` - + '(sqlite / postgres / mysql)', - ); + return { + detail: `the connected dialect (${clientName || 'unnamed client'}) is not one this sweep enumerates tables on ` + + '(sqlite / postgres / mysql)', + }; } let result: unknown; try { result = await exec(sql, []); } catch (error: unknown) { - return unreadable( - `the table-catalog read failed: ${error instanceof Error ? error.message : String(error)}`, - ); + return { detail: `the table-catalog read failed: ${error instanceof Error ? error.message : String(error)}` }; } // #10677's distinction, restated for this probe: a seam that returns no // RESULT SET did not answer "no tables" — it did not answer. if (!isResultSet(result)) { - return unreadable( - 'the raw SQL seam returned no result set — a seam that cannot answer is not a seam that answered ' - + '"no unmanaged tables"', - ); + return { + detail: 'the raw SQL seam returned no result set — a seam that cannot answer is not a seam that answered ' + + '"no unmanaged tables"', + }; } - const rows = opts.normalize(result); + const rows = normalize(result); const names: string[] = []; for (const row of rows) { const name = tableNameOf(row); if (name !== null) names.push(name); } if (rows.length > 0 && names.length === 0) { - return unreadable( - `the table catalog answered ${rows.length} row(s) carrying no recognised table-name column`, - ); + return { detail: `the table catalog answered ${rows.length} row(s) carrying no recognised table-name column` }; + } + return { names }; +} + +/** + * Sweep the physical databases the plan covers for platform-namespaced tables + * no declaration accounts for. + * + * Reads only — one SELECT against each database's catalog. Every failure lands + * as {@link UnmanagedTablesUnreadable}; none of them throws, because this + * section is informational and must never turn a working `os migrate plan` + * into a failing one. One database that cannot be read makes the whole report + * `unreadable`: a `read` answer is a claim about every database the plan + * covers. + * + * @param drivers every driver the plan diffed (`SchemaStack.drivers`): the + * primary, then the `telemetry` sibling when the serving boot keeps one. + * @param declaredObjects everything the booted stack registered (`stack.allObjects()`). + * @param composition what the boot composed — read for `servedBoot`, the + * premise above, and for the host config the reason may name. + * @param normalize `normalizeRows` — flattens the three dialect result shapes. + */ +export async function collectUnmanagedTables(opts: { + drivers: readonly unknown[]; + declaredObjects: readonly unknown[]; + composition: Pick; + normalize: (result: unknown) => Record[]; +}): Promise { + const { servedBoot } = opts.composition; + if (!servedBoot.mirrored) return unreadable(NOT_MIRRORED[servedBoot.reason](opts.composition.hostConfigPath)); + + if (opts.drivers.length === 0) { + return unreadable('the plan holds no SQL driver, so no table catalog could be read'); + } + // With more than one database, a failure names the one it is about. + const about = (driver: unknown, detail: string): string => { + if (opts.drivers.length === 1) return detail; + const config = (driver as { config?: unknown } | null | undefined)?.config; + return `${describeDriverConnection(config) ?? 'a planned database'}: ${detail}`; + }; + + const managed = new Set(); + for (const driver of opts.drivers) { + const own = readManagedTableNames(driver); + if (own === null) { + return unreadable(about( + driver, + 'the planned driver exposes no readable managed-table map, so "declared by nothing" could not be ' + + 'decided — reporting nothing rather than reporting every platform table as unmanaged', + )); + } + for (const name of own) managed.add(name); + } + + const names: string[] = []; + for (const driver of opts.drivers) { + const read = await readPhysicalTableNames(driver, opts.normalize); + if ('detail' in read) return unreadable(about(driver, read.detail)); + names.push(...read.names); } return { @@ -426,7 +494,7 @@ export function renderUnmanagedTables(report: UnmanagedTablesReport): string[] { } if (report.tables.length === 0) return []; return [ - `${report.tables.length} table(s) in this database carry a reserved platform prefix ` + `${report.tables.length} table(s) in the database(s) this plan covers carry a reserved platform prefix ` + `(${report.prefixes.join(', ')}) and are declared by no object in this plan:`, ...report.tables.map((finding) => ` • ${describeUnmanagedTable(finding)}`), 'They are reported for information only — nothing here drops them, and a plan writes nothing.', diff --git a/packages/cli/test/migrate-unloadable-host-config-exit.e2e.test.ts b/packages/cli/test/migrate-unloadable-host-config-exit.e2e.test.ts index dbe35291d79..e2cdc4a086e 100644 --- a/packages/cli/test/migrate-unloadable-host-config-exit.e2e.test.ts +++ b/packages/cli/test/migrate-unloadable-host-config-exit.e2e.test.ts @@ -203,6 +203,14 @@ describe('os migrate plan/apply refuse an unloadable host config (#12953)', () = expect(() => JSON.parse(unloadablePlan.stdout)).not.toThrow(); expect(() => JSON.parse(unloadableApply.stdout)).not.toThrow(); }); + + it('[#22580] withholds the unmanaged-tables sweep, naming the config that did not load', () => { + // A set that cannot be composed is not the served boot's: the sweep's + // reason is the composition's own, and it names the config. + const payload = JSON.parse(unloadablePlan.stdout) as { unmanagedTables?: { status?: string; detail?: string } }; + expect(payload.unmanagedTables?.status).toBe('unreadable'); + expect(payload.unmanagedTables?.detail).toContain('objectstack.config.ts'); + }); }); describe('direction 2 — there is NO host config: unchanged', () => { @@ -221,6 +229,15 @@ describe('os migrate plan/apply refuse an unloadable host config (#12953)', () = expect(absentPlan.stderr).not.toMatch(/Remedy:/); expect(absentApply.stderr).not.toMatch(/Remedy:/); }); + + it('[#22580] withholds the unmanaged-tables sweep for the reason that holds: nothing to compose', () => { + // Neither a config nor a compiled artifact: the set is the data stack + // alone. The reason used to claim a compiled artifact this project + // never had. + const payload = JSON.parse(absentPlan.stdout) as { unmanagedTables?: { status?: string; detail?: string } }; + expect(payload.unmanagedTables?.status).toBe('unreadable'); + expect(payload.unmanagedTables?.detail).toContain('neither a host config nor a compiled artifact'); + }); }); describe('direction 3 — the host config LOADS: unchanged', () => { @@ -237,5 +254,10 @@ describe('os migrate plan/apply refuse an unloadable host config (#12953)', () = expect(loadablePlan.stderr).not.toMatch(/Remedy:/); expect(loadableApply.stderr).not.toMatch(/Remedy:/); }); + + it('[#22580] runs the unmanaged-tables sweep: the set mirrors the served boot', () => { + const payload = JSON.parse(loadablePlan.stdout) as { unmanagedTables?: { status?: string } }; + expect(payload.unmanagedTables?.status).toBe('read'); + }); }); });