mirror of
https://github.com/openai/codex.git
synced 2026-05-16 17:23:57 +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;
|
||||
@@ -85,6 +87,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;
|
||||
@@ -795,6 +799,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 {
|
||||
|
||||
@@ -34,6 +34,7 @@ use codex_plugin::PluginTelemetryMetadata;
|
||||
use std::collections::HashSet;
|
||||
use std::sync::Arc;
|
||||
use std::sync::Mutex;
|
||||
use std::sync::Weak;
|
||||
use std::time::Duration;
|
||||
use tokio::sync::mpsc;
|
||||
|
||||
@@ -54,10 +55,41 @@ pub struct AnalyticsEventsClient {
|
||||
analytics_enabled: Option<bool>,
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
|
||||
pub enum AuthManagerRetention {
|
||||
Strong,
|
||||
Weak,
|
||||
}
|
||||
|
||||
enum RetainedAuthManager {
|
||||
Strong(Arc<AuthManager>),
|
||||
Weak(Weak<AuthManager>),
|
||||
}
|
||||
|
||||
impl RetainedAuthManager {
|
||||
fn upgrade(&self) -> Option<Arc<AuthManager>> {
|
||||
match self {
|
||||
Self::Strong(auth_manager) => Some(Arc::clone(auth_manager)),
|
||||
Self::Weak(auth_manager) => auth_manager.upgrade(),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
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 auth_manager = match retention {
|
||||
AuthManagerRetention::Strong => RetainedAuthManager::Strong(auth_manager),
|
||||
AuthManagerRetention::Weak => RetainedAuthManager::Weak(Arc::downgrade(&auth_manager)),
|
||||
};
|
||||
Self::spawn(auth_manager, base_url)
|
||||
}
|
||||
|
||||
fn spawn(auth_manager: RetainedAuthManager, base_url: String) -> Self {
|
||||
let (sender, mut receiver) = mpsc::channel(ANALYTICS_EVENTS_QUEUE_SIZE);
|
||||
let auth_manager = Arc::downgrade(&auth_manager);
|
||||
tokio::spawn(async move {
|
||||
let mut reducer = AnalyticsReducer::default();
|
||||
while let Some(input) = receiver.recv().await {
|
||||
@@ -122,9 +154,10 @@ impl AnalyticsEventsClient {
|
||||
auth_manager: Arc<AuthManager>,
|
||||
base_url: String,
|
||||
analytics_enabled: Option<bool>,
|
||||
retention: AuthManagerRetention,
|
||||
) -> Self {
|
||||
Self {
|
||||
queue: AnalyticsEventsQueue::new(Arc::clone(&auth_manager), base_url),
|
||||
queue: AnalyticsEventsQueue::new(auth_manager, base_url, retention),
|
||||
analytics_enabled,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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,16 +0,0 @@
|
||||
use std::sync::Arc;
|
||||
|
||||
use codex_analytics::AnalyticsEventsClient;
|
||||
use codex_core::config::Config;
|
||||
use codex_login::AuthManager;
|
||||
|
||||
pub(crate) fn analytics_events_client_from_config(
|
||||
auth_manager: Arc<AuthManager>,
|
||||
config: &Config,
|
||||
) -> AnalyticsEventsClient {
|
||||
AnalyticsEventsClient::new(
|
||||
auth_manager,
|
||||
config.chatgpt_base_url.trim_end_matches('/').to_string(),
|
||||
config.analytics_enabled,
|
||||
)
|
||||
}
|
||||
@@ -3129,6 +3129,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;
|
||||
@@ -3626,6 +3627,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,
|
||||
),
|
||||
);
|
||||
|
||||
|
||||
@@ -50,7 +50,6 @@ use std::sync::atomic::AtomicBool;
|
||||
use std::sync::atomic::Ordering;
|
||||
use std::time::Duration;
|
||||
|
||||
use crate::analytics::analytics_events_client_from_config;
|
||||
use crate::config_manager::ConfigManager;
|
||||
use crate::error_code::INTERNAL_ERROR_CODE;
|
||||
use crate::error_code::INVALID_REQUEST_ERROR_CODE;
|
||||
@@ -67,7 +66,9 @@ use crate::transport::CHANNEL_CAPACITY;
|
||||
use crate::transport::ConnectionOrigin;
|
||||
use crate::transport::OutboundConnectionState;
|
||||
use crate::transport::route_outgoing_envelope;
|
||||
use codex_analytics::AnalyticsEventsClient;
|
||||
use codex_analytics::AppServerRpcTransport;
|
||||
use codex_analytics::AuthManagerRetention;
|
||||
use codex_app_server_protocol::ClientNotification;
|
||||
use codex_app_server_protocol::ClientRequest;
|
||||
use codex_app_server_protocol::ConfigWarningNotification;
|
||||
@@ -369,8 +370,15 @@ fn start_uninitialized(args: InProcessStartArgs) -> InProcessClientHandle {
|
||||
let auth_manager =
|
||||
AuthManager::shared_from_config(args.config.as_ref(), args.enable_codex_api_key_env)
|
||||
.await;
|
||||
let analytics_events_client =
|
||||
analytics_events_client_from_config(Arc::clone(&auth_manager), args.config.as_ref());
|
||||
let analytics_events_client = AnalyticsEventsClient::new(
|
||||
Arc::clone(&auth_manager),
|
||||
args.config
|
||||
.chatgpt_base_url
|
||||
.trim_end_matches('/')
|
||||
.to_string(),
|
||||
args.config.analytics_enabled,
|
||||
AuthManagerRetention::Weak,
|
||||
);
|
||||
let outgoing_message_sender = Arc::new(OutgoingMessageSender::new(
|
||||
outgoing_tx,
|
||||
analytics_events_client,
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
#![deny(clippy::print_stdout, clippy::print_stderr)]
|
||||
|
||||
mod analytics;
|
||||
|
||||
use codex_analytics::AnalyticsEventsClient;
|
||||
use codex_analytics::AuthManagerRetention;
|
||||
use codex_arg0::Arg0DispatchPaths;
|
||||
use codex_config::ConfigLayerStackOrdering;
|
||||
use codex_config::LoaderOverrides;
|
||||
@@ -21,7 +21,6 @@ use std::sync::Arc;
|
||||
use std::sync::RwLock;
|
||||
use std::sync::atomic::AtomicBool;
|
||||
|
||||
use crate::analytics::analytics_events_client_from_config;
|
||||
use crate::config_manager::ConfigManager;
|
||||
use crate::message_processor::MessageProcessor;
|
||||
use crate::message_processor::MessageProcessorArgs;
|
||||
@@ -712,8 +711,12 @@ pub async fn run_main_with_transport_options(
|
||||
});
|
||||
|
||||
let processor_handle = tokio::spawn({
|
||||
let analytics_events_client =
|
||||
analytics_events_client_from_config(Arc::clone(&auth_manager), &config);
|
||||
let analytics_events_client = AnalyticsEventsClient::new(
|
||||
Arc::clone(&auth_manager),
|
||||
config.chatgpt_base_url.trim_end_matches('/').to_string(),
|
||||
config.analytics_enabled,
|
||||
AuthManagerRetention::Weak,
|
||||
);
|
||||
let outgoing_message_sender = Arc::new(OutgoingMessageSender::new(
|
||||
outgoing_tx,
|
||||
analytics_events_client,
|
||||
|
||||
@@ -25,6 +25,7 @@ use async_trait::async_trait;
|
||||
use axum::http::HeaderValue;
|
||||
use codex_analytics::AnalyticsEventsClient;
|
||||
use codex_analytics::AppServerRpcTransport;
|
||||
use codex_analytics::AuthManagerRetention;
|
||||
use codex_app_server_protocol::AppListUpdatedNotification;
|
||||
use codex_app_server_protocol::AuthMode as LoginAuthMode;
|
||||
use codex_app_server_protocol::ChatgptAuthTokensRefreshParams;
|
||||
@@ -278,6 +279,7 @@ impl MessageProcessor {
|
||||
Arc::clone(&auth_manager),
|
||||
config.chatgpt_base_url.trim_end_matches('/').to_string(),
|
||||
config.analytics_enabled,
|
||||
AuthManagerRetention::Strong,
|
||||
);
|
||||
let thread_manager = Arc::new(ThreadManager::new(
|
||||
config.as_ref(),
|
||||
|
||||
@@ -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