From 85ed61045484e3461e77f30de5aa59046c7f0430 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 29 Aug 2026 04:46:27 +0000 Subject: [PATCH] fix(cli): refuse a migrate run whose host config exists but could not be loaded MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `os migrate plan` and `os migrate apply` exited 0 when a host `objectstack.config.{ts,js,mjs}` was present and threw while loading -- a missing environment variable being the ordinary cause. The metadata set they then diffed was the data stack plus the platform floor: nine tables, none of them the deployment's, and zero drift over them printed "Physical schema is in sync with metadata". Maintainer ruling 2026-08-29, verbatim 「同意」: a green exit over an UNMEASURED partial metadata set is the false-green a migration tool must never emit. Both commands now exit non-zero on that path, with an error on stderr naming the config file, the underlying failure and the remedy. Scope is exactly that one shape. A config that is ABSENT, and a config that LOADS, keep today's behaviour -- both measured byte-identical, stdout and stderr, human and --json, for both commands. The refusal keys on `hostConfigPath !== null && !hostConfigLoaded`, not on the flag alone, because `hostConfigLoaded` is false on the config-absent shape too. Everything the previous behaviour emitted is kept: the loud stderr warning and the `composition.hostConfigLoaded` discriminator consumer coverage gates read. The refusal changes the exit STATUS, not the document -- the whole report is written first, and the unloadable path's JSON payload is byte-identical to the one it emitted before. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UjujZN219uFzBhSYfMykCd --- .../migrate-refuses-unloadable-host-config.md | 44 ++++ packages/cli/src/commands/migrate/apply.ts | 17 ++ packages/cli/src/commands/migrate/plan.ts | 22 ++ packages/cli/src/utils/schema-migrate.ts | 5 +- .../utils/schema-migration-plugins.test.ts | 97 ++++++- .../cli/src/utils/schema-migration-plugins.ts | 108 +++++++- ...te-unloadable-host-config-exit.e2e.test.ts | 241 ++++++++++++++++++ 7 files changed, 525 insertions(+), 9 deletions(-) create mode 100644 .changeset/migrate-refuses-unloadable-host-config.md create mode 100644 packages/cli/test/migrate-unloadable-host-config-exit.e2e.test.ts diff --git a/.changeset/migrate-refuses-unloadable-host-config.md b/.changeset/migrate-refuses-unloadable-host-config.md new file mode 100644 index 0000000000..032c20f1d3 --- /dev/null +++ b/.changeset/migrate-refuses-unloadable-host-config.md @@ -0,0 +1,44 @@ +--- +"@objectstack/cli": minor +--- + +fix(cli): `os migrate plan` / `apply` exit non-zero when the host config exists but could not be loaded (#12953) + +A host `objectstack.config.{ts,js,mjs}` that EXISTS and throws while loading — a +missing environment variable is the ordinary cause, and ObjectStack Cloud's own +control-plane config throws without `AUTH_SECRET` — used to warn loudly and then +**exit 0**. The object set the commands diffed on that path is the data stack +plus the platform floor: nine tables, none of them the deployment's, and `0` +drift over them printed "Physical schema is in sync with metadata — nothing to +migrate". Measured on the fixture this ships with, before the change: `plan`, +`plan --json`, `apply --yes` and `apply --yes --json` all returned `0`. + +Maintainer ruling 2026-08-29, verbatim 「同意」: a green exit over an UNMEASURED +partial metadata set is the false-green a migration tool must never emit, and +the population this "regresses" was computing defective plans all along. Both +commands now exit **non-zero** on that path, with an error on stderr naming the +config file, the underlying failure, and the remedy. + +**BEHAVIOUR CHANGE to exit status**, shipped as `minor` under the repo's +launch-window convention. It is scoped to exactly one shape, and the two +neighbouring ones were measured byte-identical before and after — stdout *and* +stderr, human and `--json`, for both commands: + +- host config **present and unloadable** → non-zero (this change); +- host config **absent** → unchanged, still exit 0. `hostConfigLoaded` is + `false` on that shape too, so the refusal keys on `hostConfigPath !== null` + rather than on the flag alone; +- host config **present and loadable** → unchanged, still exit 0. + +Everything the previous behaviour emitted is kept, deliberately: the loud stderr +warning, and the `composition.hostConfigLoaded` discriminator in the `--json` +payload that consumer coverage gates (objectstack-ai/cloud#1705) read — a table +count cannot replace it, because the platform floor raises the count either way. +The refusal changes the exit STATUS, not the document: the whole plan, or the +whole JSON payload, is still written before the process exits non-zero, and the +unloadable path's payload is byte-identical to the one it emitted before. + +**Migration.** A CI step that runs `os migrate plan`/`apply` against a project +whose config needs environment it was not given now fails instead of reporting +success over a fraction of the deployment. Supply that environment to the run +(the error names the missing variable), or fix the config. diff --git a/packages/cli/src/commands/migrate/apply.ts b/packages/cli/src/commands/migrate/apply.ts index 3974e00d57..ed8eeb8b15 100644 --- a/packages/cli/src/commands/migrate/apply.ts +++ b/packages/cli/src/commands/migrate/apply.ts @@ -22,6 +22,10 @@ import { groupByCategory, } from '../../utils/schema-migrate.js'; import { exitOneShotCommand } from '../../utils/one-shot-exit.js'; +import { + refuseWhenHostConfigUnloadable, + type SchemaMigrationComposition, +} from '../../utils/schema-migration-plugins.js'; import { OCCUPANCY_HINT, probeMigrationTarget } from '../../utils/migrate-occupancy-gate.js'; import { describeOccupancy } from '../../utils/sqlite-occupancy.js'; @@ -90,9 +94,21 @@ export default class MigrateApply extends Command { */ async run(): Promise { await this.apply(); + // [#12953] Same refusal as `migrate plan`, through the same choke point — + // the ruling (2026-08-29, verbatim 「同意」) named BOTH commands, and the + // reconcile an operator confirms has to be judged the same way as the plan + // they read. Applied after `apply()` for the same reason it is there: the + // report is already written and must survive the non-zero exit. + if (this.composition) refuseWhenHostConfigUnloadable(this.composition); await exitOneShotCommand(typeof process.exitCode === 'number' ? process.exitCode : 0); } + /** + * What {@link apply} composed, read by {@link run} after it returns (#12953). + * `null` until the stack has booted, and on every path where it never did. + */ + private composition: SchemaMigrationComposition | null = null; + private async apply(): Promise { const { flags } = await this.parse(MigrateApply); const timer = createTimer(); @@ -154,6 +170,7 @@ export default class MigrateApply extends Command { this.exit(1); return; } + this.composition = stack.composition; try { if (!stack.driver) { diff --git a/packages/cli/src/commands/migrate/plan.ts b/packages/cli/src/commands/migrate/plan.ts index ae9e5e25c1..2c19a88b87 100644 --- a/packages/cli/src/commands/migrate/plan.ts +++ b/packages/cli/src/commands/migrate/plan.ts @@ -20,6 +20,10 @@ import { summarizePendingSchemaWork, } from '../../utils/schema-migrate.js'; import { exitOneShotCommand } from '../../utils/one-shot-exit.js'; +import { + refuseWhenHostConfigUnloadable, + type SchemaMigrationComposition, +} from '../../utils/schema-migration-plugins.js'; import { probeMigrationTarget } from '../../utils/migrate-occupancy-gate.js'; import { describeOccupancy } from '../../utils/sqlite-occupancy.js'; import { @@ -86,9 +90,26 @@ export default class MigratePlan extends Command { */ async run(): Promise { await this.plan(); + // [#12953] A host config that EXISTS but could not be loaded means the plan + // above covered a fraction of this deployment — UNMEASURED, not "in sync" — + // and the maintainer ruled that green exit out (2026-08-29, verbatim + // 「同意」). Applied HERE, after `plan()`, deliberately: every one of its + // early returns (no SQL driver, in sync, the rendered plan) has already + // written its report by now, and the report — the human plan, or the JSON + // document whose `composition.hostConfigLoaded` the ruling kept as the + // consumer's discriminator — must survive the refusal, not be replaced by + // it. `this.composition` is `null` on the boot-failure path, which already + // exits non-zero through oclif. + if (this.composition) refuseWhenHostConfigUnloadable(this.composition); await exitOneShotCommand(typeof process.exitCode === 'number' ? process.exitCode : 0); } + /** + * What {@link plan} composed, read by {@link run} after it returns (#12953). + * `null` until the stack has booted, and on every path where it never did. + */ + private composition: SchemaMigrationComposition | null = null; + private async plan(): Promise { const { flags } = await this.parse(MigratePlan); const timer = createTimer(); @@ -132,6 +153,7 @@ export default class MigratePlan extends Command { this.exit(1); return; } + this.composition = stack.composition; try { if (!stack.driver) { diff --git a/packages/cli/src/utils/schema-migrate.ts b/packages/cli/src/utils/schema-migrate.ts index 82c0fc22e3..83af49adf1 100644 --- a/packages/cli/src/utils/schema-migrate.ts +++ b/packages/cli/src/utils/schema-migrate.ts @@ -338,7 +338,10 @@ export async function bootSchemaStack( cwd: opts.projectRoot ?? process.cwd(), skipSeedData: defer, }) - : { plugins: [], hostConfigPath: null, hostConfigLoaded: false, notes: [], coverage: null } satisfies SchemaMigrationComposition; + : { + plugins: [], hostConfigPath: null, hostConfigLoaded: false, hostConfigError: null, + notes: [], coverage: null, + } satisfies SchemaMigrationComposition; for (const plugin of composition.plugins) { await kernel.use(plugin as any); } diff --git a/packages/cli/src/utils/schema-migration-plugins.test.ts b/packages/cli/src/utils/schema-migration-plugins.test.ts index ce3de458e9..0760a34aef 100644 --- a/packages/cli/src/utils/schema-migration-plugins.test.ts +++ b/packages/cli/src/utils/schema-migration-plugins.test.ts @@ -9,6 +9,8 @@ import { composeForDeclarations, buildSchemaMigrationPlugins, measureComposedCoverage, + describeUnloadableHostConfig, + type SchemaMigrationComposition, } from './schema-migration-plugins.js'; /** @@ -185,15 +187,104 @@ describe('buildSchemaMigrationPlugins', () => { const out = await buildSchemaMigrationPlugins({ basePlugins: [], cwd: dir }); - // Not fatal — this command worked before without ever reading the config, - // and a plan that stops working is a worse regression than a reduced one. + // The composition still COMPLETES — the reduced set is composed and + // returned. What changed with #12953 is the verdict the COMMANDS draw from + // it (a non-zero exit), not whether this function throws. expect(out.hostConfigPath).toBe(join(dir, 'objectstack.config.ts')); - // …but it must be DISTINGUISHABLE. `managedTables` alone cannot say this: + // …and it must be DISTINGUISHABLE. `managedTables` alone cannot say this: // the platform floor still lands, so the count rises either way. expect(out.hostConfigLoaded).toBe(false); const said = out.notes.join(' '); expect(said).toContain('could not be loaded'); expect(said).toContain('UNMEASURED'); + // [#12953] The underlying failure, carried structurally so the refusal can + // NAME it rather than re-parsing the prose above. + expect(out.hostConfigError).toContain('OS_SOME_SECRET is required'); + }); + + it('leaves hostConfigError null when there is no host config at all', async () => { + const none = await buildSchemaMigrationPlugins({ basePlugins: [], cwd: tempProject() }); + expect(none.hostConfigError).toBeNull(); + }); + + it('leaves hostConfigError null when the host config LOADS', async () => { + const dir = tempProject(); + writeFileSync(join(dir, 'objectstack.config.ts'), 'export default { objects: [] };\n'); + const loaded = await buildSchemaMigrationPlugins({ basePlugins: [], cwd: dir }); + expect(loaded.hostConfigLoaded).toBe(true); + expect(loaded.hostConfigError).toBeNull(); + // Loading a real config runs `bundle-require`/esbuild — well past the 5 s + // default on a cold, shared box. + }, 60_000); +}); + +/** + * #12953 — the predicate behind the non-zero exit, in all three directions. + * + * Maintainer ruling 2026-08-29 (verbatim 「同意」): a host config that EXISTS + * and could not be loaded makes `os migrate plan` / `apply` exit non-zero, + * because a green exit over an UNMEASURED partial metadata set is the + * false-green a migration tool must never emit. The ruling pinned the OTHER + * two directions just as hard — config absent, and config loadable, both keep + * today's behaviour — so all three are pinned here. + * + * ⚠️ The trap this file exists to hold: `hostConfigLoaded` is `false` on the + * config-ABSENT shape too (nothing loaded, because there was nothing to load). + * A predicate written as `!hostConfigLoaded` therefore turns the untouched + * population red, and every assertion about direction 1 still passes while it + * does. The second case below is the one that fails if anyone writes it that + * way. + * + * The exit STATUS itself is pinned over a real child process in + * `packages/cli/test/migrate-unloadable-host-config-exit.e2e.test.ts` — a + * `process.exitCode` set inside a vitest worker is not an exit status. + */ +describe('describeUnloadableHostConfig (#12953)', () => { + function composition(over: Partial): SchemaMigrationComposition { + return { + plugins: [], hostConfigPath: null, hostConfigLoaded: false, hostConfigError: null, + notes: [], coverage: null, ...over, + }; + } + + it('direction 1 — config PRESENT and unloadable: names the config, the cause and the remedy', () => { + const said = describeUnloadableHostConfig(composition({ + hostConfigPath: '/srv/app/objectstack.config.ts', + hostConfigLoaded: false, + hostConfigError: 'Missing required environment variable AUTH_SECRET', + })); + + expect(said).not.toBeNull(); + // The three things the ruling requires the error to name. + expect(said).toContain('/srv/app/objectstack.config.ts'); + expect(said).toContain('Missing required environment variable AUTH_SECRET'); + expect(said).toMatch(/Remedy:/); + // And that it is a FAILURE, not another warning — the whole point. + expect(said).toContain('UNMEASURED'); + }); + + it('direction 2 — config ABSENT: null, even though hostConfigLoaded is false', () => { + // `hostConfigPath === null` with `hostConfigLoaded === false` is the + // untouched population. If this ever answers non-null, every project with + // no config starts failing `os migrate plan`. + expect(describeUnloadableHostConfig(composition({ + hostConfigPath: null, hostConfigLoaded: false, + }))).toBeNull(); + }); + + it('direction 3 — config PRESENT and loadable: null', () => { + expect(describeUnloadableHostConfig(composition({ + hostConfigPath: '/srv/app/objectstack.config.ts', hostConfigLoaded: true, + }))).toBeNull(); + }); + + it('still names something when the load threw without a message', () => { + const said = describeUnloadableHostConfig(composition({ + hostConfigPath: '/srv/app/objectstack.config.mjs', hostConfigLoaded: false, + hostConfigError: null, + })); + expect(said).toContain('/srv/app/objectstack.config.mjs'); + expect(said).toContain('the load threw without a message'); }); }); diff --git a/packages/cli/src/utils/schema-migration-plugins.ts b/packages/cli/src/utils/schema-migration-plugins.ts index 08978511f8..02c2d131cc 100644 --- a/packages/cli/src/utils/schema-migration-plugins.ts +++ b/packages/cli/src/utils/schema-migration-plugins.ts @@ -224,6 +224,13 @@ export interface SchemaMigrationComposition { * plan covers the deployment. */ hostConfigLoaded: boolean; + /** + * The underlying failure, when `hostConfigPath !== null && !hostConfigLoaded` + * — the message the load threw, carried structurally so the commands can NAME + * it in their refusal (#12953) instead of re-parsing the prose in `notes`. + * `null` on every other shape, including a config that loaded. + */ + hostConfigError: string | null; /** * 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 @@ -243,6 +250,7 @@ const NOTHING_COMPOSED: SchemaMigrationComposition = Object.freeze({ plugins: [], hostConfigPath: null, hostConfigLoaded: false, + hostConfigError: null, notes: [], coverage: null, }) as SchemaMigrationComposition; @@ -276,6 +284,7 @@ export async function buildSchemaMigrationPlugins(opts: { const plugins: unknown[] = []; const notes: string[] = []; let hostConfigLoaded = false; + let hostConfigError: string | null = null; if (hostConfigPath) { try { @@ -311,10 +320,21 @@ export async function buildSchemaMigrationPlugins(opts: { ); } catch (error: any) { // Loud, and on stderr in both modes (`--json` reserves stdout, and the - // reservation is already installed by the time this runs). NOT fatal: a - // config that cannot load — a missing env var is the common one — used to - // be irrelevant to this command, and turning that into a failed `plan` - // would break a command that works today. Say what was lost instead. + // reservation is already installed by the time this runs). + // + // ⚠️ This warning is no longer the WHOLE of the response. It used to be — + // the reasoning was that a config which cannot load (a missing env var is + // the common one) used to be irrelevant to this command, so failing the + // run would regress a command that works today. Maintainer ruling + // 2026-08-29 (#12953), verbatim 「同意」, overrode that: a green exit over + // an UNMEASURED partial metadata set is the false-green a migration tool + // must never emit, and the population it "regresses" was computing + // defective plans all along. The commands now exit non-zero on this path + // — see {@link describeUnloadableHostConfig}. + // + // The warning text and `hostConfigLoaded` stay exactly as they were: the + // ruling pinned both, because consumers (objectstack-ai/cloud#1705) read + // the discriminator and a count alone cannot replace it. const message = error?.message ?? String(error); const line = `Host config ${hostConfigPath} could not be loaded: ${message}. ` @@ -324,6 +344,7 @@ export async function buildSchemaMigrationPlugins(opts: { // eslint-disable-next-line no-console console.warn(`[migrate] ⚠ ${line}`); notes.push(line); + hostConfigError = message; } } @@ -338,7 +359,84 @@ export async function buildSchemaMigrationPlugins(opts: { notes.push('Composed PlatformObjectsPlugin (the platform floor `os serve` composes unconditionally).'); } - return { plugins, hostConfigPath, hostConfigLoaded, notes, coverage: null }; + return { plugins, hostConfigPath, hostConfigLoaded, hostConfigError, notes, coverage: null }; +} + +/** + * The one composition shape `os migrate plan` / `apply` must REFUSE (#12953), + * rendered as the operator-facing error — or `null` when this run is not it. + * + * ## Which shape, and why only this one + * + * A host config that EXISTS and could not be loaded. The object set the command + * then diffs is the data stack plus the platform floor: nine tables, none of + * them this deployment's, and `0` drift over them prints as "in sync". The + * maintainer ruled that green exit out on 2026-08-29 (verbatim 「同意」) — + * a green exit over an UNMEASURED partial metadata set is the false-green a + * migration tool must never emit, and the population this "regresses" was + * computing defective plans all along. + * + * The scope was pinned in the same ruling, in three directions, and the + * predicate below is written to hold all three: + * + * • config present + unloadable → non-zero (this function answers non-`null`); + * • config ABSENT (`hostConfigPath === null`) → today's behaviour, unchanged. + * `hostConfigLoaded` is `false` on that path too — which is exactly why the + * test is on `hostConfigPath`, not on `hostConfigLoaded` alone; + * • config present + loadable → today's behaviour, unchanged. + * + * ⛔ Not a general "the composition is partial" refusal. A plan that composed + * fine but could not EXAMINE everything it declared is #13028's `coverage` + * block, it stays exit 0, and widening this predicate to cover it would turn a + * population the ruling deliberately left alone red. + * + * The message names the three things the ruling requires of it: the config + * file, the underlying failure, and the remedy. + */ +export function describeUnloadableHostConfig( + composition: SchemaMigrationComposition, +): string | null { + if (composition.hostConfigPath === null || composition.hostConfigLoaded) return null; + const cause = composition.hostConfigError ?? 'the load threw without a message'; + return ( + `Host config ${composition.hostConfigPath} exists but could not be loaded: ${cause}. ` + + 'This run therefore covered ONLY the objects the data stack registered — a fraction of ' + + 'what this deployment serves — so its result is UNMEASURED, not "in sync", and it is ' + + 'reported as a FAILURE rather than as success. ' + + 'Remedy: supply the environment this config needs (the failure named above says which), ' + + 'or fix the config, then re-run.' + ); +} + +/** + * Apply {@link describeUnloadableHostConfig}'s verdict to the process. + * + * A shared choke point rather than two copies, for the reason `apply` already + * states about `composeHostStack`: the plan an operator reads and the reconcile + * they confirm must be the same judgement, so the two commands cannot be + * allowed to drift on it. + * + * ⚠️ Writes to **stderr**, in both modes. `printError` writes to stdout, and + * `--json` reserves stdout for the payload — the payload is still emitted in + * full on this path, because the ruling kept `composition.hostConfigLoaded` as + * the machine discriminator its consumers read. + * + * ⚠️ Sets `process.exitCode` rather than throwing oclif's `this.exit(1)`: the + * report — the plan, or the JSON document — has already been written by the + * time this runs and must survive. `migrate/plan.ts`'s `run()` wrapper reads + * `process.exitCode` and hands it to `exitOneShotCommand`. + * + * @returns `true` when this run was the refused shape. + */ +export function refuseWhenHostConfigUnloadable( + composition: SchemaMigrationComposition, +): boolean { + const line = describeUnloadableHostConfig(composition); + if (line === null) return false; + // eslint-disable-next-line no-console + console.error(`[migrate] ✗ ${line}`); + process.exitCode = 1; + return true; } /** 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 new file mode 100644 index 0000000000..dbe35291d7 --- /dev/null +++ b/packages/cli/test/migrate-unloadable-host-config-exit.e2e.test.ts @@ -0,0 +1,241 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #12953 — `os migrate plan` / `os migrate apply` EXIT NON-ZERO when a host + * `objectstack.config.{ts,js,mjs}` exists but could not be loaded. + * + * ## The measurement this inverts + * + * After #12938 the pair composes the deployment's own object set. One shape + * still slipped through: a config that EXISTS and throws while loading — a + * missing environment variable is the ordinary cause, and ObjectStack Cloud's + * own control-plane config throws without `AUTH_SECRET`. On that path the + * commands warned loudly, carried `composition.hostConfigLoaded: false` in + * `--json`, and **exited 0** over a metadata set that was the data stack plus + * the platform floor — nine tables, none of them the deployment's. Measured on + * this fixture before the fix: `plan`, `plan --json`, `apply --yes` and + * `apply --yes --json` all returned 0. + * + * Maintainer ruling 2026-08-29, verbatim 「同意」: a green exit over an + * UNMEASURED partial metadata set is the false-green a migration tool must + * never emit, and the population this "regresses" was computing defective + * plans all along. + * + * ## Why a real child process + * + * The claim is about EXIT STATUS, and `process.exitCode` set inside a vitest + * worker is not one — only a process that has actually exited has a status for + * a shell, a `set -e` script or a container entrypoint to read. That audience + * is the entire point of the ruling, and it is the one an in-process assertion + * cannot stand in for. Spawned through `bin/run-dev.js` + tsx, the pattern + * `migrate-exit-code.e2e.test.ts` and `migrate-plan-exits.e2e.test.ts` already + * use, so the suite does not depend on `packages/cli/dist` having been built. + * + * ## Why all THREE directions are here + * + * The ruling pinned the untouched populations as hard as the changed one: + * config ABSENT keeps today's behaviour, config PRESENT-AND-LOADABLE keeps + * today's behaviour. Those two are the expensive half — a refusal written as + * "the composition is incomplete" rather than "the config is present and did + * not load" turns every config-less project red, and direction 1's assertions + * all still pass while it does. `absentPlan`/`loadablePlan` below are what + * fails instead. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { execFile } from 'node:child_process'; +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { CLI, TSX, childEnv } from './helpers/serve-process.js'; + +/** + * The environment variable the unloadable fixture demands. + * + * Deliberately namespaced to this test: the fixture must fail for a reason the + * runner cannot accidentally satisfy, and it is explicitly unset in the child + * (see {@link runCli}) so an inherited value can never turn direction 1 green. + */ +const REQUIRED_VAR = 'OS_E2E_12953_SECRET'; + +/** Generous: a cold tsx compile of the command tree dominates a ~1 s plan. */ +const RUN_BUDGET_MS = 120_000; + +interface Run { + code: number; + stdout: string; + stderr: string; +} + +function runCli(args: string[], cwd: string): Promise { + return new Promise((resolvePromise) => { + execFile( + TSX, + [CLI, ...args], + { + cwd, + maxBuffer: 16 * 1024 * 1024, + // `childEnv` drops the vitest worker family; `REQUIRED_VAR: undefined` + // removes the variable the fixture needs, which is what makes the + // config unloadable rather than merely unusual. + env: childEnv({ NO_COLOR: '1', [REQUIRED_VAR]: undefined }), + }, + (err, stdout, stderr) => { + resolvePromise({ + // `err.code` is the real exit status. `null`/undefined means the + // child was SIGNALLED — a different failure, never reported as 0. + code: err + ? (typeof (err as { code?: unknown }).code === 'number' + ? (err as unknown as { code: number }).code + : 1) + : 0, + stdout: String(stdout), + stderr: String(stderr), + }); + }, + ); + }); +} + +/** A config that is PRESENT and throws while loading — direction 1. */ +const UNLOADABLE_CONFIG = [ + `const secret = process.env.${REQUIRED_VAR};`, + 'if (!secret) {', + ` throw new Error('Missing required environment variable ${REQUIRED_VAR}');`, + '}', + '', + 'export default { name: \'unloadable_e2e\', label: \'Unloadable E2E\', objects: [] };', + '', +].join('\n'); + +/** A config that is PRESENT and loads — direction 3. */ +const LOADABLE_CONFIG = [ + 'export default {', + " name: 'loadable_e2e',", + " label: 'Loadable E2E',", + ' objects: [{', + " name: 'le_ticket',", + " label: 'Ticket',", + " fields: { title: { type: 'text', label: 'Title' } },", + ' }],', + '};', + '', +].join('\n'); + +const dirs: string[] = []; +function project(config: string | null): string { + const dir = mkdtempSync(join(tmpdir(), 'os-12953-e2e-')); + dirs.push(dir); + if (config !== null) writeFileSync(join(dir, 'objectstack.config.ts'), config); + return dir; +} + +describe('os migrate plan/apply refuse an unloadable host config (#12953)', () => { + let unloadablePlan: Run; + let unloadableApply: Run; + let absentPlan: Run; + let absentApply: Run; + let loadablePlan: Run; + let loadableApply: Run; + + beforeAll(async () => { + const unloadable = project(UNLOADABLE_CONFIG); + const absent = project(null); + const loadable = project(LOADABLE_CONFIG); + + // Sequential on purpose: each run boots a kernel, and this suite shares a + // box with whatever else CI is running. + unloadablePlan = await runCli(['migrate', 'plan', '--json'], unloadable); + unloadableApply = await runCli(['migrate', 'apply', '--yes', '--json'], unloadable); + absentPlan = await runCli(['migrate', 'plan', '--json'], absent); + absentApply = await runCli(['migrate', 'apply', '--yes', '--json'], absent); + loadablePlan = await runCli(['migrate', 'plan', '--json'], loadable); + loadableApply = await runCli(['migrate', 'apply', '--yes', '--json'], loadable); + }, RUN_BUDGET_MS * 6); + + afterAll(() => { + while (dirs.length > 0) { + const dir = dirs.pop()!; + try { rmSync(dir, { recursive: true, force: true }); } catch { /* best-effort */ } + } + }); + + describe('direction 1 — the config is PRESENT and cannot be loaded', () => { + it('exits non-zero from `migrate plan`', () => { + expect(unloadablePlan.code).not.toBe(0); + }); + + it('exits non-zero from `migrate apply` — the ruling named both commands', () => { + expect(unloadableApply.code).not.toBe(0); + }); + + it('names the config, the underlying failure and the remedy, on stderr', () => { + for (const run of [unloadablePlan, unloadableApply]) { + expect(run.stderr).toContain('objectstack.config.ts'); + expect(run.stderr).toContain(`Missing required environment variable ${REQUIRED_VAR}`); + expect(run.stderr).toMatch(/Remedy:/); + } + }); + + it('KEEPS the loud warning and the hostConfigLoaded discriminator', () => { + // Both were pinned by the ruling: consumers (objectstack-ai/cloud#1705) + // read `hostConfigLoaded`, and a table count cannot replace it — the + // platform floor lands either way, so the count rises either way. + // + // ⚠️ This assertion is only worth its bytes because it can FAIL: the + // same probe run against direction 2 finds no `composition` block at all + // (`absentPlan` below asserts exactly that), so a change that dropped the + // field would turn this red rather than pass on absence. + for (const run of [unloadablePlan, unloadableApply]) { + expect(run.stderr).toContain('could not be loaded'); + expect(run.stderr).toContain('UNMEASURED'); + const payload = JSON.parse(run.stdout) as { + composition?: { hostConfig?: string; hostConfigLoaded?: boolean }; + }; + expect(payload.composition?.hostConfigLoaded).toBe(false); + expect(payload.composition?.hostConfig).toContain('objectstack.config.ts'); + } + }); + + it('still emits its whole report — the refusal replaces the STATUS, not the document', () => { + // A refusal that swallowed the plan would take the consumer's signal with + // it, which is the opposite of what the ruling preserved. + expect(() => JSON.parse(unloadablePlan.stdout)).not.toThrow(); + expect(() => JSON.parse(unloadableApply.stdout)).not.toThrow(); + }); + }); + + describe('direction 2 — there is NO host config: unchanged', () => { + it('keeps exit 0 for plan and apply', () => { + expect(absentPlan.code).toBe(0); + expect(absentApply.code).toBe(0); + }); + + it('emits no composition block and no refusal', () => { + // `hostConfigLoaded` is false on this shape too (nothing was loaded + // because there was nothing to load). A refusal keyed on that flag alone + // fails HERE and nowhere else. + const payload = JSON.parse(absentPlan.stdout) as Record; + expect(payload).not.toHaveProperty('composition'); + expect(absentPlan.stderr).not.toContain('could not be loaded'); + expect(absentPlan.stderr).not.toMatch(/Remedy:/); + expect(absentApply.stderr).not.toMatch(/Remedy:/); + }); + }); + + describe('direction 3 — the host config LOADS: unchanged', () => { + it('keeps exit 0 for plan and apply', () => { + expect(loadablePlan.code).toBe(0); + expect(loadableApply.code).toBe(0); + }); + + it('reports the config as loaded and emits no refusal', () => { + const payload = JSON.parse(loadablePlan.stdout) as { + composition?: { hostConfigLoaded?: boolean }; + }; + expect(payload.composition?.hostConfigLoaded).toBe(true); + expect(loadablePlan.stderr).not.toMatch(/Remedy:/); + expect(loadableApply.stderr).not.toMatch(/Remedy:/); + }); + }); +});