fix(plugin-webhooks, runtime): webhook remedies name the ?package= address, and a name refused whole answers 409 at every address

.changeset/33263-webhook-remedies-exact-address.md

@@ -0,0 +1,29 @@
+---
+'@objectstack/plugin-webhooks': patch
+'@objectstack/runtime': patch
+'@objectstack/plugin-security': patch
+---
+
+fix(plugin-webhooks, runtime): declared-webhook remedies name each holder's exact address, and a name refused whole is refused at every address naming both holders
+
+Clause-②: no
+Metadata-protocol: none
+
+Remedy text and refusals for declared webhooks (ADR-0150 D2/D3/D5/D8):
+
+| Where | FROM | TO |
+|:---|:---|:---|
+| Name guard (`sys_webhook` create or rename onto a declared name) | names `POST /api/v1/security/_activation/webhook/NS/NAME` per holder, which answers `409` for two co-owners of one namespace and does not exist for a package that declares no namespace | names `POST /api/v1/security/_activation/webhook/NAME?package=PACKAGE` per holder, which switches exactly that holder |
+| Redelivery veto, switched-off declared webhook | `switch it on (POST .../webhook/NS/NAME ...)` | `switch it on (POST .../webhook/NAME?package=PACKAGE ...)` |
+| Dispatcher warning, a declared webhook withheld by a `sys_webhook` row | namespace address per holder | `?package=PACKAGE` address per holder |
+| Dispatcher warning, a row naming no package under two holders | `.../webhook/<namespace>/NAME` | `.../webhook/NAME?package=PACKAGE`, as the departed-package warning already said |
+| Metadata service lists the environment's declaration beside a package's (no registry item): bare refusal sentence | "Nothing picks one of them: address it by its package." | says neither declaration is dispatched and no address switches one, names both holders, and says to remove or rename declarations |
+| Same state, `POST .../webhook/NAME?package=PACKAGE` and `POST .../webhook/NS/NAME` | `404` "no enabled package declares the name" | `409 RESOURCE_CONFLICT`, the refusal above, `candidates` naming the environment and the package; nothing written |
+| Same state, name guard | names a `<namespace>` placeholder address | says no declaration of the name is dispatched and quotes the refusal |
+| Same state, redelivery of a `declared:PACKAGE/NAME` delivery | "the package is disabled ... Enable the package" | refused with the refusal, naming both holders |
+| `?package=` or namespace address when the reader serves no slot and the bare read fails | `404` | `503 SERVICE_UNAVAILABLE`, nothing written |
+| Upgrade impact report, withheld-webhook line and `packageDisabled` doc | a disabled package's address "answers 409" | refuses the switch: `404` while the package is the name's only holder, `409` naming the other holders while another package declares it |
+
+A name that one package, or two packages each once, declare through the metadata service is unchanged: each served holder is switched at its own `?package=` address. A disabled sole holder still answers `404` at its address.
+
+No action is needed. Operators who scripted the namespace address keep it working where it worked; the `?package=PACKAGE` address the messages now name works for every package holder.

content/docs/automation/webhooks.mdx

@@ -97,7 +97,10 @@ A webhook reaches the dispatcher one of two ways:
   cannot tell apart (two packages sharing one namespace, or a package that
   declares no namespace beside another holder) are each switched by the exact
   qualifier, `POST /api/v1/security/_activation/webhook/:name?package=<package>`.
-  It writes that holder's
+  An activation row naming a package that no longer declares the name switches
+  no remaining holder (ADR-0150 D8), so to keep the webhook off, switch each
+  holder off at `POST /api/v1/security/_activation/webhook/:name?package=<package>`.
+  Each switch writes that holder's
   own `sys_metadata_activation` row of type `webhook` and overrides the
   declared `isActive` in either direction
   (ADR-0131 D6, regime C — the door, its authority tiers and its refusals are

packages/plugins/plugin-security/src/upgrade-impact-report.test.ts

@@ -437,12 +437,17 @@ describe('webhook: a sys_webhook row holding a declared name', () => {
       });
       const r = await boot(d, 'walled');
       expect(r.report.withheldWebhooks.map((w) => w.packageId).sort()).toEqual([OTHER_VENDOR, VENDOR].sort());
-      // [#33016 S2] The disabled holder's address answers 409 until it is
-      // enabled again: its finding says so, and the line says to enable first.
+      // [#33016 S2] The disabled holder's address refuses the switch until it
+      // is enabled again: its finding says so, and the line says to enable
+      // first. [#33263, record nit N5] The refusal is 404 for a sole holder
+      // and 409 beside another (the door's own pins in runtime's
+      // `catalog-activation-door.test.ts`); the line said 409 for both.
       expect(r.report.withheldWebhooks.map((w) => [w.packageId, w.packageDisabled ?? false]).sort())
         .toEqual([[order[0], false], [order[1], true]].sort());
       const line = errorLines(r).find((m) => m.includes('sys_webhook row(s) hold the name of a webhook')) ?? '';
       expect(line).toContain('A finding marked packageDisabled names a disabled package');
+      expect(line).toContain("(404 while the package is the name's only holder, 409 naming the other holders while another package declares it too)");
+      expect(line).not.toContain('answers 409 until');
       expect(line).toContain('enable the package first, then switch it there');
     });
   }

packages/plugins/plugin-security/src/upgrade-impact-report.ts

@@ -281,7 +281,10 @@ export interface WithheldDeclaredWebhook {
   organizationId: string | null;
   /**
    * [#33016] Set when that package is disabled: it holds nothing for the
-   * activation door until it is enabled again, so its address answers `409`.
+   * activation door until it is enabled again, so its address refuses the
+   * switch — `404` while it is the name's only holder, `409` naming the
+   * holders the door serves while another package declares the name too
+   * (#33263, record nit N5: this said `409` for both).
    */
   packageDisabled?: true;
 }
@@ -866,7 +869,9 @@ const CATEGORY_LINES: Record<FindingKey, (count: number) => string> = {
     'delete the row to let the declaration answer; to keep the row as well, rename it, and switch the declared ' +
     `one off if it must not fire (${WEBHOOK_SWITCH_DOOR}, with { "enabled": false }; the package is the one the ` +
     'finding names). A finding marked packageDisabled names a disabled package: it does not fire now, and that ' +
-    'address answers 409 until the package is enabled again, so enable the package first, then switch it there.',
+    'address refuses the switch until the package is enabled again (404 while the package is the name\'s only ' +
+    'holder, 409 naming the other holders while another package declares it too), so enable the package first, ' +
+    'then switch it there.',
   overlayOrphans: (n) =>
     `[security] upgrade impact: ${n} customization(s) of a view, dashboard, report, translation or email template ` +
     'are bound to a package that no longer ships the item, so each is served as an item of its own beside ' +

packages/plugins/plugin-webhooks/src/auto-enqueuer.ts

@@ -593,7 +593,7 @@ export class AutoEnqueuer {
             if (heldByRow.has(name)) {
                 this.reportDeclarationWithheld(
                     name,
-                    declared.filter((s) => s.webhook.name === name).map((s) => declaredWebhookActivationPath(name, s.namespace)),
+                    declared.filter((s) => s.webhook.name === name).map((s) => declaredWebhookActivationPath(name, s.packageId)),
                 );
                 continue;
             }

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

@@ -33,6 +33,7 @@ import {
     WEBHOOK_ACTIVATION_TYPE,
     bindDeclaredWebhookNameGuard,
     declaredWebhookRefId,
+    wholeNameRefusal,
 } from './declared-webhooks.js';
 import { SchemaRegistry } from '@objectstack/objectql';
 import { createWebhookRedeliverGuard } from './redeliver-guard.js';
@@ -772,6 +773,9 @@ describe('declared webhooks — one declaration per (package, name) slot (ADR-01
         expect(said).toHaveLength(1);
         expect(said[0][0]).toContain(`"${A}"`);
         expect(said[0][0]).toContain(`"${B}"`);
+        // [#33263] Each holder's exact address, as the departed-row line says it.
+        expect(said[0][0]).toContain('POST /api/v1/security/_activation/webhook/task_changed?package=PACKAGE.');
+        expect(said[0][0]).not.toContain('namespace>');
 
         const shippedOff = engineOver(
             registryShipping([[A, ALPHA], [B, declaredWebhook({ url: 'https://hooks.example/beta', isActive: false })]]),
@@ -1042,25 +1046,54 @@ describe('declared webhooks — one declaration per (package, name) slot (ADR-01
                 new DeclaredWebhookSource(engineOver(registry), undefined));
             const err = await hooks.beforeInsert({ input: { data: { name: T } }, session: {} }).then(() => undefined, (e: unknown) => e);
             expect(err).toBeInstanceOf(DeclaredWebhookNameConflictError);
-            expect((err as Error).message).toContain(`POST /api/v1/security/_activation/webhook/alpha/${T} with`);
-            expect((err as Error).message).not.toContain('/beta/');
+            expect((err as Error).message).toContain(`POST /api/v1/security/_activation/webhook/${T}?package=${A} with`);
+            expect((err as Error).message).not.toContain(B);
         });
     });
 
-    // [#31699 round 2, R2-D] A remedy names the slot's namespace address.
-    it('the remedies name the namespace address: the name guard each holder\'s, the redelivery veto the slot\'s', async () => {
+    // [#31699 round 2, R2-D; #33263] A remedy names the slot's exact address,
+    // `?package=PKG`: it named the namespace address, which refuses two
+    // co-owners of one namespace (the pin below).
+    it('[FROM namespace address TO ?package=] the remedies name each holder\'s exact address: the name guard each holder\'s, the redelivery veto the slot\'s', async () => {
         const hooks: Record<string, (ctx: any) => Promise<void>> = {};
         bindDeclaredWebhookNameGuard({ registerHook: (event, handler) => { hooks[event] = handler; } },
             new DeclaredWebhookSource(twoHolders(), undefined));
         const conflict = await hooks.beforeInsert({ input: { data: { name: 'task_changed' } }, session: {} })
             .then(() => undefined, (e: unknown) => (e as Error).message);
-        expect(conflict).toContain('switch the declared ones off (POST /api/v1/security/_activation/webhook/alpha/task_changed, '
-            + 'POST /api/v1/security/_activation/webhook/beta/task_changed, each with { "enabled": false })');
+        expect(conflict).toContain(`switch the declared ones off (POST /api/v1/security/_activation/webhook/task_changed?package=${A}, `
+            + `POST /api/v1/security/_activation/webhook/task_changed?package=${B}, each with { "enabled": false })`);
 
         const engine = twoHolders({ [LEDGER]: [slotRow(A, false)] });
         const guard = createWebhookRedeliverGuard(engine as any, 'sys_webhook', new DeclaredWebhookSource(engine, undefined));
         expect(await guard({ source: 'webhook', refId: declaredWebhookRefId('task_changed', A) }))
-            .toContain('(POST /api/v1/security/_activation/webhook/alpha/task_changed with { "enabled": true })');
+            .toContain(`(POST /api/v1/security/_activation/webhook/task_changed?package=${A} with { "enabled": true })`);
+    });
+
+    // [#33263] Two co-owners of one namespace: the namespace address refuses
+    // both (`catalog-activation-door.test.ts`), so each remedy names the
+    // holder's own exact address, never the shared namespace's.
+    it('two co-owners of one namespace: the name guard names each holder\'s exact address, the redelivery veto its slot\'s', async () => {
+        const registry = new SchemaRegistry();
+        registry.logLevel = 'silent';
+        for (const id of [A, B]) {
+            registry.installPackage({ id, name: id, version: '1.0.0', type: 'app', namespace: 'shared' } as never, 'code',
+                undefined, { packageIds: [A, B] });
+            registry.registerItem('webhook', { ...(id === A ? ALPHA : BETA) }, 'name' as never, id);
+        }
+        const hooks: Record<string, (ctx: any) => Promise<void>> = {};
+        bindDeclaredWebhookNameGuard({ registerHook: (event, handler) => { hooks[event] = handler; } },
+            new DeclaredWebhookSource(engineOver(registry), undefined));
+        const conflict = await hooks.beforeInsert({ input: { data: { name: 'task_changed' } }, session: {} })
+            .then(() => undefined, (e: unknown) => (e as Error).message);
+        expect(conflict).toContain(`POST /api/v1/security/_activation/webhook/task_changed?package=${A}, `
+            + `POST /api/v1/security/_activation/webhook/task_changed?package=${B}, each with`);
+        expect(conflict).not.toContain('/shared/');
+
+        const engine = engineOver(registry, { rows: { [LEDGER]: [slotRow(B, false)] } });
+        const verdict = await createWebhookRedeliverGuard(engine as any, 'sys_webhook', new DeclaredWebhookSource(engine, undefined))(
+            { source: 'webhook', refId: declaredWebhookRefId('task_changed', B) });
+        expect(verdict).toContain(`(POST /api/v1/security/_activation/webhook/task_changed?package=${B} with { "enabled": true })`);
+        expect(verdict).not.toContain('/shared/');
     });
 
     it('CONTROL — the environment\'s own declaration keeps the bare address in both remedies', async () => {
@@ -1077,3 +1110,87 @@ describe('declared webhooks — one declaration per (package, name) slot (ADR-01
             .toContain('switch the declared one off (POST /api/v1/security/_activation/webhook/task_changed with { "enabled": false })');
     });
 });
+
+// ---------------------------------------------------------------------------
+// 5. A name the metadata service lists for the environment beside a package —
+//    refused WHOLE, said so at every reader (ADR-0150 D3, #33263)
+// ---------------------------------------------------------------------------
+
+/**
+ * On a host whose registry holds no webhook, the metadata service lists the
+ * environment's own declaration beside a package's of the same name. Nothing
+ * says whether the environment's is that package's customization or a webhook
+ * of its own, so the name is refused whole: neither is dispatched, and no
+ * address switches either. BASE: the bare refusal said "address it by its
+ * package", the door's `?package=` then answered 404 "no enabled package
+ * declares the name", the name guard named a namespace placeholder and the
+ * redelivery veto said the package was disabled. Each now says the one true
+ * thing and names both holders.
+ */
+describe('declared webhooks — a name the metadata service lists for the environment beside a package (ADR-0150 D3, #33263)', () => {
+    const PKG = 'com.acme.alpha';
+    const T = 'task_changed';
+    const listing = (...items: any[]) => ({ list: (type: string) => (type === 'webhook' ? items : []) });
+    const envBesidePackage = () => new DeclaredWebhookSource(
+        newEngine(),
+        listing(declaredWebhook({ url: 'https://hooks.example/env' }), { ...declaredWebhook({ url: 'https://hooks.example/pkg' }), _packageId: PKG }),
+    );
+    const refusalOf = async (source: DeclaredWebhookSource) =>
+        (await source.declaredWebhookNamed(T).then(() => undefined, (e: unknown) => e) as DeclaredWebhookAmbiguousError).refusal;
+
+    it('[FROM "address it by its package" TO "no address switches one"] the bare refusal names both holders and says neither is dispatched', async () => {
+        const source = envBesidePackage();
+        expect(await source.list()).toEqual([]);
+        const refusal = await refusalOf(source);
+        expect(refusal.candidates).toEqual([{ kind: 'environment' }, { kind: 'package', packageId: PKG }]);
+        expect(refusal.message).toContain('the environment (an item authored in this environment, bound to no package)');
+        expect(refusal.message).toContain(`package "${PKG}"`);
+        expect(refusal.message).toContain('No declaration of it is dispatched, and no address switches one');
+        expect(refusal.message).not.toContain('address it by its package');
+        expect(await wholeNameRefusal(source, T)).toEqual(refusal);
+    });
+
+    it('CONTROL — two packages the metadata service lists once each are served: the bare refusal sends the caller to each package, and no whole refusal stands', async () => {
+        const source = new DeclaredWebhookSource(newEngine(), listing(
+            { ...declaredWebhook(), _packageId: PKG }, { ...declaredWebhook(), _packageId: 'com.acme.beta' },
+        ));
+        expect((await source.list()).map((d) => d.packageId)).toEqual([PKG, 'com.acme.beta']);
+        expect((await refusalOf(source)).message).toContain('address it by its package');
+        expect(await wholeNameRefusal(source, T)).toBeUndefined();
+        expect(await wholeNameRefusal(source, 'not_declared')).toBeUndefined();
+    });
+
+    it('CONTROL — one holder listing the name twice is refused whole, saying so', async () => {
+        const source = new DeclaredWebhookSource(newEngine(), listing({ ...declaredWebhook(), _packageId: PKG }, { ...declaredWebhook(), _packageId: PKG }));
+        const refusal = await refusalOf(source);
+        expect(refusal.message).toContain('A holder that lists the name twice holds two declarations for one slot');
+        expect(refusal.message).not.toContain("the environment's own declaration");
+    });
+
+    it('[FROM namespace placeholder TO the refusal] the name guard refuses the name, saying none is dispatched and naming both holders', async () => {
+        const hooks: Record<string, (ctx: any) => Promise<void>> = {};
+        bindDeclaredWebhookNameGuard({ registerHook: (event, handler) => { hooks[event] = handler; } }, envBesidePackage());
+        const err = await hooks.beforeInsert({ input: { data: { name: T } }, session: {} }).then(() => undefined, (e: unknown) => e);
+        expect(err).toBeInstanceOf(DeclaredWebhookNameConflictError);
+        expect(err).toMatchObject({ code: 'RESOURCE_CONFLICT', status: 409 });
+        const message = (err as Error).message;
+        expect(message).toContain(`No declaration of '${T}' is dispatched, so none needs switching off`);
+        expect(message).toContain(`package "${PKG}"`);
+        expect(message).not.toContain('switch the declared');
+        expect(message).not.toContain('namespace>');
+    });
+
+    it('[FROM "the package is disabled" TO the refusal] a delivery naming the package is refused with the refusal, never as a disable', async () => {
+        const engine = newEngine();
+        const source = new DeclaredWebhookSource(engine, listing(declaredWebhook(), { ...declaredWebhook(), _packageId: PKG }));
+        const guard = createWebhookRedeliverGuard(engine as any, 'sys_webhook', source);
+        const verdict = await guard({ source: 'webhook', refId: declaredWebhookRefId(T, PKG) });
+        expect(verdict).toContain(`declared webhook '${T}' is not served, so this delivery is refused`);
+        expect(verdict).toContain('No declaration of it is dispatched');
+        expect(verdict).not.toMatch(/disabled/);
+        // The environment's own reference names no package: the ambiguity, naming both.
+        const bare = await guard({ source: 'webhook', refId: declaredWebhookRefId(T) });
+        expect(bare).toMatch(/names no package/);
+        expect(bare).toContain(`package "${PKG}"`);
+    });
+});