From b6cf2a6b732c218fad3e5bb731dbde5e0b97717b Mon Sep 17 00:00:00 2001 From: Illia Polosukhin Date: Sat, 7 Mar 2026 20:00:40 +0000 Subject: [PATCH] fix: prevent Instant duration overflow on Windows (#657) (#664) * fix: use checked_sub to prevent Instant duration overflow on Windows (#657) On Windows, Instant starts from system boot time. Subtracting a duration longer than uptime (e.g., 1 hour on a freshly booted system) panics with "overflow when subtracting duration from instant", crashing the tokio worker thread. Replace `Instant::now() - Duration` with `Instant::now().checked_sub()` in cost_guard.rs (production), server.rs and session.rs (tests). Co-Authored-By: Claude Opus 4.6 * fix: use expect() instead of unwrap_or() in test code Address PR review: unwrap_or(Instant::now()) silently breaks test semantics when checked_sub returns None. Using expect() ensures tests fail explicitly with a clear message about insufficient system uptime. [skip-regression-check] Co-Authored-By: Claude Opus 4.6 --------- Co-authored-by: Claude Opus 4.6 --- src/agent/cost_guard.rs | 49 ++++++++++++++++++++++++++++++++------ src/channels/web/server.rs | 8 +++++-- src/tools/mcp/session.rs | 4 +++- 3 files changed, 51 insertions(+), 10 deletions(-) diff --git a/src/agent/cost_guard.rs b/src/agent/cost_guard.rs index 47362fc0..4563bbbe 100644 --- a/src/agent/cost_guard.rs +++ b/src/agent/cost_guard.rs @@ -131,10 +131,12 @@ impl CostGuard { // Check hourly rate if let Some(limit) = self.config.max_actions_per_hour { let mut window = self.action_window.lock().await; - let cutoff = Instant::now() - std::time::Duration::from_secs(3600); - // Drain expired entries - while window.front().is_some_and(|t| *t < cutoff) { - window.pop_front(); + // checked_sub avoids panic when system uptime < 1 hour (Windows) + if let Some(cutoff) = Instant::now().checked_sub(std::time::Duration::from_secs(3600)) { + // Drain expired entries + while window.front().is_some_and(|t| *t < cutoff) { + window.pop_front(); + } } let count = window.len() as u64; if count >= limit { @@ -260,9 +262,11 @@ impl CostGuard { /// Number of actions in the current hourly window. pub async fn actions_this_hour(&self) -> u64 { let mut window = self.action_window.lock().await; - let cutoff = Instant::now() - std::time::Duration::from_secs(3600); - while window.front().is_some_and(|t| *t < cutoff) { - window.pop_front(); + // checked_sub avoids panic when system uptime < 1 hour (Windows) + if let Some(cutoff) = Instant::now().checked_sub(std::time::Duration::from_secs(3600)) { + while window.front().is_some_and(|t| *t < cutoff) { + window.pop_front(); + } } window.len() as u64 } @@ -621,4 +625,35 @@ mod tests { "surcharge should be 100% of input cost for 1h cache writes" ); } + + /// Regression test for #657: Instant::now() - Duration panics on Windows + /// when system uptime is less than the subtracted duration. + #[tokio::test] + async fn test_checked_sub_no_panic_on_fresh_guard() { + // A fresh CostGuard with rate limits should not panic even if + // checked_sub returns None (simulating short uptime). + let guard = CostGuard::new(CostGuardConfig { + max_cost_per_day_cents: None, + max_actions_per_hour: Some(100), + }); + + // These must not panic regardless of system uptime + assert!(guard.check_allowed().await.is_ok()); + assert_eq!(guard.actions_this_hour().await, 0); + + // Record some actions and verify again + guard + .record_llm_call("gpt-4o", 10, 10, 0, 0, Decimal::ONE, Decimal::ONE, None) + .await; + assert!(guard.check_allowed().await.is_ok()); + assert_eq!(guard.actions_this_hour().await, 1); + } + + /// Verify that checked_sub itself behaves as expected for the pattern we use. + #[test] + fn test_instant_checked_sub_returns_none_for_overflow() { + // Duration::MAX will always exceed uptime, so checked_sub must return None + let result = Instant::now().checked_sub(std::time::Duration::MAX); + assert!(result.is_none()); + } } diff --git a/src/channels/web/server.rs b/src/channels/web/server.rs index 65264a2b..2f3b2a5b 100644 --- a/src/channels/web/server.rs +++ b/src/channels/web/server.rs @@ -2620,7 +2620,9 @@ mod tests { secrets, sse_sender: None, gateway_token: None, - created_at: std::time::Instant::now() - std::time::Duration::from_secs(600), + created_at: std::time::Instant::now() + .checked_sub(std::time::Duration::from_secs(600)) + .expect("System uptime is too low to run expired flow test"), }; ext_mgr @@ -2727,7 +2729,9 @@ mod tests { sse_sender: None, gateway_token: None, // Expired — handler will reject after lookup (no network I/O) - created_at: std::time::Instant::now() - std::time::Duration::from_secs(600), + created_at: std::time::Instant::now() + .checked_sub(std::time::Duration::from_secs(600)) + .expect("System uptime is too low to run expired flow test"), }; ext_mgr diff --git a/src/tools/mcp/session.rs b/src/tools/mcp/session.rs index a59dc33f..3f13fc72 100644 --- a/src/tools/mcp/session.rs +++ b/src/tools/mcp/session.rs @@ -204,7 +204,9 @@ mod tests { assert!(!session.is_stale(1800)); // Manually set last_activity to the past to simulate staleness - session.last_activity = std::time::Instant::now() - std::time::Duration::from_secs(10); + session.last_activity = std::time::Instant::now() + .checked_sub(std::time::Duration::from_secs(10)) + .expect("System uptime is too low to run staleness test"); assert!(session.is_stale(5)); assert!(!session.is_stale(15)); }