From 3b67b03a3f6b0590589fb85d2be26d0eb58ac159 Mon Sep 17 00:00:00 2001 From: felixxia-oai Date: Mon, 10 Aug 2026 14:38:41 +0000 Subject: [PATCH] Run plugin and skill tests on Windows (#37836) ## Why The plugin and skill test suites were excluded wholesale on Windows, even though most cases do not depend on POSIX behavior. ## What changed - Make generated TOML and JSON fixtures escape Windows paths correctly. - Use Windows-safe path canonicalization and normalize advertised skill paths. - Select test environments through the environment-aware builders. - Run the suites on Windows while narrowly skipping cases that execute POSIX commands or require matching host and executor path conventions. GitOrigin-RevId: aadf31013c2bf9bac728adc4dd96f572d4a3bf3f --- codex-rs/config/src/skills_config_tests.rs | 9 +- codex-rs/core/tests/suite/plugins.rs | 82 ++++++++----------- .../tests/suite/request_plugin_install.rs | 5 +- codex-rs/core/tests/suite/skills.rs | 19 +++-- codex-rs/ext/skills/src/host_service_tests.rs | 7 +- 5 files changed, 57 insertions(+), 65 deletions(-) diff --git a/codex-rs/config/src/skills_config_tests.rs b/codex-rs/config/src/skills_config_tests.rs index 6615f62c85..57ff1622bb 100644 --- a/codex-rs/config/src/skills_config_tests.rs +++ b/codex-rs/config/src/skills_config_tests.rs @@ -41,16 +41,15 @@ fn stack(codex_home: &TempDir, user: &str, session: &str) -> ConfigLayerStack { } fn path_toggle_config(path: &std::path::Path, enabled: bool) -> String { + let path = toml::Value::String(path.display().to_string()); format!( r#"[[skills.config]] -path = "{}" +path = {path} enabled = {enabled} -"#, - path.display() +"# ) } -#[cfg_attr(windows, ignore)] #[test] fn session_flags_can_reenable_user_disabled_path() { let codex_home = TempDir::new().expect("temp dir"); @@ -71,7 +70,6 @@ fn session_flags_can_reenable_user_disabled_path() { ); } -#[cfg_attr(windows, ignore)] #[test] fn session_flags_can_disable_user_enabled_path() { let codex_home = TempDir::new().expect("temp dir"); @@ -115,7 +113,6 @@ enabled = false ); } -#[cfg_attr(windows, ignore)] #[test] fn preserves_order_across_path_and_name_selectors() { let codex_home = TempDir::new().expect("temp dir"); diff --git a/codex-rs/core/tests/suite/plugins.rs b/codex-rs/core/tests/suite/plugins.rs index c50d8d5e0c..af940e3ef0 100644 --- a/codex-rs/core/tests/suite/plugins.rs +++ b/codex-rs/core/tests/suite/plugins.rs @@ -1,4 +1,3 @@ -#![cfg(not(target_os = "windows"))] #![allow(clippy::unwrap_used)] use std::sync::Arc; @@ -41,6 +40,7 @@ use core_test_support::responses::sse; use core_test_support::responses::start_mock_server; use core_test_support::skip_if_no_network; use core_test_support::skip_if_remote; +use core_test_support::skip_if_target_windows; use core_test_support::stdio_server_bin; use core_test_support::test_codex::TestCodex; use core_test_support::test_codex::local_selections; @@ -201,17 +201,16 @@ fn write_plugin_mcp_plugin(home: &TempDir, command: &str) { let plugin_root = write_sample_plugin_manifest_and_config(home); std::fs::write( plugin_root.join(".mcp.json"), - format!( - r#"{{ - "mcpServers": {{ - "sample": {{ - "command": "{command}", - "cwd": ".", - "startup_timeout_sec": 60.0 - }} - }} -}}"# - ), + serde_json::to_vec(&serde_json::json!({ + "mcpServers": { + "sample": { + "command": command, + "cwd": ".", + "startup_timeout_sec": 60.0, + }, + }, + })) + .expect("serialize plugin MCP configuration"), ) .expect("write plugin mcp config"); } @@ -271,10 +270,7 @@ async fn build_analytics_plugin_test_codex( .with_config(move |config| { config.chatgpt_base_url = chatgpt_base_url; }); - Ok(builder - .build(server) - .await - .expect("create new conversation")) + builder.build_with_auto_env(server).await } async fn build_apps_enabled_plugin_test_codex( @@ -292,10 +288,7 @@ async fn build_apps_enabled_plugin_test_codex( .expect("test config should allow feature update"); config.chatgpt_base_url = chatgpt_base_url; }); - Ok(builder - .build(server) - .await - .expect("create new conversation")) + builder.build_with_remote_and_local_env(server).await } async fn mount_plugin_tool_search_turn(server: &MockServer) -> ResponseMock { @@ -349,6 +342,7 @@ fn searched_plugin_tools( #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn persisted_remote_plugin_command_attribution_flows_through_turn_context() -> Result<()> { + skip_if_target_windows!(Ok(()), "executes a POSIX shell script"); skip_if_no_network!(Ok(())); skip_if_remote!( Ok(()), @@ -452,12 +446,11 @@ async fn agent_plugin_skills_use_shared_catalog_and_direct_child_discovery() -> ) .await; let codex_home = Arc::new(TempDir::new()?); - let skill_path = std::fs::canonicalize(write_agent_plugin_skill_plugin(codex_home.as_ref()))?; - let test_codex = test_codex() + let skill_path = dunce::canonicalize(write_agent_plugin_skill_plugin(codex_home.as_ref()))?; + let mut builder = test_codex() .with_home(Arc::clone(&codex_home)) - .with_extensions(skills_extensions()) - .build(&server) - .await?; + .with_extensions(skills_extensions()); + let test_codex = builder.build_with_auto_env(&server).await?; test_codex .codex @@ -625,12 +618,11 @@ async fn legacy_plugin_skill_prompt_remains_complete() -> Result<()> { "x".repeat(9_000) ); std::fs::write(&skill_path, &skill_contents)?; - let skill_path = std::fs::canonicalize(skill_path)?; - let test_codex = test_codex() + let skill_path = dunce::canonicalize(skill_path)?; + let mut builder = test_codex() .with_home(codex_home) - .with_extensions(skills_extensions()) - .build(&server) - .await?; + .with_extensions(skills_extensions()); + let test_codex = builder.build_with_auto_env(&server).await?; test_codex .codex @@ -718,19 +710,18 @@ async fn agent_plugin_root_mcp_stdio_tool_round_trip_expands_reserved_paths() -> plugin_root.join("mcp.json"), serde_json::to_vec_pretty(&mcp_config)?, )?; - let test_codex = test_codex() - .with_home(Arc::clone(&codex_home)) - .build(&server) - .await?; + let mut builder = test_codex().with_home(Arc::clone(&codex_home)); + let test_codex = builder.build_with_remote_and_local_env(&server).await?; wait_for_mcp_server(&test_codex.codex, "agent").await?; - let data_root = std::fs::read_dir(codex_home.path().join("plugins/data/agent-plugins"))? - .next() - .expect("Agent Plugin data root")? - .path() - .canonicalize()?; + let data_root = dunce::canonicalize( + std::fs::read_dir(codex_home.path().join("plugins/data/agent-plugins"))? + .next() + .expect("Agent Plugin data root")? + .path(), + )?; let expected_env = format!( "{}|{}", - plugin_root.canonicalize()?.display(), + dunce::canonicalize(&plugin_root)?.display(), data_root.display() ); @@ -1215,7 +1206,7 @@ async fn explicitly_requested_mcp_waits_for_startup(request: ExplicitMcpRequest) return Ok(()); } }; - let skill_path = std::fs::canonicalize(write_plugin_skill_plugin(codex_home.as_ref()))?; + let skill_path = dunce::canonicalize(write_plugin_skill_plugin(codex_home.as_ref()))?; write_plugin_mcp_plugin(codex_home.as_ref(), &rmcp_test_server_bin); write_plugin_app_plugin(codex_home.as_ref()); let initialize_barrier = block_plugin_mcp_startup(codex_home.as_ref(), &rmcp_test_server_bin); @@ -1229,10 +1220,7 @@ async fn explicitly_requested_mcp_waits_for_startup(request: ExplicitMcpRequest) .enable(Feature::Apps) .expect("test config should allow feature update"); }); - let test_codex = builder - .build(&server) - .await - .expect("create new conversation"); + let test_codex = builder.build_with_remote_and_local_env(&server).await?; let codex = Arc::clone(&test_codex.codex); let input = match request { @@ -1384,7 +1372,7 @@ async fn explicit_plugin_skill_invocation_tracks_remote_plugin_id() -> Result<() .await; let codex_home = Arc::new(TempDir::new()?); - let skill_path = std::fs::canonicalize(write_remote_plugin_skill_plugin(codex_home.as_ref()))?; + let skill_path = dunce::canonicalize(write_remote_plugin_skill_plugin(codex_home.as_ref()))?; persist_sample_remote_plugin_id(codex_home.as_ref()); let test_codex = build_analytics_plugin_test_codex(&server, codex_home).await?; let codex = Arc::clone(&test_codex.codex); @@ -1429,6 +1417,8 @@ enum ImplicitPluginSkillInvocation { async fn implicit_plugin_skill_invocation_tracks_remote_plugin_id( invocation: ImplicitPluginSkillInvocation, ) -> Result<()> { + skip_if_target_windows!(Ok(()), "executes POSIX cat and bash commands"); + skip_if_remote!(Ok(()), "shell commands use host plugin-cache paths"); skip_if_no_network!(Ok(())); let server = start_mock_server().await; let codex_home = Arc::new(TempDir::new()?); diff --git a/codex-rs/core/tests/suite/request_plugin_install.rs b/codex-rs/core/tests/suite/request_plugin_install.rs index f833071bcd..f37a412a1d 100644 --- a/codex-rs/core/tests/suite/request_plugin_install.rs +++ b/codex-rs/core/tests/suite/request_plugin_install.rs @@ -1,4 +1,3 @@ -#![cfg(not(target_os = "windows"))] #![allow(clippy::unwrap_used)] use anyhow::Context; @@ -159,7 +158,7 @@ async fn build_test( configure_apps_without_search_tool(config, apps_base_url.as_str()); } }); - builder.build(server).await + builder.build_with_auto_env(server).await } async fn build_gated_step_preparation_test( @@ -1029,7 +1028,7 @@ async fn endpoint_mode_with_no_eligible_candidates_exposes_no_suggestion_tools() )]; } }); - let test = builder.build(&server).await?; + let test = builder.build_with_auto_env(&server).await?; test.submit_turn("list tools").await?; diff --git a/codex-rs/core/tests/suite/skills.rs b/codex-rs/core/tests/suite/skills.rs index 35b19036b7..4dad451a89 100644 --- a/codex-rs/core/tests/suite/skills.rs +++ b/codex-rs/core/tests/suite/skills.rs @@ -1,4 +1,3 @@ -#![cfg(not(target_os = "windows"))] #![allow(clippy::unwrap_used)] use anyhow::Result; @@ -24,7 +23,7 @@ use core_test_support::responses::sse; use core_test_support::responses::start_mock_server; use core_test_support::skip_if_no_network; use core_test_support::skip_if_remote; -use core_test_support::skip_if_target_windows; +use core_test_support::skip_if_wine_exec; use core_test_support::test_codex::local_selections; use core_test_support::test_codex::test_codex; use core_test_support::test_codex::turn_permission_fields; @@ -55,8 +54,10 @@ async fn write_repo_skill( #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn user_turn_includes_skill_instructions() -> Result<()> { - // TODO(anp): Remove after skill-path helpers use target-native paths. - skip_if_target_windows!(Ok(()), "requires native cross-OS skill paths"); + skip_if_wine_exec!( + Ok(()), + "skill paths require matching host and executor path conventions" + ); skip_if_no_network!(Ok(())); let server = start_mock_server().await; @@ -219,7 +220,10 @@ async fn user_turn_selects_symlinked_skill_by_advertised_discovery_path() -> Res let request = mock.single_request(); let developer_texts = request.message_input_texts("developer"); - let advertised_path = format!("(file: {discovery_path_display})"); + let advertised_path = format!( + "(file: {})", + discovery_path.to_string_lossy().replace('\\', "/") + ); assert!( developer_texts .iter() @@ -243,7 +247,10 @@ async fn user_turn_selects_symlinked_skill_by_advertised_discovery_path() -> Res #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn idle_user_turn_includes_skill_instructions_in_the_first_request() -> Result<()> { - skip_if_target_windows!(Ok(()), "requires native cross-OS skill paths"); + skip_if_wine_exec!( + Ok(()), + "skill paths require matching host and executor path conventions" + ); skip_if_no_network!(Ok(())); let server = start_mock_server().await; diff --git a/codex-rs/ext/skills/src/host_service_tests.rs b/codex-rs/ext/skills/src/host_service_tests.rs index 0a031bb6c1..240f718816 100644 --- a/codex-rs/ext/skills/src/host_service_tests.rs +++ b/codex-rs/ext/skills/src/host_service_tests.rs @@ -143,12 +143,12 @@ fn config_stack_with_session_flags( } fn path_toggle_config(path: &std::path::Path, enabled: bool) -> String { + let path = toml::Value::String(path.display().to_string()); format!( r#"[[skills.config]] -path = "{}" +path = {path} enabled = {enabled} -"#, - path.display() +"# ) } @@ -908,7 +908,6 @@ async fn skills_for_cwd_uses_cached_result_until_force_reload() { ); } -#[cfg_attr(windows, ignore)] #[tokio::test] async fn skills_for_config_ignores_cwd_cache_when_session_flags_reenable_skill() { let codex_home = tempfile::tempdir().expect("tempdir");