Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .changeset/22580-unmanaged-tables-served-boot.md
Original file line number Diff line number Diff line change
@@ -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".
Original file line number Diff line number Diff line change
Expand Up @@ -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`),
Expand Down Expand Up @@ -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)', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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']);
Expand All @@ -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);

Expand Down
3 changes: 2 additions & 1 deletion packages/cli/src/commands/migrate/plan.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
5 changes: 4 additions & 1 deletion packages/cli/src/utils/schema-migrate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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) {
Expand Down
36 changes: 34 additions & 2 deletions packages/cli/src/utils/schema-migration-plugins.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down Expand Up @@ -468,7 +470,7 @@ describe('describeUnloadableHostConfig (#12953)', () => {
function composition(over: Partial<SchemaMigrationComposition>): SchemaMigrationComposition {
return {
plugins: [], hostConfigPath: null, hostConfigLoaded: false, hostConfigError: null,
notes: [], coverage: null, ...over,
servedBoot: { mirrored: false, reason: 'nothing-to-compose' }, notes: [], coverage: null, ...over,
};
}

Expand Down Expand Up @@ -537,7 +539,7 @@ describe('describeUnloadableHostConfig — the no-DDL notice (#13118)', () => {
function composition(over: Partial<SchemaMigrationComposition>): SchemaMigrationComposition {
return {
plugins: [], hostConfigPath: null, hostConfigLoaded: false, hostConfigError: null,
notes: [], coverage: null, ...over,
servedBoot: { mirrored: false, reason: 'nothing-to-compose' }, notes: [], coverage: null, ...over,
};
}

Expand Down Expand Up @@ -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)', () => {
Expand Down
44 changes: 43 additions & 1 deletion packages/cli/src/utils/schema-migration-plugins.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -1678,6 +1715,7 @@ export async function buildSchemaMigrationPlugins(opts: {
console.warn(`[migrate] ⚠ ${line}`);
notes.push(line);
hostConfigError = message;
servedBoot = { mirrored: false, reason: 'config-unloadable' };
}
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 } : {}),
};
}
Expand Down
6 changes: 5 additions & 1 deletion packages/cli/src/utils/unmanaged-tables.integration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<UnmanagedTablesReport> {
const { normalizeRows } = await import('@objectstack/metadata-protocol');
return collectUnmanagedTables({
driver: stack!.driver,
drivers: stack!.drivers,
declaredObjects: stack!.allObjects(),
composition: stack!.composition,
normalize: normalizeRows,
Expand Down Expand Up @@ -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');
Expand Down
Loading
Loading