fix(runtime): the package disable and DELETE routes announce the withdrawal

.changeset/33527-package-withdrawal-announce.md

@@ -0,0 +1,14 @@
+---
+'@objectstack/runtime': patch
+---
+
+fix(runtime): disabling or uninstalling a package through the runtime's package routes withdraws its flows at once (#33527)
+
+Clause-②: no
+Metadata-protocol: none
+
+`PATCH /packages/:id/disable` and `DELETE /packages/:id` now announce `metadata:reloaded` with `{ changed: ['app/ID'] }` once the registry has taken the package out of service. This is the spelling the install and enable routes and the marketplace install-local uninstall already use. Every subscriber of that event re-reads a registry that no longer serves the package. The automation engine withdraws the package's flows and unbinds their record and schedule triggers, and the authored hooks, actions and translations re-sync.
+
+Before this change, a disabled package's record-triggered flow stayed bound in the automation engine. It kept running on every matching record write until the process restarted, while `GET /meta/flow` no longer listed it. An uninstalled package's flow was withdrawn only by the asynchronous sync of its deleted metadata rows, so a write made right after the `DELETE` answered could still run it.
+
+A disable of a package that is already disabled announces nothing, and neither does an uninstall the running registry did not withdraw. A failed announce never changes either route's answer: it is logged once at `warn`, naming the restart that repairs it.

packages/runtime/src/domains/packages-reload-announce.test.ts

@@ -4,7 +4,9 @@
  * [#33302] The install door (`POST /packages`) and the enable door
  * (`PATCH /packages/:id/enable`) announce `metadata:reloaded` once they have
  * put a package into service, in the spelling the marketplace install-local
- * door announces a hot install with: `{ changed: ['app/ID'] }`.
+ * door announces a hot install with: `{ changed: ['app/ID'] }`. [#33527] The
+ * disable door and `DELETE /packages/:id` announce the withdrawal in the same
+ * spelling (the last describe below).
  *
  * ## The defect, measured on `origin/main` before the change
  *
@@ -91,10 +93,14 @@ const holder = (id: string, user: string, position: string, organization: string
     id, user_id: user, position, organization_id: organization,
 });
 
-/** Every announcement the kernel heard, with whether the registry held what it names at that moment. */
+/**
+ * Every announcement the kernel heard, with whether the registry held what it
+ * names at that moment and, for what it held, whether that package was disabled.
+ */
 interface Heard {
     readonly changed: unknown;
     readonly held: Record<string, boolean>;
+    readonly off: Record<string, boolean>;
 }
 
 const envSnapshot = { OS_HOME: process.env.OS_HOME };
@@ -134,12 +140,15 @@ beforeAll(async () => {
                 const changed = (payload as { changed?: unknown } | undefined)?.changed;
                 const registry = ctx.getService<ObjectQL>('objectql').registry;
                 const held: Record<string, boolean> = {};
+                const off: Record<string, boolean> = {};
                 if (Array.isArray(changed)) {
                     for (const entry of changed) {
-                        if (typeof entry === 'string' && entry.startsWith('app/')) held[entry] = registry.getPackage(entry.slice(4)) != null;
+                        if (typeof entry !== 'string' || !entry.startsWith('app/')) continue;
+                        held[entry] = registry.getPackage(entry.slice(4)) != null;
+                        if (held[entry]) off[entry] = registry.isPackageDisabled(entry.slice(4));
                     }
                 }
-                heard.push({ changed, held });
+                heard.push({ changed, held, off });
             });
         },
     } as Plugin);
@@ -190,7 +199,7 @@ describe('[#33302] the enable door announces a package it puts back into service
         expect(await positionsOf(['e1', 'e2'])).toEqual({ e1: 'crm_sales_rep', e2: 'crm_auditor' });
         // ⭐ The announcement, in the install-local door's spelling, with the
         // package already registered (and enabled) when the subscribers hear it.
-        expect(during).toEqual([{ changed: [`app/${CRM}`], held: { [`app/${CRM}`]: true } }]);
+        expect(during).toEqual([{ changed: [`app/${CRM}`], held: { [`app/${CRM}`]: true }, off: { [`app/${CRM}`]: false } }]);
     }, 60_000);
 
     it('an enable of a package that is already enabled changes nothing served, and announces nothing', async () => {
@@ -208,7 +217,7 @@ describe('[#33302] the install door announces the package it installs', () => {
             dispatcher.handlePackages('', 'POST', { manifest: freshManifest }, {}, PKG_ADMIN()));
 
         expect(result.response?.status).toBe(201);
-        expect(during).toEqual([{ changed: [`app/${FRESH}`], held: { [`app/${FRESH}`]: true } }]);
+        expect(during).toEqual([{ changed: [`app/${FRESH}`], held: { [`app/${FRESH}`]: true }, off: { [`app/${FRESH}`]: false } }]);
     }, 60_000);
 
     it('an overwrite install with `enableOnInstall: true` of a disabled package renaming a position: its holder holds the new name at once', async () => {
@@ -226,7 +235,7 @@ describe('[#33302] the install door announces the package it installs', () => {
         expect(result.response?.status).toBe(201);
         expect(result.response?.body?.data?.enabled).toBe(true);
         expect(await positionsOf(['i1'])).toEqual({ i1: 'authored_sales_rep' });
-        expect(during).toEqual([{ changed: [`app/${AUTHORED}`], held: { [`app/${AUTHORED}`]: true } }]);
+        expect(during).toEqual([{ changed: [`app/${AUTHORED}`], held: { [`app/${AUTHORED}`]: true }, off: { [`app/${AUTHORED}`]: false } }]);
     }, 60_000);
 });
 
@@ -249,3 +258,80 @@ describe('[#33302] a failed announce never fails the door', () => {
         }
     }, 60_000);
 });
+
+/**
+ * [#33527] The two doors that take a package OUT of service announce the
+ * withdrawal in the same spelling, so a subscriber that bound the package's
+ * items re-reads a registry that serves them no more. Measured before, on a
+ * kernel carrying the real automation engine and record-change trigger: a
+ * disabled package's record-triggered flow stayed bound and ran on every
+ * matching write until a restart, and an uninstalled one ran on the first
+ * write after the door answered (its withdrawal rode only on the asynchronous
+ * mutation sync of its deleted rows).
+ */
+describe('[#33527] the disable and uninstall doors announce the package they withdraw', () => {
+    const install = async (id: string) => {
+        const created = await dispatcher.handlePackages('', 'POST', { manifest: { id, name: id, version: '1.0.0', type: 'app', namespace: id.split('.').pop() } }, {}, PKG_ADMIN());
+        expect(created.response?.status).toBe(201);
+    };
+
+    it('a disable of an enabled package announces `app/ID` once, after the registry switched it off', async () => {
+        const id = 'com.acme.withdrawdisable';
+        await install(id);
+        const { result, heard: during } = await heardDuring(() =>
+            dispatcher.handlePackages(`/${id}/disable`, 'PATCH', {}, {}, PKG_ADMIN()));
+
+        expect(result.response?.status).toBe(200);
+        // The subscribers hear it with the package still installed, and disabled.
+        expect(during).toEqual([{ changed: [`app/${id}`], held: { [`app/${id}`]: true }, off: { [`app/${id}`]: true } }]);
+    }, 60_000);
+
+    it('a disable of a package already disabled changes nothing served, and announces nothing', async () => {
+        const id = 'com.acme.withdrawdisable';
+        expect(registry().isPackageDisabled(id)).toBe(true);
+        const { result, heard: during } = await heardDuring(() =>
+            dispatcher.handlePackages(`/${id}/disable`, 'PATCH', {}, {}, PKG_ADMIN()));
+        expect(result.response?.status).toBe(200);
+        expect(during).toEqual([]);
+    }, 60_000);
+
+    it('an uninstall announces `app/ID` once, after the registry withdrew the package', async () => {
+        const id = 'com.acme.withdrawdelete';
+        await install(id);
+        const { result, heard: during } = await heardDuring(() =>
+            dispatcher.handlePackages(`/${id}`, 'DELETE', undefined, {}, PKG_ADMIN()));
+
+        expect(result.response?.status).toBe(200);
+        expect(during).toEqual([{ changed: [`app/${id}`], held: { [`app/${id}`]: false }, off: {} }]);
+    }, 60_000);
+
+    it('an uninstall of a package the registry does not hold withdraws nothing, and announces nothing', async () => {
+        const { result, heard: during } = await heardDuring(() =>
+            dispatcher.handlePackages('/com.acme.withdrawnever', 'DELETE', undefined, {}, PKG_ADMIN()));
+        expect(result.response?.status).toBe(404);
+        expect(during).toEqual([]);
+    }, 60_000);
+
+    it('a subscriber that throws: the disable and the uninstall still answer 200, and each failure is said once at warn', async () => {
+        for (const [id, door] of [['com.acme.withdrawthrowsoff', 'disable'], ['com.acme.withdrawthrowsdel', 'delete']] as const) {
+            await install(id);
+            const trigger = vi.spyOn((kernel as any).context, 'trigger').mockRejectedValueOnce(new Error('subscriber exploded'));
+            const warn = vi.spyOn(console, 'warn').mockImplementation(() => undefined);
+            try {
+                const result = door === 'disable'
+                    ? await dispatcher.handlePackages(`/${id}/disable`, 'PATCH', {}, {}, PKG_ADMIN())
+                    : await dispatcher.handlePackages(`/${id}`, 'DELETE', undefined, {}, PKG_ADMIN());
+                expect(result.response?.status, door).toBe(200);
+                expect(trigger, door).toHaveBeenCalledWith('metadata:reloaded', { changed: [`app/${id}`] });
+                const said = warn.mock.calls.map(([m]) => String(m)).filter((m) => m.includes('metadata:reloaded'));
+                expect(said, door).toHaveLength(1);
+                expect(said[0], door).toContain(`'${id}'`);
+            } finally {
+                trigger.mockRestore();
+                warn.mockRestore();
+            }
+        }
+        expect(registry().isPackageDisabled('com.acme.withdrawthrowsoff')).toBe(true);
+        expect(registry().getPackage('com.acme.withdrawthrowsdel')).toBeUndefined();
+    }, 60_000);
+});

packages/runtime/src/domains/packages.ts

@@ -931,29 +931,49 @@ async function summaryRefreshSettled(engine: unknown): Promise<void> {
  * it does for that door: `plugin-security` carries the holders of the
  * positions the package renamed (`renamedFrom`), the automation engine
  * re-syncs flows, and the authored hooks, actions and translations re-sync.
+ * An install that lands disabled (`enableOnInstall` false, or its switch
+ * refused) is announced too; harmless, since every subscriber reads through
+ * the registry's disabled filter and so binds nothing of it.
+ *
+ * [#33527] The two doors that take a package OUT of service announce through
+ * here too, in the same spelling the install-local door announces its
+ * uninstall with: the disable door (`PATCH /packages/:id/disable`) and the
+ * uninstall door (`DELETE /packages/:id`). Measured before they did: a
+ * disabled package's record-triggered flow stayed bound in the automation
+ * engine and kept running on every matching write until an unrelated reload
+ * or a restart, while `GET /meta/flow` no longer listed it; an uninstalled
+ * one was withdrawn only by the asynchronous metadata-mutation sync of its
+ * deleted rows, after the door had answered. The same re-sync that binds a
+ * package's flows withdraws them once the registry no longer serves it.
  *
  * Called only after the registry write has landed, so a subscriber reading the
  * registry finds the package there (a scope of `app/ID` names a package the
- * registry holds NOW). Never throws: the package is installed or enabled and
- * served either way, so a failed re-sync is a FUNCTIONAL degradation, said
- * once at `warn` with the restart that repairs it — the level the
- * publish-drafts door and the install-local door use for the same announce.
+ * registry holds NOW), or, for a withdrawal, finds it disabled or gone. Never
+ * throws: the registry change is made and answered either way, so a failed
+ * re-sync is a FUNCTIONAL degradation, said once at `warn` with the restart
+ * that repairs it — the level the publish-drafts door and the install-local
+ * door use for the same announce.
  */
 async function announcePackageReload(
     deps: DomainHandlerDeps,
     context: HttpProtocolContext,
     id: string,
-    door: 'installed' | 'enabled',
+    door: 'installed' | 'enabled' | 'disabled' | 'uninstalled',
 ): Promise<void> {
     try {
         await deps.announceKernelEvent(context, 'metadata:reloaded', { changed: [`app/${id}`] });
     } catch (e: any) {
+        const kept = door === 'disabled' || door === 'uninstalled'
+            ? 'keep serving what they read of it until this process restarts: its record-triggered and scheduled '
+                + 'flows stay armed and keep running, and its authored hooks, actions and translations are not '
+                + 're-synced.'
+            : 'keep their earlier view of it until this process restarts: holders of the positions it renamed stay '
+                + 'on the old names, its record-triggered flows do not bind, and its authored hooks, actions and '
+                + 'translations are not re-synced.';
         (deps.logger ?? console).warn(
             `[Packages] package '${id}' is ${door}, but the 'metadata:reloaded' announce FAILED — boot-cached consumers `
-            + 'keep their earlier view of it until this process restarts: holders of the positions it renamed stay '
-            + 'on the old names, its record-triggered flows do not bind, and its authored hooks, actions and '
-            + 'translations are not re-synced. Nothing retries this announce; restart the process to rebuild every '
-            + `subscriber from storage. Cause: ${e?.message ?? String(e)}`,
+            + `${kept} Nothing retries this announce; restart the process to rebuild every subscriber from storage. `
+            + `Cause: ${e?.message ?? String(e)}`,
         );
     }
 }
@@ -2915,8 +2935,20 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin
                 if (err instanceof HostPackageStateRefusal) return hostPackageStateRefused(deps, err, 'disabled');
                 console.warn('[handlePackages] failed to persist disable state', { id, error: (err as Error)?.message });
             }
+            // [#33527] Whether this disable takes the package out of service,
+            // read by the registry's own predicate before it switches — the
+            // enable door's reading, mirrored. A registry that cannot say is
+            // announced to, since every subscriber is idempotent.
+            const isPackageDisabled = (registry as { isPackageDisabled?: (packageId: string) => boolean }).isPackageDisabled;
+            const wasEnabled = typeof isPackageDisabled !== 'function' || isPackageDisabled.call(registry, id) !== true;
             const pkg = registry.disablePackage(id);
             if (!pkg) return { handled: true, response: deps.error(`Package '${id}' not found`, 404) };
+            // [#33527] The registry no longer serves the package's flows and
+            // other items, but a subscriber that bound them keeps them until it
+            // re-reads: announce the withdrawal, as the install-local door
+            // announces an uninstall. A disable of a package already disabled
+            // changes nothing served, so it announces nothing.
+            if (wasEnabled) await announcePackageReload(deps, _context, id, 'disabled');
             return { handled: true, response: deps.success(pkg) };
         }
 
@@ -4174,6 +4206,16 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin
                 }
             }
 
+            // [#33527] LAST of the withdrawal, after the registry's uninstall
+            // and the verb's cleanups, as the install-local door announces its
+            // uninstall: a subscriber that bound the package's flows and other
+            // items re-reads a registry that no longer has them, before this
+            // door answers. Whatever the answer below, partial included: the
+            // package left the running registry either way. Not when the
+            // registry kept it (refused, or never held it): a re-sync would
+            // only read back what it serves.
+            if (registryRemoved) await announcePackageReload(deps, _context, id, 'uninstalled');
+
             const deletedCount = persisted?.deletedCount ?? 0;
             const failedCount = persisted?.failedCount ?? 0;