diff --git a/.changeset/23899-read-only-arms-four-more.md b/.changeset/23899-read-only-arms-four-more.md new file mode 100644 index 00000000000..85c3640c592 --- /dev/null +++ b/.changeset/23899-read-only-arms-four-more.md @@ -0,0 +1,20 @@ +--- +'@objectstack/cli': patch +'@objectstack/objectql': patch +--- + +Clause-②: no + +fix(cli): `os migrate plan`, a refused `os migrate summary-nulls --recompute-undefined-on-empty` name and every `os secret rewrap` run say what they vouch for, and that opening a SQLite file can still switch its journal mode, instead of saying nothing was written + +**What was wrong.** Each of these runs boots through the SQL driver before it prints, and the driver's connect switches a SQLite file to the journal mode it asks for (WAL, unless `OS_DATABASE_SQLITE_JOURNAL_MODE=delete` is set in the command's own environment or the project's `.env`). Measured on a file still on a rollback journal, each run moved the header's journal-mode bytes (18 and 19, from 1,1 to 2,2) and its change counter, and changed no row or table, while printing "a plan writes nothing", "nothing was written" or "a finished run writes nothing". Nothing was lost. + +**What changed.** Each sentence below now names what it vouches for. On a SQLite file, its line is followed by the note the other read-only arms already carry: "Opening a SQLite file can still switch its journal mode (to WAL by default), which changes its header only." On PostgreSQL or MySQL the line ends without it. + +- `os migrate plan`, the unmanaged-table section. FROM: "They are reported for information only — nothing here drops them, and a plan writes nothing." TO: "… and a plan writes no row or table." and the note. +- `os migrate plan`, the declaration boot's note when a plugin's write was refused. FROM: "Refused N write(s) during the declaration boot — a plan writes nothing: …" TO: "… — a plan writes no row or table: …", and the note at the end of that note's line. +- `os migrate plan`, the same note when a raw command was forwarded. FROM: "If those commands only read, nothing was written; if one wrote, it landed." TO: "If those commands only read, no row or table was written; if one wrote, it landed.", and the note at the end of that note's line. +- `os migrate summary-nulls --recompute-undefined-on-empty` naming a roll-up the run cannot find (`backfillSummaryNulls`, `@objectstack/objectql`), in either mode. FROM: "Refused before any row was read; nothing was written." TO: "Refused before the walk read or wrote any row." and the note. Under `--apply` the boot before it can also create a table a never-started database lacks, so the sentence speaks for the walk only. +- `os secret rewrap`, the "Re-running is safe" note, in both modes and in `--json` `report.notes`. FROM: "… and a finished run writes nothing." TO: "… and a finished run writes no sys_secret row." Under `--apply` the note follows it; a dry run's closing line already carries it. A finished `--apply` run on a database that has never been started also creates its tables, so the sentence names the rows the re-wrap writes. + +**Unchanged.** No exit code, flag, prompt, refusal or export signature changes, and no `--json` document gains a field: the `--json` documents carry the changed sentences, and the note is printed in the text output only. What the driver does on connect is unchanged. diff --git a/content/docs/deployment/cli.mdx b/content/docs/deployment/cli.mdx index 4e2b5821db9..dae16babc62 100644 --- a/content/docs/deployment/cli.mdx +++ b/content/docs/deployment/cli.mdx @@ -649,7 +649,9 @@ What a run guarantees: about its code-declared datasources leaves that family a gap. `--apply` then refuses and names the family. - **Resumable.** A row already sealed under the current binding is skipped as done. A run - that stopped part-way finishes the rest when re-run, and a finished run writes nothing. + that stopped part-way finishes the rest when re-run, and a finished run writes no + `sys_secret` row. On a SQLite file, opening it can still switch its journal mode (to + WAL by default), which changes its header only. - **Safe against a live deployment.** Each row is written in one statement, and only if it still holds the ciphertext the run read. A row a producer changed during the run is not overwritten. It is counted, and a re-run picks it up. diff --git a/packages/cli/src/commands/migrate/plan.journal-mode-note.test.ts b/packages/cli/src/commands/migrate/plan.journal-mode-note.test.ts new file mode 100644 index 00000000000..de5e261cccf --- /dev/null +++ b/packages/cli/src/commands/migrate/plan.journal-mode-note.test.ts @@ -0,0 +1,230 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `os migrate plan` — what its human face says about the database it read. + * + * A plan boots read-only (schema DDL deferred, no file brought into + * existence), but the SQL driver's connect still switches a SQLite file to the + * journal mode it asks for, which rewrites the file header and no row. So no + * line a plan prints says that it wrote nothing: the lines that vouch for the + * database claim no row or table, and on a SQLite target each is followed by + * the note naming the switch. Three texts make that claim: + * + * - the unmanaged-table section's closing line (`renderUnmanagedTables`); + * - the declaration boot's write-guard note when a plugin's contract write + * was refused ("— a plan writes no row or table"); + * - the same note when a raw command was forwarded ("If those commands only + * read, no row or table was written"). + * + * The two guard notes are the REAL guard's text: a `createDeclarationBootWriteGuard` + * is armed over a fake driver, made to refuse one write or forward one raw + * read, and disarmed. Only the boot, the occupancy probe and the unmanaged-table + * READ are replaced; the command's control flow and the section's renderer run + * for real. The `--json` document carries the same lines and no note. + */ + +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { dirname, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { stripVTControlCharacters } from 'node:util'; +import MigratePlan from './plan.js'; +import { bootSchemaStack } from '../../utils/schema-migrate.js'; +import { probeMigrationTarget } from '../../utils/migrate-occupancy-gate.js'; +import { collectUnmanagedTables, type UnmanagedTablesReport } from '../../utils/unmanaged-tables.js'; +import { createDeclarationBootWriteGuard } from '../../utils/schema-migration-plugins.js'; +import { JOURNAL_MODE_SWITCH_NOTE } from '../../utils/sqlite-occupancy.js'; + +vi.mock('../../utils/schema-migrate.js', async (importOriginal) => ({ + ...(await importOriginal()), + bootSchemaStack: vi.fn(), +})); +vi.mock('../../utils/migrate-occupancy-gate.js', () => ({ probeMigrationTarget: vi.fn() })); +// The sweep's READ is replaced; its renderer is the real one. +vi.mock('../../utils/unmanaged-tables.js', async (importOriginal) => ({ + ...(await importOriginal()), + collectUnmanagedTables: vi.fn(), +})); +// `run()` ends the process when the plan does (#13027); here it returns instead. +vi.mock('../../utils/one-shot-exit.js', () => ({ exitOneShotCommand: vi.fn(async () => undefined) })); + +const HERE = dirname(fileURLToPath(import.meta.url)); +const CLI_ROOT = resolve(HERE, '..', '..', '..'); +/** oclif builds its whole command table on the first `run()` in a process. */ +const RUN_TIMEOUT = 60_000; + +const SQLITE_CONFIG = { client: 'better-sqlite3', connection: { filename: '/srv/app/.objectstack/data/objectstack.db' } }; +const POSTGRES_CONFIG = { client: 'pg', connection: 'postgres://db.example.com:5432/app' }; + +const UNMANAGED_LINE = + 'They are reported for information only — nothing here drops them, and a plan writes no row or table.'; +const ONE_UNMANAGED: UnmanagedTablesReport = { + status: 'read', prefixes: ['sys_'], physicalTables: 3, tables: [{ table: 'sys_legacy_widget' }], +}; +const NONE_UNMANAGED: UnmanagedTablesReport = { status: 'read', prefixes: ['sys_'], physicalTables: 3, tables: [] }; + +/** A driver with its members on the PROTOTYPE, as every real driver's are. */ +class FakeDriver { + async create(_object: string, _data: Record): Promise> { return {}; } + async execute(_command: unknown): Promise { return []; } +} + +/** The real guard's note after `act` ran against a driver it armed. */ +async function guardNote(act: (driver: FakeDriver) => Promise): Promise { + const guard = createDeclarationBootWriteGuard(); + const driver = new FakeDriver(); + await (guard.plugin as { init(ctx: unknown): Promise }).init({ + getServices: () => new Map([['driver.default', driver]]), + }); + await act(driver); + const note = guard.disarm(); + expect(note).not.toBeNull(); + return note as string; +} + +let warn: ReturnType; +beforeEach(() => { + // The guard warns once per refused or forwarded call; not under test here. + warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + vi.mocked(probeMigrationTarget).mockResolvedValue({ status: 'free' } as never); +}); +afterEach(() => { + warn.mockRestore(); + vi.mocked(bootSchemaStack).mockReset(); + vi.mocked(collectUnmanagedTables).mockReset(); +}); + +/** Boot onto a primary driver with `config`, composing `notes`. */ +function wireBoot(config: Record, notes: string[], unmanaged: UnmanagedTablesReport): void { + vi.mocked(bootSchemaStack).mockResolvedValue({ + // The primary datasource's driver, as `SchemaStack.driver` carries it. + driver: { config }, + drivers: [], + dbLabel: 'test database', + dbSource: { kind: 'flag' }, + schemaSync: 'deferred', + telemetryDatabase: null, + managedTableCount: 3, + pendingSchemaWork: [], + detectManagedDrift: async () => [], + allObjects: () => [], + composition: { + plugins: [], + hostConfigPath: '/srv/app/objectstack.config.ts', + hostConfigLoaded: true, + hostConfigError: null, + servedBoot: { mirrored: false, reason: 'not-composed' }, + notes, + coverage: null, + }, + shutdown: vi.fn(async () => {}), + } as never); + vi.mocked(collectUnmanagedTables).mockResolvedValue(unmanaged); +} + +/** One human-mode plan; every line `console.log` received, colour stripped. */ +async function runHuman(): Promise { + const log = vi.spyOn(console, 'log').mockImplementation(() => {}); + try { + await MigratePlan.run([], { root: CLI_ROOT }); + return log.mock.calls + .map((c: unknown[]) => stripVTControlCharacters(c.map(String).join(' '))) + .join('\n') + .split('\n'); + } finally { + log.mockRestore(); + } +} + +/** One `--json` plan; the document it emitted. */ +async function runJson(): Promise> { + const chunks: string[] = []; + // `emitJson` awaits the write's DRAIN callback, so the double invokes it. + const stdout = vi.spyOn(process.stdout, 'write').mockImplementation(((chunk: unknown, ...rest: unknown[]) => { + chunks.push(String(chunk)); + const done = rest.find((a) => typeof a === 'function') as ((e?: Error | null) => void) | undefined; + done?.(null); + return true; + }) as typeof process.stdout.write); + try { + await MigratePlan.run(['--json'], { root: CLI_ROOT }); + } finally { + stdout.mockRestore(); + } + return JSON.parse(chunks.join('')) as Record; +} + +interface Text { + name: string; + /** The composed notes and the sweep's report this case boots with. */ + arrange: () => Promise<{ notes: string[]; unmanaged: UnmanagedTablesReport }>; + /** The printed line that carries the claim, without its print indent. */ + line: (notes: string[]) => string; + /** The claim as the text spells it. */ + claim: string; +} + +const TEXTS: Text[] = [ + { + name: 'the unmanaged-table section\'s closing line', + arrange: async () => ({ notes: [], unmanaged: ONE_UNMANAGED }), + line: () => UNMANAGED_LINE, + claim: 'a plan writes no row or table', + }, + { + name: 'the write guard\'s note on a refused contract write', + arrange: async () => ({ + notes: [await guardNote((d) => d.create('sys_metadata', { id: 'probe' }))], + unmanaged: NONE_UNMANAGED, + }), + line: (notes) => notes[0]!, + claim: 'Refused 1 write(s) during the declaration boot — a plan writes no row or table: create() on sys_metadata.', + }, + { + name: 'the write guard\'s note on a forwarded raw command', + arrange: async () => ({ + notes: [await guardNote((d) => d.execute('SELECT count(*) FROM sys_metadata'))], + unmanaged: NONE_UNMANAGED, + }), + line: (notes) => notes[0]!, + claim: 'If those commands only read, no row or table was written; if one wrote, it landed.', + }, +]; + +describe('os migrate plan — the human face names no write it cannot vouch for', () => { + for (const text of TEXTS) { + it(`${text.name}: on a SQLite file it claims no row or table and is followed by the journal-mode note`, async () => { + const { notes, unmanaged } = await text.arrange(); + const expected = text.line(notes); + expect(expected).toContain(text.claim); + expect(expected).not.toContain('nothing was written'); + expect(expected).not.toContain('writes nothing'); + + wireBoot(SQLITE_CONFIG, notes, unmanaged); + const lines = await runHuman(); + const printed = lines.filter((l) => l.includes(text.claim)); + expect(printed, lines.join('\n')).toEqual([` ${expected} ${JOURNAL_MODE_SWITCH_NOTE}`]); + // Once, after the claim — no other line of this plan carries it. + expect(lines.join('\n').split(JOURNAL_MODE_SWITCH_NOTE)).toHaveLength(2); + }, RUN_TIMEOUT); + + it(`${text.name}: on a target with no file header (PostgreSQL) it claims no row or table and names no switch`, async () => { + const { notes, unmanaged } = await text.arrange(); + const expected = text.line(notes); + + wireBoot(POSTGRES_CONFIG, notes, unmanaged); + const lines = await runHuman(); + const printed = lines.filter((l) => l.includes(text.claim)); + expect(printed, lines.join('\n')).toEqual([` ${expected}`]); + expect(lines.join('\n')).not.toContain(JOURNAL_MODE_SWITCH_NOTE); + }, RUN_TIMEOUT); + } + + it('--json carries the guard\'s note as it is, with no journal-mode note, on a SQLite file', async () => { + const note = await guardNote((d) => d.create('sys_metadata', { id: 'probe' })); + wireBoot(SQLITE_CONFIG, [note], ONE_UNMANAGED); + const payload = await runJson(); + expect(payload.composition.notes).toEqual([note]); + expect(payload.unmanagedTables).toEqual(ONE_UNMANAGED); + expect(JSON.stringify(payload)).not.toContain(JOURNAL_MODE_SWITCH_NOTE); + }, RUN_TIMEOUT); +}); diff --git a/packages/cli/src/commands/migrate/plan.ts b/packages/cli/src/commands/migrate/plan.ts index acfce3db076..5b084c91fd4 100644 --- a/packages/cli/src/commands/migrate/plan.ts +++ b/packages/cli/src/commands/migrate/plan.ts @@ -35,6 +35,7 @@ import { type UnmanagedTablesReport, } from '../../utils/unmanaged-tables.js'; import { describeOccupancy, JOURNAL_MODE_SWITCH_NOTE } from '../../utils/sqlite-occupancy.js'; +import { describeDriverSqliteFile } from '../../utils/connection-display.js'; import { resolveTenancyPosture, collectGlobalUniques, @@ -311,6 +312,22 @@ export default class MigratePlan extends Command { console.log(''); } + // [#23899] Two kinds of line below claim that this plan wrote no row or + // table: the declaration boot's write-guard note (one of the composition + // notes, when a plugin's write was refused or a raw command forwarded) + // and the unmanaged-table section's closing line. The read-only boot's + // connect may still have switched a SQLite file's journal mode + // (`SqlDriver.applySqliteJournalMode`, its rule for every connect), which + // rewrites the file header and no row, so on a SQLite file each line that + // makes the claim is followed by the note naming the switch. A target + // that is no SQLite file (PostgreSQL, MySQL) has no header to switch. The + // `--json` document above carries the same lines and no note. + const journalNote = describeDriverSqliteFile(stack.driver?.config) !== undefined + ? ` ${JOURNAL_MODE_SWITCH_NOTE}` + : ''; + const withJournalNote = (line: string): string => + (line.includes('no row or table') ? `${line}${journalNote}` : line); + printInfo(`Database: ${chalk.white(stack.dbLabel)} ${chalk.dim(`(${describeDatabaseSource(stack.dbSource)})`)}`); if (stack.telemetryDatabase !== null) printInfo(describeTelemetryDatabase(stack.telemetryDatabase)); if (schemaSyncSkipped) printWarning(SKIPPED_SCHEMA_SYNC_NOTE); @@ -318,7 +335,7 @@ export default class MigratePlan extends Command { // What the object set was composed from (#12938) — never silent about a // host config it could not load, and empty (so this block prints nothing) // when there was no deployment to compose. - for (const note of stack.composition.notes) console.log(chalk.dim(` ${note}`)); + for (const note of stack.composition.notes) console.log(chalk.dim(` ${withJournalNote(note)}`)); console.log(''); // [#13204] Its own block, above the drift verdict and never folded into @@ -329,7 +346,7 @@ export default class MigratePlan extends Command { const unmanagedLines = renderUnmanagedTables(unmanagedTables); if (unmanagedLines.length > 0) { printInfo(unmanagedLines[0]!); - for (const line of unmanagedLines.slice(1)) console.log(chalk.dim(` ${line}`)); + for (const line of unmanagedLines.slice(1)) console.log(chalk.dim(` ${withJournalNote(line)}`)); console.log(''); } diff --git a/packages/cli/src/commands/migrate/summary-nulls.test.ts b/packages/cli/src/commands/migrate/summary-nulls.test.ts index a94fdb8b4cd..8f7d970fd40 100644 --- a/packages/cli/src/commands/migrate/summary-nulls.test.ts +++ b/packages/cli/src/commands/migrate/summary-nulls.test.ts @@ -17,12 +17,16 @@ * boot can still switch a SQLite file's journal mode, which rewrites its * header and no row, so the report's dry-run line claims no row or table and, * on a SQLite target only, is followed by the note naming the switch. The - * report's formatter is the real one from `@objectstack/objectql`. + * report's formatter is the real one from `@objectstack/objectql`. A refused + * `--recompute-undefined-on-empty` name, thrown after the boot in either mode, + * vouches for the walk only and is followed by the same note on a SQLite + * target; its refusal is the engine's real one. */ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import { dirname, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; +import { stripVTControlCharacters } from 'node:util'; import MigrateSummaryNulls from './summary-nulls.js'; import { bootSchemaStack } from '../../utils/schema-migrate.js'; import { probeMigrationTarget } from '../../utils/migrate-occupancy-gate.js'; @@ -216,3 +220,74 @@ describe('os migrate summary-nulls — the dry run names no write it cannot vouc expect(lines.join('\n')).not.toContain(JOURNAL_MODE_SWITCH_NOTE); }, RUN_TIMEOUT); }); + +// ─────────────────────────────────────────────────────────────────────────── +// The human face: what a refused --recompute-undefined-on-empty name says +// ─────────────────────────────────────────────────────────────────────────── + +describe('os migrate summary-nulls — a refused scope names no write it cannot vouch for', () => { + /** The engine's own refusal of `customer.nope`, from the real `backfillSummaryNulls`. */ + async function realRefusal(): Promise { + const real = await vi.importActual('@objectstack/objectql'); + const quiet = { info: () => {}, warn: () => {} }; + return real.backfillSummaryNulls(engine as any, quiet, { recomputeUndefinedOnEmpty: ['customer.nope'] }) + .then(() => { throw new Error('expected the scope to be refused'); }, (e: unknown) => e as Error & { code: string }); + } + + /** Boot onto a primary driver with `config`, in either mode; every line the run printed, colour stripped. */ + async function runRefused(config: Record, argv: string[]): Promise<{ lines: string[]; message: string }> { + const refusal = await realRefusal(); + expect(refusal.code).toBe('INVALID_FIELD'); + vi.mocked(bootSchemaStack).mockResolvedValue({ + driver: { config }, + kernel: { getService: () => engine }, + dbLabel: 'test database', + shutdown: vi.fn(async () => {}), + } as any); + vi.mocked(backfillSummaryNulls).mockRejectedValue(refusal); + const err = await MigrateSummaryNulls.run( + [...argv, '--recompute-undefined-on-empty', 'customer.nope'], + { root: CLI_ROOT }, + ).catch((e: unknown) => e); + expect((err as { oclif?: { exit?: number } }).oclif?.exit).toBe(1); + const lines = log.mock.calls + .map((c: unknown[]) => stripVTControlCharacters(c.map(String).join(' '))) + .join('\n') + .split('\n'); + return { lines, message: refusal.message }; + } + + for (const [mode, argv] of [['dry run', []], ['--apply', ['--apply', '--yes']]] as const) { + it(`${mode}: on a SQLite file the refusal vouches for the walk and is followed by the journal-mode note`, async () => { + const { lines, message } = await runRefused(SQLITE_CONFIG, [...argv]); + expect(message).toMatch(/ Refused before the walk read or wrote any row\.$/); + const refused = lines.filter((line) => line.includes('this run cannot find')); + expect(refused, lines.join('\n')).toEqual([` ✗ ${message} ${JOURNAL_MODE_SWITCH_NOTE}`]); + expect(lines.join('\n')).not.toContain('nothing was written'); + }, RUN_TIMEOUT); + + it(`${mode}: on a target with no file header (PostgreSQL) the refusal names no switch`, async () => { + const { lines, message } = await runRefused(POSTGRES_CONFIG, [...argv]); + const refused = lines.filter((line) => line.includes('this run cannot find')); + expect(refused, lines.join('\n')).toEqual([` ✗ ${message}`]); + expect(lines.join('\n')).not.toContain(JOURNAL_MODE_SWITCH_NOTE); + }, RUN_TIMEOUT); + } + + it('--json: the envelope carries the engine\'s message as it is, with no journal-mode note, on a SQLite file', async () => { + const refusal = await realRefusal(); + vi.mocked(bootSchemaStack).mockResolvedValue({ + driver: { config: SQLITE_CONFIG }, + kernel: { getService: () => engine }, + dbLabel: 'test database', + shutdown: vi.fn(async () => {}), + } as any); + vi.mocked(backfillSummaryNulls).mockRejectedValue(refusal); + await MigrateSummaryNulls.run( + ['--json', '--recompute-undefined-on-empty', 'customer.nope'], + { root: CLI_ROOT }, + ).catch((e: unknown) => e); + const payload = JSON.parse(stdout.mock.calls.map((c: unknown[]) => String(c[0])).join('')); + expect(payload).toMatchObject({ code: 'INVALID_FIELD', httpStatus: 400, error: refusal.message }); + }, RUN_TIMEOUT); +}); diff --git a/packages/cli/src/commands/migrate/summary-nulls.ts b/packages/cli/src/commands/migrate/summary-nulls.ts index c51055eef27..728da33836b 100644 --- a/packages/cli/src/commands/migrate/summary-nulls.ts +++ b/packages/cli/src/commands/migrate/summary-nulls.ts @@ -290,7 +290,18 @@ export default class MigrateSummaryNulls extends Command { } catch (error: any) { if (isExitSignal(error)) throw error; if (flags.json) { await emitJson({ error: error.message, ...errorCodeFields(error) }, 0, { compact: true }); this.exit(1); } - printError(error.message || String(error)); + // [#23899] `INVALID_FIELD` here is `backfillSummaryNulls` refusing a + // `--recompute-undefined-on-empty` name it cannot find (its one throw), + // which ends "Refused before the walk read or wrote any row." The boot + // above ran first in either mode, and its connect may have switched a + // SQLite file's journal mode (its header, no row), so on a SQLite file + // the refusal is followed by the note naming the switch. The `--json` + // envelope above carries the engine's message as it is. + const journalNote = error?.code === 'INVALID_FIELD' + && describeDriverSqliteFile(stack.driver?.config) !== undefined + ? ` ${JOURNAL_MODE_SWITCH_NOTE}` + : ''; + printError(`${error.message || String(error)}${journalNote}`); this.exit(1); } finally { await stack.shutdown(); diff --git a/packages/cli/src/commands/secret/rewrap.guards.test.ts b/packages/cli/src/commands/secret/rewrap.guards.test.ts index 258bfb4f9fb..6061b32de60 100644 --- a/packages/cli/src/commands/secret/rewrap.guards.test.ts +++ b/packages/cli/src/commands/secret/rewrap.guards.test.ts @@ -34,6 +34,7 @@ import { existsSync, mkdtempSync, readdirSync, rmSync, writeFileSync } from 'nod import { tmpdir } from 'node:os'; import { dirname, join, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; +import { stripVTControlCharacters } from 'node:util'; import { ciphertextDerivationStatus } from '@objectstack/service-settings'; import SecretRewrap from './rewrap.js'; import { bootSchemaStack } from '../../utils/schema-migrate.js'; @@ -315,6 +316,14 @@ describe('os secret rewrap — guards that stop a run before any row is opened o const SQLITE_CONFIG = { client: 'better-sqlite3', connection: { filename: '/srv/app/.objectstack/data/objectstack.db' } }; const POSTGRES_CONFIG = { client: 'pg', connection: 'postgres://db.example.com:5432/app' }; +/** + * Printed by every run, both modes. A finished run writes no sys_secret row; it + * is never said to write nothing, since either mode's connect can switch a + * SQLite file's journal mode and an --apply run's plain boot creates the tables + * a never-started database lacks. + */ +const RESUMABLE_NOTE = 'Re-running is safe. A row already sealed under the current derivation is skipped as done, so a ' + + 'run that stopped part-way finishes the rest, and a finished run writes no sys_secret row.'; /** One human-mode run; every line `console.log` received. */ async function runHuman(argv: string[]): Promise { @@ -383,6 +392,40 @@ describe('os secret rewrap — the human face names no write it cannot vouch for expect(aborted, lines.join('\n')).toContain(`Aborted — no row was opened or written. ${JOURNAL_MODE_SWITCH_NOTE}`); }, 60_000); + it('an --apply run\'s "Re-running is safe" note claims no sys_secret row and names the switch on a SQLite file', async () => { + wireBoot(freshRows(), { driverConfig: SQLITE_CONFIG }); + const lines = (await runHuman(['--apply', '--yes', '--no-declared-datasources'])).map(stripVTControlCharacters); + const resumable = lines.filter((line) => line.includes('Re-running is safe.')); + expect(resumable, lines.join('\n')).toEqual([` ⚠ ${RESUMABLE_NOTE} ${JOURNAL_MODE_SWITCH_NOTE}`]); + expect(lines.join('\n')).not.toContain('writes nothing'); + }, 60_000); + + it('on a target with no file header (PostgreSQL) an --apply run\'s "Re-running is safe" note names no switch', async () => { + wireBoot(freshRows(), { driverConfig: POSTGRES_CONFIG }); + const lines = (await runHuman(['--apply', '--yes', '--no-declared-datasources'])).map(stripVTControlCharacters); + const resumable = lines.filter((line) => line.includes('Re-running is safe.')); + expect(resumable, lines.join('\n')).toEqual([` ⚠ ${RESUMABLE_NOTE}`]); + expect(lines.join('\n')).not.toContain(JOURNAL_MODE_SWITCH_NOTE); + }, 60_000); + + it('a dry run\'s "Re-running is safe" note carries no switch of its own: its closing line names it once', async () => { + wireBoot(freshRows(), { driverConfig: SQLITE_CONFIG }); + const lines = (await runHuman(['--no-declared-datasources'])).map(stripVTControlCharacters); + const resumable = lines.filter((line) => line.includes('Re-running is safe.')); + expect(resumable, lines.join('\n')).toEqual([` ⚠ ${RESUMABLE_NOTE}`]); + expect(lines.join('\n').split(JOURNAL_MODE_SWITCH_NOTE)).toHaveLength(2); + }, 60_000); + + it('--json report.notes carries the "Re-running is safe" note as it is, in both modes, with no switch', async () => { + for (const argv of [['--no-declared-datasources'], ['--apply', '--yes', '--no-declared-datasources']]) { + wireBoot(freshRows(), { driverConfig: SQLITE_CONFIG }); + const { payload } = await run(argv); + expect(payload.report.notes, argv.join(' ')).toContain(RESUMABLE_NOTE); + expect(JSON.stringify(payload.report.notes), argv.join(' ')).not.toContain('writes nothing'); + expect(JSON.stringify(payload.report.notes), argv.join(' ')).not.toContain(JOURNAL_MODE_SWITCH_NOTE); + } + }, 60_000); + it('the help says the dry run writes no row or table, never that it writes nothing', () => { expect(SecretRewrap.description).toContain('A dry run by default: it writes no row or table without --apply.'); expect(SecretRewrap.flags.apply.description).toContain('(default is a dry run that writes no row or table)'); diff --git a/packages/cli/src/commands/secret/rewrap.ts b/packages/cli/src/commands/secret/rewrap.ts index d296dfac2a1..ca921558937 100644 --- a/packages/cli/src/commands/secret/rewrap.ts +++ b/packages/cli/src/commands/secret/rewrap.ts @@ -143,6 +143,7 @@ export default class SecretRewrap extends Command { buildRewrapReport, executeSysSecretRewrap, planSysSecretRewrap, + REWRAP_NOTES, rewrapUnfinished, } = await import('../../utils/sys-secret-rewrap.js'); const { ciphertextDerivationStatus } = await import('@objectstack/service-settings'); @@ -197,6 +198,12 @@ export default class SecretRewrap extends Command { const journalNote = describeDriverSqliteFile(stack.driver?.config) !== undefined ? ` ${JOURNAL_MODE_SWITCH_NOTE}` : ''; + // [#23899] The report's "Re-running is safe" note claims that a finished + // run writes no sys_secret row. Under `--apply` no other line names the + // switch, so on a SQLite file that note is followed by it; a dry run's + // closing line already carries it, so there the note carries none. The + // `--json` `report.notes` carry the notes as they are. + const noteSuffix = new Map([[REWRAP_NOTES.resumable, flags.apply ? journalNote : '']]); const engine = stack.kernel.getService('objectql') as SecretReferenceEngineLike | undefined; if (!engine) { const message = 'No ObjectQL engine on this runtime, so no holder family can be enumerated.'; @@ -264,7 +271,7 @@ export default class SecretRewrap extends Command { keySource: null, }); if (json) { await emitJson({ error: 'union_incomplete', refused: plan.refusal, report }, 1, { compact: true }); return; } - renderReport(report); + renderReport(report, noteSuffix); printError(plan.refusal.message); for (const gap of plan.refusal.gaps) printError(` family '${gap.family}': ${gap.reason}`); this.exit(1); @@ -322,7 +329,7 @@ export default class SecretRewrap extends Command { const exitCode = flags.apply && rewrapUnfinished(result) ? 1 : 0; if (json) { await emitJson({ mode, report }, exitCode, { compact: true }); return; } - renderReport(report); + renderReport(report, noteSuffix); if (flags.apply) { printSuccess(`Re-wrapped ${report.counts.rewrap} sys_secret row(s) in ${timer.display()}.`); } else { @@ -356,8 +363,11 @@ async function confirm(question: string): Promise { } } -/** Render the report for a human. Classes and counts only. */ -function renderReport(report: SysSecretRewrapReport): void { +/** + * Render the report for a human. Classes and counts only. `noteSuffix` maps a + * note's text to what follows it on its line (the journal-mode note, #23899). + */ +function renderReport(report: SysSecretRewrapReport, noteSuffix: ReadonlyMap): void { console.log(chalk.bold('\n Reference union — one line per holder family')); for (const [family, status] of Object.entries(report.families)) { if (status.status === 'enumerated') { @@ -394,5 +404,5 @@ function renderReport(report: SysSecretRewrapReport): void { if (report.keySource) printInfo(`data key source: ${report.keySource}`); console.log(chalk.bold('\n Read before acting')); - for (const note of report.notes) printWarning(note); + for (const note of report.notes) printWarning(`${note}${noteSuffix.get(note) ?? ''}`); } diff --git a/packages/cli/src/utils/schema-migrate.deferred-ddl.integration.test.ts b/packages/cli/src/utils/schema-migrate.deferred-ddl.integration.test.ts index 398454201a5..0c8da437e03 100644 --- a/packages/cli/src/utils/schema-migrate.deferred-ddl.integration.test.ts +++ b/packages/cli/src/utils/schema-migrate.deferred-ddl.integration.test.ts @@ -226,7 +226,7 @@ describe('bootSchemaStack({ deferSchemaDdl }) — the boot writes nothing (#3917 expect(notes).toContain( `Immediate DDL was called 1 time(s) during the declaration boot (dropTable() on defer_widget via ${via})`, ); - expect(notes).not.toContain('a plan writes nothing'); + expect(notes).not.toContain('a plan writes no row or table'); } finally { await stack.shutdown(); } diff --git a/packages/cli/src/utils/schema-migrate.host-composition.integration.test.ts b/packages/cli/src/utils/schema-migrate.host-composition.integration.test.ts index 76487d91cc6..d007007b58a 100644 --- a/packages/cli/src/utils/schema-migrate.host-composition.integration.test.ts +++ b/packages/cli/src/utils/schema-migrate.host-composition.integration.test.ts @@ -752,7 +752,7 @@ describe('a plan writes nothing even when the host writes from init() (#13332)', // boot, so the outcome claim HELD and is printed with the report. // 4 = the host's `kernel:ready` write + the extra plugin's three. const notes = stack.composition.notes.join(' '); - expect(notes).toContain('Refused 4 write(s) during the declaration boot — a plan writes nothing'); + expect(notes).toContain('Refused 4 write(s) during the declaration boot — a plan writes no row or table'); expect(notes).toContain('create() on sys_metadata'); // …and what was not run is said too. expect(notes).toContain( @@ -827,7 +827,7 @@ describe('a plan writes nothing even when the host writes from init() (#13332)', const notes = stack.composition.notes.join(' '); expect(notes).toContain('Refused 2 write(s) during the declaration boot:'); expect(notes).toContain('Raw execute() was called 1 time(s) during the declaration boot'); - expect(notes).not.toContain('a plan writes nothing'); + expect(notes).not.toContain('a plan writes no row or table'); // The guard's structural surface carries it too, for `--json` consumers. expect(stack.composition.writeGuard?.rawExecutions).toEqual([ diff --git a/packages/cli/src/utils/schema-migration-plugins.declaration-boot-write-guard.test.ts b/packages/cli/src/utils/schema-migration-plugins.declaration-boot-write-guard.test.ts index d43625537e1..625973ed10e 100644 --- a/packages/cli/src/utils/schema-migration-plugins.declaration-boot-write-guard.test.ts +++ b/packages/cli/src/utils/schema-migration-plugins.declaration-boot-write-guard.test.ts @@ -279,7 +279,8 @@ describe('the declaration boot writes nothing (#13332)', () => { // execute() forwarded this run, the outcome claim HELD and is printed. const note = guard.disarm(); expect(note).toContain('Refused 1 write(s)'); - expect(note).toContain('a plan writes nothing'); + expect(note).toContain('a plan writes no row or table'); + expect(note).not.toContain('writes nothing'); expect(note).toContain('create() on sys_ai_model'); await kernel.shutdown(); @@ -325,7 +326,7 @@ describe('the declaration boot writes nothing (#13332)', () => { // Hatch)") — not a driver-sql extension. The guard cannot classify a raw // command as read-vs-write (the contract admits any native shape), so it // must not refuse — but the one unacceptable outcome is the SILENT one: a - // raw write landing while the run claims "a plan writes nothing". This + // raw write landing while the run claims "a plan writes no row or table". This // pins all three properties: forwarded, counted, and the claim dropped. const driver = new RecordingDriver(); const guard = createDeclarationBootWriteGuard(); @@ -362,13 +363,20 @@ describe('the declaration boot writes nothing (#13332)', () => { expect(guard.rawExecutions).toEqual([{ driver: 'driver.recording', count: 1 }]); // REPORTED — and the flat claim is DROPPED: neither the refusal half nor - // any other part of the note may print "a plan writes nothing" over a run + // any other part of the note may print "a plan writes no row or table" over a run // in which a raw command went through unclassified. const note = guard.disarm(); expect(note).toContain('Refused 1 write(s) during the declaration boot:'); expect(note).toContain('Raw execute() was called 1 time(s) during the declaration boot'); expect(note).toContain('1 via driver.recording'); - expect(note).not.toContain('a plan writes nothing'); + expect(note).not.toContain('a plan writes no row or table'); + // What the run CAN say about a raw command, conditionally, and no wider: + // the connect may still have switched a SQLite file's journal mode, which + // rewrites its header, so the conditional names no row or table, never + // "nothing". `os migrate plan` appends the journal-mode note on a SQLite + // file (its own pin). + expect(note).toContain('If those commands only read, no row or table was written; if one wrote, it landed.'); + expect(note).not.toContain('nothing was written'); await kernel.shutdown(); }); @@ -661,7 +669,7 @@ describe('the two named boundaries (#14126)', () => { // claim prints — the same sentence the `driver.*` path prints. const note = guard.disarm(); expect(note).toContain( - 'Refused 1 write(s) during the declaration boot — a plan writes nothing: create() on report_row.', + 'Refused 1 write(s) during the declaration boot — a plan writes no row or table: create() on report_row.', ); // disarm() gave the engine its prototype method back, and the driver its own. @@ -711,7 +719,7 @@ describe('the two named boundaries (#14126)', () => { expect(guard.refusals).toEqual([ { driver: 'engine.archive', method: 'update', object: 'archive_row', count: 1 }, ]); - expect(guard.disarm()).toContain('a plan writes nothing'); + expect(guard.disarm()).toContain('a plan writes no row or table'); await kernel.shutdown(); }); @@ -751,7 +759,7 @@ describe('the two named boundaries (#14126)', () => { { driver: 'driver.recording', method: 'create', object: 'sys_thing', count: 2 }, ]); expect(guard.disarm()).toContain( - 'Refused 2 write(s) during the declaration boot — a plan writes nothing: create() on sys_thing x2.', + 'Refused 2 write(s) during the declaration boot — a plan writes no row or table: create() on sys_thing x2.', ); await kernel.shutdown(); }); @@ -820,7 +828,7 @@ describe('the two named boundaries (#14126)', () => { + '(dropTable() on sys_old_table via driver.recording x2, rotateShards() on sys_log via driver.recording)', ); expect(note).toContain('those calls EXECUTED'); - expect(note).not.toContain('a plan writes nothing'); + expect(note).not.toContain('a plan writes no row or table'); } finally { warn.mockRestore(); await kernel.shutdown(); @@ -856,7 +864,7 @@ describe('the two named boundaries (#14126)', () => { // from a boot in which nothing happened. expect(note).not.toBeNull(); expect(note).toContain('Immediate DDL was called 1 time(s) during the declaration boot (dropTable() on sys_old_table via driver.recording)'); - expect(note).not.toContain('writes nothing'); + expect(note).not.toContain('a plan writes no row or table'); } finally { warn.mockRestore(); await kernel.shutdown(); @@ -903,7 +911,7 @@ describe('the two named boundaries (#14126)', () => { expect(note).toContain('Could NOT guard '); expect(note).toContain('driver.frozen.create'); expect(note).toContain('does NOT claim to have written nothing'); - expect(note).not.toContain('a plan writes nothing'); + expect(note).not.toContain('a plan writes no row or table'); await kernel.shutdown(); }); @@ -942,7 +950,7 @@ describe('the two named boundaries (#14126)', () => { const note = guard.disarm(); expect(note).toContain('Refused 1 write(s) during the declaration boot: create() on sys_thing.'); expect(note).toContain('Could NOT shadow objectql.registerDriver'); - expect(note).not.toContain('a plan writes nothing'); + expect(note).not.toContain('a plan writes no row or table'); await kernel.shutdown(); }); diff --git a/packages/cli/src/utils/schema-migration-plugins.ts b/packages/cli/src/utils/schema-migration-plugins.ts index ec2354f89e8..498e46872d0 100644 --- a/packages/cli/src/utils/schema-migration-plugins.ts +++ b/packages/cli/src/utils/schema-migration-plugins.ts @@ -1145,7 +1145,7 @@ export function createDeclarationBootWriteGuard(): DeclarationBootWriteGuard { .join(', '); parts.push( `Refused ${total} write(s) during the declaration boot` - + `${writesNothingHeld ? ' — a plan writes nothing' : ''}: ${detail}. ` + + `${writesNothingHeld ? ' — a plan writes no row or table' : ''}: ${detail}. ` + 'A plugin in this stack registers a writing hook outside start(); the plan below is ' + 'unaffected, but that write WOULD have landed on a served boot of the same stack.', ); @@ -1160,7 +1160,7 @@ export function createDeclarationBootWriteGuard(): DeclarationBootWriteGuard { + 'FORWARDED, not refused: a raw command is opaque to this guard — the contract admits ' + 'any native shape, and SQL text cannot be classified as read-vs-write reliably — so ' + 'whether this boot wrote is NOT verified, and this run does NOT claim to have ' - + 'written nothing. If those commands only read, nothing was written; if one wrote, it ' + + 'written nothing. If those commands only read, no row or table was written; if one wrote, it ' + 'landed. Issue row writes through the contract methods (refused and reported here), ' + 'or move raw commands out of the boot path.', ); diff --git a/packages/cli/src/utils/sys-secret-rewrap.ts b/packages/cli/src/utils/sys-secret-rewrap.ts index fb2c9325823..3db476f2ebc 100644 --- a/packages/cli/src/utils/sys-secret-rewrap.ts +++ b/packages/cli/src/utils/sys-secret-rewrap.ts @@ -497,9 +497,14 @@ export const REWRAP_NOTES = { 'A row is LEFT as it is when no holder references it, when its holders belong to different ' + 'producers, or when a holder family could not be read. Its producer cannot be attributed, and ' + 'it is never re-sealed under a guessed scope. It keeps the older binding over (namespace, key) alone.', + // [#23899] No sys_secret row, never "nothing": either mode's connect can + // switch a SQLite file's journal mode, which rewrites its header, and an + // `--apply` run's plain boot creates the tables a never-started database + // lacks. The note naming the switch follows this one in an `--apply` run's + // human face, on a SQLite file only; a dry run's closing line carries it. resumable: 'Re-running is safe. A row already sealed under the current derivation is skipped as done, so a ' - + 'run that stopped part-way finishes the rest, and a finished run writes nothing.', + + 'run that stopped part-way finishes the rest, and a finished run writes no sys_secret row.', // [#23726] No row or table, never "nothing": the dry run's connect can switch // a SQLite file's journal mode, which rewrites its header. The note naming // that switch follows the human face's closing line, on a SQLite file only. diff --git a/packages/cli/src/utils/unmanaged-tables.test.ts b/packages/cli/src/utils/unmanaged-tables.test.ts index c1b757a4520..898975274c1 100644 --- a/packages/cli/src/utils/unmanaged-tables.test.ts +++ b/packages/cli/src/utils/unmanaged-tables.test.ts @@ -469,6 +469,23 @@ describe('rendering — informational, and silent when there is nothing to say', expect(text.toLowerCase()).not.toContain('delete'); }); + it('closes with a claim of no row or table, never that a plan writes nothing', () => { + // A plan's read-only boot can still switch a SQLite file's journal mode, + // which rewrites its header: "writes nothing" is false there, "no row or + // table" is true on every target. `os migrate plan` appends the note + // naming the switch to this line on a SQLite file (its own pin). + const lines = renderUnmanagedTables({ + status: 'read', + prefixes: ['sys_'], + physicalTables: 3, + tables: [{ table: 'sys_legacy_widget' }], + }); + expect(lines[lines.length - 1]).toBe( + 'They are reported for information only — nothing here drops them, and a plan writes no row or table.', + ); + expect(lines.join('\n')).not.toContain('writes nothing'); + }); + it('describes a collapsed rotation family by its shard count', () => { expect(describeUnmanagedTable({ table: 'sys_activity', rotationShards: ['sys_activity__r20260810'] })) .toBe('sys_activity (1 rotation shard(s): sys_activity__r20260810)'); diff --git a/packages/cli/src/utils/unmanaged-tables.ts b/packages/cli/src/utils/unmanaged-tables.ts index c647aec174b..6add5c0c8d2 100644 --- a/packages/cli/src/utils/unmanaged-tables.ts +++ b/packages/cli/src/utils/unmanaged-tables.ts @@ -505,6 +505,6 @@ export function renderUnmanagedTables(report: UnmanagedTablesReport): string[] { `${report.tables.length} table(s) in the database(s) this plan covers carry a reserved platform prefix ` + `(${report.prefixes.join(', ')}) and are declared by no object in this plan:`, ...report.tables.map((finding) => ` • ${describeUnmanagedTable(finding)}`), - 'They are reported for information only — nothing here drops them, and a plan writes nothing.', + 'They are reported for information only — nothing here drops them, and a plan writes no row or table.', ]; } diff --git a/packages/objectql/src/summary-backfill.test.ts b/packages/objectql/src/summary-backfill.test.ts index 343d9f75238..dd827fa1767 100644 --- a/packages/objectql/src/summary-backfill.test.ts +++ b/packages/objectql/src/summary-backfill.test.ts @@ -560,6 +560,11 @@ describe('backfillSummaryNulls — pre-#6013 NULL roll-ups (#6063)', () => { expect(err.field, named.join(',')).toBe(named[named.length - 1]); expect(err.fields, named.join(',')).toEqual([named[named.length - 1]]); expect(err.message, named.join(',')).toContain(named[named.length - 1]); + // It vouches for the walk and nothing wider: the command that prints + // it booted first, and that boot can switch a SQLite file's journal + // mode (its header) or, under --apply, create a missing table. + expect(err.message, named.join(',')).toMatch(/ Refused before the walk read or wrote any row\.$/); + expect(err.message, named.join(',')).not.toContain('nothing was written'); } // Refused BEFORE the walk: no write at all, and even the count/sum holes // this run would otherwise have filled are still holes. diff --git a/packages/objectql/src/summary-backfill.ts b/packages/objectql/src/summary-backfill.ts index 7640c22784a..2fb88f18451 100644 --- a/packages/objectql/src/summary-backfill.ts +++ b/packages/objectql/src/summary-backfill.ts @@ -288,11 +288,16 @@ function resolveRecomputeScope( else if (!unresolved.includes(entry)) unresolved.push(entry); } if (unresolved.length > 0) { + // The closing sentence names what the walk vouches for and nothing wider: + // the command that prints this boots before it calls here, and that boot's + // connect can switch a SQLite file's journal mode (its header, no row), + // while an `--apply` boot may create a missing table. The command, which + // knows its target, appends the journal-mode note on a SQLite file. const err = new Error( `[summary-backfill] recomputeUndefinedOnEmpty names ${unresolved.length} roll-up(s) this run cannot find: ` + `${unresolved.join(', ')}. Each entry is spelled object.field and must name a summary field owned by one ` + `of the ${candidates.length} object(s) this run walks (an object left out by \`objects\` is not walked). ` + - 'Refused before any row was read; nothing was written.', + 'Refused before the walk read or wrote any row.', ) as Error & { code: string; status: number; httpStatus: number; field: string; fields: string[] }; err.code = 'INVALID_FIELD'; err.status = 400;