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

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, finding disposition, unassigned.

Mechanism

packages/runtime/src/domains/automation.ts, the GET /:name/runs branch (line ~434 as of origin/main @ 1da1f32; relocate by content, the line will drift):

const options = query ? { limit: query.limit ? Number(query.limit) : undefined, cursor: query.cursor } : undefined;
const runs = await automationService.listRuns(name, options);

This is character-for-character the coercion #6928 was filed against — Number(query.limit) with nothing checking the result — so ?limit=abc yields NaN and listRuns is called with { limit: NaN }. Whether that then reaches a driver as find({ 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 (invalidQueryParam plus the three per-parameter parsers) are deliberately module-local to notifications.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 to validationFailure in packages/runtime/src/validation-failure.ts rather 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:

refuse with the house validationFailure(...) (400 VALIDATION_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-0114 FieldErrorCode naming the parameter — no new spec spelling needed, VALIDATION_FAILED is already a registered ledger code for @objectstack/runtime;
preserve every value that has a defensible answer today, byte for byte, and pin that with tests — including whatever the automation service does with an out-of-range number, which is its declared behaviour and not this layer's business;
cursor on this route is passed through unchecked as well, so it is worth the same probe while in there — unlike the notifications route, where cursor is a spec-tombstoned key that is merely ignored.

Dedup

Searched open issues for automation NaN limit, domains/automation.ts query, and limit 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.

Findings triage: promoted — finding → pm:queue (routing domain:cli kept, landing packages/runtime per 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=abc coerces to NaN and flows through listInbox's clamp into data.find({ limit: NaN }) — unvalidated query coercion on the notifications route #6928): invalidQueryParam + per-parameter parsers exist module-local in notifications.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=abc coerces to NaN and flows through listInbox's clamp into data.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 to validationFailure in validation-failure.ts) is implementation-layer, per the card's own framing.
Dedup: filer's search confirmed; finding: GET /api/v1/notifications?limit=abc coerces to NaN and flows through listInbox's clamp into data.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 covers automation.ts.
target:v17: not applied — raw-HTTP-only exposure, no data loss (same grading as finding: GET /api/v1/notifications?limit=abc coerces to NaN and flows through listInbox's clamp into data.find({ limit: NaN }) — unvalidated query coercion on the notifications route #6928's family).

本评论来自分诊座位 Routine（#5474 试点），不构成认领。

Generated by Claude Code

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 (beside validationFailure), 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), so notifications.ts has no holder; this lane's in-flight (#7020 / #6349 / #6719) is file-disjoint (⚠️ #6719 is in the same runtime/src/domains/ directory but touches only data-path-object.test.ts).

Premise re-verified at dispatch time on origin/main: automation.ts:434 still carries the bare Number(query.limit) coercion, exactly as triage measured at 62b6a2f.

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 shared packages/runtime/src/query-param.ts (+140), and notifications.ts −115/+import — I checked that hunk line-by-line: it is a pure extraction of invalidQueryParam / 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 cursor probe the ruling asked for produced a real second refusal: the passthrough handed non-strings into a slot the contract types cursor?: 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 ?? 20 does not catch NaN, and .slice(0, NaN) is [], so ?limit=abc answered 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.status declared 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 shared parseStringParam).

Generated by Claude Code

fix(runtime): refuse malformed `limit`/`cursor` on GET /automation/:n…

fix(spec,runtime,service-automation,client): `GET /automation/:name/r…

fix(runtime): refuse a non-string descriptor filter instead of servin…

feat(automation): GET /automation/:name/runs retires `cursor` and com…

Navigation Menu

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

Mechanism

Why it is a separate card and not part of PR #7299

Suggested shape, if promoted

Dedup

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

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

Mechanism

Why it is a separate card and not part of PR #7299

Suggested shape, if promoted

Dedup

Activity

os-zhuang commented on Aug 10, 2026

os-help commented on Aug 10, 2026

os-help commented on Aug 10, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

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