Skip to content

Commit ee9d729

Browse files
Cstewart-HCgreptile-apps[bot]penso
authored
feat(cron): add heartbeat wake cooldown to prevent exec re-fire loop (#871)
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> Co-authored-by: Fabien Penso <gpg@pen.so>
1 parent 762db89 commit ee9d729

13 files changed

Lines changed: 295 additions & 6 deletions

File tree

crates/config/src/schema/system.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,10 @@ pub struct HeartbeatConfig {
182182
pub sandbox_enabled: bool,
183183
/// Override sandbox image for heartbeat. If `None`, uses the default image.
184184
pub sandbox_image: Option<String>,
185+
/// Minimum duration between exec-triggered heartbeat wakes (e.g. "5m", "0").
186+
/// Prevents exec-completion callbacks from re-waking the heartbeat in a tight loop.
187+
/// Defaults to "5m". Set to "0" to disable.
188+
pub wake_cooldown: String,
185189
}
186190

187191
impl Default for HeartbeatConfig {
@@ -198,6 +202,7 @@ impl Default for HeartbeatConfig {
198202
to: None,
199203
sandbox_enabled: true,
200204
sandbox_image: None,
205+
wake_cooldown: "5m".into(),
201206
}
202207
}
203208
}

crates/config/src/template.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -490,6 +490,7 @@ port = {port} # Port number (auto-generated for this i
490490
# ack_max_chars = 300 # Max characters for acknowledgment reply
491491
# deliver = false # Deliver heartbeat replies to a channel
492492
# sandbox_enabled = true # Run heartbeat commands in sandbox
493+
# wake_cooldown = "5m" # Min duration between exec-triggered heartbeat wakes (0 to disable)
493494
494495
# [heartbeat.active_hours]
495496
# start = "08:00"

crates/config/src/validate/schema_map.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -535,6 +535,7 @@ pub(super) fn build_schema_map() -> KnownKeys {
535535
("to", Leaf),
536536
("sandbox_enabled", Leaf),
537537
("sandbox_image", Leaf),
538+
("wake_cooldown", Leaf),
538539
])),
539540
),
540541
(

crates/cron/src/heartbeat.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -461,7 +461,7 @@ mod tests {
461461
let events = vec![
462462
crate::system_events::SystemEvent {
463463
text: "Command `ls` exited 0".into(),
464-
reason: "exec-event".into(),
464+
reason: crate::service::WAKE_REASON_EXEC_EVENT.into(),
465465
enqueued_at_ms: 1000,
466466
},
467467
crate::system_events::SystemEvent {
@@ -472,7 +472,10 @@ mod tests {
472472
];
473473
let prompt = build_event_enriched_prompt(&events, "check inbox");
474474
assert!(prompt.starts_with(EVENTS_PROMPT_PREFIX));
475-
assert!(prompt.contains("Command `ls` exited 0 [exec-event]"));
475+
assert!(prompt.contains(&format!(
476+
"Command `ls` exited 0 [{}]",
477+
crate::service::WAKE_REASON_EXEC_EVENT
478+
)));
476479
assert!(prompt.contains("Cron job fired [cron:abc]"));
477480
assert!(prompt.ends_with("check inbox"));
478481
}

crates/cron/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ pub mod system_events;
1515
pub mod types;
1616

1717
pub use error::{Error, Result};
18+
pub use service::{DEFAULT_WAKE_COOLDOWN_MS, WAKE_REASON_CRON_EVENT, WAKE_REASON_EXEC_EVENT};
1819

1920
/// Run database migrations for the cron crate.
2021
///

crates/cron/src/parse.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,19 @@ use crate::{Error, Result};
55
/// Parse a human-friendly duration string into milliseconds.
66
///
77
/// Supported suffixes: `s` (seconds), `m` (minutes), `h` (hours), `d` (days).
8-
/// Examples: `"30s"`, `"5m"`, `"2h"`, `"1d"`.
8+
/// A bare `"0"` is accepted as a disable sentinel that returns `0`.
9+
/// Examples: `"30s"`, `"5m"`, `"2h"`, `"1d"`, `"0"`.
910
pub fn parse_duration_ms(input: &str) -> Result<u64> {
1011
let input = input.trim();
1112
if input.is_empty() {
1213
return Err(Error::message("empty duration string"));
1314
}
1415

16+
// Bare "0" is the disable sentinel.
17+
if input == "0" {
18+
return Ok(0);
19+
}
20+
1521
let (num_str, suffix) = match input.find(|c: char| c.is_alphabetic()) {
1622
Some(i) => (&input[..i], &input[i..]),
1723
None => {
@@ -74,6 +80,8 @@ mod tests {
7480
#[case("2h", 7_200_000)]
7581
#[case("1d", 86_400_000)]
7682
#[case(" 10m ", 600_000)]
83+
#[case("0", 0)]
84+
#[case(" 0 ", 0)]
7785
fn test_parse_duration_ok(#[case] input: &str, #[case] expected: u64) {
7886
assert_eq!(parse_duration_ms(input).unwrap(), expected);
7987
}

crates/cron/src/service.rs

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,11 +128,30 @@ pub struct CronService {
128128
on_notify: Option<NotifyFn>,
129129
rate_limiter: Mutex<RateLimiter>,
130130
events_queue: Arc<SystemEventsQueue>,
131+
/// Minimum ms between exec-triggered heartbeat wakes. Zero disables cooldown.
132+
wake_cooldown_ms: u64,
131133
}
132134

133135
/// Max time a job can be in "running" state before we consider it stuck (2 hours).
134136
const STUCK_THRESHOLD_MS: u64 = 2 * 60 * 60 * 1000;
135137

138+
/// Minimum cooldown between exec-triggered heartbeat wake calls.
139+
///
140+
/// Prevents exec-completion callbacks from re-waking the heartbeat
141+
/// in a tight loop when the agent uses `exec` during a heartbeat turn.
142+
/// The wake is skipped if the heartbeat last completed less than this
143+
/// duration ago. This is a safety net — the scheduled interval still
144+
/// applies for normal periodic firing.
145+
///
146+
/// This cooldown only applies to exec-triggered wakes ([`WAKE_REASON_EXEC_EVENT`]).
147+
/// CronWakeMode::Now wakes ([`WAKE_REASON_CRON_EVENT`]) are never suppressed.
148+
pub const DEFAULT_WAKE_COOLDOWN_MS: u64 = 5 * 60 * 1000;
149+
150+
/// Wake reason: exec-completion callback.
151+
pub const WAKE_REASON_EXEC_EVENT: &str = "exec-event";
152+
/// Wake reason: cron job with [`CronWakeMode::Now`](crate::types::CronWakeMode::Now) finished.
153+
pub const WAKE_REASON_CRON_EVENT: &str = "cron-event";
154+
136155
fn now_ms() -> u64 {
137156
SystemTime::now()
138157
.duration_since(UNIX_EPOCH)
@@ -152,6 +171,7 @@ impl CronService {
152171
on_agent_turn,
153172
None,
154173
RateLimitConfig::default(),
174+
DEFAULT_WAKE_COOLDOWN_MS,
155175
)
156176
}
157177

@@ -168,6 +188,7 @@ impl CronService {
168188
on_agent_turn,
169189
Some(on_notify),
170190
RateLimitConfig::default(),
191+
DEFAULT_WAKE_COOLDOWN_MS,
171192
)
172193
}
173194

@@ -178,13 +199,15 @@ impl CronService {
178199
on_agent_turn: AgentTurnFn,
179200
on_notify: Option<NotifyFn>,
180201
rate_limit_config: RateLimitConfig,
202+
wake_cooldown_ms: u64,
181203
) -> Arc<Self> {
182204
Self::with_events_queue(
183205
store,
184206
on_system_event,
185207
on_agent_turn,
186208
on_notify,
187209
rate_limit_config,
210+
wake_cooldown_ms,
188211
SystemEventsQueue::new(),
189212
)
190213
}
@@ -199,6 +222,7 @@ impl CronService {
199222
on_agent_turn: AgentTurnFn,
200223
on_notify: Option<NotifyFn>,
201224
rate_limit_config: RateLimitConfig,
225+
wake_cooldown_ms: u64,
202226
events_queue: Arc<SystemEventsQueue>,
203227
) -> Arc<Self> {
204228
Arc::new(Self {
@@ -212,6 +236,7 @@ impl CronService {
212236
on_notify,
213237
rate_limiter: Mutex::new(RateLimiter::new(rate_limit_config)),
214238
events_queue,
239+
wake_cooldown_ms,
215240
})
216241
}
217242

@@ -224,13 +249,38 @@ impl CronService {
224249
///
225250
/// Multiple wake calls coalesce naturally: they all set `next_run_at_ms = now`
226251
/// idempotently, and `running_at_ms` prevents the heartbeat from firing twice.
252+
///
253+
/// When called with reason [`WAKE_REASON_EXEC_EVENT`], a cooldown guard applies: if the
254+
/// heartbeat last completed less than `wake_cooldown_ms` ago, the wake is skipped.
255+
/// This prevents exec-completion callbacks from creating a re-fire loop.
256+
/// Other reasons (e.g. [`WAKE_REASON_CRON_EVENT`]) are never suppressed.
227257
pub async fn wake(&self, reason: &str) {
228258
let now = now_ms();
229259
let mut jobs = self.jobs.write().await;
230260
if let Some(job) = jobs.iter_mut().find(|j| j.id == "__heartbeat__")
231261
&& job.enabled
232262
&& job.state.running_at_ms.is_none()
233263
{
264+
// Enforce cooldown for exec-triggered wakes only. This prevents
265+
// exec-completion callbacks from creating a re-fire loop when the
266+
// heartbeat agent uses `exec` during its turn. CronWakeMode::Now wakes
267+
// are never suppressed.
268+
if reason == WAKE_REASON_EXEC_EVENT
269+
&& self.wake_cooldown_ms > 0
270+
&& let Some(last_run) = job.state.last_run_at_ms
271+
{
272+
let elapsed = now.saturating_sub(last_run);
273+
if elapsed < self.wake_cooldown_ms {
274+
debug!(
275+
reason,
276+
elapsed_ms = elapsed,
277+
cooldown_ms = self.wake_cooldown_ms,
278+
"skipping heartbeat wake — within cooldown"
279+
);
280+
return;
281+
}
282+
}
283+
234284
debug!(reason, "waking heartbeat");
235285
job.state.next_run_at_ms = Some(now);
236286
}
@@ -665,7 +715,7 @@ impl CronService {
665715

666716
// Wake heartbeat immediately if this job requested it.
667717
if job.wake_mode == CronWakeMode::Now && job.id != "__heartbeat__" {
668-
self.wake("cron-event").await;
718+
self.wake(WAKE_REASON_CRON_EVENT).await;
669719
}
670720

671721
info!(

0 commit comments

Comments
 (0)