From 6c93b2f59191d9f7399c9ba3e5fcb862570bb61f Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 29 Aug 2026 06:53:32 +0000 Subject: [PATCH 1/2] fix(spec): widen AnalyticsResultResponseSchema and TriggerFlowResponseSchema data to producer-contract parity Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01LpRNHxWZgSUgVnFT9mQQo4 --- .../issue-13078-response-schema-parity.md | 6 + .../analytics-automation-json-erasure.test.ts | 57 ++++--- packages/client/src/index.ts | 45 ++++-- .../client/src/return-type-precision.test.ts | 21 ++- packages/spec/api-surface/api.json | 2 + packages/spec/export-origins/api.json | 2 + packages/spec/src/api/analytics.test.ts | 91 ++++++++++- packages/spec/src/api/analytics.zod.ts | 59 +++++++ .../spec/src/api/automation-api.zod.test.ts | 149 ++++++++++++++++++ packages/spec/src/api/automation-api.zod.ts | 120 +++++++++++++- 10 files changed, 500 insertions(+), 52 deletions(-) create mode 100644 .changeset/issue-13078-response-schema-parity.md diff --git a/.changeset/issue-13078-response-schema-parity.md b/.changeset/issue-13078-response-schema-parity.md new file mode 100644 index 0000000000..7edd0e6c13 --- /dev/null +++ b/.changeset/issue-13078-response-schema-parity.md @@ -0,0 +1,6 @@ +--- +"@objectstack/spec": minor +"@objectstack/client": patch +--- + +Widen two route response schemas to parity with the producer contracts their routes relay. `AnalyticsResultResponseSchema.data` now declares everything `AnalyticsResult` declares — `fields[].label` / `format` / `currency` / `percentScale` (the renderer chains) and `totals` (the marginal-aggregate channel) — and `TriggerFlowResponseSchema.data` now declares everything `AutomationResult` declares, including the paused screen-flow state (`status` / `runId` / `screen`), the closed `code` classification, the friendly terminal messages and the run `summary`. Both parities are pinned schema ≡ contract at compile time, so the two sources can no longer drift apart silently. `AnalyticsResultResponse` / `AnalyticsResultResponseParsed` are now exported: the schema previously had no nameable response type at all. This is an accept-set widening with zero wire change — every payload that parsed before still parses, and the served keys the schemas used to silently strip (a paused run's `runId` and `screen`, a measure's `label`) now survive a parse. The client SDK's `analytics.query` / `automation.trigger` docblocks are refreshed to record the new state; their bindings still target the producer contracts and are unchanged. diff --git a/packages/client/src/analytics-automation-json-erasure.test.ts b/packages/client/src/analytics-automation-json-erasure.test.ts index c97c34109c..db7dbce9bb 100644 --- a/packages/client/src/analytics-automation-json-erasure.test.ts +++ b/packages/client/src/analytics-automation-json-erasure.test.ts @@ -50,17 +50,21 @@ * 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 + * ## Two spec response schemas WERE 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. + * When #12104 landed, `AnalyticsResultResponseSchema.data` and + * `TriggerFlowResponseSchema.data` were stale projections of + * `AnalyticsResult` / `AutomationResult`: the fixtures below serve keys those + * schemas did not then 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. + * #13078 has since widened both schemas to parity (pinned schema ≡ contract + * in `spec/api/analytics.test.ts` / `spec/api/automation-api.zod.test.ts`); + * the annotations stay on the contracts, which are the source the routes + * relay. The other two route schemas (`AnalyticsMetadataResponseSchema`, + * `AnalyticsSqlResponseSchema`) agreed with their producer's declared return + * all along, and the annotations use them. * * --------------------------------------------------------------------------- * Reverse verification, direction predicted BEFORE running @@ -97,10 +101,12 @@ const CONTEXT = (): any => ({ /** * 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. + * rather than decorative: `AnalyticsResult.fields[].label` was a key + * `AnalyticsResultResponseSchema.data.fields` did not declare when #12104 + * measured it, so serving it is what proved that schema a narrower projection + * than the producer's contract — the measurement the annotation choice rests + * on (#13078 has since widened the schema to parity; the served key is the + * evidence either way). */ const ACCOUNT_CUBE: Cube = { name: 'crm_account', @@ -119,8 +125,8 @@ 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. + * `AnalyticsResult.fields[].label` from it — the key whose serving proved the + * pre-#13078 schema narrower than the contract (see the cube above). */ const DATASET = { name: 'account_metrics', @@ -160,7 +166,7 @@ const gate = defineActionDescriptor({ /** * 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. + * which `TriggerFlowResponseSchema.data` declared before #13078 widened it. */ function realAutomation(): AutomationEngine { const engine = new AutomationEngine( @@ -352,8 +358,10 @@ describe('#12104 — the four DISPATCHER-served methods resolve to the envelope, 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`. + // 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'); @@ -377,14 +385,15 @@ describe('#12104 — the REST-served method resolves to the BARE payload', () => expect(Array.isArray(body.fields)).toBe(true); }); - it('and it serves a `fields[].label` the analytics RESPONSE schema does not declare', async () => { + it('and it serves the `fields[].label` that proved the pre-#13078 response schema narrower', 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. + // really emits is a key `AnalyticsResult` really carries. When #12104 + // measured this, `AnalyticsResultResponseSchema.data.fields` declared + // only `{ name, type }`, so binding that schema on `analytics.query` + // would have been a FALSE narrowing of the contract the route relays; + // #13078 has since widened the schema to parity with the contract. const { client } = producerBackedClient(); const body = await client.analytics.queryDataset({ diff --git a/packages/client/src/index.ts b/packages/client/src/index.ts index 4ea488d19e..e9339ca1cf 100644 --- a/packages/client/src/index.ts +++ b/packages/client/src/index.ts @@ -1492,15 +1492,19 @@ export class ObjectStackClient { * 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.) + * Deliberately bound to the CONTRACT, not to `AnalyticsResultResponse` + * (`@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` / + * `format` / `currency` / `percentScale` and `totals` (measured on a real + * `AnalyticsService` in `analytics-automation-json-erasure.test.ts`), so + * binding it would have been the bound-but-false shape #12034 paid to + * remove. #13078 has since brought the schema to parity (pinned + * schema ≡ contract in `spec/api/analytics.test.ts`), but the binding + * stays on the producer's contract on purpose: the contract is what the + * route relays, and the schema is its transcription — annotating the + * source rather than the copy is what keeps this method immune to the + * transcription drifting again. */ query: async (payload: any): Promise => { const route = this.getRoute('analytics'); @@ -3806,14 +3810,21 @@ export class ObjectStackClient { * 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. + * Deliberately bound to the CONTRACT, not to `TriggerFlowResponse` + * (`@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 + * 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`), so binding it would + * have refused the very reads this docblock tells callers to make. + * #13078 has since brought the schema to parity (pinned + * schema ≡ contract in `spec/api/automation-api.zod.test.ts`), but the + * binding stays on the producer's contract on purpose: the contract is + * what the route relays, and the schema is its transcription — + * 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 => { const route = this.getRoute('automation'); diff --git a/packages/client/src/return-type-precision.test.ts b/packages/client/src/return-type-precision.test.ts index 16c9f6b6a0..5dd4fb9c75 100644 --- a/packages/client/src/return-type-precision.test.ts +++ b/packages/client/src/return-type-precision.test.ts @@ -488,14 +488,19 @@ export async function returnTypePrecisionPins12104(): Promise { // @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; + // [#13078] RE-JUDGED, not re-spelled (the #6442 treatment): two + // suppressions here used to pin that `TriggerFlowResponse.data` declared + // neither `runId` nor `screen` — the near-miss trap that made binding the + // schema a false narrowing. That premise is retired by design: + // #13078 widened both stale schemas to parity with the contracts their + // routes relay, so the old pins' suppressions would now be UNUSED + // (TS2578) precisely because the defect they pinned is fixed. The new + // truth is stronger and pinned as an equality: the schema's `data` IS the + // producer contract. Narrow the schema again and this goes red — the same + // guard, pointing in the direction that is now true. (The annotations + // above still bind the CONTRACT on purpose: it is the source the routes + // relay; the schema is its transcription.) + expectTypeOf().toEqualTypeOf(); } /** diff --git a/packages/spec/api-surface/api.json b/packages/spec/api-surface/api.json index 8f4bae3e70..04fce234d3 100644 --- a/packages/spec/api-surface/api.json +++ b/packages/spec/api-surface/api.json @@ -37,6 +37,8 @@ "AnalyticsProtocol (interface)", "AnalyticsQueryRequest (type)", "AnalyticsQueryRequestSchema (const)", + "AnalyticsResultResponse (type)", + "AnalyticsResultResponseParsed (type)", "AnalyticsResultResponseSchema (const)", "AnalyticsSqlResponse (type)", "AnalyticsSqlResponseParsed (type)", diff --git a/packages/spec/export-origins/api.json b/packages/spec/export-origins/api.json index 5586449faa..0c67a92e1d 100644 --- a/packages/spec/export-origins/api.json +++ b/packages/spec/export-origins/api.json @@ -37,6 +37,8 @@ "AnalyticsProtocol": "src/api/protocol.zod.ts#AnalyticsProtocol (interface)", "AnalyticsQueryRequest": "src/api/analytics.zod.ts#AnalyticsQueryRequest (type)", "AnalyticsQueryRequestSchema": "src/api/analytics.zod.ts#AnalyticsQueryRequestSchema (const)", + "AnalyticsResultResponse": "src/api/analytics.zod.ts#AnalyticsResultResponse (type)", + "AnalyticsResultResponseParsed": "src/api/analytics.zod.ts#AnalyticsResultResponseParsed (type)", "AnalyticsResultResponseSchema": "src/api/analytics.zod.ts#AnalyticsResultResponseSchema (const)", "AnalyticsSqlResponse": "src/api/analytics.zod.ts#AnalyticsSqlResponse (type)", "AnalyticsSqlResponseParsed": "src/api/analytics.zod.ts#AnalyticsSqlResponseParsed (type)", diff --git a/packages/spec/src/api/analytics.test.ts b/packages/spec/src/api/analytics.test.ts index 49e4a8a9ab..681a368adf 100644 --- a/packages/spec/src/api/analytics.test.ts +++ b/packages/spec/src/api/analytics.test.ts @@ -7,8 +7,8 @@ import { AnalyticsMetadataResponseSchema, AnalyticsSqlResponseSchema, } from './analytics.zod'; -import type { AnalyticsMetadataResponse } from './analytics.zod'; -import type { CubeMeta } from '../contracts/analytics-service'; +import type { AnalyticsMetadataResponse, AnalyticsResultResponse } from './analytics.zod'; +import type { AnalyticsResult, CubeMeta } from '../contracts/analytics-service'; /** Type-level identity: true iff A and B are the same type. */ type Eq< A, B > = (< T >() => T extends A ? 1 : 2) extends (< T >() => T extends B ? 1 : 2) ? true : false; @@ -30,6 +30,23 @@ type Assert< T extends true > = T; */ export type CubeMetaMatchesContract = Assert< Eq< AnalyticsMetadataResponse['data'][number], CubeMeta > >; +/** + * #13078 — the declared `data` of `AnalyticsResultResponse` IS the + * `AnalyticsResult` contract interface, not merely shaped like it. + * + * `POST /analytics/query` relays `IAnalyticsService.query`'s declared return + * through `deps.success(result)` verbatim, and the contract was already + * correct while `AnalyticsResultResponseSchema` declared a strict subset + * (`fields[].label` / `format` / `currency` / `percentScale` and `totals` + * were missing) — the same two-files-one-shape drift #6442 fixed for the + * meta sibling, caught the same way. Binding them here is what stops it + * happening a third time: narrow either one alone and this goes red. + * + * Exported deliberately — an unread alias inside a test body is TS6196, and a + * `@ts-expect-error`-style pin no program compiles is no pin at all. + */ +export type AnalyticsResultMatchesContract = Assert< Eq< AnalyticsResultResponse['data'], AnalyticsResult > >; + describe('AnalyticsEndpoint', () => { it('should accept all valid endpoints', () => { for (const ep of [ @@ -164,6 +181,76 @@ describe('AnalyticsResultResponseSchema', () => { }) ).toThrow(); }); + + // #13078 — the AnalyticsResult members the schema used to be missing. The + // PRESERVATION half matters as much as the parse: this schema strips + // undeclared keys (BaseResponseSchema.extend + plain z.object), so before + // the widening these exact payloads "passed" while the parse silently + // dropped every one of the keys a dashboard reads. + it('should preserve fields[].label/format/currency/percentScale — the renderer chains', () => { + const resp = AnalyticsResultResponseSchema.parse({ + success: true, + data: { + rows: [{ industry: 'Retail', mrr: 5000, win_rate: 0.42 }], + fields: [ + { name: 'industry', type: 'string', label: 'Industry' }, + { name: 'mrr', type: 'number', label: 'MRR', format: '$0,0', currency: 'USD' }, + { name: 'win_rate', type: 'number', label: 'Win rate', format: '0.0%', percentScale: 'fraction' }, + ], + }, + }); + expect(resp.data.fields[0].label).toBe('Industry'); + expect(resp.data.fields[1].currency).toBe('USD'); + expect(resp.data.fields[2].percentScale).toBe('fraction'); + expect(resp.data.fields[2].format).toBe('0.0%'); + }); + + it('should preserve totals — the marginal-aggregate channel, grand total included', () => { + const resp = AnalyticsResultResponseSchema.parse({ + success: true, + data: { + rows: [ + { region: 'EMEA', quarter: 'Q1', revenue: 100 }, + { region: 'EMEA', quarter: 'Q2', revenue: 150 }, + ], + fields: [ + { name: 'region', type: 'string' }, + { name: 'quarter', type: 'string' }, + { name: 'revenue', type: 'number' }, + ], + totals: [ + { dimensions: ['region'], rows: [{ region: 'EMEA', revenue: 250 }] }, + { dimensions: [], rows: [{ revenue: 250 }] }, + ], + }, + }); + expect(resp.data.totals).toHaveLength(2); + expect(resp.data.totals?.[1].dimensions).toEqual([]); + expect(resp.data.totals?.[1].rows[0].revenue).toBe(250); + }); + + it('should reject a percentScale outside the closed vocabulary, and a totals entry without dimensions', () => { + expect(() => + AnalyticsResultResponseSchema.parse({ + success: true, + data: { + rows: [], + fields: [{ name: 'win_rate', type: 'number', percentScale: 'percent' }], + }, + }) + ).toThrow(); + + expect(() => + AnalyticsResultResponseSchema.parse({ + success: true, + data: { + rows: [], + fields: [], + totals: [{ rows: [] }], + }, + }) + ).toThrow(); + }); }); describe('GetAnalyticsMetaRequestSchema', () => { diff --git a/packages/spec/src/api/analytics.zod.ts b/packages/spec/src/api/analytics.zod.ts index f398fdb757..b030f3603b 100644 --- a/packages/spec/src/api/analytics.zod.ts +++ b/packages/spec/src/api/analytics.zod.ts @@ -64,6 +64,29 @@ export const AnalyticsQueryRequestSchema = lazySchema(() => /** * Query Response (JSON) + * + * `data` IS the producer's declared return: `POST /analytics/query` ends + * `deps.success(await analyticsService.query(body, ctx))` + * (`runtime/src/domains/analytics.ts`), so the body under `data` is + * `IAnalyticsService.query`'s `AnalyticsResult` + * (`contracts/analytics-service.ts`) — member for member. #13078 restored the + * parity: this schema used to declare only `rows` / `fields{name,type}` / + * `sql?`, a strict subset of what the route relays, while the wire really + * carries `fields[].label` (measured against a real `AnalyticsService` in + * `packages/client/src/analytics-automation-json-erasure.test.ts`), + * `format` / `currency` / `percentScale` (the ADR-0053 / percent-scale + * renderer chains) and `totals` (the ADR-0021 marginal-aggregate channel). + * + * The reasoning is #6442's, recorded on `AnalyticsMetadataResponseSchema` + * below: when the TS contract and the runtime already agree, the schema is + * the lone outlier and the SCHEMA moves. A response schema that reads as the + * route's contract but is narrower than it refuses reads the wire carries — + * a consumer that bound it (exactly what a sweep reaches for, #12104) would + * ship a false declaration. Zero runtime change: only the declaration moves. + * + * Drift guard: `analytics.test.ts` binds `AnalyticsResultResponse['data']` to + * `AnalyticsResult` at compile time — narrow either side alone and it goes + * red. Keep new `AnalyticsResult` members mirrored here (and vice versa). */ export const AnalyticsResultResponseSchema = lazySchema(() => BaseResponseSchema.extend({ data: z.object({ @@ -71,8 +94,35 @@ export const AnalyticsResultResponseSchema = lazySchema(() => BaseResponseSchema fields: z.array(z.object({ name: z.string(), type: z.string(), + label: z.string().optional() + .describe('Human display label (e.g. measure `label`) — for legends/KPIs.'), + format: z.string().optional() + .describe('Display format hint (e.g. measure `format` like "$0,0", "0.0%").'), + currency: z.string().optional().describe( + 'Resolved ISO 4217 code for a MONETARY measure (explicit measure ' + + '`currency`, then source-field default, then tenant default). Absent on ' + + 'non-monetary columns, which must never render a symbol.', + ), + percentScale: z.enum(['fraction', 'whole']).optional().describe( + 'The column\'s percent SCALE, when it is a percentage: `fraction` for a ' + + '0-1 ratio (`1` renders as "100%"), `whole` for percentage points (`1` ' + + 'renders as "1%"). Resolved from metadata; absent when the column is ' + + 'not a percentage. Renderers that receive it must scale by it instead ' + + 'of guessing from the value.', + ), })).describe('Column metadata'), sql: z.string().optional().describe('Executed SQL (if debug enabled)'), + totals: z.array(z.object({ + dimensions: z.array(z.string()) + .describe('The dimension subset this marginal was grouped by (empty array = grand total)'), + rows: z.array(z.record(z.string(), z.unknown())) + .describe('The grouping\'s dimension columns plus the same measure columns as the main rows'), + })).optional().describe( + 'Marginal aggregates - one entry per requested totals grouping, in ' + + 'request order, each computed with the measure\'s true aggregate over ' + + 'the underlying data (never re-derived from bucketed values). The ' + + 'grand-total grouping yields a single dimensionless row.', + ), }), })); @@ -171,6 +221,15 @@ export const AnalyticsSqlResponseSchema = lazySchema(() => BaseResponseSchema.ex export type AnalyticsEndpoint = z.input; export type AnalyticsQueryRequest = z.input; +/** + * #13078 — previously this schema had NO exported type at all + * (`protocol.zod.ts` kept a module-local `z.infer` alias), so a consumer could + * not name the route's response even after the schema said the right thing. + * Exported exactly as every sibling in this file is: `z.input` + `Parsed`. + */ +export type AnalyticsResultResponse = z.input; +/** Post-parse shape of {@link AnalyticsResultResponse} — defaults applied, transforms run (ADR-0122). */ +export type AnalyticsResultResponseParsed = z.infer; export type AnalyticsMetadataResponse = z.input; /** Post-parse shape of {@link AnalyticsMetadataResponse} — defaults applied, transforms run (ADR-0122). */ export type AnalyticsMetadataResponseParsed = z.infer; diff --git a/packages/spec/src/api/automation-api.zod.test.ts b/packages/spec/src/api/automation-api.zod.test.ts index 032654cfff..d9d85f592c 100644 --- a/packages/spec/src/api/automation-api.zod.test.ts +++ b/packages/spec/src/api/automation-api.zod.test.ts @@ -24,7 +24,32 @@ import { AutomationApiErrorCode, AutomationApiContracts, } from './automation-api.zod'; +import type { TriggerFlowResponse } from './automation-api.zod'; import { ExecutionStatus } from '../automation/execution.zod'; +import type { AutomationResult } from '../contracts/automation-service'; + +/** Type-level identity: true iff A and B are the same type. */ +type Eq< A, B > = (< T >() => T extends A ? 1 : 2) extends (< T >() => T extends B ? 1 : 2) ? true : false; +/** Compile error when the argument is not `true`. */ +type Assert< T extends true > = T; + +/** + * #13078 — the declared `data` of `TriggerFlowResponse` IS the + * `AutomationResult` contract interface, not merely shaped like it. + * + * Both trigger routes relay `IAutomationService.execute`'s declared return + * through `deps.success(result)` verbatim, and the contract was already + * correct while `TriggerFlowResponseSchema` declared a strict subset — the + * #9378/#9510 paused third state (`status`/`runId`/`screen`), `code`, the + * friendly terminal messages and `summary` were all missing. Same + * two-files-one-shape drift #6442 fixed for `AnalyticsMetadataResponseSchema` + * (the reasoning is recorded there); binding the two here is what stops it + * recurring: narrow either one alone and this goes red. + * + * Exported deliberately — an unread alias inside a test body is TS6196, and a + * `@ts-expect-error`-style pin no program compiles is no pin at all. + */ +export type TriggerFlowDataMatchesContract = Assert< Eq< TriggerFlowResponse['data'], AutomationResult > >; // ========================================== // Path Parameters @@ -314,6 +339,130 @@ describe('TriggerFlowResponseSchema', () => { expect(result.data.success).toBe(false); expect(result.data.error).toContain('timeout'); }); + + // #13078 — the AutomationResult members the schema used to be missing. The + // PRESERVATION half matters as much as the parse: this schema strips + // undeclared keys (BaseResponseSchema.extend + plain z.object), so before + // the widening the paused-run triple below "passed" while the parse silently + // dropped `status`, the `runId` a caller resumes with and the whole screen. + it('should preserve the paused-run triple — status, runId and the screen (#9378/#9510 third state)', () => { + const result = TriggerFlowResponseSchema.parse({ + success: true, + data: { + success: true, + status: 'paused', + runId: 'run_screen_001', + screen: { + nodeId: 'collect_details', + title: 'Opportunity details', + fields: [ + { name: 'amount', label: 'Amount', type: 'number', required: true }, + { + name: 'stage', + label: 'Stage', + type: 'select', + options: [ + { value: 'prospecting', label: 'Prospecting' }, + { value: 'closed_won', label: 'Closed won' }, + ], + }, + { name: 'close_reason', type: 'text', visibleWhen: 'stage == "closed_won"' }, + ], + }, + }, + }); + expect(result.data.status).toBe('paused'); + expect(result.data.runId).toBe('run_screen_001'); + expect(result.data.screen?.nodeId).toBe('collect_details'); + expect(result.data.screen?.fields).toHaveLength(3); + expect(result.data.screen?.fields[1].options?.[1].value).toBe('closed_won'); + expect(result.data.screen?.fields[2].visibleWhen).toContain('closed_won'); + }); + + it('should preserve an object-form screen pause — the second screen kind', () => { + const result = TriggerFlowResponseSchema.parse({ + success: true, + data: { + success: true, + status: 'paused', + runId: 'run_convert_002', + screen: { + nodeId: 'create_customer', + kind: 'object-form', + objectName: 'account', + mode: 'create', + fields: [], + defaults: { name: 'Acme Corp' }, + idVariable: 'customerId', + }, + }, + }); + expect(result.data.screen?.kind).toBe('object-form'); + expect(result.data.screen?.objectName).toBe('account'); + expect(result.data.screen?.defaults?.name).toBe('Acme Corp'); + expect(result.data.screen?.idVariable).toBe('customerId'); + }); + + it('should preserve terminal message and summary on a finished run', () => { + const result = TriggerFlowResponseSchema.parse({ + success: true, + data: { + success: true, + status: 'completed', + durationMs: 87, + successMessage: 'Opportunity created.', + summary: { + selected: 1, + acted: 1, + skipped: 0, + nodes: [], + gates: [], + }, + }, + }); + expect(result.data.successMessage).toBe('Opportunity created.'); + expect(result.data.summary?.acted).toBe(1); + }); + + it('should preserve the failure classification code alongside error', () => { + const result = TriggerFlowResponseSchema.parse({ + success: true, + data: { + success: false, + error: 'Flow is disabled', + code: 'FLOW_DISABLED', + }, + }); + expect(result.data.code).toBe('FLOW_DISABLED'); + }); + + it('should reject a status or code outside the closed vocabulary, and a screen without its nodeId', () => { + expect(() => + TriggerFlowResponseSchema.parse({ + success: true, + data: { success: true, status: 'running' }, + }) + ).toThrow(); + + expect(() => + TriggerFlowResponseSchema.parse({ + success: true, + data: { success: false, code: 'SOMETHING_ELSE' }, + }) + ).toThrow(); + + expect(() => + TriggerFlowResponseSchema.parse({ + success: true, + data: { + success: true, + status: 'paused', + runId: 'run_1', + screen: { title: 'No node id', fields: [] }, + }, + }) + ).toThrow(); + }); }); // ========================================== diff --git a/packages/spec/src/api/automation-api.zod.ts b/packages/spec/src/api/automation-api.zod.ts index 6c739dd64a..8a12f42594 100644 --- a/packages/spec/src/api/automation-api.zod.ts +++ b/packages/spec/src/api/automation-api.zod.ts @@ -3,7 +3,7 @@ import { z } from 'zod'; import { BaseResponseSchema } from './contract.zod'; import { FlowSchema } from '../automation/flow.zod'; -import { ExecutionLogSchema, ExecutionStatus } from '../automation/execution.zod'; +import { ExecutionLogSchema, ExecutionStatus, FlowRunSummarySchema } from '../automation/execution.zod'; /** * Automation API Protocol @@ -218,8 +218,86 @@ export const TriggerFlowRequestSchema = lazySchema(() => AutomationFlowPathParam })); export type TriggerFlowRequest = z.input; +/** + * One input field rendered by a paused `screen` node, as the trigger route + * serves it — the wire spelling of `ScreenFieldSpec` + * (`contracts/automation-service.ts`), which is THE name for this shape: a + * second exported name here would be the permanent synonym ADR-0122 D3 + * forbids and a new dual-source export, so this stays module-local (the + * `cubeMetaMemberShape` treatment, `analytics.zod.ts`). + * `automation-api.zod.test.ts` binds the two at compile time. + */ +const screenFieldSpecShape = () => z.object({ + name: z.string(), + label: z.string().optional(), + type: z.string().optional() + .describe('Widget hint (text/select/boolean/number/date/...); the client maps it to a field widget'), + required: z.boolean().optional(), + options: z.array(z.object({ + value: z.unknown(), + label: z.string(), + })).optional().describe('Closed-enum options for select-style fields'), + defaultValue: z.unknown().optional(), + placeholder: z.string().optional(), + visibleWhen: z.string().optional().describe( + 'Conditional-visibility predicate, evaluated by the CLIENT against the ' + + 'screen\'s live collected values - bare CEL over the screen\'s own field ' + + 'names. Omit = always visible. A hidden field is not collected, so its ' + + '`required` never fires.', + ), +}); + +/** + * The screen a paused `screen` node wants the client to render — the wire + * spelling of `ScreenSpec` (`contracts/automation-service.ts`), module-local + * for the reason on {@link screenFieldSpecShape}. + */ +const screenSpecShape = () => z.object({ + nodeId: z.string() + .describe('The screen node\'s id (correlates the resume back to this pause point)'), + title: z.string().optional(), + description: z.string().optional(), + fields: z.array(screenFieldSpecShape()), + kind: z.enum(['fields', 'object-form']).optional().describe( + 'Rendering kind. `fields` (default) renders the flat field list; ' + + '`object-form` renders an object\'s full create/edit form.', + ), + objectName: z.string().optional().describe('Object whose form to render (object-form screens)'), + mode: z.enum(['create', 'edit']).optional() + .describe('Form mode for an object-form screen - defaults to `create`'), + recordId: z.string().optional().describe('Record id to edit (object-form screens in `edit` mode)'), + defaults: z.record(z.string(), z.unknown()).optional() + .describe('Prefilled field values for the object form (already interpolated)'), + idVariable: z.string().optional().describe( + 'Flow variable that receives the saved record\'s id when the client ' + + 'resumes the run, so a later step can reference it', + ), +}); + /** * Response after triggering a flow execution. + * + * `data` IS the producer's declared return: `POST /automation/:name/trigger` + * (and the legacy `POST /automation/trigger/:name`) end + * `deps.success(result)` where `result` is `IAutomationService.execute`'s + * `AutomationResult` (`contracts/automation-service.ts`), relayed verbatim — + * member for member. #13078 restored the parity: this schema used to declare + * only `success` / `output?` / `error?` / `durationMs?`, a strict subset of + * what the route relays. The missing members were not decoration: + * `status: 'paused'` + `runId` + `screen` is the whole third state of the + * #9378 / #9510 trigger contract — the payload a caller resumes a screen + * flow with — and the SDK's own docblock on `automation.trigger` tells + * callers to read exactly those. + * + * The reasoning is #6442's (recorded on `AnalyticsMetadataResponseSchema`, + * `analytics.zod.ts`): when the TS contract and the runtime already agree, + * the schema is the lone outlier and the SCHEMA moves. Zero runtime change: + * only the declaration moves. + * + * Drift guard: `automation-api.zod.test.ts` binds + * `TriggerFlowResponse['data']` to `AutomationResult` at compile time — + * narrow either side alone and it goes red. Keep new `AutomationResult` + * members mirrored here (and vice versa). */ export const TriggerFlowResponseSchema = lazySchema(() => BaseResponseSchema.extend({ data: z.object({ @@ -227,6 +305,46 @@ export const TriggerFlowResponseSchema = lazySchema(() => BaseResponseSchema.ext output: z.unknown().optional().describe('Output data from the automation'), error: z.string().optional().describe('Error message if execution failed'), durationMs: z.number().optional().describe('Execution duration in milliseconds'), + code: z.enum([ + 'PERMISSION_DENIED', + 'INVALID_SIGNAL', + 'RUN_NOT_FOUND', + 'STORE_UNAVAILABLE', + 'RESUME_IN_PROGRESS', + 'INVALID_SCREEN_INPUT', + 'FLOW_DISABLED', + 'FLOW_NO_START_NODE', + 'FLOW_INPUT_SCHEMA_INVALID', + ]).optional().describe( + 'Machine-readable failure classification, set alongside `error` when the ' + + 'caller must distinguish WHY it failed. A closed union - the members and ' + + 'their transport mappings are documented on the contract ' + + '(`AutomationResult.code`, contracts/automation-service.ts).', + ), + status: z.enum(['completed', 'paused', 'failed']).optional().describe( + 'Lifecycle status. `paused` means the run suspended at a node and can be ' + + 'continued with the resume route. Absent or `completed`/`failed` means ' + + 'the run reached a terminal state.', + ), + runId: z.string().optional() + .describe('Run id - set when `status` is `paused`, so callers can resume it'), + screen: screenSpecShape().optional().describe( + 'The screen to render - set when the run paused at a `screen` node ' + + 'awaiting user input. The client collects values for `screen.fields` ' + + 'and resumes the run with them.', + ), + successMessage: z.string().optional().describe( + 'Friendly terminal message copied from the flow definition on terminal ' + + 'success, so a screen-flow runner can show a meaningful toast', + ), + errorMessage: z.string().optional().describe( + 'Friendly terminal message copied from the flow definition on failure', + ), + summary: FlowRunSummarySchema.optional().describe( + 'What the run did - records selected / acted on, gate skips, per-node ' + + 'status. Set on a TERMINAL result (a paused run has not finished doing ' + + 'it yet).', + ), }), })); export type TriggerFlowResponse = z.input; From 9ae99605fc02392cf3989cfa57826fc7e2cdc984 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 29 Aug 2026 07:38:50 +0000 Subject: [PATCH 2/2] chore(spec): regenerate reference docs, strictness ledger and import-surface baseline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tooling output of the schema widening, not hand-written: - `gen:docs` re-renders the analytics/automation-api reference pages with the newly declared members and the now-importable `AnalyticsResultResponse`. - `gen:strictness-ledger` moves the `api/` unknown-key site count 444 -> 448. - `--update-import-baseline` discharges the shrink-only ratchet entry `api/AnalyticsResultResponse — no type export`: the gap the card names is closed, and a stale line would stay available to excuse the next one. --- content/docs/references/api/analytics.mdx | 7 ++++--- content/docs/references/api/automation-api.mdx | 9 ++++++++- .../2026-07-unknown-key-strictness-ledger.counts.md | 2 +- packages/spec/docs-import-surface.baseline.json | 1 - 4 files changed, 13 insertions(+), 6 deletions(-) diff --git a/content/docs/references/api/analytics.mdx b/content/docs/references/api/analytics.mdx index 3fc17cc513..95fdd34731 100644 --- a/content/docs/references/api/analytics.mdx +++ b/content/docs/references/api/analytics.mdx @@ -18,7 +18,7 @@ Provides endpoints for executing analytical queries and discovering metadata. ```typescript import { AnalyticsEndpoint, AnalyticsMetadataResponseSchema, AnalyticsQueryRequestSchema, AnalyticsResultResponseSchema, AnalyticsSqlResponseSchema, GetAnalyticsMetaRequestSchema } from '@objectstack/spec/api'; -import type { AnalyticsEndpoint, AnalyticsMetadataResponse, AnalyticsQueryRequest, AnalyticsSqlResponse, GetAnalyticsMetaRequest } from '@objectstack/spec/api'; +import type { AnalyticsEndpoint, AnalyticsMetadataResponse, AnalyticsQueryRequest, AnalyticsResultResponse, AnalyticsSqlResponse, GetAnalyticsMetaRequest } from '@objectstack/spec/api'; // Validate data const result = AnalyticsEndpoint.parse(data); @@ -103,7 +103,7 @@ const result = AnalyticsEndpoint.parse(data); | **success** | `boolean` | ✅ | Operation success status | | **error** | `{ code: Enum<'VALIDATION_ERROR' \| 'INVALID_FIELD' \| 'MISSING_REQUIRED_FIELD' \| …>; declaredCode?: string; message: string; userMessage?: string; … }` | optional | Error details if success is false | | **meta** | `{ timestamp: string; duration?: number; requestId?: string; traceId?: string }` | optional | Response metadata | -| **data** | `{ rows: Record[]; fields: object[]; sql?: string }` | ✅ | | +| **data** | `{ rows: Record[]; fields: object[]; sql?: string; totals?: object[] }` | ✅ | | ### Nested Shape: `AnalyticsResultResponse.error` @@ -123,8 +123,9 @@ const result = AnalyticsEndpoint.parse(data); | Property | Type | Required | Description | | :--- | :--- | :--- | :--- | | **rows** | `Record[]` | ✅ | Result rows | -| **fields** | `{ name: string; type: string }[]` | ✅ | Column metadata | +| **fields** | `{ name: string; type: string; label?: string; format?: string; … }[]` | ✅ | Column metadata | | **sql** | `string` | optional | Executed SQL (if debug enabled) | +| **totals** | `{ dimensions: string[]; rows: Record[] }[]` | optional | Marginal aggregates - one entry per requested totals grouping, in request order, each computed with the measure's true aggregate over the underlying data (never re-derived from bucketed values). The grand-total grouping yields a single dimensionless row. | --- diff --git a/content/docs/references/api/automation-api.mdx b/content/docs/references/api/automation-api.mdx index d18748fd20..91cad7f6e8 100644 --- a/content/docs/references/api/automation-api.mdx +++ b/content/docs/references/api/automation-api.mdx @@ -583,7 +583,7 @@ const result = AutomationApiErrorCode.parse(data); | **success** | `boolean` | ✅ | Operation success status | | **error** | `{ code: Enum<'VALIDATION_ERROR' \| 'INVALID_FIELD' \| 'MISSING_REQUIRED_FIELD' \| …>; declaredCode?: string; message: string; userMessage?: string; … }` | optional | Error details if success is false | | **meta** | `{ timestamp: string; duration?: number; requestId?: string; traceId?: string }` | optional | Response metadata | -| **data** | `{ success: boolean; output?: any; error?: string; durationMs?: number }` | ✅ | | +| **data** | `{ success: boolean; output?: any; error?: string; durationMs?: number; … }` | ✅ | | ### Nested Shape: `TriggerFlowResponse.error` @@ -606,6 +606,13 @@ const result = AutomationApiErrorCode.parse(data); | **output** | `any` | optional | Output data from the automation | | **error** | `string` | optional | Error message if execution failed | | **durationMs** | `number` | optional | Execution duration in milliseconds | +| **code** | `Enum<'PERMISSION_DENIED' \| 'INVALID_SIGNAL' \| 'RUN_NOT_FOUND' \| 'STORE_UNAVAILABLE' \| …>` | optional | Machine-readable failure classification, set alongside `error` when the caller must distinguish WHY it failed. A closed union - the members and their transport mappings are documented on the contract (`AutomationResult.code`, contracts/automation-service.ts). | +| **status** | `Enum<'completed' \| 'paused' \| 'failed'>` | optional | Lifecycle status. `paused` means the run suspended at a node and can be continued with the resume route. Absent or `completed`/`failed` means the run reached a terminal state. | +| **runId** | `string` | optional | Run id - set when `status` is `paused`, so callers can resume it | +| **screen** | `{ nodeId: string; title?: string; description?: string; fields: object[]; … }` | optional | The screen to render - set when the run paused at a `screen` node awaiting user input. The client collects values for `screen.fields` and resumes the run with them. | +| **successMessage** | `string` | optional | Friendly terminal message copied from the flow definition on terminal success, so a screen-flow runner can show a meaningful toast | +| **errorMessage** | `string` | optional | Friendly terminal message copied from the flow definition on failure | +| **summary** | `{ selected: integer; acted: integer; skipped: integer; unmeasured?: integer; … }` | optional | What the run did - records selected / acted on, gate skips, per-node status. Set on a TERMINAL result (a paused run has not finished doing it yet). | --- diff --git a/docs/audits/2026-07-unknown-key-strictness-ledger.counts.md b/docs/audits/2026-07-unknown-key-strictness-ledger.counts.md index dfd18ba561..dd84dc83d1 100644 --- a/docs/audits/2026-07-unknown-key-strictness-ledger.counts.md +++ b/docs/audits/2026-07-unknown-key-strictness-ledger.counts.md @@ -257,7 +257,7 @@ directory rather than per file. | Dir | Sites | |---|---| | `ai/` | 77 | -| `api/` | 444 | +| `api/` | 448 | | `cloud/` | 83 | | `identity/` | 32 | | `integration/` | 10 | diff --git a/packages/spec/docs-import-surface.baseline.json b/packages/spec/docs-import-surface.baseline.json index 9f9e1e4963..70914cfd65 100644 --- a/packages/spec/docs-import-surface.baseline.json +++ b/packages/spec/docs-import-surface.baseline.json @@ -4,7 +4,6 @@ "ai/AIModelConfig — no type export", "ai/CodeContent — no type export", "ai/ImageContent — no type export", - "api/AnalyticsResultResponse — no type export", "api/Discovery — no type export", "api/HttpFindQueryParams — no type export", "api/MetadataExportRequest — no type export",