From ab5a3824353f9373811d225770b4c8ed2796450e Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 6 Sep 2026 22:50:48 +0000 Subject: [PATCH 1/2] 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 Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N --- .../automation-runs-query-validation.test.ts | 20 ++++++++++++---- packages/runtime/src/domains/automation.ts | 23 +++++++++++-------- packages/runtime/src/query-param.ts | 4 +++- 3 files changed, 31 insertions(+), 16 deletions(-) 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..1a69cc2263 100644 --- a/packages/runtime/src/domains/automation.ts +++ b/packages/runtime/src/domains/automation.ts @@ -1796,16 +1796,19 @@ 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. + // has always declared 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 — 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. // // 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..f29b92e4e2 100644 --- a/packages/runtime/src/query-param.ts +++ b/packages/runtime/src/query-param.ts @@ -177,7 +177,9 @@ 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, + * never 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). * * 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 From bf70dbf5a34398a68ca2bdce54df512ca67125f3 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 7 Sep 2026 00:04:45 +0000 Subject: [PATCH 2/2] docs(runtime): correct three untrue claims this branch wrote down MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 5e685a7d6f7:packages/spec/src/api/automation-api.zod.ts`, `git show cf7c69421a7^:` 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 Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N --- packages/runtime/src/domains/automation.ts | 12 ++++++++---- packages/runtime/src/query-param.ts | 8 +++++--- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/packages/runtime/src/domains/automation.ts b/packages/runtime/src/domains/automation.ts index 1a69cc2263..1ccffd93f6 100644 --- a/packages/runtime/src/domains/automation.ts +++ b/packages/runtime/src/domains/automation.ts @@ -1796,10 +1796,14 @@ 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 — 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 — but it had no slot on + // 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 diff --git a/packages/runtime/src/query-param.ts b/packages/runtime/src/query-param.ts index f29b92e4e2..3729b56146 100644 --- a/packages/runtime/src/query-param.ts +++ b/packages/runtime/src/query-param.ts @@ -177,9 +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 `ExecutionStatus` itself — the enum, - * never 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). + * `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