Skip to content

Commit 901e7cf

Browse files
fix(cloud-connection): an install-local uninstall withdraws the package from the running kernel (#21581)
Fixes #21576 Clause-②: no ## What this changes - The install-local `DELETE /api/v1/marketplace/install-local/:manifestId` now withdraws the package from the running kernel. It does this right after the ledger removal, through `SchemaRegistry.uninstallPackage`, the one verb `deletePackage` uses. The uninstall cleanups from #21490 then run as before. - Every later reader of the registered packages is now right without a special case. That includes plugin-security's declared-permission seeding on `metadata:reloaded`, which is what re-projected the uninstalled package's set. - This follows triage's ruling `5968468119`: the first seam, the root. There is no "skip uninstalled" check in plugin-security. - Nothing changes in `packages/metadata-protocol`, `packages/plugins/plugin-security` or `packages/spec`. The install and rehydrate path of `marketplace-install-local-plugin.ts` is untouched. Files: - `packages/cloud-connection/src/marketplace-install-local-plugin.ts`: the uninstall path only. That is `handleUninstall`, the new `withdrawFromRunningKernel`, the corrected header block and the corrected log line. - `packages/cloud-connection/src/marketplace-install-local-uninstall-withdrawal.test.ts`: a new unit pin, 6 cases. - `packages/cli/test/package-install-local-uninstall-cleanups.integration.test.ts`: order 3's two `it.fails` promoted to plain `it`, a hot-object case in orders 1 and 2, and a new order 4. - `.changeset/21576-install-local-uninstall-withdraws-registration.md`: `@objectstack/cloud-connection` patch, `Clause-②: no`. ## A1: reproduction on `74281a8e4a` - I ran order 3 with its two `it.fails` changed to plain `it`, on packages built in this worktree. It read `Tests 2 failed | 11 passed (13)`. The 13 includes one temporary measurement case, used for A4 below. The two red cases are exactly the two set readings: - the other package's hot install re-projects the set; - the set survives the restart. - Then I restored the file: its blob equals HEAD's (`72bd1cc16e`), and `git status --porcelain` was empty. - After the fix, the same two cases are plain greens, and the grant-stays-revoked case stays green. ## A2: how the door reaches the registry - `deletePackage` reaches the registry as `this.engine.registry.uninstallPackage(packageId)`. `this.engine` is the ObjectQL engine the protocol is assembled over (`assembleMetadataProtocol(ctx, this.ql, …)` in `packages/objectql/src/plugin.ts`). - The same plugin registers that engine as the `objectql` service. The `manifest` service, which this door installs through, calls `ql.registerApp` on it. - So the door reads `ctx.getService('objectql').registry` and calls `uninstallPackage(manifestId)`. The call is typed by the spec contract `IObjectQLEngine.registry` (`EngineSchemaRegistryView.getPackage` / `uninstallPackage`), with no `as any`. - It uses one verb, and there is no second unregistration mechanism. - The id is the manifest id, which is the key `installPackage` files the record under. The cleanups get the same id. ## A3: order and failure The order is: ledger removal, then the withdrawal, then the cleanups. The withdrawal is synchronous, and nothing is awaited between it and the ledger removal. It goes first for three reasons: 1. `deletePackage` uses the same order: the registry withdrawal, then `runUninstallCleanups`. 2. It closes a window. The cleanups await the store row by row. If the package were still registered while they ran, a concurrent hot install's `metadata:reloaded` could re-project the sets they had just removed. 3. The one cleanup registered today, `security.package-permissions`, loses nothing. It selects by package id in the store and never reads the registry. Measured: orders 1 and 2 still report it as `success: true`, and the set and the grant are revoked. Nothing is withdrawn when the uninstall did not happen. The unit pin covers each case: - a refused caller (401); - an id this door never installed (404), even one the running registry holds; - a failed ledger write (500). When the withdrawal fails: - If the withdrawal throws (ADR-0029: another package extends an object this one owns), or the registry cannot be asked, the response carries one failed outcome named `registry.uninstallPackage` in `cleanups`. That is the way a failed cleanup is reported. - The cleanups still run, and the request still succeeds, because the ledger entry is already gone. - The operator log names the cause and the remedy. The thrown text stays in the log and never reaches the wire. When the registry does not hold the package (for example, a cloud install whose hot-register failed), there is nothing to withdraw: no call and no outcome. ## A4: what `uninstallPackage` withdraws, and what the object doors answer From `SchemaRegistry.uninstallPackage` (`packages/objectql/src/registry.ts`), the verb withdraws: - the package's object contributions, including the overlay layer over an owned object; - its namespace; - every metadata item keyed to the package (`unregisterItemsByPackage`); - its boot disable seed; - the package record. It does not touch tables or rows. **The object doors' answer moves, as intended.** These are the package object's answers on the data route, read hot in the same process right after the DELETE: | | order 1 | order 2 | order 3 | order 4 | |---|---|---|---|---| | before the fix (`74281a8e4a`, measured) | 200, empty list | 200, empty list | 200, empty list | (the order is new) | | after the fix (pinned) | 404 | 404 | read, not asserted | 404 | After the fix, the hot answer carries the same error code the object answers after a restart. This is the consequence of "withdraws the package from the running registry". Orders 1, 2 and 4 pin it. **The reinstall control.** I added order 4: hot install, then DELETE, then a hot reinstall of the same package in the same process. The object answers 200 again, and the set is projected again, exactly once, as the package's own. So the install path puts back everything the withdrawal removes. **One more value moves with the registry state.** A same-process reinstall after an uninstall is now classified as a fresh install, so the install answer's `upgradedFrom` is `null` instead of `previous-marketplace-version`. A reinstall after a restart already answered `null`. No key moves, no value leaves the existing set, and nothing in this repository reads the field. ## A5: the note - Before: "… The app remains loaded in the running kernel until the next restart (the kernel API does not support unregistering apps in-place)." - After: "Cached manifest removed, the package withdrawn from the running kernel, and the uninstall cleanups this runtime's plugins registered ran — each one's outcome is in `cleanups`." - When the withdrawal fails, the note says the package stays loaded until the next restart, and points at the `registry.uninstallPackage` entry. - The file header and the info log line are corrected the same way. Nothing parses the note. ## A6: reverse verification **The mutation.** `scripts/ablation-replace.mjs` replaced the anchor `registry.uninstallPackage(manifestId);` with a marker statement that logs `ABLATED_21576_withdrawal`. The anchor went x1 to x0 and the replacement x0 to x1. The blob changed from `7667b86205` to `83d263078b`. **The build.** I rebuilt `@objectstack/cloud-connection`. `ablation-dist-preflight` found the marker in `dist/index.js` and `dist/index.cjs`. **The readings:** - Unit (the withdrawal file and the #21490 cleanups file): `2 failed | 12 passed`. The two red cases are the ones that read the withdrawal. - Integration: `5 failed | 11 passed`. The red cases are: - order 3's two promoted set readings; - both hot-object cases; - order 4's precondition (404 right after the DELETE). - The controls stayed green: - orders 1 and 2: the preconditions, the DELETE reporting the security cleanup, and the set and grant revoked right after the DELETE and after a restart; - order 3: the grant stays revoked; - order 4: the reinstall. **The restore.** `ablation-replace` restored the file: its blob equals HEAD's `7667b86205`, and `git diff HEAD` is empty. After a rebuild, `ablation-dist-preflight --absent` found the marker absent from all 6 built files, with the whole tree clean. ## Tests and gates, at `9f3e65608d` - **Integration file**, on packages built in this worktree with no dist overlay (turbo build of `@objectstack/cli^...`, then `@objectstack/cloud-connection` rebuilt): `Test Files 1 passed (1) / Tests 16 passed (16)`. - **`@objectstack/cloud-connection` full suite:** `34 passed (34)` files, `420 passed (420)` tests. - **`@objectstack/cloud-connection` typecheck:** both programs are green, and `--listFiles` shows the new test file in both. - **`@objectstack/cli`:** typecheck is green, including `check:test-typecheck`, and the tier partition pin passes 22/22. The only `packages/cli` change is the integration-tier file above, which ran in full. The rest of the unit tier is declared to CI. - **`dispatch-gates --commands`:** it derives 64 commands, and all 64 exit 0. - `check:dual-build-cjs-loads` first answered PREREQUISITE NOT MET, because a whole-repo `dist/` was missing. After `pnpm build` it exits 0. - The `--ran` reconciliation with an exit code per line reads "64 run, 0 NOT-MEASURED (a DERIVED zero)". - Two path-matched families take a value from the workflow and cannot run locally: `check-issue-citations --census` and `check-shard-attestation`. NOT MEASURED, left to CI. - **`pnpm lint`** (`eslint . --no-inline-config`), the full run with no narrowing: exit 0. ## Acceptance notes - **A collision edge, not handled.** Suppose a ledger entry's id is also a config-defined app's id. The install door refuses to create that (`MANIFEST_CONFLICT`), but an older ledger can still overlay the app at rehydrate. The DELETE of that entry now withdraws the id from the running kernel until a restart registers the config app again. The #21490 cleanups already removed that id's package-managed sets. Carrier: none. - **State outside the registry is untouched by the withdrawal**, as it was before: the package's tables and rows, the handlers `bindArtifactHandlers` bound, and the i18n bundles and seed datasets stashed at install. Once the objects are withdrawn, the bound handlers have no object route to fire through. This PR does not change any of it, and I did not measure it further. - **An existing unit fake.** The `objectql.registry` fake in `marketplace-install-local-id-gate.test.ts` carries only `getAllPackages`. So its DELETE case now reports a failed `registry.uninstallPackage` outcome. That case asserts the status, `success` and the ledger removal, and all three still hold. I left it as is. - **A failed withdrawal cannot be retried through this door,** because the ledger entry is already gone. The remedy is the restart, and the log line names it. --- _Generated by [Claude Code](https://claude.ai/code/session_016GiHYRmLSNWTfbX9gVQkpz)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent a7ab047 commit 901e7cf

4 files changed

Lines changed: 461 additions & 36 deletions
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
"@objectstack/cloud-connection": patch
3+
---
4+
5+
An install-local uninstall (`DELETE /api/v1/marketplace/install-local/:manifestId`) now withdraws the package from the running kernel
6+
7+
Clause-②: no
8+
9+
- The DELETE used to remove the ledger entry and run the uninstall cleanups, but it left the package registered in the running kernel until the next restart. So another package's hot install re-ran the declared-permission seeding over the uninstalled package too. Its permission set came back as a package-managed row, and that row survived the restart as an orphan that an administrator could grant.
10+
- After the ledger entry is removed, the door now calls `SchemaRegistry.uninstallPackage`, the same verb the protocol's own uninstall uses, on the same registry. It does this before the cleanups run. The package's objects answer 404 straight away, not only after a restart, and no reader of the registered packages counts it again. A reinstall of the same package in the same process registers it again.
11+
- If the registry refuses the withdrawal, for example because another package extends an object this package owns, the uninstall still succeeds and the cleanups still run. The refusal is reported as a failed `registry.uninstallPackage` entry in `cleanups`. The operator log carries the cause and the remedy.
12+
- The response `note` no longer says the kernel cannot unregister a package in place. The request and response keys are unchanged.

‎packages/cli/test/package-install-local-uninstall-cleanups.integration.test.ts‎

Lines changed: 92 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -25,14 +25,20 @@
2525
* ## What each `it` reads
2626
*
2727
* One fixture, two orders of events, each probed through the data route a user
28-
* and an admin use: the set by name, the user grant of it by set id, and — after
29-
* the restart — the package's object. The grant is made through the data door
30-
* before the uninstall, so "no binding" is read off a row that existed, not off
31-
* an empty table.
28+
* and an admin use: the set by name, the user grant of it by set id, and the
29+
* package's object — right after the DELETE and after the restart. The grant is
30+
* made through the data door before the uninstall, so "no binding" is read off a
31+
* row that existed, not off an empty table.
3232
*
3333
* A third order of events measures the re-seed window — DELETE, then a hot
34-
* install of ANOTHER package, then restart — and records, as `it.fails`, the
35-
* defect it found there; the block above that `describe` says what it is.
34+
* install of ANOTHER package, then restart. #21576: the DELETE now withdraws the
35+
* package from the running kernel (`SchemaRegistry.uninstallPackage`, the verb
36+
* the protocol's own uninstall uses), so nothing re-projects its set there; the
37+
* block above that `describe` says what used to happen.
38+
*
39+
* A fourth order is the withdrawal's control: a hot reinstall of the SAME
40+
* package in the same process, right after its DELETE, registers it again —
41+
* whatever the withdrawal took, the install path puts back.
3642
*
3743
* ## Spawn shape
3844
*
@@ -261,6 +267,10 @@ interface Run {
261267
uninstall?: Answer;
262268
/** Same process, right after the DELETE answered. */
263269
after?: Grants;
270+
/** The package's object right before the DELETE — it answered, so a later 404 is the DELETE's. */
271+
objectBefore?: Answer;
272+
/** #21576: the package's object in the same process, right after the DELETE answered. */
273+
objectAfter?: Answer;
264274
/** A restart on the same home. */
265275
restarted?: Grants;
266276
/** The package's object after the restart — the uninstall's own effect, as the control. */
@@ -269,6 +279,10 @@ interface Run {
269279
otherInstall?: { exit: number | null; output: string };
270280
/** Re-seed order only: same process, right after that second install. */
271281
afterOtherInstall?: Grants;
282+
/** Reinstall order only: the same package installed again, in the same process, after its DELETE. */
283+
reinstall?: { exit: number | null; output: string };
284+
/** Reinstall order only: the object and the set right after that reinstall. */
285+
afterReinstall?: { object: Answer; sets: Answer };
272286
}
273287

274288
/**
@@ -288,8 +302,10 @@ async function grantThenUninstall(live: LiveStart, session: Session, run: Run):
288302
permission_set_id: setId,
289303
});
290304
run.before = await readGrants(live, session.token, setId);
305+
run.objectBefore = await http(live, 'GET', `/api/v1/data/${TASK}`, session.token);
291306
run.uninstall = await http(live, 'DELETE', UNINSTALL, session.token);
292307
run.after = await readGrants(live, session.token, setId);
308+
run.objectAfter = await http(live, 'GET', `/api/v1/data/${TASK}`, session.token);
293309
}
294310

295311
async function readAfterRestart(live: LiveStart, run: Run): Promise<void> {
@@ -298,7 +314,7 @@ async function readAfterRestart(live: LiveStart, run: Run): Promise<void> {
298314
run.object = await http(live, 'GET', `/api/v1/data/${TASK}`, token);
299315
}
300316

301-
const runs: Record<'hot' | 'restarted' | 'reseed', Run> = { hot: {}, restarted: {}, reseed: {} };
317+
const runs: Record<'hot' | 'restarted' | 'reseed' | 'reinstall', Run> = { hot: {}, restarted: {}, reseed: {}, reinstall: {} };
302318

303319
beforeAll(async () => {
304320
const root = mkdtempSync(join(tmpdir(), 'install-local-uninstall-'));
@@ -342,8 +358,9 @@ beforeAll(async () => {
342358
await stopGroup(e.child);
343359

344360
// ── order 3: hot install → DELETE → hot install of ANOTHER package → restart ──
345-
// The DELETE does not withdraw the package from the running kernel, so it is
346-
// still registered when the second install announces `metadata:reloaded`.
361+
// The second install announces `metadata:reloaded` into the process the
362+
// DELETE ran in, so whatever that process still counts as registered is
363+
// re-seeded — the withdrawal is what keeps the uninstalled package out of it.
347364
const reseedDir = join(root, 'reseed');
348365
mkdirSync(reseedDir, { recursive: true });
349366
const reseedHome = join(reseedDir, 'home');
@@ -357,7 +374,23 @@ beforeAll(async () => {
357374
const g = await bootStart(reseedDir, reseedHome, port);
358375
await readAfterRestart(g, runs.reseed);
359376
await stopGroup(g.child);
360-
}, 8 * BOOT_TIMEOUT_MS);
377+
378+
// ── order 4: hot install → DELETE → hot reinstall of the SAME package ──────
379+
// The withdrawal's control: whatever `uninstallPackage` took from the running
380+
// kernel, the install path registers again, in the same process.
381+
const reinstallDir = join(root, 'reinstall');
382+
mkdirSync(reinstallDir, { recursive: true });
383+
const h = await bootStart(reinstallDir, join(reinstallDir, 'home'), port);
384+
const hSession = await authenticate(h);
385+
runs.reinstall.install = await packageInstall(appDir, h);
386+
await grantThenUninstall(h, hSession, runs.reinstall);
387+
runs.reinstall.reinstall = await packageInstall(appDir, h);
388+
runs.reinstall.afterReinstall = {
389+
object: await http(h, 'GET', `/api/v1/data/${TASK}`, hSession.token),
390+
sets: await http(h, 'GET', `/api/v1/data/sys_permission_set?name=${PERMISSION_SET}`, hSession.token),
391+
};
392+
await stopGroup(h.child);
393+
}, 9 * BOOT_TIMEOUT_MS);
361394

362395
afterAll(async () => {
363396
for (const child of groups) await stopGroup(child);
@@ -392,6 +425,16 @@ describe('#21490: an install-local uninstall runs the registered uninstall clean
392425
expect(rowsOf(run.after!.bindings), JSON.stringify(run.after!.bindings.body)).toEqual([]);
393426
});
394427

428+
// #21576: the DELETE withdraws the package from the running kernel, so
429+
// its object answers at once what it used to answer only after a
430+
// restart — the same status and the same code.
431+
it('right after the DELETE: the package object answers what it answers after a restart — no restart needed', () => {
432+
const run = runs[name];
433+
expect(run.objectBefore?.status, JSON.stringify(run.objectBefore?.body)).toBe(200);
434+
expect(run.objectAfter?.status, JSON.stringify(run.objectAfter?.body)).toBe(404);
435+
expect(run.objectAfter?.body?.error?.code, JSON.stringify(run.objectAfter?.body)).toBe(run.object?.body?.error?.code);
436+
});
437+
395438
it('after a restart: still no set and no grant, and the package object is gone', () => {
396439
const run = runs[name];
397440
expect(run.object?.status, JSON.stringify(run.object?.body)).toBe(404);
@@ -403,19 +446,19 @@ describe('#21490: an install-local uninstall runs the registered uninstall clean
403446
});
404447
}
405448

406-
// ── The re-seed window: MEASURED RED, reported for filing, not fixed here ──
449+
// ── The re-seed window (#21576) ─────────────────────────────────────────
407450
//
408-
// This DELETE leaves the package registered in the running kernel until the
409-
// next restart (the response's own note says so), and plugin-security's
410-
// `metadata:reloaded` subscriber re-runs the declared-permission seeding over
411-
// every package the kernel holds. So another package's hot install before
412-
// that restart re-projects the uninstalled package's set as a fresh
413-
// `managed_by: package` row, and the restart leaves it orphaned: the package
414-
// is gone, its set is not. The grant does NOT come back — the cleanup deleted
415-
// the binding and the seeding writes none — and that half is pinned plainly.
451+
// This DELETE used to leave the package registered in the running kernel
452+
// until the next restart, and plugin-security's `metadata:reloaded`
453+
// subscriber re-runs the declared-permission seeding over every package the
454+
// kernel holds. So another package's hot install before that restart
455+
// re-projected the uninstalled package's set as a fresh `managed_by: package`
456+
// row, and the restart left it orphaned: the package gone, its set not. The
457+
// grant never came back — the cleanup deleted the binding and the seeding
458+
// writes none.
416459
//
417-
// The two set readings are `it.fails`: each turns red the day its half is
418-
// fixed, which is the cue to promote it to a plain assertion.
460+
// The DELETE now withdraws the package from the running kernel, so no reader
461+
// of the registered packages — that seeding included — counts it again.
419462
describe('hot install → DELETE → hot install of another package → restart (the re-seed window)', () => {
420463
it('precondition: both installs landed, and the DELETE revoked the set and its grant', () => {
421464
const run = runs.reseed;
@@ -439,14 +482,40 @@ describe('#21490: an install-local uninstall runs the registered uninstall clean
439482
expect(run.object?.status, JSON.stringify(run.object?.body)).toBe(404);
440483
});
441484

442-
it.fails('KNOWN-BROKEN: the other package\'s hot install re-projects the uninstalled package\'s set (promote to a plain assertion once fixed)', () => {
485+
it('the other package\'s hot install does not re-project the uninstalled package\'s set', () => {
443486
const run = runs.reseed;
444487
expect(rowsOf(run.afterOtherInstall!.sets), JSON.stringify(run.afterOtherInstall!.sets.body)).toEqual([]);
445488
});
446489

447-
it.fails('KNOWN-BROKEN: that re-projected set survives the restart as an orphan row (promote to a plain assertion once fixed)', () => {
490+
it('after the restart there is no package-managed set for the uninstalled package — no orphan row', () => {
448491
const run = runs.reseed;
449492
expect(rowsOf(run.restarted!.sets), JSON.stringify(run.restarted!.sets.body)).toEqual([]);
450493
});
451494
});
495+
496+
// ── The withdrawal's control (#21576) ───────────────────────────────────
497+
//
498+
// `uninstallPackage` takes the package's objects, namespace, metadata items
499+
// and record out of the running kernel. A hot reinstall of the same package
500+
// in the same process puts every one of them back: the object answers again
501+
// and the set is projected again — once, as the package's own.
502+
describe('hot install → DELETE → hot reinstall of the same package (the withdrawal\'s control)', () => {
503+
it('precondition: the install landed, and the DELETE took the object out of the running kernel', () => {
504+
const run = runs.reinstall;
505+
expect(run.install?.exit, run.install?.output).toBe(0);
506+
expect(run.uninstall?.status, JSON.stringify(run.uninstall?.body)).toBe(200);
507+
expect(run.objectBefore?.status, JSON.stringify(run.objectBefore?.body)).toBe(200);
508+
expect(run.objectAfter?.status, JSON.stringify(run.objectAfter?.body)).toBe(404);
509+
expect(rowsOf(run.after!.sets), JSON.stringify(run.after!.sets.body)).toEqual([]);
510+
});
511+
512+
it('the reinstall registers the package again: its object answers, and its set is projected once', () => {
513+
const run = runs.reinstall;
514+
expect(run.reinstall?.exit, run.reinstall?.output).toBe(0);
515+
expect(run.afterReinstall!.object.status, JSON.stringify(run.afterReinstall!.object.body)).toBe(200);
516+
expect(run.afterReinstall!.sets.status).toBe(200);
517+
expect(rowsOf(run.afterReinstall!.sets).map((r) => [r?.name, r?.managed_by, r?.package_id]), JSON.stringify(run.afterReinstall!.sets.body))
518+
.toEqual([[PERMISSION_SET, 'package', APP_ID]]);
519+
});
520+
});
452521
});

0 commit comments

Comments
 (0)