mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[codex] Fix post-merge analytics integration failures (#27285)
## Why Recent merges left `main` with analytics integration build failures. Local Cargo runs also made the trimmed-skills test depend on developer-installed skills, while Bazel used an isolated home. ## What changed - Clone `thread_metadata.thread_source` when constructing goal analytics event parameters. - Group app-server thread extension inputs into `ThreadExtensionDependencies`. - Isolate the trimmed-skills test home so its exact fixture count is stable across Cargo and Bazel. ## Validation - `cargo check -p codex-analytics` - `just test -p codex-analytics` (71 tests) - `just test -p codex-app-server` (837 tests; one unrelated zsh-fork timeout passed on retry)
This commit is contained in:
committed by
GitHub
Unverified
parent
608b8b1cc6
commit
e0cb4ede4e
@@ -1300,7 +1300,7 @@ impl AnalyticsReducer {
|
||||
thread_metadata.session_id.clone(),
|
||||
connection_state.app_server_client.clone(),
|
||||
connection_state.runtime.clone(),
|
||||
thread_metadata.thread_source,
|
||||
thread_metadata.thread_source.clone(),
|
||||
thread_metadata.subagent_source.clone(),
|
||||
thread_metadata.parent_thread_id.clone(),
|
||||
),
|
||||
|
||||
@@ -26,19 +26,32 @@ use crate::outgoing_message::OutgoingMessageSender;
|
||||
use crate::thread_state::ThreadListenerCommand;
|
||||
use crate::thread_state::ThreadStateManager;
|
||||
|
||||
pub(crate) struct ThreadExtensionDependencies {
|
||||
pub(crate) event_sink: Arc<dyn ExtensionEventSink>,
|
||||
pub(crate) auth_manager: Arc<AuthManager>,
|
||||
pub(crate) state_db: Option<StateDbHandle>,
|
||||
pub(crate) analytics_events_client: AnalyticsEventsClient,
|
||||
pub(crate) thread_manager: Weak<ThreadManager>,
|
||||
pub(crate) goal_service: Arc<GoalService>,
|
||||
pub(crate) executor_skill_provider: Arc<dyn codex_skills_extension::SkillProvider>,
|
||||
}
|
||||
|
||||
pub(crate) fn thread_extensions<S>(
|
||||
guardian_agent_spawner: S,
|
||||
event_sink: Arc<dyn ExtensionEventSink>,
|
||||
auth_manager: Arc<AuthManager>,
|
||||
state_db: Option<StateDbHandle>,
|
||||
analytics_events_client: AnalyticsEventsClient,
|
||||
thread_manager: Weak<ThreadManager>,
|
||||
goal_service: Arc<GoalService>,
|
||||
executor_skill_provider: Arc<dyn codex_skills_extension::SkillProvider>,
|
||||
dependencies: ThreadExtensionDependencies,
|
||||
) -> Arc<ExtensionRegistry<Config>>
|
||||
where
|
||||
S: AgentSpawner<StartThreadOptions, Spawned = NewThread, Error = CodexErr> + 'static,
|
||||
{
|
||||
let ThreadExtensionDependencies {
|
||||
event_sink,
|
||||
auth_manager,
|
||||
state_db,
|
||||
analytics_events_client,
|
||||
thread_manager,
|
||||
goal_service,
|
||||
executor_skill_provider,
|
||||
} = dependencies;
|
||||
let mut builder = ExtensionRegistryBuilder::<Config>::with_event_sink(event_sink);
|
||||
if let Some(state_db) = state_db {
|
||||
codex_goal_extension::install_with_backend(
|
||||
|
||||
@@ -96,6 +96,7 @@ async fn queue_refresh(
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::extensions::ThreadExtensionDependencies;
|
||||
use crate::extensions::guardian_agent_spawner;
|
||||
use crate::extensions::thread_extensions;
|
||||
use async_trait::async_trait;
|
||||
@@ -193,13 +194,15 @@ mod tests {
|
||||
Arc::clone(&environment_manager),
|
||||
thread_extensions(
|
||||
guardian_agent_spawner(thread_manager.clone()),
|
||||
Arc::new(NoopExtensionEventSink),
|
||||
auth_manager.clone(),
|
||||
Some(state_db.clone()),
|
||||
codex_analytics::AnalyticsEventsClient::disabled(),
|
||||
thread_manager.clone(),
|
||||
Arc::new(codex_goal_extension::GoalService::new()),
|
||||
Arc::clone(&executor_skill_provider),
|
||||
ThreadExtensionDependencies {
|
||||
event_sink: Arc::new(NoopExtensionEventSink),
|
||||
auth_manager: auth_manager.clone(),
|
||||
state_db: Some(state_db.clone()),
|
||||
analytics_events_client: codex_analytics::AnalyticsEventsClient::disabled(),
|
||||
thread_manager: thread_manager.clone(),
|
||||
goal_service: Arc::new(codex_goal_extension::GoalService::new()),
|
||||
executor_skill_provider: Arc::clone(&executor_skill_provider),
|
||||
},
|
||||
),
|
||||
/*analytics_events_client*/ None,
|
||||
Arc::clone(&thread_store),
|
||||
|
||||
@@ -8,6 +8,7 @@ use crate::attestation::app_server_attestation_provider;
|
||||
use crate::config_manager::ConfigManager;
|
||||
use crate::connection_rpc_gate::ConnectionRpcGate;
|
||||
use crate::error_code::invalid_request;
|
||||
use crate::extensions::ThreadExtensionDependencies;
|
||||
use crate::extensions::app_server_extension_event_sink;
|
||||
use crate::extensions::guardian_agent_spawner;
|
||||
use crate::extensions::thread_extensions;
|
||||
@@ -324,13 +325,18 @@ impl MessageProcessor {
|
||||
environment_manager,
|
||||
thread_extensions(
|
||||
guardian_agent_spawner(thread_manager.clone()),
|
||||
app_server_extension_event_sink(outgoing.clone(), thread_state_manager.clone()),
|
||||
auth_manager.clone(),
|
||||
state_db.clone(),
|
||||
analytics_events_client.clone(),
|
||||
thread_manager.clone(),
|
||||
Arc::clone(&goal_service),
|
||||
Arc::clone(&executor_skill_provider),
|
||||
ThreadExtensionDependencies {
|
||||
event_sink: app_server_extension_event_sink(
|
||||
outgoing.clone(),
|
||||
thread_state_manager.clone(),
|
||||
),
|
||||
auth_manager: auth_manager.clone(),
|
||||
state_db: state_db.clone(),
|
||||
analytics_events_client: analytics_events_client.clone(),
|
||||
thread_manager: thread_manager.clone(),
|
||||
goal_service: Arc::clone(&goal_service),
|
||||
executor_skill_provider: Arc::clone(&executor_skill_provider),
|
||||
},
|
||||
),
|
||||
Some(analytics_events_client.clone()),
|
||||
Arc::clone(&thread_store),
|
||||
|
||||
@@ -591,7 +591,15 @@ async fn turn_start_emits_thread_scoped_warning_notification_for_trimmed_skills(
|
||||
write_test_skill(codex_home.path(), "alpha-skill")?;
|
||||
write_test_skill(codex_home.path(), "beta-skill")?;
|
||||
|
||||
let mut mcp = TestAppServer::new(codex_home.path()).await?;
|
||||
let isolated_home = codex_home.path().to_string_lossy();
|
||||
let mut mcp = TestAppServer::new_with_env(
|
||||
codex_home.path(),
|
||||
&[
|
||||
("HOME", Some(isolated_home.as_ref())),
|
||||
("USERPROFILE", Some(isolated_home.as_ref())),
|
||||
],
|
||||
)
|
||||
.await?;
|
||||
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
|
||||
|
||||
let thread_req = mcp
|
||||
|
||||
Reference in New Issue
Block a user