Repository navigation
Commit 4c31821
fix(runtime): attribute the package DELETE and clear the disable record in the uninstall verb (#31183)
<!-- claude-projects-attribution: {"github_login":"marchtian"} -->
_Requested by **田騏** · [project
thread](https://claude.ai/code/project/chan_01VmuUhXU9SSUzqJYyXSmYSi?thread=cmsg_01VmuUhXU9SSUzqJYyXSmYSiUfC8G7ZWNaCdELoQcXkaCC)_
Fixes #31023
Clause-②: no
Metadata-protocol: none
## 维护者速读
- 改了什么:Studio
删除应用时,把当前登录的人传给卸载动作,被删的每条元数据在审计日志和历史里都记下是谁删的;「此包在本环境已停用」的记录改由卸载动作自己清掉,平台内部任何地方调用删除都会清。
- 为什么改:删除接口原先不传操作人,审计里删除人一栏是空的;cloud 的 AI
工具、本地安装撤销、导入回滚这些内部删除不清停用记录,重启后再装同一个包一装上就是停用。
- 风险与回滚:只改 runtime;卸载结果的 cleanups 里多一条
runtime.package-state;对一个已不存在的包发删除(404)现在也会清掉同名的停用记录;`sys_packages`
那一行的删除仍然没有审计行(见下方说明,需席位定夺)。回滚即 revert 本 PR。
-
先看:packages/runtime/src/package-state-host-store.ts(新的卸载清理)、packages/runtime/src/domains/packages.ts(DELETE
分支的操作人与「卸载动作已清则门不再写」判断)、packages/runtime/src/app-plugin.ts(start 里注册清理)。
- 两分钟验证:`pnpm --filter @objectstack/runtime exec vitest run
src/package-state-host-store.test.ts
src/domains/packages-write-actor-session.test.ts
src/package-uninstall-store-refusal.integration.test.ts`,预期三个文件全绿。
- 需要你做的:无,席位落地。
## What changes
- **The actor.** `DELETE /api/v1/packages/:id` now resolves the
request's authenticated user with `resolvePackageWriteActor` (ADR-0146
D1, the one helper its sibling doors use; no second copy) and hands it
to `protocol.deletePackage` as `actor`. The verb already forwarded
`actor` to each `deleteMetaItem`, so every metadata delete is now
credited: `provenance.attributedUserId` on the `sys_metadata` delete
(what the audit writer records as `sys_audit_log.user_id`) and
`recorded_by` on the history tombstone. A caller with no user id sends
no `actor`, as before. No new key: `actor` was already on
`DeletePackageRequest`.
- **The disable-record clear moves into the verb, through a seam it
already runs.** `@objectstack/metadata-protocol` cannot reach the
runtime's package-state store without importing `@objectstack/runtime`,
so the clear rides the protocol's uninstall-cleanup registry
(`registerUninstallCleanup`, ADR-0086 D3), which `deletePackage` runs as
its last step and install-local's `DELETE` runs too. `AppPlugin.start`
registers it once per protocol (`ensurePackageStateUninstallCleanup`,
name `runtime.package-state`); it resolves the store when it runs (the
host's `package-state-store` service, else the file under `OS_HOME`) and
clears the key the boot seed reads (`AppPlugin`'s project-context
environment). A refusing store is that cleanup's failed outcome on the
uninstall's `cleanups`, through the registry's one runner.
`protocol-package-uninstall.ts` is not edited.
- **The door writes the record only when the verb did not.** When
`persisted.cleanups` carries the `runtime.package-state` outcome, the
door leaves the record alone; with no persisted half, or a protocol on a
kernel that registered no such cleanup (no `AppPlugin`), it clears the
record itself exactly as before.
## Tests
- `package-uninstall-store-refusal.integration.test.ts` (real ObjectQL +
SQLite + protocol + dispatcher): a signed-in admin's DELETE credits
`u_admin` on the audited `sys_metadata` delete and on the history
tombstone; CONTROL: a system caller's DELETE credits no one.
- `packages-write-actor-session.test.ts`: the DELETE door hands the verb
the session user (with and without `keepData`); CONTROL: a system
context sends no `actor`.
- `package-state-host-store.test.ts` (real kernel, `ObjectQLPlugin` +
`AppPlugin` + SQLite): an in-process `deletePackage` clears the file
record and a reinstall after a restart comes up enabled; the same with a
host store; a refusing host store is the cleanup's failed outcome and
the record stays; the HTTP DELETE leaves the clear to the verb (one
write, not two).
- Four existing pins that asserted the exact `deletePackage` request now
expect the signed-in caller's `actor` beside the unchanged keys
(`packages-write-flag-arity`, `packages-write-doors-closed-query`,
`packages-uninstall-refuse-before-mutate`,
`packages-orgless-grants-capability-gate`). The edited test files take a
module-top `import '@objectstack/metadata'` (`check:test-source-alias`).
## Acceptance notes
- **Not done, for the seat to decide: the `sys_packages` delete is not
audited at all.** `sys_packages` is not an object; the package service
deletes its row with raw SQL through `engine.execute`, which runs no
hooks, so no `sys_audit_log` row exists for that delete with or without
an actor. Attributing it needs a new audit writer or a
`PackageService.delete` signature change, which this card's `Clause-②:
no` does not cover. What the person did is still recorded: each metadata
row the uninstall removed carries the actor.
- **A 404 DELETE now clears a record naming the id.** The verb runs its
cleanups for every id it is asked to uninstall, so a DELETE of an id
that is neither installed nor stored clears a disable record naming it.
Before, the door wrote nothing on a 404.
- **install-local's `DELETE` and the two rollbacks also clear the record
now**, since they reach the same runner (#18877 item 3: a package that
no longer exists has no lifecycle state).
- **The environment key is the boot seed's, not the request's.** On a
standalone boot both are unset; a host whose resolver sets a request
environment different from `AppPlugin`'s project context would already
have its disable door and boot seed disagree.
- **File surface:** `app-plugin.ts` and `package-state-host-store.ts`
(with its test) are outside the claim's declared surface; they are where
the seam has to be registered. No new package export
(`PACKAGE_STATE_UNINSTALL_CLEANUP` and
`ensurePackageStateUninstallCleanup` are module-internal; `index.ts`
names its exports).
- The audit pin reads the hook's `provenance.attributedUserId` rather
than a `sys_audit_log` row: `@objectstack/plugin-audit` is not a runtime
dependency, and its own tests pin that field into `user_id`.
## Verification
<details><summary>Reverse verification (each mutation landed on disk,
then restored to the HEAD blob)</summary>
- A1, the DELETE door sends no actor: 3 red (`a signed-in caller's
delete names the caller`, `beside keepData, the caller is still named`,
`a signed-in admin's delete names the admin on the audited delete and on
the history tombstone`); both CONTROLs stay green.
- A2, `AppPlugin.start` registers no cleanup: 3 red (the in-process
delete, the host-store clear, the refusing store).
- A3, the door clears even when the verb did: 1 red (`one write, not
two`).
</details>
<details><summary>Local runs</summary>
- `pnpm --filter @objectstack/runtime test` before the fixture update: 9
failed in 4 files, all the exact-request pins above, fixed in this PR;
the seven edited test files then: 7 files, 148 tests passed.
- `pnpm --filter @objectstack/runtime typecheck`: exit 0. eslint over
`packages/runtime` with `--no-inline-config`: exit 0.
- The derived gate list, run locally: see the report comment on the
card.
</details>
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent de3996b commit 4c31821
12 files changed
Lines changed: 305 additions & 17 deletions
File tree
- .changeset
- packages
- qa/http-conformance/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 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
162 | 162 | | |
163 | 163 | | |
164 | 164 | | |
165 | | - | |
| 165 | + | |
| 166 | + | |
166 | 167 | | |
167 | 168 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
31 | 31 | | |
32 | 32 | | |
33 | 33 | | |
34 | | - | |
| 34 | + | |
35 | 35 | | |
36 | 36 | | |
37 | 37 | | |
| |||
1396 | 1396 | | |
1397 | 1397 | | |
1398 | 1398 | | |
| 1399 | + | |
| 1400 | + | |
| 1401 | + | |
| 1402 | + | |
| 1403 | + | |
1399 | 1404 | | |
1400 | 1405 | | |
1401 | 1406 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
48 | 48 | | |
49 | 49 | | |
50 | 50 | | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
51 | 54 | | |
52 | 55 | | |
53 | 56 | | |
| |||
339 | 342 | | |
340 | 343 | | |
341 | 344 | | |
342 | | - | |
| 345 | + | |
| 346 | + | |
343 | 347 | | |
344 | 348 | | |
345 | 349 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
67 | 67 | | |
68 | 68 | | |
69 | 69 | | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
70 | 73 | | |
71 | 74 | | |
72 | 75 | | |
| |||
311 | 314 | | |
312 | 315 | | |
313 | 316 | | |
314 | | - | |
| 317 | + | |
| 318 | + | |
315 | 319 | | |
316 | 320 | | |
317 | 321 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
27 | 27 | | |
28 | 28 | | |
29 | 29 | | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
30 | 34 | | |
31 | 35 | | |
32 | 36 | | |
| |||
52 | 56 | | |
53 | 57 | | |
54 | 58 | | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
55 | 65 | | |
56 | 66 | | |
57 | 67 | | |
| |||
149 | 159 | | |
150 | 160 | | |
151 | 161 | | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
29 | 29 | | |
30 | 30 | | |
31 | 31 | | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
32 | 35 | | |
33 | 36 | | |
34 | 37 | | |
| |||
168 | 171 | | |
169 | 172 | | |
170 | 173 | | |
171 | | - | |
172 | | - | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
173 | 177 | | |
174 | 178 | | |
175 | 179 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
28 | 28 | | |
29 | 29 | | |
30 | 30 | | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
31 | 34 | | |
32 | 35 | | |
33 | 36 | | |
| |||
82 | 85 | | |
83 | 86 | | |
84 | 87 | | |
85 | | - | |
86 | | - | |
87 | | - | |
88 | | - | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
89 | 93 | | |
90 | 94 | | |
91 | 95 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
159 | 159 | | |
160 | 160 | | |
161 | 161 | | |
162 | | - | |
| 162 | + | |
163 | 163 | | |
164 | 164 | | |
165 | 165 | | |
| |||
413 | 413 | | |
414 | 414 | | |
415 | 415 | | |
416 | | - | |
| 416 | + | |
| 417 | + | |
417 | 418 | | |
418 | 419 | | |
419 | 420 | | |
| |||
3804 | 3805 | | |
3805 | 3806 | | |
3806 | 3807 | | |
| 3808 | + | |
| 3809 | + | |
| 3810 | + | |
| 3811 | + | |
| 3812 | + | |
| 3813 | + | |
| 3814 | + | |
3807 | 3815 | | |
3808 | 3816 | | |
3809 | 3817 | | |
| 3818 | + | |
3810 | 3819 | | |
3811 | 3820 | | |
3812 | 3821 | | |
| 3822 | + | |
3813 | 3823 | | |
3814 | 3824 | | |
3815 | 3825 | | |
| |||
3872 | 3882 | | |
3873 | 3883 | | |
3874 | 3884 | | |
3875 | | - | |
| 3885 | + | |
| 3886 | + | |
| 3887 | + | |
| 3888 | + | |
| 3889 | + | |
| 3890 | + | |
| 3891 | + | |
| 3892 | + | |
| 3893 | + | |
| 3894 | + | |
| 3895 | + | |
| 3896 | + | |
| 3897 | + | |
3876 | 3898 | | |
3877 | 3899 | | |
3878 | 3900 | | |
| |||
0 commit comments