Skip to content

Commit c766ec3

Browse files
os-elonclaude
andauthored
refactor(metadata-protocol,rest,runtime): one declared shape for the protocol.deletePackage seam (#10060)
* refactor(metadata-protocol,rest,runtime): one declared shape for the `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 * chore(changeset): record the deletePackage seam convergence (#9960) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019yDEhPBC3tcGkW9bkce1HM * test(rest): the `deletePackage` double speaks the declared response (#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 * fix(rest): resolve the metadata-protocol type import to source, not to 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 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 2cd5f81 commit c766ec3

7 files changed

Lines changed: 264 additions & 35 deletions

File tree

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
---
2+
"@objectstack/metadata-protocol": patch
3+
"@objectstack/rest": patch
4+
"@objectstack/runtime": patch
5+
---
6+
7+
refactor(metadata-protocol,rest,runtime): one declared shape for the `protocol.deletePackage` seam, imported by both doors (#9960)
8+
9+
`deletePackage` had **three** independent statements of its own contract, and
10+
they did not agree:
11+
12+
| site | what it said |
13+
|---|---|
14+
| `packages/metadata-protocol/src/protocol.ts` (the producer) | an inline structural type on the method — `packageId`, `organizationId?`, `allTenants?`, `actor?`, `keepData?` |
15+
| `packages/rest/src/package-routes.ts` (direct-mount option) | `{ packageId; actor?; allTenants? }` — named **neither** `organizationId` **nor** `keepData`, and its response omitted `deleted` |
16+
| `packages/runtime/src/domains/packages.ts` (dispatcher twin) | nothing at all — it reached the verb through `(protocol as any)` |
17+
18+
The twin routinely sent exactly the two keys the REST option's type could not
19+
express, and the only reason that was not a compile error was the cast.
20+
21+
`organizationId` is the member that makes this load-bearing rather than
22+
cosmetic: the protocol refuses a call naming neither it nor `allTenants`
23+
(`TENANT_SCOPE_REQUIRED`, 400), so it is precisely the key whose presence
24+
decides an uninstall's blast radius — and it was the key one of the two doors
25+
had no word for.
26+
27+
**What changes:** `DeletePackageRequest` and `DeletePackageResponse` are
28+
declared once at the producer and exported from `@objectstack/metadata-protocol`
29+
(the only user-visible half of this change — two additive type exports); both
30+
consumers import them, and the `as any` seam is gone. `@objectstack/rest` also
31+
gains three compile-time pins over its option, in compiled source rather than a
32+
test file, so a later hand-rolled restatement fails `tsc` instead of drifting
33+
green.
34+
35+
**What does not change:** nothing about what the verb accepts or returns. The
36+
members are identical to the ones the implementation already had, the live call
37+
sites send the same keys, and the emitted JavaScript of both consumers is
38+
unchanged. The member stays optional at both seams and the runtime's
39+
`typeof … === 'function'` capability probe stays — the `protocol` service slot
40+
is deliberately uncontracted, the spec's `PackageProtocol` does not declare this
41+
verb, and registrants carrying no `deletePackage` are real.
42+
43+
No `packages/spec` declaration: minting protocol surface for a verb with zero
44+
external consumers is a spec-seat decision nobody has asked for.

packages/metadata-protocol/src/index.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,11 @@ export type {
104104
SeedTenancyCollision,
105105
} from './migrations/seed-tenancy-backfill.js';
106106
export type { UninstallCleanup, UninstallCleanupOutcome } from './protocol.js';
107+
// [#9960] The ONE declared shape of the `deletePackage` seam, exported so the
108+
// two consumers that speak it (`@objectstack/rest`'s direct-mount package
109+
// registrar and the `@objectstack/runtime` dispatcher twin) type the seam
110+
// against the producer's contract instead of restating it locally.
111+
export type { DeletePackageRequest, DeletePackageResponse } from './protocol.js';
107112
export type { MetadataMutationEvent, MetadataMutationProjector, MutationProjectionOutcome } from './protocol.js';
108113
export type { MetadataAuthoringGate, MetadataAuthoringGateContext } from './protocol.js';
109114

packages/metadata-protocol/src/protocol.ts

Lines changed: 62 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3224,6 +3224,67 @@ export interface UninstallCleanupOutcome {
32243224
error?: string;
32253225
}
32263226

3227+
/**
3228+
* [#9960] The declared REQUEST shape of
3229+
* {@link ObjectStackProtocolImplementation.deletePackage} — the ONE statement
3230+
* of this verb's contract, imported by every seam that speaks it.
3231+
*
3232+
* WHY IT IS NAMED. `deletePackage` has no `packages/spec` declaration (it is
3233+
* absent from `PackageProtocol`), and until this type existed its shape was
3234+
* stated THREE times, differently, by the three modules that share the seam:
3235+
* an inline structural type on the method below; a narrower restatement on
3236+
* `PackageRoutesOptions.protocol.deletePackage` in `@objectstack/rest`, which
3237+
* named neither `organizationId` nor `keepData`; and no type at all in the
3238+
* dispatcher twin (`packages/runtime/src/domains/packages.ts`), which reached
3239+
* the verb through `(protocol as any)` and routinely sent exactly the two keys
3240+
* the REST restatement could not express. Naming the shape once, HERE at the
3241+
* producer, is what makes the other two seams compile-checked against the
3242+
* contract instead of each against its own copy.
3243+
*
3244+
* ⛔ Deliberately NOT declared in `packages/spec`: minting protocol surface for
3245+
* a verb with zero external consumers is declare-and-maintain the platform has
3246+
* not asked for. Should an external consumer ever appear, the spec declaration
3247+
* is its own card for the spec seat, not a rider on a typing convergence.
3248+
*
3249+
* `organizationId` is the load-bearing member. The tenant-scope gate in
3250+
* `deletePackage` refuses a call naming neither it nor `allTenants`
3251+
* (`TENANT_SCOPE_REQUIRED`, 400 — #7780), so it is precisely the key whose
3252+
* presence decides an uninstall's blast radius, and it was the key the REST
3253+
* seam's own type had no word for.
3254+
*/
3255+
export interface DeletePackageRequest {
3256+
packageId: string;
3257+
/**
3258+
* Scope the uninstall to ONE organization's rows (#7705). Omitted together
3259+
* with `allTenants` ⇒ refused, never inferred as "every tenant" (#7780).
3260+
*/
3261+
organizationId?: string;
3262+
/** DECLARE a cross-tenant uninstall. Never deduced from an absent org (#7780). */
3263+
allTenants?: boolean;
3264+
actor?: string;
3265+
/** Remove the metadata but PRESERVE each object's physical table. */
3266+
keepData?: boolean;
3267+
}
3268+
3269+
/**
3270+
* [#9960] The declared RESPONSE shape of
3271+
* {@link ObjectStackProtocolImplementation.deletePackage}, stated once for the
3272+
* same reason as {@link DeletePackageRequest}.
3273+
*
3274+
* `deleted` is part of it. The REST seam's restatement omitted that member
3275+
* entirely, so a caller reading the option's type was told this verb reports
3276+
* only a COUNT of what it removed — while the producer has always returned the
3277+
* per-item list beside it.
3278+
*/
3279+
export interface DeletePackageResponse {
3280+
success: boolean;
3281+
deletedCount: number;
3282+
failedCount: number;
3283+
deleted: Array<{ type: string; name: string; state: string }>;
3284+
failed: Array<{ type: string; name: string; error: string; code?: string }>;
3285+
cleanups: UninstallCleanupOutcome[];
3286+
}
3287+
32273288
/**
32283289
* Post-persistence metadata-mutation notification (#2588). Emitted by
32293290
* `saveMetaItem` / `publishMetaItem` / `deleteMetaItem` AFTER the write
@@ -15376,20 +15437,7 @@ export class ObjectStackProtocolImplementation implements
1537615437
* so each object's table is torn down once. Per-item failures are collected
1537715438
* without aborting the rest.
1537815439
*/
15379-
async deletePackage(request: {
15380-
packageId: string;
15381-
organizationId?: string;
15382-
allTenants?: boolean;
15383-
actor?: string;
15384-
keepData?: boolean;
15385-
}): Promise<{
15386-
success: boolean;
15387-
deletedCount: number;
15388-
failedCount: number;
15389-
deleted: Array<{ type: string; name: string; state: string }>;
15390-
failed: Array<{ type: string; name: string; error: string; code?: string }>;
15391-
cleanups: UninstallCleanupOutcome[];
15392-
}> {
15440+
async deletePackage(request: DeletePackageRequest): Promise<DeletePackageResponse> {
1539315441
// [#7780] A cross-tenant uninstall must be DECLARED, never inferred from
1539415442
// an absent parameter. Maintainer ruling (2026-08-12):
1539515443
// 跨租户卸载必须显式声明,缺省缺参永远不等于「全部租户」.

packages/rest/src/package-routes-query-multiplicity.test.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,10 @@ function harness(options: { protocol?: boolean } = {}) {
6767
protocol: {
6868
deletePackage: async () => {
6969
spy.protocolCalls += 1;
70-
return { success: true, deletedCount: 3, failedCount: 0, failed: [], cleanups: [] };
70+
// [#9960] `deleted` is part of the verb's declared response —
71+
// the option's own type says so now, so a double that omits it no
72+
// longer compiles. Empty here: these cases count CALLS, not rows.
73+
return { success: true, deletedCount: 3, failedCount: 0, deleted: [], failed: [], cleanups: [] };
7174
},
7275
},
7376
}

packages/rest/src/package-routes.ts

Lines changed: 72 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,18 @@ import { readSingleQueryValue, repeatedQueryParamMessage } from './query-multipl
2929
// while this file keeps compiling green. Same discipline as the sibling
3030
// meta-read doors in `rest-server.ts` (#9805 / #9741).
3131
import type { GetMetaItemsRequest, GetMetaItemsResponse } from '@objectstack/spec/api';
32+
// [#9960] The declared uninstall shapes for the `protocol.deletePackage` seam
33+
// below, imported from the PRODUCER for the same reason the meta-read shapes
34+
// above come from the spec: so this module's idea of the request/response is
35+
// the one statement of that contract rather than a local restatement the
36+
// producer can drift away from while this file keeps compiling green. There is
37+
// no spec shape to import for this verb — `deletePackage` is deliberately
38+
// undeclared in `packages/spec` (zero external consumers, #9960) — so the
39+
// producer's own exported type IS the contract. Type-only: no runtime import of
40+
// `@objectstack/metadata-protocol` exists here, and this package still does not
41+
// depend on it at run time (see `query-multiplicity.ts` and `rest-server.ts`,
42+
// which duck-type the same seam for exactly that reason).
43+
import type { DeletePackageRequest, DeletePackageResponse } from '@objectstack/metadata-protocol';
3244

3345
/**
3446
* [#7033 / #7023] The authorization gate for the REST package transport.
@@ -250,16 +262,25 @@ export interface PackageRoutesOptions {
250262
* tolerates — a behaviour question, deliberately not answered here.
251263
*/
252264
getMetaItems?(req: GetMetaItemsRequest): Promise<GetMetaItemsResponse>;
253-
// [#7780] `allTenants` is the explicit carrier for cross-tenant uninstall
254-
// semantics; the protocol refuses a call that names neither it nor an
255-
// `organizationId` (`TENANT_SCOPE_REQUIRED`, 400).
256-
deletePackage?(req: { packageId: string; actor?: string; allTenants?: boolean }): Promise<{
257-
success: boolean;
258-
deletedCount: number;
259-
failedCount: number;
260-
failed: Array<{ type: string; name: string; error: string; code?: string }>;
261-
cleanups: Array<{ name: string; success: boolean; removed: number; error?: string }>;
262-
}>;
265+
/**
266+
* [#7780] `allTenants` is the explicit carrier for cross-tenant uninstall
267+
* semantics; the protocol refuses a call that names neither it nor an
268+
* `organizationId` (`TENANT_SCOPE_REQUIRED`, 400).
269+
*
270+
* [#9960] Request/response are the PRODUCER's declared shapes. The local
271+
* restatement they replace named neither `organizationId` nor `keepData`
272+
* and omitted `deleted` from the response — so the one key that decides an
273+
* uninstall's blast radius had no word for it here, while the dispatcher
274+
* twin sent that key on every org-scoped call. The member stays OPTIONAL
275+
* and the call site below keeps its `typeof … === 'function'`
276+
* feature-detection: the `protocol` service slot is deliberately
277+
* uncontracted (`ServiceSlotContracts`), the spec's own `PackageProtocol`
278+
* does not declare this verb at all, and registrants that carry no
279+
* `deletePackage` are real — so requiring the member here would change what
280+
* this seam tolerates, which is a behaviour question this card does not
281+
* answer.
282+
*/
283+
deletePackage?(req: DeletePackageRequest): Promise<DeletePackageResponse>;
263284
};
264285
/**
265286
* [#7033 / #7023] Resolve the caller's execution context for a package route
@@ -329,6 +350,47 @@ export type _PinGetMetaItemsStaysOptional = Pinned<
329350
undefined extends NonNullable<PackageRoutesOptions['protocol']>['getMetaItems'] ? true : false
330351
>;
331352

353+
/**
354+
* [#9960] The same three pins for the `protocol.deletePackage` seam, and for
355+
* the same reason — with one difference worth stating: `getMetaItems` above is
356+
* pinned to the SPEC's declared shapes, while this verb has no spec
357+
* declaration, so the producer (`@objectstack/metadata-protocol`) is the
358+
* contract these pin against. That is the adjudicated shape of #9960, not an
359+
* oversight: declaring a protocol verb for a surface with zero external
360+
* consumers is a spec-seat decision nobody has asked for.
361+
*
362+
* WHAT THEY CATCH: that this option's request/response are still the producer's
363+
* types rather than a hand-rolled restatement of them. Exact equality, not
364+
* mutual assignability — the shape this card removed (`{ packageId; actor?;
365+
* allTenants? }`) is assignable to `DeletePackageRequest` in one direction, so
366+
* an assignability check would have passed on the very divergence that made
367+
* `organizationId` and `keepData` unsayable here.
368+
*
369+
* They live in compiled source, not a `*.test.ts`, for the reason spelled out
370+
* above the `getMetaItems` pins: this package's `tsconfig.json` excludes its
371+
* test files, so a type-level assertion written there is compiled by nothing.
372+
*/
373+
type DeclaredDeletePackage = NonNullable<NonNullable<PackageRoutesOptions['protocol']>['deletePackage']>;
374+
375+
/** The REQUEST type is exactly the producer's `DeletePackageRequest`. */
376+
export type _PinDeletePackageRequestIsProducerDeclared = Pinned<
377+
ExactlyEqual<Parameters<DeclaredDeletePackage>[0], DeletePackageRequest>
378+
>;
379+
380+
/** The RESPONSE type is exactly the producer's `DeletePackageResponse`. */
381+
export type _PinDeletePackageResponseIsProducerDeclared = Pinned<
382+
ExactlyEqual<Awaited<ReturnType<DeclaredDeletePackage>>, DeletePackageResponse>
383+
>;
384+
385+
/**
386+
* The member stays OPTIONAL — see the option's own note. This pin fails if a
387+
* later edit quietly makes it required, which would turn a protocol registrant
388+
* without the verb from a supported shape into a type error.
389+
*/
390+
export type _PinDeletePackageStaysOptional = Pinned<
391+
undefined extends NonNullable<PackageRoutesOptions['protocol']>['deletePackage'] ? true : false
392+
>;
393+
332394
/**
333395
* Register package management API routes
334396
*

packages/rest/tsconfig.json

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,53 @@
22
"extends": "../../tsconfig.json",
33
"compilerOptions": {
44
"outDir": "./dist",
5-
"rootDir": "./src",
6-
"types": ["node"]
5+
// [#9960] Widened from `./src` as a CONSEQUENCE of the `paths` rule below,
6+
// not as a preference. Redirecting the producer's specifier to its source
7+
// puts `packages/metadata-protocol/src/**` into this program, and `rootDir`
8+
// is enforced over every program file even under `--noEmit`: measured here
9+
// at 20 x `TS6059: File '.../packages/metadata-protocol/src/...' is not
10+
// under 'rootDir'`, and with `rootDir` DELETED tsc infers one from this
11+
// package's own inputs and reports the identical 20. `..` is the directory
12+
// that genuinely contains every file in the program, which is what the
13+
// diagnostic asks for. Emit is unaffected in everything that ships: this
14+
// package builds with tsup (`tsup --config ../../tsup.config.ts`), which
15+
// takes entry and out dir from that config, and `typecheck` passes
16+
// `--noEmit`. The one script that emits through tsc is `dev` (`tsc -w`).
17+
"rootDir": "..",
18+
"types": ["node"],
19+
// [#9960] `@objectstack/metadata-protocol` is type-imported by
20+
// `src/package-routes.ts` (the declared `deletePackage` request/response
21+
// shapes). Without this rule tsc resolves that specifier through the
22+
// dependency's `exports` map — i.e. `dist/index.d.ts`, A BUILD ARTIFACT —
23+
// so this package's `typecheck` would render a verdict about the last
24+
// `pnpm build` rather than about the producer's source in the checkout.
25+
// `check:type-source-resolution` refuses exactly that, and its header
26+
// states why the dangerous case is a typecheck that PASSES.
27+
//
28+
// ONE rule, not two, and that is the predicate rather than a shortcut:
29+
// `@objectstack/metadata-protocol` publishes a single entry point (its
30+
// `exports` map has only `"."`), so there is no namespace subpath for a
31+
// `@objectstack/metadata-protocol/*` rule to redirect. The gate judges
32+
// rules INDIVIDUALLY against the specifiers actually imported and does not
33+
// demand a block for imports that do not exist — a second rule here would
34+
// point at files that are not on disk, which is worse than absent: a
35+
// `paths` target that does not exist makes tsc fall back to node
36+
// resolution, i.e. to `dist`, silently.
37+
//
38+
// ⛔ Never spell the key `@objectstack/metadata-protocol*` (star NOT
39+
// preceded by a slash). That matches the bare name AND every subpath and
40+
// folds them all onto one target; because the index re-exports most of the
41+
// surface it does not crash, it type-checks against the wrong module and
42+
// stays green. See `packages/qa/downstream-contract/tsconfig.json`, the
43+
// repo's other `paths` block, for the measured version of that trap.
44+
//
45+
// No `lib` / `types` mirroring was needed, unlike that block: both packages
46+
// extend the same root config and both already declare `types: ["node"]`,
47+
// so the producer's source compiles under this package's environment with
48+
// zero errors (measured: `tsc --noEmit` exit 0 over the merged program).
49+
"paths": {
50+
"@objectstack/metadata-protocol": ["../metadata-protocol/src/index.ts"]
51+
}
752
},
853
"include": ["src/**/*"],
954
"exclude": ["node_modules", "dist", "**/*.spec.ts", "**/*.test.ts"]

0 commit comments

Comments
 (0)