Repository navigation
fix(usage): parse numeric-string quota reset timestamps as Unix sec/ms - #4493
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Code Review
This pull request updates the parseResetTime function in both codexUsageQuotas.ts and usage.ts to support parsing numeric strings as Unix timestamps, and adds corresponding unit tests. The review feedback suggests refining the generic digit regex to specifically match 10-digit or 13-digit timestamps to prevent false positives from other numeric date formats like YYYYMMDD.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| date = new Date(resetValue < 1e12 ? resetValue * 1000 : resetValue); | ||
| } else if (typeof resetValue === "string") { | ||
| // Numeric strings are Unix timestamps too (seconds or milliseconds). | ||
| if (/^\d+$/.test(resetValue)) { |
There was a problem hiding this comment.
Using a generic /^\d+$/ regex matches any numeric string of any length. This can lead to false positives if a provider returns other numeric date formats (such as an 8-digit YYYYMMDD or a 14-digit YYYYMMDDHHMMSS string), which would then be incorrectly parsed as a Unix timestamp (e.g., 20260425 would be parsed as a timestamp in seconds, resulting in an incorrect date in 1970).
Restricting the regex to exactly 10 digits (seconds) or 13 digits (milliseconds) via /^\d{10}$|^\d{13}$/ is much more robust and specifically targets standard Unix timestamps while avoiding these false positives.
| if (/^\d+$/.test(resetValue)) { | |
| if (/^\d{10}$|^\d{13}$/.test(resetValue)) { |
| date = new Date(resetValue); | ||
| // Numeric strings are Unix timestamps too (seconds or milliseconds). | ||
| // `new Date("1700000000")` otherwise returns Invalid Date. | ||
| if (/^\d+$/.test(resetValue)) { |
There was a problem hiding this comment.
Using a generic /^\d+$/ regex matches any numeric string of any length. This can lead to false positives if a provider returns other numeric date formats (such as an 8-digit YYYYMMDD or a 14-digit YYYYMMDDHHMMSS string), which would then be incorrectly parsed as a Unix timestamp (e.g., 20260425 would be parsed as a timestamp in seconds, resulting in an incorrect date in 1970).
Restricting the regex to exactly 10 digits (seconds) or 13 digits (milliseconds) via /^\d{10}$|^\d{13}$/ is much more robust and specifically targets standard Unix timestamps while avoiding these false positives.
| if (/^\d+$/.test(resetValue)) { | |
| if (/^\d{10}$|^\d{13}$/.test(resetValue)) { |
`parseResetTime` in `open-sse/services/usage.ts` and `codexUsageQuotas.ts` already handled `number` reset values with the `< 1e12 ? * 1000 : value` seconds-vs-ms heuristic, but for `string` inputs it always called `new Date(str)`. When a provider returned the reset timestamp as a numeric string (e.g. `"1700000000"`), `new Date(str)` returned Invalid Date, the try/catch swallowed it, and the function returned `null` — the reset window was silently dropped from quota snapshots. Numeric strings (`/^\d+$/`) are now detected and treated as Unix timestamps with the same seconds-vs-ms heuristic. ISO/parseable strings are untouched. Applied symmetrically in both parsers. TDD: 2 failing tests added in `tests/unit/usage-utils.test.ts` for the seconds-string and ms-string cases; both green after the fix (47/47 in file, 27/27 in adjacent usage/quota tests). Inspired-by: decolua/9router#768 Co-authored-by: Zhen <zhen@dkzhen.org>
add72d5 to
9db3c03
Compare
cheaper-inference#4493) Rebuilt onto release/v3.8.33 (squash-base-stale). Integrated into release/v3.8.33.
…heaper-inference#4512) Rebuilt onto release/v3.8.33; usage.ts baseline reconciled for the cheaper-inference#4493/cheaper-inference#4494/cheaper-inference#4512 quota trio. Integrated into release/v3.8.33.
Summary
Ports an upstream fix — quota reset timestamp parsing for numeric-string Unix timestamps.
OmniRoute's
parseResetTimealready handlednumberUnix-seconds via the< 1e12 ? * 1000 : valueheuristic, but thestringbranch passed everything tonew Date(str). When a provider returned the reset timestamp as a numeric string (e.g."1700000000"),new Date(str)returned Invalid Date, the try/catch swallowed it, and the reset window was silently dropped from quota snapshots (showing as "no reset" in the UI).Numeric strings (
/^\d+$/) are now detected and treated as Unix timestamps with the same seconds-vs-ms heuristic. ISO/parseable strings are untouched. Applied symmetrically in bothopen-sse/services/usage.ts::parseResetTimeandopen-sse/services/codexUsageQuotas.ts::parseResetTime.Changes
open-sse/services/usage.ts— string branch detects/^\d+$/and routes through the< 1e12ms/sec heuristic.open-sse/services/codexUsageQuotas.ts— same logic, refactored ternary into if/else for clarity.tests/unit/usage-utils.test.ts— 2 new cases (seconds-string, ms-string) covering the gap.Validation (Hard Rule #18 — TDD)
__testing.parseResetTime("1700000000")returnednull(logged in PR work).node --import tsx/esm --test tests/unit/usage-utils.test.ts→ 47/47.usage-extractor+quota-pool-usage-shape+copilot-usage+api-key-usage-limits→ 27/27.npm run typecheck:core→ clean.npm run check:docs-sync→ PASS (41 i18n mirrors validated).npx eslinton touched files → 0 warnings.Attribution
Inspired by upstream contribution.