Commit 2af301e
committed
fix(spec,runtime,service-automation,client):
Fixes #7359. Triage's option 1 (ENFORCE), as ruled by the PM.
## What was measured
Relocated by content on the branch point (afdc6ea), not by triage's `8a9c079`
line numbers. All three layers confirmed still defective:
- `packages/spec/src/api/automation-api.zod.ts:274` — `ListRunsRequestSchema`
declares `status: z.enum([...8 members]).optional()`, described as "Filter by
execution status".
- `packages/spec/src/contracts/automation-service.ts:416` —
`listRuns?(flowName, options?: { limit?: number; cursor?: string })`. No slot.
- `packages/runtime/src/domains/automation.ts:462` — the runs branch built
`{ limit, cursor }` only. `status` never left the HTTP layer.
So `?status=failed` was dropped silently and the caller got 200 + every run of
the flow, capped by `limit`. A monitoring caller paging for failures read the
first 20 runs of any status and concluded those were the failures.
The existing test `automation-runs-query-validation.test.ts` carried an explicit
pin of that behaviour ("`?status=failed` is ignored, not refused"), written by
#7300 which deliberately declined to decide between honouring and retiring the
key. That pin is SUPERSEDED here — replaced by cases asserting the opposite on
the same input, not deleted silently.
## What changed, and why
1. **Contract** (`contracts/automation-service.ts`) — `status?: ExecutionStatus`
added to the `listRuns` option. One optional key, no breaking change.
2. **Boundary** (`runtime/src/domains/automation.ts`, runs branch ONLY) — reads
the parameter through the shared `query-param.ts` helper, as directed. That
module had no enum gate, so this adds `parseEnumParam` alongside the existing
boolean/integer/string ones rather than hand-rolling a comparison at the call
site. It refuses in the house shape: `validationFailure` -> 400
`VALIDATION_FAILED` (ADR-0112) + `details.fields[]`. No new error vocabulary
— ADR-0114's closed catalog already carries `invalid_option` ("not a member
of the field's declared options") for a non-member, and `invalid_type` for a
value that was never a single string (repeated `?status=a&status=b`,
structured `?status[$ne]=x`), which is the mapping `parseStringParam` already
makes for that same condition.
3. **Engine** (`service-automation/src/engine.ts`) — filters at the MERGE point.
## Which store each filter arm covers
The dispatch called out that a one-armed filter is the same class of wrong
answer as the bug. It is applied once, to the merged map, which covers both:
- in-memory ring buffer (`this.executionLogs`) — the live half;
- durable rows (`store.listHistory`) — the half that survives a restart.
Filtering after the merge rather than on each arm is deliberate and load-bearing
for a second reason I found while writing it: `executionLogs` holds MORE THAN
ONE entry per run id (a run that pauses appends 'paused', then its terminal
entry), and the merge is what collapses them to the freshest. Filtering the arms
before that collapse drops the terminal entry for `?status=paused` and lets the
stale 'paused' one survive — every approval/screen/wait run that had since
completed would report itself as still paused. That is precisely the defect
`run-history.test.ts`'s "latest entry wins" block pins for `getRun`, and I very
nearly re-introduced it one method over; my first draft filtered each arm.
There is now a test for it.
KNOWN, DOCUMENTED LIMIT (not fixed, not silently narrowed): the durable arm's
window is still `listHistory(flowName, limit)` — that store method has no status
slot, so the filter is applied to the rows that come back rather than pushed
down. A status filter can therefore return fewer than `limit` matches while
older matching rows exist. This is the merge's pre-existing shape (durable was
already capped at `limit` before the sort-and-slice); closing it properly is a
store-contract change and belongs in its own card. What it never does is return
a run of another status. Stated in code, changeset, and here rather than left
for someone to discover.
## How I proved the tests can fail
Every new test was mutated to red and reverted. Six mutations:
| mutation | red |
|-------------------------------------------------|----------------------------------------|
| handler drops `status` from the options object | 20 runtime tests |
| `parseStringParam` instead of `parseEnumParam` | 6 runtime (refusals + empty spelling) |
| engine ignores `status` entirely | 4 engine tests |
| engine filters the IN-MEMORY arm only | 3 engine (incl. the durable one) |
| engine filters the DURABLE arm only | 3 engine (incl. the in-memory one) |
| engine filter made non-optional | preservation test + 4 pre-existing |
| schema re-lists members, drops `retrying` | the new spec drift pin |
The one-armed mutations are the two that matter for the dispatch's warning, and
both are caught.
This also FOUND A WEAK TEST: my first "merged listing" case used a fixture whose
runs were all `failed`, so it passed even with the filter removed entirely. It
now seeds one status per store (a durable failure + a live success under one
flow name) and asserts each answer comes from a different store and neither
leaks the other's run. It fails under all three engine mutations.
## Decisions the dispatch did not specify
1. **Empty `?status=` means NO filter, not a 400.** `parseIntegerParam`'s falsy
gate, not `parseBooleanParam`'s refusal. Reason: the prior answers differ in
kind. `?read=` used to serve the wrong HALF of the inbox, so refusing it was
a strict improvement; `?status=` already served every run, which is exactly
what "no filter" means, so it had a defensible answer that must not become a
new 400. It is also what an "All statuses" `<select>` submits.
2. **`ListRunsRequestSchema` now references `ExecutionStatus`** instead of
re-listing its eight members inline. I had written a comment claiming the
boundary and the wire could not drift; that was not true as written, since
the schema held a hand-copied duplicate. Made it true rather than softening
the comment. Identical members, so no wire change — plus a spec test pinning
that every `ExecutionStatus` member parses, which fails if the lists diverge.
3. **The typed client can send it** (`packages/client`). The issue names the SDK
gap as the reason nothing had tripped over this. Leaving it out would make
the enforced filter reachable only from raw HTTP, while the Runs view triage
cited as the real consumer goes through this client. Additive optional
param on both `automation.listRuns` and `automation.runs.list`. This is a
fourth package beyond the three the dispatch listed — flagging it explicitly;
it is additive and does not touch `domains/automation.ts`, so it carries no
conflict risk with #7360.
## Constraints honoured
- Descriptor routes (`GET /actions`, `GET /connectors`) NOT touched — #7360's
diff in this file will not conflict; my change is confined to the runs branch.
- `content/docs/releases/**` not touched. Changeset added instead.
- Worktree-first (`../objectstack-7359`); no `git stash` at any point.
## Gates
- `pnpm lint` — clean
- `pnpm typecheck` — 126/126 tasks
- `packages/spec` — 373 files, 9790 tests
- `packages/runtime` — 121 files, 1939 tests
- `packages/services/service-automation` — 73 files, 895 tests
- `packages/client` — 21 files, 279 tests
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WPDMabeKZnjXeoG41wb4rKGET /automation/:name/runs?status= filters instead of being dropped (#7359)1 parent afdc6ea commit 2af301e
10 files changed
Lines changed: 535 additions & 33 deletions
File tree
- .changeset
- packages
- client/src
- runtime/src
- domains
- services/service-automation/src
- spec/src
- api
- contracts
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
83 | 83 | | |
84 | 84 | | |
85 | 85 | | |
| 86 | + | |
86 | 87 | | |
87 | 88 | | |
88 | 89 | | |
| |||
3130 | 3131 | | |
3131 | 3132 | | |
3132 | 3133 | | |
3133 | | - | |
| 3134 | + | |
3134 | 3135 | | |
3135 | 3136 | | |
3136 | 3137 | | |
3137 | 3138 | | |
3138 | 3139 | | |
| 3140 | + | |
| 3141 | + | |
| 3142 | + | |
| 3143 | + | |
| 3144 | + | |
3139 | 3145 | | |
3140 | 3146 | | |
3141 | 3147 | | |
| |||
5261 | 5267 | | |
5262 | 5268 | | |
5263 | 5269 | | |
5264 | | - | |
| 5270 | + | |
5265 | 5271 | | |
5266 | 5272 | | |
5267 | | - | |
| 5273 | + | |
5268 | 5274 | | |
5269 | 5275 | | |
5270 | 5276 | | |
5271 | 5277 | | |
| 5278 | + | |
| 5279 | + | |
5272 | 5280 | | |
5273 | 5281 | | |
5274 | 5282 | | |
| |||
Lines changed: 122 additions & 20 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
3 | 3 | | |
4 | | - | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
5 | 16 | | |
6 | 17 | | |
7 | 18 | | |
| |||
132 | 143 | | |
133 | 144 | | |
134 | 145 | | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
135 | 200 | | |
136 | 201 | | |
137 | 202 | | |
| |||
141 | 206 | | |
142 | 207 | | |
143 | 208 | | |
144 | | - | |
145 | | - | |
146 | | - | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
147 | 212 | | |
148 | 213 | | |
149 | 214 | | |
150 | | - | |
151 | | - | |
| 215 | + | |
| 216 | + | |
152 | 217 | | |
153 | 218 | | |
154 | 219 | | |
155 | 220 | | |
156 | | - | |
157 | | - | |
158 | | - | |
159 | | - | |
160 | | - | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
161 | 226 | | |
162 | 227 | | |
163 | | - | |
164 | | - | |
165 | | - | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
166 | 231 | | |
167 | 232 | | |
168 | 233 | | |
| |||
180 | 245 | | |
181 | 246 | | |
182 | 247 | | |
183 | | - | |
184 | | - | |
185 | | - | |
186 | | - | |
187 | | - | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
188 | 261 | | |
189 | 262 | | |
190 | 263 | | |
191 | | - | |
| 264 | + | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
| 271 | + | |
| 272 | + | |
| 273 | + | |
| 274 | + | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
| 281 | + | |
| 282 | + | |
| 283 | + | |
| 284 | + | |
| 285 | + | |
| 286 | + | |
| 287 | + | |
| 288 | + | |
| 289 | + | |
| 290 | + | |
| 291 | + | |
| 292 | + | |
| 293 | + | |
192 | 294 | | |
193 | 295 | | |
194 | 296 | | |
| |||
0 commit comments