Skip to content

Commit d723d59

Browse files
committed
fix(cloud-connection): an install-local uninstall runs the protocol's registered uninstall cleanups
DELETE /api/v1/marketplace/install-local/:manifestId removed the ledger entry and nothing else, so the package's managed_by: package permission sets and every grant of them outlived the uninstall. The door now calls the protocol's uninstall-cleanup runner (the registry deletePackage runs) once the ledger entry is gone, with the manifest id and no organization, and answers each outcome as cleanups. No second revocation path lives in cloud-connection. The runner itself (protocol.runUninstallCleanups) is a metadata-protocol edit sequenced separately; until it lands this door reports one failed outcome naming it rather than an empty list. Claude-Session: https://claude.ai/code/session_016GiHYRmLSNWTfbX9gVQkpz Co-Authored-By: Claude <noreply@anthropic.com>
1 parent ebf0b8e commit d723d59

5 files changed

Lines changed: 394 additions & 6 deletions

File tree

‎packages/cloud-connection/package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
},
2626
"dependencies": {
2727
"@objectstack/core": "workspace:*",
28+
"@objectstack/metadata-protocol": "workspace:*",
2829
"@objectstack/runtime": "workspace:*",
2930
"@objectstack/spec": "workspace:*",
3031
"@objectstack/types": "workspace:*"

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

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

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

Lines changed: 119 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,12 @@
3636
* outright (#8976).
3737
*
3838
* DELETE /api/v1/marketplace/install-local/:manifestId
39-
* → removes the cached manifest. Kernel must be restarted to fully
40-
* unload — `engine.registerApp` is additive only. We document
41-
* this in the response message.
39+
* → removes the cached manifest, then runs the uninstall cleanups
40+
* domain plugins registered with the protocol (#21490) — the
41+
* package's permission sets and their grants go with it — and
42+
* reports each outcome as `cleanups`. Kernel must be restarted to
43+
* fully unload — `engine.registerApp` is additive only. We
44+
* document this in the response message.
4245
*
4346
* Persistence layout:
4447
* <cwd>/.objectstack/installed-packages/<safe-manifest-id>.json
@@ -87,9 +90,36 @@ import {
8790
import { ConnectionCredentialStore } from './connection-credential-store.js';
8891
import { MARKETPLACE_INSTALLED_UI_BUNDLE } from './marketplace-ui.js';
8992
import type { IHttpServer, IMetadataService, IObjectQLEngine } from '@objectstack/spec/contracts';
93+
import type { DeletePackageRequest, UninstallCleanupOutcome } from '@objectstack/metadata-protocol';
9094

9195
const ROUTE_BASE = '/api/v1/marketplace/install-local';
9296

97+
/**
98+
* [#21490] The one `protocol` verb this plugin calls: the runner of the
99+
* uninstall cleanups domain plugins register through
100+
* `registerUninstallCleanup` — the same registry, through the same runner,
101+
* that the protocol's own uninstall (`deletePackage`) runs.
102+
*
103+
* The `protocol` slot is uncontracted (`ServiceSlotContracts` leaves it
104+
* unmapped), so this is a per-consumer narrowing, the shape
105+
* `PackagesDomainProtocol` gives `deletePackage` in
106+
* `packages/runtime/src/domains/packages.ts`: the verb's name is this file's,
107+
* its request and outcome types are the producer's own declared ones — never
108+
* a restatement here.
109+
*
110+
* Optional, and asked as a capability question at the call site: a protocol
111+
* from a build without the runner holds cleanups this door cannot run, and
112+
* the uninstall says so instead of answering as if they had run.
113+
*/
114+
type UninstallCleanupRunner = {
115+
runUninstallCleanups?(
116+
request: Pick<DeletePackageRequest, 'packageId' | 'organizationId' | 'actor'>,
117+
): Promise<UninstallCleanupOutcome[]>;
118+
};
119+
120+
/** The outcome name this door reports when the runner itself could not run. */
121+
const UNINSTALL_CLEANUP_RUNNER = 'protocol.runUninstallCleanups';
122+
93123
/**
94124
* [#8976] The capability every MUTATING install-local route demands.
95125
*
@@ -1092,16 +1122,100 @@ export class MarketplaceInstallLocalPlugin implements Plugin {
10921122
} catch (err: any) {
10931123
return c.json({ success: false, error: { code: 'MARKETPLACE_STORAGE_FAILED', message: err?.message ?? String(err) } }, 500);
10941124
}
1095-
ctx.logger?.info?.(`[MarketplaceInstallLocal] uninstalled ${manifestId} (cached manifest removed; restart runtime to unload from running kernel)`);
1125+
// [#21490] Only now — the ledger entry is gone, so the package will not
1126+
// come back at the next restart — revoke what its metadata granted.
1127+
// Never before: an uninstall whose ledger write failed above leaves the
1128+
// package installed, and it must keep its grants.
1129+
const cleanups = await this.runUninstallCleanups(ctx, manifestId, admission.userId);
1130+
ctx.logger?.info?.(`[MarketplaceInstallLocal] uninstalled ${manifestId} (cached manifest removed; ${cleanups.length} uninstall cleanup(s) ran; restart runtime to unload from running kernel)`);
10961131
return c.json({
10971132
success: true,
10981133
data: {
10991134
manifestId,
1100-
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).',
1135+
cleanups,
1136+
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).',
11011137
},
11021138
}, 200);
11031139
};
11041140

1141+
/**
1142+
* [#21490] Run the protocol's registered uninstall cleanups for a package
1143+
* this door just removed from its ledger — ADR-0086 D3's data-plane
1144+
* revocation, which ADR-0090 states as "(removing its sets by `packageId`,
1145+
* ADR-0086 D3) revokes it everywhere at once. No ghost grants."
1146+
*
1147+
* Before this, the door removed the ledger entry and nothing else: the
1148+
* package's object answered 404 after a restart, while its `managed_by:
1149+
* package` `sys_permission_set` row — and every grant of it — survived.
1150+
* `plugin-security` registers exactly that revocation
1151+
* (`security.package-permissions`); only the protocol's own uninstall ran it.
1152+
*
1153+
* ⛔ No second revocation path lives here. The cleanups are whatever domain
1154+
* plugins registered with the protocol, run by the protocol's own runner —
1155+
* this door knows no table, and a cleanup registered tomorrow fires here
1156+
* with no edit to this file.
1157+
*
1158+
* The request:
1159+
* - `packageId` is the MANIFEST id, never the ledger's `packageId`. The
1160+
* registry, and so every row a domain plugin stamped for the package,
1161+
* knows the package by `manifest.id`; the ledger's `packageId` is the
1162+
* marketplace catalog's id on a cloud install and can differ from it.
1163+
* - no `organizationId`: an install-local package is installed for the
1164+
* whole runtime (the ledger is per runtime, and the rehydrate registers
1165+
* it for every tenant), so its revocation is too.
1166+
* - `actor` is the operator the admission resolved.
1167+
*
1168+
* What comes back is reported, never swallowed — on the response, as the
1169+
* protocol's own uninstall reports it (`cleanups`). Never throws: the
1170+
* uninstall has already happened, so a cleanup that could not run is an
1171+
* outcome, not a failed request. Three answers:
1172+
* - no `protocol` service — no cleanup registry exists, so none is
1173+
* registered and none is owed: `[]`;
1174+
* - a protocol without the runner, or a runner that throws — one failed
1175+
* outcome named {@link UNINSTALL_CLEANUP_RUNNER}, so the caller can
1176+
* tell "nothing to revoke" from "the revocation never ran";
1177+
* - otherwise the runner's outcomes, verbatim.
1178+
*/
1179+
private runUninstallCleanups = async (
1180+
ctx: PluginContext,
1181+
manifestId: string,
1182+
actor: string,
1183+
): Promise<UninstallCleanupOutcome[]> => {
1184+
let protocol: UninstallCleanupRunner | undefined;
1185+
try { protocol = ctx.getService<UninstallCleanupRunner>('protocol'); } catch { /* no protocol service */ }
1186+
if (!protocol) return [];
1187+
1188+
const notRun = (error: string): UninstallCleanupOutcome[] => [
1189+
{ name: UNINSTALL_CLEANUP_RUNNER, success: false, removed: 0, error },
1190+
];
1191+
let outcomes: UninstallCleanupOutcome[];
1192+
if (typeof protocol.runUninstallCleanups !== 'function') {
1193+
outcomes = notRun(
1194+
'this runtime\'s protocol cannot run uninstall cleanups — upgrade @objectstack/metadata-protocol '
1195+
+ 'alongside @objectstack/cloud-connection',
1196+
);
1197+
} else {
1198+
try {
1199+
outcomes = await protocol.runUninstallCleanups({ packageId: manifestId, actor });
1200+
} catch (err: any) {
1201+
ctx.logger?.warn?.(`[MarketplaceInstallLocal] the uninstall cleanups of ${manifestId} could not be run: ${err?.message ?? err}`);
1202+
outcomes = notRun('the uninstall cleanups could not be run');
1203+
}
1204+
}
1205+
1206+
const failed = outcomes.filter((o) => o.success !== true);
1207+
if (failed.length > 0) {
1208+
ctx.logger?.warn?.(
1209+
`[MarketplaceInstallLocal] uninstalled ${manifestId}, but ${failed.length} uninstall cleanup(s) did not `
1210+
+ `complete (${failed.map((o) => o.name).join(', ')}) — what they revoke, such as the package's `
1211+
+ 'permission sets and their grants, SURVIVES the uninstall. Each outcome is on the response '
1212+
+ '(`cleanups`). Remedy: once the cause is fixed, install the package again and uninstall it again — '
1213+
+ 'the cleanups re-select by package id, so a second pass removes whatever the first could not.',
1214+
);
1215+
}
1216+
return outcomes;
1217+
};
1218+
11051219
/**
11061220
* [ADR-0120 D5e] Decide whether this install must stop for the
11071221
* `isolated`-posture confirmation, and what attestation the ledger entry

0 commit comments

Comments
 (0)