mirror of
https://github.com/openai/codex.git
synced 2026-10-21 07:42:01 +00:00
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
This commit is contained in:
committed by
copyberry
parent
dd916428cd
commit
f65ea998c7
@@ -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" {
|
||||
|
||||
@@ -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()?;
|
||||
|
||||
Reference in New Issue
Block a user