huskies: merge 496_bug_hard_rate_limit_without_reset_at_never_auto_schedules_retry
This commit is contained in:
+80
-17
@@ -363,26 +363,25 @@ fn run_agent_pty_blocking(
|
|||||||
.map(|dt| dt.with_timezone(&chrono::Utc));
|
.map(|dt| dt.with_timezone(&chrono::Utc));
|
||||||
|
|
||||||
if is_hard_block {
|
if is_hard_block {
|
||||||
if let Some(reset_at) = reset_at {
|
let reset_at = reset_at.unwrap_or_else(|| {
|
||||||
|
let default = chrono::Utc::now()
|
||||||
|
+ chrono::Duration::minutes(5);
|
||||||
slog!(
|
slog!(
|
||||||
"[agent:{story_id}:{agent_name}] API rate limit hard block \
|
"[agent:{story_id}:{agent_name}] API rate limit hard block \
|
||||||
(status={status}); resets at {reset_at}"
|
(status={status}); no reset_at in rate_limit_info, \
|
||||||
|
defaulting to 5-minute backoff ({default})"
|
||||||
);
|
);
|
||||||
let _ = watcher_tx.send(WatcherEvent::RateLimitHardBlock {
|
default
|
||||||
story_id: story_id.to_string(),
|
});
|
||||||
agent_name: agent_name.to_string(),
|
slog!(
|
||||||
reset_at,
|
"[agent:{story_id}:{agent_name}] API rate limit hard block \
|
||||||
});
|
(status={status}); resets at {reset_at}"
|
||||||
} else {
|
);
|
||||||
slog!(
|
let _ = watcher_tx.send(WatcherEvent::RateLimitHardBlock {
|
||||||
"[agent:{story_id}:{agent_name}] API rate limit hard block \
|
story_id: story_id.to_string(),
|
||||||
(status={status}); no reset_at in rate_limit_info"
|
agent_name: agent_name.to_string(),
|
||||||
);
|
reset_at,
|
||||||
let _ = watcher_tx.send(WatcherEvent::RateLimitWarning {
|
});
|
||||||
story_id: story_id.to_string(),
|
|
||||||
agent_name: agent_name.to_string(),
|
|
||||||
});
|
|
||||||
}
|
|
||||||
} else {
|
} else {
|
||||||
slog!(
|
slog!(
|
||||||
"[agent:{story_id}:{agent_name}] API rate limit warning received \
|
"[agent:{story_id}:{agent_name}] API rate limit warning received \
|
||||||
@@ -575,6 +574,70 @@ mod tests {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Bug 496: hard block WITHOUT `reset_at` must still emit `RateLimitHardBlock`
|
||||||
|
/// (not `RateLimitWarning`), using a default 5-minute backoff so the
|
||||||
|
/// auto-scheduler can set a retry timer.
|
||||||
|
#[tokio::test]
|
||||||
|
async fn rate_limit_hard_block_without_reset_at_sends_hard_block_event() {
|
||||||
|
use std::os::unix::fs::PermissionsExt;
|
||||||
|
|
||||||
|
let tmp = tempfile::tempdir().unwrap();
|
||||||
|
let script = tmp.path().join("emit_hard_block_no_reset.sh");
|
||||||
|
std::fs::write(
|
||||||
|
&script,
|
||||||
|
"#!/bin/sh\nprintf '%s\\n' '{\"type\":\"rate_limit_event\",\"rate_limit_info\":{\"status\":\"rejected\"}}'\n",
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
std::fs::set_permissions(&script, std::fs::Permissions::from_mode(0o755)).unwrap();
|
||||||
|
|
||||||
|
let (tx, _rx) = broadcast::channel::<AgentEvent>(64);
|
||||||
|
let (watcher_tx, mut watcher_rx) = broadcast::channel::<WatcherEvent>(16);
|
||||||
|
let event_log = Arc::new(Mutex::new(Vec::new()));
|
||||||
|
let child_killers = Arc::new(Mutex::new(HashMap::new()));
|
||||||
|
|
||||||
|
let before = chrono::Utc::now();
|
||||||
|
let result = run_agent_pty_streaming(
|
||||||
|
"496_bug_hard_rate_limit",
|
||||||
|
"coder-1",
|
||||||
|
"sh",
|
||||||
|
&[script.to_string_lossy().to_string()],
|
||||||
|
"--",
|
||||||
|
"/tmp",
|
||||||
|
&tx,
|
||||||
|
&event_log,
|
||||||
|
None,
|
||||||
|
0,
|
||||||
|
child_killers,
|
||||||
|
watcher_tx,
|
||||||
|
)
|
||||||
|
.await;
|
||||||
|
let after = chrono::Utc::now();
|
||||||
|
|
||||||
|
assert!(result.is_ok(), "PTY run should succeed: {:?}", result.err());
|
||||||
|
|
||||||
|
let evt = watcher_rx
|
||||||
|
.try_recv()
|
||||||
|
.expect("Expected a RateLimitHardBlock to be sent on watcher_tx");
|
||||||
|
match evt {
|
||||||
|
WatcherEvent::RateLimitHardBlock {
|
||||||
|
story_id,
|
||||||
|
agent_name,
|
||||||
|
reset_at,
|
||||||
|
} => {
|
||||||
|
assert_eq!(story_id, "496_bug_hard_rate_limit");
|
||||||
|
assert_eq!(agent_name, "coder-1");
|
||||||
|
// reset_at should be ~5 minutes from when the event fired
|
||||||
|
let min_expected = before + chrono::Duration::minutes(4);
|
||||||
|
let max_expected = after + chrono::Duration::minutes(6);
|
||||||
|
assert!(
|
||||||
|
reset_at >= min_expected && reset_at <= max_expected,
|
||||||
|
"reset_at {reset_at} should be ~5 minutes from now"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
other => panic!("Expected RateLimitHardBlock (with default backoff), got: {other:?}"),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn test_emit_event_writes_to_log_writer() {
|
fn test_emit_event_writes_to_log_writer() {
|
||||||
let tmp = tempfile::tempdir().unwrap();
|
let tmp = tempfile::tempdir().unwrap();
|
||||||
|
|||||||
Reference in New Issue
Block a user