fix(cli): os migrate plan no longer lists sys_packages as unmanaged after os migrate apply created it

.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.

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)', () => {

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);
 

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,
       });

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);
 });

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 =>

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 () => {