diff --git a/codex-rs/core/src/session/turn.rs b/codex-rs/core/src/session/turn.rs index 2a044ec5e7..89af4c8707 100644 --- a/codex-rs/core/src/session/turn.rs +++ b/codex-rs/core/src/session/turn.rs @@ -479,11 +479,30 @@ async fn build_skills_and_plugins( let mentioned_plugins = collect_explicit_plugin_mentions(&user_input, loaded_plugins.capability_summaries()); let explicitly_mentioned_apps = collect_explicit_app_ids(&user_input); + let skills_outcome = turn_context.turn_skills.outcome.as_ref(); + // Skill injection can contain app references, and plain skill mentions share + // resolution space with apps. Preserve explicit-turn behavior by resolving + // app inventory before finalizing any selected skill. + let has_explicit_skill_selection = turn_context.apps_enabled() + && !collect_explicit_skill_mentions( + &user_input, + &skills_outcome.skills, + &skills_outcome.disabled_paths, + &HashMap::new(), + ) + .is_empty(); let mut explicitly_requested_mcp_servers = mentioned_plugins .iter() .flat_map(|plugin| plugin.mcp_server_names.iter().cloned()) .collect::>(); - if turn_context.apps_enabled() && !explicitly_mentioned_apps.is_empty() { + let mentioned_plugin_uses_apps = mentioned_plugins + .iter() + .any(|plugin| !plugin.app_connector_ids.is_empty()); + if turn_context.apps_enabled() + && (!explicitly_mentioned_apps.is_empty() + || mentioned_plugin_uses_apps + || has_explicit_skill_selection) + { explicitly_requested_mcp_servers.insert(CODEX_APPS_MCP_SERVER_NAME.to_string()); } if !explicitly_requested_mcp_servers.is_empty() { @@ -544,7 +563,6 @@ async fn build_skills_and_plugins( } else { Vec::new() }; - let skills_outcome = turn_context.turn_skills.outcome.as_ref(); let connector_slug_counts = build_connector_slug_counts(&available_connectors); let skill_name_counts_lower = build_skill_name_counts(&skills_outcome.skills, &skills_outcome.disabled_paths).1; diff --git a/codex-rs/core/tests/common/apps_test_server.rs b/codex-rs/core/tests/common/apps_test_server.rs index 7f7873e30b..e14828e42c 100644 --- a/codex-rs/core/tests/common/apps_test_server.rs +++ b/codex-rs/core/tests/common/apps_test_server.rs @@ -1,3 +1,5 @@ +use std::time::Duration; + use crate::test_codex::TestCodexBuilder; use crate::test_codex::test_codex; use anyhow::Result; @@ -72,6 +74,7 @@ impl AppsTestServer { CONNECTOR_DESCRIPTION.to_string(), /*searchable*/ true, /*include_app_only_tool*/ false, + /*tools_list_delay*/ None, ) .await; Ok(Self { @@ -82,6 +85,19 @@ impl AppsTestServer { pub async fn mount_with_connector_name( server: &MockServer, connector_name: &str, + ) -> Result { + Self::mount_with_connector_name_and_tools_list_delay( + server, + connector_name, + /*tools_list_delay*/ None, + ) + .await + } + + pub async fn mount_with_connector_name_and_tools_list_delay( + server: &MockServer, + connector_name: &str, + tools_list_delay: Option, ) -> Result { mount_oauth_metadata(server).await; mount_connectors_directory(server).await; @@ -91,6 +107,7 @@ impl AppsTestServer { CONNECTOR_DESCRIPTION.to_string(), /*searchable*/ false, /*include_app_only_tool*/ false, + tools_list_delay, ) .await; Ok(Self { @@ -110,6 +127,7 @@ impl AppsTestServer { CONNECTOR_DESCRIPTION.to_string(), matches!(tool_loading, AppsTestToolLoading::Searchable), /*include_app_only_tool*/ true, + /*tools_list_delay*/ None, ) .await; Ok(Self { @@ -264,6 +282,7 @@ async fn mount_streamable_http_json_rpc( connector_description: String, searchable: bool, include_app_only_tool: bool, + tools_list_delay: Option, ) { Mock::given(method("POST")) .and(path_regex("^/api/codex/apps/?$")) @@ -272,6 +291,7 @@ async fn mount_streamable_http_json_rpc( connector_description, searchable, include_app_only_tool, + tools_list_delay, }) .mount(server) .await; @@ -282,6 +302,7 @@ struct CodexAppsJsonRpcResponder { connector_description: String, searchable: bool, include_app_only_tool: bool, + tools_list_delay: Option, } impl Respond for CodexAppsJsonRpcResponder { @@ -475,7 +496,12 @@ impl Respond for CodexAppsJsonRpcResponder { } })); } - ResponseTemplate::new(200).set_body_json(response) + let response = ResponseTemplate::new(/*s*/ 200).set_body_json(response); + if let Some(delay) = self.tools_list_delay { + response.set_delay(delay) + } else { + response + } } "tools/call" => { let id = body.get("id").cloned().unwrap_or(Value::Null); diff --git a/codex-rs/core/tests/suite/plugins.rs b/codex-rs/core/tests/suite/plugins.rs index 596f32e198..3182992cb9 100644 --- a/codex-rs/core/tests/suite/plugins.rs +++ b/codex-rs/core/tests/suite/plugins.rs @@ -329,6 +329,113 @@ async fn explicit_plugin_mentions_inject_plugin_guidance() -> Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn explicit_app_only_plugin_mention_waits_for_pending_apps_startup() -> Result<()> { + skip_if_no_network!(Ok(())); + let server = start_mock_server().await; + let apps_server = AppsTestServer::mount_with_connector_name_and_tools_list_delay( + &server, + "Google Calendar", + Some(Duration::from_millis(/*millis*/ 250)), + ) + .await?; + let mock = mount_sse_once( + &server, + sse(vec![ev_response_created("resp-1"), ev_completed("resp-1")]), + ) + .await; + + let codex_home = Arc::new(TempDir::new()?); + write_plugin_app_plugin(codex_home.as_ref()); + let codex = + build_apps_enabled_plugin_test_codex(&server, codex_home, apps_server.chatgpt_base_url) + .await?; + + codex + .submit(Op::UserInput { + environments: None, + items: vec![codex_protocol::user_input::UserInput::Mention { + name: "sample".into(), + path: format!("plugin://{SAMPLE_PLUGIN_CONFIG_NAME}"), + }], + final_output_json_schema: None, + responsesapi_client_metadata: None, + additional_context: Default::default(), + thread_settings: Default::default(), + }) + .await?; + wait_for_event(&codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; + + let request = mock.single_request(); + let developer_messages = request.message_input_texts("developer"); + assert!( + developer_messages + .iter() + .any(|text| text.contains("Apps from this plugin")), + "expected plugin app guidance after delayed startup: {developer_messages:?}" + ); + assert!( + tool_names(&request.body_json()) + .iter() + .any(|name| name == "mcp__codex_apps__google_calendar"), + "expected explicitly invoked plugin app tools on the first turn" + ); + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn explicitly_selected_skill_waits_for_pending_apps_startup() -> Result<()> { + skip_if_no_network!(Ok(())); + let server = start_mock_server().await; + let apps_server = AppsTestServer::mount_with_connector_name_and_tools_list_delay( + &server, + "Google Calendar", + Some(Duration::from_millis(/*millis*/ 250)), + ) + .await?; + let mock = mount_sse_once( + &server, + sse(vec![ev_response_created("resp-1"), ev_completed("resp-1")]), + ) + .await; + + let codex_home = Arc::new(TempDir::new()?); + let skill_path = write_plugin_skill_plugin(codex_home.as_ref()); + std::fs::write( + &skill_path, + "---\ndescription: inspect sample data\n---\n\nUse [$calendar](app://calendar).\n", + ) + .expect("write plugin app skill"); + let codex = + build_apps_enabled_plugin_test_codex(&server, codex_home, apps_server.chatgpt_base_url) + .await?; + + codex + .submit(Op::UserInput { + environments: None, + items: vec![codex_protocol::user_input::UserInput::Skill { + name: "sample:sample-search".into(), + path: skill_path, + }], + final_output_json_schema: None, + responsesapi_client_metadata: None, + additional_context: Default::default(), + thread_settings: Default::default(), + }) + .await?; + wait_for_event(&codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; + + assert!( + tool_names(&mock.single_request().body_json()) + .iter() + .any(|name| name == "mcp__codex_apps__google_calendar"), + "expected app referenced by selected skill on the first turn" + ); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn explicit_plugin_mentions_track_plugin_used_analytics() -> Result<()> { skip_if_no_network!(Ok(()));