refactor(metadata-protocol,rest,runtime): one declared shape for the protocol.deletePackage seam - #10060
Conversation
…`deletePackage` seam
`protocol.deletePackage` had three independent statements of its own contract
and none of them agreed:
1. the producer's own inline structural type on the method,
2. `PackageRoutesOptions.protocol.deletePackage` in `@objectstack/rest`, which
named neither `organizationId` nor `keepData` and omitted `deleted` from
the response,
3. the dispatcher twin in `@objectstack/runtime`, which typed the seam not at
all and reached it through `(protocol as any)` — while routinely sending
exactly the two keys (2) could not express.
`organizationId` is the key that decides an uninstall's blast radius (the
protocol refuses a call naming neither it nor `allTenants` —
`TENANT_SCOPE_REQUIRED`, 400), so the member the REST seam had no word for is
the one that matters most.
`DeletePackageRequest` / `DeletePackageResponse` are now declared once at the
producer and exported from `@objectstack/metadata-protocol`; both consumers
import them and the `as any` is gone. Types only — identical members, identical
call, no accept-set or behaviour change. No `packages/spec` declaration: minting
protocol surface for a verb with zero external consumers is a spec-seat decision
nobody has asked for.
The member stays OPTIONAL and the runtime's `typeof … === 'function'` capability
probe stays: the `protocol` service slot is deliberately uncontracted, the
spec's `PackageProtocol` does not declare this verb, and registrants that carry
no `deletePackage` are real in-tree.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019yDEhPBC3tcGkW9bkce1HM
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019yDEhPBC3tcGkW9bkce1HM
…9960) Bounded in-place fix, same defect class as the card: `package-routes-query-multiplicity.test.ts` built a protocol double whose uninstall response omitted `deleted`. That compiled only while the option's type omitted it too — with the option now carrying the producer's `DeletePackageResponse`, the double stops type-checking, and `@objectstack/rest`'s TEST_DEBT ledger entry drifted 155 → 156. Fixed at the author's end (`deleted: []`), never by raising the ratchet: the ledger is shrink-only and raising it is maintainer-only. Re-measured back to 155, and `check:type-check-debt` reports "none above its recorded number". The cases in this file count protocol CALLS, not deleted rows, so an empty `deleted` changes nothing they assert. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019yDEhPBC3tcGkW9bkce1HM
📓 Docs Drift CheckThis PR changes 3 package(s): 13 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 4 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 32 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 8c61a388ed0b9056364d6a3e479972b61e197218 && git checkout 8c61a388ed0b9056364d6a3e479972b61e197218
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin eb2bebe532aad5fdc99007923520df6d9d1dd9c9 379ae9788ccf773e91c0eb8a4613f4cde5374eb0 && git checkout -B drift-repro eb2bebe532aad5fdc99007923520df6d9d1dd9c9 && git merge --no-ff 379ae9788ccf773e91c0eb8a4613f4cde5374eb0
node scripts/docs-audit/affected-docs.mjs --json eb2bebe532aad5fdc99007923520df6d9d1dd9c9
|
…o dist (#9960) `check:type-source-resolution` went red on the branch: ✗ @objectstack/rest: NEW dist-resolved type import(s) since this entry was measured: @objectstack/metadata-protocol. Correct, and caused by this card: `src/package-routes.ts` type-imports the declared `deletePackage` shapes, and with no `paths` rule tsc resolved that specifier through the dependency's `exports` map — `dist/index.d.ts`, a build artifact. This package's `typecheck` was therefore a verdict about the last `pnpm build` rather than about the producer's source in the checkout, which is the failure mode whose symptom is a typecheck that PASSES. Fixed where the gate prescribes — the package's own `tsconfig.json`: * ONE `paths` rule, bare key, targeting `../metadata-protocol/src/index.ts`. No `/*` sibling: that package's `exports` map has only `"."`, so there is no subpath to redirect, and a rule pointing at files that are not on disk makes tsc fall back to node resolution silently. * `rootDir` widened from `./src` to `..`, as a consequence rather than a preference: the producer's source is now in the program, `rootDir` is enforced even under `--noEmit` (measured: 20 x TS6059, and deleting the key makes tsc infer one and report the identical 20), and `..` is the directory that genuinely contains every file in the program. Nothing that ships reads it — the package builds with tsup. ⛔ The `KNOWN_DIST_RESOLVED_TYPE_IMPORTS` registry was NOT widened. It is shrink-only and maintainer territory; `@objectstack/rest`'s entry keeps its eight other dist-resolved deps unchanged, and the gate audits that set for equality in both directions. Verified red-to-green, in that order: `node scripts/check-type-source-resolution.mjs` exit 1 with the exact CI message, then exit 0 ("76 packages scanned; 51 registered"). `pnpm --filter @objectstack/rest typecheck` is exit 0 with ZERO errors now that it reads the producer's source — no error was hiding behind the stale artifact, and no ledger was touched to get there. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019yDEhPBC3tcGkW9bkce1HM
PM review — ACCEPT, enqueued
All 25 checks green, each read on its own conclusion rather than the aggregate — Neither of the two reds this PR carried was its ownRecording this because the diff was correct throughout and two rounds were spent proving it:
⭐ The The one red that WAS yours, and how it was fixed
⭐ The control on that fix is the right one: the gate reports 51 registered packages before and after — this package neither joined the registry nor left it. A count that did not move is what proves the fix removed the dep from the measured set rather than adding it to the recorded one. ⭐ And the reasoning in the tsconfig comment earns its length. One rule and not two, because that package's
Verified independentlyHead matches. Per-file divergence 0 on every file (positive control: commits on other paths ≠ 0). Merge-tree conflicts 0. ⛔ Debt ledger not in the diff; The guard enumeration, which answered a better question than was askedThe order asked whether any protocol implementation lacks
Plus: the spec's ⭐ Ablation (A) — pasting Enqueued. Generated by Claude Code |
Fixes #9960 — route B as adjudicated in the triage comment
5344247751: converge the three hand-rolleddeletePackageshapes on one exported request/response type from@objectstack/metadata-protocol(the producer), imported by both consumers, and delete theas anyseam. Nopackages/specdeclaration, and the rollback clause was not reached.The three statements that disagreed
packages/metadata-protocol/src/protocol.ts(producer)packageId,organizationId?,allTenants?,actor?,keepData?packages/rest/src/package-routes.ts(direct-mount option){ packageId; actor?; allTenants? }— named neitherorganizationIdnorkeepData, and its response omitteddeletedpackages/runtime/src/domains/packages.ts(dispatcher twin)(protocol as any)and routinely sent exactly the two keys the REST option's type could not expressAll three line numbers in the card were re-verified against
origin/mainbefore any edit (:15379,:256,:895— all three still accurate at09b880b0c).organizationIdis what makes this load-bearing rather than cosmetic: the protocol refuses a call naming neither it norallTenants(TENANT_SCOPE_REQUIRED, 400), so it is precisely the key whose presence decides an uninstall's blast radius — and it was the key one of the two doors had no word for.The measurement the grading asked for first
Question: does the runtime's
typeof (protocol as any).deletePackage === 'function'guard exist because some protocol implementations genuinely lack the verb?Answer: optional-per-protocol. The guard is load-bearing and stays; the shared type expresses the optionality at both seams. Four anchored probes, not one grep:
*Protocolinterface, multiline so a two-lineimplementsclause cannot hide:grep -rn --include=*.ts -Pzo '(?s)class\s+\w+\s+implements[^{]{0,200}?Protocol[^{]{0,200}?\{' packages examples apps→ exactly one hit,ObjectStackProtocolImplementation implements DataProtocol, MetadataProtocol, PackageProtocol. It carries the verb.packages/runtime/src/domains/packages.tsresolves the service by string name, andprotocolis a deliberately unmapped slot inServiceSlotContracts(packages/spec/src/contracts/core-service-contracts.ts): "protocolandmcphave no written contract, so they stay unmapped and visible rather than being given a shape nothing checks." The string overload ofresolveServicereturnsany— which is why a wrong key was unsayable-but-sendable here.PackageProtocoldoes not declaredeletePackageat all (listPackages/getPackage/installPackage/uninstallPackage/enablePackage/disablePackage, every member optional). A host implementing the declared contract conformantly therefore has nodeletePackage.registerService('protocol', …)(multiline-tolerant probe) hitspackages/metadata-protocol/src/plugin.ts:251plus fiveservice-automationtest harnesses; a per-file count ofdeletePackagein those five returns 0, 0, 0, 0, 0.Anti-vacuity: probe 1 returns a non-zero hit list, and probe 4's positive control is the same probe's own producer hit (
plugin.ts:251) — both pre-exist this diff and survive it. No probe used a symbol this diff introduces.What changed
packages/metadata-protocol/src/protocol.ts—DeletePackageRequest/DeletePackageResponsedeclared once, next toUninstallCleanupOutcome(which the response already anchors); the method's signature is nowdeletePackage(request: DeletePackageRequest): Promise(DeletePackageResponse). Same members, no reordering, no widening.packages/metadata-protocol/src/index.ts— both types exported. This is the only user-visible half of the change: two additive type exports.packages/rest/src/package-routes.ts— the option imports them (type-only) and staysdeletePackage?, plus three compile-time pins in the same idiom [finding] package-routes.ts re-declares getMetaItems/deletePackage as a narrower local structural type instead of reading the spec's #9846 established one member above: exact request equality, exact response equality, and "the member stays optional". Exact equality rather than assignability, because the shape removed here is assignable to the producer's in one direction — an assignability check would have passed on the very divergence that madeorganizationIdunsayable.packages/runtime/src/domains/packages.ts— the resolved service is narrowed to the producer's declared verb, theas anycasts are gone (call site plus three response reads), and the capability probe stays.packages/rest/tsconfig.json— thepathsrule the type import requires. See "Round 2" below.packages/rest/src/package-routes-query-multiplicity.test.ts— the bounded in-place fix, declared here rather than left silent: that file's protocol double returned an uninstall response withoutdeleted, which compiled only while the option's own type omitted it too. With the option now carryingDeletePackageResponse, the double stops type-checking and@objectstack/rest's TEST_DEBT entry drifted 155 → 156. Repaired at the author's end (deleted: []) and re-measured back to 155. The ratchet was not raised — that is maintainer-only, and this card is not authorised for it. The cases in that file count protocol calls, not deleted rows.Types only. Accept set unchanged, no public route or behaviour change, and the emitted JavaScript of both consumers is identical (a removed cast and a narrowed
constannotation both erase).Reverse verification — two ablations, both legs run
Producer dist first, since both consumers type-check through it, never through its sources:
packages/metadata-protocol/dist/index.d.tscarriesinterface DeletePackageRequest(:596),interface DeletePackageResponse(:619) and both names in the terminal export list — the mutation reached the resolution path. Both ablations below then mutate a file inside the package under test, whichtsc --noEmitcompiles directly.A. The runtime seam now rejects a key it used to swallow. Pasted
organisationId: 'ABLATION'(the misspelling theanyused to accept) into the request literal:Restore leg (
git checkout HEAD -- …, from the committed state):tsc --noEmitexit 0, working tree byte-clean.B. The pins fail on the shape this card removed. Put the pre-#9960 hand-rolled option back with the pins untouched:
Two of three pins, which is the correct direction: the request and response pins fire, the optionality pin stays green because the mutated member was still optional. Restore leg: exit 0.
Round 2 — the
check:type-source-resolutionfailure, and why the first round's own reassurance was wrongCI went red on
Lint & Repo Gates:Correct, and caused by this card. Round 1 reported that type correctness rested "on both consumers resolving through its rebuilt
dist/*.d.ts" and treated that as sufficient. It is not: a verdict about a build artifact is not a verdict about the producer's source in the checkout, which is precisely the failure whose symptom is a typecheck that passes. The separate invariant round 1 did establish — that the type never reaches rest's publishedd.ts— remains true and re-verified below, but it is a different gate.Fixed where the gate prescribes, in
packages/rest/tsconfig.json:pathsrule, bare key, targeting../metadata-protocol/src/index.ts. No/*sibling: that package'sexportsmap has only".", so there is no subpath to redirect, and apathstarget that is not on disk makes tsc fall back to node resolution — i.e. back todist— silently. The gate judges rules individually against the specifiers actually imported and does not ask for a block covering imports that do not exist.rootDirwidened from./srcto.., as a consequence rather than a preference. The producer's source is now in the program androotDiris enforced even under--noEmit: measured at 20 xTS6059, and deleting the key does not help either — tsc infers one from this package's own inputs and reports the identical 20...is the directory that genuinely contains every file in the program. Nothing that ships reads it: the package builds with tsup, andtypecheckpasses--noEmit.⛔
KNOWN_DIST_RESOLVED_TYPE_IMPORTSwas not widened — it is shrink-only and maintainer territory.@objectstack/rest's entry keeps its eight other dist-resolved deps unchanged, and the gate audits that set for equality in both directions.Red-to-green, reproduced in that order before anything was pushed:
And the re-run the gate change makes necessary:
pnpm --filter @objectstack/rest typecheckis exit 0 with zero errors now that it reads the producer's source — nothing was hiding behind the stale artifact, and no ledger was touched to get there. One measurement worth recording: on the first attempt that same command reported 135 x TS2307 plus a 46 x TS7006 cascade, which was not a finding — it was an unbuilt closure in a freshly re-created worktree. With the closure built, 20 x TS6059 remained and nothing else, and those were therootDirdiagnostic above.@objectstack/rest's TEST_DEBT re-measure also passes with thepathsblock active (the re-measure project inherits it): still exactly 155, "surplus: none".Checks — all run locally at
ad2a23d3e, the pushed headcheck:type-source-resolutioncheck:test-source-aliaspnpm --filter @objectstack/rest typecheckpnpm --filter @objectstack/runtime typechecktsc --noEmit)pnpm --filter @objectstack/rest build(tsup)dist/index.d.tsstill has zeroDeletePackageRequestreferences and nometadata-protocolimport line — the only mention is a pre-existing prose commentpnpm --filter @objectstack/metadata-protocol testpnpm --filter @objectstack/rest testpnpm --filter @objectstack/runtime testcheck:type-check-coveragecheck:type-check-debt(re-measure, closure built)node scripts/pm/dispatch-gates.mjs(no paths)check:cross-package-test-inputs,check:dispatcher-error-vocabulary,check:durability-log-level,check:filter-alias-parity,check:route-envelope,check:slot-lookup,check:changeset-gate-self-tests,check:objectui-changeset,check-empty-changeset,check-changeset-no-major,check-adr-0087-registration,docs-audit/check-affected-docs— all exit 0check:nul-bytes,check:adr-anchors,check:query-options-erasure,check:published-files,eslint --no-inline-configon the five changed source files@objectstack/metadata-protocolcarries notypecheckscript (it is a DEBT-ledger package); its type correctness here is now carried by@objectstack/rest's typecheck reading its source, plus its own tsup DTS build.Concurrency
origin/mainwas merged atfe94d2e42(7 commits, no conflicts, no deferred regeneration recorded). It has since moved a further 18 commits, none of which touchpackages/rest,packages/runtime,packages/metadata-protocol, or either gate script — left unmerged deliberately, since the merge queue rebuilds the PR onto currentmainanyway. The in-flight check before the first edit found no claim on any of the three declared files; the region was declared by symbol on the issue because #9612 sharesprotocol.ts— this diff does not touch the publish-gate seam.Draft on purpose: the PM lands it through the merge queue after review.
Generated by Claude Code