mirror of
https://github.com/openai/codex.git
synced 2026-10-26 18:38:47 +00:00
Default loop alarms to idle recurrence
This commit is contained in:
@@ -145,7 +145,7 @@ Example with notification opt-out:
|
||||
- `thread/unsubscribe` — unsubscribe this connection from thread turn/item events. If this was the last subscriber, the server shuts down and unloads the thread, then emits `thread/closed`.
|
||||
- `thread/name/set` — set or update a thread’s user-facing name for either a loaded thread or a persisted rollout; returns `{}` on success and emits `thread/name/updated` to initialized, opted-in clients. Thread names are not required to be unique; name lookups resolve to the most recently updated thread.
|
||||
- `thread/unarchive` — move an archived rollout file back into the sessions directory; returns the restored `thread` on success and emits `thread/unarchived`.
|
||||
- `thread/alarm/create` — create a runtime alarm on a loaded thread; accepts a structured `trigger`, `prompt`, and `delivery`, returns the created alarm, and requires the under-development feature flag `alarm_tool` to be enabled for that thread. Delay triggers use `seconds` plus optional `repeat`; schedule triggers use optional `dtstart` and/or `rrule`.
|
||||
- `thread/alarm/create` — create a runtime alarm on a loaded thread; accepts a structured `trigger`, `prompt`, and `delivery`, returns the created alarm, and requires the under-development feature flag `alarm_tool` to be enabled for that thread. Delay triggers use `seconds` plus optional `repeat`; `seconds: 0, repeat: true` means run whenever the thread is idle. Schedule triggers use optional `dtstart` and/or `rrule`.
|
||||
- `thread/alarm/delete` — delete one runtime alarm from a loaded thread by id; returns `{ deleted }` and requires `alarm_tool`.
|
||||
- `thread/alarm/list` — list runtime alarms for a loaded thread; returns `{ data }` and requires `alarm_tool`.
|
||||
- `thread/alarm/updated` — notification emitted when a loaded thread’s alarm list changes, including one-shot alarms deleting themselves after execution or alarms restored from disk.
|
||||
|
||||
@@ -48,6 +48,16 @@ impl AlarmTrigger {
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn is_idle_recurring(&self) -> bool {
|
||||
matches!(
|
||||
self,
|
||||
Self::Delay {
|
||||
seconds: 0,
|
||||
repeat: Some(true),
|
||||
}
|
||||
)
|
||||
}
|
||||
|
||||
pub(crate) fn display(&self) -> String {
|
||||
match self {
|
||||
Self::Delay { seconds, repeat } => {
|
||||
@@ -115,7 +125,7 @@ fn timing_for_new_trigger_with_timezone(
|
||||
AlarmTrigger::Delay { seconds, repeat } => {
|
||||
let repeat = repeat.unwrap_or(false);
|
||||
if repeat && *seconds == 0 {
|
||||
return Err("delay.repeat requires seconds to be greater than 0".to_string());
|
||||
return Ok(timing(normalized, /*pending_run*/ true, None, now));
|
||||
}
|
||||
let next_run_at = checked_add_seconds(created_at, *seconds)?;
|
||||
let pending_run = next_run_at <= now;
|
||||
@@ -170,7 +180,7 @@ fn timing_for_restored_trigger_with_timezone(
|
||||
AlarmTrigger::Delay { seconds, repeat } => {
|
||||
let repeat = repeat.unwrap_or(false);
|
||||
if repeat && *seconds == 0 {
|
||||
return Err("delay.repeat requires seconds to be greater than 0".to_string());
|
||||
return Ok(timing(normalized, /*pending_run*/ true, None, now));
|
||||
}
|
||||
let next_run_at = persisted_next_run_at
|
||||
.or_else(|| next_delay_run_at(created_at, *seconds))
|
||||
@@ -221,6 +231,9 @@ fn next_run_after_due_with_timezone(
|
||||
match trigger {
|
||||
AlarmTrigger::Delay { seconds, repeat } => {
|
||||
if repeat.unwrap_or(false) {
|
||||
if *seconds == 0 {
|
||||
return Ok(None);
|
||||
}
|
||||
next_delay_recurring_run_at_from_timestamp(created_at, *seconds, now)
|
||||
} else {
|
||||
Ok(None)
|
||||
@@ -239,12 +252,7 @@ fn normalize_trigger(
|
||||
timezone: Tz,
|
||||
) -> Result<AlarmTrigger, String> {
|
||||
match trigger {
|
||||
AlarmTrigger::Delay { seconds, repeat } => {
|
||||
if repeat.unwrap_or(false) && seconds == 0 {
|
||||
return Err("delay.repeat requires seconds to be greater than 0".to_string());
|
||||
}
|
||||
Ok(AlarmTrigger::Delay { seconds, repeat })
|
||||
}
|
||||
AlarmTrigger::Delay { seconds, repeat } => Ok(AlarmTrigger::Delay { seconds, repeat }),
|
||||
AlarmTrigger::Schedule { dtstart, rrule } => {
|
||||
let dtstart = normalize_optional_string(dtstart);
|
||||
let rrule = normalize_optional_string(rrule);
|
||||
@@ -480,19 +488,41 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn delay_repeat_rejects_zero_seconds() {
|
||||
fn delay_repeat_zero_is_idle_recurring() {
|
||||
let timing = timing_for_new_trigger_with_timezone(
|
||||
AlarmTrigger::Delay {
|
||||
seconds: 0,
|
||||
repeat: Some(true),
|
||||
},
|
||||
utc(TS_100),
|
||||
utc(TS_100),
|
||||
Tz::UTC,
|
||||
)
|
||||
.expect("zero repeat should be a valid idle-recurring trigger");
|
||||
assert_eq!(
|
||||
timing_for_new_trigger_with_timezone(
|
||||
AlarmTrigger::Delay {
|
||||
timing,
|
||||
TriggerTiming {
|
||||
trigger: AlarmTrigger::Delay {
|
||||
seconds: 0,
|
||||
repeat: Some(true),
|
||||
},
|
||||
utc(TS_100),
|
||||
pending_run: true,
|
||||
next_run_at: None,
|
||||
timer_delay: None,
|
||||
}
|
||||
);
|
||||
assert_eq!(
|
||||
next_run_after_due_with_timezone(
|
||||
&AlarmTrigger::Delay {
|
||||
seconds: 0,
|
||||
repeat: Some(true),
|
||||
},
|
||||
/*created_at*/ 100,
|
||||
utc(TS_100),
|
||||
Tz::UTC,
|
||||
)
|
||||
.expect_err("zero repeat should be invalid"),
|
||||
"delay.repeat requires seconds to be greater than 0"
|
||||
.expect("zero repeat should remain timer-free"),
|
||||
None
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -291,6 +291,12 @@ impl AlarmsState {
|
||||
.values()
|
||||
.filter(|runtime| runtime.pending_run)
|
||||
.filter_map(|runtime| {
|
||||
if runtime.alarm.trigger.is_idle_recurring() {
|
||||
if can_after_turn {
|
||||
return Some((runtime, AlarmDelivery::AfterTurn));
|
||||
}
|
||||
return None;
|
||||
}
|
||||
let actual_delivery = match runtime.alarm.delivery {
|
||||
AlarmDelivery::AfterTurn if can_after_turn => AlarmDelivery::AfterTurn,
|
||||
AlarmDelivery::AfterTurn => return None,
|
||||
@@ -326,11 +332,12 @@ impl AlarmsState {
|
||||
}
|
||||
} else {
|
||||
alarm.last_run_at = Some(now.timestamp());
|
||||
let pending_run = alarm.trigger.is_idle_recurring();
|
||||
self.alarms.insert(
|
||||
alarm.id.clone(),
|
||||
AlarmRuntime {
|
||||
alarm: alarm.clone(),
|
||||
pending_run: false,
|
||||
pending_run,
|
||||
timer_cancel,
|
||||
},
|
||||
);
|
||||
@@ -486,6 +493,7 @@ mod tests {
|
||||
use super::CreateAlarm;
|
||||
use super::MAX_ACTIVE_ALARMS_PER_THREAD;
|
||||
use super::PersistedAlarm;
|
||||
use super::ThreadAlarm;
|
||||
use super::alarm_prompt_input_item;
|
||||
use super::alarm_sidecar_path_for_rollout;
|
||||
use super::load_alarm_sidecar;
|
||||
@@ -588,6 +596,73 @@ mod tests {
|
||||
assert_eq!(second.context.current_alarm_id, "alarm-2");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn idle_recurring_alarm_remains_pending_after_claim() {
|
||||
let now = Utc.timestamp_opt(100, 0).single().expect("valid timestamp");
|
||||
let mut alarms = AlarmsState::default();
|
||||
let (alarm, timer_spec) = alarms
|
||||
.create_alarm(
|
||||
CreateAlarm {
|
||||
id: "alarm-1".to_string(),
|
||||
trigger: delay(ZERO_SECONDS, Some(true)),
|
||||
prompt: "keep going".to_string(),
|
||||
delivery: AlarmDelivery::AfterTurn,
|
||||
now,
|
||||
},
|
||||
/*timer_cancel*/ None,
|
||||
)
|
||||
.expect("alarm should be created");
|
||||
assert_eq!(timer_spec, None);
|
||||
|
||||
let claimed = alarms
|
||||
.claim_next_alarm(
|
||||
now, /*can_after_turn*/ true, /*can_steer_current_turn*/ true,
|
||||
)
|
||||
.expect("alarm should be claimed");
|
||||
assert!(!claimed.deleted_one_shot_alarm);
|
||||
assert_eq!(
|
||||
alarms.persisted_alarms(),
|
||||
vec![PersistedAlarm {
|
||||
alarm: ThreadAlarm {
|
||||
last_run_at: Some(100),
|
||||
..alarm
|
||||
},
|
||||
pending_run: true,
|
||||
}]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn idle_recurring_alarm_waits_for_idle_even_if_delivery_requests_steer() {
|
||||
let now = Utc.timestamp_opt(100, 0).single().expect("valid timestamp");
|
||||
let mut alarms = AlarmsState::default();
|
||||
alarms
|
||||
.create_alarm(
|
||||
CreateAlarm {
|
||||
id: "alarm-1".to_string(),
|
||||
trigger: delay(ZERO_SECONDS, Some(true)),
|
||||
prompt: "keep going".to_string(),
|
||||
delivery: AlarmDelivery::SteerCurrentTurn,
|
||||
now,
|
||||
},
|
||||
/*timer_cancel*/ None,
|
||||
)
|
||||
.expect("alarm should be created");
|
||||
|
||||
assert_eq!(
|
||||
alarms.claim_next_alarm(
|
||||
now, /*can_after_turn*/ false, /*can_steer_current_turn*/ true,
|
||||
),
|
||||
None
|
||||
);
|
||||
let claimed = alarms
|
||||
.claim_next_alarm(
|
||||
now, /*can_after_turn*/ true, /*can_steer_current_turn*/ false,
|
||||
)
|
||||
.expect("alarm should be claimed when idle");
|
||||
assert_eq!(claimed.context.delivery, AlarmDelivery::AfterTurn);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn create_alarm_rejects_more_than_maximum_active_alarms() {
|
||||
let now = Utc.timestamp_opt(100, 0).single().expect("valid timestamp");
|
||||
|
||||
@@ -26,7 +26,10 @@ pub fn create_alarm_create_tool() -> ToolSpec {
|
||||
(
|
||||
"repeat".to_string(),
|
||||
JsonSchema::Boolean {
|
||||
description: Some("Delay trigger recurrence flag.".to_string()),
|
||||
description: Some(
|
||||
"Delay trigger recurrence flag. With seconds 0, repeat means run whenever the thread is idle."
|
||||
.to_string(),
|
||||
),
|
||||
},
|
||||
),
|
||||
(
|
||||
|
||||
@@ -275,12 +275,13 @@ Return only the JSON object requested by the response schema.
|
||||
Rules:
|
||||
- Extract the alarm prompt by removing the scheduling phrase but preserving the user's requested task.
|
||||
- Use delivery "after-turn" unless the user clearly asks for same-turn/current-turn steering; then use "steer-current-turn".
|
||||
- For "now", "immediately", or specs with no explicit timing, use { "kind": "delay", "seconds": 0, "repeat": false }.
|
||||
- Treat `/loop` as recurring by default. Only create a one-shot alarm when the user clearly asks for a single run, one-time reminder, or other one-off action.
|
||||
- For "now", "immediately", or specs with no explicit timing, use { "kind": "delay", "seconds": 0, "repeat": true } unless the user clearly asked for one-shot behavior. This means the alarm fires whenever the thread is idle.
|
||||
- For delay triggers, set dtstart and rrule to null.
|
||||
- For schedule triggers, set seconds and repeat to null.
|
||||
- For relative one-shot timing like "in 30 seconds", use a delay trigger with seconds set to the relative delay and repeat false.
|
||||
- For relative timing like "in 30 seconds", use a delay trigger with seconds set to the relative delay and repeat true unless the user clearly asked for one-shot behavior.
|
||||
- For interval timing like "every 5 minutes", use a delay trigger with seconds set to the interval and repeat true.
|
||||
- For one-shot wall-clock timing like "at 9pm", use a schedule trigger with dtstart set to the next matching local datetime in YYYY-MM-DDTHH:MM:SS and rrule null.
|
||||
- For one-shot wall-clock timing like "at 9pm once" or "one time at 9pm", use a schedule trigger with dtstart set to the next matching local datetime in YYYY-MM-DDTHH:MM:SS and rrule null.
|
||||
- For recurring calendar timing, use a schedule trigger with rrule set to an RFC 5545 RRULE string and dtstart set when the user supplies a start datetime; otherwise null.
|
||||
- For schedule triggers, use floating local wall-clock datetimes without timezone suffixes.
|
||||
"#;
|
||||
@@ -316,6 +317,18 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parser_prompt_defaults_ambiguous_loop_to_idle_recurring() {
|
||||
assert!(
|
||||
PARSE_ALARM_SYSTEM_PROMPT
|
||||
.contains(r#"{ "kind": "delay", "seconds": 0, "repeat": true }"#)
|
||||
);
|
||||
assert!(
|
||||
PARSE_ALARM_SYSTEM_PROMPT
|
||||
.contains("Only create a one-shot alarm when the user clearly asks")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parsed_alarm_spec_accepts_permissive_delay_trigger_shape() {
|
||||
let parsed: ParsedAlarmSpec = serde_json::from_value(json!({
|
||||
|
||||
Reference in New Issue
Block a user