From a95a1ae8513ea769866621001ea73bc25f64fa59 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Mon, 4 May 2026 09:43:36 -0700 Subject: [PATCH] Fix goal plan mode pause handling --- .../request_processors/thread_lifecycle.rs | 6 - .../tests/suite/v2/thread_resume.rs | 314 +++++++++--------- codex-rs/core/src/codex_thread.rs | 7 - codex-rs/core/src/goals.rs | 93 +----- codex-rs/core/src/thread_manager.rs | 6 - codex-rs/core/src/thread_manager_tests.rs | 12 - codex-rs/tui/src/app/event_dispatch.rs | 4 + codex-rs/tui/src/app/tests.rs | 143 ++++++++ codex-rs/tui/src/app/thread_goal_actions.rs | 68 +++- codex-rs/tui/src/app/thread_routing.rs | 35 ++ codex-rs/tui/src/app_event.rs | 5 + codex-rs/tui/src/app_server_session.rs | 25 ++ codex-rs/tui/src/bottom_pane/footer.rs | 23 +- codex-rs/tui/src/chatwidget.rs | 45 ++- codex-rs/tui/src/chatwidget/goal_menu.rs | 38 +-- ...et__tests__goal_menu_paused_plan_mode.snap | 13 + ...tus_line_goal_active_plan_mode_footer.snap | 9 + .../tui/src/chatwidget/tests/goal_menu.rs | 18 + .../tui/src/chatwidget/tests/plan_mode.rs | 65 ++++ .../src/chatwidget/tests/status_and_layout.rs | 46 +++ codex-rs/tui/src/goal_display.rs | 22 ++ 21 files changed, 689 insertions(+), 308 deletions(-) create mode 100644 codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__goal_menu_paused_plan_mode.snap create mode 100644 codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_line_goal_active_plan_mode_footer.snap diff --git a/codex-rs/app-server/src/request_processors/thread_lifecycle.rs b/codex-rs/app-server/src/request_processors/thread_lifecycle.rs index 9b7b9fd576..03e178bffd 100644 --- a/codex-rs/app-server/src/request_processors/thread_lifecycle.rs +++ b/codex-rs/app-server/src/request_processors/thread_lifecycle.rs @@ -592,12 +592,6 @@ pub(super) async fn handle_pending_thread_resume_request( } } - if pending.emit_thread_goal_update - && let Err(err) = conversation.apply_goal_resume_runtime_effects().await - { - tracing::warn!("failed to apply goal resume runtime effects: {err}"); - } - let ThreadConfigSnapshot { model, model_provider_id, 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 55c0d96eb5..3a78402f81 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_resume.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_resume.rs @@ -48,7 +48,10 @@ use codex_app_server_protocol::UserInput; use codex_config::types::AuthCredentialsStoreMode; use codex_login::REFRESH_TOKEN_URL_OVERRIDE_ENV_VAR; use codex_protocol::ThreadId; +use codex_protocol::config_types::CollaborationMode; +use codex_protocol::config_types::ModeKind; use codex_protocol::config_types::Personality; +use codex_protocol::config_types::Settings; use codex_protocol::models::ContentItem; use codex_protocol::models::ResponseItem; use codex_protocol::protocol::AgentMessageEvent; @@ -181,36 +184,14 @@ async fn thread_resume_rejects_unmaterialized_thread() -> Result<()> { async fn thread_goal_get_rejects_unmaterialized_thread() -> Result<()> { let server = create_mock_responses_server_repeating_assistant("Done").await; let codex_home = TempDir::new()?; - create_config_toml(codex_home.path(), &server.uri())?; - let config_path = codex_home.path().join("config.toml"); - let config = std::fs::read_to_string(&config_path)?; - std::fs::write( - &config_path, - config.replace("personality = true\n", "personality = true\ngoals = true\n"), - )?; - - let mut mcp = McpProcess::new_without_managed_config(codex_home.path()).await?; - timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; - - let start_id = mcp - .send_thread_start_request(ThreadStartParams { - model: Some("gpt-5.2-codex".to_string()), - ephemeral: Some(true), - ..Default::default() - }) - .await?; - let start_resp: JSONRPCResponse = timeout( - DEFAULT_READ_TIMEOUT, - mcp.read_stream_until_response_message(RequestId::Integer(start_id)), - ) - .await??; - let ThreadStartResponse { thread, .. } = to_response::(start_resp)?; + let mut mcp = new_goals_enabled_mcp(codex_home.path(), &server.uri()).await?; + let thread_id = start_test_thread(&mut mcp, /*ephemeral*/ true).await?; let goal_id = mcp .send_raw_request( "thread/goal/get", Some(json!({ - "threadId": thread.id, + "threadId": thread_id, })), ) .await?; @@ -385,59 +366,18 @@ async fn thread_resume_can_skip_turns_for_metadata_only_resume() -> Result<()> { } #[tokio::test] -async fn thread_resume_keeps_paused_goal_paused() -> Result<()> { +async fn thread_resume_emits_paused_goal_snapshot_without_continuation() -> Result<()> { let server = create_mock_responses_server_repeating_assistant("Done").await; let codex_home = TempDir::new()?; - create_config_toml(codex_home.path(), &server.uri())?; - let config_path = codex_home.path().join("config.toml"); - let config = std::fs::read_to_string(&config_path)?; - std::fs::write( - &config_path, - config.replace("personality = true\n", "personality = true\ngoals = true\n"), - )?; - - let mut mcp = McpProcess::new_without_managed_config(codex_home.path()).await?; - timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; - - let start_id = mcp - .send_thread_start_request(ThreadStartParams { - model: Some("gpt-5.2-codex".to_string()), - ..Default::default() - }) - .await?; - let start_resp: JSONRPCResponse = timeout( - DEFAULT_READ_TIMEOUT, - mcp.read_stream_until_response_message(RequestId::Integer(start_id)), - ) - .await??; - let ThreadStartResponse { thread, .. } = to_response::(start_resp)?; - - let turn_id = mcp - .send_turn_start_request(TurnStartParams { - thread_id: thread.id.clone(), - input: vec![UserInput::Text { - text: "materialize this thread".to_string(), - text_elements: Vec::new(), - }], - ..Default::default() - }) - .await?; - let _turn_resp: JSONRPCResponse = timeout( - DEFAULT_READ_TIMEOUT, - mcp.read_stream_until_response_message(RequestId::Integer(turn_id)), - ) - .await??; - timeout( - DEFAULT_READ_TIMEOUT, - mcp.read_stream_until_notification_message("turn/completed"), - ) - .await??; + let mut mcp = new_goals_enabled_mcp(codex_home.path(), &server.uri()).await?; + let thread_id = start_test_thread(&mut mcp, /*ephemeral*/ false).await?; + materialize_thread(&mut mcp, &thread_id, /*collaboration_mode*/ None).await?; let goal_id = mcp .send_raw_request( "thread/goal/set", Some(json!({ - "threadId": thread.id, + "threadId": thread_id, "objective": "keep polishing", "status": "paused", })), @@ -458,7 +398,7 @@ async fn thread_resume_keeps_paused_goal_paused() -> Result<()> { let resume_id = mcp .send_thread_resume_request(ThreadResumeParams { - thread_id: thread.id.clone(), + thread_id, ..Default::default() }) .await?; @@ -482,7 +422,7 @@ async fn thread_resume_keeps_paused_goal_paused() -> Result<()> { !mcp.pending_notification_methods() .iter() .any(|method| method == "turn/started"), - "paused goal should not continue after thread resume" + "paused goals should not continue automatically on thread resume" ); Ok(()) @@ -492,56 +432,15 @@ async fn thread_resume_keeps_paused_goal_paused() -> Result<()> { async fn thread_goal_set_preserves_budget_limited_same_objective() -> Result<()> { let server = create_mock_responses_server_repeating_assistant("Done").await; let codex_home = TempDir::new()?; - create_config_toml(codex_home.path(), &server.uri())?; - let config_path = codex_home.path().join("config.toml"); - let config = std::fs::read_to_string(&config_path)?; - std::fs::write( - &config_path, - config.replace("personality = true\n", "personality = true\ngoals = true\n"), - )?; - - let mut mcp = McpProcess::new_without_managed_config(codex_home.path()).await?; - timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; - - let start_id = mcp - .send_thread_start_request(ThreadStartParams { - model: Some("gpt-5.2-codex".to_string()), - ..Default::default() - }) - .await?; - let start_resp: JSONRPCResponse = timeout( - DEFAULT_READ_TIMEOUT, - mcp.read_stream_until_response_message(RequestId::Integer(start_id)), - ) - .await??; - let ThreadStartResponse { thread, .. } = to_response::(start_resp)?; - - let turn_id = mcp - .send_turn_start_request(TurnStartParams { - thread_id: thread.id.clone(), - input: vec![UserInput::Text { - text: "materialize this thread".to_string(), - text_elements: Vec::new(), - }], - ..Default::default() - }) - .await?; - let _turn_resp: JSONRPCResponse = timeout( - DEFAULT_READ_TIMEOUT, - mcp.read_stream_until_response_message(RequestId::Integer(turn_id)), - ) - .await??; - timeout( - DEFAULT_READ_TIMEOUT, - mcp.read_stream_until_notification_message("turn/completed"), - ) - .await??; + let mut mcp = new_goals_enabled_mcp(codex_home.path(), &server.uri()).await?; + let thread_id = start_test_thread(&mut mcp, /*ephemeral*/ false).await?; + materialize_thread(&mut mcp, &thread_id, /*collaboration_mode*/ None).await?; let goal_id = mcp .send_raw_request( "thread/goal/set", Some(json!({ - "threadId": thread.id, + "threadId": thread_id, "objective": "keep polishing", "status": "budgetLimited", "tokenBudget": 10, @@ -566,7 +465,7 @@ async fn thread_goal_set_preserves_budget_limited_same_objective() -> Result<()> .send_raw_request( "thread/goal/set", Some(json!({ - "threadId": thread.id, + "threadId": thread_id, "objective": "keep polishing", })), ) @@ -587,46 +486,75 @@ async fn thread_goal_set_preserves_budget_limited_same_objective() -> Result<()> } #[tokio::test] -async fn thread_goal_clear_deletes_goal_and_notifies() -> Result<()> { +async fn thread_goal_set_active_continues_after_explicit_resume() -> Result<()> { let server = create_mock_responses_server_repeating_assistant("Done").await; let codex_home = TempDir::new()?; - create_config_toml(codex_home.path(), &server.uri())?; - let config_path = codex_home.path().join("config.toml"); - let config = std::fs::read_to_string(&config_path)?; - std::fs::write( - &config_path, - config.replace("personality = true\n", "personality = true\ngoals = true\n"), - )?; + let mut mcp = new_goals_enabled_mcp(codex_home.path(), &server.uri()).await?; + let thread_id = start_test_thread(&mut mcp, /*ephemeral*/ false).await?; + let plan_mode = CollaborationMode { + mode: ModeKind::Plan, + settings: Settings { + model: "gpt-5.2-codex".to_string(), + reasoning_effort: None, + developer_instructions: None, + }, + }; + materialize_thread(&mut mcp, &thread_id, Some(plan_mode)).await?; - let mut mcp = McpProcess::new_without_managed_config(codex_home.path()).await?; - timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; - - let start_id = mcp - .send_thread_start_request(ThreadStartParams { - model: Some("gpt-5.2-codex".to_string()), - ..Default::default() - }) + let goal_id = mcp + .send_raw_request( + "thread/goal/set", + Some(json!({ + "threadId": thread_id, + "objective": "keep polishing", + "status": "paused", + })), + ) .await?; - let start_resp: JSONRPCResponse = timeout( + let goal_resp: JSONRPCResponse = timeout( DEFAULT_READ_TIMEOUT, - mcp.read_stream_until_response_message(RequestId::Integer(start_id)), + mcp.read_stream_until_response_message(RequestId::Integer(goal_id)), ) .await??; - let ThreadStartResponse { thread, .. } = to_response::(start_resp)?; - - let turn_id = mcp - .send_turn_start_request(TurnStartParams { - thread_id: thread.id.clone(), - input: vec![UserInput::Text { - text: "materialize this thread".to_string(), - text_elements: Vec::new(), - }], - ..Default::default() - }) - .await?; - let _turn_resp: JSONRPCResponse = timeout( + let _goal: ThreadGoalSetResponse = to_response(goal_resp)?; + timeout( DEFAULT_READ_TIMEOUT, - mcp.read_stream_until_response_message(RequestId::Integer(turn_id)), + mcp.read_stream_until_notification_message("thread/goal/updated"), + ) + .await??; + mcp.clear_message_buffer(); + + let resume_id = mcp + .send_raw_request( + "thread/goal/set", + Some(json!({ + "threadId": thread_id, + "status": "active", + })), + ) + .await?; + let resume_resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(resume_id)), + ) + .await??; + let resumed_goal: ThreadGoalSetResponse = to_response(resume_resp)?; + assert_eq!(resumed_goal.goal.status, ThreadGoalStatus::Active); + + let notification = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_notification_message("thread/goal/updated"), + ) + .await??; + let notification: ServerNotification = notification.try_into()?; + let ServerNotification::ThreadGoalUpdated(notification) = notification else { + anyhow::bail!("expected thread goal update notification"); + }; + assert_eq!(notification.goal.status, ThreadGoalStatus::Active); + + timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_notification_message("turn/started"), ) .await??; timeout( @@ -635,11 +563,22 @@ async fn thread_goal_clear_deletes_goal_and_notifies() -> Result<()> { ) .await??; + Ok(()) +} + +#[tokio::test] +async fn thread_goal_clear_deletes_goal_and_notifies() -> Result<()> { + let server = create_mock_responses_server_repeating_assistant("Done").await; + let codex_home = TempDir::new()?; + let mut mcp = new_goals_enabled_mcp(codex_home.path(), &server.uri()).await?; + let thread_id = start_test_thread(&mut mcp, /*ephemeral*/ false).await?; + materialize_thread(&mut mcp, &thread_id, /*collaboration_mode*/ None).await?; + let goal_id = mcp .send_raw_request( "thread/goal/set", Some(json!({ - "threadId": thread.id, + "threadId": thread_id, "objective": "keep polishing", })), ) @@ -660,7 +599,7 @@ async fn thread_goal_clear_deletes_goal_and_notifies() -> Result<()> { .send_raw_request( "thread/goal/clear", Some(json!({ - "threadId": thread.id, + "threadId": thread_id, })), ) .await?; @@ -682,7 +621,7 @@ async fn thread_goal_clear_deletes_goal_and_notifies() -> Result<()> { .send_raw_request( "thread/goal/get", Some(json!({ - "threadId": thread.id, + "threadId": thread_id, })), ) .await?; @@ -698,7 +637,7 @@ async fn thread_goal_clear_deletes_goal_and_notifies() -> Result<()> { .send_raw_request( "thread/goal/clear", Some(json!({ - "threadId": thread.id, + "threadId": thread_id, })), ) .await?; @@ -2868,6 +2807,71 @@ async fn thread_resume_accepts_personality_override() -> Result<()> { } // Helper to create a config.toml pointing at the mock model server. +async fn new_goals_enabled_mcp(codex_home: &Path, server_uri: &str) -> Result { + create_config_toml(codex_home, server_uri)?; + enable_goals_in_config(codex_home)?; + + let mut mcp = McpProcess::new_without_managed_config(codex_home).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; + Ok(mcp) +} + +fn enable_goals_in_config(codex_home: &Path) -> Result<()> { + let config_path = codex_home.join("config.toml"); + let config = std::fs::read_to_string(&config_path)?; + std::fs::write( + &config_path, + config.replace("personality = true\n", "personality = true\ngoals = true\n"), + )?; + Ok(()) +} + +async fn start_test_thread(mcp: &mut McpProcess, ephemeral: bool) -> Result { + let start_id = mcp + .send_thread_start_request(ThreadStartParams { + model: Some("gpt-5.2-codex".to_string()), + ephemeral: ephemeral.then_some(true), + ..Default::default() + }) + .await?; + let start_resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(start_id)), + ) + .await??; + let ThreadStartResponse { thread, .. } = to_response::(start_resp)?; + Ok(thread.id) +} + +async fn materialize_thread( + mcp: &mut McpProcess, + thread_id: &str, + collaboration_mode: Option, +) -> Result<()> { + let turn_id = mcp + .send_turn_start_request(TurnStartParams { + thread_id: thread_id.to_string(), + input: vec![UserInput::Text { + text: "materialize this thread".to_string(), + text_elements: Vec::new(), + }], + collaboration_mode, + ..Default::default() + }) + .await?; + let _turn_resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(turn_id)), + ) + .await??; + timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_notification_message("turn/completed"), + ) + .await??; + Ok(()) +} + fn create_config_toml(codex_home: &std::path::Path, server_uri: &str) -> std::io::Result<()> { let config_toml = codex_home.join("config.toml"); std::fs::write( diff --git a/codex-rs/core/src/codex_thread.rs b/codex-rs/core/src/codex_thread.rs index cc83c0a7c1..28c75cae6c 100644 --- a/codex-rs/core/src/codex_thread.rs +++ b/codex-rs/core/src/codex_thread.rs @@ -133,13 +133,6 @@ impl CodexThread { self.codex.session_loop_termination.clone().await; } - pub async fn apply_goal_resume_runtime_effects(&self) -> anyhow::Result<()> { - self.codex - .session - .goal_runtime_apply(GoalRuntimeEvent::ThreadResumed) - .await - } - pub async fn continue_active_goal_if_idle(&self) -> anyhow::Result<()> { self.codex .session diff --git a/codex-rs/core/src/goals.rs b/codex-rs/core/src/goals.rs index 4165c05146..1994a34890 100644 --- a/codex-rs/core/src/goals.rs +++ b/codex-rs/core/src/goals.rs @@ -12,7 +12,6 @@ use crate::state::TurnState; use crate::tasks::RegularTask; use anyhow::Context; use codex_features::Feature; -use codex_protocol::config_types::ModeKind; use codex_protocol::models::ContentItem; use codex_protocol::models::ResponseInputItem; use codex_protocol::protocol::Event; @@ -99,7 +98,6 @@ pub(crate) enum GoalRuntimeEvent<'a> { status: codex_state::ThreadGoalStatus, }, ExternalClear, - ThreadResumed, } pub(crate) struct GoalRuntimeState { @@ -266,15 +264,13 @@ impl Session { /// Applies runtime policy for a goal lifecycle event. /// /// Goal data methods validate and persist state; this dispatcher owns the - /// cross-cutting runtime behavior: plan mode ignores continuations, turn - /// 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, 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. + /// cross-cutting runtime behavior: turn 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, 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,10 +335,6 @@ impl Session { self.clear_stopped_thread_goal_runtime_state().await; Ok(()) }), - GoalRuntimeEvent::ThreadResumed => Box::pin(async move { - self.restore_thread_goal_runtime_after_resume().await?; - Ok(()) - }), } } @@ -659,10 +651,6 @@ impl Session { if !self.enabled(Feature::Goals) { return; } - if should_ignore_goal_for_mode(turn_context.collaboration_mode.mode) { - self.clear_active_goal_accounting(turn_context).await; - return; - } let state_db = match self.state_db_for_thread_goals().await { Ok(Some(state_db)) => state_db, Ok(None) => return, @@ -791,9 +779,6 @@ impl Session { if !self.enabled(Feature::Goals) { return Ok(()); } - if should_ignore_goal_for_mode(turn_context.collaboration_mode.mode) { - return Ok(()); - } let Some(state_db) = self.state_db_for_thread_goals().await? else { return Ok(()); }; @@ -969,10 +954,6 @@ impl Session { } async fn pause_active_thread_goal_for_interrupt(&self) -> anyhow::Result<()> { - if should_ignore_goal_for_mode(self.collaboration_mode().await.mode) { - return Ok(()); - } - if !self.enabled(Feature::Goals) { return Ok(()); } @@ -1017,48 +998,6 @@ impl Session { Ok(()) } - async fn restore_thread_goal_runtime_after_resume(&self) -> anyhow::Result<()> { - if !self.enabled(Feature::Goals) { - return Ok(()); - } - if should_ignore_goal_for_mode(self.collaboration_mode().await.mode) { - tracing::debug!( - "skipping goal runtime restore while current collaboration mode ignores goals" - ); - return Ok(()); - } - - let _continuation_guard = self - .goal_runtime - .continuation_lock - .acquire() - .await - .context("goal continuation semaphore closed")?; - let Some(state_db) = self.state_db_for_thread_goals().await? else { - return Ok(()); - }; - let Some(goal) = state_db.get_thread_goal(self.conversation_id).await? else { - self.clear_stopped_thread_goal_runtime_state().await; - return Ok(()); - }; - match goal.status { - codex_state::ThreadGoalStatus::Active => { - self.goal_runtime - .accounting - .lock() - .await - .wall_clock - .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; - } - } - Ok(()) - } - async fn maybe_continue_goal_if_idle_runtime(self: &Arc) { self.maybe_start_turn_for_pending_work().await; self.maybe_start_goal_continuation_turn().await; @@ -1149,10 +1088,6 @@ impl Session { if !self.enabled(Feature::Goals) { return None; } - if should_ignore_goal_for_mode(self.collaboration_mode().await.mode) { - tracing::debug!("skipping active goal continuation while plan mode is active"); - return None; - } if self.active_turn.lock().await.is_some() { tracing::debug!("skipping active goal continuation because a turn is already active"); return None; @@ -1289,10 +1224,6 @@ impl Session { } } -fn should_ignore_goal_for_mode(mode: ModeKind) -> bool { - mode == ModeKind::Plan -} - // Builds the hidden developer prompt used to continue an active goal after the // previous turn completes. Runtime-owned state such as budget exhaustion is // reported as context, but the model is only asked to mark goals active, @@ -1415,23 +1346,13 @@ mod tests { use super::continuation_prompt; use super::escape_xml_text; use super::goal_token_delta_for_usage; - use super::should_ignore_goal_for_mode; use codex_protocol::ThreadId; - use codex_protocol::config_types::ModeKind; use codex_protocol::protocol::ThreadGoal; use codex_protocol::protocol::ThreadGoalStatus; use codex_protocol::protocol::TokenUsage; use std::time::Duration; use std::time::Instant; - #[test] - fn goal_continuation_is_ignored_only_in_plan_mode() { - assert!(should_ignore_goal_for_mode(ModeKind::Plan)); - assert!(!should_ignore_goal_for_mode(ModeKind::Default)); - assert!(!should_ignore_goal_for_mode(ModeKind::PairProgramming)); - assert!(!should_ignore_goal_for_mode(ModeKind::Execute)); - } - #[test] fn goal_token_delta_excludes_cached_input_and_does_not_double_count_reasoning() { let usage = TokenUsage { diff --git a/codex-rs/core/src/thread_manager.rs b/codex-rs/core/src/thread_manager.rs index eb7419076d..5827d7841c 100644 --- a/codex-rs/core/src/thread_manager.rs +++ b/codex-rs/core/src/thread_manager.rs @@ -1076,7 +1076,6 @@ impl ThreadManagerState { environments: Vec, user_shell_override: Option, ) -> CodexResult { - let is_resumed_thread = matches!(&initial_history, InitialHistory::Resumed(_)); if let InitialHistory::Resumed(resumed) = &initial_history { let mut threads = self.threads.write().await; if let Some(thread) = threads.get(&resumed.conversation_id).cloned() { @@ -1147,11 +1146,6 @@ impl ThreadManagerState { let new_thread = self .finalize_thread_spawn(codex, thread_id, tracked_session_source, watch_registration) .await?; - if is_resumed_thread - && let Err(err) = new_thread.thread.apply_goal_resume_runtime_effects().await - { - warn!("failed to apply goal resume runtime effects: {err}"); - } Ok(new_thread) } diff --git a/codex-rs/core/src/thread_manager_tests.rs b/codex-rs/core/src/thread_manager_tests.rs index 75dcc86ae3..fbdfb6fcdd 100644 --- a/codex-rs/core/src/thread_manager_tests.rs +++ b/codex-rs/core/src/thread_manager_tests.rs @@ -1200,18 +1200,6 @@ async fn resumed_thread_keeps_paused_goal_paused() -> anyhow::Result<()> { .is_none() ); - resumed.thread.continue_active_goal_if_idle().await?; - assert!( - resumed - .thread - .codex - .session - .active_turn - .lock() - .await - .is_none() - ); - resumed.thread.shutdown_and_wait().await?; Ok(()) } diff --git a/codex-rs/tui/src/app/event_dispatch.rs b/codex-rs/tui/src/app/event_dispatch.rs index 5af8e7d2a7..bd5713417f 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -665,6 +665,10 @@ impl App { self.set_thread_goal_status(app_server, thread_id, status) .await; } + AppEvent::PauseActiveGoalIfNeeded { thread_id } => { + self.pause_active_goal_if_needed(app_server, thread_id) + .await; + } AppEvent::ClearThreadGoal { thread_id } => { self.clear_thread_goal(app_server, thread_id).await; } diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index 799ac69e37..6311e6f401 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -66,6 +66,7 @@ use codex_app_server_protocol::TurnStatus; use codex_app_server_protocol::UserInput; use codex_app_server_protocol::UserInput as AppServerUserInput; use codex_app_server_protocol::WarningNotification; +use codex_features::Feature; use codex_otel::SessionTelemetry; use codex_protocol::ThreadId; use codex_protocol::config_types::CollaborationMode; @@ -1144,6 +1145,7 @@ async fn replay_thread_snapshot_restores_collaboration_mode_without_input() { .chat_widget .capture_thread_input_state() .expect("expected collaboration-only input state"); + assert!(input_state.is_plan_mode_active()); let (chat_widget, _app_event_tx, _rx, _new_op_rx) = make_chatwidget_manual_with_sender().await; app.chat_widget = chat_widget; @@ -1180,6 +1182,147 @@ async fn replay_thread_snapshot_restores_collaboration_mode_without_input() { ); } +#[test] +fn refresh_snapshot_session_pauses_active_goal_before_plan_mode_resume() -> Result<()> { + const WORKER_THREADS: usize = 1; + const TEST_STACK_SIZE_BYTES: usize = 8 * 1024 * 1024; + + let runtime = tokio::runtime::Builder::new_multi_thread() + .worker_threads(WORKER_THREADS) + .thread_stack_size(TEST_STACK_SIZE_BYTES) + .enable_all() + .build()?; + + runtime.block_on(async { + let (mut app, _app_event_rx, _op_rx) = make_test_app_with_channels().await; + app.chat_widget + .set_feature_enabled(Feature::Goals, /*enabled*/ true); + app.chat_widget + .set_feature_enabled(Feature::CollaborationModes, /*enabled*/ true); + app.config = app.chat_widget.config_ref().clone(); + + let mut app_server = + Box::pin(crate::start_embedded_app_server_for_picker(&app.config)).await?; + let displayed_started = app_server.start_thread(&app.config).await?; + let displayed_thread_id = displayed_started.session.thread_id; + app.chat_widget + .handle_thread_session(displayed_started.session.clone()); + let plan_mask = + crate::collaboration_modes::plan_mask(app.chat_widget.model_catalog().as_ref()) + .expect("expected plan collaboration mask"); + app.chat_widget.set_collaboration_mask(plan_mask); + assert!(app.chat_widget.is_plan_mode_active()); + let plan_input_state = app + .chat_widget + .capture_thread_input_state() + .expect("plan input state should be captured"); + let default_mask = + crate::collaboration_modes::default_mask(app.chat_widget.model_catalog().as_ref()) + .expect("expected default collaboration mask"); + app.chat_widget.set_collaboration_mask(default_mask); + assert!(!app.chat_widget.is_plan_mode_active()); + assert_eq!(app.current_displayed_thread_id(), Some(displayed_thread_id)); + + let target_started = app_server.start_thread(&app.config).await?; + let thread_id = target_started.session.thread_id; + + let state_db = codex_state::StateRuntime::init( + app.config.sqlite_home.clone(), + app.config.model_provider_id.clone(), + ) + .await + .expect("state db should initialize"); + let mut metadata = codex_state::ThreadMetadataBuilder::new( + thread_id, + target_started + .session + .rollout_path + .clone() + .expect("target thread should have a rollout path"), + chrono::Utc::now(), + codex_protocol::protocol::SessionSource::Cli, + ); + metadata.cwd = app.config.cwd.to_path_buf(); + metadata.model_provider = Some(app.config.model_provider_id.clone()); + state_db + .upsert_thread(&metadata.build(app.config.model_provider_id.as_str())) + .await + .expect("thread metadata should seed"); + state_db + .replace_thread_goal( + thread_id, + "Keep planning safely", + codex_state::ThreadGoalStatus::Active, + /*token_budget*/ None, + ) + .await + .expect("active goal should seed"); + + let mut snapshot = ThreadEventSnapshot { + session: None, + turns: Vec::new(), + events: Vec::new(), + input_state: Some(plan_input_state), + }; + + app.refresh_snapshot_session_if_needed( + &mut app_server, + thread_id, + /*is_replay_only*/ false, + &mut snapshot, + ) + .await; + + assert!(snapshot.session.is_some()); + let goal = state_db + .get_thread_goal(thread_id) + .await + .expect("goal should be readable") + .expect("goal should still exist"); + assert_eq!(goal.status, codex_state::ThreadGoalStatus::Paused); + Ok(()) + }) +} + +#[tokio::test] +async fn snapshot_resume_plan_mode_uses_target_state_or_same_thread_fallback() { + let (mut app, _app_event_rx, _op_rx) = make_test_app_with_channels().await; + let target_thread_id = ThreadId::new(); + let other_thread_id = ThreadId::new(); + let plan_mask = crate::collaboration_modes::plan_mask(app.chat_widget.model_catalog().as_ref()) + .expect("expected plan collaboration mask"); + app.chat_widget.set_collaboration_mask(plan_mask); + let plan_input_state = app + .chat_widget + .capture_thread_input_state() + .expect("plan input state should be captured"); + let snapshot = |input_state| ThreadEventSnapshot { + session: None, + turns: Vec::new(), + events: Vec::new(), + input_state, + }; + + assert!(thread_routing::plan_mode_active_for_snapshot_resume( + &snapshot(Some(plan_input_state)), + target_thread_id, + /*current_displayed_thread_id*/ None, + /*current_widget_plan_mode_active*/ false, + )); + assert!(thread_routing::plan_mode_active_for_snapshot_resume( + &snapshot(None), + target_thread_id, + Some(target_thread_id), + /*current_widget_plan_mode_active*/ true, + )); + assert!(!thread_routing::plan_mode_active_for_snapshot_resume( + &snapshot(None), + target_thread_id, + Some(other_thread_id), + /*current_widget_plan_mode_active*/ true, + )); +} + #[tokio::test] async fn replayed_interrupted_turn_restores_queued_input_to_composer() { let (mut app, _app_event_rx, _op_rx) = make_test_app_with_channels().await; diff --git a/codex-rs/tui/src/app/thread_goal_actions.rs b/codex-rs/tui/src/app/thread_goal_actions.rs index d5dfb332fe..b920dfd303 100644 --- a/codex-rs/tui/src/app/thread_goal_actions.rs +++ b/codex-rs/tui/src/app/thread_goal_actions.rs @@ -6,8 +6,11 @@ use crate::bottom_pane::SelectionAction; use crate::bottom_pane::SelectionItem; use crate::bottom_pane::SelectionViewParams; use crate::bottom_pane::popup_consts::standard_popup_hint_line; +use crate::goal_display::GOAL_CONTINUATION_PAUSED_IN_PLAN_MODE_HINT; use crate::goal_display::goal_status_label; use crate::goal_display::goal_usage_summary; +use crate::goal_display::show_goal_plan_mode_hint; +use codex_app_server_protocol::ThreadGoal; use codex_app_server_protocol::ThreadGoalStatus; use codex_protocol::ThreadId; @@ -96,11 +99,13 @@ impl App { } } + let plan_mode_active = self.chat_widget.is_plan_mode_active(); + let status = goal_status_for_plan_mode(ThreadGoalStatus::Active, plan_mode_active); let result = app_server .thread_goal_set( thread_id, Some(objective), - Some(ThreadGoalStatus::Active), + Some(status), /*token_budget*/ None, ) .await; @@ -109,10 +114,7 @@ impl App { } match result { - Ok(response) => self.chat_widget.add_info_message( - format!("Goal {}", goal_status_label(response.goal.status)), - Some(goal_usage_summary(&response.goal)), - ), + Ok(response) => self.show_thread_goal_updated(&response.goal), Err(err) => self .chat_widget .add_error_message(format!("Failed to set thread goal: {err}")), @@ -125,6 +127,8 @@ impl App { thread_id: ThreadId, status: ThreadGoalStatus, ) { + let plan_mode_active = self.chat_widget.is_plan_mode_active(); + let status = goal_status_for_plan_mode(status, plan_mode_active); let result = app_server .thread_goal_set( thread_id, @@ -138,16 +142,36 @@ impl App { } match result { - Ok(response) => self.chat_widget.add_info_message( - format!("Goal {}", goal_status_label(response.goal.status)), - Some(goal_usage_summary(&response.goal)), - ), + Ok(response) => self.show_thread_goal_updated(&response.goal), Err(err) => self .chat_widget .add_error_message(format!("Failed to update thread goal: {err}")), } } + pub(super) async fn pause_active_goal_if_needed( + &mut self, + app_server: &mut AppServerSession, + thread_id: ThreadId, + ) { + if self.current_displayed_thread_id() != Some(thread_id) + || !self.chat_widget.is_plan_mode_active() + { + return; + } + + let result = app_server + .pause_active_goal_if_needed(&self.config, thread_id) + .await; + if self.current_displayed_thread_id() != Some(thread_id) { + return; + } + if let Err(err) = result { + self.chat_widget + .add_error_message(format!("Failed to pause thread goal: {err}")); + } + } + pub(super) async fn clear_thread_goal( &mut self, app_server: &mut AppServerSession, @@ -208,4 +232,30 @@ impl App { ..Default::default() }); } + + fn show_thread_goal_updated(&mut self, goal: &ThreadGoal) { + let show_plan_mode_hint = + show_goal_plan_mode_hint(goal.status, self.chat_widget.is_plan_mode_active()); + self.chat_widget.add_info_message( + format!("Goal {}", goal_status_label(goal.status)), + Some(goal_update_hint(goal, show_plan_mode_hint)), + ); + } +} + +fn goal_update_hint(goal: &ThreadGoal, show_plan_mode_hint: bool) -> String { + let mut hint = goal_usage_summary(goal); + if show_plan_mode_hint { + hint.push(' '); + hint.push_str(GOAL_CONTINUATION_PAUSED_IN_PLAN_MODE_HINT); + } + hint +} + +fn goal_status_for_plan_mode(status: ThreadGoalStatus, plan_mode_active: bool) -> ThreadGoalStatus { + if status == ThreadGoalStatus::Active && plan_mode_active { + ThreadGoalStatus::Paused + } else { + status + } } diff --git a/codex-rs/tui/src/app/thread_routing.rs b/codex-rs/tui/src/app/thread_routing.rs index acd79989b6..17b28da46c 100644 --- a/codex-rs/tui/src/app/thread_routing.rs +++ b/codex-rs/tui/src/app/thread_routing.rs @@ -1122,6 +1122,24 @@ impl App { return; } + let plan_mode_active = plan_mode_active_for_snapshot_resume( + snapshot, + thread_id, + self.current_displayed_thread_id(), + self.chat_widget.is_plan_mode_active(), + ); + if plan_mode_active + && let Err(err) = app_server + .pause_active_goal_if_needed(&self.config, thread_id) + .await + { + tracing::warn!( + thread_id = %thread_id, + error = %err, + "failed to pause active goal before Plan-mode thread resume; continuing resume" + ); + } + match app_server .resume_thread(self.config.clone(), thread_id) .await @@ -1479,3 +1497,20 @@ impl App { Ok(()) } } + +pub(super) fn plan_mode_active_for_snapshot_resume( + snapshot: &ThreadEventSnapshot, + thread_id: ThreadId, + current_displayed_thread_id: Option, + current_widget_plan_mode_active: bool, +) -> bool { + snapshot.input_state.as_ref().map_or_else( + || { + // A missing input_state can only inherit live widget mode when the widget already + // displays this thread. During a thread switch it still belongs to the previous + // thread, and using it would pause the target thread incorrectly. + current_displayed_thread_id == Some(thread_id) && current_widget_plan_mode_active + }, + ThreadInputState::is_plan_mode_active, + ) +} diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index 1032823f24..f857f51eba 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -235,6 +235,11 @@ pub(crate) enum AppEvent { status: ThreadGoalStatus, }, + /// Pause the current thread goal if it is active. + PauseActiveGoalIfNeeded { + thread_id: ThreadId, + }, + /// Clear the current thread goal. ClearThreadGoal { thread_id: ThreadId, diff --git a/codex-rs/tui/src/app_server_session.rs b/codex-rs/tui/src/app_server_session.rs index 449da8e212..c71089d286 100644 --- a/codex-rs/tui/src/app_server_session.rs +++ b/codex-rs/tui/src/app_server_session.rs @@ -102,6 +102,7 @@ use codex_app_server_protocol::TurnStartResponse; use codex_app_server_protocol::TurnSteerParams; use codex_app_server_protocol::TurnSteerResponse; use codex_app_server_protocol::UserInput; +use codex_features::Feature; use codex_otel::TelemetryAuthMode; use codex_protocol::ThreadId; use codex_protocol::approvals::GuardianAssessmentEvent; @@ -681,6 +682,30 @@ impl AppServerSession { .wrap_err("thread/goal/get failed in TUI") } + pub(crate) async fn pause_active_goal_if_needed( + &mut self, + config: &Config, + thread_id: ThreadId, + ) -> Result<()> { + if !config.features.enabled(Feature::Goals) { + return Ok(()); + } + let response = self.thread_goal_get(thread_id).await?; + if response + .goal + .is_some_and(|goal| goal.status == ThreadGoalStatus::Active) + { + self.thread_goal_set( + thread_id, + /*objective*/ None, + Some(ThreadGoalStatus::Paused), + /*token_budget*/ None, + ) + .await?; + } + Ok(()) + } + pub(crate) async fn thread_goal_set( &mut self, thread_id: ThreadId, diff --git a/codex-rs/tui/src/bottom_pane/footer.rs b/codex-rs/tui/src/bottom_pane/footer.rs index 0b6aabf5a9..a87a639c47 100644 --- a/codex-rs/tui/src/bottom_pane/footer.rs +++ b/codex-rs/tui/src/bottom_pane/footer.rs @@ -566,14 +566,33 @@ pub(crate) fn goal_status_indicator_line( Some(Line::from(vec![Span::from(label).magenta()])) } +fn goal_paused_in_plan_mode_line(usage: Option<&str>) -> Line<'static> { + let label = if let Some(usage) = usage { + format!("Goal paused in Plan mode ({usage})") + } else { + "Goal paused in Plan mode".to_string() + }; + + Line::from(vec![Span::from(label).magenta()]) +} + pub(crate) fn status_line_right_indicator_line( collaboration_mode_indicator: Option, goal_status_indicator: Option<&GoalStatusIndicator>, ide_context_active: bool, show_cycle_hint: bool, ) -> Option> { - let primary_indicator = mode_indicator_line(collaboration_mode_indicator, show_cycle_hint) - .or_else(|| goal_status_indicator_line(goal_status_indicator)); + let primary_indicator = match (collaboration_mode_indicator, goal_status_indicator) { + ( + Some(CollaborationModeIndicator::Plan), + active @ Some(GoalStatusIndicator::Active { .. }), + ) => goal_status_indicator_line(active), + (Some(CollaborationModeIndicator::Plan), Some(GoalStatusIndicator::Paused)) => { + Some(goal_paused_in_plan_mode_line(/*usage*/ None)) + } + _ => mode_indicator_line(collaboration_mode_indicator, show_cycle_hint) + .or_else(|| goal_status_indicator_line(goal_status_indicator)), + }; let ide_context_indicator = ide_context_active.then(|| Line::from(vec!["IDE context".cyan()])); let mut line: Option> = None; diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index e315f0d60f..35dd4446b6 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -1153,6 +1153,15 @@ pub(crate) struct ThreadInputState { agent_turn_running: bool, } +impl ThreadInputState { + pub(crate) fn is_plan_mode_active(&self) -> bool { + self.active_collaboration_mask + .as_ref() + .and_then(|mask| mask.mode) + == Some(ModeKind::Plan) + } +} + impl From for UserMessage { fn from(text: String) -> Self { Self { @@ -9438,6 +9447,10 @@ impl ChatWidget { self.active_mode_kind() } + pub(crate) fn is_plan_mode_active(&self) -> bool { + self.active_mode_kind() == ModeKind::Plan + } + fn is_session_configured(&self) -> bool { self.thread_id.is_some() } @@ -9581,11 +9594,7 @@ impl ChatWidget { fn update_collaboration_mode_indicator(&mut self) { let indicator = self.collaboration_mode_indicator(); - let goal_indicator = if indicator.is_none() { - self.goal_status_indicator(Instant::now()) - } else { - None - }; + let goal_indicator = self.goal_status_indicator(Instant::now()); self.current_goal_status_indicator = goal_indicator.clone(); self.bottom_pane.set_collaboration_mode_indicator(indicator); self.bottom_pane.set_goal_status_indicator(goal_indicator); @@ -9628,8 +9637,29 @@ impl ChatWidget { { self.budget_limited_turn_ids.insert(turn_id); } + let pause_active_goal_for_plan_mode = + goal.status == AppThreadGoalStatus::Active && self.is_plan_mode_active(); self.current_goal_status = Some(GoalStatusState::new(goal, Instant::now())); self.update_collaboration_mode_indicator(); + if pause_active_goal_for_plan_mode { + self.pause_active_goal_for_plan_mode_if_needed(); + } + } + + fn pause_active_goal_for_plan_mode_if_needed(&mut self) { + if !self.is_plan_mode_active() { + return; + } + if !self.config.features.enabled(Feature::Goals) { + return; + } + let Some(thread_id) = self.thread_id else { + return; + }; + // Goal continuation remains mode-agnostic below the TUI; Plan mode is + // client state, so the TUI owns pausing active goals while it is active. + self.app_event_tx + .send(AppEvent::PauseActiveGoalIfNeeded { thread_id }); } fn personality_label(personality: Personality) -> &'static str { @@ -9687,8 +9717,11 @@ impl ChatWidget { self.refresh_plan_mode_nudge(); self.refresh_model_dependent_surfaces(); let next_mode = self.active_mode_kind(); - let next_model = self.current_model(); + let next_model = self.current_model().to_string(); let next_effort = self.effective_reasoning_effort(); + if previous_mode != ModeKind::Plan && next_mode == ModeKind::Plan { + self.pause_active_goal_for_plan_mode_if_needed(); + } if previous_mode != next_mode && (previous_model != next_model || previous_effort != next_effort) { diff --git a/codex-rs/tui/src/chatwidget/goal_menu.rs b/codex-rs/tui/src/chatwidget/goal_menu.rs index 83a26dce0d..56e57ac1a6 100644 --- a/codex-rs/tui/src/chatwidget/goal_menu.rs +++ b/codex-rs/tui/src/chatwidget/goal_menu.rs @@ -1,12 +1,16 @@ //! Goal summary for the bare `/goal` command. use super::*; +use crate::goal_display::GOAL_CONTINUATION_PAUSED_IN_PLAN_MODE_HINT; use crate::goal_display::format_goal_elapsed_seconds; +use crate::goal_display::goal_status_label; +use crate::goal_display::show_goal_plan_mode_hint; use crate::status::format_tokens_compact; impl ChatWidget { pub(crate) fn show_goal_summary(&mut self, goal: AppThreadGoal) { - self.add_plain_history_lines(goal_summary_lines(&goal)); + let show_plan_mode_hint = show_goal_plan_mode_hint(goal.status, self.is_plan_mode_active()); + self.add_plain_history_lines(goal_summary_lines(&goal, show_plan_mode_hint)); } pub(crate) fn show_resume_paused_goal_prompt( @@ -55,7 +59,7 @@ impl ChatWidget { } } -fn goal_summary_lines(goal: &AppThreadGoal) -> Vec> { +fn goal_summary_lines(goal: &AppThreadGoal, show_plan_mode_hint: bool) -> Vec> { let mut lines = vec![ Line::from("Goal".bold()), Line::from(vec![ @@ -63,15 +67,20 @@ fn goal_summary_lines(goal: &AppThreadGoal) -> Vec> { goal_status_label(goal.status).to_string().into(), ]), Line::from(vec!["Objective: ".dim(), goal.objective.clone().into()]), - Line::from(vec![ - "Time used: ".dim(), - format_goal_elapsed_seconds(goal.time_used_seconds).into(), - ]), - Line::from(vec![ - "Tokens used: ".dim(), - format_tokens_compact(goal.tokens_used).into(), - ]), ]; + if show_plan_mode_hint { + lines.push(Line::from( + GOAL_CONTINUATION_PAUSED_IN_PLAN_MODE_HINT.magenta(), + )); + } + lines.push(Line::from(vec![ + "Time used: ".dim(), + format_goal_elapsed_seconds(goal.time_used_seconds).into(), + ])); + lines.push(Line::from(vec![ + "Tokens used: ".dim(), + format_tokens_compact(goal.tokens_used).into(), + ])); if let Some(token_budget) = goal.token_budget { lines.push(Line::from(vec![ "Token budget: ".dim(), @@ -89,12 +98,3 @@ fn goal_summary_lines(goal: &AppThreadGoal) -> Vec> { lines.push(Line::from(command_hint.dim())); lines } - -fn goal_status_label(status: AppThreadGoalStatus) -> &'static str { - match status { - AppThreadGoalStatus::Active => "active", - AppThreadGoalStatus::Paused => "paused", - AppThreadGoalStatus::BudgetLimited => "limited by budget", - AppThreadGoalStatus::Complete => "complete", - } -} diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__goal_menu_paused_plan_mode.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__goal_menu_paused_plan_mode.snap new file mode 100644 index 0000000000..cd509ec889 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__goal_menu_paused_plan_mode.snap @@ -0,0 +1,13 @@ +--- +source: tui/src/chatwidget/tests/goal_menu.rs +expression: rendered_goal_summary(&mut rx) +--- +Goal +Status: paused +Objective: Keep improving the bare goal command until it feels calm and useful. +Goal continuation is paused in Plan mode. Switch out of Plan mode, then use /goal resume to continue. +Time used: 1m +Tokens used: 12.5K +Token budget: 80K + +Commands: /goal resume, /goal clear diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_line_goal_active_plan_mode_footer.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_line_goal_active_plan_mode_footer.snap new file mode 100644 index 0000000000..c2daa06cb4 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_line_goal_active_plan_mode_footer.snap @@ -0,0 +1,9 @@ +--- +source: tui/src/chatwidget/tests/status_and_layout.rs +expression: normalized_backend_snapshot(terminal.backend()) +--- +" " +" " +"› Ask Codex to do anything " +" " +" gpt-5 Pursuing goal (40K / 50K) " diff --git a/codex-rs/tui/src/chatwidget/tests/goal_menu.rs b/codex-rs/tui/src/chatwidget/tests/goal_menu.rs index 85f277ff4b..32927a879f 100644 --- a/codex-rs/tui/src/chatwidget/tests/goal_menu.rs +++ b/codex-rs/tui/src/chatwidget/tests/goal_menu.rs @@ -28,6 +28,24 @@ async fn goal_menu_paused_snapshot() { assert_chatwidget_snapshot!("goal_menu_paused", rendered_goal_summary(&mut rx)); } +#[tokio::test] +async fn goal_menu_paused_plan_mode_snapshot() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(Some("gpt-5")).await; + let thread_id = ThreadId::new(); + let plan_mask = collaboration_modes::plan_mask(chat.model_catalog.as_ref()) + .expect("expected plan collaboration mode"); + chat.set_collaboration_mask(plan_mask); + drain_insert_history(&mut rx); + + chat.show_goal_summary(test_goal( + thread_id, + AppThreadGoalStatus::Paused, + /*token_budget*/ Some(80_000), + )); + + assert_chatwidget_snapshot!("goal_menu_paused_plan_mode", rendered_goal_summary(&mut rx)); +} + #[tokio::test] async fn goal_menu_budget_limited_snapshot() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; diff --git a/codex-rs/tui/src/chatwidget/tests/plan_mode.rs b/codex-rs/tui/src/chatwidget/tests/plan_mode.rs index b6afdf0f75..175c5913d5 100644 --- a/codex-rs/tui/src/chatwidget/tests/plan_mode.rs +++ b/codex-rs/tui/src/chatwidget/tests/plan_mode.rs @@ -1304,6 +1304,71 @@ async fn collab_mode_shift_tab_cycles_only_when_idle() { assert_eq!(chat.active_collaboration_mode_kind(), before); } +#[tokio::test] +async fn entering_plan_mode_requests_goal_pause() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(Some("gpt-5")).await; + let thread_id = ThreadId::new(); + chat.thread_id = Some(thread_id); + chat.set_feature_enabled(Feature::Goals, /*enabled*/ true); + + let plan_mask = collaboration_modes::plan_mask(chat.model_catalog.as_ref()) + .expect("expected plan collaboration mode"); + chat.set_collaboration_mask(plan_mask); + + expect_goal_pause_event(&mut rx, thread_id); +} + +#[tokio::test] +async fn active_goal_update_in_plan_mode_requests_goal_pause() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(Some("gpt-5")).await; + let thread_id = ThreadId::new(); + chat.thread_id = Some(thread_id); + chat.set_feature_enabled(Feature::Goals, /*enabled*/ true); + + let plan_mask = collaboration_modes::plan_mask(chat.model_catalog.as_ref()) + .expect("expected plan collaboration mode"); + chat.set_collaboration_mask(plan_mask); + expect_goal_pause_event(&mut rx, thread_id); + + chat.handle_server_notification( + ServerNotification::ThreadGoalUpdated( + codex_app_server_protocol::ThreadGoalUpdatedNotification { + thread_id: thread_id.to_string(), + turn_id: None, + goal: codex_app_server_protocol::ThreadGoal { + thread_id: thread_id.to_string(), + objective: "Keep planning safely".to_string(), + status: codex_app_server_protocol::ThreadGoalStatus::Active, + token_budget: None, + tokens_used: 0, + time_used_seconds: 0, + created_at: 1, + updated_at: 1, + }, + }, + ), + /*replay_kind*/ None, + ); + + expect_goal_pause_event(&mut rx, thread_id); +} + +fn expect_goal_pause_event( + rx: &mut tokio::sync::mpsc::UnboundedReceiver, + expected_thread_id: ThreadId, +) { + let event_thread_id = loop { + match rx.try_recv() { + Ok(AppEvent::PauseActiveGoalIfNeeded { thread_id }) => { + break thread_id; + } + Ok(_) => {} + Err(err) => panic!("expected PauseActiveGoalIfNeeded event, got {err:?}"), + } + }; + assert_eq!(event_thread_id, expected_thread_id); +} + #[tokio::test] async fn mode_switch_surfaces_model_change_notification_when_effective_model_changes() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(Some("gpt-5")).await; diff --git a/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs b/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs index 73f0d3b7ac..8c81ca6266 100644 --- a/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs +++ b/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs @@ -1642,6 +1642,52 @@ async fn status_line_goal_active_token_budget_footer_snapshot() { ); } +#[tokio::test] +async fn status_line_goal_active_plan_mode_footer_snapshot() { + use ratatui::Terminal; + use ratatui::backend::TestBackend; + + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5")).await; + chat.set_feature_enabled(Feature::Goals, /*enabled*/ true); + chat.show_welcome_banner = false; + chat.config.tui_status_line = Some(vec!["model-name".to_string()]); + let plan_mask = collaboration_modes::plan_mask(chat.model_catalog.as_ref()) + .expect("expected plan collaboration mode"); + chat.set_collaboration_mask(plan_mask); + chat.refresh_status_line(); + chat.handle_server_notification( + ServerNotification::ThreadGoalUpdated( + codex_app_server_protocol::ThreadGoalUpdatedNotification { + thread_id: "thread-1".to_string(), + turn_id: None, + goal: test_thread_goal( + codex_app_server_protocol::ThreadGoalStatus::Active, + /*token_budget*/ Some(50_000), + /*tokens_used*/ 40_000, + ), + }, + ), + /*replay_kind*/ None, + ); + assert_eq!( + chat.current_goal_status_indicator, + Some(GoalStatusIndicator::Active { + usage: Some("40K / 50K".to_string()), + }) + ); + + let width = 80; + let height = chat.desired_height(width); + let mut terminal = Terminal::new(TestBackend::new(width, height)).expect("create terminal"); + terminal + .draw(|f| chat.render(f.area(), f.buffer_mut())) + .expect("draw goal status footer"); + assert_chatwidget_snapshot!( + "status_line_goal_active_plan_mode_footer", + normalized_backend_snapshot(terminal.backend()) + ); +} + #[tokio::test] async fn status_line_goal_complete_elapsed_footer_snapshot() { use ratatui::Terminal; diff --git a/codex-rs/tui/src/goal_display.rs b/codex-rs/tui/src/goal_display.rs index d0455054b7..6370f5bfa2 100644 --- a/codex-rs/tui/src/goal_display.rs +++ b/codex-rs/tui/src/goal_display.rs @@ -2,6 +2,12 @@ use crate::status::format_tokens_compact; use codex_app_server_protocol::ThreadGoal; use codex_app_server_protocol::ThreadGoalStatus; +pub(crate) const GOAL_CONTINUATION_PAUSED_IN_PLAN_MODE_HINT: &str = "Goal continuation is paused in Plan mode. Switch out of Plan mode, then use /goal resume to continue."; + +pub(crate) fn show_goal_plan_mode_hint(status: ThreadGoalStatus, plan_mode_active: bool) -> bool { + plan_mode_active && status == ThreadGoalStatus::Paused +} + pub(crate) fn format_goal_elapsed_seconds(seconds: i64) -> String { let seconds = seconds.max(0) as u64; if seconds < 60 { @@ -104,4 +110,20 @@ mod tests { "Objective: Complete the task described in ../gameboy-long-running-prompt5.txt Time: 2m. Tokens: 63.9K/50K." ); } + + #[test] + fn plan_mode_hint_only_shows_for_paused_goals() { + assert!(show_goal_plan_mode_hint( + ThreadGoalStatus::Paused, + /*plan_mode_active*/ true + )); + assert!(!show_goal_plan_mode_hint( + ThreadGoalStatus::Active, + /*plan_mode_active*/ true + )); + assert!(!show_goal_plan_mode_hint( + ThreadGoalStatus::Paused, + /*plan_mode_active*/ false + )); + } }