Skip to content

Commit bf45e62

Browse files
os-litantclaude
andauthored
test(runtime): the runs ?status= pin reads ExecutionStatus instead of copying it (#16426)
* test(runtime): the runs `?status=` pin reads ExecutionStatus instead of copying it `automation-runs-query-validation.test.ts` asserted "forwards every declared ExecutionStatus member" over a list of eight members typed into the test body. It was identical to the enum the day it was written and short by one the day `refused` was appended (#14945) -- so the row stayed green while covering eight of nine, under a name that promises every member. A pin that copies the vocabulary it claims to cover stops measuring that claim the moment the vocabulary moves, and says nothing when it does. The rows now come from `ExecutionStatus.options`, which is the discipline both neighbours already apply: the boundary hands `parseEnumParam` that same array (`domains/automation.ts`), and `automation-api.zod.test.ts` turned its own inline copy into the same read under #7359. Iterating `.options` only reads it -- nothing here sorts or re-seats it, because `execution.zod.ts` reserves those positions for readers that index them ("Appended last so every reader that indexes `.options` keeps its positions"). Two prose citations of the same set were counting it rather than naming it: - `query-param.ts` said `ListRunsRequestSchema` "bounds it to the eight `ExecutionStatus` members"; - `domains/automation.ts` said the schema declares "`z.enum([...8 ExecutionStatus members])`". Both now say what the set is defined BY -- the enum itself, never a copy of its members -- rather than how many are in it. Deliberately NOT "nine": a fresh literal re-arms the identical trap and reads authoritative the whole time, which is the correction #16414 landed hours ago and the reason it refused to mint a replacement number of its own. Population, swept rather than assumed: every tracked non-dist file was scanned for windows carrying four or more of the nine members, and separately for count words next to `ExecutionStatus`. The matcher's positive control is this file's own pre-change text, which it reports at eight of nine members with `refused` missing. Outside the three sites above it finds no live stale copy: `execution.test.ts`'s eight-member list is the deliberate PREFIX pin for the appended-last invariant, `AUTOMATION_RESULT_STATUSES` is a different, five member union held exhaustive by a type-level `Eq`, plugin-approvals' `TERMINAL_RUN_STATUSES` is a documented narrower subset, and the generated reference docs carry all nine. One stale copy does survive, in `docs/qa/platform-checklist/areas/automation.json` -- filed separately rather than edited here: a JSON checklist has no enum to read, so its repair is an authoring decision and a different gate family. Test-only plus comment text. No behaviour, no asserted value and no minted count changes; the CHANGELOG copies stay untouched as historical records. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N * docs(runtime): correct three untrue claims this branch wrote down Round 2 on #15890, after review. Comment prose only — no executable line moves, the pin and its ablation are untouched and settled. 1. HISTORY. The previous commit on this branch wrote, of `ListRunsRequestSchema`, in `domains/automation.ts`: "has always declared it — as `ExecutionStatus.optional()`, the enum itself rather than a copy of its members" and in `query-param.ts`: "bounds it to `ExecutionStatus` itself — the enum, never a copy of its members" Both are FALSE of history, and — the whole lesson — the sentence the first one replaced was accurate. `ListRunsRequestSchema` declared `status` as an inline `z.enum(['pending', 'running', 'paused', 'completed', 'failed', 'cancelled', 'timed_out', 'retrying']).optional()` from the schema's introduction (`5e685a7d6f7`, 2026-02-21) until #7359 replaced that copy with `ExecutionStatus.optional()` — landed as `cf7c69421a7` (2026-08-11), in the same commit that made this boundary read the parameter. So "always" and "never" are both wrong: it was a copy of eight members for most of its life. Verified by `git show 5e685a7:packages/spec/src/api/automation-api.zod.ts`, `git show cf7c694^:` on the same path, and `git log -S` on that literal — which names exactly two commits, the introduction and #7359. Repairing the stale COUNT had introduced a falsehood about HISTORY, in a card whose entire subject is untrue claims about this vocabulary. Both sites now say what the line declares today AND when it started saying it. No count of the LIVE enum is minted; "eight" is a property of a deleted literal, frozen by `cf7c69421a7`, and it is what makes the sentence explain itself. 2. `TERMINAL_RUN_STATUSES`. The previous commit called it "a documented narrower subset" Measured in `plugin-approvals/src/approval-service.ts`, it is `{completed, failed, cancelled, timed_out}` — a set its own header calls "A CLOSED set, deliberately", whose dead-run sweep treats every other answer, an unknown status included, as ALIVE so an unrecognised state can never cost someone a live approval. That is a fail-safe default, not a narrowing anyone documented relative to `ExecutionStatus`. It was closed before `refused` existed, and `refused` — "Terminal, never resumed" — is not in it, so once lane 2 (#15788) makes a run reach that status, its still-pending approval reads as alive to that sweep. A reader of this vocabulary that has not learned the new member, owned by the `refused` lanes, not by this card. Corrected in the PR body; no tree text carried the wrong wording. 3. "#16414 landed hours ago" (previous commit and PR body). #16414 landed `0ea5f9d9f79` at 2026-09-06T21:22:02Z; the commit that said "hours" was authored 22:50:48Z the same day — 89 minutes by those two stamps, 66 by the review's anchor. Two anchors, two numbers, and the elapsed time was never load-bearing: the citation of #16414 stays, the time reference is dropped rather than replaced by a third number. Also corrected in the PR body, not in the tree: "No `dist` is on the resolved path" was true of the two ablation legs and false of the suite. Measured on a fresh worktree with nothing built — `pnpm --filter @objectstack/runtime exec vitest run src/domains/automation-runs-query-validation.test.ts` cannot collect the file at all: "Failed to resolve entry for package @objectstack/observability". `http-dispatcher.ts`, which the suite imports, imports that package; `packages/runtime/vitest.config.ts` has no alias entry for it; its `exports` map serves `./dist/index.js`. The two legs stay source-resolved and `ablation-dist-preflight` still does not apply to them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent c14ebfe commit bf45e62

3 files changed

Lines changed: 37 additions & 16 deletions

File tree

packages/runtime/src/domains/automation-runs-query-validation.test.ts

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@
5858
* `notifications.hono.integration.test.ts` (#6928).
5959
*/
6060

61+
import { ExecutionStatus } from '@objectstack/spec/automation';
6162
import { describe, it, expect, vi } from 'vitest';
6263

6364
import { HttpDispatcher } from '../http-dispatcher.js';
@@ -317,13 +318,22 @@ describe('#7300 — every value that had a defensible answer keeps it', () => {
317318
expect(listRuns).toHaveBeenCalledWith('welcome_flow', { limit: 2, cursor: undefined, status: 'failed' });
318319
});
319320

320-
it.each(
321-
['pending', 'running', 'paused', 'completed', 'failed', 'cancelled', 'timed_out', 'retrying'],
322-
)('forwards every declared ExecutionStatus member — ?status=%s', async (member) => {
323-
// The gate reads its members from the spec's `ExecutionStatus` enum, the
324-
// same one `ListRunsRequestSchema` is built from, so this pins that the
321+
it.each(ExecutionStatus.options)('forwards every declared ExecutionStatus member — ?status=%s', async (member) => {
322+
// The rows are READ off the spec's `ExecutionStatus` — the same enum
323+
// `ListRunsRequestSchema` is built from, and the same one the boundary
324+
// hands `parseEnumParam` as its accepted set — so this pins that the
325325
// wire's declared set and the boundary's accepted set are one set. A
326326
// member added to the enum and refused here would fail this row.
327+
//
328+
// The members used to be re-listed inline here: identical to the enum
329+
// on the day it was written, and short by one the day `refused` was
330+
// appended (#14945), with this row still green under a name that says
331+
// EVERY declared member — a test named for a property it no longer
332+
// measured. Reading the vocabulary is what makes the name true, and it
333+
// is the discipline the boundary and the wire schema already apply
334+
// (#7359); `automation-api.zod.test.ts` turned the same copy into the
335+
// same read. ⛔ Iterating `.options` does not reorder it — the enum's
336+
// own note reserves those positions for readers that index them.
327337
const { result, listRuns } = await listWith({ status: member });
328338

329339
expect(result.response?.status).toBe(200);

packages/runtime/src/domains/automation.ts

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1796,16 +1796,23 @@ export async function handleAutomationRequest(deps: DomainHandlerDeps, path: str
17961796
//
17971797
// [#7359] `status` is the THIRD declared parameter, and until
17981798
// now the only one this handler never read. `ListRunsRequestSchema`
1799-
// has always declared it (`z.enum([...8 ExecutionStatus members])
1800-
// .optional()`), but it had no slot on `IAutomationService.listRuns`
1801-
// and was never built into this object — so `?status=failed` was
1802-
// dropped here, silently, and the caller was answered 200 with
1803-
// EVERY run of the flow capped by `limit`. That is worse than an
1804-
// empty page: a monitoring caller paging for failures reads the
1805-
// first 20 runs of any status and concludes those are the
1806-
// failures. #7300 deliberately left the key ignored rather than
1807-
// decide between honouring and retiring it; this card takes the
1808-
// enforce route (ADR-0049), so the declared surface is true.
1799+
// declares it as `ExecutionStatus.optional()` — the enum itself
1800+
// rather than a copy of its members — so what the wire bounds
1801+
// the filter to is read from that vocabulary rather than
1802+
// restated here. It has NOT always been spelled that way: from
1803+
// the schema's introduction until #7359 that line was an inline
1804+
// `z.enum([...]).optional()` copy of eight members, and #7359
1805+
// replaced the copy with the enum in the same change that made
1806+
// this boundary read the parameter. But `status` had no slot on
1807+
// `IAutomationService.listRuns` and was never built into this
1808+
// object, so `?status=failed` was dropped here, silently, and
1809+
// the caller was answered 200 with EVERY run of the flow capped
1810+
// by `limit`. That is worse than an empty page: a monitoring
1811+
// caller paging for failures reads the first 20 runs of any
1812+
// status and concludes those are the failures. #7300
1813+
// deliberately left the key ignored rather than decide between
1814+
// honouring and retiring it; this card takes the enforce route
1815+
// (ADR-0049), so the declared surface is true.
18091816
//
18101817
// The members come from the spec's own `ExecutionStatus` enum
18111818
// rather than a list copied into this file: the wire schema is

packages/runtime/src/query-param.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,11 @@ export function parseIntegerParam(
177177
/**
178178
* A CLOSED-SET parameter — a filter whose declared values are an enum on the
179179
* wire (`?status=failed` on `GET /api/automation/:name/runs`, whose
180-
* `ListRunsRequestSchema` bounds it to the eight `ExecutionStatus` members).
180+
* `ListRunsRequestSchema` bounds it to `ExecutionStatus` itself — the enum
181+
* rather than a copy of its members, so the bound is whatever that vocabulary
182+
* declares rather than a count fixed on the day this line was written. #7359
183+
* put that spelling there, replacing an inline `z.enum([...])` copy of eight
184+
* members; until then the wire's bound WAS exactly such a fixed count).
181185
*
182186
* Written for #7359, which is the third shape in this module's family and the
183187
* one that fails widest. The other two are coercions that invent a value; this

0 commit comments

Comments
 (0)