diff --git a/.changeset/24833-dev-recover-line-names-no-origin.md b/.changeset/24833-dev-recover-line-names-no-origin.md new file mode 100644 index 00000000000..603f8bd799d --- /dev/null +++ b/.changeset/24833-dev-recover-line-names-no-origin.md @@ -0,0 +1,11 @@ +--- +'@objectstack/cli': patch +--- + +fix(cli): `os dev`'s in-place upgrade line no longer says every refused database is "still in the v17 organization-ownership shape" + +Clause-②: no (printed text of `os dev` only: no exit code, refusal, recovered boot or IPC message moves) + +When the organization-ownership boot guard refuses the server `os dev` starts, `os dev` copies the development database and upgrades it in place. The line it printed first said the database was "still in the v17 organization-ownership shape", for every refusal. That was wrong for a database an 18.0 pre-release created, and for every refusal that is not about the database's origin at all: a marker that is unverified, unreadable or from an older shape generation, or a read or write the guard could not make. + +The line now names no origin. It points at the guard's own refusal, printed just above it, which says what the guard found, and it says that `os dev` upgrades the database in place, keeping a copy first. The verdict line printed after the upgrade still gives the copy's path. Which databases `os dev` upgrades, how it upgrades them, and what a bare `os serve` prints are unchanged. diff --git a/packages/cli/src/commands/dev-organization-ownership-recover.test.ts b/packages/cli/src/commands/dev-organization-ownership-recover.test.ts new file mode 100644 index 00000000000..23f14c21714 --- /dev/null +++ b/packages/cli/src/commands/dev-organization-ownership-recover.test.ts @@ -0,0 +1,96 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect } from 'vitest'; +import { stripVTControlCharacters } from 'node:util'; +import { upgradeRefusedDevelopmentDatabase } from './dev.js'; +import type { DevelopmentUpgradeResult } from '../utils/dev-organization-ownership-upgrade.js'; + +/** + * #24833 — what `os dev` prints when the organization-ownership boot guard + * refused its `serve` child and it upgrades the development database in place. + * + * The coordinator's `recover` in `dev.ts` calls `upgradeRefusedDevelopmentDatabase` + * with `console.log` and the real upgrade. Here the upgrade is faked: what it + * does is `utils/dev-organization-ownership-upgrade.test.ts`'s subject, and the + * line it prints is this file's. + * + * ## The defect + * + * The first line used to say the database was "still in the v17 + * organization-ownership shape" for every refusal. The guard refuses a database + * whose marker is absent, unverified, unreadable or from an older shape + * generation, or whose shape or birth marker it could not read or write, and + * only the absent marker means "17.x or an 18.0 pre-release created it". So the + * line names no origin at all: the guard's own refusal, printed just above it + * by the failed boot, does. + * + * ⛔ The sentence is not pinned verbatim. The pins are what it must not say (an + * origin) and the things it must name: the refusal above, the in-place + * upgrade, the copy. The copy's path is the verdict line's, from the result. + */ + +const COPY = '.objectstack/data/backups/organization-ownership-2026-10-20T15-33-45-773Z'; + +/** Drive the seam with a fake upgrade, recording every printed line (colour stripped) and when the upgrade ran. */ +async function drive(result: DevelopmentUpgradeResult): Promise<{ lines: string[]; printedBeforeUpgrade: number; ok: boolean }> { + const lines: string[] = []; + let printedBeforeUpgrade = -1; + const ok = await upgradeRefusedDevelopmentDatabase({ + print: (line) => lines.push(stripVTControlCharacters(line)), + upgrade: async () => { + printedBeforeUpgrade = lines.length; + return result; + }, + }); + return { lines, printedBeforeUpgrade, ok }; +} + +const upgraded: DevelopmentUpgradeResult = { + ok: true, + backupDir: COPY, + warnings: ['8 shipped permission set row(s) …'], + lines: [`Upgraded the development database to the v18 organization ownership. The database as it was before the upgrade is copied to ${COPY}, beside the ceremony's log. The plan and the report are there too.`], +}; + +describe('#24833 os dev — the line before the in-place upgrade of a refused development database', () => { + it('names no origin for the database: no "v17", no 17.x, no 18.0 pre-release', async () => { + const { lines } = await drive(upgraded); + const notice = lines[0]!; + expect(notice, notice).not.toMatch(/v17/i); + expect(notice, notice).not.toMatch(/\b17\.x\b|\b18\.0\b|pre-?release|next\.\*|created/i); + }); + + it('points at the refusal above, and says os dev upgrades the database in place keeping a copy first', async () => { + const { lines, printedBeforeUpgrade } = await drive(upgraded); + const notice = lines[0]!; + expect(notice, notice).toMatch(/refus/i); + expect(notice, notice).toMatch(/above/i); + expect(notice, notice).toMatch(/os dev/); + expect(notice, notice).toMatch(/in place/); + expect(notice, notice).toMatch(/copy first/); + // Printed BEFORE the copy and the ceremony run, which take seconds. + expect(printedBeforeUpgrade).toBe(1); + }); + + it('the copy\'s path stays on the verdict line, after the warnings, and the server boots again', async () => { + const { lines, ok } = await drive(upgraded); + expect(ok).toBe(true); + expect(lines.slice(1)).toEqual([ + ' ⚠ 8 shipped permission set row(s) …', + ` ✓ ${upgraded.lines[0]}`, + ]); + expect(lines[2]).toContain(COPY); + }); + + it('an upgrade that did not finish prints its refusal lines and boots nothing', async () => { + const { lines, ok } = await drive({ + ok: false, + backupDir: COPY, + warnings: [], + lines: ['Refused (sys_mystery): …', `The upgrade plan was refused, and nothing was changed. The copy is at ${COPY}.`], + }); + expect(ok).toBe(false); + expect(lines.slice(1).every((l) => l.startsWith(' ✗ '))).toBe(true); + expect(lines).toHaveLength(3); + }); +}); diff --git a/packages/cli/src/commands/dev.ts b/packages/cli/src/commands/dev.ts index 17fc74e9889..3485d18cd59 100644 --- a/packages/cli/src/commands/dev.ts +++ b/packages/cli/src/commands/dev.ts @@ -16,7 +16,11 @@ import { assessArtifactStaleness, formatMtimeGap, } from '../utils/dev-restart.js'; -import { isOrganizationOwnershipRequiredMessage, upgradeDevelopmentDatabase } from '../utils/dev-organization-ownership-upgrade.js'; +import { + isOrganizationOwnershipRequiredMessage, + upgradeDevelopmentDatabase, + type DevelopmentUpgradeResult, +} from '../utils/dev-organization-ownership-upgrade.js'; import { childEnvWithResolvedArtifact } from '../utils/internal-artifact-channel.js'; import { makeDirectorySync } from '../utils/make-directory.js'; // THE artifact precedence, written once (#21501) — shared with `start`, and @@ -247,6 +251,42 @@ export function devWatchActive(opts: { watch: boolean; artifact?: string; config return opts.watch && !opts.artifact && opts.configExists; } +/** + * What `os dev` prints when the organization-ownership boot guard refused its + * `serve` child, and the upgrade it then runs (ADR-0131 D10, development + * exception; `utils/dev-organization-ownership-upgrade.ts`). The coordinator's + * `recover` calls this once per session; `upgrade` copies the development + * database, runs the ceremony on it and answers its verdict. + * + * ⛔ The first line names no origin for the database. Every refusal the guard + * throws reaches this path — a marker absent, unverified, unreadable or from an + * older shape generation, a shape read that failed, a birth write that failed — + * and only the absent marker means "17.x or an 18.0 pre-release created it". The + * guard's own refusal, printed just above this line by the failed boot, already + * names what it found, so this line points at it rather than carrying a second + * copy of that wording, which would be wrong for the other five. + * + * The copy's path is printed by the verdict lines below, from the upgrade's own + * result. + * + * @returns whether the upgrade finished and the server may boot again. + */ +export async function upgradeRefusedDevelopmentDatabase(io: { + /** Writes one line for the developer. */ + print: (line: string) => void; + /** Copies the development database, then plans and applies the ceremony on it. */ + upgrade: () => Promise; +}): Promise { + io.print(chalk.yellow( + ' ⟳ The organization-ownership boot guard refused this development database (the refusal above), so ' + + 'os dev upgrades it in place, keeping a copy first…', + )); + const result = await io.upgrade(); + for (const warning of result.warnings) io.print(chalk.yellow(` ⚠ ${warning}`)); + for (const line of result.lines) io.print(result.ok ? chalk.green(` ✓ ${line}`) : chalk.red(` ✗ ${line}`)); + return result.ok; +} + export default class Dev extends Command { static override description = 'Start development mode — watch sources, rebuild the artifact, and restart the server on change'; @@ -725,11 +765,12 @@ export default class Dev extends Command { // load is the one path that applies the whole artifact, so the // coordinator restarts the serve child after each successful rebuild // (nodemon-style, default-on; opt out with --no-restart). - // [ADR-0131 D10, development exception] A serve child refused for the - // v17 organization-ownership shape says so before it exits; the + // [ADR-0131 D10, development exception] A serve child the + // organization-ownership boot guard refused says so before it exits; the // coordinator then asks `recover` below, which copies the development // database, runs the ceremony on it and boots the server again — once - // per session (`utils/dev-organization-ownership-upgrade.ts`). + // per session (`utils/dev-organization-ownership-upgrade.ts`, + // `upgradeRefusedDevelopmentDatabase` above for what it prints). let ownershipUpgrade: 'none' | 'pending' | 'attempted' = 'none'; let upgradedBoot = false; const spawnServeChild = (info: { restartIndex: number }) => { @@ -818,28 +859,27 @@ export default class Dev extends Command { recover: async () => { if (ownershipUpgrade !== 'pending') return false; ownershipUpgrade = 'attempted'; - console.log(chalk.yellow( - ' ⟳ This development database is still in the v17 organization-ownership shape — upgrading it in place ' - + '(a copy is kept first)…', - )); - const { resolveStandaloneDatabase } = await import('@objectstack/runtime'); - let sqliteFile: string | null = null; - try { - sqliteFile = resolveStandaloneDatabase({ projectRoot: process.cwd(), databaseUrl: effectiveDb }).sqliteFile; - } catch { - sqliteFile = null; - } - const result = upgradeDevelopmentDatabase({ - binPath, - databaseUrl: effectiveDb, - sqliteFile, - env: localEnv, - cwd: process.cwd(), + const ok = await upgradeRefusedDevelopmentDatabase({ + print: (line) => console.log(line), + upgrade: async () => { + const { resolveStandaloneDatabase } = await import('@objectstack/runtime'); + let sqliteFile: string | null = null; + try { + sqliteFile = resolveStandaloneDatabase({ projectRoot: process.cwd(), databaseUrl: effectiveDb }).sqliteFile; + } catch { + sqliteFile = null; + } + return upgradeDevelopmentDatabase({ + binPath, + databaseUrl: effectiveDb, + sqliteFile, + env: localEnv, + cwd: process.cwd(), + }); + }, }); - for (const warning of result.warnings) console.log(chalk.yellow(` ⚠ ${warning}`)); - for (const line of result.lines) console.log(result.ok ? chalk.green(` ✓ ${line}`) : chalk.red(` ✗ ${line}`)); - if (result.ok) upgradedBoot = true; - return result.ok; + if (ok) upgradedBoot = true; + return ok; }, log: (line) => { const trimmed = line.trimStart();