fix(gateway): reject invalid session history limit with 400 instead of coercing to 1

[tzy-17]

What Problem This Solves

The direct Gateway session-history endpoint silently treated invalid limit query values as 1. Requests such as ?limit=abc, ?limit=0, negative values, and decimals returned HTTP 200 with one message, hiding caller bugs.

Why This Change Was Made

The endpoint now uses parseStrictPositiveInteger from @openclaw/normalization-core/number-coercion and the shared Result type:

• omitted limit remains optional;
• empty, whitespace, malformed, zero, negative, and fractional values return HTTP 400 invalid_request_error;
• valid positive values remain bounded by MAX_SESSION_HISTORY_LIMIT;
• oversized unsigned all-digit values preserve the shipped clamp behavior added by fix(gateway): cap history when numeric limit is huge #104263.

The fix stays in the raw Node Gateway handler. It does not add a route framework or a shared query helper.

User Impact

Clients now receive an explicit HTTP 400 for invalid explicit limits instead of a misleading one-message success response. Valid requests are unchanged, including the existing oversized all-digit clamp to 1,000 messages.

Evidence

Exact maintainer head: df3793296fd5a8a5b9eab38c55362a889786e27f.

• Blacksmith Testbox tbx_01kxrgxeq6yk40m74jf3z76xt2: 9 focused Gateway HTTP tests passed for empty, whitespace, malformed, zero, negative, fractional, valid bounded (1, +1), and oversized-clamped values.
• Canonical oxfmt and git diff --check passed.
• Final uncommitted autoreview: clean, no accepted/actionable findings.
• check:changed passed formatting, core/core-test typechecks, changed-file lint, dependency checks, and architecture guards before the Testbox transport ended with exit 255 during later generic guards; no check failure was reported.
• The full session-history test file has unrelated sequence-number assertion drift. The same failure was reproduced independently on exact origin/main SHA 8a8706cb5d0448fe2a15b1afae81bfe419322e55, while all changed limit tests passed.
• Exact-head hosted CI is the final landing gate.

[clawsweeper]

Codex review: needs real behavior proof before merge. Reviewed July 17, 2026, 12:25 PM ET / 16:25 UTC.

Summary
The PR changes the direct Gateway session-history endpoint to return HTTP 400 for malformed or below-one limit values and adds two regression tests.

PR surface: Source +7, Tests +23. Total +30 across 2 files.

Reproducibility: yes. from source: current main maps non-numeric and below-one limit values to 1. The contributor has not supplied a real HTTP transcript, so runtime reproduction evidence remains incomplete.

Review metrics: 1 noteworthy metric.

• HTTP query contract: 1 changed (limit). Malformed and below-one values move from HTTP 200 coercion to HTTP 400, requiring explicit upgrade awareness.

Stored data model
Persistent data-model change detected: serialized state: src/gateway/sessions-history-http.test.ts, serialized state: src/gateway/sessions-history-http.ts. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🦐 gold shrimp
Result: blocked until stronger real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

• [P1] Add safe-integer rejection and a regression test for the oversized value.
• Post a redacted real Gateway HTTP transcript for malformed, zero, valid, and clamped limits.
• Obtain Gateway-owner acceptance of the intentional HTTP compatibility change.

Proof guidance:

• [P1] Needs stronger real behavior proof before merge: The PR body contains hand-written expected output rather than a real Gateway HTTP transcript, and its unsafe-integer claim contradicts the patch; add redacted terminal requests showing invalid values return 400 and valid or clamped values return 200, then update the PR body to trigger review or ask a maintainer for @clawsweeper re-review.

Risk before merge

• [P1] Existing clients that accidentally or intentionally send malformed or zero limits will change from HTTP 200 with a bounded response to HTTP 400, potentially breaking shipped callers despite preserving well-formed requests.

Maintainer options:

1. Fix validation, prove behavior, then approve (recommended)
   Require safe-integer coverage plus a redacted real HTTP transcript before a Gateway owner accepts the strict contract.
2. Keep the shipped permissive contract
   Pause or close the PR if existing malformed-input compatibility should remain supported.

Next step before merge

• [P1] The contributor must fix the repeated unsafe-integer defect and add real HTTP proof; a Gateway owner must then decide whether to accept the shipped 200-to-400 compatibility change.

Maintainer decision needed

• Question: Should the shipped session-history HTTP endpoint intentionally change malformed and below-one limit requests from a successful coerced response to HTTP 400?
• Rationale: The docs do not define malformed-query behavior, and the change can break callers relying on permissive coercion even though strict validation is cleaner.
• Likely owner: clay-datacurve — They introduced the direct session-history HTTP/SSE endpoint and its original request contract.
• Options:
  • Adopt strict validation (recommended): Fix the unsafe-integer gap, prove the real HTTP behavior, and intentionally accept the 200-to-400 compatibility change.
  • Preserve permissive behavior: Keep the shipped coercion contract and close this PR if compatibility outweighs strict input validation.

Security
Cleared: The focused validation and test diff adds no dependency, secret, permission, code-execution, or supply-chain surface.

Review findings

• [P2] Reject unsafe integer limits before clamping — src/gateway/sessions-history-http.ts:81

Review details

Best possible solution:

Reject non-digit, below-one, non-finite, and non-safe-integer limits before clamping; add focused coverage for every claimed invalid class, then approve the HTTP contract change only after a redacted real Gateway transcript proves invalid, valid, and clamped requests.

Do we have a high-confidence way to reproduce the issue?

Yes from source: current main maps non-numeric and below-one limit values to 1. The contributor has not supplied a real HTTP transcript, so runtime reproduction evidence remains incomplete.

Is this the best way to solve the issue?

No in its current form: strict validation is the right layer, but the implementation must reject unsafe integers before clamping and prove the actual endpoint behavior; the compatibility decision remains human-owned.

Full review comments:

• [P2] Reject unsafe integer limits before clamping — src/gateway/sessions-history-http.ts:81
99999999999999999 matches the digit regex and converts to a finite number, so this branch accepts it and Math.min returns 1000 instead of the claimed 400. This prior review blocker remains unfixed; require Number.isSafeInteger(value) and a regression test for the oversized input.
  Confidence: 0.99

Overall correctness: patch is incorrect
Overall confidence: 0.99

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 4bfaccafd62a.

Label changes

Label justifications:

• P2: This is a bounded Gateway API correctness change with limited surface but real client compatibility implications.
• merge-risk: 🚨 compatibility: Merging changes shipped malformed limit requests from successful coerced responses to HTTP 400 errors.
• rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
• status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR body contains hand-written expected output rather than a real Gateway HTTP transcript, and its unsafe-integer claim contradicts the patch; add redacted terminal requests showing invalid values return 400 and valid or clamped values return 200, then update the PR body to trigger review or ask a maintainer for @clawsweeper re-review.

Evidence reviewed

PR surface:

Source +7, Tests +23. Total +30 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	14	7	+7
Tests	1	23	0	+23
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	37	7	+30

What I checked:

• Current main behavior: Current main parses digit-only values with Number, coerces non-numeric or below-one values to 1, and clamps larger values to MAX_SESSION_HISTORY_LIMIT. (src/gateway/sessions-history-http.ts:71, 4bfaccafd62a)
• Unfixed prior finding: The PR checks Number.isFinite(value) but not Number.isSafeInteger(value), so 99999999999999999 remains accepted and clamped instead of rejected as claimed. (src/gateway/sessions-history-http.ts:81, 47b1c5423fc1)
• Regression coverage gap: The added tests cover only limit=abc and limit=0; no test covers the unsafe-integer input named in the PR body and prior review. (src/gateway/sessions-history-http.test.ts:740, 47b1c5423fc1)
• Public HTTP contract: Session-history documentation describes limit as a bounded message count that the server clamps, but does not define malformed-query rejection, making the 200-to-400 change compatibility-sensitive. Public docs: docs/concepts/session-tool.md. (docs/concepts/session-tool.md:83, 4bfaccafd62a)
• Feature provenance: The direct session-history HTTP/SSE endpoint was introduced by commit 7b61ca1 through Session management improvements and dashboard API #50101. (src/gateway/sessions-history-http.ts:1, 7b61ca1b0615)
• Shipped baseline: Latest release v2026.7.1 points at 2d2ddc4 and does not contain this open PR’s strict-validation commit, so the proposed HTTP 400 behavior is not shipped. (src/gateway/sessions-history-http.ts:71, 2d2ddc43d0dc)

Likely related people:

• clay-datacurve: Authored the merged session-management work that added the direct session-history HTTP/SSE endpoint in Session management improvements and dashboard API #50101. (role: introduced behavior; confidence: high; commits: 7b61ca1b0615; files: src/gateway/sessions-history-http.ts, src/gateway/sessions-history-http.test.ts, docs/concepts/session-tool.md)
• Eva: Carried multiple follow-up fixes in the session-history HTTP/SSE path, including bounded refresh sanitization and sequence handling. (role: recent area contributor; confidence: medium; commits: b099427570, dea515e833, 3d9c6affce; files: src/gateway/sessions-history-http.ts, src/gateway/session-history-state.ts)
• steipete: Recent history shows repeated session and Gateway contract maintenance across the surrounding subsystem and merged feature follow-ups. (role: adjacent owner; confidence: medium; commits: 0d938748a5, 1dd88c6288; files: src/gateway/sessions-history-http.ts, src/gateway/session-utils.ts, src/sessions/session-id-resolution.ts)

What the crustacean ranks mean

• 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
• 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
• 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
• 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
• 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
• 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
• 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

Review history (3 earlier review cycles)

• reviewed 2026-07-17T07:07:56.920Z sha a7d45ee :: needs real behavior proof before merge. :: none
• reviewed 2026-07-17T08:27:20.201Z sha a7d45ee :: needs real behavior proof before merge. :: none
• reviewed 2026-07-17T14:51:45.230Z sha a7d45ee :: needs real behavior proof before merge. :: [P2] Reject unsafe integer limits before clamping

[Patrick-Erichsen]

Maintainer pass complete on exact head df3793296fd5a8a5b9eab38c55362a889786e27f.

• Replaced local numeric conversion with parseStrictPositiveInteger and the shared Result type.
• Invalid explicit limits now return HTTP 400: empty, whitespace, malformed, zero, negative, and fractional values.
• Valid positive values remain bounded, including encoded +1 accepted by the shared parser.
• Preserved the shipped oversized unsigned all-digit clamp from fix(gateway): cap history when numeric limit is huge #104263.
• Blacksmith Testbox tbx_01kxrgxeq6yk40m74jf3z76xt2: 9 focused Gateway HTTP tests passed.
• Exact origin/main SHA 8a8706cb5d0448fe2a15b1afae81bfe419322e55 independently reproduces the unrelated session sequence assertion failure seen in the full file.
• check:changed passed formatting, core/core-test typechecks, changed-file lint, dependency checks, and architecture guards before the Testbox transport ended with exit 255 during later generic guards; no check failure was reported.
• Canonical oxfmt, git diff --check, required review artifacts, and final autoreview are clean.

Best-fix verdict: the direct Gateway handler owns this query contract; the shared strict parser owns numeric conversion; the narrow digit-only overflow branch preserves shipped compatibility. Ready for exact-head CI and repo-native prepare/merge.

@clawsweeper re-review

[clawsweeper]

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

• State: Complete
• Detail: The item is closed; no stale verdict was published.
• Run: https://github.com/openclaw/clawsweeper/actions/runs/29603050348
• Updated: 2026-07-17T18:15:54.261Z

[Patrick-Erichsen]

Merged via squash.

• Prepared head SHA: df3793296fd5a8a5b9eab38c55362a889786e27f
• Landed commit: 8470a5befddd676d1a8379d44ce5fd6c268fa401