From f65ea998c7536fffd89e4e4d4c90ab135eb8b0f9 Mon Sep 17 00:00:00 2001 From: stefanstokic-oai Date: Sat, 8 Aug 2026 03:46:25 +0000 Subject: [PATCH] Keep external agent detection from blocking config requests (#37528) ## Why External agent detection can spend time reading session transcripts. Sharing the configuration serialization queue causes unrelated configuration requests to wait for detection to finish. ## What changed - Give `externalAgentConfig/detect` its own global serialization queue. - Keep configuration reads, requirements reads, writes, and external agent imports responsive while detection is still running. ## Testing Add a Unix app-server test that blocks transcript detection on a FIFO and verifies the unrelated requests complete before detection is released. GitOrigin-RevId: d2764dcdb0e96d810c790c570c48fa45690a3dc1 --- .../src/protocol/common.rs | 2 +- .../tests/suite/v2/external_agent_config.rs | 123 ++++++++++++++++++ 2 files changed, 124 insertions(+), 1 deletion(-) diff --git a/codex-rs/app-server-protocol/src/protocol/common.rs b/codex-rs/app-server-protocol/src/protocol/common.rs index 62aa253bdb..2644966b3a 100644 --- a/codex-rs/app-server-protocol/src/protocol/common.rs +++ b/codex-rs/app-server-protocol/src/protocol/common.rs @@ -1179,7 +1179,7 @@ client_request_definitions! { }, ExternalAgentConfigDetect => "externalAgentConfig/detect" { params: v2::ExternalAgentConfigDetectParams, - serialization: global("config"), + serialization: global("external-agent-detect"), response: v2::ExternalAgentConfigDetectResponse, }, ExternalAgentConfigImport => "externalAgentConfig/import" { diff --git a/codex-rs/app-server/tests/suite/v2/external_agent_config.rs b/codex-rs/app-server/tests/suite/v2/external_agent_config.rs index b7b0d4915a..8f4e949017 100644 --- a/codex-rs/app-server/tests/suite/v2/external_agent_config.rs +++ b/codex-rs/app-server/tests/suite/v2/external_agent_config.rs @@ -8,6 +8,16 @@ use app_test_support::TestAppServer; use app_test_support::create_mock_responses_server_repeating_assistant; use app_test_support::start_analytics_events_server; use app_test_support::write_chatgpt_auth; +#[cfg(unix)] +use codex_app_server_protocol::ConfigReadParams; +#[cfg(unix)] +use codex_app_server_protocol::ConfigReadResponse; +#[cfg(unix)] +use codex_app_server_protocol::ConfigRequirementsReadResponse; +#[cfg(unix)] +use codex_app_server_protocol::ConfigValueWriteParams; +#[cfg(unix)] +use codex_app_server_protocol::ConfigWriteResponse; use codex_app_server_protocol::ExternalAgentConfigDetectResponse; use codex_app_server_protocol::ExternalAgentConfigImportCompletedNotification; use codex_app_server_protocol::ExternalAgentConfigImportHistoriesReadResponse; @@ -17,6 +27,8 @@ use codex_app_server_protocol::ExternalAgentConfigImportResponse; use codex_app_server_protocol::ExternalAgentConfigMigrationItemType; use codex_app_server_protocol::ExternalAgentImportedConnectorCandidate; use codex_app_server_protocol::ExternalAgentImportedConnectorSource; +#[cfg(unix)] +use codex_app_server_protocol::MergeStrategy; use codex_app_server_protocol::PluginListParams; use codex_app_server_protocol::PluginListResponse; use codex_app_server_protocol::RequestId; @@ -30,6 +42,8 @@ use codex_app_server_protocol::ThreadResumeResponse; use codex_app_server_protocol::TurnStartParams; use codex_app_server_protocol::TurnStartResponse; use codex_app_server_protocol::UserInput; +#[cfg(unix)] +use codex_app_server_protocol::WriteStatus; use codex_config::types::AuthCredentialsStoreMode; use core_test_support::responses; use pretty_assertions::assert_eq; @@ -127,6 +141,115 @@ async fn external_agent_config_detect_accepts_migration_source_and_defaults_unkn Ok(()) } +#[cfg(unix)] +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn external_agent_config_detect_does_not_block_configuration_reads() -> Result<()> { + let codex_home = TempDir::new()?; + let project_root = codex_home.path().join("repo"); + let session_dir = external_agent_home(codex_home.path()).join("projects/repo"); + let session_path = session_dir.join("session.jsonl"); + std::fs::create_dir_all(&project_root)?; + std::fs::create_dir_all(&session_dir)?; + let status = std::process::Command::new("mkfifo") + .arg(&session_path) + .status()?; + assert!(status.success()); + + let home_dir = codex_home.path().display().to_string(); + let mut mcp = TestAppServer::builder() + .with_codex_home(codex_home.path()) + .without_auto_env() + .with_env_overrides(&[("HOME", Some(home_dir.as_str()))]) + .build_initialized_with_timeout(DEFAULT_TIMEOUT) + .await?; + + let detect_request_id = mcp + .send_raw_request( + "externalAgentConfig/detect", + Some(serde_json::json!({ "includeHome": true })), + ) + .await?; + + // Opening the FIFO for writing succeeds only after detection opens it for + // reading. Keep the writer open without writing so transcript parsing stays + // blocked while unrelated configuration requests run. + let mut blocked_session_writer = timeout( + DEFAULT_TIMEOUT, + tokio::fs::OpenOptions::new() + .write(true) + .open(&session_path), + ) + .await??; + + let config_request_id = mcp + .send_config_read_request(ConfigReadParams { + include_layers: false, + cwd: None, + }) + .await?; + let _: ConfigReadResponse = + timeout(DEFAULT_TIMEOUT, mcp.read_response(config_request_id)).await??; + + let requirements_request_id = mcp.send_config_requirements_read_request().await?; + let _: ConfigRequirementsReadResponse = + timeout(DEFAULT_TIMEOUT, mcp.read_response(requirements_request_id)).await??; + + let write_request_id = mcp + .send_config_value_write_request(ConfigValueWriteParams { + file_path: None, + key_path: "model".to_string(), + value: serde_json::json!("gpt-concurrent"), + merge_strategy: MergeStrategy::Replace, + expected_version: None, + }) + .await?; + let write: ConfigWriteResponse = + timeout(DEFAULT_TIMEOUT, mcp.read_response(write_request_id)).await??; + assert_eq!(write.status, WriteStatus::Ok); + + let import_request_id = mcp + .send_raw_request( + "externalAgentConfig/import", + Some(serde_json::json!({ "migrationItems": [] })), + ) + .await?; + let import_response: ExternalAgentConfigImportResponse = + timeout(DEFAULT_TIMEOUT, mcp.read_response(import_request_id)).await??; + assert!(!import_response.import_id.is_empty()); + + let recent_timestamp = chrono::Utc::now().to_rfc3339_opts(chrono::SecondsFormat::Secs, true); + let session_contents = serde_json::json!({ + "type": "user", + "cwd": &project_root, + "timestamp": &recent_timestamp, + "message": { "content": "first request" }, + }) + .to_string(); + + // Later connector discovery reopens the transcript. Replace the named FIFO + // before releasing its existing reader so subsequent opens see a real file. + std::fs::remove_file(&session_path)?; + std::fs::write(&session_path, &session_contents)?; + blocked_session_writer + .write_all(session_contents.as_bytes()) + .await?; + drop(blocked_session_writer); + + let detected: ExternalAgentConfigDetectResponse = + timeout(DEFAULT_TIMEOUT, mcp.read_response(detect_request_id)).await??; + assert_eq!(detected.items.len(), 1); + assert_eq!( + detected.items[0].item_type, + ExternalAgentConfigMigrationItemType::Sessions + ); + assert!( + std::fs::read_to_string(codex_home.path().join("config.toml"))? + .contains("model = \"gpt-concurrent\"") + ); + + Ok(()) +} + #[tokio::test] async fn external_agent_config_migration_source_drives_detect_and_import() -> Result<()> { let codex_home = TempDir::new()?;