Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 61 additions & 0 deletions .changeset/client-packages-write-verbs-bare-row.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
---
"@objectstack/client": minor
---

fix(client): `packages.install` / `enable` / `disable` declare the bare `InstalledPackage` row the only serving surface actually sends (#12034)

**Accept-set narrowing on a published SDK (clause-②), and a false declaration
deleted.** No runtime change: the value each method resolves to is
byte-identical before and after. What moved is the DECLARED type — and unlike
its #11925 siblings this one was not merely erased, it was **wrong**.

FROM → TO, all three methods:

| method | declared before | declares now |
|---|---|---|
| `client.packages.install(manifest, opts?)` | `{ package: any; message?: string }` | `InstalledPackage` |
| `client.packages.enable(id)` | `{ package: any; message?: string }` | `InstalledPackage` |
| `client.packages.disable(id)` | `{ package: any; message?: string }` | `InstalledPackage` |

No surface has ever emitted `{ package, message }` for these three. Each is
served by exactly one implementation — `runtime`'s `/packages` dispatcher domain
— and it answers `success(pkg)`, i.e. `{ success: true, data: <row> }`, which
`unwrapResponse` strips to the bare row. `@objectstack/rest`'s registrar mounts
no twin for any of them (it mounts only `POST /packages/publish`,
`GET /packages`, `GET /packages/:id`, `DELETE /packages/:id`), so there was
never a question of which surface to match.

**Migration — read the row, not `.package`.** Because the member was `any`,
the false read compiled and silently produced `undefined` at runtime:

```ts
// BEFORE — compiled, and `pkg` was `undefined` at runtime
const pkg = (await client.packages.enable(id)).package;
const note = (await client.packages.install(manifest)).message;

// AFTER — the response IS the row
const pkg = await client.packages.enable(id);
pkg.enabled; // the state the verb just changed
pkg.manifest.version;
```

A consumer stops compiling where it reads `.package` or `.message` off these
three results, or assigns the result somewhere `InstalledPackage` does not fit.
That break is the point: those call sites are already broken at runtime today
and the `any` is what hid it. The compiler is the channel that reaches every
affected consumer, and it is strictly more precise than a release note.

**What deliberately did NOT change: `client.packages.get`.** It keeps
`{ package: any }`. That route is a real fork — the dispatcher answers the bare
row while the REST registrar answers `{ package: { ...row, source } }`, both
measured by driving each registrar — so no declaration is true on both surfaces.
Binding either member would harden a falsehood, which is the defect this change
removes for its neighbours. Making `get` bindable requires converging the two
PRODUCERS, a wire-behaviour change to two mounted surfaces; the measured
convergence cost is recorded on #12034 for that ruling.

No ADR-0087 ledger entry: nothing here is a metadata surface. No Zod schema, no
`packages/spec` declaration and no stored representation changed — the phantom
members existed only in a TypeScript return annotation — so `objectstack migrate
meta` has nothing to rewrite. This is the disposition #11925 and #8140 recorded
for the same class of SDK return-type narrowing.
10 changes: 8 additions & 2 deletions packages/client/src/client.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -2509,9 +2509,15 @@ describe('HTTP error shaping — envelope normalisation', () => {

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

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

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

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

const body = JSON.parse(fetchMock.mock.calls[0][1].body);
Expand Down
84 changes: 60 additions & 24 deletions packages/client/src/index.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -1673,14 +1673,25 @@ export class ObjectStackClient {
/**
* Get a specific installed package by its ID (reverse domain identifier).
*
* ⛔ [#11925] NOT bound, and the `{ package }` envelope is left exactly as
* it was — the two mounted surfaces answer this route with DIFFERENT
* envelopes, so no single declaration is true (#12034). `runtime`'s
* `/packages` domain sends `success(pkg)` — the bare row — while `rest`'s
* `GET {base}/packages/:id` sends `sendOk(res, { package: { ...pkg,
* source } })`, and the REST routes "shadow live dispatcher twins" only
* where a `package` service is registered. Binding the member here would
* harden a claim that is already false on one of the two.
* ⛔ [#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('/<id>', '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
Expand All@@ -1695,14 +1706,25 @@ export class ObjectStackClient {
/**
* Install a new package from its manifest.
*
* ⛔ [#11925] NOT bound. This method and its `enable` / `disable`
* neighbours declare `{ package; message? }`, and the ONLY surface that
* serves them — `runtime`'s `/packages` domain; `rest` mounts no twin for
* any of the three — answers `success(pkg)`, the bare row (#12034). The
* declared envelope is not merely erased, it is false, and the `any`
* member is what keeps that invisible. Correcting it is a response-shape
* decision with its own clause-② analysis, not the `any`-binding this card
* carries, so the shape is left untouched here.
* [#12034] Bound to `InstalledPackage` — the BARE row, no envelope.
*
* What this REPLACED was not an erasure but a FALSEHOOD: the declaration
* read `{ package: any; message?: string }`, a shape no surface has ever
* sent, and the `any` member is what kept that invisible —
* `(await client.packages.install(m)).package` compiled and was
* `undefined` at runtime. There is exactly ONE serving surface, so there
* was never a "which surface do we match" question: `rest`'s registrar
* mounts only `POST /packages/publish`, `GET /packages`,
* `GET /packages/:id` and `DELETE /packages/:id` (measured by driving
* `registerPackageRoutes` and enumerating what it mounted — this route is
* `NO_HANDLER` there), leaving `runtime`'s `/packages` domain alone to
* answer, and it answers `success(pkg)`: `{ success: true, data: <row> }`,
* status 201. `message` is gone with the wrapper — no surface sends one.
*
* The wire fact is pinned end-to-end in
* `packages-write-envelope.test.ts` (the real dispatcher answering a real
* client call), and the DECLARATION in `return-type-precision.test.ts` —
* a runtime test cannot observe a return-type narrowing at all.
*
* By default the server rejects a manifest whose `id` is already
* installed with **409 Conflict** (duplicate-id guard) instead of
Expand All@@ -1712,7 +1734,7 @@ export class ObjectStackClient {
install: async (
manifest: any,
options?: { settings?: Record<string, any>; enableOnInstall?: boolean; overwrite?: boolean },
) => {
): Promise<InstalledPackage> => {
const route = this.getRoute('packages');
const res = await this.fetch(`${this.baseUrl}${route}`, {
method: 'POST',
Expand All@@ -1723,7 +1745,7 @@ export class ObjectStackClient {
...(options?.overwrite !== undefined ? { overwrite: options.overwrite } : {}),
}),
});
return this.unwrapResponse<{ package: any; message?: string }>(res);
return this.unwrapResponse<InstalledPackage>(res);
},

/**
Expand All@@ -1739,24 +1761,38 @@ export class ObjectStackClient {

/**
* Enable a disabled package.
*/
enable: async (id: string) => {
*
* [#12034] Bound to `InstalledPackage` — the BARE row, no envelope, for
* the reason spelled out on `install` above: one serving surface
* (`PATCH /packages/:id/enable` is `NO_HANDLER` on the REST registrar),
* and it answers `success(registry.enablePackage(id))`. The
* `{ package: any; message?: string }` this replaces was never emitted by
* anything.
*/
enable: async (id: string): Promise<InstalledPackage> => {
const route = this.getRoute('packages');
const res = await this.fetch(`${this.baseUrl}${route}/${encodeURIComponent(id)}/enable`, {
method: 'PATCH',
});
return this.unwrapResponse<{ package: any; message?: string }>(res);
return this.unwrapResponse<InstalledPackage>(res);
},

/**
* Disable an installed package.
*/
disable: async (id: string) => {
*
* [#12034] Bound to `InstalledPackage` — the BARE row, no envelope, same
* single-producer argument as `install` / `enable` above
* (`PATCH /packages/:id/disable` is `NO_HANDLER` on the REST registrar).
* The dispatcher answers `success(registry.disablePackage(id))`, so the
* row comes back with `enabled: false` — the caller reads the row itself,
* never a `.package` member.
*/
disable: async (id: string): Promise<InstalledPackage> => {
const route = this.getRoute('packages');
const res = await this.fetch(`${this.baseUrl}${route}/${encodeURIComponent(id)}/disable`, {
method: 'PATCH',
});
return this.unwrapResponse<{ package: any; message?: string }>(res);
return this.unwrapResponse<InstalledPackage>(res);
},

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