mirror of
https://github.com/openai/codex.git
synced 2026-10-17 17:08:55 +00:00
Attribute external agent imports by provider (#34451)
## What changed - Add an optional opaque `providerId` to `externalAgentConfig/import` for analytics attribution, independently of the `migrationSource` selector. - Include the provider ID in completed-import and import-failure analytics events. - Have TUI imports identify their selected migration source as the provider. ## Testing - Cover provider attribution in app-server import completion and failure tests and analytics serialization tests. GitOrigin-RevId: 0ee7313a43652e5305939c526bf6b903a79334ad
This commit is contained in:
committed by
copyberry
parent
3bc49e1721
commit
a30aee8d90
@@ -3801,6 +3801,7 @@ async fn reducer_ingests_external_agent_config_import_completed_fact() {
|
||||
ExternalAgentConfigImportCompletedInput {
|
||||
import_id: "import-1".to_string(),
|
||||
source: "app_server".to_string(),
|
||||
provider_id: "test-provider-42".to_string(),
|
||||
item_type: "PLUGINS".to_string(),
|
||||
success_count: 2,
|
||||
failed_count: 1,
|
||||
@@ -3818,6 +3819,7 @@ async fn reducer_ingests_external_agent_config_import_completed_fact() {
|
||||
"event_params": {
|
||||
"import_id": "import-1",
|
||||
"source": "app_server",
|
||||
"provider_id": "test-provider-42",
|
||||
"type": "PLUGINS",
|
||||
"success_count": 2,
|
||||
"failed_count": 1,
|
||||
@@ -3835,6 +3837,7 @@ fn external_agent_config_import_failure_event_serializes_expected_shape() {
|
||||
event_params: CodexOnboardingExternalAgentImportFailureMetadata {
|
||||
import_id: "import-1".to_string(),
|
||||
source: "app_server".to_string(),
|
||||
provider_id: "test-provider-42".to_string(),
|
||||
item_type: "PLUGINS".to_string(),
|
||||
failure_stage: "plugin_import".to_string(),
|
||||
error_type: "plugin_import".to_string(),
|
||||
@@ -3853,6 +3856,7 @@ fn external_agent_config_import_failure_event_serializes_expected_shape() {
|
||||
"event_params": {
|
||||
"import_id": "import-1",
|
||||
"source": "app_server",
|
||||
"provider_id": "test-provider-42",
|
||||
"type": "PLUGINS",
|
||||
"failure_stage": "plugin_import",
|
||||
"error_type": "plugin_import",
|
||||
@@ -3874,6 +3878,7 @@ async fn reducer_ingests_external_agent_config_import_failure_fact() {
|
||||
ExternalAgentConfigImportFailureInput {
|
||||
import_id: "import-1".to_string(),
|
||||
source: "app_server".to_string(),
|
||||
provider_id: "test-provider-42".to_string(),
|
||||
item_type: "PLUGINS".to_string(),
|
||||
failure_stage: "plugin_import".to_string(),
|
||||
error_type: "plugin_import".to_string(),
|
||||
@@ -3892,6 +3897,7 @@ async fn reducer_ingests_external_agent_config_import_failure_fact() {
|
||||
"event_params": {
|
||||
"import_id": "import-1",
|
||||
"source": "app_server",
|
||||
"provider_id": "test-provider-42",
|
||||
"type": "PLUGINS",
|
||||
"failure_stage": "plugin_import",
|
||||
"error_type": "plugin_import",
|
||||
|
||||
@@ -1025,6 +1025,7 @@ pub(crate) struct CodexPluginInstallFailedEventRequest {
|
||||
pub(crate) struct CodexOnboardingExternalAgentImportCompleteMetadata {
|
||||
pub(crate) import_id: String,
|
||||
pub(crate) source: String,
|
||||
pub(crate) provider_id: String,
|
||||
#[serde(rename = "type")]
|
||||
pub(crate) item_type: String,
|
||||
pub(crate) success_count: usize,
|
||||
@@ -1042,6 +1043,7 @@ pub(crate) struct CodexOnboardingExternalAgentImportCompleteEventRequest {
|
||||
pub(crate) struct CodexOnboardingExternalAgentImportFailureMetadata {
|
||||
pub(crate) import_id: String,
|
||||
pub(crate) source: String,
|
||||
pub(crate) provider_id: String,
|
||||
#[serde(rename = "type")]
|
||||
pub(crate) item_type: String,
|
||||
pub(crate) failure_stage: String,
|
||||
|
||||
@@ -597,6 +597,7 @@ pub(crate) struct PluginInstallFailedInput {
|
||||
pub struct ExternalAgentConfigImportCompletedInput {
|
||||
pub import_id: String,
|
||||
pub source: String,
|
||||
pub provider_id: String,
|
||||
pub item_type: String,
|
||||
pub success_count: usize,
|
||||
pub failed_count: usize,
|
||||
@@ -605,6 +606,7 @@ pub struct ExternalAgentConfigImportCompletedInput {
|
||||
pub struct ExternalAgentConfigImportFailureInput {
|
||||
pub import_id: String,
|
||||
pub source: String,
|
||||
pub provider_id: String,
|
||||
pub item_type: String,
|
||||
pub failure_stage: String,
|
||||
pub error_type: String,
|
||||
|
||||
@@ -867,6 +867,7 @@ impl AnalyticsReducer {
|
||||
event_params: CodexOnboardingExternalAgentImportCompleteMetadata {
|
||||
import_id: input.import_id,
|
||||
source: input.source,
|
||||
provider_id: input.provider_id,
|
||||
item_type: input.item_type,
|
||||
success_count: input.success_count,
|
||||
failed_count: input.failed_count,
|
||||
@@ -887,6 +888,7 @@ impl AnalyticsReducer {
|
||||
event_params: CodexOnboardingExternalAgentImportFailureMetadata {
|
||||
import_id: input.import_id,
|
||||
source: input.source,
|
||||
provider_id: input.provider_id,
|
||||
item_type: input.item_type,
|
||||
failure_stage: input.failure_stage,
|
||||
error_type: input.error_type,
|
||||
|
||||
@@ -926,6 +926,13 @@
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"providerId": {
|
||||
"description": "Opaque provider identifier supplied by the caller for analytics attribution. This does not select the migration source.",
|
||||
"type": [
|
||||
"string",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"source": {
|
||||
"description": "Optional identifier for the product that initiated the import.",
|
||||
"type": [
|
||||
|
||||
+7
@@ -9790,6 +9790,13 @@
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"providerId": {
|
||||
"description": "Opaque provider identifier supplied by the caller for analytics attribution. This does not select the migration source.",
|
||||
"type": [
|
||||
"string",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"source": {
|
||||
"description": "Optional identifier for the product that initiated the import.",
|
||||
"type": [
|
||||
|
||||
+7
@@ -5968,6 +5968,13 @@
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"providerId": {
|
||||
"description": "Opaque provider identifier supplied by the caller for analytics attribution. This does not select the migration source.",
|
||||
"type": [
|
||||
"string",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"source": {
|
||||
"description": "Optional identifier for the product that initiated the import.",
|
||||
"type": [
|
||||
|
||||
+7
@@ -217,6 +217,13 @@
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"providerId": {
|
||||
"description": "Opaque provider identifier supplied by the caller for analytics attribution. This does not select the migration source.",
|
||||
"type": [
|
||||
"string",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"source": {
|
||||
"description": "Optional identifier for the product that initiated the import.",
|
||||
"type": [
|
||||
|
||||
+5
@@ -8,6 +8,11 @@ export type ExternalAgentConfigImportParams = { migrationItems: Array<ExternalAg
|
||||
* Optional identifier for the product that initiated the import.
|
||||
*/
|
||||
source?: string | null,
|
||||
/**
|
||||
* Opaque provider identifier supplied by the caller for analytics attribution. This does not
|
||||
* select the migration source.
|
||||
*/
|
||||
providerId?: string | null,
|
||||
/**
|
||||
* Migration-source selector used to produce the migration items. Pass the same value to
|
||||
* detection and import; missing or unrecognized values use the default source.
|
||||
|
||||
@@ -722,6 +722,10 @@ pub struct ExternalAgentConfigImportParams {
|
||||
/// Optional identifier for the product that initiated the import.
|
||||
#[ts(optional = nullable)]
|
||||
pub source: Option<String>,
|
||||
/// Opaque provider identifier supplied by the caller for analytics attribution. This does not
|
||||
/// select the migration source.
|
||||
#[ts(optional = nullable)]
|
||||
pub provider_id: Option<String>,
|
||||
/// Migration-source selector used to produce the migration items. Pass the same value to
|
||||
/// detection and import; missing or unrecognized values use the default source.
|
||||
#[ts(optional = nullable)]
|
||||
|
||||
@@ -436,6 +436,7 @@ fn external_agent_config_import_params_accept_legacy_plugin_details() {
|
||||
}),
|
||||
}],
|
||||
source: None,
|
||||
provider_id: None,
|
||||
migration_source: None,
|
||||
}
|
||||
);
|
||||
|
||||
@@ -244,7 +244,7 @@ Example with notification opt-out:
|
||||
- `feedback/upload` — submit a feedback report (classification + optional reason/logs, conversation_id, and optional `extraLogFiles` attachments array); returns the tracking thread id.
|
||||
- `config/read` — fetch the effective config on disk after resolving config layering, including opaque `desktop` values stored in `config.toml`.
|
||||
- `externalAgentConfig/detect` — detect migratable external-agent artifacts with `includeHome`, optional `cwds`, and an optional `migrationSource` selector. Omitted, `null`, or unrecognized migration-source values retain the default behavior. The deprecated optional `source` field remains accepted for compatibility but does not select the migration source. Each detected item includes `cwd` (`null` for home), and multi-item migrations may additionally include structured `details` with plugin ids, skill names, memory, session metadata, or other artifact names.
|
||||
- `externalAgentConfig/import` — apply selected external-agent migration items by passing explicit `migrationItems` with `cwd` (`null` for home) and any `details` returned by detect. Pass the same optional `migrationSource` used for detection so the server reads from the matching source; omitted, `null`, or unrecognized values retain the default behavior. The optional `source` continues to identify the product that initiated the import. The response acknowledges the synchronous import phase with an `importId`. Expected migration failures are reported as per-item failures rather than JSON-RPC errors, so the server still returns that `importId` and emits `externalAgentConfig/import/completed` with the same ID once all synchronous and background work finishes. The completion notification contains type-level `itemTypeResults` with successes and failures, including raw failure messages for the client to report separately.
|
||||
- `externalAgentConfig/import` — apply selected external-agent migration items by passing explicit `migrationItems` with `cwd` (`null` for home) and any `details` returned by detect. Pass the same optional `migrationSource` used for detection so the server reads from the matching source; omitted, `null`, or unrecognized values retain the default behavior. The optional `source` identifies the product that initiated the import, while the optional opaque `providerId` attributes analytics to the provider selected by that product without affecting migration-source selection. The response acknowledges the synchronous import phase with an `importId`. Expected migration failures are reported as per-item failures rather than JSON-RPC errors, so the server still returns that `importId` and emits `externalAgentConfig/import/completed` with the same ID once all synchronous and background work finishes. The completion notification contains type-level `itemTypeResults` with successes and failures, including raw failure messages for the client to report separately.
|
||||
- `externalAgentConfig/import/readHistories` — read completed import histories and connector candidates detected from successfully imported session histories. Connector candidates include a normalized display `name`, the number of imported sessions that used the connector, and the source metadata field used for detection.
|
||||
- `config/value/write` — write a single config key/value to the user's config.toml on disk; dotted paths such as `desktop.someKey` use the same generic write surface.
|
||||
- `config/batchWrite` — apply multiple config edits atomically to the user's config.toml on disk, with optional `reloadUserConfig: true` to hot-reload loaded threads, including multiple `desktop.*` edits.
|
||||
|
||||
@@ -172,6 +172,7 @@ impl ExternalAgentConfigRequestProcessor {
|
||||
}
|
||||
let import_id = Uuid::new_v4().to_string();
|
||||
let analytics_source = params.source.clone().unwrap_or_default();
|
||||
let provider_id = params.provider_id.clone().unwrap_or_default();
|
||||
let migration_service = self
|
||||
.migration_service
|
||||
.with_migration_source(params.migration_source.as_deref());
|
||||
@@ -223,6 +224,7 @@ impl ExternalAgentConfigRequestProcessor {
|
||||
&self.analytics_events_client,
|
||||
import_id,
|
||||
analytics_source,
|
||||
provider_id,
|
||||
&completed_item_results,
|
||||
)
|
||||
.await;
|
||||
@@ -317,6 +319,7 @@ impl ExternalAgentConfigRequestProcessor {
|
||||
&analytics_events_client,
|
||||
import_id,
|
||||
analytics_source,
|
||||
provider_id,
|
||||
&completed_item_results,
|
||||
)
|
||||
.await;
|
||||
@@ -480,11 +483,17 @@ async fn send_completed_import_notification(
|
||||
analytics_events_client: &AnalyticsEventsClient,
|
||||
import_id: String,
|
||||
analytics_source: String,
|
||||
provider_id: String,
|
||||
item_results: &[CoreImportItemResult],
|
||||
) {
|
||||
let notification = completed_notification(import_id, item_results);
|
||||
log_completed_import_failures(¬ification);
|
||||
track_completed_import_notification(analytics_events_client, &analytics_source, ¬ification);
|
||||
track_completed_import_notification(
|
||||
analytics_events_client,
|
||||
&analytics_source,
|
||||
&provider_id,
|
||||
¬ification,
|
||||
);
|
||||
if let Some(state_db) = state_db
|
||||
&& let Err(err) = record_completed_import_notification(state_db, ¬ification).await
|
||||
{
|
||||
@@ -522,6 +531,7 @@ fn log_completed_import_failures(notification: &ExternalAgentConfigImportComplet
|
||||
fn track_completed_import_notification(
|
||||
analytics_events_client: &AnalyticsEventsClient,
|
||||
analytics_source: &str,
|
||||
provider_id: &str,
|
||||
notification: &ExternalAgentConfigImportCompletedNotification,
|
||||
) {
|
||||
for type_result in ¬ification.item_type_results {
|
||||
@@ -530,6 +540,7 @@ fn track_completed_import_notification(
|
||||
ExternalAgentConfigImportCompletedInput {
|
||||
import_id: notification.import_id.clone(),
|
||||
source: analytics_source.to_string(),
|
||||
provider_id: provider_id.to_string(),
|
||||
item_type: item_type.clone(),
|
||||
success_count: type_result.successes.len(),
|
||||
failed_count: type_result.failures.len(),
|
||||
@@ -540,6 +551,7 @@ fn track_completed_import_notification(
|
||||
ExternalAgentConfigImportFailureInput {
|
||||
import_id: notification.import_id.clone(),
|
||||
source: analytics_source.to_string(),
|
||||
provider_id: provider_id.to_string(),
|
||||
item_type: item_type.clone(),
|
||||
failure_stage: failure.failure_stage.clone(),
|
||||
error_type: import_failure_error_type(failure),
|
||||
|
||||
@@ -1068,6 +1068,7 @@ async fn external_agent_config_import_reports_failed_sync_import_in_completion()
|
||||
"externalAgentConfig/import",
|
||||
Some(serde_json::json!({
|
||||
"source": "test_import",
|
||||
"providerId": "test-provider-42",
|
||||
"migrationItems": [
|
||||
{
|
||||
"itemType": "CONFIG",
|
||||
@@ -1165,6 +1166,7 @@ async fn external_agent_config_import_reports_failed_sync_import_in_completion()
|
||||
let event_params = &event["event_params"];
|
||||
assert_eq!(event_params["import_id"], import_id);
|
||||
assert_eq!(event_params["source"], "test_import");
|
||||
assert_eq!(event_params["provider_id"], "test-provider-42");
|
||||
assert_eq!(event_params["type"], "CONFIG");
|
||||
assert_eq!(event_params["failure_stage"], "import_request_failed");
|
||||
assert_eq!(event_params["error_type"], "invalid_existing_config");
|
||||
@@ -1209,6 +1211,8 @@ async fn external_agent_config_import_completed_tracks_analytics_event() -> Resu
|
||||
"externalAgentConfig/import",
|
||||
Some(serde_json::json!({
|
||||
"source": "test_import",
|
||||
"providerId": "test-provider-42",
|
||||
"migrationSource": SECONDARY_MIGRATION_SOURCE,
|
||||
"migrationItems": [{
|
||||
"itemType": "SESSIONS",
|
||||
"description": "Migrate recent sessions",
|
||||
@@ -1259,6 +1263,7 @@ async fn external_agent_config_import_completed_tracks_analytics_event() -> Resu
|
||||
let event_params = &event["event_params"];
|
||||
assert_eq!(event_params["import_id"], serde_json::json!(import_id));
|
||||
assert_eq!(event_params["source"], "test_import");
|
||||
assert_eq!(event_params["provider_id"], "test-provider-42");
|
||||
assert_eq!(event_params["type"], "SESSIONS");
|
||||
assert_eq!(event_params["success_count"], 0);
|
||||
assert_eq!(event_params["failed_count"], 1);
|
||||
@@ -1273,6 +1278,7 @@ async fn external_agent_config_import_completed_tracks_analytics_event() -> Resu
|
||||
let event_params = &event["event_params"];
|
||||
assert_eq!(event_params["import_id"], serde_json::json!(import_id));
|
||||
assert_eq!(event_params["source"], "test_import");
|
||||
assert_eq!(event_params["provider_id"], "test-provider-42");
|
||||
assert_eq!(event_params["type"], "SESSIONS");
|
||||
assert_eq!(event_params["failure_stage"], "session_missing");
|
||||
assert_eq!(event_params["error_type"], "session_missing");
|
||||
|
||||
@@ -471,6 +471,7 @@ impl AppServerSession {
|
||||
params: ExternalAgentConfigImportParams {
|
||||
migration_items,
|
||||
source: Some("cli".to_string()),
|
||||
provider_id: Some(migration_source.clone()),
|
||||
migration_source: Some(migration_source),
|
||||
},
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user