diff --git a/.changeset/client-json-erasure-in-repo-families.md b/.changeset/client-json-erasure-in-repo-families.md new file mode 100644 index 0000000000..66ae223315 --- /dev/null +++ b/.changeset/client-json-erasure-in-repo-families.md @@ -0,0 +1,72 @@ +--- +"@objectstack/client": minor +--- + +fix(client): bind the five in-repo `return res.json()` methods, whose published type was `Promise< any >` (part of #12104) + +**Return-type narrowing on a published SDK (clause-②).** No runtime change: the +value each method resolves to is byte-identical before and after. Only the +DECLARED type moved, off `any` — which is exactly why a runtime test cannot +observe it and the pins for it are type-level. + +> ⓘ Angle brackets are spaced throughout (`Promise< any >`) on purpose — +> GitHub's body sanitizer strips tag-shaped spans, backticks and fenced code +> included. + +Each of the five carried no return annotation and ended `return res.json()`, so +its published type was `Promise< any >`, inherited from `lib.dom`'s +`Response.json(): Promise< any >`. The method text names neither `any` nor +`Promise` nor `unwrapResponse`, which is why the class was invisible to the +greps two earlier censuses used. + +## What each method declares now + +| method | declared before | declares now | why | +|---|---|---|---| +| `client.analytics.query` | `any` | `BaseResponse & { data: AnalyticsResult }` | dispatcher-served; `deps.success(v)` wraps and `res.json()` strips nothing | +| `client.analytics.meta` | `any` | `AnalyticsMetadataResponse` | same envelope; `data` is the bare `CubeMeta[]` projection | +| `client.analytics.explain` | `any` | `AnalyticsSqlResponse` | same envelope; `data` is `{ sql, params }` | +| `client.automation.trigger` | `any` | `BaseResponse & { data: AutomationResult }` | same envelope, over the payload its sibling `automation.execute` unwraps | +| `client.analytics.queryDataset` | `any` | `AnalyticsResult` | served by `@objectstack/rest`, which ends `res.json(result)` — no envelope | + +`any` is assignable to everything and admits every property read, so a +consumer's code can stop compiling where it previously did not. Concretely: + +- **Reading a payload key off one of the four ENVELOPED results.** + `(await client.analytics.query(q)).rows` compiled and was `undefined` at + runtime; the read the wire always required is `.data.rows`. Same for + `.data` on `meta` / `explain`, and `.data.runId` / `.data.screen` on + `automation.trigger`. +- **Reading `.data` off `queryDataset`**, which is served bare — likewise + `undefined` today, likewise refused now. +- Assigning any of the five results to an unrelated annotation, or forwarding + one to a differently-typed parameter. + +That break is the point: those call sites are already wrong at runtime and the +`any` is what hid it. The compiler is the channel that reaches every affected +consumer, and it is strictly more precise than a release note. + +## How the shapes were established + +By DRIVING the real producers — a real `AnalyticsService`, a real +`AutomationEngine`, the real `HttpDispatcher` and the real `RestServer`, with +only the socket stood in for — not by reading source and not by asserting +against a mock. Two spec response types that look like the right binding are +NARROWER than the contract their route relays +(`AnalyticsResultResponseSchema.data.fields` and +`TriggerFlowResponseSchema.data`), so those two annotations bind the producer's +contract instead; the near-miss is pinned so a later sweep cannot retarget them. + +## Scope + +The five families whose producers live in this repo. The 38 better-auth-backed +`auth.*` / `organizations.*` / `oauth.*` methods of the same class are untouched +and keep their erased `any` — they are exactly as permissive as before, and no +consumer loses anything by that. + +No ADR-0087 ledger entry: nothing here is a metadata surface. No Zod schema, no +`packages/spec` declaration and no stored representation changed — the erasure +lived only in a TypeScript return annotation — so `objectstack migrate meta` has +nothing to rewrite and an entry would have no artifact to project into. This is +the disposition #8140, #11925 and #12034 recorded for the same class of SDK +return-type narrowing. diff --git a/packages/client/exported-any-returns.json b/packages/client/exported-any-returns.json index 5d3ea7d5e2..8f6af1e0e4 100644 --- a/packages/client/exported-any-returns.json +++ b/packages/client/exported-any-returns.json @@ -2,10 +2,6 @@ "$comment": "Exported callables of @objectstack/client whose AWAITED return type resolves to `any` (#11927). Judged against the BUILT dist by `pnpm --filter @objectstack/client check:exported-any-returns`, because the erasure is invisible in source text when a method carries no return annotation. SHRINK-ONLY and EXACT in both directions: a site here that no longer resolves to `any` is RED until its entry is deleted, and a site NOT here that resolves to `any` is RED — that unlisted case is the everyday one and the reason this file exists. There is deliberately NO --update flag: every entry is debt with a name on it, and a reason a tool wrote is a silencer rather than a worklist. SCOPE, and the one exclusion worth stating out loud: a return type that CONTAINS `any` (`{ packages: any[]; total: number }`, `Promise>`) is not listed, because it is not flagged — the gate asks whether the type IS `any`, the same line packages/spec's check:exported-any draws, and admitting the broader question costs the gate its zero-false-positive property. That is why 21 of #11925's 38 unannotated methods are absent here: they are `any`-CONTAINING, and they remain #11925's to close. Nothing is silently absorbed in either direction. A caller-supplied `` is likewise never listed: the record type and the action payload really are the caller's, and flagging them is the pressure that turns a correct generic into a wrong concrete type.", "entries": { "ObjectStackClient.meta.migrateStored": "#11925 — no return annotation; the published type comes from `this.unwrapResponse(res)`. Invisible to a `Promise<` grep because the text never appears in the method. Bind the contract the route actually answers, minding the envelope.", - "ObjectStackClient.analytics.query": "#12104 — no return annotation; `return res.json()`, and lib.dom declares `Response.json(): Promise`. Invisible to every grep #8140's census and #11925 used: the method names neither `any` nor `Promise` nor `unwrapResponse`. Bind the contract the route actually answers, minding the envelope.", - "ObjectStackClient.analytics.meta": "#12104 — no return annotation; `return res.json()`, and lib.dom declares `Response.json(): Promise`. Invisible to every grep #8140's census and #11925 used: the method names neither `any` nor `Promise` nor `unwrapResponse`. Bind the contract the route actually answers, minding the envelope.", - "ObjectStackClient.analytics.explain": "#12104 — no return annotation; `return res.json()`, and lib.dom declares `Response.json(): Promise`. Invisible to every grep #8140's census and #11925 used: the method names neither `any` nor `Promise` nor `unwrapResponse`. Bind the contract the route actually answers, minding the envelope.", - "ObjectStackClient.analytics.queryDataset": "#12104 — no return annotation; `return res.json()`, and lib.dom declares `Response.json(): Promise`. Invisible to every grep #8140's census and #11925 used: the method names neither `any` nor `Promise` nor `unwrapResponse`. Bind the contract the route actually answers, minding the envelope.", "ObjectStackClient.organizations.create": "#12104 — no return annotation; `return res.json()`, and lib.dom declares `Response.json(): Promise`. Invisible to every grep #8140's census and #11925 used: the method names neither `any` nor `Promise` nor `unwrapResponse`. Bind the contract the route actually answers, minding the envelope.", "ObjectStackClient.organizations.update": "#12104 — no return annotation; `return res.json()`, and lib.dom declares `Response.json(): Promise`. Invisible to every grep #8140's census and #11925 used: the method names neither `any` nor `Promise` nor `unwrapResponse`. Bind the contract the route actually answers, minding the envelope.", "ObjectStackClient.organizations.setActive": "#12104 — no return annotation; `return res.json()`, and lib.dom declares `Response.json(): Promise`. Invisible to every grep #8140's census and #11925 used: the method names neither `any` nor `Promise` nor `unwrapResponse`. Bind the contract the route actually answers, minding the envelope.", @@ -45,7 +41,6 @@ "ObjectStackClient.auth.twoFactor.disable": "#12104 — no return annotation; `return res.json()`, and lib.dom declares `Response.json(): Promise`. Invisible to every grep #8140's census and #11925 used: the method names neither `any` nor `Promise` nor `unwrapResponse`. Bind the contract the route actually answers, minding the envelope.", "ObjectStackClient.auth.twoFactor.verifyBackupCode": "#12104 — no return annotation; `return res.json()`, and lib.dom declares `Response.json(): Promise`. Invisible to every grep #8140's census and #11925 used: the method names neither `any` nor `Promise` nor `unwrapResponse`. Bind the contract the route actually answers, minding the envelope.", "ObjectStackClient.auth.accounts.unlink": "#12104 — no return annotation; `return res.json()`, and lib.dom declares `Response.json(): Promise`. Invisible to every grep #8140's census and #11925 used: the method names neither `any` nor `Promise` nor `unwrapResponse`. Bind the contract the route actually answers, minding the envelope.", - "ObjectStackClient.automation.trigger": "#12104 — no return annotation; `return res.json()`, and lib.dom declares `Response.json(): Promise`. Invisible to every grep #8140's census and #11925 used: the method names neither `any` nor `Promise` nor `unwrapResponse`. Bind the contract the route actually answers, minding the envelope.", "ObjectStackClient.automation.create": "#11924 — DELIBERATE `Promise`: `POST /automation` ends `deps.success(body)`, echoing the caller's own unvalidated bytes, and `IAutomationService.registerFlow` returns nothing, so the service contract has no return shape to relay. This needs a DECISION (keep echoing, or answer the registered `FlowParsed`), not an annotation.", "ObjectStackClient.automation.update": "#11924 — DELIBERATE `Promise`: `PUT /automation/:name` ends `deps.success(definition)` where `definition = body.definition ?? body`. Same missing contract as `automation.create`, and the two should be answered together since they are one route class." } diff --git a/packages/client/package.json b/packages/client/package.json index 5764b160cd..0aa6222e87 100644 --- a/packages/client/package.json +++ b/packages/client/package.json @@ -33,7 +33,10 @@ "@objectstack/metadata-protocol": "workspace:*", "@objectstack/objectql": "workspace:*", "@objectstack/plugin-hono-server": "workspace:*", + "@objectstack/rest": "workspace:*", "@objectstack/runtime": "workspace:*", + "@objectstack/service-analytics": "workspace:*", + "@objectstack/service-automation": "workspace:*", "tsx": "^4.23.12", "typescript": "^6.0.3", "vitest": "^4.1.10" diff --git a/packages/client/src/analytics-automation-json-erasure.test.ts b/packages/client/src/analytics-automation-json-erasure.test.ts new file mode 100644 index 0000000000..c97c34109c --- /dev/null +++ b/packages/client/src/analytics-automation-json-erasure.test.ts @@ -0,0 +1,417 @@ +// 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. + * + * ## The erasure these five carried + * + * Each method carried no return annotation and never named `any`, `Promise` or + * `unwrapResponse` — so no grep could see it — while its published type was + * `Promise< any >`, inherited from `lib.dom`'s `Response.json(): Promise< any >`. + * + * ## The half a wire test can prove, and the half it cannot + * + * `return-type-precision.test.ts`'s header states the rule and it is why this + * file exists as the second half of the pair: + * + * - a runtime test cannot observe a return-type narrowing at all — the value + * is identical whatever the declaration says. The DECLARATION half is + * pinned type-level in `return-type-precision.test.ts`. + * - a type test cannot observe whether the declaration is TRUE. That is this + * file's job, and it is why nothing here mocks a response body: a mock body + * would assert my own assumption about the producer, which is the mistake + * that produces a bound-but-false declaration. + * + * So every chain below is real end to end — the real `AnalyticsService` + * (`@objectstack/service-analytics`), the real `AutomationEngine` + * (`@objectstack/service-automation`), the real `HttpDispatcher` + * (`@objectstack/runtime`) and the real `RestServer` (`@objectstack/rest`) + * answering them, and the real `ObjectStackClient` reading the result. The only + * stand-in is the socket: `fetch` hands the request to the producer in-process + * instead of over TCP, and hands back the producer's own body untouched. + * + * ## 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: + * + * - `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`. + * - `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. + * + * 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. + * + * ## Two spec response schemas are NARROWER than their producer — measured here + * + * `AnalyticsResultResponseSchema.data` and `TriggerFlowResponseSchema.data` are + * stale projections of `AnalyticsResult` / `AutomationResult`: the fixtures + * below serve keys those schemas do not declare (`fields[].label`, and a paused + * run's `status` / `runId` / `screen`). That is why the two annotations bind the + * PRODUCER's contract type rather than those two response types — binding them + * would have been a false narrowing of exactly the kind #12034 removed. The + * other two route schemas (`AnalyticsMetadataResponseSchema`, + * `AnalyticsSqlResponseSchema`) DO agree with their producer's declared return, + * and the annotations use them. + * + * --------------------------------------------------------------------------- + * Reverse verification, direction predicted BEFORE running + * --------------------------------------------------------------------------- + * 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. + */ + +import { describe, it, expect, vi } from 'vitest'; +import type { Cube } from '@objectstack/spec/data'; +import type { Logger } from '@objectstack/spec/contracts'; +import { defineActionDescriptor } from '@objectstack/spec/automation'; +import { AnalyticsService } from '@objectstack/service-analytics'; +import { AutomationEngine, InMemorySuspendedRunStore } from '@objectstack/service-automation'; +import { HttpDispatcher } from '@objectstack/runtime'; +import { RestServer } from '@objectstack/rest'; +import { ObjectStackClient } from './index'; + +const BASE_URL = 'http://localhost:3000'; + +const silent: Logger = { debug() {}, info() {}, warn() {}, error() {} }; + +const CONTEXT = (): any => ({ + request: {}, + executionContext: { userId: 'usr_1', isSystem: false, systemPermissions: [] }, +}); + +// ───────────────────────────────────────────────────────────────────────────── +// analytics — a REAL AnalyticsService on the native-SQL path +// ───────────────────────────────────────────────────────────────────────────── + +/** + * One cube with a LABELLED measure and dimension. The labels are load-bearing + * rather than decorative: `AnalyticsResult.fields[].label` is a key + * `AnalyticsResultResponseSchema.data.fields` does not declare, so serving it + * is what proves that schema is a narrower projection than the producer's + * contract — the measurement the annotation choice rests on. + */ +const ACCOUNT_CUBE: Cube = { + name: 'crm_account', + title: 'Accounts', + sql: 'crm_account', + measures: { + account_count: { name: 'account_count', label: 'Account count', type: 'count', sql: '*' }, + }, + dimensions: { + industry: { name: 'industry', label: 'Industry', type: 'string', sql: 'industry' }, + }, +}; + +const ROWS = [{ industry: 'tech', account_count: 3 }]; + +/** + * The ADR-0021 dataset the REST-served `queryDataset` route runs. Its dimension + * carries a `label` on purpose: the dataset executor enriches + * `AnalyticsResult.fields[].label` from it, which is the key + * `AnalyticsResultResponseSchema.data.fields` does not declare. + */ +const DATASET = { + name: 'account_metrics', + label: 'Account metrics', + object: 'crm_account', + dimensions: [{ name: 'industry', label: 'Industry', field: 'industry', type: 'string' }], + measures: [{ name: 'account_count', label: 'Account count', aggregate: 'count' }], +}; + +function realAnalytics(): AnalyticsService { + return new AnalyticsService({ + cubes: [ACCOUNT_CUBE], + logger: silent as any, + queryCapabilities: () => ({ nativeSql: true, objectqlAggregate: false, inMemory: false }), + executeRawSql: async () => ROWS, + isRegisteredObject: (n: string) => n === 'crm_account', + getObjectFieldNames: (n: string) => (n === 'crm_account' ? ['id', 'industry'] : undefined), + }); +} + +// ───────────────────────────────────────────────────────────────────────────── +// automation — a REAL AutomationEngine that PAUSES at a screen node +// ───────────────────────────────────────────────────────────────────────────── + +/** + * `resumeAuthority: 'any'` because a node type declaring none is gated shut + * since #5561; this fixture only needs the pause to be reachable. + */ +const gate = defineActionDescriptor({ + type: 'gate', + version: '1.0.0', + name: 'gate', + supportsPause: true, + resumeAuthority: 'any', +}); + +/** + * start → gate (pauses) → end. A PAUSED run is chosen deliberately: it is the + * arm whose `AutomationResult` carries `status` / `runId` / `screen`, none of + * which `TriggerFlowResponseSchema.data` declares. + */ +function realAutomation(): AutomationEngine { + const engine = new AutomationEngine( + { info() {}, warn() {}, error() {}, debug() {}, child() { return this; } } as never, + new InMemorySuspendedRunStore(), + ); + engine.registerNodeExecutor({ + type: 'gate', + descriptor: gate, + async execute() { + return { + success: true, + suspend: true, + correlation: 'approval:req-1', + screen: { + nodeId: 'gate', + title: 'Approve the account', + fields: [{ name: 'verdict', type: 'text', label: 'Verdict' }], + }, + }; + }, + } as never); + engine.registerFlow('approve_account', { + name: 'approve_account', + label: 'Approve account', + type: 'autolaunched', + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { id: 'gate', type: 'gate', label: 'Approval' }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e0', source: 'start', target: 'gate' }, + { id: 'e1', source: 'gate', target: 'end' }, + ], + } as never); + return engine; +} + +// ───────────────────────────────────────────────────────────────────────────── +// the in-process socket +// ───────────────────────────────────────────────────────────────────────────── + +/** The REST route object for `POST {base}/analytics/dataset/query`, really registered. */ +function datasetRoute(analytics: AnalyticsService) { + const noopServer = { + get: vi.fn(), post: vi.fn(), put: vi.fn(), delete: vi.fn(), patch: vi.fn(), + use: vi.fn(), listen: vi.fn().mockResolvedValue(undefined), close: vi.fn().mockResolvedValue(undefined), + }; + const protocol = { + getDiscovery: vi.fn().mockResolvedValue({ version: 'v0', routes: { data: '', metadata: '' } }), + getMetaTypes: vi.fn().mockResolvedValue([]), + getMetaItems: vi.fn().mockResolvedValue([]), + }; + const rest = new RestServer( + noopServer as any, protocol as any, { api: { requireAuth: false } } as any, + undefined, undefined, undefined, undefined, undefined, undefined, undefined, + undefined, undefined, undefined, undefined, + async () => analytics, + ); + (rest as any).resolveExecCtx = async () => ({ userId: 'usr_1' }); + rest.registerRoutes(); + const route = rest.getRoutes().find( + (r: any) => r.method === 'POST' && r.path.endsWith('/analytics/dataset/query'), + ); + expect(route, 'POST /analytics/dataset/query must be mounted by @objectstack/rest').toBeTruthy(); + return route as any; +} + +/** + * Everything either side of this function is production code: the URL the + * client built goes in, the body the producer wrote comes back, and nothing in + * between rewrites a key. + */ +function producerBackedClient() { + const analytics = realAnalytics(); + const automation = realAutomation(); + const services: Record = { analytics, automation }; + const resolve = (name: string): unknown => services[name]; + const kernel: any = { + getService: resolve, + getServiceAsync: async (name: string) => resolve(name), + context: { getService: resolve }, + }; + const dispatcher = new HttpDispatcher(kernel); + const dataset = datasetRoute(analytics); + + const fetchImpl = async (url: string, init: RequestInit = {}): Promise => { + const parsed = new URL(String(url)); + const method = init.method ?? 'GET'; + const body = init.body ? JSON.parse(String(init.body)) : undefined; + const query = Object.fromEntries(parsed.searchParams); + + // The REST-served route first — it is a longer prefix of the same family. + if (parsed.pathname === '/api/v1/analytics/dataset/query') { + const res: any = { statusCode: 200, body: undefined }; + res.status = (c: number) => { res.statusCode = c; return res; }; + res.json = (b: any) => { res.body = b; return res; }; + res.end = () => res; + await dataset.handler({ method: 'POST', params: {}, headers: {}, query, body } as any, res); + return { + ok: res.statusCode >= 200 && res.statusCode < 300, + status: res.statusCode, + statusText: String(res.statusCode), + headers: new Headers(), + json: async () => res.body, + }; + } + + const dispatched = parsed.pathname.startsWith('/api/v1/analytics') + ? await dispatcher.handleAnalytics( + parsed.pathname.slice('/api/v1/analytics'.length), method, body, CONTEXT(), query) + : await dispatcher.handleAutomation( + parsed.pathname.slice('/api/v1/automation'.length), method, body, CONTEXT(), query); + + expect(dispatched.handled, `the dispatcher must serve ${method} ${parsed.pathname}`).toBe(true); + const status = dispatched.response?.status ?? 500; + return { + ok: status >= 200 && status < 300, + status, + statusText: String(status), + headers: new Headers(), + json: async () => dispatched.response?.body, + }; + }; + + const client = new ObjectStackClient({ baseUrl: BASE_URL, fetch: fetchImpl as any }); + return { client, dispatcher, analytics, automation }; +} + +// ───────────────────────────────────────────────────────────────────────────── + +describe('#12104 — the four DISPATCHER-served methods resolve to the envelope, not the payload', () => { + it('analytics.query answers `{ success, data: AnalyticsResult }`', async () => { + const { client, analytics } = producerBackedClient(); + + const body = 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 + // returned, asserted against a second call to the service itself + // rather than against a literal written here. + expect(body.data).toEqual(await analytics.query({ + cube: 'crm_account', + measures: ['account_count'], + dimensions: ['industry'], + })); + expect(body.data.rows).toEqual(ROWS); + }); + + it('analytics.meta answers `{ success, data: CubeMeta[] }`', async () => { + const { client, analytics } = producerBackedClient(); + + const body = 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'); + }); + + it('analytics.explain answers `{ success, data: { sql, params } }`', async () => { + const { client } = producerBackedClient(); + + const body = 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); + }); + + it('automation.trigger answers `{ success, data: AutomationResult }` — the whole result', async () => { + const { client } = producerBackedClient(); + + const body = await client.automation.trigger('approve_account', {}); + + expect(body.success).toBe(true); + // The keys `TriggerFlowResponseSchema.data` does NOT declare, served by + // the real engine: this is why the annotation binds `AutomationResult`. + expect(body.data.status).toBe('paused'); + expect(typeof body.data.runId).toBe('string'); + expect(body.data.screen?.title).toBe('Approve the account'); + }); +}); + +describe('#12104 — the REST-served method resolves to the BARE payload', () => { + it('analytics.queryDataset answers the AnalyticsResult itself, with no envelope', async () => { + const { client } = producerBackedClient(); + + const body = await client.analytics.queryDataset({ + dataset: DATASET, + selection: { measures: ['account_count'], dimensions: ['industry'] }, + }); + + // No envelope keys at all — the route ends `res.json(result)`. + expect('success' in (body as object)).toBe(false); + expect('data' in (body as object)).toBe(false); + // …and the payload is right there at the top level. + expect(body.rows).toEqual(ROWS); + expect(Array.isArray(body.fields)).toBe(true); + }); + + it('and it serves a `fields[].label` the analytics RESPONSE schema does not declare', async () => { + // The measurement behind one of the two annotation choices. `query` and + // `queryDataset` are the SAME contract return — `IAnalyticsService` + // declares `Promise< AnalyticsResult >` for both — so a key the service + // really emits is a key `AnalyticsResult` really carries. And + // `AnalyticsResultResponseSchema.data.fields` declares only + // `{ name, type }`, so binding that schema on `analytics.query` would + // have been a FALSE narrowing of the contract the route relays. + const { client } = producerBackedClient(); + + const body = await client.analytics.queryDataset({ + dataset: DATASET, + selection: { measures: ['account_count'], dimensions: ['industry'] }, + }); + + const labelled = (body.fields ?? []).filter((f) => f.label !== undefined); + expect(labelled.map((f) => f.label)).toContain('Industry'); + }); +}); + +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. + const { client, dispatcher } = producerBackedClient(); + + const raw = await dispatcher.handleAnalytics('/meta', 'GET', undefined, CONTEXT(), {}); + const produced: any = raw.response?.body; + + 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); + }); +}); diff --git a/packages/client/src/index.ts b/packages/client/src/index.ts index d83df55e4d..4ea488d19e 100644 --- a/packages/client/src/index.ts +++ b/packages/client/src/index.ts @@ -80,6 +80,16 @@ 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, + AnalyticsMetadataResponse, + AnalyticsSqlResponse, // [#12038] The meta history/diagnostics and package lifecycle response // contracts, bound under the recorded ruling (1C/2C/3A/4A/5A). Each is the // PAYLOAD the route answers — the value after `unwrapResponse` strips the @@ -115,6 +125,7 @@ import type { ApprovalRecallResult, ApprovalResubmitResult, ApprovalSendBackResult, + AnalyticsResult, AudienceBindingSuggestion, AudienceBindingSuggestionSync, AutomationResult, @@ -1463,8 +1474,35 @@ export class ObjectStackClient { /** * Analytics Services */ + /** + * [#12104] ⚠️ THE THREE DISPATCHER-SERVED METHODS HERE RESOLVE TO THE + * ENVELOPE, not to the payload — read `.data`. + * + * 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. + */ analytics = { - query: async (payload: any) => { + /** + * 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(…))`). + * + * ⛔ NOT bound to `AnalyticsResultResponseSchema` (`@objectstack/spec/api`), + * which looks like the route's own response type: its + * `data.fields` declares `{ name, type }` only, while the contract this + * route relays also carries `label` / `format` / `currency` / + * `percentScale` and `totals` — measured on a real `AnalyticsService` in + * `analytics-automation-json-erasure.test.ts`. Binding it would have + * narrowed the declaration below what the producer serves, which is the + * bound-but-false shape #12034 paid to remove. (The schema's own drift is + * a spec-side question, filed separately.) + */ + query: async (payload: any): Promise => { const route = this.getRoute('analytics'); const res = await this.fetch(`${this.baseUrl}${route}/query`, { method: 'POST', @@ -1476,8 +1514,15 @@ export class ObjectStackClient { * 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). - */ - meta: async (cube?: string) => { + * + * [#12104] `AnalyticsMetadataResponse` is the route's own declared response + * type and it AGREES with the producer: its `data` is the bare `CubeMeta[]` + * discovery projection `IAnalyticsService.getMeta` returns (#6442 narrowed + * the 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 => { const route = this.getRoute('analytics'); const qs = cube ? `?cube=${encodeURIComponent(cube)}` : ''; const res = await this.fetch(`${this.baseUrl}${route}/meta${qs}`); @@ -1487,8 +1532,13 @@ export class ObjectStackClient { * 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 + * `IAnalyticsService.generateSql`'s declared return, so the schema and the + * producer say one thing here. */ - explain: async (payload: any) => { + explain: async (payload: any): Promise => { const route = this.getRoute('analytics'); const res = await this.fetch(`${this.baseUrl}${route}/sql`, { method: 'POST', @@ -1502,13 +1552,21 @@ export class ObjectStackClient { * dialect. Provide `dataset` (inline definition, Studio preview) or * `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 + * halves measured on the real route in + * `analytics-automation-json-erasure.test.ts`; the shape is + * `IAnalyticsService.queryDataset`'s declared return. */ queryDataset: async (payload: { dataset?: any; datasetName?: string; selection: { measures: string[]; [k: string]: any }; previewDrafts?: boolean; - }) => { + }): Promise => { const route = this.getRoute('analytics'); const res = await this.fetch(`${this.baseUrl}${route}/dataset/query`, { method: 'POST', @@ -3740,8 +3798,24 @@ export class ObjectStackClient { * | `422` | `FLOW_NO_START_NODE` | the stored definition has no `start` node | fix the flow; retrying cannot help | * | `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. + * + * ⛔ NOT bound to `TriggerFlowResponse` (`@objectstack/spec/api`), which + * looks like this route's response type: its `data` declares + * `{ success, output?, error?, durationMs? }`, and the door also serves + * `status` / `runId` / `screen` (a paused run — see the row above), + * `code`, `successMessage` / `errorMessage` and `summary`. Measured + * against the real `AutomationEngine` in + * `analytics-automation-json-erasure.test.ts`. Binding the narrower + * schema would refuse the very reads this docblock tells callers to make. */ - trigger: async (triggerName: string, payload: any) => { + trigger: async (triggerName: string, payload: any): Promise => { const route = this.getRoute('automation'); const res = await this.fetch(`${this.baseUrl}${route}/trigger/${triggerName}`, { method: 'POST', diff --git a/packages/client/src/return-type-precision.test.ts b/packages/client/src/return-type-precision.test.ts index f79145b817..16c9f6b6a0 100644 --- a/packages/client/src/return-type-precision.test.ts +++ b/packages/client/src/return-type-precision.test.ts @@ -36,6 +36,13 @@ import { ObjectStackClient, ScopedEnvironmentClient } from './index'; import type { CloneDataResult } from './index'; import type { SearchAllResponse } from '@objectstack/spec/api'; import type { + AnalyticsMetadataResponse, + AnalyticsSqlResponse, + BaseResponse, + TriggerFlowResponse, +} from '@objectstack/spec/api'; +import type { + AnalyticsResult, AutomationResult, DelegableScope, ImportObjectResult, @@ -418,6 +425,79 @@ export async function returnTypePrecisionPins12034(): Promise { expectTypeOf(await client.packages.get('com.acme.crm')).toEqualTypeOf<{ package: any }>(); } +/** + * [#12104 — the in-repo half] The SIXTH erasure spelling: no return annotation + * and `return res.json()`, whose published type came from `lib.dom`'s + * `Response.json(): Promise< any >`. The method names neither `any` nor + * `Promise` nor `unwrapResponse`, which is how the class survived two censuses. + * + * Five of the card's 43 are bound here — the families whose producers are IN + * THIS REPO, so the true type is measurable by DRIVING them + * (`analytics-automation-json-erasure.test.ts`: a real `AnalyticsService`, a + * real `AutomationEngine`, the real `HttpDispatcher` and the real `RestServer`, + * 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 + * + * `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. + * + * Type-level for the reason this file's header gives: a runtime test cannot + * observe a return-type narrowing at all. + */ +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)`. + expectTypeOf(await client.analytics.query({ cube: 'crm_account', measures: ['n'] })) + .toEqualTypeOf(); + expectTypeOf(await client.analytics.meta()).toEqualTypeOf(); + expectTypeOf(await client.analytics.explain({ cube: 'crm_account', measures: ['n'] })) + .toEqualTypeOf(); + + // ── the trigger door: the ENVELOPE over the same payload its sibling + // `automation.execute` unwraps ───────────────────────────────────── + expectTypeOf(await client.automation.trigger('approve_account', {})) + .toEqualTypeOf(); + + // ── the one REST-served method: the BARE payload ────────────────────── + 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. + // @ts-expect-error `queryDataset` is served bare by @objectstack/rest — there is no envelope + void (await client.analytics.queryDataset({ selection: { measures: ['n'] } })).data; + + // The two spec response types that LOOK like the right binding and are + // narrower than the contract their route relays. Pinned at the binding so a + // future sweep cannot "tidy" either annotation onto them: the keys below are + // served by the real producers and neither schema declares them. + // @ts-expect-error `TriggerFlowResponse.data` declares no `runId` — a paused run carries one + void (undefined as unknown as TriggerFlowResponse).data.runId; + // @ts-expect-error `TriggerFlowResponse.data` declares no `screen` — a screen-flow pause carries one + void (undefined as unknown as TriggerFlowResponse).data.screen; +} + /** * ⚠️ GREEN IN BOTH STATES — regression guards, recorded as such rather than * counted as evidence that this card's change was needed. Each pins a @@ -463,6 +543,7 @@ describe('client SDK return-type precision (#8140)', () => { expect(typeof returnTypePrecisionPins11925).toBe('function'); expect(typeof returnTypePrecisionPins12038).toBe('function'); expect(typeof returnTypePrecisionPins12034).toBe('function'); + expect(typeof returnTypePrecisionPins12104).toBe('function'); expect(typeof commitRollbackResponseIsNotTheVersionRollbackShape).toBe('function'); expect(typeof environmentIsNotTheCloudWireRow).toBe('function'); }); diff --git a/packages/client/tsconfig.json b/packages/client/tsconfig.json index d880003e4f..b8cf584353 100644 --- a/packages/client/tsconfig.json +++ b/packages/client/tsconfig.json @@ -28,9 +28,20 @@ // no file in it imports either specifier and neither producer's source is // pulled in here. The test program that does pull them in already sets // `rootDir` to the workspace root. + // [#12104] Same rule, same reason, for the three producers + // `src/analytics-automation-json-erasure.test.ts` DRIVES to measure what + // the five `return res.json()` methods really resolve to. That suite's + // claim is "the annotation matches what the producer sends", so its verdict + // must be about the producer's SOURCE; through `exports` it would be about + // the last `pnpm build` of three other packages. Each publishes a single + // `"."` entry point, so one bare-name rule each — no star, per the + // paragraph above. "paths": { "@objectstack/metadata-core": ["../metadata-core/src/index.ts"], - "@objectstack/metadata-protocol": ["../metadata-protocol/src/index.ts"] + "@objectstack/metadata-protocol": ["../metadata-protocol/src/index.ts"], + "@objectstack/rest": ["../rest/src/index.ts"], + "@objectstack/service-analytics": ["../services/service-analytics/src/index.ts"], + "@objectstack/service-automation": ["../services/service-automation/src/index.ts"] } }, "include": ["src/**/*"], diff --git a/packages/client/vitest.config.ts b/packages/client/vitest.config.ts index f3d7d26f2d..87a1e01008 100644 --- a/packages/client/vitest.config.ts +++ b/packages/client/vitest.config.ts @@ -40,6 +40,26 @@ export default defineConfig({ find: /^@objectstack\/metadata-protocol$/, replacement: path.resolve(__dirname, '../metadata-protocol/src/index.ts'), }, + // [#12104] The three producers `analytics-automation-json-erasure.test.ts` + // DRIVES to measure what the five `return res.json()` methods really + // resolve to. Same reason as the pair above, plus one specific to that + // suite: its whole claim is "the annotation matches what the producer + // sends", so it must read the producer IN THIS CHECKOUT. Against `dist/` + // a producer whose envelope had already changed in source would keep the + // suite green on the old shape — certifying a declaration that is by then + // false, which is precisely the failure the annotations exist to prevent. + { + find: /^@objectstack\/rest$/, + replacement: path.resolve(__dirname, '../rest/src/index.ts'), + }, + { + find: /^@objectstack\/service-analytics$/, + replacement: path.resolve(__dirname, '../services/service-analytics/src/index.ts'), + }, + { + find: /^@objectstack\/service-automation$/, + replacement: path.resolve(__dirname, '../services/service-automation/src/index.ts'), + }, ], }, }); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 08ac498d5c..a3aa6f2707 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -361,7 +361,7 @@ importers: version: 6.0.3 vitest: specifier: ^4.1.10 - version: 4.1.10(@opentelemetry/api@1.9.1)(@types/node@26.2.0)(@vitest/coverage-v8@4.1.10)(happy-dom@20.10.2)(jsdom@30.0.1(@noble/hashes@2.3.0))(msw@2.14.6(@types/node@26.2.0)(typescript@6.0.3))(vite@8.0.16(@types/node@26.2.0)(esbuild@0.28.1)(jiti@2.7.0)(tsx@4.23.12)(yaml@2.9.0)) + version: 4.1.10(@opentelemetry/api@1.9.1)(@types/node@26.2.0)(@vitest/coverage-v8@4.1.10)(happy-dom@20.10.2)(jsdom@30.0.1(@noble/hashes@2.3.0))(msw@2.14.6(@types/node@26.2.0)(typescript@6.0.3))(vite@8.0.16(@types/node@26.2.0)(esbuild@0.28.2)(jiti@2.7.0)(tsx@4.23.12)(yaml@2.9.0)) packages/apps/setup: dependencies: @@ -383,7 +383,7 @@ importers: version: 6.0.3 vitest: specifier: ^4.1.10 - version: 4.1.10(@opentelemetry/api@1.9.1)(@types/node@26.2.0)(@vitest/coverage-v8@4.1.10)(happy-dom@20.10.2)(jsdom@30.0.1(@noble/hashes@2.3.0))(msw@2.14.6(@types/node@26.2.0)(typescript@6.0.3))(vite@8.0.16(@types/node@26.2.0)(esbuild@0.28.2)(jiti@2.7.0)(tsx@4.23.12)(yaml@2.9.0)) + version: 4.1.10(@opentelemetry/api@1.9.1)(@types/node@26.2.0)(@vitest/coverage-v8@4.1.10)(happy-dom@20.10.2)(jsdom@30.0.1(@noble/hashes@2.3.0))(msw@2.14.6(@types/node@26.2.0)(typescript@6.0.3))(vite@8.0.16(@types/node@26.2.0)(esbuild@0.28.1)(jiti@2.7.0)(tsx@4.23.12)(yaml@2.9.0)) packages/apps/studio: dependencies: @@ -643,9 +643,18 @@ importers: '@objectstack/plugin-hono-server': specifier: workspace:* version: link:../plugins/plugin-hono-server + '@objectstack/rest': + specifier: workspace:* + version: link:../rest '@objectstack/runtime': specifier: workspace:* version: link:../runtime + '@objectstack/service-analytics': + specifier: workspace:* + version: link:../services/service-analytics + '@objectstack/service-automation': + specifier: workspace:* + version: link:../services/service-automation tsx: specifier: ^4.23.12 version: 4.23.12