Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
Expand Down
27 changes: 17 additions & 10 deletions packages/runtime/src/domains/automation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 5 additions & 1 deletion packages/runtime/src/query-param.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading