mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-08-27 08:00:17 +00:00
feat(timezone): add timezone-aware session context (#671)
* feat(timezone): add timezone-aware session context (#661) All timestamps were UTC-only, causing daily logs to split at UTC midnight, cron schedules to fire in UTC, and no quiet hours for heartbeat. This adds timezone as a per-session property flowing from the client. Key changes: - New `src/timezone.rs` module with resolution chain, parsing, and detection - `IncomingMessage` carries optional timezone from client - `JobContext.user_timezone` flows timezone to tools - `next_cron_fire()` accepts timezone for schedule evaluation - `Trigger::Cron` stores optional timezone (backward-compatible) - Workspace gains `_tz` variants for daily logs and system prompt - Heartbeat supports quiet hours (`HEARTBEAT_QUIET_START/END`) - Web frontend sends `Intl.DateTimeFormat().resolvedOptions().timeZone` - REPL auto-detects system timezone - `DEFAULT_TIMEZONE` env var / settings for server-wide default Storage stays UTC. Conversion happens at display boundaries. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix(timezone): address review feedback on timezone-aware sessions - Validate quiet hours values (0-23) in HeartbeatConfig::resolve() - Fall back to settings values when env vars are unset for quiet hours - Validate IANA timezone strings in routine_create/update with parse_timezone - Add timezone field to routine_create tool schema - Allow standalone timezone update on cron routines without changing schedule - Return path from append_daily_log_tz to avoid TOCTOU race at midnight - Delegate append_daily_log to append_daily_log_tz(entry, UTC) to avoid drift - Preserve timezone through approval flow via PendingApproval.user_timezone - Improve test_today_in_tz to not depend on hardcoded year - Add 3 regression tests for quiet hours config validation Co-Authored-By: Claude Opus 4.6 <[email protected]> * style: fix formatting in routine.rs Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix(timezone): address second round of review feedback - Remove .claude/scheduled_tasks.lock from repo and add to .gitignore - Store resolved timezone (not raw message.timezone) in PendingApproval - Carry forward user_timezone through chained approvals in thread_ops - Wire quiet_hours_start/end from config to HeartbeatRunner - Support X-Timezone header as fallback in chat_send_handler [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix(timezone): include user's local time in time tool response The time tool's "now" operation now returns local_iso and timezone fields based on ctx.user_timezone, so the LLM can report time in the user's timezone instead of always UTC. Co-Authored-By: Claude Opus 4.6 <[email protected]> * style: fix formatting in time.rs Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix(timezone): address Copilot review round 3 — validation, deterministic tests, schema fixes - Validate DEFAULT_TIMEZONE and HEARTBEAT_TIMEZONE at config load time - Add timezone field to HeartbeatSettings and config::HeartbeatConfig - Wire heartbeat timezone from config through agent_loop to HeartbeatRunner - Add timezone to routine_update tool schema (was accepted but not advertised) - Error on schedule/timezone update for non-cron routines - Validate timezone in Trigger::from_db (coerce invalid to None with warning) - Validate timezone in approval path (thread_ops.rs) before overwriting - Time tool always includes timezone/local_iso fields (fallback to UTC) - Make quiet hours tests deterministic using current UTC hour - Add regression tests for config validation [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> --------- Co-authored-by: Claude Opus 4.6 <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 4.6
parent
a20e19ab16
commit
df3635d6be
@@ -356,6 +356,12 @@ impl Agent {
|
||||
if let Some(workspace) = self.workspace() {
|
||||
let mut config = AgentHeartbeatConfig::default()
|
||||
.with_interval(std::time::Duration::from_secs(hb_config.interval_secs));
|
||||
config.quiet_hours_start = hb_config.quiet_hours_start;
|
||||
config.quiet_hours_end = hb_config.quiet_hours_end;
|
||||
config.timezone = hb_config
|
||||
.timezone
|
||||
.clone()
|
||||
.or_else(|| Some(self.config.default_timezone.clone()));
|
||||
if let (Some(user), Some(channel)) =
|
||||
(&hb_config.notify_user, &hb_config.notify_channel)
|
||||
{
|
||||
|
||||
+17
-1
@@ -50,8 +50,18 @@ impl Agent {
|
||||
|
||||
// Load workspace system prompt (identity files: AGENTS.md, SOUL.md, etc.)
|
||||
// In group chats, MEMORY.md is excluded to prevent leaking personal context.
|
||||
// Resolve the user's timezone
|
||||
let user_tz = crate::timezone::resolve_timezone(
|
||||
message.timezone.as_deref(),
|
||||
None, // user setting lookup can be added later
|
||||
&self.config.default_timezone,
|
||||
);
|
||||
|
||||
let system_prompt = if let Some(ws) = self.workspace() {
|
||||
match ws.system_prompt_for_context(is_group_chat).await {
|
||||
match ws
|
||||
.system_prompt_for_context_tz(is_group_chat, user_tz)
|
||||
.await
|
||||
{
|
||||
Ok(prompt) if !prompt.is_empty() => Some(prompt),
|
||||
Ok(_) => None,
|
||||
Err(e) => {
|
||||
@@ -130,6 +140,7 @@ impl Agent {
|
||||
let mut job_ctx =
|
||||
JobContext::with_user(&message.user_id, "chat", "Interactive chat session");
|
||||
job_ctx.http_interceptor = self.deps.http_interceptor.clone();
|
||||
job_ctx.user_timezone = user_tz.name().to_string();
|
||||
|
||||
// Build system prompts once for this turn. Two variants: with tools
|
||||
// (normal iterations) and without (force_text final iteration).
|
||||
@@ -785,6 +796,7 @@ impl Agent {
|
||||
tool_call_id: tc.id.clone(),
|
||||
context_messages: context_messages.clone(),
|
||||
deferred_tool_calls: tool_calls[approval_idx + 1..].to_vec(),
|
||||
user_timezone: Some(user_tz.name().to_string()),
|
||||
};
|
||||
|
||||
return Ok(AgenticLoopResult::NeedApproval { pending });
|
||||
@@ -1146,6 +1158,7 @@ mod tests {
|
||||
max_actions_per_hour: None,
|
||||
max_tool_iterations: 50,
|
||||
auto_approve_tools: false,
|
||||
default_timezone: "UTC".to_string(),
|
||||
},
|
||||
deps,
|
||||
Arc::new(ChannelManager::new()),
|
||||
@@ -1248,6 +1261,7 @@ mod tests {
|
||||
arguments: serde_json::json!({"message": "done"}),
|
||||
},
|
||||
],
|
||||
user_timezone: None,
|
||||
};
|
||||
|
||||
let json = serde_json::to_string(&pending).expect("serialize");
|
||||
@@ -1900,6 +1914,7 @@ mod tests {
|
||||
max_actions_per_hour: None,
|
||||
max_tool_iterations,
|
||||
auto_approve_tools: true,
|
||||
default_timezone: "UTC".to_string(),
|
||||
},
|
||||
deps,
|
||||
Arc::new(ChannelManager::new()),
|
||||
@@ -2015,6 +2030,7 @@ mod tests {
|
||||
max_actions_per_hour: None,
|
||||
max_tool_iterations: max_iter,
|
||||
auto_approve_tools: true,
|
||||
default_timezone: "UTC".to_string(),
|
||||
},
|
||||
deps,
|
||||
Arc::new(ChannelManager::new()),
|
||||
|
||||
@@ -48,6 +48,12 @@ pub struct HeartbeatConfig {
|
||||
pub notify_user_id: Option<String>,
|
||||
/// Channel to notify on heartbeat findings.
|
||||
pub notify_channel: Option<String>,
|
||||
/// Hour (0-23) when quiet hours start.
|
||||
pub quiet_hours_start: Option<u32>,
|
||||
/// Hour (0-23) when quiet hours end.
|
||||
pub quiet_hours_end: Option<u32>,
|
||||
/// Timezone for quiet hours evaluation (IANA name).
|
||||
pub timezone: Option<String>,
|
||||
}
|
||||
|
||||
impl Default for HeartbeatConfig {
|
||||
@@ -58,6 +64,9 @@ impl Default for HeartbeatConfig {
|
||||
max_failures: 3,
|
||||
notify_user_id: None,
|
||||
notify_channel: None,
|
||||
quiet_hours_start: None,
|
||||
quiet_hours_end: None,
|
||||
timezone: None,
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -75,6 +84,26 @@ impl HeartbeatConfig {
|
||||
self
|
||||
}
|
||||
|
||||
/// Check whether the current time falls within configured quiet hours.
|
||||
pub fn is_quiet_hours(&self) -> bool {
|
||||
use chrono::Timelike;
|
||||
let (Some(start), Some(end)) = (self.quiet_hours_start, self.quiet_hours_end) else {
|
||||
return false;
|
||||
};
|
||||
let tz = self
|
||||
.timezone
|
||||
.as_deref()
|
||||
.and_then(crate::timezone::parse_timezone)
|
||||
.unwrap_or(chrono_tz::UTC);
|
||||
let now_hour = crate::timezone::now_in_tz(tz).hour();
|
||||
if start <= end {
|
||||
now_hour >= start && now_hour < end
|
||||
} else {
|
||||
// Wraps midnight, e.g. 22..06
|
||||
now_hour >= start || now_hour < end
|
||||
}
|
||||
}
|
||||
|
||||
/// Set the notification target.
|
||||
pub fn with_notify(mut self, user_id: impl Into<String>, channel: impl Into<String>) -> Self {
|
||||
self.notify_user_id = Some(user_id.into());
|
||||
@@ -162,6 +191,12 @@ impl HeartbeatRunner {
|
||||
loop {
|
||||
interval.tick().await;
|
||||
|
||||
// Skip during quiet hours
|
||||
if self.config.is_quiet_hours() {
|
||||
tracing::debug!("Heartbeat skipped: quiet hours");
|
||||
continue;
|
||||
}
|
||||
|
||||
// Run memory hygiene in the background so it never delays the
|
||||
// heartbeat checklist. Failures are logged inside run_if_due.
|
||||
let hygiene_workspace = Arc::clone(&self.workspace);
|
||||
@@ -532,6 +567,83 @@ mod tests {
|
||||
assert!(!is_effectively_empty(content));
|
||||
}
|
||||
|
||||
// ==================== quiet hours ====================
|
||||
|
||||
#[test]
|
||||
fn test_quiet_hours_inside() {
|
||||
use chrono::{Timelike, Utc};
|
||||
|
||||
let now_utc = Utc::now();
|
||||
let hour = now_utc.hour();
|
||||
let start = hour;
|
||||
let end = (hour + 1) % 24;
|
||||
|
||||
let config = HeartbeatConfig {
|
||||
quiet_hours_start: Some(start),
|
||||
quiet_hours_end: Some(end),
|
||||
timezone: Some("UTC".to_string()),
|
||||
..HeartbeatConfig::default()
|
||||
};
|
||||
// Current UTC hour is inside [start, end) by construction
|
||||
assert!(config.is_quiet_hours());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_quiet_hours_outside() {
|
||||
use chrono::{Timelike, Utc};
|
||||
|
||||
let now_utc = Utc::now();
|
||||
let hour = now_utc.hour();
|
||||
let start = (hour + 1) % 24;
|
||||
let end = (hour + 2) % 24;
|
||||
|
||||
let config = HeartbeatConfig {
|
||||
quiet_hours_start: Some(start),
|
||||
quiet_hours_end: Some(end),
|
||||
timezone: Some("UTC".to_string()),
|
||||
..HeartbeatConfig::default()
|
||||
};
|
||||
// Current UTC hour is outside [start, end) by construction
|
||||
assert!(!config.is_quiet_hours());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_quiet_hours_wraparound_excludes_now() {
|
||||
use chrono::{Timelike, Utc};
|
||||
|
||||
let now_utc = Utc::now();
|
||||
let hour = now_utc.hour();
|
||||
// Window covers all hours except the current one
|
||||
let start = (hour + 1) % 24;
|
||||
let end = hour;
|
||||
|
||||
let config = HeartbeatConfig {
|
||||
quiet_hours_start: Some(start),
|
||||
quiet_hours_end: Some(end),
|
||||
timezone: Some("UTC".to_string()),
|
||||
..HeartbeatConfig::default()
|
||||
};
|
||||
assert!(!config.is_quiet_hours());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_quiet_hours_none_configured() {
|
||||
let config = HeartbeatConfig::default();
|
||||
assert!(!config.is_quiet_hours());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_quiet_hours_same_start_end() {
|
||||
let config = HeartbeatConfig {
|
||||
quiet_hours_start: Some(10),
|
||||
quiet_hours_end: Some(10),
|
||||
timezone: Some("UTC".to_string()),
|
||||
..HeartbeatConfig::default()
|
||||
};
|
||||
// start == end means zero-width window, should be false
|
||||
assert!(!config.is_quiet_hours());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_spawn_heartbeat_accepts_store_param() {
|
||||
// Regression: spawn_heartbeat must accept an optional Database store
|
||||
|
||||
+87
-9
@@ -57,7 +57,11 @@ pub struct Routine {
|
||||
#[serde(tag = "type", rename_all = "snake_case")]
|
||||
pub enum Trigger {
|
||||
/// Fire on a cron schedule (e.g. "0 9 * * MON-FRI" or "every 2h").
|
||||
Cron { schedule: String },
|
||||
Cron {
|
||||
schedule: String,
|
||||
#[serde(default)]
|
||||
timezone: Option<String>,
|
||||
},
|
||||
/// Fire when a channel message matches a pattern.
|
||||
Event {
|
||||
/// Optional channel filter (e.g. "telegram", "slack").
|
||||
@@ -99,7 +103,21 @@ impl Trigger {
|
||||
field: "schedule".into(),
|
||||
})?
|
||||
.to_string();
|
||||
Ok(Trigger::Cron { schedule })
|
||||
let timezone = config
|
||||
.get("timezone")
|
||||
.and_then(|v| v.as_str())
|
||||
.and_then(|tz| {
|
||||
if crate::timezone::parse_timezone(tz).is_some() {
|
||||
Some(tz.to_string())
|
||||
} else {
|
||||
tracing::warn!(
|
||||
"Ignoring invalid timezone '{}' from DB for cron trigger",
|
||||
tz
|
||||
);
|
||||
None
|
||||
}
|
||||
});
|
||||
Ok(Trigger::Cron { schedule, timezone })
|
||||
}
|
||||
"event" => {
|
||||
let pattern = config
|
||||
@@ -137,7 +155,10 @@ impl Trigger {
|
||||
/// Serialize trigger-specific config to JSON for DB storage.
|
||||
pub fn to_config_json(&self) -> serde_json::Value {
|
||||
match self {
|
||||
Trigger::Cron { schedule } => serde_json::json!({ "schedule": schedule }),
|
||||
Trigger::Cron { schedule, timezone } => serde_json::json!({
|
||||
"schedule": schedule,
|
||||
"timezone": timezone,
|
||||
}),
|
||||
Trigger::Event { channel, pattern } => serde_json::json!({
|
||||
"pattern": pattern,
|
||||
"channel": channel,
|
||||
@@ -415,12 +436,25 @@ pub fn content_hash(content: &str) -> u64 {
|
||||
}
|
||||
|
||||
/// Parse a cron expression and compute the next fire time from now.
|
||||
pub fn next_cron_fire(schedule: &str) -> Result<Option<DateTime<Utc>>, RoutineError> {
|
||||
///
|
||||
/// When `timezone` is provided and valid, the schedule is evaluated in that
|
||||
/// timezone and the result is converted back to UTC. Otherwise UTC is used.
|
||||
pub fn next_cron_fire(
|
||||
schedule: &str,
|
||||
timezone: Option<&str>,
|
||||
) -> Result<Option<DateTime<Utc>>, RoutineError> {
|
||||
let cron_schedule =
|
||||
cron::Schedule::from_str(schedule).map_err(|e| RoutineError::InvalidCron {
|
||||
reason: e.to_string(),
|
||||
})?;
|
||||
Ok(cron_schedule.upcoming(Utc).next())
|
||||
if let Some(tz) = timezone.and_then(crate::timezone::parse_timezone) {
|
||||
Ok(cron_schedule
|
||||
.upcoming(tz)
|
||||
.next()
|
||||
.map(|dt| dt.with_timezone(&Utc)))
|
||||
} else {
|
||||
Ok(cron_schedule.upcoming(Utc).next())
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
@@ -433,10 +467,11 @@ mod tests {
|
||||
fn test_trigger_roundtrip() {
|
||||
let trigger = Trigger::Cron {
|
||||
schedule: "0 9 * * MON-FRI".to_string(),
|
||||
timezone: None,
|
||||
};
|
||||
let json = trigger.to_config_json();
|
||||
let parsed = Trigger::from_db("cron", json).expect("parse cron");
|
||||
assert!(matches!(parsed, Trigger::Cron { schedule } if schedule == "0 9 * * MON-FRI"));
|
||||
assert!(matches!(parsed, Trigger::Cron { schedule, .. } if schedule == "0 9 * * MON-FRI"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -509,16 +544,58 @@ mod tests {
|
||||
#[test]
|
||||
fn test_next_cron_fire_valid() {
|
||||
// Every minute should always have a next fire
|
||||
let next = next_cron_fire("* * * * * *").expect("valid cron");
|
||||
let next = next_cron_fire("* * * * * *", None).expect("valid cron");
|
||||
assert!(next.is_some());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_next_cron_fire_invalid() {
|
||||
let result = next_cron_fire("not a cron");
|
||||
let result = next_cron_fire("not a cron", None);
|
||||
assert!(result.is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_trigger_cron_timezone_roundtrip() {
|
||||
let trigger = Trigger::Cron {
|
||||
schedule: "0 9 * * MON-FRI".to_string(),
|
||||
timezone: Some("America/New_York".to_string()),
|
||||
};
|
||||
let json = trigger.to_config_json();
|
||||
let parsed = Trigger::from_db("cron", json).expect("parse cron");
|
||||
assert!(matches!(parsed, Trigger::Cron { schedule, timezone }
|
||||
if schedule == "0 9 * * MON-FRI"
|
||||
&& timezone.as_deref() == Some("America/New_York")));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_trigger_cron_no_timezone_backward_compat() {
|
||||
let json = serde_json::json!({"schedule": "0 9 * * *"});
|
||||
let parsed = Trigger::from_db("cron", json).expect("parse cron");
|
||||
assert!(matches!(parsed, Trigger::Cron { timezone, .. } if timezone.is_none()));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_trigger_cron_invalid_timezone_coerced_to_none() {
|
||||
let json = serde_json::json!({"schedule": "0 9 * * *", "timezone": "Fake/Zone"});
|
||||
let parsed = Trigger::from_db("cron", json).expect("parse cron");
|
||||
assert!(
|
||||
matches!(parsed, Trigger::Cron { timezone, .. } if timezone.is_none()),
|
||||
"invalid timezone should be coerced to None"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_next_cron_fire_with_timezone() {
|
||||
let next_utc = next_cron_fire("0 0 9 * * * *", None)
|
||||
.expect("valid cron")
|
||||
.expect("has next");
|
||||
let next_est = next_cron_fire("0 0 9 * * * *", Some("America/New_York"))
|
||||
.expect("valid cron")
|
||||
.expect("has next");
|
||||
// EST is UTC-5 (or EDT UTC-4), so the UTC result should differ
|
||||
assert_ne!(next_utc, next_est, "timezone should shift the fire time");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_guardrails_default() {
|
||||
let g = RoutineGuardrails::default();
|
||||
@@ -531,7 +608,8 @@ mod tests {
|
||||
fn test_trigger_type_tag() {
|
||||
assert_eq!(
|
||||
Trigger::Cron {
|
||||
schedule: String::new()
|
||||
schedule: String::new(),
|
||||
timezone: None,
|
||||
}
|
||||
.type_tag(),
|
||||
"cron"
|
||||
|
||||
@@ -170,7 +170,7 @@ impl RoutineEngine {
|
||||
continue;
|
||||
}
|
||||
|
||||
let detail = if let Trigger::Cron { ref schedule } = routine.trigger {
|
||||
let detail = if let Trigger::Cron { ref schedule, .. } = routine.trigger {
|
||||
Some(schedule.clone())
|
||||
} else {
|
||||
None
|
||||
@@ -380,8 +380,12 @@ async fn execute_routine(ctx: EngineContext, routine: Routine, run: RoutineRun)
|
||||
|
||||
// Update routine runtime state
|
||||
let now = Utc::now();
|
||||
let next_fire = if let Trigger::Cron { ref schedule } = routine.trigger {
|
||||
next_cron_fire(schedule).unwrap_or(None)
|
||||
let next_fire = if let Trigger::Cron {
|
||||
ref schedule,
|
||||
ref timezone,
|
||||
} = routine.trigger
|
||||
{
|
||||
next_cron_fire(schedule, timezone.as_deref()).unwrap_or(None)
|
||||
} else {
|
||||
None
|
||||
};
|
||||
|
||||
@@ -164,6 +164,10 @@ pub struct PendingApproval {
|
||||
/// executed yet when approval was requested.
|
||||
#[serde(default)]
|
||||
pub deferred_tool_calls: Vec<ToolCall>,
|
||||
/// User timezone at the time the approval was requested, so it persists
|
||||
/// through the approval flow even if the approval message lacks timezone.
|
||||
#[serde(default)]
|
||||
pub user_timezone: Option<String>,
|
||||
}
|
||||
|
||||
/// A conversation thread within a session.
|
||||
@@ -976,6 +980,7 @@ mod tests {
|
||||
tool_call_id: "call_123".to_string(),
|
||||
context_messages: vec![ChatMessage::user("do it")],
|
||||
deferred_tool_calls: vec![],
|
||||
user_timezone: None,
|
||||
};
|
||||
|
||||
thread.await_approval(approval);
|
||||
@@ -1001,6 +1006,7 @@ mod tests {
|
||||
tool_call_id: "call_456".to_string(),
|
||||
context_messages: vec![],
|
||||
deferred_tool_calls: vec![],
|
||||
user_timezone: None,
|
||||
};
|
||||
|
||||
thread.await_approval(approval);
|
||||
|
||||
@@ -746,6 +746,16 @@ impl Agent {
|
||||
let mut job_ctx =
|
||||
JobContext::with_user(&message.user_id, "chat", "Interactive chat session");
|
||||
job_ctx.http_interceptor = self.deps.http_interceptor.clone();
|
||||
// Prefer a valid timezone from the approval message, fall back to the
|
||||
// resolved timezone stored when the approval was originally requested.
|
||||
let tz_candidate = message
|
||||
.timezone
|
||||
.as_deref()
|
||||
.filter(|tz| crate::timezone::parse_timezone(tz).is_some())
|
||||
.or(pending.user_timezone.as_deref());
|
||||
if let Some(tz) = tz_candidate {
|
||||
job_ctx.user_timezone = tz.to_string();
|
||||
}
|
||||
|
||||
let _ = self
|
||||
.channels
|
||||
@@ -1111,6 +1121,8 @@ impl Agent {
|
||||
tool_call_id: tc.id.clone(),
|
||||
context_messages: context_messages.clone(),
|
||||
deferred_tool_calls: deferred_tool_calls[approval_idx + 1..].to_vec(),
|
||||
// Carry forward the resolved timezone from the original pending approval
|
||||
user_timezone: pending.user_timezone.clone(),
|
||||
};
|
||||
|
||||
let request_id = new_pending.request_id;
|
||||
|
||||
Reference in New Issue
Block a user