Skip to content

Commit 7bdbb42

Browse files
committed
fix(cli): three more read-only arms claim no row or table, and name the journal-mode switch on a SQLite file
`os storage orphans`, the `os secret rewrap` dry-run note and the `os migrate summary-nulls` dry run said "nothing was written" after a boot whose connect can switch a SQLite file's journal mode (header only). Each now claims no row or table; the storage and summary-nulls commands append JOURNAL_MODE_SWITCH_NOTE to the formatter's line on a SQLite target. Claude-Session: https://claude.ai/code/session_018t6a5rvqPRvEDMCVVW9kYT Co-authored-by: Claude <noreply@anthropic.com>
1 parent 6422da5 commit 7bdbb42

13 files changed

Lines changed: 228 additions & 20 deletions

File tree

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
---
2+
'@objectstack/cli': patch
3+
'@objectstack/service-storage': patch
4+
'@objectstack/objectql': patch
5+
---
6+
7+
Clause-②: no
8+
9+
fix(cli): `os storage orphans`, the `os secret rewrap` dry run and the `os migrate summary-nulls` dry run say they write no row or table, and that opening a SQLite file can still switch its journal mode, instead of saying nothing was written
10+
11+
**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 "nothing was written". Nothing was lost.
12+
13+
**What changed.** Each sentence below now claims no row or table. Where the run opened a SQLite file, the report's 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.
14+
15+
- `os storage orphans` (`formatStrandedOrphanInventory`, `@objectstack/service-storage`). FROM: "This pass is READ-ONLY: nothing was written, tombstoned or deleted." TO: "This pass is READ-ONLY: no row or table was written, tombstoned or deleted." and the note.
16+
- `os secret rewrap` (the dry run's note, `REWRAP_NOTES.dryRun`, also in `--json` `report.notes`). FROM: "Dry run: every attributed row was opened, re-sealed and verified in memory, and nothing was written. Re-run with --apply to write." TO: "… and no row or table was written. Re-run with --apply to write." The note already follows the dry run's closing line on a SQLite file, so this sentence carries none.
17+
- `os migrate summary-nulls` (the dry run that found NULL roll-ups, `formatSummaryBackfillReport`, `@objectstack/objectql`). FROM: "Dry run — nothing was written. Re-run with --apply to backfill." TO: "Dry run — no row or table was written. Re-run with --apply to backfill." and the note.
18+
19+
**Unchanged.** No exit code, flag, prompt, refusal or export signature changes, and no `--json` document gains a field; 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/recorded-by.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ async function confirm(question: string): Promise<boolean> {
6161
*
6262
* Dry run by default, like every other `os migrate` subcommand (#2186): a
6363
* bare invocation reports how many rows still carry the sentinel and writes
64-
* nothing. `--apply` runs the conversion through the ADR-0119 D2 migration
64+
* no row or table. `--apply` runs the conversion through the ADR-0119 D2 migration
6565
* journal, so each chunk is one transaction and an interrupted run is
6666
* recoverable via `os migrate resume`.
6767
*

‎packages/cli/src/commands/migrate/summary-nulls.test.ts‎

Lines changed: 66 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,12 @@
1212
* (the unscoped run the ruling keeps byte-for-byte). The seams that would boot
1313
* a database or walk a real engine are replaced; the command's own parse and
1414
* control flow run for real.
15+
*
16+
* And what the human face says about the database: the dry run's read-only
17+
* boot can still switch a SQLite file's journal mode, which rewrites its
18+
* header and no row, so the report's dry-run line claims no row or table and,
19+
* on a SQLite target only, is followed by the note naming the switch. The
20+
* report's formatter is the real one from `@objectstack/objectql`.
1521
*/
1622

1723
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
@@ -21,6 +27,7 @@ import MigrateSummaryNulls from './summary-nulls.js';
2127
import { bootSchemaStack } from '../../utils/schema-migrate.js';
2228
import { probeMigrationTarget } from '../../utils/migrate-occupancy-gate.js';
2329
import { isExitSignal } from '../../utils/format.js';
30+
import { JOURNAL_MODE_SWITCH_NOTE } from '../../utils/sqlite-occupancy.js';
2431
import { backfillSummaryNulls } from '@objectstack/objectql';
2532

2633
vi.mock('../../utils/schema-migrate.js', () => ({ bootSchemaStack: vi.fn() }));
@@ -29,10 +36,15 @@ vi.mock('../../utils/migrate-occupancy-gate.js', () => ({
2936
probeMigrationTarget: vi.fn(),
3037
}));
3138
vi.mock('../../utils/data-migration-plugins.js', () => ({ buildDataMigrationPlugins: vi.fn(async () => []) }));
32-
vi.mock('@objectstack/objectql', () => ({
33-
backfillSummaryNulls: vi.fn(),
34-
formatSummaryBackfillReport: vi.fn(() => []),
35-
}));
39+
// The walk is replaced; the formatter is the real one, so the human-face pins
40+
// below read the report's own dry-run line.
41+
vi.mock('@objectstack/objectql', async (importOriginal) => {
42+
const real = await importOriginal<typeof import('@objectstack/objectql')>();
43+
return {
44+
backfillSummaryNulls: vi.fn(),
45+
formatSummaryBackfillReport: real.formatSummaryBackfillReport,
46+
};
47+
});
3648

3749
const HERE = dirname(fileURLToPath(import.meta.url));
3850
const CLI_ROOT = resolve(HERE, '..', '..', '..');
@@ -154,3 +166,53 @@ describe('os migrate summary-nulls', () => {
154166
expect(payload.error).toContain('customer.nope');
155167
}, RUN_TIMEOUT);
156168
});
169+
170+
// ───────────────────────────────────────────────────────────────────────────
171+
// The human face: what the dry run says about the database
172+
// ───────────────────────────────────────────────────────────────────────────
173+
174+
const SQLITE_CONFIG = { client: 'better-sqlite3', connection: { filename: '/srv/app/.objectstack/data/objectstack.db' } };
175+
const POSTGRES_CONFIG = { client: 'pg', connection: 'postgres://db.example.com:5432/app' };
176+
const DRY_RUN_LINE = 'Dry run — no row or table was written. Re-run with --apply to backfill.';
177+
178+
/** A dry run that found one NULL roll-up, so the report closes with its dry-run line. */
179+
const DRY_REPORT_WITH_NULLS = {
180+
...EMPTY_REPORT,
181+
scannedObjects: ['customer'],
182+
scannedRecords: 1,
183+
fields: [{
184+
object: 'customer', field: 'open_count', fn: 'count', childObject: 'case',
185+
nullRows: 1, nonEmpty: 0, filled: 0, sampleRecordIds: ['c_1'],
186+
}],
187+
nullRows: 1,
188+
};
189+
190+
describe('os migrate summary-nulls — the dry run names no write it cannot vouch for', () => {
191+
/** Boot onto a primary driver with `config`; every report line the run printed. */
192+
async function runDry(config: Record<string, unknown>): Promise<string[]> {
193+
vi.mocked(bootSchemaStack).mockResolvedValue({
194+
// The primary datasource's driver, as `SchemaStack.driver` carries it.
195+
driver: { config },
196+
kernel: { getService: () => engine },
197+
dbLabel: 'test database',
198+
shutdown: vi.fn(async () => {}),
199+
} as any);
200+
vi.mocked(backfillSummaryNulls).mockResolvedValue(DRY_REPORT_WITH_NULLS as any);
201+
await MigrateSummaryNulls.run([], { root: CLI_ROOT });
202+
return log.mock.calls.map((c: unknown[]) => c.map(String).join(' ')).join('\n').split('\n');
203+
}
204+
205+
it('on a SQLite file the dry-run line claims no row or table and names the journal-mode switch', async () => {
206+
const lines = await runDry(SQLITE_CONFIG);
207+
const dryRun = lines.find((line) => line.startsWith('Dry run —'));
208+
expect(dryRun, lines.join('\n')).toBe(`${DRY_RUN_LINE} ${JOURNAL_MODE_SWITCH_NOTE}`);
209+
expect(lines.join('\n')).not.toContain('nothing was written');
210+
}, RUN_TIMEOUT);
211+
212+
it('on a target with no file header (PostgreSQL) the dry-run line claims no row or table and names no switch', async () => {
213+
const lines = await runDry(POSTGRES_CONFIG);
214+
const dryRun = lines.find((line) => line.startsWith('Dry run —'));
215+
expect(dryRun, lines.join('\n')).toBe(DRY_RUN_LINE);
216+
expect(lines.join('\n')).not.toContain(JOURNAL_MODE_SWITCH_NOTE);
217+
}, RUN_TIMEOUT);
218+
});

‎packages/cli/src/commands/migrate/summary-nulls.ts‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {
1818
import { bootSchemaStack } from '../../utils/schema-migrate.js';
1919
import { OCCUPANCY_HINT, probeMigrationTarget } from '../../utils/migrate-occupancy-gate.js';
2020
import { describeOccupancy, JOURNAL_MODE_SWITCH_NOTE } from '../../utils/sqlite-occupancy.js';
21+
import { describeDriverSqliteFile } from '../../utils/connection-display.js';
2122
import { buildDataMigrationPlugins } from '../../utils/data-migration-plugins.js';
2223
import { refuseUndeclaredObjects } from '../../utils/migrate-object-scope.js';
2324
// Type-only, so the heavy engine package is still loaded lazily below: this is
@@ -254,7 +255,19 @@ export default class MigrateSummaryNulls extends Command {
254255

255256
printInfo(`Database: ${chalk.white(stack.dbLabel)}`);
256257
console.log('');
257-
console.log(formatSummaryBackfillReport(report).join('\n'));
258+
// [#23726] A dry run that found NULL roll-ups ends the report with "Dry
259+
// run — no row or table was written." (its last line). The read-only
260+
// boot's connect may still have switched a SQLite file's journal mode
261+
// (`SqlDriver.applySqliteJournalMode`, its rule for every connect), which
262+
// rewrites the file header and no row, so on a SQLite file that line is
263+
// followed by the note naming the switch. A target that is no SQLite file
264+
// (PostgreSQL, MySQL) has no header to switch. The `--json` document
265+
// carries no sentence and is unchanged.
266+
const journalNote = !report.applied && report.nullRows > 0
267+
&& describeDriverSqliteFile(stack.driver?.config) !== undefined
268+
? ` ${JOURNAL_MODE_SWITCH_NOTE}`
269+
: '';
270+
console.log(`${formatSummaryBackfillReport(report).join('\n')}${journalNote}`);
258271
console.log('');
259272

260273
if (report.failures.length > 0) {

‎packages/cli/src/commands/secret/orphans.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ export default class SecretOrphans extends Command {
218218
// [#21471] Composed through the one-shot helper, never with the
219219
// service's default provider: in a development posture with no key,
220220
// that default mints a key file in the key home, and this report
221-
// promises to write nothing.
221+
// promises to write no row or table, and writes no key file either.
222222
extraPlugins: [new PlatformObjectsPlugin(), await oneShotSettingsPlugin()],
223223
// [#21391] The report boots READ-ONLY, the boot `os migrate plan`
224224
// takes: `deferSchemaDdl` holds schema DDL back on every SQL

‎packages/cli/src/commands/secret/rewrap.guards.test.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,21 @@ describe('os secret rewrap — the human face names no write it cannot vouch for
350350
expect(closing, lines.join('\n')).toContain(`Dry run — no row or table was written. ${JOURNAL_MODE_SWITCH_NOTE}`);
351351
}, 60_000);
352352

353+
it('the dry run\'s own note claims no row or table, in the human face and in --json report.notes', async () => {
354+
const DRY_RUN_NOTE = 'Dry run: every attributed row was opened, re-sealed and verified in memory, and no row or '
355+
+ 'table was written. Re-run with --apply to write.';
356+
wireBoot(freshRows(), { driverConfig: SQLITE_CONFIG });
357+
const lines = await runHuman(['--no-declared-datasources']);
358+
const note = lines.find((line) => line.includes('Dry run: every attributed row'));
359+
expect(note, lines.join('\n')).toContain(DRY_RUN_NOTE);
360+
expect(lines.join('\n')).not.toContain('nothing was written');
361+
362+
wireBoot(freshRows(), { driverConfig: SQLITE_CONFIG });
363+
const { payload } = await run(['--no-declared-datasources']);
364+
expect(payload.report.notes).toContain(DRY_RUN_NOTE);
365+
expect(JSON.stringify(payload.report.notes)).not.toContain('nothing was written');
366+
}, 60_000);
367+
353368
it('on a target with no file header (PostgreSQL) the dry run claims no row or table and names no switch', async () => {
354369
wireBoot(freshRows(), { driverConfig: POSTGRES_CONFIG });
355370
const lines = await runHuman(['--no-declared-datasources']);

‎packages/cli/src/commands/secret/rewrap.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ import { readDeclaredDatasources } from './orphans.js';
4343
*
4444
* - **A dry run by default.** It boots read-only (the `os migrate plan` boot),
4545
* opens, re-seals and verifies every attributed row in memory, prints
46-
* classes and counts, and writes nothing. `--apply` writes.
46+
* classes and counts, and writes no row or table. `--apply` writes.
4747
* - **Never run for you.** Nothing on any boot or upgrade path invokes it, and
4848
* it must not grow such a caller.
4949
* - **Operator-only.** ⛔ No HTTP surface, so no refusal here answers a
Lines changed: 76 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,94 @@
11
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
22

33
/**
4-
* `os storage orphans` — what its help says the inventory writes.
4+
* `os storage orphans` — what its help and its report say the inventory writes.
55
*
66
* The inventory boots read-only (deferred DDL, no seed, no file brought into
77
* existence), but the boot's connect can switch a SQLite file's journal mode,
88
* which rewrites the file's header and no row (measured on a rollback-journal
99
* file: header bytes 18 and 19 from 1,1 to 2,2, the change counter up by one,
10-
* schema and rows byte-identical). So the help claims no row or table, never
11-
* that the command writes nothing. The help names no target, so it carries no
12-
* SQLite note.
10+
* schema and rows byte-identical). So the help and the report claim no row or
11+
* table, never that the command writes nothing. The help names no target, so
12+
* it carries no SQLite note; the report's READ-ONLY line carries it on a
13+
* SQLite target, and only there.
14+
*
15+
* The report case replaces only the boot. The inventory and its formatter
16+
* (`@objectstack/service-storage`) run for real over an engine with no files.
1317
*/
1418

15-
import { describe, expect, it } from 'vitest';
19+
import { afterEach, describe, expect, it, vi } from 'vitest';
20+
import { dirname, resolve } from 'node:path';
21+
import { fileURLToPath } from 'node:url';
1622
import StorageOrphans from './orphans.js';
23+
import { bootSchemaStack } from '../../utils/schema-migrate.js';
24+
import { JOURNAL_MODE_SWITCH_NOTE } from '../../utils/sqlite-occupancy.js';
25+
26+
vi.mock('../../utils/schema-migrate.js', () => ({ bootSchemaStack: vi.fn() }));
27+
vi.mock('../../utils/data-migration-plugins.js', () => ({ buildDataMigrationPlugins: vi.fn(async () => []) }));
28+
29+
const HERE = dirname(fileURLToPath(import.meta.url));
30+
const CLI_ROOT = resolve(HERE, '..', '..', '..');
31+
/** oclif builds its whole command table on the first `run()` in a process. */
32+
const RUN_TIMEOUT = 60_000;
33+
34+
const SQLITE_CONFIG = { client: 'better-sqlite3', connection: { filename: '/srv/app/.objectstack/data/objectstack.db' } };
35+
const POSTGRES_CONFIG = { client: 'pg', connection: 'postgres://db.example.com:5432/app' };
36+
const READ_ONLY_LINE = 'This pass is READ-ONLY: no row or table was written, tombstoned or deleted.';
37+
38+
/** Wire the mocked boot to an engine that registers both objects and stores no file. */
39+
function wireBoot(driverConfig: Record<string, unknown>): void {
40+
const engine = {
41+
getObject: (name: string) => (name === 'sys_file' || name === 'sys_attachment' ? { name } : undefined),
42+
find: async () => [],
43+
};
44+
vi.mocked(bootSchemaStack).mockResolvedValue({
45+
// The primary datasource's driver, as `SchemaStack.driver` carries it.
46+
driver: { config: driverConfig },
47+
dbLabel: 'test database',
48+
kernel: { getService: (name: string) => (name === 'objectql' ? engine : undefined) },
49+
tableAbsent: () => false,
50+
shutdown: async () => {},
51+
} as never);
52+
}
53+
54+
/** One human-mode run; every line `console.log` received. */
55+
async function runHuman(): Promise<string[]> {
56+
const log = vi.spyOn(console, 'log').mockImplementation(() => {});
57+
const savedExitCode = process.exitCode;
58+
try {
59+
await StorageOrphans.run([], { root: CLI_ROOT });
60+
return log.mock.calls.map((c) => c.map(String).join(' ')).join('\n').split('\n');
61+
} finally {
62+
process.exitCode = savedExitCode;
63+
log.mockRestore();
64+
}
65+
}
66+
67+
afterEach(() => {
68+
vi.mocked(bootSchemaStack).mockReset();
69+
});
1770

1871
describe('os storage orphans --help', () => {
1972
it('says the inventory writes no row or table, tombstones nothing and deletes nothing', () => {
2073
expect(StorageOrphans.description).toContain('Writes no row or table, tombstones nothing, deletes nothing.');
2174
expect(StorageOrphans.description).not.toContain('Writes nothing');
2275
});
2376
});
77+
78+
describe('os storage orphans — the report names no write it cannot vouch for', () => {
79+
it('on a SQLite file the READ-ONLY line claims no row or table and names the journal-mode switch', async () => {
80+
wireBoot(SQLITE_CONFIG);
81+
const lines = await runHuman();
82+
const readOnly = lines.find((line) => line.includes('READ-ONLY'));
83+
expect(readOnly, lines.join('\n')).toBe(`${READ_ONLY_LINE} ${JOURNAL_MODE_SWITCH_NOTE}`);
84+
expect(lines.join('\n')).not.toContain('nothing was written');
85+
}, RUN_TIMEOUT);
86+
87+
it('on a target with no file header (PostgreSQL) the READ-ONLY line claims no row or table and names no switch', async () => {
88+
wireBoot(POSTGRES_CONFIG);
89+
const lines = await runHuman();
90+
const readOnly = lines.find((line) => line.includes('READ-ONLY'));
91+
expect(readOnly, lines.join('\n')).toBe(READ_ONLY_LINE);
92+
expect(lines.join('\n')).not.toContain(JOURNAL_MODE_SWITCH_NOTE);
93+
}, RUN_TIMEOUT);
94+
});

‎packages/cli/src/commands/storage/orphans.ts‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ import {
1717
import { bootSchemaStack } from '../../utils/schema-migrate.js';
1818
import { buildDataMigrationPlugins } from '../../utils/data-migration-plugins.js';
1919
import { absentTableReads } from '../../utils/absent-table-reads.js';
20+
import { describeDriverSqliteFile } from '../../utils/connection-display.js';
21+
import { JOURNAL_MODE_SWITCH_NOTE } from '../../utils/sqlite-occupancy.js';
2022
import type { IDataEngine } from '@objectstack/spec/contracts';
2123
import type { StrandedOrphanInventoryEngine } from '@objectstack/service-storage';
2224

@@ -181,7 +183,17 @@ export default class StorageOrphans extends Command {
181183
printInfo(`Database: ${chalk.white(stack.dbLabel)}`);
182184
reads.notice(false);
183185
console.log('');
184-
console.log(formatStrandedOrphanInventory(report));
186+
// [#23726] The report's last line says the pass is READ-ONLY and wrote
187+
// no row or table. The boot's connect may still have switched a SQLite
188+
// file's journal mode (`SqlDriver.applySqliteJournalMode`, its rule for
189+
// every connect), which rewrites the file header and no row, so on a
190+
// SQLite file that line is followed by the note naming the switch. A
191+
// target that is no SQLite file (PostgreSQL, MySQL) has no header to
192+
// switch. The `--json` document carries no sentence and is unchanged.
193+
const journalNote = describeDriverSqliteFile(stack.driver?.config) !== undefined
194+
? ` ${JOURNAL_MODE_SWITCH_NOTE}`
195+
: '';
196+
console.log(`${formatStrandedOrphanInventory(report)}${journalNote}`);
185197
console.log('');
186198
if (report.truncated) {
187199
printWarning(

‎packages/cli/src/utils/sys-secret-rewrap.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -500,9 +500,12 @@ export const REWRAP_NOTES = {
500500
resumable:
501501
'Re-running is safe. A row already sealed under the current derivation is skipped as done, so a '
502502
+ 'run that stopped part-way finishes the rest, and a finished run writes nothing.',
503+
// [#23726] No row or table, never "nothing": the dry run's connect can switch
504+
// a SQLite file's journal mode, which rewrites its header. The note naming
505+
// that switch follows the human face's closing line, on a SQLite file only.
503506
dryRun:
504-
'Dry run: every attributed row was opened, re-sealed and verified in memory, and nothing was '
505-
+ 'written. Re-run with --apply to write.',
507+
'Dry run: every attributed row was opened, re-sealed and verified in memory, and no row or table '
508+
+ 'was written. Re-run with --apply to write.',
506509
unfinished:
507510
'Some rows were not re-wrapped (see the refused and not-written counts). Each was left exactly as '
508511
+ 'it was. A row that changed during the run is picked up by a re-run.',

0 commit comments

Comments
 (0)