mirror of
https://github.com/openai/codex.git
synced 2026-11-05 20:10:33 +00:00
[codex-analytics] make auth retention explicit
This commit is contained in:
@@ -1,4 +1,6 @@
|
||||
use crate::client::AnalyticsEventsClient;
|
||||
use crate::client::AnalyticsEventsQueue;
|
||||
use crate::client::AuthManagerRetention;
|
||||
use crate::events::AppServerRpcTransport;
|
||||
use crate::events::CodexAppMentionedEventRequest;
|
||||
use crate::events::CodexAppServerClientMetadata;
|
||||
@@ -83,6 +85,8 @@ use codex_app_server_protocol::TurnStatus as AppServerTurnStatus;
|
||||
use codex_app_server_protocol::TurnSteerParams;
|
||||
use codex_app_server_protocol::TurnSteerResponse;
|
||||
use codex_app_server_protocol::UserInput;
|
||||
use codex_login::AuthManager;
|
||||
use codex_login::CodexAuth;
|
||||
use codex_login::default_client::DEFAULT_ORIGINATOR;
|
||||
use codex_login::default_client::originator;
|
||||
use codex_plugin::AppConnectorId;
|
||||
@@ -793,6 +797,37 @@ fn app_used_dedupe_is_keyed_by_turn_and_connector() {
|
||||
assert_eq!(queue.should_enqueue_app_used(&turn_2, &app), true);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn default_client_retains_auth_manager() {
|
||||
let auth_manager = AuthManager::from_auth_for_testing(CodexAuth::from_api_key("test"));
|
||||
let weak_auth_manager = Arc::downgrade(&auth_manager);
|
||||
|
||||
let _client = AnalyticsEventsClient::new(
|
||||
auth_manager,
|
||||
"http://localhost".to_string(),
|
||||
None,
|
||||
AuthManagerRetention::Strong,
|
||||
);
|
||||
|
||||
assert!(weak_auth_manager.upgrade().is_some());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn non_owning_client_does_not_retain_auth_manager() {
|
||||
let auth_manager = AuthManager::from_auth_for_testing(CodexAuth::from_api_key("test"));
|
||||
let weak_auth_manager = Arc::downgrade(&auth_manager);
|
||||
|
||||
let _client = AnalyticsEventsClient::new(
|
||||
Arc::clone(&auth_manager),
|
||||
"http://localhost".to_string(),
|
||||
None,
|
||||
AuthManagerRetention::Weak,
|
||||
);
|
||||
drop(auth_manager);
|
||||
|
||||
assert!(weak_auth_manager.upgrade().is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn thread_initialized_event_serializes_expected_shape() {
|
||||
let event = TrackEventRequest::ThreadInitialized(ThreadInitializedEvent {
|
||||
|
||||
@@ -54,16 +54,29 @@ pub struct AnalyticsEventsClient {
|
||||
queue: Option<AnalyticsEventsQueue>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Copy)]
|
||||
pub enum AuthManagerRetention {
|
||||
Strong,
|
||||
Weak,
|
||||
}
|
||||
|
||||
impl AnalyticsEventsQueue {
|
||||
pub(crate) fn new(auth_manager: Arc<AuthManager>, base_url: String) -> Self {
|
||||
pub(crate) fn new(
|
||||
auth_manager: Arc<AuthManager>,
|
||||
base_url: String,
|
||||
retention: AuthManagerRetention,
|
||||
) -> Self {
|
||||
let (sender, mut receiver) = mpsc::channel(ANALYTICS_EVENTS_QUEUE_SIZE);
|
||||
let auth_manager = Arc::downgrade(&auth_manager);
|
||||
let auth_manager = match retention {
|
||||
AuthManagerRetention::Strong => AuthManagerHandle::Strong(auth_manager),
|
||||
AuthManagerRetention::Weak => AuthManagerHandle::Weak(Arc::downgrade(&auth_manager)),
|
||||
};
|
||||
tokio::spawn(async move {
|
||||
let mut reducer = AnalyticsReducer::default();
|
||||
while let Some(input) = receiver.recv().await {
|
||||
let mut events = Vec::new();
|
||||
reducer.ingest(input, &mut events).await;
|
||||
let Some(auth_manager) = auth_manager.upgrade() else {
|
||||
let Some(auth_manager) = auth_manager.get() else {
|
||||
break;
|
||||
};
|
||||
send_track_events(&auth_manager, &base_url, events).await;
|
||||
@@ -117,15 +130,35 @@ impl AnalyticsEventsQueue {
|
||||
}
|
||||
}
|
||||
|
||||
enum AuthManagerHandle {
|
||||
Strong(Arc<AuthManager>),
|
||||
Weak(std::sync::Weak<AuthManager>),
|
||||
}
|
||||
|
||||
impl AuthManagerHandle {
|
||||
fn get(&self) -> Option<Arc<AuthManager>> {
|
||||
match self {
|
||||
Self::Strong(auth_manager) => Some(Arc::clone(auth_manager)),
|
||||
Self::Weak(auth_manager) => auth_manager.upgrade(),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl AnalyticsEventsClient {
|
||||
pub fn new(
|
||||
auth_manager: Arc<AuthManager>,
|
||||
base_url: String,
|
||||
analytics_enabled: Option<bool>,
|
||||
auth_manager_retention: AuthManagerRetention,
|
||||
) -> Self {
|
||||
Self {
|
||||
queue: (analytics_enabled != Some(false))
|
||||
.then(|| AnalyticsEventsQueue::new(Arc::clone(&auth_manager), base_url)),
|
||||
queue: (analytics_enabled != Some(false)).then(|| {
|
||||
AnalyticsEventsQueue::new(
|
||||
Arc::clone(&auth_manager),
|
||||
base_url,
|
||||
auth_manager_retention,
|
||||
)
|
||||
}),
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -7,6 +7,7 @@ use std::time::SystemTime;
|
||||
use std::time::UNIX_EPOCH;
|
||||
|
||||
pub use client::AnalyticsEventsClient;
|
||||
pub use client::AuthManagerRetention;
|
||||
pub use events::AppServerRpcTransport;
|
||||
pub use events::GuardianApprovalRequestSource;
|
||||
pub use events::GuardianReviewAnalyticsResult;
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
use std::sync::Arc;
|
||||
|
||||
use codex_analytics::AnalyticsEventsClient;
|
||||
use codex_analytics::AuthManagerRetention;
|
||||
use codex_core::config::Config;
|
||||
use codex_login::AuthManager;
|
||||
|
||||
@@ -12,5 +13,6 @@ pub(crate) fn analytics_events_client_from_config(
|
||||
auth_manager,
|
||||
config.chatgpt_base_url.trim_end_matches('/').to_string(),
|
||||
config.analytics_enabled,
|
||||
AuthManagerRetention::Weak,
|
||||
)
|
||||
}
|
||||
|
||||
@@ -3118,6 +3118,7 @@ mod tests {
|
||||
use anyhow::Result;
|
||||
use anyhow::anyhow;
|
||||
use anyhow::bail;
|
||||
use codex_analytics::AuthManagerRetention;
|
||||
use codex_app_server_protocol::AutoReviewDecisionSource;
|
||||
use codex_app_server_protocol::GuardianApprovalReviewStatus;
|
||||
use codex_app_server_protocol::JSONRPCErrorError;
|
||||
@@ -3615,6 +3616,7 @@ mod tests {
|
||||
),
|
||||
"http://localhost".to_string(),
|
||||
Some(false),
|
||||
AuthManagerRetention::Strong,
|
||||
),
|
||||
codex_home: codex_home.path().to_path_buf(),
|
||||
};
|
||||
|
||||
@@ -467,6 +467,7 @@ mod tests {
|
||||
use super::*;
|
||||
use crate::config_manager::apply_runtime_feature_enablement;
|
||||
use codex_analytics::AnalyticsEventsClient;
|
||||
use codex_analytics::AuthManagerRetention;
|
||||
use codex_arg0::Arg0DispatchPaths;
|
||||
use codex_config::CloudRequirementsLoader;
|
||||
use codex_config::LoaderOverrides;
|
||||
@@ -832,6 +833,7 @@ mod tests {
|
||||
.trim_end_matches('/')
|
||||
.to_string(),
|
||||
analytics_config.analytics_enabled,
|
||||
AuthManagerRetention::Strong,
|
||||
),
|
||||
);
|
||||
|
||||
|
||||
@@ -46,6 +46,7 @@ use async_channel::Sender;
|
||||
use chrono::Local;
|
||||
use chrono::Utc;
|
||||
use codex_analytics::AnalyticsEventsClient;
|
||||
use codex_analytics::AuthManagerRetention;
|
||||
use codex_analytics::SubAgentThreadStartedInput;
|
||||
use codex_app_server_protocol::McpServerElicitationRequest;
|
||||
use codex_app_server_protocol::McpServerElicitationRequestParams;
|
||||
|
||||
@@ -776,6 +776,7 @@ impl Session {
|
||||
Arc::clone(&auth_manager),
|
||||
config.chatgpt_base_url.trim_end_matches('/').to_string(),
|
||||
config.analytics_enabled,
|
||||
AuthManagerRetention::Strong,
|
||||
)
|
||||
});
|
||||
let services = SessionServices {
|
||||
|
||||
@@ -3423,6 +3423,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) {
|
||||
Arc::clone(&auth_manager),
|
||||
config.chatgpt_base_url.trim_end_matches('/').to_string(),
|
||||
config.analytics_enabled,
|
||||
AuthManagerRetention::Strong,
|
||||
),
|
||||
hooks: Hooks::new(HooksConfig {
|
||||
legacy_notify_argv: config.notify.clone(),
|
||||
@@ -4782,6 +4783,7 @@ where
|
||||
Arc::clone(&auth_manager),
|
||||
config.chatgpt_base_url.trim_end_matches('/').to_string(),
|
||||
config.analytics_enabled,
|
||||
AuthManagerRetention::Strong,
|
||||
),
|
||||
hooks: Hooks::new(HooksConfig {
|
||||
legacy_notify_argv: config.notify.clone(),
|
||||
|
||||
Reference in New Issue
Block a user