Skip to content

Commit 6fcecb2

Browse files
fix(cloud-connection,metadata-protocol)!: install-local's uninstall door asks the uninstall verb's guards and writes its audit row (#31439)
<!-- claude-projects-attribution: {"github_login":"os-project-manager"} --> _Requested by **Leehom** · [project thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1XTVkGrf2CjQTF79hBERgqiJ)_ domain:server#5 · os-project-manager · session_01FEkQ5ktJb5rQ5n2GUXSQMR Fixes #31283 Clause-②: yes (narrowing) — both arms. Narrowing: `DELETE /api/v1/marketplace/install-local/:manifestId` refuses, with the uninstall verb's guard refusal (today the runtime's `409 PACKAGE_DISABLE_RESTRICTED_BY_VALIDATION_RULE`), an uninstall it answered `200` before. Widening: two new public members of `ObjectStackProtocolImplementation` (`runUninstallGuards`, `recordPackageUninstall`) and one new exported type (`PackageUninstallRecord`), `@objectstack/metadata-protocol`. Metadata-protocol: none Skills-docs: one move — `content/docs/deployment/marketplace-handbook.mdx` (Uninstalling) states the refusal and the audit row; no `skills/**` page teaches this door's uninstall. ## 维护者速读 - 改了什么:本地安装自己的卸载入口 `DELETE /api/v1/marketplace/install-local/:manifestId` 现在和平台统一卸载动作走同一套守卫与审计:卸载前先问卸载守卫(其中有"别的包的启用校验规则正读着这个包扩展进来的字段"这条),被拒就回守卫给的 409 和原话、什么都不动;卸载成功后写一条 `package_uninstall` 审计记录。 - 为什么改:这个入口不能直接调用 `deletePackage`(没有存储行时它回 `success: false`,而且会删对象表),所以以前两件事都漏了:有依赖规则也照样卸载(字段消失后那条规则对该对象每次写入都失败关闭),也没有审计记录。 - 风险与回滚:协议上新增两个公开方法 `runUninstallGuards`、`recordPackageUninstall`,它们就是 `deletePackage` 自己调用的那一个守卫执行器和那一个审计写入器,裁决和写入都只有一份,入口里没有复制。老版本协议没有这两个方法时入口照旧。回滚即还原本 PR。 - 先看:`packages/cloud-connection/src/marketplace-install-local-plugin.ts` 的 `handleUninstall`(新的守卫块和末尾的 `recordUninstall`)与 `askUninstallGuards` / `recordUninstall`;`packages/metadata-protocol/src/protocol-package-uninstall.ts` 的 `runUninstallGuards` / `recordPackageUninstall`;`protocol.ts` 两个新成员。 - 两分钟验证:`pnpm --filter @objectstack/cloud-connection exec vitest run src/marketplace-install-local-uninstall-guards-audit.test.ts`,预期 2 个用例全绿(拒绝 409 + 对照写一条审计)。 - 需要你做的:无,席位落地。 ## What changes **Route.** The order prefers delegating the door to `deletePackage` if that keeps the door's withdrawal semantics. It does not: the verb answers `success: false` for a package with no stored `sys_metadata` rows (every install-local package; this is why `runUninstallCleanups` was extracted for this door, #21490, and its docblock says so), it deletes the `sys_packages` row and drops the tables of the objects it removes, and it does not do the door's own steps (the ledger, the object-copy withdrawal, the extended-copy restore, the `metadata:reloaded` re-sync). So the door calls public entry points instead, as the order's second route reads. Neither is a `packages/spec` member: like `runUninstallCleanups`, both live on the class only, so no `needs_decision`. 1. **`@objectstack/metadata-protocol`.** - `runUninstallGuards(request)`: the one runner of the pre-uninstall guard registry (#31232). `deletePackage` now asks its guards through it, so the verb and the door run the same function. It throws the first refusal with the guard's `code`, `status` and `message`; a guard's own throw propagates. - `recordPackageUninstall(record)`: the one writer of the `package_uninstall` row (#31186). `deletePackage` now writes its row through it. It adds the protocol's environment and hands the `audit` slot one event; no slot writes nothing, and a slot that throws is logged, never thrown. - `PackageUninstallRecord`, the record type, exported beside `UninstallGuardRefusal`. The module-private writer is renamed `writePackageUninstallEvent`; nothing else in `deletePackage` moves. 2. **`@objectstack/cloud-connection`, `handleUninstall`.** - After the wall check and before the registry's own uninstall refusal (#28511), the door asks `protocol.runUninstallGuards({ packageId, actor })`. A coded 4xx refusal is the request's answer, as the guard spelled it, with nothing moved. Any other throw propagates, as it does out of the verb. - After the uninstall is durable (ledger entry gone, withdrawal, cleanups, re-sync), the door calls `protocol.recordPackageUninstall({ packageId, version, actor, itemsRemoved: 0, itemsFailed: 0 })`. The version is the ledger entry's, read before its removal. The two counts are of stored `sys_metadata` items, which an install-local package does not have. - A protocol without either member (an older build) is not asked, and the door behaves as before: the capability question the door already asks of `runUninstallCleanups`. The narrowing `UninstallCleanupRunner` type gains the two members as indexed accesses on the class, never restated. 3. **Docs.** `marketplace-handbook.mdx`, step 9 (Uninstalling): the refusal and the audit row, one short paragraph. ## Tests - New `packages/cloud-connection/src/marketplace-install-local-uninstall-guards-audit.test.ts`. Real `ObjectKernel` + `ObjectQLPlugin` (the real registry and the real `ObjectStackProtocolImplementation` it registers as `protocol`), plus the runtime's `AppPlugin`, whose `start()` registers the real dependent-rule guard. The `audit` slot is a recorder. Packages are registered as the runtime's #30999 rig registers them; ledger entries as an install writes them. - A package whose `objectExtensions` field an active rule of another package reads: `409 PACKAGE_DISABLE_RESTRICTED_BY_VALIDATION_RULE` with exactly the sentence `formatPackageDisableDependents(...)` builds under the uninstall lead. Nothing moved: ledger entry kept, package still registered, field still served, no cleanup run, no audit event. - CONTROL: a package whose field no rule reads uninstalls, `200`. The audit slot receives exactly one event: `{ action: 'package_uninstall', packageId, version: '1.0.0', actor, itemsRemoved: 0, itemsFailed: 0 }`. - `packages/metadata-protocol/src/protocol.uninstall-guards.test.ts`, four new cases. `runUninstallGuards` throws the first refusal, hands the guard the actor, and reads or moves nothing. It resolves with no refusal and on a bare protocol. `recordPackageUninstall` hands the slot one event carrying the protocol's environment, and writes nothing and does not throw with no slot. The existing #31232 / #31186 `deletePackage` cases stay green through the refactor. <details><summary>Red on main (new door test run at base 405a78e, before the fix)</summary> ``` × a package whose extension field an active rule of another package reads is refused with the verb's 409; nothing moves × CONTROL — a package whose field no rule reads uninstalls and the protocol records one package_uninstall event AssertionError: {"success":true,"data":{"manifestId":"com.acme.tiers","cleanups":[{"name":"runtime.package-state","success":true,"removed":0}],"note":"Cached manifest removed, the package withdrawn from the running kernel, …"}}: expected 200 to be 409 AssertionError: expected [] to deeply equal [ { …(6) } ] Tests 2 failed (2) ``` </details> <details><summary>Ablations (fix committed at ef3daa6, each restored)</summary> Both through `node scripts/ablation-replace.mjs` on `marketplace-install-local-plugin.ts`. The suite imports the plugin from `src/` relatively, so no build applies. 1. Guard ask removed (`const refused = await this.askUninstallGuards(…)` → `const refused = undefined;`): ``` ok mutation landed: anchor 1 -> 0, blob e9f4cc2 -> 2e788c35b432 × a package whose extension field an active rule of another package reads is refused with the verb's 409; nothing moves AssertionError: {"success":true,…}: expected 200 to be 409 Tests 1 failed | 1 passed (2) ok restored: blob == HEAD (e9f4cc2) and `git diff HEAD` is empty ``` 2. Audit write removed (`await this.recordUninstall(…)` → `void removedVersion;`): ``` ok mutation landed: anchor 1 -> 0, blob e9f4cc2 -> 1a0037367d2a × CONTROL — a package whose field no rule reads uninstalls and the protocol records one package_uninstall event AssertionError: expected [] to deeply equal [ { …(6) } ] Tests 1 failed | 1 passed (2) ok restored: blob == HEAD (e9f4cc2) and `git diff HEAD` is empty ``` </details> ## Verification All on head `645c9512` (the fix, the changeset and docs, and one merge of `origin/main`), each exit code captured before any pipe: - **Full build** `pnpm turbo run build --concurrency=2` under the verify lock: VERDICT command-exit 0. - **Derived gate union**, `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack`: 102 families, every one exit 0 on its first run. `--ran`: "102 derived famil(ies) accounted for — 102 run, 0 NOT-MEASURED (a DERIVED zero — all 102 recorded an exit code and none of them is 3)". `check:type-check-debt` was also rerun after the dogfood run released the lock: exit 0. - **Packages**: `pnpm --filter @objectstack/cloud-connection test`: Test Files 75 passed, Tests 975 passed (VERDICT 0). `pnpm --filter @objectstack/metadata-protocol test`: Test Files 326 passed / 4 skipped, Tests 22563 passed / 26 skipped (VERDICT 0). `typecheck` on both: exit 0. - **Lint**: `eslint --no-inline-config` on the seven touched `.ts` files: exit 0. - **Full dogfood battery** (`(narrowing)` arm), `pnpm --filter @objectstack/dogfood test` under the verify lock: VERDICT command-exit 0, Test Files 438 passed / 3 skipped (441), Tests 3622 passed / 41 skipped (3663). - CI-only families (Test Core shards, the type-check lanes, Build Docs) are read from this PR's check-runs, not re-run here. ## Acceptance notes - **Why not delegate.** See *Route* above: `deletePackage` would answer `success: false`, delete a `sys_packages` row the package never had, and drop object tables the door has always kept ("Records stored in the app's objects stay in the database", marketplace handbook). - **Order of the door's refusals.** The wall check (ADR-0131 D12), then the verb's guards (this PR), then the registry's own uninstall refusal (`assertPackageUninstallable`, 409 `RESOURCE_CONFLICT`). The verb asks guards first too. - **`itemsRemoved: 0`.** The row's counts are of stored `sys_metadata` items removed, which an install-local package has none of. The door says so in the comment beside the call. - **Serial note.** Open PRs #31402 and #31398 touch the same file outside the uninstall door. `origin/main` was merged once (merge `645c9512`); neither had landed. - **Governed surface.** None: two packages' sources and tests, one changeset, one docs page (`content/docs/**`, not a register row). 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01FEkQ5ktJb5rQ5n2GUXSQMR --- _Generated by [Claude Code](https://claude.ai/code/session_01FEkQ5ktJb5rQ5n2GUXSQMR)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 0e1311d commit 6fcecb2

9 files changed

Lines changed: 492 additions & 10 deletions
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
---
2+
'@objectstack/cloud-connection': patch
3+
'@objectstack/metadata-protocol': patch
4+
---
5+
6+
fix(cloud-connection,metadata-protocol)!: install-local's uninstall door asks the uninstall verb's guards and writes its audit row (#31283)
7+
8+
Clause-②: yes (narrowing) — both arms. Narrowing: `DELETE /api/v1/marketplace/install-local/:manifestId` refuses, with the uninstall verb's guard refusal (today the runtime's `409 PACKAGE_DISABLE_RESTRICTED_BY_VALIDATION_RULE`), an uninstall it answered `200` before: one that would withdraw a field an active validation rule of another enabled package reads. Widening: two new public members of `ObjectStackProtocolImplementation` (`@objectstack/metadata-protocol`), `runUninstallGuards` and `recordPackageUninstall`, and one new exported type, `PackageUninstallRecord`.
9+
Metadata-protocol: none
10+
Skills-docs: one move — `content/docs/deployment/marketplace-handbook.mdx` (Uninstalling) states the refusal and the audit row; no `skills/**` page teaches this door's uninstall.
11+
12+
<!-- adr-0087: not-required (no-migration-prescription) no key of any schema is added, removed or re-shaped and no stored row is read differently: the narrowing is one runtime refusal at one door, the refusal `DELETE /api/v1/packages/:id` already answers for the same package, and its remedy is the rule it names (switch the rule off or change it, or keep the package installed); the two protocol members are additions, so there is nothing for `objectstack migrate meta` to rewrite. -->
13+
14+
**BREAKING** for a caller of install-local's uninstall door that uninstalled a package another package's active validation rule depends on.
15+
16+
- **Before.** `DELETE /api/v1/marketplace/install-local/:manifestId` withdrew the package and ran the protocol's uninstall cleanups, but never went through the uninstall verb (`deletePackage`), so it skipped what the verb does before and after: the pre-uninstall guards (the runtime's dependent-rule refusal) and the package's own `package_uninstall` audit row. Uninstalling a package whose `objectExtensions` field an active rule of another package reads answered `200`; the field left the served object and the rule then failed closed on every write to it. No uninstall through this door was audited.
17+
- **Now.** The door asks the verb's guards before anything moves, through the protocol's one runner of them (`runUninstallGuards`, which `deletePackage` asks too), and answers a refusal as the guard spelled it — code, status and message — with the ledger entry, the registration and the served fields as they were. A completed uninstall writes one `package_uninstall` row through the protocol's one writer of it (`recordPackageUninstall`, which `deletePackage` calls too), naming the package, the version removed and the operator.
18+
- **Unchanged.** `deletePackage` asks the same guards and writes the same row as before. A runtime with no audit plugin writes no row; a protocol from a build without the two members is not asked, and the door behaves as before.
19+
20+
**The remedy.** Switch off or change the rule the refusal names, or keep the package installed.

‎content/docs/deployment/marketplace-handbook.mdx‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,11 @@ the detail.
254254
from the running kernel and revokes its permission sets and their grants. The
255255
answer's `note` and `cleanups` say whether any part stays loaded until the
256256
next restart. **Records stored in the app's objects stay in the database.**
257+
An uninstall that would withdraw a field an active validation rule of
258+
another enabled package reads is refused with
259+
`409 PACKAGE_DISABLE_RESTRICTED_BY_VALIDATION_RULE`, naming the rule, and
260+
nothing is removed — the same refusal `DELETE /api/v1/packages/:id` answers.
261+
A completed uninstall is recorded in the audit log (`package_uninstall`).
257262

258263
## Staying current with platform versions
259264

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

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -427,6 +427,17 @@ type UninstallCleanupRunner = {
427427
runUninstallCleanups?(
428428
request: Pick<DeletePackageRequest, 'packageId' | 'actor'> & { declared?: UninstallDeclared },
429429
): Promise<UninstallCleanupOutcome[]>;
430+
/**
431+
* [#31283] The verb's pre-uninstall guards, asked through the protocol's
432+
* one runner of them — the runner `deletePackage` asks at its first step.
433+
*/
434+
runUninstallGuards?: ObjectStackProtocolImplementation['runUninstallGuards'];
435+
/**
436+
* [#31283] The verb's `package_uninstall` ledger row (#31186), written
437+
* through the protocol's one writer of it — the writer `deletePackage`
438+
* calls at its last step.
439+
*/
440+
recordPackageUninstall?: ObjectStackProtocolImplementation['recordPackageUninstall'];
430441
};
431442

432443
/** The outcome name this door reports when the runner itself could not run. */
@@ -3343,6 +3354,20 @@ export class MarketplaceInstallLocalPlugin implements Plugin {
33433354
const walled = this.refuseWalledCatalogPackage(c, ctx, 'Uninstalling', manifestId, admission);
33443355
if (walled) return walled;
33453356
}
3357+
// [#31283] THE UNINSTALL VERB'S PRE-UNINSTALL GUARDS, ASKED BEFORE
3358+
// ANYTHING MOVES. This door cannot delegate to `deletePackage` (it
3359+
// answers `success: false` for a package with no stored rows, and
3360+
// drops the tables of the objects it removes), so it asks the verb's
3361+
// guards through the protocol's one runner of them
3362+
// (`runUninstallGuards`) — the runtime's dependent-rule refusal (#31232:
3363+
// an active validation rule of another enabled package reads a field
3364+
// this package adds through `objectExtensions`) among them — and holds
3365+
// no copy of any verdict. A refusal is the request's, spelled as the
3366+
// guard spelled it (`code`, `status`, `message`), with nothing moved.
3367+
{
3368+
const refused = await this.askUninstallGuards(c, ctx, manifestId, admission.userId);
3369+
if (refused) return refused;
3370+
}
33463371
// [#28511, ADR-0146 D3] THE REGISTRY'S UNINSTALL REFUSAL, ASKED BEFORE
33473372
// THE LEDGER WRITE. The withdrawal below asks the registry to
33483373
// uninstall the package, and the registry refuses a base whose paired
@@ -3383,6 +3408,8 @@ export class MarketplaceInstallLocalPlugin implements Plugin {
33833408
}
33843409
}
33853410
}
3411+
// [#31283] The version this uninstall removes, for the package's ledger row.
3412+
const removedVersion = this.ledger.read(manifestId).entry?.version;
33863413
try {
33873414
this.ledger.remove(manifestId);
33883415
} catch (err: any) {
@@ -3444,6 +3471,13 @@ export class MarketplaceInstallLocalPlugin implements Plugin {
34443471
// re-sync would only read it back.
34453472
const resync = withdrawal ? undefined : await this.announceUninstall(ctx, manifestId);
34463473
if (resync) cleanups.push(resync);
3474+
// [#31283] The package's own `package_uninstall` ledger row (#31186),
3475+
// written once the uninstall is durable — the ledger entry is gone —
3476+
// through the protocol's one writer of it (`recordPackageUninstall`),
3477+
// never on a refusal above. `itemsRemoved` / `itemsFailed` count the
3478+
// stored `sys_metadata` items an uninstall removes: an install-local
3479+
// package is registered from its ledger entry and has none.
3480+
await this.recordUninstall(ctx, manifestId, removedVersion, admission.userId);
34473481
const withdrawals = (withdrawal ? 1 : 0) + (copies ? 1 : 0) + (resync ? 1 : 0);
34483482
ctx.logger?.info?.(
34493483
`[MarketplaceInstallLocal] uninstalled ${manifestId} (cached manifest removed; `
@@ -3696,6 +3730,65 @@ export class MarketplaceInstallLocalPlugin implements Plugin {
36963730
};
36973731
};
36983732

3733+
/**
3734+
* [#31283] Ask the uninstall verb's pre-uninstall guards about `manifestId`
3735+
* through the protocol's one runner of them (`runUninstallGuards`), and
3736+
* answer the first refusal as this request's: `{ success: false, error:
3737+
* { code, message } }` under the guard's `status`. `undefined` when no
3738+
* guard refuses.
3739+
*
3740+
* No `protocol` service, or a protocol from a build without the runner,
3741+
* holds no guard this door can ask, and the door behaves as before — the
3742+
* capability question {@link runUninstallCleanups} asks of its runner. A
3743+
* throw that is not a coded client refusal is the guard's failure to
3744+
* judge: it propagates, as it does out of `deletePackage`, and the
3745+
* uninstall does not proceed.
3746+
*/
3747+
private askUninstallGuards = async (
3748+
c: any,
3749+
ctx: PluginContext,
3750+
manifestId: string,
3751+
actor: string,
3752+
): Promise<Response | undefined> => {
3753+
let protocol: UninstallCleanupRunner | undefined;
3754+
try { protocol = ctx.getService<UninstallCleanupRunner>('protocol'); } catch { /* no protocol service */ }
3755+
if (typeof protocol?.runUninstallGuards !== 'function') return undefined;
3756+
try {
3757+
await protocol.runUninstallGuards({ packageId: manifestId, ...(actor ? { actor } : {}) });
3758+
return undefined;
3759+
} catch (err: any) {
3760+
const status = err?.status;
3761+
if (typeof err?.code === 'string' && typeof status === 'number' && status >= 400 && status < 500) {
3762+
return c.json({ success: false, error: { code: err.code, message: String(err.message) } }, status);
3763+
}
3764+
throw err;
3765+
}
3766+
};
3767+
3768+
/**
3769+
* [#31283] Write the package's `package_uninstall` ledger row through the
3770+
* protocol's one writer of it (`recordPackageUninstall`). No `protocol`
3771+
* service, or a protocol from a build without the writer, has no row to
3772+
* write; the writer itself never throws.
3773+
*/
3774+
private recordUninstall = async (
3775+
ctx: PluginContext,
3776+
manifestId: string,
3777+
version: string | undefined,
3778+
actor: string,
3779+
): Promise<void> => {
3780+
let protocol: UninstallCleanupRunner | undefined;
3781+
try { protocol = ctx.getService<UninstallCleanupRunner>('protocol'); } catch { /* no protocol service */ }
3782+
if (typeof protocol?.recordPackageUninstall !== 'function') return;
3783+
await protocol.recordPackageUninstall({
3784+
packageId: manifestId,
3785+
...(version ? { version } : {}),
3786+
...(actor ? { actor } : {}),
3787+
itemsRemoved: 0,
3788+
itemsFailed: 0,
3789+
});
3790+
};
3791+
36993792
/**
37003793
* [#21490] Run the protocol's registered uninstall cleanups for a package
37013794
* this door just removed from its ledger — ADR-0086 D3's data-plane

0 commit comments

Comments
 (0)