fix(cli): six read-only arms claim no row or table, and name the journal-mode switch on a SQLite file

.changeset/23622-read-only-arms-journal-note-remainder.md

@@ -0,0 +1,18 @@
+---
+'@objectstack/cli': patch
+---
+
+fix(cli): `os migrate security-catalog-overlays`, `recorded-by` and `resume`, `os secret orphans` and `rewrap`, and `os storage orphans` say they write no row or table, and that opening a SQLite file can still switch its journal mode, instead of saying they write nothing
+
+**What was wrong.** Each of these commands 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, every command's default mode 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 "writes nothing", "nothing was written" or "Nothing was changed". The same held for an `--apply`, `--run` or `--delete` declined at its prompt, whose plain boot can also create a table the database is missing. Nothing was lost.
+
+**What changed.** Each sentence below now claims no row or table (no row, after a declined prompt) and, where the command opened a SQLite file, is followed by the note the read-only `os migrate` 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 sentence ends without it.
+
+- `os migrate security-catalog-overlays` (the preview), beside a process holding the file. FROM: "The preview below writes nothing." TO: "The preview below writes no row or table." and the note.
+- `os migrate recorded-by` (the dry run). FROM: "Nothing was changed. Re-run with --apply to rewrite them to NULL." TO: "No row or table was changed. Re-run with --apply to rewrite them to NULL." Its `--apply` declined at the prompt. FROM: "Aborted — nothing changed." TO: "Aborted — no row was changed."
+- `os migrate resume` (the list). FROM: `Nothing was changed. Re-run with --run <id> to act on one.` TO: `No row or table was changed. Re-run with --run <id> to act on one.` Its `--run` declined at the prompt. FROM: "Aborted — nothing changed." TO: "Aborted — no row was changed."
+- `os secret orphans` (the report). FROM: "Report only — nothing was written or deleted." TO: "Report only — no row or table was written or deleted." The note sits before the elapsed time. Its `--delete` declined at the prompt. FROM: "Aborted — nothing was written or deleted." TO: "Aborted — no export was written and no row was deleted." Its help description. FROM: "… Report-only by default: it writes nothing and deletes nothing." TO: "… it writes no row or table and deletes nothing." Its `--delete` flag description. FROM: "(default is a report that writes nothing)" TO: "(default is a report that writes no row or table)".
+- `os secret rewrap` (the dry run). FROM: "Dry run — nothing was written." TO: "Dry run — no row or table was written." The note sits before the elapsed time. Its `--apply` declined at the prompt. FROM: "Aborted — nothing was opened or written." TO: "Aborted — no row was opened or written." Its help description. FROM: "… A dry run by default: it writes nothing without --apply." TO: "… it writes no row or table without --apply." Its `--apply` flag description. FROM: "(default is a dry run that writes nothing)" TO: "(default is a dry run that writes no row or table)".
+- `os storage orphans`, its help description. FROM: "… Writes nothing, tombstones nothing, deletes nothing." TO: "… Writes no row or table, tombstones nothing, deletes nothing." Help text carries no note.
+
+**Unchanged.** No exit code, `--json` document, flag, prompt or refusal changes; the note is printed in the text output only. What the driver does on connect is unchanged: once a file is in the mode the driver asks for, these runs leave it byte-identical.

packages/cli/src/commands/migrate/data-commands.absent-database.integration.test.ts

@@ -503,7 +503,7 @@ const DOORS: readonly Door[] = [
         expect(family.status).toBe('enumerated');
       }
     },
-    humanEmpty: /Report only — nothing was written or deleted/,
+    humanEmpty: /Report only — no row or table was written or deleted/,
     tables: ['sys_secret', 'sys_setting', 'sys_metadata'],
     work: (doc) => expect(doc.plan.counts.total).toBe(1),
   },
@@ -516,7 +516,7 @@ const DOORS: readonly Door[] = [
         expect(family.status).toBe('enumerated');
       }
     },
-    humanEmpty: /Dry run — nothing was written/,
+    humanEmpty: /Dry run — no row or table was written/,
     tables: ['sys_secret', 'sys_setting', 'sys_metadata'],
     work: (doc) => expect(doc.report.counts.total).toBe(1),
   },

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

@@ -207,7 +207,7 @@ export default class MigratePlan extends Command {
       // are separate CUSTOMERS. The HARD gate runs at app install; this covers
       // the two populations it cannot reach (installs predating the gate, and
       // environments whose posture changed after install). Never fatal: a plan
-      // writes nothing, and this reports a metadata decision, not drift.
+      // writes no row or table, and this reports a metadata decision, not drift.
       const uniqueScopeAdvisory = postureGatesGlobalUniques(resolveTenancyPosture())
         ? collectGlobalUniques(stack.allObjects())
         : [];

packages/cli/src/commands/migrate/recorded-by.test.ts

@@ -8,16 +8,43 @@
  * What is pinned here is the thing a unit test of the plan cannot see: that
  * this subcommand honours #2186 — a bare `os migrate <topic>` must never
  * mutate the database by surprise, so writing is opt-in behind `--apply`.
+ *
+ * And what its human face says about the database once the boot has run.
+ * Either mode's connect can switch a SQLite file's journal mode, which
+ * rewrites its header and no row, so neither the dry run nor a declined
+ * `--apply` says that nothing was changed; each names the switch on a SQLite
+ * target, and only there. The boot is replaced by a stack over a fake engine.
  */
 
-import { describe, it, expect } from 'vitest';
+import { afterEach, beforeAll, beforeEach, describe, it, expect, vi } from 'vitest';
+import { Config } from '@oclif/core';
+import { fileURLToPath } from 'node:url';
 import MigrateRecordedBy from './recorded-by.js';
+import { JOURNAL_MODE_SWITCH_NOTE } from '../../utils/sqlite-occupancy.js';
 import {
   RECORDED_BY_SENTINEL,
   RECORDED_BY_SENTINEL_PLAN_ID,
   createRecordedBySentinelPlan,
 } from '@objectstack/metadata-protocol';
 
+const boot = vi.hoisted(() => vi.fn());
+vi.mock('../../utils/schema-migrate.js', () => ({ bootSchemaStack: boot }));
+// The plugins the real boot composes; the replaced boot never reads them.
+vi.mock('../../utils/data-migration-plugins.js', () => ({ buildDataMigrationPlugins: async () => [] }));
+// The `--apply` confirmation, answered without a terminal: `undefined` leaves
+// readline as it is.
+const prompt = vi.hoisted(() => ({ answer: undefined as string | undefined }));
+vi.mock('node:readline', async (importOriginal) => {
+  const real = await importOriginal<typeof import('node:readline')>();
+  return {
+    ...real,
+    createInterface: ((...args: Parameters<typeof real.createInterface>) => (prompt.answer === undefined
+      ? real.createInterface(...args)
+      : { question: (_q: string, answer: (a: string) => void) => answer(prompt.answer as string), close: () => {} }
+    )) as typeof real.createInterface,
+  };
+});
+
 describe('os migrate recorded-by', () => {
   it('is a dry run by default — --apply is opt-in (#2186)', () => {
     expect(MigrateRecordedBy.flags.apply.default).toBe(false);
@@ -41,3 +68,91 @@ describe('os migrate recorded-by', () => {
     expect(RECORDED_BY_SENTINEL).toBe('system');
   });
 });
+
+const CLI_ROOT = fileURLToPath(new URL('../../..', import.meta.url));
+
+// oclif reads the package's config (its whole command table) on a first run;
+// it is paid here, outside the cases.
+let oclifConfig: Config;
+beforeAll(async () => {
+  oclifConfig = await Config.load({ root: CLI_ROOT });
+}, 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' };
+
+/**
+ * A booted stack over one history row holding the sentinel. The engine can
+ * only read: a write the command attempted would throw, and the run with it.
+ */
+function stackOver(config: Record<string, unknown>): void {
+  const rows = [{ id: 'h_sentinel', recorded_by: RECORDED_BY_SENTINEL }];
+  const engine = {
+    find: async (_object: string, query: { where?: Record<string, unknown>; limit?: number }) => {
+      const matched = rows.filter((row) =>
+        Object.entries(query.where ?? {}).every(([key, value]) => (row as Record<string, unknown>)[key] === value));
+      return typeof query?.limit === 'number' ? matched.slice(0, query.limit) : matched;
+    },
+  };
+  boot.mockResolvedValue({
+    driver: { config },
+    kernel: { getService: (name: string) => (name === 'objectql' ? engine : undefined) },
+    tableAbsent: () => false,
+    shutdown: async () => {},
+  });
+}
+
+async function runHuman(argv: string[]): Promise<string[]> {
+  const log = vi.spyOn(console, 'log').mockImplementation(() => {});
+  const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
+  const error = vi.spyOn(console, 'error').mockImplementation(() => {});
+  const savedExitCode = process.exitCode;
+  try {
+    await MigrateRecordedBy.run(argv, oclifConfig);
+    return log.mock.calls.map((c) => c.map(String).join(' '));
+  } finally {
+    process.exitCode = savedExitCode;
+    for (const spy of [log, warn, error]) spy.mockRestore();
+  }
+}
+
+describe('os migrate recorded-by — the human face names no change it cannot vouch for', () => {
+  let savedIsTTY: PropertyDescriptor | undefined;
+  beforeEach(() => {
+    savedIsTTY = Object.getOwnPropertyDescriptor(process.stdin, 'isTTY');
+  });
+  afterEach(() => {
+    prompt.answer = undefined;
+    boot.mockReset();
+    if (savedIsTTY) Object.defineProperty(process.stdin, 'isTTY', savedIsTTY);
+    else delete (process.stdin as { isTTY?: boolean }).isTTY;
+  });
+
+  it('the dry run claims no row or table, and names the journal-mode switch on a SQLite file', async () => {
+    stackOver(SQLITE_CONFIG);
+    const lines = await runHuman([]);
+    expect(boot).toHaveBeenCalledWith(expect.objectContaining({ deferSchemaDdl: true, readOnlyProbe: true }));
+    const closing = lines.find((line) => line.includes('Re-run with --apply'));
+    expect(closing, lines.join('\n')).toContain(
+      `No row or table was changed. Re-run with --apply to rewrite them to NULL. ${JOURNAL_MODE_SWITCH_NOTE}`,
+    );
+    expect(lines.join('\n')).not.toContain('Nothing was changed');
+  }, 60_000);
+
+  it('on a target with no file header (PostgreSQL) the dry run claims no row or table and names no switch', async () => {
+    stackOver(POSTGRES_CONFIG);
+    const lines = await runHuman([]);
+    const closing = lines.find((line) => line.includes('Re-run with --apply'));
+    expect(closing, lines.join('\n')).toContain('No row or table was changed. Re-run with --apply to rewrite them to NULL.');
+    expect(lines.join('\n')).not.toContain(JOURNAL_MODE_SWITCH_NOTE);
+  }, 60_000);
+
+  it('an --apply declined at the prompt claims no row changed, and names the switch on a SQLite file', async () => {
+    Object.defineProperty(process.stdin, 'isTTY', { value: true, configurable: true });
+    prompt.answer = 'n';
+    stackOver(SQLITE_CONFIG);
+    const lines = await runHuman(['--apply']);
+    const aborted = lines.find((line) => line.includes('Aborted'));
+    expect(aborted, lines.join('\n')).toContain(`Aborted — no row was changed. ${JOURNAL_MODE_SWITCH_NOTE}`);
+  }, 60_000);
+});

packages/cli/src/commands/migrate/recorded-by.ts

@@ -30,6 +30,8 @@ import {
 } from '../../utils/format.js';
 import { bootSchemaStack } from '../../utils/schema-migrate.js';
 import { buildDataMigrationPlugins } from '../../utils/data-migration-plugins.js';
+import { describeDriverSqliteFile } from '../../utils/connection-display.js';
+import { JOURNAL_MODE_SWITCH_NOTE } from '../../utils/sqlite-occupancy.js';
 
 async function confirm(question: string): Promise<boolean> {
   if (!process.stdin.isTTY) return false; // non-interactive → require --yes
@@ -116,6 +118,17 @@ export default class MigrateRecordedBy extends Command {
     }
 
     try {
+      // [#23622] Either mode's connect may have switched a SQLite file's
+      // journal mode (`SqlDriver.applySqliteJournalMode`, one rule for every
+      // connect), which rewrites the file header and no row. So no line below
+      // says that nothing changed: the read-only mode claims no row or table
+      // (its boot defers DDL), an abort after the plain boot claims no row
+      // (that boot may have created a missing table), and on a SQLite file
+      // each says what the connect may still have changed. A target that is
+      // no SQLite file (PostgreSQL, MySQL) has no header to switch.
+      const journalNote = describeDriverSqliteFile(stack.driver?.config) !== undefined
+        ? ` ${JOURNAL_MODE_SWITCH_NOTE}`
+        : '';
       const engine: IObjectQLEngine = stack.kernel.getService('objectql');
       if (typeof engine?.find !== 'function') {
         throw new Error('No ObjectQL engine on this stack — cannot read sys_metadata_history.');
@@ -155,7 +168,7 @@ export default class MigrateRecordedBy extends Command {
           return;
         }
         printWarning(`${pending.length} sys_metadata_history row(s) hold recorded_by = '${RECORDED_BY_SENTINEL}'.`);
-        printInfo("Nothing was changed. Re-run with --apply to rewrite them to NULL.");
+        printInfo(`No row or table was changed. Re-run with --apply to rewrite them to NULL.${journalNote}`);
         return;
       }
 
@@ -176,7 +189,7 @@ export default class MigrateRecordedBy extends Command {
           return;
         }
         const ok = await confirm(chalk.bold(`\n${summary}? [y/N] `));
-        if (!ok) { printInfo('Aborted — nothing changed.'); return; }
+        if (!ok) { printInfo(`Aborted — no row was changed.${journalNote}`); return; }
       }
 
       if (!flags.json) printStep('Converting…');