fix(cloud-connection): an install-local uninstall runs the protocol's registered uninstall cleanups - #21512
Conversation
…ission set or grant behind Reproduces the defect at the public door on both orders of events (hot install -> DELETE -> restart; install -> restart -> DELETE -> restart). Red on main until the uninstall runs the registered cleanups. Claude-Session: https://claude.ai/code/session_016GiHYRmLSNWTfbX9gVQkpz Co-Authored-By: Claude <noreply@anthropic.com>
… 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>
…ll cleanups Claude-Session: https://claude.ai/code/session_016GiHYRmLSNWTfbX9gVQkpz Co-Authored-By: Claude <noreply@anthropic.com>
…anups Claude-Session: https://claude.ai/code/session_016GiHYRmLSNWTfbX9gVQkpz Co-Authored-By: Claude <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 2 package(s): 19 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: ⛔ 4 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails. What this run could not see
Coarse fallback — 13 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 292fffcece61c7fa6c02ae8aff7fba06b534fa84 && git checkout 292fffcece61c7fa6c02ae8aff7fba06b534fa84
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 6dd99b82c38cd68b51c86241d53c5b7dda670a08 cd5cabce34aff75de8855ca4aa501c18d3a55020 && git checkout -B drift-repro 6dd99b82c38cd68b51c86241d53c5b7dda670a08 && git merge --no-ff cd5cabce34aff75de8855ca4aa501c18d3a55020
node scripts/docs-audit/affected-docs.mjs --json 6dd99b82c38cd68b51c86241d53c5b7dda670a08
|
…stall-local-uninstall-cleanups
…runUninstallCleanups deletePackage's step-7 loop over the registerUninstallCleanup registry moves verbatim into a public runUninstallCleanups method, and deletePackage calls it. The only change inside the loop is the warn tag, now [protocol.runUninstallCleanups]. install-local's uninstall door calls the same runner, so both doors run one registry through one runner. The new test pins the args each cleanup receives, failure as an outcome with driver text withheld, and the control: deletePackage reports exactly the runner's outcomes. Claude-Session: https://claude.ai/code/session_016GiHYRmLSNWTfbX9gVQkpz Co-Authored-By: Claude <noreply@anthropic.com>
…anups; Clause-② is yes The extracted runner is a new public method on the exported ObjectStackProtocolImplementation, an additive widening of @objectstack/metadata-protocol's public surface, so that package is graded minor and the declaration line reads Clause-②: yes. cloud-connection stays a patch. Claude-Session: https://claude.ai/code/session_016GiHYRmLSNWTfbX9gVQkpz Co-Authored-By: Claude <noreply@anthropic.com>
A third order of events in the uninstall pin: hot install, DELETE, then a hot install of another package, then restart. The DELETE leaves the package registered until the restart, and the second install's metadata:reloaded re-runs plugin-security's declared-permission seeding over it, so the uninstalled package's set is re-projected as a fresh managed_by: package row and survives the restart as an orphan. Its grant stays revoked; that half is a plain assertion. The two set readings are it.fails, measured red and reported for filing, not fixed here. Claude-Session: https://claude.ai/code/session_016GiHYRmLSNWTfbX9gVQkpz Co-Authored-By: Claude <noreply@anthropic.com>
…stall-local-uninstall-cleanups
Contract reviewServed-tier: Inputs: card #21490 (body and all 7 comments, triage through ACCEPT), PR #21512 (body, 9-file list, net diff against ① Derived judgments
Nothing in the diff was judged wrong. ② Semver level
③ Boundary flagsOpen question, round 1 (Clause-② A or B for the protocol edit) — answered by the seat ( Open questions, round 2 — none declared; none found. Round-1 deviations, each resolved at the head:
Round-2 deviations:
Out-of-scope findings:
Check-runs on the head: every latest run per name is Escalated: nothing. Implemented-by: VERDICT: PASS Generated by Claude Code |
Fixes #21490
Clause-②: yes
Status: both halves are in this diff; draft. This PR now carries the install-local door and the protocol runner the door calls. The runner is
runUninstallCleanups, a new public method on the exportedObjectStackProtocolImplementation. That widens@objectstack/metadata-protocol's public surface, so the seat ruledClause-②: yeson the card, and that package takes aminorchangeset. An at-tier contract review is owed before this PR enqueues.The defect, measured at the public door
On
main9ff7428, with an emptyos start, I installed a package that declares a permission set throughos package install. I granted the set to the operator through the data door, sentDELETE /api/v1/marketplace/install-local/com.example.tasksapp, and probed over REST.managed_by: package)handleUninstallremoved the ledger entry and nothing else.plugin-securityregisters the revocation (security.package-permissions) with the protocol's uninstall-cleanup registry, and before this change onlydeletePackageran that registry.Why not call an existing protocol door (measured)
DELETE /api/v1/packages/:idrefuses an install-local package outright:422 WRITABLE_PACKAGE_REQUIRED. The set survives, and the ledger keeps the package across a restart.deletePackage, measured over an install-local package's shape (registry entry only, nosys_packagesrow, nosys_metadatarows):400 TENANT_SCOPE_REQUIRED).allTenants: trueit answerssuccess: false(0 rows deleted).sys_packagesdelete and callsregistry.uninstallPackage, which withdraws the package from the running kernel. The door never did that, and it would leave the package's bound handlers and flows behind. It would also delete anysys_metadataoverlay rows bound to the package, dropping their tables by default.deletePackageand this door both call (the triage direction: one registration seam, both doors run it).What this PR changes
@objectstack/metadata-protocol(minor)runUninstallCleanups({ packageId, organizationId?, actor? })sits right afterregisterUninstallCleanup. Its request type isDeletePackageRequestpicked down to those three keys, and it answers oneUninstallCleanupOutcomeper registered cleanup.deletePackage's step-7 loop, moved verbatim. The only change inside the loop is the warn tag, now[protocol.runUninstallCleanups]instead of[protocol.deletePackage].deletePackagecalls it in place of the loop:const cleanups = await this.runUninstallCleanups(request);. There is no other change todeletePackage, and its existing suites are the control (see Tests).protocol.uninstall-cleanups-runner.test.ts. It pins the arguments each cleanup receives (with and without an organization and actor), failure as an outcome with the driver text withheld while the remaining cleanups still run, and the control:deletePackagecalls the runner once with its own request and reports exactly the runner's outcomes.@objectstack/cloud-connection(patch)handleUninstallruns the protocol's uninstall cleanups once the ledger entry is gone, throughprotocol.runUninstallCleanups.manifest.id, and a cloud install's ledgerpackageIdis the catalog id. It carries no organization, because an install-local package is installed for the whole runtime.actoris the admitted operator.cloud-connection.data.cleanups, the waydeletePackagereports them.warnwith its remedy: install the package again, then uninstall it again. The ledger entry is gone, so retrying the DELETE answers 404.protocolservice means no registry exists, socleanups: [].protocol.runUninstallCleanups, so "nothing to revoke" and "the revocation never ran" read differently.protocolslot is uncontracted, so this door narrows it per consumer: the verb name is this file's, and the request and outcome types are the producer's (DeletePackageRequest,UninstallCleanupOutcome). This is the same shapePackagesDomainProtocoluses fordeletePackageinpackages/runtime.@objectstack/cloud-connectiontherefore declares@objectstack/metadata-protocol, which it already received through@objectstack/runtime(check:undeclared-dep-imports).Tests
All at head
cd5cabce34(this branch after mergingorigin/main6dd99b82c3), on real built packages:turbo run build --filter=@objectstack/cli^...built every dependency,@objectstack/metadata-protocoland@objectstack/cloud-connectionincluded, andablation-dist-preflightread the runner's own warn tag in both of@objectstack/metadata-protocol's runtime bundles. No dist overlay.packages/cli/test/package-install-local-uninstall-cleanups.integration.test.ts,--project integration): 10 passed, 2 expected fail (12).security.package-permissionsreportedsuccess: true, no set and no grant right after, and no set, no grant and object 404 after a restart. The previous round read this green only through a temporary overlay of the built runner; it now reads green on the real build.main(ebf0b8e329, test only) the same 8 cases read 6 failed, 2 passed. That is the reproduction.protocol.uninstall-cleanups-runner,durable-package,protocol.driver-text-disclosure,protocol.marked-refusal-classification,protocol.package-delete-refusal): 5 files, 54/54.pnpm --filter @objectstack/metadata-protocol typecheckgreen, and the full suite: 207 files passed, 3 skipped; 3192 tests passed, 19 skipped. The new test is in the tsc program (--listFiles, count 1).pnpm --filter @objectstack/cloud-connection typecheck(both programs) green, and the full suite: 33 files, 414/414. The unit pinmarketplace-install-local-uninstall-cleanups.test.tsis 8/8 in it.pnpm --filter @objectstack/cli exec vitest run --project unit: 250 files passed. The 2 that failed,published-subpath-console.pinandpublished-subpath-hook-body.pin, refused at collection becausepackages/cliitself had nodist/(packages/cli is not built). That is a prerequisite, not a reading. Afterpnpm --filter @objectstack/cli buildthey passed, 29/29.test/vitest-tiers-partition.test.tsis in the unit run.The re-seed window (dispatch A4): measured RED, pinned as
it.fails, not fixed hereOrder 3 in the integration pin: hot install of the package, grant its set, DELETE it, then a hot install of a DIFFERENT package (
com.example.notesapp), then restart.managed_by: package,package_id: com.example.tasksappWhy: this DELETE leaves the package registered in the running kernel until the next restart (the response's note says so). The other install announces
metadata:reloaded, and plugin-security's subscriber re-runsbootstrapDeclaredPermissionsover every package the kernel still holds. That re-projects the uninstalled package's set. After the restart nothing selects it again: the package is gone, the row is not. The grant does not come back, because the cleanup deleted the binding and the seeding writes none.In the pin, the precondition and the "grant stays revoked, object gone" reading are plain assertions, both green. The two set readings are
it.fails: each turns red the day its half is fixed, which is the cue to promote it to a plain assertion. The finding goes to the seat for filing (see the report on the card); this PR does not fix it.Reverse verification
Both legs ran through
scripts/ablation-replace.mjs(WRAP mode, with a planted marker), from the committed headea93d2754e.marketplace-install-local-plugin.ts, the anchorconst cleanups = await this.runUninstallCleanups(ctx, manifestId, admission.userId);(1 hit) was replaced with an empty list plus the markerABLATION-21490-DOOR. The tool reported "anchor 1 → 0, blob f9929f2f700a → 1771b1b9b2f3".ablation-dist-preflightfound the marker in both runtime bundles,index.jsandindex.cjs. The package's DTS step failed on the now-unused private helper (TS6133), but the JS bundles the suites load were emitted and carried the marker.git diff HEADis empty". Rebuilt (exit 0).ablation-dist-preflight --absent: the marker is absent from all 6 built files, and the working tree is clean against HEAD.protocol.ts, the anchorfor (const [name, cleanup] of this.uninstallCleanups) {(1 hit, now only insiderunUninstallCleanups) was replaced with a loop over an empty Map plus the markerABLATION-21490-RUNNER. The tool reported "anchor 1 → 0, blob 4308b1f47cce → 721cc1b5c8b8". The build exited 0, and the marker was present inindex.jsandindex.cjs.deletePackagesuites went red, which showsdeletePackagereally runs through the runner.git diff HEADis empty". Rebuilt (exit 0).--absent: the marker is absent from all 24 built files, and the tree is clean.Gates
cd5cabce34,node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstackderived 77 commands, and all 77 exited 0.--ranreconciles to "77 derived, 77 run, 0 NOT-MEASURED, 0 UNRUN".pnpm check:dual-build-cjs-loadsfirst answeredPREREQUISITE NOT MET(exit 3: 9 packages had nodist/). Later gates in the same sweep built them, and the re-run exited 0.pnpm --filter @objectstack/spec check:generatedafter the merge: all 15 generated artifacts are up to date.pnpm lint(eslint . --no-inline-config) exited 0 atcd5cabce34.Acceptance notes
it.failsand reported for filing, not fixed here. A fix belongs where the package outlives its uninstall: in the running registry, or in the seeding's selection of packages.SchemaRegistry.uninstallPackageexists anddeletePackageuses it. The wording is kept here because withdrawing the package from the running kernel is outside this card. That withdrawal is also what would close the re-seed window.sys_packagesrow is gone. The warn names the remedy that works: install the package again, then uninstall it again.Generated by Claude Code