From 897585a74b30078c8edbc9c2041f8d3d4fdf0250 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 04:16:05 +0000 Subject: [PATCH 1/7] feat(client)!: converge analytics.query/meta/explain and automation.trigger on unwrapResponse The four dispatcher-served methods ended `return res.json()` and handed their callers the `{ success, data }` envelope whole, while every other dispatcher-served method of ObjectStackClient strips it through `unwrapResponse`. All four now end `return this.unwrapResponse(res)` and declare the payload: `AnalyticsResult`, `AnalyticsMetadataResponse['data']`, `AnalyticsSqlResponse['data']`, `AutomationResult` (the value `automation.execute` already answered for the same handler). `analytics.queryDataset` is protected: served bare by @objectstack/rest, it keeps `res.json()`; its body is byte-identical (only its docblock says why). Tests: `envelope-convergence.test.ts` (new, mocked transport: the payload on all four, the unchanged rejection path, the no-`data` pass-through, and the protected method); `analytics-automation-json-erasure.test.ts` re-driven against the real producers to the payload; `return-type-precision.test.ts` pins reversed (payload compiles, `.data` is the type error); the caller census regenerated to the post-convergence ledger with the cloud census command kept and reworded. One breaking changeset with the migration table. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza --- .changeset/client-envelope-convergence.md | 64 ++++ .../analytics-automation-json-erasure.test.ts | 121 ++++---- packages/client/src/client.test.ts | 7 +- .../client/src/envelope-caller-census.test.ts | 289 ++++++++++++------ .../client/src/envelope-convergence.test.ts | 260 ++++++++++++++++ packages/client/src/index.ts | 111 ++++--- .../client/src/return-type-precision.test.ts | 78 ++--- 7 files changed, 703 insertions(+), 227 deletions(-) create mode 100644 .changeset/client-envelope-convergence.md create mode 100644 packages/client/src/envelope-convergence.test.ts diff --git a/.changeset/client-envelope-convergence.md b/.changeset/client-envelope-convergence.md new file mode 100644 index 0000000000..06b61cd531 --- /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 +``` + +Every old read is a compile error under the new declarations (`Property 'data' does not exist on type …`), so a TypeScript consumer finds each site at build time; 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 a resolved `{ success: false, error }` is not a value you will receive from them. +- **What you lose.** The envelope's `success` flag and its `meta` (`timestamp` / `requestId` / `traceId`) are no longer on the resolved value. They were never on any other SDK method's value either; a read of `meta.requestId` off one of these four has no SDK-level replacement, the same as on every other unwrapped method. + +### 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..9e02d279ed 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,51 @@ 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, from `NAME: async` to the closing + * `},` at the same indentation — so section 4 reads what each method ENDS with + * off the source rather than restating it. Anchored to the NAMESPACE 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. + */ +function methodSource(src: string, ns: string, name: string): string { + const block = new RegExp(`\\n ${ns} = \\{[\\s\\S]*?\\n \\};`).exec(src); + if (!block) throw new Error(`namespace \`${ns}\` not found in index.ts`); + const m = new RegExp(`\\n(\\s+)${name}: async[\\s\\S]*?\\n\\1\\},`).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 +427,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 +435,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 +469,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 +482,70 @@ 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'); + expect(body).toMatch(/analytics\/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; From 5ce08b0f0c2b3ee660e2946b120f758c51c084c5 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 04:20:30 +0000 Subject: [PATCH 2/7] test(client): let the census method slicer close on a namespace's last property MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `queryDataset` is the last member of `analytics = {…}` and closes with a bare `}`; the slicer required `},` and could not find it inside the block. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza --- packages/client/src/envelope-caller-census.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/client/src/envelope-caller-census.test.ts b/packages/client/src/envelope-caller-census.test.ts index 9e02d279ed..6a482c29c7 100644 --- a/packages/client/src/envelope-caller-census.test.ts +++ b/packages/client/src/envelope-caller-census.test.ts @@ -400,7 +400,8 @@ const verdictTotal = (v: Verdict) => function methodSource(src: string, ns: string, name: string): string { const block = new RegExp(`\\n ${ns} = \\{[\\s\\S]*?\\n \\};`).exec(src); if (!block) throw new Error(`namespace \`${ns}\` not found in index.ts`); - const m = new RegExp(`\\n(\\s+)${name}: async[\\s\\S]*?\\n\\1\\},`).exec(block[0]); + // `},?` — the LAST property of a namespace closes with a bare `}`. + const m = new RegExp(`\\n(\\s+)${name}: async[\\s\\S]*?\\n\\1\\},?`).exec(block[0]); if (!m) throw new Error(`method \`${ns}.${name}\` not found in index.ts`); return m[0]; } From 8f9001c665ecc2736497c80be09543700c028223 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 09:39:03 +0000 Subject: [PATCH 3/7] test(client): slice census methods on masked source, anchored to the next sibling The previous anchor (a method's own closing brace) stopped at queryDataset's parameter type literal, and a `\s+` indentation capture absorbed newlines. Comments are masked first so docblock prose cannot satisfy a code assertion. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza --- .../client/src/envelope-caller-census.test.ts | 29 +++++++++++++------ 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/packages/client/src/envelope-caller-census.test.ts b/packages/client/src/envelope-caller-census.test.ts index 6a482c29c7..fa44b9272e 100644 --- a/packages/client/src/envelope-caller-census.test.ts +++ b/packages/client/src/envelope-caller-census.test.ts @@ -390,18 +390,29 @@ 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, from `NAME: async` to the closing - * `},` at the same indentation — so section 4 reads what each method ENDS with - * off the source rather than restating it. Anchored to the NAMESPACE 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 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 block = new RegExp(`\\n ${ns} = \\{[\\s\\S]*?\\n \\};`).exec(src); + 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`); - // `},?` — the LAST property of a namespace closes with a bare `}`. - const m = new RegExp(`\\n(\\s+)${name}: async[\\s\\S]*?\\n\\1\\},?`).exec(block[0]); + // `[ \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]; } From 9e5a10ee5cb9295cdf9f2e4c8adf65062ec420e7 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 10:25:21 +0000 Subject: [PATCH 4/7] test(client): assert queryDataset's route by its code spelling, not the docblock's `analytics/dataset/query` only ever appeared in prose; the method spells it `getRoute('analytics')` + `${route}/dataset/query`, which is what a comment-masked slice can see. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza --- packages/client/src/envelope-caller-census.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/client/src/envelope-caller-census.test.ts b/packages/client/src/envelope-caller-census.test.ts index fa44b9272e..52705e87a9 100644 --- a/packages/client/src/envelope-caller-census.test.ts +++ b/packages/client/src/envelope-caller-census.test.ts @@ -534,7 +534,10 @@ describe('#13079 §4 — the four end in `unwrapResponse`; `analytics.queryDatas it('is served BARE by @objectstack/rest and still reads `res.json()` — there is no envelope to strip', () => { const body = methodSource(src, 'analytics', 'queryDataset'); - expect(body).toMatch(/analytics\/dataset\/query/); + // 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. From d543e098859cf2f101af7040777e421ec0994aee Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 14:17:59 +0000 Subject: [PATCH 5/7] docs(changeset): scope the #13079 failure-path claims to the envelope level MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Contract review corrections, prose only. `client.automation.trigger` is excepted from the universal compile-error claim: `r.success` and `r.error` compile before and after (both declared on `AutomationResult`), and their meaning moves from the envelope's to the run's own — a refusal the door does not classify as 400/409/422 is answered 200 through `deps.success(result)` (`respondToFlowTrigger`, classification in `flow-dispatch-status.ts`), as `execute` already does. The "not a value you will receive" sentence is scoped to the envelope level, and the `meta` sentence no longer implies the four doors ever populated it. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza --- .changeset/client-envelope-convergence.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.changeset/client-envelope-convergence.md b/.changeset/client-envelope-convergence.md index 06b61cd531..54757066e8 100644 --- a/.changeset/client-envelope-convergence.md +++ b/.changeset/client-envelope-convergence.md @@ -43,15 +43,15 @@ r4.data.status; // before ('paused' | 'completed' | 'failed') r4.status; // now — exactly what `client.automation.execute` already answered ``` -Every old read is a compile error under the new declarations (`Property 'data' does not exist on type …`), so a TypeScript consumer finds each site at build time; a JavaScript consumer reads `undefined` from `.data` and has to search for the four spellings. +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 a resolved `{ success: false, error }` is not a value you will receive from them. -- **What you lose.** The envelope's `success` flag and its `meta` (`timestamp` / `requestId` / `traceId`) are no longer on the resolved value. They were never on any other SDK method's value either; a read of `meta.requestId` off one of these four has no SDK-level replacement, the same as on every other unwrapped method. +- **`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 From 24f2507c0a1536e7d01f7d8cb8987a837b902a74 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 01:44:55 +0000 Subject: [PATCH 6/7] feat(spec): register the client envelope convergence in the ADR-0087 ledger (#13079) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The changeset for this branch carried `adr-0087: not-required (runtime-interface-only …)` and the gate refused it: `runtime-interface-only` inherits the migration-prescription refusal, and this body ships a `## Migration` table. The truthful disposition is `registered`. The ledger already carries two entries of exactly this class on this package, landed four days before this branch — `client-delete-result-success` and `client-meta-reset-result-reset`, both `@objectstack/client` return-shape moves, both "same call, same wire body", both pure `unwrapResponse` passthroughs with no `packages/spec` schema movement. This one is the stronger case of the three: those corrected declarations that were UNINHABITED, so they revealed a defect rather than breaking working code, whereas this moves reads that work today. - `entries/semantic/18.client-envelope-convergence-analytics-automation.ts` — one `SemanticMigration` for major 18, the open window. surface: the resolved value of `client.analytics.query` / `.meta` / `.explain` and `client.automation.trigger`, from the dispatcher's `{ success, data }` envelope to `data`. replacement: the payload, `r.data.X` -> `r.X`, the four rows of this branch's own `## Migration` table. reason: one SDK calling convention, and the wire is byte-identical — this branch's diff touches no `packages/spec` path at all (`git diff --name-only HEAD -- packages/spec` is empty). `tsc` names every site on the three analytics methods (TS2339), but NOT on `automation.trigger`: `AutomationResult` itself declares `success: boolean` and `error?: string` (`packages/spec/src/contracts/automation-service.ts`, byte-identical at the merge base and here), so `r.success` / `r.error` compile on both sides while their subject moves from the envelope to the run. An untyped JS caller has no channel at all, which is why the ledger entry is the only notification that reaches it. acceptanceCriteria: no code reads `.data` off the four, and every branch on `r.success` / `r.error` off `trigger` is re-read by hand. - `migrations/registry.ts` — regenerated by `gen:migration-registry` (144 semantic, 75 retired-key, 88 retired-def); never hand-edited. - The changeset's ADR-0087 marker moves to `registered client-envelope-convergence-analytics-automation`; the rest of the file is byte-identical (one line changed). `gen:spec-changes` and `gen:upgrade-guide` were re-run and write no diff: `PROTOCOL_VERSION` is 17.0.0, so both artifacts project majors up to 17 and neither carries any step-18 entry yet — the 2026-08-29 precedent `client-meta-reset-result-reset` appears zero times in either file too. Issue anchors live in a source comment at the top of the entry rather than in its strings: the strings are projected into `spec-changes.json` and `docs/protocol-upgrade-guide.md` when 18 becomes current, and this repo's issue numbers do not resolve for the consumers who read those. Registration requested on #14996; the landing PR is #14526, which stays the `domain:cli` seat's to land. Claude-Session: https://claude.ai/code/session_0174WZTU6XcFcS7g2kykC53i Co-authored-by: Claude --- .changeset/client-envelope-convergence.md | 2 +- ...velope-convergence-analytics-automation.ts | 100 ++++++++++++++++++ packages/spec/src/migrations/registry.ts | 78 ++++++++++++++ 3 files changed, 179 insertions(+), 1 deletion(-) create mode 100644 packages/spec/src/migrations/entries/semantic/18.client-envelope-convergence-analytics-automation.ts diff --git a/.changeset/client-envelope-convergence.md b/.changeset/client-envelope-convergence.md index 54757066e8..3ce53e09d0 100644 --- a/.changeset/client-envelope-convergence.md +++ b/.changeset/client-envelope-convergence.md @@ -4,7 +4,7 @@ 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. diff --git a/packages/spec/src/migrations/entries/semantic/18.client-envelope-convergence-analytics-automation.ts b/packages/spec/src/migrations/entries/semantic/18.client-envelope-convergence-analytics-automation.ts new file mode 100644 index 0000000000..2be47052fb --- /dev/null +++ b/packages/spec/src/migrations/entries/semantic/18.client-envelope-convergence-analytics-automation.ts @@ -0,0 +1,100 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +// Anchors for this entry, kept in source rather than in the strings below: the +// entry's prose is projected into `packages/spec/spec-changes.json` and +// `docs/protocol-upgrade-guide.md`, which are read by consumers who cannot +// resolve this repo's internal issue numbers. +// +// card objectstack-ai/objectstack#13079 +// landing PR objectstack-ai/objectstack#14526 +// registration objectstack-ai/objectstack#14996 +// precedents objectstack-ai/objectstack#13023 (the two entries this is +// shaped on: `client-delete-result-success`, +// `client-meta-reset-result-reset`) +// consumer objectstack-ai/objectui#7028 (the one measured production +// call site, tightened after this lands) +// census `packages/client/src/envelope-caller-census.test.ts` +// contract `AutomationResult` in +// `packages/spec/src/contracts/automation-service.ts` +// (`success: boolean`, `error?: string`) + +import type { SemanticMigration } from '../../types.js'; + +export const entry: SemanticMigration = { + id: 'client-envelope-convergence-analytics-automation', + surface: + 'client.analytics.query / client.analytics.meta / client.analytics.explain / ' + + 'client.automation.trigger (the resolved value of four published ' + + '`@objectstack/client` methods — the runtime dispatcher\'s ' + + '`{ success, data }` envelope before, its `data` member after)', + replacement: + 'the payload — `r.data.X` → `r.X` on all four. `client.analytics.query(q)` now ' + + 'resolves to `AnalyticsResult` (`r.data.rows` → `r.rows`); ' + + '`client.analytics.meta(cube?)` to `AnalyticsMetadataResponse[\'data\']`, the bare ' + + 'cube list (`r.data[0].name` → `r[0].name`); `client.analytics.explain(q)` to ' + + '`AnalyticsSqlResponse[\'data\']`, `{ sql, params }` (`r.data.sql` → `r.sql`); ' + + '`client.automation.trigger(name, payload)` to `AutomationResult` ' + + '(`r.data.status` → `r.status`, `r.data.runId` → `r.runId`) — the same value ' + + '`client.automation.execute` already answered for the same handler. Same call, ' + + 'same wire body, one SDK calling convention', + reason: + '`ObjectStackClient` had two response readers. `unwrapResponse` strips the runtime ' + + 'dispatcher\'s `{ success, data }` envelope and hands back `data`; every other ' + + 'dispatcher-served method already used it, and these four alone ended ' + + '`return res.json()`, so their callers alone had to read `.data`. All four now end ' + + '`return this.unwrapResponse(res)` and their return declarations are the payload ' + + 'types. THE WIRE IS BYTE-IDENTICAL: every route answers exactly the body it ' + + 'answered before, no Zod schema moves, no `packages/spec` declaration moves, no ' + + 'authorable key and no stored representation is involved — the landing diff ' + + 'touches no `packages/spec` path at all — so a raw-HTTP caller is unaffected and ' + + '`objectstack migrate meta` has nothing to rewrite. This is registered rather than ' + + 'exempted because the change is NOT wholly compiler-delivered, and the gap is ' + + 'exact rather than theoretical. For the three analytics methods it is: every old ' + + 'read is `error TS2339: Property \'data\' does not exist on type …`, so tsc names ' + + 'each site. `client.automation.trigger` is the exception — `AutomationResult` ' + + 'itself declares `success: boolean` and `error?: string` ' + + '(`AutomationResult` in `packages/spec/src/contracts/automation-service.ts`, ' + + 'byte-identical at the merge base and at this landing), so `r.success` and ' + + '`r.error` COMPILE ON BOTH SIDES while 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, and a refusal the door does not ' + + 'classify as 400 / 409 / 422 is answered 200 carrying `success: false` with ' + + '`error` set. A consumer branching on either reads a DIFFERENT QUESTION at the ' + + 'same spelling, with no diagnostic anywhere. And there is no authored source for ' + + 'the conversion chain to rewrite: this is a published TypeScript surface whose ' + + 'enforced channel is tsc at the call site, and for an untyped JS caller there is ' + + 'no constrained channel at all — `.data` simply reads `undefined` — which is why ' + + 'the ledger entry is the only notification that reaches them. That is the same ' + + 'argument the two sibling entries on this package make ' + + '(`client-delete-result-success`, `client-meta-reset-result-reset`), and this one ' + + 'is the stronger case of the three: those corrected declarations that were ' + + 'UNINHABITED, revealing a defect rather than breaking working code, whereas this ' + + 'moves reads that work today. ⛔ Do not write `r.rows ?? r.data.rows`: there is ' + + 'one producer shape, and a consumer accepting two spellings is what contract-first ' + + 'exists to prevent. The failure path is unchanged and deliberately so — ' + + '`ObjectStackClient.fetch` rejects on every non-2xx BEFORE either reader runs, ' + + 'carrying the ADR-0112 error envelope, and `unwrapResponse` itself never throws. ' + + 'ADR-0087 D3.', + acceptanceCriteria: + 'No code reads `.data` off a `client.analytics.query()`, `client.analytics.meta()`, ' + + '`client.analytics.explain()` or `client.automation.trigger()` result. For the ' + + 'three analytics methods `tsc` names every site for a typed caller (TS2339); an ' + + 'untyped JS caller must be swept by hand for the four spellings, because nothing ' + + 'will report it. ⚠️ `client.automation.trigger` needs the hand sweep even WITH a ' + + 'type-checker: every branch on `r.success` or `r.error` off `trigger` has to be ' + + 're-read one by one, because both compile before and after while their subject ' + + 'moved from the envelope to the run. A branch that treated `r.success` as "the ' + + 'call was accepted" now asks "the run succeeded" — the two differ on exactly the ' + + '200-answered refusals — and a `catch`-only error path now has a resolved ' + + '`success: false` sibling it never had to consider. Nothing about the request, the ' + + 'route, the status codes or the thrown error shapes changes, and no server needs ' + + 'upgrading: the value you may now read is the one that was already arriving, one ' + + 'level in. `client.analytics.queryDataset` is NOT part of this move — it is served ' + + 'with no envelope at all and resolved to the bare payload before and after. ' + + 'Populations: in the ObjectStack repo, zero production call sites and the loud ' + + 'test pins that ship with this change; in objectui, one production row-extraction ' + + 'chain that tolerates both spellings today and is tightened to the post-unwrap ' + + 'spelling once this lands; `objectstack-ai/cloud` is NOT MEASURED — a `.data` read ' + + 'on any of the four there is a runtime break after this change, and this entry is ' + + 'the only notice it gets.', +}; diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index a92f299b91..9b26bad797 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -5670,6 +5670,84 @@ const step18: MigrationStep = { + 'never resolved commands from this declaration — commands are ' + 'oclif-auto-discovered, before and after.', }, + { + id: 'client-envelope-convergence-analytics-automation', + surface: + 'client.analytics.query / client.analytics.meta / client.analytics.explain / ' + + 'client.automation.trigger (the resolved value of four published ' + + '`@objectstack/client` methods — the runtime dispatcher\'s ' + + '`{ success, data }` envelope before, its `data` member after)', + replacement: + 'the payload — `r.data.X` → `r.X` on all four. `client.analytics.query(q)` now ' + + 'resolves to `AnalyticsResult` (`r.data.rows` → `r.rows`); ' + + '`client.analytics.meta(cube?)` to `AnalyticsMetadataResponse[\'data\']`, the bare ' + + 'cube list (`r.data[0].name` → `r[0].name`); `client.analytics.explain(q)` to ' + + '`AnalyticsSqlResponse[\'data\']`, `{ sql, params }` (`r.data.sql` → `r.sql`); ' + + '`client.automation.trigger(name, payload)` to `AutomationResult` ' + + '(`r.data.status` → `r.status`, `r.data.runId` → `r.runId`) — the same value ' + + '`client.automation.execute` already answered for the same handler. Same call, ' + + 'same wire body, one SDK calling convention', + reason: + '`ObjectStackClient` had two response readers. `unwrapResponse` strips the runtime ' + + 'dispatcher\'s `{ success, data }` envelope and hands back `data`; every other ' + + 'dispatcher-served method already used it, and these four alone ended ' + + '`return res.json()`, so their callers alone had to read `.data`. All four now end ' + + '`return this.unwrapResponse(res)` and their return declarations are the payload ' + + 'types. THE WIRE IS BYTE-IDENTICAL: every route answers exactly the body it ' + + 'answered before, no Zod schema moves, no `packages/spec` declaration moves, no ' + + 'authorable key and no stored representation is involved — the landing diff ' + + 'touches no `packages/spec` path at all — so a raw-HTTP caller is unaffected and ' + + '`objectstack migrate meta` has nothing to rewrite. This is registered rather than ' + + 'exempted because the change is NOT wholly compiler-delivered, and the gap is ' + + 'exact rather than theoretical. For the three analytics methods it is: every old ' + + 'read is `error TS2339: Property \'data\' does not exist on type …`, so tsc names ' + + 'each site. `client.automation.trigger` is the exception — `AutomationResult` ' + + 'itself declares `success: boolean` and `error?: string` ' + + '(`AutomationResult` in `packages/spec/src/contracts/automation-service.ts`, ' + + 'byte-identical at the merge base and at this landing), so `r.success` and ' + + '`r.error` COMPILE ON BOTH SIDES while 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, and a refusal the door does not ' + + 'classify as 400 / 409 / 422 is answered 200 carrying `success: false` with ' + + '`error` set. A consumer branching on either reads a DIFFERENT QUESTION at the ' + + 'same spelling, with no diagnostic anywhere. And there is no authored source for ' + + 'the conversion chain to rewrite: this is a published TypeScript surface whose ' + + 'enforced channel is tsc at the call site, and for an untyped JS caller there is ' + + 'no constrained channel at all — `.data` simply reads `undefined` — which is why ' + + 'the ledger entry is the only notification that reaches them. That is the same ' + + 'argument the two sibling entries on this package make ' + + '(`client-delete-result-success`, `client-meta-reset-result-reset`), and this one ' + + 'is the stronger case of the three: those corrected declarations that were ' + + 'UNINHABITED, revealing a defect rather than breaking working code, whereas this ' + + 'moves reads that work today. ⛔ Do not write `r.rows ?? r.data.rows`: there is ' + + 'one producer shape, and a consumer accepting two spellings is what contract-first ' + + 'exists to prevent. The failure path is unchanged and deliberately so — ' + + '`ObjectStackClient.fetch` rejects on every non-2xx BEFORE either reader runs, ' + + 'carrying the ADR-0112 error envelope, and `unwrapResponse` itself never throws. ' + + 'ADR-0087 D3.', + acceptanceCriteria: + 'No code reads `.data` off a `client.analytics.query()`, `client.analytics.meta()`, ' + + '`client.analytics.explain()` or `client.automation.trigger()` result. For the ' + + 'three analytics methods `tsc` names every site for a typed caller (TS2339); an ' + + 'untyped JS caller must be swept by hand for the four spellings, because nothing ' + + 'will report it. ⚠️ `client.automation.trigger` needs the hand sweep even WITH a ' + + 'type-checker: every branch on `r.success` or `r.error` off `trigger` has to be ' + + 're-read one by one, because both compile before and after while their subject ' + + 'moved from the envelope to the run. A branch that treated `r.success` as "the ' + + 'call was accepted" now asks "the run succeeded" — the two differ on exactly the ' + + '200-answered refusals — and a `catch`-only error path now has a resolved ' + + '`success: false` sibling it never had to consider. Nothing about the request, the ' + + 'route, the status codes or the thrown error shapes changes, and no server needs ' + + 'upgrading: the value you may now read is the one that was already arriving, one ' + + 'level in. `client.analytics.queryDataset` is NOT part of this move — it is served ' + + 'with no envelope at all and resolved to the bare payload before and after. ' + + 'Populations: in the ObjectStack repo, zero production call sites and the loud ' + + 'test pins that ship with this change; in objectui, one production row-extraction ' + + 'chain that tolerates both spellings today and is tightened to the post-unwrap ' + + 'spelling once this lands; `objectstack-ai/cloud` is NOT MEASURED — a `.data` read ' + + 'on any of the four there is a runtime break after this change, and this entry is ' + + 'the only notice it gets.', + }, { id: 'client-meta-reset-result-reset', surface: From 5dc1af239ea89af464e9764243936b3fee585497 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 02:15:55 +0000 Subject: [PATCH 7/7] fix(spec): respell the four methods in the ADR-0087 entry so the #13079 census does not count its prose (#13079) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `packages/client/src/envelope-caller-census.test.ts` (§3, this branch's own ratchet) walks the whole workspace for `analytics.query|meta|explain (` and `automation.trigger (` — over comment-masked source, so a docblock is exempt but a STRING LITERAL is not — and requires every hit to be classified in its hand ledger. The entry added in 24f2507c wrote the four methods in their call-site spelling inside its `surface` / `replacement` / `acceptanceCriteria` prose, so the census counted 9 sites in the entry file and the same 9 in the generated `registry.ts`: `sdkSites.length` 46 instead of 28, 18 unclassified, `Test Core (1/6)` red on 24f2507c while the same shard was green on d543e098. Fixed inside this registration's own surface only. Every mention of the four is respelled so no `(` follows the method name, whitespace included: - surface — `client.automation.trigger (the resolved value …)` becomes an em dash and a colon, no parenthesis and no closing one - replacement — `client.analytics.query(q)` / `.meta(cube?)` / `.explain(q)` / `client.automation.trigger(name, payload)` now name their arguments in words ("called with a query", "with or without a cube name", "given a trigger name and a payload"), and the four rewrite rows keep their `r.data.X` → `r.X` spelling exactly, which the census does not match - acceptanceCriteria — the four `client.…()` forms lose their empty parens ⛔ The census test itself is untouched: it is the `domain:cli` seat's file, and the fix belongs on the side that introduced the text. Verified with the census's own matcher (its regex, over the tree's own `maskComments`): the entry file and `registry.ts` now yield 0 hits each, down from 9 and 9. `pnpm --filter @objectstack/client exec vitest run src/envelope-caller-census.test.ts` — 14 passed, including `sdkSites.length === 28`, the mechanical-enumeration-vs-hand-ledger equality and the zero-unclassified assertion. `registry.ts` regenerated by `gen:migration-registry`; no other file touched, and the entry's meaning is unchanged. Claude-Session: https://claude.ai/code/session_0174WZTU6XcFcS7g2kykC53i Co-authored-by: Claude --- ...velope-convergence-analytics-automation.ts | 26 ++++++++++--------- packages/spec/src/migrations/registry.ts | 26 ++++++++++--------- 2 files changed, 28 insertions(+), 24 deletions(-) diff --git a/packages/spec/src/migrations/entries/semantic/18.client-envelope-convergence-analytics-automation.ts b/packages/spec/src/migrations/entries/semantic/18.client-envelope-convergence-analytics-automation.ts index 2be47052fb..5d3b014d92 100644 --- a/packages/spec/src/migrations/entries/semantic/18.client-envelope-convergence-analytics-automation.ts +++ b/packages/spec/src/migrations/entries/semantic/18.client-envelope-convergence-analytics-automation.ts @@ -24,17 +24,19 @@ export const entry: SemanticMigration = { id: 'client-envelope-convergence-analytics-automation', surface: 'client.analytics.query / client.analytics.meta / client.analytics.explain / ' - + 'client.automation.trigger (the resolved value of four published ' - + '`@objectstack/client` methods — the runtime dispatcher\'s ' - + '`{ success, data }` envelope before, its `data` member after)', + + 'client.automation.trigger — the resolved value of four published ' + + '`@objectstack/client` methods: the runtime dispatcher\'s ' + + '`{ success, data }` envelope before, its `data` member after', replacement: - 'the payload — `r.data.X` → `r.X` on all four. `client.analytics.query(q)` now ' - + 'resolves to `AnalyticsResult` (`r.data.rows` → `r.rows`); ' - + '`client.analytics.meta(cube?)` to `AnalyticsMetadataResponse[\'data\']`, the bare ' - + 'cube list (`r.data[0].name` → `r[0].name`); `client.analytics.explain(q)` to ' - + '`AnalyticsSqlResponse[\'data\']`, `{ sql, params }` (`r.data.sql` → `r.sql`); ' - + '`client.automation.trigger(name, payload)` to `AutomationResult` ' - + '(`r.data.status` → `r.status`, `r.data.runId` → `r.runId`) — the same value ' + 'the payload — `r.data.X` → `r.X` on all four. `client.analytics.query`, called ' + + 'with a query, now resolves to `AnalyticsResult`: `r.data.rows` → `r.rows`. ' + + '`client.analytics.meta`, with or without a cube name, resolves to ' + + '`AnalyticsMetadataResponse[\'data\']`, the bare cube list: `r.data[0].name` → ' + + '`r[0].name`. `client.analytics.explain` resolves to ' + + '`AnalyticsSqlResponse[\'data\']`, `{ sql, params }`: `r.data.sql` → `r.sql`. ' + + '`client.automation.trigger`, given a trigger name and a payload, resolves to ' + + '`AutomationResult`: `r.data.status` → `r.status` and `r.data.runId` → ' + + '`r.runId` — the same value ' + '`client.automation.execute` already answered for the same handler. Same call, ' + 'same wire body, one SDK calling convention', reason: @@ -76,8 +78,8 @@ export const entry: SemanticMigration = { + 'carrying the ADR-0112 error envelope, and `unwrapResponse` itself never throws. ' + 'ADR-0087 D3.', acceptanceCriteria: - 'No code reads `.data` off a `client.analytics.query()`, `client.analytics.meta()`, ' - + '`client.analytics.explain()` or `client.automation.trigger()` result. For the ' + 'No code reads `.data` off a `client.analytics.query`, `client.analytics.meta`, ' + + '`client.analytics.explain` or `client.automation.trigger` result. For the ' + 'three analytics methods `tsc` names every site for a typed caller (TS2339); an ' + 'untyped JS caller must be swept by hand for the four spellings, because nothing ' + 'will report it. ⚠️ `client.automation.trigger` needs the hand sweep even WITH a ' diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index 9b26bad797..2973c27dd1 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -5674,17 +5674,19 @@ const step18: MigrationStep = { id: 'client-envelope-convergence-analytics-automation', surface: 'client.analytics.query / client.analytics.meta / client.analytics.explain / ' - + 'client.automation.trigger (the resolved value of four published ' - + '`@objectstack/client` methods — the runtime dispatcher\'s ' - + '`{ success, data }` envelope before, its `data` member after)', + + 'client.automation.trigger — the resolved value of four published ' + + '`@objectstack/client` methods: the runtime dispatcher\'s ' + + '`{ success, data }` envelope before, its `data` member after', replacement: - 'the payload — `r.data.X` → `r.X` on all four. `client.analytics.query(q)` now ' - + 'resolves to `AnalyticsResult` (`r.data.rows` → `r.rows`); ' - + '`client.analytics.meta(cube?)` to `AnalyticsMetadataResponse[\'data\']`, the bare ' - + 'cube list (`r.data[0].name` → `r[0].name`); `client.analytics.explain(q)` to ' - + '`AnalyticsSqlResponse[\'data\']`, `{ sql, params }` (`r.data.sql` → `r.sql`); ' - + '`client.automation.trigger(name, payload)` to `AutomationResult` ' - + '(`r.data.status` → `r.status`, `r.data.runId` → `r.runId`) — the same value ' + 'the payload — `r.data.X` → `r.X` on all four. `client.analytics.query`, called ' + + 'with a query, now resolves to `AnalyticsResult`: `r.data.rows` → `r.rows`. ' + + '`client.analytics.meta`, with or without a cube name, resolves to ' + + '`AnalyticsMetadataResponse[\'data\']`, the bare cube list: `r.data[0].name` → ' + + '`r[0].name`. `client.analytics.explain` resolves to ' + + '`AnalyticsSqlResponse[\'data\']`, `{ sql, params }`: `r.data.sql` → `r.sql`. ' + + '`client.automation.trigger`, given a trigger name and a payload, resolves to ' + + '`AutomationResult`: `r.data.status` → `r.status` and `r.data.runId` → ' + + '`r.runId` — the same value ' + '`client.automation.execute` already answered for the same handler. Same call, ' + 'same wire body, one SDK calling convention', reason: @@ -5726,8 +5728,8 @@ const step18: MigrationStep = { + 'carrying the ADR-0112 error envelope, and `unwrapResponse` itself never throws. ' + 'ADR-0087 D3.', acceptanceCriteria: - 'No code reads `.data` off a `client.analytics.query()`, `client.analytics.meta()`, ' - + '`client.analytics.explain()` or `client.automation.trigger()` result. For the ' + 'No code reads `.data` off a `client.analytics.query`, `client.analytics.meta`, ' + + '`client.analytics.explain` or `client.automation.trigger` result. For the ' + 'three analytics methods `tsc` names every site for a typed caller (TS2339); an ' + 'untyped JS caller must be swept by hand for the four spellings, because nothing ' + 'will report it. ⚠️ `client.automation.trigger` needs the hand sweep even WITH a '