Skip to content

fix: cap the refresh interval read from settings.json at a year - #323

Merged
HemSoft merged 1 commit into
mainfrom
fix/issue-322-refresh-interval-date
Nov 7, 2026
Merged

HemSoft merged 1 commit into
mainfrom
fix/issue-322-refresh-interval-date

Conversation

@HemSoft

@HemSoft HemSoft commented Nov 7, 2026

Copy link
Copy Markdown
Collaborator

Summary

Settings::refresh_interval_secs now caps a positive refreshIntervalSeconds at one year (MAX_REFRESH_INTERVAL_SECS), the same ceiling the Claude and Copilot backoffs put on Retry-After. Every value the dashboard gets back now fits chrono's Duration::seconds and DateTime + Duration, so a hand-edited interval can no longer crash the once-a-second tick or the status line's "next refresh" countdown. The cap applies only when the value is read. settings.json keeps the number it holds, and saving another setting leaves it alone. No format, schema or migration change.

Acceptance Criteria

  • Any positive i64, including i64::MAX, opens, ticks and renders without panicking: a_refresh_interval_too_long_for_a_date_waits_instead_of_panicking (UI, 9000000000000000000) and refresh_interval_past_a_year_reads_as_a_year_and_stays_in_the_file (store, i64::MAX).
  • Intervals up to the cap read unchanged and longer ones read as the cap (one year): store test cases 3600, year, year + 1 and i64::MAX.
  • 0, negative and missing keep their meaning (off, off, 120 s): the unchanged refresh_interval_off_and_default and the unchanged <= 0 branch.
  • Saving another setting keeps the hand-edited value: the store test saves a zoom change and reads 9000000000000000000 back from disk.

Definition of Done

  • A headless UI test that panicked before the fix passes after it: on main's store it failed with TimeDelta::seconds out of bounds (chrono lib.rs:717); with the fix it passes.
  • A store test covers the cap's boundaries and the value staying in the file.
  • Local gates: see Validation.
  • Required CI and SFL review on the current head.

Validation

  • cargo test -p codexbar-app --locked a_refresh_interval_too_long_for_a_date with the store change reverted: FAILED, panicked at chrono-0.4.45\src\lib.rs:717:17: TimeDelta::seconds out of bounds.
  • The same command with the fix: 1 passed.
  • cargo fmt --all --check: pass (after cargo fmt --all).
  • cargo clippy --workspace --all-targets --locked -- -D warnings: pass.
  • cargo check -p codexbar-app --bin codexbar --locked: pass.
  • cargo test --workspace --locked: pass, 1111 passed, 0 failed across 10 test binaries; the pre-commit hook passed again on commit 538ea26.

No visible UI changes for real settings: intervals up to a year read as before, and the only new test is headless.

Risks and deferred work

A hand-edited interval longer than a year now refreshes once a year instead of crashing. The Settings dropdown shows it as its own option ("Every 525600 minutes"), as it already does for any non-standard value.

Closes #322

🤖 Generated with Claude Code

A hand-edited refreshIntervalSeconds too long for chrono panicked the
dashboard's tick (TimeDelta::seconds out of bounds) and its next-refresh
countdown on every start. Read it capped at a year, as the provider
backoffs cap Retry-After; the file keeps its value.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@HemSoft HemSoft added bug Something isn't working model:claude-opus-5.5 PR authored by claude-opus-5.5 labels Nov 7, 2026
@HemSoft

HemSoft commented Nov 7, 2026

Copy link
Copy Markdown
Collaborator Author

@sfl review

@hemsoft-sfl-reviewer hemsoft-sfl-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Verdict: PASS • Round 1

Confidence: 9/10

Review done by claude-opus-5-5 (same family: same-family: codex gated: codex Weekly 165% SUS 61 🔴 pause, defer)

Settings::refresh_interval_secs now caps positive refreshIntervalSeconds values at MAX_REFRESH_INTERVAL_SECS (one year) and leaves the stored value untouched. A headless UI test and a store test come with it. Every reader of the value now feeds chrono::Duration::seconds a value inside its range. I found no evidenced Critical, High or Medium defect in the diff or its callers.

🔨 Full-Spectrum Review Summary

Effort: high
Review time: 1m 9s
Token count: Unavailable

Pass 🔴 Critical 🟠 High 🟡 Medium ⚪ Low
🔒 Security 0 0 0 0
✅ Accuracy 0 0 0 0
🧰 Quality & Maintainability 0 0 0 0
Total 0 0 0 0

Key Findings

No material defects found.

Coverage

  • Files reviewed:
    • crates/codexbar-store/src/settings.rs
    • crates/codexbar-app/src/ui_tests.rs
  • Context read:
    • crates/codexbar-app/src/dashboard.rs
    • crates/codexbar-app/src/settings_view.rs
    • crates/codexbar-core/src/format.rs
    • crates/codexbar-providers/src/claude.rs
    • crates/codexbar-providers/src/copilot.rs
    • crates/codexbar-store/Cargo.toml
    • AGENTS.md
  • Verified:
    • Compare base_commit.sha is d9e38de394bf5a9c8c7ad4403b04bda143743a3a and the last commit is 538ea267ed02a4efaf0be53fb2e83d8caa70c096 (ahead 1, behind 0)
    • PR head and base SHAs match the reserved SHAs
    • Only two Rust callers read the interval: dashboard.rs:381-386 (tick due check) and dashboard.rs:2527-2532 (status countdown). Both use Duration::seconds(secs as i64), and a year in seconds is well inside chrono's range
    • settings_view.rs:100-105 current_minutes turns a year into 525600 minutes, which matches the PR's description of the dropdown
    • set_refresh_interval_secs is unchanged; dropdown values are minutes times 60, far below i64::MAX
    • The <= 0 and missing (120) branches are unchanged; unsigned_abs only runs on positive values, so it equals the old cast
    • The one-year ceiling matches copilot.rs:266 (365 * 24 * 3600) and claude.rs:42 (RATE_LIMIT_BACKOFF_MAX_DAYS = 365)
    • format::clock_countdown takes any Duration without panicking
    • codexbar-store depends on chrono, and the test module's TempDir::with/read and json! helpers exist
    • Comparing the stored 9000000000000000000 with json!(…i64) is sound because serde_json keeps both as a positive integer
  • Not verified:
    • Runtime behavior and the PR's claimed test, clippy and fmt results, because execution is forbidden
    • Large values as_i64 cannot read (above i64::MAX or floats) still fall back to 120. That behavior predates this change and the diff does not touch it
    • GitHub code search may be incomplete; Rust callers were confirmed by grepping the downloaded exact-SHA files
    • CI status on the head was not checked
  • Tests run: None. Reviewer execution is disabled.

HemSoft full-spectrum review • 3 evidence-based passes • Reviewed head 538ea267ed02a4efaf0be53fb2e83d8caa70c096 • SFL v0

@HemSoft
HemSoft merged commit 78e91a4 into main Nov 7, 2026
7 checks passed
@HemSoft
HemSoft deleted the fix/issue-322-refresh-interval-date branch November 7, 2026 10:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working model:claude-opus-5.5 PR authored by claude-opus-5.5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Keep the dashboard running when settings.json holds a refresh interval too long for a date

1 participant