Skip to content

Commit d9aa041

Browse files
os-litantclaude
andauthored
fix(client): packages.install/enable/disable declare the bare row the only serving surface sends (#13060)
`client.packages.install`, `.enable` and `.disable` declared `{ package: any; message?: string }` — a body no surface has ever emitted. Each is served by exactly one implementation (`runtime`'s `/packages` dispatcher domain; the REST registrar mounts no twin for any of the three) and it answers `success(pkg)`, which `unwrapResponse` strips to the bare `InstalledPackage` row. Because the member was `any`, `(await client.packages.enable(id)).package` compiled and was `undefined` at runtime; the `any` is what kept the falsehood invisible. All three now declare `InstalledPackage`. `message` goes with the wrapper — no surface sends one. `client.packages.get` is deliberately untouched: it is a real fork (the dispatcher answers the bare row, the REST registrar answers `{ package: { ...row, source } }`), so no declaration is true on both surfaces. Converging the two producers is a wire-behaviour ruling above this change; the measured cost is recorded on the issue. Two pins, because neither half can observe the other: the WIRE fact is driven end-to-end against a real `SchemaRegistry` + real `HttpDispatcher` + real client in `packages-write-envelope.test.ts`, and the DECLARATION is pinned type-level in `return-type-precision.test.ts`. Claude-Session: https://claude.ai/code/session_01UjujZN219uFzBhSYfMykCd Co-authored-by: Claude <noreply@anthropic.com>
1 parent 269167f commit d9aa041

5 files changed

Lines changed: 420 additions & 26 deletions

File tree

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
---
2+
"@objectstack/client": minor
3+
---
4+
5+
fix(client): `packages.install` / `enable` / `disable` declare the bare `InstalledPackage` row the only serving surface actually sends (#12034)
6+
7+
**Accept-set narrowing on a published SDK (clause-②), and a false declaration
8+
deleted.** No runtime change: the value each method resolves to is
9+
byte-identical before and after. What moved is the DECLARED type — and unlike
10+
its #11925 siblings this one was not merely erased, it was **wrong**.
11+
12+
FROM → TO, all three methods:
13+
14+
| method | declared before | declares now |
15+
|---|---|---|
16+
| `client.packages.install(manifest, opts?)` | `{ package: any; message?: string }` | `InstalledPackage` |
17+
| `client.packages.enable(id)` | `{ package: any; message?: string }` | `InstalledPackage` |
18+
| `client.packages.disable(id)` | `{ package: any; message?: string }` | `InstalledPackage` |
19+
20+
No surface has ever emitted `{ package, message }` for these three. Each is
21+
served by exactly one implementation — `runtime`'s `/packages` dispatcher domain
22+
— and it answers `success(pkg)`, i.e. `{ success: true, data: <row> }`, which
23+
`unwrapResponse` strips to the bare row. `@objectstack/rest`'s registrar mounts
24+
no twin for any of them (it mounts only `POST /packages/publish`,
25+
`GET /packages`, `GET /packages/:id`, `DELETE /packages/:id`), so there was
26+
never a question of which surface to match.
27+
28+
**Migration — read the row, not `.package`.** Because the member was `any`,
29+
the false read compiled and silently produced `undefined` at runtime:
30+
31+
```ts
32+
// BEFORE — compiled, and `pkg` was `undefined` at runtime
33+
const pkg = (await client.packages.enable(id)).package;
34+
const note = (await client.packages.install(manifest)).message;
35+
36+
// AFTER — the response IS the row
37+
const pkg = await client.packages.enable(id);
38+
pkg.enabled; // the state the verb just changed
39+
pkg.manifest.version;
40+
```
41+
42+
A consumer stops compiling where it reads `.package` or `.message` off these
43+
three results, or assigns the result somewhere `InstalledPackage` does not fit.
44+
That break is the point: those call sites are already broken at runtime today
45+
and the `any` is what hid it. The compiler is the channel that reaches every
46+
affected consumer, and it is strictly more precise than a release note.
47+
48+
**What deliberately did NOT change: `client.packages.get`.** It keeps
49+
`{ package: any }`. That route is a real fork — the dispatcher answers the bare
50+
row while the REST registrar answers `{ package: { ...row, source } }`, both
51+
measured by driving each registrar — so no declaration is true on both surfaces.
52+
Binding either member would harden a falsehood, which is the defect this change
53+
removes for its neighbours. Making `get` bindable requires converging the two
54+
PRODUCERS, a wire-behaviour change to two mounted surfaces; the measured
55+
convergence cost is recorded on #12034 for that ruling.
56+
57+
No ADR-0087 ledger entry: nothing here is a metadata surface. No Zod schema, no
58+
`packages/spec` declaration and no stored representation changed — the phantom
59+
members existed only in a TypeScript return annotation — so `objectstack migrate
60+
meta` has nothing to rewrite. This is the disposition #11925 and #8140 recorded
61+
for the same class of SDK return-type narrowing.

packages/client/src/client.test.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2509,9 +2509,15 @@ describe('HTTP error shaping — envelope normalisation', () => {
25092509

25102510
describe('packages.install', () => {
25112511
const MANIFEST = { id: 'com.acme.crm', name: 'Acme CRM', version: '1.0.0', type: 'app' };
2512+
// [#12034] The body the ONLY serving surface actually sends: `success(pkg)`,
2513+
// i.e. the bare `InstalledPackage` row under `data`. These two cases assert
2514+
// the REQUEST and never read the response, so the fixture is inert either
2515+
// way — but it used to spell `data: { package: … }`, a body nothing emits,
2516+
// and a decoy fixture is how the next sweep concludes the envelope is real.
2517+
const INSTALLED_ROW = { manifest: MANIFEST, status: 'installed', enabled: true };
25122518

25132519
it('POSTs the manifest and omits `overwrite` unless requested', async () => {
2514-
const { client, fetchMock } = createMockClient({ success: true, data: { package: { manifest: MANIFEST } } });
2520+
const { client, fetchMock } = createMockClient({ success: true, data: INSTALLED_ROW });
25152521
await client.packages.install(MANIFEST, { enableOnInstall: true });
25162522

25172523
expect(fetchMock).toHaveBeenCalledWith('http://localhost:3000/api/v1/packages', expect.any(Object));
@@ -2524,7 +2530,7 @@ describe('packages.install', () => {
25242530
});
25252531

25262532
it('passes `overwrite: true` through for intentional upgrade / re-install', async () => {
2527-
const { client, fetchMock } = createMockClient({ success: true, data: { package: { manifest: MANIFEST } } });
2533+
const { client, fetchMock } = createMockClient({ success: true, data: INSTALLED_ROW });
25282534
await client.packages.install(MANIFEST, { overwrite: true });
25292535

25302536
const body = JSON.parse(fetchMock.mock.calls[0][1].body);

packages/client/src/index.ts

Lines changed: 60 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1673,14 +1673,25 @@ export class ObjectStackClient {
16731673
/**
16741674
* Get a specific installed package by its ID (reverse domain identifier).
16751675
*
1676-
* ⛔ [#11925] NOT bound, and the `{ package }` envelope is left exactly as
1677-
* it was — the two mounted surfaces answer this route with DIFFERENT
1678-
* envelopes, so no single declaration is true (#12034). `runtime`'s
1679-
* `/packages` domain sends `success(pkg)` — the bare row — while `rest`'s
1680-
* `GET {base}/packages/:id` sends `sendOk(res, { package: { ...pkg,
1681-
* source } })`, and the REST routes "shadow live dispatcher twins" only
1682-
* where a `package` service is registered. Binding the member here would
1683-
* harden a claim that is already false on one of the two.
1676+
* ⛔ [#11925 / #12034] STILL NOT bound, and the `{ package }` envelope is
1677+
* left exactly as it was. #12034 shipped its `install` / `enable` /
1678+
* `disable` neighbours (one producer each) and deliberately did NOT ship
1679+
* this one, because this route is a REAL fork with no single true type.
1680+
* Both bodies below were MEASURED by driving each registrar, not read off
1681+
* the source:
1682+
*
1683+
* dispatcher handlePackages('/<id>', 'GET')
1684+
* -> { success: true, data: { id, manifest, enabled, status } }
1685+
* rest GET /api/v1/packages/:id
1686+
* -> { success: true, data: { package: { …row, source } } }
1687+
*
1688+
* `unwrapResponse` strips one envelope, so the post-unwrap value is the
1689+
* BARE row on the dispatcher and `{ package }` on REST. Binding either
1690+
* member here hardens a claim that is false on the other surface. Making
1691+
* it bindable means converging the two PRODUCERS — a wire-behaviour change
1692+
* to two mounted surfaces, above this card's authority, with a clause-②
1693+
* narrowing analysis of its own. The measured convergence cost is recorded
1694+
* on #12034 for that ruling.
16841695
*
16851696
* Its SCOPED twin `ScopedEnvironmentClient.packages.get` IS bound, because
16861697
* only the REST registrar serves the scoped mount — one surface, one
@@ -1695,14 +1706,25 @@ export class ObjectStackClient {
16951706
/**
16961707
* Install a new package from its manifest.
16971708
*
1698-
* ⛔ [#11925] NOT bound. This method and its `enable` / `disable`
1699-
* neighbours declare `{ package; message? }`, and the ONLY surface that
1700-
* serves them — `runtime`'s `/packages` domain; `rest` mounts no twin for
1701-
* any of the three — answers `success(pkg)`, the bare row (#12034). The
1702-
* declared envelope is not merely erased, it is false, and the `any`
1703-
* member is what keeps that invisible. Correcting it is a response-shape
1704-
* decision with its own clause-② analysis, not the `any`-binding this card
1705-
* carries, so the shape is left untouched here.
1709+
* [#12034] Bound to `InstalledPackage` — the BARE row, no envelope.
1710+
*
1711+
* What this REPLACED was not an erasure but a FALSEHOOD: the declaration
1712+
* read `{ package: any; message?: string }`, a shape no surface has ever
1713+
* sent, and the `any` member is what kept that invisible —
1714+
* `(await client.packages.install(m)).package` compiled and was
1715+
* `undefined` at runtime. There is exactly ONE serving surface, so there
1716+
* was never a "which surface do we match" question: `rest`'s registrar
1717+
* mounts only `POST /packages/publish`, `GET /packages`,
1718+
* `GET /packages/:id` and `DELETE /packages/:id` (measured by driving
1719+
* `registerPackageRoutes` and enumerating what it mounted — this route is
1720+
* `NO_HANDLER` there), leaving `runtime`'s `/packages` domain alone to
1721+
* answer, and it answers `success(pkg)`: `{ success: true, data: <row> }`,
1722+
* status 201. `message` is gone with the wrapper — no surface sends one.
1723+
*
1724+
* The wire fact is pinned end-to-end in
1725+
* `packages-write-envelope.test.ts` (the real dispatcher answering a real
1726+
* client call), and the DECLARATION in `return-type-precision.test.ts` —
1727+
* a runtime test cannot observe a return-type narrowing at all.
17061728
*
17071729
* By default the server rejects a manifest whose `id` is already
17081730
* installed with **409 Conflict** (duplicate-id guard) instead of
@@ -1712,7 +1734,7 @@ export class ObjectStackClient {
17121734
install: async (
17131735
manifest: any,
17141736
options?: { settings?: Record<string, any>; enableOnInstall?: boolean; overwrite?: boolean },
1715-
) => {
1737+
): Promise<InstalledPackage> => {
17161738
const route = this.getRoute('packages');
17171739
const res = await this.fetch(`${this.baseUrl}${route}`, {
17181740
method: 'POST',
@@ -1723,7 +1745,7 @@ export class ObjectStackClient {
17231745
...(options?.overwrite !== undefined ? { overwrite: options.overwrite } : {}),
17241746
}),
17251747
});
1726-
return this.unwrapResponse<{ package: any; message?: string }>(res);
1748+
return this.unwrapResponse<InstalledPackage>(res);
17271749
},
17281750

17291751
/**
@@ -1739,24 +1761,38 @@ export class ObjectStackClient {
17391761

17401762
/**
17411763
* Enable a disabled package.
1742-
*/
1743-
enable: async (id: string) => {
1764+
*
1765+
* [#12034] Bound to `InstalledPackage` — the BARE row, no envelope, for
1766+
* the reason spelled out on `install` above: one serving surface
1767+
* (`PATCH /packages/:id/enable` is `NO_HANDLER` on the REST registrar),
1768+
* and it answers `success(registry.enablePackage(id))`. The
1769+
* `{ package: any; message?: string }` this replaces was never emitted by
1770+
* anything.
1771+
*/
1772+
enable: async (id: string): Promise<InstalledPackage> => {
17441773
const route = this.getRoute('packages');
17451774
const res = await this.fetch(`${this.baseUrl}${route}/${encodeURIComponent(id)}/enable`, {
17461775
method: 'PATCH',
17471776
});
1748-
return this.unwrapResponse<{ package: any; message?: string }>(res);
1777+
return this.unwrapResponse<InstalledPackage>(res);
17491778
},
17501779

17511780
/**
17521781
* Disable an installed package.
1753-
*/
1754-
disable: async (id: string) => {
1782+
*
1783+
* [#12034] Bound to `InstalledPackage` — the BARE row, no envelope, same
1784+
* single-producer argument as `install` / `enable` above
1785+
* (`PATCH /packages/:id/disable` is `NO_HANDLER` on the REST registrar).
1786+
* The dispatcher answers `success(registry.disablePackage(id))`, so the
1787+
* row comes back with `enabled: false` — the caller reads the row itself,
1788+
* never a `.package` member.
1789+
*/
1790+
disable: async (id: string): Promise<InstalledPackage> => {
17551791
const route = this.getRoute('packages');
17561792
const res = await this.fetch(`${this.baseUrl}${route}/${encodeURIComponent(id)}/disable`, {
17571793
method: 'PATCH',
17581794
});
1759-
return this.unwrapResponse<{ package: any; message?: string }>(res);
1795+
return this.unwrapResponse<InstalledPackage>(res);
17601796
},
17611797

17621798
/* [#3563 PR-4] Lifecycle beyond install/enable — these eleven routes

0 commit comments

Comments
 (0)