From 04c0d95ec9856165874b8d845fbbff8bcd19f0d7 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 17 Oct 2026 21:53:06 +0000 Subject: [PATCH] fix(cli): the unmanaged-table sweep counts the tables a composition declares outside the object set os migrate plan listed sys_packages under "declared by no object in this plan" right after os migrate apply created it. The sweep's known set was the planned drivers' managed tables plus the declared objects; sys_packages is neither. bootSchemaStack now reads the package.storage declaration once (findPackageStorage), plans the table from it as before, and exposes its table name on every boot as SchemaStack.nonObjectTables, which plan.ts hands to collectUnmanagedTables and buildKnownTableNames adds verbatim. Claude-Session: https://claude.ai/code/session_01QyoWKK9RwUNxBxmevy9Neb Co-Authored-By: Claude --- .../24059-unmanaged-sweep-sys-packages.md | 14 ++++ .../plan.boot-parity.integration.test.ts | 13 +-- ...plan.telemetry-sibling.integration.test.ts | 5 +- packages/cli/src/commands/migrate/plan.ts | 3 + ...-migrate.package-table.integration.test.ts | 10 ++- packages/cli/src/utils/schema-migrate.ts | 80 ++++++++++++++----- .../unmanaged-tables.integration.test.ts | 20 ++++- .../cli/src/utils/unmanaged-tables.test.ts | 65 +++++++++++++-- packages/cli/src/utils/unmanaged-tables.ts | 22 ++++- 9 files changed, 189 insertions(+), 43 deletions(-) create mode 100644 .changeset/24059-unmanaged-sweep-sys-packages.md diff --git a/.changeset/24059-unmanaged-sweep-sys-packages.md b/.changeset/24059-unmanaged-sweep-sys-packages.md new file mode 100644 index 00000000000..beef3ccf37b --- /dev/null +++ b/.changeset/24059-unmanaged-sweep-sys-packages.md @@ -0,0 +1,14 @@ +--- +'@objectstack/cli': patch +--- + +fix(cli): `os migrate plan` no longer lists `sys_packages` among the tables "declared by no object in this plan" after `os migrate apply` has created it + +Clause-②: no + +`sys_packages` is the package registry's table. It is not an object: `PackageServicePlugin` creates it with its own DDL, and `os migrate apply` creates it from the registry's `package.storage` declaration. The plan's unmanaged-table sweep built its known set from the planned drivers' managed tables and the declared objects only, so on every applied deployment the plan reported the live registry table as if it were stranded. + +- **Now:** the sweep also counts the tables the boot's composition declares outside the object set. It reads that name from the same `package.storage` declaration the plan creates the table from. It reads it on every boot, whether or not the table exists yet, the skip-schema-sync boot of a Turso remote primary included. On `examples/app-crm` after `os migrate apply --yes`, the plan lists only a genuinely stranded `sys_*` table. +- **Still reported:** a `sys_packages` table in the database of a deployment that composes no package registry. Nothing there declares it. +- **Refused in one more case:** a `package.storage` service whose shape this CLI does not read now fails `plan`, `apply` and `organization-ownership` on a skip-schema-sync boot too, with the error those commands already gave on every other boot: `@objectstack/cli` and `@objectstack/service-package` must come from the same release. +- **Unchanged:** the `--json` keys, the section's wording, and what `apply` creates. 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 cc086148451..71195d64238 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 @@ -40,12 +40,14 @@ import Database from 'better-sqlite3'; * included) — with `pending` empty, equal counts are equal sets; * - 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`. + * included (#22580) — and names nothing: [#24059] not even `sys_packages`, + * which the composed package registry declares outside the object set. * * `sys_packages` is the one table outside the object set, by construction: * `PackageServicePlugin.start()` creates it with raw DDL (`package-table.ts`), - * and no object declares it. A new raw-DDL table fails this pin and has to be - * named here with the same reason. + * and no object declares it — its `package.storage` declaration does, and the + * sweep reads that declaration. A new raw-DDL table fails this pin and has to + * be named here with the same reason. * * Both sides run with `OS_TELEMETRY_DB=0`, so the pin compares object sets on * one database. A development boot's `telemetry` sibling (ADR-0057 §3.6) — a @@ -145,10 +147,11 @@ function expectParity(r: Reading): void { // … 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. + // with a reason that no longer held. [#24059] It reports nothing: the + // raw-DDL table above exists, and its declaration accounts for it. 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); + expect(unmanaged).toEqual([]); } 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 c9102fdd509..68aea502606 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 @@ -194,9 +194,10 @@ describe('os migrate plan / apply plan each object against the database it lives // 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. + // reported — and [#24059] the primary's raw-DDL one is not, its + // declaration accounts for it. 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()); + expect(plan.payload.unmanagedTables.tables.map((t: any) => t.table)).toEqual([retired]); // 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 c7eff151612..e3011dcc785 100644 --- a/packages/cli/src/commands/migrate/plan.ts +++ b/packages/cli/src/commands/migrate/plan.ts @@ -232,6 +232,9 @@ export default class MigratePlan extends Command { // resolved from it are two connections to one file. drivers: distinctDatabaseDrivers(stack.drivers), declaredObjects: stack.allObjects(), + // [#24059] …and the tables the composition declares outside the object + // set — `sys_packages`, which `os migrate apply` creates. + nonObjectTables: stack.nonObjectTables, composition: stack.composition, normalize: normalizeRows, }); diff --git a/packages/cli/src/utils/schema-migrate.package-table.integration.test.ts b/packages/cli/src/utils/schema-migrate.package-table.integration.test.ts index 1b442391432..06b8cfaca6c 100644 --- a/packages/cli/src/utils/schema-migrate.package-table.integration.test.ts +++ b/packages/cli/src/utils/schema-migrate.package-table.integration.test.ts @@ -32,7 +32,10 @@ * still absent while the plan is on screen), the flush creates it and * reports it, and the table is the one the boot's own DDL creates — * `PRIMARY KEY (id, version)`, `TEXT` timestamps, `idx_packages_latest`; - * 3. a plan over the migrated database lists nothing for it. + * 3. a plan over the migrated database lists nothing for it — and [#24059] + * still names it among the tables the composition declares outside the + * object set (`nonObjectTables`), which is what keeps `os migrate plan`'s + * unmanaged-table sweep from reporting the table this apply created. * * The flag is SET for every boot here: the documented path must not depend on * it. That the boot itself creates nothing under the flag is @@ -125,6 +128,7 @@ describe('os migrate plan / apply cover sys_packages, flag set (#23022)', () => stack = await boot('plan'); expect(stack.composition.servedBoot).toEqual({ mirrored: true }); expect(packageWork(stack)).toEqual([{ table: 'sys_packages', kind: 'create_table', columns: COLUMNS }]); + expect(stack.nonObjectTables).toEqual(['sys_packages']); await stack.shutdown(); stack = null; expect(existsSync(dbFile), 'a dry run brought the database into existence').toBe(false); @@ -150,8 +154,10 @@ describe('os migrate plan / apply cover sys_packages, flag set (#23022)', () => index: [{ name: 'id' }, { name: 'created_at' }], }); - // 3. plan again — the table is there, so nothing is pending for it. + // 3. plan again — the table is there, so nothing is pending for it… stack = await boot('plan'); expect(packageWork(stack)).toEqual([]); + // …and [#24059] the declaration still names it, from the same service. + expect(stack.nonObjectTables).toEqual(['sys_packages']); }, 300_000); }); diff --git a/packages/cli/src/utils/schema-migrate.ts b/packages/cli/src/utils/schema-migrate.ts index 12b249f0d10..f97922f7ea0 100644 --- a/packages/cli/src/utils/schema-migrate.ts +++ b/packages/cli/src/utils/schema-migrate.ts @@ -198,6 +198,19 @@ export interface SchemaStack { * (#3917). Always `[]` unless the stack was booted with `deferSchemaDdl`. */ pendingSchemaWork: PendingSchemaWork[]; + /** + * [#24059] The physical tables this boot's composition declares OUTSIDE the + * object set: the package registry's `sys_packages`, named by the same + * `package.storage` declaration {@link pendingSchemaWork} plans it from + * (#23022) — or `[]` when the boot composed no package registry. + * + * Named whether or not the table exists, and on every boot: the plan lists the + * table as `create_table` only while it is absent, so once `os migrate apply` + * has created it the pending work names nothing. `os migrate plan`'s + * unmanaged-table sweep reads this, so the table the apply created is not + * reported as declared by nothing. + */ + nonObjectTables: readonly string[]; /** * [#21529] Did this boot MEASURE `objectName`'s table as absent? `true` only * when the held-back sync above would CREATE it (`create_table`), which the @@ -331,29 +344,21 @@ interface PackageStorageDeclaration { } /** - * [#23022] `sys_packages` as pending schema work, when it is absent. + * [#23022] The package registry's declaration of `sys_packages`, as this boot + * composed it — or `null` when the boot composed no package registry. * - * ## Why the plan needs this + * [#24059] The ONE reader of that declaration. {@link planPackageTable} plans + * the table from what this returns, and {@link SchemaStack.nonObjectTables} + * names the table from the same object, so `os migrate plan`'s unmanaged-table + * sweep knows the table by the source the plan creates it from — and knows it + * whether or not the table exists, which the plan's pending work cannot say: + * that lists the table only while it is absent. * - * `PackageServicePlugin.start()` creates `sys_packages` with raw DDL, because - * the table cannot be a declared object (`service-package`'s - * `package-table.ts` records the measurement: the driver's managed tables key - * on `id` alone, and this one keys on `(id, version)`). So no driver deferral - * ever listed it, and a deployment that sets `OS_SKIP_SCHEMA_SYNC=1` — whose - * boot now creates nothing — was left with no way to get it from the documented - * path. The plugin declares the table in `init()`, which a migrate boot runs, - * and this lists it as `create_table` beside the objects' tables; `apply`'s - * flush then runs the plugin's own statements, so `apply` creates exactly what - * the boot creates. - * - * `null` when there is nothing to plan: the plugin is not composed, or the - * table exists. When its presence could not be READ, that is not absence: a - * note says the table was not examined, and nothing is planned. + * Throws when the service answers in a shape this CLI does not read: the two + * packages come from different releases, and neither the plan nor the sweep can + * name the table. */ -async function planPackageTable( - kernel: unknown, - notes: string[], -): Promise<{ storage: PackageStorageDeclaration; work: PendingSchemaWork } | null> { +function findPackageStorage(kernel: unknown): PackageStorageDeclaration | null { let service: unknown; try { service = (kernel as { getService?: (name: string) => unknown }).getService?.(PACKAGE_STORAGE_SERVICE); @@ -372,7 +377,33 @@ async function planPackageTable( + 'from the same release.', ); } - const declared = storage as PackageStorageDeclaration; + return storage as PackageStorageDeclaration; +} + +/** + * [#23022] `sys_packages` as pending schema work, when it is absent. + * + * ## Why the plan needs this + * + * `PackageServicePlugin.start()` creates `sys_packages` with raw DDL, because + * the table cannot be a declared object (`service-package`'s + * `package-table.ts` records the measurement: the driver's managed tables key + * on `id` alone, and this one keys on `(id, version)`). So no driver deferral + * ever listed it, and a deployment that sets `OS_SKIP_SCHEMA_SYNC=1` — whose + * boot now creates nothing — was left with no way to get it from the documented + * path. The plugin declares the table in `init()`, which a migrate boot runs, + * and this lists it as `create_table` beside the objects' tables; `apply`'s + * flush then runs the plugin's own statements, so `apply` creates exactly what + * the boot creates. + * + * `null` when there is nothing to plan: the table exists. When its presence + * could not be READ, that is not absence: a note says the table was not + * examined, and nothing is planned. + */ +async function planPackageTable( + declared: PackageStorageDeclaration, + notes: string[], +): Promise<{ storage: PackageStorageDeclaration; work: PendingSchemaWork } | null> { const present = await declared.present(); if (present === true) return null; if (present === undefined) { @@ -1386,7 +1417,11 @@ export async function bootSchemaStack( // [#23022] …and the one table outside the object set, `sys_packages`, which // no driver deferral can see. Read-only here; `flushSchemaDdl` creates it. // Not on a skip-sync boot: the table lives on the primary, which held nothing back. - let packageTable = defer ? await planPackageTable(kernel, composition.notes) : null; + // [#24059] The declaration itself is read on every boot, a skip-sync one + // included: `nonObjectTables` below names the table from it whether or not + // anything is planned for it. + const packageStorage = findPackageStorage(kernel); + let packageTable = defer && packageStorage ? await planPackageTable(packageStorage, composition.notes) : null; if (packageTable) pendingSchemaWork.push(packageTable.work); sortPendingSchemaWork(pendingSchemaWork); // [#21529] The tables the held-back sync would create: measured absent. @@ -1454,6 +1489,7 @@ export async function bootSchemaStack( managedTableCount, kernel, pendingSchemaWork, + nonObjectTables: packageStorage ? [packageStorage.table] : [], // The driver keys its deferred work by physical table, so ask in the same // name (`StorageNameMapping`, the mapping the driver itself applies). tableAbsent: (objectName: string): boolean => diff --git a/packages/cli/src/utils/unmanaged-tables.integration.test.ts b/packages/cli/src/utils/unmanaged-tables.integration.test.ts index 80a8a0a93fb..f65e102e3ca 100644 --- a/packages/cli/src/utils/unmanaged-tables.integration.test.ts +++ b/packages/cli/src/utils/unmanaged-tables.integration.test.ts @@ -23,10 +23,12 @@ * seam, against tables that really exist. * * ⛔ The NEGATIVE control is the point. `sys_user_permission_set` (from the composed - * plugin) and `sys_secret` (from the platform floor) are put in the database - * BEFORE the boot and must NOT be reported, alongside `_objectstack_sequences` - * and an application table. A sweep observed only in the presence of an orphan - * would be indistinguishable from one that reports everything. + * plugin), `sys_secret` (from the platform floor) and [#24059] `sys_packages` + * (the package registry's, declared outside the object set) are put in the + * database BEFORE the boot and must NOT be reported, alongside + * `_objectstack_sequences` and an application table. A sweep observed only in + * the presence of an orphan would be indistinguishable from one that reports + * everything. * * The positive control is the card's own measured case: a `sys_`-prefixed table * left behind by a retired object, which no plan can mention today. @@ -81,6 +83,9 @@ const NEGATIVE_CONTROLS = [ // declared by nothing, so the sweep is RIGHT to report it. 'sys_user_permission_set', // declared by the composed host plugin 'sys_secret', // declared by the platform floor + // [#24059] Not an object: the composed package registry declares it through + // `package.storage`, and `os migrate apply` creates it from that declaration. + 'sys_packages', '_objectstack_sequences', // the driver's own autonumber ledger 'app_leftovers', // no reserved prefix at all ]; @@ -143,6 +148,7 @@ describe('os migrate plan — unmanaged tables, against a real database (#13204) return collectUnmanagedTables({ drivers: stack!.drivers, declaredObjects: stack!.allObjects(), + nonObjectTables: stack!.nonObjectTables, composition: stack!.composition, normalize: normalizeRows, }); @@ -209,6 +215,12 @@ describe('os migrate plan — unmanaged tables, against a real database (#13204) const declared = (stack!.allObjects() as Array<{ name?: string }>).map((o) => o?.name); expect(declared).toContain('sys_user_permission_set'); expect(declared).toContain('sys_secret'); + // [#24059] The table outside the object set is named by the declaration the + // plan creates it from — the composed registry's own `package.storage` — + // although the table exists here, so the plan has nothing pending for it. + const storage = stack!.kernel.getService('package.storage') as { table: string }; + expect(stack!.nonObjectTables).toEqual([storage.table]); + expect(stack!.pendingSchemaWork.filter((w) => w.table === storage.table)).toEqual([]); }); it('REPORTS the stranded table, and reports ONLY it', async () => { diff --git a/packages/cli/src/utils/unmanaged-tables.test.ts b/packages/cli/src/utils/unmanaged-tables.test.ts index 898975274c1..932e0cb0e04 100644 --- a/packages/cli/src/utils/unmanaged-tables.test.ts +++ b/packages/cli/src/utils/unmanaged-tables.test.ts @@ -18,7 +18,9 @@ * with `strategy: 'rotation'`, 14 daily shards, and only the BASE name is * ever a `managedObjectFields` key); * - driver-internal tables (`_objectstack_sequences`, `__os_mig_*`); - * - an application table with no reserved prefix. + * - an application table with no reserved prefix; + * - [#24059] a table the composition declares OUTSIDE the object set + * (`sys_packages`, the package registry's), which `os migrate apply` creates. * * And the third direction, which is neither: every way the sweep can fail to * OBTAIN an answer must report `unreadable`, never an empty `tables` list. @@ -86,6 +88,8 @@ const MIRRORED: Premise = { servedBoot: { mirrored: true }, hostConfigPath: '/ap async function sweep(opts: { managed?: string[] | null; declared?: Array<{ name: string }>; + /** `SchemaStack.nonObjectTables` — none unless a case declares one. */ + nonObjectTables?: string[]; client?: string | null; answer?: unknown | (() => unknown); composition?: Premise; @@ -93,6 +97,7 @@ async function sweep(opts: { return collectUnmanagedTables({ drivers: [fakeDriver(opts)], declaredObjects: opts.declared ?? [], + nonObjectTables: opts.nonObjectTables ?? [], composition: opts.composition ?? MIRRORED, normalize, }); @@ -166,23 +171,66 @@ describe('readManagedTableNames — an unreadable map is null, never an empty se describe('buildKnownTableNames — the managed set UNION every declared object', () => { it('carries both, so a declared-but-unexamined object is accounted for', () => { - const known = buildKnownTableNames(new Set(['sys_user']), [{ name: 'sys_secret' }]); + const known = buildKnownTableNames(new Set(['sys_user']), [{ name: 'sys_secret' }], []); expect(known.has('sys_user')).toBe(true); expect(known.has('sys_secret')).toBe(true); }); it('adds a legacy double-underscore name under both spellings', () => { - const known = buildKnownTableNames(new Set(), [{ name: 'crm__account' }]); + const known = buildKnownTableNames(new Set(), [{ name: 'crm__account' }], []); expect(known.has('crm__account')).toBe(true); expect(known.has('account')).toBe(true); }); it('ignores objects with no usable name', () => { - const known = buildKnownTableNames(new Set(), [{}, { name: '' }, null, undefined] as never[]); + const known = buildKnownTableNames(new Set(), [{}, { name: '' }, null, undefined] as never[], []); expect(known.size).toBe(0); }); }); +/** + * [#24059] The table a composition declares OUTSIDE the object set: + * `sys_packages`, which `os migrate apply` creates from the package registry's + * `package.storage` declaration (#23022) and which no object declares. Neither + * the managed set nor the declared objects name it, so every plan after an + * apply reported the table that apply had just created. Its name reaches the + * sweep from that same declaration (`SchemaStack.nonObjectTables`); these hold + * the sweep to it in both directions. + */ +describe('[#24059] a table the composition declares outside the object set', () => { + it('is in the known set, verbatim — and only when declared', () => { + expect(buildKnownTableNames(new Set(['sys_user']), [{ name: 'sys_secret' }], ['sys_packages']).has('sys_packages')) + .toBe(true); + // The managed set and the declared objects alone never name it. + expect(buildKnownTableNames(new Set(['sys_user']), [{ name: 'sys_secret' }], []).has('sys_packages')) + .toBe(false); + }); + + it('after an apply: the registry table is not reported, a genuinely stranded table still is', async () => { + const report = await sweep({ + managed: ['sys_user'], + declared: [{ name: 'sys_secret' }], + nonObjectTables: ['sys_packages'], + answer: rows('sys_user', 'sys_secret', 'sys_packages', 'sys_legacy_widget'), + }); + expect(report).toMatchObject({ status: 'read', physicalTables: 4, tables: [{ table: 'sys_legacy_widget' }] }); + }); + + it('DECISION: a boot that composed no package registry declares no such table, so a sys_packages left in its database IS reported', async () => { + // No declaration accounts for it there: nothing on that deployment creates + // or reads the table, which is what this section reports. + const report = await sweep({ + managed: ['sys_user'], + nonObjectTables: [], + answer: rows('sys_user', 'sys_packages', 'sys_legacy_widget'), + }); + expect(report).toMatchObject({ + status: 'read', + tables: [{ table: 'sys_legacy_widget' }, { table: 'sys_packages' }], + }); + }); +}); + describe('selectUnmanagedTables — the predicate', () => { it('REPORTS a platform-prefixed table nothing declares', () => { // The card's measured case: `sys_scim_provider` retired, table still there. @@ -200,7 +248,7 @@ describe('selectUnmanagedTables — the predicate', () => { // one real orphan. const declared = Array.from({ length: 72 }, (_, i) => ({ name: `sys_declared_${i}` })); const physical = [...declared.map((o) => o.name), 'sys_scim_provider']; - const known = buildKnownTableNames(new Set(['sys_user']), declared); + const known = buildKnownTableNames(new Set(['sys_user']), declared, []); expect(selectUnmanagedTables(physical, known)).toEqual([{ table: 'sys_scim_provider' }]); }); @@ -361,7 +409,9 @@ describe('collectUnmanagedTables — every no-answer path is unreadable, not emp }); it('is unreadable when the plan holds no driver at all', async () => { - const report = await collectUnmanagedTables({ drivers: [], declaredObjects: [], composition: MIRRORED, normalize }); + const report = await collectUnmanagedTables({ + drivers: [], declaredObjects: [], nonObjectTables: [], composition: MIRRORED, normalize, + }); expect(report.status).toBe('unreadable'); }); }); @@ -383,6 +433,7 @@ describe('collectUnmanagedTables — every planned database, or an answer about const report = await collectUnmanagedTables({ drivers: [primary(rows('sys_user', 'contacts')), sibling(rows('sys_audit_log', 'sys_retired_event'))], declaredObjects: [], + nonObjectTables: [], composition: MIRRORED, normalize, }); @@ -395,6 +446,7 @@ describe('collectUnmanagedTables — every planned database, or an answer about const report = await collectUnmanagedTables({ drivers: [primary(rows('sys_user', 'sys_audit_log')), sibling(rows('sys_audit_log', 'sys_user'))], declaredObjects: [], + nonObjectTables: [], composition: MIRRORED, normalize, }); @@ -405,6 +457,7 @@ describe('collectUnmanagedTables — every planned database, or an answer about const report = await collectUnmanagedTables({ drivers: [primary(rows('sys_user', 'sys_scim_provider')), sibling(null)], declaredObjects: [], + nonObjectTables: [], composition: MIRRORED, normalize, }); diff --git a/packages/cli/src/utils/unmanaged-tables.ts b/packages/cli/src/utils/unmanaged-tables.ts index 6add5c0c8d2..99e653e8d5d 100644 --- a/packages/cli/src/utils/unmanaged-tables.ts +++ b/packages/cli/src/utils/unmanaged-tables.ts @@ -57,6 +57,14 @@ * 3. **Driver-internal tables.** `_objectstack_sequences` and the SQLite * rebuild scratch table `__os_mig_` carry no platform prefix, so the * prefix test excludes them without an allowlist to maintain. + * 4. **A table a composed service declares outside the object set (#24059).** + * `sys_packages` is the package registry's, created by its own raw DDL and + * by `os migrate apply` from the registry's `package.storage` declaration + * (#23022) — never an object, so neither set above names it, and every plan + * after an apply reported the table that apply had just created. The name + * comes from that same declaration ({@link SchemaStack.nonObjectTables}), + * never from a list here; a boot that composed no package registry declares + * none, and a `sys_packages` left in its database IS reported. * * The prefix itself is read from `PLATFORM_OBJECT_PREFIXES` rather than * spelled `'sys_'` here. That constant is the repo's own registry of namespaces @@ -122,6 +130,7 @@ import { PLATFORM_OBJECT_PREFIXES, StorageNameMapping, hasPlatformObjectPrefix } // 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 { SchemaStack } from './schema-migrate.js'; import type { SchemaMigrationComposition } from './schema-migration-plugins.js'; /** How a raw SELECT is issued against the driver the plan diffed. */ @@ -236,7 +245,8 @@ export function readManagedTableNames(driver: unknown): ReadonlySet | nu /** * Every table name a declaration accounts for: the planned driver's managed set - * UNION the table names of every object the booted stack registered. + * UNION the table names of every object the booted stack registered UNION the + * tables the boot's composition declares outside the object set. * * The managed set is the plan's own (premise: one source, so the section cannot * disagree with the plan beside it). The declared set is the wider one, and it @@ -246,12 +256,17 @@ export function readManagedTableNames(driver: unknown): ReadonlySet | nu * Both an object's `name` and its resolved table name are added: legacy * double-underscore names resolve to a different table, and over-excluding is * the safe direction for a section whose failure mode is crying wolf. + * + * `nonObjectTables` is {@link SchemaStack.nonObjectTables} (#24059): physical + * names, added verbatim — they are not objects, so no object mapping applies. */ export function buildKnownTableNames( managed: ReadonlySet, declaredObjects: readonly unknown[], + nonObjectTables: readonly string[], ): ReadonlySet { const known = new Set(managed); + for (const table of nonObjectTables) known.add(table); for (const object of declaredObjects) { const name = (object as { name?: unknown } | null | undefined)?.name; if (typeof name !== 'string' || !name) continue; @@ -430,6 +445,8 @@ async function readPhysicalTableNames( * @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 nonObjectTables the tables the boot's composition declares outside the + * object set (`stack.nonObjectTables`, #24059). * @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. @@ -437,6 +454,7 @@ async function readPhysicalTableNames( export async function collectUnmanagedTables(opts: { drivers: readonly unknown[]; declaredObjects: readonly unknown[]; + nonObjectTables: SchemaStack['nonObjectTables']; composition: Pick; normalize: (result: unknown) => Record[]; }): Promise { @@ -477,7 +495,7 @@ export async function collectUnmanagedTables(opts: { status: 'read', prefixes: PLATFORM_OBJECT_PREFIXES, physicalTables: names.length, - tables: selectUnmanagedTables(names, buildKnownTableNames(managed, opts.declaredObjects)), + tables: selectUnmanagedTables(names, buildKnownTableNames(managed, opts.declaredObjects, opts.nonObjectTables)), }; }