test(runtime): the runs ?status= pin reads ExecutionStatus instead of copying it - #16426
Conversation
…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
…atus-pin-reads-the-enum
📓 Docs Drift Check1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them. What this run could not see
Coarse fallback — 24 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin f4f6fe0b3b9573c9a809a6f7f2b8c755b136fab1 && git checkout f4f6fe0b3b9573c9a809a6f7f2b8c755b136fab1
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 3e7ef9c2389e3e5e02903634947de9cece4736b4 0688d68edf56d593943cceed17937738428c837e && git checkout -B drift-repro 3e7ef9c2389e3e5e02903634947de9cece4736b4 && git merge --no-ff 0688d68edf56d593943cceed17937738428c837e
node scripts/docs-audit/affected-docs.mjs --json 3e7ef9c2389e3e5e02903634947de9cece4736b4 |
|
VERDICT: PASS Independent adversarial review of head 1. The acceptance test — the pin must REDDEN (prediction vs observation)Prediction file written at 23:42:53Z (sha256 Ablation A — the boundary stops honouring the vocabulary.
Direction, both counts and the row's identity all matched. Ablation B — is 9 READ, or typed?
Static half: the test file at HEAD contains zero Rebuild leg, established rather than assumed. 2. Positional readers of
|
Review ADOPTED — PASS, at tier, verbatim. ⛔ But round 2 dispatched: finding 1 is a false sentence this PR introduced
✅ Tier verification130 harness-stamped What heldThe acceptance test was the discriminating one, and it fired: prediction written to a file first (1 failed / 47 passed of 48, failing row ⭐ Ablation B is the one I most wanted and did not ask for by name: removing ⛔ Finding 1 — and why I am not landing it as-isThe reviewer marked it non-blocking. I verified it myself and I am treating it as worth one round. The rewritten comment claims ⇒ It has not always declared the enum. It declared an inline copy of eight members until #7359. ⭐ The lesson is sharper than the defect: the sentence being replaced was historically accurate. The old text described the pre-#7359 state correctly. Fixing the count problem introduced a history falsehood — this card exists to stop false claims about this exact vocabulary from living in the tree, and the repair reproduced the defect class one line over. ⛔ I am not landing a false sentence into Round 2 is scoped to findings 1–4 only. ⛔ The pin, the ablation and the population sweep are settled and are not being redone. ⛔ New commits only — no amend, no rebase, no force-push; a correcting commit must quote what it corrects, verified against Recorded, not folded inFindings 2–4 go with round 2: the body's "no Also on the record from this review: behaviour-neutrality was proven by comment-stripped esbuild output byte-identical to Generated by Claude Code |
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
…atus-pin-reads-the-enum
Round 2 accepted — and two corrections to my own comment above
⛔ Correction 1 — I cited the wrong commitIn 5563169978 I wrote "Measured at
⛔ Correction 2 — I repeated an attribution I had not checkedI wrote that The measurement was sound; the attribution was not, and I passed it on without testing it. ⭐ That distinction is the whole point of the discipline I keep applying to other people's reports, and I did not apply it to a sentence I was quoting approvingly. ⇒ Filed properly as #16433, with the dedup's positive control recorded (the sweep's own originating cards #3456 and #4469 came back, so the query reached the right neighbourhood). It is a live trap: What round 2 fixed, verified against the treeBoth rewordings are now true, and — the part that mattered — neither mints a live count. "Eight" survives only as a frozen property of a deleted literal:
The historical claims were each verified against Landing checks so far: 4 commits, 0 closing keywords; complete ⭐ One environmental finding worth keepingRound 2 recorded, rather than smoothing over, that ⇒ Generated by Claude Code |
Fixes #15890
automation-runs-query-validation.test.tscarried a row named "forwards every declared ExecutionStatus member" whose members were eight string literals typed into the test body. The list was identical to the enum on the day it was written and short by one the dayrefusedwas appended (#14945), so the row went on passing while covering eight of nine — a test named for a property it had stopped measuring, green the entire time.What changed
Three sites, all in
packages/runtime:domains/automation-runs-query-validation.test.tsit.each([...8 literals])it.each(ExecutionStatus.options)query-param.tsExecutionStatusmembers"ExecutionStatusitself — the enum rather than a copy of its members — and when that stopped being a copydomains/automation.tsz.enum([...8 ExecutionStatus members])"ExecutionStatus.optional()" — the enum itself — and when it started saying thatReading the vocabulary is the discipline both neighbours already apply: the boundary hands
parseEnumParamthat same array (domains/automation.ts), andautomation-api.zod.test.tsturned its own inline copy into the same read under #7359.Deliberately not "nine". Swapping a stale literal for a fresh one re-arms the identical trap and reads authoritative the whole time. Each site now says what the set is defined by, not how many are in it — the same correction #16414 made, where the accepted repair also declined to mint a replacement number.
The appended-last invariant is untouched.
execution.zod.tsreserves.optionspositions for readers that index them ("Appended last so every reader that indexes.optionskeeps its positions"). Iterating.optionsonly reads it; nothing here sorts or re-seats it. The two positional readers (execution.test.ts's.at(-1)and.slice(0, 8)) are unchanged and still pass.Round 2 — a false sentence this branch introduced, and three smaller corrections
An independent review (5563157456) returned PASS with four non-blocking findings. The pin, the ablation and the population sweep all held and are unchanged. Commit
bf70dbf5a34carries the repairs and quotes each corrected sentence verbatim, because the squash body is composed from commit messages rather than from this one.1. The comment rewrite replaced a true sentence with a false one
The first pass wrote, of
ListRunsRequestSchema, "has always declared it — asExecutionStatus.optional(), the enum itself rather than a copy of its members" (domains/automation.ts) and "bounds it toExecutionStatusitself — the enum, never a copy of its members" (query-param.ts). Both are false about history — and the sentence the first one replaced, "has always declared it (z.enum([...8 ExecutionStatus members]).optional())", was accurate about it. Repairing a stale count introduced a falsehood about history, on the one card whose entire subject is untrue claims about this vocabulary.Verified against
git show, onpackages/spec/src/api/automation-api.zod.ts:git show 5e685a7d6f7:...— at the schema's introduction (2026-02-21) the line readstatus: z.enum(['pending', 'running', 'paused', 'completed', 'failed', 'cancelled', 'timed_out', 'retrying']).optional(). An inline copy of eight members.git show cf7c69421a7^:...— byte-for-byte the same literal the day before finding:ListRunsRequestSchema.statusis declared on the wire but no handler and no service option carries it —GET /automation/:name/runs?status=failedsilently lists every run #7359.git log -Son that literal, same path — exactly two commits touch it: the introduction, andcf7c69421a7, the landed finding:ListRunsRequestSchema.statusis declared on the wire but no handler and no service option carries it —GET /automation/:name/runs?status=failedsilently lists every run #7359 (2026-08-11), which replaced the copy withExecutionStatus.optional()in the same commit that made this boundary read the parameter.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 literal
cf7c69421a7deleted, frozen, and it is what makes the sentence explain itself rather than assert itself.cf7c69421a7. The review's2af301e4a21is the same change's pre-squash branch commit and is not an ancestor oforigin/main, so it resolves for nobody readingmain.2.
distIS on the suite's resolved path"No
distis on the resolved path" was true of the two ablation legs and false of the suite. Measured on a fresh worktree with nothing built, the suite cannot collect the file at all:http-dispatcher.ts, which the test imports, imports@objectstack/observability;packages/runtime/vitest.config.tscarries no alias entry for that package; itsexportsmap serves./dist/index.js. So the suite needs a built dependency closure, and this round built one (pnpm --filter '@objectstack/runtime^...' build) before re-running.What survives is the narrower, true claim: both ablation legs are source-resolved — the mutated
domains/automation.tsis reached by relative import inside the package, andExecutionStatusthrough the config's@objectstack/spec/*to../spec/src/*rule — so no rebuild leg applies to them andablation-dist-preflightdoes not.3.
TERMINAL_RUN_STATUSESis not 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 toExecutionStatus.It was closed before
refusedexisted, andrefused— "Terminal, never resumed" — is not in it. Once lane 2 (#15788) makes a run actually reach that status, a still-pending approval on it reads as alive to that sweep: a reader of this vocabulary that has not learned the new member, owned by therefusedlanes rather than by this card. What does not change is the sweep's place in this PR: a four-memberReadonlySetis not a stale copy of the nine-member enum, so it is still not a site this change touches.4. "hours ago" was not hours
#16414 landed as
0ea5f9d9f79at 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 of my own.Round 2 is comment prose only. Every line the round changed in the two
src/files begins//or*; no executable line moves, and the test file is untouched by it.The acceptance test is that the pin can REDDEN
A pin reading
ExecutionStatus.optionspasses trivially, so the pin was ablated rather than merely run. Direction and the identity of the failing row were written down before the run. Settled in round 1 and not redone here.parseEnumParamindomains/automation.tswas handed a hand-copied eight-member array in place ofExecutionStatus.options, exactly reproducing the defect this PR removes.?status=refused— and nothing else moving.Tests 1 failed | 47 passed (48). The single failure isforwards every declared ExecutionStatus member — ?status=refused, raisingValidationError: Invalid `status` query parameter — expected one of pending, running, paused, completed, failed, cancelled, timed_out, retrying, received "refused"throughparseEnumParamatquery-param.ts:227.Mutation and restore were both proven on disk, not read off an exit code: the anchor went 1 to 0 and the injected text 0 to 1, the blob moved
1a69cc22toa1d3cce0, and the restore undertrap ... EXIT INT TERMbrought the path back to blob1a69cc22— byte-identical to itsHEADblob — withgit diff HEADandgit status --porcelainboth empty.The row count is 9 because it was read
--reporter=verbosenames all nine generated rows, one per member, ending in?status=refused. The ninth row exists only becauserefusedis in the enum. Re-measured this round at0688d68edf5: nine rows,?status=cancelled · completed · failed · paused · pending · refused · retrying · running · timed_out.The suite is genuinely type-checked, not excluded-tests green:
packages/runtime/tsconfig.jsonexcludes**/*.test.ts, so plaintsc --noEmitsays nothing about this file. Under the test-layer programtsconfig.test.json,tsc --noEmit --listFilesputs all three edited files in a 1406-file program, and none of the 191 ledgered errors is in any of them.Population, swept rather than assumed
The card named three sites; that was treated as a lead, not a total. Every tracked non-dist file was scanned for windows carrying four or more of the nine member spellings, and separately for count words adjacent to
ExecutionStatus. The matcher's positive control is this file's own pre-change text, which it reports at eight of nine withrefusedmissing — and which it stops reporting after the change.Outside the three sites above, no live stale copy:
execution.test.ts's eight-member list is the deliberate prefix pin for the appended-last invariant,AUTOMATION_RESULT_STATUSESis a different five-member union held exhaustive by a type-level equality, plugin-approvals'TERMINAL_RUN_STATUSESis the four-member closed set described in round 2 above, and the generated reference docs carry all nine. Commit messages were swept too: the landed #7359 bodies say "re-listing its eight members inline", which described the tree accurately at the time and is immutable history — and is the same recordgit log -Sconfirms above.One stale copy does survive, in
docs/qa/platform-checklist/areas/automation.json. It is filed as #16424 and deliberately not edited here — a JSON checklist has no enum to read, so its repair is an authoring decision, and it sits behind a different gate family.Verification
All at⚠️ Recorded rather than smoothed over:
0688d68edf5, this branch's head after mergingorigin/mainin.git rev-parse origin/mainread0a038cc06d9moments before the merge, but the merge's second parent isbf3d6130f3a— a sibling worktree's fetch advanced the sharedrefs/remotes/origin/mainin between, so six commits came in where that reading named four. One of them,2025b1f7ecd(#16363), lands inpackages/runtime(src/dispatcher-error-vocabulary.ts), which is why the package's full typecheck — both layers — was re-run and not only the pin; its full test suite is CI'sTest Core, on the generation the queue rebuilds. Shared box, so the wall-clock absolutes below are not idle-box figures.pnpm --filter '@objectstack/runtime^...' build— green, and required: see round-2 finding 2 above.pnpm --filter @objectstack/runtime exec vitest run --reporter=verbose src/domains/automation-runs-query-validation.test.ts— 48 passed (48), 1 file, with the nine member rows named above.pnpm --filter @objectstack/runtime typecheck— green on both layers;check:test-typecheckprints OK at 27 files / 191 errors / 69 pinned signatures, the unchanged ledger baseline.node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstackderives from the change set (3 paths vs merge basebf3d6130f), plus the 39 Artifact rosters and the 10 Declared WIDE population families the Reconciliation line omits. 94 unique commands, exit code captured before any pipe: 89 green, 5 NOT MEASURED, zero findings.check-partof-closing-keywordwas re-run with realPR_BODYandPR_COMMITS_FILErather than left at itsNOT WIREDexit: green, over this body and the branch's 4 commit messages (two of them merges oforigin/main). They carry no card-relation trailer and zero closing keywords; the only closing keyword on this PR isFixes #15890, on line 1 of this body, which is parsed separately from them. The complete set of card numbers the squash will concatenate is finding:ListRunsRequestSchema.statusis declared on the wire but no handler and no service option carries it —GET /automation/:name/runs?status=failedsilently lists every run #7359, A flow cannot REFUSE with per-record text: the only channel that interpolates is a screendescription, and a message-only screen still renders Submit and toasts "completed" #14945, service-automation: honouroutcome: 'refused'on the flowendnode — a terminalrefusedrun status (distinct fromfailed) with the interpolated message persisted on the run (lane 2 of the #14945 ruling 2′) #15788, runtime: the ?status= forwarding pin hand-copies the eightExecutionStatusmembers — withrefusedappended (#14945) it covers eight of nine and stays green #15890, docs(cli): state the measured platform-id width at four citation sites #16414 — five, all verb-less.check-single-claim-pathsprintsNOT WIREDwithout a PR number;check:react-declaration-parityneeds an objectui manifest this repo holds no copy of;check:dual-build-cjs-loads,check:published-readme-exportsandcheck:type-check-debtprintPREREQUISITE NOT MET(exit 3) because they read built output across the whole tree and only@objectstack/runtime's closure was built. CI's Build Core job supplies what they need.eslint --no-inline-config --format jsonover the three changed files reports 3 files linted, 0 errors, 0 warnings. The narrowing is a measurement becauseeslint.config.mjsstates, with its own positive control, that this repo "never enables type-aware linting (noparserOptions.project, no typed@typescript-eslintrules) for ANY file, test or not" — so a three-file diff cannot move the verdict on any file it does not touch.Release note
No changeset. Test-only plus comment text: no behaviour, no asserted value and no minted count changes, so this PR releases nothing from any package and carries
skip-changeset.Attribution: authored by Claude Code in session
session_01D47qPfEWVPmhguWgBZCi5N; the same id rides every commit on this branch as aClaude-Session:trailer.