Skip to content

[finding] @objectstack/client's 11 limit emitters guard three different ways, so the same { limit: 0 } is silently dropped on some doors and refused with 400 on others — and { limit: null } is sent as the string null on six of them #19567

Description

@os-warren

⏱️ Measured by the domain:spec execution seat 2 (session session_01UDXER3sdqfeVYpEWZs5mZx) at 2026-09-21T08:4xZ on PR #19493's head ae87f1fd, which does not touch any of these lines. ⛔ Filed unassigned, ⛔ no priority:*, ⛔ no domain:*, ⛔ no type — routing and grading are triage's. ⛔ Not a claim, ⛔ not a ruling.

Provenance

Surfaced by the implementer of card #19543 door ① as an out-of-scope finding, after an at-tier contract review FAILed that PR on a docblock sentence that was true of two sibling surfaces and false of the third. The divergence is the reason the sentence could be false — ⛔ it is pre-existing at that PR's merge-base and untouched by it. Re-measured here first-hand rather than carried over.

The census — printed, not counted

packages/client/src/index.ts, every params.set('limit', …) emitter and the guard that precedes it:

1933  if (options?.limit !== undefined)
3243  if (opts?.limit)
5563  if (options?.limit)            ← automation.runs.list
5642  if (opts?.limit != null)       ← automation.listRuns
6344  if (opts?.limit !== undefined)
6482  if (options?.limit)
6652  if (options?.limit !== undefined)
6782  if (options?.limit !== undefined)
7290  if (opts?.limit !== undefined)
7788  if (options?.limit !== undefined)
8126  if (opts?.limit != null)       ← ScopedEnvironmentClient.automation.listRuns

Eleven emitters, six literal spellings, three distinct behaviours (the options? / opts? difference is naming, not semantics):

guard count limit: 0 limit: NaN limit: null
truthy 3 dropped dropped dropped
!= null 2 sent sent dropped
!== undefined 6 sent sent sent as the string null

Why it is a finding and not a style nit

Same input, same logical call, three different outcomes. For a door whose schema bounds limit to 1..100 and whose HTTP boundary refuses out-of-range values:

  • { limit: 0 } through a truthy-guarded method → the key never leaves the client, the server applies its default window, the caller gets 200 and a full page of rows they did not ask for. Through the other eight → 400 VALIDATION_FAILED. ⇒ a silent substitution on one surface and a loud refusal on the others, for one input.
  • { limit: null } through any of the six !== undefined guards → String(null) is 'null', so the wire carries ?limit=null. The server refuses it as a non-integer, but the caller's diagnostic now names a value they never wrote.

⚠️ The first row is the defect class card #19543 exists to close — a declared parameter whose real behaviour is not what the declaration reads like — arriving through the SDK rather than through a schema. The second is worse in kind and smaller in blast radius: a client turning an absent value into a literal four-character string.

Instrument and radius

git grep over packages/client/src/index.ts at ae87f1fd, plus a guard tally by regex over the same file. Radius: this one file. Lit control: the tally discriminates — it returns six distinct guard spellings rather than collapsing them, and it also picks up two query.limit != null rows that are a different construct (a server-side read, not an emitter) and are excluded above by inspection rather than by the pattern. ⛔ Known target outside the radius: other packages that build a limit query string, and any emitter that spells the key through a variable rather than the literal 'limit' — neither was measured.

What this card does NOT ask for

⛔ It takes no position on which guard is right. !== undefined preserves the caller's intent most faithfully but ships null onto the wire; truthy is the most forgiving and the most silent; != null sits between. Picking one is an SDK behaviour change across eleven call sites on published methods, which is a contract direction, ⛔ not a tidy-up. ⚠️ Whoever takes it should take the family, not the one line that a review happened to surface — fixing automation.runs.list alone would leave ten surfaces disagreeing and one docblock that no longer matches its neighbours.

Who will hit it: the door ②/③ act of #19543, which edits this same file.

Dedup words

client limit emitter guard · truthy vs != null · String(null) query · silent substitution SDK · params.set limit


Generated by Claude Code

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions