Skip to content

Commit f2dc3ac

Browse files
os-litantclaude
andauthored
fix(cli): refuse a migrate run whose host config exists but could not be loaded (#13113)
`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. Claude-Session: https://claude.ai/code/session_01UjujZN219uFzBhSYfMykCd Co-authored-by: Claude <noreply@anthropic.com>
1 parent ca1965f commit f2dc3ac

7 files changed

Lines changed: 525 additions & 9 deletions

File tree

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
---
2+
"@objectstack/cli": minor
3+
---
4+
5+
fix(cli): `os migrate plan` / `apply` exit non-zero when the host config exists but could not be loaded (#12953)
6+
7+
A host `objectstack.config.{ts,js,mjs}` that EXISTS and throws while loading — a
8+
missing environment variable is the ordinary cause, and ObjectStack Cloud's own
9+
control-plane config throws without `AUTH_SECRET` — used to warn loudly and then
10+
**exit 0**. The object set the commands diffed on that path is the data stack
11+
plus the platform floor: nine tables, none of them the deployment's, and `0`
12+
drift over them printed "Physical schema is in sync with metadata — nothing to
13+
migrate". Measured on the fixture this ships with, before the change: `plan`,
14+
`plan --json`, `apply --yes` and `apply --yes --json` all returned `0`.
15+
16+
Maintainer ruling 2026-08-29, verbatim 「同意」: a green exit over an UNMEASURED
17+
partial metadata set is the false-green a migration tool must never emit, and
18+
the population this "regresses" was computing defective plans all along. Both
19+
commands now exit **non-zero** on that path, with an error on stderr naming the
20+
config file, the underlying failure, and the remedy.
21+
22+
**BEHAVIOUR CHANGE to exit status**, shipped as `minor` under the repo's
23+
launch-window convention. It is scoped to exactly one shape, and the two
24+
neighbouring ones were measured byte-identical before and after — stdout *and*
25+
stderr, human and `--json`, for both commands:
26+
27+
- host config **present and unloadable** → non-zero (this change);
28+
- host config **absent** → unchanged, still exit 0. `hostConfigLoaded` is
29+
`false` on that shape too, so the refusal keys on `hostConfigPath !== null`
30+
rather than on the flag alone;
31+
- host config **present and loadable** → unchanged, still exit 0.
32+
33+
Everything the previous behaviour emitted is kept, deliberately: the loud stderr
34+
warning, and the `composition.hostConfigLoaded` discriminator in the `--json`
35+
payload that consumer coverage gates (objectstack-ai/cloud#1705) read — a table
36+
count cannot replace it, because the platform floor raises the count either way.
37+
The refusal changes the exit STATUS, not the document: the whole plan, or the
38+
whole JSON payload, is still written before the process exits non-zero, and the
39+
unloadable path's payload is byte-identical to the one it emitted before.
40+
41+
**Migration.** A CI step that runs `os migrate plan`/`apply` against a project
42+
whose config needs environment it was not given now fails instead of reporting
43+
success over a fraction of the deployment. Supply that environment to the run
44+
(the error names the missing variable), or fix the config.

packages/cli/src/commands/migrate/apply.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,10 @@ import {
2222
groupByCategory,
2323
} from '../../utils/schema-migrate.js';
2424
import { exitOneShotCommand } from '../../utils/one-shot-exit.js';
25+
import {
26+
refuseWhenHostConfigUnloadable,
27+
type SchemaMigrationComposition,
28+
} from '../../utils/schema-migration-plugins.js';
2529
import { OCCUPANCY_HINT, probeMigrationTarget } from '../../utils/migrate-occupancy-gate.js';
2630
import { describeOccupancy } from '../../utils/sqlite-occupancy.js';
2731

@@ -90,9 +94,21 @@ export default class MigrateApply extends Command {
9094
*/
9195
async run(): Promise<void> {
9296
await this.apply();
97+
// [#12953] Same refusal as `migrate plan`, through the same choke point —
98+
// the ruling (2026-08-29, verbatim 「同意」) named BOTH commands, and the
99+
// reconcile an operator confirms has to be judged the same way as the plan
100+
// they read. Applied after `apply()` for the same reason it is there: the
101+
// report is already written and must survive the non-zero exit.
102+
if (this.composition) refuseWhenHostConfigUnloadable(this.composition);
93103
await exitOneShotCommand(typeof process.exitCode === 'number' ? process.exitCode : 0);
94104
}
95105

106+
/**
107+
* What {@link apply} composed, read by {@link run} after it returns (#12953).
108+
* `null` until the stack has booted, and on every path where it never did.
109+
*/
110+
private composition: SchemaMigrationComposition | null = null;
111+
96112
private async apply(): Promise<void> {
97113
const { flags } = await this.parse(MigrateApply);
98114
const timer = createTimer();
@@ -154,6 +170,7 @@ export default class MigrateApply extends Command {
154170
this.exit(1);
155171
return;
156172
}
173+
this.composition = stack.composition;
157174

158175
try {
159176
if (!stack.driver) {

packages/cli/src/commands/migrate/plan.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,10 @@ import {
2020
summarizePendingSchemaWork,
2121
} from '../../utils/schema-migrate.js';
2222
import { exitOneShotCommand } from '../../utils/one-shot-exit.js';
23+
import {
24+
refuseWhenHostConfigUnloadable,
25+
type SchemaMigrationComposition,
26+
} from '../../utils/schema-migration-plugins.js';
2327
import { probeMigrationTarget } from '../../utils/migrate-occupancy-gate.js';
2428
import { describeOccupancy } from '../../utils/sqlite-occupancy.js';
2529
import {
@@ -86,9 +90,26 @@ export default class MigratePlan extends Command {
8690
*/
8791
async run(): Promise<void> {
8892
await this.plan();
93+
// [#12953] A host config that EXISTS but could not be loaded means the plan
94+
// above covered a fraction of this deployment — UNMEASURED, not "in sync" —
95+
// and the maintainer ruled that green exit out (2026-08-29, verbatim
96+
// 「同意」). Applied HERE, after `plan()`, deliberately: every one of its
97+
// early returns (no SQL driver, in sync, the rendered plan) has already
98+
// written its report by now, and the report — the human plan, or the JSON
99+
// document whose `composition.hostConfigLoaded` the ruling kept as the
100+
// consumer's discriminator — must survive the refusal, not be replaced by
101+
// it. `this.composition` is `null` on the boot-failure path, which already
102+
// exits non-zero through oclif.
103+
if (this.composition) refuseWhenHostConfigUnloadable(this.composition);
89104
await exitOneShotCommand(typeof process.exitCode === 'number' ? process.exitCode : 0);
90105
}
91106

107+
/**
108+
* What {@link plan} composed, read by {@link run} after it returns (#12953).
109+
* `null` until the stack has booted, and on every path where it never did.
110+
*/
111+
private composition: SchemaMigrationComposition | null = null;
112+
92113
private async plan(): Promise<void> {
93114
const { flags } = await this.parse(MigratePlan);
94115
const timer = createTimer();
@@ -132,6 +153,7 @@ export default class MigratePlan extends Command {
132153
this.exit(1);
133154
return;
134155
}
156+
this.composition = stack.composition;
135157

136158
try {
137159
if (!stack.driver) {

packages/cli/src/utils/schema-migrate.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -338,7 +338,10 @@ export async function bootSchemaStack(
338338
cwd: opts.projectRoot ?? process.cwd(),
339339
skipSeedData: defer,
340340
})
341-
: { plugins: [], hostConfigPath: null, hostConfigLoaded: false, notes: [], coverage: null } satisfies SchemaMigrationComposition;
341+
: {
342+
plugins: [], hostConfigPath: null, hostConfigLoaded: false, hostConfigError: null,
343+
notes: [], coverage: null,
344+
} satisfies SchemaMigrationComposition;
342345
for (const plugin of composition.plugins) {
343346
await kernel.use(plugin as any);
344347
}

packages/cli/src/utils/schema-migration-plugins.test.ts

Lines changed: 94 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import {
99
composeForDeclarations,
1010
buildSchemaMigrationPlugins,
1111
measureComposedCoverage,
12+
describeUnloadableHostConfig,
13+
type SchemaMigrationComposition,
1214
} from './schema-migration-plugins.js';
1315

1416
/**
@@ -185,15 +187,104 @@ describe('buildSchemaMigrationPlugins', () => {
185187

186188
const out = await buildSchemaMigrationPlugins({ basePlugins: [], cwd: dir });
187189

188-
// Not fatal — this command worked before without ever reading the config,
189-
// and a plan that stops working is a worse regression than a reduced one.
190+
// The composition still COMPLETES — the reduced set is composed and
191+
// returned. What changed with #12953 is the verdict the COMMANDS draw from
192+
// it (a non-zero exit), not whether this function throws.
190193
expect(out.hostConfigPath).toBe(join(dir, 'objectstack.config.ts'));
191-
// …but it must be DISTINGUISHABLE. `managedTables` alone cannot say this:
194+
// …and it must be DISTINGUISHABLE. `managedTables` alone cannot say this:
192195
// the platform floor still lands, so the count rises either way.
193196
expect(out.hostConfigLoaded).toBe(false);
194197
const said = out.notes.join(' ');
195198
expect(said).toContain('could not be loaded');
196199
expect(said).toContain('UNMEASURED');
200+
// [#12953] The underlying failure, carried structurally so the refusal can
201+
// NAME it rather than re-parsing the prose above.
202+
expect(out.hostConfigError).toContain('OS_SOME_SECRET is required');
203+
});
204+
205+
it('leaves hostConfigError null when there is no host config at all', async () => {
206+
const none = await buildSchemaMigrationPlugins({ basePlugins: [], cwd: tempProject() });
207+
expect(none.hostConfigError).toBeNull();
208+
});
209+
210+
it('leaves hostConfigError null when the host config LOADS', async () => {
211+
const dir = tempProject();
212+
writeFileSync(join(dir, 'objectstack.config.ts'), 'export default { objects: [] };\n');
213+
const loaded = await buildSchemaMigrationPlugins({ basePlugins: [], cwd: dir });
214+
expect(loaded.hostConfigLoaded).toBe(true);
215+
expect(loaded.hostConfigError).toBeNull();
216+
// Loading a real config runs `bundle-require`/esbuild — well past the 5 s
217+
// default on a cold, shared box.
218+
}, 60_000);
219+
});
220+
221+
/**
222+
* #12953 — the predicate behind the non-zero exit, in all three directions.
223+
*
224+
* Maintainer ruling 2026-08-29 (verbatim 「同意」): a host config that EXISTS
225+
* and could not be loaded makes `os migrate plan` / `apply` exit non-zero,
226+
* because a green exit over an UNMEASURED partial metadata set is the
227+
* false-green a migration tool must never emit. The ruling pinned the OTHER
228+
* two directions just as hard — config absent, and config loadable, both keep
229+
* today's behaviour — so all three are pinned here.
230+
*
231+
* ⚠️ The trap this file exists to hold: `hostConfigLoaded` is `false` on the
232+
* config-ABSENT shape too (nothing loaded, because there was nothing to load).
233+
* A predicate written as `!hostConfigLoaded` therefore turns the untouched
234+
* population red, and every assertion about direction 1 still passes while it
235+
* does. The second case below is the one that fails if anyone writes it that
236+
* way.
237+
*
238+
* The exit STATUS itself is pinned over a real child process in
239+
* `packages/cli/test/migrate-unloadable-host-config-exit.e2e.test.ts` — a
240+
* `process.exitCode` set inside a vitest worker is not an exit status.
241+
*/
242+
describe('describeUnloadableHostConfig (#12953)', () => {
243+
function composition(over: Partial<SchemaMigrationComposition>): SchemaMigrationComposition {
244+
return {
245+
plugins: [], hostConfigPath: null, hostConfigLoaded: false, hostConfigError: null,
246+
notes: [], coverage: null, ...over,
247+
};
248+
}
249+
250+
it('direction 1 — config PRESENT and unloadable: names the config, the cause and the remedy', () => {
251+
const said = describeUnloadableHostConfig(composition({
252+
hostConfigPath: '/srv/app/objectstack.config.ts',
253+
hostConfigLoaded: false,
254+
hostConfigError: 'Missing required environment variable AUTH_SECRET',
255+
}));
256+
257+
expect(said).not.toBeNull();
258+
// The three things the ruling requires the error to name.
259+
expect(said).toContain('/srv/app/objectstack.config.ts');
260+
expect(said).toContain('Missing required environment variable AUTH_SECRET');
261+
expect(said).toMatch(/Remedy:/);
262+
// And that it is a FAILURE, not another warning — the whole point.
263+
expect(said).toContain('UNMEASURED');
264+
});
265+
266+
it('direction 2 — config ABSENT: null, even though hostConfigLoaded is false', () => {
267+
// `hostConfigPath === null` with `hostConfigLoaded === false` is the
268+
// untouched population. If this ever answers non-null, every project with
269+
// no config starts failing `os migrate plan`.
270+
expect(describeUnloadableHostConfig(composition({
271+
hostConfigPath: null, hostConfigLoaded: false,
272+
}))).toBeNull();
273+
});
274+
275+
it('direction 3 — config PRESENT and loadable: null', () => {
276+
expect(describeUnloadableHostConfig(composition({
277+
hostConfigPath: '/srv/app/objectstack.config.ts', hostConfigLoaded: true,
278+
}))).toBeNull();
279+
});
280+
281+
it('still names something when the load threw without a message', () => {
282+
const said = describeUnloadableHostConfig(composition({
283+
hostConfigPath: '/srv/app/objectstack.config.mjs', hostConfigLoaded: false,
284+
hostConfigError: null,
285+
}));
286+
expect(said).toContain('/srv/app/objectstack.config.mjs');
287+
expect(said).toContain('the load threw without a message');
197288
});
198289
});
199290

0 commit comments

Comments
 (0)