fix(sakana): parse weekly/5-hour reset time as UTC, not device-local#1827
fix(sakana): parse weekly/5-hour reset time as UTC, not device-local#1827ss251 wants to merge 3 commits into
Conversation
console.sakana.ai/billing always server-renders "Resets on <date>" in UTC (confirmed via the raw HTML's embedded "timeZone":"UTC" hydration payload marker, and Sakana's own "Weekly usage resets every Monday at 00:00 UTC" copy elsewhere on the page). The browser only corrects that text to the viewer's local timezone client-side, after JS hydration -- which this HTML-only fetcher never runs. SakanaUsageFetcher.parseResetDate parsed the raw string using TimeZone.current instead, so every non-UTC user got a reset time off by exactly their UTC offset -- 5.5h early on an IST machine, verified by building main into the codexbar CLI and running it against a real account: the tool's own JSON output landed 5.5h before the true reset instant shown in a browser on the same page at the same moment. Removes the timeZone parameter entirely (it no longer has a legitimate caller-supplied value; the source data is always UTC) and hardcodes UTC in parseResetDate. Updates the existing fixture-based tests, which previously asserted the buggy local-timezone interpretation as if it were correct, and adds a regression test pinned to a literal Unix timestamp independent of the date-building helper. Fixes steipete#1826.
|
Codex review: needs maintainer review before merge. Reviewed July 1, 2026, 5:16 PM ET / 21:16 UTC. Summary Reproducibility: yes. Current main passes Review metrics: 3 noteworthy metrics.
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 UTC parser, focused regression coverage, and docs update after normal maintainer validation so the linked Sakana bug closes through the merge. Do we have a high-confidence way to reproduce the issue? Yes. Current main passes Is this the best way to solve the issue? Yes. Parsing the raw server-rendered Sakana reset text as UTC and removing the caller-supplied timezone is the narrow maintainable fix; a new setting or product choice is not needed. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against c3d33308ac06. Label changesLabel changes:
Label justifications:
Evidence reviewedWhat 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
|
clawsweeper
Bot
added
proof: sufficient
- The UTC regression test didn't actually pin the fix: on a UTC CI runner, the pre-fix TimeZone.current code would coincidentally still produce the correct instant, so the test would pass either way. Force NSTimeZone.default to UTC+14 for the test's duration (same pattern as BedrockUsageStatsTests) so it only passes when the parser genuinely ignores the device timezone. Verified by temporarily reverting the fix locally and confirming the test now fails with the expected 5.5h-off result before restoring it. - docs/sakana.md still said reset dates use the device's local time zone (TimeZone.current) -- exactly the behavior this PR removes. Updated to describe the actual UTC parsing and why, linking steipete#1826.
|
Addressed both P3 findings from clawsweeper's review:
12/12 Sakana tests pass, lint clean. |
clawsweeper
Bot
added
rating: 🦐 gold shrimp
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
clawsweeper's re-review flagged that the new NSTimeZone.default override test lives in a non-serialized suite, unlike the existing BedrockUsageStatsTests that uses the same pattern -- unrelated parallel Swift Testing cases could observe UTC+14 and fail nondeterministically. Mark @suite(.serialized), matching Bedrock's precedent exactly.
|
Addressed the P2 finding from the re-review: marked @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
clawsweeper
Bot
added
rating: 🐚 platinum hermit
1 participant
Fixes #1826.
Summary
console.sakana.ai/billingalways server-renders "Resets on " in UTC — confirmed by the raw HTML's embedded"timeZone":"UTC"hydration marker and Sakana's own "Weekly usage resets every Monday at 00:00 UTC" copy elsewhere on the page. The browser only corrects that string to the viewer's local timezone client-side, after JS hydration, whichSakanaUsageFetcher(an HTML-only fetcher) never runs.parseResetDateparsed the raw string usingTimeZone.currentinstead of UTC, so every non-UTC user got a reset countdown off by exactly their UTC offset.timeZoneparameter fromparseBillingHTML/parseWindow/parseResetDateentirely — there's no legitimate caller-supplied value here, the source text is always UTC — and hardcodes UTC in the formatter.shanghaiTimeZonetest parameter) as if it were correct.Verification
I built this branch (and, for comparison, unmodified
main) into thecodexbarCLI and ran both against a real Sakana account (Standard plan, active weekly window) with a live cookie, comparing to the same account's billing page rendered in a real browser at the same moment:main(before)2026-07-06T00:00:00Z)2026-07-05T18:30:00Z(5.5h early)2026-07-06T00:00:00Z✓Plan name, 5-hour %, and weekly % were already correct in both and are untouched by this change.
Test plan
swift build --target CodexBarCore/swift build --target CodexBar— clean./Scripts/lint.sh format— 0 files reformatted./Scripts/lint.sh lint(swiftlint --strict) — 0 violationsswift test --filter SakanaUsageFetcherTests— 12/12 passcodexbarCLI before and after the fix (table above)