From a36dd4bb966184350bb494b5e96e072a50cd8224 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Wed, 10 Jun 2026 12:48:42 -0700 Subject: [PATCH] Fix remote goal file paths --- codex-rs/app-server-client/src/lib.rs | 37 +++-- codex-rs/app-server-client/src/remote.rs | 111 +++++++++----- codex-rs/tui/src/app_server_session.rs | 96 +++++++++++- .../tui/src/chatwidget/tests/goal_menu.rs | 6 +- codex-rs/tui/src/goal_display.rs | 6 +- codex-rs/tui/src/goal_files.rs | 141 ++++++++++++++---- codex-rs/tui/src/goal_files_tests.rs | 93 +++++++++--- 7 files changed, 390 insertions(+), 100 deletions(-) diff --git a/codex-rs/app-server-client/src/lib.rs b/codex-rs/app-server-client/src/lib.rs index eba983dd9d..e755a9ac64 100644 --- a/codex-rs/app-server-client/src/lib.rs +++ b/codex-rs/app-server-client/src/lib.rs @@ -38,6 +38,7 @@ use codex_app_server_protocol::ConfigWarningNotification; use codex_app_server_protocol::InitializeCapabilities; use codex_app_server_protocol::InitializeParams; use codex_app_server_protocol::JSONRPCErrorError; +use codex_app_server_protocol::JSONRPCRequest; use codex_app_server_protocol::RequestId; use codex_app_server_protocol::Result as JsonRpcResult; use codex_app_server_protocol::ServerNotification; @@ -53,7 +54,6 @@ 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; @@ -847,6 +847,16 @@ impl AppServerClient { } } + pub async fn request_json_rpc(&self, request: JSONRPCRequest) -> IoResult { + match self { + Self::InProcess(_) => Err(IoError::new( + ErrorKind::InvalidInput, + "raw JSON-RPC requests are only supported by the remote app-server client", + )), + Self::Remote(client) => client.request_json_rpc(request).await, + } + } + pub async fn request_typed(&self, request: ClientRequest) -> Result where T: DeserializeOwned, @@ -900,13 +910,20 @@ impl AppServerClient { } } - pub fn remote_codex_home(&self) -> Option<&AbsolutePathBuf> { + pub fn remote_codex_home(&self) -> Option<&str> { match self { Self::InProcess(_) => None, Self::Remote(client) => client.codex_home(), } } + pub fn remote_platform_family(&self) -> Option<&str> { + match self { + Self::InProcess(_) => None, + Self::Remote(client) => client.platform_family(), + } + } + pub fn request_handle(&self) -> AppServerRequestHandle { match self { Self::InProcess(client) => AppServerRequestHandle::InProcess(client.request_handle()), @@ -1102,6 +1119,8 @@ mod tests { where S: tokio::io::AsyncRead + tokio::io::AsyncWrite + Unpin, { + const TEST_REMOTE_CODEX_HOME: &str = r"C:\Users\codex\.codex"; + let JSONRPCMessage::Request(request) = read_websocket_message(websocket).await else { panic!("expected initialize request"); }; @@ -1112,9 +1131,9 @@ mod tests { id: request.id, result: serde_json::json!({ "userAgent": "codex_cli_rs/9.8.7-test (Test OS; x86_64) rust", - "codexHome": test_remote_codex_home().display().to_string(), - "platformFamily": "unix", - "platformOs": "linux", + "codexHome": TEST_REMOTE_CODEX_HOME, + "platformFamily": "windows", + "platformOs": "windows", }), }), ) @@ -1127,11 +1146,6 @@ mod tests { assert_eq!(notification.method, "initialized"); } - fn test_remote_codex_home() -> AbsolutePathBuf { - AbsolutePathBuf::from_absolute_path(std::env::temp_dir().join("codex-remote-home")) - .expect("test remote codex home should be absolute") - } - async fn read_websocket_message( websocket: &mut tokio_tungstenite::WebSocketStream, ) -> JSONRPCMessage @@ -1456,7 +1470,8 @@ mod tests { .expect("remote client should connect"); assert_eq!(client.server_version(), Some("9.8.7-test")); - assert_eq!(client.codex_home(), Some(&test_remote_codex_home())); + assert_eq!(client.codex_home(), Some(r"C:\Users\codex\.codex")); + assert_eq!(client.platform_family(), Some("windows")); let response: GetAccountResponse = client .request_typed(ClientRequest::GetAccount { request_id: RequestId::Integer(1), diff --git a/codex-rs/app-server-client/src/remote.rs b/codex-rs/app-server-client/src/remote.rs index b4bed4c217..b94cf47379 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,14 +151,16 @@ pub struct RemoteAppServerClient { event_rx: mpsc::UnboundedReceiver, pending_events: VecDeque, server_version: Option, - codex_home: Option, + codex_home: Option, + platform_family: Option, worker_handle: tokio::task::JoinHandle<()>, } #[derive(Debug, Default)] struct RemoteInitializeInfo { server_version: Option, - codex_home: Option, + codex_home: Option, + platform_family: Option, } #[derive(Clone)] @@ -192,8 +194,12 @@ impl RemoteAppServerClient { self.server_version.as_deref() } - pub fn codex_home(&self) -> Option<&AbsolutePathBuf> { - self.codex_home.as_ref() + pub fn codex_home(&self) -> Option<&str> { + self.codex_home.as_deref() + } + + pub fn platform_family(&self) -> Option<&str> { + self.platform_family.as_deref() } async fn connect_with_stream( @@ -229,36 +235,17 @@ impl RemoteAppServerClient { }; match command { RemoteClientCommand::Request { request, response_tx } => { - let request_id = request_id_from_client_request(&request); - if pending_requests.contains_key(&request_id) { - let _ = response_tx.send(Err(IoError::new( - ErrorKind::InvalidInput, - format!("duplicate remote app-server request id `{request_id}`"), - ))); - continue; - } - pending_requests.insert(request_id.clone(), response_tx); - if let Err(err) = write_jsonrpc_message( + if !write_remote_request( &mut stream, - JSONRPCMessage::Request(jsonrpc_request_from_client_request(*request)), &endpoint, + &mut pending_requests, + *request, + response_tx, + &event_tx, + &mut worker_exit_error, ) .await { - let err_message = err.to_string(); - let message = format!( - "remote app server at `{endpoint}` write failed: {err_message}" - ); - if let Some(response_tx) = pending_requests.remove(&request_id) { - let _ = response_tx.send(Err(err)); - } - let _ = deliver_event( - &event_tx, - AppServerEvent::Disconnected { - message: message.clone(), - }, - ); - worker_exit_error = Some((ErrorKind::BrokenPipe, message)); break; } } @@ -484,6 +471,7 @@ impl RemoteAppServerClient { pending_events: pending_events.into(), server_version: initialize_info.server_version, codex_home: initialize_info.codex_home, + platform_family: initialize_info.platform_family, worker_handle, }) } @@ -495,6 +483,11 @@ impl RemoteAppServerClient { } 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 { @@ -626,6 +619,7 @@ impl RemoteAppServerClient { pending_events: _pending_events, server_version: _server_version, codex_home: _codex_home, + platform_family: _platform_family, worker_handle, } = self; let mut worker_handle = worker_handle; @@ -650,6 +644,11 @@ impl RemoteAppServerClient { impl RemoteAppServerRequestHandle { pub async fn request(&self, request: ClientRequest) -> IoResult { + self.request_json_rpc(jsonrpc_request_from_client_request(request)) + .await + } + + async fn request_json_rpc(&self, request: JSONRPCRequest) -> IoResult { let (response_tx, response_rx) = oneshot::channel(); self.command_tx .send(RemoteClientCommand::Request { @@ -854,8 +853,14 @@ where initialize_info.codex_home = response .result .get("codexHome") - .cloned() - .and_then(|value| serde_json::from_value(value).ok()); + .and_then(serde_json::Value::as_str) + .filter(|codex_home| !codex_home.is_empty()) + .map(str::to_string); + initialize_info.platform_family = response + .result + .get("platformFamily") + .and_then(serde_json::Value::as_str) + .map(str::to_string); break Ok(()); } JSONRPCMessage::Error(error) if error.id == initialize_request_id => { @@ -969,8 +974,45 @@ fn deliver_event( }) } -fn request_id_from_client_request(request: &ClientRequest) -> RequestId { - jsonrpc_request_from_client_request(request.clone()).id +async fn write_remote_request( + stream: &mut WebSocketStream, + endpoint: &str, + pending_requests: &mut HashMap>>, + request: JSONRPCRequest, + response_tx: oneshot::Sender>, + event_tx: &mpsc::UnboundedSender, + worker_exit_error: &mut Option<(ErrorKind, String)>, +) -> bool +where + S: AsyncRead + AsyncWrite + Unpin, +{ + let request_id = request.id.clone(); + if pending_requests.contains_key(&request_id) { + let _ = response_tx.send(Err(IoError::new( + ErrorKind::InvalidInput, + format!("duplicate remote app-server request id `{request_id}`"), + ))); + return true; + } + pending_requests.insert(request_id.clone(), response_tx); + if let Err(err) = + write_jsonrpc_message(stream, JSONRPCMessage::Request(request), endpoint).await + { + let err_message = err.to_string(); + let message = format!("remote app server at `{endpoint}` write failed: {err_message}"); + if let Some(response_tx) = pending_requests.remove(&request_id) { + let _ = response_tx.send(Err(err)); + } + let _ = deliver_event( + event_tx, + AppServerEvent::Disconnected { + message: message.clone(), + }, + ); + *worker_exit_error = Some((ErrorKind::BrokenPipe, message)); + return false; + } + true } fn jsonrpc_request_from_client_request(request: ClientRequest) -> JSONRPCRequest { @@ -1043,6 +1085,7 @@ mod tests { pending_events: VecDeque::new(), server_version: None, codex_home: None, + platform_family: None, worker_handle, }; diff --git a/codex-rs/tui/src/app_server_session.rs b/codex-rs/tui/src/app_server_session.rs index 3cb83ba4d0..cf56e924ba 100644 --- a/codex-rs/tui/src/app_server_session.rs +++ b/codex-rs/tui/src/app_server_session.rs @@ -4,6 +4,7 @@ //! request/response plumbing out of `App` and `ChatWidget`. use crate::bottom_pane::FeedbackAudience; +use crate::goal_files::GoalFilePath; use crate::legacy_core::config::Config; use crate::permission_compat::legacy_compatible_permission_profile; use crate::service_tier_resolution; @@ -34,6 +35,7 @@ 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; @@ -129,6 +131,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::time::Duration; @@ -149,6 +153,11 @@ fn is_thread_settings_update_unsupported(source: &JSONRPCErrorError) -> bool { && source.message.contains(THREAD_SETTINGS_UPDATE_METHOD)) } +fn local_goal_file_path(path: &GoalFilePath) -> Result { + AbsolutePathBuf::from_absolute_path_checked(path.as_str()) + .wrap_err_with(|| format!("invalid local goal file path {}", path.display())) +} + /// Data collected during the TUI bootstrap phase that the main event loop /// needs to configure the UI, telemetry, and initial rate-limit prefetch. /// @@ -241,10 +250,13 @@ impl AppServerSession { pub(crate) fn goal_files_codex_home( &self, local_codex_home: &AbsolutePathBuf, - ) -> Option { + ) -> Option { match self.thread_params_mode { - ThreadParamsMode::Embedded => Some(local_codex_home.clone()), - ThreadParamsMode::Remote => self.client.remote_codex_home().cloned(), + ThreadParamsMode::Embedded => Some(GoalFilePath::from_local(local_codex_home)), + ThreadParamsMode::Remote => self + .client + .remote_codex_home() + .map(|path| GoalFilePath::from_remote(path, self.client.remote_platform_family())), } } @@ -890,6 +902,19 @@ impl AppServerSession { .wrap_err("fs/readFile returned invalid base64 data") } + pub(crate) async fn fs_read_file_for_goal(&mut self, path: GoalFilePath) -> Result> { + if !self.uses_remote_workspace() { + return self.fs_read_file(local_goal_file_path(&path)?).await; + } + + let response: FsReadFileResponse = self + .request_json_rpc_typed("fs/readFile", json!({ "path": path.as_str() })) + .await?; + STANDARD + .decode(response.data_base64) + .wrap_err("fs/readFile returned invalid base64 data") + } + pub(crate) async fn fs_write_file( &mut self, path: AbsolutePathBuf, @@ -910,6 +935,29 @@ impl AppServerSession { Ok(()) } + pub(crate) async fn fs_write_file_for_goal( + &mut self, + path: GoalFilePath, + bytes: Vec, + ) -> Result<()> { + if !self.uses_remote_workspace() { + return self + .fs_write_file(local_goal_file_path(&path)?, bytes) + .await; + } + + let _: FsWriteFileResponse = self + .request_json_rpc_typed( + "fs/writeFile", + json!({ + "path": path.as_str(), + "dataBase64": STANDARD.encode(bytes), + }), + ) + .await?; + Ok(()) + } + pub(crate) async fn fs_create_directory(&mut self, path: AbsolutePathBuf) -> Result<()> { let request_id = self.next_request_id(); let _: FsCreateDirectoryResponse = self @@ -926,6 +974,48 @@ impl AppServerSession { Ok(()) } + pub(crate) async fn fs_create_directory_for_goal(&mut self, path: GoalFilePath) -> Result<()> { + if !self.uses_remote_workspace() { + return self.fs_create_directory(local_goal_file_path(&path)?).await; + } + + let _: FsCreateDirectoryResponse = self + .request_json_rpc_typed( + "fs/createDirectory", + json!({ + "path": path.as_str(), + "recursive": true, + }), + ) + .await?; + Ok(()) + } + + async fn request_json_rpc_typed( + &mut self, + method: &str, + params: serde_json::Value, + ) -> Result + where + T: DeserializeOwned, + { + let request_id = self.next_request_id(); + let response = self + .client + .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/chatwidget/tests/goal_menu.rs b/codex-rs/tui/src/chatwidget/tests/goal_menu.rs index cbac5a3b23..c955725603 100644 --- a/codex-rs/tui/src/chatwidget/tests/goal_menu.rs +++ b/codex-rs/tui/src/chatwidget/tests/goal_menu.rs @@ -84,8 +84,10 @@ async fn goal_menu_managed_file_snapshot() { .join("attachments") .join("00000000-0000-4000-8000-000000000000") .join("goal-objective.md"); - goal.objective = - crate::goal_files::objective_file_reference(&path).expect("goal objective file reference"); + goal.objective = crate::goal_files::objective_file_reference( + &crate::goal_files::GoalFilePath::from_local(&path), + ) + .expect("goal objective file reference"); chat.show_goal_summary(goal); diff --git a/codex-rs/tui/src/goal_display.rs b/codex-rs/tui/src/goal_display.rs index 0927f318b8..f0b39acce6 100644 --- a/codex-rs/tui/src/goal_display.rs +++ b/codex-rs/tui/src/goal_display.rs @@ -125,8 +125,10 @@ mod tests { .join("goal-objective.md"), ) .expect("absolute path"); - let objective = - crate::goal_files::objective_file_reference(&path).expect("goal file reference"); + let objective = crate::goal_files::objective_file_reference( + &crate::goal_files::GoalFilePath::from_local(&path), + ) + .expect("goal file reference"); let mut goal = test_thread_goal(/*token_budget*/ None, /*tokens_used*/ 0); goal.objective = objective; diff --git a/codex-rs/tui/src/goal_files.rs b/codex-rs/tui/src/goal_files.rs index 60e71770c3..e8dd8ee9aa 100644 --- a/codex-rs/tui/src/goal_files.rs +++ b/codex-rs/tui/src/goal_files.rs @@ -39,36 +39,120 @@ pub(crate) struct GoalDraft { pub(crate) trait GoalFileStore { fn create_directory( &mut self, - path: AbsolutePathBuf, + path: GoalFilePath, ) -> impl std::future::Future> + Send; fn write_file( &mut self, - path: AbsolutePathBuf, + path: GoalFilePath, bytes: Vec, ) -> impl std::future::Future> + Send; fn read_file( &mut self, - path: AbsolutePathBuf, + path: GoalFilePath, ) -> impl std::future::Future>> + Send; } +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct GoalFilePath { + raw: String, + separator: char, +} + +impl GoalFilePath { + pub(crate) fn from_local(path: &AbsolutePathBuf) -> Self { + Self { + raw: path.display().to_string(), + separator: std::path::MAIN_SEPARATOR, + } + } + + pub(crate) fn from_remote(path: &str, platform_family: Option<&str>) -> Self { + Self { + raw: path.to_string(), + separator: if platform_family == Some("windows") || is_windows_absolute_path(path) { + '\\' + } else { + '/' + }, + } + } + + pub(crate) fn as_str(&self) -> &str { + &self.raw + } + + pub(crate) fn display(&self) -> &str { + &self.raw + } + + fn join(&self, segment: impl AsRef) -> Self { + let segment = segment.as_ref(); + let trimmed = self.raw.trim_end_matches(['/', '\\']); + let mut raw = if trimmed.is_empty() { + self.separator.to_string() + } else { + trimmed.to_string() + }; + if !raw.ends_with(self.separator) { + raw.push(self.separator); + } + raw.push_str(segment); + Self { + raw, + separator: self.separator, + } + } + + fn from_managed_reference(raw: &str) -> Option { + let separator = if is_windows_absolute_path(raw) { + '\\' + } else if raw.starts_with('/') { + '/' + } else { + return None; + }; + let normalized = raw.replace('\\', "/"); + let parts = normalized + .split('/') + .filter(|part| !part.is_empty()) + .collect::>(); + if parts.len() < 3 { + return None; + } + let file_name = parts.last()?; + let attachment_id = parts.get(parts.len() - 2)?; + let attachment_dir = parts.get(parts.len() - 3)?; + if *file_name == GOAL_FILE_NAME + && *attachment_dir == GOAL_ATTACHMENT_DIR + && Uuid::parse_str(attachment_id).is_ok() + { + Some(Self { + raw: raw.to_string(), + separator, + }) + } else { + None + } + } +} + impl GoalFileStore for AppServerSession { - async fn create_directory(&mut self, path: AbsolutePathBuf) -> Result<()> { - self.fs_create_directory(path) + async fn create_directory(&mut self, path: GoalFilePath) -> Result<()> { + self.fs_create_directory_for_goal(path) .await .map_err(|err| anyhow::anyhow!("{err}")) } - async fn write_file(&mut self, path: AbsolutePathBuf, bytes: Vec) -> Result<()> { - self.fs_write_file(path, bytes) + async fn write_file(&mut self, path: GoalFilePath, bytes: Vec) -> Result<()> { + self.fs_write_file_for_goal(path, bytes) .await .map_err(|err| anyhow::anyhow!("{err}")) } - async fn read_file(&mut self, path: AbsolutePathBuf) -> Result> { - self.fs_read_file(path) + async fn read_file(&mut self, path: GoalFilePath) -> Result> { + self.fs_read_file_for_goal(path) .await .map_err(|err| anyhow::anyhow!("{err}")) } @@ -76,7 +160,7 @@ impl GoalFileStore for AppServerSession { pub(crate) async fn materialize_goal_draft( store: &mut impl GoalFileStore, - codex_home: Option<&AbsolutePathBuf>, + codex_home: Option<&GoalFilePath>, draft: GoalDraft, ) -> Result { let mut objective = draft.objective; @@ -172,33 +256,22 @@ pub(crate) async fn objective_text_for_edit( .with_context(|| format!("Goal objective file {} is not valid UTF-8", path.display())) } -pub(crate) fn objective_file_path(objective: &str) -> Option { +pub(crate) fn objective_file_path(objective: &str) -> Option { let mut lines = objective.lines(); let path = lines .next()? .strip_prefix(GOAL_FILE_PREFIX) .map(str::trim) .filter(|path| !path.is_empty()) - .and_then(|path| AbsolutePathBuf::from_absolute_path_checked(path).ok())?; + .and_then(GoalFilePath::from_managed_reference)?; if lines.next() != Some(GOAL_FILE_INSTRUCTION) { return None; } - let parent = path.parent()?; - let attachment_id = parent.file_name()?.to_str()?; - let attachment_parent = parent.parent()?; - let attachment_dir = attachment_parent.file_name()?.to_str()?; - if path.file_name()?.to_str()? == GOAL_FILE_NAME - && attachment_dir == GOAL_ATTACHMENT_DIR - && Uuid::parse_str(attachment_id).is_ok() - { - Some(path) - } else { - None - } + Some(path) } -pub(crate) fn objective_file_reference(path: &Path) -> Result { +pub(crate) fn objective_file_reference(path: &GoalFilePath) -> Result { let reference = format!( "{GOAL_FILE_PREFIX}{}\n{GOAL_FILE_INSTRUCTION}", path.display() @@ -214,9 +287,9 @@ pub(crate) fn objective_file_reference(path: &Path) -> Result { async fn ensure_output_dir( store: &mut impl GoalFileStore, - codex_home: Option<&AbsolutePathBuf>, - output_dir: &mut Option, -) -> Result { + codex_home: Option<&GoalFilePath>, + output_dir: &mut Option, +) -> Result { if let Some(output_dir) = output_dir { return Ok(output_dir.clone()); } @@ -240,7 +313,7 @@ async fn ensure_output_dir( async fn write_file( store: &mut impl GoalFileStore, - path: AbsolutePathBuf, + path: GoalFilePath, bytes: Vec, ) -> Result<()> { store @@ -249,6 +322,16 @@ async fn write_file( .with_context(|| format!("Could not write goal file {}", path.display())) } +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("//") +} + fn append_section(objective: &mut String, heading: &str, lines: Vec) { if lines.is_empty() { return; diff --git a/codex-rs/tui/src/goal_files_tests.rs b/codex-rs/tui/src/goal_files_tests.rs index abeb1c7e68..3f08c008e5 100644 --- a/codex-rs/tui/src/goal_files_tests.rs +++ b/codex-rs/tui/src/goal_files_tests.rs @@ -3,30 +3,56 @@ use super::*; use codex_protocol::protocol::MAX_THREAD_GOAL_OBJECTIVE_CHARS; use codex_protocol::user_input::TextElement; use pretty_assertions::assert_eq; +use std::path::Path; struct LocalStore; impl GoalFileStore for LocalStore { - async fn create_directory(&mut self, path: AbsolutePathBuf) -> Result<()> { - fs::create_dir_all(path.as_path())?; + async fn create_directory(&mut self, path: GoalFilePath) -> Result<()> { + fs::create_dir_all(path.as_str())?; Ok(()) } - async fn write_file(&mut self, path: AbsolutePathBuf, bytes: Vec) -> Result<()> { - fs::write(path.as_path(), bytes)?; + async fn write_file(&mut self, path: GoalFilePath, bytes: Vec) -> Result<()> { + fs::write(path.as_str(), bytes)?; Ok(()) } - async fn read_file(&mut self, path: AbsolutePathBuf) -> Result> { - Ok(fs::read(path.as_path())?) + async fn read_file(&mut self, path: GoalFilePath) -> Result> { + Ok(fs::read(path.as_str())?) + } +} + +#[derive(Default)] +struct RecordingStore { + created_dirs: Vec, + writes: Vec<(String, Vec)>, +} + +impl GoalFileStore for RecordingStore { + async fn create_directory(&mut self, path: GoalFilePath) -> Result<()> { + self.created_dirs.push(path.as_str().to_string()); + Ok(()) + } + + async fn write_file(&mut self, path: GoalFilePath, bytes: Vec) -> Result<()> { + self.writes.push((path.as_str().to_string(), bytes)); + Ok(()) + } + + async fn read_file(&mut self, path: GoalFilePath) -> Result> { + self.writes + .iter() + .find(|(write_path, _)| write_path == path.as_str()) + .map(|(_, bytes)| bytes.clone()) + .ok_or_else(|| anyhow::anyhow!("missing recording for {}", path.display())) } } #[tokio::test] async fn materializes_and_reads_oversized_objective_through_store() { let temp_dir = tempfile::tempdir().expect("tempdir"); - let codex_home = - AbsolutePathBuf::from_absolute_path_checked(temp_dir.path()).expect("absolute codex home"); + let codex_home = local_goal_home(temp_dir.path()); let objective = "x".repeat(MAX_THREAD_GOAL_OBJECTIVE_CHARS + 1); let mut store = LocalStore; @@ -43,7 +69,7 @@ async fn materializes_and_reads_oversized_objective_through_store() { let path = objective_file_path(&reference).expect("goal file path"); assert_eq!( - fs::read_to_string(path.as_path()).expect("read file"), + fs::read_to_string(path.as_str()).expect("read file"), objective ); let edit_text = objective_text_for_edit(&mut store, &reference) @@ -55,8 +81,7 @@ async fn materializes_and_reads_oversized_objective_through_store() { #[tokio::test] async fn materializes_paste_and_image_through_store() { let temp_dir = tempfile::tempdir().expect("tempdir"); - let codex_home = - AbsolutePathBuf::from_absolute_path_checked(temp_dir.path()).expect("absolute codex home"); + let codex_home = local_goal_home(temp_dir.path()); let image_path = temp_dir.path().join("local-image.png"); fs::write(&image_path, b"png bytes").expect("write image"); let mut store = LocalStore; @@ -98,17 +123,42 @@ async fn materializes_paste_and_image_through_store() { let paste_path = path_after(&objective, "pasted text file: "); let image_path = path_after(&objective, "image file: "); + assert_eq!(fs::read_to_string(paste_path).expect("read paste"), "hello"); + assert_eq!(fs::read(image_path).expect("read image"), b"png bytes"); +} + +#[tokio::test] +async fn materializes_oversized_objective_with_windows_remote_path() { + let objective = "x".repeat(MAX_THREAD_GOAL_OBJECTIVE_CHARS + 1); + let codex_home = GoalFilePath::from_remote(r"C:\Users\codex\.codex", Some("windows")); + let mut store = RecordingStore::default(); + + let reference = materialize_goal_draft( + &mut store, + Some(&codex_home), + GoalDraft { + objective: objective.clone(), + ..Default::default() + }, + ) + .await + .expect("materialize goal draft"); + + assert_eq!(store.created_dirs.len(), 1); + assert_eq!(store.writes.len(), 1); + let (path, bytes) = &store.writes[0]; + assert!(path.starts_with(r"C:\Users\codex\.codex\attachments\")); + assert!(path.ends_with(r"\goal-objective.md")); + assert_eq!(bytes, objective.as_bytes()); assert_eq!( - fs::read_to_string(paste_path.as_path()).expect("read paste"), - "hello" - ); - assert_eq!( - fs::read(image_path.as_path()).expect("read image"), - b"png bytes" + objective_file_path(&reference) + .expect("goal file path") + .as_str(), + path ); } -fn path_after(text: &str, prefix: &str) -> AbsolutePathBuf { +fn path_after(text: &str, prefix: &str) -> String { let path = text .split_once(prefix) .unwrap_or_else(|| panic!("expected {prefix:?} in {text:?}")) @@ -116,7 +166,12 @@ fn path_after(text: &str, prefix: &str) -> AbsolutePathBuf { .split_whitespace() .next() .expect("path"); - AbsolutePathBuf::from_absolute_path_checked(path).expect("absolute path") + path.to_string() +} + +fn local_goal_home(path: &Path) -> GoalFilePath { + let path = AbsolutePathBuf::from_absolute_path_checked(path).expect("absolute codex home"); + GoalFilePath::from_local(&path) } #[tokio::test]