Skip to content

Commit b4f2cda

Browse files
os-trumpclaude
andauthored
fix(cli): os migrate apply refuses before writing any DDL on an unloadable host config (#13383)
* fix(cli): os migrate apply refuses before any DDL on an unloadable host config (#13118) Maintainer ruling 2026-08-29, verbatim 「同意」, option 2: refuse first, write no DDL, exit non-zero. #12953 had ruled only the exit status, so the same run reconciled the operator's schema against a set it had just called UNMEASURED. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TvqBFLRzXdSPcbusDoED9k * test(cli): type the sqlite catalog probe structurally so TEST_DEBT stays at 144 `check:type-check-debt --re-measure` measured @objectstack/cli at 145 raw errors against a frozen 144: the knex handle was reached through `{ knex: (t: string) => unknown }`, so `.select()` on it was TS2571. The ratchet is shrink-only, so the fix is the error, never the entry. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TvqBFLRzXdSPcbusDoED9k --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 491cadc commit b4f2cda

5 files changed

Lines changed: 560 additions & 2 deletions

File tree

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
---
2+
"@objectstack/cli": minor
3+
---
4+
5+
fix(cli): `os migrate apply` refuses BEFORE writing any DDL when the host config exists but could not be loaded (#13118)
6+
7+
#12953 ruled the exit STATUS on that path and said nothing about the mutation.
8+
So `apply` went on flushing its deferred schema work and applying drift over the
9+
reduced object set, and THEN exited non-zero — one run saying both "this result
10+
is UNMEASURED, not in sync" and "…and I changed your schema on that basis".
11+
Measured on this change's own fixture before the fix: the refused run created
12+
**9 tables** (`sys_metadata`, `sys_metadata_activation`, `sys_metadata_audit`,
13+
`sys_metadata_commit`, `sys_metadata_history`, `sys_migration`,
14+
`sys_migration_journal`, `sys_secret`, `sys_view_definition`), none of them the
15+
deployment's, and exited 1.
16+
17+
Maintainer ruling 2026-08-29, verbatim 「同意」, option 2: 采**选项 2**
18+
`os migrate apply` 在 host config 存在但不可加载时,**先拒绝、不写任何 DDL**,退出非零。
19+
`os migrate apply` now returns above `flushSchemaDdl()` and
20+
`applyMigrationEntries()` on that path — the two calls in the command that
21+
write — and the refusal on stderr reuses the ruled #12953 wording and
22+
**additionally states that no DDL was executed**, so an operator does not have
23+
to guess whether the database was touched. Under `--json` the document carries
24+
`message: "refused_unloadable_host_config"` with `created: []` and
25+
`applied: []`.
26+
27+
**BEHAVIOUR CHANGE to a mutating command**, shipped as `minor` for the same
28+
reason #12953's exit-status half was: the repo's launch-window convention treats
29+
a deliberate change to a published command's observable behaviour as `minor`
30+
rather than `patch`, and this one additionally adds a new `--json` `message`
31+
value that a consumer can branch on.
32+
33+
Scoped to exactly one shape; the ruling pinned the neighbours as hard as the
34+
changed one, and all three are measured in
35+
`packages/cli/test/migrate-apply-refuses-before-ddl.e2e.test.ts` on both halves
36+
— exit status *and* what the database holds afterwards:
37+
38+
- host config **present and unloadable** → non-zero **and zero tables created**
39+
(this change);
40+
- host config **absent** → unchanged: exit 0, platform floor still created;
41+
- host config **present and loadable** → unchanged: exit 0, the deployment's own
42+
tables still created.
43+
44+
**No flag, env var or other escape hatch.** Option 3 was refused in the same
45+
ruling — this repo does not add a published surface before the need for it is
46+
measured.
47+
48+
`os migrate plan` is untouched: it never wrote to the database, its refusal
49+
message is byte-identical to #12953's, and the no-DDL sentence is opt-in per
50+
call site rather than deduced from the command.
51+
52+
**Recoverability, measured for the ruling.** A partial apply over the reduced
53+
set DOES converge: after repairing the config, a full `apply` on the same
54+
database produces a schema identical to one a never-degraded database gets from
55+
a single full run (verified with a positive control — the same comparison
56+
detects a deliberately introduced one-column difference). So this change is
57+
contract honesty rather than data rescue; the ruling holds either way, and the
58+
cost is simply low.
59+
60+
**Migration.** A CI step that runs `os migrate apply` against a project whose
61+
config needs environment it was not given already failed (#12953); it now also
62+
leaves the database untouched instead of reconciling it against a fraction of
63+
the deployment. Supply that environment to the run (the error names the missing
64+
variable), or fix the config, then re-run.

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

Lines changed: 71 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import {
2323
} from '../../utils/schema-migrate.js';
2424
import { exitOneShotCommand } from '../../utils/one-shot-exit.js';
2525
import {
26+
describeUnloadableHostConfig,
2627
refuseWhenHostConfigUnloadable,
2728
type SchemaMigrationComposition,
2829
} from '../../utils/schema-migration-plugins.js';
@@ -56,6 +57,14 @@ async function confirm(question: string): Promise<boolean> {
5657
* 2. **A database somebody else is using is not migrated by accident.** The
5758
* SQLite target is probed for other attached connections before boot, and a
5859
* busy database refuses without `--force`.
60+
*
61+
* A third, added by #13118 (maintainer ruling 2026-08-29, verbatim 「同意」):
62+
*
63+
* 3. **An UNMEASURED run does not write.** When the host
64+
* `objectstack.config.{ts,js,mjs}` exists and could not be loaded, the object
65+
* set this command can see is the data stack plus the platform floor — not
66+
* the deployment's. #12953 made that run exit non-zero; it still applied its
67+
* DDL first. It now refuses above every write, and says so in the refusal.
5968
*/
6069
export default class MigrateApply extends Command {
6170
static override description =
@@ -99,7 +108,13 @@ export default class MigrateApply extends Command {
99108
// reconcile an operator confirms has to be judged the same way as the plan
100109
// they read. Applied after `apply()` for the same reason it is there: the
101110
// report is already written and must survive the non-zero exit.
102-
if (this.composition) refuseWhenHostConfigUnloadable(this.composition);
111+
// [#13118] `noDdlExecuted` is true because `apply()` above RETURNS on this
112+
// path before `flushSchemaDdl()` / `applyMigrationEntries()` — see the
113+
// refusal gate there. The two must move together: the sentence is a claim
114+
// about this run, not a label on the command.
115+
if (this.composition) {
116+
refuseWhenHostConfigUnloadable(this.composition, { noDdlExecuted: true });
117+
}
103118
await exitOneShotCommand(typeof process.exitCode === 'number' ? process.exitCode : 0);
104119
}
105120

@@ -253,6 +268,61 @@ export default class MigrateApply extends Command {
253268
}
254269
}
255270

271+
// ── [#13118] REFUSE BEFORE ANY DDL ──────────────────────────────
272+
//
273+
// Maintainer ruling 2026-08-29, verbatim 「同意」, option 2: when the host
274+
// config EXISTS and could not be loaded, `os migrate apply` refuses
275+
// WITHOUT writing DDL, and exits non-zero. #12953 had ruled only the exit
276+
// status, so until this gate the same run said "this result is UNMEASURED,
277+
// not in sync" and reconciled the operator's schema against that very
278+
// set — nine platform/data-stack tables, none of them the deployment's.
279+
//
280+
// Placement is the whole change, and it is exact:
281+
//
282+
// • BELOW the report. Everything above this line is read-only — the
283+
// boot deferred its DDL (#3917), `detectManagedDrift()` only reads the
284+
// catalog — and #12953 pinned that the refusal replaces the STATUS,
285+
// not the document. The plan and the `--json` payload still reach
286+
// their reader, `composition.hostConfigLoaded` included.
287+
// • ABOVE the confirmation gate. An operator must not be asked to
288+
// confirm a reconcile this command has already decided to refuse.
289+
// • ABOVE `flushSchemaDdl()` and `applyMigrationEntries()` — the two
290+
// calls in this file that write. That is what makes `run()`'s
291+
// `noDdlExecuted: true` a true sentence rather than a hopeful one.
292+
//
293+
// ⛔ No flag, env var or escape hatch: option 3 was refused in the same
294+
// ruling ("需求未测不加面"). The path back to a working apply is to fix the
295+
// config — which is also the only path back to a bootable deployment, as
296+
// `os serve` refuses the identical shape.
297+
//
298+
// Convergence, measured for the ruling before this gate was written: a
299+
// partial apply over the reduced set then a repaired full apply DOES
300+
// converge to the schema a never-degraded run produces. So this refusal
301+
// is contract honesty, not data rescue — which is exactly the outcome the
302+
// ruling said to implement anyway, at low cost.
303+
if (describeUnloadableHostConfig(stack.composition) !== null) {
304+
if (flags.json) {
305+
// `skipped`/`pending` carry what was NOT done, in the vocabulary the
306+
// other "we did not apply" payloads above already use. `created` and
307+
// `applied` are empty because nothing was created or applied — a
308+
// consumer must be able to read "no DDL" off the document too, not
309+
// only off stderr.
310+
await emitJson({
311+
database: stack.dbLabel,
312+
created: [],
313+
applied: [],
314+
skipped: drift,
315+
pending,
316+
message: 'refused_unloadable_host_config',
317+
...compositionPayload,
318+
}, 0, { compact: true });
319+
return;
320+
}
321+
// The refusal sentence itself is printed once, on stderr, by `run()`'s
322+
// shared choke point — not duplicated here.
323+
return;
324+
}
325+
256326
const totalIntended = intended.length + pending.length;
257327
if (totalIntended === 0) {
258328
if (flags.json) { await emitJson({ applied: [], skipped: deferred, created: [], message: 'nothing_safe_to_apply' }, 0, { compact: true }); return; }

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

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
buildSchemaMigrationPlugins,
1111
measureComposedCoverage,
1212
describeUnloadableHostConfig,
13+
NO_DDL_EXECUTED_NOTICE,
1314
type SchemaMigrationComposition,
1415
} from './schema-migration-plugins.js';
1516

@@ -288,6 +289,86 @@ describe('describeUnloadableHostConfig (#12953)', () => {
288289
});
289290
});
290291

292+
/**
293+
* #13118 — the MUTATING command additionally says it wrote nothing.
294+
*
295+
* Maintainer ruling 2026-08-29, verbatim 「同意」, option 2: `os migrate apply`
296+
* refuses on the unloadable-config path WITHOUT touching the database, and the
297+
* refusal must say so — "an operator reading it must not have to guess whether
298+
* the database was touched".
299+
*
300+
* The zero-DDL behaviour itself is pinned over a real child process and a real
301+
* `sqlite_master` read in
302+
* `packages/cli/test/migrate-apply-refuses-before-ddl.e2e.test.ts`; this file
303+
* owns the WORDING, and the two things the option must not do:
304+
*
305+
* • it must not leak into `os migrate plan`, whose message the ruling pinned
306+
* unchanged (`plan` 行为不变) — so the default spelling is byte-identical to
307+
* the #12953 text;
308+
* • it must not widen the predicate. An option that made the untouched
309+
* populations answer non-null would turn every config-less and every
310+
* healthy project red, and direction 1 would keep passing while it did.
311+
*/
312+
describe('describeUnloadableHostConfig — the no-DDL notice (#13118)', () => {
313+
function composition(over: Partial<SchemaMigrationComposition>): SchemaMigrationComposition {
314+
return {
315+
plugins: [], hostConfigPath: null, hostConfigLoaded: false, hostConfigError: null,
316+
notes: [], coverage: null, ...over,
317+
};
318+
}
319+
320+
const unloadable = composition({
321+
hostConfigPath: '/srv/app/objectstack.config.ts',
322+
hostConfigLoaded: false,
323+
hostConfigError: 'Missing required environment variable AUTH_SECRET',
324+
});
325+
326+
it('states that no DDL ran when the caller asserts it', () => {
327+
const said = describeUnloadableHostConfig(unloadable, { noDdlExecuted: true });
328+
expect(said).toContain(NO_DDL_EXECUTED_NOTICE.trim());
329+
// ⭐ And it is an ADDITION, not a replacement: the ruling said to REUSE the
330+
// #12953 wording and add to it, so every element that ruling required is
331+
// still there.
332+
expect(said).toContain('/srv/app/objectstack.config.ts');
333+
expect(said).toContain('Missing required environment variable AUTH_SECRET');
334+
expect(said).toContain('UNMEASURED');
335+
expect(said).toMatch(/Remedy:/);
336+
});
337+
338+
it("says nothing about DDL by default — `plan`'s message is byte-identical to #12953's", () => {
339+
// The default spelling and the explicit-false spelling are the same
340+
// string, and neither carries the notice. `plan` passes no options at all,
341+
// so this is the exact text it emits.
342+
const byDefault = describeUnloadableHostConfig(unloadable);
343+
expect(byDefault).not.toContain('NO DDL');
344+
expect(byDefault).toBe(describeUnloadableHostConfig(unloadable, {}));
345+
expect(byDefault).toBe(describeUnloadableHostConfig(unloadable, { noDdlExecuted: false }));
346+
});
347+
348+
it('the notice is the only difference the option makes', () => {
349+
// Written as a subtraction rather than as a second copy of the sentence:
350+
// a test that re-spells the message is a test that stops holding it.
351+
const withNotice = describeUnloadableHostConfig(unloadable, { noDdlExecuted: true })!;
352+
expect(withNotice.replace(NO_DDL_EXECUTED_NOTICE, '')).toBe(
353+
describeUnloadableHostConfig(unloadable),
354+
);
355+
});
356+
357+
it('⛔ does not widen the predicate — the untouched populations stay null', () => {
358+
// Both with the option set: an option that could turn "no config" or
359+
// "config loads fine" into a refusal is the failure #12953 already named,
360+
// and direction 1 above passes identically while it happens.
361+
expect(describeUnloadableHostConfig(
362+
composition({ hostConfigPath: null, hostConfigLoaded: false }),
363+
{ noDdlExecuted: true },
364+
)).toBeNull();
365+
expect(describeUnloadableHostConfig(
366+
composition({ hostConfigPath: '/srv/app/objectstack.config.ts', hostConfigLoaded: true }),
367+
{ noDdlExecuted: true },
368+
)).toBeNull();
369+
});
370+
});
371+
291372
/**
292373
* #13028 — the plan reports its own BOUNDARY.
293374
*

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

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -392,9 +392,56 @@ export async function buildSchemaMigrationPlugins(opts: {
392392
*
393393
* The message names the three things the ruling requires of it: the config
394394
* file, the underlying failure, and the remedy.
395+
*
396+
* ## The mutation half (#13118), and why it is a caller's claim
397+
*
398+
* #12953 ruled the exit STATUS and said nothing about the mutation, so `apply`
399+
* shipped writing its DDL over the reduced object set and THEN exiting
400+
* non-zero — the same run saying "this result is UNMEASURED" and "…and I
401+
* changed your schema on that basis". Maintainer ruling 2026-08-29, verbatim
402+
* 「同意」, option 2: `os migrate apply` refuses on this path **without touching
403+
* the database**, and the refusal must say so explicitly, so an operator
404+
* reading it does not have to guess.
405+
*
406+
* That extra sentence is opt-in ({@link UnloadableHostConfigRefusalOptions})
407+
* rather than automatic. It is a claim about what a particular run did, and the
408+
* only site that can honestly make it is one that returned before its own
409+
* mutating work.
395410
*/
411+
export interface UnloadableHostConfigRefusalOptions {
412+
/**
413+
* Say, in the refusal itself, that this run performed **no DDL** (#13118).
414+
*
415+
* ⛔ Not a default, and deliberately not deduced from the command name. The
416+
* sentence is a claim about what THIS run did to the operator's database,
417+
* and only a call site that has actually returned before its mutating work
418+
* can make it. `os migrate apply` passes `true` because #13118 moved its
419+
* refusal above `flushSchemaDdl()` / `applyMigrationEntries()`; a future
420+
* caller that refuses AFTER writing must say nothing here and get the
421+
* #12953 wording unchanged, rather than inherit a false all-clear by
422+
* omission.
423+
*
424+
* `os migrate plan` never passes it: `plan` writes nothing on ANY path, so
425+
* the sentence would be noise there — and its message is pinned unchanged by
426+
* the ruling's "plan 行为不变".
427+
*/
428+
noDdlExecuted?: boolean;
429+
}
430+
431+
/**
432+
* The sentence #13118 requires of the MUTATING command's refusal, verbatim.
433+
*
434+
* Exported so the pin and the message have one source: an operator reading the
435+
* refusal "must not have to guess whether the database was touched", and a
436+
* test that re-spells the sentence stops holding it the day the wording moves.
437+
*/
438+
export const NO_DDL_EXECUTED_NOTICE =
439+
'NO DDL WAS EXECUTED: this run refused before touching the database, so the physical '
440+
+ 'schema is exactly as it was before the command ran. ';
441+
396442
export function describeUnloadableHostConfig(
397443
composition: SchemaMigrationComposition,
444+
options: UnloadableHostConfigRefusalOptions = {},
398445
): string | null {
399446
if (composition.hostConfigPath === null || composition.hostConfigLoaded) return null;
400447
const cause = composition.hostConfigError ?? 'the load threw without a message';
@@ -403,6 +450,7 @@ export function describeUnloadableHostConfig(
403450
+ 'This run therefore covered ONLY the objects the data stack registered — a fraction of '
404451
+ 'what this deployment serves — so its result is UNMEASURED, not "in sync", and it is '
405452
+ 'reported as a FAILURE rather than as success. '
453+
+ (options.noDdlExecuted === true ? NO_DDL_EXECUTED_NOTICE : '')
406454
+ 'Remedy: supply the environment this config needs (the failure named above says which), '
407455
+ 'or fix the config, then re-run.'
408456
);
@@ -426,12 +474,15 @@ export function describeUnloadableHostConfig(
426474
* time this runs and must survive. `migrate/plan.ts`'s `run()` wrapper reads
427475
* `process.exitCode` and hands it to `exitOneShotCommand`.
428476
*
477+
* @param options see {@link UnloadableHostConfigRefusalOptions} — `apply`
478+
* passes `noDdlExecuted: true` (#13118); `plan` passes nothing.
429479
* @returns `true` when this run was the refused shape.
430480
*/
431481
export function refuseWhenHostConfigUnloadable(
432482
composition: SchemaMigrationComposition,
483+
options: UnloadableHostConfigRefusalOptions = {},
433484
): boolean {
434-
const line = describeUnloadableHostConfig(composition);
485+
const line = describeUnloadableHostConfig(composition, options);
435486
if (line === null) return false;
436487
// eslint-disable-next-line no-console
437488
console.error(`[migrate] ✗ ${line}`);

0 commit comments

Comments
 (0)