fix(infra): add NaN guard for unparseable timestamp in cost usage by krissding · Pull Request #99420 · openclaw/openclaw

krissding · 2026-07-03T07:33:29Z

What Problem This Solves

new Date(parsed.timestamp).getTime() in session-cost-usage.ts can return NaN when the timestamp string is unparseable (e.g. "not-a-date", empty string, malformed). Without a guard, NaN silently propagates into downstream usage/cost calculations, corrupting billing data.

Evidence

Real behavior proof (before vs after NaN guard)

============================================================
Proof: NaN timestamp guard in session-cost-usage.ts
============================================================
  "valid ISO 8601": 1705314600000 ✓
  "garbage string": fallback to 0 ✓
  "empty string": fallback to 0 ✓
  "non-date keyword": fallback to 0 ✓
  "'null' string": fallback to 0 ✓

  All 5/5 cases NaN-safe ✓

Without the guard, the 4 non-date inputs produce NaN which corrupts downstream calculations. With the guard, they safely fall back to 0 (the same default as when no timestamp key is present).

Existing tests

57 of 58 session-cost-usage tests pass (the 1 failure is a pre-existing DST/Intl stub issue unrelated to this change).

Build verification

Full project builds successfully with the change.

Why This Change Was Made

This is a "present-but-invalid" boundary case — the timestamp key exists as a string, so we enter the typeof parsed.timestamp === "string" branch, but the parsed value is garbage. The same guard pattern already exists elsewhere in the codebase. The fix is a 3-line addition with no change to any other code path.

Merge Risk

Zero. The guard only activates when new Date(...).getTime() returns NaN, which was previously unhandled and would propagate corrupt data. All valid timestamps pass through unchanged.

Fixes #99413

🤖 Generated with Claude Code

…sage new Date(parsed.timestamp).getTime() can return NaN for unparseable strings. Without a guard, NaN silently propagates into downstream usage/cost calculations and corrupts billing data. Add Number.isNaN(timestamp) check, falling back to 0 (same default as the path when no timestamp key is present). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

clawsweeper · 2026-07-03T07:57:03Z

Codex review: needs maintainer review before merge. Reviewed July 3, 2026, 4:00 AM ET / 08:00 UTC.

Summary
The PR adds a Number.isNaN guard after parsing parsed.timestamp in loadSessionLogs, falling back to 0 for unparseable timestamp strings.

PR surface: Source +3. Total +3 across 1 file.

Reproducibility: yes. at source level: current main assigns new Date(parsed.timestamp).getTime() in loadSessionLogs, and Node confirms an unparseable string produces NaN that serializes as null. I did not run a full gateway session-file repro in this read-only review.

Review metrics: none identified.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #99413
Summary: The linked issue is the canonical report for invalid top-level transcript timestamp parsing in loadSessionLogs, and this PR is one of two open candidate fixes.

Members:

canonical: fix(infra): add NaN guard for date parse in session-cost-usage #99413 - The issue body reports the same unparseable timestamp path and this PR uses closing syntax for it.
fixed_by_candidate: fix(infra): add NaN guard for date parse in session cost usage #99418 - That open PR changes the same timestamp parse for the same canonical issue, but it is not a safe superseding close target because it is still open with failing checks.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

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

Rank-up moves:

[P2] Add a focused loadSessionLogs regression test for unparseable top-level timestamps if maintainers want stronger coverage before merge.
Resolve the overlapping open candidate PR so only one branch lands.

Risk before merge

[P1] There is an overlapping open candidate PR for the same issue, so maintainers should land only one of the two branches.
[P1] The patch does not add a focused regression test for unparseable top-level timestamps in loadSessionLogs; the source path is clear, but durable coverage would reduce recurrence risk.

Maintainer options:

Decide the mitigation before merge
Land one canonical linked PR that keeps loadSessionLogs timestamps finite, then close the duplicate candidate after the fix merges.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

[P2] Human review should choose which overlapping candidate PR to land; there is no narrow ClawSweeper repair job to queue for this branch.

Security
Cleared: The diff only adds a local numeric guard in TypeScript and does not touch dependencies, CI, scripts, permissions, secrets, or other supply-chain surfaces.

Review details

Best possible solution:

Land one canonical linked PR that keeps loadSessionLogs timestamps finite, then close the duplicate candidate after the fix merges.

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

Yes at source level: current main assigns new Date(parsed.timestamp).getTime() in loadSessionLogs, and Node confirms an unparseable string produces NaN that serializes as null. I did not run a full gateway session-file repro in this read-only review.

Is this the best way to solve the issue?

Yes; guarding the string parse in loadSessionLogs is the narrow maintainable fix and matches the same module's invalid-date guard pattern. The best landable version should be one canonical PR, preferably with focused regression coverage.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 7cb41ccc63f5.

Label changes

Label justifications:

P2: This is a normal-priority data integrity bug in the session usage log path with a narrow, low-blast-radius fix.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes copied live output for valid and invalid timestamp cases showing after-fix fallback to 0; tests and build claims are supplemental.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes copied live output for valid and invalid timestamp cases showing after-fix fallback to 0; tests and build claims are supplemental.

Evidence reviewed

PR surface:

Source +3. Total +3 across 1 file.

View PR surface stats

Area	Files	Added	Net
Source	1	3	+3
Tests	0	0	0
Docs	0	0	0
Config	0	0	0
Generated	0	0	0
Other	0	0	0
Total	1	3	+3

What I checked:

Current main still has the bug: loadSessionLogs on origin/main assigns new Date(parsed.timestamp).getTime() for string top-level timestamps without checking for NaN. (src/infra/session-cost-usage.ts:2732, 7cb41ccc63f5)
PR diff fixes the implicated line: The PR adds a guard immediately after the string timestamp parse and resets NaN to the existing default 0. (src/infra/session-cost-usage.ts:2732, cc88cbabc824)
Gateway caller exposes returned log timestamps: The sessions.usage.logs gateway method calls loadSessionLogs and returns { logs }, so non-finite timestamps can cross the user-facing gateway response boundary. (src/gateway/server-methods/usage.ts:1565, 7cb41ccc63f5)
Sibling parser uses the same invalid-date guard pattern: The same module's parseTimestamp helper rejects invalid string dates with Number.isNaN(parsed.valueOf()), supporting the local fix shape. (src/infra/session-cost-usage.ts:867, 7cb41ccc63f5)
Runtime behavior confirms the bad value: Node reports new Date("not-a-date").getTime() as NaN, and JSON serialization turns that timestamp into null.
Latest release still has the unguarded parse: v2026.6.11 contains the same direct string timestamp parse, so this has not shipped fixed yet. (src/infra/session-cost-usage.ts:2623, e085fa1a3ffd)

Likely related people:

Takhoffman: PR metadata for Web UI: add token usage dashboard #10072 shows the token usage dashboard introduced the usage log endpoint and session-cost usage surfaces. (role: introduced behavior; confidence: high; commits: 8a352c8f9dfd; files: src/infra/session-cost-usage.ts, src/gateway/server-methods/usage.ts, ui/src/ui/views/usage.ts)
steipete: Commit history shows a refactor of the JSONL transcript reader in the same session-cost usage module. (role: area refactor contributor; confidence: medium; commits: 8cd20e220fbc; files: src/infra/session-cost-usage.ts)
vincentkoc: Recent release history touched the same session-cost usage source and tests, making this a plausible routing candidate for current usage-cost behavior. (role: recent area contributor; confidence: medium; commits: e085fa1a3ffd; files: src/infra/session-cost-usage.ts, src/infra/session-cost-usage.test.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.

…sage (openclaw#99420) new Date(parsed.timestamp).getTime() can return NaN for unparseable strings. Without a guard, NaN silently propagates into downstream usage/cost calculations and corrupts billing data. Add Number.isNaN(timestamp) check, falling back to 0 (same default as the path when no timestamp key is present). Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

openclaw-barnacle Bot added the size: XS label Jul 3, 2026

clawsweeper Bot mentioned this pull request Jul 3, 2026

fix(infra): add NaN guard for date parse in session-cost-usage #99413

Closed

clawsweeper Bot mentioned this pull request Jul 3, 2026

fix(infra): add NaN guard for date parse in session cost usage #99418

Closed

openclaw-clownfish Bot merged commit e183d66 into openclaw:main Jul 6, 2026
48 of 56 checks passed

steipete mentioned this pull request Jul 6, 2026

refactor(infra): canonicalize session usage timestamps #100687

Merged

vincentkoc mentioned this pull request Jul 6, 2026

chore(jobs): prune resolved OpenClaw inbox jobs openclaw/clownfish#221

Merged

krissding mentioned this pull request Jul 7, 2026

fix(infra): add NaN guard for unparseable timestamps from external inputs #101602

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(infra): add NaN guard for unparseable timestamp in cost usage#99420

fix(infra): add NaN guard for unparseable timestamp in cost usage#99420
openclaw-clownfish[bot] merged 1 commit into
openclaw:mainfrom
krissding:fix/issue-99413-nan-timestamp-guard

krissding commented Jul 3, 2026

Uh oh!

clawsweeper Bot commented Jul 3, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

krissding commented Jul 3, 2026

What Problem This Solves

Evidence

Real behavior proof (before vs after NaN guard)

Existing tests

Build verification

Why This Change Was Made

Merge Risk

Uh oh!

clawsweeper Bot commented Jul 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

clawsweeper Bot commented Jul 3, 2026 •

edited

Loading