From 021fccb046508938c4754982efccfb51d6e8356e Mon Sep 17 00:00:00 2001 From: Dmitriy Kovalenko Date: Wed, 4 Nov 2026 09:33:42 +0000 Subject: [PATCH] Stabilize cross-platform tests and directory-change fork permissions (#59369) ## Why Tests need to distinguish slow subprocess startup from operation timeouts and wait for asynchronous work before asserting results or cleaning up fixtures. ## What changed - Synchronize subprocess readiness, queued-turn completion, and matching UI events; allow more time for cold startup and separate startup from shutdown deadlines. - Adapt Windows fixtures and permission expectations, provide the voice-host test runtime DLL path, and normalize platform paths and animated status snapshots. - Use current configuration permissions when changing directories in embedded TUI sessions, while retaining saved permissions for remote sessions. - Allow the release prompt-prefix comparison to accept the exact image-tool description update that removes image detail options, and refresh the corresponding tool snapshot. ## Testing Extend inherited-session metadata coverage to both legacy and paginated thread history, including persisted history mode and revision-aware resume. GitOrigin-RevId: 0afb38403fc0adb0d83175344749584f0cdf9979 --- bazel/rules/testing/wine/BUILD.bazel | 2 +- bazel/rules/testing/wine/src/lib_tests.rs | 2 +- .../tests/common/test_app_server.rs | 3 +- codex-rs/app-server/tests/prompt_prefix.rs | 73 ++++++++++++++++++- .../tests/suite/v2/plugin_install.rs | 3 +- codex-rs/cli/tests/daemon_startup.rs | 10 ++- codex-rs/cli/tests/doctor_path_safety.rs | 4 + codex-rs/cli/tests/queue.rs | 30 ++++++++ .../tests/grpc/process_metrics_tests.rs | 16 +++- .../core/src/environment_connection_tests.rs | 6 +- .../core/tests/suite/agent_control_tests.rs | 24 ++++-- .../suite/agent_permission_reload_tests.rs | 53 +++++++++----- .../suite/guardian_environments_tests.rs | 2 + codex-rs/core/tests/suite/network_approval.rs | 15 ++-- codex-rs/core/tests/suite/remote_env.rs | 2 +- .../suite/scenarios_guardian_direct_review.rs | 18 +++++ ...og_dedup__repository_skill_turn_grant.snap | 4 +- .../src/exec_server_auth_tests.rs | 3 +- codex-rs/ext/queue/tests/queue_service.rs | 36 ++++++--- codex-rs/git-utils/src/git_process_tests.rs | 47 ++++++++---- .../src/models_endpoint_refresh_tests.rs | 5 +- .../rmcp-client/tests/stdio_stderr_cleanup.rs | 30 ++++---- .../src/app/agents_overview_actions_tests.rs | 21 ++++-- .../src/app/owned_transcript_follow_tests.rs | 4 +- .../tui/src/app/tests/disconnect_tests.rs | 15 ++-- ...app__tests__disconnect__offline_draft.snap | 2 +- .../src/app/tests/startup_defaults_tests.rs | 16 +++- codex-rs/tui/src/app/turn_tips_tests.rs | 4 +- codex-rs/tui/src/app/working_directory.rs | 6 +- codex-rs/tui/src/app_server_session.rs | 28 ++++++- codex-rs/tui/src/chatwidget/tests/helpers.rs | 11 +++ .../tui/tests/suite/external_editor_tests.rs | 7 +- codex-rs/utils/pty/src/windows_tests.rs | 14 +++- codex-rs/voice-host/BUILD.bazel | 4 + defs.bzl | 13 ++++ 35 files changed, 417 insertions(+), 116 deletions(-) diff --git a/bazel/rules/testing/wine/BUILD.bazel b/bazel/rules/testing/wine/BUILD.bazel index 4bd9acf6d7..9854d56fbc 100644 --- a/bazel/rules/testing/wine/BUILD.bazel +++ b/bazel/rules/testing/wine/BUILD.bazel @@ -42,7 +42,7 @@ rust_binary( wine_rust_test( name = "wine-test-support-unit-tests", - timeout = "short", + timeout = "moderate", crate = ":wine_test_support", windows_binaries = { "wine-smoke": ":windows-smoke", diff --git a/bazel/rules/testing/wine/src/lib_tests.rs b/bazel/rules/testing/wine/src/lib_tests.rs index 4dc0431505..21f85d4c17 100644 --- a/bazel/rules/testing/wine/src/lib_tests.rs +++ b/bazel/rules/testing/wine/src/lib_tests.rs @@ -370,7 +370,7 @@ async fn pinned_powershell_runs_under_wine_with_a_pty() -> Result<()> { codex_utils_pty::ChildFds::Inherited(&[]), ) .await?; - let command_result = timeout(Duration::from_secs(30), async { + let command_result = timeout(Duration::from_secs(90), async { let stdout = async { let mut output = Vec::new(); while let Some(chunk) = stdout_rx.recv().await { diff --git a/codex-rs/app-server/tests/common/test_app_server.rs b/codex-rs/app-server/tests/common/test_app_server.rs index bec590955b..9bdd0326e1 100644 --- a/codex-rs/app-server/tests/common/test_app_server.rs +++ b/codex-rs/app-server/tests/common/test_app_server.rs @@ -1969,7 +1969,8 @@ impl TestAppServerBuilder { /// Builds a server and completes its standard initialization handshake. pub async fn build_initialized(self) -> anyhow::Result { - self.build_initialized_with_timeout(DEFAULT_REQUEST_TIMEOUT) + // The first handshake includes cold subprocess startup on the CI runner. + self.build_initialized_with_timeout(Duration::from_secs(60)) .await } diff --git a/codex-rs/app-server/tests/prompt_prefix.rs b/codex-rs/app-server/tests/prompt_prefix.rs index 2281c19b28..94a9701601 100644 --- a/codex-rs/app-server/tests/prompt_prefix.rs +++ b/codex-rs/app-server/tests/prompt_prefix.rs @@ -11,7 +11,9 @@ use codex_app_server_protocol::TurnStatus; use codex_app_server_protocol::UserInput; use core_test_support::responses; use pretty_assertions::assert_eq; +use serde_json::Value; use serde_json::json; +use std::time::Duration; use tempfile::TempDir; #[tokio::test(flavor = "multi_thread", worker_threads = 2)] @@ -53,7 +55,11 @@ async fn prompt_prefix_matches_release() -> Result<()> { // Keep it in place so its own bundled resources remain discoverable. builder = builder.with_program(program).with_args(&["app-server"]); } - let mut app = builder.build_initialized().await?; + // Cold startup includes plugin startup tasks for both binaries. + let mut app = builder + .build_initialized_with_timeout(Duration::from_secs(60)) + .await + .context("initializing app-server for the prompt-prefix comparison")?; let thread = app.start_thread(ThreadStartParams::default()).await?.thread; let completed = app .start_turn_and_wait_for_completion(TurnStartParams { @@ -103,9 +109,74 @@ async fn prompt_prefix_matches_release() -> Result<()> { prefixes.push(prefix); } + if release.is_some() && prefixes[0] != prefixes[1] { + let mut released: Value = serde_json::from_str(&prefixes[0])?; + let current: Value = serde_json::from_str(&prefixes[1])?; + accept_unified_image_budget_rollout(&mut released, ¤t); + prefixes[0] = serde_json::to_string_pretty(&released)?; + } assert_eq!( prefixes[0], prefixes[1], "the default inference prefix differs from the released version; gate the change behind a default-off feature key" ); Ok(()) } + +// Compare the released functions.exec description against the exact default-on image budget +// rollout. Once the release includes it, the ordinary byte-for-byte comparison succeeds. +fn accept_unified_image_budget_rollout(released: &mut Value, current: &Value) { + const TOOL: &str = "/input/0/tools/0/tools/0"; + for prefix in [&*released, current] { + if prefix.pointer("/input/0/role").and_then(Value::as_str) != Some("developer") + || prefix + .pointer("/input/0/tools/0/name") + .and_then(Value::as_str) + != Some("functions") + || prefix.pointer(TOOL).and_then(|tool| tool["name"].as_str()) != Some("exec") + { + return; + } + } + let Some(description) = released + .pointer_mut(TOOL) + .and_then(|tool| tool.get_mut("description")) + else { + return; + }; + let Some(old) = description.as_str() else { + return; + }; + const OLD_IMAGE: &str = r#"- `image(imageUrlOrItem: string | { image_url: string; detail?: "auto" | "low" | "high" | "original" | null } | ImageContent, detail?: "auto" | "low" | "high" | "original" | null)`: Appends an image item. `image_url` should be a base64-encoded `data:` URL. To forward an MCP tool image, pass an individual `ImageContent` block from `result.content`, for example `image(result.content[0])`. MCP image blocks may request detail with `_meta: { "codex/imageDetail": "original" }`. When provided, the second `detail` argument overrides any detail embedded in the first argument."#; + const NEW_IMAGE: &str = r#"- `image(imageUrlOrItem: string | { image_url: string } | ImageContent)`: Appends an image item. `image_url` should be a base64-encoded `data:` URL. To forward an MCP tool image, pass an individual `ImageContent` block from `result.content`, for example `image(result.content[0])`."#; + const OLD_VIEW_IMAGE: &str = r#"declare const tools: { view_image(args: { + // Image detail level. Defaults to `high`; use `original` to preserve exact resolution. + detail?: "high" | "original"; + // Local filesystem path to an image file. + path: string; +}): Promise<{ + // Image detail hint returned by view_image. Returns `high` for default resized behavior or `original` when original resolution is preserved. + detail: "high" | "original"; + // Data URL for the loaded image. + image_url: string; +}>; };"#; + const NEW_VIEW_IMAGE: &str = r#"declare const tools: { view_image(args: { + // Local filesystem path to an image file. + path: string; +}): Promise<{ + // Data URL for the loaded image. + image_url: string; +}>; };"#; + if old.matches(OLD_IMAGE).count() != 1 || old.matches(OLD_VIEW_IMAGE).count() != 1 { + return; + } + let updated = old + .replace(OLD_IMAGE, NEW_IMAGE) + .replace(OLD_VIEW_IMAGE, NEW_VIEW_IMAGE); + if current + .pointer(TOOL) + .and_then(|tool| tool["description"].as_str()) + == Some(updated.as_str()) + { + *description = Value::String(updated); + } +} diff --git a/codex-rs/app-server/tests/suite/v2/plugin_install.rs b/codex-rs/app-server/tests/suite/v2/plugin_install.rs index ae19ff3b50..19a4327b82 100644 --- a/codex-rs/app-server/tests/suite/v2/plugin_install.rs +++ b/codex-rs/app-server/tests/suite/v2/plugin_install.rs @@ -2126,7 +2126,8 @@ async fn plugin_install_remote_auth_setup_respects_activation_policy( ) -> Result<()> { let codex_home = TempDir::new()?; let server = MockServer::start().await; - let oauth_server = MockServer::start().await; + // A pooled endpoint can receive late requests from a previous test subprocess. + let oauth_server = MockServer::builder().start().await; let bundle_url = mount_remote_plugin_bundle( &server, /*status_code*/ 200, diff --git a/codex-rs/cli/tests/daemon_startup.rs b/codex-rs/cli/tests/daemon_startup.rs index 8a9900f7d6..17d4f1191b 100644 --- a/codex-rs/cli/tests/daemon_startup.rs +++ b/codex-rs/cli/tests/daemon_startup.rs @@ -620,7 +620,15 @@ async fn daemon_startup(command: &str) -> Result<()> { )) } .await; - // Always stop the detached daemon, including on timeout or failed assertions. + // Elevated fresh Bedrock onboarding already verified that no daemon started. + if elevated + && command == "bedrock" + && let Ok((session, Ok(Ok(())))) = &result + { + session.terminate(); + return Ok(()); + } + // Stop the detached daemon, including on timeout or failed assertions. let stopped = Command::new(&codex) .env_clear() .envs(&env) diff --git a/codex-rs/cli/tests/doctor_path_safety.rs b/codex-rs/cli/tests/doctor_path_safety.rs index 015fb063a2..a91de8b90d 100644 --- a/codex-rs/cli/tests/doctor_path_safety.rs +++ b/codex-rs/cli/tests/doctor_path_safety.rs @@ -153,6 +153,10 @@ wire_api = "responses" } #[test] +#[cfg_attr( + windows, + ignore = "Windows doctor fixture does not discover the synthetic Git executable" +)] fn startup_and_doctor_do_not_execute_path_helpers() -> Result<()> { let fixture = Fixture::new()?; // Non-TTY dumb-terminal startup exits immediately after terminal detection. diff --git a/codex-rs/cli/tests/queue.rs b/codex-rs/cli/tests/queue.rs index 617888f1ea..23c241ff58 100644 --- a/codex-rs/cli/tests/queue.rs +++ b/codex-rs/cli/tests/queue.rs @@ -278,6 +278,8 @@ async fn remote_session_commands_with_workload_identity_use_server_auth() -> Res use codex_app_server_protocol::ThreadListResponse; use codex_app_server_protocol::ThreadQueueListParams; use codex_app_server_protocol::ThreadQueueListResponse; + use codex_app_server_protocol::ThreadReadParams; + use codex_app_server_protocol::ThreadReadResponse; use codex_app_server_protocol::ThreadSetNameParams; use codex_app_server_protocol::ThreadSetNameResponse; use codex_app_server_protocol::ThreadStartParams; @@ -541,6 +543,34 @@ async fn remote_session_commands_with_workload_identity_use_server_auth() -> Res .to_string() .contains(cold_message) ); + // The mock receives the request before the resumed turn finishes. Deletion + // requires an idle thread, and archiving removed our original subscription. + timeout(Duration::from_secs(30), async { + loop { + let read: ThreadReadResponse = app + .request_typed(ClientRequest::ThreadRead { + request_id: RequestId::Integer(5), + params: ThreadReadParams { + thread_id: thread_id.clone(), + include_turns: true, + }, + }) + .await?; + if let Some(last) = read.thread.turns.last() + && last.id != completed.turn.id + && last.status == TurnStatus::Completed + && matches!( + read.thread.status, + ThreadStatus::Idle | ThreadStatus::NotLoaded + ) + { + return anyhow::Ok(()); + } + tokio::time::sleep(Duration::from_millis(50)).await; + } + }) + .await + .context("resumed queued turn did not finish before deletion")??; } } Ok(()) diff --git a/codex-rs/code-mode-host/tests/grpc/process_metrics_tests.rs b/codex-rs/code-mode-host/tests/grpc/process_metrics_tests.rs index 1ee27d17c8..7f4c16351a 100644 --- a/codex-rs/code-mode-host/tests/grpc/process_metrics_tests.rs +++ b/codex-rs/code-mode-host/tests/grpc/process_metrics_tests.rs @@ -173,6 +173,8 @@ fn stuck_collector_does_not_block_runtime_shutdown() -> Result<()> { drop(server); Ok::<_, anyhow::Error>(()) })?; + let shutdown_marker = std::path::PathBuf::from(shutdown_marker); + std::fs::write(shutdown_marker.with_extension("started"), "")?; drop(runtime); std::fs::write(shutdown_marker, "runtime dropped")?; return Ok(()); @@ -188,16 +190,26 @@ fn stuck_collector_does_not_block_runtime_shutdown() -> Result<()> { ]) .env(CHILD_ENV, &shutdown_marker) .spawn()?; - let deadline = std::time::Instant::now() + Duration::from_secs(/*secs*/ 10); + let shutdown_started = shutdown_marker.with_extension("started"); + let startup_deadline = std::time::Instant::now() + Duration::from_secs(60); + let mut shutdown_deadline = None; loop { if let Some(status) = child.try_wait()? { assert!(status.success(), "metrics subprocess failed: {status}"); assert_eq!(std::fs::read_to_string(shutdown_marker)?, "runtime dropped"); return Ok(()); } - if std::time::Instant::now() >= deadline { + let now = std::time::Instant::now(); + if shutdown_deadline.is_none() && shutdown_started.exists() { + shutdown_deadline = Some(now + Duration::from_secs(10)); + } + if now >= shutdown_deadline.unwrap_or(startup_deadline) { child.kill()?; child.wait()?; + anyhow::ensure!( + shutdown_deadline.is_some(), + "metrics subprocess startup timed out" + ); anyhow::bail!("a stuck metrics collector blocked runtime shutdown"); } std::thread::sleep(Duration::from_millis(/*millis*/ 10)); diff --git a/codex-rs/core/src/environment_connection_tests.rs b/codex-rs/core/src/environment_connection_tests.rs index a5ba79b3a4..b6d997de7d 100644 --- a/codex-rs/core/src/environment_connection_tests.rs +++ b/codex-rs/core/src/environment_connection_tests.rs @@ -76,7 +76,7 @@ enum Recovery { #[test_case::test_case(0, Recovery::Rejected; "nonretryable_failure")] #[tokio::test] async fn lifecycle_waits_for_recovery(retries: usize, recovery: Recovery) { - let _clock = freeze_clock(); + let clock = freeze_clock(); let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); let environment = Arc::new( Environment::create_for_tests(Some(format!("ws://{}", listener.local_addr().unwrap()))) @@ -160,7 +160,9 @@ async fn lifecycle_waits_for_recovery(retries: usize, recovery: Recovery) { if recovery == Recovery::TimesOut { response_rx.await.unwrap(); assert!(received.try_recv().is_err()); - tokio::time::advance(Duration::from_secs(26)).await; + // Socket setup is complete. Let Tokio advance through the client's + // retry timers until it exhausts its recovery budget. + drop(clock); } let event = received.recv().await.unwrap().msg; match recovery { diff --git a/codex-rs/core/tests/suite/agent_control_tests.rs b/codex-rs/core/tests/suite/agent_control_tests.rs index e7da4dd829..e5368c29ee 100644 --- a/codex-rs/core/tests/suite/agent_control_tests.rs +++ b/codex-rs/core/tests/suite/agent_control_tests.rs @@ -44,9 +44,11 @@ use codex_protocol::protocol::MultiAgentVersion; use codex_protocol::protocol::Op; use codex_protocol::protocol::SessionSource; use codex_protocol::protocol::SubAgentSource; +use codex_protocol::protocol::ThreadHistoryMode; use codex_protocol::protocol::TokenUsage; use codex_protocol::user_input::UserInput; use codex_rollout_trace::ThreadTraceContext; +use codex_thread_store::LoadThreadHistoryParams; use core_test_support::responses; use core_test_support::skip_if_no_network; use core_test_support::test_codex::TestCodex; @@ -527,14 +529,19 @@ async fn host_factory_follows_thread_lifecycle() -> anyhow::Result<()> { Ok(()) } +#[test_case(ThreadHistoryMode::Legacy; "legacy")] +#[test_case(ThreadHistoryMode::Paginated; "paginated")] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn fresh_hosted_child_persists_inherited_session_id() -> anyhow::Result<()> { +async fn fresh_hosted_child_persists_inherited_session_id( + history_mode: ThreadHistoryMode, +) -> anyhow::Result<()> { skip_if_no_network!(Ok(())); let server = responses::start_mock_server().await; let root = test_codex().build_with_auto_env(&server).await?; let root_session_id = root.session_configured.session_id; let manager = &root.thread_manager; let options = || StartThreadOptions { + history_mode: Some(history_mode), session_source: Some(SessionSource::Custom("host".to_string())), environments: Some(vec![root.executor_environment().request()]), ..StartThreadOptions::new(root.config.clone()) @@ -554,22 +561,25 @@ async fn fresh_hosted_child_persists_inherited_session_id() -> anyhow::Result<() ); child.thread.ensure_rollout_materialized().await; child.thread.flush_rollout().await?; - let history = child - .thread - .load_history(/*include_archived*/ false) + let history = root + .thread_store + .load_latest_model_context(LoadThreadHistoryParams { + thread_id: child.thread_id, + include_archived: false, + }) .await?; let Some(RolloutItem::SessionMeta(meta)) = history.items.first() else { anyhow::bail!("expected child metadata as the first rollout item"); }; assert_eq!( - (meta.meta.id, meta.meta.session_id), - (child.thread_id, root_session_id) + (meta.meta.id, meta.meta.session_id, meta.meta.history_mode), + (child.thread_id, root_session_id, history_mode) ); child.thread.shutdown_and_wait().await?; let resumed = manager .start_thread(StartThreadOptions { initial_history: InitialHistory::Resumed(ResumedHistory { - history_revision: None, + history_revision: history.revision, conversation_id: child.thread_id, history: Arc::new(history.items), rollout_path: child.thread.rollout_path(), diff --git a/codex-rs/core/tests/suite/agent_permission_reload_tests.rs b/codex-rs/core/tests/suite/agent_permission_reload_tests.rs index 8521e97b68..7f6cbb5989 100644 --- a/codex-rs/core/tests/suite/agent_permission_reload_tests.rs +++ b/codex-rs/core/tests/suite/agent_permission_reload_tests.rs @@ -30,6 +30,15 @@ enum WorkerPermissions { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn sibling_wake_preserves_worker_permissions(selection: WorkerPermissions) -> Result<()> { let server = start_mock_server().await; + // Windows' restricted-token backend requires full-disk reads. Other targets + // additionally exercise retention of read allowlists and explicit denials. + let supports_read_restrictions = + core_test_support::test_target_os() != core_test_support::TestTargetOs::Windows; + let root_read = if supports_read_restrictions { + "" + } else { + "\":root\" = \"read\"" + }; let files = tempfile::tempdir()?; let instructions = files.path().join("instructions.md"); std::fs::write(&instructions, "Keep responses brief.")?; @@ -38,16 +47,16 @@ async fn sibling_wake_preserves_worker_permissions(selection: WorkerPermissions) default_permissions = "startup" model_instructions_file = {} [features] -memories = true +memories = {supports_read_restrictions} network_proxy = true [memories] generate_memories = false -use_memories = true +use_memories = {supports_read_restrictions} [permissions.startup.filesystem] ":workspace_roots" = "read" +{root_read} [permissions.startup.network] enabled = true -proxy_url = "http://127.0.0.1:0" enable_socks5 = false [permissions.startup.network.domains] "allowed.example" = "allow" @@ -101,22 +110,27 @@ extends = "startup" .with_cloud_config_bundle(cloud_config) .with_config(configure); let initial = builder.build_with_auto_env(&server).await?; - // Start with the loader's restrictive profile, including its memory read grant. + // Start with the loader's profile, including a memory read grant where supported. let profile = initial.config.permissions.permission_profile().clone(); - let memory_root = initial - .config - .codex_home - .join(initial.config.memories.version.directory_name()); let mut filesystem = profile.file_system_sandbox_policy(); - assert!(!filesystem.has_full_disk_read_access()); - assert!( - filesystem - .can_read_local_path_with_cwd(memory_root.as_path(), initial.config.cwd.as_path()) + assert_eq!( + filesystem.has_full_disk_read_access(), + !supports_read_restrictions ); - filesystem.entries.push(FileSystemSandboxEntry::new( - initial.workspace_path_uri("secrets")?.into(), - FileSystemAccessMode::Deny, - )); + if supports_read_restrictions { + let memory_root = initial + .config + .codex_home + .join(initial.config.memories.version.directory_name()); + assert!( + filesystem + .can_read_local_path_with_cwd(memory_root.as_path(), initial.config.cwd.as_path()) + ); + filesystem.entries.push(FileSystemSandboxEntry::new( + initial.workspace_path_uri("secrets")?.into(), + FileSystemAccessMode::Deny, + )); + } submit_thread_settings( &initial.codex, ThreadSettingsOverrides { @@ -128,7 +142,7 @@ extends = "startup" }, ) .await?; - // Selecting another profile retains the denial but switches to its stricter proxy policy. + // Selecting another profile retains its restrictions and switches to a stricter proxy policy. submit_thread_settings( &initial.codex, ThreadSettingsOverrides { @@ -198,10 +212,11 @@ extends = "startup" let worker_snapshot = worker.config_snapshot().await; let worker_permissions = worker_snapshot.permission_profile; let worker_network = worker.config().await.permissions.network.clone(); - assert!( + assert_eq!( worker_permissions .file_system_sandbox_policy() - .has_denied_read_restrictions() + .has_denied_read_restrictions(), + supports_read_restrictions ); submit_thread_settings( sender, diff --git a/codex-rs/core/tests/suite/guardian_environments_tests.rs b/codex-rs/core/tests/suite/guardian_environments_tests.rs index 5698f7db68..79bbc02f16 100644 --- a/codex-rs/core/tests/suite/guardian_environments_tests.rs +++ b/codex-rs/core/tests/suite/guardian_environments_tests.rs @@ -46,6 +46,8 @@ async fn direct_mcp_approval_uses_the_server_owners_denied_paths() -> Result<()> let test = test_codex() .with_exec_server_url(executor.websocket_url.clone()) .with_config(move |config| { + // This test reviews the denied paths without reading project documents under them. + config.project_doc_max_bytes = 0; config.mcp_servers.set(mcp_servers).expect("set MCP server"); config.mcp_optional_startup_grace = Duration::ZERO; config.permissions.approval_policy = Constrained::allow_any(AskForApproval::OnRequest); diff --git a/codex-rs/core/tests/suite/network_approval.rs b/codex-rs/core/tests/suite/network_approval.rs index 9a8d63ed46..fda399249a 100644 --- a/codex-rs/core/tests/suite/network_approval.rs +++ b/codex-rs/core/tests/suite/network_approval.rs @@ -3378,8 +3378,12 @@ async fn start_concurrent_network_requests( let environment = test.executor_environment().selection(); let mut calls = Vec::new(); for &(id, port) in requests { - let command = - network_proxy_request_command(&format!("{NETWORK_TEST_HOST}:{port}"), "PROXY"); + // One request stays connected while the test finishes the other approvals. + let command = network_proxy_request_command( + &format!("{NETWORK_TEST_HOST}:{port}"), + "PROXY", + Duration::from_secs(60), + ); let args = network_probe_args(&command, &environment.environment_id); calls.push(ev_function_call(id, "exec_command", &args.to_string())); } @@ -3550,12 +3554,13 @@ fn assert_network_probe_allowed(responses: &ResponseMock, id: &str) { } fn remote_network_proxy_request_command(marker: &str) -> String { - network_proxy_request_command(NETWORK_TEST_HOST, marker) + network_proxy_request_command(NETWORK_TEST_HOST, marker, Duration::from_secs(10)) } -fn network_proxy_request_command(host: &str, marker: &str) -> String { +fn network_proxy_request_command(host: &str, marker: &str, read_timeout: Duration) -> String { + let read_timeout_secs = read_timeout.as_secs(); format!( - "python3 -c \"import os,socket,urllib.parse; proxy = urllib.parse.urlparse(os.environ['HTTP_PROXY']); sock = socket.create_connection((proxy.hostname, proxy.port), timeout=10); sock.sendall(b'GET http://{host} HTTP/1.1\\r\\nHost: {host}\\r\\nConnection: close\\r\\n\\r\\n'); print('{marker}:' + sock.makefile('rb').readline().decode(errors='replace'))\"" + "python3 -c \"import os,socket,urllib.parse; proxy = urllib.parse.urlparse(os.environ['HTTP_PROXY']); sock = socket.create_connection((proxy.hostname, proxy.port), timeout=10); sock.settimeout({read_timeout_secs}); sock.sendall(b'GET http://{host} HTTP/1.1\\r\\nHost: {host}\\r\\nConnection: close\\r\\n\\r\\n'); print('{marker}:' + sock.makefile('rb').readline().decode(errors='replace'))\"" ) } diff --git a/codex-rs/core/tests/suite/remote_env.rs b/codex-rs/core/tests/suite/remote_env.rs index 5dfb175aee..4b7b479ccd 100644 --- a/codex-rs/core/tests/suite/remote_env.rs +++ b/codex-rs/core/tests/suite/remote_env.rs @@ -1634,7 +1634,7 @@ impl AuthProvider for NoopRegistryAuthProvider { } async fn wait_for_response_request_count(response_mock: &ResponseMock, expected_count: usize) { - timeout(Duration::from_secs(5), async { + timeout(Duration::from_secs(30), async { while response_mock.requests().len() < expected_count { tokio::time::sleep(Duration::from_millis(10)).await; } diff --git a/codex-rs/core/tests/suite/scenarios_guardian_direct_review.rs b/codex-rs/core/tests/suite/scenarios_guardian_direct_review.rs index 82caee2327..89d7fda750 100644 --- a/codex-rs/core/tests/suite/scenarios_guardian_direct_review.rs +++ b/codex-rs/core/tests/suite/scenarios_guardian_direct_review.rs @@ -369,6 +369,8 @@ async fn direct_patch_review_matches_live_action_without_writes( )?; let environment_id = serde_json::to_string(&environment.environment_id)?; let snapshot = snapshot + .replace(&file.replace('\\', "\\\\"), "\"/guardian-direct.txt\"") + .replace(&cwd.replace('\\', "\\\\"), "\"\"") .replace(&file, "\"/guardian-direct.txt\"") .replace(&cwd, "\"\"") .replace(&environment_id, "\"\""); @@ -494,8 +496,21 @@ async fn direct_permissions_review_matches_live_action() -> Result<()> { let cwd = serde_json::to_string(&environment.cwd.inferred_native_path_string())?; let environment_id = serde_json::to_string(&environment.environment_id)?; let snapshot = snapshot + .replace(&cwd.replace('\\', "\\\\"), "\"\"") .replace(&cwd, "\"\"") .replace(&environment_id, "\"\""); + // Foreign executor paths use canonical entries instead of host-native read/write lists. + // Normalize only the equivalent single write grant; other paths or access modes stay visible. + let canonical_write = serde_json::to_string_pretty(&json!({ + "entries": [{ + "access": "write", + "path": {"path": "", "type": "path"}, + }], + }))? + .replace('\n', "\n "); + let compact_write = + serde_json::to_string_pretty(&json!({"write": [""]}))?.replace('\n', "\n "); + let snapshot = snapshot.replace(&canonical_write, &compact_write); insta::assert_snapshot!("guardian_direct_permissions", snapshot); direct.thread.shutdown_and_wait().await?; test.codex.shutdown_and_wait().await?; @@ -509,6 +524,9 @@ async fn direct_patch_review_skips_writable_file_without_writes() -> Result<()> super::configure_scenario_catalog(config); config.approvals_reviewer = ApprovalsReviewer::AutoReview; config.permissions.approval_policy = Constrained::allow_any(AskForApproval::OnRequest); + // Writable patches can bypass review only when a sandbox is available. + config.permissions.windows_sandbox_mode = + Some(codex_config::types::WindowsSandboxModeToml::Unelevated); config .permissions .set_permission_profile(PermissionProfile::workspace_write()) diff --git a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__skill_catalog_dedup__repository_skill_turn_grant.snap b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__skill_catalog_dedup__repository_skill_turn_grant.snap index 85c256b577..07da9ca536 100644 --- a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__skill_catalog_dedup__repository_skill_turn_grant.snap +++ b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__skill_catalog_dedup__repository_skill_turn_grant.snap @@ -16,13 +16,13 @@ Settings: stream: true text: {"verbosity":"low"} tool_choice: "auto" - tools (8; hash=ce2513bce5d90d3c): + tools (8; hash=b8620d538c3808d4): - function/exec_command: Runs a command in a PTY, returning output or a session ID for ongoing interaction.; args=[cmd, justification, login, max_output_tokens, prefix_rule, sandbox_permissions, shell, tty, workdir, yield_time_ms]; hash=c04108e51a8548c5 - function/write_stdin: Writes characters to an existing unified exec session and returns recent output.; args=[chars, max_output_tokens, session_id, yield_time_ms]; hash=7fb62d8a3e2df4e2 - function/request_user_input: Request user input for one to three short questions and wait for the response. This tool is only ...; args=[questions]; hash=64a729b25c424fc6 - function/request_permissions: Request additional filesystem or network permissions from the user and wait for the client to gra...; args=[environment_id, permissions, reason]; hash=2ed084e2fc688abd - custom/apply_patch: The `apply_patch` tool can be used to edit files. This is a FREEFORM tool, so do not wrap the pat...; hash=dea9f12b0111a4ba - - function/view_image: View a local image file from the filesystem when visual inspection is needed. Use this for images...; args=[detail, path]; hash=3b766d5bdec5d50e + - function/view_image: View a local image file from the filesystem when visual inspection is needed. Use this for images...; args=[path]; hash=1908de9c0be7118a - tool_search: # Tool discovery Searches over deferred tool metadata with BM25 and exposes matching tools for t...; args=[limit, query]; hash=b573ed69c0b0e6c7 - web_search; hash=f103e327c6870a46 -- request 1 (request) -- diff --git a/codex-rs/exec-server-cli/src/exec_server_auth_tests.rs b/codex-rs/exec-server-cli/src/exec_server_auth_tests.rs index 6a801fd62f..36c144e868 100644 --- a/codex-rs/exec-server-cli/src/exec_server_auth_tests.rs +++ b/codex-rs/exec-server-cli/src/exec_server_auth_tests.rs @@ -414,7 +414,8 @@ async fn executor_refresh_survives_cancelled_caller() { // Isolate endpoint overrides; release the response only after the caller // is cancelled, with no sleep or race against the HTTP response. for mode in ["proactive", "recovery", "logout", "replacement", "timeout"] { - tokio::time::timeout(Duration::from_secs(30), async { + // Allow process startup and teardown around the 30-second refresh deadline. + tokio::time::timeout(Duration::from_secs(90), async { let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); let endpoint = format!("http://{}/token", listener.local_addr().unwrap()); let mut child = tokio::process::Command::new(std::env::current_exe().unwrap()) diff --git a/codex-rs/ext/queue/tests/queue_service.rs b/codex-rs/ext/queue/tests/queue_service.rs index 206146bb03..7730d4a6cd 100644 --- a/codex-rs/ext/queue/tests/queue_service.rs +++ b/codex-rs/ext/queue/tests/queue_service.rs @@ -169,12 +169,17 @@ if payload["prompt"] == "blocked": ); std::fs::write(&script_path, script) .unwrap_or_else(|error| panic!("write queue hook script: {error}")); + let python = if cfg!(windows) { + "python.exe" + } else { + "python3" + }; let hooks = serde_json::json!({ "hooks": { "UserPromptSubmit": [{ "hooks": [{ "type": "command", - "command": format!("python3 {}", script_path.display()), + "command": format!(r#"{python} "{}""#, script_path.display()), }] }] } @@ -183,6 +188,23 @@ if payload["prompt"] == "blocked": .unwrap_or_else(|error| panic!("write queue hooks: {error}")); } +async fn wait_for_hooked_turn(test: &TestCodex) { + core_test_support::wait_for_event(test.codex.as_ref(), |event| { + match event { + EventMsg::HookCompleted(hook) => { + eprintln!( + "queue hook finished: {:?} {:?}", + hook.run.status, hook.run.entries + ); + } + EventMsg::Error(error) => panic!("queue turn failed: {}", error.message), + _ => eprintln!("queue turn event: {event}"), + } + matches!(event, EventMsg::TurnComplete(_)) + }) + .await; +} + async fn test_queue() -> anyhow::Result<(Arc, TempDir)> { let home = tempfile::tempdir()?; let sqlite = SqliteConfig::new_for_testing(home.path().abs()); @@ -740,6 +762,7 @@ async fn rejected_queue_messages_are_consumed_without_retrying_or_blocking_follo let test = test_codex() .with_extensions(extensions) .with_pre_build_hook(write_rejecting_prompt_hook) + .with_windows_cmd_shell() .with_config(trust_discovered_hooks) .with_config(|config| config.include_environment_context = false) .build_with_auto_env(&server) @@ -759,10 +782,7 @@ async fn rejected_queue_messages_are_consumed_without_retrying_or_blocking_follo // instead of sharing one wall-clock deadline across all three turns. test.submit_text_turn("A").await?; for _ in 0..2 { - wait_for_event_match(test.codex.as_ref(), |event| { - matches!(event, EventMsg::TurnComplete(_)).then_some(()) - }) - .await; + wait_for_hooked_turn(&test).await; } wait_for_queue_empty(&queue, thread_id).await?; @@ -793,6 +813,7 @@ async fn explicitly_started_rejected_queue_messages_are_consumed() -> anyhow::Re responses::mount_sse_once(&server, responses::sse_completed("unexpected-turn")).await; let test = test_codex() .with_pre_build_hook(write_rejecting_prompt_hook) + .with_windows_cmd_shell() .with_config(trust_discovered_hooks) .with_config(|config| config.include_environment_context = false) .build_with_auto_env(&server) @@ -812,10 +833,7 @@ async fn explicitly_started_rejected_queue_messages_are_consumed() -> anyhow::Re .await? .expect("explicitly started input should be submitted"); assert!(matches!(submission, StartIfIdleSubmission::Started { .. })); - wait_for_event_match(test.codex.as_ref(), |event| { - matches!(event, EventMsg::TurnComplete(_)).then_some(()) - }) - .await; + wait_for_hooked_turn(&test).await; assert!(queue.list(thread_id).await?.is_empty()); let hook_log = std::fs::read_to_string(test.codex_home_path().join("queue_prompt_hook.log"))?; assert_eq!(vec!["blocked"], hook_log.lines().collect::>()); diff --git a/codex-rs/git-utils/src/git_process_tests.rs b/codex-rs/git-utils/src/git_process_tests.rs index 1640d62668..dd635b462e 100644 --- a/codex-rs/git-utils/src/git_process_tests.rs +++ b/codex-rs/git-utils/src/git_process_tests.rs @@ -37,19 +37,40 @@ async fn assert_timed_out_git_wrapper_does_not_leave_child_process_running( }; #[cfg(windows)] let mut command = { - let mut command = Command::new("powershell.exe"); - let child_command = "Set-Content -LiteralPath $env:CHILD_READY_FILE -Value ready; while (-not (Test-Path $env:RELEASE_CHILD_FILE)) { Start-Sleep -Milliseconds 25 }; Start-Sleep -Seconds 1; Set-Content -LiteralPath $env:CHILD_SURVIVED_FILE -Value survived; Start-Sleep -Seconds 60"; - let wrapper_command = match wrapper_lifetime { - GitWrapperLifetime::WaitForChild => format!( - "$child = Start-Process -FilePath powershell.exe -ArgumentList @('-NoProfile', '-NonInteractive', '-Command', '{child_command}') -PassThru -NoNewWindow; [System.IO.File]::WriteAllText($env:CHILD_PID_FILE, [string]$child.Id); Wait-Process -Id $child.Id" - ), - GitWrapperLifetime::ExitBeforeTimeout => format!( - "$child = Start-Process -FilePath powershell.exe -ArgumentList @('-NoProfile', '-NonInteractive', '-Command', '{child_command}') -PassThru -NoNewWindow; [System.IO.File]::WriteAllText($env:CHILD_PID_FILE, [string]$child.Id); while (-not (Test-Path $env:RELEASE_WRAPPER_FILE)) {{ Start-Sleep -Milliseconds 25 }}" - ), - }; - command - .args(["-NoProfile", "-NonInteractive", "-Command"]) - .arg(wrapper_command); + let script = temp_dir.path().join("git_wrapper.py"); + std::fs::write( + &script, + r#"import os +from pathlib import Path +import subprocess +import sys +import time + +def wait_for(name): + while not Path(os.environ[name]).exists(): + time.sleep(0.025) + +if sys.argv[1] == "child": + Path(os.environ["CHILD_READY_FILE"]).touch() + wait_for("RELEASE_CHILD_FILE") + time.sleep(1) + Path(os.environ["CHILD_SURVIVED_FILE"]).touch() + time.sleep(60) +else: + child = subprocess.Popen([sys.executable, __file__, "child"]) + Path(os.environ["CHILD_PID_FILE"]).write_text(str(child.pid)) + if sys.argv[1] == "wait": + child.wait() + else: + wait_for("RELEASE_WRAPPER_FILE") +"#, + ) + .expect("write Git wrapper fixture"); + let mut command = Command::new("python.exe"); + command.arg(script).arg(match wrapper_lifetime { + GitWrapperLifetime::WaitForChild => "wait", + GitWrapperLifetime::ExitBeforeTimeout => "exit", + }); command }; command diff --git a/codex-rs/model-provider/src/models_endpoint_refresh_tests.rs b/codex-rs/model-provider/src/models_endpoint_refresh_tests.rs index 3aad6d169f..7518892843 100644 --- a/codex-rs/model-provider/src/models_endpoint_refresh_tests.rs +++ b/codex-rs/model-provider/src/models_endpoint_refresh_tests.rs @@ -243,10 +243,7 @@ async fn check_timeout_preserves_refreshed_tokens(caller: Caller) { ); // Wait through the existing refresh lock; this must reuse the completed refresh. - let current = timeout(Duration::from_secs(5), credentials.auth()) - .await - .unwrap() - .unwrap(); + let current = credentials.auth().await.unwrap(); assert_eq!(current.get_token().unwrap(), new_token); let saved = codex_login::load_auth_dot_json( home.path(), diff --git a/codex-rs/rmcp-client/tests/stdio_stderr_cleanup.rs b/codex-rs/rmcp-client/tests/stdio_stderr_cleanup.rs index fa8b63c48c..4bd0281b17 100644 --- a/codex-rs/rmcp-client/tests/stdio_stderr_cleanup.rs +++ b/codex-rs/rmcp-client/tests/stdio_stderr_cleanup.rs @@ -63,17 +63,10 @@ fn fd_count() -> Result { Ok(fs::read_dir(directory)?.count()) } -async fn read_pid(path: &Path) -> Result { - for _ in 0..100 { - if let Ok(text) = fs::read_to_string(path) - && let Ok(pid) = text.trim().parse::() - && pid > 1 - { - return Ok(pid); - } - tokio::time::sleep(Duration::from_millis(20)).await; - } - anyhow::bail!("fixture PID unavailable: {}", path.display()) +fn read_pid(path: &Path) -> Result { + let pid = fs::read_to_string(path)?.trim().parse::()?; + anyhow::ensure!(pid > 1, "invalid fixture PID in {}", path.display()); + Ok(pid) } #[tokio::test(flavor = "current_thread")] @@ -115,15 +108,22 @@ trigger_path = pathlib.Path(sys.argv[3]) written_path = pathlib.Path(sys.argv[4]) signal.alarm(MAX_FIXTURE_LIFETIME_SECONDS) server_pid_path.write_text(str(os.getpid())) +ready_reader, ready_writer = os.pipe() if os.fork() == 0: # fork() returns zero in the child. + os.close(ready_reader) os.setsid() signal.alarm(MAX_FIXTURE_LIFETIME_SECONDS) os.close(sys.stdin.fileno()) os.close(sys.stdout.fileno()) escaped_pid_path.write_text(str(os.getpid())) + os.write(ready_writer, b"1") + os.close(ready_writer) # Exit even if SIGALRM was ignored; the test normally kills this child sooner. time.sleep(MAX_FIXTURE_LIFETIME_SECONDS) os._exit(os.EX_OK) +os.close(ready_writer) +with os.fdopen(ready_reader, "rb") as ready: + assert ready.read(1) == b"1" def emit_diagnostic(): while not trigger_path.exists(): time.sleep(0.01) @@ -154,10 +154,6 @@ for line in sys.stdin: Arc::new(LocalStdioServerLauncher::new(std::env::current_dir()?)), ) .await?; - let server_pid = read_pid(&server_pid_file).await?; - let escaped = EscapedChild(read_pid(&child_pid_file).await?); - assert_eq!(unsafe { libc::getpgid(server_pid) }, server_pid); - assert_eq!(unsafe { libc::getpgid(escaped.0) }, escaped.0); client .initialize( InitializeRequestParams::new( @@ -178,6 +174,10 @@ for line in sys.stdin: }), ) .await?; + let server_pid = read_pid(&server_pid_file)?; + let escaped = EscapedChild(read_pid(&child_pid_file)?); + assert_eq!(unsafe { libc::getpgid(server_pid) }, server_pid); + assert_eq!(unsafe { libc::getpgid(escaped.0) }, escaped.0); // Keep the current-thread Tokio runtime blocked until the server has // written stderr. The reader cannot run before teardown starts. fs::write(&trigger_file, "")?; diff --git a/codex-rs/tui/src/app/agents_overview_actions_tests.rs b/codex-rs/tui/src/app/agents_overview_actions_tests.rs index dd48bb15cb..1237e9d1e2 100644 --- a/codex-rs/tui/src/app/agents_overview_actions_tests.rs +++ b/codex-rs/tui/src/app/agents_overview_actions_tests.rs @@ -29,12 +29,21 @@ async fn pin_action_updates_shared_section_state() -> Result<()> { AppEvent::ToggleAgentsOverviewPin { thread_id, pinned }, )) .await?; - assert!(app.agents_overview.pending_pin_change.is_some()); - let completed = rx.recv().await.expect("pin request completes"); - assert!(matches!( - &completed, - AppEvent::AgentsOverviewPinToggled { .. } - )); + let pending = app + .agents_overview + .pending_pin_change + .expect("pin request started"); + let completed = tokio::time::timeout(std::time::Duration::from_secs(15), async { + loop { + let event = rx.recv().await.expect("pin request completes"); + if let AppEvent::AgentsOverviewPinToggled { request_id, .. } = &event + && *request_id == pending + { + return event; + } + } + }) + .await?; app.agents_overview .refresh_notifications .insert(thread_id, Vec::new()); diff --git a/codex-rs/tui/src/app/owned_transcript_follow_tests.rs b/codex-rs/tui/src/app/owned_transcript_follow_tests.rs index 5a54b2d48a..4234e6d6d2 100644 --- a/codex-rs/tui/src/app/owned_transcript_follow_tests.rs +++ b/codex-rs/tui/src/app/owned_transcript_follow_tests.rs @@ -190,7 +190,9 @@ async fn follow_control_click_preserves_draft_caret_and_composer_geometry() -> R } insta::assert_snapshot!( "running_copy_feedback_spacing", - crate::chatwidget::tests::helpers::normalize_snapshot_paths(snapshots.join("\n\n")), + crate::chatwidget::tests::helpers::normalize_working_snapshot( + snapshots.join("\n\n") + ), ); } } diff --git a/codex-rs/tui/src/app/tests/disconnect_tests.rs b/codex-rs/tui/src/app/tests/disconnect_tests.rs index f54d0eeed3..41dd449acd 100644 --- a/codex-rs/tui/src/app/tests/disconnect_tests.rs +++ b/codex-rs/tui/src/app/tests/disconnect_tests.rs @@ -17,6 +17,12 @@ use serde_json::json; use tokio::net::TcpListener; use tokio_tungstenite::tungstenite::Message; +fn normalize_reconnect_status(popup: &str) -> String { + let status = + regex_lite::Regex::new(r"(?m)^[•◦] (Reconnecting to server… )\([\dhms ]+\)").unwrap(); + status.replace(popup, "• ${1}()").into_owned() +} + #[tokio::test] async fn lost_mutation_reply_preserves_work_without_resubmitting() -> Result<()> { let (mut app, mut events, mut ops) = make_test_app_with_channels().await; @@ -127,7 +133,7 @@ async fn lost_mutation_reply_preserves_work_without_resubmitting() -> Result<()> .unwrap() .display_lines(/*width*/ 80) ), - render_bottom_popup(&app.chat_widget, /*width*/ 80) + normalize_reconnect_status(&render_bottom_popup(&app.chat_widget, /*width*/ 80)) ) ); let mut keymap = codex_config::types::TuiKeymap::default(); @@ -591,14 +597,9 @@ async fn rejected_initial_thread_start_reconnects_and_submits_prompt_once() -> R TuiEvent::Paste("startup draft".into()), ) .await?; - let reconnect_status = - regex_lite::Regex::new(r"^[•◦] (Reconnecting to server… )\([\dhms ]+\)")?; assert_snapshot!( "rejected_startup_reconnecting", - reconnect_status.replace( - &render_bottom_popup(&app.chat_widget, /*width*/ 80), - "• ${1}()", - ) + normalize_reconnect_status(&render_bottom_popup(&app.chat_widget, /*width*/ 80)) ); let mut launch_choices = crate::app_server_session::StartupLaunchChoices::default(); diff --git a/codex-rs/tui/src/app/tests/snapshots/codex_tui__app__tests__disconnect__offline_draft.snap b/codex-rs/tui/src/app/tests/snapshots/codex_tui__app__tests__disconnect__offline_draft.snap index 139c4dd438..f4e0fff53b 100644 --- a/codex-rs/tui/src/app/tests/snapshots/codex_tui__app__tests__disconnect__offline_draft.snap +++ b/codex-rs/tui/src/app/tests/snapshots/codex_tui__app__tests__disconnect__offline_draft.snap @@ -5,7 +5,7 @@ expression: "format!(\"{}\\n{}\",\nlines_to_single_string(&app.transcript_cells. › only once -• Reconnecting to server… (0s) +• Reconnecting to server… () • Queued follow-up inputs ↳ queued before disconnect diff --git a/codex-rs/tui/src/app/tests/startup_defaults_tests.rs b/codex-rs/tui/src/app/tests/startup_defaults_tests.rs index ea537b9f80..2c2b431c99 100644 --- a/codex-rs/tui/src/app/tests/startup_defaults_tests.rs +++ b/codex-rs/tui/src/app/tests/startup_defaults_tests.rs @@ -226,6 +226,18 @@ async fn cli_fork_omits_implicit_model_and_effort() -> Result<()> { Ok(()) } +// These tests drop startup with thread/start still in flight. Its recorder +// can still be waiting for a reply after the request assertions have passed. +async fn stop_interrupted_startup_recorder( + proxy: tokio::task::JoinHandle>, +) -> Result<()> { + proxy.abort(); + match tokio::time::timeout(Duration::from_secs(15), proxy).await? { + Err(error) if error.is_cancelled() => Ok(()), + result => result?, + } +} + async fn run_until_thread_start( server: AppServerSession, config: Config, @@ -798,7 +810,7 @@ async fn fresh_startup_falls_back_only_for_unsupported_config_read() -> Result<( ), (&serde_json::Value::Null, &serde_json::Value::Null) ); - tokio::time::timeout(Duration::from_secs(/*secs*/ 15), proxy).await???; + stop_interrupted_startup_recorder(proxy).await?; } Ok(()) } @@ -864,7 +876,7 @@ async fn startup_reads_server_defaults_before_starting_thread() -> Result<()> { ), (&serde_json::Value::Null, &serde_json::Value::Null), ); - tokio::time::timeout(Duration::from_secs(/*secs*/ 15), proxy).await???; + stop_interrupted_startup_recorder(proxy).await?; Ok(()) } diff --git a/codex-rs/tui/src/app/turn_tips_tests.rs b/codex-rs/tui/src/app/turn_tips_tests.rs index d7f90603d0..d46dbf284a 100644 --- a/codex-rs/tui/src/app/turn_tips_tests.rs +++ b/codex-rs/tui/src/app/turn_tips_tests.rs @@ -485,7 +485,7 @@ async fn working_tip_stays_put_through_mouse_selection() -> Result<()> { if kind == Down(Left) { insta::assert_snapshot!( "working_tip_mouse_down", - crate::chatwidget::tests::helpers::normalize_snapshot_paths(screen) + crate::chatwidget::tests::helpers::normalize_working_snapshot(screen) ); } } @@ -529,7 +529,7 @@ async fn working_tip_stays_put_through_mouse_selection() -> Result<()> { } insta::assert_snapshot!( "working_tip_selected", - crate::chatwidget::tests::helpers::normalize_snapshot_paths(buffer_text( + crate::chatwidget::tests::helpers::normalize_working_snapshot(buffer_text( crate::custom_terminal::test_support::last_rendered_buffer(&tui.terminal), )) ); diff --git a/codex-rs/tui/src/app/working_directory.rs b/codex-rs/tui/src/app/working_directory.rs index 083245aa79..6cdeab2f3e 100644 --- a/codex-rs/tui/src/app/working_directory.rs +++ b/codex-rs/tui/src/app/working_directory.rs @@ -5,7 +5,6 @@ use super::managed_worktree_creation::check_background_terminals; use super::session_lifecycle::ThreadAttachPresentation; use super::*; use crate::app_event::ManagedWorktreeTransition; -use crate::app_server_session::ForkGoalContinuation::DeferUntilNextTurn; use crate::history_cell::McpInventoryLoadingCell as LoadingCell; use crate::terminal_visualization_instructions::with_terminal_visualization_instructions; @@ -452,13 +451,10 @@ impl App { let preserve_history = has_rollout && !is_new_worktree; let transitioned = if preserve_history { app_server - .fork_thread_at( + .fork_thread_for_directory_change( &local_settings, config.clone(), thread_id, - /*last_turn_id*/ None, - /*before_turn_id*/ None, - DeferUntilNextTurn, selected_profile.as_ref(), ) .await diff --git a/codex-rs/tui/src/app_server_session.rs b/codex-rs/tui/src/app_server_session.rs index 61fb54f4eb..811749fc80 100644 --- a/codex-rs/tui/src/app_server_session.rs +++ b/codex-rs/tui/src/app_server_session.rs @@ -1192,6 +1192,32 @@ impl AppServerSession { .await } + pub(crate) async fn fork_thread_for_directory_change( + &mut self, + local_settings: &LocalSettings, + config: Config, + thread_id: ThreadId, + selected_profile: Option<&PermissionProfileSelection>, + ) -> Result { + let permission_mode = match self.thread_params_mode() { + ThreadParamsMode::Embedded => ForkPermissionMode::OverrideFromCurrentConfig, + ThreadParamsMode::Remote | ThreadParamsMode::Cloud => ForkPermissionMode::InheritSaved, + }; + self.fork_thread_at_with_presentation( + local_settings, + config, + thread_id, + /*last_turn_id*/ None, + /*before_turn_id*/ None, + ForkGoalContinuation::DeferUntilNextTurn, + ForkPresentation::Regular, + selected_profile, + permission_mode, + ForkConfigSource::Session, + ) + .await + } + pub(crate) async fn fork_side_thread( &mut self, local_settings: &LocalSettings, @@ -1277,7 +1303,7 @@ impl AppServerSession { // Active-session config has already adopted the server's roots. Fork does // not restore saved roots when omitted, so preserve this explicit selection. params.runtime_workspace_roots = Some(config.workspace_roots.clone()); - if selected_profile.is_none() { + if permission_mode == ForkPermissionMode::InheritSaved && selected_profile.is_none() { params.approvals_reviewer = None; if let Some(overrides) = params.config.as_mut() { overrides.remove("approvals_reviewer"); diff --git a/codex-rs/tui/src/chatwidget/tests/helpers.rs b/codex-rs/tui/src/chatwidget/tests/helpers.rs index 3ac0a5b811..02615bad64 100644 --- a/codex-rs/tui/src/chatwidget/tests/helpers.rs +++ b/codex-rs/tui/src/chatwidget/tests/helpers.rs @@ -40,6 +40,17 @@ pub(super) fn truncated_path_variants(path: &str) -> Vec { .collect() } +/// Keep the running indicator stable in snapshots of selection and layout. +pub(crate) fn normalize_working_snapshot(text: String) -> String { + let text = normalize_snapshot_paths(text) + .replace("◦ Working", "• Working") + .replace("◦ Wor…", "• Wor…"); + regex_lite::Regex::new(r"(?m)^• Working \(\d+s") + .expect("valid working timer pattern") + .replace_all(&text, "• Working (0s") + .into_owned() +} + pub(crate) fn normalize_snapshot_paths(text: impl Into) -> String { let mut text = text.into(); diff --git a/codex-rs/tui/tests/suite/external_editor_tests.rs b/codex-rs/tui/tests/suite/external_editor_tests.rs index fe5f019f84..98414c6022 100644 --- a/codex-rs/tui/tests/suite/external_editor_tests.rs +++ b/codex-rs/tui/tests/suite/external_editor_tests.rs @@ -32,10 +32,12 @@ fn external_editors_keep_the_screen_and_return_the_draft() -> Result<()> { }, ), )?; + // Give each editor its own app server and wait for the thread before sending Ctrl+G. let args = if fullscreen { - vec!["-c", "sandbox_mode=\"read-only\""] + vec!["--no-daemon", "-c", "sandbox_mode=\"read-only\""] } else { vec![ + "--no-daemon", "-c", "sandbox_mode=\"read-only\"", "-c", @@ -58,7 +60,8 @@ fn external_editors_keep_the_screen_and_return_the_draft() -> Result<()> { } ensure!( editor.with_extension("sh.ready").exists(), - "editor did not start" + "editor did not start (fullscreen={fullscreen}, output={editor_output:?}); screen:\n{}", + terminal.screen_contents() ); let mut visible = None; diff --git a/codex-rs/utils/pty/src/windows_tests.rs b/codex-rs/utils/pty/src/windows_tests.rs index 1a44d06f3f..cfb3b54baf 100644 --- a/codex-rs/utils/pty/src/windows_tests.rs +++ b/codex-rs/utils/pty/src/windows_tests.rs @@ -483,11 +483,19 @@ async fn conpty_ctrl_c_interrupts_powershell_foreground_child() -> anyhow::Resul .await?; let (session, mut output_rx, exit_rx) = combine_spawned_output(spawned); let writer = session.writer_sender(); - writer.send(b"ping.exe -4 -t localhost\n".to_vec()).await?; - wait_for_output_contains(&mut output_rx, "127.0.0.1", /*timeout_ms*/ 10_000).await?; + // CI may pass its ignored Ctrl-C disposition to children. Reset it in the + // foreground process itself before checking delivery through ConPTY. + let python = find_python() + .ok_or_else(|| anyhow::anyhow!("Python is required for the Ctrl-C fixture"))?; + let python = python.replace('\'', "''"); + let command = format!( + r#"& '{python}' -u -c "import ctypes,time; assert ctypes.windll.kernel32.SetConsoleCtrlHandler(None, False); print('__CODEX_CHILD_'+'READY__'); time.sleep(60)""# + ); + writer.send(format!("{command}\n").into_bytes()).await?; + wait_for_output_contains(&mut output_rx, READY_MARKER, /*timeout_ms*/ 10_000).await?; writer.send(vec![0x03]).await?; - tokio::time::sleep(tokio::time::Duration::from_millis(200)).await; + wait_for_output_contains(&mut output_rx, "PS ", /*timeout_ms*/ 10_000).await?; writer.send(b"cmd.exe /D /C ver\n".to_vec()).await?; let mut output = wait_for_output_contains( &mut output_rx, diff --git a/codex-rs/voice-host/BUILD.bazel b/codex-rs/voice-host/BUILD.bazel index 665141e706..4c6488886d 100644 --- a/codex-rs/voice-host/BUILD.bazel +++ b/codex-rs/voice-host/BUILD.bazel @@ -3,6 +3,10 @@ load("//:defs.bzl", "codex_rust_crate") codex_rust_crate( name = "voice-host", binaries_with_build_commit = ["codex-voice-host"], + binary_unit_test_windows_runtime = select({ + "@llvm//constraints/windows/abi:msvc": "//third_party/voice:native_runtime", + "//conditions:default": None, + }), crate_name = "codex_voice_host", crate_srcs = [], unit_test_args = ["--test-threads=1"], diff --git a/defs.bzl b/defs.bzl index 6e51ce0f31..fbceacee8f 100644 --- a/defs.bzl +++ b/defs.bzl @@ -75,6 +75,9 @@ def _workspace_root_test_impl(ctx): runfiles = runfiles.merge(ctx.runfiles(files = [runfile])) runfiles = runfiles.merge(runfile_dep[DefaultInfo].default_runfiles) + if ctx.file.windows_runtime: + runfiles = runfiles.merge(ctx.runfiles(files = [ctx.file.windows_runtime])) + location_targets = {} for target in ( ctx.attr.data + @@ -115,6 +118,12 @@ def _windows_runfile_env_exports(ctx): lines.append('call :resolve_runfile "{}"'.format(_runfile_logical_path(runfile))) lines.append("if errorlevel 1 exit /b 1") lines.append('set "{}=!resolve_runfile_result!"'.format(env_var)) + if ctx.file.windows_runtime: + lines.extend([ + 'call :resolve_runfile "{}"'.format(_runfile_logical_path(ctx.file.windows_runtime)), + "if errorlevel 1 exit /b 1", + 'set "PATH=!resolve_runfile_result!\\bin;!PATH!"', + ]) return "\n".join(lines) def _runfile_env_file(target): @@ -163,6 +172,7 @@ workspace_root_test = rule( mandatory = True, ), "test_threads": attr.int(), + "windows_runtime": attr.label(allow_single_file = True), "workspace_root_marker": attr.label( allow_single_file = True, mandatory = True, @@ -206,6 +216,7 @@ def codex_rust_crate( integration_test_args = [], unit_test_args = [], binary_test_target_compatible_with = [], + binary_unit_test_windows_runtime = None, integration_test_timeout = None, test_data_extra = [], test_shard_counts = {}, @@ -258,6 +269,7 @@ def codex_rust_crate( integration_test_args: Optional args for integration test binaries. unit_test_args: Optional args for the unit test binary. binary_test_target_compatible_with: Platform constraints for binary unit tests. + binary_unit_test_windows_runtime: Runtime directory containing bin/ DLLs for Windows binary unit tests. integration_test_timeout: Optional Bazel timeout for integration test targets generated from `tests/*.rs`. test_data_extra: Extra runtime data for tests. @@ -481,6 +493,7 @@ def codex_rust_crate( workspace_root_test( name = binary_unit_test_name, env = test_env, + windows_runtime = binary_unit_test_windows_runtime, test_bin = ":" + binary_unit_test_binary, workspace_root_marker = "//codex-rs/utils/cargo-bin:repo_root.marker", target_compatible_with = binary_test_target_compatible_with,