diff --git a/packages/runtime/src/domains/automation-runs-query-validation.test.ts b/packages/runtime/src/domains/automation-runs-query-validation.test.ts index 70e66707fc..682fda1ad8 100644 --- a/packages/runtime/src/domains/automation-runs-query-validation.test.ts +++ b/packages/runtime/src/domains/automation-runs-query-validation.test.ts @@ -58,6 +58,7 @@ * `notifications.hono.integration.test.ts` (#6928). */ +import { ExecutionStatus } from '@objectstack/spec/automation'; import { describe, it, expect, vi } from 'vitest'; import { HttpDispatcher } from '../http-dispatcher.js'; @@ -317,13 +318,22 @@ describe('#7300 — every value that had a defensible answer keeps it', () => { expect(listRuns).toHaveBeenCalledWith('welcome_flow', { limit: 2, cursor: undefined, status: 'failed' }); }); - it.each( - ['pending', 'running', 'paused', 'completed', 'failed', 'cancelled', 'timed_out', 'retrying'], - )('forwards every declared ExecutionStatus member — ?status=%s', async (member) => { - // The gate reads its members from the spec's `ExecutionStatus` enum, the - // same one `ListRunsRequestSchema` is built from, so this pins that the + it.each(ExecutionStatus.options)('forwards every declared ExecutionStatus member — ?status=%s', async (member) => { + // The rows are READ off the spec's `ExecutionStatus` — the same enum + // `ListRunsRequestSchema` is built from, and the same one the boundary + // hands `parseEnumParam` as its accepted set — so this pins that the // wire's declared set and the boundary's accepted set are one set. A // member added to the enum and refused here would fail this row. + // + // The members used to be re-listed inline here: identical to the enum + // on the day it was written, and short by one the day `refused` was + // appended (#14945), with this row still green under a name that says + // EVERY declared member — a test named for a property it no longer + // measured. Reading the vocabulary is what makes the name true, and it + // is the discipline the boundary and the wire schema already apply + // (#7359); `automation-api.zod.test.ts` turned the same copy into the + // same read. ⛔ Iterating `.options` does not reorder it — the enum's + // own note reserves those positions for readers that index them. const { result, listRuns } = await listWith({ status: member }); expect(result.response?.status).toBe(200); diff --git a/packages/runtime/src/domains/automation.ts b/packages/runtime/src/domains/automation.ts index 0ff5f6bdbc..1ccffd93f6 100644 --- a/packages/runtime/src/domains/automation.ts +++ b/packages/runtime/src/domains/automation.ts @@ -1796,16 +1796,23 @@ export async function handleAutomationRequest(deps: DomainHandlerDeps, path: str // // [#7359] `status` is the THIRD declared parameter, and until // now the only one this handler never read. `ListRunsRequestSchema` - // has always declared it (`z.enum([...8 ExecutionStatus members]) - // .optional()`), but it had no slot on `IAutomationService.listRuns` - // and was never built into this object — so `?status=failed` was - // dropped here, silently, and the caller was answered 200 with - // EVERY run of the flow capped by `limit`. That is worse than an - // empty page: a monitoring caller paging for failures reads the - // first 20 runs of any status and concludes those are the - // failures. #7300 deliberately left the key ignored rather than - // decide between honouring and retiring it; this card takes the - // enforce route (ADR-0049), so the declared surface is true. + // declares it as `ExecutionStatus.optional()` — the enum itself + // rather than a copy of its members — so what the wire bounds + // the filter to is read from that vocabulary rather than + // restated here. It has NOT always been spelled that way: from + // the schema's introduction until #7359 that line was an inline + // `z.enum([...]).optional()` copy of eight members, and #7359 + // replaced the copy with the enum in the same change that made + // this boundary read the parameter. But `status` had no slot on + // `IAutomationService.listRuns` and was never built into this + // object, so `?status=failed` was dropped here, silently, and + // the caller was answered 200 with EVERY run of the flow capped + // by `limit`. That is worse than an empty page: a monitoring + // caller paging for failures reads the first 20 runs of any + // status and concludes those are the failures. #7300 + // deliberately left the key ignored rather than decide between + // honouring and retiring it; this card takes the enforce route + // (ADR-0049), so the declared surface is true. // // The members come from the spec's own `ExecutionStatus` enum // rather than a list copied into this file: the wire schema is diff --git a/packages/runtime/src/query-param.ts b/packages/runtime/src/query-param.ts index 3e9d3be481..3729b56146 100644 --- a/packages/runtime/src/query-param.ts +++ b/packages/runtime/src/query-param.ts @@ -177,7 +177,11 @@ export function parseIntegerParam( /** * A CLOSED-SET parameter — a filter whose declared values are an enum on the * wire (`?status=failed` on `GET /api/automation/:name/runs`, whose - * `ListRunsRequestSchema` bounds it to the eight `ExecutionStatus` members). + * `ListRunsRequestSchema` bounds it to `ExecutionStatus` itself — the enum + * rather than a copy of its members, so the bound is whatever that vocabulary + * declares rather than a count fixed on the day this line was written. #7359 + * put that spelling there, replacing an inline `z.enum([...])` copy of eight + * members; until then the wire's bound WAS exactly such a fixed count). * * Written for #7359, which is the third shape in this module's family and the * one that fails widest. The other two are coercions that invent a value; this