fix(usage): reject inverted startDate-endDate range in usage.cost and sessions.usage#94096
Conversation
bba436c to
71a9928
Compare
|
Codex review: needs maintainer review before merge. Reviewed June 30, 2026, 3:49 PM ET / 19:49 UTC. Summary PR surface: Source -4, Tests +36. Total +32 across 2 files. Reproducibility: yes. Source inspection shows current main builds an inverted Review metrics: 1 noteworthy metric.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the centralized validation after maintainers accept the stricter gateway RPC contract; otherwise ask for a compatibility-preserving design for reversed explicit ranges. Do we have a high-confidence way to reproduce the issue? Yes. Source inspection shows current main builds an inverted Is this the best way to solve the issue? Yes, pending maintainer compatibility acceptance. Centralizing invalid-date and inverted-range checks in AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 076da567f434. Label changesLabel justifications:
Evidence reviewedPR surface: Source -4, Tests +36. Total +32 across 2 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
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
|
71a9928 to
eb91411
Compare
|
@clawsweeper re-review Added/clarified real-behavior proof for the inverted explicit date-range validation. Behavior addressed Focused verification run on this branch
The focused test exercises the real exported handler/testApi path for both Typecheck note |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review — Added a Real behavior proof section. It drives the real |
|
@clawsweeper re-review — Added the |
|
@clawsweeper re-review — Updated the PR body with a machine-readable |
b215447 to
348d141
Compare
348d141 to
c054ea0
Compare
|
Land-ready maintainer pass on exact head
Remote Crabbox was unavailable because this machine lacks the Blacksmith CLI and broker credentials; the deterministic gateway boundary was exercised through the real exported handlers. |
|
Merged via squash.
|
… sessions.usage (openclaw#94096) * fix(usage): reject inverted startDate-endDate range * chore: retrigger CI for real behavior proof check * refactor(usage): resolve validated date ranges once --------- Co-authored-by: Peter Steinberger <[email protected]>
… sessions.usage (openclaw#94096) * fix(usage): reject inverted startDate-endDate range * chore: retrigger CI for real behavior proof check * refactor(usage): resolve validated date ranges once --------- Co-authored-by: Peter Steinberger <[email protected]>
… sessions.usage (openclaw#94096) * fix(usage): reject inverted startDate-endDate range * chore: retrigger CI for real behavior proof check * refactor(usage): resolve validated date ranges once --------- Co-authored-by: Peter Steinberger <[email protected]>
… sessions.usage (openclaw#94096) * fix(usage): reject inverted startDate-endDate range * chore: retrigger CI for real behavior proof check * refactor(usage): resolve validated date ranges once --------- Co-authored-by: Peter Steinberger <[email protected]>
What Problem This Solves
usage.cost/sessions.usageaccept an inverted explicit date range (startDateafterendDate) from any gateway client.parseDateRangeinsrc/gateway/server-methods/usage.tsnever checksstartMs <= endMswhen both dates are explicitly given, so a backwards window (startMs > endMs) reaches the cost/session loader andrespond(true, ...)returns an empty/wrong result with no error — a silently-wrong-window class bug. Callers (CLI/UI/scripts) get a successful response that hides the bad input instead of a clear rejection. This is the symmetric gap to merged #93745 (invalid calendar date rejection) and sibling to merged #93903 (cron absolute-timestamp validation): same family, the missing case where both dates are individually valid but ordered backwards.Evidence
Real exported
usageHandlers["usage.cost"]andusageHandlers["sessions.usage"]from the changedusage.tswere driven under tsx on Node v22.22.0 with only the outermost data IO insrc/infra/session-cost-usage.tsreplaced (viamodule.register()) by recorders capturing the(startMs,endMs)window; all validation runs as production code.{start=2026-02-03, end=2026-02-02}): both handlers returnrespond.ok=false,error={code:"INVALID_REQUEST", message:"startDate must not be after endDate"}, and the loader is never reached (reached loader? = false).usage.ts, fix removed —hasInvertedExplicitDateRange = undefined): same inverted range is silently accepted (respond.ok=true,error=null) and the real loader is reached with a backwards window (loadCostUsageSummaryFromCache(startMs=1770076800000,endMs=1770076799999,backwards=true)/discoverAllSessions(...backwards=true)).{start=2026-02-01, end=2026-02-02}: still passes (respond.ok=true,reachedLoader=true,err=null) on the patched tree — the fix only rejects genuinely inverted ranges.2 files / 44 tests passed(28s), including the newhasInvertedExplicitDateRangeboundary case and the table-drivenusage.cost/sessions.usagerejection case.Full proof transcript (sanity line, both handlers, negative + forward controls) is in Real behavior proof below.
Summary
#93745 added
findInvalidExplicitDateto reject invalid calendar dates inusage.cost/sessions.usage(merged). ButparseDateRangeinsrc/gateway/server-methods/usage.tsdoes not validatestartMs <= endMswhen both dates are explicitly given — an inverted range (startDate after endDate) yields a backwards window, and the cost loader silently returns empty whilerespond(true)succeeds. This is the symmetric gap to #93745 (same "silently wrong window" family; sibling to merged #93903's cron absolute-timestamp validation).Changes
hasInvertedExplicitDateRangehelper (reusing extractedhasExplicitDateValue). Bothusage.costandsessions.usagenow rejectstartDate > endDatewithINVALID_REQUEST: "startDate must not be after endDate"before touching runtime config / cost loader / session scan.parseDateRange's return contract is unchanged (validation is at the handler entry).testApiexposes the helper. +96/-1 across 2 files (one is the test).Real behavior proof
Behavior addressed: an inverted explicit date range (
startDateafterendDate) used to be silently accepted byusage.cost/sessions.usage—parseDateRangeproduced a backwards window (startMs > endMs), the real cost/session loader was reached with that backwards window, andrespond(true, ...)returned an empty/wrong result with no error. The fix rejects it withrespond(false, INVALID_REQUEST "startDate must not be after endDate")before any loader is touched.Real environment tested: real exported
usageHandlers["usage.cost"]andusageHandlers["sessions.usage"]imported from the changedsrc/gateway/server-methods/usage.tsand driven under tsx on Node v22.22.0 (Linux). Only the outermost data IO insrc/infra/session-cost-usage.ts(loadCostUsageSummaryFromCache,discoverAllSessions,loadSessionLogs) was replaced — via amodule.register()loader hook — with recorders that capture the exact(startMs,endMs)window each handler computes and then return empty data. Everything else in the handler (param validation,hasInvertedExplicitDateRange,resolveDateInterpretation,parseDateRange) runs as production code.Exact steps or command run after this patch:
git worktree addPR headfix/usage-reject-inverted-date-range.{startDate: "2026-02-03", endDate: "2026-02-02"}and reads back therespond(ok, payload, error)args plus whether the real loader was reached and with what window.node scripts/run-vitest.mjs src/gateway/server-methods/usage.test.ts --run.usage.ts(fix removed,hasInvertedExplicitDateRangeabsent) and re-run the same proof.{startDate: "2026-02-01", endDate: "2026-02-02"}driven through both handlers to confirm the fix is not over-broad.Evidence after fix (patched tree):
Negative control (parent
usage.ts, fix removed) — inverted range is silently accepted, the real loader is reached with a backwards window, no error:Vitest:
2 files / 44 tests passed(28s), including the new helper boundary case and the table-drivenusage.cost/sessions.usagerejection case.Observed result after fix: both handlers reject the inverted range with
respond(false, undefined, {code: "INVALID_REQUEST", message: "startDate must not be after endDate"})and the cost/session loader is never invoked; the forward range{2026-02-01 → 2026-02-02}still resolves and reaches the loader (respond(true)), so the fix only rejects genuinely inverted ranges. Reverting the source flips this back to silent acceptance with a backwardsstartMs > endMswindow.What was not tested: real on-disk cost/session data aggregation (the loaders were stubbed to return empty data — they are the layer below the validation under test); the rejection happens before any loader runs, so loader internals are out of scope here. UI/client surfacing of the new error and timezone/
utcOffsetinteraction beyond the parsed-window comparison were not exercised.Scope / context
No linked issue — symmetric follow-up to merged #93745 (usage invalid-date rejection) and #93903 (cron invalid absolute timestamp). No competing PR touches this gap.
Real behavior proof
Behavior addressed: usage.cost and sessions.usage used to accept an explicit inverted date range where startDate is after endDate. That produced a backwards startMs/endMs window, reached the cost/session loader, and returned a successful empty or wrong result. This patch rejects that input before any loader runs.
Real environment tested: Local Linux runtime with Node v22.22.0 and tsx, importing the real exported usageHandlers["usage.cost"] and usageHandlers["sessions.usage"] from src/gateway/server-methods/usage.ts. Only the outermost data IO in src/infra/session-cost-usage.ts was replaced with recorder functions so the computed startMs/endMs windows could be captured; validation and date parsing ran as production code.
Exact steps or command run after this patch: In the PR head worktree fork/fix/usage-reject-inverted-date-range, ran a tsx proof script that drives both handlers with {startDate:"2026-02-03", endDate:"2026-02-02"}, records respond(ok,payload,error), records whether loaders are reached, then repeats a forward-range control and a negative control with the parent usage.ts restored. Also ran node scripts/run-vitest.mjs src/gateway/server-methods/usage.test.ts --run.
Evidence after fix: Live terminal output from the proof script:
Negative control with the parent usage.ts restored showed respond.ok=true, respond.error=null, and loaders reached with backwards=true windows for both handlers.
Observed result after fix: Both handlers now return respond(false, undefined, {code:"INVALID_REQUEST", message:"startDate must not be after endDate"}) for the inverted range and never invoke the cost/session loader. The forward range still succeeds and reaches the loader. Vitest passed 44 tests.
What was not tested: Real on-disk cost/session data aggregation was not exercised because the loaders were stubbed below the validation layer. UI/client rendering of the new error and deeper timezone/utcOffset behavior beyond the parsed-window comparison were not exercised.