Skip to content

Commit 2e4ade0

Browse files
committed
Merge origin/main into claude/issue-23514-sentinel-owner-save
Claude-Session: https://claude.ai/code/session_01TkXMn8bJqX6ZgMRwuZTxpf Co-authored-by: Claude <noreply@anthropic.com>
2 parents 814ee78 + 9bb9370 commit 2e4ade0

23 files changed

Lines changed: 1521 additions & 28 deletions
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
---
2+
'@objectstack/cli': patch
3+
---
4+
5+
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
6+
7+
**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.
8+
9+
**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.
10+
11+
- `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.
12+
- `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."
13+
- `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."
14+
- `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)".
15+
- `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)".
16+
- `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.
17+
18+
**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.
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
---
2+
'@objectstack/metadata-protocol': patch
3+
---
4+
5+
fix(metadata-protocol): an orphan adoption no longer makes a draft view live; an adopted draft view is rebound in storage only (ADR-0070 D5, ADR-0048)
6+
7+
Clause-②: no
8+
9+
`reassignOrphanedMetadata`, the method `POST /api/v1/packages/:id/adopt-orphans` calls, sends each rebound view row through the registry write-through so the registry follows it at once. It admitted a view row whatever its `state`. So adopting a package-less DRAFT view registered its draft body as live, stamped with the target package, until the next restart. The boot registers active rows only, so it dropped the view again. A draft adopted beside a published row of the same view replaced the published body the runtime served.
10+
11+
Only an active row now reaches the registry, for a view as for an object. An adopted draft view is rebound in storage only: the registry serves nothing for it until it is published, and the next boot agrees.
12+
13+
What stays as before:
14+
15+
- Which rows the adoption rebinds, the stored `state` of each, and the response (`reassignedCount`, `reassigned`).
16+
- An adopted active view is served at its new binding at once, and an adopted active object moves its registry owner, as before.
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
---
2+
'@objectstack/runtime': patch
3+
---
4+
5+
fix(runtime): a sandboxed body that throws a plain `{ code, message }` object is answered with its own `message` as the client sentence, never `undefined: MESSAGE` or `[object Object]`; its code is kept (#23718)
6+
7+
**Client sentence change, status and code unchanged.** It applies to `POST /api/v1/actions/:object/:action` on a `type: 'script'` action whose sandboxed body throws a value that is not an `Error`, and to a sandboxed hook body that does the same (its refusal at the action door behind a write, and in the `/data` error classification). The QuickJS runner flattened every thrown value as `NAME: MESSAGE`, else `String(value)`. A plain object has no class name and no `toString` of its own, so the doors served `undefined: MESSAGE`, or `[object Object]` when it had no `message`.
8+
9+
- `throw { code: 'EMAIL_TAKEN', message: 'That address is already on file' }`
10+
- FROM `{"success":false,"error":{"code":"VALIDATION_ERROR","message":"undefined: That address is already on file","httpStatus":400,"declaredCode":"EMAIL_TAKEN"}}`
11+
- TO `{"success":false,"error":{"code":"VALIDATION_ERROR","message":"That address is already on file","httpStatus":400,"declaredCode":"EMAIL_TAKEN"}}`
12+
- `throw { code: 'DUPLICATE_RECORD' }`
13+
- FROM `{"success":false,"error":{"code":"DUPLICATE_RECORD","message":"[object Object]","httpStatus":400}}`
14+
- TO `{"success":false,"error":{"code":"DUPLICATE_RECORD","message":"Internal server error","httpStatus":400}}`
15+
16+
A value with no usable `message` and no `toString` of its own is answered with `Internal server error`, the sentence every door already answers for a fault with no business message. To give the caller words, throw a `message` (or an `Error`).
17+
18+
Two outcomes move with it, both for a thrown plain object only:
19+
20+
- A `name` on a plain object is a field, not a class: `throw { name: 'TypeError', message: 'forged' }` was read as a crash (`500 INTERNAL_ERROR`); it is now a refusal, `400` with `forged`.
21+
- A plain object whose `String()` is empty (`throw []`) recorded no rejection, so the request waited for the sandbox's wall-clock ceiling and answered `500`; it now answers at once with the no-message sentence.
22+
23+
Control, unchanged: a thrown `Error` (or subclass) keeps `NAME: MESSAGE` in the server log and the client sentence it got before. A thrown string is its own sentence, and an object with a `toString` of its own keeps it.
24+
25+
In-process: the `SandboxError` a caller catches for such a throw carries the value's `message` as `innerMessage`, and its log-only `message` reads `KIND 'NAME' threw: Error: MESSAGE`.
26+
27+
Clause-②: no

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -503,7 +503,7 @@ const DOORS: readonly Door[] = [
503503
expect(family.status).toBe('enumerated');
504504
}
505505
},
506-
humanEmpty: /Report only — nothing was written or deleted/,
506+
humanEmpty: /Report only — no row or table was written or deleted/,
507507
tables: ['sys_secret', 'sys_setting', 'sys_metadata'],
508508
work: (doc) => expect(doc.plan.counts.total).toBe(1),
509509
},
@@ -516,7 +516,7 @@ const DOORS: readonly Door[] = [
516516
expect(family.status).toBe('enumerated');
517517
}
518518
},
519-
humanEmpty: /Dry run — nothing was written/,
519+
humanEmpty: /Dry run — no row or table was written/,
520520
tables: ['sys_secret', 'sys_setting', 'sys_metadata'],
521521
work: (doc) => expect(doc.report.counts.total).toBe(1),
522522
},

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

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

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

Lines changed: 116 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,43 @@
88
* What is pinned here is the thing a unit test of the plan cannot see: that
99
* this subcommand honours #2186 — a bare `os migrate <topic>` must never
1010
* mutate the database by surprise, so writing is opt-in behind `--apply`.
11+
*
12+
* And what its human face says about the database once the boot has run.
13+
* Either mode's connect can switch a SQLite file's journal mode, which
14+
* rewrites its header and no row, so neither the dry run nor a declined
15+
* `--apply` says that nothing was changed; each names the switch on a SQLite
16+
* target, and only there. The boot is replaced by a stack over a fake engine.
1117
*/
1218

13-
import { describe, it, expect } from 'vitest';
19+
import { afterEach, beforeAll, beforeEach, describe, it, expect, vi } from 'vitest';
20+
import { Config } from '@oclif/core';
21+
import { fileURLToPath } from 'node:url';
1422
import MigrateRecordedBy from './recorded-by.js';
23+
import { JOURNAL_MODE_SWITCH_NOTE } from '../../utils/sqlite-occupancy.js';
1524
import {
1625
RECORDED_BY_SENTINEL,
1726
RECORDED_BY_SENTINEL_PLAN_ID,
1827
createRecordedBySentinelPlan,
1928
} from '@objectstack/metadata-protocol';
2029

30+
const boot = vi.hoisted(() => vi.fn());
31+
vi.mock('../../utils/schema-migrate.js', () => ({ bootSchemaStack: boot }));
32+
// The plugins the real boot composes; the replaced boot never reads them.
33+
vi.mock('../../utils/data-migration-plugins.js', () => ({ buildDataMigrationPlugins: async () => [] }));
34+
// The `--apply` confirmation, answered without a terminal: `undefined` leaves
35+
// readline as it is.
36+
const prompt = vi.hoisted(() => ({ answer: undefined as string | undefined }));
37+
vi.mock('node:readline', async (importOriginal) => {
38+
const real = await importOriginal<typeof import('node:readline')>();
39+
return {
40+
...real,
41+
createInterface: ((...args: Parameters<typeof real.createInterface>) => (prompt.answer === undefined
42+
? real.createInterface(...args)
43+
: { question: (_q: string, answer: (a: string) => void) => answer(prompt.answer as string), close: () => {} }
44+
)) as typeof real.createInterface,
45+
};
46+
});
47+
2148
describe('os migrate recorded-by', () => {
2249
it('is a dry run by default — --apply is opt-in (#2186)', () => {
2350
expect(MigrateRecordedBy.flags.apply.default).toBe(false);
@@ -41,3 +68,91 @@ describe('os migrate recorded-by', () => {
4168
expect(RECORDED_BY_SENTINEL).toBe('system');
4269
});
4370
});
71+
72+
const CLI_ROOT = fileURLToPath(new URL('../../..', import.meta.url));
73+
74+
// oclif reads the package's config (its whole command table) on a first run;
75+
// it is paid here, outside the cases.
76+
let oclifConfig: Config;
77+
beforeAll(async () => {
78+
oclifConfig = await Config.load({ root: CLI_ROOT });
79+
}, 60_000);
80+
81+
const SQLITE_CONFIG = { client: 'better-sqlite3', connection: { filename: '/srv/app/.objectstack/data/objectstack.db' } };
82+
const POSTGRES_CONFIG = { client: 'pg', connection: 'postgres://db.example.com:5432/app' };
83+
84+
/**
85+
* A booted stack over one history row holding the sentinel. The engine can
86+
* only read: a write the command attempted would throw, and the run with it.
87+
*/
88+
function stackOver(config: Record<string, unknown>): void {
89+
const rows = [{ id: 'h_sentinel', recorded_by: RECORDED_BY_SENTINEL }];
90+
const engine = {
91+
find: async (_object: string, query: { where?: Record<string, unknown>; limit?: number }) => {
92+
const matched = rows.filter((row) =>
93+
Object.entries(query.where ?? {}).every(([key, value]) => (row as Record<string, unknown>)[key] === value));
94+
return typeof query?.limit === 'number' ? matched.slice(0, query.limit) : matched;
95+
},
96+
};
97+
boot.mockResolvedValue({
98+
driver: { config },
99+
kernel: { getService: (name: string) => (name === 'objectql' ? engine : undefined) },
100+
tableAbsent: () => false,
101+
shutdown: async () => {},
102+
});
103+
}
104+
105+
async function runHuman(argv: string[]): Promise<string[]> {
106+
const log = vi.spyOn(console, 'log').mockImplementation(() => {});
107+
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
108+
const error = vi.spyOn(console, 'error').mockImplementation(() => {});
109+
const savedExitCode = process.exitCode;
110+
try {
111+
await MigrateRecordedBy.run(argv, oclifConfig);
112+
return log.mock.calls.map((c) => c.map(String).join(' '));
113+
} finally {
114+
process.exitCode = savedExitCode;
115+
for (const spy of [log, warn, error]) spy.mockRestore();
116+
}
117+
}
118+
119+
describe('os migrate recorded-by — the human face names no change it cannot vouch for', () => {
120+
let savedIsTTY: PropertyDescriptor | undefined;
121+
beforeEach(() => {
122+
savedIsTTY = Object.getOwnPropertyDescriptor(process.stdin, 'isTTY');
123+
});
124+
afterEach(() => {
125+
prompt.answer = undefined;
126+
boot.mockReset();
127+
if (savedIsTTY) Object.defineProperty(process.stdin, 'isTTY', savedIsTTY);
128+
else delete (process.stdin as { isTTY?: boolean }).isTTY;
129+
});
130+
131+
it('the dry run claims no row or table, and names the journal-mode switch on a SQLite file', async () => {
132+
stackOver(SQLITE_CONFIG);
133+
const lines = await runHuman([]);
134+
expect(boot).toHaveBeenCalledWith(expect.objectContaining({ deferSchemaDdl: true, readOnlyProbe: true }));
135+
const closing = lines.find((line) => line.includes('Re-run with --apply'));
136+
expect(closing, lines.join('\n')).toContain(
137+
`No row or table was changed. Re-run with --apply to rewrite them to NULL. ${JOURNAL_MODE_SWITCH_NOTE}`,
138+
);
139+
expect(lines.join('\n')).not.toContain('Nothing was changed');
140+
}, 60_000);
141+
142+
it('on a target with no file header (PostgreSQL) the dry run claims no row or table and names no switch', async () => {
143+
stackOver(POSTGRES_CONFIG);
144+
const lines = await runHuman([]);
145+
const closing = lines.find((line) => line.includes('Re-run with --apply'));
146+
expect(closing, lines.join('\n')).toContain('No row or table was changed. Re-run with --apply to rewrite them to NULL.');
147+
expect(lines.join('\n')).not.toContain(JOURNAL_MODE_SWITCH_NOTE);
148+
}, 60_000);
149+
150+
it('an --apply declined at the prompt claims no row changed, and names the switch on a SQLite file', async () => {
151+
Object.defineProperty(process.stdin, 'isTTY', { value: true, configurable: true });
152+
prompt.answer = 'n';
153+
stackOver(SQLITE_CONFIG);
154+
const lines = await runHuman(['--apply']);
155+
const aborted = lines.find((line) => line.includes('Aborted'));
156+
expect(aborted, lines.join('\n')).toContain(`Aborted — no row was changed. ${JOURNAL_MODE_SWITCH_NOTE}`);
157+
}, 60_000);
158+
});

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

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ import {
3030
} from '../../utils/format.js';
3131
import { bootSchemaStack } from '../../utils/schema-migrate.js';
3232
import { buildDataMigrationPlugins } from '../../utils/data-migration-plugins.js';
33+
import { describeDriverSqliteFile } from '../../utils/connection-display.js';
34+
import { JOURNAL_MODE_SWITCH_NOTE } from '../../utils/sqlite-occupancy.js';
3335

3436
async function confirm(question: string): Promise<boolean> {
3537
if (!process.stdin.isTTY) return false; // non-interactive → require --yes
@@ -116,6 +118,17 @@ export default class MigrateRecordedBy extends Command {
116118
}
117119

118120
try {
121+
// [#23622] Either mode's connect may have switched a SQLite file's
122+
// journal mode (`SqlDriver.applySqliteJournalMode`, one rule for every
123+
// connect), which rewrites the file header and no row. So no line below
124+
// says that nothing changed: the read-only mode claims no row or table
125+
// (its boot defers DDL), an abort after the plain boot claims no row
126+
// (that boot may have created a missing table), and on a SQLite file
127+
// each says what the connect may still have changed. A target that is
128+
// no SQLite file (PostgreSQL, MySQL) has no header to switch.
129+
const journalNote = describeDriverSqliteFile(stack.driver?.config) !== undefined
130+
? ` ${JOURNAL_MODE_SWITCH_NOTE}`
131+
: '';
119132
const engine: IObjectQLEngine = stack.kernel.getService('objectql');
120133
if (typeof engine?.find !== 'function') {
121134
throw new Error('No ObjectQL engine on this stack — cannot read sys_metadata_history.');
@@ -155,7 +168,7 @@ export default class MigrateRecordedBy extends Command {
155168
return;
156169
}
157170
printWarning(`${pending.length} sys_metadata_history row(s) hold recorded_by = '${RECORDED_BY_SENTINEL}'.`);
158-
printInfo("Nothing was changed. Re-run with --apply to rewrite them to NULL.");
171+
printInfo(`No row or table was changed. Re-run with --apply to rewrite them to NULL.${journalNote}`);
159172
return;
160173
}
161174

@@ -176,7 +189,7 @@ export default class MigrateRecordedBy extends Command {
176189
return;
177190
}
178191
const ok = await confirm(chalk.bold(`\n${summary}? [y/N] `));
179-
if (!ok) { printInfo('Aborted — nothing changed.'); return; }
192+
if (!ok) { printInfo(`Aborted — no row was changed.${journalNote}`); return; }
180193
}
181194

182195
if (!flags.json) printStep('Converting…');

0 commit comments

Comments
 (0)