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 <[email protected]>

* 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 <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>

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());
+    }
 }

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

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));
     }