Skip to content

Commit c7368a9

Browse files
fix(cli): os db orphans sweeps details whose controlled-by-parent master is optional (#31226)
<!-- claude-projects-attribution: {"github_login":"os-project-manager"} --> _Requested by **Leehom** · [project thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1X9EjYahUm1cF2Gdc8QN2miR)_ Fixes #30976 Clause-②: no Metadata-protocol: none ## 维护者速读 - 改了什么:`os db orphans` 现在也扫描「主表可选」的子对象(`controlled_by_parent` 对象唯一的 `master_detail` 声明为 `required: false`,#30905 裁决 A 放行的写法):这类行若指向一条已不存在的主记录,会被列为孤儿(`--delete` 时先导出再随级联删除);主表字段为空的行是合法的,不列出、不删除。 - 为什么改:扫描只认 `required: true` 的主表字段(spec 的 `isControlledByParentMasterField`),PR #30940 引入可选主表后,这类悬空行既没有任何用户入口能读到(访问权从不存在的主记录派生),`os db orphans` 也看不见。 - 风险与回滚:只改 CLI 的运维命令(手动执行、默认只报告不写);删除范围只多出「可选主表且主记录确已不存在」的行,判定复用 spec 已发布的 `controlledByParentMasterOf`,不改 spec。回滚即还原本 PR。 - 先看:`packages/cli/src/utils/orphan-detail-sweep.ts` 的 `masterFieldsOf`(新判定,含 `sharingModel` 限定);`packages/cli/src/commands/db/orphans.integration.test.ts` 新夹具 `od_tip` 与改动的删除断言;`orphan-detail-sweep.test.ts` 新 describe。 - 两分钟验证:`pnpm --filter @objectstack/cli exec vitest run src/commands/db/orphans.integration.test.ts`,预期 10 个用例全绿,其中 `[#30976]` 用例证明空主表行 `t_none` 不被列出、悬空行 `t_orphan` 被列为孤儿。 - 需要你做的:无,席位落地。 ## What changes - **`packages/cli/src/utils/orphan-detail-sweep.ts`** — the sweep's "which field is a master" judgement moves into one local predicate, `masterFieldsOf(object)`: - every REQUIRED `master_detail` (`isControlledByParentMasterField`, unchanged — the CONTROL); - on a `controlled_by_parent` object, its OPTIONAL master: `controlledByParentMasterOf(fields)` from `@objectstack/spec/data` answering `optional: true` (the one `master_detail`, declared `required: false`, with no required one beside it). The spec function reads any field map and leaves the sharing model to the caller, so the sweep asks `sharingModel === 'controlled_by_parent'` itself: only such an object derives its rows' access from that field (plugin-security's cbp read filter, `security-plugin-cbp-read-filter.ts`, returns no filter for any other model). - `objectsWithRequiredMaster` → `objectsWithMaster`, `isRequiredMasterFinding` → `isMasterFinding` (module-internal to the CLI; not on any `exports` subpath). Docblocks and the "other dangling" note no longer say "required master-detail". - A row whose optional master is EMPTY is never a finding: the dangling-reference audit treats `null` / `undefined` / `''` as "no link" (`packages/objectql/src/integrity/dangling-reference-audit.ts`, "Empty is not a reference"), so the sweep adds no null check of its own. - **`packages/cli/src/commands/db/orphans.ts`** — the command's `description`, docblock and human heading said "required master-detail master"; that became false with this change, so they now say "master" (description: "Report detail rows whose master-detail reference names a master that does not exist, …"). - **`content/docs/deployment/cli.mdx`** — the `os db orphans` table row and section say the same, and name the optional master and the empty-master row. - `.changeset/30976-orphans-optional-master.md` — `@objectstack/cli` patch. No `packages/spec` edit: the optional-master judgement already exists there (`controlledByParentMasterOf`, public since PR #30940); the order's stop-line (a new spec predicate) is not reached. ## Tests - `packages/cli/src/utils/orphan-detail-sweep.test.ts` (unit, fake read-only engine): - fixture gains `sw_optional` (cbp, one `master_detail` `required: false`, plus a lookup), `sw_optional_private` (the same lone optional `master_detail` on a `private` object) and `sw_required_cbp` (cbp with a required master and an optional `master_detail` beside it); - the existing "reads only the objects that declare a master" case now expects `sw_optional` and `sw_required_cbp` read, `sw_optional_private` not; - new `[#30976]` describe: `masterFieldsOf` per object (CONTROL: `sw_detail` still answers its two required masters), and a dangling optional master is an orphan while a dangling lookup on the same object and a dangling optional `master_detail` beside a required master stay in `otherDangling`, never deletable. - `packages/cli/src/commands/db/orphans.integration.test.ts` (integration, real boot + SQLite): fixture gains `od_tip`, cbp with an optional master on `od_campaign`, seeded through a system write with three rows — `t_none` (no campaign), `t_live` (the live campaign), `t_orphan` (the missing campaign). - new case `[#30976] an optional master: …`: `t_orphan` is an orphan; `t_none` and `t_live` appear in neither `orphans` nor `otherDangling`; - the existing cases now carry the second orphan: the report lists `od_tip/t_orphan` (counts `orphans: 2`), `--delete` removes it and keeps `t_none` and `t_live` (`od_tip: ['t_live', 't_none']` after the run), the export holds it, and in the restrict case the unrestricted `od_tip` orphan is deleted while `od_member` is refused as before. - six module-top side-effect imports (`@objectstack/service-datasource`, `driver-turso`, `runtime`, `platform-objects/plugin`, `plugin-security`, `rest`) — `check:test-source-alias` asks them of an edited test file whose source reaches those dynamic imports, copying `serve-host-crypto-provider.test.ts`. <details><summary>Runs</summary> - The three orphan files (`orphan-detail-sweep.test.ts`, `orphans.integration.test.ts`, `orphans.no-auto-run.test.ts`) on head `ec530681`: `Test Files 3 passed (3) · Tests 25 passed (25)`. - `pnpm --filter @objectstack/cli exec vitest run --project unit` on `0967087c` (the `origin/main` merge; `ec530681` only adds the six imports to the integration file): `Test Files 2 failed | 396 passed (398) · Tests 5595 passed | 29 skipped`. The two files were `test/published-subpath-console.pin.test.ts` and `test/published-subpath-hook-body.pin.test.ts`, refusing "packages/cli is not built (./dist/index.js is absent)"; after `pnpm --filter @objectstack/cli build` both ran: `Test Files 2 passed (2) · Tests 29 passed (29)`. - `pnpm --filter @objectstack/cli typecheck` (`tsc --noEmit && pnpm check:test-typecheck`): exit 0. - The integration layer is run here because the diff edits an integration-tier file; the rest of that layer is CI's. </details> <details><summary>Ablations (both restored to the HEAD blob, `git diff HEAD` empty after each)</summary> 1. **The optional-master arm** — `node scripts/ablation-replace.mjs --file packages/cli/src/utils/orphan-detail-sweep.ts --anchor 'if (master?.optional === true) out.push(master.field);' --delete` (anchor 1 → 0, blob `7e854c86` → `766c6ecb`), running the unit file and the integration file: **8 failed | 10 passed (18)**. Red: the three unit cases above, e.g. `AssertionError: expected [] to deeply equal [ { object: 'sw_optional', …(3) } ]`, and five integration cases, e.g. `[#30976] an optional master: …` `AssertionError: expected [ 'od_member', 'od_member_note', …(1) ] to include 'od_tip'` and the report case `expected [ { object: 'od_member', …(3) } ] to deeply equal [ { object: 'od_member', …(3) }, …(1) ]`. The CONTROL case ("an orphan is a dangling REQUIRED master-detail …") stayed green under the ablation. 2. **The `controlled_by_parent` limit** (negative pin: a lone optional `master_detail` on a `private` object is not a master) — anchor `if (def?.sharingModel === 'controlled_by_parent') {` replaced by `if (def !== undefined || def?.sharingModel === 'controlled_by_parent') {` (blob `7e854c86` → `9bc35e09`), unit file: **2 failed | 6 passed (8)** — `expected [ 'master' ] to deeply equal []` for `sw_optional_private`, and the objects list with one extra entry. The CLI suites resolve `orphan-detail-sweep.ts` and `orphans.ts` from `src/` (relative imports), so no build leg was needed. </details> ## Acceptance notes - **Premise measured, holds.** On `origin/main` `658d84f5`, `isControlledByParentMasterField` (`packages/spec/src/data/controlled-by-parent-master.ts`) answers `type === 'master_detail' && required === true` only; PR #30940 left it a required-master predicate and added `controlledByParentMasterOf` beside it for the optional master. The sweep called only the former. - **Deleting an optional-master orphan is the master-detail's own semantics**, which is what the #24964 ruling (record 6303808312) set as the remedy: a `master_detail` cascades or restricts (`deleteBehavior: 'set_null'` on it is a parse-time refusal, `field.zod.ts`), so the master's own delete would have removed such a row or been refused while it stood. Nulling the reference instead would be a different remedy and is not offered. - **Not widened:** a lone `required: false` `master_detail` on a non-cbp object, and one beside a required master on a cbp object, stay "other dangling" — named, never deleted. A row with an empty optional master is never read as an orphan. - **Surface beyond the order's expected surface** (`orphan-detail-sweep.ts` + its test): `orphans.ts` text, `content/docs/deployment/cli.mdx`, and `orphans.integration.test.ts`. The first two said "required master-detail master", which this change makes false; the integration file is the only place the null-master half can fail for real (the unit file's audit is a fake, and the audit is what skips empty values). Declared in the report. ## Verification Gate union derived by `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` (92 families for this diff, the same list at `0967087c` and `ec530681`), run on head **`ec530681`**, each exit code captured before any pipe, reconciled with `--ran`: - **92 run, exit 0 each.** `--ran`: "92 derived famil(ies) accounted for — 92 run, 0 NOT-MEASURED (a DERIVED zero — all 92 recorded an exit code and none of them is 3)". - Prerequisites built first: `pnpm --filter '@objectstack/cli^...' build` (VERDICT command-exit 0), `pnpm --filter @objectstack/cli build`, and the build closure `check:i18n-coverage` names plus `@objectstack/client` / `@objectstack/client-react` for `check:skill-examples` (turbo, VERDICT command-exit 0). `check-plugin-teardown-shape.mjs --self-test` needed `git fetch --depth=1 origin 621a487` for its pinned fixture on this shallow clone. - The first round, on `0967087c`, answered `check:test-source-alias` exit 1 — the edited integration file reached six dynamic imports with no module-top load — fixed in `ec530681` (the six imports above), then exit 0; and four exit 3 PREREQUISITE NOT MET (the three above plus `check:dual-build-cjs-loads`, which on the second round found enough built `dist/` to measure: "116 published require entry point(s) across 71 package(s) load"). - CI families (every required check) are read from this PR's check-runs, not re-run here. `pnpm lint` scoped: `node --stack-size=4000 node_modules/eslint/bin/eslint.js packages/cli --no-inline-config`, exit 0 (the package has no `lint` script); re-run on the edited integration file at `ec530681`, exit 0. `node scripts/check-platform-checklist.mjs --changed-from origin/main` (two module-internal renames): exit 0, "0 anchored files in the change set". --- _Generated by [Claude Code](https://claude.ai/code/session_01FEkQ5madiyLr75cvc8cS31)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 733efbd commit c7368a9

6 files changed

Lines changed: 219 additions & 62 deletions

File tree

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
'@objectstack/cli': patch
3+
---
4+
5+
fix(cli): `os db orphans` sweeps details whose controlled-by-parent master is optional
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
A `controlled_by_parent` object may declare its one master as an optional `master_detail` (`required: false`). `os db orphans` read only required masters, so a detail row of such an object that still named a master that no longer exists was never reported, and `--delete` never removed it. The sweep now reads that optional master too, judged by `controlledByParentMasterOf` from `@objectstack/spec/data`: a row naming a missing master is reported (and deleted with its cascade under `--delete`, after the pre-delete export), and a row that names no master is valid and is never listed. A lone optional `master_detail` on an object that is not `controlled_by_parent`, or one beside a required master, stays an ordinary reference: named, never deleted.

‎content/docs/deployment/cli.mdx‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ predicate/schema/binding mistakes that fail silently at runtime), and `os dev --
8282
| `os dev [package]` | | Start development mode with hot reload |
8383
| `os serve [config]` | | Start the ObjectStack server with plugin auto-detection |
8484
| `os db clean` | | Reclaim SQLite free space with a one-time `VACUUM` (ADR-0057) |
85-
| `os db orphans` | | Report detail rows whose required master-detail master does not exist; delete them only with `--delete` |
85+
| `os db orphans` | | Report detail rows whose master-detail reference names a master that does not exist; delete them only with `--delete` |
8686

8787
#### `os init`
8888

@@ -445,8 +445,11 @@ os db clean --database file:./data/app.db # explicit target
445445
446446
#### `os db orphans`
447447
448-
Reports the detail rows whose **required** `master_detail` master does not exist, and —
449-
only behind `--delete` — deletes them. **Report-only by default: without `--delete` it
448+
Reports the detail rows whose `master_detail` master does not exist, and — only behind
449+
`--delete` — deletes them. The master is a **required** `master_detail`, or the
450+
**optional** master of a `controlled_by_parent` object (its one `master_detail`, declared
451+
`required: false`); a row whose optional master is empty names no master, is valid, and
452+
is never listed. **Report-only by default: without `--delete` it
450453
writes no row or table and creates no file.** It is never run for you; nothing on any
451454
boot, upgrade, migrate or lifecycle path invokes it.
452455
@@ -459,7 +462,7 @@ where an operator acts on that report.
459462
The report lists each orphan's object, record id, field and missing master id, and the
460463
deeper detail rows its delete removes with it (the master-detail's own cascade). It
461464
also names, without ever deleting them, the dangling references on those objects that
462-
are not a required master-detail, and counts the references whose target could not be
465+
are not on such a master, and counts the references whose target could not be
463466
checked at all: unknown is not missing, and none of those rows is listed or deleted.
464467
465468
```bash

‎packages/cli/src/commands/db/orphans.integration.test.ts‎

Lines changed: 72 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,11 @@
2020
* reference cannot be probed, so it is UNDETERMINED, never an orphan;
2121
* - a plain lookup naming no campaign, on the live member — dangling, listed,
2222
* never deleted, and no reason to delete the row it sits on;
23-
* - an object with no required master-detail at all — not read.
23+
* - an object with no required master-detail at all — not read;
24+
* - [#30976] tips, `controlled_by_parent` children of a campaign through an
25+
* OPTIONAL master (`required: false`, ruling A of #30905): one names no
26+
* campaign (valid, never listed), one names the live campaign, and one names
27+
* the missing campaign — an orphan like the member, deleted with it.
2428
*
2529
* ## What each case proves
2630
*
@@ -47,6 +51,15 @@ import { isExitSignal } from '../../utils/format.js';
4751
import { bootSchemaStack } from '../../utils/schema-migrate.js';
4852
import DbOrphans from './orphans.js';
4953

54+
// Pay the first load of what `bootSchemaStack` and the command reach through
55+
// dynamic `import()`s at MODULE LOAD, outside any clocked case.
56+
import '@objectstack/service-datasource';
57+
import '@objectstack/driver-turso';
58+
import '@objectstack/runtime';
59+
import '@objectstack/platform-objects/plugin';
60+
import '@objectstack/plugin-security';
61+
import '@objectstack/rest';
62+
5063
const HERE = dirname(fileURLToPath(import.meta.url));
5164
const CLI_ROOT = resolve(HERE, '..', '..', '..');
5265
const CASE_TIMEOUT_MS = 120_000;
@@ -94,6 +107,12 @@ const ARTIFACT = {
94107
name: 'od_ref',
95108
fields: { name: { type: 'text' }, campaign_ref: { type: 'lookup', reference: 'od_campaign' } },
96109
},
110+
{
111+
// [#30976] An OPTIONAL controlled-by-parent master: a row may name no campaign.
112+
name: 'od_tip',
113+
sharingModel: 'controlled_by_parent',
114+
fields: { body: { type: 'text' }, campaign: { type: 'master_detail', reference: 'od_campaign', required: false } },
115+
},
97116
],
98117
};
99118

@@ -121,7 +140,7 @@ async function idsOf(dbFile: string): Promise<Record<string, string[]>> {
121140
const k = (driver as any).knex;
122141
try {
123142
const out: Record<string, string[]> = {};
124-
for (const table of ['od_campaign', 'od_member', 'od_member_note', 'od_ghost_member', 'od_hold', 'od_ref']) {
143+
for (const table of ['od_campaign', 'od_member', 'od_member_note', 'od_ghost_member', 'od_hold', 'od_ref', 'od_tip']) {
125144
out[table] = ((await k(table).select('id').orderBy('id')) as Array<{ id: string }>).map((r) => r.id);
126145
}
127146
return out;
@@ -212,6 +231,9 @@ beforeAll(async () => {
212231
await seedRow(ql, 'od_member_note', { ...ORG, id: 'n_orphan', body: 'goes with its member', member: 'm_orphan' });
213232
await seedRow(ql, 'od_ghost_member', { ...ORG, id: 'g_row', name: 'unknown master', ghost: 'nobody_knows' });
214233
await seedRow(ql, 'od_ref', { ...ORG, id: 'r_row', name: 'dangling lookup', campaign_ref: 'Gone Campaign' });
234+
await seedRow(ql, 'od_tip', { ...ORG, id: 't_none', body: 'names no campaign' });
235+
await seedRow(ql, 'od_tip', { ...ORG, id: 't_live', body: 'kept', campaign: 'c_live' });
236+
await seedRow(ql, 'od_tip', { ...ORG, id: 't_orphan', body: 'orphan', campaign: MISSING_CAMPAIGN });
215237
} finally {
216238
await stack.shutdown();
217239
}
@@ -224,6 +246,7 @@ beforeAll(async () => {
224246
od_ghost_member: ['g_row'],
225247
od_hold: [],
226248
od_ref: ['r_row'],
249+
od_tip: ['t_live', 't_none', 't_orphan'],
227250
});
228251
}, 180_000);
229252

@@ -248,13 +271,21 @@ describe('[#24964] os db orphans — the default run is a report that writes not
248271

249272
expect(exitCode).toBe(0);
250273
expect(payload.mode).toBe('report');
251-
expect(payload.plan.orphans).toEqual([{
252-
object: 'od_member',
253-
recordId: 'm_orphan',
254-
fields: [{ field: 'campaign', master: 'od_campaign', missingMasterId: MISSING_CAMPAIGN }],
255-
cascade: [{ object: 'od_member_note', id: 'n_orphan' }],
256-
}]);
257-
expect(payload.plan.counts).toMatchObject({ orphans: 1, deeperDetails: 1, withheld: 0 });
274+
expect(payload.plan.orphans).toEqual([
275+
{
276+
object: 'od_member',
277+
recordId: 'm_orphan',
278+
fields: [{ field: 'campaign', master: 'od_campaign', missingMasterId: MISSING_CAMPAIGN }],
279+
cascade: [{ object: 'od_member_note', id: 'n_orphan' }],
280+
},
281+
{
282+
object: 'od_tip',
283+
recordId: 't_orphan',
284+
fields: [{ field: 'campaign', master: 'od_campaign', missingMasterId: MISSING_CAMPAIGN }],
285+
cascade: [],
286+
},
287+
]);
288+
expect(payload.plan.counts).toMatchObject({ orphans: 2, deeperDetails: 1, withheld: 0 });
258289
// Zero writes: no row, no table, and no file beside the database.
259290
expect(await readState(c.dbFile)).toEqual(before);
260291
expect(existsSync(join(c.caseDir, 'export.json'))).toBe(false);
@@ -280,6 +311,24 @@ describe('[#24964] os db orphans — the default run is a report that writes not
280311
// An object with no required master-detail is not this command's to read.
281312
expect(payload.plan.objects).not.toContain('od_ref');
282313
}, CASE_TIMEOUT_MS);
314+
315+
it('[#30976] an optional master: one naming a missing campaign is an orphan; an empty one is valid and never listed', async () => {
316+
const c = prepareCase();
317+
318+
const { payload } = await runJson(['--database-url', c.url, '--json']);
319+
320+
// The scan read the optional-master object.
321+
expect(payload.plan.objects).toContain('od_tip');
322+
const listed = [
323+
...payload.plan.orphans.map((o: { object: string; recordId: string }) => `${o.object}/${o.recordId}`),
324+
...payload.plan.otherDangling.map((d: { objectName: string; recordId: string }) => `${d.objectName}/${d.recordId}`),
325+
];
326+
expect(payload.plan.orphans.map((o: { object: string; recordId: string }) => `${o.object}/${o.recordId}`))
327+
.toContain('od_tip/t_orphan');
328+
// A tip that names no campaign, and one that names the live campaign, are listed nowhere.
329+
expect(listed).not.toContain('od_tip/t_none');
330+
expect(listed).not.toContain('od_tip/t_live');
331+
}, CASE_TIMEOUT_MS);
283332
});
284333

285334
describe('[#24964] os db orphans --delete', () => {
@@ -291,11 +340,14 @@ describe('[#24964] os db orphans --delete', () => {
291340

292341
expect(payload.error, JSON.stringify(payload).slice(0, 400)).toBeUndefined();
293342
expect(exitCode).toBe(0);
294-
expect(payload.deleted).toEqual([expect.objectContaining({
295-
object: 'od_member',
296-
recordId: 'm_orphan',
297-
removed: [{ object: 'od_member', id: 'm_orphan' }, { object: 'od_member_note', id: 'n_orphan' }],
298-
})]);
343+
expect(payload.deleted).toEqual([
344+
expect.objectContaining({
345+
object: 'od_member',
346+
recordId: 'm_orphan',
347+
removed: [{ object: 'od_member', id: 'm_orphan' }, { object: 'od_member_note', id: 'n_orphan' }],
348+
}),
349+
expect.objectContaining({ object: 'od_tip', recordId: 't_orphan', removed: [{ object: 'od_tip', id: 't_orphan' }] }),
350+
]);
299351
// The roll-up on the master that does not exist cannot be recomputed; the run says so, and deleted anyway.
300352
expect(payload.deleted[0].notes.join(' ')).toMatch(/od_campaign\.member_total was not recomputed for Spring Promo/);
301353
expect(payload.failed).toEqual([]);
@@ -307,12 +359,14 @@ describe('[#24964] os db orphans --delete', () => {
307359
od_ghost_member: ['g_row'],
308360
od_hold: [],
309361
od_ref: ['r_row'],
362+
// The tip that names no campaign stays, with the one whose campaign is live.
363+
od_tip: ['t_live', 't_none'],
310364
});
311365

312366
const doc = JSON.parse(readFileSync(exportPath, 'utf8'));
313367
expect(doc.format).toBe('objectstack.orphan-detail.pre-delete-export.v1');
314368
expect(doc.rows.map((r: { object: string; id: string }) => `${r.object}/${r.id}`)).toEqual([
315-
'od_member/m_orphan', 'od_member_note/n_orphan',
369+
'od_member/m_orphan', 'od_member_note/n_orphan', 'od_tip/t_orphan',
316370
]);
317371
expect(doc.rows[0].row).toMatchObject({ id: 'm_orphan', name: 'orphan', campaign: MISSING_CAMPAIGN });
318372
expect(doc.rows[1].row).toMatchObject({ id: 'n_orphan', body: 'goes with its member', member: 'm_orphan' });
@@ -339,7 +393,7 @@ describe('[#24964] os db orphans --delete', () => {
339393

340394
// Non-vacuity: the command reached its delete through the intercepted method.
341395
expect(seenAtFirstDelete).toBeDefined();
342-
expect(seenAtFirstDelete).toEqual(['od_member/m_orphan', 'od_member_note/n_orphan']);
396+
expect(seenAtFirstDelete).toEqual(['od_member/m_orphan', 'od_member_note/n_orphan', 'od_tip/t_orphan']);
343397
expect(exitCode).toBe(1);
344398
expect(payload.failed).toEqual([expect.objectContaining({ object: 'od_member', recordId: 'm_orphan', code: 'INJECTED_REFUSAL' })]);
345399
// Refused, so nothing went.
@@ -364,7 +418,8 @@ describe('[#24964] os db orphans --delete — the delete is the engine\'s own',
364418
const { payload, exitCode } = await runJson(['--delete', '--export', exportPath, '--yes', '--database-url', c.url, '--json']);
365419

366420
expect(exitCode).toBe(1);
367-
expect(payload.deleted).toEqual([]);
421+
// The other orphan, which nothing restricts, is deleted as before.
422+
expect(payload.deleted).toEqual([expect.objectContaining({ object: 'od_tip', recordId: 't_orphan' })]);
368423
expect(payload.failed).toEqual([expect.objectContaining({
369424
object: 'od_member', recordId: 'm_orphan', code: 'DELETE_RESTRICTED',
370425
remaining: [{ object: 'od_member', id: 'm_orphan' }, { object: 'od_member_note', id: 'n_orphan' }],

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

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,10 @@ import type {
2929
} from '../../utils/orphan-detail-sweep.js';
3030

3131
/**
32-
* `os db orphans` — the operator's door to the detail rows whose required
33-
* master-detail master does not exist (#24964).
32+
* `os db orphans` — the operator's door to the detail rows whose master does
33+
* not exist (#24964): a required master-detail, or the optional master of a
34+
* controlled-by-parent object (#30976). A row of the latter that names no
35+
* master is valid and is never listed.
3436
*
3537
* ## Why such a row needs a door, and why this is the one
3638
*
@@ -49,9 +51,8 @@ import type {
4951
* 1. An explicit operator command. ⛔ Nothing on any boot, upgrade, migrate
5052
* or lifecycle-sweep path invokes it, and it must not grow such a caller
5153
* (`orphans.no-auto-run.test.ts` holds it).
52-
* 2. The default run is a dry-run report: each detail row whose required
53-
* master-detail master does not exist — object, record id, field, and the
54-
* missing master id.
54+
* 2. The default run is a dry-run report: each detail row whose master
55+
* does not exist — object, record id, field, and the missing master id.
5556
* 3. Deletion only behind `--delete`, and only of the rows the same run's
5657
* report lists.
5758
* 4. A mandatory pre-delete export of every row the run deletes, written and
@@ -93,7 +94,7 @@ import type {
9394
*/
9495
export default class DbOrphans extends Command {
9596
static override description =
96-
'Report detail rows whose required master-detail master does not exist, and (only with --delete) delete them '
97+
'Report detail rows whose master-detail reference names a master that does not exist, and (only with --delete) delete them '
9798
+ 'with their cascade, after writing a pre-delete export. Report-only by default: it writes no row or table.';
9899

99100
static override examples = [
@@ -410,7 +411,7 @@ async function confirm(question: string): Promise<boolean> {
410411

411412
/** Render the plan for a human. */
412413
function renderPlan(plan: OrphanDetailPlan): void {
413-
console.log(chalk.bold('\n Detail rows whose required master does not exist'));
414+
console.log(chalk.bold('\n Detail rows whose master does not exist'));
414415
printInfo(
415416
`scanned ${plan.scanned} row(s) of ${plan.objects.length} object(s) · orphans ${plan.counts.orphans} · `
416417
+ `deeper details ${plan.counts.deeperDetails} · withheld ${plan.counts.withheld}`,

‎packages/cli/src/utils/orphan-detail-sweep.test.ts‎

Lines changed: 64 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@ import { describe, it, expect, vi } from 'vitest';
1313
import type { DanglingReference, DanglingReferenceReport } from '@objectstack/objectql';
1414
import {
1515
buildOrphanDetailExport,
16-
objectsWithRequiredMaster,
16+
masterFieldsOf,
17+
objectsWithMaster,
1718
planOrphanDetailSweep,
1819
recordKey,
1920
recordsToDelete,
@@ -33,10 +34,34 @@ const CONFIGS = {
3334
},
3435
sw_absent: { name: 'sw_absent', fields: { master: { type: 'master_detail', reference: 'sw_master', required: true } } },
3536
sw_plain: { name: 'sw_plain', fields: { link: { type: 'lookup', reference: 'sw_master' } } },
37+
// [#30976] A controlled-by-parent object whose one master is OPTIONAL (ruling A of #30905).
38+
sw_optional: {
39+
name: 'sw_optional',
40+
sharingModel: 'controlled_by_parent',
41+
fields: {
42+
master: { type: 'master_detail', reference: 'sw_master', required: false },
43+
link: { type: 'lookup', reference: 'sw_master' },
44+
},
45+
},
46+
// The same lone optional `master_detail` on an object that is not controlled by a parent: no master.
47+
sw_optional_private: {
48+
name: 'sw_optional_private',
49+
sharingModel: 'private',
50+
fields: { master: { type: 'master_detail', reference: 'sw_master', required: false } },
51+
},
52+
// Controlled by a parent through its REQUIRED master; the optional `master_detail` beside it is an ordinary parent.
53+
sw_required_cbp: {
54+
name: 'sw_required_cbp',
55+
sharingModel: 'controlled_by_parent',
56+
fields: {
57+
master: { type: 'master_detail', reference: 'sw_master', required: true },
58+
other_parent: { type: 'master_detail', reference: 'sw_master', required: false },
59+
},
60+
},
3661
};
3762

38-
const finding = (recordId: string, field: string, value: string): DanglingReference =>
39-
({ objectName: 'sw_detail', recordId, field, target: 'sw_master', value });
63+
const finding = (recordId: string, field: string, value: string, objectName = 'sw_detail'): DanglingReference =>
64+
({ objectName, recordId, field, target: 'sw_master', value });
4065

4166
function report(dangling: DanglingReference[], extra: Partial<DanglingReferenceReport> = {}): DanglingReferenceReport {
4267
return {
@@ -58,15 +83,17 @@ function engineWith(rep: DanglingReferenceReport, cascade: (object: string, id:
5883
const rootOnly = async (object: string, id: string | number) => [{ object, id: String(id) }];
5984

6085
describe('[#24964] planOrphanDetailSweep', () => {
61-
it('reads only the objects that declare a REQUIRED master-detail, and not a table the boot measured absent', async () => {
62-
expect(objectsWithRequiredMaster(CONFIGS)).toEqual(['sw_detail', 'sw_absent']);
86+
it('reads only the objects that declare a master, and not a table the boot measured absent', async () => {
87+
expect(objectsWithMaster(CONFIGS)).toEqual(['sw_detail', 'sw_absent', 'sw_optional', 'sw_required_cbp']);
6388
const { engine, inspectDanglingReferences } = engineWith(report([]), rootOnly);
6489

6590
const plan = await planOrphanDetailSweep({ engine, maxRows: 50, absent: (o) => o === 'sw_absent' });
6691

67-
expect(inspectDanglingReferences).toHaveBeenCalledWith({ objects: ['sw_detail'], rowsPerObject: 50, maxRows: 50 });
92+
expect(inspectDanglingReferences).toHaveBeenCalledWith({
93+
objects: ['sw_detail', 'sw_optional', 'sw_required_cbp'], rowsPerObject: 50, maxRows: 50,
94+
});
6895
expect(plan.notRead).toEqual(['sw_absent']);
69-
expect(plan.objects).toEqual(['sw_detail']);
96+
expect(plan.objects).toEqual(['sw_detail', 'sw_optional', 'sw_required_cbp']);
7097
});
7198

7299
it('an orphan is a dangling REQUIRED master-detail; a lookup or an optional master-detail is named and never deletable', async () => {
@@ -119,6 +146,36 @@ describe('[#24964] planOrphanDetailSweep', () => {
119146
});
120147
});
121148

149+
describe('[#30976] the optional master of a controlled-by-parent object', () => {
150+
it('is a master field; a lone optional master-detail elsewhere, or one beside a required master, is not', () => {
151+
expect(masterFieldsOf(CONFIGS.sw_optional)).toEqual(['master']);
152+
expect(masterFieldsOf(CONFIGS.sw_optional_private)).toEqual([]);
153+
expect(masterFieldsOf(CONFIGS.sw_required_cbp)).toEqual(['master']);
154+
// CONTROL: the required masters read as they did before the optional master existed.
155+
expect(masterFieldsOf(CONFIGS.sw_detail)).toEqual(['master', 'second']);
156+
});
157+
158+
it('a dangling optional master makes its row an orphan; a dangling lookup on the same object is named, never deletable', async () => {
159+
const { engine } = engineWith(report([
160+
finding('o1', 'master', 'gone_a', 'sw_optional'),
161+
finding('o2', 'link', 'gone_b', 'sw_optional'),
162+
finding('r1', 'other_parent', 'gone_c', 'sw_required_cbp'),
163+
]), rootOnly);
164+
165+
const plan = await planOrphanDetailSweep({ engine, maxRows: 50 });
166+
167+
expect(plan.orphans).toEqual([{
168+
object: 'sw_optional',
169+
recordId: 'o1',
170+
fields: [{ field: 'master', master: 'sw_master', missingMasterId: 'gone_a' }],
171+
cascade: [],
172+
}]);
173+
expect(plan.otherDangling.map((d) => `${d.objectName}/${d.recordId}.${d.field}`))
174+
.toEqual(['sw_optional/o2.link', 'sw_required_cbp/r1.other_parent']);
175+
expect(recordsToDelete(plan).map((r) => `${r.object}/${r.id}`)).toEqual(['sw_optional/o1']);
176+
});
177+
});
178+
122179
describe('[#24964] the records a run deletes, and the export that must hold them', () => {
123180
const plan = {
124181
orphans: [

0 commit comments

Comments
 (0)