diff --git a/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-experimental.json.zst b/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-experimental.json.zst index 8052517fed..cc70847e71 100644 Binary files a/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-experimental.json.zst and b/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-experimental.json.zst differ diff --git a/codex-rs/app-server-protocol/src/protocol/common.rs b/codex-rs/app-server-protocol/src/protocol/common.rs index 5015ea9a60..52ed76c606 100644 --- a/codex-rs/app-server-protocol/src/protocol/common.rs +++ b/codex-rs/app-server-protocol/src/protocol/common.rs @@ -941,6 +941,12 @@ client_request_definitions! { serialization: thread_id(params.thread_id), response: v2::ThreadInjectItemsResponse, }, + #[experimental("thread/prepareApprovedPlan")] + ThreadPrepareApprovedPlan => "thread/prepareApprovedPlan" { + params: v2::ThreadPrepareApprovedPlanParams, + serialization: thread_id(params.thread_id), + response: v2::ThreadPrepareApprovedPlanResponse, + }, SkillsList => "skills/list" { params: v2::SkillsListParams, serialization: global_shared_read("config"), diff --git a/codex-rs/app-server-protocol/src/protocol/v2/thread.rs b/codex-rs/app-server-protocol/src/protocol/v2/thread.rs index 047915303e..dca33cb611 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/thread.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/thread.rs @@ -43,6 +43,7 @@ use serde::Deserialize; use serde::Serialize; use serde_json::Value as JsonValue; use std::collections::HashMap; +use std::fmt; use std::path::PathBuf; #[derive(Serialize, Deserialize, Debug, Clone, Copy, PartialEq, Eq, JsonSchema, TS)] @@ -1830,6 +1831,34 @@ pub struct ThreadInjectItemsParams { #[ts(export_to = "v2/")] pub struct ThreadInjectItemsResponse {} +/// Prepare an approved plan for the first turn of a fresh thread. +#[derive(Serialize, Deserialize, Clone, PartialEq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct ThreadPrepareApprovedPlanParams { + pub thread_id: String, + pub markdown: String, + /// Absolute path to the optional app-server copy of the plan. + #[ts(optional = nullable)] + pub attachment_path: Option, +} + +impl fmt::Debug for ThreadPrepareApprovedPlanParams { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("ThreadPrepareApprovedPlanParams") + .field("thread_id", &self.thread_id) + .finish_non_exhaustive() + } +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct ThreadPrepareApprovedPlanResponse { + /// Initial user prompt. If no attachment is readable, the plan is already in the thread's context. + pub prompt: String, +} + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] diff --git a/codex-rs/app-server/src/message_processor.rs b/codex-rs/app-server/src/message_processor.rs index 309d579403..4bf10fc6cc 100644 --- a/codex-rs/app-server/src/message_processor.rs +++ b/codex-rs/app-server/src/message_processor.rs @@ -1773,6 +1773,11 @@ impl MessageProcessor { .thread_inject_items(&request_id, params) .await } + ClientRequest::ThreadPrepareApprovedPlan { params, .. } => { + self.turn_processor + .thread_prepare_approved_plan(&request_id, params) + .await + } ClientRequest::TurnSteer { params, .. } => { self.turn_processor.turn_steer(&request_id, params).await } diff --git a/codex-rs/app-server/src/request_processors/turn_processor.rs b/codex-rs/app-server/src/request_processors/turn_processor.rs index a86d84a432..6a69cc899d 100644 --- a/codex-rs/app-server/src/request_processors/turn_processor.rs +++ b/codex-rs/app-server/src/request_processors/turn_processor.rs @@ -5,6 +5,8 @@ use codex_app_server_protocol::ThreadGuardianAssessment; use codex_app_server_protocol::ThreadGuardianReviewParams; use codex_app_server_protocol::ThreadGuardianReviewResponse; use codex_app_server_protocol::ThreadItem; +use codex_app_server_protocol::ThreadPrepareApprovedPlanParams; +use codex_app_server_protocol::ThreadPrepareApprovedPlanResponse; use codex_protocol::error::CodexErrorDetails; use codex_protocol::models::ContentItem; use codex_protocol::models::FunctionCallOutputBody; @@ -16,6 +18,7 @@ use codex_protocol::protocol::TurnSettingsUpdate; use codex_protocol::protocol::TurnSettingsUpdateOutcome; use codex_protocol::turn_input::AnnotatedResponseItem; use codex_protocol::turn_input::ResponseItemAnnotations; +use codex_utils_path_uri::PathUri; use crate::image_url::REMOTE_IMAGE_URL_ERROR; use crate::image_url::is_remote_image_url; @@ -239,6 +242,33 @@ impl TurnRequestProcessor { .map(|response| Some(response.into())) } + pub(crate) async fn thread_prepare_approved_plan( + &self, + request_id: &ConnectionRequestId, + params: ThreadPrepareApprovedPlanParams, + ) -> Result, JSONRPCErrorError> { + let (_, thread) = self.load_thread(¶ms.thread_id).await?; + self.ensure_direct_input_allowed(request_id, thread.as_ref()) + .await?; + let actual_chars = params.markdown.chars().count(); + if actual_chars > MAX_USER_INPUT_TEXT_CHARS { + return Err(Self::input_too_large_error(actual_chars)); + } + let attachment_path = params + .attachment_path + .map(PathUri::from_host_native_path) + .transpose() + .map_err(|_| invalid_params("`attachmentPath` must be absolute on the app server"))?; + let prompt = thread + .prepare_approved_plan(attachment_path.as_ref(), ¶ms.markdown) + .await + .map_err(|err| match err.details() { + CodexErrorDetails::InvalidRequest(message) => invalid_request(message.clone()), + _ => internal_error(format!("failed to prepare the approved plan: {err}")), + })?; + Ok(Some(ThreadPrepareApprovedPlanResponse { prompt }.into())) + } + pub(crate) async fn thread_settings_update( &self, request_id: &ConnectionRequestId, diff --git a/codex-rs/core/src/codex_thread.rs b/codex-rs/core/src/codex_thread.rs index 78c3afeb35..3b694fccf4 100644 --- a/codex-rs/core/src/codex_thread.rs +++ b/codex-rs/core/src/codex_thread.rs @@ -1030,6 +1030,19 @@ impl CodexThread { Ok(()) } + /// Returns the first prompt for an approved plan, recording untrusted context if needed. + pub async fn prepare_approved_plan( + &self, + path: Option<&PathUri>, + markdown: &str, + ) -> CodexResult { + let (prompt, items) = self.session.prepare_approved_plan(path, markdown).await?; + if !items.is_empty() { + self.inject_response_items(items).await?; + } + Ok(prompt) + } + /// Record raw Responses API items immediately before admitting a user turn. /// /// The caller must submit the associated user input while retaining its diff --git a/codex-rs/core/src/session/approved_plan.rs b/codex-rs/core/src/session/approved_plan.rs new file mode 100644 index 0000000000..869717d560 --- /dev/null +++ b/codex-rs/core/src/session/approved_plan.rs @@ -0,0 +1,134 @@ +//! Prepares an approved plan using the fresh thread's configured executor and model. + +use std::time::Duration; + +use codex_file_system::EnvironmentAccess; +use codex_file_system::GetMetadataOptions; +use codex_protocol::error::CodexErr; +use codex_protocol::error::Result; +use codex_protocol::models::FunctionCallOutputPayload; +use codex_protocol::models::ResponseItem; +use codex_utils_output_truncation::TruncationPolicy; +use codex_utils_output_truncation::approx_bytes_for_tokens; +use codex_utils_path_uri::LegacyAppPathString; +use codex_utils_path_uri::PathUri; +use futures::StreamExt; + +use super::session::Session; +use super::turn_context::TurnContext; + +const ATTACHMENT_READ_TIMEOUT: Duration = Duration::from_secs(5); +const MAX_FALLBACK_TOKENS: usize = 10_000; +const MAX_FALLBACK_ITEMS: usize = 32; + +impl Session { + pub(crate) async fn prepare_approved_plan( + &self, + path: Option<&PathUri>, + markdown: &str, + ) -> Result<(String, Vec)> { + if markdown.trim().is_empty() { + return Err(CodexErr::InvalidRequest( + "The approved plan is empty.".into(), + )); + } + if self.active_turn.lock().await.is_some() { + return Err(CodexErr::InvalidRequest( + "The thread already has an active turn.".into(), + )); + } + let turn = self.new_inject_items_context().await; + if let Some(path) = path + && let Ok(Some(path)) = tokio::time::timeout( + ATTACHMENT_READ_TIMEOUT, + readable_path(&turn, path, markdown), + ) + .await + { + return Ok(( + format!("Read the approved plan at {path} and implement it."), + Vec::new(), + )); + } + Ok(( + "Implement the plan.".to_string(), + fallback_items(&turn, markdown)?, + )) + } +} + +fn fallback_items(turn: &TurnContext, mut markdown: &str) -> Result> { + let model = turn.model_info(); + let context_budget = model + .usable_context_window() + .map_or(MAX_FALLBACK_TOKENS, |window| { + usize::try_from(window / 4) + .unwrap_or_default() + .min(MAX_FALLBACK_TOKENS) + }); + let policy = TruncationPolicy::from(model.truncation_policy); + let chunk_bytes = policy + .byte_budget() + .min(approx_bytes_for_tokens(context_budget)); + let too_large = || { + CodexErr::InvalidRequest( + "The executor could not read the plan attachment and the complete plan is too large to send as context. Implement it in the original thread.".into(), + ) + }; + if markdown.len() > approx_bytes_for_tokens(context_budget) || chunk_bytes == 0 { + return Err(too_large()); + } + + let mut items = Vec::new(); + while !markdown.is_empty() { + let end = markdown.floor_char_boundary(chunk_bytes.min(markdown.len())); + if end == 0 || items.len() == MAX_FALLBACK_ITEMS { + return Err(too_large()); + } + let (chunk, rest) = markdown.split_at(end); + items.push(ResponseItem::FunctionCallOutput { + id: None, + call_id: None, + namespace: Some("agent".to_string()), + name: Some("proposed_plan".to_string()), + output: FunctionCallOutputPayload::from_text(chunk.to_string()), + internal_chat_message_metadata_passthrough: None, + }); + markdown = rest; + } + Ok(items) +} + +async fn readable_path(turn: &TurnContext, path: &PathUri, markdown: &str) -> Option { + let environment = turn.initial_environments.primary()?; + let convention = environment.cwd().infer_path_convention()?; + let native = LegacyAppPathString::from_path_uri(path, convention).ok()?; + if native.to_path_uri(convention).ok().as_ref() != Some(path) { + return None; + } + let native = native.into_string(); + if native.chars().any(char::is_control) { + return None; + } + + let grants = turn.granted_permissions(&environment.selection().environment_id); + let fs = environment.fs_accessor(grants); + let metadata = fs + .get_metadata( + path, + GetMetadataOptions { + follow_symlinks: false, + }, + ) + .await + .ok()?; + if !metadata.is_file || metadata.is_symlink || metadata.size != markdown.len() as u64 { + return None; + } + let mut remaining = markdown.as_bytes(); + let mut stream = fs.read_file_stream(path).await.ok()?; + while let Some(chunk) = stream.next().await { + remaining = remaining.strip_prefix(chunk.ok()?.as_ref())?; + } + remaining.is_empty().then_some(native) +} diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 5bde090b99..ce93060cd4 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -253,6 +253,7 @@ pub(crate) mod async_compaction; pub(crate) mod file_audio; pub(crate) use file_audio::FileAudioDurations; +mod approved_plan; mod code_mode_warning; mod config_refresh; pub(crate) mod context_window; diff --git a/codex-rs/tui/BUILD.bazel b/codex-rs/tui/BUILD.bazel index 0ab6eed6ea..6dbf443b15 100644 --- a/codex-rs/tui/BUILD.bazel +++ b/codex-rs/tui/BUILD.bazel @@ -22,6 +22,8 @@ codex_rust_crate( }, # The startup symlink test applies its own Seatbelt policy, which cannot be nested. test_tags = ["no-sandbox"], + # Large async test futures can overflow the default libtest stack. + unit_test_env = {"RUST_MIN_STACK": "25165824"}, # 24 MiB unit_test_srcs = glob([ "src/**/*_test_support.rs", "src/**/*_tests.rs", diff --git a/codex-rs/tui/src/app/event_dispatch.rs b/codex-rs/tui/src/app/event_dispatch.rs index 875476a8e0..881d272ed1 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -545,26 +545,8 @@ impl App { AppEvent::RawOutputModeChanged { enabled } => { self.apply_raw_output_mode(tui, enabled, /*notify*/ false); } - AppEvent::ClearUiAndSubmitUserMessage { text } => { - if self.reject_pending_permission_root_switch() { - self.chat_widget.restore_user_message_to_composer(text.into()); - return Ok(AppRunControl::Continue); - } - self.clear_terminal_ui(tui, /*redraw_header*/ false)?; - self.reset_app_ui_state_after_clear(); - - self.start_fresh_session( - tui, - app_server, - Some(ThreadStartSource::Clear), - crate::chatwidget::create_initial_user_message( - Some(text), - Vec::new(), - Vec::new(), - ), - /*new_thread_name*/ None, - ) - .await; + AppEvent::ImplementPlanInFreshSession { plan } => { + crate::boxed_future!(self.implement_plan_in_fresh_session(tui, app_server, plan)).await?; } AppEvent::OpenResumePicker => { self.pending_open_resume_picker = true; diff --git a/codex-rs/tui/src/app/session_lifecycle.rs b/codex-rs/tui/src/app/session_lifecycle.rs index fcf812cc4b..ac4d2b0376 100644 --- a/codex-rs/tui/src/app/session_lifecycle.rs +++ b/codex-rs/tui/src/app/session_lifecycle.rs @@ -20,6 +20,7 @@ use crate::app_server_session::source_agent_path; use crate::app_server_session::thread_blocks_direct_input; use crate::chatwidget::ThreadInputStateRestoreMode; use crate::streaming::commit_tick::CommitTickScope; +use codex_app_server_protocol::ThreadPrepareApprovedPlanParams; use codex_model_provider_info::OPENAI_PROVIDER_ID; use std::collections::HashSet; @@ -1242,38 +1243,15 @@ impl App { // Start a fresh in-memory session while preserving resumability via persisted rollout // history. If an initial message is provided, `enqueue_primary_thread_session` suppresses it // until the new session is configured and any replayed turns have been rendered. - let (config, local_settings) = match self.load_new_session_config(app_server).await { - Ok(config) => config, - Err(err) => { - if let Some(message) = initial_user_message { - self.chat_widget.restore_user_message_to_composer(message); - } - self.chat_widget - .add_error_message(format!("Failed to read new session defaults: {err}")); - tui.frame_requester().schedule_frame(); - return; - } - }; - match app_server - .start_thread_with_session_start_source( - &self.local_settings, - &config, - session_start_source, - /*remote_cwd_override*/ None, - /*selected_profile*/ None, - self.backend_preference_context.local_cwd.as_deref(), - ) + match self + .prepare_fresh_session(app_server, session_start_source) .await { - Ok(started) => { + Ok(prepared) => { self.adopt_fresh_session( tui, app_server, - PreparedFreshSession { - started, - config, - local_settings, - }, + prepared, ThreadAttachPresentation::Fresh, initial_user_message, new_thread_name, @@ -1281,9 +1259,7 @@ impl App { .await; } Err(err) => { - self.chat_widget.add_error_message(format!( - "Failed to start a fresh session through the app server: {err}" - )); + self.chat_widget.add_error_message(err.to_string()); if let Some(message) = initial_user_message { self.chat_widget.restore_user_message_to_composer(message); } @@ -1292,6 +1268,98 @@ impl App { tui.frame_requester().schedule_frame(); } + async fn prepare_fresh_session( + &mut self, + app_server: &mut AppServerSession, + session_start_source: Option, + ) -> Result { + let (config, local_settings) = + crate::boxed_future!(self.load_new_session_config(app_server)) + .await + .map_err(|err| { + color_eyre::eyre::eyre!("Failed to read new session defaults: {err}") + })?; + let started = crate::boxed_future!(app_server.start_thread_with_session_start_source( + &self.local_settings, + &config, + session_start_source, + /*remote_cwd_override*/ None, + /*selected_profile*/ None, + self.backend_preference_context.local_cwd.as_deref(), + )) + .await + .map_err(|err| { + color_eyre::eyre::eyre!("Failed to start a fresh session through the app server: {err}") + })?; + Ok(PreparedFreshSession { + started, + config, + local_settings, + }) + } + + pub(super) async fn implement_plan_in_fresh_session( + &mut self, + tui: &mut tui::Tui, + app_server: &mut AppServerSession, + plan: String, + ) -> Result<()> { + if self.reject_pending_permission_root_switch() { + return Ok(()); + } + let prepared = match self + .prepare_fresh_session(app_server, Some(ThreadStartSource::Clear)) + .await + { + Ok(prepared) => prepared, + Err(err) => { + self.chat_widget.add_error_message(err.to_string()); + tui.frame_requester().schedule_frame(); + return Ok(()); + } + }; + let thread_id = prepared.started.session.thread_id; + let codex_home = app_server.codex_home_path(&prepared.config.codex_home); + let attachment_path = crate::attachment_files::write_approved_plan( + app_server, + codex_home.as_ref(), + plan.clone(), + ) + .await + .map(|path| path.to_string()) + .inspect_err(|err| tracing::debug!(%err, "approved plan attachment unavailable")) + .ok(); + match app_server + .thread_prepare_approved_plan(ThreadPrepareApprovedPlanParams { + thread_id: thread_id.to_string(), + markdown: plan, + attachment_path, + }) + .await + { + Ok(prompt) => { + self.clear_terminal_ui(tui, /*redraw_header*/ false)?; + self.reset_app_ui_state_after_clear(); + self.adopt_fresh_session( + tui, + app_server, + prepared, + ThreadAttachPresentation::Fresh, + Some(prompt.into()), + /*new_thread_name*/ None, + ) + .await; + } + Err(err) => { + let _ = app_server.thread_unsubscribe(thread_id).await; + self.chat_widget + .add_error_message(format!("Failed to hand off the approved plan: {err:#}")); + } + } + tui.frame_requester().schedule_frame(); + Ok(()) + } + pub(super) async fn adopt_fresh_session( &mut self, tui: &mut tui::Tui, diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index efe7ad668d..cf56ac7a0b 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -9878,7 +9878,7 @@ async fn copy_picker_opening_preserves_terminal_scrollback_without_reflow() { !matches!( event, AppEvent::ClearUi { .. } - | AppEvent::ClearUiAndSubmitUserMessage { .. } + | AppEvent::ImplementPlanInFreshSession { .. } | AppEvent::InsertHistoryCell(_) | AppEvent::ConsolidateAgentMessage { .. } ), diff --git a/codex-rs/tui/src/app/tests/new_session_tests.rs b/codex-rs/tui/src/app/tests/new_session_tests.rs index 5da939cd71..74bf110ff5 100644 --- a/codex-rs/tui/src/app/tests/new_session_tests.rs +++ b/codex-rs/tui/src/app/tests/new_session_tests.rs @@ -6,6 +6,9 @@ use codex_app_server_protocol::SkillMetadata; use codex_app_server_protocol::SkillScope; use codex_app_server_protocol::SkillsListEntry; use codex_config::types::HistoryPersistence; +use core_test_support::context_snapshot; +use core_test_support::context_snapshot::ContextSnapshotOptions; +use core_test_support::responses; use crossterm::event::KeyCode; use crossterm::event::KeyEvent; use crossterm::event::KeyModifiers; @@ -523,6 +526,207 @@ async fn replacement_uses_server_defaults_and_preserves_explicit_launch_settings Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn plan_implementation_reads_approved_plan_file_in_fresh_session() -> Result<()> { + let (mut app, mut events, _ops) = make_test_app_with_channels().await; + // Match the canonical CODEX_HOME used at launch. + app.config.codex_home = app.config.codex_home.canonicalize()?; + let model = responses::start_mock_server().await; + app_test_support::MockResponsesConfig::new(&model.uri()) + // Exercise the local executor's file read without requiring a platform sandbox helper. + .with_sandbox_mode("danger-full-access") + .with_root_config("instructions = 'Help the user.'") + .write(app.config.codex_home.as_path())?; + let workspace = tempdir()?; + app.config = app + .rebuild_config_for_cwd(workspace.path().canonicalize()?) + .await?; + let mut server = crate::start_embedded_app_server_for_picker(&app.config).await?; + let mut tui = crate::tui::test_support::make_test_tui()?; + let plan = format!( + "# Plan\n{}\n- Verify the final step.\n", + "- Update the parser.\n".repeat(3_000) + ); + let source = server.start_thread(&app.config).await?; + let source_id = source.session.thread_id; + app.enqueue_primary_thread_session(source.session, source.turns) + .await?; + Box::pin(app.handle_event( + &mut tui, + &mut server, + AppEvent::ImplementPlanInFreshSession { plan: plan.clone() }, + )) + .await?; + let thread_id = app.chat_widget.thread_id().expect("fresh thread"); + assert_ne!(thread_id, source_id); + let attachments = app.config.codex_home.join("attachments"); + let mut dirs = std::fs::read_dir(&attachments)?; + let path = dirs + .next() + .expect("attachment directory")? + .path() + .join("approved-plan.md"); + assert!(dirs.next().is_none()); + assert_eq!(std::fs::read(&path)?, plan.as_bytes()); + let path = path.to_str().expect("UTF-8 attachment path"); + let command = if cfg!(windows) { + format!( + "Get-Content -LiteralPath '{}' -Tail 1", + path.replace("'", "''") + ) + } else { + format!("tail -n 1 {}", shlex::try_quote(path)?) + }; + let read_args = serde_json::json!({ "cmd": command }); + let response = responses::mount_sse_sequence( + &model, + vec![ + responses::sse(vec![ + responses::ev_response_created("read"), + responses::ev_function_call("read_plan", "exec_command", &read_args.to_string()), + responses::ev_completed("read"), + ]), + responses::sse_completed("implemented"), + ], + ) + .await; + + let mut user_rows = Vec::new(); + while let Ok(event) = events.try_recv() { + match event { + AppEvent::InsertHistoryCell(cell) if cell.as_any().is::() => { + user_rows + .push(lines_to_single_string(&cell.raw_lines()).replace(path, "")); + } + event @ (AppEvent::SubmitThreadOp { .. } | AppEvent::CodexOp(_)) => { + Box::pin(app.handle_event(&mut tui, &mut server, event)).await?; + } + _ => {} + } + } + insta::assert_snapshot!(user_rows.join("\n"), @"Read the approved plan at and implement it."); + super::safety_buffering::wait_for_turn_completed(&mut app, &mut server, thread_id).await; + let requests = response.requests(); + let [initial, after_read] = requests.as_slice() else { + panic!("expected a plan read and a follow-up request") + }; + let output = after_read + .function_call_output_text("read_plan") + .expect("plan read result"); + assert!(output.contains("- Verify the final step."), "{output}"); + let mut body = initial.body_json(); + let prompt = body["input"].as_array_mut().unwrap().last_mut().unwrap(); + assert_eq!(prompt["role"], "user"); + let text = &mut prompt["content"][0]["text"]; + *text = text.as_str().unwrap().replace(path, "").into(); + insta::assert_snapshot!( + "clear_context_plan_request", + context_snapshot::format_context_snapshot( + "Clear context and implement the approved plan in a fresh thread.", + &[context_snapshot::SnapshotEntry::body(&body)], + &ContextSnapshotOptions::default().rewrite_known_segments(), + ) + ); + server.shutdown().await?; + Ok(()) +} + +#[tokio::test] +async fn plan_implementation_handles_unavailable_attachment() -> Result<()> { + for (write_fails, oversized) in [(true, false), (false, false), (false, true)] { + crate::boxed_future!(assert_plan_implementation_with_unavailable_attachment( + write_fails, + oversized, + )) + .await?; + } + Ok(()) +} + +async fn assert_plan_implementation_with_unavailable_attachment( + write_fails: bool, + oversized: bool, +) -> Result<()> { + let (mut app, mut events, _ops) = make_test_app_with_channels().await; + let model = responses::start_mock_server().await; + let response = responses::mount_sse_once(&model, responses::sse_completed("implemented")).await; + let attachments = app.config.codex_home.join("attachments"); + let config = app_test_support::MockResponsesConfig::new(&model.uri()); + let config = if write_fails { + std::fs::write(&attachments, "")?; + config.with_sandbox_mode("danger-full-access") + } else { + let denied_path = serde_json::to_string(attachments.as_path())?; + config + // The embedded test executor has no sandbox helper for AGENTS.md discovery. + .with_root_config("default_permissions = 'no-attachments'\nproject_doc_max_bytes = 0") + .with_extra_config(&format!( + "[permissions.no-attachments.filesystem]\n\":root\" = 'read'\n{denied_path} = 'deny'" + )) + }; + config.write(app.config.codex_home.as_path())?; + let workspace = tempdir()?; + app.config = app + .rebuild_config_for_cwd(workspace.path().canonicalize()?) + .await?; + let mut server = crate::start_embedded_app_server_for_picker(&app.config).await?; + let mut tui = crate::tui::test_support::make_test_tui()?; + let source = server.start_thread(&app.config).await?; + let source_id = source.session.thread_id; + app.enqueue_primary_thread_session(source.session, source.turns) + .await?; + let steps = if oversized { 3_000 } else { 1_000 }; + let plan = format!( + "# Plan\n{}- Verify the final step.", + "- Update the parser.\n".repeat(steps) + ); + crate::boxed_future!(app.handle_event( + &mut tui, + &mut server, + AppEvent::ImplementPlanInFreshSession { plan: plan.clone() }, + )) + .await?; + + if !write_fails { + let path = std::fs::read_dir(&attachments)? + .next() + .expect("the app server saved the plan")? + .path() + .join("approved-plan.md"); + assert_eq!(std::fs::read_to_string(path)?, plan); + } + let thread_id = app.chat_widget.thread_id().expect("current thread"); + if oversized { + assert_eq!(thread_id, source_id); + assert!(response.requests().is_empty()); + } else { + assert_ne!(thread_id, source_id); + while let Ok(event) = events.try_recv() { + if matches!( + event, + AppEvent::SubmitThreadOp { .. } | AppEvent::CodexOp(_) + ) { + crate::boxed_future!(app.handle_event(&mut tui, &mut server, event)).await?; + } + } + super::safety_buffering::wait_for_turn_completed(&mut app, &mut server, thread_id).await; + let request = response.single_request(); + let messages = request.inputs_of_type("message"); + let user = messages.last().expect("implementation prompt"); + assert_eq!(user["role"], "user"); + assert_eq!(user["content"][0]["text"], "Implement the plan."); + let recovered = request + .inputs_of_type("function_call_output") + .iter() + .filter(|item| item["name"] == "proposed_plan") + .map(|item| item["output"].as_str().expect("plan chunk")) + .collect::(); + assert_eq!(recovered, plan); + } + server.shutdown().await?; + Ok(()) +} + #[tokio::test] async fn replacement_failure_keeps_current_task_and_restores_input() -> Result<()> { for fail_read in [false, true] { diff --git a/codex-rs/tui/src/app/tests/snapshots/codex_tui__app__tests__session_lifecycle_requests__new_session_tests__clear_context_plan_request.snap b/codex-rs/tui/src/app/tests/snapshots/codex_tui__app__tests__session_lifecycle_requests__new_session_tests__clear_context_plan_request.snap new file mode 100644 index 0000000000..369b57dd79 --- /dev/null +++ b/codex-rs/tui/src/app/tests/snapshots/codex_tui__app__tests__session_lifecycle_requests__new_session_tests__clear_context_plan_request.snap @@ -0,0 +1,22 @@ +--- +source: tui/src/app/tests/new_session_tests.rs +expression: "context_snapshot::format_context_snapshot(\"Clear context and implement the approved plan in a fresh thread.\",\n&[context_snapshot::SnapshotEntry::body(&body)],\n&ContextSnapshotOptions::default().rewrite_known_segments(),)" +--- +Scenario: Clear context and implement the approved plan in a fresh thread. + +## Window 1 +-- request 1 (request) -- +00:message/developer: + Help the user. +01:message/developer[4]: + [01] + [02] + [03] + [04] + Deferred tool namespaces: + - codex_tui: Manage Codex tasks available through the connected app server. + +02:message/user: + +03:message/user: + Read the approved plan at and implement it. diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index 56a1693c1c..c858e8196f 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -772,12 +772,9 @@ pub(crate) enum AppEvent { enabled: bool, }, - /// Clear the current context, start a fresh session, and submit an initial user message. - /// - /// This is the Plan Mode handoff path: the previous thread remains resumable, but the model - /// sees only the explicit prompt carried in `text` once the new session is configured. - ClearUiAndSubmitUserMessage { - text: String, + /// Start a fresh session with a reference to the approved plan file. + ImplementPlanInFreshSession { + plan: String, }, /// Open the resume picker inside the running TUI session. diff --git a/codex-rs/tui/src/app_server_session.rs b/codex-rs/tui/src/app_server_session.rs index 74c4c3ec5a..7a026d04c5 100644 --- a/codex-rs/tui/src/app_server_session.rs +++ b/codex-rs/tui/src/app_server_session.rs @@ -118,6 +118,8 @@ use codex_app_server_protocol::ThreadMemoryModeSetResponse; use codex_app_server_protocol::ThreadMetadataGitInfoUpdateParams; use codex_app_server_protocol::ThreadMetadataUpdateParams; use codex_app_server_protocol::ThreadMetadataUpdateResponse; +use codex_app_server_protocol::ThreadPrepareApprovedPlanParams; +use codex_app_server_protocol::ThreadPrepareApprovedPlanResponse; use codex_app_server_protocol::ThreadReadParams; use codex_app_server_protocol::ThreadReadResponse; use codex_app_server_protocol::ThreadResumeParams; @@ -1831,6 +1833,19 @@ impl AppServerSession { .wrap_err("thread/inject_items failed during TUI side conversation setup") } + pub(crate) async fn thread_prepare_approved_plan( + &mut self, + params: ThreadPrepareApprovedPlanParams, + ) -> Result { + let request_id = self.next_request_id(); + let response: ThreadPrepareApprovedPlanResponse = self + .request_handle + .request_typed(ClientRequest::ThreadPrepareApprovedPlan { request_id, params }) + .await + .wrap_err("thread/prepareApprovedPlan failed in TUI")?; + Ok(response.prompt) + } + #[allow(clippy::too_many_arguments)] async fn turn_start( &mut self, diff --git a/codex-rs/tui/src/attachment_files.rs b/codex-rs/tui/src/attachment_files.rs new file mode 100644 index 0000000000..70bd73ef95 --- /dev/null +++ b/codex-rs/tui/src/attachment_files.rs @@ -0,0 +1,54 @@ +//! Stores TUI attachments on the app-server host so later turns can reference them. + +use crate::app_server_session::AppServerSession; +use anyhow::Context; +use anyhow::Result; +use codex_app_server_client::AppServerPath; +use uuid::Uuid; + +pub(crate) const ATTACHMENT_DIR: &str = "attachments"; + +pub(crate) async fn create_attachment_dir( + app_server: &mut AppServerSession, + codex_home: Option<&AppServerPath>, +) -> Result { + let codex_home = + codex_home.context("App server did not report $CODEX_HOME; cannot create attachment")?; + let path = codex_home + .join(ATTACHMENT_DIR) + .join(Uuid::new_v4().to_string()); + app_server + .file_system() + .fs_create_directory_all_path(&path) + .await + .map_err(|err| anyhow::anyhow!("{err}")) + .with_context(|| format!("Could not create attachment directory {path}"))?; + Ok(path) +} + +pub(crate) async fn write_attachment( + app_server: &mut AppServerSession, + path: AppServerPath, + bytes: Vec, +) -> Result<()> { + app_server + .file_system() + .fs_write_file_path(&path, bytes) + .await + .map_err(|err| anyhow::anyhow!("{err}")) + .with_context(|| format!("Could not write attachment {path}")) +} + +pub(crate) async fn write_approved_plan( + app_server: &mut AppServerSession, + codex_home: Option<&AppServerPath>, + plan: String, +) -> Result { + let dir = create_attachment_dir(app_server, codex_home).await?; + let path = dir.join("approved-plan.md"); + if let Err(err) = write_attachment(app_server, path.clone(), plan.into_bytes()).await { + let _ = app_server.file_system().fs_remove_path(&dir).await; + return Err(err); + } + Ok(path) +} diff --git a/codex-rs/tui/src/chatwidget/plan_implementation.rs b/codex-rs/tui/src/chatwidget/plan_implementation.rs index 9e273d8883..f275f1412e 100644 --- a/codex-rs/tui/src/chatwidget/plan_implementation.rs +++ b/codex-rs/tui/src/chatwidget/plan_implementation.rs @@ -11,12 +11,6 @@ const PLAN_IMPLEMENTATION_YES: &str = "Yes, implement this plan"; const PLAN_IMPLEMENTATION_CLEAR_CONTEXT: &str = "Yes, clear context and implement"; const PLAN_IMPLEMENTATION_NO: &str = "No, stay in Plan mode"; pub(super) const PLAN_IMPLEMENTATION_CODING_MESSAGE: &str = "Implement the plan."; -pub(super) const PLAN_IMPLEMENTATION_CLEAR_CONTEXT_PREFIX: &str = concat!( - "A previous agent produced the plan below to accomplish the user's task. ", - "Implement the plan in a fresh context. Treat the plan as the source of ", - "user intent, re-read files as needed, and carry the work through ", - "implementation and verification." -); pub(super) const PLAN_IMPLEMENTATION_DEFAULT_UNAVAILABLE: &str = "Default mode unavailable"; pub(super) const PLAN_IMPLEMENTATION_NO_APPROVED_PLAN: &str = "No approved plan available"; @@ -55,12 +49,9 @@ pub(super) fn selection_view_params( Some(PLAN_IMPLEMENTATION_DEFAULT_UNAVAILABLE.to_string()), ), (Some(_), Some(plan_markdown)) if !plan_markdown.trim().is_empty() => { - let user_text = - format!("{PLAN_IMPLEMENTATION_CLEAR_CONTEXT_PREFIX}\n\n{plan_markdown}"); + let plan = plan_markdown.to_owned(); let actions: Vec = vec![Box::new(move |tx| { - tx.send(AppEvent::ClearUiAndSubmitUserMessage { - text: user_text.clone(), - }); + tx.send(AppEvent::ImplementPlanInFreshSession { plan: plan.clone() }); })]; (actions, None) } diff --git a/codex-rs/tui/src/chatwidget/tests/plan_mode.rs b/codex-rs/tui/src/chatwidget/tests/plan_mode.rs index fbf2246ccc..a87c0edbb6 100644 --- a/codex-rs/tui/src/chatwidget/tests/plan_mode.rs +++ b/codex-rs/tui/src/chatwidget/tests/plan_mode.rs @@ -138,16 +138,10 @@ async fn plan_implementation_popup_clear_context_emits_clear_submit_event() { chat.handle_key_event(KeyEvent::from(KeyCode::Enter)); let event = rx.try_recv().expect("expected AppEvent"); - let AppEvent::ClearUiAndSubmitUserMessage { text } = event else { - panic!("expected ClearUiAndSubmitUserMessage, got {event:?}"); + let AppEvent::ImplementPlanInFreshSession { plan } = event else { + panic!("expected ImplementPlanInFreshSession, got {event:?}"); }; - assert_eq!( - text, - "A previous agent produced the plan below to accomplish the user's task. \ - Implement the plan in a fresh context. Treat the plan as the source of \ - user intent, re-read files as needed, and carry the work through \ - implementation and verification.\n\n- Step 1\n- Step 2\n" - ); + assert_eq!(plan, plan_markdown); } #[tokio::test] diff --git a/codex-rs/tui/src/goal_files.rs b/codex-rs/tui/src/goal_files.rs index 163ec282dc..b646d818b2 100644 --- a/codex-rs/tui/src/goal_files.rs +++ b/codex-rs/tui/src/goal_files.rs @@ -4,6 +4,9 @@ use std::fs; use std::path::Path; use crate::app_server_session::AppServerSession; +use crate::attachment_files::ATTACHMENT_DIR; +use crate::attachment_files::create_attachment_dir; +use crate::attachment_files::write_attachment; use crate::bottom_pane::ChatComposer; use crate::bottom_pane::LocalImageAttachment; use anyhow::Context; @@ -14,7 +17,6 @@ use codex_protocol::protocol::MAX_THREAD_GOAL_OBJECTIVE_CHARS; use codex_protocol::user_input::TextElement; use uuid::Uuid; -const GOAL_ATTACHMENT_DIR: &str = "attachments"; const GOAL_FILE_PREFIX: &str = "Read the Codex goal objective file at "; const GOAL_FILE_SUFFIX: &str = " before continuing."; const GOAL_FILE_NAME: &str = "goal-objective.md"; @@ -69,7 +71,7 @@ pub(crate) async fn materialize_goal_draft( let path = ensure_goal_output_dir(app_server, codex_home, &mut output_dir) .await? .join(format!("pasted-text-{}.txt", replacements.len() + 1)); - write_goal_file(app_server, path.clone(), text.as_bytes().to_vec()).await?; + write_attachment(app_server, path.clone(), text.as_bytes().to_vec()).await?; replacements.push(( placeholder.clone(), @@ -94,7 +96,7 @@ pub(crate) async fn materialize_goal_draft( .join(format!("image-{}.{}", idx + 1, extension)); let bytes = fs::read(&image.path) .with_context(|| format!("Could not read goal image {}", image.path.display()))?; - write_goal_file(app_server, path.clone(), bytes).await?; + write_attachment(app_server, path.clone(), bytes).await?; if image.placeholder.is_empty() { image_lines.push(format!("- [Image #{}]: {path}", idx + 1)); } else { @@ -131,7 +133,7 @@ pub(crate) async fn materialize_goal_draft( return Err(err); } }; - write_goal_file(app_server, path.clone(), objective.as_bytes().to_vec()).await?; + write_attachment(app_server, path.clone(), objective.as_bytes().to_vec()).await?; objective = reference; } Ok((objective, output_dir)) @@ -166,7 +168,7 @@ pub(crate) fn objective_file_path( let parts = path.components(); let attachment_id = parts.get(parts.len().checked_sub(2)?)?; let expected = codex_home? - .join(GOAL_ATTACHMENT_DIR) + .join(ATTACHMENT_DIR) .join(attachment_id) .join(GOAL_FILE_NAME); (path == expected && Uuid::parse_str(attachment_id).is_ok()).then_some(path) @@ -191,33 +193,11 @@ async fn ensure_goal_output_dir( if let Some(output_dir) = output_dir { return Ok(output_dir.clone()); } - let codex_home = codex_home - .context("App server did not report $CODEX_HOME; cannot materialize goal files")?; - let path = codex_home - .join(GOAL_ATTACHMENT_DIR) - .join(Uuid::new_v4().to_string()); - app_server - .file_system() - .fs_create_directory_all_path(&path) - .await - .map_err(|err| anyhow::anyhow!("{err}")) - .with_context(|| format!("Could not create goal attachment directory {path}"))?; + let path = create_attachment_dir(app_server, codex_home).await?; *output_dir = Some(path.clone()); Ok(path) } -async fn write_goal_file( - app_server: &mut AppServerSession, - path: GoalFilePath, - bytes: Vec, -) -> Result<()> { - app_server - .file_system() - .fs_write_file_path(&path, bytes) - .await - .map_err(|err| anyhow::anyhow!("{err}")) - .with_context(|| format!("Could not write goal file {path}")) -} fn append_section(objective: &mut String, heading: &str, lines: Vec) { if lines.is_empty() { return; diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index 2ece37efee..8b3608cf9c 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -145,6 +145,7 @@ mod security_setup; mod worktree_browser; pub use custom_terminal::Terminal; mod assistant_directives; +mod attachment_files; mod auto_review_denials; mod cwd_prompt; mod debug_config; diff --git a/defs.bzl b/defs.bzl index 2b37b1aab4..caca84b315 100644 --- a/defs.bzl +++ b/defs.bzl @@ -215,6 +215,7 @@ def codex_rust_crate( integration_compile_data_extra = [], integration_test_args = [], unit_test_args = [], + unit_test_env = {}, binary_test_target_compatible_with = [], binary_unit_test_windows_runtime = None, integration_test_timeout = None, @@ -268,6 +269,7 @@ def codex_rust_crate( tests, including resources embedded with include_str! or include_bytes!. integration_test_args: Optional args for integration test binaries. unit_test_args: Optional args for the unit test binary. + unit_test_env: Extra runtime environment entries for library unit tests. binary_test_target_compatible_with: Platform constraints for binary unit tests. binary_unit_test_windows_runtime: Runtime directory containing bin/ DLLs for Windows binary unit tests. integration_test_timeout: Optional Bazel timeout for integration test @@ -410,7 +412,7 @@ def codex_rust_crate( workspace_root_test( name = unit_test_name, - env = test_env, + env = test_env | unit_test_env, runfile_env = { binary: "CARGO_BIN_EXE_" + Label(binary).name for binary in extra_binaries