Repository navigation
Commit 4363945
fix(metadata-protocol,runtime)!: the in-process uninstall verb refuses while an active validation rule reads a field the package adds (#31280)
domain:server#4 · marchtian · session_017ihL2dLQEXWGsE2SZ3Qn5F
<!-- claude-projects-attribution: {"github_login":"marchtian"} -->
_Requested by **田騏** · [project
thread](https://claude.ai/code/project/chan_01VmuUhXU9SSUzqJYyXSmYSi?thread=cmsg_01VmuUhXU9SSUzqJYyXSmYSiRpCzq4DthSjDjvD4ZgZ1Su)_
Fixes #31232
Clause-②: yes (narrowing)
Metadata-protocol: none
## 维护者速读
-
**改了什么**:卸载动词本身(`protocol.deletePackage`)现在在动任何东西之前,先问一遍「卸载前守卫」。runtime
把停用门/卸载门已经在用的那个检查注册成守卫:如果别的已启用包有生效中的校验规则在读这个包通过 `objectExtensions`
加的字段,进程内卸载也直接拒绝,抛 409
`PACKAGE_DISABLE_RESTRICTED_BY_VALIDATION_RULE`,消息同门上那句话,列出每条规则。拒绝时什么都不读、不动。
- **为什么改**:#31227 只让 HTTP 卸载门拒绝;平台内部直接调用卸载的路径(cloud 的卸载 AI
工具、本地安装的配对安装回滚、非托管导入回滚)不走那道门,仍会把字段撤掉,之后那条规则会让该对象的每次写入都报 400
`VALIDATION_FAILED`,而包已没了,无法再启用。
- **风险与回滚**:这是一次收窄(BREAKING):进程内调用方以前能卸载成功的请求,在有依赖规则时改为被 409
拒绝;两个回滚路径会把拒绝当作「未能完全删除」的残留照常报告。没有依赖规则的卸载、已停用包的卸载、没注册守卫的协议,行为都不变。没改
spec、没加错误码。#31227 已合并,本 PR 已合入 main:拒绝文案用卸载措辞,HTTP 门和卸载动词的守卫共用同一个函数
`packageUninstallDependentRulesRefusal`。回滚就是 revert 本 PR。
-
**先看**:`packages/metadata-protocol/src/protocol-package-uninstall.ts`(`askUninstallGuards`
及其在 `deletePackage`
里的调用位置:退役键拒绝之后、第一次读库之前);`packages/runtime/src/domains/packages.ts` 的
`ensureDependentRulesUninstallGuard` /
`packageUninstallDependentRulesRefusal`;`packages/runtime/src/domains/packages-disable-module-extension-fields.test.ts`
末尾的 `[#31232]` describe。
- **两分钟验证**:`pnpm --filter @objectstack/runtime exec vitest run
src/domains/packages-disable-module-extension-fields.test.ts -t
31232`,预期 4 条全绿:进程内卸载被 409 拒绝且什么都没读没动、HTTP 门答同一句话、两条
CONTROL(无依赖的包照常卸载并清停用记录;规则在租户覆盖层关掉后照常卸载)。
- **需要你做的**:维护者在 PR 上确认这次收窄(进程内卸载动词在有依赖规则时改为拒绝)。
## What changes
After #31227 the package door (`DELETE /api/v1/packages/:id`) refuses an
uninstall that would withdraw a field an active validation rule of
another enabled package reads. The uninstall verb itself,
`protocol.deletePackage` in `@objectstack/metadata-protocol`, never
asked, so every in-process caller withdrew the field unchecked. This
adds the seat-ruled seam and fills it:
- **The seam (`@objectstack/metadata-protocol`).**
`ObjectStackProtocolImplementation#registerUninstallGuard(name, guard)`,
the before-half of `registerUninstallCleanup`. A guard is handed `{
packageId, actor? }` and answers `undefined` (proceed) or `{ code,
status, message }` (refuse). `deletePackage` asks every registered guard
in registration order right after its retired-key refusal and BEFORE its
first read (`sys_metadata`), the registry's own refusal, the store
delete, the per-item deletes, the registry withdrawal and the cleanups;
the first refusal is thrown as an `Error` carrying the guard's
`message`, `code` and `status`. A guard that throws refuses with its
throw. One guard per name; a second registration replaces the first. The
types `UninstallGuard` and `UninstallGuardRefusal` are exported from the
package entry. With no guard registered the verb is byte-for-byte the
old path.
- **The registration (`@objectstack/runtime`).**
`ensureDependentRulesUninstallGuard(ctx)` registers, once per protocol,
the guard `runtime.package-dependent-rules`, called from
`AppPlugin.start` beside `ensurePackageStateUninstallCleanup` (the
after-half #31023 added). The guard resolves the engine's registry when
it is asked and answers `packageUninstallDependentRulesRefusal(registry,
packageId)`: the doors' verdict `readDisableDependentRules` and sentence
`disableDependentRulesMessage` under the lead `Package 'ID' cannot be
uninstalled`, as the wire pair `409`
`PACKAGE_DISABLE_RESTRICTED_BY_VALIDATION_RULE`.
`@objectstack/metadata-protocol` imports nothing from the runtime and
holds no copy of the rule.
- **No spec change, no new error code.** The code is the one already
ledgered under `@objectstack/runtime`.
## Tests
- **`packages/metadata-protocol/src/protocol.uninstall-guards.test.ts`
(new, 6 cases)**, a protocol over an engine double that counts every
step that reads or moves something (the `sys_metadata` read, the
registry's uninstallability question, the store delete, the registry
withdrawal, the cleanups):
- a refusing guard: the verb rejects with the guard's code, status and
message, and all five counts are 0;
- each guard is handed the package id and the request's actor; the first
refusal wins, a later guard is not asked, counts 0;
- a guard that throws: the same error object is rejected, counts 0;
- a second registration under one name replaces the first;
- CONTROL: guards that answer nothing, every step runs once;
- CONTROL: a bare protocol (no guard registered), every step runs once,
as before.
-
**`packages/runtime/src/domains/packages-disable-module-extension-fields.test.ts`,
new top-level describe `[#31232]` (4 cases)** in the shared
dependent-rule harness, on a real kernel (ObjectQL on SQLite,
MetadataPlugin, AppPlugin), the verb called in process on the kernel's
`protocol` service. Two writable packages extend the app's object with
one field each; a third package's active rule reads the first one's
field.
- **Refusal.** `deletePackage` rejects with `409` and the code; the
message equals the exported builder over the exported verdict under the
uninstall lead. Nothing was read or moved: `findEnvironmentRows`,
`deleteMetaItem`, `runUninstallCleanups` and `registry.uninstallPackage`
were never called, the package and its field are still served, the state
file is unchanged, and a write to the object lands.
- **The door answers the same sentence.** `DELETE /packages/:id` answers
`409`, the code, the same message, no `details`.
- **CONTROL: no dependent.** The second package, disabled first,
uninstalls in process; the verb's cleanups include
`runtime.package-state`, the package leaves the registry, its field
leaves the served object, its disable record is cleared, and the guard
stays registered.
- **CONTROL: rule switched off through the tenant overlay.** The first
package then uninstalls in process, its field leaves the served object,
and a write to the object lands.
## Acceptance notes
- **Re-measure at branch start (premise holds).** On this rig with the
guard call removed from the verb: the in-process `deletePackage`
answered `{ success: false, deletedCount: 0, failedCount: 0, cleanups:
[runtime.package-state ok] }` without throwing, the package left the
registry and its field left the served object, and the next write to the
object answered `400 VALIDATION_FAILED`: `Validation rule
'qz_tier_known' could not be evaluated (runtime: No such key: qz_tier) —
write rejected.`
- **Uninstall form in use.** #31227 landed (`57675a21`). The guard's
sentence is the builder's uninstall form,
`formatPackageDisableDependents(lead, dependents, { operation:
'uninstall' })` ("which the uninstall would withdraw", "or keep the
package installed"). The in-process refusal pin holds that sentence word
for word.
- **The door and the guard share
`packageUninstallDependentRulesRefusal`.** The `DELETE
/api/v1/packages/:id` arm asks that one function, with no inline scan,
and answers its code, status and message. The door keeps its check ahead
of `resolveProtocol` (seat ruling A), because a host whose protocol has
no `deletePackage` or no guard registered uninstalls through the
registry. With a guard registered, the door answers first and the verb's
guard is not reached on that path. An ablation of the door's call shows
the guard alone answers the identical 409, code and message.
- **The two rollback callers.** Install-local's pair-install undo and
the unmanaged-import rollback already catch a thrown uninstall and
report it as residue in their own failure answer, so a refusal there
leaves the partial package installed and says so; nothing new is
swallowed.
- **Not covered: install-local's own `DELETE` door.** `DELETE
/api/v1/marketplace/install-local/:manifestId`
(`@objectstack/cloud-connection`) does not call `deletePackage`: it
withdraws through `registry.uninstallPackage` and runs the cleanups
itself, so it neither asks the guards nor the verdict. Raised in the
report for the seat; outside this card's file surface.
## Verification
<details><summary>Ablation: the guard call removed from the verb, the
refusal pins go red</summary>
`node scripts/ablation-replace.mjs` replaced `await
askUninstallGuards(this.uninstallGuards, request);` in
`protocol-package-uninstall.ts` with a marker assignment (anchor `1 ->
0`, blob `24b1fe27dc2a -> b53ac531fceb` on the first spelling, whose
comment marker no build emits, so it was restored and redone with a code
marker), `@objectstack/metadata-protocol` was rebuilt, and `node
scripts/ablation-dist-preflight.mjs` found the marker in `dist/index.js`
and `dist/index.cjs` (the runtime suite reaches the verb through that
`dist/`). Then restored (blob equals HEAD, `git diff HEAD` empty),
rebuilt, and the preflight `--absent` leg found the marker in none of
the 24 built files.
- `protocol.uninstall-guards.test.ts`: 5 red, 1 green (the bare-protocol
CONTROL, which registers no guard). Refusal case: `promise resolved "{
success: false, …(5) }" instead of rejecting`.
- `[#31232]` describe: the refusal case red (`the in-process uninstall
was not refused`), the door case red by cascade (`expected { status: 404
… } to deeply equal { status: 409 … }`: the unrefused uninstall had
already removed the package); both CONTROLs green.
</details>
<details><summary>Rework 1 at head ec3a142 (after #31227
landed)</summary>
- `node scripts/pm/dispatch-gates.mjs --commands` derived 73 commands;
all 73 exit 0. `--ran`: 73 derived, 73 run, 0 NOT-MEASURED, 0 UNRUN.
- `pnpm --filter @objectstack/runtime test`: 555 files, 8953 tests
passed, 19 skipped. `pnpm --filter @objectstack/metadata-protocol test`:
325 files, 22542 tests passed, 26 skipped. Typecheck on both, and
`eslint packages/metadata-protocol packages/runtime --no-inline-config`:
exit 0.
- Harness `packages-disable-module-extension-fields.test.ts` +
`package-state-host-store.test.ts`: 56/56 (#31227's 4 and this PR's 4).
- Dogfood battery not rerun on this head (declared narrowing; last full
run green at e052b19).
</details>
<details><summary>Local gates and suites at head e052b19 (after
merging main)</summary>
- `node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack` derived 67 commands; all 67 exit 0.
`check:dual-build-cjs-loads` first exited 3 before a full `pnpm build`
and was rerun after it; `check:test-source-alias` first exited 1 on the
new metadata-protocol test (the protocol reaches `@objectstack/lint`
through a dynamic import) and was fixed by a module-top `import
'@objectstack/lint'`, as the gate prescribes.
- `--ran` reconciliation: 67 derived, 67 run, 0 NOT-MEASURED, 0 UNRUN.
- `pnpm --filter @objectstack/metadata-protocol test`: 323 files passed,
22533 tests passed, 26 skipped; `typecheck` exit 0.
- `pnpm --filter @objectstack/runtime test`: 555 files passed, 8943
tests passed, 19 skipped; `typecheck` exit 0.
- `eslint packages/metadata-protocol packages/runtime
--no-inline-config` (the root `pnpm lint` over the touched packages):
exit 0.
- Dogfood battery (`pnpm --filter @objectstack/dogfood test`, full, no
shard): 435 files passed, 3 skipped; 3610 tests passed, 41 skipped.
</details>
## Human Minimum
- [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic.
(`packages/spec` is not touched.)
- [x] 2. Every name I added uses the casing for its kind.
- [x] 3. No formatter ran; `pnpm lint` passes.
- [x] 4. One issue, one branch off an up-to-date `main`, one pull
request.
- [x] 5. A change that publishes carries a changeset, and both
declaration lines are filled in here and in the changeset.
- [x] 6. `packages/spec` touched: its generated artifacts are
regenerated and committed. (Not touched.)
- [x] 7. No hand edits to `content/docs/references/`,
`packages/*/CHANGELOG.md` or `content/docs/releases/`.
- [x] 8. Title, body and comments are in English.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---
_Generated by [Claude
Code](https://claude.ai/code/session_017ihL2c1rk1PGZ51s32cxZt)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent dc7f5c4 commit 4363945
11 files changed
Lines changed: 518 additions & 13 deletions
File tree
- .changeset
- packages
- metadata-protocol/src
- runtime/src
- domains
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
135 | 135 | | |
136 | 136 | | |
137 | 137 | | |
138 | | - | |
| 138 | + | |
139 | 139 | | |
140 | 140 | | |
141 | 141 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
12 | 12 | | |
13 | 13 | | |
14 | 14 | | |
15 | | - | |
| 15 | + | |
| 16 | + | |
16 | 17 | | |
17 | 18 | | |
18 | 19 | | |
| |||
52 | 53 | | |
53 | 54 | | |
54 | 55 | | |
| 56 | + | |
55 | 57 | | |
56 | 58 | | |
57 | 59 | | |
| |||
104 | 106 | | |
105 | 107 | | |
106 | 108 | | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
107 | 114 | | |
108 | 115 | | |
109 | 116 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
259 | 259 | | |
260 | 260 | | |
261 | 261 | | |
| 262 | + | |
262 | 263 | | |
263 | 264 | | |
264 | 265 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
54 | 54 | | |
55 | 55 | | |
56 | 56 | | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
57 | 88 | | |
58 | 89 | | |
59 | 90 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
57 | 57 | | |
58 | 58 | | |
59 | 59 | | |
| 60 | + | |
60 | 61 | | |
61 | 62 | | |
62 | 63 | | |
63 | | - | |
| 64 | + | |
64 | 65 | | |
65 | 66 | | |
66 | 67 | | |
| |||
83 | 84 | | |
84 | 85 | | |
85 | 86 | | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
86 | 106 | | |
87 | 107 | | |
88 | 108 | | |
| |||
392 | 412 | | |
393 | 413 | | |
394 | 414 | | |
| 415 | + | |
| 416 | + | |
| 417 | + | |
| 418 | + | |
| 419 | + | |
| 420 | + | |
| 421 | + | |
| 422 | + | |
| 423 | + | |
| 424 | + | |
| 425 | + | |
| 426 | + | |
| 427 | + | |
| 428 | + | |
| 429 | + | |
| 430 | + | |
| 431 | + | |
| 432 | + | |
| 433 | + | |
| 434 | + | |
395 | 435 | | |
396 | 436 | | |
397 | 437 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
527 | 527 | | |
528 | 528 | | |
529 | 529 | | |
| 530 | + | |
530 | 531 | | |
531 | 532 | | |
532 | 533 | | |
| |||
537 | 538 | | |
538 | 539 | | |
539 | 540 | | |
| 541 | + | |
| 542 | + | |
540 | 543 | | |
541 | 544 | | |
542 | 545 | | |
| |||
761 | 764 | | |
762 | 765 | | |
763 | 766 | | |
| 767 | + | |
| 768 | + | |
| 769 | + | |
764 | 770 | | |
765 | 771 | | |
766 | 772 | | |
| |||
1379 | 1385 | | |
1380 | 1386 | | |
1381 | 1387 | | |
| 1388 | + | |
1382 | 1389 | | |
1383 | 1390 | | |
1384 | 1391 | | |
| |||
1413 | 1420 | | |
1414 | 1421 | | |
1415 | 1422 | | |
| 1423 | + | |
| 1424 | + | |
| 1425 | + | |
| 1426 | + | |
| 1427 | + | |
| 1428 | + | |
| 1429 | + | |
| 1430 | + | |
| 1431 | + | |
| 1432 | + | |
| 1433 | + | |
| 1434 | + | |
| 1435 | + | |
| 1436 | + | |
| 1437 | + | |
| 1438 | + | |
| 1439 | + | |
| 1440 | + | |
| 1441 | + | |
| 1442 | + | |
| 1443 | + | |
1416 | 1444 | | |
1417 | 1445 | | |
1418 | 1446 | | |
| |||
10915 | 10943 | | |
10916 | 10944 | | |
10917 | 10945 | | |
10918 | | - | |
| 10946 | + | |
| 10947 | + | |
| 10948 | + | |
| 10949 | + | |
10919 | 10950 | | |
10920 | 10951 | | |
10921 | 10952 | | |
| |||
10938 | 10969 | | |
10939 | 10970 | | |
10940 | 10971 | | |
| 10972 | + | |
10941 | 10973 | | |
10942 | 10974 | | |
10943 | 10975 | | |
| |||
10947 | 10979 | | |
10948 | 10980 | | |
10949 | 10981 | | |
| 10982 | + | |
10950 | 10983 | | |
10951 | 10984 | | |
10952 | 10985 | | |
| |||
0 commit comments