diff --git a/codex-rs/tui/src/app/event_dispatch.rs b/codex-rs/tui/src/app/event_dispatch.rs index 548aa54427..a4a586944d 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -697,12 +697,12 @@ impl App { AppEvent::OpenThreadGoalEditor { thread_id } => { self.open_thread_goal_editor(app_server, thread_id).await; } - AppEvent::SetThreadGoalObjective { + AppEvent::SetThreadGoalDraft { thread_id, - objective, + draft, mode, } => { - self.set_thread_goal_objective(app_server, thread_id, objective, mode) + self.set_thread_goal_draft(app_server, thread_id, draft, mode) .await; } AppEvent::SetThreadGoalStatus { thread_id, status } => { diff --git a/codex-rs/tui/src/app/thread_goal_actions.rs b/codex-rs/tui/src/app/thread_goal_actions.rs index 6132ae3829..6a0c1eefd6 100644 --- a/codex-rs/tui/src/app/thread_goal_actions.rs +++ b/codex-rs/tui/src/app/thread_goal_actions.rs @@ -109,7 +109,42 @@ impl App { return; }; - self.chat_widget.show_goal_edit_prompt(thread_id, goal); + let objective = match goal_files::objective_text_for_edit(app_server, &goal.objective).await + { + Ok(objective) => objective, + Err(err) => { + self.chat_widget.add_error_message(err.to_string()); + goal.objective.clone() + } + }; + if self.current_displayed_thread_id() != Some(thread_id) { + return; + } + self.chat_widget + .show_goal_edit_prompt(thread_id, goal, objective); + } + + pub(super) async fn set_thread_goal_draft( + &mut self, + app_server: &mut AppServerSession, + thread_id: ThreadId, + draft: goal_files::GoalDraft, + mode: ThreadGoalSetMode, + ) { + let result = + goal_files::materialize_goal_draft(app_server, &self.config.codex_home, draft).await; + let objective = match result { + Ok(objective) => objective, + Err(err) => { + if self.current_displayed_thread_id() == Some(thread_id) { + self.chat_widget.add_error_message(err.to_string()); + } + return; + } + }; + + self.set_thread_goal_objective(app_server, thread_id, objective, mode) + .await; } pub(super) async fn set_thread_goal_objective( @@ -253,9 +288,12 @@ impl App { format!("New objective: {objective}") }; let replace_actions: Vec = vec![Box::new(move |tx| { - tx.send(AppEvent::SetThreadGoalObjective { + tx.send(AppEvent::SetThreadGoalDraft { thread_id, - objective: replace_objective.clone(), + draft: goal_files::GoalDraft { + objective: replace_objective.clone(), + ..Default::default() + }, mode: ThreadGoalSetMode::ReplaceExisting, }); })]; diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index c2f9ee3f31..70dda8cdb7 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -38,6 +38,7 @@ use crate::bottom_pane::ApprovalRequest; use crate::bottom_pane::StatusLineItem; use crate::bottom_pane::TerminalTitleItem; use crate::chatwidget::UserMessage; +use crate::goal_files::GoalDraft; use codex_app_server_protocol::AskForApproval; use codex_config::types::ApprovalsReviewer; use codex_features::Feature; @@ -271,10 +272,10 @@ pub(crate) enum AppEvent { thread_id: Option, }, - /// Set or replace the current thread goal objective. - SetThreadGoalObjective { + /// Materialize and set or replace the current thread goal objective. + SetThreadGoalDraft { thread_id: ThreadId, - objective: String, + draft: GoalDraft, mode: ThreadGoalSetMode, }, diff --git a/codex-rs/tui/src/app_server_session.rs b/codex-rs/tui/src/app_server_session.rs index e29b40111c..7d9cc8816d 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,6 +29,12 @@ 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; @@ -887,6 +895,57 @@ impl AppServerSession { .wrap_err("thread/goal/clear failed in TUI") } + pub(crate) async fn fs_read_file(&mut self, path: AbsolutePathBuf) -> Result> { + let request_id = self.next_request_id(); + let response: FsReadFileResponse = self + .client + .request_typed(ClientRequest::FsReadFile { + request_id, + params: FsReadFileParams { path }, + }) + .await + .wrap_err("fs/readFile failed in TUI")?; + STANDARD + .decode(response.data_base64) + .wrap_err("fs/readFile returned invalid base64 data") + } + + pub(crate) async fn fs_write_file( + &mut self, + path: AbsolutePathBuf, + bytes: Vec, + ) -> Result<()> { + let request_id = self.next_request_id(); + let _: FsWriteFileResponse = self + .client + .request_typed(ClientRequest::FsWriteFile { + request_id, + params: FsWriteFileParams { + path, + data_base64: STANDARD.encode(bytes), + }, + }) + .await + .wrap_err("fs/writeFile failed in TUI")?; + Ok(()) + } + + pub(crate) async fn fs_create_directory(&mut self, path: AbsolutePathBuf) -> Result<()> { + let request_id = self.next_request_id(); + let _: FsCreateDirectoryResponse = self + .client + .request_typed(ClientRequest::FsCreateDirectory { + request_id, + params: FsCreateDirectoryParams { + path, + recursive: Some(true), + }, + }) + .await + .wrap_err("fs/createDirectory failed in TUI")?; + Ok(()) + } + 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/goal_menu.rs b/codex-rs/tui/src/chatwidget/goal_menu.rs index 0fae3b75ba..102db1b705 100644 --- a/codex-rs/tui/src/chatwidget/goal_menu.rs +++ b/codex-rs/tui/src/chatwidget/goal_menu.rs @@ -10,42 +10,27 @@ impl ChatWidget { self.add_plain_history_lines(goal_summary_lines(&goal)); } - pub(crate) fn show_goal_edit_prompt(&mut self, thread_id: ThreadId, goal: AppThreadGoal) { + pub(crate) fn show_goal_edit_prompt( + &mut self, + thread_id: ThreadId, + goal: AppThreadGoal, + objective: String, + ) { let tx = self.app_event_tx.clone(); let status = edited_goal_status(goal.status); let token_budget = goal.token_budget; - let objective = match goal_files::objective_text_for_edit(&goal.objective) { - Ok(objective) => objective, - Err(err) => { - self.add_error_message(err.to_string()); - goal.objective - } - }; - let codex_home = self.config.codex_home.clone(); let view = CustomPromptView::new( "Edit goal".to_string(), "Type a goal objective and press Enter".to_string(), objective, /*context_label*/ None, Box::new(move |objective: String| { - let objective = match goal_files::materialize_goal_draft( - codex_home.as_path(), - goal_files::GoalDraft { + tx.send(AppEvent::SetThreadGoalDraft { + thread_id, + draft: goal_files::GoalDraft { objective, ..Default::default() }, - ) { - Ok(objective) => objective, - Err(err) => { - tx.send(AppEvent::InsertHistoryCell(Box::new( - crate::history_cell::new_error_event(err.to_string()), - ))); - return; - } - }; - tx.send(AppEvent::SetThreadGoalObjective { - thread_id, - objective, mode: crate::app_event::ThreadGoalSetMode::UpdateExisting { status, token_budget, diff --git a/codex-rs/tui/src/chatwidget/slash_dispatch.rs b/codex-rs/tui/src/chatwidget/slash_dispatch.rs index 6ed7cc151f..eb680f826e 100644 --- a/codex-rs/tui/src/chatwidget/slash_dispatch.rs +++ b/codex-rs/tui/src/chatwidget/slash_dispatch.rs @@ -7,6 +7,7 @@ use super::*; use crate::app_event::ThreadGoalSetMode; +use crate::bottom_pane::ChatComposer; use crate::bottom_pane::prompt_args::parse_slash_name; use crate::bottom_pane::slash_commands::BuiltinCommandFlags; use crate::bottom_pane::slash_commands::ServiceTierCommand; @@ -756,34 +757,36 @@ impl ChatWidget { } else { Vec::new() }; - let objective = match crate::goal_files::materialize_goal_draft( - self.config.codex_home.as_path(), - GoalDraft { - objective: args, - text_elements, - pending_pastes, - local_images, - remote_image_urls, - }, - ) { - Ok(objective) => objective, - Err(err) => { - self.add_error_message(err.to_string()); - return if source == SlashCommandDispatchSource::Queued { - QueueDrain::Continue - } else { - self.queued_command_drain_result(cmd) - }; - } + let draft = GoalDraft { + objective: args, + text_elements, + pending_pastes, + local_images, + remote_image_urls, }; let Some(thread_id) = self.thread_id else { if source == SlashCommandDispatchSource::Live { + const GOAL_PREFIX: &str = "/goal "; + let (objective, text_elements) = ChatComposer::expand_pending_pastes( + &draft.objective, + draft.text_elements, + &draft.pending_pastes, + ); + let text_elements = text_elements + .into_iter() + .map(|element| { + element.map_range(|range| ByteRange { + start: range.start + GOAL_PREFIX.len(), + end: range.end + GOAL_PREFIX.len(), + }) + }) + .collect(); self.queue_user_message_with_options( UserMessage { - text: format!("/goal {objective}"), - local_images: Vec::new(), - remote_image_urls: Vec::new(), - text_elements: Vec::new(), + text: format!("{GOAL_PREFIX}{objective}"), + local_images: draft.local_images, + remote_image_urls: draft.remote_image_urls, + text_elements, mention_bindings: Vec::new(), }, QueuedInputAction::ParseSlash, @@ -797,12 +800,13 @@ impl ChatWidget { } return self.queued_command_drain_result(cmd); }; - self.app_event_tx.send(AppEvent::SetThreadGoalObjective { + let history_objective = draft.objective.clone(); + self.app_event_tx.send(AppEvent::SetThreadGoalDraft { thread_id, - objective: objective.clone(), + draft, mode: ThreadGoalSetMode::ConfirmIfExists, }); - self.append_message_history_entry(format!("/goal {objective}")); + self.append_message_history_entry(format!("/goal {history_objective}")); if source == SlashCommandDispatchSource::Live { self.clear_live_goal_submission(); } diff --git a/codex-rs/tui/src/chatwidget/tests/goal_menu.rs b/codex-rs/tui/src/chatwidget/tests/goal_menu.rs index 376d193bf7..cbac5a3b23 100644 --- a/codex-rs/tui/src/chatwidget/tests/goal_menu.rs +++ b/codex-rs/tui/src/chatwidget/tests/goal_menu.rs @@ -1,6 +1,4 @@ use super::*; -use codex_protocol::protocol::MAX_THREAD_GOAL_OBJECTIVE_CHARS; - #[tokio::test] async fn goal_menu_active_snapshot() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; @@ -80,15 +78,14 @@ async fn goal_menu_managed_file_snapshot() { AppThreadGoalStatus::Active, /*token_budget*/ Some(80_000), ); - goal.objective = crate::goal_files::materialize_goal_draft( - chat.config.codex_home.as_path(), - crate::goal_files::GoalDraft { - objective: "x".repeat(MAX_THREAD_GOAL_OBJECTIVE_CHARS + 1), - ..Default::default() - }, - ) - .expect("materialize goal objective"); - let path = crate::goal_files::objective_file_path(&goal.objective).expect("goal file path"); + let path = chat + .config + .codex_home + .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"); chat.show_goal_summary(goal); @@ -127,6 +124,7 @@ async fn goal_edit_prompt_snapshot() { AppThreadGoalStatus::Active, /*token_budget*/ Some(80_000), ), + "Keep improving the bare goal command until it feels calm and useful.".to_string(), ); assert_chatwidget_snapshot!( @@ -147,14 +145,15 @@ async fn goal_edit_prompt_submits_preserved_status_and_budget() { AppThreadGoalStatus::Paused, /*token_budget*/ Some(80_000), ), + "Keep improving the bare goal command until it feels calm and useful.".to_string(), ); chat.handle_paste(" with clearer wording".to_string()); chat.handle_key_event(KeyEvent::from(KeyCode::Enter)); match rx.try_recv() { - Ok(AppEvent::SetThreadGoalObjective { + Ok(AppEvent::SetThreadGoalDraft { thread_id: event_thread_id, - objective, + draft, mode: crate::app_event::ThreadGoalSetMode::UpdateExisting { status, @@ -163,57 +162,17 @@ async fn goal_edit_prompt_submits_preserved_status_and_budget() { }) => { assert_eq!(event_thread_id, thread_id); assert_eq!( - objective, + draft.objective, "Keep improving the bare goal command until it feels calm and useful. with clearer wording" ); assert_eq!(status, AppThreadGoalStatus::Paused); assert_eq!(token_budget, Some(80_000)); } - other => panic!("expected SetThreadGoalObjective event, got {other:?}"), + other => panic!("expected SetThreadGoalDraft event, got {other:?}"), } assert!(chat.no_modal_or_popup_active()); } -#[tokio::test] -async fn goal_edit_prompt_hydrates_and_materializes_oversized_objective_file() { - let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; - let thread_id = ThreadId::new(); - let objective = "x".repeat(MAX_THREAD_GOAL_OBJECTIVE_CHARS + 1); - let mut goal = test_goal( - thread_id, - AppThreadGoalStatus::Paused, - /*token_budget*/ Some(80_000), - ); - goal.objective = crate::goal_files::materialize_goal_draft( - chat.config.codex_home.as_path(), - crate::goal_files::GoalDraft { - objective: objective.clone(), - ..Default::default() - }, - ) - .expect("materialize goal objective"); - - chat.show_goal_edit_prompt(thread_id, goal); - chat.handle_key_event(KeyEvent::from(KeyCode::Enter)); - - match rx.try_recv() { - Ok(AppEvent::SetThreadGoalObjective { - objective: actual_objective, - .. - }) => { - let path = - crate::goal_files::objective_file_path(&actual_objective).unwrap_or_else(|| { - panic!("expected goal file objective, got {actual_objective:?}") - }); - assert_eq!( - std::fs::read_to_string(path).expect("read goal file"), - objective - ); - } - other => panic!("expected SetThreadGoalObjective event, got {other:?}"), - } -} - #[tokio::test] async fn goal_edit_prompt_preserves_resumable_stopped_statuses() { for stopped_status in [ @@ -230,11 +189,12 @@ async fn goal_edit_prompt_preserves_resumable_stopped_statuses() { stopped_status, /*token_budget*/ Some(80_000), ), + "Keep improving the bare goal command until it feels calm and useful.".to_string(), ); chat.handle_key_event(KeyEvent::from(KeyCode::Enter)); match rx.try_recv() { - Ok(AppEvent::SetThreadGoalObjective { + Ok(AppEvent::SetThreadGoalDraft { mode: crate::app_event::ThreadGoalSetMode::UpdateExisting { status, @@ -245,7 +205,7 @@ async fn goal_edit_prompt_preserves_resumable_stopped_statuses() { assert_eq!(status, stopped_status); assert_eq!(token_budget, Some(80_000)); } - other => panic!("expected SetThreadGoalObjective event, got {other:?}"), + other => panic!("expected SetThreadGoalDraft event, got {other:?}"), } } } @@ -268,11 +228,12 @@ async fn goal_edit_prompt_resets_terminal_status_to_active() { terminal_status, /*token_budget*/ Some(80_000), ), + "Keep improving the bare goal command until it feels calm and useful.".to_string(), ); chat.handle_key_event(KeyEvent::from(KeyCode::Enter)); match rx.try_recv() { - Ok(AppEvent::SetThreadGoalObjective { + Ok(AppEvent::SetThreadGoalDraft { mode: crate::app_event::ThreadGoalSetMode::UpdateExisting { status, @@ -283,7 +244,7 @@ async fn goal_edit_prompt_resets_terminal_status_to_active() { assert_eq!(status, AppThreadGoalStatus::Active); assert_eq!(token_budget, Some(80_000)); } - other => panic!("expected SetThreadGoalObjective event, got {other:?}"), + other => panic!("expected SetThreadGoalDraft event, got {other:?}"), } } } diff --git a/codex-rs/tui/src/chatwidget/tests/goal_validation.rs b/codex-rs/tui/src/chatwidget/tests/goal_validation.rs index 6ed193075c..5d2c1241f7 100644 --- a/codex-rs/tui/src/chatwidget/tests/goal_validation.rs +++ b/codex-rs/tui/src/chatwidget/tests/goal_validation.rs @@ -1,7 +1,6 @@ use super::*; use codex_protocol::protocol::MAX_THREAD_GOAL_OBJECTIVE_CHARS; use pretty_assertions::assert_eq; -use std::path::PathBuf; fn complete_turn_with_message(chat: &mut ChatWidget, turn_id: &str, message: Option<&str>) { if let Some(message) = message { @@ -33,34 +32,20 @@ fn queue_composer_text_with_tab(chat: &mut ChatWidget, text: &str) { chat.handle_key_event(KeyEvent::new(KeyCode::Tab, KeyModifiers::NONE)); } -fn goal_file_path(objective: &str) -> PathBuf { - crate::goal_files::objective_file_path(objective) - .unwrap_or_else(|| panic!("expected goal file objective, got {objective:?}")) -} - -fn path_after_prefix(text: &str, prefix: &str) -> PathBuf { - let rest = text - .split_once(prefix) - .unwrap_or_else(|| panic!("expected {prefix:?} in {text:?}")) - .1; - PathBuf::from(rest.lines().next().expect("path line").trim()) -} - -fn next_goal_objective( +fn next_goal_draft( rx: &mut tokio::sync::mpsc::UnboundedReceiver, expected_thread_id: ThreadId, -) -> String { - let event = rx.try_recv().expect("expected goal objective event"); - let AppEvent::SetThreadGoalObjective { - thread_id, - objective, - .. - } = event - else { - panic!("expected SetThreadGoalObjective, got {event:?}"); - }; - assert_eq!(thread_id, expected_thread_id); - objective +) -> crate::goal_files::GoalDraft { + loop { + let event = rx.try_recv().expect("expected goal draft event"); + if let AppEvent::SetThreadGoalDraft { + thread_id, draft, .. + } = event + { + assert_eq!(thread_id, expected_thread_id); + return draft; + } + } } #[test] @@ -85,7 +70,7 @@ async fn goal_slash_command_accepts_objective_at_limit() { submit_composer_text(&mut chat, &command); - assert_eq!(next_goal_objective(&mut rx, thread_id), objective); + assert_eq!(next_goal_draft(&mut rx, thread_id).objective, objective); assert_no_submit_op(&mut op_rx); } @@ -99,31 +84,12 @@ async fn goal_slash_command_accepts_multiline_objective_after_blank_first_line() submit_composer_text(&mut chat, &format!("/goal \n\n{objective}")); - assert_eq!(next_goal_objective(&mut rx, thread_id), objective); + assert_eq!(next_goal_draft(&mut rx, thread_id).objective, objective); assert_no_submit_op(&mut op_rx); } #[tokio::test] -async fn goal_slash_command_materializes_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); - 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 actual_objective = next_goal_objective(&mut rx, thread_id); - let path = goal_file_path(&actual_objective); - assert_eq!( - std::fs::read_to_string(path).expect("read goal file"), - objective - ); - assert_no_submit_op(&mut op_rx); -} - -#[tokio::test] -async fn goal_slash_command_materializes_only_paste_text_element() { +async fn goal_slash_command_emits_only_inserted_paste_text_element() { let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await; chat.set_feature_enabled(Feature::Goals, /*enabled*/ true); let thread_id = ThreadId::new(); @@ -139,99 +105,42 @@ async fn goal_slash_command_materializes_only_paste_text_element() { submit_current_composer(&mut chat); - let actual_objective = next_goal_objective(&mut rx, thread_id); + let draft = next_goal_draft(&mut rx, thread_id); assert!( - actual_objective.contains(&format!( - "keep literal {placeholder} and pasted text file: " - )), - "expected only the inserted paste element to be materialized, got {actual_objective:?}" - ); - let path = path_after_prefix(&actual_objective, "pasted text file: "); - assert_eq!( - std::fs::read_to_string(path).expect("read paste file"), - paste + draft + .objective + .contains(&format!("keep literal {placeholder} and {placeholder}")), + "expected literal placeholder and inserted paste placeholder, got {:?}", + draft.objective ); + assert_eq!(draft.pending_pastes, vec![(placeholder, paste)]); assert_no_submit_op(&mut op_rx); } #[tokio::test] -async fn goal_slash_command_materializes_large_paste() { +async fn queued_goal_before_thread_preserves_large_paste() { let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await; chat.set_feature_enabled(Feature::Goals, /*enabled*/ true); - let thread_id = ThreadId::new(); - chat.thread_id = Some(thread_id); 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.clone()); - assert!( - chat.bottom_pane.composer_text().contains("[Pasted Content"), - "expected large paste placeholder in composer" - ); submit_current_composer(&mut chat); - - let actual_objective = next_goal_objective(&mut rx, thread_id); - let path = path_after_prefix(&actual_objective, "pasted text file: "); - assert_eq!( - std::fs::read_to_string(path).expect("read paste file"), - objective - ); + assert_eq!(chat.input_queue.queued_user_messages.len(), 1); assert_no_submit_op(&mut op_rx); + + let thread_id = ThreadId::new(); + chat.thread_id = Some(thread_id); + chat.maybe_send_next_queued_input(); + + let draft = next_goal_draft(&mut rx, thread_id); + assert_eq!(draft.objective, objective); + assert!(draft.pending_pastes.is_empty()); } #[tokio::test] -async fn goal_slash_command_rejects_whitespace_only_large_paste() { - 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(" ".repeat(MAX_THREAD_GOAL_OBJECTIVE_CHARS + 1)); - - submit_current_composer(&mut chat); - - let events = std::iter::from_fn(|| rx.try_recv().ok()).collect::>(); - assert!( - events - .iter() - .all(|event| !matches!(event, AppEvent::SetThreadGoalObjective { .. })), - "expected no goal objective event, got {events:?}" - ); - assert_no_submit_op(&mut op_rx); -} - -#[tokio::test] -async fn queued_goal_materialization_error_drains_next_input() { - let (mut chat, _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 codex_home_file = chat.config.codex_home.join("not-a-directory"); - std::fs::write(&codex_home_file, b"not a directory").expect("write codex home file"); - chat.config.codex_home = codex_home_file; - handle_turn_started(&mut chat, "turn-1"); - let objective = "x".repeat(MAX_THREAD_GOAL_OBJECTIVE_CHARS + 1); - - queue_composer_text_with_tab(&mut chat, &format!("/goal {objective}")); - queue_composer_text_with_tab(&mut chat, "continue after failed goal"); - - complete_turn_with_message(&mut chat, "turn-1", Some("done")); - - match next_submit_op(&mut op_rx) { - Op::UserTurn { items, .. } => assert_eq!( - items, - vec![UserInput::Text { - text: "continue after failed goal".to_string(), - text_elements: Vec::new(), - }] - ), - other => panic!("expected queued message after failed goal, got {other:?}"), - } - assert!(chat.input_queue.queued_user_messages.is_empty()); -} - -#[tokio::test] -async fn queued_goal_slash_command_materializes_oversized_objective_and_stops_queue() { +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); let thread_id = ThreadId::new(); @@ -247,20 +156,14 @@ async fn queued_goal_slash_command_materializes_oversized_objective_and_stops_qu let (actual_thread_id, actual_objective) = loop { match rx.try_recv().expect("expected goal objective event") { - AppEvent::SetThreadGoalObjective { - thread_id, - objective, - .. - } => break (thread_id, objective), + AppEvent::SetThreadGoalDraft { + thread_id, draft, .. + } => break (thread_id, draft.objective), _ => continue, } }; assert_eq!(actual_thread_id, thread_id); - let path = goal_file_path(&actual_objective); - assert_eq!( - std::fs::read_to_string(path).expect("read goal file"), - objective - ); + assert_eq!(actual_objective, 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/slash_commands.rs b/codex-rs/tui/src/chatwidget/tests/slash_commands.rs index b084f72b86..e432afb61c 100644 --- a/codex-rs/tui/src/chatwidget/tests/slash_commands.rs +++ b/codex-rs/tui/src/chatwidget/tests/slash_commands.rs @@ -608,17 +608,17 @@ async fn goal_slash_command_emits_set_goal_event() { submit_composer_text(&mut chat, command); - let event = rx.try_recv().expect("expected goal objective event"); - let AppEvent::SetThreadGoalObjective { + let event = rx.try_recv().expect("expected goal draft event"); + let AppEvent::SetThreadGoalDraft { thread_id: actual_thread_id, - objective, + draft, mode, } = event else { - panic!("expected SetThreadGoalObjective, got {event:?}"); + panic!("expected SetThreadGoalDraft, got {event:?}"); }; assert_eq!(actual_thread_id, thread_id); - assert_eq!(objective, "--tokens 98.5K improve benchmark coverage"); + assert_eq!(draft.objective, "--tokens 98.5K improve benchmark coverage"); assert_eq!(mode, crate::app_event::ThreadGoalSetMode::ConfirmIfExists); assert_no_submit_op(&mut op_rx); assert_eq!(recall_latest_after_clearing(&mut chat), command); @@ -645,22 +645,22 @@ async fn goal_slash_command_uses_plain_text_for_mentions() { chat.handle_key_event(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE)); chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); - let event = rx.try_recv().expect("expected goal objective event"); - let AppEvent::SetThreadGoalObjective { + let event = rx.try_recv().expect("expected goal draft event"); + let AppEvent::SetThreadGoalDraft { thread_id: actual_thread_id, - objective, + draft, .. } = event else { - panic!("expected SetThreadGoalObjective, got {event:?}"); + panic!("expected SetThreadGoalDraft, got {event:?}"); }; assert_eq!(actual_thread_id, thread_id); - assert_eq!(objective, "use $figma for the mockup"); + assert_eq!(draft.objective, "use $figma for the mockup"); assert_no_submit_op(&mut op_rx); } #[tokio::test] -async fn goal_slash_command_materializes_attached_images() { +async fn goal_slash_command_emits_attached_images() { let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await; chat.set_feature_enabled(Feature::Goals, /*enabled*/ true); let thread_id = ThreadId::new(); @@ -683,30 +683,24 @@ async fn goal_slash_command_materializes_attached_images() { chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); - let event = rx.try_recv().expect("expected goal objective event"); - let AppEvent::SetThreadGoalObjective { + let event = rx.try_recv().expect("expected goal draft event"); + let AppEvent::SetThreadGoalDraft { thread_id: actual_thread_id, - objective, + draft, .. } = event else { - panic!("expected SetThreadGoalObjective, got {event:?}"); + panic!("expected SetThreadGoalDraft, got {event:?}"); }; assert_eq!(actual_thread_id, thread_id); - assert!(objective.contains(&format!("literal {placeholder} describe image file: "))); - assert!(objective.contains("Referenced image URLs:")); - assert!(objective.contains(&remote_url)); - let copied_image = objective - .split_once("image file: ") - .expect("image file reference") - .1 - .lines() - .next() - .expect("image path"); assert_eq!( - std::fs::read(copied_image).expect("read copied image"), - b"png bytes" + draft.objective, + format!("literal {placeholder} describe {placeholder}") ); + assert_eq!(draft.remote_image_urls, vec![remote_url]); + assert_eq!(draft.local_images.len(), 1); + assert_eq!(draft.local_images[0].placeholder, placeholder); + assert_eq!(draft.local_images[0].path, local_image.to_path_buf()); assert!(chat.remote_image_urls().is_empty()); assert!(chat.bottom_pane.composer_local_image_paths().is_empty()); assert_no_submit_op(&mut op_rx); @@ -827,17 +821,17 @@ async fn queued_goal_slash_command_emits_set_goal_event_after_thread_starts() { chat.thread_id = Some(thread_id); chat.maybe_send_next_queued_input(); - let event = rx.try_recv().expect("expected goal objective event"); - let AppEvent::SetThreadGoalObjective { + let event = rx.try_recv().expect("expected goal draft event"); + let AppEvent::SetThreadGoalDraft { thread_id: actual_thread_id, - objective, + draft, .. } = event else { - panic!("expected SetThreadGoalObjective, got {event:?}"); + panic!("expected SetThreadGoalDraft, got {event:?}"); }; assert_eq!(actual_thread_id, thread_id); - assert_eq!(objective, "improve benchmark coverage"); + assert_eq!(draft.objective, "improve benchmark coverage"); assert_no_submit_op(&mut op_rx); } @@ -872,7 +866,7 @@ async fn queued_goal_slash_command_preserves_current_draft_metadata() { let event = rx.try_recv().expect("expected goal objective event"); assert_matches!( event, - AppEvent::SetThreadGoalObjective { + AppEvent::SetThreadGoalDraft { thread_id: actual_thread_id, .. } if actual_thread_id == thread_id @@ -910,7 +904,7 @@ async fn restored_queued_goal_slash_command_emits_set_goal_event() { .expect("expected goal objective event"); assert_matches!( event, - AppEvent::SetThreadGoalObjective { + AppEvent::SetThreadGoalDraft { thread_id: actual_thread_id, .. } if actual_thread_id == thread_id diff --git a/codex-rs/tui/src/goal_display.rs b/codex-rs/tui/src/goal_display.rs index ff2119ccf6..0927f318b8 100644 --- a/codex-rs/tui/src/goal_display.rs +++ b/codex-rs/tui/src/goal_display.rs @@ -117,16 +117,16 @@ mod tests { #[test] fn goal_usage_summary_formats_managed_file_objective() { let temp_dir = tempfile::tempdir().expect("tempdir"); - let objective = crate::goal_files::materialize_goal_draft( - temp_dir.path(), - crate::goal_files::GoalDraft { - objective: "x" - .repeat(codex_protocol::protocol::MAX_THREAD_GOAL_OBJECTIVE_CHARS + 1), - ..Default::default() - }, + let path = codex_utils_absolute_path::AbsolutePathBuf::from_absolute_path_checked( + temp_dir + .path() + .join("attachments") + .join("00000000-0000-4000-8000-000000000000") + .join("goal-objective.md"), ) - .expect("materialize goal objective"); - let path = crate::goal_files::objective_file_path(&objective).expect("goal file path"); + .expect("absolute path"); + let objective = + crate::goal_files::objective_file_reference(&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 b4c08cc512..cda2d31b15 100644 --- a/codex-rs/tui/src/goal_files.rs +++ b/codex-rs/tui/src/goal_files.rs @@ -1,13 +1,13 @@ //! File materialization helpers for TUI goal objectives. //! -//! Long objectives, pasted text, and local images are written under the Codex -//! home directory. The persisted goal objective keeps references to those files -//! so later continuations can read them by path. +//! Long objectives, pasted text, and local images are written under the app +//! server's Codex home directory. The persisted goal objective keeps references +//! to those files so later continuations can read them by path. use std::fs; use std::path::Path; -use std::path::PathBuf; +use crate::app_server_session::AppServerSession; use crate::bottom_pane::ChatComposer; use crate::bottom_pane::LocalImageAttachment; use anyhow::Context; @@ -15,14 +15,15 @@ use anyhow::Result; use anyhow::bail; use codex_protocol::protocol::MAX_THREAD_GOAL_OBJECTIVE_CHARS; use codex_protocol::user_input::TextElement; +use codex_utils_absolute_path::AbsolutePathBuf; use uuid::Uuid; const GOAL_ATTACHMENT_DIR: &str = "attachments"; const GOAL_FILE_PREFIX: &str = "Codex goal objective file: "; -const GOAL_FILE_INSTRUCTION: &str = "Read that file before continuing."; +const GOAL_FILE_INSTRUCTION: &str = "Read that Codex-created file before continuing."; const GOAL_FILE_NAME: &str = "goal-objective.md"; -#[derive(Debug, Default)] +#[derive(Clone, Debug, Default)] pub(crate) struct GoalDraft { pub(crate) objective: String, pub(crate) text_elements: Vec, @@ -31,7 +32,53 @@ pub(crate) struct GoalDraft { pub(crate) remote_image_urls: Vec, } -pub(crate) fn materialize_goal_draft(codex_home: &Path, draft: GoalDraft) -> Result { +/// Host-side file operations needed to materialize goal inputs. +/// +/// Implementations must operate on the same filesystem that the app server and +/// agent will use to resolve persisted goal file references. +pub(crate) trait GoalFileStore { + fn create_directory( + &mut self, + path: AbsolutePathBuf, + ) -> impl std::future::Future> + Send; + + fn write_file( + &mut self, + path: AbsolutePathBuf, + bytes: Vec, + ) -> impl std::future::Future> + Send; + + fn read_file( + &mut self, + path: AbsolutePathBuf, + ) -> impl std::future::Future>> + Send; +} + +impl GoalFileStore for AppServerSession { + async fn create_directory(&mut self, path: AbsolutePathBuf) -> Result<()> { + self.fs_create_directory(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) + .await + .map_err(|err| anyhow::anyhow!("{err}")) + } + + async fn read_file(&mut self, path: AbsolutePathBuf) -> Result> { + self.fs_read_file(path) + .await + .map_err(|err| anyhow::anyhow!("{err}")) + } +} + +pub(crate) async fn materialize_goal_draft( + store: &mut impl GoalFileStore, + codex_home: &AbsolutePathBuf, + draft: GoalDraft, +) -> Result { let mut objective = draft.objective; if objective.trim().is_empty() { bail!("Goal objective must not be empty."); @@ -49,9 +96,10 @@ pub(crate) fn materialize_goal_draft(codex_home: &Path, draft: GoalDraft) -> Res let mut output_dir = None; let mut materialized_pastes = Vec::new(); for (idx, (placeholder, text)) in draft.pending_pastes.iter().enumerate() { - let path = ensure_output_dir(codex_home, &mut output_dir)? + let path = ensure_output_dir(store, codex_home, &mut output_dir) + .await? .join(format!("pasted-text-{}.txt", idx + 1)); - write_file(&path, text)?; + write_file(store, path.clone(), text.as_bytes().to_vec()).await?; if !placeholder.is_empty() { materialized_pastes.push(( @@ -68,18 +116,12 @@ pub(crate) fn materialize_goal_draft(codex_home: &Path, draft: GoalDraft) -> Res let mut materialized_images = Vec::new(); for (idx, image) in draft.local_images.iter().enumerate() { let extension = image_extension(&image.path); - let path = ensure_output_dir(codex_home, &mut output_dir)?.join(format!( - "image-{}.{}", - idx + 1, - extension - )); - fs::copy(&image.path, &path).with_context(|| { - format!( - "Could not copy goal image from {} to {}", - image.path.display(), - path.display() - ) - })?; + let path = ensure_output_dir(store, codex_home, &mut output_dir) + .await? + .join(format!("image-{}.{}", idx + 1, extension)); + let bytes = fs::read(&image.path) + .with_context(|| format!("Could not read goal image {}", image.path.display()))?; + write_file(store, path.clone(), bytes).await?; if image.placeholder.is_empty() { image_lines.push(format!("- [Image #{}]: {}", idx + 1, path.display())); } else { @@ -105,39 +147,48 @@ pub(crate) fn materialize_goal_draft(codex_home: &Path, draft: GoalDraft) -> Res ); if objective.chars().count() > MAX_THREAD_GOAL_OBJECTIVE_CHARS { - let path = ensure_output_dir(codex_home, &mut output_dir)?.join(GOAL_FILE_NAME); - write_file(&path, &objective)?; + let path = ensure_output_dir(store, codex_home, &mut output_dir) + .await? + .join(GOAL_FILE_NAME); + write_file(store, path.clone(), objective.as_bytes().to_vec()).await?; objective = objective_file_reference(&path)?; } Ok(objective) } -pub(crate) fn objective_text_for_edit(objective: &str) -> Result { +pub(crate) async fn objective_text_for_edit( + store: &mut impl GoalFileStore, + objective: &str, +) -> Result { let Some(path) = objective_file_path(objective) else { return Ok(objective.to_string()); }; - fs::read_to_string(&path) - .with_context(|| format!("Could not read goal objective file {}", path.display())) + let bytes = store + .read_file(path.clone()) + .await + .with_context(|| format!("Could not read goal objective file {}", path.display()))?; + String::from_utf8(bytes) + .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()) - .map(PathBuf::from)?; + .and_then(|path| AbsolutePathBuf::from_absolute_path_checked(path).ok())?; if lines.next() != Some(GOAL_FILE_INSTRUCTION) { return None; } let parent = path.parent()?; let attachment_id = parent.file_name()?.to_str()?; - let attachment_dir = parent.parent()?.file_name()?.to_str()?; - if path.is_file() - && path.file_name()?.to_str()? == GOAL_FILE_NAME + 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() { @@ -147,7 +198,7 @@ pub(crate) fn objective_file_path(objective: &str) -> Option { } } -fn objective_file_reference(path: &Path) -> Result { +pub(crate) fn objective_file_reference(path: &Path) -> Result { let reference = format!( "{GOAL_FILE_PREFIX}{}\n{GOAL_FILE_INSTRUCTION}", path.display() @@ -161,25 +212,38 @@ fn objective_file_reference(path: &Path) -> Result { Ok(reference) } -fn ensure_output_dir(codex_home: &Path, output_dir: &mut Option) -> Result { +async fn ensure_output_dir( + store: &mut impl GoalFileStore, + codex_home: &AbsolutePathBuf, + output_dir: &mut Option, +) -> Result { if let Some(output_dir) = output_dir { return Ok(output_dir.clone()); } let path = codex_home .join(GOAL_ATTACHMENT_DIR) .join(Uuid::new_v4().to_string()); - fs::create_dir_all(&path).with_context(|| { - format!( - "Could not create goal attachment directory {}", - path.display() - ) - })?; + store + .create_directory(path.clone()) + .await + .with_context(|| { + format!( + "Could not create goal attachment directory {}", + path.display() + ) + })?; *output_dir = Some(path.clone()); Ok(path) } -fn write_file(path: &Path, content: &str) -> Result<()> { - fs::write(path, content) +async fn write_file( + store: &mut impl GoalFileStore, + path: AbsolutePathBuf, + bytes: Vec, +) -> Result<()> { + store + .write_file(path.clone(), bytes) + .await .with_context(|| format!("Could not write goal file {}", path.display())) } @@ -201,3 +265,7 @@ fn image_extension(path: &Path) -> &str { .filter(|extension| !extension.is_empty()) .unwrap_or("png") } + +#[cfg(test)] +#[path = "goal_files_tests.rs"] +mod tests; diff --git a/codex-rs/tui/src/goal_files_tests.rs b/codex-rs/tui/src/goal_files_tests.rs new file mode 100644 index 0000000000..566c68a6a5 --- /dev/null +++ b/codex-rs/tui/src/goal_files_tests.rs @@ -0,0 +1,120 @@ +use super::*; + +use codex_protocol::protocol::MAX_THREAD_GOAL_OBJECTIVE_CHARS; +use codex_protocol::user_input::TextElement; +use pretty_assertions::assert_eq; + +struct LocalStore; + +impl GoalFileStore for LocalStore { + async fn create_directory(&mut self, path: AbsolutePathBuf) -> Result<()> { + fs::create_dir_all(path.as_path())?; + Ok(()) + } + + async fn write_file(&mut self, path: AbsolutePathBuf, bytes: Vec) -> Result<()> { + fs::write(path.as_path(), bytes)?; + Ok(()) + } + + async fn read_file(&mut self, path: AbsolutePathBuf) -> Result> { + Ok(fs::read(path.as_path())?) + } +} + +#[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 objective = "x".repeat(MAX_THREAD_GOAL_OBJECTIVE_CHARS + 1); + let mut store = LocalStore; + + let reference = materialize_goal_draft( + &mut store, + &codex_home, + GoalDraft { + objective: objective.clone(), + ..Default::default() + }, + ) + .await + .expect("materialize goal draft"); + + let path = objective_file_path(&reference).expect("goal file path"); + assert_eq!( + fs::read_to_string(path.as_path()).expect("read file"), + objective + ); + let edit_text = objective_text_for_edit(&mut store, &reference) + .await + .expect("read objective text"); + assert_eq!(edit_text, objective); +} + +#[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 image_path = temp_dir.path().join("local-image.png"); + fs::write(&image_path, b"png bytes").expect("write image"); + let mut store = LocalStore; + let objective = "Use [Pasted Content 5 chars] and [Image #1]".to_string(); + let paste_placeholder = "[Pasted Content 5 chars]"; + let image_placeholder = "[Image #1]"; + let paste_start = objective + .find(paste_placeholder) + .expect("paste placeholder"); + let image_start = objective + .find(image_placeholder) + .expect("image placeholder"); + + let objective = materialize_goal_draft( + &mut store, + &codex_home, + GoalDraft { + objective, + text_elements: vec![ + TextElement::new( + (paste_start..paste_start + paste_placeholder.len()).into(), + Some(paste_placeholder.to_string()), + ), + TextElement::new( + (image_start..image_start + image_placeholder.len()).into(), + Some(image_placeholder.to_string()), + ), + ], + pending_pastes: vec![(paste_placeholder.to_string(), "hello".to_string())], + local_images: vec![LocalImageAttachment { + placeholder: image_placeholder.to_string(), + path: image_path, + }], + ..Default::default() + }, + ) + .await + .expect("materialize goal draft"); + + 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.as_path()).expect("read paste"), + "hello" + ); + assert_eq!( + fs::read(image_path.as_path()).expect("read image"), + b"png bytes" + ); +} + +fn path_after(text: &str, prefix: &str) -> AbsolutePathBuf { + let path = text + .split_once(prefix) + .unwrap_or_else(|| panic!("expected {prefix:?} in {text:?}")) + .1 + .split_whitespace() + .next() + .expect("path"); + AbsolutePathBuf::from_absolute_path_checked(path).expect("absolute path") +}