mirror of
https://github.com/openai/codex.git
synced 2026-11-06 20:20:34 +00:00
fix: stabilize config trust regression tests
Keep canonical project trust keys first so persisted trusted project entries keep their existing lookup shape while still matching symlink aliases. Use the per-thread derived config for app-server thread-initialized analytics and isolate app-server integration subprocesses from host managed config by default. Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
@@ -444,7 +444,6 @@ struct ListenerTaskContext {
|
||||
thread_state_manager: ThreadStateManager,
|
||||
outgoing: Arc<OutgoingMessageSender>,
|
||||
analytics_events_client: AnalyticsEventsClient,
|
||||
general_analytics_enabled: bool,
|
||||
thread_watch_manager: ThreadWatchManager,
|
||||
fallback_model_provider: String,
|
||||
codex_home: PathBuf,
|
||||
@@ -2103,7 +2102,6 @@ impl CodexMessageProcessor {
|
||||
thread_state_manager: self.thread_state_manager.clone(),
|
||||
outgoing: Arc::clone(&self.outgoing),
|
||||
analytics_events_client: self.analytics_events_client.clone(),
|
||||
general_analytics_enabled: self.config.features.enabled(Feature::GeneralAnalytics),
|
||||
thread_watch_manager: self.thread_watch_manager.clone(),
|
||||
fallback_model_provider: self.config.model_provider_id.clone(),
|
||||
codex_home: self.config.codex_home.clone(),
|
||||
@@ -2288,6 +2286,7 @@ impl CodexMessageProcessor {
|
||||
};
|
||||
}
|
||||
|
||||
let thread_general_analytics_enabled = config.features.enabled(Feature::GeneralAnalytics);
|
||||
let dynamic_tools = dynamic_tools.unwrap_or_default();
|
||||
let core_dynamic_tools = if dynamic_tools.is_empty() {
|
||||
Vec::new()
|
||||
@@ -2418,7 +2417,7 @@ impl CodexMessageProcessor {
|
||||
sandbox: config_snapshot.sandbox_policy.into(),
|
||||
reasoning_effort: config_snapshot.reasoning_effort,
|
||||
};
|
||||
if listener_task_context.general_analytics_enabled {
|
||||
if thread_general_analytics_enabled {
|
||||
listener_task_context
|
||||
.analytics_events_client
|
||||
.track_response(
|
||||
@@ -7270,7 +7269,6 @@ impl CodexMessageProcessor {
|
||||
thread_state_manager: self.thread_state_manager.clone(),
|
||||
outgoing: Arc::clone(&self.outgoing),
|
||||
analytics_events_client: self.analytics_events_client.clone(),
|
||||
general_analytics_enabled: self.config.features.enabled(Feature::GeneralAnalytics),
|
||||
thread_watch_manager: self.thread_watch_manager.clone(),
|
||||
fallback_model_provider: self.config.model_provider_id.clone(),
|
||||
codex_home: self.config.codex_home.clone(),
|
||||
@@ -7359,7 +7357,6 @@ impl CodexMessageProcessor {
|
||||
thread_state_manager: self.thread_state_manager.clone(),
|
||||
outgoing: Arc::clone(&self.outgoing),
|
||||
analytics_events_client: self.analytics_events_client.clone(),
|
||||
general_analytics_enabled: self.config.features.enabled(Feature::GeneralAnalytics),
|
||||
thread_watch_manager: self.thread_watch_manager.clone(),
|
||||
fallback_model_provider: self.config.model_provider_id.clone(),
|
||||
codex_home: self.config.codex_home.clone(),
|
||||
@@ -7392,7 +7389,6 @@ impl CodexMessageProcessor {
|
||||
thread_manager,
|
||||
thread_state_manager,
|
||||
analytics_events_client: _,
|
||||
general_analytics_enabled: _,
|
||||
thread_watch_manager,
|
||||
fallback_model_provider,
|
||||
codex_home,
|
||||
|
||||
@@ -7,10 +7,10 @@ use codex_arg0::arg0_dispatch_or_else;
|
||||
use codex_core::config_loader::LoaderOverrides;
|
||||
use codex_protocol::protocol::SessionSource;
|
||||
use codex_utils_cli::CliConfigOverrides;
|
||||
use std::path::PathBuf;
|
||||
|
||||
// Debug-only test hook: lets integration tests point the server at a temporary
|
||||
// managed config file without writing to /etc.
|
||||
// Debug-only test hook: lets integration tests disable host-managed config or
|
||||
// point the server at a temporary managed config file without writing to /etc.
|
||||
#[cfg(debug_assertions)]
|
||||
const MANAGED_CONFIG_PATH_ENV_VAR: &str = "CODEX_APP_SERVER_MANAGED_CONFIG_PATH";
|
||||
|
||||
#[derive(Debug, Parser)]
|
||||
@@ -40,11 +40,7 @@ struct AppServerArgs {
|
||||
fn main() -> anyhow::Result<()> {
|
||||
arg0_dispatch_or_else(|arg0_paths: Arg0DispatchPaths| async move {
|
||||
let args = AppServerArgs::parse();
|
||||
let managed_config_path = managed_config_path_from_debug_env();
|
||||
let loader_overrides = LoaderOverrides {
|
||||
managed_config_path,
|
||||
..Default::default()
|
||||
};
|
||||
let loader_overrides = loader_overrides_from_debug_env();
|
||||
let transport = args.listen;
|
||||
let session_source = args.session_source;
|
||||
let auth = args.auth.try_into_settings()?;
|
||||
@@ -63,17 +59,20 @@ fn main() -> anyhow::Result<()> {
|
||||
})
|
||||
}
|
||||
|
||||
fn managed_config_path_from_debug_env() -> Option<PathBuf> {
|
||||
fn loader_overrides_from_debug_env() -> LoaderOverrides {
|
||||
#[cfg(debug_assertions)]
|
||||
{
|
||||
if let Ok(value) = std::env::var(MANAGED_CONFIG_PATH_ENV_VAR) {
|
||||
return if value.is_empty() {
|
||||
None
|
||||
LoaderOverrides::without_managed_config_for_tests()
|
||||
} else {
|
||||
Some(PathBuf::from(value))
|
||||
LoaderOverrides {
|
||||
managed_config_path: Some(std::path::PathBuf::from(value)),
|
||||
..Default::default()
|
||||
}
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
None
|
||||
LoaderOverrides::default()
|
||||
}
|
||||
|
||||
@@ -133,6 +133,7 @@ impl McpProcess {
|
||||
cmd.stderr(Stdio::piped());
|
||||
cmd.current_dir(codex_home);
|
||||
cmd.env("CODEX_HOME", codex_home);
|
||||
cmd.env("CODEX_APP_SERVER_MANAGED_CONFIG_PATH", "");
|
||||
cmd.env("RUST_LOG", "info");
|
||||
cmd.env_remove(CODEX_INTERNAL_ORIGINATOR_OVERRIDE_ENV_VAR);
|
||||
cmd.args(args);
|
||||
|
||||
@@ -98,26 +98,29 @@ pub(crate) async fn enable_analytics_capture(server: &MockServer, codex_home: &P
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub(crate) async fn wait_for_analytics_payload(
|
||||
pub(crate) async fn wait_for_thread_initialized_payload(
|
||||
server: &MockServer,
|
||||
read_timeout: Duration,
|
||||
) -> Result<Value> {
|
||||
let body = timeout(read_timeout, async {
|
||||
timeout(read_timeout, async {
|
||||
loop {
|
||||
let Some(requests) = server.received_requests().await else {
|
||||
tokio::time::sleep(Duration::from_millis(25)).await;
|
||||
continue;
|
||||
};
|
||||
if let Some(request) = requests.iter().find(|request| {
|
||||
for request in requests.iter().filter(|request| {
|
||||
request.method == "POST" && request.url.path() == "/codex/analytics-events/events"
|
||||
}) {
|
||||
break request.body.clone();
|
||||
let payload: Value = serde_json::from_slice(&request.body)
|
||||
.map_err(|err| anyhow::anyhow!("invalid analytics payload: {err}"))?;
|
||||
if thread_initialized_event(&payload).is_ok() {
|
||||
return Ok(payload);
|
||||
}
|
||||
}
|
||||
tokio::time::sleep(Duration::from_millis(25)).await;
|
||||
}
|
||||
})
|
||||
.await?;
|
||||
serde_json::from_slice(&body).map_err(|err| anyhow::anyhow!("invalid analytics payload: {err}"))
|
||||
.await?
|
||||
}
|
||||
|
||||
pub(crate) fn thread_initialized_event(payload: &Value) -> Result<&Value> {
|
||||
|
||||
@@ -41,7 +41,7 @@ use wiremock::matchers::path;
|
||||
use super::analytics::assert_basic_thread_initialized_event;
|
||||
use super::analytics::enable_analytics_capture;
|
||||
use super::analytics::thread_initialized_event;
|
||||
use super::analytics::wait_for_analytics_payload;
|
||||
use super::analytics::wait_for_thread_initialized_payload;
|
||||
|
||||
const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10);
|
||||
|
||||
@@ -221,7 +221,7 @@ async fn thread_fork_tracks_thread_initialized_analytics() -> Result<()> {
|
||||
.await??;
|
||||
let ThreadForkResponse { thread, .. } = to_response::<ThreadForkResponse>(fork_resp)?;
|
||||
|
||||
let payload = wait_for_analytics_payload(&server, DEFAULT_READ_TIMEOUT).await?;
|
||||
let payload = wait_for_thread_initialized_payload(&server, DEFAULT_READ_TIMEOUT).await?;
|
||||
let event = thread_initialized_event(&payload)?;
|
||||
assert_basic_thread_initialized_event(event, &thread.id, "mock-model", "forked");
|
||||
Ok(())
|
||||
|
||||
@@ -73,7 +73,7 @@ use wiremock::matchers::path;
|
||||
use super::analytics::assert_basic_thread_initialized_event;
|
||||
use super::analytics::enable_analytics_capture;
|
||||
use super::analytics::thread_initialized_event;
|
||||
use super::analytics::wait_for_analytics_payload;
|
||||
use super::analytics::wait_for_thread_initialized_payload;
|
||||
|
||||
const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10);
|
||||
const CODEX_5_2_INSTRUCTIONS_TEMPLATE_DEFAULT: &str = "You are Codex, a coding agent based on GPT-5. You and the user share the same workspace and collaborate to achieve the user's goals.";
|
||||
@@ -194,7 +194,7 @@ async fn thread_resume_tracks_thread_initialized_analytics() -> Result<()> {
|
||||
.await??;
|
||||
let ThreadResumeResponse { thread, .. } = to_response::<ThreadResumeResponse>(resume_resp)?;
|
||||
|
||||
let payload = wait_for_analytics_payload(&server, DEFAULT_READ_TIMEOUT).await?;
|
||||
let payload = wait_for_thread_initialized_payload(&server, DEFAULT_READ_TIMEOUT).await?;
|
||||
let event = thread_initialized_event(&payload)?;
|
||||
assert_basic_thread_initialized_event(event, &thread.id, "gpt-5.2-codex", "resumed");
|
||||
Ok(())
|
||||
|
||||
@@ -41,7 +41,7 @@ use wiremock::matchers::path;
|
||||
use super::analytics::assert_basic_thread_initialized_event;
|
||||
use super::analytics::enable_analytics_capture;
|
||||
use super::analytics::thread_initialized_event;
|
||||
use super::analytics::wait_for_analytics_payload;
|
||||
use super::analytics::wait_for_thread_initialized_payload;
|
||||
|
||||
const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10);
|
||||
|
||||
@@ -188,7 +188,7 @@ async fn thread_start_tracks_thread_initialized_analytics() -> Result<()> {
|
||||
.await??;
|
||||
let ThreadStartResponse { thread, .. } = to_response::<ThreadStartResponse>(resp)?;
|
||||
|
||||
let payload = wait_for_analytics_payload(&server, DEFAULT_READ_TIMEOUT).await?;
|
||||
let payload = wait_for_thread_initialized_payload(&server, DEFAULT_READ_TIMEOUT).await?;
|
||||
assert_eq!(payload["events"].as_array().expect("events array").len(), 1);
|
||||
let event = thread_initialized_event(&payload)?;
|
||||
assert_basic_thread_initialized_event(event, &thread.id, "mock-model", "new");
|
||||
@@ -221,7 +221,7 @@ async fn thread_start_does_not_track_thread_initialized_analytics_without_featur
|
||||
.await??;
|
||||
let _ = to_response::<ThreadStartResponse>(resp)?;
|
||||
|
||||
let payload = wait_for_analytics_payload(&server, Duration::from_millis(250)).await;
|
||||
let payload = wait_for_thread_initialized_payload(&server, Duration::from_millis(250)).await;
|
||||
assert!(
|
||||
payload.is_err(),
|
||||
"thread analytics should be gated off when general_analytics is disabled"
|
||||
@@ -826,11 +826,7 @@ fn create_config_toml_with_chatgpt_base_url(
|
||||
chatgpt_base_url: &str,
|
||||
general_analytics_enabled: bool,
|
||||
) -> std::io::Result<()> {
|
||||
let general_analytics_toml = if general_analytics_enabled {
|
||||
"\ngeneral_analytics = true".to_string()
|
||||
} else {
|
||||
String::new()
|
||||
};
|
||||
let general_analytics_toml = format!("general_analytics = {general_analytics_enabled}");
|
||||
let config_toml = codex_home.join("config.toml");
|
||||
std::fs::write(
|
||||
config_toml,
|
||||
|
||||
@@ -719,9 +719,9 @@ fn normalized_project_lookup_keys(path: &Path) -> Vec<String> {
|
||||
.to_string(),
|
||||
);
|
||||
if normalized_path == normalized_canonical_path {
|
||||
vec![normalized_path]
|
||||
vec![normalized_canonical_path]
|
||||
} else {
|
||||
vec![normalized_path, normalized_canonical_path]
|
||||
vec![normalized_canonical_path, normalized_path]
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -708,9 +708,9 @@ fn normalized_project_trust_keys(path: &Path) -> Vec<String> {
|
||||
.to_string(),
|
||||
);
|
||||
if normalized_path == normalized_canonical_path {
|
||||
vec![normalized_path]
|
||||
vec![normalized_canonical_path]
|
||||
} else {
|
||||
vec![normalized_path, normalized_canonical_path]
|
||||
vec![normalized_canonical_path, normalized_path]
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user