Clarify plugin lifecycle terminal outcomes

codex-rs/core/src/plugin_script_lifecycle.rs

@@ -34,10 +34,18 @@ pub(crate) struct PluginScriptExecution {
     emitted: std::sync::Mutex<Vec<CodexPluginScriptLifecycleEvent>>,
 }
 
+/// Terminal process result used to classify one plugin script execution.
+pub(crate) enum PluginScriptTerminalOutcome {
+    Exited { exit_code: i32 },
+    Failed { exit_code: Option<i32> },
+}
+
 #[derive(Clone)]
 struct PluginScriptEvent {
+    session_id: String,
     thread_id: String,
     turn_id: String,
+    product_client_id: String,
     plugin_id: String,
     execution_id: String,
     script_path: String,
@@ -66,8 +74,10 @@ impl PluginScriptExecution {
         Some(Arc::new(Self::new(
             session.services.analytics_events_client.clone(),
             PluginScriptEvent {
+                session_id: session.session_id().to_string(),
                 thread_id: session.thread_id.to_string(),
                 turn_id: turn.sub_id.clone(),
+                product_client_id: turn.originator.clone(),
                 plugin_id: resolved.plugin_id,
                 execution_id: Uuid::new_v4().to_string(),
                 script_path: resolved.script_path,
@@ -91,7 +101,7 @@ impl PluginScriptExecution {
         self.interrupted.store(true, Ordering::Release);
     }
 
-    pub(crate) fn finish(&self, exit_code: Option<i32>, failed: bool) {
+    pub(crate) fn finish(&self, outcome: PluginScriptTerminalOutcome) {
         let Some(started_at) = self.started_at.get() else {
             return;
         };
@@ -99,12 +109,21 @@ impl PluginScriptExecution {
             return;
         }
 
+        let (exit_code, terminal_status) = match outcome {
+            PluginScriptTerminalOutcome::Exited { exit_code: 0 } => {
+                (Some(0), PluginScriptLifecycleStatus::Completed)
+            }
+            PluginScriptTerminalOutcome::Exited { exit_code } => {
+                (Some(exit_code), PluginScriptLifecycleStatus::Failed)
+            }
+            PluginScriptTerminalOutcome::Failed { exit_code } => {
+                (exit_code, PluginScriptLifecycleStatus::Failed)
+            }
+        };
         let status = if self.interrupted.load(Ordering::Acquire) {
             PluginScriptLifecycleStatus::Interrupted
-        } else if !failed && exit_code == Some(0) {
-            PluginScriptLifecycleStatus::Completed
         } else {
-            PluginScriptLifecycleStatus::Failed
+            terminal_status
         };
         let duration_ms = u64::try_from(started_at.elapsed().as_millis()).unwrap_or(u64::MAX);
         self.emit(self.event(status, Some(duration_ms), exit_code));
@@ -129,8 +148,10 @@ impl PluginScriptExecution {
         exit_code: Option<i32>,
     ) -> CodexPluginScriptLifecycleEvent {
         CodexPluginScriptLifecycleEvent {
+            session_id: self.event.session_id.clone(),
             thread_id: self.event.thread_id.clone(),
             turn_id: self.event.turn_id.clone(),
+            product_client_id: self.event.product_client_id.clone(),
             plugin_id: self.event.plugin_id.clone(),
             execution_id: self.event.execution_id.clone(),
             script_path: self.event.script_path.clone(),

codex-rs/core/src/plugin_script_lifecycle_tests.rs

@@ -13,8 +13,10 @@ fn execution() -> PluginScriptExecution {
     PluginScriptExecution::new(
         AnalyticsEventsClient::disabled(),
         PluginScriptEvent {
+            session_id: "session".to_string(),
             thread_id: "thread".to_string(),
             turn_id: "turn".to_string(),
+            product_client_id: "codex-test".to_string(),
             plugin_id: "plugin".to_string(),
             execution_id: "execution".to_string(),
             script_path: "scripts/run.py".to_string(),
@@ -47,7 +49,7 @@ fn statuses(execution: &PluginScriptExecution) -> Vec<&'static str> {
 fn finish_before_start_emits_no_transition() {
     let execution = execution();
 
-    execution.finish(Some(0), /*failed*/ false);
+    execution.finish(PluginScriptTerminalOutcome::Exited { exit_code: 0 });
 
     assert_eq!(statuses(&execution), Vec::<&str>::new());
 }
@@ -67,17 +69,20 @@ fn exit_zero_completes() {
     let execution = execution();
 
     execution.mark_started();
-    execution.finish(Some(0), /*failed*/ false);
+    execution.finish(PluginScriptTerminalOutcome::Exited { exit_code: 0 });
 
     assert_eq!(statuses(&execution), vec!["started", "completed"]);
 }
 
 #[test]
-fn nonzero_exit_and_executor_error_fail() {
-    for (exit_code, failed) in [(Some(9), false), (None, true)] {
+fn nonzero_exit_and_failed_outcome_fail() {
+    for outcome in [
+        PluginScriptTerminalOutcome::Exited { exit_code: 9 },
+        PluginScriptTerminalOutcome::Failed { exit_code: None },
+    ] {
         let execution = execution();
         execution.mark_started();
-        execution.finish(exit_code, failed);
+        execution.finish(outcome);
 
         assert_eq!(statuses(&execution), vec!["started", "failed"]);
     }
@@ -89,7 +94,7 @@ fn cancellation_wins_over_terminal_classification() {
 
     execution.mark_started();
     execution.mark_interrupted();
-    execution.finish(Some(0), /*failed*/ false);
+    execution.finish(PluginScriptTerminalOutcome::Exited { exit_code: 0 });
 
     assert_eq!(statuses(&execution), vec!["started", "interrupted"]);
 }
@@ -99,8 +104,8 @@ fn terminal_transition_is_idempotent() {
     let execution = execution();
 
     execution.mark_started();
-    execution.finish(Some(0), /*failed*/ false);
-    execution.finish(Some(9), /*failed*/ true);
+    execution.finish(PluginScriptTerminalOutcome::Exited { exit_code: 0 });
+    execution.finish(PluginScriptTerminalOutcome::Failed { exit_code: Some(9) });
 
     assert_eq!(statuses(&execution), vec!["started", "completed"]);
 }
@@ -112,7 +117,7 @@ fn duration_begins_at_actual_start() {
 
     thread::sleep(Duration::from_millis(25));
     execution.mark_started();
-    execution.finish(Some(0), /*failed*/ false);
+    execution.finish(PluginScriptTerminalOutcome::Exited { exit_code: 0 });
 
     let duration_ms = emitted(&execution)[1].duration_ms.expect("duration");
     let total_elapsed_ms = before_pre_start_delay.elapsed().as_millis();
@@ -120,12 +125,16 @@ fn duration_begins_at_actual_start() {
 }
 
 #[test]
-fn start_and_terminal_facts_retain_one_execution_id() {
+fn start_and_terminal_facts_retain_execution_and_session_identity() {
     let execution = execution();
 
     execution.mark_started();
-    execution.finish(Some(0), /*failed*/ false);
+    execution.finish(PluginScriptTerminalOutcome::Exited { exit_code: 0 });
     let events = emitted(&execution);
 
     assert_eq!(events[0].execution_id, events[1].execution_id);
+    assert_eq!(events[0].session_id, "session");
+    assert_eq!(events[1].session_id, "session");
+    assert_eq!(events[0].product_client_id, "codex-test");
+    assert_eq!(events[1].product_client_id, "codex-test");
 }