Skip to content

Commit fb6a2de

Browse files
os-justinclaude
andauthored
fix(client): bind packages.get to the bare InstalledPackage row on both clients (#12034) (#17419)
`client.packages.get` declared `{ package: any }` and its environment-scoped twin declared `{ package: InstalledPackage }`. Neither body is emitted by any surface: the runtime dispatcher's `/packages` domain has been the single implementation of `GET /packages/:id` since #16628 removed the REST registrar's read routes, and it answers the bare row — `success(withWritableVerdict(qlService, toPackageResponse(pkg)))`, the same expression it maps over every `list` row. Maintainer ruling of 2026-09-09 (Option A) converged this route on the bare row. The producer half arrived by subtraction rather than convergence, so this change is SDK-side only. Also corrects the rationale pinned beside the scoped binding, which reasoned from a REST mount that #16628 deleted, and moves the scoped method's direction-2 evidence to the pin function of this card, where it is red before the fix rather than green in both states. Clause-②: yes — a `.package` read compiles today and will not after this. Claude-Session: https://claude.ai/code/session_01DapQyvYrFb1MxSYe7BL2nt Co-authored-by: Claude <noreply@anthropic.com>
1 parent 9224204 commit fb6a2de

3 files changed

Lines changed: 186 additions & 72 deletions

File tree

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
---
2+
"@objectstack/client": minor
3+
---
4+
5+
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)
6+
7+
`client.packages.get(id)` and `ScopedEnvironmentClient.packages.get(id)` now resolve to **`InstalledPackage`** — the row itself — instead of an object wrapping it.
8+
9+
**Migration — read the row directly, not `.package`:**
10+
11+
```ts
12+
// before
13+
const { package: pkg } = await client.packages.get('com.acme.crm');
14+
const pkg2 = (await scoped.packages.get('com.acme.crm')).package;
15+
16+
// after
17+
const pkg = await client.packages.get('com.acme.crm');
18+
const pkg2 = await scoped.packages.get('com.acme.crm');
19+
```
20+
21+
FROM `{ package: any }` (global) and `{ package: InstalledPackage }` (scoped) TO `InstalledPackage` on both.
22+
23+
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.
24+
25+
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.
26+
27+
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.

packages/client/src/index.ts

Lines changed: 78 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -2410,34 +2410,49 @@ export class ObjectStackClient {
24102410
/**
24112411
* Get a specific installed package by its ID (reverse domain identifier).
24122412
*
2413-
* ⛔ [#11925 / #12034] STILL NOT bound, and the `{ package }` envelope is
2414-
* left exactly as it was. #12034 shipped its `install` / `enable` /
2415-
* `disable` neighbours (one producer each) and deliberately did NOT ship
2416-
* this one, because this route is a REAL fork with no single true type.
2417-
* Both bodies below were MEASURED by driving each registrar, not read off
2418-
* the source:
2419-
*
2420-
* dispatcher handlePackages('/<id>', 'GET')
2421-
* -> { success: true, data: { id, manifest, enabled, status } }
2422-
* rest GET /api/v1/packages/:id
2423-
* -> { success: true, data: { package: { …row, source } } }
2424-
*
2425-
* `unwrapResponse` strips one envelope, so the post-unwrap value is the
2426-
* BARE row on the dispatcher and `{ package }` on REST. Binding either
2427-
* member here hardens a claim that is false on the other surface. Making
2428-
* it bindable means converging the two PRODUCERS — a wire-behaviour change
2429-
* to two mounted surfaces, above this card's authority, with a clause-②
2430-
* narrowing analysis of its own. The measured convergence cost is recorded
2431-
* on #12034 for that ruling.
2432-
*
2433-
* Its SCOPED twin `ScopedEnvironmentClient.packages.get` IS bound, because
2434-
* only the REST registrar serves the scoped mount — one surface, one
2435-
* shape.
2436-
*/
2437-
get: async (id: string) => {
2413+
* [#12034] Bound to `InstalledPackage` — the BARE row, no envelope. This
2414+
* was the last of the four `packages.*` methods #11925 left unbound, and
2415+
* the reason it was unbindable is GONE.
2416+
*
2417+
* What blocked it was a REAL fork: two mounted surfaces answering
2418+
* different envelopes, dispatcher `success(pkg)` against REST
2419+
* `sendOk(res, { package: { …row, source } })`. #16628 removed the REST
2420+
* twin outright. `registerPackageRoutes` mounts ONE route now —
2421+
* `POST /packages/publish` — which is not a claim about registration
2422+
* order but about the single `routes` array it hands to
2423+
* `mountDirectRoutes`, the same array it reports back as the description
2424+
* of what it mounted (`packages/rest/src/package-routes.ts`). So
2425+
* `runtime`'s `/packages` domain is the one implementation left, and it
2426+
* answers the bare row:
2427+
*
2428+
* GET /packages/:id
2429+
* -> success(withWritableVerdict(qlService, toPackageResponse(pkg)))
2430+
*
2431+
* That is the SAME projection its `list` neighbour maps over every row
2432+
* (`packages/runtime/src/domains/packages.ts` — one expression, two
2433+
* doors), and `list` is already declared `InstalledPackage[]` directly
2434+
* above. This binding therefore makes two doors of one domain agree
2435+
* rather than making a new claim about either. `packages/spec` has
2436+
* declared the same thing all along and was never the fork's casualty:
2437+
* `GetInstalledPackageResponseSchema` is `data: InstalledPackageSchema`,
2438+
* the bare row.
2439+
*
2440+
* `source` stays undeclared because there is no longer anything that
2441+
* emits it on this route; `writable` stays undeclared for the reason
2442+
* `list` leaves it undeclared.
2443+
*
2444+
* ⚠️ Clause-② narrowing. `{ package: any }` is what let
2445+
* `(await client.packages.get(id)).package` compile, and on the only
2446+
* surface that has served this route since #16628 it was `undefined` at
2447+
* runtime — the falsehood was invisible precisely because the member was
2448+
* `any`. Callers read the row itself. Pinned in
2449+
* `return-type-precision.test.ts`, which is the only place it CAN be
2450+
* pinned: a runtime test cannot observe a return-type narrowing at all.
2451+
*/
2452+
get: async (id: string): Promise<InstalledPackage> => {
24382453
const route = this.getRoute('packages');
24392454
const res = await this.fetch(`${this.baseUrl}${route}/${encodeURIComponent(id)}`);
2440-
return this.unwrapResponse<{ package: any }>(res);
2455+
return this.unwrapResponse<InstalledPackage>(res);
24412456
},
24422457

24432458
/**
@@ -7549,25 +7564,47 @@ export class ScopedEnvironmentClient {
75497564
return this.parent._unwrap<{ packages: InstalledPackage[]; total: number }>(res);
75507565
},
75517566
/**
7552-
* [#11925] The asymmetry #8140 recorded, now closed. Its neighbour `list`
7553-
* above carried BOTH a return annotation and a type argument, so #8140
7554-
* bound it; this method carried neither and was left erased — same object
7555-
* literal, same route family, opposite treatment purely because one lacked
7556-
* the annotation.
7567+
* [#11925 bound it · #12034 corrected the shape] The BARE row, no
7568+
* envelope — the same type its global twin `client.packages.get` now
7569+
* carries, and for the same reason.
7570+
*
7571+
* ⚠️ The rationale this binding shipped with was FALSIFIED, and it is
7572+
* worth stating what it claimed because the claim is what made the
7573+
* `{ package }` envelope look safe: *"only the REST registrar serves the
7574+
* scoped path — so the `{ package }` envelope declared here is the one
7575+
* that route actually sends."* #16628 deleted the registrar's
7576+
* `GET /packages/:id`. `registerPackageRoutes` is still mounted on BOTH
7577+
* `{base}/packages` and `{base}/environments/:environmentId/packages`
7578+
* (`direct-mount-composition.ts` iterates that list of bases), so the
7579+
* mount the sentence named is still there — it just mounts one route now,
7580+
* `POST /packages/publish`, and no read. ⇒ Between #16628 and this
7581+
* change the declaration here described a body NO surface emitted
7582+
* anywhere, which is strictly worse than the erasure #11925 removed.
7583+
*
7584+
* What serves this path is the dispatcher, reached through the
7585+
* `@objectstack/hono` catch-all the scoped hosts mount: `dispatch()`
7586+
* strips the `/environments/:environmentId` prefix — that catch-all is
7587+
* the ONLY entry that hands `dispatch()` a still-scoped path
7588+
* (`packages/runtime/src/http-dispatcher.ts` says so at the stripping
7589+
* site) — and the `/packages` domain answers
7590+
* `success(withWritableVerdict(qlService, toPackageResponse(pkg)))`. That
7591+
* is the identical projection its `list` neighbour above maps over, which
7592+
* is why `list` already declares `InstalledPackage[]` and needed no
7593+
* correction here.
7594+
*
7595+
* ⚠️ Clause-② narrowing, and the sharper of the two: this member was
7596+
* `InstalledPackage`, not `any`, so `(await scoped.packages.get(id))
7597+
* .package` compiled with a REAL type behind it and was `undefined` at
7598+
* runtime.
75577599
*
7558-
* The scoped mount is unambiguous, which is what makes it bindable while
7559-
* the GLOBAL `client.packages.get` is not: `registerPackageRoutes` is
7560-
* mounted at both `{base}/packages` and
7561-
* `{base}/environments/:environmentId/packages`, and only the REST
7562-
* registrar serves the scoped path — so the `{ package }` envelope
7563-
* declared here is the one that route actually sends. The handler also
7564-
* spreads a `source: 'database' | 'registry'` discriminator onto the row,
7565-
* left undeclared for the same reason `list` leaves it undeclared.
7600+
* `version` is unchanged and deliberately not touched by this card — it
7601+
* is a request-side question, and it is a live one: see the acceptance
7602+
* notes on #12034.
75667603
*/
7567-
get: async (id: string, version?: string): Promise<{ package: InstalledPackage }> => {
7604+
get: async (id: string, version?: string): Promise<InstalledPackage> => {
75687605
const qs = version ? `?version=${encodeURIComponent(version)}` : '';
75697606
const res = await this.parent._fetch(this.url(`/packages/${encodeURIComponent(id)}${qs}`));
7570-
return this.parent._unwrap<{ package: InstalledPackage }>(res);
7607+
return this.parent._unwrap<InstalledPackage>(res);
75717608
},
75727609
};
75737610

0 commit comments

Comments
 (0)