From 04a57e92c2e6f02e559e6cc13a899a338b5a9e86 Mon Sep 17 00:00:00 2001 From: dave Date: Fri, 15 May 2026 20:57:43 +0000 Subject: [PATCH] huskies: merge 1103 bug Rate-limit warning at session start sticks the `rate_limit_exit` flag, causing 1053's fast-path bypass to skip completion on clean session exits --- server/src/agents/pty/mod.rs | 58 +++++++++++++++++++++++++++++++++ server/src/agents/pty/runner.rs | 6 +++- 2 files changed, 63 insertions(+), 1 deletion(-) diff --git a/server/src/agents/pty/mod.rs b/server/src/agents/pty/mod.rs index c44a956f..bfcb82c2 100644 --- a/server/src/agents/pty/mod.rs +++ b/server/src/agents/pty/mod.rs @@ -445,4 +445,62 @@ mod tests { the respawn's lookup_session returns it (warm), not None (cold)" ); } + + // ── bug 1103: soft rate-limit warning (status=allowed) must NOT set rate_limit_exit ── + + /// Regression: a `rate_limit_event` with `status="allowed"` is a soft + /// warning — the request was permitted. The session that follows should + /// complete normally and report `rate_limit_exit == false`, not trigger the + /// rate-limit respawn path in the spawn handler. + #[tokio::test] + async fn rate_limit_allowed_status_does_not_set_rate_limit_exit() { + use std::os::unix::fs::PermissionsExt; + + let tmp = tempfile::tempdir().unwrap(); + let script = tmp.path().join("emit_allowed_then_exit.sh"); + // Emit status="allowed" (soft warning), then exit cleanly. + std::fs::write( + &script, + "#!/bin/sh\nprintf '%s\\n' '{\"type\":\"rate_limit_event\",\"rate_limit_info\":{\"status\":\"allowed\",\"reset_at\":\"2099-01-01T12:00:00Z\"}}'\n", + ) + .unwrap(); + std::fs::set_permissions(&script, std::fs::Permissions::from_mode(0o755)).unwrap(); + + let (tx, _rx) = broadcast::channel::(64); + let (watcher_tx, mut watcher_rx) = broadcast::channel::(16); + let event_log = Arc::new(Mutex::new(Vec::new())); + + let result = run_agent_pty_streaming( + "1103_soft_warning_no_exit_flag", + "coder-1", + "sh", + &[script.to_string_lossy().to_string()], + "--", + "/tmp", + &tx, + &event_log, + None, + 0, + watcher_tx, + None, + None, + ) + .await; + + let pty = result.expect("PTY run should succeed"); + assert!( + !pty.rate_limit_exit, + "rate_limit_exit must be false for a soft 'allowed' warning; \ + only genuine hard blocks (rejected) should set it" + ); + + // Watcher must have received RateLimitWarning, not RateLimitHardBlock. + let evt = watcher_rx + .try_recv() + .expect("Expected a RateLimitWarning watcher event"); + assert!( + matches!(evt, WatcherEvent::RateLimitWarning { .. }), + "Expected RateLimitWarning for status=allowed, got: {evt:?}" + ); + } } diff --git a/server/src/agents/pty/runner.rs b/server/src/agents/pty/runner.rs index ee719422..d55aeef3 100644 --- a/server/src/agents/pty/runner.rs +++ b/server/src/agents/pty/runner.rs @@ -347,7 +347,11 @@ fn run_agent_pty_blocking( .and_then(|i| i.get("status")) .and_then(|s| s.as_str()) .unwrap_or(""); - let is_hard_block = !status.is_empty() && status != "allowed_warning"; + // "allowed" and "allowed_warning" are soft warnings — the request was + // permitted; only statuses that actually block the request (e.g. "rejected") + // are genuine hard blocks that warrant a rate-limit exit respawn. + let is_hard_block = + !status.is_empty() && status != "allowed" && status != "allowed_warning"; let reset_at = rate_limit_info .and_then(|i| i.get("reset_at")) .and_then(|r| r.as_str())