diff --git a/.changeset/client-envelope-convergence.md b/.changeset/client-envelope-convergence.md new file mode 100644 index 0000000000..54757066e8 --- /dev/null +++ b/.changeset/client-envelope-convergence.md @@ -0,0 +1,64 @@ +--- +"@objectstack/client": minor +--- + +feat(client)!: `analytics.query` / `analytics.meta` / `analytics.explain` and `automation.trigger` resolve to the payload — the dispatcher envelope is unwrapped, as on every other SDK method (#13079) + + + +**BREAKING** — a runtime change to what four published SDK methods resolve to. It ships as `minor` under the lockstep launch-window convention (`scripts/check-changeset-no-major.mjs`): the version number is not the migration signal here, this entry is. + +Maintainer ruling on #13079 (2026-08-31, verbatim): 「裁决:A,cloud 未测量照裁」 — 「四方法(`analytics.query` / `analytics.meta` / `analytics.explain` / `automation.trigger`)收敛 `unwrapResponse`,SDK 一套读法。」 + +## What changed + +`ObjectStackClient` had two response readers. `unwrapResponse` strips the runtime dispatcher's `{ success, data }` envelope and hands back `data` — every other dispatcher-served method uses it, and every return type bound since #8140 is that post-unwrap payload. These four ended `return res.json()`, which strips nothing, so their callers alone had to read `.data`; the sharpest case was `automation.trigger` and `automation.execute` answering two shapes for one handler. All four now end `return this.unwrapResponse(res)`, and their return declarations are the payload types, derived from the route's declared `data` member where the spec already transcribes it. + +## Migration + +| method | resolved to (before) | resolves to (now) | rewrite | +|:--|:--|:--|:--| +| `client.analytics.query(q)` | `{ success, data: AnalyticsResult, meta? }` | `AnalyticsResult` | `r.data.rows` → `r.rows` | +| `client.analytics.meta(cube?)` | `AnalyticsMetadataResponse` — `{ success, data: CubeMeta[], meta? }` | `AnalyticsMetadataResponse['data']` — the bare cube list | `r.data[0].name` → `r[0].name` | +| `client.analytics.explain(q)` | `AnalyticsSqlResponse` — `{ success, data: { sql, params }, meta? }` | `AnalyticsSqlResponse['data']` — `{ sql, params }` | `r.data.sql` → `r.sql` | +| `client.automation.trigger(name, payload)` | `{ success, data: AutomationResult, meta? }` | `AutomationResult` — the same value `client.automation.execute` resolves to | `r.data.status` → `r.status`, `r.data.runId` → `r.runId` | + +Before / after, per method: + +```ts +const r1 = await client.analytics.query({ cube: 'crm_account', measures: ['account_count'] }); +r1.data.rows; // before +r1.rows; // now + +const r2 = await client.analytics.meta(); +r2.data[0].name; // before +r2[0].name; // now + +const r3 = await client.analytics.explain({ cube: 'crm_account', measures: ['account_count'] }); +r3.data.sql; // before +r3.sql; // now + +const r4 = await client.automation.trigger('approve_account', {}); +r4.data.status; // before ('paused' | 'completed' | 'failed') +r4.status; // now — exactly what `client.automation.execute` already answered +``` + +For the three analytics methods every old read is a compile error under the new declarations (`Property 'data' does not exist on type …`, TS2339), so a TypeScript consumer finds each site at build time. `client.automation.trigger` is the exception: `r.data.…` is a compile error there too, but `r.success` and `r.error` compile before AND after, because `AutomationResult` itself declares `success: boolean` and `error?: string` (`packages/spec/src/contracts/automation-service.ts`). Their MEANING moves: before, `r.success` was the envelope's flag — always `true` on a resolved call — and `r.error` was never set on a 2xx; now they are the run's own — `success: false` / `error: string` on a refusal the door does not classify as 400/409/422 and answers 200. A consumer branching on `r.success` or `r.error` off `trigger` must re-read that branch by hand; the compiler will not point at it. A JavaScript consumer reads `undefined` from `.data` and has to search for the four spellings. + +### The failure path — read this before touching a `catch` + +Nothing changes there, and it is stated per door because a convergence on `unwrapResponse` could be misread as "errors now throw": + +- **Non-2xx answers threw before and throw now.** `ObjectStackClient.fetch` rejects on every non-2xx status BEFORE either reader runs, carrying the ADR-0112 envelope on the error (`err.code`, `err.httpStatus`, `err.message`, `err.details`). A failed `trigger` run has been a thrown `400 FLOW_FAILED` since #9378 (`409 FLOW_DISABLED` / `422 FLOW_NO_START_NODE` since #9415); a query the analytics service refuses is a thrown 4xx. Your `catch` blocks are unchanged. +- **`unwrapResponse` never throws.** A 2xx body with a boolean `success` and a `data` key resolves to `data`. A 2xx body with no `data` key resolves unchanged (pass-through) — and no dispatcher door behind these four routes sends a 2xx without `data`, so at the ENVELOPE level a resolved `{ success: false, error }` is not a value you will receive from them. ⚠️ The PAYLOAD level differs on one door: `client.automation.trigger` can resolve to an `AutomationResult` whose own `success` is `false` (with `error` set) — a run the door does not classify as 400 `FLOW_FAILED` / 409 `FLOW_DISABLED` / 422 `FLOW_NO_START_NODE` is answered 200 through `deps.success(result)` (`respondToFlowTrigger` in `packages/runtime/src/domains/automation.ts`; the classification table is `classifyFlowRefusal` in `packages/runtime/src/flow-dispatch-status.ts`), exactly as `client.automation.execute` already does for the same handler. Before this change that run reached you as `{ success: true, data: { success: false, error } }`; now it reaches you as the inner object. +- **What you lose.** The envelope's `success` flag — always `true` on a resolved call — is no longer on the resolved value. Its `meta` slot is gone too, but these four doors never populated it: each answers `deps.success(result)` with no meta argument and JSON serialisation drops the `undefined`, so there was never a `meta.requestId` to read here. Neither key was ever on any other SDK method's value. + +### Not changed + +- `client.analytics.queryDataset(...)` — served by `@objectstack/rest` with no envelope at all; it resolved to the bare `AnalyticsResult` before and still does (ruling item 1: protected, not converted). +- The wire. Every route answers exactly the body it answered before; a raw-HTTP caller is unaffected. +- `client.automation.execute`, `client.automation.resume` and every other method that already used `unwrapResponse`. + +### Populations measured, and the one ruled NOT MEASURED + +`packages/client/src/envelope-caller-census.test.ts` (PR #13647) measured the callers: in this repo, zero production call sites and 13 loud test pins — this change's own diff — and in objectui one production site whose row-extraction chain accepts both spellings today (objectui#7028 tightens it to the post-unwrap spelling after this lands). `objectstack-ai/cloud` was not measured (ruling item 4); the census file carries `CLOUD_CENSUS_COMMAND`, and a `.data` read on any of these four there is a runtime break after this change. diff --git a/packages/client/src/analytics-automation-json-erasure.test.ts b/packages/client/src/analytics-automation-json-erasure.test.ts index db7dbce9bb..e966780b41 100644 --- a/packages/client/src/analytics-automation-json-erasure.test.ts +++ b/packages/client/src/analytics-automation-json-erasure.test.ts @@ -1,9 +1,9 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. /** - * [#12104 — the in-repo half] What the five `return res.json()` methods of the - * `analytics.*` / `automation.*` families actually resolve to, measured against - * their REAL producers. + * [#12104 — the in-repo half; #13079 — the convergence] What the five + * `analytics.*` / `automation.trigger` methods actually resolve to, measured + * against their REAL producers. * * ## The erasure these five carried * @@ -35,20 +35,26 @@ * ## The load-bearing fact these five share, and the one that splits them * * `unwrapResponse` strips the `{ success, data }` envelope; `res.json()` does - * NOT. So a `res.json()` method resolves to the WHOLE body, and the shape of - * that body is decided by which surface serves the route: + * NOT. The shape of the body is decided by which surface serves the route, + * and the reader has to match it: * * - `query` / `meta` / `explain` and `automation.trigger` are DISPATCHER * routes, and every dispatcher domain answers through `deps.success(v)` — - * `{ success: true, data: v }`. Their true type is the envelope, not `v`. + * `{ success: true, data: v }`. Since #13079 (maintainer ruling + * 2026-08-31, option A) all four end `unwrapResponse`, so they resolve to + * `v`; until then they ended `res.json()` and resolved to the envelope, + * which #12104 had stated in their declarations. * - `queryDataset` is a REST route (`@objectstack/rest` mounts it; the - * dispatcher mounts no twin) and it answers `res.json(result)` — BARE. Its - * true type is `v` itself. + * dispatcher mounts no twin) and it answers `res.json(result)` — BARE. It + * keeps `res.json()`, which there IS the payload read; ⛔ PROTECTED by the + * ruling from being "converged" into the others' shape. * - * Binding the payload where the envelope is served (or the reverse) would - * typecheck against `any` and ship a false declaration, which is the census's - * highest-risk band (`return-type-precision.test.ts`, shape class 2). Hence one - * driven case per method rather than a family-wide assumption. + * So all five now resolve to `v`, by two different readers, and each + * (reader, surface) pair is driven here against the real producer: converting + * the bare route to `unwrapResponse`, or sliding a dispatcher route back to + * `res.json()`, would leave the declarations false with no type error — this + * file's cases are what go red. Hence one driven case per method rather than + * a family-wide assumption. * * ## Two spec response schemas WERE narrower than their producer — measured here * @@ -72,8 +78,10 @@ * Removing an annotation from any of the five leaves THIS file green — the wire * value does not change — and turns `return-type-precision.test.ts` RED under * `tsc`, plus `check:exported-any-returns` red on the un-deleted ledger entry. - * That asymmetry is the whole reason both files exist; the ablation is recorded - * on the PR against the halves a declaration change can move. + * Reverting one of the four #13079 conversions (`unwrapResponse` back to + * `res.json()`) turns THIS file red on that method's case — the resolved value + * regains the envelope — and `return-type-precision.test.ts` red on its + * reversed pin. Both asymmetries are why the files exist as a pair. */ import { describe, it, expect, vi } from 'vitest'; @@ -299,72 +307,73 @@ function producerBackedClient() { // ───────────────────────────────────────────────────────────────────────────── -describe('#12104 — the four DISPATCHER-served methods resolve to the envelope, not the payload', () => { - it('analytics.query answers `{ success, data: AnalyticsResult }`', async () => { +describe('#13079 — the four DISPATCHER-served methods resolve to the PAYLOAD, measured against the real producers', () => { + it('analytics.query answers the AnalyticsResult itself — the producer\'s own return, no envelope', async () => { const { client, analytics } = producerBackedClient(); - const body = await client.analytics.query({ + const result = await client.analytics.query({ cube: 'crm_account', measures: ['account_count'], dimensions: ['industry'], }); - // ① The envelope is the value — NOT the payload. This is the whole - // difference between `res.json()` and `unwrapResponse`. - expect(Object.keys(body).sort()).toEqual(['data', 'meta', 'success']); - expect(body.success).toBe(true); - // ② …and `data` is verbatim what the producer's own contract method + // ① The payload is the value — NOT the envelope. Before #13079 the + // keys here were `['data', 'meta', 'success']`; `unwrapResponse` + // strips exactly that layer and nothing else. + expect('success' in result).toBe(false); + expect('data' in result).toBe(false); + // ② …and the value is verbatim what the producer's own contract method // returned, asserted against a second call to the service itself // rather than against a literal written here. - expect(body.data).toEqual(await analytics.query({ + expect(result).toEqual(await analytics.query({ cube: 'crm_account', measures: ['account_count'], dimensions: ['industry'], })); - expect(body.data.rows).toEqual(ROWS); + expect(result.rows).toEqual(ROWS); }); - it('analytics.meta answers `{ success, data: CubeMeta[] }`', async () => { + it('analytics.meta answers the bare CubeMeta[] — no envelope, no `cubes` wrapper', async () => { const { client, analytics } = producerBackedClient(); - const body = await client.analytics.meta(); + const cubes = await client.analytics.meta(); - expect(body.success).toBe(true); - expect(body.data).toEqual(await analytics.getMeta()); - // A BARE array under `data` — there is no `cubes` wrapper (#6442). - expect(Array.isArray(body.data)).toBe(true); - expect(body.data[0]?.name).toBe('crm_account'); - expect(body.data[0]?.measures.map((m) => m.name)).toContain('crm_account.account_count'); + expect(cubes).toEqual(await analytics.getMeta()); + // A BARE array — there is no `cubes` wrapper (#6442) and, since + // #13079, no `{ success, data }` around it either. + expect(Array.isArray(cubes)).toBe(true); + expect(cubes[0]?.name).toBe('crm_account'); + expect(cubes[0]?.measures.map((m) => m.name)).toContain('crm_account.account_count'); }); - it('analytics.explain answers `{ success, data: { sql, params } }`', async () => { + it('analytics.explain answers `{ sql, params }`', async () => { const { client } = producerBackedClient(); - const body = await client.analytics.explain({ + const dryRun = await client.analytics.explain({ cube: 'crm_account', measures: ['account_count'], dimensions: ['industry'], }); - expect(body.success).toBe(true); - expect(Object.keys(body.data).sort()).toEqual(['params', 'sql']); - expect(body.data.sql).toMatch(/SELECT/i); - expect(Array.isArray(body.data.params)).toBe(true); + expect(Object.keys(dryRun).sort()).toEqual(['params', 'sql']); + expect(dryRun.sql).toMatch(/SELECT/i); + expect(Array.isArray(dryRun.params)).toBe(true); }); - it('automation.trigger answers `{ success, data: AutomationResult }` — the whole result', async () => { + it('automation.trigger answers the AutomationResult — the whole run, the same value `execute` answers', async () => { const { client } = producerBackedClient(); - const body = await client.automation.trigger('approve_account', {}); + const run = await client.automation.trigger('approve_account', {}); - expect(body.success).toBe(true); // The keys `TriggerFlowResponseSchema.data` did NOT declare before - // #13078, served by the real engine: this measurement is why the - // annotation binds `AutomationResult` (and, since #13078, why the - // schema had to move to parity with it). - expect(body.data.status).toBe('paused'); - expect(typeof body.data.runId).toBe('string'); - expect(body.data.screen?.title).toBe('Approve the account'); + // #13078, served by the real engine and — since #13079 — read at the + // top level, exactly where `automation.execute` has always put them. + expect(run.status).toBe('paused'); + expect(typeof run.runId).toBe('string'); + expect(run.screen?.title).toBe('Approve the account'); + // `AutomationResult` carries its OWN `success`; the envelope's is gone. + expect(run.success).toBe(true); + expect('data' in run).toBe(false); }); }); @@ -406,12 +415,13 @@ describe('#12104 — the REST-served method resolves to the BARE payload', () => }); }); -describe('#12104 — the premise the four envelope annotations rest on', () => { - it('the dispatcher wraps exactly once, and `res.json()` strips nothing', async () => { - // Runtime-observable and deliberately so: every envelope annotation this - // card adds describes the PRE-unwrap value, so if a domain stopped - // wrapping (or the SDK started unwrapping here) the declarations would - // become false without a single type error. +describe('#13079 — the premise the four payload annotations rest on', () => { + it('the dispatcher wraps exactly once, and `unwrapResponse` strips exactly once', async () => { + // Runtime-observable and deliberately so: every payload annotation + // describes the POST-unwrap value, so if a domain stopped wrapping (the + // SDK would then hand back `data`'s `data`, or the pass-through) or a + // method slid back to `res.json()` (the envelope would return) the + // declarations would become false without a single type error. const { client, dispatcher } = producerBackedClient(); const raw = await dispatcher.handleAnalytics('/meta', 'GET', undefined, CONTEXT(), {}); @@ -420,7 +430,8 @@ describe('#12104 — the premise the four envelope annotations rest on', () => { expect(produced.success).toBe(true); expect(Array.isArray(produced.data)).toBe(true); - // The SDK hands the caller the producer's body itself — envelope included. - expect(await client.analytics.meta()).toEqual(produced); + // The SDK hands the caller the producer's `data` — one envelope + // stripped, nothing else touched. + expect(await client.analytics.meta()).toEqual(produced.data); }); }); diff --git a/packages/client/src/client.test.ts b/packages/client/src/client.test.ts index 05154d1d2d..1b6d8b3144 100644 --- a/packages/client/src/client.test.ts +++ b/packages/client/src/client.test.ts @@ -1546,9 +1546,10 @@ describe('ObjectStackClient.automation', () => { // fetch layer throws on non-2xx before any unwrapping, so both surfaces // REJECT. No SDK code changed; the contract did, and these are its pins. // - // Both spellings are pinned, not one: `trigger()` reads `res.json()` while - // `execute()` reads `unwrapResponse()`, so a regression in either unwrap - // path would be invisible from the other's test. + // Both spellings are pinned, not one: `trigger()` and `execute()` are two + // URLs into one handler, and since #13079 both read `unwrapResponse()`; + // a regression on either door's rejection path would still be invisible + // from the other's test (the paths diverge before the shared reader). const failedRunBody = { success: false, error: { diff --git a/packages/client/src/envelope-caller-census.test.ts b/packages/client/src/envelope-caller-census.test.ts index cfa98d0754..52705e87a9 100644 --- a/packages/client/src/envelope-caller-census.test.ts +++ b/packages/client/src/envelope-caller-census.test.ts @@ -1,36 +1,46 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. /** - * [#13079] HOW MANY EXISTING CALL SITES WOULD BREAK if the four - * envelope-returning SDK methods started unwrapping? The per-site census. + * [#13079] THE CALL-SITE CENSUS of the four SDK methods the 2026-08-31 ruling + * converged on `unwrapResponse` — regenerated to describe the POST-convergence + * world, and kept as the ratchet that stops the envelope read from returning. * - * ⛔ This is a MEASUREMENT file. It converts nothing, repairs nothing and - * proposes nothing. The convergence #13079 contemplates is a RUNTIME BREAKING - * CHANGE to four published methods and a decision reserved for the maintainer; - * this file exists only to put a number under it. + * ⛔ This is a MEASUREMENT file. It converts nothing and repairs nothing. PR + * #13647 wrote it to put a number under the decision the card carried ("how + * many callers break if the four start unwrapping?" — 13 loud in-repo pins, + * zero production sites); the ruling took option A on that number; and this + * revision re-measures the same population after the conversion, so a green + * run now means: zero call sites in this repo still read the envelope off + * these four, and the four really end in `unwrapResponse`. * * ## The four, and the one that must not join them * * `unwrapResponse` strips the dispatcher's `{ success, data }` envelope; - * `res.json()` strips nothing. Four methods take the second path against a - * DISPATCHER-served route, so their callers receive the envelope: - * `analytics.query`, `analytics.meta`, `analytics.explain`, - * `automation.trigger`. + * `res.json()` strips nothing. Four DISPATCHER-served methods used to take the + * second path and hand their callers the envelope: `analytics.query`, + * `analytics.meta`, `analytics.explain`, `automation.trigger`. Since #13079 + * all four end `unwrapResponse` — section 4 reads that off the SDK source. * * ⚠️ `analytics.queryDataset` also ends `res.json()` and is NOT one of them. * Its route is mounted by `@objectstack/rest` and ends `res.json(result)` with - * no envelope to strip, so it is correct as it stands. It is deliberately - * absent from `METHODS` below and section 4 asserts that absence, because - * folding it in is the one thing a mechanical sweep gets wrong. + * no envelope to strip, so `res.json()` there IS the payload read. It is + * deliberately absent from `METHODS` below, section 4 asserts that absence + * AND that its source still reads `res.json()`, because folding it in is the + * one thing a mechanical sweep gets wrong (ruling item 1: protected). * - * ## Why `tsc` and a type search cannot answer this + * ## Why `tsc` and a type search could not answer the original question * * All five were erased to `Promise< any >` until #12104 (no annotation, and * `lib.dom` declares `Response.json(): Promise< any >`). Under `any` BOTH * spellings compiled: `(await client.analytics.query(q)).rows` and * `.data.rows` were equally legal, and nothing in the type system - * distinguished them. So the population cannot be recovered from types — it - * has to come from reading call sites, which is what this file enumerates. + * distinguished them. So the population could not be recovered from types — + * it had to come from reading call sites, which is what this file enumerates. + * After #13079 the type system DOES refuse the envelope read (the reversed + * `@ts-expect-error` pins in `return-type-precision.test.ts`) — for + * TypeScript callers. A JavaScript caller, or a call reached through a + * runtime alias, is still only visible to a source scan, which is why the + * scan stays. * * ## ⭐ The method, and the false negative it was built to catch * @@ -39,8 +49,8 @@ * assumed, both in section 2: * * 1. **Comments dominate the raw signal.** A plain `git grep` for these four - * spellings returns 30 hits in this repo, and 10 of them are docblocks and - * inline comments naming the method — not call sites. `scanCallSites` + * spellings returns dozens of hits in this repo, and many are docblocks + * and inline comments naming the method — not call sites. `scanCallSites` * blanks comments before matching. * 2. **A line-based matcher UNDER-REPORTS.** `git grep` is line-oriented, so * it cannot see a call split across lines — and this repo contains exactly @@ -49,10 +59,10 @@ * const err: any = await client.automation * .trigger('my_flow', { amount: 0 }) * - * Two real `automation.trigger` call sites are spelled that way in - * `client.test.ts` and a line-oriented sweep misses BOTH. A short list - * reads as compliance, which is why section 2 pins the gap as a number - * instead of trusting the matcher. + * Five SDK call sites are spelled that way today (two in `client.test.ts`, + * three in `envelope-convergence.test.ts`) and a line-oriented sweep misses + * ALL of them. A short list reads as compliance, which is why section 2 + * pins the gap as a number instead of trusting the matcher. * * ## The receiver split — a false POSITIVE, measured the same way * @@ -64,25 +74,34 @@ * * ## The verdicts * - * - `ENVELOPE_DEPENDENT` — the site reads `.data`, reads the envelope's - * `success`, compares the whole envelope, or pins the envelope TYPE. These - * break if the methods start unwrapping. Every one is loud: an assertion - * failure or a compile error, never a silent wrong value. + * - `ENVELOPE_DEPENDENT` — the site reads `.data` off the resolved value, + * reads the envelope's `success`, compares the whole envelope, or pins the + * envelope TYPE. After #13079 such a site is a BUG (a runtime `undefined` + * or a type error), and section 3 ratchets the count at ZERO. + * - `PAYLOAD_DEPENDENT` — the site reads the post-unwrap payload (`.rows`, + * `[0].name`, `.sql`, `.status` …), asserts the envelope keys are absent, + * or pins the payload TYPE. These are the convergence's own pins: every one + * goes red if a method slides back to `res.json()`. * - `RESULT_INSENSITIVE` — the site discards the resolved value (it asserts - * the URL the SDK dialled) or takes the REJECTION path and reads - * `err.code` / `err.httpStatus`. Unwrapping cannot reach it. + * the URL the SDK dialled), takes the REJECTION path and reads + * `err.code` / `err.httpStatus`, or reads a body BOTH readers hand back + * unchanged (a 2xx with no `data` key). The reader cannot reach it. * - `NOT_SDK` — a call on the producer service, not on the client. * * ## ⚠️ What this file does NOT measure, stated so a green run cannot be read * ## as a clean bill * - * - **`objectstack-ai/cloud` — NOT MEASURED.** It is not reachable from the - * session class that produced this census. `CLOUD_CENSUS_COMMAND` below is - * the ready-to-run sweep for whoever has access. An unreachable repo is not - * a clean one. + * - **`objectstack-ai/cloud` — NOT MEASURED, and RULED so.** It is not + * reachable from the session class that produced this census, and the + * maintainer ruled the convergence with that cell recorded as unmeasured + * (2026-08-31, 「A,cloud 未测量照裁」). ⚠️ Since #13079 a `.data` read on any of + * these four methods in cloud is a RUNTIME BREAK, not a style difference: + * the value is the payload, so `.data` is `undefined`. `CLOUD_CENSUS_COMMAND` + * below is the ready-to-run sweep; any seat with access should run it + * inside the migration wave and record a non-zero result on #13079. * - **Published npm consumers outside these repos — UNMEASURABLE.** The SDK - * ships to consumers no repo sweep can see. This census bounds the - * first-party population only. + * ships to consumers no repo sweep can see; the changeset carries their + * migration. This census bounds the first-party population only. * - **`objectui` is a RECORDED constant, not a live scan.** A test in this * repo cannot read that checkout. `OBJECTUI_CENSUS` carries the revision it * was measured at and the command that reproduces it. @@ -118,8 +137,8 @@ const HERE = dirname(fileURLToPath(import.meta.url)); const REPO_ROOT = resolve(HERE, '../../..'); /** - * The four DISPATCHER-served `res.json()` methods. ⛔ `queryDataset` is - * deliberately NOT here — see the header and section 4. + * The four DISPATCHER-served methods the ruling converged. ⛔ `queryDataset` + * is deliberately NOT here — see the header and section 4. */ const METHODS: ReadonlyArray = [ ['analytics', 'query'], @@ -193,7 +212,7 @@ const CENSUS = scanCallSites(REPO_ROOT); // The classification ledger — reviewed by hand, cross-checked mechanically // --------------------------------------------------------------------------- -type Verdict = 'ENVELOPE_DEPENDENT' | 'RESULT_INSENSITIVE' | 'NOT_SDK'; +type Verdict = 'ENVELOPE_DEPENDENT' | 'PAYLOAD_DEPENDENT' | 'RESULT_INSENSITIVE' | 'NOT_SDK'; interface LedgerRow { file: string; @@ -209,55 +228,88 @@ interface LedgerRow { * (file, method, receiver) rather than by line so an unrelated edit that * shifts a line does not turn this red — while a NEW call site anywhere in * the workspace still does, which is the point: the number cannot drift in - * silence. + * silence. A key may carry MORE than one row when its sites split across + * verdicts (a payload pin beside a rejection-path pin in one file); section 3 + * sums a key's rows before comparing. */ const LEDGER: readonly LedgerRow[] = [ - // ── the #12104 wire measurement: reads `.data` and `success` throughout ── + // ── the mocked-transport convergence pins (`envelope-convergence.test.ts`) ── + { + file: 'packages/client/src/envelope-convergence.test.ts', + method: 'analytics.query', receiver: 'sdk', count: 1, verdict: 'PAYLOAD_DEPENDENT', + why: 'asserts the value equals the `data` member and that success/data/meta are absent', + }, + { + file: 'packages/client/src/envelope-convergence.test.ts', + method: 'analytics.query', receiver: 'sdk', count: 1, verdict: 'RESULT_INSENSITIVE', + why: 'takes the rejection path of a 400 envelope and reads err.code / err.httpStatus', + }, + { + file: 'packages/client/src/envelope-convergence.test.ts', + method: 'analytics.meta', receiver: 'sdk', count: 1, verdict: 'PAYLOAD_DEPENDENT', + why: 'asserts a bare array equal to `data`, reads [0].name and [0].measures', + }, + { + file: 'packages/client/src/envelope-convergence.test.ts', + method: 'analytics.explain', receiver: 'sdk', count: 1, verdict: 'PAYLOAD_DEPENDENT', + why: 'asserts Object.keys(value) === [params, sql] and reads .sql / .params', + }, + { + file: 'packages/client/src/envelope-convergence.test.ts', + method: 'automation.trigger', receiver: 'sdk', count: 2, verdict: 'PAYLOAD_DEPENDENT', + why: 'the run itself (status / runId / screen, no `data`), and the exactly-once strip on a payload carrying its own `success`', + }, + { + file: 'packages/client/src/envelope-convergence.test.ts', + method: 'automation.trigger', receiver: 'sdk', count: 3, verdict: 'RESULT_INSENSITIVE', + why: 'two rejection-path reads (400 FLOW_FAILED, 409 FLOW_DISABLED) and the 2xx no-`data` pass-through, which both readers hand back unchanged', + }, + // ── the producer-backed wire measurement: reads the payload throughout ── { file: 'packages/client/src/analytics-automation-json-erasure.test.ts', - method: 'analytics.query', receiver: 'sdk', count: 1, verdict: 'ENVELOPE_DEPENDENT', - why: 'asserts Object.keys(body) === [data, meta, success], body.success and body.data.rows', + method: 'analytics.query', receiver: 'sdk', count: 1, verdict: 'PAYLOAD_DEPENDENT', + why: 'asserts success/data absent and the value equals what the producer returned, reads .rows', }, { file: 'packages/client/src/analytics-automation-json-erasure.test.ts', method: 'analytics.query', receiver: 'service', count: 1, verdict: 'NOT_SDK', - why: 'the real AnalyticsService, called to assert body.data equals what the producer returned', + why: 'the real AnalyticsService, called to assert the SDK value equals what the producer returned', }, { file: 'packages/client/src/analytics-automation-json-erasure.test.ts', - method: 'analytics.meta', receiver: 'sdk', count: 2, verdict: 'ENVELOPE_DEPENDENT', - why: 'body.success / body.data, and a whole-envelope toEqual against the dispatcher body', + method: 'analytics.meta', receiver: 'sdk', count: 2, verdict: 'PAYLOAD_DEPENDENT', + why: 'a bare array equal to getMeta(), and a whole-value toEqual against the dispatcher body\'s `data`', }, { file: 'packages/client/src/analytics-automation-json-erasure.test.ts', - method: 'analytics.explain', receiver: 'sdk', count: 1, verdict: 'ENVELOPE_DEPENDENT', - why: 'body.success and Object.keys(body.data) === [params, sql]', + method: 'analytics.explain', receiver: 'sdk', count: 1, verdict: 'PAYLOAD_DEPENDENT', + why: 'Object.keys(value) === [params, sql]', }, { file: 'packages/client/src/analytics-automation-json-erasure.test.ts', - method: 'automation.trigger', receiver: 'sdk', count: 1, verdict: 'ENVELOPE_DEPENDENT', - why: 'body.success and body.data.status / runId / screen', + method: 'automation.trigger', receiver: 'sdk', count: 1, verdict: 'PAYLOAD_DEPENDENT', + why: 'reads .status / .runId / .screen at the top level, asserts no `data`, equals execute()\'s value', }, - // ── the #12104 type pins: bind the envelope, and refuse the payload read ── + // ── the type pins: bind the payload, and refuse the envelope read ───────── { file: 'packages/client/src/return-type-precision.test.ts', - method: 'analytics.query', receiver: 'sdk', count: 2, verdict: 'ENVELOPE_DEPENDENT', - why: 'expectTypeOf === BaseResponse & { data: AnalyticsResult }, plus a @ts-expect-error on .rows', + method: 'analytics.query', receiver: 'sdk', count: 2, verdict: 'PAYLOAD_DEPENDENT', + why: 'expectTypeOf === AnalyticsResult, plus a @ts-expect-error on .data', }, { file: 'packages/client/src/return-type-precision.test.ts', - method: 'analytics.meta', receiver: 'sdk', count: 2, verdict: 'ENVELOPE_DEPENDENT', - why: 'expectTypeOf === AnalyticsMetadataResponse, plus a @ts-expect-error on .length', + method: 'analytics.meta', receiver: 'sdk', count: 2, verdict: 'PAYLOAD_DEPENDENT', + why: 'expectTypeOf === AnalyticsMetadataResponse[data], plus a @ts-expect-error on .data', }, { file: 'packages/client/src/return-type-precision.test.ts', - method: 'analytics.explain', receiver: 'sdk', count: 2, verdict: 'ENVELOPE_DEPENDENT', - why: 'expectTypeOf === AnalyticsSqlResponse, plus a @ts-expect-error on .sql', + method: 'analytics.explain', receiver: 'sdk', count: 2, verdict: 'PAYLOAD_DEPENDENT', + why: 'expectTypeOf === AnalyticsSqlResponse[data], plus a @ts-expect-error on .data', }, { file: 'packages/client/src/return-type-precision.test.ts', - method: 'automation.trigger', receiver: 'sdk', count: 2, verdict: 'ENVELOPE_DEPENDENT', - why: 'expectTypeOf === BaseResponse & { data: AutomationResult }, plus a @ts-expect-error on .runId', + method: 'automation.trigger', receiver: 'sdk', count: 2, verdict: 'PAYLOAD_DEPENDENT', + why: 'expectTypeOf === AutomationResult, plus a @ts-expect-error on .data', }, // ── URL and rejection pins: the resolved value is never read ───────────── { @@ -283,20 +335,23 @@ const LEDGER: readonly LedgerRow[] = [ * checkout. Reproduce with `OBJECTUI_CENSUS_COMMAND`. * * ⭐ The single production call site in either reachable repo, and it SURVIVES - * convergence untouched. `aggregate()` feeds the resolved value through a - * tolerant chain that already accepts both spellings: + * the convergence untouched. `aggregate()` feeds the resolved value through a + * tolerant chain that accepts both spellings: * * const rawRows = Array.isArray(data) ? data - * : data?.rows && Array.isArray(data.rows) ? data.rows // post-unwrap + * : data?.rows && Array.isArray(data.rows) ? data.rows // post-#13079: this branch * : data?.data && Array.isArray(data.data) ? data.data - * : data?.data?.rows && Array.isArray(data.data.rows) ? data.data.rows // envelope, today + * : data?.data?.rows && Array.isArray(data.data.rows) ? data.data.rows // pre-#13079: this branch * : data?.results && Array.isArray(data.results) ? data.results * : []; * - * Today branch 4 matches; after convergence branch 2 matches. ⚠️ It survives + * Before #13079 branch 4 matched; after it branch 2 matches. ⚠️ It survives * by DEFENSIVE CODING, not by a designed migration path — the adapter was * written tolerant because the producer was ambiguous, which is the shape a - * contract-first fix is supposed to remove rather than rely on. + * contract-first fix removes rather than relies on. Ruling item 3: objectui#7028 + * tightens the chain to the single post-unwrap spelling, time-gated behind + * this convergence landing and objectui taking the SDK version — the chain is + * load-bearing until then, so it is deliberately NOT part of this change. */ const OBJECTUI_CENSUS = { revision: 'b84dc1854922c266850d6e573daf3ad59cbd0623', @@ -305,24 +360,63 @@ const OBJECTUI_CENSUS = { productionCallSites: 1, wouldBreak: 0, site: 'packages/data-objectstack/src/index.ts:4846 (analytics.query, via this.client)', + tighteningOwner: 'objectui#7028', } as const; const OBJECTUI_CENSUS_COMMAND = "git -C grep -nE 'analytics\\s*\\.\\s*(query|meta|explain)\\s*\\(|automation\\s*\\.\\s*trigger\\s*\\(' origin/main -- '*.ts' '*.tsx' '*.js'"; -/** ⛔ `objectstack-ai/cloud` is NOT MEASURED — see the header. */ +/** + * ⛔ `objectstack-ai/cloud` is NOT MEASURED — see the header. Post-#13079 + * semantics for whoever runs it: every hit is a call that now resolves to the + * PAYLOAD, so a `.data` read on the resolved value at that site is a runtime + * break (`undefined`), and a read of the envelope's `success` is one too. + * Classify each hit the way `LEDGER` does; record non-zero results on #13079. + */ const CLOUD_CENSUS_COMMAND = "git -C fetch origin main && git -C grep -nE 'analytics\\s*\\.\\s*(query|meta|explain)\\s*\\(|automation\\s*\\.\\s*trigger\\s*\\(' origin/main -- '*.ts' '*.tsx' '*.js'" + " # then re-check for SPLIT calls, which a line-oriented grep misses:" - + " git -C grep -nE '\\.(analytics|automation)\\s*$' origin/main -- '*.ts' '*.tsx' '*.js'"; + + " git -C grep -nE '\\.(analytics|automation)\\s*$' origin/main -- '*.ts' '*.tsx' '*.js'" + + " # post-#13079: a `.data` read on any hit is a RUNTIME BREAK (the value is the payload now); record non-zero on #13079"; -const CLOUD_CENSUS = { status: 'NOT_MEASURED', reason: 'repo not reachable from this session class' } as const; +const CLOUD_CENSUS = { + status: 'NOT_MEASURED', + reason: 'repo not reachable from this session class; ruled as such on 2026-08-31 (「A,cloud 未测量照裁」)', +} as const; const sdkSites = CENSUS.sites.filter((s) => s.receiver === 'sdk'); const ledgerTotal = LEDGER.reduce((n, r) => n + r.count, 0); const verdictTotal = (v: Verdict) => LEDGER.filter((r) => r.verdict === v).reduce((n, r) => n + r.count, 0); +/** + * The SDK source of one namespace's method — so section 4 reads what each + * method ENDS with off the source rather than restating it. + * + * Three anchoring decisions, each paid for by a wrong slice: + * - comments are MASKED first (the tree's own `maskComments`), so a docblock + * naming `res.json()` or `unwrapResponse` can neither satisfy nor fail a + * CODE assertion; + * - the NAMESPACE is located first (`analytics = {` … `};` is a class field + * at two-space indentation) because `query: async` and `explain: async` + * are spelled in other namespaces too, and the first match in the file is + * not the analytics one; + * - the slice runs from `NAME: async` to the next sibling property at the + * same indentation or the namespace's closing `};` — a method's own closing + * `}` is not a safe anchor: `queryDataset`'s parameter type literal closes + * with one at that indentation, and the last property closes without a + * comma. + */ +function methodSource(src: string, ns: string, name: string): string { + const masked = maskComments(src); + const block = new RegExp(`\\n ${ns} = \\{[\\s\\S]*?\\n \\};`).exec(masked); + if (!block) throw new Error(`namespace \`${ns}\` not found in index.ts`); + // `[ \t]+`, not `\s+`: the indentation capture must not absorb a newline. + const m = new RegExp(`\\n([ \\t]+)${name}: async[\\s\\S]*?(?=\\n\\1[A-Za-z_$][\\w$]*: |\\n \\};)`).exec(block[0]); + if (!m) throw new Error(`method \`${ns}.${name}\` not found in index.ts`); + return m[0]; +} + // --------------------------------------------------------------------------- describe('#13079 §1 — the population actually swept', () => { @@ -345,6 +439,7 @@ describe('#13079 §2 — positive controls on the matcher itself', () => { it('finds real call sites (a zero here would invalidate every count below)', () => { expect(sdkSites.length).toBeGreaterThan(0); const files = new Set(sdkSites.map((s) => s.file)); + expect(files.has('packages/client/src/envelope-convergence.test.ts')).toBe(true); expect(files.has('packages/client/src/analytics-automation-json-erasure.test.ts')).toBe(true); expect(files.has('packages/client/src/return-type-precision.test.ts')).toBe(true); expect(files.has('packages/client/src/client.test.ts')).toBe(true); @@ -352,10 +447,16 @@ describe('#13079 §2 — positive controls on the matcher itself', () => { it('⭐ catches the SPLIT calls a line-oriented grep misses, and reports the gap', () => { const missedByLineGrep = CENSUS.sites.filter((s) => !s.visibleToLineGrep); - // Both are `await client.automation` / newline / `.trigger(...)`. - expect(missedByLineGrep.length).toBe(2); - expect(missedByLineGrep.every((s) => s.method === 'automation.trigger')).toBe(true); - expect(missedByLineGrep.every((s) => s.file === 'packages/client/src/client.test.ts')).toBe(true); + // `await client.automation` / newline / `.trigger(...)` twice in + // `client.test.ts`; the same shape twice for `trigger` and once for + // `analytics.query` in `envelope-convergence.test.ts`. + expect(missedByLineGrep.length).toBe(5); + const byFile = new Map(); + for (const s of missedByLineGrep) byFile.set(s.file, [...(byFile.get(s.file) ?? []), s.method]); + expect(Object.fromEntries([...byFile].map(([f, ms]) => [f, ms.sort()]))).toEqual({ + 'packages/client/src/client.test.ts': ['automation.trigger', 'automation.trigger'], + 'packages/client/src/envelope-convergence.test.ts': ['analytics.query', 'automation.trigger', 'automation.trigger'], + }); }); it('does not count comment mentions as call sites', () => { @@ -380,7 +481,10 @@ describe('#13079 §3 — every call site is classified', () => { enumerated.set(k, (enumerated.get(k) ?? 0) + 1); } const ledgered = new Map(); - for (const r of LEDGER) ledgered.set(`${r.file}|${r.method}|${r.receiver}`, r.count); + for (const r of LEDGER) { + const k = `${r.file}|${r.method}|${r.receiver}`; + ledgered.set(k, (ledgered.get(k) ?? 0) + r.count); + } // An UNCLASSIFIED site is the failure this ratchet exists to produce: // whoever adds a call site to these four methods must say what it reads. @@ -390,51 +494,73 @@ describe('#13079 §3 — every call site is classified', () => { expect(ledgerTotal).toBe(CENSUS.sites.length); }); - it('⭐ THE NUMBER: zero call sites in this repo would break SILENTLY', () => { - // Every envelope-dependent site is a test assertion or a type pin — - // loud by construction. There is no production call site in this repo. + it('⭐ THE NUMBER, post-convergence: ZERO call sites still read the envelope, and none is production code', () => { + // The ratchet the convergence leaves behind. A site that reads `.data` + // or the envelope's `success` off these four now reads `undefined` + // (or fails to compile); the only legal way to add one is to classify + // it here — and this line refuses the classification. + expect(verdictTotal('ENVELOPE_DEPENDENT')).toBe(0); + // Every site is a test pin. There is still no production call site in + // this repo, so the migration in-repo was exactly this change's diff. const production = sdkSites.filter((s) => !/\.test\.tsx?$/.test(s.file)); expect(production).toEqual([]); }); - it('records the split: 13 loud pin sites, 6 result-insensitive, 1 not-SDK', () => { - expect(verdictTotal('ENVELOPE_DEPENDENT')).toBe(13); - expect(verdictTotal('RESULT_INSENSITIVE')).toBe(6); + it('records the split: 18 payload pins, 10 result-insensitive, 1 not-SDK', () => { + expect(verdictTotal('PAYLOAD_DEPENDENT')).toBe(18); + expect(verdictTotal('RESULT_INSENSITIVE')).toBe(10); expect(verdictTotal('NOT_SDK')).toBe(1); - expect(sdkSites.length).toBe(19); + expect(sdkSites.length).toBe(28); }); }); -describe('#13079 §4 — `analytics.queryDataset` is a protected counter-example', () => { +describe('#13079 §4 — the four end in `unwrapResponse`; `analytics.queryDataset` is the protected counter-example', () => { + // Read from the SDK source rather than restated, so a change to a method + // cannot leave these claims behind. + const src = readFileSync(join(HERE, 'index.ts'), 'utf8'); + + it('each of the four methods ends `return this.unwrapResponse(...)` and no longer reads `res.json()`', () => { + for (const [ns, method] of METHODS) { + const body = methodSource(src, ns, method); + expect(body, `${ns}.${method} must unwrap`).toMatch(/return this\.unwrapResponse { expect(METHODS.some(([, m]) => m === 'queryDataset')).toBe(false); expect(CENSUS.sites.some((s) => s.method.includes('queryDataset'))).toBe(false); }); - it('is served BARE by @objectstack/rest — there is no envelope to strip', () => { - // Read from the SDK source rather than restated, so a change to the - // method cannot leave this claim behind. - const src = readFileSync(join(HERE, 'index.ts'), 'utf8'); - expect(src).toMatch(/queryDataset:\s*async/); - expect(src).toMatch(/analytics\/dataset\/query/); + it('is served BARE by @objectstack/rest and still reads `res.json()` — there is no envelope to strip', () => { + const body = methodSource(src, 'analytics', 'queryDataset'); + // The CODE spelling: the prefix comes from `getRoute('analytics')`, the + // literal `analytics/dataset/query` only ever lived in the docblock. + expect(body).toMatch(/getRoute\('analytics'\)/); + expect(body).toMatch(/\$\{route\}\/dataset\/query`/); + expect(body).toMatch(/return res\.json\(\);/); + expect(body).not.toMatch(/unwrapResponse/); // Its sibling `query` dials the dispatcher route; these are two dialects. - expect(src).toMatch(/\$\{route\}\/query/); + expect(methodSource(src, 'analytics', 'query')).toMatch(/\$\{route\}\/query`/); }); }); describe('#13079 §5 — what was NOT measured', () => { - it('objectui is recorded with its revision, and its one production site survives', () => { + it('objectui is recorded with its revision, its one production site survives, and its tightening has an owner', () => { expect(OBJECTUI_CENSUS.productionCallSites).toBe(1); expect(OBJECTUI_CENSUS.wouldBreak).toBe(0); expect(OBJECTUI_CENSUS.revision).toMatch(/^[0-9a-f]{40}$/); + expect(OBJECTUI_CENSUS.tighteningOwner).toBe('objectui#7028'); expect(OBJECTUI_CENSUS_COMMAND).toContain('origin/main'); }); - it('⛔ cloud is NOT MEASURED and must not be read as clean', () => { + it('⛔ cloud is NOT MEASURED, ruled so, and must not be read as clean', () => { expect(CLOUD_CENSUS.status).toBe('NOT_MEASURED'); // The ready-to-run sweep includes the split-call second pass, because - // the single-line form alone under-reported by 2 in THIS repo. + // the single-line form alone under-reported by 2 in THIS repo before + // #13079 (and by 5 after it), and states the post-#13079 reading. expect(CLOUD_CENSUS_COMMAND).toContain('fetch origin main'); expect(CLOUD_CENSUS_COMMAND).toContain('(analytics|automation)'); + expect(CLOUD_CENSUS_COMMAND).toContain('RUNTIME BREAK'); }); }); diff --git a/packages/client/src/envelope-convergence.test.ts b/packages/client/src/envelope-convergence.test.ts new file mode 100644 index 0000000000..22c3ebf0ef --- /dev/null +++ b/packages/client/src/envelope-convergence.test.ts @@ -0,0 +1,260 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#13079] `analytics.query` / `analytics.meta` / `analytics.explain` and + * `automation.trigger` resolve to the PAYLOAD — the value under the + * dispatcher's `{ success, data }` envelope — through `unwrapResponse`, the + * reader every other dispatcher-served method of `ObjectStackClient` uses. + * One SDK, one calling convention (maintainer ruling on #13079, 2026-08-31: + * option A, with the `cloud` population ruled NOT MEASURED). + * + * ## What this file pins, and what its sibling pins instead + * + * The transport is MOCKED here on purpose. `fetch` answers a literal + * dispatcher body, so every assertion below is about what the SDK DOES with + * such a body — strips the envelope exactly once, hands `data` back, changes + * nothing on the rejection path. Whether the body a real producer sends has + * that shape is a different question, and + * `analytics-automation-json-erasure.test.ts` answers it by driving the real + * `AnalyticsService`, `AutomationEngine`, `HttpDispatcher` and `RestServer`. + * Two halves, separately falsifiable: a mocked body cannot vouch for the + * producer, and a producer-backed run cannot isolate the reader. + * + * ## Red first + * + * Against the pre-#13079 client every payload case in section 1 FAILS: the + * four methods ended `return res.json()`, which strips nothing, so `value` + * was the envelope — `value.rows` undefined, `'success' in value` true. The + * `'success' in value` / `'data' in value` refusals are the direction that + * catches a HALF-conversion: a method switched to `unwrapResponse` against a + * route that answered bare would still satisfy the equality, and only the key + * refusals say the envelope was really there to strip. + * + * ## The failure path, stated per door — the migration's sharpest edge + * + * `unwrapResponse` NEVER throws. Every non-2xx answer is thrown by + * `ObjectStackClient.fetch` BEFORE any reader runs, carrying the ADR-0112 + * envelope on the error (`err.code`, `err.httpStatus`) — true before #13079, + * true after it — so a `catch` written for a failed `trigger` (#9378: 400 + * `FLOW_FAILED`; #9415: 409 `FLOW_DISABLED` / 422 `FLOW_NO_START_NODE`) does + * not change. A 2xx body WITHOUT a `data` key passes through `unwrapResponse` + * unchanged; no dispatcher door behind these four routes sends one, and the + * pass-through is pinned so the migration note's claim stays mechanically + * true rather than remembered. + * + * `analytics.queryDataset` is the PROTECTED counter-example (ruling item 1): + * served bare by `@objectstack/rest`, it keeps `res.json()` and resolves to + * the same bare body — pinned last so a sweep cannot fold it in. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { ObjectStackClient } from './index'; + +const BASE_URL = 'http://localhost:3000'; + +/** A client whose transport answers `body` at `status`, nothing else. */ +function clientAnswering(body: unknown, status = 200) { + const fetchMock = vi.fn().mockResolvedValue({ + ok: status >= 200 && status < 300, + status, + statusText: String(status), + headers: new Headers(), + json: async () => body, + }); + const client = new ObjectStackClient({ baseUrl: BASE_URL, fetch: fetchMock }); + return { client, fetchMock }; +} + +/** The envelope every dispatcher domain answers through `deps.success(v)`. */ +function enveloped(data: T) { + return { success: true as const, data, meta: { timestamp: '2026-09-02T00:00:00.000Z' } }; +} + +const QUERY = { cube: 'crm_account', measures: ['account_count'], dimensions: ['industry'] }; + +const RESULT = { + rows: [{ industry: 'tech', account_count: 3 }], + fields: [ + { name: 'industry', type: 'string', label: 'Industry' }, + { name: 'account_count', type: 'number', label: 'Account count' }, + ], +}; + +// ───────────────────────────────────────────────────────────────────────────── + +describe('#13079 §1 — the four dispatcher-served methods resolve to the PAYLOAD', () => { + it('analytics.query resolves to the AnalyticsResult under `data`, not to the envelope', async () => { + const { client } = clientAnswering(enveloped(RESULT)); + + const value = await client.analytics.query(QUERY); + + expect(value).toEqual(RESULT); + expect(value.rows).toEqual(RESULT.rows); + // The envelope keys are GONE — this is what `res.json()` never did. + expect('success' in value).toBe(false); + expect('data' in value).toBe(false); + expect('meta' in value).toBe(false); + }); + + it('analytics.meta resolves to the bare cube list — no `data`, no `cubes` wrapper', async () => { + const cubes = [ + { + name: 'crm_account', + title: 'Accounts', + measures: [{ name: 'crm_account.account_count', type: 'count' }], + dimensions: [{ name: 'crm_account.industry', type: 'string' }], + }, + ]; + const { client } = clientAnswering(enveloped(cubes)); + + const value = await client.analytics.meta(); + + expect(Array.isArray(value)).toBe(true); + expect(value).toEqual(cubes); + expect(value[0]?.name).toBe('crm_account'); + expect(value[0]?.measures.map((m) => m.name)).toEqual(['crm_account.account_count']); + }); + + it('analytics.explain resolves to `{ sql, params }`', async () => { + const dryRun = { sql: 'SELECT industry, COUNT(*) FROM crm_account GROUP BY industry', params: [] }; + const { client } = clientAnswering(enveloped(dryRun)); + + const value = await client.analytics.explain(QUERY); + + expect(Object.keys(value).sort()).toEqual(['params', 'sql']); + expect(value.sql).toMatch(/^SELECT/); + expect(value.params).toEqual([]); + }); + + it('automation.trigger resolves to the AutomationResult — the run itself, as `execute` does', async () => { + // A PAUSED run: the arm whose result carries `status` / `runId` / + // `screen`, the keys a caller most needs to reach without `.data`. + const run = { + success: true, + status: 'paused', + runId: 'run_1', + screen: { nodeId: 'gate', title: 'Approve the account', fields: [] }, + durationMs: 4, + }; + const { client } = clientAnswering(enveloped(run)); + + const value = await client.automation.trigger('approve_account', {}); + + expect(value).toEqual(run); + expect(value.status).toBe('paused'); + expect(value.runId).toBe('run_1'); + expect(value.screen?.title).toBe('Approve the account'); + // `AutomationResult` carries its OWN `success` (the run's flag), so on + // this door the envelope-vs-payload difference is `data`, not + // `success`: the value has no `data` under it, and its `success` is + // the run's, reached at the top level exactly as `execute` hands it. + expect('data' in value).toBe(false); + expect(value.success).toBe(true); + }); + + it('strips the envelope exactly ONCE — a payload that itself carries `success` is not unwrapped again', async () => { + // `unwrapResponse` keys on `success` + `data` together (its #12038 §8.2 + // hazard is pinned in `unwrapper-misfire`-style suites for the REST + // surface). A run result has `success` but no `data`, so after the + // one strip nothing looks like a second envelope. + const run = { success: false, status: 'failed', error: 'node create_opportunity failed', durationMs: 9 }; + const { client } = clientAnswering(enveloped(run)); + + const value = await client.automation.trigger('flow_that_reports_its_own_flag', {}); + + expect(value).toEqual(run); + expect(value.success).toBe(false); + }); +}); + +// ───────────────────────────────────────────────────────────────────────────── + +describe('#13079 §2 — the failure path is unchanged: a non-2xx throws BEFORE any reader runs', () => { + const failedRun = { + success: false, + error: { + code: 'FLOW_FAILED', + message: "Node 'create_opportunity' failed: amount must be positive", + httpStatus: 400, + details: { errorMessage: 'Check the amount and try again.' }, + }, + }; + + it('automation.trigger: 400 FLOW_FAILED rejects with the ADR-0112 code and status', async () => { + const { client } = clientAnswering(failedRun, 400); + + const err: any = await client.automation + .trigger('my_flow', { amount: 0 }) + .then(() => { throw new Error('expected the failed trigger to reject'); }, (e) => e); + + // The classification, not merely that it threw (a bare `Error` would + // satisfy `.rejects.toThrow()` while losing everything a caller + // branches on). + expect(err.code).toBe('FLOW_FAILED'); + expect(err.httpStatus).toBe(400); + expect(err.message).toMatch(/Node 'create_opportunity' failed/); + expect(err.details?.errorMessage).toBe('Check the amount and try again.'); + }); + + it('automation.trigger: 409 FLOW_DISABLED rejects the same way — never dispatched, never FLOW_FAILED', async () => { + const { client } = clientAnswering({ + success: false, + error: { code: 'FLOW_DISABLED', message: "Flow 'welcome_flow' is disabled", httpStatus: 409 }, + }, 409); + + const err: any = await client.automation + .trigger('welcome_flow', {}) + .then(() => { throw new Error('expected the disabled flow to reject'); }, (e) => e); + + expect(err.code).toBe('FLOW_DISABLED'); + expect(err.httpStatus).toBe(409); + expect(err.code).not.toBe('FLOW_FAILED'); + }); + + it('analytics.query: a refused query rejects with the envelope code and status', async () => { + const { client } = clientAnswering({ + success: false, + error: { code: 'VALIDATION_ERROR', message: "Unknown measure 'revenue' on cube 'crm_account'", httpStatus: 400 }, + }, 400); + + const err: any = await client.analytics + .query({ cube: 'crm_account', measures: ['revenue'] }) + .then(() => { throw new Error('expected the refused query to reject'); }, (e) => e); + + expect(err.code).toBe('VALIDATION_ERROR'); + expect(err.httpStatus).toBe(400); + expect(err.message).toMatch(/Unknown measure 'revenue'/); + }); + + it('a 2xx body with NO `data` key passes through `unwrapResponse` unchanged (documented pass-through)', async () => { + // No dispatcher door behind these four routes sends a 2xx without + // `data` (a failed run has been a thrown 400 since #9378). The + // pass-through is `unwrapResponse`'s own contract on every method that + // uses it, pinned here so the migration note can say so mechanically. + const stray = { success: false, error: { code: 'FLOW_FAILED', message: 'a 200 nothing sends' } }; + const { client } = clientAnswering(stray, 200); + + const value: unknown = await client.automation.trigger('my_flow', {}); + + expect(value).toEqual(stray); + }); +}); + +// ───────────────────────────────────────────────────────────────────────────── + +describe('#13079 §3 — `analytics.queryDataset` is PROTECTED: served bare by @objectstack/rest, kept on res.json()', () => { + it('resolves to the bare body the REST route answers, with nothing stripped and nothing added', async () => { + // ⛔ Deliberately NOT `enveloped(...)`: the route ends `res.json(result)`. + const { client } = clientAnswering(RESULT); + + const value = await client.analytics.queryDataset({ + datasetName: 'account_metrics', + selection: { measures: ['account_count'], dimensions: ['industry'] }, + }); + + expect(value).toEqual(RESULT); + expect(value.rows).toEqual(RESULT.rows); + expect('success' in value).toBe(false); + expect('data' in value).toBe(false); + }); +}); diff --git a/packages/client/src/index.ts b/packages/client/src/index.ts index 95732239a2..3575b137c0 100644 --- a/packages/client/src/index.ts +++ b/packages/client/src/index.ts @@ -87,14 +87,13 @@ import { // [#11924] The GLOBAL cross-object search body — NOT the per-object // `SearchResult` in `@objectstack/spec/contracts` (the #8140 near-miss trap). SearchAllResponse, - // [#12104] The `{ success, data }` envelope SKELETON, plus the two analytics - // route response types that already transcribe their producer's declared - // return. The four `return res.json()` methods bound by that card resolve to - // the WHOLE body — `res.json()` strips nothing, unlike `unwrapResponse` — so - // the envelope IS the annotation, not the payload under it. `BaseResponse` is - // reused rather than hand-spelled so a field added to the envelope reaches - // these annotations with it. - BaseResponse, + // [#12104 → #13079] The two analytics route response types whose `data` + // member already transcribes the producer's declared return. Since #13079 + // the methods resolve to that `data` member (post-`unwrapResponse`), so the + // annotations INDEX these envelope types — `AnalyticsMetadataResponse['data']`, + // `AnalyticsSqlResponse['data']` — rather than re-declaring the payload: a + // change to the route's declared `data` reaches the SDK with it, and no + // payload type has to be declared in `packages/spec` for the SDK's sake. AnalyticsMetadataResponse, AnalyticsSqlResponse, // [#12038] The meta history/diagnostics and package lifecycle response @@ -1498,24 +1497,31 @@ export class ObjectStackClient { * Analytics Services */ /** - * [#12104] ⚠️ THE THREE DISPATCHER-SERVED METHODS HERE RESOLVE TO THE - * ENVELOPE, not to the payload — read `.data`. + * [#13079] EVERY method here resolves to the PAYLOAD — never to the + * dispatcher's `{ success, data }` envelope. * - * They end `return res.json()` rather than `return this.unwrapResponse(res)`, - * and `res.json()` strips nothing, so the caller receives the dispatcher's - * `{ success, data }` body whole. That was invisible while the methods were - * erased to `Promise< any >` (no annotation, and `Response.json()` is declared - * `Promise< any >` in `lib.dom`); it is stated by the declarations now. - * `queryDataset` below is the exception and says why. + * `query` / `meta` / `explain` are dispatcher-served and end + * `return this.unwrapResponse(res)`, which strips that envelope, exactly as + * every other dispatcher-served method of this class does. Until #13079 + * they ended `return res.json()`, which strips nothing, and handed the + * envelope back whole — so their callers alone had to read `.data` + * (invisible while the methods were erased to `Promise< any >`, stated by + * the #12104 declarations, converged by the 2026-08-31 ruling on #13079). + * `queryDataset` is served by `@objectstack/rest` with no envelope at all + * and keeps `res.json()` — see its docblock; ⛔ it is PROTECTED, not a fifth + * instance. Either way the caller reads one shape: the producer's declared + * return. */ analytics = { /** * Run an `AnalyticsQuery` (`POST /analytics/query`). * - * The `data` member is `IAnalyticsService.query`'s declared return, relayed - * verbatim by the domain (`deps.success(await analyticsService.query(…))`). + * Resolves to `IAnalyticsService.query`'s declared return — relayed + * verbatim by the domain (`deps.success(await analyticsService.query(…))`) + * and unwrapped here. **BREAKING since #13079**: read `result.rows`, not + * `result.data.rows`; the method used to resolve to the whole envelope. * - * Deliberately bound to the CONTRACT, not to `AnalyticsResultResponse` + * Deliberately bound to the CONTRACT, not to `AnalyticsResultResponse['data']` * (`@objectstack/spec/api`). When #12104 wrote this annotation the schema * was a stale projection — its `data.fields` declared `{ name, type }` * only, while the contract this route relays also carries `label` / @@ -1529,49 +1535,53 @@ export class ObjectStackClient { * source rather than the copy is what keeps this method immune to the * transcription drifting again. */ - query: async (payload: any): Promise => { + query: async (payload: any): Promise => { const route = this.getRoute('analytics'); const res = await this.fetch(`${this.baseUrl}${route}/query`, { method: 'POST', body: JSON.stringify(payload) }); - return res.json(); + return this.unwrapResponse(res); }, /** * Cube metadata listing. Pass `cube` to filter to a single cube * (`?cube=` — [#3584] the dispatcher shape; the old `/meta/:cube` path * segment was served by nothing and 404ed everywhere). * - * [#12104] `AnalyticsMetadataResponse` is the route's own declared response - * type and it AGREES with the producer: its `data` is the bare `CubeMeta[]` + * Resolves to `AnalyticsMetadataResponse['data']` — the bare `CubeMeta[]` * discovery projection `IAnalyticsService.getMeta` returns (#6442 narrowed - * the schema to exactly that, and `spec`'s `analytics.test.ts` pins + * the route's schema to exactly that, and `spec`'s `analytics.test.ts` pins * `data[number]` ≡ `CubeMeta` at compile time). There is no `cubes` - * wrapper under `data`. - */ - meta: async (cube?: string): Promise => { + * wrapper and, **since #13079**, no envelope either: + * `(await client.analytics.meta())[0].name`, where the method used to + * resolve to `{ success, data }` and the caller read `.data[0].name`. The + * annotation INDEXES the route's declared response type rather than + * re-declaring the payload, so the schema and this method cannot drift + * into saying two things. + */ + meta: async (cube?: string): Promise => { const route = this.getRoute('analytics'); const qs = cube ? `?cube=${encodeURIComponent(cube)}` : ''; const res = await this.fetch(`${this.baseUrl}${route}/meta${qs}`); - return res.json(); + return this.unwrapResponse(res); }, /** * Dry-run a query to its generated SQL (`POST /analytics/sql` — [#3584] * the dispatcher route; the old `/explain` route name was served by * nothing and 404ed everywhere). * - * [#12104] `AnalyticsSqlResponse` is the route's own declared response type - * and its `data` — `{ sql, params }` — is exactly + * Resolves to `AnalyticsSqlResponse['data']` — `{ sql, params }`, exactly * `IAnalyticsService.generateSql`'s declared return, so the schema and the - * producer say one thing here. + * producer say one thing here. **Since #13079** the value is that payload + * itself: read `result.sql`, not `result.data.sql`. */ - explain: async (payload: any): Promise => { + explain: async (payload: any): Promise => { const route = this.getRoute('analytics'); const res = await this.fetch(`${this.baseUrl}${route}/sql`, { method: 'POST', body: JSON.stringify(payload) }); - return res.json(); + return this.unwrapResponse(res); }, /** * ADR-0021 semantic-layer dataset query — the REST dialect @@ -1580,10 +1590,13 @@ export class ObjectStackClient { * `datasetName` (saved), plus `selection.measures`; `previewDrafts` * runs over draft-overlaid definitions (ADR-0037 P3). (#3587 gap closure) * - * [#12104] ⚠️ The ONE method in this namespace that resolves to the BARE - * payload. It is served by `@objectstack/rest` (the dispatcher mounts no - * twin), and that route ends `res.json(result)` with no envelope around it - * — so unlike its three siblings above there is no `.data` to read. Both + * [#12104] ⚠️ The one method in this namespace served with NO envelope at + * all: `@objectstack/rest` mounts it (the dispatcher mounts no twin) and + * the route ends `res.json(result)`, so `res.json()` here IS the payload + * read — there is nothing for `unwrapResponse` to strip. ⛔ PROTECTED by + * the #13079 ruling: its three siblings above were converged on + * `unwrapResponse` because their routes answer the envelope; this one was + * correct as it stood and must not be "fixed" into their shape. Both * halves measured on the real route in * `analytics-automation-json-erasure.test.ts`; the shape is * `IAnalyticsService.queryDataset`'s declared return. @@ -3860,14 +3873,20 @@ export class ObjectStackClient { * | `400` | `FLOW_FAILED` | the flow RAN and was rejected | read `err.details.summary` for the failing node | * | `404` | — | no such flow in this deployment | check the name | * - * [#12104] ⚠️ **This method resolves to the ENVELOPE — read `.data`.** It - * ends `return res.json()`, which strips nothing, so the value is the - * dispatcher's `{ success, data }` body; its sibling - * `automation.execute` calls the SAME door through `unwrapResponse` and - * therefore resolves to the `AutomationResult` alone. The two differ in - * the wrapper only, which is why the payload type is the same one. + * [#13079] **Resolves to the `AutomationResult` alone — the SAME value + * its sibling `automation.execute` resolves to**, because both reach one + * handler through `unwrapResponse`. **BREAKING since #13079**: read + * `result.status` / `result.runId` / `result.screen`, not + * `result.data.…` — until then this method ended `return res.json()`, + * which strips nothing, and handed the dispatcher's `{ success, data }` + * envelope back whole while `execute` unwrapped it (#12104 stated that + * split in the declarations; the 2026-08-31 ruling closed it). The + * rejection table above is untouched: every non-2xx throws out of the + * fetch layer BEFORE either reader runs, and a 2xx body without `data` + * (nothing on this door sends one) passes through `unwrapResponse` + * unchanged, as on every other unwrapped method. * - * Deliberately bound to the CONTRACT, not to `TriggerFlowResponse` + * Deliberately bound to the CONTRACT, not to `TriggerFlowResponse['data']` * (`@objectstack/spec/api`). When #12104 wrote this annotation the * schema was a stale projection — its `data` declared * `{ success, output?, error?, durationMs? }` while the door also @@ -3883,13 +3902,13 @@ export class ObjectStackClient { * annotating the source rather than the copy is what keeps this method * immune to the transcription drifting again. */ - trigger: async (triggerName: string, payload: any): Promise => { + trigger: async (triggerName: string, payload: any): Promise => { const route = this.getRoute('automation'); const res = await this.fetch(`${this.baseUrl}${route}/trigger/${triggerName}`, { method: 'POST', body: JSON.stringify(payload) }); - return res.json(); + return this.unwrapResponse(res); }, /** diff --git a/packages/client/src/return-type-precision.test.ts b/packages/client/src/return-type-precision.test.ts index f31452952f..2566236cc2 100644 --- a/packages/client/src/return-type-precision.test.ts +++ b/packages/client/src/return-type-precision.test.ts @@ -38,7 +38,6 @@ import type { SearchAllResponse } from '@objectstack/spec/api'; import type { AnalyticsMetadataResponse, AnalyticsSqlResponse, - BaseResponse, TriggerFlowResponse, } from '@objectstack/spec/api'; import type { @@ -448,53 +447,64 @@ export async function returnTypePrecisionPins12034(): Promise { * with only the socket stood in for). The other 38 are the better-auth-backed * `auth.*` / `organizations.*` / `oauth.*` families and are NOT touched here. * - * ## What makes these five different from every binding above + * ## What makes these five different from every binding above — and what + * ## #13079 changed * * `unwrapResponse` strips the `{ success, data }` envelope; `res.json()` does - * not. So four of the five resolve to the ENVELOPE and the annotation says so; - * the fifth is served by `@objectstack/rest` with no envelope at all and binds - * the bare payload. Getting that split wrong in either direction typechecks - * against `any` and ships a false declaration — the census's highest-risk band. + * not. When #12104 bound them, four of the five ended `res.json()` and + * resolved to the ENVELOPE, and the annotations said so. #13079 (maintainer + * ruling 2026-08-31, option A) converged those four on `unwrapResponse`, so + * all five now resolve to the PAYLOAD: three to the route's declared `data` + * member, one to the same `AutomationResult` its sibling `execute` unwraps, + * and `queryDataset` — served by `@objectstack/rest` with no envelope at all, + * PROTECTED by the ruling — to the bare payload it always answered. The pins + * below are the #12104 pins REVERSED, not deleted: the payload read compiles, + * the envelope read (`.data`) is the type error. Getting any of them wrong + * typechecks against `any` and ships a false declaration — the census's + * highest-risk band. * * Type-level for the reason this file's header gives: a runtime test cannot - * observe a return-type narrowing at all. + * observe a return-type change at all — `envelope-convergence.test.ts` pins + * the VALUE, this function pins the DECLARATION. */ export async function returnTypePrecisionPins12104(): Promise { - // ── the three dispatcher-served analytics reads: the ENVELOPE ───────── - // `data` is the producer's declared return, relayed by `deps.success(v)`. + // ── [#13079] the three dispatcher-served analytics reads: the PAYLOAD ── + // Each is the route's declared `data` member — what `deps.success(v)` + // wrapped and `unwrapResponse` hands back — indexed off the envelope type + // rather than re-declared, so the annotation follows the route's schema. expectTypeOf(await client.analytics.query({ cube: 'crm_account', measures: ['n'] })) - .toEqualTypeOf(); - expectTypeOf(await client.analytics.meta()).toEqualTypeOf(); + .toEqualTypeOf(); + expectTypeOf(await client.analytics.meta()).toEqualTypeOf(); expectTypeOf(await client.analytics.explain({ cube: 'crm_account', measures: ['n'] })) - .toEqualTypeOf(); + .toEqualTypeOf(); - // ── the trigger door: the ENVELOPE over the same payload its sibling - // `automation.execute` unwraps ───────────────────────────────────── + // ── the trigger door: the SAME payload its sibling `automation.execute` + // unwraps — one handler, one shape, since #13079 ───────────────────── expectTypeOf(await client.automation.trigger('approve_account', {})) - .toEqualTypeOf(); + .toEqualTypeOf(); - // ── the one REST-served method: the BARE payload ────────────────────── + // ── the one REST-served method: the BARE payload, unchanged ─────────── expectTypeOf(await client.analytics.queryDataset({ selection: { measures: ['n'] } })) .toEqualTypeOf(); - // ── direction 2: the reads the erasure allowed must now FAIL ────────── - // Each suppression is unused — a TS2578 error — while the method still - // returns `any`, because `any` satisfies every one of these. - - // The envelope/payload confusion, in the direction a caller writes it: - // reading a payload key off the enveloped value. - // @ts-expect-error `analytics.query` answers the envelope; the rows are under `.data` - void (await client.analytics.query({ cube: 'crm_account', measures: ['n'] })).rows; - // @ts-expect-error `analytics.meta` answers the envelope; the cubes are under `.data` - void (await client.analytics.meta()).length; - // @ts-expect-error `analytics.explain` answers the envelope; the statement is under `.data` - void (await client.analytics.explain({ cube: 'crm_account', measures: ['n'] })).sql; - // @ts-expect-error `automation.trigger` answers the envelope; the run is under `.data` - void (await client.automation.trigger('approve_account', {})).runId; - - // …and the SAME confusion in the opposite direction on the one method that - // really is bare. This is the half that makes the split load-bearing rather - // than a family-wide guess. + // ── direction 2, REVERSED by #13079: the ENVELOPE reads must now FAIL ── + // Before #13079 these four suppressions sat on the PAYLOAD read (`.rows`, + // `.length`, `.sql`, `.runId`) because the methods answered the envelope. + // Reversed, not deleted: each now sits on the `.data` read a pre-#13079 + // caller wrote, so a method that slid back to `res.json()` — or a + // declaration that slid back to the envelope — leaves its suppression + // unused (TS2578) and this file red. + // @ts-expect-error `analytics.query` answers the AnalyticsResult itself; there is no `.data` + void (await client.analytics.query({ cube: 'crm_account', measures: ['n'] })).data; + // @ts-expect-error `analytics.meta` answers the bare cube list; there is no `.data` + void (await client.analytics.meta()).data; + // @ts-expect-error `analytics.explain` answers `{ sql, params }`; there is no `.data` + void (await client.analytics.explain({ cube: 'crm_account', measures: ['n'] })).data; + // @ts-expect-error `automation.trigger` answers the run itself; there is no `.data` + void (await client.automation.trigger('approve_account', {})).data; + + // …and the method that was ALWAYS bare keeps its pin verbatim: the + // convergence made the other four look like it, it did not touch it. // @ts-expect-error `queryDataset` is served bare by @objectstack/rest — there is no envelope void (await client.analytics.queryDataset({ selection: { measures: ['n'] } })).data;