diff --git a/.changeset/client-packages-get-single-true-type.md b/.changeset/client-packages-get-single-true-type.md new file mode 100644 index 0000000000..c7791d29f7 --- /dev/null +++ b/.changeset/client-packages-get-single-true-type.md @@ -0,0 +1,27 @@ +--- +"@objectstack/client": minor +--- + +fix(client): `packages.get` binds the bare `InstalledPackage` row on both the global and the environment-scoped client, replacing a `{ package }` envelope no surface emits (#12034) + +`client.packages.get(id)` and `ScopedEnvironmentClient.packages.get(id)` now resolve to **`InstalledPackage`** — the row itself — instead of an object wrapping it. + +**Migration — read the row directly, not `.package`:** + +```ts +// before +const { package: pkg } = await client.packages.get('com.acme.crm'); +const pkg2 = (await scoped.packages.get('com.acme.crm')).package; + +// after +const pkg = await client.packages.get('com.acme.crm'); +const pkg2 = await scoped.packages.get('com.acme.crm'); +``` + +FROM `{ package: any }` (global) and `{ package: InstalledPackage }` (scoped) TO `InstalledPackage` on both. + +This is a **narrowing**: a `.package` read compiles today and stops compiling after this change. That is the point of the change rather than a side effect of it — the wrapper was never what the wire sent, so every one of those reads was already `undefined` at runtime, and on the global method the `any` member is what kept the falsehood invisible. Nothing about the request or the wire changes; only the declaration moves to match what the server has been sending. + +Why it can be bound now, when #11925 deliberately left it erased: this route used to be served by two implementations that disagreed — the runtime dispatcher sent the bare row, the `@objectstack/rest` registrar sent `{ package }` — so no declaration was true on both. The registrar's read routes were removed in #16628, leaving the dispatcher's `/packages` domain as the single implementation. It builds the detail body with the same expression it maps over every `list` row, which is why this type now agrees with the `InstalledPackage[]` that `packages.list` has already declared, and with `GetInstalledPackageResponseSchema` in `@objectstack/spec`, which has declared `data: InstalledPackageSchema` all along. + +The environment-scoped method is the sharper half of the change: its member was a real `InstalledPackage`, not `any`, so `.package` reads there looked type-safe while returning `undefined` against every surface that has served that path since #16628. diff --git a/packages/client/src/index.ts b/packages/client/src/index.ts index 25f5c9605d..d3a09695fe 100644 --- a/packages/client/src/index.ts +++ b/packages/client/src/index.ts @@ -2410,34 +2410,49 @@ export class ObjectStackClient { /** * Get a specific installed package by its ID (reverse domain identifier). * - * ⛔ [#11925 / #12034] STILL NOT bound, and the `{ package }` envelope is - * left exactly as it was. #12034 shipped its `install` / `enable` / - * `disable` neighbours (one producer each) and deliberately did NOT ship - * this one, because this route is a REAL fork with no single true type. - * Both bodies below were MEASURED by driving each registrar, not read off - * the source: - * - * dispatcher handlePackages('/', 'GET') - * -> { success: true, data: { id, manifest, enabled, status } } - * rest GET /api/v1/packages/:id - * -> { success: true, data: { package: { …row, source } } } - * - * `unwrapResponse` strips one envelope, so the post-unwrap value is the - * BARE row on the dispatcher and `{ package }` on REST. Binding either - * member here hardens a claim that is false on the other surface. Making - * it bindable means converging the two PRODUCERS — a wire-behaviour change - * to two mounted surfaces, above this card's authority, with a clause-② - * narrowing analysis of its own. The measured convergence cost is recorded - * on #12034 for that ruling. - * - * Its SCOPED twin `ScopedEnvironmentClient.packages.get` IS bound, because - * only the REST registrar serves the scoped mount — one surface, one - * shape. - */ - get: async (id: string) => { + * [#12034] Bound to `InstalledPackage` — the BARE row, no envelope. This + * was the last of the four `packages.*` methods #11925 left unbound, and + * the reason it was unbindable is GONE. + * + * What blocked it was a REAL fork: two mounted surfaces answering + * different envelopes, dispatcher `success(pkg)` against REST + * `sendOk(res, { package: { …row, source } })`. #16628 removed the REST + * twin outright. `registerPackageRoutes` mounts ONE route now — + * `POST /packages/publish` — which is not a claim about registration + * order but about the single `routes` array it hands to + * `mountDirectRoutes`, the same array it reports back as the description + * of what it mounted (`packages/rest/src/package-routes.ts`). So + * `runtime`'s `/packages` domain is the one implementation left, and it + * answers the bare row: + * + * GET /packages/:id + * -> success(withWritableVerdict(qlService, toPackageResponse(pkg))) + * + * That is the SAME projection its `list` neighbour maps over every row + * (`packages/runtime/src/domains/packages.ts` — one expression, two + * doors), and `list` is already declared `InstalledPackage[]` directly + * above. This binding therefore makes two doors of one domain agree + * rather than making a new claim about either. `packages/spec` has + * declared the same thing all along and was never the fork's casualty: + * `GetInstalledPackageResponseSchema` is `data: InstalledPackageSchema`, + * the bare row. + * + * `source` stays undeclared because there is no longer anything that + * emits it on this route; `writable` stays undeclared for the reason + * `list` leaves it undeclared. + * + * ⚠️ Clause-② narrowing. `{ package: any }` is what let + * `(await client.packages.get(id)).package` compile, and on the only + * surface that has served this route since #16628 it was `undefined` at + * runtime — the falsehood was invisible precisely because the member was + * `any`. Callers read the row itself. Pinned in + * `return-type-precision.test.ts`, which is the only place it CAN be + * pinned: a runtime test cannot observe a return-type narrowing at all. + */ + get: async (id: string): Promise => { const route = this.getRoute('packages'); const res = await this.fetch(`${this.baseUrl}${route}/${encodeURIComponent(id)}`); - return this.unwrapResponse<{ package: any }>(res); + return this.unwrapResponse(res); }, /** @@ -7549,25 +7564,47 @@ export class ScopedEnvironmentClient { return this.parent._unwrap<{ packages: InstalledPackage[]; total: number }>(res); }, /** - * [#11925] The asymmetry #8140 recorded, now closed. Its neighbour `list` - * above carried BOTH a return annotation and a type argument, so #8140 - * bound it; this method carried neither and was left erased — same object - * literal, same route family, opposite treatment purely because one lacked - * the annotation. + * [#11925 bound it · #12034 corrected the shape] The BARE row, no + * envelope — the same type its global twin `client.packages.get` now + * carries, and for the same reason. + * + * ⚠️ The rationale this binding shipped with was FALSIFIED, and it is + * worth stating what it claimed because the claim is what made the + * `{ package }` envelope look safe: *"only the REST registrar serves the + * scoped path — so the `{ package }` envelope declared here is the one + * that route actually sends."* #16628 deleted the registrar's + * `GET /packages/:id`. `registerPackageRoutes` is still mounted on BOTH + * `{base}/packages` and `{base}/environments/:environmentId/packages` + * (`direct-mount-composition.ts` iterates that list of bases), so the + * mount the sentence named is still there — it just mounts one route now, + * `POST /packages/publish`, and no read. ⇒ Between #16628 and this + * change the declaration here described a body NO surface emitted + * anywhere, which is strictly worse than the erasure #11925 removed. + * + * What serves this path is the dispatcher, reached through the + * `@objectstack/hono` catch-all the scoped hosts mount: `dispatch()` + * strips the `/environments/:environmentId` prefix — that catch-all is + * the ONLY entry that hands `dispatch()` a still-scoped path + * (`packages/runtime/src/http-dispatcher.ts` says so at the stripping + * site) — and the `/packages` domain answers + * `success(withWritableVerdict(qlService, toPackageResponse(pkg)))`. That + * is the identical projection its `list` neighbour above maps over, which + * is why `list` already declares `InstalledPackage[]` and needed no + * correction here. + * + * ⚠️ Clause-② narrowing, and the sharper of the two: this member was + * `InstalledPackage`, not `any`, so `(await scoped.packages.get(id)) + * .package` compiled with a REAL type behind it and was `undefined` at + * runtime. * - * The scoped mount is unambiguous, which is what makes it bindable while - * the GLOBAL `client.packages.get` is not: `registerPackageRoutes` is - * mounted at both `{base}/packages` and - * `{base}/environments/:environmentId/packages`, and only the REST - * registrar serves the scoped path — so the `{ package }` envelope - * declared here is the one that route actually sends. The handler also - * spreads a `source: 'database' | 'registry'` discriminator onto the row, - * left undeclared for the same reason `list` leaves it undeclared. + * `version` is unchanged and deliberately not touched by this card — it + * is a request-side question, and it is a live one: see the acceptance + * notes on #12034. */ - get: async (id: string, version?: string): Promise<{ package: InstalledPackage }> => { + get: async (id: string, version?: string): Promise => { const qs = version ? `?version=${encodeURIComponent(version)}` : ''; const res = await this.parent._fetch(this.url(`/packages/${encodeURIComponent(id)}${qs}`)); - return this.parent._unwrap<{ package: InstalledPackage }>(res); + return this.parent._unwrap(res); }, }; diff --git a/packages/client/src/return-type-precision.test.ts b/packages/client/src/return-type-precision.test.ts index 59d6deecba..7189fe155d 100644 --- a/packages/client/src/return-type-precision.test.ts +++ b/packages/client/src/return-type-precision.test.ts @@ -318,27 +318,52 @@ export async function returnTypePrecisionPins11925(): Promise { // ── bound 3/3: the asymmetry named on the card, closed ─────────────── // `scoped.packages.list` was bound by #8140 because it happened to carry // an annotation; its neighbour `get` was not, purely because it lacked - // one. Same object literal, same route family. The scoped mount is served - // ONLY by the REST registrar, so unlike the global `client.packages.get` - // there is one surface and one shape. - expectTypeOf(await scoped.packages.get('com.acme.crm')).toEqualTypeOf<{ - package: InstalledPackage; - }>(); + // one. Same object literal, same route family. + // + // ⚠️ [#12034] The SHAPE moved after this pin was written; the fact that + // the method is BOUND — all #11925 claimed here — did not. What stood in + // these lines was "the scoped mount is served ONLY by the REST registrar, + // so unlike the global `client.packages.get` there is one surface and one + // shape", and #16628 deleted that registrar's `GET /packages/:id`: the + // mount named there still exists but serves no read, so the sentence was + // reasoning from a surface that was gone. The dispatcher serves this path + // — via the `@objectstack/hono` catch-all, which strips the + // `/environments/:environmentId` prefix before the `/packages` domain + // sees it — and answers the bare row. ⇒ "one surface and one shape" is + // still true; the shape is the ROW, and #12034 moved the declaration to + // match it. + expectTypeOf(await scoped.packages.get('com.acme.crm')).toEqualTypeOf(); // ── direction 2: a WRONG shape must now be rejected ─────────────────── - // ⚠️ Only ONE of the three below is red before this change, and the split - // is stated here rather than glossed, because a suppression that was + // ⚠️ Only ONE of the two below was red before #11925's change, and the + // split is stated here rather than glossed, because a suppression that was // already used is a regression guard and not evidence the binding was // needed. Ablation (revert `index.ts` to `origin/main`, keep this file) // measured it: the ablated run reports TS2578 at `wrongUpdate` ONLY. // // `packages.update` was bare `any` before, and `any` IS assignable to - // `string`, so its suppression went unused → TS2578. The other two were - // never bare: they declared a real envelope (`{ packages: any[]; total }` - // and `{ package: any }`) whose MEMBER was the erased part, and an - // envelope is not assignable to a bare row or array in either state. Their - // suppressions are used before AND after — regression guards against a - // future "narrowing" that flattens the envelope away. + // `string`, so its suppression went unused → TS2578. `packages.list` was + // never bare: it declared a real envelope (`{ packages: any[]; total }`) + // whose MEMBER was the erased part, and an envelope is not assignable to a + // bare array in either state. Its suppression is used before AND after — a + // regression guard against a future "narrowing" that flattens the envelope + // away. + // + // ⚠️ [#12034] A THIRD line stood here and is gone rather than reworded: + // + // // @ts-expect-error the scoped detail route answers `{ package }`, not the bare row + // const wrongScopedGet: InstalledPackage = await scoped.packages.get('com.acme.crm'); + // + // labelled, correctly for its time, GREEN IN BOTH STATES. Both halves of + // it died with #16628: the claim is false (the route answers the bare row, + // so the assignment is legal and the suppression would be UNUSED — TS2578, + // a red gate), and the label cannot be restored by flipping the claim, + // because a line that is red before #12034 is not green in both states of + // #11925 and this paragraph is the record of #11925's ablation, not a + // description of today's tree. ⇒ The scoped `get`'s direction-2 evidence + // moved, in the same PR, to `returnTypePrecisionPins12034` below, where it + // sits beside the three siblings that fail the identical way and where its + // red-before measurement belongs. // GREEN IN BOTH STATES — regression guard, not red-before evidence. // @ts-expect-error the route answers `{ packages, total }`, not a bare array @@ -349,13 +374,8 @@ export async function returnTypePrecisionPins11925(): Promise { // @ts-expect-error `packages.update` answers the row, not a string const wrongUpdate: string = await client.packages.update('com.acme.crm', { name: 'Acme' }); - // GREEN IN BOTH STATES — regression guard, not red-before evidence. - // @ts-expect-error the scoped detail route answers `{ package }`, not the bare row - const wrongScopedGet: InstalledPackage = await scoped.packages.get('com.acme.crm'); - void wrongList; void wrongUpdate; - void wrongScopedGet; } /** @@ -524,20 +544,33 @@ export async function returnTypePrecisionPins13523(): Promise { * dispatcher in `packages-write-envelope.test.ts`; that the DECLARATION says * so can only be pinned here, for this file's standing reason. * - * ⛔ `packages.get` is deliberately ABSENT from this list. It is the half of - * #12034 that was NOT shipped: its two mounted surfaces answer different - * envelopes (dispatcher `success(pkg)`, REST `sendOk(res, { package })`), so - * no declaration is true on both and binding either member would harden a - * falsehood — the very defect this function closes for its neighbours. Making - * it bindable requires converging the PRODUCERS, which is a wire-behaviour - * ruling of its own. + * ⭐ `packages.get` was deliberately ABSENT from this list and is now IN it — + * both the global method and its scoped twin. It was the half of #12034 that + * did not ship, because its two mounted surfaces answered different envelopes + * (dispatcher `success(pkg)`, REST `sendOk(res, { package })`), so no + * declaration was true on both and binding either member would have hardened a + * falsehood — the very defect this function closes for its neighbours. + * + * The maintainer ruled Option A on 2026-09-09 (converge on the bare row), and + * the convergence then arrived from an unexpected direction: #16628 deleted + * the REST twin instead of changing it, so the producers converged by + * SUBTRACTION and no producer edit was left for this card to make. The fork is + * gone either way, and what it leaves behind is worse than an erasure — a + * declaration describing a body no surface emits anywhere — which is why the + * scoped member (`InstalledPackage`, never `any`) is the sharper of the two. */ export async function returnTypePrecisionPins12034(): Promise { - // ── direction 1: the bare row, on all three ────────────────────────── + // ── direction 1: the bare row, on all five ─────────────────────────── expectTypeOf(await client.packages.install({ id: 'com.acme.crm', version: '1.0.0' })) .toEqualTypeOf(); expectTypeOf(await client.packages.enable('com.acme.crm')).toEqualTypeOf(); expectTypeOf(await client.packages.disable('com.acme.crm')).toEqualTypeOf(); + // The two the ruling added. Same row, same projection: the `/packages` + // domain builds the detail body and every `list` row with ONE expression, + // `withWritableVerdict(qlService, toPackageResponse(pkg))`, so this pin and + // the `InstalledPackage[]` on `list` are two readings of one producer. + expectTypeOf(await client.packages.get('com.acme.crm')).toEqualTypeOf(); + expectTypeOf(await scoped.packages.get('com.acme.crm')).toEqualTypeOf(); // ── direction 2: the read the false declaration invited must now FAIL ─ // ⚠️ RED BEFORE, all three: while the member was `any`, `.package` was a @@ -560,10 +593,27 @@ export async function returnTypePrecisionPins12034(): Promise { // @ts-expect-error no surface sends a `message` alongside the row void (await client.packages.enable('com.acme.crm')).message; - // ── the UNSHIPPED half, pinned as unchanged ────────────────────────── - // Not evidence for this card — a guard that `get` is not "tidied up" into - // one of the two shapes while the fork is still open. - expectTypeOf(await client.packages.get('com.acme.crm')).toEqualTypeOf<{ package: any }>(); + // ── the half that shipped LAST, same direction, same mechanism ─────── + // ⚠️ RED BEFORE, both. Each `.package` read below compiled against the + // declaration it replaces, so each suppression went UNUSED and tsc + // reported TS2578 — and the two get there by different routes, which is + // why both are pinned rather than one standing for the pair: + // + // client.packages.get declared `{ package: any }` — the member existed + // and was `any`, the same erasure-hidden falsehood as the three above. + // scoped.packages.get declared `{ package: InstalledPackage }` — the + // member existed with a REAL type behind it, so nothing about the read + // looked erased at all. Nothing on any surface has emitted that body + // since #16628. + // + // After the binding neither row has a `package` key, both suppressions are + // used, and the read is refused at the call site where a consumer would + // have written it. `(await client.packages.get(id)).package` is exactly the + // read the card was filed about. + // @ts-expect-error the detail route answers the row; there is no `.package` + void (await client.packages.get('com.acme.crm')).package; + // @ts-expect-error the scoped detail route answers the row; there is no `.package` + void (await scoped.packages.get('com.acme.crm')).package; } /**