From 3c2dcbef853de6f0e0a2ad702635883759e339a7 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Mon, 4 May 2026 08:58:07 -0700 Subject: [PATCH] Keep paused goals paused on thread resume (#20790) ## Summary Early adopters of the `/goal` feature have provided feedback that they expect a goal they explicitly paused to remain paused when they resume a thread. Previously, resuming a thread would reactivate a paused goal. This PR keeps persisted goal status unchanged during thread resume. This honors the user feedback while also simplifying the core goal logic. Rather than have the core logic automatically resume a paused goal, that responsibility is transferred to the client. The TUI now detects a resumed thread with a paused goal and asks the user whether to `Resume goal` or `Leave paused`. The prompt appears only for quiet resume flows, so users who resume with an immediate prompt are not interrupted. image --- .../tests/suite/v2/thread_resume.rs | 6 +- codex-rs/core/src/goals.rs | 94 +++++-------------- codex-rs/core/src/thread_manager_tests.rs | 6 +- codex-rs/tui/src/app.rs | 11 +++ codex-rs/tui/src/app/session_lifecycle.rs | 6 ++ codex-rs/tui/src/app/tests.rs | 36 +++++++ codex-rs/tui/src/app/thread_goal_actions.rs | 27 ++++++ codex-rs/tui/src/app/thread_routing.rs | 10 ++ codex-rs/tui/src/chatwidget/goal_menu.rs | 35 +++++++ ...get__tests__resume_paused_goal_prompt.snap | 11 +++ .../tui/src/chatwidget/tests/goal_menu.rs | 50 ++++++++++ 11 files changed, 214 insertions(+), 78 deletions(-) create mode 100644 codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__resume_paused_goal_prompt.snap diff --git a/codex-rs/app-server/tests/suite/v2/thread_resume.rs b/codex-rs/app-server/tests/suite/v2/thread_resume.rs index 48673387b..55c0d96eb 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_resume.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_resume.rs @@ -385,7 +385,7 @@ async fn thread_resume_can_skip_turns_for_metadata_only_resume() -> Result<()> { } #[tokio::test] -async fn thread_resume_emits_active_goal_update_before_continuation() -> Result<()> { +async fn thread_resume_keeps_paused_goal_paused() -> Result<()> { let server = create_mock_responses_server_repeating_assistant("Done").await; let codex_home = TempDir::new()?; create_config_toml(codex_home.path(), &server.uri())?; @@ -477,12 +477,12 @@ async fn thread_resume_emits_active_goal_update_before_continuation() -> Result< let ServerNotification::ThreadGoalUpdated(notification) = notification else { anyhow::bail!("expected thread goal update notification"); }; - assert_eq!(notification.goal.status, ThreadGoalStatus::Active); + assert_eq!(notification.goal.status, ThreadGoalStatus::Paused); assert!( !mcp.pending_notification_methods() .iter() .any(|method| method == "turn/started"), - "goal continuation should start only after the resume goal snapshot" + "paused goal should not continue after thread resume" ); Ok(()) diff --git a/codex-rs/core/src/goals.rs b/codex-rs/core/src/goals.rs index f1805bb75..4165c0514 100644 --- a/codex-rs/core/src/goals.rs +++ b/codex-rs/core/src/goals.rs @@ -270,10 +270,11 @@ impl Session { /// starts capture the active goal and token baseline, tool completions /// account usage and may inject budget steering, completion accounting /// suppresses that steering, external mutations account best-effort before - /// changing state, interrupts pause active goals, resumes reactivate paused - /// goals, explicit maybe-continue events start idle goal continuation turns, - /// and continuation turns with no counted autonomous activity suppress the - /// next automatic continuation until user/tool/external activity resets it. + /// changing state, interrupts pause active goals, thread resumes restore + /// runtime state for already-active goals, explicit maybe-continue events + /// start idle goal continuation turns, and continuation turns with no counted + /// autonomous activity suppress the next automatic continuation until + /// user/tool/external activity resets it. pub(crate) fn goal_runtime_apply<'a>( self: &'a Arc, event: GoalRuntimeEvent<'a>, @@ -339,7 +340,7 @@ impl Session { Ok(()) }), GoalRuntimeEvent::ThreadResumed => Box::pin(async move { - self.activate_paused_thread_goal_after_resume().await?; + self.restore_thread_goal_runtime_after_resume().await?; Ok(()) }), } @@ -1016,15 +1017,15 @@ impl Session { Ok(()) } - async fn activate_paused_thread_goal_after_resume(&self) -> anyhow::Result { + async fn restore_thread_goal_runtime_after_resume(&self) -> anyhow::Result<()> { if !self.enabled(Feature::Goals) { - return Ok(false); + return Ok(()); } if should_ignore_goal_for_mode(self.collaboration_mode().await.mode) { tracing::debug!( - "skipping paused goal auto-resume while current collaboration mode ignores goals" + "skipping goal runtime restore while current collaboration mode ignores goals" ); - return Ok(false); + return Ok(()); } let _continuation_guard = self @@ -1034,79 +1035,28 @@ impl Session { .await .context("goal continuation semaphore closed")?; let Some(state_db) = self.state_db_for_thread_goals().await? else { - return Ok(false); + return Ok(()); }; let Some(goal) = state_db.get_thread_goal(self.conversation_id).await? else { - *self.goal_runtime.budget_limit_reported_goal_id.lock().await = None; - self.goal_runtime - .accounting - .lock() - .await - .wall_clock - .clear_active_goal(); - return Ok(false); + self.clear_stopped_thread_goal_runtime_state().await; + return Ok(()); }; - if goal.status != codex_state::ThreadGoalStatus::Paused { - let goal_id = goal.goal_id.clone(); - let is_active = goal.status == codex_state::ThreadGoalStatus::Active; - if is_active { + match goal.status { + codex_state::ThreadGoalStatus::Active => { self.goal_runtime .accounting .lock() .await .wall_clock - .mark_active_goal(goal_id); - } else { - self.goal_runtime - .accounting - .lock() - .await - .wall_clock - .clear_active_goal(); + .mark_active_goal(goal.goal_id); + } + codex_state::ThreadGoalStatus::Paused + | codex_state::ThreadGoalStatus::BudgetLimited + | codex_state::ThreadGoalStatus::Complete => { + self.clear_stopped_thread_goal_runtime_state().await; } - return Ok(false); } - - let Some(goal) = state_db - .update_thread_goal( - self.conversation_id, - codex_state::ThreadGoalUpdate { - status: Some(codex_state::ThreadGoalStatus::Active), - token_budget: None, - expected_goal_id: Some(goal.goal_id.clone()), - }, - ) - .await? - else { - *self.goal_runtime.budget_limit_reported_goal_id.lock().await = None; - self.goal_runtime - .accounting - .lock() - .await - .wall_clock - .clear_active_goal(); - return Ok(false); - }; - let goal_id = goal.goal_id.clone(); - let goal = protocol_goal_from_state(goal); - *self.goal_runtime.budget_limit_reported_goal_id.lock().await = None; - let active_turn_id = self - .active_turn_context() - .await - .map(|turn_context| turn_context.sub_id.clone()); - let current_token_usage = self.total_token_usage().await.unwrap_or_default(); - self.mark_active_goal_accounting(goal_id, active_turn_id, current_token_usage) - .await; - self.send_event_raw(Event { - id: uuid::Uuid::new_v4().to_string(), - msg: EventMsg::ThreadGoalUpdated(ThreadGoalUpdatedEvent { - thread_id: self.conversation_id, - turn_id: None, - goal, - }), - }) - .await; - Ok(true) + Ok(()) } async fn maybe_continue_goal_if_idle_runtime(self: &Arc) { diff --git a/codex-rs/core/src/thread_manager_tests.rs b/codex-rs/core/src/thread_manager_tests.rs index 2fe2f97bb..75dcc86ae 100644 --- a/codex-rs/core/src/thread_manager_tests.rs +++ b/codex-rs/core/src/thread_manager_tests.rs @@ -1123,7 +1123,7 @@ async fn interrupted_fork_snapshot_uses_persisted_mid_turn_history_without_live_ } #[tokio::test] -async fn resumed_thread_activates_paused_goal_and_continues_on_request() -> anyhow::Result<()> { +async fn resumed_thread_keeps_paused_goal_paused() -> anyhow::Result<()> { let temp_dir = tempdir().expect("tempdir"); let mut config = test_config().await; config.codex_home = temp_dir.path().join("codex-home").abs(); @@ -1188,7 +1188,7 @@ async fn resumed_thread_activates_paused_goal_and_continues_on_request() -> anyh .get_thread_goal(resumed.thread_id) .await? .expect("goal should still exist after resume"); - assert_eq!(codex_state::ThreadGoalStatus::Active, goal.status); + assert_eq!(codex_state::ThreadGoalStatus::Paused, goal.status); assert!( resumed .thread @@ -1209,7 +1209,7 @@ async fn resumed_thread_activates_paused_goal_and_continues_on_request() -> anyh .active_turn .lock() .await - .is_some() + .is_none() ); resumed.thread.shutdown_and_wait().await?; diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 1f1f2d708..8e17571ac 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -714,6 +714,12 @@ impl App { let enhanced_keys_supported = tui.enhanced_keys_supported(); let wait_for_initial_session_configured = Self::should_wait_for_initial_session(&session_selection); + let should_prompt_for_paused_goal_after_startup_resume = + Self::should_prompt_for_paused_goal_after_startup_resume( + &session_selection, + &initial_prompt, + &initial_images, + ); let (mut chat_widget, initial_started_thread) = match session_selection { SessionSelection::StartFresh | SessionSelection::Exit => { let started = app_server.start_thread(&config).await?; @@ -889,8 +895,13 @@ See the Codex keymap documentation for supported actions and examples." pending_hook_enabled_writes: HashMap::new(), }; if let Some(started) = initial_started_thread { + let thread_id = started.session.thread_id; app.enqueue_primary_thread_session(started.session, started.turns) .await?; + if should_prompt_for_paused_goal_after_startup_resume { + app.maybe_prompt_resume_paused_goal_after_resume(&mut app_server, thread_id) + .await; + } } // On startup, if a managed filesystem sandbox is active, warn about diff --git a/codex-rs/tui/src/app/session_lifecycle.rs b/codex-rs/tui/src/app/session_lifecycle.rs index 4eded21f5..e83abcd0f 100644 --- a/codex-rs/tui/src/app/session_lifecycle.rs +++ b/codex-rs/tui/src/app/session_lifecycle.rs @@ -680,6 +680,7 @@ impl App { .await { Ok(resumed) => { + let resumed_thread_id = resumed.session.thread_id; self.shutdown_current_thread(app_server).await; self.config = resume_config; tui.set_notification_settings( @@ -707,6 +708,11 @@ impl App { } self.chat_widget.add_plain_history_lines(lines); } + self.maybe_prompt_resume_paused_goal_after_resume( + app_server, + resumed_thread_id, + ) + .await; } Err(err) => { self.chat_widget.add_error_message(format!( diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index 84500e3ed..9411cb03d 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -152,6 +152,42 @@ fn startup_waiting_gate_is_only_for_fresh_or_exit_session_selection() { ); } +#[test] +fn startup_paused_goal_prompt_gate_is_only_for_quiet_resume() { + let resume = SessionSelection::Resume(crate::resume_picker::SessionTarget { + path: Some(PathBuf::from("/tmp/restore")), + thread_id: ThreadId::new(), + }); + let fork = SessionSelection::Fork(crate::resume_picker::SessionTarget { + path: Some(PathBuf::from("/tmp/fork")), + thread_id: ThreadId::new(), + }); + let no_images: Vec = Vec::new(); + let initial_images = vec![PathBuf::from("/tmp/image.png")]; + + assert!(App::should_prompt_for_paused_goal_after_startup_resume( + &resume, &None, &no_images + )); + assert!(!App::should_prompt_for_paused_goal_after_startup_resume( + &resume, + &Some("continue from here".to_string()), + &no_images + )); + assert!(!App::should_prompt_for_paused_goal_after_startup_resume( + &resume, + &None, + &initial_images + )); + assert!(!App::should_prompt_for_paused_goal_after_startup_resume( + &SessionSelection::StartFresh, + &None, + &no_images + )); + assert!(!App::should_prompt_for_paused_goal_after_startup_resume( + &fork, &None, &no_images + )); +} + #[test] fn startup_waiting_gate_holds_active_thread_events_until_primary_thread_configured() { let mut wait_for_initial_session = diff --git a/codex-rs/tui/src/app/thread_goal_actions.rs b/codex-rs/tui/src/app/thread_goal_actions.rs index bf589b6a5..d5dfb332f 100644 --- a/codex-rs/tui/src/app/thread_goal_actions.rs +++ b/codex-rs/tui/src/app/thread_goal_actions.rs @@ -42,6 +42,33 @@ impl App { self.chat_widget.show_goal_summary(goal); } + pub(super) async fn maybe_prompt_resume_paused_goal_after_resume( + &mut self, + app_server: &mut AppServerSession, + thread_id: ThreadId, + ) { + let result = app_server.thread_goal_get(thread_id).await; + if self.current_displayed_thread_id() != Some(thread_id) { + return; + } + + let response = match result { + Ok(response) => response, + Err(err) => { + tracing::warn!("failed to read thread goal after resume: {err}"); + return; + } + }; + + let Some(goal) = response.goal else { + return; + }; + if goal.status == ThreadGoalStatus::Paused { + self.chat_widget + .show_resume_paused_goal_prompt(thread_id, goal.objective); + } + } + pub(super) async fn set_thread_goal_objective( &mut self, app_server: &mut AppServerSession, diff --git a/codex-rs/tui/src/app/thread_routing.rs b/codex-rs/tui/src/app/thread_routing.rs index 009121f78..84da7aced 100644 --- a/codex-rs/tui/src/app/thread_routing.rs +++ b/codex-rs/tui/src/app/thread_routing.rs @@ -1281,6 +1281,16 @@ impl App { ) } + pub(super) fn should_prompt_for_paused_goal_after_startup_resume( + session_selection: &SessionSelection, + initial_prompt: &Option, + initial_images: &[PathBuf], + ) -> bool { + matches!(session_selection, SessionSelection::Resume(_)) + && initial_prompt.is_none() + && initial_images.is_empty() + } + pub(super) fn should_handle_active_thread_events( waiting_for_initial_session_configured: bool, has_active_thread_receiver: bool, diff --git a/codex-rs/tui/src/chatwidget/goal_menu.rs b/codex-rs/tui/src/chatwidget/goal_menu.rs index 86562778e..83a26dce0 100644 --- a/codex-rs/tui/src/chatwidget/goal_menu.rs +++ b/codex-rs/tui/src/chatwidget/goal_menu.rs @@ -9,6 +9,41 @@ impl ChatWidget { self.add_plain_history_lines(goal_summary_lines(&goal)); } + pub(crate) fn show_resume_paused_goal_prompt( + &mut self, + thread_id: ThreadId, + objective: String, + ) { + let resume_actions: Vec = vec![Box::new(move |tx| { + tx.send(AppEvent::SetThreadGoalStatus { + thread_id, + status: AppThreadGoalStatus::Active, + }); + })]; + self.show_selection_view(SelectionViewParams { + title: Some("Resume paused goal?".to_string()), + subtitle: Some(format!("Goal: {objective}")), + footer_hint: Some(standard_popup_hint_line()), + initial_selected_idx: Some(0), + items: vec![ + SelectionItem { + name: "Resume goal".to_string(), + description: Some("Mark it active and continue when idle".to_string()), + actions: resume_actions, + dismiss_on_select: true, + ..Default::default() + }, + SelectionItem { + name: "Leave paused".to_string(), + description: Some("Keep it paused; use /goal resume later".to_string()), + dismiss_on_select: true, + ..Default::default() + }, + ], + ..Default::default() + }); + } + pub(crate) fn on_thread_goal_cleared(&mut self, thread_id: &str) { if self .thread_id diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__resume_paused_goal_prompt.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__resume_paused_goal_prompt.snap new file mode 100644 index 000000000..704945c4d --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__resume_paused_goal_prompt.snap @@ -0,0 +1,11 @@ +--- +source: tui/src/chatwidget/tests/goal_menu.rs +expression: "render_bottom_popup(&chat, 100)" +--- + Resume paused goal? + Goal: Keep improving the bare goal command until it feels calm and useful. + +› 1. Resume goal Mark it active and continue when idle + 2. Leave paused Keep it paused; use /goal resume later + + Press enter to confirm or esc to go back diff --git a/codex-rs/tui/src/chatwidget/tests/goal_menu.rs b/codex-rs/tui/src/chatwidget/tests/goal_menu.rs index d90d47cca..85f277ff4 100644 --- a/codex-rs/tui/src/chatwidget/tests/goal_menu.rs +++ b/codex-rs/tui/src/chatwidget/tests/goal_menu.rs @@ -42,6 +42,56 @@ async fn goal_menu_budget_limited_snapshot() { assert_chatwidget_snapshot!("goal_menu_budget_limited", rendered_goal_summary(&mut rx)); } +#[tokio::test] +async fn resume_paused_goal_prompt_snapshot() { + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + let thread_id = ThreadId::new(); + + chat.show_resume_paused_goal_prompt( + thread_id, + "Keep improving the bare goal command until it feels calm and useful.".to_string(), + ); + + assert_chatwidget_snapshot!( + "resume_paused_goal_prompt", + render_bottom_popup(&chat, /*width*/ 100) + ); +} + +#[tokio::test] +async fn resume_paused_goal_prompt_default_resumes_goal() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + let thread_id = ThreadId::new(); + + chat.show_resume_paused_goal_prompt(thread_id, "Finish the paused goal.".to_string()); + chat.handle_key_event(KeyEvent::from(KeyCode::Enter)); + + match rx.try_recv() { + Ok(AppEvent::SetThreadGoalStatus { + thread_id: event_thread_id, + status, + }) => { + assert_eq!(event_thread_id, thread_id); + assert_eq!(status, AppThreadGoalStatus::Active); + } + other => panic!("expected SetThreadGoalStatus event, got {other:?}"), + } + assert!(chat.no_modal_or_popup_active()); +} + +#[tokio::test] +async fn resume_paused_goal_prompt_can_leave_goal_paused() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + let thread_id = ThreadId::new(); + + chat.show_resume_paused_goal_prompt(thread_id, "Finish the paused goal.".to_string()); + chat.handle_key_event(KeyEvent::from(KeyCode::Down)); + chat.handle_key_event(KeyEvent::from(KeyCode::Enter)); + + assert!(matches!(rx.try_recv(), Err(TryRecvError::Empty))); + assert!(chat.no_modal_or_popup_active()); +} + fn test_goal( thread_id: ThreadId, status: AppThreadGoalStatus,