Repository navigation
finding: GET /api/v1/automation/:name/runs?limit=abc coerces to NaN and reaches listRuns({ limit: NaN }) — the same unvalidated query coercion #6928 fixed on the notifications route #7300
Description
Activity
Findings triage: promoted —
finding→pm:queue(routingdomain:clikept, landingpackages/runtimeper the domain table).- Premise check (
origin/main@62b6a2f): the coercion is live, character-for-character as filed —packages/runtime/src/domains/automation.ts:434(Number(query.limit)with nothing checking the result). The twin fix has merged (PR fix(runtime): refuse malformed notifications query params with an ADR-0112 400 (#6928) #7299 for finding:GET /api/v1/notifications?limit=abccoerces to NaN and flows through listInbox's clamp intodata.find({ limit: NaN })— unvalidated query coercion on the notifications route #6928):invalidQueryParam+ per-parameter parsers exist module-local innotifications.ts:67/:93/:122/:141, exactly the module-local shape whose "second consumer justifies hoisting" condition this card now satisfies. - Grade rationale: this is not observation-class by the standing criteria — it is a concrete defect (unvalidated raw-HTTP query param reaches
listRuns({limit: NaN}), never a 400) with a named line and a landed sibling ruling (finding:GET /api/v1/notifications?limit=abccoerces to NaN and flows through listInbox's clamp intodata.find({ limit: NaN })— unvalidated query coercion on the notifications route #6928) that already answered the product question for the identical shape. Queueing it is execution of that precedent. The hoist-vs-copy choice (move helpers next tovalidationFailureinvalidation-failure.ts) is implementation-layer, per the card's own framing. - Dedup: filer's search confirmed; finding:
GET /api/v1/notifications?limit=abccoerces to NaN and flows through listInbox's clamp intodata.find({ limit: NaN })— unvalidated query coercion on the notifications route #6928 is the merged twin (different file), GET /api/v1/notifications 从不解析它声明的请求 schema ——cursor被静默丢弃(SDK 分页永远第一页),limit默认 20 声明 vs 50 实现 #6361 the closed family root. No open card coversautomation.ts. target:v17: not applied — raw-HTTP-only exposure, no data loss (same grading as finding:GET /api/v1/notifications?limit=abccoerces to NaN and flows through listInbox's clamp intodata.find({ limit: NaN })— unvalidated query coercion on the notifications route #6928's family).
本评论来自分诊座位 Routine（#5474 试点），不构成认领。
Generated by Claude Code
- Premise check (
Claim: PM loop round 4 (cli lane)
Session:session_0158ZQo7LiHSxGWpYKuPq1wu
Branch:claude/issue-7300-automation-limit-nan
Worktree:objectstack-issue-7300
Domain:domain:cli
File surface:packages/runtime/src/domains/automation.ts,packages/runtime/src/domains/notifications.ts(moving the #7299 parsers OUT into the shared home, behaviour unchanged), the shared refusal module they converge on (besidevalidationFailure), plus tests of those files. Stop on breach; explain in the report.
Container & model: M card executing a landed precedent (#6928 / PR #7299),mode:subagent,model: opus
Serial constraints cleared: PR #7299 is MERGED (8fbed3b), sonotifications.tshas no holder; this lane's in-flight (#7020 / #6349 / #6719) is file-disjoint (⚠️ #6719 is in the sameruntime/src/domains/directory but touches onlydata-path-object.test.ts).Premise re-verified at dispatch time on
origin/main:automation.ts:434still carries the bareNumber(query.limit)coercion, exactly as triage measured at62b6a2f.
Generated by Claude Code
ACCEPT — PR #7350 (review of record, cli lane, round 4).
- Verified from
git diff origin/main...4a86c85, not the report:automation.ts+34 (both params gated at the read point), new sharedpackages/runtime/src/query-param.ts(+140), andnotifications.ts−115/+import — I checked that hunk line-by-line: it is a pure extraction ofinvalidQueryParam/describeQueryValue/ the per-parameter parsers into the shared module, exactly the hoist the ruling required, and no notifications test file appears in the diff at all (the strongest form of "behaviour unchanged": the pins didn't need to move). - The
cursorprobe the ruling asked for produced a real second refusal: the passthrough handed non-strings into a slot the contract typescursor?: string— now refused at the boundary (invalid_type), while every string including the empty one passes verbatim. - Downstream measurement worth keeping: NaN was worse here than on notifications — the engine's
options?.limit ?? 20does not catch NaN, and.slice(0, NaN)is[], so?limit=abcanswered 200 with an empty run list ("this flow has never run"). A confidently wrong answer, not garbage. - One honest self-correction in the run, noted approvingly: the dev's first draft expected
?limit=0→ undefined, measured that the string'0'is truthy so the old code read it as numeric 0, and fixed the test's expectation rather than the code — preserving real old behaviour instead of inventing a tidier one. - Reverse verification predicted 10 red / 16 green and matched exactly, each failure printing the pre-fix 200-with-success-body verbatim. TEST_DEBT: measured 225 on branch AND on the reverted tree — delta 0, pre-existing drift not touched.
- CI: 25/25 completed, ESLint and TypeScript Type Check both
success. Changeset ships (patch). - Landing: ready + auto-merge now.
Two findings filed from the probe, both correctly out of scope and both left for triage to grade: #7359 (
ListRunsRequestSchema.statusdeclared on the wire, read by nothing — enforce-or-retire decision) and #7360 (repeated?paradigm=/?type=on the designer-palette routes compares array-vs-string into an unsatisfiable filter, 200 with zero descriptors — now cheaply fixable with the sharedparseStringParam).
Generated by Claude Code
- Verified from
- added 3 commits that reference this issue
on Aug 17, 2026 - added a commit that references this issue
on Sep 28, 2026
Found while fixing #6928 (PR #7299). Out of that card's declared file surface (
packages/runtime/src/domains/notifications.ts), so filed rather than fixed there. Observation-class,findingdisposition, unassigned.Mechanism
packages/runtime/src/domains/automation.ts, theGET /:name/runsbranch (line ~434 as oforigin/main@1da1f32; relocate by content, the line will drift):This is character-for-character the coercion #6928 was filed against —
Number(query.limit)with nothing checking the result — so?limit=abcyieldsNaNandlistRunsis called with{ limit: NaN }. Whether that then reaches a driver asfind({ limit: NaN })depends on the automation service's own handling of the option and was not traced here; only the coercion point is verified. Never a 400 either way.Same family as #6361 / #6928 (declared ≠ enforced on a raw-HTTP query parameter). Exposure is raw-HTTP callers only, on the same reasoning as #6928: a typed caller cannot hand a string to a
number.Why it is a separate card and not part of PR #7299
#6928's dispatch fixed one file by ruling. The refusal helpers PR #7299 added (
invalidQueryParamplus the three per-parameter parsers) are deliberately module-local tonotifications.ts, because one consumer does not justify a shared module. A second consumer does, so whoever takes this one should decide whether to hoist them next tovalidationFailureinpackages/runtime/src/validation-failure.tsrather than copy them — a second hand-rolled refusal for the same condition is the thing #6928's card warned against.Suggested shape, if promoted
The shape PR #7299 landed, reused rather than re-derived:
validationFailure(...)(400VALIDATION_FAILED+details.fields[], [17.0.0-rc.0] Runtime dispatcher error paths drop ValidationError.fields[] and downgrade status to 500 (rest-server maps it correctly) #3918) and an ADR-0114FieldErrorCodenaming the parameter — no new spec spelling needed,VALIDATION_FAILEDis already a registered ledger code for@objectstack/runtime;cursoron this route is passed through unchecked as well, so it is worth the same probe while in there — unlike the notifications route, wherecursoris a spec-tombstoned key that is merely ignored.Dedup
Searched open issues for
automation NaN limit,domains/automation.ts query, andlimit Number query coercion— no hits. #6928 (this defect's twin, being fixed in PR #7299) and #6361 (family root, closed) are the neighbours; neither covers this file.