diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index ed89c72878..b66bc57f97 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -88,6 +88,7 @@ use codex_protocol::models::ActivePermissionProfile; use codex_protocol::models::FileSystemPermissions; use codex_protocol::models::NetworkPermissions; use codex_protocol::models::PermissionProfile; +use codex_protocol::protocol::MAX_THREAD_GOAL_OBJECTIVE_CHARS; use codex_protocol::request_permissions::RequestPermissionProfile; use codex_protocol::user_input::TextElement; use codex_utils_absolute_path::AbsolutePathBuf; @@ -4148,6 +4149,51 @@ async fn make_test_app_with_channels() -> ( ) } +#[tokio::test] +async fn set_thread_goal_draft_materializes_long_objective_before_goal_set() -> Result<()> { + let mut app = make_test_app().await; + let mut app_server = crate::start_embedded_app_server_for_picker(app.chat_widget.config_ref()) + .await + .expect("embedded app server"); + let started = app_server + .start_thread(app.chat_widget.config_ref()) + .await + .expect("thread/start should succeed"); + let thread_id = started.session.thread_id; + app.enqueue_primary_thread_session(started.session, started.turns) + .await + .expect("primary thread should be registered"); + let objective = "x".repeat(MAX_THREAD_GOAL_OBJECTIVE_CHARS + 1); + + app.set_thread_goal_draft( + &mut app_server, + thread_id, + crate::goal_files::GoalDraft { + objective: objective.clone(), + ..Default::default() + }, + crate::app_event::ThreadGoalSetMode::ConfirmIfExists, + ) + .await; + + let goal = app_server + .thread_goal_get(thread_id) + .await? + .goal + .expect("goal should be set"); + let path = crate::goal_files::objective_file_path(&goal.objective) + .expect("goal objective should be a managed file reference"); + assert!(path.contains("attachments")); + assert_eq!( + crate::goal_files::objective_text_for_edit(&mut app_server, &goal.objective) + .await + .expect("managed goal file should be readable"), + objective + ); + app_server.shutdown().await?; + Ok(()) +} + fn test_thread_session(thread_id: ThreadId, cwd: PathBuf) -> ThreadSessionState { ThreadSessionState { thread_id, diff --git a/codex-rs/tui/src/app_server_session.rs b/codex-rs/tui/src/app_server_session.rs index 982ffaf1a1..8c8cd2d1c0 100644 --- a/codex-rs/tui/src/app_server_session.rs +++ b/codex-rs/tui/src/app_server_session.rs @@ -12,6 +12,8 @@ use crate::session_state::ThreadSessionState; use crate::status::StatusAccountDisplay; use crate::status::plan_type_display_name; use crate::terminal_visualization_instructions::with_terminal_visualization_instructions; +use base64::Engine; +use base64::engine::general_purpose::STANDARD; use codex_app_server_client::AppServerClient; use codex_app_server_client::AppServerEvent; use codex_app_server_client::AppServerRequestHandle; @@ -27,10 +29,17 @@ use codex_app_server_protocol::ExternalAgentConfigDetectResponse; use codex_app_server_protocol::ExternalAgentConfigImportParams; use codex_app_server_protocol::ExternalAgentConfigImportResponse; use codex_app_server_protocol::ExternalAgentConfigMigrationItem; +use codex_app_server_protocol::FsCreateDirectoryParams; +use codex_app_server_protocol::FsCreateDirectoryResponse; +use codex_app_server_protocol::FsReadFileParams; +use codex_app_server_protocol::FsReadFileResponse; +use codex_app_server_protocol::FsWriteFileParams; +use codex_app_server_protocol::FsWriteFileResponse; use codex_app_server_protocol::GetAccountParams; use codex_app_server_protocol::GetAccountRateLimitsResponse; use codex_app_server_protocol::GetAccountResponse; use codex_app_server_protocol::JSONRPCErrorError; +use codex_app_server_protocol::JSONRPCRequest; use codex_app_server_protocol::LogoutAccountResponse; use codex_app_server_protocol::MemoryResetResponse; use codex_app_server_protocol::Model as ApiModel; @@ -126,6 +135,8 @@ use codex_utils_absolute_path::AbsolutePathBuf; use color_eyre::eyre::ContextCompat; use color_eyre::eyre::Result; use color_eyre::eyre::WrapErr; +use serde::de::DeserializeOwned; +use serde_json::json; use std::collections::HashMap; use std::path::PathBuf; use std::sync::atomic::AtomicBool; @@ -931,6 +942,123 @@ impl AppServerSession { .wrap_err("thread/goal/clear failed in TUI") } + pub(crate) async fn fs_create_directory_path( + &mut self, + path: &str, + recursive: bool, + ) -> Result<()> { + let _: FsCreateDirectoryResponse = self + .request_fs_path( + "fs/createDirectory", + path, + |request_id, path| ClientRequest::FsCreateDirectory { + request_id, + params: FsCreateDirectoryParams { + path, + recursive: Some(recursive), + }, + }, + json!({ + "path": path, + "recursive": recursive, + }), + ) + .await?; + Ok(()) + } + + pub(crate) async fn fs_write_file_path(&mut self, path: &str, bytes: Vec) -> Result<()> { + let data_base64 = STANDARD.encode(bytes); + let _: FsWriteFileResponse = self + .request_fs_path( + "fs/writeFile", + path, + |request_id, path| ClientRequest::FsWriteFile { + request_id, + params: FsWriteFileParams { + path, + data_base64: data_base64.clone(), + }, + }, + json!({ + "path": path, + "dataBase64": data_base64, + }), + ) + .await?; + Ok(()) + } + + pub(crate) async fn fs_read_file_path(&mut self, path: &str) -> Result> { + let response: FsReadFileResponse = self + .request_fs_path( + "fs/readFile", + path, + |request_id, path| ClientRequest::FsReadFile { + request_id, + params: FsReadFileParams { path }, + }, + json!({ "path": path }), + ) + .await?; + STANDARD + .decode(response.data_base64) + .wrap_err("fs/readFile returned invalid base64 data") + } + + async fn request_fs_path( + &mut self, + method: &str, + path: &str, + local_request: impl FnOnce(RequestId, AbsolutePathBuf) -> ClientRequest, + remote_params: serde_json::Value, + ) -> Result + where + T: DeserializeOwned, + { + let request_id = self.next_request_id(); + if self.uses_remote_workspace() { + return self + .request_remote_json_rpc_typed(request_id, method, remote_params) + .await; + } + let path = AbsolutePathBuf::from_absolute_path_checked(path) + .wrap_err_with(|| format!("invalid local app-server fs path {path}"))?; + self.client + .request_typed(local_request(request_id, path)) + .await + .wrap_err_with(|| format!("{method} failed in TUI")) + } + + async fn request_remote_json_rpc_typed( + &self, + request_id: RequestId, + method: &str, + params: serde_json::Value, + ) -> Result + where + T: DeserializeOwned, + { + let AppServerRequestHandle::Remote(handle) = self.request_handle() else { + color_eyre::eyre::bail!( + "raw JSON-RPC requests are only supported by the remote app-server client" + ); + }; + let response = handle + .request_json_rpc(JSONRPCRequest { + id: request_id, + method: method.to_string(), + params: Some(params), + trace: None, + }) + .await + .wrap_err_with(|| format!("{method} failed in TUI"))?; + let result = response.map_err(|source| { + color_eyre::eyre::eyre!("{method} failed in TUI: {}", source.message) + })?; + serde_json::from_value(result).wrap_err_with(|| format!("{method} returned invalid data")) + } + pub(crate) async fn logout_account(&mut self) -> Result<()> { let request_id = self.next_request_id(); let _: LogoutAccountResponse = self diff --git a/codex-rs/tui/src/goal_files.rs b/codex-rs/tui/src/goal_files.rs index 17ddbb99fb..b290b71c09 100644 --- a/codex-rs/tui/src/goal_files.rs +++ b/codex-rs/tui/src/goal_files.rs @@ -4,6 +4,7 @@ //! server's Codex home directory. The persisted goal objective keeps references //! to those files so later continuations can read them by path. +use std::collections::HashMap; use std::fs; use std::path::Path; @@ -13,23 +14,9 @@ use crate::bottom_pane::LocalImageAttachment; use anyhow::Context; use anyhow::Result; use anyhow::bail; -use base64::Engine; -use base64::engine::general_purpose::STANDARD; -use codex_app_server_client::AppServerRequestHandle; -use codex_app_server_protocol::ClientRequest; -use codex_app_server_protocol::FsCreateDirectoryParams; -use codex_app_server_protocol::FsCreateDirectoryResponse; -use codex_app_server_protocol::FsReadFileParams; -use codex_app_server_protocol::FsReadFileResponse; -use codex_app_server_protocol::FsWriteFileParams; -use codex_app_server_protocol::FsWriteFileResponse; -use codex_app_server_protocol::JSONRPCRequest; -use codex_app_server_protocol::RequestId; use codex_protocol::protocol::MAX_THREAD_GOAL_OBJECTIVE_CHARS; use codex_protocol::user_input::TextElement; use codex_utils_absolute_path::AbsolutePathBuf; -use serde::de::DeserializeOwned; -use serde_json::json; use uuid::Uuid; const GOAL_ATTACHMENT_DIR: &str = "attachments"; @@ -74,113 +61,24 @@ pub(crate) type GoalFilePath = String; impl GoalFileStore for AppServerSession { async fn create_directory(&mut self, path: GoalFilePath) -> Result<()> { - let _: FsCreateDirectoryResponse = request_goal_fs( - self, - "fs/createDirectory", - ClientRequest::FsCreateDirectory { - request_id: goal_request_id(), - params: FsCreateDirectoryParams { - path: local_goal_file_path(&path)?, - recursive: Some(true), - }, - }, - json!({ - "path": path, - "recursive": true, - }), - ) - .await?; - Ok(()) + self.fs_create_directory_path(&path, /*recursive*/ true) + .await + .map_err(|err| anyhow::anyhow!("{err}")) } async fn write_file(&mut self, path: GoalFilePath, bytes: Vec) -> Result<()> { - let data_base64 = STANDARD.encode(bytes); - let _: FsWriteFileResponse = request_goal_fs( - self, - "fs/writeFile", - ClientRequest::FsWriteFile { - request_id: goal_request_id(), - params: FsWriteFileParams { - path: local_goal_file_path(&path)?, - data_base64: data_base64.clone(), - }, - }, - json!({ - "path": path, - "dataBase64": data_base64, - }), - ) - .await?; - Ok(()) + self.fs_write_file_path(&path, bytes) + .await + .map_err(|err| anyhow::anyhow!("{err}")) } async fn read_file(&mut self, path: GoalFilePath) -> Result> { - let response: FsReadFileResponse = request_goal_fs( - self, - "fs/readFile", - ClientRequest::FsReadFile { - request_id: goal_request_id(), - params: FsReadFileParams { - path: local_goal_file_path(&path)?, - }, - }, - json!({ "path": path }), - ) - .await?; - STANDARD - .decode(response.data_base64) - .context("fs/readFile returned invalid base64 data") + self.fs_read_file_path(&path) + .await + .map_err(|err| anyhow::anyhow!("{err}")) } } -async fn request_goal_fs( - app_server: &AppServerSession, - method: &str, - local_request: ClientRequest, - remote_params: serde_json::Value, -) -> Result -where - T: DeserializeOwned, -{ - if app_server.uses_remote_workspace() { - return remote_json_rpc_typed(app_server, method, remote_params).await; - } - app_server - .request_handle() - .request_typed(local_request) - .await - .with_context(|| format!("{method} failed in TUI")) -} - -async fn remote_json_rpc_typed( - app_server: &AppServerSession, - method: &str, - params: serde_json::Value, -) -> Result -where - T: DeserializeOwned, -{ - let AppServerRequestHandle::Remote(handle) = app_server.request_handle() else { - bail!("raw JSON-RPC requests are only supported by the remote app-server client"); - }; - let response = handle - .request_json_rpc(JSONRPCRequest { - id: goal_request_id(), - method: method.to_string(), - params: Some(params), - trace: None, - }) - .await - .with_context(|| format!("{method} failed in TUI"))?; - let result = - response.map_err(|source| anyhow::anyhow!("{method} failed in TUI: {}", source.message))?; - serde_json::from_value(result).with_context(|| format!("{method} returned invalid data")) -} - -fn goal_request_id() -> RequestId { - RequestId::String(format!("goal-files-{}", Uuid::new_v4())) -} - pub(crate) fn codex_home_for_app_server( app_server: &AppServerSession, local_codex_home: &AbsolutePathBuf, @@ -192,11 +90,6 @@ pub(crate) fn codex_home_for_app_server( } } -fn local_goal_file_path(path: &GoalFilePath) -> Result { - AbsolutePathBuf::from_absolute_path_checked(path) - .with_context(|| format!("invalid local goal file path {path}")) -} - fn join_goal_path(path: &str, segment: impl AsRef) -> GoalFilePath { let separator = if is_windows_absolute_path(path) { '\\' @@ -240,13 +133,20 @@ pub(crate) async fn materialize_goal_draft( bail!("Goal objective must not be empty."); } let text_elements = draft.text_elements; + let mut active_paste_placeholders = + active_paste_placeholder_counts(&objective, &text_elements, &draft.pending_pastes); let mut output_dir = None; let mut replacements = Vec::new(); - for (idx, (placeholder, text)) in draft.pending_pastes.iter().enumerate() { + let mut paste_idx = 0; + for (placeholder, text) in draft.pending_pastes.iter() { + if !take_active_placeholder(&mut active_paste_placeholders, placeholder) { + continue; + } + paste_idx += 1; let path = join_goal_path( &ensure_output_dir(store, codex_home, &mut output_dir).await?, - format!("pasted-text-{}.txt", idx + 1), + format!("pasted-text-{paste_idx}.txt"), ); write_file(store, path.clone(), text.as_bytes().to_vec()).await?; @@ -298,6 +198,37 @@ pub(crate) async fn materialize_goal_draft( Ok(objective) } +fn active_paste_placeholder_counts( + objective: &str, + text_elements: &[TextElement], + pending_pastes: &[(String, String)], +) -> HashMap { + let mut counts = pending_pastes + .iter() + .map(|(placeholder, _)| (placeholder.clone(), 0)) + .collect::>(); + for element in text_elements { + if let Some(count) = element + .placeholder(objective) + .and_then(|placeholder| counts.get_mut(placeholder)) + { + *count += 1; + } + } + counts +} + +fn take_active_placeholder(counts: &mut HashMap, placeholder: &str) -> bool { + let Some(count) = counts.get_mut(placeholder) else { + return false; + }; + if *count == 0 { + return false; + } + *count -= 1; + true +} + pub(crate) async fn objective_text_for_edit( store: &mut impl GoalFileStore, objective: &str, diff --git a/codex-rs/tui/src/goal_files_tests.rs b/codex-rs/tui/src/goal_files_tests.rs index e5cec75452..0bef5fd0f9 100644 --- a/codex-rs/tui/src/goal_files_tests.rs +++ b/codex-rs/tui/src/goal_files_tests.rs @@ -101,3 +101,23 @@ async fn plain_objective_does_not_need_codex_home() { assert_eq!(objective, "read src/lib.rs"); } + +#[tokio::test] +async fn deleted_paste_placeholder_does_not_materialize_or_need_codex_home() { + let mut store = RecordingStore::default(); + + let objective = materialize_goal_draft( + &mut store, + /*codex_home*/ None, + GoalDraft { + objective: "small goal".to_string(), + pending_pastes: vec![("[Pasted Content 5 chars]".to_string(), "hello".to_string())], + ..Default::default() + }, + ) + .await + .expect("materialize plain goal draft"); + + assert_eq!(objective, "small goal"); + assert!(store.writes.is_empty()); +}