fix(cloud-connection): an install-local uninstall runs the protocol's registered uninstall cleanups

.changeset/21490-install-local-uninstall-cleanups.md

@@ -0,0 +1,18 @@
+---
+'@objectstack/cloud-connection': patch
+'@objectstack/metadata-protocol': minor
+---
+
+fix(cloud-connection): an install-local uninstall runs the protocol's registered uninstall cleanups, so the package's permission sets and their grants go with it
+
+Clause-②: yes
+
+`DELETE /api/v1/marketplace/install-local/:manifestId` removed the package's ledger entry and nothing else. After a restart the package's objects were gone, but its `managed_by: package` rows in `sys_permission_set`, and every grant of them, survived the uninstall. That broke ADR-0090's "No ghost grants" promise on this door.
+
+The door now runs the uninstall cleanups that domain plugins register with the protocol (`registerUninstallCleanup`) once the ledger entry is gone. It uses the same registry and the same runner as the protocol's own uninstall, so `plugin-security`'s `security.package-permissions` cleanup removes the package's sets with their position and user bindings, and any cleanup registered later fires here too. The cleanups run with the package's manifest id and no organization, because an install-local package is installed for the whole runtime.
+
+The response carries each outcome as `data.cleanups`, the way the protocol's uninstall reports them. A failed cleanup is reported there and named in the operator log with its remedy (install the package again, then uninstall it again). When the protocol cannot run the cleanups, the response says so as one failed `protocol.runUninstallCleanups` outcome. An uninstall that does not happen (a refused caller, an id this door never installed, a ledger write that fails) revokes nothing.
+
+`@objectstack/metadata-protocol`: `ObjectStackProtocolImplementation` gains `runUninstallCleanups({ packageId, organizationId?, actor? })`, the one runner of the uninstall-cleanup registry. It runs every registered cleanup for the package and answers one `UninstallCleanupOutcome` per cleanup. It never throws: a failed cleanup is an outcome, and a thrown fault's driver text goes to the operator log, not into the outcome. `deletePackage` now calls it as its last step in place of its own loop, and its `cleanups` are unchanged. The only visible difference there is the log tag of a failed cleanup's warning, now `[protocol.runUninstallCleanups]` instead of `[protocol.deletePackage]`.
+
+`@objectstack/cloud-connection` now declares its dependency on `@objectstack/metadata-protocol`, which it already received through `@objectstack/runtime`, for the cleanup outcome types.

packages/cloud-connection/package.json

@@ -25,6 +25,7 @@
   },
   "dependencies": {
     "@objectstack/core": "workspace:*",
+    "@objectstack/metadata-protocol": "workspace:*",
     "@objectstack/runtime": "workspace:*",
     "@objectstack/spec": "workspace:*",
     "@objectstack/types": "workspace:*"

packages/cloud-connection/src/cloud-connection-route-ledger.ts

@@ -223,7 +223,9 @@ export const CLOUD_CONNECTION_ROUTE_LEDGER: readonly CloudConnectionRouteLedgerE
         mountedIn: 'marketplace-install-local-plugin.ts',
         disposition: 'server-only',
         note:
-            'removes the cached manifest from this runtime\'s disk; the kernel must restart to fully unload, since '
+            'removes the cached manifest from this runtime\'s disk and runs the uninstall cleanups registered with the '
+            + 'protocol (the package\'s permission sets and their grants are revoked, each outcome reported as `cleanups`); '
+            + 'the kernel must restart to fully unload, since '
             + '`engine.registerApp` is additive only. A filesystem-mutating, restart-coupled operation local to one runtime '
             + 'is deliberately not SDK surface — the CLI and the Console Setup view drive it. Requires `manage_metadata`, '
             + 'never merely a signed-in session.',

packages/cloud-connection/src/marketplace-install-local-plugin.ts

@@ -36,9 +36,12 @@
  *            outright (#8976).
  *
  *   DELETE /api/v1/marketplace/install-local/:manifestId
- *          → removes the cached manifest. Kernel must be restarted to fully
- *            unload — `engine.registerApp` is additive only. We document
- *            this in the response message.
+ *          → removes the cached manifest, then runs the uninstall cleanups
+ *            domain plugins registered with the protocol (#21490) — the
+ *            package's permission sets and their grants go with it — and
+ *            reports each outcome as `cleanups`. Kernel must be restarted to
+ *            fully unload — `engine.registerApp` is additive only. We
+ *            document this in the response message.
  *
  * Persistence layout:
  *   <cwd>/.objectstack/installed-packages/<safe-manifest-id>.json
@@ -87,9 +90,36 @@ import {
 import { ConnectionCredentialStore } from './connection-credential-store.js';
 import { MARKETPLACE_INSTALLED_UI_BUNDLE } from './marketplace-ui.js';
 import type { IHttpServer, IMetadataService, IObjectQLEngine } from '@objectstack/spec/contracts';
+import type { DeletePackageRequest, UninstallCleanupOutcome } from '@objectstack/metadata-protocol';
 
 const ROUTE_BASE = '/api/v1/marketplace/install-local';
 
+/**
+ * [#21490] The one `protocol` verb this plugin calls: the runner of the
+ * uninstall cleanups domain plugins register through
+ * `registerUninstallCleanup` — the same registry, through the same runner,
+ * that the protocol's own uninstall (`deletePackage`) runs.
+ *
+ * The `protocol` slot is uncontracted (`ServiceSlotContracts` leaves it
+ * unmapped), so this is a per-consumer narrowing, the shape
+ * `PackagesDomainProtocol` gives `deletePackage` in
+ * `packages/runtime/src/domains/packages.ts`: the verb's name is this file's,
+ * its request and outcome types are the producer's own declared ones — never
+ * a restatement here.
+ *
+ * Optional, and asked as a capability question at the call site: a protocol
+ * from a build without the runner holds cleanups this door cannot run, and
+ * the uninstall says so instead of answering as if they had run.
+ */
+type UninstallCleanupRunner = {
+    runUninstallCleanups?(
+        request: Pick<DeletePackageRequest, 'packageId' | 'organizationId' | 'actor'>,
+    ): Promise<UninstallCleanupOutcome[]>;
+};
+
+/** The outcome name this door reports when the runner itself could not run. */
+const UNINSTALL_CLEANUP_RUNNER = 'protocol.runUninstallCleanups';
+
 /**
  * [#8976] The capability every MUTATING install-local route demands.
  *
@@ -1092,16 +1122,100 @@ export class MarketplaceInstallLocalPlugin implements Plugin {
         } catch (err: any) {
             return c.json({ success: false, error: { code: 'MARKETPLACE_STORAGE_FAILED', message: err?.message ?? String(err) } }, 500);
         }
-        ctx.logger?.info?.(`[MarketplaceInstallLocal] uninstalled ${manifestId} (cached manifest removed; restart runtime to unload from running kernel)`);
+        // [#21490] Only now — the ledger entry is gone, so the package will not
+        // come back at the next restart — revoke what its metadata granted.
+        // Never before: an uninstall whose ledger write failed above leaves the
+        // package installed, and it must keep its grants.
+        const cleanups = await this.runUninstallCleanups(ctx, manifestId, admission.userId);
+        ctx.logger?.info?.(`[MarketplaceInstallLocal] uninstalled ${manifestId} (cached manifest removed; ${cleanups.length} uninstall cleanup(s) ran; restart runtime to unload from running kernel)`);
         return c.json({
             success: true,
             data: {
                 manifestId,
-                note: 'Cached manifest removed. The app remains loaded in the running kernel until the next restart (the kernel API does not support unregistering apps in-place).',
+                cleanups,
+                note: 'Cached manifest removed, and the uninstall cleanups this runtime\'s plugins registered ran — each one\'s outcome is in `cleanups`. The app remains loaded in the running kernel until the next restart (the kernel API does not support unregistering apps in-place).',
             },
         }, 200);
     };
 
+    /**
+     * [#21490] Run the protocol's registered uninstall cleanups for a package
+     * this door just removed from its ledger — ADR-0086 D3's data-plane
+     * revocation, which ADR-0090 states as "(removing its sets by `packageId`,
+     * ADR-0086 D3) revokes it everywhere at once. No ghost grants."
+     *
+     * Before this, the door removed the ledger entry and nothing else: the
+     * package's object answered 404 after a restart, while its `managed_by:
+     * package` `sys_permission_set` row — and every grant of it — survived.
+     * `plugin-security` registers exactly that revocation
+     * (`security.package-permissions`); only the protocol's own uninstall ran it.
+     *
+     * ⛔ No second revocation path lives here. The cleanups are whatever domain
+     * plugins registered with the protocol, run by the protocol's own runner —
+     * this door knows no table, and a cleanup registered tomorrow fires here
+     * with no edit to this file.
+     *
+     * The request:
+     *   - `packageId` is the MANIFEST id, never the ledger's `packageId`. The
+     *     registry, and so every row a domain plugin stamped for the package,
+     *     knows the package by `manifest.id`; the ledger's `packageId` is the
+     *     marketplace catalog's id on a cloud install and can differ from it.
+     *   - no `organizationId`: an install-local package is installed for the
+     *     whole runtime (the ledger is per runtime, and the rehydrate registers
+     *     it for every tenant), so its revocation is too.
+     *   - `actor` is the operator the admission resolved.
+     *
+     * What comes back is reported, never swallowed — on the response, as the
+     * protocol's own uninstall reports it (`cleanups`). Never throws: the
+     * uninstall has already happened, so a cleanup that could not run is an
+     * outcome, not a failed request. Three answers:
+     *   - no `protocol` service — no cleanup registry exists, so none is
+     *     registered and none is owed: `[]`;
+     *   - a protocol without the runner, or a runner that throws — one failed
+     *     outcome named {@link UNINSTALL_CLEANUP_RUNNER}, so the caller can
+     *     tell "nothing to revoke" from "the revocation never ran";
+     *   - otherwise the runner's outcomes, verbatim.
+     */
+    private runUninstallCleanups = async (
+        ctx: PluginContext,
+        manifestId: string,
+        actor: string,
+    ): Promise<UninstallCleanupOutcome[]> => {
+        let protocol: UninstallCleanupRunner | undefined;
+        try { protocol = ctx.getService<UninstallCleanupRunner>('protocol'); } catch { /* no protocol service */ }
+        if (!protocol) return [];
+
+        const notRun = (error: string): UninstallCleanupOutcome[] => [
+            { name: UNINSTALL_CLEANUP_RUNNER, success: false, removed: 0, error },
+        ];
+        let outcomes: UninstallCleanupOutcome[];
+        if (typeof protocol.runUninstallCleanups !== 'function') {
+            outcomes = notRun(
+                'this runtime\'s protocol cannot run uninstall cleanups — upgrade @objectstack/metadata-protocol '
+                + 'alongside @objectstack/cloud-connection',
+            );
+        } else {
+            try {
+                outcomes = await protocol.runUninstallCleanups({ packageId: manifestId, actor });
+            } catch (err: any) {
+                ctx.logger?.warn?.(`[MarketplaceInstallLocal] the uninstall cleanups of ${manifestId} could not be run: ${err?.message ?? err}`);
+                outcomes = notRun('the uninstall cleanups could not be run');
+            }
+        }
+
+        const failed = outcomes.filter((o) => o.success !== true);
+        if (failed.length > 0) {
+            ctx.logger?.warn?.(
+                `[MarketplaceInstallLocal] uninstalled ${manifestId}, but ${failed.length} uninstall cleanup(s) did not `
+                + `complete (${failed.map((o) => o.name).join(', ')}) — what they revoke, such as the package's `
+                + 'permission sets and their grants, SURVIVES the uninstall. Each outcome is on the response '
+                + '(`cleanups`). Remedy: once the cause is fixed, install the package again and uninstall it again — '
+                + 'the cleanups re-select by package id, so a second pass removes whatever the first could not.',
+            );
+        }
+        return outcomes;
+    };
+
     /**
      * [ADR-0120 D5e] Decide whether this install must stop for the
      * `isolated`-posture confirmation, and what attestation the ledger entry