diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index 5b90feab3..54b40c296 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -2810,24 +2810,27 @@ impl ChatComposer { if !self.slash_commands_enabled() || self.is_bash_mode { return None; } - let first_line = self.textarea.text().lines().next().unwrap_or(""); - if let Some((name, rest, _rest_offset)) = parse_slash_name(first_line) - && rest.is_empty() - && let Some(cmd) = - slash_commands::find_builtin_command(name, self.builtin_command_flags()) - { - if self.reject_slash_command_if_unavailable(cmd) { - self.stage_slash_command_history(); - self.record_pending_slash_command_history(); - return Some(InputResult::None); - } - self.stage_slash_command_history(); - self.textarea.set_text_clearing_elements(""); - self.is_bash_mode = false; - Some(InputResult::Command(cmd)) - } else { - None + let text = self.textarea.text(); + let first_line = text.lines().next().unwrap_or(""); + let (name, rest, _rest_offset) = parse_slash_name(first_line)?; + if !rest.is_empty() { + return None; } + let cmd = slash_commands::find_builtin_command(name, self.builtin_command_flags())?; + if cmd.supports_inline_args() + && parse_slash_name(text).is_some_and(|(_, full_rest, _)| !full_rest.is_empty()) + { + return None; + } + if self.reject_slash_command_if_unavailable(cmd) { + self.stage_slash_command_history(); + self.record_pending_slash_command_history(); + return Some(InputResult::None); + } + self.stage_slash_command_history(); + self.textarea.set_text_clearing_elements(""); + self.is_bash_mode = false; + Some(InputResult::Command(cmd)) } /// Check if the input is a slash command with args (e.g., /review args) and dispatch it. diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 2b6166ff2..5908ceb37 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -320,6 +320,7 @@ 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 interrupts; diff --git a/codex-rs/tui/src/chatwidget/goal_validation.rs b/codex-rs/tui/src/chatwidget/goal_validation.rs new file mode 100644 index 000000000..2f9bcb931 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/goal_validation.rs @@ -0,0 +1,64 @@ +//! 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 aaa71cca8..1293b3767 100644 --- a/codex-rs/tui/src/chatwidget/slash_dispatch.rs +++ b/codex-rs/tui/src/chatwidget/slash_dispatch.rs @@ -5,6 +5,7 @@ //! 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; @@ -475,6 +476,12 @@ 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 { @@ -672,6 +679,13 @@ impl ChatWidget { } 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( @@ -804,6 +818,11 @@ 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/snapshots/codex_tui__chatwidget__tests__goal_slash_command_oversized_objective_error.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__goal_slash_command_oversized_objective_error.snap new file mode 100644 index 000000000..470beccf0 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__goal_slash_command_oversized_objective_error.snap @@ -0,0 +1,5 @@ +--- +source: tui/src/chatwidget/tests/goal_validation.rs +expression: rendered +--- +■ Goal objective is too long: 4,001 characters. Limit: 4,000 characters. Put longer instructions in a file and refer to that file in the goal, for example: /goal follow the instructions in docs/goal.md. diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index 323d0b749..8ed8ca7db 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -226,6 +226,7 @@ mod approval_requests; mod composer_submission; mod exec_flow; mod goal_menu; +mod goal_validation; mod guardian; mod helpers; mod history_replay; diff --git a/codex-rs/tui/src/chatwidget/tests/goal_validation.rs b/codex-rs/tui/src/chatwidget/tests/goal_validation.rs new file mode 100644 index 000000000..85ac34ebd --- /dev/null +++ b/codex-rs/tui/src/chatwidget/tests/goal_validation.rs @@ -0,0 +1,224 @@ +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>) { + if let Some(message) = message { + complete_assistant_message( + chat, + &format!("{turn_id}-message"), + message, + Some(MessagePhase::FinalAnswer), + ); + } + handle_turn_completed(chat, turn_id, /*duration_ms*/ None); +} + +fn submit_composer_text(chat: &mut ChatWidget, text: &str) { + chat.bottom_pane + .set_composer_text(text.to_string(), Vec::new(), Vec::new()); + submit_current_composer(chat); +} + +fn submit_current_composer(chat: &mut ChatWidget) { + chat.handle_key_event(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE)); + chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); + chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); +} + +fn queue_composer_text_with_tab(chat: &mut ChatWidget, text: &str) { + chat.bottom_pane + .set_composer_text(text.to_string(), Vec::new(), Vec::new()); + 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") +} + +#[tokio::test] +async fn goal_slash_command_accepts_objective_at_limit() { + 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); + let command = format!("/goal {objective}"); + + submit_composer_text(&mut chat, &command); + + let event = rx.try_recv().expect("expected goal objective event"); + let AppEvent::SetThreadGoalObjective { + thread_id: actual_thread_id, + objective: actual_objective, + .. + } = event + else { + panic!("expected SetThreadGoalObjective, got {event:?}"); + }; + assert_eq!(actual_thread_id, thread_id); + assert_eq!(actual_objective, objective); + assert_no_submit_op(&mut op_rx); +} + +#[tokio::test] +async fn goal_slash_command_accepts_multiline_objective_after_blank_first_line() { + 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 = "follow these instructions\npreserve this detail"; + + submit_composer_text(&mut chat, &format!("/goal \n\n{objective}")); + + let event = rx.try_recv().expect("expected goal objective event"); + let AppEvent::SetThreadGoalObjective { + thread_id: actual_thread_id, + objective: actual_objective, + .. + } = event + else { + panic!("expected SetThreadGoalObjective, got {event:?}"); + }; + assert_eq!(actual_thread_id, thread_id); + assert_eq!(actual_objective, objective); + assert_no_submit_op(&mut op_rx); +} + +#[tokio::test] +async fn goal_slash_command_rejects_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 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_chatwidget_snapshot!("goal_slash_command_oversized_objective_error", rendered); + assert_no_submit_op(&mut op_rx); +} + +#[tokio::test] +async fn goal_slash_command_rejects_large_paste_using_expanded_length() { + 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()); + let objective = "x".repeat(MAX_THREAD_GOAL_OBJECTIVE_CHARS + 1); + chat.handle_paste(objective); + + assert!( + chat.bottom_pane.composer_text().contains("[Pasted Content"), + "expected large paste placeholder in composer" + ); + 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_no_submit_op(&mut op_rx); +} + +#[tokio::test] +async fn goal_slash_command_giant_paste_uses_goal_specific_error() { + 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()); + 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"); + assert_eq!(chat.queued_user_messages.len(), 2); + + 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.queued_user_messages.is_empty()); + 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 283237737..e493c83d0 100644 --- a/codex-rs/tui/src/chatwidget/tests/slash_commands.rs +++ b/codex-rs/tui/src/chatwidget/tests/slash_commands.rs @@ -16,6 +16,10 @@ fn complete_turn_with_message(chat: &mut ChatWidget, turn_id: &str, message: Opt fn submit_composer_text(chat: &mut ChatWidget, text: &str) { chat.bottom_pane .set_composer_text(text.to_string(), Vec::new(), Vec::new()); + submit_current_composer(chat); +} + +fn submit_current_composer(chat: &mut ChatWidget) { chat.handle_key_event(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE)); chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));