fix(plugin-webhooks, cli): a departed package's activation row switches no remaining webhook holder, and the ownership ceremony never picks a holder for a two-holder copy (ADR-0150 D8)

.changeset/32695-webhook-departed-row.md

@@ -0,0 +1,21 @@
+---
+'@objectstack/plugin-webhooks': patch
+'@objectstack/cli': patch
+---
+
+fix(plugin-webhooks, cli): an activation row a departed package left switches no remaining webhook holder, and the 18.x ownership ceremony writes a two-holder copy's switch as a row naming no package instead of picking one holder (ADR-0150 D8)
+
+Clause-②: yes (widening; narrowing on a departed row switched on) — widening: a departed package's row switched off no longer keeps a remaining holder off, so its outbound deliveries start on upgrade. Narrowing: a departed package's row switched on no longer turns on a holder shipped inactive.
+Metadata-protocol: none
+
+**A departed package's activation row (plugin-webhooks).** A declared webhook's slot with no activation row of its own read the name's rows naming a package that no longer declares the name as its switch while the name had one holder, and under two holders such a row switched off kept both off. Such a row was the departed package's switch, and it now switches no remaining holder, in either direction:
+
+- one holder, a departed package's row switched off: FROM the holder off TO the holder at its own row, else its declared `isActive`;
+- one holder shipped inactive, a departed package's row switched on: FROM the holder on TO the holder off (its declared default);
+- two holders, a departed package's row switched off: FROM both off TO each at its own row or its declared default.
+
+The source says so once per name at `warn`, naming the departed package, the holders and the per-holder address (`POST /api/v1/security/_activation/webhook/NAME?package=PACKAGE`). A row naming no package is read exactly as before: the one holder's switch, and under two holders off keeps every holder with no row of its own off and on turns none on. A holder's own row still wins.
+
+**Operator action on upgrade.** A webhook you switched off while another package held its name, and which a remaining package now declares, starts delivering again after this release unless that holder has its own row. To keep it off, switch the holder off at its own address: `POST /api/v1/security/_activation/webhook/NAME?package=PACKAGE` (no `package` for the environment's own). The warning above names each such webhook and its holders.
+
+**The 18.x ownership ceremony's declared-webhooks step (cli).** A customized `sys_webhook` copy writes the declaration's activation row. When the copy's definition names no package, the row named the holder the composition listed LAST for the name. Of a name one holder declares it still names that holder; of a name two or more holders declare it now names no package — the name-keyed switch the copy was — and the step's notes name every holder and the per-holder address. When the environment is one of those holders, a row naming no package would be the environment's own switch, so the step writes no switch row and its notes report the copy's switch (on or off) and name every holder; switch each at its own address. A copy whose definition names its package writes that package's row, as before.

packages/cli/src/utils/organization-ownership-apply.generation-1.test.ts

@@ -48,6 +48,16 @@ import {
   type NotNullPort,
   type OrganizationOwnershipApplyReport,
 } from './organization-ownership-apply.js';
+// Clocked windows measure behaviour, never loading: the ceremony's dynamic
+// imports are paid here, at collection (`check:test-source-alias`).
+import '@objectstack/driver-sql';
+import '@objectstack/driver-turso';
+import '@objectstack/metadata-protocol';
+import '@objectstack/platform-objects/plugin';
+import '@objectstack/plugin-security';
+import '@objectstack/rest';
+import '@objectstack/runtime';
+import '@objectstack/service-datasource';
 
 const ORG = 'org_default';
 
@@ -111,7 +121,14 @@ const RULES = ['share_custom', 'share_collides'];
 
 async function apply(
   l: ReturnType<typeof ledger> = ledger(),
-  opts: { posture?: string; webhooks?: boolean; rules?: boolean; notNull?: boolean; notNullPort?: NotNullPort } = {},
+  opts: {
+    posture?: string;
+    webhooks?: boolean;
+    webhookList?: ReadonlyArray<{ name: string; packageId: string | null }>;
+    rules?: boolean;
+    notNull?: boolean;
+    notNullPort?: NotNullPort;
+  } = {},
 ): Promise<OrganizationOwnershipApplyReport> {
   const posture = opts.posture ?? 'single';
   const database = join(dir, 'fixture.db');
@@ -129,7 +146,7 @@ async function apply(
       conflicts: reviewedConflictsOf(reviewed),
     },
     catalog: { async resolve() { return undefined; } },
-    ...(opts.webhooks === false ? {} : { declaredWebhooks: { async list() { return WEBHOOKS; } } }),
+    ...(opts.webhooks === false ? {} : { declaredWebhooks: { async list() { return opts.webhookList ?? WEBHOOKS; } } }),
     ...(opts.rules === false
       ? {}
       : { declaredSharingRules: { async rules() { return RULES.map((name) => ({ name, refused: null, packageId: 'com.example.app' })); } } }),
@@ -384,6 +401,73 @@ describe('ownership shape generation 1 — the ceremony steps an 18.0 database o
     expect(rows(`SELECT COUNT(*) AS n FROM sys_metadata_activation WHERE metadata_type = 'sharing_rule'`)).toEqual([{ n: 0 }]);
   });
 
+  /**
+   * [ADR-0150 D8, #32695] A customized copy writes the declaration's
+   * activation row, which names one holder. Of a name two packages declare,
+   * a copy whose definition names no package names none of them: its row
+   * names no package (the name-keyed switch it was), and the run says so,
+   * naming each holder. BASE: the holder listed last was picked
+   * (`new Map(list)`), so the row named whichever package the composition
+   * listed last.
+   */
+  for (const [order, webhookList] of [
+    ['listed last', [...WEBHOOKS, { name: 'deal_won', packageId: 'com.example.other' }]],
+    ['listed first', [{ name: 'deal_won', packageId: 'com.example.other' }, ...WEBHOOKS]],
+  ] as const) {
+    it(`5a [#32695] two holders (the second ${order}), a customized copy naming no package: its row names no package, said naming both`, async () => {
+      const other = 'com.example.other';
+      db.exec(`UPDATE sys_webhook SET definition_json = json_remove(definition_json, '$._packageId') WHERE id = 'whk_switched'`);
+      const report = await apply(ledger(), { webhookList });
+      const done = step(report, 'declared-webhooks');
+      expect(done.status).toBe('done');
+      expect(rows(`SELECT name, package_id, active FROM sys_metadata_activation WHERE metadata_type = 'webhook' AND name = 'deal_won'`))
+        .toEqual([{ name: 'deal_won', package_id: null, active: 1 }]);
+      const said = done.notes.filter((n) => n.includes('whk_switched') && n.includes('names no package'));
+      expect(said).toHaveLength(1);
+      expect(said[0]).toContain("'com.example.app'");
+      expect(said[0]).toContain(`'${other}'`);
+    });
+  }
+
+  /**
+   * [ADR-0150 D8, #32695 review S1] When the environment is one of the
+   * holders, a row naming no package is the environment slot's OWN switch
+   * (`webhookSlotSwitch` reads `rows.get(null)` as it), so writing one would
+   * pick the environment. Nothing is written and the run reports the copy's
+   * switch, naming each holder, in either listed order.
+   */
+  for (const [order, webhookList] of [
+    ['listed last', [...WEBHOOKS, { name: 'deal_won', packageId: null }]],
+    ['listed first', [{ name: 'deal_won', packageId: null }, ...WEBHOOKS]],
+  ] as const) {
+    it(`5a [#32695] the environment and a package hold the name (the environment ${order}), a customized copy naming no package: no switch row, said naming both`, async () => {
+      db.exec(`UPDATE sys_webhook SET definition_json = json_remove(definition_json, '$._packageId') WHERE id = 'whk_switched'`);
+      const report = await apply(ledger(), { webhookList });
+      const done = step(report, 'declared-webhooks');
+      expect(done.status).toBe('done');
+      expect(rows(`SELECT COUNT(*) AS n FROM sys_metadata_activation WHERE metadata_type = 'webhook' AND name = 'deal_won'`))
+        .toEqual([{ n: 0 }]);
+      const said = done.notes.filter((n) => n.includes('whk_switched') && n.includes('no switch row was written'));
+      expect(said).toHaveLength(1);
+      expect(said[0]).toContain('the environment');
+      expect(said[0]).toContain("'com.example.app'");
+      expect(done.notes.filter((n) => n.includes('whk_switched') && n.includes('switched off it keeps'))).toHaveLength(0);
+    });
+  }
+
+  it('5a [#32695] CONTROL a customized copy whose definition names its package writes that package\'s row, beside a second holder listed first', async () => {
+    await apply(ledger(), { webhookList: [{ name: 'deal_won', packageId: 'com.example.other' }, ...WEBHOOKS] });
+    expect(rows(`SELECT name, package_id, active FROM sys_metadata_activation WHERE metadata_type = 'webhook' AND name = 'deal_won'`))
+      .toEqual([{ name: 'deal_won', package_id: 'com.example.app', active: 1 }]);
+  });
+
+  it('5a [#32695] CONTROL one holder, a customized copy naming no package: that holder\'s row, as before', async () => {
+    db.exec(`UPDATE sys_webhook SET definition_json = json_remove(definition_json, '$._packageId') WHERE id = 'whk_switched'`);
+    await apply();
+    expect(rows(`SELECT name, package_id, active FROM sys_metadata_activation WHERE metadata_type = 'webhook' AND name = 'deal_won'`))
+      .toEqual([{ name: 'deal_won', package_id: 'com.example.app', active: 1 }]);
+  });
+
   it('5a without the declared webhooks to read, a seeded row blocks the step and nothing is deleted', async () => {
     const report = await apply(ledger(), { webhooks: false });
     expect(step(report, 'declared-webhooks')).toMatchObject({ status: 'blocked', blockers: [expect.stringMatching(/no registry to read them from/)] });

packages/cli/src/utils/organization-ownership-apply.ts

@@ -3214,6 +3214,12 @@ export interface DeclaredWebhooksPort {
   list(): Promise<ReadonlyArray<{ name: string; packageId: string | null }>>;
 }
 
+/** The package a `sys_webhook` row's own definition names (`_packageId`), or `null`. */
+function ownPackageOf(row: Record<string, unknown>): string | null {
+  const def = webhookDefinition(row);
+  return typeof def?._packageId === 'string' && def._packageId ? def._packageId : null;
+}
+
 /** A `sys_webhook` row's `definition_json`, parsed, or `null`. */
 function webhookDefinition(row: Record<string, unknown>): Record<string, unknown> | null {
   const raw = row.definition_json;
@@ -3317,7 +3323,13 @@ async function runDeclaredWebhooks(run: Run): Promise<StepReport> {
         "deployment's own composition, then re-run.",
     ]);
   }
-  const declared = new Map((await run.opts.declaredWebhooks.list()).map((d) => [d.name, d.packageId] as const));
+  // [ADR-0150 D4/D8] Every holder of each declared name, never the one listed last.
+  const declared = new Map<string, Array<string | null>>();
+  for (const d of await run.opts.declaredWebhooks.list()) {
+    const holders = declared.get(d.name) ?? [];
+    if (!holders.includes(d.packageId)) holders.push(d.packageId);
+    declared.set(d.name, holders);
+  }
   const held = rows.filter((r) => declared.has(String(r.name)));
   summary.undeclaredKept = rows.length - held.length;
   if (summary.undeclaredKept > 0) {
@@ -3373,9 +3385,41 @@ async function runDeclaredWebhooks(run: Run): Promise<StepReport> {
     const name = String(row.name);
     const id = String(row.id);
     const def = webhookDefinition(row);
-    const packageId = (typeof def?._packageId === 'string' && def._packageId ? def._packageId : null) ?? declared.get(name) ?? null;
+    // [ADR-0150 D8] The activation row a customized copy writes names the
+    // copy's own package, else the name's one holder. Of a name two or more
+    // package holders declare, a copy that names no package names none of
+    // them: its row names no package, as the name-keyed switch it was — off,
+    // it keeps every holder with no row of its own off; on, it turns none on —
+    // and the run says so, naming each holder. When the environment is one of
+    // those holders, a row naming no package IS the environment slot's own
+    // switch, so writing it would pick the environment: nothing is written and
+    // the run reports the copy's switch, naming each holder. Never the first or
+    // last one listed.
+    const holders = declared.get(name) ?? [];
+    const ownPackage = ownPackageOf(row);
+    const packageId = ownPackage ?? (holders.length === 1 ? holders[0] : null);
+    const unnamed = truthy(row.customized) && ownPackage === null && holders.length > 1;
+    const ambiguous = unnamed && holders.includes(null);
+    const holderList = holders.map((p) => (p === null ? 'the environment' : `'${p}'`)).join(', ');
+    if (ambiguous) {
+      notes.push(
+        `${table} ${id}: a customized copy of the declared webhook ${name}, which ${holders.length} holders declare ` +
+          `(${holderList}), and its definition names no package. A switch row naming no package would be the ` +
+          `environment's own, so which holder the copy's switch (${truthy(row.active) ? 'on' : 'off'}) belonged to is ` +
+          'not decided here and no switch row was written. Switch each holder at its own address ' +
+          `(POST /api/v1/security/_activation/webhook/${name}?package=PACKAGE, no package for the environment).`,
+      );
+    } else if (unnamed) {
+      notes.push(
+        `${table} ${id}: a customized copy of the declared webhook ${name}, which ${holders.length} holders declare ` +
+          `(${holderList}), and its definition names no ` +
+          'package, so its switch is written as a row naming no package: switched off it keeps each holder with no row of ' +
+          'its own off, switched on it turns none on. Switch each holder at its own address ' +
+          `(POST /api/v1/security/_activation/webhook/${name}?package=PACKAGE).`,
+      );
+    }
     if (!truthy(row.customized) || isSwitchOnlyEdit(row)) {
-      if (truthy(row.customized) && (await setActivation(run, 'webhook', name, packageId, truthy(row.active)))) summary.activationRows++;
+      if (truthy(row.customized) && !ambiguous && (await setActivation(run, 'webhook', name, packageId, truthy(row.active)))) summary.activationRows++;
       summary.deliveriesResourced += await resource(id, `${DECLARED_WEBHOOK_REF_PREFIX}${name}`);
       await deleteCopy(row);
       orphaned.push(...secretIdsOf(row));
@@ -3398,11 +3442,12 @@ async function runDeclaredWebhooks(run: Run): Promise<StepReport> {
     );
     const [landed] = await sql.read(table, `SELECT ${q('name')} AS ${q('name')} FROM ${q(table)} WHERE ${q('id')} = ?`, [row.id]);
     if (text(landed?.name) !== clone) throw new Error(`The clone of the declared webhook ${name} (${id}) did not read back as ${clone}.`);
-    if (await setActivation(run, 'webhook', name, packageId, false)) summary.activationRows++;
+    if (!ambiguous && (await setActivation(run, 'webhook', name, packageId, false))) summary.activationRows++;
     summary.clones++;
     notes.push(
       `${table} ${id}: customized beyond its switch, so it is now the administrator's own webhook ${clone} (its ` +
-        `credentials and deliveries with it), and the declared ${name} is switched off.`,
+        `credentials and deliveries with it), and the declared ${name} is ` +
+        `${ambiguous ? 'not switched (see the note naming its holders)' : 'switched off'}.`,
     );
   }
 

packages/plugins/plugin-webhooks/src/declared-webhooks.test.ts

@@ -784,11 +784,61 @@ describe('declared webhooks — one declaration per (package, name) slot (ADR-01
         expect(await switches(own)).toEqual([[A, true], [B, false]]);
     });
 
-    it('[D8] one holder: a row naming no package, or a package that no longer declares the name, is its switch, as before', async () => {
+    it('[D8] one holder: a row naming no package is its switch, as before; its own row wins', async () => {
         const one = (rows: any[]) => new DeclaredWebhookSource(engineOver(registryShipping([[A, ALPHA]]), { rows: { [LEDGER]: rows } }), undefined);
         expect(await switches(one([slotRow(null, false)]))).toEqual([[A, false]]);
-        expect(await switches(one([slotRow('com.acme.gone', false)]))).toEqual([[A, false]]);
-        expect(await switches(one([slotRow('com.acme.gone', false), slotRow(A, true)]))).toEqual([[A, true]]);
+        expect(await switches(one([slotRow(null, false), slotRow(A, true)]))).toEqual([[A, true]]);
+    });
+
+    /**
+     * [ADR-0150 D8, #32695] A row naming a package that no longer declares
+     * the name was that package's switch: it switches no remaining holder, in
+     * either direction, at any holder count, and is said once per name.
+     * BASE: under one holder it was read as that holder's switch (off turned
+     * a shipped-on holder off, on turned a shipped-off holder on); under two,
+     * off kept both off.
+     */
+    describe('[#32695] a departed package\'s row switches no remaining holder', () => {
+        const GONE = 'com.acme.gone';
+        const SHIPPED_OFF = declaredWebhook({ url: 'https://hooks.example/alpha', isActive: false });
+        const sourceOver = (packages: Array<[string, any]>, rows: any[], warn = vi.fn()) =>
+            new DeclaredWebhookSource(engineOver(registryShipping(packages), { rows: { [LEDGER]: rows } }), undefined, loggerWith(warn));
+
+        it('one holder: the departed row switched off leaves a shipped-on holder on; switched on leaves a shipped-off holder off', async () => {
+            expect(await switches(sourceOver([[A, ALPHA]], [slotRow(GONE, false)]))).toEqual([[A, true]]);
+            expect(await switches(sourceOver([[A, SHIPPED_OFF]], [slotRow(GONE, true)]))).toEqual([[A, false]]);
+        });
+
+        it('two holders, in both registration orders: a departed row switched off keeps neither off', async () => {
+            expect(await switches(sourceOver([[A, ALPHA], [B, BETA]], [slotRow(GONE, false)]))).toEqual([[A, true], [B, true]]);
+            expect(await switches(sourceOver([[B, BETA], [A, ALPHA]], [slotRow(GONE, false)]))).toEqual([[B, true], [A, true]]);
+        });
+
+        it('said once per name, naming the departed package and the holders, with the per-holder address', async () => {
+            const warn = vi.fn();
+            const source = sourceOver([[A, ALPHA]], [slotRow(GONE, false)], warn);
+            await source.list();
+            await source.list();
+            const said = warn.mock.calls.filter(([msg]) => String(msg).includes('no longer declares it'));
+            expect(said).toHaveLength(1);
+            expect(said[0][0]).toContain(`package "${GONE}"`);
+            expect(said[0][0]).toContain(`"${A}"`);
+            expect(said[0][0]).toContain('?package=PACKAGE');
+        });
+
+        it('two holders, a departed row and a row naming no package: each is told once (review N1)', async () => {
+            const warn = vi.fn();
+            const source = sourceOver([[A, ALPHA], [B, BETA]], [slotRow(GONE, false), slotRow(null, false)], warn);
+            await source.list();
+            await source.list();
+            expect(warn.mock.calls.filter(([msg]) => String(msg).includes('no longer declares it'))).toHaveLength(1);
+            expect(warn.mock.calls.filter(([msg]) => String(msg).includes('no holder\'s own (naming no package)'))).toHaveLength(1);
+        });
+
+        it('CONTROL beside the departed row, a row naming no package is still the one holder\'s switch, and its own row wins over both', async () => {
+            expect(await switches(sourceOver([[A, ALPHA]], [slotRow(GONE, true), slotRow(null, false)]))).toEqual([[A, false]]);
+            expect(await switches(sourceOver([[A, ALPHA]], [slotRow(GONE, false), slotRow(null, false), slotRow(A, true)]))).toEqual([[A, true]]);
+        });
     });
 
     it('[Q4] a delivery naming no package (`declared:NAME`, the CLI ceremony\'s shape): one holder judges it as before; two refuse it naming both, sent to neither', async () => {