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
19 changes: 19 additions & 0 deletions .changeset/plain-pandas-battle.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
---
"@objectstack/driver-sql": patch
"@objectstack/cli": patch
---

`os migrate plan` / `apply` examine the object set the composed host DECLARED, and report the boundary when they cannot

A composed host stack (#12938) registers its plugins for their DECLARATIONS: `init()` runs, `start()` is suppressed. The pass that hands every registered object to its driver — the one that fills the `managedObjectFields` map `detectManagedDrift()` diffs — lives in `ObjectQLPlugin.start()`, and a host that brings its own `ObjectQLPlugin` (under the framework's own plugin name, so the CLI's capability injector de-dups against it) DISPLACES the standalone one, since duplicate registration overwrites by name. The result was a boot where no `ObjectQLPlugin.start()` ran at all: every host plugin declared its objects, and not one reached a driver.

Measured on ObjectStack Cloud's staging control plane: 36 host plugins composed, ~80 `sys_*` tables declared, **8** examined — all eight belonging to the single service that provisions its own tables from a `kernel:ready` hook rather than relying on that pass. Every consumer-visible signal was green, and `Physical schema is in sync with metadata` was one composed plugin away from printing over seventy unexamined tables.

Two changes:

- **The composed boot now drives that pass itself**, over the deferral it already armed: `engine.syncObjectSchema(name)` per declared object, which reaches `SqlDriver.initObjects` exactly as the suppressed `start()` would have. A plan still writes nothing — the deferral records the create-table work instead of running it.
- **`plan` / `apply` report what they could NOT examine.** `--json` payloads gain `composition.coverage` (`registeredObjects`, `examinedObjects`, `unexaminedObjects`, and per-reason counts: federated, unbound, on another datasource, on a driver without schema registration, refused). When `unexaminedObjects > 0`, the human output refuses the unqualified "in sync" line and says the plan is PARTIAL instead. A consumer gate asserting coverage should read `composition.coverage.unexaminedObjects` — `managedTables` alone cannot tell a small deployment apart from a mostly unexamined one.

`@objectstack/driver-sql`: `initObjects` no longer calls `ensureDatabaseExists()` while DDL is deferred. It is the one line there that can write — `mkdir -p` for a sqlite parent directory, and on Postgres/MySQL a `SELECT 1` that CREATEs the database on `3D000` / `ER_BAD_DB_ERROR` — and under the deferral there is no DDL for a database to exist for. `flushDeferredSchemaDdl` clears the flag before re-entering, so the confirmed `os migrate apply` still ensures the database ahead of the first `CREATE TABLE`.

A project with neither an `objectstack.config.*` nor a compiled artifact is unchanged: it composes nothing, carries no `composition` key, and diffs the same five data-stack tables it always did.
13 changes: 13 additions & 0 deletions .changeset/silver-eagles-shout.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
---
"@objectstack/cli": patch
---

`os migrate plan` / `os migrate apply` exit when their work is done

Measured on ObjectStack Cloud's staging control plane, inside `docker run --rm`: the CLI finished in 4.3 seconds and printed its own `Graceful shutdown complete`, and the run was cancelled by hand **78 minutes later** — the shell's next statement never ran, so it was still blocked on that one `docker run`.

The composition these commands perform (#12938) registers a host's plugins for their DECLARATIONS: `init()` runs, `start()` is replaced with a no-op. Anything a host plugin arms during Phase 1 whose release would have been installed by Phase 2 — an interval, a pool, a watcher, a `kernel:ready` hook that starts a dispatcher — has no release path at all, so the event loop never drains while the kernel reports a clean shutdown.

Both commands now end the process deliberately once their document is written, after the kernel teardown they already ran. Chasing the handle instead would mean auditing host code this repo cannot see, which is the same argument that made the composition declaration-only in the first place. `stdout` and `stderr` are drained before the exit, so a `--json` payload on a pipe is not truncated — and the drain itself is bounded, so a pipe whose reader has gone away cannot become a second way for the command not to return.

Failure paths are unchanged: `this.exit(n)` throws an oclif `ExitError` that oclif's own handler already turns into a `process.exit`.
47 changes: 46 additions & 1 deletion packages/cli/src/commands/migrate/apply.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
summarizePendingSchemaWork,
groupByCategory,
} from '../../utils/schema-migrate.js';
import { exitOneShotCommand } from '../../utils/one-shot-exit.js';
import { OCCUPANCY_HINT, probeMigrationTarget } from '../../utils/migrate-occupancy-gate.js';
import { describeOccupancy } from '../../utils/sqlite-occupancy.js';

Expand Down Expand Up @@ -81,7 +82,18 @@ export default class MigrateApply extends Command {
json: Flags.boolean({ description: 'Output as JSON (implies non-interactive; requires --yes to mutate)' }),
};

/**
* #13027 — the process must end when the apply does.
*
* See `migrate/plan.ts`'s twin for the measurement, and for why the failure
* paths deliberately stay on oclif's own exit.
*/
async run(): Promise<void> {
await this.apply();
await exitOneShotCommand(typeof process.exitCode === 'number' ? process.exitCode : 0);
}

private async apply(): Promise<void> {
const { flags } = await this.parse(MigrateApply);
const timer = createTimer();
const allowDestructive = flags['allow-destructive'];
Expand Down Expand Up @@ -167,8 +179,40 @@ export default class MigrateApply extends Command {
// target database, so it belongs in the plan and behind the prompt.
const pending = stack.pendingSchemaWork;

// [#13028] The boundary of what was reconciled, carried on every payload
// that can be read as "this deployment is migrated". A consumer gate
// needs `unexaminedObjects` — `applied: []` alone cannot tell "nothing to
// do" apart from "most of it was never looked at".
const compositionPayload = stack.composition.notes.length > 0
? {
composition: {
hostConfig: stack.composition.hostConfigPath,
hostConfigLoaded: stack.composition.hostConfigLoaded,
...(stack.composition.coverage ? { coverage: stack.composition.coverage } : {}),
notes: stack.composition.notes,
},
}
: {};
const unexamined = stack.composition.coverage?.unexaminedObjects ?? 0;

if (drift.length === 0 && pending.length === 0) {
if (flags.json) { await emitJson({ applied: [], skipped: [], created: [], message: 'in_sync' }, 0, { compact: true }); return; }
if (flags.json) {
await emitJson(
{ applied: [], skipped: [], created: [], message: unexamined > 0 ? 'in_sync_partial' : 'in_sync', ...compositionPayload },
0,
{ compact: true },
);
return;
}
if (unexamined > 0) {
const c = stack.composition.coverage!;
printWarning(
`Nothing to apply over the ${c.examinedObjects} object(s) this run examined — but `
+ `${c.unexaminedObjects} of ${c.registeredObjects} declared object(s) were NOT examined (see above). `
+ 'This is a PARTIAL reconcile: it is not evidence that the deployment is in sync.',
);
return;
}
printSuccess('Physical schema is already in sync with metadata — nothing to apply.');
return;
}
Expand Down Expand Up @@ -223,6 +267,7 @@ export default class MigrateApply extends Command {
created,
applied,
skipped,
...compositionPayload,
duration: timer.elapsed(),
});
return;
Expand Down
42 changes: 41 additions & 1 deletion packages/cli/src/commands/migrate/plan.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
summarize,
summarizePendingSchemaWork,
} from '../../utils/schema-migrate.js';
import { exitOneShotCommand } from '../../utils/one-shot-exit.js';
import { probeMigrationTarget } from '../../utils/migrate-occupancy-gate.js';
import { describeOccupancy } from '../../utils/sqlite-occupancy.js';
import {
Expand Down Expand Up @@ -69,7 +70,26 @@ export default class MigratePlan extends Command {
json: Flags.boolean({ description: 'Output as JSON' }),
};

/**
* #13027 — the process must end when the plan does.
*
* The body is {@link plan}; this wrapper exists so every one of its early
* `return`s funnels through one deliberate exit. A composed host stack can
* leave the event loop alive — its `start()` was suppressed, so anything it
* armed during `init()` has no release path — and this command has measurably
* outlived its own "Graceful shutdown complete" by 78 minutes.
*
* ⛔ The FAILURE paths are deliberately NOT routed here: `this.exit(n)` throws
* an `ExitError` that oclif's `handle()` turns into a `process.exit` of its
* own, so they already terminate — and catching them here to exit "tidily"
* would swallow the report with them.
*/
async run(): Promise<void> {
await this.plan();
await exitOneShotCommand(typeof process.exitCode === 'number' ? process.exitCode : 0);
}

private async plan(): Promise<void> {
const { flags } = await this.parse(MigratePlan);
const timer = createTimer();

Expand Down Expand Up @@ -168,6 +188,11 @@ export default class MigratePlan extends Command {
composition: {
hostConfig: stack.composition.hostConfigPath,
hostConfigLoaded: stack.composition.hostConfigLoaded,
// [#13028] The plan's own boundary, so a consumer gate can
// refuse a PARTIAL plan instead of reading `managedTables`
// as coverage. `unexaminedObjects > 0` is the discriminator;
// `reasons` says which kind of partial it is.
...(stack.composition.coverage ? { coverage: stack.composition.coverage } : {}),
notes: stack.composition.notes,
},
}
Expand Down Expand Up @@ -201,7 +226,22 @@ export default class MigratePlan extends Command {
console.log('');

if (drift.length === 0 && pending.length === 0) {
printSuccess('Physical schema is in sync with metadata — nothing to migrate.');
// [#13028] "In sync" is a claim about the objects this plan EXAMINED.
// On a composed host that examined a strict subset — a control plane
// declaring ~80 tables of which 8 reached the diffed driver — printing
// the unqualified sentence tells an operator the deployment is
// migrated when most of it was never looked at. Say which it is.
const partial = (stack.composition.coverage?.unexaminedObjects ?? 0) > 0;
if (partial) {
const c = stack.composition.coverage!;
printWarning(
`No drift over the ${c.examinedObjects} object(s) this plan examined — but `
+ `${c.unexaminedObjects} of ${c.registeredObjects} declared object(s) were NOT examined (see above). `
+ 'This is a PARTIAL plan: it is not evidence that the deployment is in sync.',
);
} else {
printSuccess('Physical schema is in sync with metadata — nothing to migrate.');
}
console.log('');
return;
}
Expand Down
85 changes: 85 additions & 0 deletions packages/cli/src/utils/one-shot-exit.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* #13027 — the exit primitive, pinned where the e2e cannot look.
*
* `test/migrate-plan-exits.e2e.test.ts` owns the end-to-end fact (a real child
* returns). This file owns the two properties that make that safe and which a
* child process cannot show you: the streams are DRAINED before the exit — the
* pipe-truncation `emitJson` exists to prevent, re-introduced one statement
* later would be invisible from outside — and the drain cannot itself hang.
*/

import { describe, it, expect, vi } from 'vitest';
import { exitOneShotCommand } from './one-shot-exit.js';

/** A stream whose no-op write callback fires, in order, when told to. */
function drainableStream() {
const pending: Array<() => void> = [];
return {
writes: 0,
write(_chunk: string, cb?: () => void) {
this.writes++;
if (cb) pending.push(cb);
return true;
},
flush() { for (const cb of pending.splice(0)) cb(); },
};
}

describe('exitOneShotCommand (#13027)', () => {
it('drains every stream before it exits', async () => {
const out = drainableStream();
const err = drainableStream();
const order: string[] = [];
const exit = vi.fn((code: number) => { order.push(`exit:${code}`); return undefined as never; });

const promise = exitOneShotCommand(0, { streams: [out, err], exit });

// Not yet: the streams have been asked to drain and have not answered.
await Promise.resolve();
expect(exit).not.toHaveBeenCalled();
expect(out.writes).toBe(1);
expect(err.writes).toBe(1);

out.flush();
err.flush();
await promise;
expect(order).toEqual(['exit:0']);
});

it('carries the exit code through', async () => {
const exit = vi.fn(() => undefined as never);
await exitOneShotCommand(1, { streams: [], exit });
expect(exit).toHaveBeenCalledWith(1);
});

it('exits anyway when a stream never drains', async () => {
// A pipe whose reader has gone away never drains. This function's whole
// job is to stop a command from failing to return, so it must not become a
// second way to do exactly that.
const stuck = { write: () => true }; // callback never invoked
const exit = vi.fn(() => undefined as never);
const timers: Array<() => void> = [];
const setTimeoutFn = ((fn: () => void) => {
timers.push(fn);
return { unref() { /* noop */ } };
}) as unknown as typeof setTimeout;

const promise = exitOneShotCommand(0, { streams: [stuck], exit, setTimeoutFn });
await Promise.resolve();
expect(exit).not.toHaveBeenCalled();

// The budget expires.
for (const fire of timers) fire();
await promise;
expect(exit).toHaveBeenCalledWith(0);
});

it('tolerates a stream that throws on write, and one that is absent', async () => {
const exit = vi.fn(() => undefined as never);
const throwing = { write() { throw new Error('EPIPE'); } };
await exitOneShotCommand(0, { streams: [throwing, undefined], exit });
expect(exit).toHaveBeenCalledWith(0);
});
});
117 changes: 117 additions & 0 deletions packages/cli/src/utils/one-shot-exit.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* End a one-shot command's PROCESS once its work is done (#13027).
*
* ## The measurement
*
* `os migrate plan`, ObjectStack Cloud's staging control plane, `apply=false`,
* inside `docker run --rm`. The command finished and said so:
*
* ```
* 15:03:52.522 17 change(s): 0 safe, 0 needs-confirm, 17 destructive
* 15:03:52.517 INFO Graceful shutdown started
* 15:03:52.555 INFO OK Graceful shutdown complete
* ```
*
* Elapsed inside the CLI: 4.3 seconds. The next line in the log is the run
* being cancelled by hand **78 minutes later**. The shell's very next statement
* was an `echo` that never printed, so the shell was still blocked on that one
* `docker run`: the process printed its own graceful-shutdown line and then did
* not exit.
*
* ## Why the fix is "exit deliberately" and not "find the handle"
*
* The composition `os migrate plan` performs (#12938) registers a host's
* plugins for their DECLARATIONS: `init()` runs, `start()` is replaced with a
* no-op. A host plugin that acquires something live during Phase 1 — an
* interval, a pool, a watcher, a `kernel:ready` hook that starts a dispatcher —
* and releases it from a path the suppressed `start()` (or a `destroy` the
* host's own wrapper never forwards) would have installed now has no release
* path at all. The event loop stays alive; the kernel has already reported a
* clean shutdown, so nothing looks wrong.
*
* Chasing the handle means auditing host code this repo cannot see — the same
* argument that made the composition declaration-only in the first place. A
* one-shot command that has written its document owes the operator an exit, and
* that is true whatever the host left running. ⛔ This is NOT a licence to skip
* teardown: the caller still runs `stack.shutdown()` first, and this is the
* last statement after it.
*
* ## Why it cannot simply be `process.exit(code)`
*
* `process.exit` tears the process down with an unflushed stdout **pipe**
* buffer — the exact truncation `emitJson` exists to prevent, re-introduced one
* statement later. `emitJson`/`emitText` already await their own write, but the
* human-readable path is `console.log`, which does not. So this drains both
* streams first, and refuses to hang while doing it: a stream that will not
* drain must not become a second way for this command to never return.
*/

/** Injection seam — production values, replaced wholesale in unit tests. */
export interface OneShotExitDeps {
/** Streams to drain before exiting. */
streams?: Array<{ write(chunk: string, cb?: () => void): unknown } | undefined>;
/** The exit call itself. */
exit?: (code: number) => never;
/** Upper bound on waiting for a drain, in ms. */
drainTimeoutMs?: number;
/** Timer factory, so a test does not have to wait in real time. */
setTimeoutFn?: typeof setTimeout;
}

/**
* Wait for everything already queued on `stream` to reach the OS.
*
* A no-op write's callback fires once every write queued **before** it has been
* flushed, which is the documented way to ask this question. The timeout is not
* belt-and-braces: a pipe whose reader has gone away never drains, and this
* function's whole job is to stop a command from hanging.
*/
function drain(
stream: { write(chunk: string, cb?: () => void): unknown } | undefined,
timeoutMs: number,
setTimeoutFn: typeof setTimeout,
): Promise<void> {
return new Promise<void>((resolve) => {
if (!stream || typeof stream.write !== 'function') {
resolve();
return;
}
let settled = false;
const done = (): void => {
if (settled) return;
settled = true;
resolve();
};
const timer = setTimeoutFn(done, timeoutMs);
(timer as { unref?: () => void })?.unref?.();
try {
stream.write('', done);
} catch {
done();
}
});
}

/**
* Flush stdout/stderr, then end the process with `code`.
*
* Returns `Promise<never>` in production. In a test the injected `exit` may
* return normally, and then so does this — that is deliberate, so a unit test
* can assert the code without killing its own runner.
*/
export async function exitOneShotCommand(
code = 0,
deps: OneShotExitDeps = {},
): Promise<never> {
const {
streams = [process.stdout, process.stderr],
exit = process.exit.bind(process) as (c: number) => never,
drainTimeoutMs = 2_000,
setTimeoutFn = setTimeout,
} = deps;

await Promise.all(streams.map((s) => drain(s, drainTimeoutMs, setTimeoutFn)));
return exit(code);
}
Loading
Loading