diff --git a/codex-rs/app-server-client/src/lib.rs b/codex-rs/app-server-client/src/lib.rs index 1faae917eb9..73083322f77 100644 --- a/codex-rs/app-server-client/src/lib.rs +++ b/codex-rs/app-server-client/src/lib.rs @@ -15,6 +15,7 @@ //! bridging async `mpsc` channels on both sides. Queues are bounded so overload //! surfaces as channel-full errors rather than unbounded memory growth. +mod path; mod remote; use std::error::Error; @@ -58,6 +59,7 @@ pub use codex_exec_server::EnvironmentManager; pub use codex_exec_server::ExecServerRuntimePaths; use codex_feedback::CodexFeedback; use codex_protocol::protocol::SessionSource; +use codex_utils_absolute_path::AbsolutePathBuf; use serde::de::DeserializeOwned; use tokio::sync::mpsc; use tokio::sync::oneshot; @@ -65,6 +67,7 @@ use tokio::time::timeout; use toml::Value as TomlValue; use tracing::warn; +pub use crate::path::AppServerPath; pub use crate::remote::RemoteAppServerClient; pub use crate::remote::RemoteAppServerConnectArgs; pub use crate::remote::RemoteAppServerEndpoint; @@ -845,6 +848,15 @@ impl AppServerRequestHandle { } impl AppServerClient { + pub fn codex_home(&self, local_codex_home: &AbsolutePathBuf) -> Option { + match self { + Self::InProcess(_) => Some(AppServerPath::from_app_server( + local_codex_home.display().to_string(), + )), + Self::Remote(client) => client.codex_home().map(AppServerPath::from_app_server), + } + } + pub async fn request(&self, request: ClientRequest) -> IoResult { match self { Self::InProcess(client) => client.request(request).await, @@ -1110,6 +1122,7 @@ mod tests { id: request.id, result: serde_json::json!({ "userAgent": "codex_cli_rs/9.8.7-test (Test OS; x86_64) rust", + "codexHome": "/server/.codex", }), }), ) @@ -1446,6 +1459,7 @@ mod tests { .expect("remote client should connect"); assert_eq!(client.server_version(), Some("9.8.7-test")); + assert_eq!(client.codex_home(), Some("/server/.codex")); let response: GetAccountResponse = client .request_typed(ClientRequest::GetAccount { request_id: RequestId::Integer(1), diff --git a/codex-rs/app-server-client/src/path.rs b/codex-rs/app-server-client/src/path.rs new file mode 100644 index 00000000000..b2d782ecc7d --- /dev/null +++ b/codex-rs/app-server-client/src/path.rs @@ -0,0 +1,58 @@ +//! Paths resolved using the app-server host's platform rules. + +use std::fmt; + +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct AppServerPath(String); + +impl AppServerPath { + pub fn from_app_server(path: impl Into) -> Self { + Self(path.into()) + } + + pub fn from_absolute_str(raw: &str) -> Option { + (raw.starts_with('/') || is_windows_absolute_path(raw)).then(|| Self(raw.to_string())) + } + + pub fn as_str(&self) -> &str { + &self.0 + } + + pub fn components(&self) -> Vec<&str> { + let separators = if is_windows_absolute_path(&self.0) { + &['/', '\\'][..] + } else { + &['/'][..] + }; + self.0 + .split(separators) + .filter(|part| !part.is_empty()) + .collect() + } + + pub fn join(&self, segment: impl AsRef) -> Self { + let is_windows = is_windows_absolute_path(&self.0); + let (path, separator) = if is_windows { + (self.0.trim_end_matches(['/', '\\']), '\\') + } else { + (self.0.trim_end_matches('/'), '/') + }; + Self(format!("{path}{separator}{}", segment.as_ref())) + } +} + +impl fmt::Display for AppServerPath { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.0.fmt(f) + } +} + +fn is_windows_absolute_path(path: &str) -> bool { + let bytes = path.as_bytes(); + (bytes.len() >= 3 + && bytes[0].is_ascii_alphabetic() + && bytes[1] == b':' + && matches!(bytes[2], b'\\' | b'/')) + || path.starts_with("\\\\") + || path.starts_with("//") +} diff --git a/codex-rs/app-server-client/src/remote.rs b/codex-rs/app-server-client/src/remote.rs index 98020c041ce..e1c9f16c4a8 100644 --- a/codex-rs/app-server-client/src/remote.rs +++ b/codex-rs/app-server-client/src/remote.rs @@ -124,7 +124,7 @@ pub(crate) fn websocket_url_supports_auth_token(url: &Url) -> bool { enum RemoteClientCommand { Request { - request: Box, + request: Box, response_tx: oneshot::Sender>, }, Notify { @@ -151,6 +151,7 @@ pub struct RemoteAppServerClient { event_rx: mpsc::UnboundedReceiver, pending_events: VecDeque, server_version: Option, + codex_home: Option, worker_handle: tokio::task::JoinHandle<()>, } @@ -185,6 +186,10 @@ impl RemoteAppServerClient { self.server_version.as_deref() } + pub fn codex_home(&self) -> Option<&str> { + self.codex_home.as_deref() + } + async fn connect_with_stream( channel_capacity: usize, endpoint: String, @@ -195,7 +200,7 @@ impl RemoteAppServerClient { S: AsyncRead + AsyncWrite + Unpin + Send + 'static, { let mut stream = stream; - let (pending_events, server_version) = initialize_remote_connection( + let (pending_events, server_version, codex_home) = initialize_remote_connection( &mut stream, &endpoint, initialize_params, @@ -218,7 +223,7 @@ impl RemoteAppServerClient { }; match command { RemoteClientCommand::Request { request, response_tx } => { - let request_id = request_id_from_client_request(&request); + let request_id = request.id.clone(); if pending_requests.contains_key(&request_id) { let _ = response_tx.send(Err(IoError::new( ErrorKind::InvalidInput, @@ -229,7 +234,7 @@ impl RemoteAppServerClient { pending_requests.insert(request_id.clone(), response_tx); if let Err(err) = write_jsonrpc_message( &mut stream, - JSONRPCMessage::Request(jsonrpc_request_from_client_request(*request)), + JSONRPCMessage::Request(*request), &endpoint, ) .await @@ -472,6 +477,7 @@ impl RemoteAppServerClient { event_rx, pending_events: pending_events.into(), server_version, + codex_home, worker_handle, }) } @@ -483,25 +489,7 @@ impl RemoteAppServerClient { } pub async fn request(&self, request: ClientRequest) -> IoResult { - let (response_tx, response_rx) = oneshot::channel(); - self.command_tx - .send(RemoteClientCommand::Request { - request: Box::new(request), - response_tx, - }) - .await - .map_err(|_| { - IoError::new( - ErrorKind::BrokenPipe, - "remote app-server worker channel is closed", - ) - })?; - response_rx.await.map_err(|_| { - IoError::new( - ErrorKind::BrokenPipe, - "remote app-server request channel is closed", - ) - })? + self.request_handle().request(request).await } pub async fn request_typed(&self, request: ClientRequest) -> Result @@ -613,6 +601,7 @@ impl RemoteAppServerClient { event_rx, pending_events: _pending_events, server_version: _server_version, + codex_home: _codex_home, worker_handle, } = self; let mut worker_handle = worker_handle; @@ -637,6 +626,11 @@ impl RemoteAppServerClient { impl RemoteAppServerRequestHandle { pub async fn request(&self, request: ClientRequest) -> IoResult { + self.request_json_rpc(jsonrpc_request_from_client_request(request)) + .await + } + + pub async fn request_json_rpc(&self, request: JSONRPCRequest) -> IoResult { let (response_tx, response_rx) = oneshot::channel(); self.command_tx .send(RemoteClientCommand::Request { @@ -800,13 +794,14 @@ async fn initialize_remote_connection( endpoint: &str, params: InitializeParams, initialize_timeout: Duration, -) -> IoResult<(Vec, Option)> +) -> IoResult<(Vec, Option, Option)> where S: AsyncRead + AsyncWrite + Unpin, { let initialize_request_id = RequestId::String("initialize".to_string()); let mut pending_events = Vec::new(); let mut server_version = None; + let mut codex_home = None; write_jsonrpc_message( stream, JSONRPCMessage::Request(jsonrpc_request_from_client_request( @@ -838,6 +833,12 @@ where let (_, rest) = user_agent.split_once('/')?; rest.split_whitespace().next().map(str::to_string) }); + codex_home = response + .result + .get("codexHome") + .and_then(serde_json::Value::as_str) + .filter(|codex_home| !codex_home.is_empty()) + .map(str::to_string); break Ok(()); } JSONRPCMessage::Error(error) if error.id == initialize_request_id => { @@ -929,7 +930,7 @@ where ) .await?; - Ok((pending_events, server_version)) + Ok((pending_events, server_version, codex_home)) } fn app_server_event_from_notification(notification: JSONRPCNotification) -> Option { @@ -951,10 +952,6 @@ fn deliver_event( }) } -fn request_id_from_client_request(request: &ClientRequest) -> RequestId { - jsonrpc_request_from_client_request(request.clone()).id -} - fn jsonrpc_request_from_client_request(request: ClientRequest) -> JSONRPCRequest { let value = match serde_json::to_value(request) { Ok(value) => value, @@ -1024,6 +1021,7 @@ mod tests { event_rx, pending_events: VecDeque::new(), server_version: None, + codex_home: None, worker_handle, }; diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index e8ed3e33da1..25c6f21d3d7 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -11,10 +11,12 @@ use crate::app_backtrack::user_count; use crate::chatwidget::ChatWidgetInit; use crate::chatwidget::create_initial_user_message; +use crate::chatwidget::tests::helpers::render_bottom_popup; use crate::chatwidget::tests::make_chatwidget_manual_with_sender; use crate::chatwidget::tests::set_chatgpt_auth; use crate::chatwidget::tests::set_fast_mode_test_catalog; use crate::file_search::FileSearchManager; +use crate::goal_files; use crate::history_cell::AgentMarkdownCell; use crate::history_cell::AgentMessageCell; use crate::history_cell::HistoryCell; @@ -31,6 +33,7 @@ use crate::legacy_core::config::ConfigBuilder; use crate::legacy_core::config::ConfigOverrides; use crate::legacy_core::config::PermissionProfileSnapshot; use crate::legacy_core::config::TerminalResizeReflowMaxRows; +use codex_app_server_client::AppServerPath; use codex_app_server_protocol::AdditionalFileSystemPermissions; use codex_app_server_protocol::AdditionalNetworkPermissions; use codex_app_server_protocol::AdditionalPermissionProfile; @@ -90,6 +93,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; @@ -4150,6 +4154,97 @@ async fn make_test_app_with_channels() -> ( ) } +#[tokio::test] +async fn set_thread_goal_objective_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?; + let started = app_server + .start_thread(app.chat_widget.config_ref()) + .await?; + let thread_id = started.session.thread_id; + app.enqueue_primary_thread_session(started.session, started.turns) + .await?; + let objective = "x".repeat(MAX_THREAD_GOAL_OBJECTIVE_CHARS + 1); + + app.set_thread_goal_objective( + &mut app_server, + thread_id, + objective.clone(), + crate::app_event::ThreadGoalSetMode::ConfirmIfExists, + ) + .await; + + let response = app_server.thread_goal_get(thread_id).await?; + let goal = response.goal.expect("goal should be set"); + let saved_objective = goal.objective.clone(); + let codex_home = app_server + .codex_home_path(&app.chat_widget.config_ref().codex_home) + .expect("codex home"); + assert!(goal_files::objective_file_path(&goal.objective, Some(&codex_home)).is_some()); + assert_eq!( + goal_files::objective_text_for_edit(&mut app_server, Some(&codex_home), &goal.objective) + .await + .expect("managed goal file should be readable"), + objective + ); + let is_managed = |home: &AppServerPath, path: &str| { + let reference = goal_files::objective_file_reference(&AppServerPath::from_app_server(path)) + .expect("goal objective reference"); + goal_files::objective_file_path(&reference, Some(home)).is_some() + }; + let suffix = "attachments/00000000-0000-4000-8000-000000000000/goal-objective.md"; + for path in [ + format!("/tmp/{suffix}"), + format!("{codex_home}/../other/{suffix}"), + format!("{codex_home}/other/{suffix}"), + ] { + assert!(!is_managed(&codex_home, &path)); + } + assert!(!is_managed( + &AppServerPath::from_app_server("/tmp/codex\\home"), + &format!("/tmp/codex/home/{suffix}") + )); + let unix_path = AppServerPath::from_app_server("/tmp/codex\\").join("a"); + assert_eq!(unix_path.as_str(), "/tmp/codex\\/a"); + let attachments_dir = app.chat_widget.config_ref().codex_home.join("attachments"); + let attachment_count = std::fs::read_dir(&attachments_dir)?.count(); + + app.set_thread_goal_objective( + &mut app_server, + thread_id, + "x".repeat(MAX_THREAD_GOAL_OBJECTIVE_CHARS + 1), + crate::app_event::ThreadGoalSetMode::ConfirmIfExists, + ) + .await; + + assert_eq!( + std::fs::read_dir(&attachments_dir)?.count(), + attachment_count + ); + assert_eq!( + app_server + .thread_goal_get(thread_id) + .await? + .goal + .expect("goal should still be set") + .objective, + saved_objective + ); + app_server.shutdown().await?; + Ok(()) +} + +#[tokio::test] +async fn replace_goal_confirmation_snapshot() { + let mut app = make_test_app().await; + app.show_replace_thread_goal_confirmation(ThreadId::new(), "New goal".to_string()); + assert_app_snapshot!( + "replace_goal_confirmation", + render_bottom_popup(&app.chat_widget, /*width*/ 80) + ); +} + fn test_thread_session(thread_id: ThreadId, cwd: PathBuf) -> ThreadSessionState { ThreadSessionState { thread_id, diff --git a/codex-rs/tui/src/app/thread_goal_actions.rs b/codex-rs/tui/src/app/thread_goal_actions.rs index b1a1bd36786..ed55be0cae3 100644 --- a/codex-rs/tui/src/app/thread_goal_actions.rs +++ b/codex-rs/tui/src/app/thread_goal_actions.rs @@ -9,6 +9,8 @@ use crate::bottom_pane::popup_consts::standard_popup_hint_line; use crate::goal_display::GOAL_USAGE; use crate::goal_display::goal_status_label; use crate::goal_display::goal_usage_summary; +use crate::goal_files; +use crate::text_formatting::truncate_text; use codex_app_server_protocol::ThreadGoal; use codex_app_server_protocol::ThreadGoalStatus; use codex_protocol::ThreadId; @@ -103,11 +105,23 @@ impl App { } }; - let Some(goal) = response.goal else { + let Some(mut goal) = response.goal else { self.show_no_thread_goal_to_edit(); return; }; + let codex_home = app_server.codex_home_path(&self.config.codex_home); + match goal_files::objective_text_for_edit(app_server, codex_home.as_ref(), &goal.objective) + .await + { + Ok(objective) => goal.objective = objective, + Err(err) => { + self.chat_widget.add_error_message(err.to_string()); + } + } + if self.current_displayed_thread_id() != Some(thread_id) { + return; + } self.chat_widget.show_goal_edit_prompt(thread_id, goal); } @@ -118,6 +132,7 @@ impl App { objective: String, mode: ThreadGoalSetMode, ) { + let codex_home = app_server.codex_home_path(&self.config.codex_home); let mode = if matches!(mode, ThreadGoalSetMode::ConfirmIfExists) { let result = app_server.thread_goal_get(thread_id).await; if self.current_displayed_thread_id() != Some(thread_id) { @@ -143,11 +158,28 @@ impl App { mode }; + let (objective, output_dir) = match goal_files::materialize_goal_objective( + app_server, + codex_home.as_ref(), + objective, + ) + .await + { + Ok(objective) => objective, + Err(err) => { + if self.current_displayed_thread_id() == Some(thread_id) { + self.chat_widget.add_error_message(err.to_string()); + } + return; + } + }; + let replacing_goal = matches!(mode, ThreadGoalSetMode::ReplaceExisting); if replacing_goal { let result = app_server.thread_goal_clear(thread_id).await; if let Err(err) = result { + cleanup_materialized_goal_files(app_server, output_dir).await; if self.current_displayed_thread_id() != Some(thread_id) { return; } @@ -170,16 +202,23 @@ impl App { let result = app_server .thread_goal_set(thread_id, Some(objective), Some(status), token_budget) .await; - if self.current_displayed_thread_id() != Some(thread_id) { - return; - } 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) => { + if self.current_displayed_thread_id() != Some(thread_id) { + return; + } + self.chat_widget.add_info_message( + format!("Goal {}", goal_status_label(response.goal.status)), + Some(goal_usage_summary(&response.goal)), + ); + self.chat_widget.maybe_send_next_queued_input(); + } Err(err) => { + cleanup_materialized_goal_files(app_server, output_dir).await; + if self.current_displayed_thread_id() != Some(thread_id) { + return; + } let action = if replacing_goal { "replace" } else { "set" }; self.chat_widget .add_error_message(thread_goal_error_message(action, &err)); @@ -244,7 +283,11 @@ impl App { } } - fn show_replace_thread_goal_confirmation(&mut self, thread_id: ThreadId, objective: String) { + pub(super) fn show_replace_thread_goal_confirmation( + &mut self, + thread_id: ThreadId, + objective: String, + ) { let replace_objective = objective.clone(); let replace_actions: Vec = vec![Box::new(move |tx| { tx.send(AppEvent::SetThreadGoalObjective { @@ -270,7 +313,10 @@ impl App { ]; self.chat_widget.show_selection_view(SelectionViewParams { title: Some("Replace goal?".to_string()), - subtitle: Some(format!("New objective: {objective}")), + subtitle: Some(format!( + "New objective: {}", + truncate_text(&objective, /*max_graphemes*/ 200) + )), footer_hint: Some(standard_popup_hint_line()), items, ..Default::default() @@ -287,6 +333,17 @@ impl App { } } +async fn cleanup_materialized_goal_files( + app_server: &mut AppServerSession, + output_dir: Option, +) { + if let Some(output_dir) = output_dir + && let Err(err) = app_server.fs_remove_path(&output_dir).await + { + tracing::warn!("failed to clean up materialized goal files at {output_dir}: {err}"); + } +} + fn thread_goal_error_message(action: &str, err: &color_eyre::Report) -> String { if is_ephemeral_thread_goal_error(err) { EPHEMERAL_THREAD_GOAL_ERROR_MESSAGE.to_string() diff --git a/codex-rs/tui/src/app_server_session.rs b/codex-rs/tui/src/app_server_session.rs index 47184cfee28..856defffcf8 100644 --- a/codex-rs/tui/src/app_server_session.rs +++ b/codex-rs/tui/src/app_server_session.rs @@ -3,6 +3,8 @@ //! This module owns the typed JSON-RPC calls needed by the TUI and keeps //! request/response plumbing out of `App` and `ChatWidget`. +mod fs; + use crate::bottom_pane::FeedbackAudience; use crate::legacy_core::config::Config; use crate::permission_compat::legacy_compatible_permission_profile; @@ -14,6 +16,7 @@ use crate::status::plan_type_display_name; use crate::terminal_visualization_instructions::with_terminal_visualization_instructions; use codex_app_server_client::AppServerClient; use codex_app_server_client::AppServerEvent; +use codex_app_server_client::AppServerPath; use codex_app_server_client::AppServerRequestHandle; use codex_app_server_client::TypedRequestError; use codex_app_server_protocol::Account; @@ -247,6 +250,13 @@ impl AppServerSession { matches!(&self.client, AppServerClient::InProcess(_)) } + pub(crate) fn codex_home_path( + &self, + local_codex_home: &AbsolutePathBuf, + ) -> Option { + self.client.codex_home(local_codex_home) + } + pub(crate) fn server_version(&self) -> Option<&str> { let AppServerClient::Remote(client) = &self.client else { return None; diff --git a/codex-rs/tui/src/app_server_session/fs.rs b/codex-rs/tui/src/app_server_session/fs.rs new file mode 100644 index 00000000000..9a15f2585da --- /dev/null +++ b/codex-rs/tui/src/app_server_session/fs.rs @@ -0,0 +1,135 @@ +use super::AppServerSession; +use base64::Engine; +use base64::engine::general_purpose::STANDARD; +use codex_app_server_client::AppServerPath; +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::FsRemoveParams; +use codex_app_server_protocol::FsRemoveResponse; +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_utils_absolute_path::AbsolutePathBuf; +use color_eyre::eyre::Result; +use color_eyre::eyre::WrapErr; +use serde::de::DeserializeOwned; +use serde_json::json; + +impl AppServerSession { + pub(crate) async fn fs_create_directory_all_path( + &mut self, + path: &AppServerPath, + ) -> Result<()> { + self.request_fs_path::( + "fs/createDirectory", + path, + |request_id, path| ClientRequest::FsCreateDirectory { + request_id, + params: FsCreateDirectoryParams { + path, + recursive: Some(true), + }, + }, + json!({ "path": path.as_str(), "recursive": true }), + ) + .await + .map(drop) + } + + pub(crate) async fn fs_write_file_path( + &mut self, + path: &AppServerPath, + bytes: Vec, + ) -> Result<()> { + let data_base64 = STANDARD.encode(bytes); + 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.as_str(), "dataBase64": data_base64 }), + ) + .await + .map(drop) + } + + pub(crate) async fn fs_read_file_path(&mut self, path: &AppServerPath) -> Result> { + let response: FsReadFileResponse = self + .request_fs_path( + "fs/readFile", + path, + |request_id, path| ClientRequest::FsReadFile { + request_id, + params: FsReadFileParams { path }, + }, + json!({ "path": path.as_str() }), + ) + .await?; + STANDARD + .decode(response.data_base64) + .wrap_err("fs/readFile returned invalid base64 data") + } + + pub(crate) async fn fs_remove_path(&mut self, path: &AppServerPath) -> Result<()> { + self.request_fs_path::( + "fs/remove", + path, + |request_id, path| ClientRequest::FsRemove { + request_id, + params: FsRemoveParams { + path, + recursive: None, + force: None, + }, + }, + json!({ "path": path.as_str() }), + ) + .await + .map(drop) + } + + async fn request_fs_path( + &mut self, + method: &str, + path: &AppServerPath, + local_request: impl FnOnce(RequestId, AbsolutePathBuf) -> ClientRequest, + remote_params: serde_json::Value, + ) -> Result { + let request_id = self.next_request_id(); + match self.request_handle() { + AppServerRequestHandle::Remote(handle) => { + let response = handle + .request_json_rpc(JSONRPCRequest { + id: request_id, + method: method.to_string(), + params: Some(remote_params), + trace: None, + }) + .await + .wrap_err_with(|| format!("{method} failed in TUI"))?; + serde_json::from_value(response.map_err(|source| { + color_eyre::eyre::eyre!("{method} failed in TUI: {}", source.message) + })?) + .wrap_err_with(|| format!("{method} returned invalid data")) + } + AppServerRequestHandle::InProcess(_) => { + let path = AbsolutePathBuf::from_absolute_path_checked(path.as_str()) + .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")) + } + } + } +} diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index cdd4c752d3f..70299e53802 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -858,10 +858,6 @@ impl BottomPane { self.composer.input_enabled() } - pub(crate) fn composer_pending_pastes(&self) -> Vec<(String, String)> { - self.composer.pending_pastes() - } - pub(crate) fn apply_external_edit(&mut self, text: String) { self.composer.apply_external_edit(text); self.request_redraw(); diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index d8326f31012..defa1b5de0b 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -343,7 +343,6 @@ use self::goal_status::GoalStatusState; #[cfg(test)] use self::goal_status::goal_status_indicator_from_app_goal; mod goal_menu; -mod goal_validation; mod ide_context; use self::ide_context::IdeContextState; mod input_queue; diff --git a/codex-rs/tui/src/chatwidget/goal_validation.rs b/codex-rs/tui/src/chatwidget/goal_validation.rs deleted file mode 100644 index 2f9bcb931cd..00000000000 --- a/codex-rs/tui/src/chatwidget/goal_validation.rs +++ /dev/null @@ -1,64 +0,0 @@ -//! Validation helpers for `/goal` objective text. - -use super::*; -use crate::bottom_pane::ChatComposer; -use codex_protocol::num_format::format_with_separators; -use codex_protocol::protocol::MAX_THREAD_GOAL_OBJECTIVE_CHARS; - -const GOAL_TOO_LONG_FILE_HINT: &str = "Put longer instructions in a file and refer to that file in the goal, for example: /goal follow the instructions in docs/goal.md."; - -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub(super) enum GoalObjectiveValidationSource { - Live, - Queued, -} - -impl ChatWidget { - pub(super) fn goal_objective_with_pending_pastes_is_allowed( - &mut self, - args: &str, - text_elements: &[TextElement], - ) -> bool { - let pending_pastes = self.bottom_pane.composer_pending_pastes(); - let objective_chars = if pending_pastes.is_empty() { - args.trim().chars().count() - } else { - let (expanded, _) = - ChatComposer::expand_pending_pastes(args, text_elements.to_vec(), &pending_pastes); - expanded.trim().chars().count() - }; - self.goal_objective_char_count_is_allowed( - objective_chars, - GoalObjectiveValidationSource::Live, - ) - } - - pub(super) fn goal_objective_is_allowed( - &mut self, - objective: &str, - source: GoalObjectiveValidationSource, - ) -> bool { - self.goal_objective_char_count_is_allowed(objective.chars().count(), source) - } - - fn goal_objective_char_count_is_allowed( - &mut self, - actual_chars: usize, - source: GoalObjectiveValidationSource, - ) -> bool { - if actual_chars <= MAX_THREAD_GOAL_OBJECTIVE_CHARS { - return true; - } - let actual_chars = format_with_separators(actual_chars as i64); - let max_chars = format_with_separators(MAX_THREAD_GOAL_OBJECTIVE_CHARS as i64); - self.add_error_message(format!( - "Goal objective is too long: {actual_chars} characters. Limit: {max_chars} characters. {GOAL_TOO_LONG_FILE_HINT}" - )); - if source == GoalObjectiveValidationSource::Live { - self.bottom_pane - .set_composer_text(String::new(), Vec::new(), Vec::new()); - self.bottom_pane.drain_pending_submission_state(); - } - false - } -} diff --git a/codex-rs/tui/src/chatwidget/slash_dispatch.rs b/codex-rs/tui/src/chatwidget/slash_dispatch.rs index 5d0efea2fc3..b5da5ca8fd9 100644 --- a/codex-rs/tui/src/chatwidget/slash_dispatch.rs +++ b/codex-rs/tui/src/chatwidget/slash_dispatch.rs @@ -5,7 +5,6 @@ //! dispatch step and records the staged entry once the command has been handled, so //! slash-command recall follows the same submitted-input rule as ordinary text. -use super::goal_validation::GoalObjectiveValidationSource; use super::*; use crate::app_event::ThreadGoalSetMode; use crate::bottom_pane::prompt_args::parse_slash_name; @@ -570,12 +569,6 @@ impl ChatWidget { return; } - if cmd == SlashCommand::Goal - && !self.goal_objective_with_pending_pastes_is_allowed(&args, &text_elements) - { - return; - } - let Some((prepared_args, prepared_elements)) = self.prepare_live_inline_args(args, text_elements) else { @@ -607,6 +600,12 @@ impl ChatWidget { } } + fn clear_live_goal_submission(&mut self) { + self.bottom_pane + .set_composer_text(String::new(), Vec::new(), Vec::new()); + self.bottom_pane.drain_pending_submission_state(); + } + fn prepared_inline_user_message( &mut self, args: String, @@ -714,6 +713,9 @@ impl ChatWidget { } SlashCommand::Goal if !trimmed.is_empty() => { if !self.config.features.enabled(Feature::Goals) { + if source == SlashCommandDispatchSource::Live { + self.clear_live_goal_submission(); + } return; } enum GoalControlCommand { @@ -727,7 +729,7 @@ impl ChatWidget { thread_id: self.thread_id, }); if source == SlashCommandDispatchSource::Live { - self.bottom_pane.drain_pending_submission_state(); + self.clear_live_goal_submission(); } return; } @@ -743,6 +745,9 @@ impl ChatWidget { "The session must start before you can change a goal.".to_string(), ), ); + if source == SlashCommandDispatchSource::Live { + self.clear_live_goal_submission(); + } return; }; match command { @@ -757,29 +762,11 @@ impl ChatWidget { } self.append_message_history_entry(format!("/goal {trimmed}")); if source == SlashCommandDispatchSource::Live { - self.bottom_pane.drain_pending_submission_state(); + self.clear_live_goal_submission(); } return; } let objective = args.trim(); - if objective.is_empty() { - self.add_error_message("Goal objective must not be empty.".to_string()); - self.add_info_message( - GOAL_USAGE.to_string(), - Some(GOAL_USAGE_HINT.to_string()), - ); - if source == SlashCommandDispatchSource::Live { - self.bottom_pane.drain_pending_submission_state(); - } - return; - } - let validation_source = match source { - SlashCommandDispatchSource::Live => GoalObjectiveValidationSource::Live, - SlashCommandDispatchSource::Queued => GoalObjectiveValidationSource::Queued, - }; - if !self.goal_objective_is_allowed(objective, validation_source) { - return; - } let Some(thread_id) = self.thread_id else { if source == SlashCommandDispatchSource::Live { self.queue_user_message_with_options( @@ -792,7 +779,7 @@ impl ChatWidget { }, QueuedInputAction::ParseSlash, ); - self.bottom_pane.drain_pending_submission_state(); + self.clear_live_goal_submission(); } else { self.add_info_message( GOAL_USAGE.to_string(), @@ -808,7 +795,7 @@ impl ChatWidget { }); self.append_message_history_entry(format!("/goal {trimmed}")); if source == SlashCommandDispatchSource::Live { - self.bottom_pane.drain_pending_submission_state(); + self.clear_live_goal_submission(); } } SlashCommand::Side | SlashCommand::Btw if !trimmed.is_empty() => { @@ -945,11 +932,6 @@ impl ChatWidget { rest_offset + leading_trimmed, &text_elements, ); - if cmd == SlashCommand::Goal - && !self.goal_objective_is_allowed(trimmed_rest, GoalObjectiveValidationSource::Queued) - { - return QueueDrain::Continue; - } self.dispatch_prepared_command_with_args( cmd, PreparedSlashCommandArgs { diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index c20768e10e1..0aaa7374dec 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -227,7 +227,7 @@ mod exec_flow; mod goal_menu; mod goal_validation; mod guardian; -mod helpers; +pub(crate) mod helpers; mod history_replay; mod mcp_startup; mod permissions; diff --git a/codex-rs/tui/src/chatwidget/tests/goal_validation.rs b/codex-rs/tui/src/chatwidget/tests/goal_validation.rs index b186cdf7ff6..379be6ad60d 100644 --- a/codex-rs/tui/src/chatwidget/tests/goal_validation.rs +++ b/codex-rs/tui/src/chatwidget/tests/goal_validation.rs @@ -1,6 +1,5 @@ use super::*; use codex_protocol::protocol::MAX_THREAD_GOAL_OBJECTIVE_CHARS; -use codex_protocol::user_input::MAX_USER_INPUT_TEXT_CHARS; use pretty_assertions::assert_eq; fn complete_turn_with_message(chat: &mut ChatWidget, turn_id: &str, message: Option<&str>) { @@ -33,25 +32,22 @@ fn queue_composer_text_with_tab(chat: &mut ChatWidget, text: &str) { chat.handle_key_event(KeyEvent::new(KeyCode::Tab, KeyModifiers::NONE)); } -fn drain_app_events(rx: &mut tokio::sync::mpsc::UnboundedReceiver) -> Vec { - std::iter::from_fn(|| rx.try_recv().ok()).collect() -} - -fn rendered_insert_history(events: &[AppEvent]) -> String { - events - .iter() - .filter_map(|event| match event { - AppEvent::InsertHistoryCell(cell) => Some( - cell.display_lines(/*width*/ 80) - .into_iter() - .map(|line| line.to_string()) - .collect::>() - .join("\n"), - ), - _ => None, - }) - .collect::>() - .join("\n") +fn next_goal_objective( + rx: &mut tokio::sync::mpsc::UnboundedReceiver, + expected_thread_id: ThreadId, +) -> String { + loop { + let event = rx.try_recv().expect("expected goal objective event"); + if let AppEvent::SetThreadGoalObjective { + thread_id, + objective, + .. + } = event + { + assert_eq!(thread_id, expected_thread_id); + return objective; + } + } } #[tokio::test] @@ -104,40 +100,29 @@ async fn goal_slash_command_accepts_multiline_objective_after_blank_first_line() } #[tokio::test] -async fn goal_slash_command_rejects_oversized_objective() { +async fn goal_slash_command_emits_oversized_objective() { let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await; chat.set_feature_enabled(Feature::Goals, /*enabled*/ true); - chat.thread_id = Some(ThreadId::new()); + let thread_id = ThreadId::new(); + chat.thread_id = Some(thread_id); let objective = "x".repeat(MAX_THREAD_GOAL_OBJECTIVE_CHARS + 1); submit_composer_text(&mut chat, &format!("/goal {objective}")); - let events = drain_app_events(&mut rx); - assert!( - !events - .iter() - .any(|event| matches!(event, AppEvent::SetThreadGoalObjective { .. })), - "oversized goal should not emit a SetThreadGoalObjective event: {events:?}" - ); - let rendered = rendered_insert_history(&events); - assert!(rendered.contains("Goal objective is too long")); - assert!(rendered.contains("Put longer instructions in a file")); - assert!( - !rendered.contains("Message exceeds the maximum length"), - "expected goal-specific length error, got {rendered:?}" - ); + assert_eq!(next_goal_objective(&mut rx, thread_id), objective); assert_no_submit_op(&mut op_rx); } #[tokio::test] -async fn goal_slash_command_rejects_large_paste_using_expanded_length() { +async fn goal_slash_command_expands_large_pasted_objective() { let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await; chat.set_feature_enabled(Feature::Goals, /*enabled*/ true); - chat.thread_id = Some(ThreadId::new()); + let thread_id = ThreadId::new(); + chat.thread_id = Some(thread_id); + let objective = "x".repeat(MAX_THREAD_GOAL_OBJECTIVE_CHARS + 1); chat.bottom_pane .set_composer_text("/goal ".to_string(), Vec::new(), Vec::new()); - let objective = "x".repeat(MAX_THREAD_GOAL_OBJECTIVE_CHARS + 1); - chat.handle_paste(objective); + chat.handle_paste(objective.clone()); assert!( chat.bottom_pane.composer_text().contains("[Pasted Content"), @@ -145,56 +130,16 @@ async fn goal_slash_command_rejects_large_paste_using_expanded_length() { ); submit_current_composer(&mut chat); - let events = drain_app_events(&mut rx); - assert!( - !events - .iter() - .any(|event| matches!(event, AppEvent::SetThreadGoalObjective { .. })), - "oversized pasted goal should not emit a SetThreadGoalObjective event: {events:?}" - ); - let rendered = rendered_insert_history(&events); - assert!(rendered.contains("Goal objective is too long")); - assert!(rendered.contains("Put longer instructions in a file")); - assert!( - !rendered.contains("Message exceeds the maximum length"), - "expected goal-specific length error, got {rendered:?}" - ); + assert_eq!(next_goal_objective(&mut rx, thread_id), objective); assert_no_submit_op(&mut op_rx); } #[tokio::test] -async fn goal_slash_command_giant_paste_uses_goal_specific_error() { +async fn queued_goal_slash_command_emits_oversized_objective_and_stops_queue() { let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await; chat.set_feature_enabled(Feature::Goals, /*enabled*/ true); - chat.thread_id = Some(ThreadId::new()); - chat.bottom_pane - .set_composer_text("/goal ".to_string(), Vec::new(), Vec::new()); - chat.handle_paste("x".repeat(MAX_USER_INPUT_TEXT_CHARS + 1)); - - submit_current_composer(&mut chat); - - let events = drain_app_events(&mut rx); - assert!( - !events - .iter() - .any(|event| matches!(event, AppEvent::SetThreadGoalObjective { .. })), - "giant pasted goal should not emit a SetThreadGoalObjective event: {events:?}" - ); - let rendered = rendered_insert_history(&events); - assert!(rendered.contains("Goal objective is too long")); - assert!(rendered.contains("Put longer instructions in a file")); - assert!( - !rendered.contains("Message exceeds the maximum length"), - "expected goal-specific length error, got {rendered:?}" - ); - assert_no_submit_op(&mut op_rx); -} - -#[tokio::test] -async fn queued_goal_slash_command_rejects_oversized_objective_and_drains_next_input() { - let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await; - chat.set_feature_enabled(Feature::Goals, /*enabled*/ true); - chat.thread_id = Some(ThreadId::new()); + let thread_id = ThreadId::new(); + chat.thread_id = Some(thread_id); handle_turn_started(&mut chat, "turn-1"); let objective = "x".repeat(MAX_THREAD_GOAL_OBJECTIVE_CHARS + 1); @@ -204,26 +149,7 @@ async fn queued_goal_slash_command_rejects_oversized_objective_and_drains_next_i complete_turn_with_message(&mut chat, "turn-1", Some("done")); - let events = drain_app_events(&mut rx); - assert!( - !events - .iter() - .any(|event| matches!(event, AppEvent::SetThreadGoalObjective { .. })), - "oversized queued goal should not emit a SetThreadGoalObjective event: {events:?}" - ); - let rendered = rendered_insert_history(&events); - assert!(rendered.contains("Goal objective is too long")); - assert!(rendered.contains("Put longer instructions in a file")); - match next_submit_op(&mut op_rx) { - Op::UserTurn { items, .. } => assert_eq!( - items, - vec![UserInput::Text { - text: "continue".to_string(), - text_elements: Vec::new(), - }] - ), - other => panic!("expected queued follow-up after oversized goal, got {other:?}"), - } - assert!(chat.input_queue.queued_user_messages.is_empty()); + assert_eq!(next_goal_objective(&mut rx, thread_id), objective); + assert_eq!(chat.input_queue.queued_user_messages.len(), 1); assert_no_submit_op(&mut op_rx); } diff --git a/codex-rs/tui/src/chatwidget/tests/helpers.rs b/codex-rs/tui/src/chatwidget/tests/helpers.rs index 92a903b7de3..40370ee98d7 100644 --- a/codex-rs/tui/src/chatwidget/tests/helpers.rs +++ b/codex-rs/tui/src/chatwidget/tests/helpers.rs @@ -1208,7 +1208,7 @@ pub(super) fn render_bottom_first_row(chat: &ChatWidget, width: u16) -> String { String::new() } -pub(super) fn render_bottom_popup(chat: &ChatWidget, width: u16) -> String { +pub(crate) fn render_bottom_popup(chat: &ChatWidget, width: u16) -> String { let height = chat.desired_height(width); let area = Rect::new(0, 0, width, height); let mut buf = Buffer::empty(area); diff --git a/codex-rs/tui/src/goal_files.rs b/codex-rs/tui/src/goal_files.rs new file mode 100644 index 00000000000..149e767c4d1 --- /dev/null +++ b/codex-rs/tui/src/goal_files.rs @@ -0,0 +1,94 @@ +//! Materializes oversized TUI goal objectives as app-server-host files. + +use crate::app_server_session::AppServerSession; +use anyhow::Context; +use anyhow::Result; +use anyhow::bail; +use anyhow::ensure; +use codex_app_server_client::AppServerPath; +use codex_protocol::protocol::MAX_THREAD_GOAL_OBJECTIVE_CHARS; +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"; + +pub(crate) type GoalFilePath = AppServerPath; + +pub(crate) async fn materialize_goal_objective( + app_server: &mut AppServerSession, + codex_home: Option<&GoalFilePath>, + objective: String, +) -> Result<(String, Option)> { + let objective = objective.trim().to_string(); + ensure!(!objective.is_empty(), "Goal objective must not be empty."); + + if objective.chars().count() <= MAX_THREAD_GOAL_OBJECTIVE_CHARS { + return Ok((objective, None)); + } + + let codex_home = codex_home + .context("App server did not report $CODEX_HOME; cannot materialize goal files")?; + let output_dir = codex_home + .join(GOAL_ATTACHMENT_DIR) + .join(Uuid::new_v4().to_string()); + let path = output_dir.join(GOAL_FILE_NAME); + let reference = objective_file_reference(&path)?; + app_server + .fs_create_directory_all_path(&output_dir) + .await + .map_err(|err| anyhow::anyhow!("{err}")) + .with_context(|| format!("Could not create goal attachment directory {output_dir}"))?; + app_server + .fs_write_file_path(&path, objective.as_bytes().to_vec()) + .await + .map_err(|err| anyhow::anyhow!("{err}")) + .with_context(|| format!("Could not write goal file {path}"))?; + Ok((reference, Some(output_dir))) +} + +pub(crate) async fn objective_text_for_edit( + app_server: &mut AppServerSession, + codex_home: Option<&GoalFilePath>, + objective: &str, +) -> Result { + let Some(path) = objective_file_path(objective, codex_home) else { + return Ok(objective.to_string()); + }; + let bytes = app_server + .fs_read_file_path(&path) + .await + .map_err(|err| anyhow::anyhow!("{err}")) + .with_context(|| format!("Could not read goal objective file {path}"))?; + String::from_utf8(bytes) + .with_context(|| format!("Goal objective file {path} is not valid UTF-8")) +} + +pub(crate) fn objective_file_path( + objective: &str, + codex_home: Option<&GoalFilePath>, +) -> Option { + let path = objective + .strip_prefix(GOAL_FILE_PREFIX) + .and_then(|path| path.strip_suffix(GOAL_FILE_SUFFIX))?; + let path = AppServerPath::from_absolute_str(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_id) + .join(GOAL_FILE_NAME); + (path == expected && Uuid::parse_str(attachment_id).is_ok()).then_some(path) +} + +pub(crate) fn objective_file_reference(path: &GoalFilePath) -> Result { + let reference = format!("{GOAL_FILE_PREFIX}{path}{GOAL_FILE_SUFFIX}"); + let actual_chars = reference.chars().count(); + if actual_chars > MAX_THREAD_GOAL_OBJECTIVE_CHARS { + bail!( + "Goal objective file reference is too long: {actual_chars} characters. Limit: {MAX_THREAD_GOAL_OBJECTIVE_CHARS} characters." + ); + } + Ok(reference) +} diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index 13aecf217bb..e3827704b75 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -138,6 +138,7 @@ mod frames; mod get_git_diff; mod git_action_directives; mod goal_display; +mod goal_files; mod history_cell; mod hooks_rpc; mod ide_context; diff --git a/codex-rs/tui/src/snapshots/codex_tui__app__tests__replace_goal_confirmation.snap b/codex-rs/tui/src/snapshots/codex_tui__app__tests__replace_goal_confirmation.snap new file mode 100644 index 00000000000..d7d6bf0f8d5 --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__app__tests__replace_goal_confirmation.snap @@ -0,0 +1,11 @@ +--- +source: tui/src/app/tests.rs +expression: "crate::chatwidget::tests::helpers::render_bottom_popup(&app.chat_widget, 80)" +--- + Replace goal? + New objective: New goal + +› 1. Replace current goal Set the new objective and start it now + 2. Cancel Keep the current goal + + Press enter to confirm or esc to go back