fix(cli): os db orphans sweeps details whose controlled-by-parent master is optional

.changeset/30976-orphans-optional-master.md

@@ -0,0 +1,10 @@
+---
+'@objectstack/cli': patch
+---
+
+fix(cli): `os db orphans` sweeps details whose controlled-by-parent master is optional
+
+Clause-②: no
+Metadata-protocol: none
+
+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

@@ -82,7 +82,7 @@ predicate/schema/binding mistakes that fail silently at runtime), and `os dev --
 | `os dev [package]` | | Start development mode with hot reload |
 | `os serve [config]` | | Start the ObjectStack server with plugin auto-detection |
 | `os db clean` | | Reclaim SQLite free space with a one-time `VACUUM` (ADR-0057) |
-| `os db orphans` | | Report detail rows whose required master-detail master does not exist; delete them only with `--delete` |
+| `os db orphans` | | Report detail rows whose master-detail reference names a master that does not exist; delete them only with `--delete` |
 
 #### `os init`
 
@@ -445,8 +445,11 @@ os db clean --database file:./data/app.db        # explicit target
 
 #### `os db orphans`
 
-Reports the detail rows whose **required** `master_detail` master does not exist, and —
-only behind `--delete` — deletes them. **Report-only by default: without `--delete` it
+Reports the detail rows whose `master_detail` master does not exist, and — only behind
+`--delete` — deletes them. The master is a **required** `master_detail`, or the
+**optional** master of a `controlled_by_parent` object (its one `master_detail`, declared
+`required: false`); a row whose optional master is empty names no master, is valid, and
+is never listed. **Report-only by default: without `--delete` it
 writes no row or table and creates no file.** It is never run for you; nothing on any
 boot, upgrade, migrate or lifecycle path invokes it.
 
@@ -459,7 +462,7 @@ where an operator acts on that report.
 The report lists each orphan's object, record id, field and missing master id, and the
 deeper detail rows its delete removes with it (the master-detail's own cascade). It
 also names, without ever deleting them, the dangling references on those objects that
-are not a required master-detail, and counts the references whose target could not be
+are not on such a master, and counts the references whose target could not be
 checked at all: unknown is not missing, and none of those rows is listed or deleted.
 
 ```bash

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

@@ -20,7 +20,11 @@
  *    reference cannot be probed, so it is UNDETERMINED, never an orphan;
  *  - a plain lookup naming no campaign, on the live member — dangling, listed,
  *    never deleted, and no reason to delete the row it sits on;
- *  - an object with no required master-detail at all — not read.
+ *  - an object with no required master-detail at all — not read;
+ *  - [#30976] tips, `controlled_by_parent` children of a campaign through an
+ *    OPTIONAL master (`required: false`, ruling A of #30905): one names no
+ *    campaign (valid, never listed), one names the live campaign, and one names
+ *    the missing campaign — an orphan like the member, deleted with it.
  *
  * ## What each case proves
  *
@@ -47,6 +51,15 @@ import { isExitSignal } from '../../utils/format.js';
 import { bootSchemaStack } from '../../utils/schema-migrate.js';
 import DbOrphans from './orphans.js';
 
+// Pay the first load of what `bootSchemaStack` and the command reach through
+// dynamic `import()`s at MODULE LOAD, outside any clocked case.
+import '@objectstack/service-datasource';
+import '@objectstack/driver-turso';
+import '@objectstack/runtime';
+import '@objectstack/platform-objects/plugin';
+import '@objectstack/plugin-security';
+import '@objectstack/rest';
+
 const HERE = dirname(fileURLToPath(import.meta.url));
 const CLI_ROOT = resolve(HERE, '..', '..', '..');
 const CASE_TIMEOUT_MS = 120_000;
@@ -94,6 +107,12 @@ const ARTIFACT = {
       name: 'od_ref',
       fields: { name: { type: 'text' }, campaign_ref: { type: 'lookup', reference: 'od_campaign' } },
     },
+    {
+      // [#30976] An OPTIONAL controlled-by-parent master: a row may name no campaign.
+      name: 'od_tip',
+      sharingModel: 'controlled_by_parent',
+      fields: { body: { type: 'text' }, campaign: { type: 'master_detail', reference: 'od_campaign', required: false } },
+    },
   ],
 };
 
@@ -121,7 +140,7 @@ async function idsOf(dbFile: string): Promise<Record<string, string[]>> {
   const k = (driver as any).knex;
   try {
     const out: Record<string, string[]> = {};
-    for (const table of ['od_campaign', 'od_member', 'od_member_note', 'od_ghost_member', 'od_hold', 'od_ref']) {
+    for (const table of ['od_campaign', 'od_member', 'od_member_note', 'od_ghost_member', 'od_hold', 'od_ref', 'od_tip']) {
       out[table] = ((await k(table).select('id').orderBy('id')) as Array<{ id: string }>).map((r) => r.id);
     }
     return out;
@@ -212,6 +231,9 @@ beforeAll(async () => {
     await seedRow(ql, 'od_member_note', { ...ORG, id: 'n_orphan', body: 'goes with its member', member: 'm_orphan' });
     await seedRow(ql, 'od_ghost_member', { ...ORG, id: 'g_row', name: 'unknown master', ghost: 'nobody_knows' });
     await seedRow(ql, 'od_ref', { ...ORG, id: 'r_row', name: 'dangling lookup', campaign_ref: 'Gone Campaign' });
+    await seedRow(ql, 'od_tip', { ...ORG, id: 't_none', body: 'names no campaign' });
+    await seedRow(ql, 'od_tip', { ...ORG, id: 't_live', body: 'kept', campaign: 'c_live' });
+    await seedRow(ql, 'od_tip', { ...ORG, id: 't_orphan', body: 'orphan', campaign: MISSING_CAMPAIGN });
   } finally {
     await stack.shutdown();
   }
@@ -224,6 +246,7 @@ beforeAll(async () => {
     od_ghost_member: ['g_row'],
     od_hold: [],
     od_ref: ['r_row'],
+    od_tip: ['t_live', 't_none', 't_orphan'],
   });
 }, 180_000);
 
@@ -248,13 +271,21 @@ describe('[#24964] os db orphans — the default run is a report that writes not
 
     expect(exitCode).toBe(0);
     expect(payload.mode).toBe('report');
-    expect(payload.plan.orphans).toEqual([{
-      object: 'od_member',
-      recordId: 'm_orphan',
-      fields: [{ field: 'campaign', master: 'od_campaign', missingMasterId: MISSING_CAMPAIGN }],
-      cascade: [{ object: 'od_member_note', id: 'n_orphan' }],
-    }]);
-    expect(payload.plan.counts).toMatchObject({ orphans: 1, deeperDetails: 1, withheld: 0 });
+    expect(payload.plan.orphans).toEqual([
+      {
+        object: 'od_member',
+        recordId: 'm_orphan',
+        fields: [{ field: 'campaign', master: 'od_campaign', missingMasterId: MISSING_CAMPAIGN }],
+        cascade: [{ object: 'od_member_note', id: 'n_orphan' }],
+      },
+      {
+        object: 'od_tip',
+        recordId: 't_orphan',
+        fields: [{ field: 'campaign', master: 'od_campaign', missingMasterId: MISSING_CAMPAIGN }],
+        cascade: [],
+      },
+    ]);
+    expect(payload.plan.counts).toMatchObject({ orphans: 2, deeperDetails: 1, withheld: 0 });
     // Zero writes: no row, no table, and no file beside the database.
     expect(await readState(c.dbFile)).toEqual(before);
     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
     // An object with no required master-detail is not this command's to read.
     expect(payload.plan.objects).not.toContain('od_ref');
   }, CASE_TIMEOUT_MS);
+
+  it('[#30976] an optional master: one naming a missing campaign is an orphan; an empty one is valid and never listed', async () => {
+    const c = prepareCase();
+
+    const { payload } = await runJson(['--database-url', c.url, '--json']);
+
+    // The scan read the optional-master object.
+    expect(payload.plan.objects).toContain('od_tip');
+    const listed = [
+      ...payload.plan.orphans.map((o: { object: string; recordId: string }) => `${o.object}/${o.recordId}`),
+      ...payload.plan.otherDangling.map((d: { objectName: string; recordId: string }) => `${d.objectName}/${d.recordId}`),
+    ];
+    expect(payload.plan.orphans.map((o: { object: string; recordId: string }) => `${o.object}/${o.recordId}`))
+      .toContain('od_tip/t_orphan');
+    // A tip that names no campaign, and one that names the live campaign, are listed nowhere.
+    expect(listed).not.toContain('od_tip/t_none');
+    expect(listed).not.toContain('od_tip/t_live');
+  }, CASE_TIMEOUT_MS);
 });
 
 describe('[#24964] os db orphans --delete', () => {
@@ -291,11 +340,14 @@ describe('[#24964] os db orphans --delete', () => {
 
     expect(payload.error, JSON.stringify(payload).slice(0, 400)).toBeUndefined();
     expect(exitCode).toBe(0);
-    expect(payload.deleted).toEqual([expect.objectContaining({
-      object: 'od_member',
-      recordId: 'm_orphan',
-      removed: [{ object: 'od_member', id: 'm_orphan' }, { object: 'od_member_note', id: 'n_orphan' }],
-    })]);
+    expect(payload.deleted).toEqual([
+      expect.objectContaining({
+        object: 'od_member',
+        recordId: 'm_orphan',
+        removed: [{ object: 'od_member', id: 'm_orphan' }, { object: 'od_member_note', id: 'n_orphan' }],
+      }),
+      expect.objectContaining({ object: 'od_tip', recordId: 't_orphan', removed: [{ object: 'od_tip', id: 't_orphan' }] }),
+    ]);
     // The roll-up on the master that does not exist cannot be recomputed; the run says so, and deleted anyway.
     expect(payload.deleted[0].notes.join(' ')).toMatch(/od_campaign\.member_total was not recomputed for Spring Promo/);
     expect(payload.failed).toEqual([]);
@@ -307,12 +359,14 @@ describe('[#24964] os db orphans --delete', () => {
       od_ghost_member: ['g_row'],
       od_hold: [],
       od_ref: ['r_row'],
+      // The tip that names no campaign stays, with the one whose campaign is live.
+      od_tip: ['t_live', 't_none'],
     });
 
     const doc = JSON.parse(readFileSync(exportPath, 'utf8'));
     expect(doc.format).toBe('objectstack.orphan-detail.pre-delete-export.v1');
     expect(doc.rows.map((r: { object: string; id: string }) => `${r.object}/${r.id}`)).toEqual([
-      'od_member/m_orphan', 'od_member_note/n_orphan',
+      'od_member/m_orphan', 'od_member_note/n_orphan', 'od_tip/t_orphan',
     ]);
     expect(doc.rows[0].row).toMatchObject({ id: 'm_orphan', name: 'orphan', campaign: MISSING_CAMPAIGN });
     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', () => {
 
     // Non-vacuity: the command reached its delete through the intercepted method.
     expect(seenAtFirstDelete).toBeDefined();
-    expect(seenAtFirstDelete).toEqual(['od_member/m_orphan', 'od_member_note/n_orphan']);
+    expect(seenAtFirstDelete).toEqual(['od_member/m_orphan', 'od_member_note/n_orphan', 'od_tip/t_orphan']);
     expect(exitCode).toBe(1);
     expect(payload.failed).toEqual([expect.objectContaining({ object: 'od_member', recordId: 'm_orphan', code: 'INJECTED_REFUSAL' })]);
     // Refused, so nothing went.
@@ -364,7 +418,8 @@ describe('[#24964] os db orphans --delete — the delete is the engine\'s own',
     const { payload, exitCode } = await runJson(['--delete', '--export', exportPath, '--yes', '--database-url', c.url, '--json']);
 
     expect(exitCode).toBe(1);
-    expect(payload.deleted).toEqual([]);
+    // The other orphan, which nothing restricts, is deleted as before.
+    expect(payload.deleted).toEqual([expect.objectContaining({ object: 'od_tip', recordId: 't_orphan' })]);
     expect(payload.failed).toEqual([expect.objectContaining({
       object: 'od_member', recordId: 'm_orphan', code: 'DELETE_RESTRICTED',
       remaining: [{ object: 'od_member', id: 'm_orphan' }, { object: 'od_member_note', id: 'n_orphan' }],

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

@@ -29,8 +29,10 @@ import type {
 } from '../../utils/orphan-detail-sweep.js';
 
 /**
- * `os db orphans` — the operator's door to the detail rows whose required
- * master-detail master does not exist (#24964).
+ * `os db orphans` — the operator's door to the detail rows whose master does
+ * not exist (#24964): a required master-detail, or the optional master of a
+ * controlled-by-parent object (#30976). A row of the latter that names no
+ * master is valid and is never listed.
  *
  * ## Why such a row needs a door, and why this is the one
  *
@@ -49,9 +51,8 @@ import type {
  *  1. An explicit operator command. ⛔ Nothing on any boot, upgrade, migrate
  *     or lifecycle-sweep path invokes it, and it must not grow such a caller
  *     (`orphans.no-auto-run.test.ts` holds it).
- *  2. The default run is a dry-run report: each detail row whose required
- *     master-detail master does not exist — object, record id, field, and the
- *     missing master id.
+ *  2. The default run is a dry-run report: each detail row whose master
+ *     does not exist — object, record id, field, and the missing master id.
  *  3. Deletion only behind `--delete`, and only of the rows the same run's
  *     report lists.
  *  4. A mandatory pre-delete export of every row the run deletes, written and
@@ -93,7 +94,7 @@ import type {
  */
 export default class DbOrphans extends Command {
   static override description =
-    'Report detail rows whose required master-detail master does not exist, and (only with --delete) delete them '
+    'Report detail rows whose master-detail reference names a master that does not exist, and (only with --delete) delete them '
     + 'with their cascade, after writing a pre-delete export. Report-only by default: it writes no row or table.';
 
   static override examples = [
@@ -410,7 +411,7 @@ async function confirm(question: string): Promise<boolean> {
 
 /** Render the plan for a human. */
 function renderPlan(plan: OrphanDetailPlan): void {
-  console.log(chalk.bold('\n  Detail rows whose required master does not exist'));
+  console.log(chalk.bold('\n  Detail rows whose master does not exist'));
   printInfo(
     `scanned ${plan.scanned} row(s) of ${plan.objects.length} object(s) · orphans ${plan.counts.orphans} · `
     + `deeper details ${plan.counts.deeperDetails} · withheld ${plan.counts.withheld}`,

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

@@ -13,7 +13,8 @@ import { describe, it, expect, vi } from 'vitest';
 import type { DanglingReference, DanglingReferenceReport } from '@objectstack/objectql';
 import {
   buildOrphanDetailExport,
-  objectsWithRequiredMaster,
+  masterFieldsOf,
+  objectsWithMaster,
   planOrphanDetailSweep,
   recordKey,
   recordsToDelete,
@@ -33,10 +34,34 @@ const CONFIGS = {
   },
   sw_absent: { name: 'sw_absent', fields: { master: { type: 'master_detail', reference: 'sw_master', required: true } } },
   sw_plain: { name: 'sw_plain', fields: { link: { type: 'lookup', reference: 'sw_master' } } },
+  // [#30976] A controlled-by-parent object whose one master is OPTIONAL (ruling A of #30905).
+  sw_optional: {
+    name: 'sw_optional',
+    sharingModel: 'controlled_by_parent',
+    fields: {
+      master: { type: 'master_detail', reference: 'sw_master', required: false },
+      link: { type: 'lookup', reference: 'sw_master' },
+    },
+  },
+  // The same lone optional `master_detail` on an object that is not controlled by a parent: no master.
+  sw_optional_private: {
+    name: 'sw_optional_private',
+    sharingModel: 'private',
+    fields: { master: { type: 'master_detail', reference: 'sw_master', required: false } },
+  },
+  // Controlled by a parent through its REQUIRED master; the optional `master_detail` beside it is an ordinary parent.
+  sw_required_cbp: {
+    name: 'sw_required_cbp',
+    sharingModel: 'controlled_by_parent',
+    fields: {
+      master: { type: 'master_detail', reference: 'sw_master', required: true },
+      other_parent: { type: 'master_detail', reference: 'sw_master', required: false },
+    },
+  },
 };
 
-const finding = (recordId: string, field: string, value: string): DanglingReference =>
-  ({ objectName: 'sw_detail', recordId, field, target: 'sw_master', value });
+const finding = (recordId: string, field: string, value: string, objectName = 'sw_detail'): DanglingReference =>
+  ({ objectName, recordId, field, target: 'sw_master', value });
 
 function report(dangling: DanglingReference[], extra: Partial<DanglingReferenceReport> = {}): DanglingReferenceReport {
   return {
@@ -58,15 +83,17 @@ function engineWith(rep: DanglingReferenceReport, cascade: (object: string, id:
 const rootOnly = async (object: string, id: string | number) => [{ object, id: String(id) }];
 
 describe('[#24964] planOrphanDetailSweep', () => {
-  it('reads only the objects that declare a REQUIRED master-detail, and not a table the boot measured absent', async () => {
-    expect(objectsWithRequiredMaster(CONFIGS)).toEqual(['sw_detail', 'sw_absent']);
+  it('reads only the objects that declare a master, and not a table the boot measured absent', async () => {
+    expect(objectsWithMaster(CONFIGS)).toEqual(['sw_detail', 'sw_absent', 'sw_optional', 'sw_required_cbp']);
     const { engine, inspectDanglingReferences } = engineWith(report([]), rootOnly);
 
     const plan = await planOrphanDetailSweep({ engine, maxRows: 50, absent: (o) => o === 'sw_absent' });
 
-    expect(inspectDanglingReferences).toHaveBeenCalledWith({ objects: ['sw_detail'], rowsPerObject: 50, maxRows: 50 });
+    expect(inspectDanglingReferences).toHaveBeenCalledWith({
+      objects: ['sw_detail', 'sw_optional', 'sw_required_cbp'], rowsPerObject: 50, maxRows: 50,
+    });
     expect(plan.notRead).toEqual(['sw_absent']);
-    expect(plan.objects).toEqual(['sw_detail']);
+    expect(plan.objects).toEqual(['sw_detail', 'sw_optional', 'sw_required_cbp']);
   });
 
   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', () => {
   });
 });
 
+describe('[#30976] the optional master of a controlled-by-parent object', () => {
+  it('is a master field; a lone optional master-detail elsewhere, or one beside a required master, is not', () => {
+    expect(masterFieldsOf(CONFIGS.sw_optional)).toEqual(['master']);
+    expect(masterFieldsOf(CONFIGS.sw_optional_private)).toEqual([]);
+    expect(masterFieldsOf(CONFIGS.sw_required_cbp)).toEqual(['master']);
+    // CONTROL: the required masters read as they did before the optional master existed.
+    expect(masterFieldsOf(CONFIGS.sw_detail)).toEqual(['master', 'second']);
+  });
+
+  it('a dangling optional master makes its row an orphan; a dangling lookup on the same object is named, never deletable', async () => {
+    const { engine } = engineWith(report([
+      finding('o1', 'master', 'gone_a', 'sw_optional'),
+      finding('o2', 'link', 'gone_b', 'sw_optional'),
+      finding('r1', 'other_parent', 'gone_c', 'sw_required_cbp'),
+    ]), rootOnly);
+
+    const plan = await planOrphanDetailSweep({ engine, maxRows: 50 });
+
+    expect(plan.orphans).toEqual([{
+      object: 'sw_optional',
+      recordId: 'o1',
+      fields: [{ field: 'master', master: 'sw_master', missingMasterId: 'gone_a' }],
+      cascade: [],
+    }]);
+    expect(plan.otherDangling.map((d) => `${d.objectName}/${d.recordId}.${d.field}`))
+      .toEqual(['sw_optional/o2.link', 'sw_required_cbp/r1.other_parent']);
+    expect(recordsToDelete(plan).map((r) => `${r.object}/${r.id}`)).toEqual(['sw_optional/o1']);
+  });
+});
+
 describe('[#24964] the records a run deletes, and the export that must hold them', () => {
   const plan = {
     orphans: [