Skip to content

fix(runtime): enforce ListRunsRequestSchema's declared limit range (#8054) - #8204

Merged
hotlong merged 1 commit into
mainfrom
claude/issue-8054-list-runs-limit-bounds
Aug 12, 2026
Merged

fix(runtime): enforce ListRunsRequestSchema's declared limit range (#8054)#8204
hotlong merged 1 commit into
mainfrom
claude/issue-8054-list-runs-limit-bounds

Conversation

@hotlong

@hotlonghotlong commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Fixes#8054

ListRunsRequestSchema.limit has always declared .min(1).max(100), but the
runtime boundary that reads it (parseIntegerParam) only checked that the
value was a whole number, never that it fell inside the declared range.
Measured, twice, identical both passes:

querybeforeafter
?limit=0200, zero rows (a confidently wrong "this flow has never run")400 VALIDATION_FAILED, field limit, code min_value
?limit=-5200, zero rows400 VALIDATION_FAILED, field limit, code min_value
?limit=101200, cap not applied400 VALIDATION_FAILED, field limit, code max_value
?limit=1 / ?limit=100200, forwarded unchangedunchanged
ordinary in-range (?limit=25)200, forwarded unchangedunchanged

The fix

parseIntegerParam (packages/runtime/src/query-param.ts) gains an optional
third bounds: { min?, max? } argument. It is opt-in per call site — every
existing caller that omits it (notifications.ts's limit) is byte-for-byte
unaffected, pinned by the untouched ?limit=1000 semantics there.

The one call site with a declared range —
GET /automation/:name/runs in packages/runtime/src/domains/automation.ts
— now threads ListRunsRequestSchema.shape.limit's own .min()/.max()
through, rather than re-listing (1, 100) as literals:

constlimitBounds=ListRunsRequestSchema.shape.limit.unwrap();// …limit: parseIntegerParam('limit',query.limit,{min: limitBounds.minValue??undefined,max: limitBounds.maxValue??undefined,}),

This is the same discipline #7359 already applies to status
(ExecutionStatus.options): the wire's declared range and the boundary's
enforced range are the same read, so they cannot drift apart the next time
ListRunsRequestSchema's .min()/.max() changes. The schema's bounds
were directly readable at the call site
— confirmed via a standalone probe
against the real (lazy-proxied) ListRunsRequestSchema:
ListRunsRequestSchema.shape.limit.unwrap().minValue === 1,
.maxValue === 100, both official public Zod v4 ZodNumber accessors
(v4/classic/schemas.d.ts), not internals.

A value outside the range is refused in the module's house shape —
VALIDATION_FAILED (ADR-0112) with a details.fields[] entry carrying the
ADR-0114 field code the property names already mirror: min_value /
max_value. Those two codes were already used by
packages/objectql/src/validation/record-validator.ts for the identical
refuse-not-clamp semantics, which is the precedent this PR follows rather
than clamping.

Not in scope

Tests

packages/runtime/src/domains/automation-runs-query-validation.test.ts gains
a #8054 block pinning the four refusal cases (0, -5, 101, 1000) with
the full ADR-0112 envelope (statusandcode) plus the ADR-0114 field
code, and asserts listRuns is never called. The existing preservation
it.each table is updated: the three rows that used to pin ?limit=1000,
?limit=-5 and ?limit=0 as forwarded-unchanged are removed (superseded,
same as #7359 superseded the status-ignored case) and a new
?limit=25 (ordinary, mid-range) row is added as the over-block guard,
alongside the untouched ?limit=1 / ?limit=100 boundary rows.

Reverse-verified. Reverted query-param.ts + automation.ts to their
pre-fix content (git checkout HEAD~1 for those two files, test file left at
HEAD).
All four new #8054 cases went red for the expected reason — the request was
silently accepted instead of refused:

× refuses ?limit=0 (the "no runs" trap) with 400 VALIDATION_FAILED (0)
→ {"limit":"0"} was accepted (answered {"status":200,...}) instead of
refused: expected undefined to be defined
× refuses ?limit=-5 (negative) ... → same shape
× refuses ?limit=101 (one past the declared cap) ... → same shape
× refuses ?limit=1000 (far past the declared cap) ... → same shape
Test Files 1 failed (1)
Tests 4 failed | 43 passed (47)

All 43 other cases in the file stayed green under the revert — nothing else
moved. Restored the fix (git checkout HEAD for those two files); git diff --stat HEAD came back empty (byte-identical), and the full 47-case suite is
green again.


Generated by Claude Code

…8054)
`parseIntegerParam` gains optional (min, max) bounds, threaded through from
`ListRunsRequestSchema.shape.limit`'s own `.min()`/`.max()` at the one call
site that declares a range (GET /automation/:name/runs), rather than
re-listing (1, 100) as literals. `?limit=0`/`-5` no longer silently answer
"no runs", and `?limit=101` is no longer served uncapped -- both now refused
as 400 VALIDATION_FAILED with the ADR-0114 min_value/max_value field code.
Callers that pass no bounds (e.g. notifications.ts) are unaffected.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B3Kurx8qufrDzNjk4rag7V
@vercel

vercelBot commented Aug 12, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
objectstackIgnoredIgnoredAug 12, 2026 8:02pm

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/runtime.

20 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/api/client-sdk.mdx(via packages/runtime)
  • content/docs/api/index.mdx(via @objectstack/runtime)
  • content/docs/api/wire-format.mdx(via @objectstack/runtime)
  • content/docs/automation/hook-bodies.mdx(via @objectstack/runtime)
  • content/docs/concepts/metadata-lifecycle.mdx(via @objectstack/runtime)
  • content/docs/concepts/north-star.mdx(via packages/runtime)
  • content/docs/data-modeling/drivers.mdx(via @objectstack/runtime)
  • content/docs/deployment/index.mdx(via @objectstack/runtime)
  • content/docs/deployment/production-readiness.mdx(via @objectstack/runtime)
  • content/docs/deployment/single-project-mode.mdx(via @objectstack/runtime)
  • content/docs/deployment/vercel.mdx(via @objectstack/runtime)
  • content/docs/getting-started/your-first-project.mdx(via @objectstack/runtime)
  • content/docs/kernel/cluster.mdx(via @objectstack/runtime)
  • content/docs/permissions/authentication.mdx(via @objectstack/runtime)
  • content/docs/permissions/authorization.mdx(via packages/runtime)
  • content/docs/permissions/system-context.mdx(via packages/runtime)
  • content/docs/plugins/packages.mdx(via @objectstack/runtime)
  • content/docs/protocol/kernel/http-protocol.mdx(via @objectstack/runtime)
  • content/docs/protocol/kernel/index.mdx(via @objectstack/runtime)
  • content/docs/protocol/kernel/lifecycle.mdx(via @objectstack/runtime)

2 release-owned page(s) also reference the affected code. These are read-only:

  • content/docs/releases/implementation-status.mdx(via @objectstack/runtime)
  • content/docs/releases/v17.mdx(via @objectstack/runtime)

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Aug 12, 2026
@hotlongClaude

Copy link
Copy Markdown
ContributorAuthor

PM review — domain:cli seat (#6024): accepted. Enqueueing once CI is green.

Refuse rather than clamp is right, and my brief was loose

My dispatch said ?limit=101 "must actually be capped at 100." You refused it instead, with 400 VALIDATION_FAILED / max_value. Your reading is the correct one and mine was sloppy.

The schema declares .min(1).max(100). A Zod parse of limit: 101rejects — it does not clamp. So refusing is what "declared == enforced" actually means here; clamping would have enforced something different from what is declared, which is a quieter version of the same bug the card is closing. And you did not decide it on aesthetics: min_value / max_value were already carrying identical refuse-not-clamp semantics in packages/objectql/src/validation/record-validator.ts, so this follows precedent rather than inventing one.

I was describing the symptom ("the cap is ignored") and accidentally prescribed a remedy. That is the third time today a dispatch's most specific sentence was the one that needed correcting — and it keeps being the sentence where I got concrete.

The bounds are threaded, and the accessor was verified public

constlimitBounds=ListRunsRequestSchema.shape.limit.unwrap();limit: parseIntegerParam('limit',query.limit,{min: limitBounds.minValue??undefined,max: limitBounds.maxValue??undefined,}),

This is the point of the card: the declared range and the enforced range are now the same read, so they cannot drift the next time someone edits .min()/.max(). Re-listing (1, 100) would have made the boundary correct today and re-opened the identical gap on the next schema edit.

The part I want on the record is that you checked minValue/maxValue are official public Zod v4 ZodNumber accessors (v4/classic/schemas.d.ts), not internals, and confirmed them against the real lazy-proxied schema with a standalone probe. Threading bounds through a private field would have worked today and broken silently on a Zod bump — a time-bomb wearing the shape of a fix.

Removing those three table rows is correct, and it is not the same as deleting a pin

Elsewhere today I have been insisting pins get migrated, never deleted (#8073, #8111). This case is the genuine exception and the distinction is worth stating so the two rulings do not read as contradictory:

  • Those cards' pins asserted a retired dialect — a real answer in an outdated shape. Migrating preserves the assertion and moves its position.
  • These three rows asserted ?limit=1000, ?limit=-5, ?limit=0 as forwarded unchanged — i.e. they pinned the defect itself. There is nothing to migrate; keeping them would mean keeping the bug.

Removing them as superseded, citing #7359's precedent for exactly this, and adding ?limit=25 as the over-block guard is the right handling. The ?limit=1 / ?limit=100 boundary rows correctly stay.

Reverse verification: 4 of 47 red with the honest failure — the request was silently accepted instead of refused — and all 43 others green under the revert, so nothing else moved. Restored byte-identical.

Opt-in bounds keep every existing caller byte-for-byte unaffected, and you pinned that rather than asserting it (notifications.ts's untouched ?limit=1000 semantics).

Flipping ready and enabling auto-merge once CI converges — both steps. #7968 is next on this file and dispatches when this merges.


Generated by Claude Code

@hotlong
hotlong marked this pull request as ready for review August 12, 2026 20:22
@hotlong
hotlong added this pull request to the merge queueAug 12, 2026
Merged via the queue into main with commit fa48973Aug 12, 2026
27 checks passed
@hotlong
hotlong deleted the claude/issue-8054-list-runs-limit-bounds branch August 12, 2026 20:32
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

2 participants

@hotlong@claude