From c54d56b8fb7e8532f8f65f64503738032a13da13 Mon Sep 17 00:00:00 2001 From: Miguel Rasero Date: Thu, 2 Jul 2026 15:03:38 +0000 Subject: [PATCH 1/3] feat(loop-automation): handle session-limit reset + 529 Overloaded banners MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extends the agentic-loop supervisor's banner detection with two more stall reasons so the "Keep going" loop recovers from them automatically: - "You've hit your session limit · resets 3:10pm (UTC)" is now classified as a usage-/session-window limit. The reset time is parsed timezone-aware ("(UTC)", "(GMT)", "Z", or an explicit +/-HH[:MM] offset) and converted to UTC, and the wake fires 5 minutes after the stated reset so the window has actually rolled (was a 60s buffer). - "API Error: 529 Overloaded" is a new transient LimitSignal::Overloaded with a 4-minute backoff (within the provider's suggested 3-5 min), retried until it clears. Backed by a new WakeupKind::OverloadRetry (plain TEXT column, no migration; renders as a short retry in the UI). The reset-time parser now returns the raw UTC instant and the scheduler owns the post-reset buffer, so usage and session windows can differ cleanly. Adds unit tests for the session-limit banner, 529 detection, and offset conversion (UTC-5, UTC+05:30, bare offsets). Regenerated shared types. --- crates/db/src/models/loop_automation.rs | 9 +- .../local-deployment/src/loop_supervisor.rs | 283 ++++++++++++++++-- shared/types.ts | 2 +- 3 files changed, 260 insertions(+), 34 deletions(-) diff --git a/crates/db/src/models/loop_automation.rs b/crates/db/src/models/loop_automation.rs index 8a813dd0f6..3c4709448c 100644 --- a/crates/db/src/models/loop_automation.rs +++ b/crates/db/src/models/loop_automation.rs @@ -21,7 +21,12 @@ pub enum WakeupKind { /// Transient "Server is temporarily limiting requests · Rate limited" — a /// short backoff retry (every `retry_interval_secs`). RateLimitRetry, - /// A usage-window limit ("usage limit reached") — wake at the reset time. + /// Anthropic "API Error: 529 Overloaded" — a transient server-side error; + /// retry a few minutes out (distinct from `RateLimitRetry` so the two + /// dedupe and back off independently). + OverloadRetry, + /// A usage- or session-window limit ("usage limit reached", "You've hit + /// your session limit · resets 3:10pm") — wake shortly after the reset time. UsageLimitWake, /// User-scheduled ("ping at 05:00 UTC", next-day). Manual, @@ -31,6 +36,7 @@ impl WakeupKind { pub fn as_str(&self) -> &'static str { match self { WakeupKind::RateLimitRetry => "rate_limit_retry", + WakeupKind::OverloadRetry => "overload_retry", WakeupKind::UsageLimitWake => "usage_limit_wake", WakeupKind::Manual => "manual", } @@ -39,6 +45,7 @@ impl WakeupKind { pub fn parse(s: &str) -> Self { match s { "rate_limit_retry" => WakeupKind::RateLimitRetry, + "overload_retry" => WakeupKind::OverloadRetry, "usage_limit_wake" => WakeupKind::UsageLimitWake, _ => WakeupKind::Manual, } diff --git a/crates/local-deployment/src/loop_supervisor.rs b/crates/local-deployment/src/loop_supervisor.rs index 338295f498..abd68f6b14 100644 --- a/crates/local-deployment/src/loop_supervisor.rs +++ b/crates/local-deployment/src/loop_supervisor.rs @@ -18,7 +18,7 @@ use std::time::Duration; -use chrono::{DateTime, Utc}; +use chrono::{DateTime, FixedOffset, TimeZone, Utc}; use db::{ DBService, models::{ @@ -37,6 +37,17 @@ const POLL_INTERVAL: Duration = Duration::from_secs(20); /// limited, so this only bounds how often we re-check during a long window. const USAGE_BACKOFF_SECS: i64 = 1800; // 30 minutes +/// Wake this long after a usage/session window's stated reset time, so the +/// window has actually rolled before we re-prompt (a "resets 3:10pm" banner is +/// only precise to the minute, and the provider clears the cap slightly late). +const USAGE_POST_RESET_BUFFER_SECS: i64 = 300; // 5 minutes + +/// Backoff for an Anthropic "API Error: 529 Overloaded" — a transient +/// server-side error that usually clears within a few minutes; re-detection at +/// wake time reschedules another backoff if it hasn't. Kept inside the +/// provider's suggested 3–5 minute window. +const OVERLOAD_BACKOFF_SECS: i64 = 240; // 4 minutes + /// Env kill-switch (mirrors `DISABLE_CLI_SESSION_REAP`). const DISABLE_ENV: &str = "DISABLE_LOOP_AUTOMATION"; @@ -49,8 +60,12 @@ pub enum LimitSignal { /// Transient provider throttling ("temporarily limiting requests · Rate /// limited") — retry after a short backoff. RateLimited, - /// A usage-window limit ("reached your usage limit") — wake at the reset - /// time when we can parse one. + /// Anthropic "API Error: 529 Overloaded" — a transient server-side error; + /// retry a few minutes out. + Overloaded, + /// A usage- or session-window limit ("reached your usage limit", "You've + /// hit your session limit · resets 3:10pm") — wake at the reset time when we + /// can parse one. UsageLimited { reset_at: Option> }, } @@ -70,9 +85,18 @@ fn detect_limit_at(pane: &str, now: DateTime) -> Option { return Some(LimitSignal::RateLimited); } - // Usage-window limit (5-hour / weekly). Checked after rate-limit so the - // "(not your usage limit)" disclaimer never lands here. + // Transient "API Error: 529 Overloaded" — a server-side blip, not a usage + // cap. Checked before the usage matcher; "overloaded" is distinctive enough + // that a false positive would only cost one harmless retry. + if lower.contains("overloaded") { + return Some(LimitSignal::Overloaded); + } + + // Usage-/session-window limit (5-hour / weekly / session). Checked after the + // transient cases so the rate-limit "(not your usage limit)" disclaimer and + // the 529 banner never land here. if lower.contains("usage limit") + || lower.contains("session limit") || lower.contains("limit reached") || lower.contains("limit will reset") || lower.contains("5-hour limit") @@ -88,29 +112,61 @@ fn detect_limit_at(pane: &str, now: DateTime) -> Option { } /// Best-effort parse of a reset time ("resets at 3:45pm", "limit will reset at -/// 23:00") into the next occurrence of that clock time at-or-after `now`. The -/// banner's timezone isn't reliably stated, so the clock time is interpreted as -/// UTC; an imperfect guess self-corrects because re-detection at wake time -/// reschedules while the limit persists. Returns None when no time is found. +/// 23:00", "resets 3:10pm (UTC)") into the next occurrence of that clock time +/// at-or-after `now`, as a UTC instant (no post-reset buffer — the caller adds +/// one). The clock is interpreted in whatever timezone the banner states +/// (`(UTC)`, `(GMT)`, `Z`, or an explicit `±HH[:MM]` offset) and converted to +/// UTC; when no timezone is stated it's read as UTC, and an imperfect guess +/// self-corrects because re-detection at wake time reschedules while the limit +/// persists. Returns None when no time is found. fn parse_reset_at(text: &str, now: DateTime) -> Option> { let lower = text.to_lowercase(); let anchor = lower.find("reset")?; - let (hour, minute) = parse_clock_after(&lower[anchor..])?; + let after = &lower[anchor..]; + let (hour, minute, clock_end) = parse_clock_after(after)?; + + // The banner states the reset clock time in some zone (often the machine's + // local zone, sometimes an explicit "(UTC)"); read that offset so scheduling + // — which is always in UTC — lands on the right absolute instant. + let offset_secs = parse_tz_offset_secs(&after[clock_end..]).unwrap_or(0); + let zone = FixedOffset::east_opt(offset_secs)?; + + // Next occurrence of `hour:minute` in `zone`, at or after `now`. + let today = now.with_timezone(&zone).date_naive(); + let fire = next_occurrence(zone, today, hour, minute, now)?; + Some(fire) +} - let today = now.date_naive().and_hms_opt(hour, minute, 0)?.and_utc(); - let fire = if today <= now { - today + chrono::Duration::days(1) - } else { - today +/// The UTC instant for `hour:minute` on `date` in `zone`, rolled to the next day +/// if that is already at/before `now`. +fn next_occurrence( + zone: FixedOffset, + date: chrono::NaiveDate, + hour: u32, + minute: u32, + now: DateTime, +) -> Option> { + let in_zone = |d: chrono::NaiveDate| -> Option> { + let naive = d.and_hms_opt(hour, minute, 0)?; + Some( + zone.from_local_datetime(&naive) + .single()? + .with_timezone(&Utc), + ) }; - // Wake a touch after the stated reset so the window has actually rolled. - Some(fire + chrono::Duration::seconds(60)) + let today = in_zone(date)?; + if today <= now { + in_zone(date + chrono::Duration::days(1)) + } else { + Some(today) + } } /// Scan for the first `H`, `H:MM`, optionally `am`/`pm` token in `s` and return -/// `(hour24, minute)`. Returns None if nothing time-like is found or it's out of -/// range. -fn parse_clock_after(s: &str) -> Option<(u32, u32)> { +/// `(hour24, minute, end_index)`, where `end_index` is the byte offset just past +/// the matched time (so the caller can look for a trailing timezone). Returns +/// None if nothing time-like is found or it's out of range. +fn parse_clock_after(s: &str) -> Option<(u32, u32, usize)> { let bytes = s.as_bytes(); let mut i = 0; while i < bytes.len() && !bytes[i].is_ascii_digit() { @@ -142,23 +198,107 @@ fn parse_clock_after(s: &str) -> Option<(u32, u32)> { } // Optional am/pm (allowing a space and "a.m."-style dots). - let rest: String = s[i..] + let marker: String = s[i..] .chars() .take(6) .filter(|c| !c.is_whitespace() && *c != '.') .collect(); - if rest.starts_with("pm") { + if marker.starts_with("pm") { if hour < 12 { hour += 12; } - } else if rest.starts_with("am") && hour == 12 { - hour = 0; + i = advance_past_ampm(s, i); + } else if marker.starts_with("am") { + if hour == 12 { + hour = 0; + } + i = advance_past_ampm(s, i); } if hour > 23 || minute > 59 { return None; } - Some((hour, minute)) + Some((hour, minute, i)) +} + +/// Advance past an `am`/`pm` marker starting at/after `i`, tolerating a leading +/// space and interspersed dots ("a.m."). Consumes at most the two meridiem +/// letters plus their spacing. +fn advance_past_ampm(s: &str, mut i: usize) -> usize { + let bytes = s.as_bytes(); + let mut letters = 0; + while i < bytes.len() && letters < 2 { + match bytes[i] { + b'a' | b'p' | b'm' => { + letters += 1; + i += 1; + } + b'.' | b' ' => i += 1, + _ => break, + } + } + i +} + +/// Parse an explicit timezone token at the start of `s` (already lowercased), +/// returning its offset from UTC in seconds. Recognizes `utc`/`gmt`/`z` +/// (optionally followed by a `±HH[:MM]` offset) and bare numeric offsets like +/// `+05:30`, `-0800`, `+2`, tolerating a leading space and `(`. Returns None +/// when no timezone-like token is present, so the caller falls back to UTC. +fn parse_tz_offset_secs(s: &str) -> Option { + let t = s.trim_start_matches([' ', '(']); + if let Some(rest) = t.strip_prefix("utc").or_else(|| t.strip_prefix("gmt")) { + return Some(parse_signed_offset(rest).unwrap_or(0)); + } + if t.starts_with('z') { + return Some(0); + } + if t.starts_with(['+', '-']) { + return parse_signed_offset(t); + } + None +} + +/// Parse a leading `±HH[:MM]` / `±HHMM` offset into seconds. Returns None if `s` +/// doesn't start with a sign followed by an hour. +fn parse_signed_offset(s: &str) -> Option { + let (sign, rest) = match s.strip_prefix('+') { + Some(r) => (1, r), + None => (-1, s.strip_prefix('-')?), + }; + let bytes = rest.as_bytes(); + let mut i = 0; + while i < bytes.len() && bytes[i].is_ascii_digit() && i < 2 { + i += 1; + } + if i == 0 { + return None; + } + let hours: i32 = rest[..i].parse().ok()?; + + // Optional minutes as ":MM" or a bare "MM" tail ("HHMM"). + let mut minutes: i32 = 0; + if i < bytes.len() && bytes[i] == b':' { + let tail = &rest[i + 1..]; + let mm: String = tail + .chars() + .take(2) + .filter(|c| c.is_ascii_digit()) + .collect(); + if !mm.is_empty() { + minutes = mm.parse().ok()?; + } + } else if i < bytes.len() && bytes[i].is_ascii_digit() { + let tail = &rest[i..]; + if tail.len() >= 2 && tail.as_bytes()[1].is_ascii_digit() { + minutes = tail[..2].parse().ok()?; + } + } + + if hours > 14 || minutes > 59 { + return None; + } + Some(sign * (hours * 3600 + minutes * 60)) } pub struct LoopSupervisor; @@ -294,9 +434,28 @@ async fn detect_and_schedule(db: &DBService, now: DateTime) -> Result<(), s tracing::info!("loop: rate limit on workspace {wid}; retry at {fire_at}"); } } + Some(LimitSignal::Overloaded) => { + if !ScheduledWakeup::has_pending(pool, wid, WakeupKind::OverloadRetry).await? { + let fire_at = now + chrono::Duration::seconds(OVERLOAD_BACKOFF_SECS); + ScheduledWakeup::create( + pool, + wid, + fire_at, + WakeupKind::OverloadRetry, + None, + policy.attempts_used + 1, + ) + .await?; + tracing::info!("loop: 529 overloaded on workspace {wid}; retry at {fire_at}"); + } + } Some(LimitSignal::UsageLimited { reset_at }) => { if !ScheduledWakeup::has_pending(pool, wid, WakeupKind::UsageLimitWake).await? { + // Wake a few minutes after the stated reset so the window has + // rolled; if we couldn't parse a reset time, re-check on a + // coarse backoff instead. let fire_at = reset_at + .map(|r| r + chrono::Duration::seconds(USAGE_POST_RESET_BUFFER_SECS)) .unwrap_or_else(|| now + chrono::Duration::seconds(USAGE_BACKOFF_SECS)); ScheduledWakeup::create( pool, @@ -355,17 +514,18 @@ mod tests { #[test] fn parses_12h_reset_time_to_next_occurrence() { - // now 09:00 UTC, "resets at 3:45pm" -> today 15:45 (+60s buffer). + // now 09:00 UTC, "resets at 3:45pm" -> today 15:45 (raw; buffer added by + // the scheduler, not this parser). let now = at(2026, 6, 30, 9, 0); let got = parse_reset_at("Your limit will reset at 3:45pm", now).unwrap(); - assert_eq!(got, at(2026, 6, 30, 15, 45) + chrono::Duration::seconds(60)); + assert_eq!(got, at(2026, 6, 30, 15, 45)); } #[test] fn parses_24h_reset_time() { let now = at(2026, 6, 30, 9, 0); let got = parse_reset_at("limit will reset at 23:00", now).unwrap(); - assert_eq!(got, at(2026, 6, 30, 23, 0) + chrono::Duration::seconds(60)); + assert_eq!(got, at(2026, 6, 30, 23, 0)); } #[test] @@ -373,7 +533,7 @@ mod tests { // now 16:00, "resets 3pm" -> 15:00 already past -> tomorrow 15:00. let now = at(2026, 6, 30, 16, 0); let got = parse_reset_at("resets 3pm", now).unwrap(); - assert_eq!(got, at(2026, 7, 1, 15, 0) + chrono::Duration::seconds(60)); + assert_eq!(got, at(2026, 7, 1, 15, 0)); } #[test] @@ -387,8 +547,67 @@ mod tests { #[test] fn parse_clock_handles_noon_and_midnight() { - assert_eq!(parse_clock_after("at 12am"), Some((0, 0))); - assert_eq!(parse_clock_after("at 12pm"), Some((12, 0))); - assert_eq!(parse_clock_after("at 12:30pm"), Some((12, 30))); + let hm = |s| parse_clock_after(s).map(|(h, m, _)| (h, m)); + assert_eq!(hm("at 12am"), Some((0, 0))); + assert_eq!(hm("at 12pm"), Some((12, 0))); + assert_eq!(hm("at 12:30pm"), Some((12, 30))); + } + + #[test] + fn session_limit_banner_is_classified_as_usage_with_reset() { + // The exact Claude Code banner, reset time stated in UTC. + let now = at(2026, 6, 30, 9, 0); + let banner = "You've hit your session limit · resets 3:10pm (UTC)"; + match detect_limit_at(banner, now) { + Some(LimitSignal::UsageLimited { reset_at }) => { + assert_eq!(reset_at, Some(at(2026, 6, 30, 15, 10))); + } + other => panic!("expected session/usage limit, got {other:?}"), + } + } + + #[test] + fn overloaded_529_is_transient() { + let now = at(2026, 6, 30, 9, 0); + let banner = "API Error: 529 Overloaded. This is a server-side issue, usually temporary - try again in a moment."; + assert_eq!(detect_limit_at(banner, now), Some(LimitSignal::Overloaded)); + } + + #[test] + fn reset_time_with_explicit_offset_converts_to_utc() { + let now = at(2026, 6, 30, 9, 0); + // 3:10pm in UTC-5 == 20:10 UTC. + let got = parse_reset_at("resets 3:10pm (UTC-5)", now).unwrap(); + assert_eq!(got, at(2026, 6, 30, 20, 10)); + + // 09:00 in UTC+05:30 == 03:30 UTC (already past 09:00 UTC now → rolls to + // tomorrow). + let got = parse_reset_at("limit will reset at 09:00 (UTC+05:30)", now).unwrap(); + assert_eq!(got, at(2026, 7, 1, 3, 30)); + } + + #[test] + fn utc_label_matches_bare_utc_interpretation() { + let now = at(2026, 6, 30, 9, 0); + let labeled = parse_reset_at("resets 3:10pm (UTC)", now).unwrap(); + let bare = parse_reset_at("resets 3:10pm", now).unwrap(); + assert_eq!(labeled, bare); + assert_eq!(labeled, at(2026, 6, 30, 15, 10)); + } + + #[test] + fn parses_signed_offsets() { + assert_eq!(parse_tz_offset_secs("(utc)"), Some(0)); + assert_eq!(parse_tz_offset_secs(" utc"), Some(0)); + assert_eq!(parse_tz_offset_secs("gmt"), Some(0)); + assert_eq!(parse_tz_offset_secs("z"), Some(0)); + assert_eq!(parse_tz_offset_secs("(utc+2)"), Some(2 * 3600)); + assert_eq!( + parse_tz_offset_secs("(utc-05:30)"), + Some(-(5 * 3600 + 30 * 60)) + ); + assert_eq!(parse_tz_offset_secs("+0800"), Some(8 * 3600)); + assert_eq!(parse_tz_offset_secs(" reset soon"), None); + assert_eq!(parse_tz_offset_secs(""), None); } } diff --git a/shared/types.ts b/shared/types.ts index 30b7c4599e..c65c1ff5a1 100644 --- a/shared/types.ts +++ b/shared/types.ts @@ -321,7 +321,7 @@ export type AgentPresetOptionsQuery = { executor: BaseCodingAgent, variant: stri export type CurrentUserResponse = { user_id: string, }; -export type WakeupKind = "rate_limit_retry" | "usage_limit_wake" | "manual"; +export type WakeupKind = "rate_limit_retry" | "overload_retry" | "usage_limit_wake" | "manual"; export type LoopAutomation = { workspace_id: string, enabled: boolean, retry_interval_secs: bigint, continuation_prompt: string, max_attempts: bigint, attempts_used: bigint, updated_at: string, }; From 102308ef20831296a29bed7c5697157e6046730a Mon Sep 17 00:00:00 2001 From: Miguel Rasero Date: Thu, 2 Jul 2026 15:58:10 +0000 Subject: [PATCH 2/3] refactor(loop-automation): harden limit detection from review feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Applies the consensus findings from a multi-agent review (Codex + council + code-review xhigh) of the session-limit / 529 additions: - Reset times with no explicit zone are now read in the host's local timezone (converted to UTC), not assumed-UTC — CLI agents render reset clocks in local time. Explicit (UTC)/(GMT)/Z/±HH[:MM] still win. `parse_reset_at` takes an injectable default_tz so this is deterministically testable. - Delivery now checks the *same* limit kind is still visible (not just "some limit"), so an OverloadRetry can't fire into a now-different usage/rate cap and burn an attempt. Adds `LimitSignal::wakeup_kind()`. - Detection order is rate-limit -> usage/session -> 529, so a stale "overloaded" line above a live usage banner no longer masks the window's reset (which would hammer a hard-capped agent every 4 min). - 529 detection is line-scoped (529 and "overloaded" on the same line) to avoid false positives from incidental pane text. - Day-roll fix: `parse_reset_at` returns today's clock instant; the new `usage_wake_at` adds the 5-min buffer and only rolls to tomorrow when the buffered wake has already elapsed — a reset seen minutes late wakes soon instead of ~24h later (matters across the frequent fork restarts). - `advance_past_ampm` consumes the trailing dot of "3:10 p.m." so a following "(UTC-5)" isn't dropped. - New `WakeupKind::Unknown`: unknown persisted kinds parse to it instead of `Manual`, so a rolled-back/foreign row is skipped by the kind-match gate rather than delivered unconditionally. - Collapses the three near-identical scheduler arms into one shared insert. Not changed (with reason): the migration comment listing kinds is left stale — editing an already-applied migration changes its SQLx checksum and breaks validation on existing DBs. The fixed 4-min overload backoff is intentional per the desired 3-5 min retry; the attempt cap bounds any sustained-outage hammering. Adds tests for local-tz default, dotted-meridiem offset, usage_wake_at buffer/ roll boundary, signal->kind mapping, usage-wins-over-stale-529, line-scoped 529, and unknown-kind parsing. Regenerated shared types. --- crates/db/src/models/loop_automation.rs | 35 +- .../local-deployment/src/loop_supervisor.rs | 427 ++++++++++++------ shared/types.ts | 2 +- 3 files changed, 324 insertions(+), 140 deletions(-) diff --git a/crates/db/src/models/loop_automation.rs b/crates/db/src/models/loop_automation.rs index 3c4709448c..0fae4ad46d 100644 --- a/crates/db/src/models/loop_automation.rs +++ b/crates/db/src/models/loop_automation.rs @@ -30,6 +30,12 @@ pub enum WakeupKind { UsageLimitWake, /// User-scheduled ("ping at 05:00 UTC", next-day). Manual, + /// A kind this build doesn't recognize — e.g. a row written by a newer + /// version and read back after a rollback. Deliberately distinct from + /// `Manual` (which delivers unconditionally): an `Unknown` wake-up matches + /// no live limit signal, so the supervisor skips it rather than firing a + /// spurious prompt. Never produced by this build's own scheduling. + Unknown, } impl WakeupKind { @@ -39,6 +45,7 @@ impl WakeupKind { WakeupKind::OverloadRetry => "overload_retry", WakeupKind::UsageLimitWake => "usage_limit_wake", WakeupKind::Manual => "manual", + WakeupKind::Unknown => "unknown", } } @@ -47,7 +54,8 @@ impl WakeupKind { "rate_limit_retry" => WakeupKind::RateLimitRetry, "overload_retry" => WakeupKind::OverloadRetry, "usage_limit_wake" => WakeupKind::UsageLimitWake, - _ => WakeupKind::Manual, + "manual" => WakeupKind::Manual, + _ => WakeupKind::Unknown, } } } @@ -383,3 +391,28 @@ impl ScheduledWakeup { Ok(()) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn wakeup_kind_parse_roundtrips_known_variants() { + for kind in [ + WakeupKind::RateLimitRetry, + WakeupKind::OverloadRetry, + WakeupKind::UsageLimitWake, + WakeupKind::Manual, + ] { + assert_eq!(WakeupKind::parse(kind.as_str()), kind); + } + } + + #[test] + fn wakeup_kind_parse_maps_unknown_strings_to_unknown_not_manual() { + // A newer version's kind read back after a rollback must NOT become + // `Manual` (which delivers unconditionally) — it maps to `Unknown`. + assert_eq!(WakeupKind::parse("some_future_kind"), WakeupKind::Unknown); + assert_eq!(WakeupKind::parse(""), WakeupKind::Unknown); + } +} diff --git a/crates/local-deployment/src/loop_supervisor.rs b/crates/local-deployment/src/loop_supervisor.rs index abd68f6b14..05a68b2d88 100644 --- a/crates/local-deployment/src/loop_supervisor.rs +++ b/crates/local-deployment/src/loop_supervisor.rs @@ -18,7 +18,7 @@ use std::time::Duration; -use chrono::{DateTime, FixedOffset, TimeZone, Utc}; +use chrono::{DateTime, FixedOffset, Local, Offset, TimeZone, Utc}; use db::{ DBService, models::{ @@ -69,14 +69,33 @@ pub enum LimitSignal { UsageLimited { reset_at: Option> }, } +impl LimitSignal { + /// The wake-up kind used to schedule (and dedupe) a retry for this signal, + /// and to check on delivery that the *same* limit is still showing. + fn wakeup_kind(&self) -> WakeupKind { + match self { + LimitSignal::RateLimited => WakeupKind::RateLimitRetry, + LimitSignal::Overloaded => WakeupKind::OverloadRetry, + LimitSignal::UsageLimited { .. } => WakeupKind::UsageLimitWake, + } + } +} + +/// The machine's current UTC offset — CLI-agent banners render reset times in +/// the host's local zone (the supervisor runs on the same host), so this is the +/// default used to interpret a reset clock time that carries no explicit zone. +fn local_offset() -> FixedOffset { + Local::now().offset().fix() +} + /// Classify a CLI pane's visible content. Rate-limit is checked first because /// the provider's rate-limit banner explicitly says "(not your usage limit)", /// which would otherwise trip the usage-limit matcher. pub fn detect_limit(pane: &str) -> Option { - detect_limit_at(pane, Utc::now()) + detect_limit_at(pane, Utc::now(), local_offset()) } -fn detect_limit_at(pane: &str, now: DateTime) -> Option { +fn detect_limit_at(pane: &str, now: DateTime, default_tz: FixedOffset) -> Option { let lower = pane.to_lowercase(); // Transient rate limit — the user's exact phrasing: "Server is temporarily @@ -85,18 +104,15 @@ fn detect_limit_at(pane: &str, now: DateTime) -> Option { return Some(LimitSignal::RateLimited); } - // Transient "API Error: 529 Overloaded" — a server-side blip, not a usage - // cap. Checked before the usage matcher; "overloaded" is distinctive enough - // that a false positive would only cost one harmless retry. - if lower.contains("overloaded") { - return Some(LimitSignal::Overloaded); - } - - // Usage-/session-window limit (5-hour / weekly / session). Checked after the - // transient cases so the rate-limit "(not your usage limit)" disclaimer and - // the 529 banner never land here. + // Usage-/session-window limit (5-hour / weekly / session). Checked before the + // 529 case: a pane can show a stale "overloaded" line above a current + // usage/session banner, and the window limit (with its concrete reset time) + // must win over a transient retry — otherwise we'd hammer a hard-capped agent + // every few minutes instead of waking once at its reset. "session limit" is + // narrowed to the actual banner phrasing ("hit your session limit") so it + // can't match chatter. if lower.contains("usage limit") - || lower.contains("session limit") + || lower.contains("hit your session limit") || lower.contains("limit reached") || lower.contains("limit will reset") || lower.contains("5-hour limit") @@ -104,61 +120,79 @@ fn detect_limit_at(pane: &str, now: DateTime) -> Option { || lower.contains("out of usage") { return Some(LimitSignal::UsageLimited { - reset_at: parse_reset_at(pane, now), + reset_at: parse_reset_at(pane, now, default_tz), }); } + // Transient "API Error: 529 Overloaded" — a server-side blip, not a usage + // cap. Require the 529 code and the word on the SAME line so ordinary pane + // text (code, logs, docs) that merely mentions either in passing isn't + // misread as a provider error. + if lower + .lines() + .any(|line| line.contains("overloaded") && line.contains("529")) + { + return Some(LimitSignal::Overloaded); + } + None } /// Best-effort parse of a reset time ("resets at 3:45pm", "limit will reset at -/// 23:00", "resets 3:10pm (UTC)") into the next occurrence of that clock time -/// at-or-after `now`, as a UTC instant (no post-reset buffer — the caller adds -/// one). The clock is interpreted in whatever timezone the banner states -/// (`(UTC)`, `(GMT)`, `Z`, or an explicit `±HH[:MM]` offset) and converted to -/// UTC; when no timezone is stated it's read as UTC, and an imperfect guess -/// self-corrects because re-detection at wake time reschedules while the limit -/// persists. Returns None when no time is found. -fn parse_reset_at(text: &str, now: DateTime) -> Option> { +/// 23:00", "resets 3:10pm (UTC)") into the UTC instant for that clock time on +/// *today's* date. The clock is interpreted in the timezone the banner states +/// (`(UTC)`, `(GMT)`, `Z`, or an explicit `±HH[:MM]` offset); when no zone is +/// stated it's read in `default_tz` — the host's local zone in production, since +/// CLI agents render reset times in local time — then converted to UTC so +/// scheduling (always in UTC) lands on the right absolute instant. An imperfect +/// guess self-corrects because re-detection at wake time reschedules while the +/// limit persists. +/// +/// The result may be in the past (we detected the banner after its reset +/// elapsed) — deciding whether that means "wake soon" or "the next reset is +/// tomorrow" belongs to [`usage_wake_at`], not here, so the day-roll accounts +/// for the post-reset buffer. Returns None when no time is found. +fn parse_reset_at( + text: &str, + now: DateTime, + default_tz: FixedOffset, +) -> Option> { let lower = text.to_lowercase(); let anchor = lower.find("reset")?; let after = &lower[anchor..]; let (hour, minute, clock_end) = parse_clock_after(after)?; - // The banner states the reset clock time in some zone (often the machine's - // local zone, sometimes an explicit "(UTC)"); read that offset so scheduling - // — which is always in UTC — lands on the right absolute instant. - let offset_secs = parse_tz_offset_secs(&after[clock_end..]).unwrap_or(0); - let zone = FixedOffset::east_opt(offset_secs)?; + let zone = match parse_tz_offset_secs(&after[clock_end..]) { + Some(secs) => FixedOffset::east_opt(secs)?, + None => default_tz, + }; - // Next occurrence of `hour:minute` in `zone`, at or after `now`. let today = now.with_timezone(&zone).date_naive(); - let fire = next_occurrence(zone, today, hour, minute, now)?; - Some(fire) + let naive = today.and_hms_opt(hour, minute, 0)?; + Some( + zone.from_local_datetime(&naive) + .single()? + .with_timezone(&Utc), + ) } -/// The UTC instant for `hour:minute` on `date` in `zone`, rolled to the next day -/// if that is already at/before `now`. -fn next_occurrence( - zone: FixedOffset, - date: chrono::NaiveDate, - hour: u32, - minute: u32, - now: DateTime, -) -> Option> { - let in_zone = |d: chrono::NaiveDate| -> Option> { - let naive = d.and_hms_opt(hour, minute, 0)?; - Some( - zone.from_local_datetime(&naive) - .single()? - .with_timezone(&Utc), - ) - }; - let today = in_zone(date)?; - if today <= now { - in_zone(date + chrono::Duration::days(1)) - } else { - Some(today) +/// When to actually re-prompt after a usage/session-window limit whose window +/// resets at `reset_at` (None if the banner named no time). Adds the post-reset +/// buffer to the stated reset; if that instant has *already elapsed* (we saw the +/// banner well after its reset — e.g. after a supervisor restart), assume the +/// next same-clock reset is a day out rather than hammering the provider +/// immediately. Falls back to a coarse backoff when the banner named no time. +fn usage_wake_at(reset_at: Option>, now: DateTime) -> DateTime { + match reset_at { + Some(reset) => { + let fire = reset + chrono::Duration::seconds(USAGE_POST_RESET_BUFFER_SECS); + if fire <= now { + fire + chrono::Duration::days(1) + } else { + fire + } + } + None => now + chrono::Duration::seconds(USAGE_BACKOFF_SECS), } } @@ -222,14 +256,16 @@ fn parse_clock_after(s: &str) -> Option<(u32, u32, usize)> { } /// Advance past an `am`/`pm` marker starting at/after `i`, tolerating a leading -/// space and interspersed dots ("a.m."). Consumes at most the two meridiem -/// letters plus their spacing. +/// space and interspersed/trailing dots ("a.m."). Consumes the two meridiem +/// letters plus surrounding dots and spaces, so a following timezone token +/// (`(UTC-5)`) is left cleanly for the caller. The two-letter cap keeps it from +/// eating into a trailing word (e.g. the "m" of a later "may"). fn advance_past_ampm(s: &str, mut i: usize) -> usize { let bytes = s.as_bytes(); let mut letters = 0; - while i < bytes.len() && letters < 2 { + while i < bytes.len() { match bytes[i] { - b'a' | b'p' | b'm' => { + b'a' | b'p' | b'm' if letters < 2 => { letters += 1; i += 1; } @@ -372,15 +408,19 @@ async fn deliver_due_wakeups(db: &DBService, _now: DateTime) -> Result<(), continue; } - // For a limit-kind wake, only re-prompt if the limit banner is STILL - // showing — a manually-resumed agent must not be interrupted. Manual - // wake-ups are the user's explicit intent and always deliver. + // For a limit-kind wake, only re-prompt if the SAME kind of limit is + // still showing — a manually-resumed agent must not be interrupted, and + // an overload retry must not fire into a now-different (usage/rate) + // limit, which would waste an attempt and re-hit a real cap. Matching + // the visible signal's kind also makes an `Unknown` wake (no signal maps + // to it) skip itself. Manual wake-ups are the user's explicit intent and + // always deliver. if is_limit { - let still_limited = capture_cli_pane(wid) + let still_matches = capture_cli_pane(wid) .await - .map(|pane| detect_limit(&pane).is_some()) - .unwrap_or(false); - if !still_limited { + .and_then(|pane| detect_limit(&pane)) + .is_some_and(|sig| sig.wakeup_kind() == wakeup.kind); + if !still_matches { ScheduledWakeup::mark_fired(pool, wakeup.id).await?; continue; } @@ -417,60 +457,28 @@ async fn detect_and_schedule(db: &DBService, now: DateTime) -> Result<(), s continue; }; - match detect_limit_at(&pane, now) { - Some(LimitSignal::RateLimited) => { - if !ScheduledWakeup::has_pending(pool, wid, WakeupKind::RateLimitRetry).await? { - let fire_at = - now + chrono::Duration::seconds(policy.retry_interval_secs.max(1)); - ScheduledWakeup::create( - pool, - wid, - fire_at, - WakeupKind::RateLimitRetry, - None, - policy.attempts_used + 1, - ) - .await?; - tracing::info!("loop: rate limit on workspace {wid}; retry at {fire_at}"); - } - } - Some(LimitSignal::Overloaded) => { - if !ScheduledWakeup::has_pending(pool, wid, WakeupKind::OverloadRetry).await? { - let fire_at = now + chrono::Duration::seconds(OVERLOAD_BACKOFF_SECS); - ScheduledWakeup::create( - pool, - wid, - fire_at, - WakeupKind::OverloadRetry, - None, - policy.attempts_used + 1, - ) - .await?; - tracing::info!("loop: 529 overloaded on workspace {wid}; retry at {fire_at}"); - } - } - Some(LimitSignal::UsageLimited { reset_at }) => { - if !ScheduledWakeup::has_pending(pool, wid, WakeupKind::UsageLimitWake).await? { - // Wake a few minutes after the stated reset so the window has - // rolled; if we couldn't parse a reset time, re-check on a - // coarse backoff instead. - let fire_at = reset_at - .map(|r| r + chrono::Duration::seconds(USAGE_POST_RESET_BUFFER_SECS)) - .unwrap_or_else(|| now + chrono::Duration::seconds(USAGE_BACKOFF_SECS)); - ScheduledWakeup::create( - pool, - wid, - fire_at, - WakeupKind::UsageLimitWake, - None, - policy.attempts_used + 1, - ) - .await?; - tracing::info!("loop: usage limit on workspace {wid}; wake at {fire_at}"); - } - } - None => {} + // All three limit kinds schedule the same way — dedupe by kind, then + // create one wake-up — differing only in when to fire, so compute the + // kind + fire time and share the insert. + let Some(signal) = detect_limit_at(&pane, now, local_offset()) else { + continue; + }; + let kind = signal.wakeup_kind(); + if ScheduledWakeup::has_pending(pool, wid, kind).await? { + continue; } + let fire_at = match &signal { + LimitSignal::RateLimited => { + now + chrono::Duration::seconds(policy.retry_interval_secs.max(1)) + } + LimitSignal::Overloaded => now + chrono::Duration::seconds(OVERLOAD_BACKOFF_SECS), + LimitSignal::UsageLimited { reset_at } => usage_wake_at(*reset_at, now), + }; + ScheduledWakeup::create(pool, wid, fire_at, kind, None, policy.attempts_used + 1).await?; + tracing::info!( + "loop: {} on workspace {wid}; wake at {fire_at}", + kind.as_str() + ); } Ok(()) } @@ -485,6 +493,22 @@ mod tests { Utc.with_ymd_and_hms(y, mo, d, h, mi, 0).unwrap() } + /// A fixed UTC offset for the `default_tz` slot — keeps banner-parsing tests + /// deterministic regardless of the host's actual local zone. + fn utc() -> FixedOffset { + FixedOffset::east_opt(0).unwrap() + } + + /// `detect_limit_at` with a deterministic (UTC) default zone. + fn detect(pane: &str, now: DateTime) -> Option { + detect_limit_at(pane, now, utc()) + } + + /// `parse_reset_at` with a deterministic (UTC) default zone. + fn reset_at(text: &str, now: DateTime) -> Option> { + parse_reset_at(text, now, utc()) + } + #[test] fn rate_limit_banner_is_classified_first() { let pane = "API Error: Server is temporarily limiting requests (not your usage limit) · Rate limited"; @@ -502,7 +526,7 @@ mod tests { #[test] fn usage_limit_banner_is_classified() { let now = at(2026, 6, 30, 9, 0); - let got = detect_limit_at("You've reached your usage limit.", now); + let got = detect("You've reached your usage limit.", now); assert!(matches!(got, Some(LimitSignal::UsageLimited { .. }))); } @@ -513,33 +537,34 @@ mod tests { } #[test] - fn parses_12h_reset_time_to_next_occurrence() { - // now 09:00 UTC, "resets at 3:45pm" -> today 15:45 (raw; buffer added by - // the scheduler, not this parser). + fn parses_12h_reset_time() { + // now 09:00 UTC, "resets at 3:45pm" -> today 15:45 (raw; buffer + any + // day-roll are the scheduler's job, not this parser's). let now = at(2026, 6, 30, 9, 0); - let got = parse_reset_at("Your limit will reset at 3:45pm", now).unwrap(); + let got = reset_at("Your limit will reset at 3:45pm", now).unwrap(); assert_eq!(got, at(2026, 6, 30, 15, 45)); } #[test] fn parses_24h_reset_time() { let now = at(2026, 6, 30, 9, 0); - let got = parse_reset_at("limit will reset at 23:00", now).unwrap(); + let got = reset_at("limit will reset at 23:00", now).unwrap(); assert_eq!(got, at(2026, 6, 30, 23, 0)); } #[test] - fn reset_time_already_past_rolls_to_tomorrow() { - // now 16:00, "resets 3pm" -> 15:00 already past -> tomorrow 15:00. + fn parse_reset_at_returns_todays_clock_even_when_past() { + // now 16:00, "resets 3pm" -> today 15:00 (already past). The parser does + // NOT roll to tomorrow; usage_wake_at owns that decision. let now = at(2026, 6, 30, 16, 0); - let got = parse_reset_at("resets 3pm", now).unwrap(); - assert_eq!(got, at(2026, 7, 1, 15, 0)); + let got = reset_at("resets 3pm", now).unwrap(); + assert_eq!(got, at(2026, 6, 30, 15, 0)); } #[test] fn usage_limit_without_a_time_has_no_reset() { let now = at(2026, 6, 30, 9, 0); - match detect_limit_at("You are out of usage for now.", now) { + match detect("You are out of usage for now.", now) { Some(LimitSignal::UsageLimited { reset_at }) => assert!(reset_at.is_none()), other => panic!("expected usage limit, got {other:?}"), } @@ -558,7 +583,7 @@ mod tests { // The exact Claude Code banner, reset time stated in UTC. let now = at(2026, 6, 30, 9, 0); let banner = "You've hit your session limit · resets 3:10pm (UTC)"; - match detect_limit_at(banner, now) { + match detect(banner, now) { Some(LimitSignal::UsageLimited { reset_at }) => { assert_eq!(reset_at, Some(at(2026, 6, 30, 15, 10))); } @@ -570,31 +595,131 @@ mod tests { fn overloaded_529_is_transient() { let now = at(2026, 6, 30, 9, 0); let banner = "API Error: 529 Overloaded. This is a server-side issue, usually temporary - try again in a moment."; - assert_eq!(detect_limit_at(banner, now), Some(LimitSignal::Overloaded)); + assert_eq!(detect(banner, now), Some(LimitSignal::Overloaded)); + } + + #[test] + fn usage_limit_wins_over_a_stale_overloaded_line() { + // A pane showing a lingering 529 line ABOVE a current session-limit banner + // must classify as the usage window (wake once at reset), not as a + // transient overload (which would hammer the capped agent every 4 min). + let now = at(2026, 6, 30, 9, 0); + let pane = "API Error: 529 Overloaded\n\nYou've hit your session limit · resets 3:10pm (UTC)"; + match detect(pane, now) { + Some(LimitSignal::UsageLimited { reset_at }) => { + assert_eq!(reset_at, Some(at(2026, 6, 30, 15, 10))); + } + other => panic!("expected usage limit to win, got {other:?}"), + } + } + + #[test] + fn tightened_matchers_reject_casual_mentions() { + let now = at(2026, 6, 30, 9, 0); + // "overloaded" without the 529 code is ordinary output, not a provider error. + assert_eq!(detect("the worker pool looks overloaded", now), None); + // A bare "session limit" mention (e.g. discussing the feature) is not a cap. + assert_eq!( + detect("TODO: document the session limit behavior", now), + None + ); + // 529 and overloaded on DIFFERENT lines is not the provider banner. + assert_eq!( + detect("saw error 529 earlier\nthe queue looked overloaded", now), + None + ); } #[test] fn reset_time_with_explicit_offset_converts_to_utc() { let now = at(2026, 6, 30, 9, 0); // 3:10pm in UTC-5 == 20:10 UTC. - let got = parse_reset_at("resets 3:10pm (UTC-5)", now).unwrap(); - assert_eq!(got, at(2026, 6, 30, 20, 10)); + assert_eq!( + reset_at("resets 3:10pm (UTC-5)", now), + Some(at(2026, 6, 30, 20, 10)) + ); + // 20:00 in UTC+05:30 == 14:30 UTC. + assert_eq!( + reset_at("limit will reset at 20:00 (UTC+05:30)", now), + Some(at(2026, 6, 30, 14, 30)) + ); + } + + #[test] + fn dotted_meridiem_keeps_explicit_offset() { + // "3:00 p.m." must not swallow/block the trailing "(UTC+2)". + let now = at(2026, 6, 30, 9, 0); + // 15:00 in UTC+2 == 13:00 UTC. + assert_eq!( + reset_at("limit will reset at 3:00 p.m. (UTC+2)", now), + Some(at(2026, 6, 30, 13, 0)) + ); + } - // 09:00 in UTC+05:30 == 03:30 UTC (already past 09:00 UTC now → rolls to - // tomorrow). - let got = parse_reset_at("limit will reset at 09:00 (UTC+05:30)", now).unwrap(); - assert_eq!(got, at(2026, 7, 1, 3, 30)); + #[test] + fn bare_time_uses_the_default_timezone() { + // No explicit zone → interpret in default_tz (the host's local zone in + // production). 3:10pm in UTC-7 == 22:10 UTC. + let now = at(2026, 6, 30, 9, 0); + let pdt = FixedOffset::east_opt(-7 * 3600).unwrap(); + assert_eq!( + parse_reset_at("resets 3:10pm", now, pdt), + Some(at(2026, 6, 30, 22, 10)) + ); } #[test] fn utc_label_matches_bare_utc_interpretation() { + // With a UTC default, a bare time and an explicit "(UTC)" agree. let now = at(2026, 6, 30, 9, 0); - let labeled = parse_reset_at("resets 3:10pm (UTC)", now).unwrap(); - let bare = parse_reset_at("resets 3:10pm", now).unwrap(); + let labeled = reset_at("resets 3:10pm (UTC)", now).unwrap(); + let bare = reset_at("resets 3:10pm", now).unwrap(); assert_eq!(labeled, bare); assert_eq!(labeled, at(2026, 6, 30, 15, 10)); } + #[test] + fn usage_wake_at_applies_buffer_and_rolls_only_when_elapsed() { + let buffer = chrono::Duration::seconds(USAGE_POST_RESET_BUFFER_SECS); + let reset = at(2026, 6, 30, 15, 10); + // Reset ahead of now → wake reset+buffer today. + assert_eq!( + usage_wake_at(Some(reset), at(2026, 6, 30, 9, 0)), + reset + buffer + ); + // Reset a minute ago but within the buffer → still today (NOT tomorrow). + assert_eq!( + usage_wake_at(Some(reset), at(2026, 6, 30, 15, 12)), + reset + buffer + ); + // Reset elapsed beyond the buffer → assume next same-clock reset tomorrow. + assert_eq!( + usage_wake_at(Some(reset), at(2026, 6, 30, 15, 30)), + reset + buffer + chrono::Duration::days(1) + ); + // No parsed time → coarse backoff from now. + assert_eq!( + usage_wake_at(None, at(2026, 6, 30, 9, 0)), + at(2026, 6, 30, 9, 0) + chrono::Duration::seconds(USAGE_BACKOFF_SECS) + ); + } + + #[test] + fn signal_maps_to_expected_wakeup_kind() { + assert_eq!( + LimitSignal::RateLimited.wakeup_kind(), + WakeupKind::RateLimitRetry + ); + assert_eq!( + LimitSignal::Overloaded.wakeup_kind(), + WakeupKind::OverloadRetry + ); + assert_eq!( + LimitSignal::UsageLimited { reset_at: None }.wakeup_kind(), + WakeupKind::UsageLimitWake + ); + } + #[test] fn parses_signed_offsets() { assert_eq!(parse_tz_offset_secs("(utc)"), Some(0)); @@ -610,4 +735,30 @@ mod tests { assert_eq!(parse_tz_offset_secs(" reset soon"), None); assert_eq!(parse_tz_offset_secs(""), None); } + + #[test] + fn parses_bare_hhmm_offset_minutes() { + // "HHMM" form with non-zero minutes must not be truncated to the hour. + assert_eq!(parse_tz_offset_secs("+0530"), Some(5 * 3600 + 30 * 60)); + assert_eq!( + parse_tz_offset_secs("(utc-0830)"), + Some(-(8 * 3600 + 30 * 60)) + ); + } + + #[test] + fn parse_clock_after_is_utf8_boundary_safe() { + // Multi-byte chars (middot, em dash, accented letter) around the clock + // must never slice on a non-char boundary. + let hm = |s| parse_clock_after(s).map(|(h, m, _)| (h, m)); + assert_eq!(hm("reset · 3:10pm"), Some((15, 10))); + assert_eq!(hm("reset — 9am"), Some((9, 0))); + assert_eq!(hm("résets 23:00"), Some((23, 0))); + // Full pipeline on the real banner (leading apostrophe + middot). + let now = at(2026, 6, 30, 9, 0); + assert_eq!( + reset_at("You've hit your session limit · resets 3:10pm (UTC)", now), + Some(at(2026, 6, 30, 15, 10)) + ); + } } diff --git a/shared/types.ts b/shared/types.ts index c65c1ff5a1..778506992b 100644 --- a/shared/types.ts +++ b/shared/types.ts @@ -321,7 +321,7 @@ export type AgentPresetOptionsQuery = { executor: BaseCodingAgent, variant: stri export type CurrentUserResponse = { user_id: string, }; -export type WakeupKind = "rate_limit_retry" | "overload_retry" | "usage_limit_wake" | "manual"; +export type WakeupKind = "rate_limit_retry" | "overload_retry" | "usage_limit_wake" | "manual" | "unknown"; export type LoopAutomation = { workspace_id: string, enabled: boolean, retry_interval_secs: bigint, continuation_prompt: string, max_attempts: bigint, attempts_used: bigint, updated_at: string, }; From 1c828bfcb4b733305cddc7211d8c92fdc65b1928 Mon Sep 17 00:00:00 2001 From: Miguel Rasero Date: Thu, 2 Jul 2026 16:04:29 +0000 Subject: [PATCH 3/3] fix(loop-automation): address CodeRabbit review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Leave a due wake-up pending (retry next tick) when pane capture fails transiently, instead of marking it fired and losing the retry — the session still exists (checked earlier), so a capture blip must not drop the wake-up. - parse_tz_offset_secs no longer collapses a malformed UTC/GMT offset (e.g. "UTC+99", "UTCx") to plain UTC; it returns None so the caller falls back to its default zone. Adds tests for both malformed forms. --- .../local-deployment/src/loop_supervisor.rs | 34 +++++++++++++++---- 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/crates/local-deployment/src/loop_supervisor.rs b/crates/local-deployment/src/loop_supervisor.rs index 05a68b2d88..575af2d389 100644 --- a/crates/local-deployment/src/loop_supervisor.rs +++ b/crates/local-deployment/src/loop_supervisor.rs @@ -280,11 +280,24 @@ fn advance_past_ampm(s: &str, mut i: usize) -> usize { /// returning its offset from UTC in seconds. Recognizes `utc`/`gmt`/`z` /// (optionally followed by a `±HH[:MM]` offset) and bare numeric offsets like /// `+05:30`, `-0800`, `+2`, tolerating a leading space and `(`. Returns None -/// when no timezone-like token is present, so the caller falls back to UTC. +/// when no timezone-like token is present — or when a `utc`/`gmt` token is +/// followed by a malformed offset (e.g. `UTC+99`, `UTCx`) — so the caller falls +/// back to its default zone rather than silently pretending it's UTC. fn parse_tz_offset_secs(s: &str) -> Option { let t = s.trim_start_matches([' ', '(']); if let Some(rest) = t.strip_prefix("utc").or_else(|| t.strip_prefix("gmt")) { - return Some(parse_signed_offset(rest).unwrap_or(0)); + let rest = rest.trim_start(); + if rest.starts_with(['+', '-']) { + // An explicit offset attached to UTC/GMT: honor it, or reject the + // whole token if it's out of range / unparseable. + return parse_signed_offset(rest); + } + // Bare `UTC`/`GMT` — allow only a trailing separator (closing paren, + // comma, period). Anything else is unrecognized, not UTC. + return rest + .trim_start_matches([')', ',', '.', ' ']) + .is_empty() + .then_some(0); } if t.starts_with('z') { return Some(0); @@ -416,10 +429,15 @@ async fn deliver_due_wakeups(db: &DBService, _now: DateTime) -> Result<(), // to it) skip itself. Manual wake-ups are the user's explicit intent and // always deliver. if is_limit { - let still_matches = capture_cli_pane(wid) - .await - .and_then(|pane| detect_limit(&pane)) - .is_some_and(|sig| sig.wakeup_kind() == wakeup.kind); + // A failed capture is transient (the session still exists — that was + // checked above); leave the wake-up pending and retry next tick + // rather than marking it fired and losing the retry. + let Some(pane) = capture_cli_pane(wid).await else { + tracing::warn!("loop: pane capture failed for due wake-up on workspace {wid}"); + continue; + }; + let still_matches = + detect_limit(&pane).is_some_and(|sig| sig.wakeup_kind() == wakeup.kind); if !still_matches { ScheduledWakeup::mark_fired(pool, wakeup.id).await?; continue; @@ -734,6 +752,10 @@ mod tests { assert_eq!(parse_tz_offset_secs("+0800"), Some(8 * 3600)); assert_eq!(parse_tz_offset_secs(" reset soon"), None); assert_eq!(parse_tz_offset_secs(""), None); + // A malformed offset on UTC/GMT is rejected (→ caller's default zone), + // not silently collapsed to UTC. + assert_eq!(parse_tz_offset_secs("(utc+99)"), None); + assert_eq!(parse_tz_offset_secs("(utcx)"), None); } #[test]