diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index ec43bd6f3..e37fc6288 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -3794,7 +3794,6 @@ dependencies = [ "codex-utils-oss", "codex-utils-path", "codex-utils-plugins", - "codex-utils-pty", "codex-utils-sandbox-summary", "codex-utils-sleep-inhibitor", "codex-utils-string", diff --git a/codex-rs/core/src/exec_policy_tests.rs b/codex-rs/core/src/exec_policy_tests.rs index 776aa8e54..0bcc3b047 100644 --- a/codex-rs/core/src/exec_policy_tests.rs +++ b/codex-rs/core/src/exec_policy_tests.rs @@ -260,6 +260,26 @@ async fn returns_empty_policy_when_no_policy_files_exist() { assert!(!temp_dir.path().join(RULES_DIR_NAME).exists()); } +#[tokio::test] +async fn rules_path_file_returns_read_dir_error() { + let temp_dir = tempdir().expect("create temp dir"); + let rules_path = temp_dir.path().join(RULES_DIR_NAME); + fs::write(&rules_path, "rules should be a directory").expect("write malformed rules path"); + let config_stack = config_stack_for_dot_codex_folder(temp_dir.path()); + + let err = load_exec_policy(&config_stack) + .await + .expect_err("rules file should fail policy loading"); + + assert!( + matches!( + err, + ExecPolicyError::ReadDir { ref dir, .. } if dir == &rules_path + ), + "expected malformed rules path to surface as ReadDir, got {err:?}" + ); +} + #[tokio::test] async fn collect_policy_files_returns_empty_when_dir_missing() { let temp_dir = tempdir().expect("create temp dir"); diff --git a/codex-rs/tui/Cargo.toml b/codex-rs/tui/Cargo.toml index f539db5f5..abd1ccb5c 100644 --- a/codex-rs/tui/Cargo.toml +++ b/codex-rs/tui/Cargo.toml @@ -154,7 +154,6 @@ codex-cli = { workspace = true } codex-mcp = { workspace = true } core_test_support = { workspace = true } codex-utils-cargo-bin = { workspace = true } -codex-utils-pty = { workspace = true } assert_matches = { workspace = true } chrono = { workspace = true, features = ["serde"] } insta = { workspace = true } diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index bab42be2f..f3a7aaf99 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -755,12 +755,15 @@ impl App { &initial_prompt, &initial_images, ); + let startup_tooltip_override = + if Self::should_prepare_startup_tooltip_override(&session_selection) { + prepare_startup_tooltip_override(&mut config, &available_models, is_first_run).await + } else { + None + }; let (mut chat_widget, initial_started_thread) = match session_selection { SessionSelection::StartFresh | SessionSelection::Exit => { let started = app_server.start_thread(&config).await?; - let startup_tooltip_override = - prepare_startup_tooltip_override(&mut config, &available_models, is_first_run) - .await; let init = crate::chatwidget::ChatWidgetInit { config: config.clone(), environment_manager: environment_manager.clone(), diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index ec20fffea..fc095fc2b 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -1,6 +1,8 @@ //! App-level orchestration tests for the TUI. mod model_catalog; +mod session_summary; +mod startup; use super::*; use crate::app_backtrack::BacktrackSelection; @@ -127,183 +129,6 @@ async fn handle_mcp_inventory_result_clears_committed_loading_cell() { assert_eq!(app.transcript_cells.len(), 0); } -#[test] -fn startup_waiting_gate_is_only_for_fresh_or_exit_session_selection() { - assert_eq!( - App::should_wait_for_initial_session(&SessionSelection::StartFresh), - true - ); - assert_eq!( - App::should_wait_for_initial_session(&SessionSelection::Exit), - true - ); - assert_eq!( - App::should_wait_for_initial_session(&SessionSelection::Resume( - crate::resume_picker::SessionTarget { - path: Some(PathBuf::from("/tmp/restore")), - thread_id: ThreadId::new(), - } - )), - false - ); - assert_eq!( - App::should_wait_for_initial_session(&SessionSelection::Fork( - crate::resume_picker::SessionTarget { - path: Some(PathBuf::from("/tmp/fork")), - thread_id: ThreadId::new(), - } - )), - false - ); -} - -#[test] -fn startup_paused_goal_prompt_gate_is_only_for_quiet_resume() { - let resume = SessionSelection::Resume(crate::resume_picker::SessionTarget { - path: Some(PathBuf::from("/tmp/restore")), - thread_id: ThreadId::new(), - }); - let fork = SessionSelection::Fork(crate::resume_picker::SessionTarget { - path: Some(PathBuf::from("/tmp/fork")), - thread_id: ThreadId::new(), - }); - let no_images: Vec = Vec::new(); - let initial_images = vec![PathBuf::from("/tmp/image.png")]; - - assert!(App::should_prompt_for_paused_goal_after_startup_resume( - &resume, &None, &no_images - )); - assert!(!App::should_prompt_for_paused_goal_after_startup_resume( - &resume, - &Some("continue from here".to_string()), - &no_images - )); - assert!(!App::should_prompt_for_paused_goal_after_startup_resume( - &resume, - &None, - &initial_images - )); - assert!(!App::should_prompt_for_paused_goal_after_startup_resume( - &SessionSelection::StartFresh, - &None, - &no_images - )); - assert!(!App::should_prompt_for_paused_goal_after_startup_resume( - &fork, &None, &no_images - )); -} - -#[test] -fn startup_waiting_gate_holds_active_thread_events_until_primary_thread_configured() { - let mut wait_for_initial_session = - App::should_wait_for_initial_session(&SessionSelection::StartFresh); - assert_eq!(wait_for_initial_session, true); - assert_eq!( - App::should_handle_active_thread_events( - wait_for_initial_session, - /*has_active_thread_receiver*/ true - ), - false - ); - - assert_eq!( - App::should_stop_waiting_for_initial_session( - wait_for_initial_session, - /*primary_thread_id*/ None - ), - false - ); - if App::should_stop_waiting_for_initial_session(wait_for_initial_session, Some(ThreadId::new())) - { - wait_for_initial_session = false; - } - assert_eq!(wait_for_initial_session, false); - - assert_eq!( - App::should_handle_active_thread_events( - wait_for_initial_session, - /*has_active_thread_receiver*/ true - ), - true - ); -} - -#[test] -fn startup_waiting_gate_not_applied_for_resume_or_fork_session_selection() { - let wait_for_resume = App::should_wait_for_initial_session(&SessionSelection::Resume( - crate::resume_picker::SessionTarget { - path: Some(PathBuf::from("/tmp/restore")), - thread_id: ThreadId::new(), - }, - )); - assert_eq!( - App::should_handle_active_thread_events( - wait_for_resume, - /*has_active_thread_receiver*/ true - ), - true - ); - let wait_for_fork = App::should_wait_for_initial_session(&SessionSelection::Fork( - crate::resume_picker::SessionTarget { - path: Some(PathBuf::from("/tmp/fork")), - thread_id: ThreadId::new(), - }, - )); - assert_eq!( - App::should_handle_active_thread_events( - wait_for_fork, - /*has_active_thread_receiver*/ true - ), - true - ); -} - -#[tokio::test] -async fn ignore_same_thread_resume_reports_noop_for_current_thread() { - let (mut app, mut app_event_rx, _op_rx) = make_test_app_with_channels().await; - let thread_id = ThreadId::new(); - let session = test_thread_session(thread_id, test_path_buf("/tmp/project")); - app.chat_widget.handle_thread_session(session.clone()); - app.thread_event_channels.insert( - thread_id, - ThreadEventChannel::new_with_session(THREAD_EVENT_CHANNEL_CAPACITY, session, Vec::new()), - ); - app.activate_thread_channel(thread_id).await; - while app_event_rx.try_recv().is_ok() {} - - let ignored = app.ignore_same_thread_resume(&crate::resume_picker::SessionTarget { - path: Some(test_path_buf("/tmp/project")), - thread_id, - }); - - assert!(ignored); - let cell = match app_event_rx.try_recv() { - Ok(AppEvent::InsertHistoryCell(cell)) => cell, - other => panic!("expected info message after same-thread resume, saw {other:?}"), - }; - let rendered = lines_to_single_string(&cell.display_lines(/*width*/ 80)); - assert!(rendered.contains(&format!( - "Already viewing {}.", - test_path_display("/tmp/project") - ))); -} - -#[tokio::test] -async fn ignore_same_thread_resume_allows_reattaching_displayed_inactive_thread() { - let mut app = make_test_app().await; - let thread_id = ThreadId::new(); - let session = test_thread_session(thread_id, test_path_buf("/tmp/project")); - app.chat_widget.handle_thread_session(session); - - let ignored = app.ignore_same_thread_resume(&crate::resume_picker::SessionTarget { - path: Some(test_path_buf("/tmp/project")), - thread_id, - }); - - assert!(!ignored); - assert!(app.transcript_cells.is_empty()); -} - #[test] fn bypass_hook_trust_startup_warning_snapshot() { let rendered = lines_to_single_string( @@ -5417,90 +5242,3 @@ async fn backtrack_esc_does_not_steal_empty_vim_insert_escape() { assert!(!app.chat_widget.should_handle_vim_insert_escape(esc)); assert!(app.should_handle_backtrack_esc(esc)); } - -#[tokio::test] -async fn session_summary_skips_when_no_usage_or_resume_hint() { - assert!( - session_summary( - TokenUsage::default(), - /*thread_id*/ None, - /*thread_name*/ None, - /*rollout_path*/ None, - ) - .is_none() - ); -} - -#[tokio::test] -async fn session_summary_skips_resume_hint_until_rollout_exists() { - let usage = TokenUsage::default(); - let conversation = ThreadId::from_string("123e4567-e89b-12d3-a456-426614174000").unwrap(); - let temp_dir = tempdir().expect("temp dir"); - let rollout_path = temp_dir.path().join("rollout.jsonl"); - - assert!( - session_summary( - usage, - Some(conversation), - /*thread_name*/ None, - Some(&rollout_path), - ) - .is_none() - ); -} - -#[tokio::test] -async fn session_summary_includes_resume_hint_for_persisted_rollout() { - let usage = TokenUsage { - input_tokens: 10, - output_tokens: 2, - total_tokens: 12, - ..Default::default() - }; - let conversation = ThreadId::from_string("123e4567-e89b-12d3-a456-426614174000").unwrap(); - let temp_dir = tempdir().expect("temp dir"); - let rollout_path = temp_dir.path().join("rollout.jsonl"); - std::fs::write(&rollout_path, "{}\n").expect("write rollout"); - - let summary = session_summary( - usage, - Some(conversation), - /*thread_name*/ None, - Some(&rollout_path), - ) - .expect("summary"); - assert_eq!( - summary.usage_line, - Some("Token usage: total=12 input=10 output=2".to_string()) - ); - assert_eq!( - summary.resume_command, - Some("codex resume 123e4567-e89b-12d3-a456-426614174000".to_string()) - ); -} - -#[tokio::test] -async fn session_summary_uses_id_even_when_thread_has_name() { - let usage = TokenUsage { - input_tokens: 10, - output_tokens: 2, - total_tokens: 12, - ..Default::default() - }; - let conversation = ThreadId::from_string("123e4567-e89b-12d3-a456-426614174000").unwrap(); - let temp_dir = tempdir().expect("temp dir"); - let rollout_path = temp_dir.path().join("rollout.jsonl"); - std::fs::write(&rollout_path, "{}\n").expect("write rollout"); - - let summary = session_summary( - usage, - Some(conversation), - Some("my-session".to_string()), - Some(&rollout_path), - ) - .expect("summary"); - assert_eq!( - summary.resume_command, - Some("codex resume 123e4567-e89b-12d3-a456-426614174000".to_string()) - ); -} diff --git a/codex-rs/tui/src/app/tests/model_catalog.rs b/codex-rs/tui/src/app/tests/model_catalog.rs index ba04a3634..7d2eb9c12 100644 --- a/codex-rs/tui/src/app/tests/model_catalog.rs +++ b/codex-rs/tui/src/app/tests/model_catalog.rs @@ -174,6 +174,46 @@ fn select_model_availability_nux_returns_none_when_all_models_are_exhausted() { assert_eq!(selected, None); } +#[tokio::test] +async fn prepare_startup_tooltip_override_persists_model_availability_nux_count() { + let codex_home = tempdir().expect("temp codex home"); + let mut config = ConfigBuilder::default() + .codex_home(codex_home.path().to_path_buf()) + .build() + .await + .expect("config"); + let mut presets = all_model_presets(); + presets.iter_mut().for_each(|preset| { + preset.availability_nux = None; + }); + let target = presets + .iter_mut() + .find(|preset| preset.model == "gpt-5.4") + .expect("target preset present"); + target.availability_nux = Some(ModelAvailabilityNux { + message: "gpt-5.4 is available".to_string(), + }); + + let tooltip = + prepare_startup_tooltip_override(&mut config, &presets, /*is_first_run*/ false).await; + + assert_eq!(tooltip.as_deref(), Some("gpt-5.4 is available")); + assert_eq!( + config.model_availability_nux.shown_count, + HashMap::from([("gpt-5.4".to_string(), 1)]) + ); + + let reloaded = ConfigBuilder::default() + .codex_home(codex_home.path().to_path_buf()) + .build() + .await + .expect("reloaded config"); + assert_eq!( + reloaded.model_availability_nux.shown_count, + HashMap::from([("gpt-5.4".to_string(), 1)]) + ); +} + #[tokio::test] async fn accepted_model_migration_persists_target_default_reasoning_effort() { let codex_home = tempdir().expect("temp codex home"); diff --git a/codex-rs/tui/src/app/tests/session_summary.rs b/codex-rs/tui/src/app/tests/session_summary.rs new file mode 100644 index 000000000..2a96f9ad3 --- /dev/null +++ b/codex-rs/tui/src/app/tests/session_summary.rs @@ -0,0 +1,89 @@ +use super::*; +use pretty_assertions::assert_eq; + +#[tokio::test] +async fn session_summary_skips_when_no_usage_or_resume_hint() { + assert!( + session_summary( + TokenUsage::default(), + /*thread_id*/ None, + /*thread_name*/ None, + /*rollout_path*/ None, + ) + .is_none() + ); +} + +#[tokio::test] +async fn session_summary_skips_resume_hint_until_rollout_exists() { + let usage = TokenUsage::default(); + let conversation = ThreadId::from_string("123e4567-e89b-12d3-a456-426614174000").unwrap(); + let temp_dir = tempdir().expect("temp dir"); + let rollout_path = temp_dir.path().join("rollout.jsonl"); + + assert!( + session_summary( + usage, + Some(conversation), + /*thread_name*/ None, + Some(&rollout_path), + ) + .is_none() + ); +} + +#[tokio::test] +async fn session_summary_includes_resume_hint_for_persisted_rollout() { + let usage = TokenUsage { + input_tokens: 10, + output_tokens: 2, + total_tokens: 12, + ..Default::default() + }; + let conversation = ThreadId::from_string("123e4567-e89b-12d3-a456-426614174000").unwrap(); + let temp_dir = tempdir().expect("temp dir"); + let rollout_path = temp_dir.path().join("rollout.jsonl"); + std::fs::write(&rollout_path, "{}\n").expect("write rollout"); + + let summary = session_summary( + usage, + Some(conversation), + /*thread_name*/ None, + Some(&rollout_path), + ) + .expect("summary"); + assert_eq!( + summary.usage_line, + Some("Token usage: total=12 input=10 output=2".to_string()) + ); + assert_eq!( + summary.resume_command, + Some("codex resume 123e4567-e89b-12d3-a456-426614174000".to_string()) + ); +} + +#[tokio::test] +async fn session_summary_uses_id_even_when_thread_has_name() { + let usage = TokenUsage { + input_tokens: 10, + output_tokens: 2, + total_tokens: 12, + ..Default::default() + }; + let conversation = ThreadId::from_string("123e4567-e89b-12d3-a456-426614174000").unwrap(); + let temp_dir = tempdir().expect("temp dir"); + let rollout_path = temp_dir.path().join("rollout.jsonl"); + std::fs::write(&rollout_path, "{}\n").expect("write rollout"); + + let summary = session_summary( + usage, + Some(conversation), + Some("my-session".to_string()), + Some(&rollout_path), + ) + .expect("summary"); + assert_eq!( + summary.resume_command, + Some("codex resume 123e4567-e89b-12d3-a456-426614174000".to_string()) + ); +} diff --git a/codex-rs/tui/src/app/tests/startup.rs b/codex-rs/tui/src/app/tests/startup.rs new file mode 100644 index 000000000..9f98f50fe --- /dev/null +++ b/codex-rs/tui/src/app/tests/startup.rs @@ -0,0 +1,201 @@ +use super::*; +use pretty_assertions::assert_eq; + +#[test] +fn startup_waiting_gate_is_only_for_fresh_or_exit_session_selection() { + assert_eq!( + App::should_wait_for_initial_session(&SessionSelection::StartFresh), + true + ); + assert_eq!( + App::should_wait_for_initial_session(&SessionSelection::Exit), + true + ); + assert_eq!( + App::should_wait_for_initial_session(&SessionSelection::Resume( + crate::resume_picker::SessionTarget { + path: Some(PathBuf::from("/tmp/restore")), + thread_id: ThreadId::new(), + } + )), + false + ); + assert_eq!( + App::should_wait_for_initial_session(&SessionSelection::Fork( + crate::resume_picker::SessionTarget { + path: Some(PathBuf::from("/tmp/fork")), + thread_id: ThreadId::new(), + } + )), + false + ); +} + +#[test] +fn startup_tooltip_override_is_only_prepared_for_fresh_or_exit_session_selection() { + assert!(App::should_prepare_startup_tooltip_override( + &SessionSelection::StartFresh + )); + assert!(App::should_prepare_startup_tooltip_override( + &SessionSelection::Exit + )); + assert!(!App::should_prepare_startup_tooltip_override( + &SessionSelection::Resume(crate::resume_picker::SessionTarget { + path: Some(PathBuf::from("/tmp/restore")), + thread_id: ThreadId::new(), + }) + )); + assert!(!App::should_prepare_startup_tooltip_override( + &SessionSelection::Fork(crate::resume_picker::SessionTarget { + path: Some(PathBuf::from("/tmp/fork")), + thread_id: ThreadId::new(), + }) + )); +} + +#[test] +fn startup_paused_goal_prompt_gate_is_only_for_quiet_resume() { + let resume = SessionSelection::Resume(crate::resume_picker::SessionTarget { + path: Some(PathBuf::from("/tmp/restore")), + thread_id: ThreadId::new(), + }); + let fork = SessionSelection::Fork(crate::resume_picker::SessionTarget { + path: Some(PathBuf::from("/tmp/fork")), + thread_id: ThreadId::new(), + }); + let no_images: Vec = Vec::new(); + let initial_images = vec![PathBuf::from("/tmp/image.png")]; + + assert!(App::should_prompt_for_paused_goal_after_startup_resume( + &resume, &None, &no_images + )); + assert!(!App::should_prompt_for_paused_goal_after_startup_resume( + &resume, + &Some("continue from here".to_string()), + &no_images + )); + assert!(!App::should_prompt_for_paused_goal_after_startup_resume( + &resume, + &None, + &initial_images + )); + assert!(!App::should_prompt_for_paused_goal_after_startup_resume( + &SessionSelection::StartFresh, + &None, + &no_images + )); + assert!(!App::should_prompt_for_paused_goal_after_startup_resume( + &fork, &None, &no_images + )); +} + +#[test] +fn startup_waiting_gate_holds_active_thread_events_until_primary_thread_configured() { + let mut wait_for_initial_session = + App::should_wait_for_initial_session(&SessionSelection::StartFresh); + assert_eq!(wait_for_initial_session, true); + assert_eq!( + App::should_handle_active_thread_events( + wait_for_initial_session, + /*has_active_thread_receiver*/ true + ), + false + ); + + assert_eq!( + App::should_stop_waiting_for_initial_session( + wait_for_initial_session, + /*primary_thread_id*/ None + ), + false + ); + if App::should_stop_waiting_for_initial_session(wait_for_initial_session, Some(ThreadId::new())) + { + wait_for_initial_session = false; + } + assert_eq!(wait_for_initial_session, false); + + assert_eq!( + App::should_handle_active_thread_events( + wait_for_initial_session, + /*has_active_thread_receiver*/ true + ), + true + ); +} + +#[test] +fn startup_waiting_gate_not_applied_for_resume_or_fork_session_selection() { + let wait_for_resume = App::should_wait_for_initial_session(&SessionSelection::Resume( + crate::resume_picker::SessionTarget { + path: Some(PathBuf::from("/tmp/restore")), + thread_id: ThreadId::new(), + }, + )); + assert_eq!( + App::should_handle_active_thread_events( + wait_for_resume, + /*has_active_thread_receiver*/ true + ), + true + ); + let wait_for_fork = App::should_wait_for_initial_session(&SessionSelection::Fork( + crate::resume_picker::SessionTarget { + path: Some(PathBuf::from("/tmp/fork")), + thread_id: ThreadId::new(), + }, + )); + assert_eq!( + App::should_handle_active_thread_events( + wait_for_fork, + /*has_active_thread_receiver*/ true + ), + true + ); +} + +#[tokio::test] +async fn ignore_same_thread_resume_reports_noop_for_current_thread() { + let (mut app, mut app_event_rx, _op_rx) = make_test_app_with_channels().await; + let thread_id = ThreadId::new(); + let session = test_thread_session(thread_id, test_path_buf("/tmp/project")); + app.chat_widget.handle_thread_session(session.clone()); + app.thread_event_channels.insert( + thread_id, + ThreadEventChannel::new_with_session(THREAD_EVENT_CHANNEL_CAPACITY, session, Vec::new()), + ); + app.activate_thread_channel(thread_id).await; + while app_event_rx.try_recv().is_ok() {} + + let ignored = app.ignore_same_thread_resume(&crate::resume_picker::SessionTarget { + path: Some(test_path_buf("/tmp/project")), + thread_id, + }); + + assert!(ignored); + let cell = match app_event_rx.try_recv() { + Ok(AppEvent::InsertHistoryCell(cell)) => cell, + other => panic!("expected info message after same-thread resume, saw {other:?}"), + }; + let rendered = lines_to_single_string(&cell.display_lines(/*width*/ 80)); + assert!(rendered.contains(&format!( + "Already viewing {}.", + test_path_display("/tmp/project") + ))); +} + +#[tokio::test] +async fn ignore_same_thread_resume_allows_reattaching_displayed_inactive_thread() { + let mut app = make_test_app().await; + let thread_id = ThreadId::new(); + let session = test_thread_session(thread_id, test_path_buf("/tmp/project")); + app.chat_widget.handle_thread_session(session); + + let ignored = app.ignore_same_thread_resume(&crate::resume_picker::SessionTarget { + path: Some(test_path_buf("/tmp/project")), + thread_id, + }); + + assert!(!ignored); + assert!(app.transcript_cells.is_empty()); +} diff --git a/codex-rs/tui/src/app/thread_routing.rs b/codex-rs/tui/src/app/thread_routing.rs index f25398b0b..931931956 100644 --- a/codex-rs/tui/src/app/thread_routing.rs +++ b/codex-rs/tui/src/app/thread_routing.rs @@ -1269,6 +1269,15 @@ impl App { ) } + pub(super) fn should_prepare_startup_tooltip_override( + session_selection: &SessionSelection, + ) -> bool { + matches!( + session_selection, + SessionSelection::StartFresh | SessionSelection::Exit + ) + } + pub(super) fn should_prompt_for_paused_goal_after_startup_resume( session_selection: &SessionSelection, initial_prompt: &Option, 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 deleted file mode 100644 index 470beccf0..000000000 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__goal_slash_command_oversized_objective_error.snap +++ /dev/null @@ -1,5 +0,0 @@ ---- -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/goal_validation.rs b/codex-rs/tui/src/chatwidget/tests/goal_validation.rs index b18b43392..b186cdf7f 100644 --- a/codex-rs/tui/src/chatwidget/tests/goal_validation.rs +++ b/codex-rs/tui/src/chatwidget/tests/goal_validation.rs @@ -120,7 +120,12 @@ async fn goal_slash_command_rejects_oversized_objective() { "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!(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); } diff --git a/codex-rs/tui/tests/suite/mod.rs b/codex-rs/tui/tests/suite/mod.rs index b205ead32..7e19b10f2 100644 --- a/codex-rs/tui/tests/suite/mod.rs +++ b/codex-rs/tui/tests/suite/mod.rs @@ -1,6 +1,4 @@ // Aggregates all former standalone integration tests as modules. -mod model_availability_nux; -mod no_panic_on_startup; mod resize_reflow; mod status_indicator; mod vt100_history; diff --git a/codex-rs/tui/tests/suite/model_availability_nux.rs b/codex-rs/tui/tests/suite/model_availability_nux.rs deleted file mode 100644 index 836dfe7ea..000000000 --- a/codex-rs/tui/tests/suite/model_availability_nux.rs +++ /dev/null @@ -1,226 +0,0 @@ -use std::collections::HashMap; -use std::time::Duration; - -use anyhow::Context; -use anyhow::Result; -use codex_models_manager::bundled_models_response; -use core_test_support::responses; -use core_test_support::skip_if_no_network; -use serde_json::Value as JsonValue; -use tempfile::tempdir; -use tokio::select; -use tokio::time::sleep; -use tokio::time::timeout; -use wiremock::MockServer; - -#[tokio::test] -async fn resume_startup_does_not_consume_model_availability_nux_count() -> Result<()> { - // run_codex_cli() does not work on Windows due to PTY limitations. - if cfg!(windows) { - return Ok(()); - } - skip_if_no_network!(Ok(())); - - let repo_root = codex_utils_cargo_bin::repo_root()?; - let codex_home = tempdir()?; - - let mut source_catalog: JsonValue = serde_json::to_value(bundled_models_response()?)?; - let models = source_catalog - .get_mut("models") - .and_then(JsonValue::as_array_mut) - .context("models array missing")?; - for model in models.iter_mut() { - if let Some(object) = model.as_object_mut() { - object.remove("availability_nux"); - } - } - let first_model = models.first_mut().context("models array is empty")?; - let first_model_object = first_model - .as_object_mut() - .context("first model was not a JSON object")?; - let model_slug = first_model_object - .get("slug") - .and_then(JsonValue::as_str) - .context("first model missing slug")? - .to_string(); - first_model_object.insert( - "availability_nux".to_string(), - serde_json::json!({ - "message": "Model now available", - }), - ); - - let custom_catalog_path = codex_home.path().join("catalog.json"); - std::fs::write( - &custom_catalog_path, - serde_json::to_string(&source_catalog)?, - )?; - - let repo_root_display = repo_root.display(); - let catalog_display = custom_catalog_path.display(); - let config_contents = format!( - r#"model = "{model_slug}" -model_provider = "openai" -model_catalog_json = "{catalog_display}" - -[projects."{repo_root_display}"] -trust_level = "trusted" - -[tui.model_availability_nux] -"{model_slug}" = 1 -"# - ); - std::fs::write(codex_home.path().join("config.toml"), config_contents)?; - - let server = MockServer::start().await; - let sse = responses::sse(vec![ - responses::ev_response_created("resp-seed-session"), - responses::ev_assistant_message("msg-seed-session", "seed session response"), - responses::ev_completed("resp-seed-session"), - ]); - let _response_mock = responses::mount_sse_once(&server, sse).await; - let openai_base_url_config = format!("openai_base_url=\"{}/v1\"", server.uri()); - let codex = if let Ok(path) = codex_utils_cargo_bin::cargo_bin("codex") { - path - } else { - let fallback = repo_root.join("codex-rs/target/debug/codex"); - if fallback.is_file() { - fallback - } else { - eprintln!("skipping integration test because codex binary is unavailable"); - return Ok(()); - } - }; - - let exec_output = std::process::Command::new(&codex) - .arg("exec") - .arg("--skip-git-repo-check") - .arg("-c") - .arg(&openai_base_url_config) - .arg("-C") - .arg(&repo_root) - .arg("seed session for resume") - .env("CODEX_HOME", codex_home.path()) - .env("OPENAI_API_KEY", "dummy") - .output() - .context("failed to execute codex exec")?; - anyhow::ensure!( - exec_output.status.success(), - "codex exec failed: {}", - String::from_utf8_lossy(&exec_output.stderr) - ); - - let mut env = HashMap::new(); - env.insert( - "CODEX_HOME".to_string(), - codex_home.path().display().to_string(), - ); - env.insert("OPENAI_API_KEY".to_string(), "dummy".to_string()); - - let args = vec![ - "resume".to_string(), - "--last".to_string(), - "--no-alt-screen".to_string(), - "-C".to_string(), - repo_root.display().to_string(), - "-c".to_string(), - "analytics.enabled=false".to_string(), - "-c".to_string(), - openai_base_url_config, - ]; - - let spawned = codex_utils_pty::spawn_pty_process( - codex.to_string_lossy().as_ref(), - &args, - &repo_root, - &env, - &None, - codex_utils_pty::TerminalSize::default(), - ) - .await?; - - let mut output = Vec::new(); - let codex_utils_pty::SpawnedProcess { - session, - stdout_rx, - stderr_rx, - exit_rx, - } = spawned; - let mut output_rx = codex_utils_pty::combine_output_receivers(stdout_rx, stderr_rx); - let mut exit_rx = exit_rx; - let writer_tx = session.writer_sender(); - let interrupt_writer = writer_tx.clone(); - let mut startup_ready = false; - let mut answered_cursor_query = false; - - let exit_code_result = timeout(Duration::from_secs(30), async { - loop { - select! { - result = output_rx.recv() => match result { - Ok(chunk) => { - let has_cursor_query = chunk.windows(4).any(|window| window == b"\x1b[6n"); - if has_cursor_query { - let _ = writer_tx.send(b"\x1b[1;1R".to_vec()).await; - answered_cursor_query = true; - } - output.extend_from_slice(&chunk); - if !startup_ready && answered_cursor_query && !has_cursor_query { - startup_ready = true; - for _ in 0..4 { - let _ = interrupt_writer.send(vec![3]).await; - sleep(Duration::from_millis(500)).await; - } - } - } - Err(tokio::sync::broadcast::error::RecvError::Closed) => break exit_rx.await, - Err(tokio::sync::broadcast::error::RecvError::Lagged(_)) => {} - }, - result = &mut exit_rx => break result, - } - } - }) - .await; - - let exit_code = match exit_code_result { - Ok(Ok(code)) => code, - Ok(Err(err)) => return Err(err.into()), - Err(_) => { - session.terminate(); - anyhow::bail!("timed out waiting for codex resume to exit"); - } - }; - let output_text = String::from_utf8_lossy(&output); - let rendered_output = { - let mut parser = vt100::Parser::new( - /*rows*/ 24, /*cols*/ 80, /*scrollback_len*/ 0, - ); - parser.process(&output); - parser.screen().contents() - }; - let interrupted_during_terminal_startup = { - let trimmed_output = rendered_output.trim(); - trimmed_output.is_empty() - || trimmed_output - .chars() - .all(|character| character == '^' || character == 'C' || character.is_whitespace()) - }; - anyhow::ensure!( - exit_code == 0 - || exit_code == 130 - || (exit_code == 1 && interrupted_during_terminal_startup), - "unexpected exit code from codex resume: {exit_code}; output: {output_text}", - ); - - let config_contents = std::fs::read_to_string(codex_home.path().join("config.toml"))?; - let config: toml::Value = toml::from_str(&config_contents)?; - let shown_count = config - .get("tui") - .and_then(|tui| tui.get("model_availability_nux")) - .and_then(|nux| nux.get(&model_slug)) - .and_then(toml::Value::as_integer) - .context("missing tui.model_availability_nux count")?; - - assert_eq!(shown_count, 1); - - Ok(()) -} diff --git a/codex-rs/tui/tests/suite/no_panic_on_startup.rs b/codex-rs/tui/tests/suite/no_panic_on_startup.rs deleted file mode 100644 index 6b9a36969..000000000 --- a/codex-rs/tui/tests/suite/no_panic_on_startup.rs +++ /dev/null @@ -1,127 +0,0 @@ -use std::collections::HashMap; -use std::path::Path; -use std::time::Duration; -use tokio::select; -use tokio::time::timeout; - -/// Regression test for https://github.com/openai/codex/issues/8803. -#[tokio::test] -#[ignore = "TODO(mbolin): flaky"] -async fn malformed_rules_should_not_panic() -> anyhow::Result<()> { - // run_codex_cli() does not work on Windows due to PTY limitations. - if cfg!(windows) { - return Ok(()); - } - - let tmp = tempfile::tempdir()?; - let codex_home = tmp.path(); - std::fs::write( - codex_home.join("rules"), - "rules should be a directory not a file", - )?; - - // TODO(mbolin): Figure out why using a temp dir as the cwd causes this test - // to hang. - let cwd = std::env::current_dir()?; - let config_contents = format!( - r#" -# Pick a local provider so the CLI doesn't prompt for OpenAI auth in this test. -model_provider = "ollama" - -[projects] -"{cwd}" = {{ trust_level = "trusted" }} -"#, - cwd = cwd.display() - ); - std::fs::write(codex_home.join("config.toml"), config_contents)?; - - let CodexCliOutput { exit_code, output } = run_codex_cli(codex_home, cwd).await?; - assert_ne!(0, exit_code, "Codex CLI should exit nonzero."); - assert!( - output.contains("ERROR: Failed to initialize codex:"), - "expected startup error in output, got: {output}" - ); - assert!( - output.contains("failed to read rules files"), - "expected rules read error in output, got: {output}" - ); - Ok(()) -} - -struct CodexCliOutput { - exit_code: i32, - output: String, -} - -async fn run_codex_cli( - codex_home: impl AsRef, - cwd: impl AsRef, -) -> anyhow::Result { - let codex_cli = codex_utils_cargo_bin::cargo_bin("codex")?; - let mut env = HashMap::new(); - env.insert( - "CODEX_HOME".to_string(), - codex_home.as_ref().display().to_string(), - ); - - let args = vec!["-c".to_string(), "analytics.enabled=false".to_string()]; - let spawned = codex_utils_pty::spawn_pty_process( - codex_cli.to_string_lossy().as_ref(), - &args, - cwd.as_ref(), - &env, - &None, - codex_utils_pty::TerminalSize::default(), - ) - .await?; - let mut output = Vec::new(); - let codex_utils_pty::SpawnedProcess { - session, - stdout_rx, - stderr_rx, - exit_rx, - } = spawned; - let mut output_rx = codex_utils_pty::combine_output_receivers(stdout_rx, stderr_rx); - let mut exit_rx = exit_rx; - let writer_tx = session.writer_sender(); - let exit_code_result = timeout(Duration::from_secs(10), async { - // Read PTY output until the process exits while replying to cursor - // position queries so the TUI can initialize without a real terminal. - loop { - select! { - result = output_rx.recv() => match result { - Ok(chunk) => { - // The TUI asks for the cursor position via ESC[6n. - // Respond with a valid position to unblock startup. - if chunk.windows(4).any(|window| window == b"\x1b[6n") { - let _ = writer_tx.send(b"\x1b[1;1R".to_vec()).await; - } - output.extend_from_slice(&chunk); - } - Err(tokio::sync::broadcast::error::RecvError::Closed) => break exit_rx.await, - Err(tokio::sync::broadcast::error::RecvError::Lagged(_)) => {} - }, - result = &mut exit_rx => break result, - } - } - }) - .await; - let exit_code = match exit_code_result { - Ok(Ok(code)) => code, - Ok(Err(err)) => return Err(err.into()), - Err(_) => { - session.terminate(); - anyhow::bail!("timed out waiting for codex CLI to exit"); - } - }; - // Drain any output that raced with the exit notification. - while let Ok(chunk) = output_rx.try_recv() { - output.extend_from_slice(&chunk); - } - - let output = String::from_utf8_lossy(&output); - Ok(CodexCliOutput { - exit_code, - output: output.to_string(), - }) -}