From d6b13276c79e0d7c03ad58d3f4cf1b16aae7dba2 Mon Sep 17 00:00:00 2001 From: rhan-oai Date: Tue, 14 Apr 2026 13:20:46 -0700 Subject: [PATCH] [codex-analytics] enable general analytics by default (#17389) ## Summary - Make GeneralAnalytics stable and enabled by default. - Update feature tests and app-server lifecycle fixtures for explicit general_analytics=false. - Keep app-server integration tests isolated from host managed config so explicit feature fixtures are deterministic. ## Validation - cargo test -p codex-features - cargo test -p codex-app-server general_analytics (matched 0 tests) - cargo test -p codex-app-server thread_start_ - cargo test -p codex-app-server thread_fork_ - cargo test -p codex-app-server thread_resume_ - cargo test -p codex-app-server config_read_includes_system_layer_and_overrides --- codex-rs/app-server/src/main.rs | 7 ++--- .../app-server/tests/common/mcp_process.rs | 5 ++++ .../app-server/tests/suite/v2/thread_start.rs | 30 ++++++++++++------- codex-rs/features/src/lib.rs | 4 +-- codex-rs/features/src/tests.rs | 6 ++-- 5 files changed, 32 insertions(+), 20 deletions(-) diff --git a/codex-rs/app-server/src/main.rs b/codex-rs/app-server/src/main.rs index d896f2f8e..069227070 100644 --- a/codex-rs/app-server/src/main.rs +++ b/codex-rs/app-server/src/main.rs @@ -44,10 +44,9 @@ fn main() -> anyhow::Result<()> { let loader_overrides = if disable_managed_config_from_debug_env() { LoaderOverrides::without_managed_config_for_tests() } else { - LoaderOverrides { - managed_config_path: managed_config_path_from_debug_env(), - ..Default::default() - } + managed_config_path_from_debug_env() + .map(LoaderOverrides::with_managed_config_path_for_tests) + .unwrap_or_default() }; let transport = args.listen; let session_source = args.session_source; diff --git a/codex-rs/app-server/tests/common/mcp_process.rs b/codex-rs/app-server/tests/common/mcp_process.rs index eddab545a..22225c7c9 100644 --- a/codex-rs/app-server/tests/common/mcp_process.rs +++ b/codex-rs/app-server/tests/common/mcp_process.rs @@ -144,6 +144,11 @@ impl McpProcess { cmd.current_dir(codex_home); cmd.env("CODEX_HOME", codex_home); cmd.env("RUST_LOG", "info"); + // Keep integration tests isolated from host managed configuration. + cmd.env( + "CODEX_APP_SERVER_MANAGED_CONFIG_PATH", + codex_home.join("managed_config.toml"), + ); cmd.env_remove(CODEX_INTERNAL_ORIGINATOR_OVERRIDE_ENV_VAR); cmd.args(args); diff --git a/codex-rs/app-server/tests/suite/v2/thread_start.rs b/codex-rs/app-server/tests/suite/v2/thread_start.rs index 978260f8e..f7d314741 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_start.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_start.rs @@ -42,7 +42,6 @@ use wiremock::matchers::path; use super::analytics::assert_basic_thread_initialized_event; use super::analytics::mount_analytics_capture; use super::analytics::thread_initialized_event; -use super::analytics::wait_for_analytics_event; use super::analytics::wait_for_analytics_payload; const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10); @@ -281,16 +280,25 @@ async fn thread_start_does_not_track_thread_initialized_analytics_without_featur .await??; let _ = to_response::(resp)?; - let payload = wait_for_analytics_event( - &server, - Duration::from_millis(250), - "codex_thread_initialized", - ) - .await; - assert!( - payload.is_err(), - "thread analytics should be gated off when general_analytics is disabled" - ); + assert_no_thread_initialized_analytics(&server, Duration::from_millis(250)).await?; + Ok(()) +} + +async fn assert_no_thread_initialized_analytics( + server: &MockServer, + wait_duration: Duration, +) -> Result<()> { + tokio::time::sleep(wait_duration).await; + let requests = server.received_requests().await.unwrap_or_default(); + for request in requests.iter().filter(|request| { + request.method == "POST" && request.url.path() == "/codex/analytics-events/events" + }) { + let payload: Value = serde_json::from_slice(&request.body)?; + assert!( + thread_initialized_event(&payload).is_err(), + "thread analytics should be gated off when general_analytics is disabled; payload={payload}" + ); + } Ok(()) } diff --git a/codex-rs/features/src/lib.rs b/codex-rs/features/src/lib.rs index 727c32ccd..6a919485c 100644 --- a/codex-rs/features/src/lib.rs +++ b/codex-rs/features/src/lib.rs @@ -660,8 +660,8 @@ pub const FEATURES: &[FeatureSpec] = &[ FeatureSpec { id: Feature::GeneralAnalytics, key: "general_analytics", - stage: Stage::UnderDevelopment, - default_enabled: false, + stage: Stage::Stable, + default_enabled: true, }, FeatureSpec { id: Feature::Sqlite, diff --git a/codex-rs/features/src/tests.rs b/codex-rs/features/src/tests.rs index 818ffc8bd..367527ba1 100644 --- a/codex-rs/features/src/tests.rs +++ b/codex-rs/features/src/tests.rs @@ -134,9 +134,9 @@ fn tool_search_is_under_development_and_disabled_by_default() { } #[test] -fn general_analytics_is_under_development_and_disabled_by_default() { - assert_eq!(Feature::GeneralAnalytics.stage(), Stage::UnderDevelopment); - assert_eq!(Feature::GeneralAnalytics.default_enabled(), false); +fn general_analytics_is_stable_and_enabled_by_default() { + assert_eq!(Feature::GeneralAnalytics.stage(), Stage::Stable); + assert_eq!(Feature::GeneralAnalytics.default_enabled(), true); } #[test]