Skip to content

fix(cloud-connection,metadata-protocol)!: install-local's uninstall door asks the uninstall verb's guards and writes its audit row - #31439

Draft
objectstack-fleet[bot] wants to merge 4 commits into
mainfrom
claude/project-thread-f6p2x0
Draft

objectstack-fleet[bot] wants to merge 4 commits into
mainfrom
claude/project-thread-f6p2x0

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Requested by Leehom · project thread

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.
  2. @objectstack/cloud-connection, handleUninstall.
  3. Docs. marketplace-handbook.mdx, step 9 (Uninstalling): the refusal and the audit row, one short paragraph.

Tests

Red on main (new door test run at base 405a78e, before the fix)
× 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)
Ablations (fix committed at ef3daa6, each restored)

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 e9f4cc22a5db -> 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 (e9f4cc22a5db) and `git diff HEAD` is empty
  1. Audit write removed (await this.recordUninstall(…) → void removedVersion;):
ok mutation landed: anchor 1 -> 0, blob e9f4cc22a5db -> 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 (e9f4cc22a5db) and `git diff HEAD` is empty

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

🤖 Generated with Claude Code

https://claude.ai/code/session_01FEkQ5ktJb5rQ5n2GUXSQMR


Generated by Claude Code

…or asks the verb's guards and writes its audit row

The door cannot delegate to deletePackage, so it skipped the verb's
pre-uninstall guards (the dependent-rule refusal) and its package_uninstall
ledger row. The protocol now exposes the one runner of its guards
(runUninstallGuards) and the one writer of that row
(recordPackageUninstall), both of which deletePackage itself calls; the
door asks the guards before its first write and records the row after a
completed uninstall.

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added the size/l label Nov 8, 2026
@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling Fix lands in tooling, not a product package — enters pm:queue only with Unblocks: or a named surface labels Nov 8, 2026
@github-actions

github-actions Bot commented Nov 8, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/cloud-connection, @objectstack/metadata-protocol, touching 30 documentable anchor(s). ⚠️ 1 changed file(s) yielded no anchor (packages/metadata-protocol/src/index.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

28 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 3d73c1d618e3eede73eb71f647bb6ac1b8bfbb2c.

⛔ 11 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 1 changed file(s) yielded no anchor (packages/metadata-protocol/src/index.ts) — pages documenting those are invisible to this run
  • 2 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 64 of 227 client-bound route-ledger rows — the other 163 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 163: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 59 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 104 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 15 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 3d73c1d618e3eede73eb71f647bb6ac1b8bfbb2c → packageMentionDocs.

Which tree this was computed on

This run read content/docs from bfaa1afe3e3a243a0fc29de63cf6abb17a97a923 — the merge of head 332beed051a7138b53c5b0afa3c020fd1190c027 into base 3d73c1d618e3eede73eb71f647bb6ac1b8bfbb2c, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin bfaa1afe3e3a243a0fc29de63cf6abb17a97a923 && git checkout bfaa1afe3e3a243a0fc29de63cf6abb17a97a923
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 3d73c1d618e3eede73eb71f647bb6ac1b8bfbb2c 332beed051a7138b53c5b0afa3c020fd1190c027 && git checkout -B drift-repro 3d73c1d618e3eede73eb71f647bb6ac1b8bfbb2c && git merge --no-ff 332beed051a7138b53c5b0afa3c020fd1190c027

node scripts/docs-audit/affected-docs.mjs --json 3d73c1d618e3eede73eb71f647bb6ac1b8bfbb2c

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 3d73c1d618e3eede73eb71f647bb6ac1b8bfbb2c → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#5 · os-project-manager · session_01FEkQ5o8PE7ZtVjcor51wBZ

Contract-review-claim: session_01FEkQ5o8PE7ZtVjcor51wBZ · head 645c951 · 2026-11-08T08:01Z

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#5 · os-project-manager · session_01FEkQ5o8PE7ZtVjcor51wBZ

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 645c95126b5e7006d9944a06dcd738cc47217c2d
Local-runs: none
Scope: packages/cloud-connection/src/marketplace-install-local-plugin.ts, packages/cloud-connection/src/marketplace-install-local-uninstall-guards-audit.test.ts, packages/metadata-protocol/src/index.ts, packages/metadata-protocol/src/protocol-package-lifecycle-types.ts, packages/metadata-protocol/src/protocol-package-uninstall.ts, packages/metadata-protocol/src/protocol.ts, packages/metadata-protocol/src/protocol.uninstall-guards.test.ts + packages/runtime/src/domains/packages.ts (the dependent-rule guard and the DELETE /packages/:id door), packages/metadata-protocol/src/protocol-class-head.ts (ProtocolClassHeadFacet.runUninstallCleanups), packages/cloud-connection/src/local-manifest-source.ts (read/has/remove), packages/runtime/src/http-dispatcher.ts (errorFromThrown), packages/metadata-protocol/package.json (exports), scripts/check-facade-delegation-only.mjs + scripts/facade-delegation-registry.json (the gate the head reds). Prose faces read for ② and for agreement with the code: .changeset/31283-install-local-uninstall-guards-audit.md, content/docs/deployment/marketplace-handbook.mdx.

Face: narrowing (Clause-②: yes (narrowing) on the PR body and the changeset; record-recognisers --paths on the nine files decides no path face and defers to that line). Net diff read as git diff origin/main...pr/31439 at the head (merge-base 383c353, base 18ed2db): nine files, +498/−10; no commit on origin/main after the merge-base touches any of them.

① Derived judgments

  • Which install-local uninstalls are now refused. handleUninstall (plugin l.3340) asks, in this order: capability admission → manifestId present → ledger has (404 otherwise) → walled-catalog refusal (403) → new: askUninstallGuards → registry assertPackageUninstallable (409 RESOURCE_CONFLICT) → ledger.remove (first write). So exactly the uninstalls a registered pre-uninstall guard refuses are newly refused, and only for a ledgered package. Today's one guard is the runtime's runtime.package-dependent-rules (packages/runtime/src/domains/packages.ts l.982–991): a package whose objectExtensions field an active validation rule of another enabled package reads → 409 PACKAGE_DISABLE_RESTRICTED_BY_VALIDATION_RULE with disableDependentRulesMessage(..., { operation: 'uninstall' }). The door answers { success: false, error: { code, message } } under the guard's status, the same body shape as its other refusals. Nothing withdrawn, nothing written: the refusal returns before ledger.read/ledger.remove, captureUninstallDeclared, withdrawFromRunningKernel, the object-copy withdrawal, the cleanups, the re-sync and the audit write. The door test pins exactly this (ledger entry kept, package registered, field served, runUninstallCleanups not called, no audit event).
  • One package_uninstall row per passing uninstall. recordUninstall is called once, after the re-sync, on the one success path; every refusal above returns earlier and the one failure path (ledger.remove throws → 500) returns before it. The row is { action: 'package_uninstall', packageId, version? (read from the ledger entry before its removal), environmentId? (the protocol's), actor? (admission.userId), itemsRemoved: 0, itemsFailed: 0 }. No audit slot → nothing written, never thrown (writePackageUninstallEvent returns on a missing recordPackageLifecycleEvent; a slot that throws is logged). CONTROL case of the door test pins exactly one event.
  • deletePackage is behaviourally identical after the refactor. Read side by side at the head: (a) guard step — was await askUninstallGuards(this.uninstallGuards, request), now await runUninstallGuards.call(this, request), whose whole body is that same call; askUninstallGuards's body is untouched (only its parameter type narrowed to a Pick of DeletePackageRequest's packageId and actor, the two keys it ever read); same position (first thing after the retired-key refusal, before the sys_metadata read); same throw shape (Error with code, status, message). (b) audit step — same guard condition installed.held, or at least one deleted or failed item, same position (after runUninstallCleanups), and the event writePackageUninstallEvent receives has the same keys in the same order with the same inclusion conditions (version when truthy, environmentId when truthy, actor when truthy, keepData only when === true, both counts). The this handed to recordPackageUninstall.call(this, ...) inside the verb is the verb's host, whose environmentId/getServicesRegistry getters are the ones the old inline write read. The existing packages: the in-process uninstall verb (deletePackage) withdraws extension fields a dependent active validation rule reads, with no refusal — only the HTTP door asks #31232 / packages: an uninstall leaves no sys_audit_log row for the package itself (the sys_packages delete is raw SQL, so no actor can reach it) #31186 deletePackage cases stay in the suite unchanged.
  • One verdict, one writer. The door holds no copy: askUninstallGuards (plugin) only calls protocol.runUninstallGuards; recordUninstall only calls protocol.recordPackageUninstall. In the protocol the guard loop exists once (module-private askUninstallGuards) and the slot write once (module-private writePackageUninstallEvent); both public members and the verb reach those two functions. The class members' host literals use the same reader-closure pattern as deletePackage and resolveFlowCanonicalizer; getServicesRegistry is a stored constructor closure, so calling it on the host literal is sound.
  • Capability question. UninstallCleanupRunner gains runUninstallGuards? / recordPackageUninstall? typed as indexed accesses on ObjectStackProtocolImplementation (not restated). A protocol without either member is not asked and the door behaves as before — consistent with the existing runUninstallCleanups probe.

② Semver level

  • Declaration. Clause-②: yes (narrowing) on the PR body and the changeset, line-initial, identical. scripts/pm/clause2-line.mjs reads yes (narrowing) as "a diff that widens one surface and narrows another", which is what the prose "both arms" states. The narrowing arm is right: DELETE /api/v1/marketplace/install-local/:manifestId now answers 409 for an uninstall it answered 200 — a consumer-visible accept-set shrink, so BREAKING: ! in the title, the BREAKING banner and the FROM/NOW/remedy paragraphs are present, and the changeset carries one ADR-0087 marker (not-required (no-migration-prescription)), as item 3 of AGENTS.md requires of a breaking changeset.
  • Widening arm — reachability. ObjectStackProtocolImplementation is exported from packages/metadata-protocol/src/index.ts l.3 and exports["."] maps to dist/index.d.ts, so the two new unmodified (public) methods are published accept set, not shipped bytes. PackageUninstallRecord is exported as a type at index.ts l.138 (and re-exported through protocol.ts), so it is reachable too. Hence yes is required; the claim's no (narrowing) would have been wrong for this diff and the dev's re-declaration is correct.
  • Level under ruling A (changeset levels: minor only when the metadata protocol changes (added / narrowed / semantic), patch otherwise; release notes grouped by what the reader must do (maintainer ruling A, 2026-10-21) #25105). Metadata-protocol: none is right: nothing a metadata author can write changes (no spec key, value or type; the refusal is a runtime verdict at a door). AGENTS.md item 3: "none takes patch; a non-protocol change, widening or breaking, ships patch and keeps its BREAKING banner and ADR-0087 disposition." So @objectstack/cloud-connection: patch and @objectstack/metadata-protocol: patch are the ruled levels; Check Changeset, PR-body guards and Governed Surface Queue Guard are green on the head.
  • Prose faces agree with the code. The changeset's Before/Now/Unchanged match ①. The handbook paragraph (step 9) states the 409 code, "naming the rule" (the sentence names it), "nothing is removed", and "the same refusal DELETE /api/v1/packages/:id answers" (packages.ts l.945 says so). Nit: it says a completed uninstall "is recorded in the audit log" without the changeset's qualifier "a runtime with no audit plugin writes no row".

③ Boundary flags

  • Report 6719019099: open_questions: [], security_findings: none, out_of_scope_findings: []. PR author is objectstack-fleet[bot]; no extra author note owed.
  • Deviation 1 — content/docs/deployment/marketplace-handbook.mdx outside the order's surface: prose face, declared on the Skills-docs: line, agrees with the code (② above). Accepted.
  • Deviation 2 — Clause-② moved from the claim's no (narrowing) to yes (narrowing): correct, see ②.
  • Deviation 3 — one merge of origin/main (645c951); main moved 11 commits after, not re-merged: verified that none of those commits touch the nine files, so the net diff against today's main is the PR's own. feat(driver-sql,cloud-connection): a declared renamedFrom carries the old column's values on upgrade #31402 / feat(cloud-connection,metadata-protocol): an unmanaged copy can take the package's sample data, owned by the installer #31398 (same file, other hunks) still open: unchanged serial note, nothing to escalate here.
  • Gate discrepancy (escalated, see ④ finding 1). The report states "dispatch-gates --commands: 102 families, all exit 0", but the head's check-run Lint · repo & tooling gates is red on check:facade-delegation-only, which names both new class members. scripts/pm/dispatch-gates.mjs does not derive that family for packages/metadata-protocol/src/protocol.ts (no mention of it under scripts/pm/), so the dev's local union could not have measured it — a gap in the derivation, a tooling follow-up for the seat, not a false statement. The report also says the CI families were left to the seat; this is one of them.
  • Route choice ("why not delegate"): verified against the verb at the head. For a package with no stored rows deletePackage would (step 4) hand packageStore.delete(packageId) an id the store never held and throw packagePersistFailureError on a returned/thrown refusal, answer success: false (deleted.length === 0) otherwise, drop object tables by default, and skip the door's ledger/copy/re-sync steps — so public entry points are the right route, and no packages/spec member is involved (both live on the class only, like runUninstallCleanups), so no needs_decision was owed.
  • Check-runs on 645c951 at 2026-11-08T08:08Z (read by gh api): 22 completed success (among them Check Changeset, PR-body guards, Governed Surface Queue Guard, Dogfood Regression Gate, Spec property liveness, Lint · PM dispatch-gates, Lint · ESLint, Lint · docs & code guards, Lint · CI/release/engine gates, both scoped lints, Test UI, Build Docs, Type Check · source gates / debt ledger), 12 skipped (opt-in/UI matrices), 2 failure: Lint · repo & tooling gates and its aggregate Lint & Repo Gates, 12 still in progress (Build Core, Live E2E UI, Test Core 1/8–8/8, Type Check · consumer gates, Type Check · workspace). The red is PR-introduced, not a merge-base signature: the same job is green on the merge-base 383c353 and on the base head 18ed2db (job 128669971074).

④ Design judgment

  1. must-fix — the two new facade members hold bodies the facade gate refuses; the head is red on check:facade-delegation-only. scripts/facade-delegation-registry.json registers ObjectStackProtocolImplementation (card Codemod split: generate the rest of the metadata-protocol protocol.ts split with a deterministic script, verified byte-identical #29985) as delegation-only; judgeDelegationBody accepts const host = {...}; return fn.call(host, ...) but not an object literal built inline as the .call argument (self-test H5). Both new members return runUninstallGuards.call({ get uninstallGuards() {...} }, request) / recordPackageUninstall.call({ get environmentId() {...}, get getServicesRegistry() {...} }, record), and the lane verdict says so: "ObjectStackProtocolImplementation.runUninstallGuards holds a body, not a delegation ... (2 statement(s))", "recordPackageUninstall ... (3 statement(s))". Fix in either accepted shape: (a) bind the host literal to a const host typed as the function's ThisParameterType, holding the literal, and return runUninstallGuards.call(host, request) exactly as deletePackage (l.11014) and resolveFlowCanonicalizer (l.941) do; or (b) move the two bodies into ProtocolClassHeadFacet beside runUninstallCleanups and registerUninstallGuard and delegate return ProtocolClassHeadFacet.x(protocolHost(this), ...). No exceptions row is warranted (the bodies can move). Behaviour is unchanged by either; the gate, not the semantics, is what fails. Must-fix because the diff reds a repo gate that is green on its merge-base, which blocks landing (② all checks green) and breaks the Codemod split: generate the rest of the metadata-protocol protocol.ts split with a deterministic script, verified byte-identical #29985 facade rule the committed registry enforces.
  2. nit — refusal status window at the door. askUninstallGuards answers a thrown refusal only when code is a string and status is in the 400–499 range; anything else propagates. The verb's REST door renders every throw through deps.errorFromThrown(e, 500), which honours the thrown status. A guard refusing with a non-4xx status (the UninstallGuardRefusal type allows any number) would thus be answered as spelled at DELETE /packages/:id but surface as an unhandled throw at install-local. Not reachable today (the only guard answers 409) and the PR declares this exact behaviour, so no contract effect.
  3. nit — the door's row cannot say the uninstall was partial. itemsRemoved: 0, itemsFailed: 0 is always written, including when the registry withdrawal or a cleanup failed (those ride on cleanups). The verb's row has the same blind spot for its own withdrawal and cleanups (it counts only sys_metadata item failures, of which this door has none), so parity holds; accepted deviation, documented beside the call.
  4. Guard runs before the door's first write — confirmed (①). After the guard resolves, the path to ledger.remove and registry.uninstallPackage is synchronous (registry predicate, ledger.read, ledger.remove, captureUninstallDeclared, withdrawFromRunningKernel), so no other request interleaves between verdict and withdrawal; tighter than the verb, which awaits a store read in between. Concurrency with the cleanups is unchanged from [finding] An install-local uninstall (DELETE /api/v1/marketplace/install-local/:id) leaves the package's permission sets in sys_permission_set: the "no ghost grants" uninstall cleanup never runs on that door #21490.
  5. A guard that needs the package still registered — satisfied: the runtime guard resolves objectql.registry at ask time and the door asks before withdrawal, under the same id (manifestId) that assertPackageUninstallable and uninstallPackage use. A package the kernel never registered (the withdrawal-outcome case) yields no dependents and passes, as it would through the verb.
  6. Actor / organization — admission.userId (the authenticated principal) is the actor, as the pair-install undo in this file already passes to deletePackage; the verb's door passes resolvePackageWriteActor. The row carries the protocol's environmentId and no organization on both doors (ADR-0131: an uninstall is environment-wide), so parity holds.
  7. Uninstall that fails after guards pass — ledger.remove throws → 500, nothing moved, no row (the package stays installed; correct). Past that point the uninstall is durable; failures are outcomes on cleanups and one row is still written, as the verb writes after its sys_packages delete regardless of per-item failures. The writer never throws for slot faults.
  8. Cloud vs inline branch — the split is on install (cloud-fetched vs inline manifest); both write a ledger entry and the one uninstall door is keyed on the ledger, so guards and row cover both. The pair-install undo reaches the guards through deletePackage itself.
  9. Guard throws vs refuses — a guard's own throw (no code/status) propagates out of the door exactly as out of the verb; an uninstall a guard could not judge does not proceed and nothing has moved.
  10. Tests — door test is a real ObjectKernel + ObjectQLPlugin + AppPlugin (the real guard registration) with a recorder audit slot; the four protocol cases pin the runner's throw/pass-through and the writer's event/no-slot paths. Dev-reported red-on-main and two ablations are consistent with the hunks but were not re-run here (read-only).

Implemented-by: claude/project-thread-f6p2x0
Reviewed-by: session_01FEkQ5o8PE7ZtVjcor51wBZ

VERDICT: FAIL


Generated by Claude Code

…egate to their facet bodies

Co-Authored-By: Claude <noreply@anthropic.com>
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#5 · os-project-manager · session_01FEkQ5o8PE7ZtVjcor51wBZ

Contract-review-claim: session_01FEkQ5o8PE7ZtVjcor51wBZ · head 332beed · 2026-11-08T08:15Z

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#5 · os-project-manager · session_01FEkQ5o8PE7ZtVjcor51wBZ

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 332beed051a7138b53c5b0afa3c020fd1190c027
Local-runs: none
Scope: packages/metadata-protocol/src/protocol.ts, packages/metadata-protocol/src/protocol-package-uninstall.ts (the delta's two files) and, re-confirmed unchanged since 645c951, packages/cloud-connection/src/marketplace-install-local-plugin.ts, packages/cloud-connection/src/marketplace-install-local-uninstall-guards-audit.test.ts, packages/metadata-protocol/src/index.ts, packages/metadata-protocol/src/protocol-package-lifecycle-types.ts, packages/metadata-protocol/src/protocol.uninstall-guards.test.ts + packages/metadata-protocol/src/protocol-host.ts (ProtocolHost, protocolHost), scripts/check-facade-delegation-only.mjs (judgeDelegationBody, isForwardingReturn, isForwardingCall, isArgument, isReference), scripts/facade-delegation-registry.json. Prose faces unchanged since round 1.

Delta review of the round-1 record on 645c951 (FAIL, one must-fix ④.1). Delta read as git diff 645c9512..pr/31439: one plain commit 332beed, two files, +9/−15, both in packages/metadata-protocol/src. Net diff against origin/main (merge-base 383c353, base 18ed2db) is still the same nine files, now +492/−10; the install-local door, its test, the lifecycle types, the entry file, the protocol test, the changeset and the handbook are byte-identical to round 1.

① Derived judgments

  • Must-fix ④.1 answered in the gate's own terms. Both class members are now one statement: return runUninstallGuards(protocolHost(this), request) and return recordPackageUninstall(protocolHost(this), record). Read against scripts/check-facade-delegation-only.mjs: judgeDelegationBody → one statement → isForwardingReturn → isForwardingCall (callee is a plain identifier, no optional chain; every argument passes isArgument: protocolHost(this) is a nested forwarding call whose one argument this is a reference, and request / record are references) → shape "host delegation (return Fn(host(this), …))". The callees are imported from protocol-package-uninstall.ts, not declared in the facade file, so H13 does not apply; no exceptions row was added to facade-delegation-registry.json. This is the exact shape of the sibling runUninstallCleanups member (return ProtocolClassHeadFacet.runUninstallCleanups(protocolHost(this), request)), green on main.
  • protocolHost(this) hands the bodies the same fields the round-1 host literals did. ProtocolHost (protocol-host.ts) declares environmentId (l.111), getServicesRegistry (l.128) and uninstallGuards (l.262), each typed as the class's indexed access; the static-block factory in protocol.ts fills them as live getters over the instance (get environmentId() l.1238, get getServicesRegistry() l.1255, get uninstallGuards() l.1390). The two functions take a host parameter typed as the Pick of PackageUninstallHost of exactly those names; the ProtocolHost slots are assignable to them (PackageUninstallHost.environmentId is string | undefined, the class field is environmentId?: string; the other two are the same indexed accesses). getServicesRegistry remains a stored constructor closure, so host.getServicesRegistry?.() is sound.
  • deletePackage stays behaviour-identical to round 1's ① (and so to main). The verb now calls runUninstallGuards(this, request) at the same position (first after the retired-key refusal, before the sys_metadata read) and recordPackageUninstall(this, {...}) under the same condition (installed.held, or at least one deleted or failed item) with the same record literal; the function bodies only renamed their receiver from this to host — the askUninstallGuards(host.uninstallGuards, request) call and the event literal (keys, order, inclusion conditions) are unchanged. The verb's this is its own host (deleteMetaItem, engine, environmentId, findEnvironmentRows, getServicesRegistry, packagelessAddress, runUninstallCleanups, shouldDropStorage, uninstallGuards), a superset of each callee's Pick.
  • The install-local door is untouched. marketplace-install-local-plugin.ts is identical to 645c951: askUninstallGuards still asks protocol.runUninstallGuards before the registry refusal and ledger.remove; recordUninstall still calls protocol.recordPackageUninstall once after the re-sync; the refusal shape and the row are those judged in round 1 ①. One verdict, one writer: the guard loop (askUninstallGuards) and the slot write (writePackageUninstallEvent) still exist once each, module-private.

② Semver level

  • Unchanged from round 1: the public members' signatures (runUninstallGuards(request), recordPackageUninstall(record)) and the exported type PackageUninstallRecord are the same; index.ts is byte-identical. The module-level runUninstallGuards / recordPackageUninstall in protocol-package-uninstall.ts changed from a this parameter to a leading host parameter, but that module is not re-exported from the package entry (index.ts has no export * and exports no name from ./protocol-package-uninstall.js; protocol.ts only imports them), so under exports["."] → dist/index.d.ts they are shipped bytes, not accept set. No public surface moved beyond round 1's.
  • Clause-②: yes (narrowing), Metadata-protocol: none, both patch, ! title, BREAKING banner and ADR-0087 marker: unchanged, and still right under ruling A (changeset levels: minor only when the metadata protocol changes (added / narrowed / semantic), patch otherwise; release notes grouped by what the reader must do (maintainer ruling A, 2026-10-21) #25105) for the reasons in the round-1 record.

③ Boundary flags

  • No new dev report comment on the card for this head; the delta is the one commit the coordinator relayed, and its content is exactly the must-fix's route (b) from round 1 (bodies in the facet module, taking the host first, delegated in one forwarding call). Nothing else changed, so round-1 ③ (handbook deviation accepted, Clause-② re-declaration correct, one merge of main with no later main commit touching the nine files, author objectstack-fleet[bot]) stands.
  • The dev's "facade gate and its self-test exit 0 locally" is not taken; the delegation shape was verified against the gate's source above. Round 1's tooling follow-up stands: scripts/pm/dispatch-gates.mjs does not derive check:facade-delegation-only for packages/metadata-protocol/src/protocol.ts, which is why round 1's red surfaced only in CI.
  • Check-runs on 332beed at 2026-11-08T08:16Z (read by gh api): 13 completed success (Auto Label, Check Changeset, Check Documentation Links, Check PR Size, Dogfood Regression Gate, Flag docs affected, Governed Surface Queue Guard, Lint · PM dispatch-gates, Lint · scoped · tooling self-tests, PR-body guards, Spec property liveness, Test UI, filter), 12 skipped (opt-in / UI matrices), 0 failure, 14 in progress — among them Lint · repo & tooling gates (the job round 1 found red; two runs in progress), Build Core, Build Docs, the ESLint / docs-and-code / CI-release-engine lints, both scoped lints, Live E2E UI, Plan Test Core shards (so Test Core has not started) and all four Type Check lanes. The landing precondition "all checks green" is therefore not yet met on this head; the seat reads the concluded runs before queueing. The verdict below is the contract verdict on the tree, which no completed check contradicts.

④ Design judgment

  1. Round-1 must-fix ④.1 — resolved (① above). The fix takes route (b), which is also the shape every other Codemod split: generate the rest of the metadata-protocol protocol.ts split with a deterministic script, verified byte-identical #29985 member of the class uses, so no registry exception and no second idiom.
  2. Round-1 nits 2 (4xx-only refusal window at the door) and 3 (the door's row is always itemsRemoved: 0, itemsFailed: 0; handbook lacks the no-audit-plugin qualifier) — unchanged, still nits, no effect on correctness or the contract; recorded as accepted deviations.
  3. Nothing new to find in the delta: the host indirection is read-only (getters over the instance), adds no await, no state, no new path; the guard still runs before the door's and the verb's first write; the row is still written once on the one success path.

Implemented-by: claude/project-thread-f6p2x0
Reviewed-by: session_01FEkQ5o8PE7ZtVjcor51wBZ

VERDICT: PASS


Generated by Claude Code

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tests tooling Fix lands in tooling, not a product package — enters pm:queue only with Unblocks: or a named surface

Projects

None yet

1 participant