diff --git a/.changeset/migrate-meta-default-range-terminus.md b/.changeset/migrate-meta-default-range-terminus.md new file mode 100644 index 0000000000..2ef119d6d7 --- /dev/null +++ b/.changeset/migrate-meta-default-range-terminus.md @@ -0,0 +1,20 @@ +--- +"@objectstack/cli": minor +--- + +fix(cli): `os migrate meta --from N` — the invocation every tombstone prescribes — lists the conversions it was sent to list, and an empty range stops reading as success (#17134) + +`--to` defaulted to `PROTOCOL_MAJOR`, the major the runtime implements. But retirements land throughout a major's line, and their ADR-0087 conversions are registered under the NEXT one: `@objectstack/spec@17.4.0` tombstones `dashboard.refreshInterval` while the conversion that renames it is `toMajor: 18`. The `retiredKey()` house sentence names the major the source was **authored** against — `Run \`os migrate meta --from 17\` …` — so the prescribed invocation composed the range `17 → 17`, which `composeMigrationChain` selects **no step** for, and the command answered: + +``` +✓ Nothing to migrate — the metadata is already canonical for this range. +``` + +exit 0, printed immediately under the five refusals that named that exact command. **29 shipped tombstones across 15 source files prescribe it.** + +Two changes, both in `packages/cli`: + +- **`--to` now defaults to the highest major this build of `@objectstack/spec` carries a migration step for** (`Math.max(PROTOCOL_MAJOR, ...MIGRATION_MAJORS)`), so the tombstone template's presumption holds in every window rather than only after the next major has shipped. Nothing is migrated "past" the runtime: every registered conversion maps a shape the installed schemas already **refuse** onto the one they accept, which is why the terminus is the only target for which the command's own `schemaValid` verdict is reachable. `Math.max` keeps the runtime's major as the floor for the reverse case. +- **A range holding no step is answered as one.** `already canonical` was a green verdict on a check that never ran, so the empty-range case now says so, names the range that would list the conversions (`--to N`), and no longer returns past the schema verdict that contradicted it — the same run used to report `schemaValid: false` in `--json` while the human output claimed the metadata was canonical and stopped. + +**What changes for you.** `os migrate meta --from ` with no `--to` now replays one hop further than it did, so a cross-major run prints that hop's semantic TODOs as well — the same wall a `--from N-1` run has always printed, one major on. The mechanical rewrite list is still first. `--to` is unchanged when you pass it, `--stored` is untouched, exit codes are unchanged (this command reports findings, it does not exit on them), and a range that holds real steps and rewrote nothing still answers `Nothing to migrate`. diff --git a/packages/cli/src/commands/migrate/meta.ts b/packages/cli/src/commands/migrate/meta.ts index 823ea01127..7af7441d12 100644 --- a/packages/cli/src/commands/migrate/meta.ts +++ b/packages/cli/src/commands/migrate/meta.ts @@ -11,6 +11,7 @@ import { composeSpecChanges, normalizeStackInput, MigrationFloorError, + MIGRATION_MAJORS, } from '@objectstack/spec'; import { PROTOCOL_MAJOR, PROTOCOL_VERSION } from '@objectstack/spec/kernel'; import { FILE_REFERENCE_TYPES, REFERENCE_VALUE_TYPES, STRUCTURED_JSON_TYPES } from '@objectstack/spec/data'; @@ -47,6 +48,44 @@ async function confirm(question: string): Promise { /** The protocol major that introduced the per-deployment value-shape gates. */ const VALUE_SHAPE_GATE_MAJOR = 17; +/** + * Where the chain ENDS when the author does not say — the highest major this + * build of `@objectstack/spec` carries a migration step for, never below the + * protocol major the runtime implements. + * + * ## Why not `PROTOCOL_MAJOR` (the answer this replaces) + * + * A tombstone closes with the house sentence "Run `os migrate meta --from N` + * to list the mechanical edits for existing sources", and its `N` is the major + * the source was AUTHORED against — one below the `toMajor` of the ADR-0087 + * conversion that performs the rename. That template presumes the default + * terminus is at least the conversion's own `toMajor`. + * + * The presumption held only AFTER the next major shipped. Retirements land + * throughout a major's line: `@objectstack/spec@17.4.0` tombstones keys whose + * conversion is registered `toMajor: 18` and whose semantic siblings are + * already removed from its own exports — the build's authorable surface is + * ahead of the version it calls itself. Defaulting `--to` to `PROTOCOL_MAJOR` + * (17) then composed `17 → 17`, which selects NO step at all + * (`composeMigrationChain` keeps `m > fromMajor`), so the invocation 29 + * shipped tombstones prescribe replayed an empty chain and reported + * `Nothing to migrate` — for the very conversions that sent the author here. + * + * Reading the terminus off `MIGRATION_MAJORS` makes the template's presumption + * true in every window instead of only after a major release, and it stays + * true one major later by construction: when 18 ships, `PROTOCOL_MAJOR` + * becomes 18, the 19 entries accumulating in the registry become the terminus, + * and `--from 18` lists them the same way. + * + * ⛔ Not "migrating past what the runtime runs": every conversion in the + * registry maps a shape the installed schemas REFUSE onto the one they accept + * (that is what makes it a conversion), so the terminus is the only target + * for which the command's own `schemaValid` verdict is reachable. `Math.max` + * keeps `PROTOCOL_MAJOR` as the floor for the reverse case — a runtime whose + * major moved past the last registered step. + */ +const CHAIN_TERMINUS_MAJOR = Math.max(PROTOCOL_MAJOR, ...MIGRATION_MAJORS); + /** Flags that mean something only in `--stored` mode (#4327). */ const STORED_ONLY_FLAGS = ['apply', 'yes', 'force', 'type', 'database-url'] as const; @@ -127,6 +166,61 @@ function pendingDataMigrations(stack: any, fromMajor: number, toMajor: number): return pending; } +/** + * The answer a range holding NO migration step owes the author (#17134). + * + * `composeMigrationChain` keeps the majors `m > fromMajor && m <= toMajor`, so + * `--from 17 --to 17` composes zero steps and every stack — canonical or not — + * comes back with an empty `applied` and an empty `todos`. Reporting that as + * `✓ Nothing to migrate — the metadata is already canonical for this range` is + * not merely unhelpful: it is a green verdict on a check that never ran, and + * it is the SECOND signal an upgrading author has already been told to trust + * (the first was the tombstone that named this command). So an empty range is + * answered as an empty range, and — when a wider one would rewrite this very + * stack — with the range that lists them, because "which `--to` do I need" is + * the question the author is left holding. + * + * The probe is exact rather than advisory: it replays the widest chain this + * build carries against the SAME normalized stack, so it can only speak up + * when there is real work for THIS source. A genuinely canonical stack in an + * empty range is still told its range was empty — that much is a fact about + * the invocation — but is offered no phantom conversions. + */ +function printEmptyRangeAnswer( + stack: Record, + fromMajor: number, + toMajor: number, +): void { + printWarning( + `No migration step exists for protocol ${fromMajor} → ${toMajor}, so this run replayed nothing ` + + '— that is not a finding that the metadata is canonical.', + ); + + if (toMajor >= CHAIN_TERMINUS_MAJOR) { + printInfo( + `Protocol ${CHAIN_TERMINUS_MAJOR} is the highest major this build carries a step for, so no ` + + 'wider range is available; check `--from` against the major the metadata was authored ' + + 'against.', + ); + return; + } + + const wider = applyMetaMigrations(stack, fromMajor, CHAIN_TERMINUS_MAJOR); + if (wider.applied.length === 0 && wider.todos.length === 0) { + printInfo( + `The widest range this build carries (protocol ${fromMajor} → ${CHAIN_TERMINUS_MAJOR}) has ` + + 'nothing for this stack either.', + ); + return; + } + + printWarning( + `Protocol ${fromMajor} → ${CHAIN_TERMINUS_MAJOR} has ${wider.applied.length} mechanical and ` + + `${wider.todos.length} manual change(s) for this stack — re-run with ` + + `\`--to ${CHAIN_TERMINUS_MAJOR}\` to list them.`, + ); +} + /** Print the data-migration advice — the last thing a crossing upgrade sees. */ function printPendingDataMigrations(pending: PendingDataMigration[]): void { if (pending.length === 0) return; @@ -147,9 +241,11 @@ function printPendingDataMigrations(pending: PendingDataMigration[]): void { /** * `os migrate meta --from N` — replay the ADR-0087 D3 migration chain. * - * Composes the per-major steps N+1 → … → current and applies each major's - * mechanical transforms (the graduated D2 conversions) to the loaded stack in - * one run — cross-major is the designed-for case, not an edge. It reports a + * Composes the per-major steps N+1 → … → {@link CHAIN_TERMINUS_MAJOR} (the + * highest major this build has a step for, which is where `--to` defaults) and + * applies each major's mechanical transforms (the graduated D2 conversions) to + * the loaded stack in one run — cross-major is the designed-for case, not an + * edge. It reports a * generated, schema-validated diff (the mechanical rewrites) plus the structured * TODOs for the semantic changes the chain cannot apply, so the consumer agent * reviews a provably-valid change instead of hand-porting from prose. @@ -199,7 +295,9 @@ export default class MigrateMeta extends Command { exclusive: ['stored'], }), to: Flags.integer({ - description: `Target protocol major (defaults to this runtime's, ${PROTOCOL_MAJOR}).`, + description: + `Target protocol major (defaults to ${CHAIN_TERMINUS_MAJOR}, the highest major this build ` + + `has a migration step for; this runtime implements protocol ${PROTOCOL_MAJOR}).`, exclusive: ['stored'], }), step: Flags.boolean({ @@ -286,7 +384,7 @@ export default class MigrateMeta extends Command { return; } const fromMajor = flags.from; - const toMajor = flags.to ?? PROTOCOL_MAJOR; + const toMajor = flags.to ?? CHAIN_TERMINUS_MAJOR; if (!flags.json) printHeader('Migrate · meta'); @@ -373,53 +471,74 @@ export default class MigrateMeta extends Command { console.log(''); if (result.applied.length === 0 && result.todos.length === 0) { - printSuccess('Nothing to migrate — the metadata is already canonical for this range.'); + // ⚠️ Two different facts wear the same empty result, and only one of + // them is good news (#17134). A range that CONTAINS steps and rewrote + // nothing is a finding about the metadata. A range that contains no + // step at all replayed nothing and therefore found nothing — saying + // "already canonical" over it is a claim about a check that never ran. + if (result.hops.length === 0) { + printEmptyRangeAnswer(normalized, fromMajor, toMajor); + } else { + printSuccess('Nothing to migrate — the metadata is already canonical for this range.'); + } // Still advertise: metadata needing no rewrite says nothing about // whether this deployment's DATA has been migrated. console.log(''); printPendingDataMigrations(dataMigrations); - return; - } + // ⛔ NOT a `return`. The schema verdict at the end of this block is the + // only line that can contradict a "nothing to do" answer, and returning + // past it was the second half of #17134: on a stack authoring a + // tombstoned key the same run reported `schemaValid: false` in `--json` + // while the human output said the metadata was canonical and stopped. + } else { + // Mechanical rewrites (auto-applied). + if (result.applied.length > 0) { + console.log(chalk.bold(` Applied ${result.applied.length} mechanical change(s):`)); + for (const a of result.applied) { + console.log(` • ${a.path}: ${chalk.red(a.from)} → ${chalk.green(a.to)} ${chalk.dim(`(${a.conversionId})`)}`); + } + console.log(''); + } - // Mechanical rewrites (auto-applied). - if (result.applied.length > 0) { - console.log(chalk.bold(` Applied ${result.applied.length} mechanical change(s):`)); - for (const a of result.applied) { - console.log(` • ${a.path}: ${chalk.red(a.from)} → ${chalk.green(a.to)} ${chalk.dim(`(${a.conversionId})`)}`); + // Per-hop checkpoints. + if (flags.step) { + for (const hop of result.hops) { + console.log(chalk.bold(` ── protocol ${hop.toMajor} ──`)); + console.log(chalk.dim(` ${hop.rationale}`)); + console.log(chalk.dim(` ${hop.applied.length} mechanical, ${hop.todos.length} manual`)); + } + console.log(''); } - console.log(''); - } - // Per-hop checkpoints. - if (flags.step) { - for (const hop of result.hops) { - console.log(chalk.bold(` ── protocol ${hop.toMajor} ──`)); - console.log(chalk.dim(` ${hop.rationale}`)); - console.log(chalk.dim(` ${hop.applied.length} mechanical, ${hop.todos.length} manual`)); + // Semantic TODOs (delegated to the agent — never auto-applied). + if (result.todos.length > 0) { + console.log(chalk.bold(chalk.yellow(` ${result.todos.length} manual change(s) require your judgment:`))); + for (const t of result.todos) { + console.log(` ${chalk.yellow('⚠')} [protocol ${t.toMajor}] ${t.surface} → ${t.replacement}`); + console.log(chalk.dim(` why: ${t.reason}`)); + console.log(chalk.dim(` verify: ${t.acceptanceCriteria}`)); + } + console.log(''); } - console.log(''); - } - // Semantic TODOs (delegated to the agent — never auto-applied). - if (result.todos.length > 0) { - console.log(chalk.bold(chalk.yellow(` ${result.todos.length} manual change(s) require your judgment:`))); - for (const t of result.todos) { - console.log(` ${chalk.yellow('⚠')} [protocol ${t.toMajor}] ${t.surface} → ${t.replacement}`); - console.log(chalk.dim(` why: ${t.reason}`)); - console.log(chalk.dim(` verify: ${t.acceptanceCriteria}`)); + if (flags.out) { + writeFileSync(resolve(flags.out), JSON.stringify(result.stack, null, 2)); + printInfo(`Wrote migrated stack snapshot → ${chalk.white(resolve(flags.out))}`); } - console.log(''); - } - if (flags.out) { - writeFileSync(resolve(flags.out), JSON.stringify(result.stack, null, 2)); - printInfo(`Wrote migrated stack snapshot → ${chalk.white(resolve(flags.out))}`); + printPendingDataMigrations(dataMigrations); } - printPendingDataMigrations(dataMigrations); - if (parsed.success) { printSuccess(`Migrated stack is schema-valid ${chalk.dim(`(${timer.display()})`)}`); + } else if (result.hops.length === 0) { + // "Resolve the changes above" has nothing to point at when the range + // held no step: this run rewrote nothing, so the refusals are exactly + // the ones the source had before it (#17134). + printWarning( + 'Stack does not pass schema validation, and this run replayed no conversion — nothing ' + + 'here has been fixed. Widen the range above, or run `os validate` for the refusals.', + ); } else { printWarning( 'Migrated stack does not yet pass schema validation — resolve the manual changes above, ' + diff --git a/packages/cli/test/migrate-meta-default-range.test.ts b/packages/cli/test/migrate-meta-default-range.test.ts new file mode 100644 index 0000000000..e168127752 --- /dev/null +++ b/packages/cli/test/migrate-meta-default-range.test.ts @@ -0,0 +1,229 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `os migrate meta` — the DEFAULT `--to`, and what an empty range is allowed to + * claim (#17134). + * + * ## The defect these pin + * + * A `retiredKey()` tombstone closes with the house sentence "Run + * `os migrate meta --from N` …", whose `N` is the major the source was AUTHORED + * against — one below the `toMajor` of the ADR-0087 conversion that performs + * the rename. That template presumes the default terminus is at least the + * conversion's own `toMajor`, and the presumption held only after the next + * major shipped: `@objectstack/spec@17.4.0` tombstones keys registered + * `toMajor: 18`, so `--from 17` defaulted to `--to 17`, + * `composeMigrationChain` (which keeps `m > fromMajor`) selected NO step, and + * the command answered `✓ Nothing to migrate — the metadata is already + * canonical for this range` at exit 0 — for the very conversions the tombstone + * had just sent the author to run it for. 29 shipped tombstones across 15 + * source files prescribe that invocation. + * + * ⚠️ The sharp half is that an empty chain makes the answer UNFALSIFIABLE: + * with no step selected, `applied` and `todos` are empty for every input, so + * that invocation at the installed major could not have reported anything + * else, for any stack, ever. + * + * ## Why these spawn, and why the file is QUEUE tier rather than `.e2e` + * + * The subject is a FLAG DEFAULT and the sentence a real terminal prints, both + * of which live above every seam an in-process test could reach: the default is + * resolved by oclif from the flag declaration, and the answer is chosen in the + * human-output branch `--json` skips entirely. So the CLI is spawned — but the + * file deliberately does NOT carry the `.e2e` name, because that name selects + * the NIGHTLY population (`vitest-tiers.ts` → "The NIGHTLY tiers"), and a p1 + * whose only pin runs nightly is not protected by the merge queue's required + * set. The tier header sanctions exactly this combination: the name decides the + * run and the behaviour decides the project, so this is queue-tier by name and + * `integration` by behaviour. Cost is held down by running each distinct + * invocation once and sharing it across the assertions that read it. + * + * ⛔ No expectation here hard-codes 17 or 18. Both majors move every release; + * what does not move is that the default terminus is the highest major the + * installed build carries a step for, so every expectation is derived from + * `MIGRATION_MAJORS` and `PROTOCOL_MAJOR` and stays true one major later. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { execFile } from 'node:child_process'; +import { promisify } from 'node:util'; +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { MIGRATION_MAJORS } from '@objectstack/spec'; +import { PROTOCOL_MAJOR } from '@objectstack/spec/kernel'; +import { childEnv } from './helpers/serve-process.js'; + +const execFileP = promisify(execFile); +const HERE = resolve(fileURLToPath(import.meta.url), '..'); +const CLI = resolve(HERE, '../bin/run-dev.js'); +const TSX = resolve(HERE, '../../../node_modules/.bin/tsx'); + +/** What the command must now default `--to` to — derived, never written down. */ +const TERMINUS = Math.max(PROTOCOL_MAJOR, ...MIGRATION_MAJORS); +const INSTALLED = String(PROTOCOL_MAJOR); + +/** + * The card's reproduction: a stack on the installed line authoring the + * tombstoned `dashboard.refreshInterval` five times. Five rather than one so no + * assertion can pass on a single incidental rewrite. + */ +const RETIRED_KEY_CONFIG = ` +export default { + manifest: { id: 'default_range_repro', name: 'Default Range Repro', version: '1.0.0', type: 'app' }, + objects: [{ name: 'dr_ticket', label: 'Ticket', fields: { title: { type: 'text', label: 'Title' } } }], + dashboards: [ + { name: 'kpi_a', label: 'KPI A', widgets: [], refreshInterval: 300 }, + { name: 'kpi_b', label: 'KPI B', widgets: [], refreshInterval: 60 }, + { name: 'kpi_c', label: 'KPI C', widgets: [], refreshInterval: 120 }, + { name: 'kpi_d', label: 'KPI D', widgets: [], refreshInterval: 900 }, + { name: 'kpi_e', label: 'KPI E', widgets: [], refreshInterval: 30 }, + ], +}; +`; + +/** The same shape already canonical — the control every "it fired" line needs. */ +const CANONICAL_CONFIG = ` +export default { + manifest: { id: 'default_range_canon', name: 'Default Range Canon', version: '1.0.0', type: 'app' }, + objects: [{ name: 'dr_thing', label: 'Thing', fields: { title: { type: 'text', label: 'Title' } } }], + dashboards: [{ name: 'kpi_a', label: 'KPI A', widgets: [], refreshIntervalSeconds: 300 }], +}; +`; + +const RENAME_CONVERSION = 'dashboard-refresh-interval-to-refresh-interval-seconds'; + +interface Run { stdout: string; code: number } + +let retiredDir: string; +let canonicalDir: string; +const runs = new Map>(); + +/** + * Spawn the real CLI once per distinct invocation. The exit code is returned + * beside stdout because it is half of what "reads as success" meant here. + */ +function runMeta(args: string[], cwd: string): Promise { + const key = `${cwd}::${args.join(' ')}`; + const hit = runs.get(key); + if (hit) return hit; + const started = execFileP(TSX, [CLI, 'migrate', 'meta', ...args], { + cwd, + maxBuffer: 64 * 1024 * 1024, + env: childEnv({ NO_COLOR: '1' }), + }).then( + ({ stdout }) => ({ stdout, code: 0 }), + (error: any) => ({ stdout: String(error.stdout ?? ''), code: Number(error.code ?? 1) }), + ); + runs.set(key, started); + return started; +} + +beforeAll(() => { + retiredDir = mkdtempSync(join(tmpdir(), 'os-meta-default-range-')); + writeFileSync(join(retiredDir, 'objectstack.config.ts'), RETIRED_KEY_CONFIG); + canonicalDir = mkdtempSync(join(tmpdir(), 'os-meta-default-canon-')); + writeFileSync(join(canonicalDir, 'objectstack.config.ts'), CANONICAL_CONFIG); +}); + +afterAll(() => { + for (const d of [retiredDir, canonicalDir]) { + try { rmSync(d, { recursive: true, force: true }); } catch { /* ignore */ } + } +}); + +describe('os migrate meta — the invocation the tombstones prescribe (#17134)', () => { + it('defaults --to to the highest major this build has a step for, not the runtime major', async () => { + const { stdout, code } = await runMeta(['--from', INSTALLED, '--json'], retiredDir); + const parsed = JSON.parse(stdout); + expect(code).toBe(0); + expect(parsed.from).toBe(PROTOCOL_MAJOR); + expect(parsed.to).toBe(TERMINUS); + // ⛔ Anti-vacuity. If the terminus ever equalled PROTOCOL_MAJOR the line + // above would hold for the very default this card exists to replace, so the + // premise is asserted rather than assumed: this is the line that speaks up + // when a major ships and the registry has no entry past it yet. + expect(TERMINUS, 'the registry carries a step past the runtime major').toBeGreaterThan(PROTOCOL_MAJOR); + }, 120_000); + + it('lists every retired-key rewrite with no --to given at all', async () => { + const { stdout } = await runMeta(['--from', INSTALLED, '--json'], retiredDir); + const parsed = JSON.parse(stdout); + + const renames = parsed.applied.filter((a: any) => a.conversionId === RENAME_CONVERSION); + expect(renames.map((a: any) => a.path)).toEqual([ + 'dashboards[0].refreshIntervalSeconds', + 'dashboards[1].refreshIntervalSeconds', + 'dashboards[2].refreshIntervalSeconds', + 'dashboards[3].refreshIntervalSeconds', + 'dashboards[4].refreshIntervalSeconds', + ]); + for (const r of renames) { + expect(r.from).toBe('refreshInterval'); + expect(r.to).toBe('refreshIntervalSeconds'); + } + // The command's own success criterion, unreachable before this fix: the + // stack the author is asked to adopt parses under the INSTALLED schema. + // Pre-fix this same run reported `applied: [], schemaValid: false`. + expect(parsed.schemaValid).toBe(true); + }, 120_000); + + it('the human run prints the rewrites instead of `Nothing to migrate`', async () => { + const { stdout, code } = await runMeta(['--from', INSTALLED], retiredDir); + expect(code).toBe(0); + expect(stdout).toContain('Applied 5 mechanical change(s)'); + expect(stdout).toContain(RENAME_CONVERSION); + // ⛔ The whole sentence, never the phrase. Two step-18 semantic entries open + // their `replacement` with "Nothing to migrate to, because …", so a bare + // `not.toContain('Nothing to migrate')` fails on prose that is not this + // command's verdict at all — and, run the other way round, a grep for the + // phrase reports the verdict present on a run that never printed it. That + // collision is why the published acceptance check this PR corrects reads + // `applied` from `--json` instead of grepping the headline. + expect(stdout).not.toContain('Nothing to migrate — the metadata is already canonical'); + }, 120_000); +}); + +describe('os migrate meta — an empty range answers as an empty range (#17134)', () => { + /** + * The pre-fix default, now reachable only by typing it. The command is right + * that this range holds no conversion; what it may not do is turn that into a + * verdict about the metadata. + */ + it('refuses to call an un-migrated stack canonical when the range holds no step', async () => { + const { stdout, code } = await runMeta(['--from', INSTALLED, '--to', INSTALLED], retiredDir); + + expect(stdout).not.toContain('already canonical'); + expect(stdout).toContain(`No migration step exists for protocol ${INSTALLED} → ${INSTALLED}`); + // Triage's requirement: name the range that WOULD list them. + expect(stdout).toContain(`--to ${TERMINUS}`); + expect(stdout).toContain(`Protocol ${INSTALLED} → ${TERMINUS} has 5 mechanical`); + // ⛔ The exit code is deliberately unchanged. This command reports findings + // rather than exiting on them — its schema-invalid arm beside this one has + // always been a warning at exit 0. What changed is that the text no longer + // reads as success while it does so. + expect(code).toBe(0); + }, 120_000); + + it('no longer returns past the schema verdict that contradicts it', async () => { + const { stdout } = await runMeta(['--from', INSTALLED, '--to', INSTALLED], retiredDir); + // Unreachable before the fix: the zero-change branch returned first, so the + // same run could report `schemaValid: false` in `--json` while the human + // output claimed the metadata was canonical and stopped. + expect(stdout).toContain('does not pass schema validation'); + expect(stdout).toContain('replayed no conversion'); + }, 120_000); + + it('still says `Nothing to migrate` for a range that HAS steps and rewrote nothing', async () => { + // ⛔ The success sentence is not collateral damage: a range holding real + // steps that matched nothing is a finding about the metadata, and it keeps + // the answer published acceptance checks grep for. `13 → 14` is chosen + // because step 14 carries no semantic entries, so a canonical stack comes + // back with both lists empty for a NON-empty chain. + const { stdout, code } = await runMeta(['--from', '13', '--to', '14'], canonicalDir); + expect(code).toBe(0); + expect(stdout).toContain('Nothing to migrate'); + expect(stdout).toContain('Migrated stack is schema-valid'); + }, 120_000); +}); diff --git a/skills/objectstack-upgrade/SKILL.md b/skills/objectstack-upgrade/SKILL.md index 9d6929aef3..fe9ae32873 100644 --- a/skills/objectstack-upgrade/SKILL.md +++ b/skills/objectstack-upgrade/SKILL.md @@ -71,7 +71,7 @@ node -e "console.log(require('fs').readFileSync(require.resolve('@objectstack/sp # 3 · acceptance — all four, not three os validate # green (compare against validate-before.txt) tsc --noEmit # tombstones type the retired keys as `never` -os migrate meta --from 17 # must say "Nothing to migrate" +os migrate meta --from 17 --json # `applied` must be [] (see §3.3) # → write .upgrade/REPORT.md (template in §3.4) ``` @@ -436,9 +436,14 @@ part of it. > criterion that closes the gap is the replay: > > ```bash -> os migrate meta --from # must report "Nothing to migrate" +> os migrate meta --from --json # `applied` must be [] > ``` > +> Read `applied`, not the headline: `--to` defaults to the highest major this +> build carries a step for, which is one PAST the installed major for most of a +> release line, so `todos` carries the next major's semantic residue — real, but +> not this upgrade's business. +> > Run both. A report that cites only `validate` cannot see this class at all. ### 3.4 The report — the human half @@ -452,7 +457,7 @@ maintainer can read in five minutes and a year from now. Write **Status:** complete | complete with N open decisions **Spec:** · **Chain:** 16 → 17 -**Verified:** `os validate` green · `tsc --noEmit` green · replay-from-17 applies 0 changes +**Verified:** `os validate` green · `tsc --noEmit` green · replay-from-17 applies 0 mechanical changes ## 1 · Mechanical (applied by the chain)