From 8a40200880168ee7ba520cdd95c778e7e28737b4 Mon Sep 17 00:00:00 2001 From: jameswt-oai Date: Tue, 16 Jun 2026 12:28:45 -0700 Subject: [PATCH] [codex-app-server-test-client] Plugin Install/Uninstall Analytics Smoke Test (#27100) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## This PR The original [combined remote plugin analytics PR #26281](https://github.com/openai/codex/pull/26281) mixed reusable analytics test infrastructure, two manual smoke workflows, a metadata refactor, and the final identity behavior. This PR adds the account-mutating validation workflow separately so its cleanup and recovery guarantees can be reviewed without the final analytics behavior change. - Add a manually invoked remote plugin install/uninstall smoke workflow. - Require explicit account-mutation confirmation and an initially uninstalled plugin. - Validate the current `codex_plugin_installed` contract, where `plugin_id` is the backend ID. - Restore and verify the original uninstalled state, with a dedicated recovery command. This baseline intentionally does not require `codex_plugin_uninstalled`, because production does not emit that event yet. The final PR will update this smoke to require local `plugin_id`, `remote_plugin_id`, and uninstall emission. Review this PR as the net diff against #27099. ## Testing - `just test -p codex-app-server-test-client` (3 focused capture/validation tests passed) - The live workflow was previously exercised on the green combined reference branch, and the original uninstalled account state was restored. - CI is green across the required platform matrix. ## Split Overview ```text main ├── #27093 Debug analytics capture │ └── #27099 Non-mutating plugin smoke │ └── #27100 Remote install/uninstall smoke ← you are here └── #27102 Plugin telemetry metadata refactor After #27093, #27099, #27100, and #27102 merge: └── Final PR: add remote_plugin_id to plugin analytics ``` Review order and dependencies: 1. [#27093 Add debug-only analytics event capture](https://github.com/openai/codex/pull/27093) (based on `main`) 2. [#27099 Add a plugin analytics smoke workflow](https://github.com/openai/codex/pull/27099) (stacked on #27093) 3. [#27100 Add a remote plugin analytics mutation smoke workflow](https://github.com/openai/codex/pull/27100) **(this PR, stacked on #27099)** 4. [#27102 Centralize plugin telemetry metadata construction](https://github.com/openai/codex/pull/27102) (independent, based on `main`) 5. Final remote-ID behavior PR (created after PRs 1-4 merge) The original [#26281](https://github.com/openai/codex/pull/26281) remains open as the green aggregate reference until the final PR is published. --- codex-rs/app-server-test-client/Cargo.toml | 1 - codex-rs/app-server-test-client/README.md | 52 ++ codex-rs/app-server-test-client/src/lib.rs | 66 +++ .../src/plugin_analytics_capture.rs | 102 ++++ .../src/plugin_analytics_capture_tests.rs | 93 ++++ .../src/plugin_analytics_mutation_smoke.rs | 483 ++++++++++++++++++ .../src/plugin_analytics_smoke.rs | 4 +- 7 files changed, 798 insertions(+), 3 deletions(-) create mode 100644 codex-rs/app-server-test-client/src/plugin_analytics_capture.rs create mode 100644 codex-rs/app-server-test-client/src/plugin_analytics_capture_tests.rs create mode 100644 codex-rs/app-server-test-client/src/plugin_analytics_mutation_smoke.rs diff --git a/codex-rs/app-server-test-client/Cargo.toml b/codex-rs/app-server-test-client/Cargo.toml index 603a5caf2..631b40207 100644 --- a/codex-rs/app-server-test-client/Cargo.toml +++ b/codex-rs/app-server-test-client/Cargo.toml @@ -25,5 +25,4 @@ url = { workspace = true } uuid = { workspace = true, features = ["v4"] } [lib] -test = false doctest = false diff --git a/codex-rs/app-server-test-client/README.md b/codex-rs/app-server-test-client/README.md index 4b4ed309a..5499706b9 100644 --- a/codex-rs/app-server-test-client/README.md +++ b/codex-rs/app-server-test-client/README.md @@ -49,6 +49,58 @@ prints the events and leaves the JSONL file in place for inspection. It does not install or uninstall plugins and does not modify the profile's persistent config. +### Testing remote install and uninstall analytics + +`plugin-analytics-mutation-smoke` is a manually invoked live smoke test. It +contacts the configured remote plugin API and temporarily changes the active +account's installed-plugin state. It is not run by `cargo test`, `just test`, +or CI. + +Choose a remote plugin that is available to the active account and is not +currently installed. The command refuses to run when the plugin is already +installed, installs it, validates `codex_plugin_installed`, uninstalls it, and +verifies that the original uninstalled state was restored. The current install +event uses the backend ID as `plugin_id`. Uninstall is part of cleanup but is +not yet an analytics assertion. + +`--remote-plugin-id` takes the backend ID, such as `plugins~Plugin_...`, not the +local `@` ID. + +```bash +cargo run -p codex-app-server-test-client -- \ + --codex-bin ./target/debug/codex \ + plugin-analytics-mutation-smoke \ + --remote-plugin-id \ + --confirm-account-mutation \ + --capture-file /tmp/plugin-mutation-analytics.jsonl +``` + +Analytics use the normal queue, reduction, batching, and serialization path, +but the debug capture destination suppresses analytics network delivery. The +command prints one of these final states: + +- `PASS`: the install event validated and the plugin is uninstalled. +- `FAIL-CLEAN`: validation failed, but the original uninstalled state was + restored. +- `FAIL-LOCAL-CACHE`: the backend is uninstalled, but local cleanup reported + an error. +- `FAIL-DIRTY`: cleanup failed and the plugin still appears installed. +- `FAIL-UNKNOWN`: the command could not verify the final installed state. + +For a dirty or uncertain result, retry cleanup with: + +```bash +cargo run -p codex-app-server-test-client -- \ + --codex-bin ./target/debug/codex \ + plugin-remote-uninstall \ + --remote-plugin-id \ + --confirm-account-mutation +``` + +Cleanup does not require analytics capture or a debug Codex binary. When the +smoke uses global `--config` overrides, its printed recovery command preserves +them so cleanup targets the same backend and account. + ## Watching Raw Inbound Traffic Initialize a connection, then print every inbound JSON-RPC message until you stop it with diff --git a/codex-rs/app-server-test-client/src/lib.rs b/codex-rs/app-server-test-client/src/lib.rs index b4e09d3e1..6edaa045b 100644 --- a/codex-rs/app-server-test-client/src/lib.rs +++ b/codex-rs/app-server-test-client/src/lib.rs @@ -88,6 +88,8 @@ use url::Url; use uuid::Uuid; mod loopback_responses_server; +mod plugin_analytics_capture; +mod plugin_analytics_mutation_smoke; mod plugin_analytics_smoke; const NOTIFICATIONS_TO_OPT_OUT: &[&str] = &[ @@ -285,6 +287,29 @@ enum CliCommand { #[arg(long)] capture_file: Option, }, + /// Install and uninstall one remote plugin while validating analytics capture. + #[command(name = "plugin-analytics-mutation-smoke")] + PluginAnalyticsMutationSmoke { + /// Backend remote plugin id. The plugin must be initially uninstalled. + #[arg(long)] + remote_plugin_id: String, + /// Acknowledge that this command mutates the active account's plugin state. + #[arg(long)] + confirm_account_mutation: bool, + /// JSONL output path. Defaults to a PID-specific file under the system temp directory. + #[arg(long)] + capture_file: Option, + }, + /// Best-effort recovery command that uninstalls one remote plugin. + #[command(name = "plugin-remote-uninstall")] + PluginRemoteUninstall { + /// Backend remote plugin id to uninstall. + #[arg(long)] + remote_plugin_id: String, + /// Acknowledge that this command mutates the active account's plugin state. + #[arg(long)] + confirm_account_mutation: bool, + }, } pub async fn run() -> Result<()> { @@ -447,6 +472,47 @@ pub async fn run() -> Result<()> { let codex_bin = codex_bin.context("plugin-analytics-smoke requires --codex-bin")?; plugin_analytics_smoke::run(&codex_bin, &config_overrides, &plugin_id, capture_file) } + CliCommand::PluginAnalyticsMutationSmoke { + remote_plugin_id, + confirm_account_mutation, + capture_file, + } => { + ensure_dynamic_tools_unused(&dynamic_tools, "plugin-analytics-mutation-smoke")?; + if url.is_some() { + bail!( + "plugin-analytics-mutation-smoke requires --codex-bin and does not support --url" + ); + } + let codex_bin = + codex_bin.context("plugin-analytics-mutation-smoke requires --codex-bin")?; + plugin_analytics_mutation_smoke::run( + &codex_bin, + &config_overrides, + &remote_plugin_id, + plugin_analytics_mutation_smoke::AccountMutationConfirmation::from_flag( + confirm_account_mutation, + ), + capture_file, + ) + } + CliCommand::PluginRemoteUninstall { + remote_plugin_id, + confirm_account_mutation, + } => { + ensure_dynamic_tools_unused(&dynamic_tools, "plugin-remote-uninstall")?; + if url.is_some() { + bail!("plugin-remote-uninstall requires --codex-bin and does not support --url"); + } + let codex_bin = codex_bin.context("plugin-remote-uninstall requires --codex-bin")?; + plugin_analytics_mutation_smoke::run_cleanup( + &codex_bin, + &config_overrides, + &remote_plugin_id, + plugin_analytics_mutation_smoke::AccountMutationConfirmation::from_flag( + confirm_account_mutation, + ), + ) + } } } diff --git a/codex-rs/app-server-test-client/src/plugin_analytics_capture.rs b/codex-rs/app-server-test-client/src/plugin_analytics_capture.rs new file mode 100644 index 000000000..4ffd3fd23 --- /dev/null +++ b/codex-rs/app-server-test-client/src/plugin_analytics_capture.rs @@ -0,0 +1,102 @@ +use anyhow::Context; +use anyhow::Result; +use anyhow::bail; +use serde_json::Value; +use std::fs; +use std::io; +use std::path::Path; + +pub(super) fn read_events_for_remote_plugin( + path: &Path, + remote_plugin_id: &str, +) -> Result> { + let contents = match fs::read_to_string(path) { + Ok(contents) => contents, + Err(err) if err.kind() == io::ErrorKind::NotFound => return Ok(Vec::new()), + Err(err) => { + return Err(err).with_context(|| format!("read capture file {}", path.display())); + } + }; + let mut matching = Vec::new(); + for (index, line) in contents.lines().enumerate() { + if line.trim().is_empty() { + continue; + } + let payload: Value = serde_json::from_str(line).with_context(|| { + format!( + "parse analytics capture line {} from {}", + index + 1, + path.display() + ) + })?; + let events = payload["events"] + .as_array() + .context("analytics capture payload is missing events")?; + matching.extend( + events + .iter() + .filter(|event| event["event_params"]["plugin_id"] == remote_plugin_id) + .cloned(), + ); + } + Ok(matching) +} + +pub(super) struct PluginEventIdentity<'a> { + pub(super) plugin_id: &'a str, + pub(super) plugin_name: &'a str, + pub(super) marketplace_name: &'a str, +} + +pub(super) fn validate_mutation_events( + events: Vec, + expected: PluginEventIdentity<'_>, +) -> Result> { + let event_type = "codex_plugin_installed"; + let matching = events + .iter() + .filter(|event| event["event_type"] == event_type) + .collect::>(); + let [event] = matching.as_slice() else { + bail!( + "expected exactly one `{event_type}` event for `{}`, found {}", + expected.plugin_id, + matching.len() + ); + }; + validate_event(event, &expected)?; + Ok(vec![(*event).clone()]) +} + +fn validate_event(event: &Value, expected: &PluginEventIdentity<'_>) -> Result<()> { + let params = &event["event_params"]; + require_string(params, "plugin_id", expected.plugin_id)?; + require_string(params, "plugin_name", expected.plugin_name)?; + require_string(params, "marketplace_name", expected.marketplace_name)?; + for field in [ + "has_skills", + "mcp_server_count", + "connector_ids", + "product_client_id", + ] { + if params.get(field).is_none_or(Value::is_null) { + bail!( + "{} event has null or missing `{field}`", + event["event_type"] + ); + } + } + Ok(()) +} + +fn require_string(params: &Value, field: &str, expected: &str) -> Result<()> { + let actual = params.get(field).and_then(Value::as_str); + if actual != Some(expected) { + bail!("expected `{field}` to be `{expected}`, got {actual:?}"); + } + Ok(()) +} + +#[cfg(test)] +#[path = "plugin_analytics_capture_tests.rs"] +mod tests; diff --git a/codex-rs/app-server-test-client/src/plugin_analytics_capture_tests.rs b/codex-rs/app-server-test-client/src/plugin_analytics_capture_tests.rs new file mode 100644 index 000000000..45a9c865b --- /dev/null +++ b/codex-rs/app-server-test-client/src/plugin_analytics_capture_tests.rs @@ -0,0 +1,93 @@ +use super::PluginEventIdentity; +use super::read_events_for_remote_plugin; +use super::validate_mutation_events; +use serde_json::Value; +use serde_json::json; +use std::fs; +use std::path::PathBuf; +use std::process; +use std::time::SystemTime; + +const REMOTE_PLUGIN_ID: &str = "plugins~Plugin_test"; + +#[test] +fn reads_and_validates_remote_plugin_mutation_events() { + let path = unique_capture_path("valid"); + let installed = mutation_event("codex_plugin_installed"); + let unrelated = json!({ + "event_type": "codex_plugin_installed", + "event_params": { + "plugin_id": "plugins~Plugin_other" + } + }); + let contents = [ + json!({"events": [unrelated]}), + json!({"events": [installed]}), + ] + .into_iter() + .map(|payload| serde_json::to_string(&payload).expect("serialize capture payload")) + .collect::>() + .join("\n"); + fs::write(&path, contents).expect("write capture file"); + + let events = read_events_for_remote_plugin(&path, REMOTE_PLUGIN_ID) + .expect("read matching plugin events"); + let validated = + validate_mutation_events(events, expected_identity()).expect("validate mutation events"); + + assert_eq!(validated, vec![installed]); + fs::remove_file(path).expect("remove capture file"); +} + +#[test] +fn rejects_duplicate_mutation_events() { + let installed = mutation_event("codex_plugin_installed"); + let error = validate_mutation_events(vec![installed.clone(), installed], expected_identity()) + .expect_err("duplicate install events should fail validation"); + + assert!(error.to_string().contains("found 2")); +} + +#[test] +fn rejects_missing_capability_metadata() { + let mut installed = mutation_event("codex_plugin_installed"); + installed["event_params"]["has_skills"] = Value::Null; + let error = validate_mutation_events(vec![installed], expected_identity()) + .expect_err("missing capability metadata should fail validation"); + + assert!(error.to_string().contains("has_skills")); +} + +fn mutation_event(event_type: &str) -> Value { + json!({ + "event_type": event_type, + "event_params": { + "plugin_id": REMOTE_PLUGIN_ID, + "plugin_name": "sample", + "marketplace_name": "openai-curated-remote", + "has_skills": true, + "mcp_server_count": 0, + "connector_ids": [], + "product_client_id": "test-client" + } + }) +} + +fn expected_identity() -> PluginEventIdentity<'static> { + PluginEventIdentity { + plugin_id: REMOTE_PLUGIN_ID, + plugin_name: "sample", + marketplace_name: "openai-curated-remote", + } +} + +fn unique_capture_path(name: &str) -> PathBuf { + let nonce = SystemTime::now() + .duration_since(SystemTime::UNIX_EPOCH) + .expect("system clock should be after Unix epoch") + .as_nanos(); + std::env::temp_dir().join(format!( + "codex-plugin-analytics-capture-{name}-{}-{nonce}.jsonl", + process::id() + )) +} diff --git a/codex-rs/app-server-test-client/src/plugin_analytics_mutation_smoke.rs b/codex-rs/app-server-test-client/src/plugin_analytics_mutation_smoke.rs new file mode 100644 index 000000000..2c6d50541 --- /dev/null +++ b/codex-rs/app-server-test-client/src/plugin_analytics_mutation_smoke.rs @@ -0,0 +1,483 @@ +use super::CodexClient; +use super::plugin_analytics_capture::PluginEventIdentity; +use super::plugin_analytics_capture::read_events_for_remote_plugin; +use super::plugin_analytics_capture::validate_mutation_events; +use super::plugin_analytics_smoke::ANALYTICS_CAPTURE_ENV_VAR; +use super::plugin_analytics_smoke::prepare_capture_file; +use super::plugin_analytics_smoke::wait_until_capture_is_ready; +use super::shell_quote; +use anyhow::Context; +use anyhow::Result; +use anyhow::anyhow; +use anyhow::bail; +use codex_app_server_protocol::ClientRequest; +use codex_app_server_protocol::PluginAvailability; +use codex_app_server_protocol::PluginInstallParams; +use codex_app_server_protocol::PluginInstallPolicy; +use codex_app_server_protocol::PluginInstallResponse; +use codex_app_server_protocol::PluginReadParams; +use codex_app_server_protocol::PluginReadResponse; +use codex_app_server_protocol::PluginUninstallParams; +use codex_app_server_protocol::PluginUninstallResponse; +use serde_json::Value; +use std::ffi::OsString; +use std::path::Path; +use std::path::PathBuf; +use std::process; +use std::thread; +use std::time::Duration; +use std::time::Instant; + +const REMOTE_MARKETPLACE_HINT: &str = "openai-curated-remote"; +const STATE_TIMEOUT: Duration = Duration::from_secs(15); +const CAPTURE_TIMEOUT: Duration = Duration::from_secs(10); +const POLL_INTERVAL: Duration = Duration::from_millis(100); + +pub(super) fn run( + codex_bin: &Path, + config_overrides: &[String], + remote_plugin_id: &str, + confirmation: AccountMutationConfirmation, + capture_file: Option, +) -> Result<()> { + require_confirmation(confirmation)?; + let capture_path = capture_file.unwrap_or_else(|| { + std::env::temp_dir().join(format!( + "codex-plugin-analytics-mutation-{}.jsonl", + process::id() + )) + }); + prepare_capture_file(&capture_path)?; + let mut client = spawn_client(codex_bin, config_overrides, &capture_path)?; + wait_until_capture_is_ready(&capture_path)?; + client.initialize()?; + + let initial = read_remote_plugin(&mut client, remote_plugin_id)?; + validate_initial_plugin(&initial, remote_plugin_id)?; + println!( + "remote plugin mutation smoke: local_id={} remote_id={} marketplace={}", + initial.plugin_id, initial.remote_plugin_id, initial.marketplace_name + ); + + let MutationSequenceResult { + result: sequence_result, + uninstall_rpc_failed, + } = run_mutation_sequence(&mut client, &capture_path, &initial); + let restoration = restore_uninstalled_state(&mut client, remote_plugin_id); + println!("capture file: {}", capture_path.display()); + + match (sequence_result, restoration) { + (Ok(events), RestorationStatus::Clean) => { + println!( + "\n[plugin analytics mutation smoke validated]\n{}", + serde_json::to_string_pretty(&events)? + ); + println!("PASS: analytics validated; original uninstalled state restored"); + Ok(()) + } + (Err(err), RestorationStatus::Clean) if uninstall_rpc_failed => { + eprintln!( + "FAIL-LOCAL-CACHE: backend state is uninstalled, but the uninstall RPC failed after the backend mutation: {err:#}" + ); + Err(err) + } + (Err(err), RestorationStatus::Clean) => { + eprintln!("FAIL-CLEAN: {err:#}"); + eprintln!("The original uninstalled account state was restored."); + Err(err) + } + (sequence_result, RestorationStatus::LocalCleanupFailure(cleanup_err)) => { + let sequence_err = sequence_result.err(); + eprintln!( + "FAIL-LOCAL-CACHE: backend state is uninstalled, but local cleanup reported an error: {cleanup_err:#}" + ); + Err(sequence_err.unwrap_or(cleanup_err)) + } + (sequence_result, RestorationStatus::Dirty(cleanup_err)) => { + if let Err(err) = sequence_result { + eprintln!("mutation smoke failed before cleanup: {err:#}"); + } + print_dirty_recovery(codex_bin, config_overrides, remote_plugin_id, &cleanup_err); + Err(cleanup_err) + } + (sequence_result, RestorationStatus::Unknown(cleanup_err)) => { + if let Err(err) = sequence_result { + eprintln!("mutation smoke failed before final state verification: {err:#}"); + } + eprintln!( + "FAIL-UNKNOWN: could not verify whether `{remote_plugin_id}` is installed: {cleanup_err:#}" + ); + print_recovery_command(codex_bin, config_overrides, remote_plugin_id); + Err(cleanup_err) + } + } +} + +pub(super) fn run_cleanup( + codex_bin: &Path, + config_overrides: &[String], + remote_plugin_id: &str, + confirmation: AccountMutationConfirmation, +) -> Result<()> { + require_confirmation(confirmation)?; + let mut overrides = config_overrides.to_vec(); + overrides.extend([ + "analytics.enabled=false".to_string(), + "features.plugins=true".to_string(), + "features.remote_plugin=true".to_string(), + ]); + let mut client = CodexClient::spawn_stdio(codex_bin, &overrides)?; + client.initialize()?; + + match restore_uninstalled_state(&mut client, remote_plugin_id) { + RestorationStatus::Clean => { + println!("PASS: `{remote_plugin_id}` is uninstalled"); + Ok(()) + } + RestorationStatus::LocalCleanupFailure(err) => { + eprintln!( + "FAIL-LOCAL-CACHE: backend state is uninstalled, but local cleanup reported an error: {err:#}" + ); + Err(err) + } + RestorationStatus::Dirty(err) => { + print_dirty_recovery(codex_bin, config_overrides, remote_plugin_id, &err); + Err(err) + } + RestorationStatus::Unknown(err) => { + eprintln!( + "FAIL-UNKNOWN: could not verify whether `{remote_plugin_id}` is installed: {err:#}" + ); + Err(err) + } + } +} + +#[derive(Clone, Copy)] +pub(super) enum AccountMutationConfirmation { + Confirmed, + Missing, +} + +impl AccountMutationConfirmation { + pub(super) fn from_flag(confirm_account_mutation: bool) -> Self { + if confirm_account_mutation { + Self::Confirmed + } else { + Self::Missing + } + } +} + +fn require_confirmation(confirmation: AccountMutationConfirmation) -> Result<()> { + if matches!(confirmation, AccountMutationConfirmation::Missing) { + bail!( + "this command installs and uninstalls a plugin on the active account; rerun with --confirm-account-mutation" + ); + } + Ok(()) +} + +#[derive(Clone, Copy, Debug)] +enum ExpectedInstalledState { + Installed, + Uninstalled, +} + +impl ExpectedInstalledState { + fn is_installed(self) -> bool { + matches!(self, Self::Installed) + } +} + +fn spawn_client( + codex_bin: &Path, + config_overrides: &[String], + capture_path: &Path, +) -> Result { + let mut overrides = config_overrides.to_vec(); + overrides.extend([ + "analytics.enabled=true".to_string(), + "features.plugins=true".to_string(), + "features.remote_plugin=true".to_string(), + ]); + let environment = vec![( + OsString::from(ANALYTICS_CAPTURE_ENV_VAR), + capture_path.as_os_str().to_os_string(), + )]; + CodexClient::spawn_stdio_with_env(codex_bin, &overrides, &environment) +} + +#[derive(Clone, Debug)] +struct RemotePluginExpectation { + plugin_id: String, + remote_plugin_id: String, + plugin_name: String, + marketplace_name: String, + installed: bool, + install_policy: PluginInstallPolicy, + availability: PluginAvailability, +} + +fn read_remote_plugin( + client: &mut CodexClient, + remote_plugin_id: &str, +) -> Result { + let request_id = client.request_id(); + let response: PluginReadResponse = client.send_request( + ClientRequest::PluginRead { + request_id: request_id.clone(), + params: PluginReadParams { + marketplace_path: None, + remote_marketplace_name: Some(REMOTE_MARKETPLACE_HINT.to_string()), + plugin_name: remote_plugin_id.to_string(), + }, + }, + request_id, + "plugin/read", + )?; + let summary = response.plugin.summary; + let actual_remote_plugin_id = summary + .remote_plugin_id + .with_context(|| format!("plugin/read returned no remote id for `{remote_plugin_id}`"))?; + if actual_remote_plugin_id != remote_plugin_id { + bail!( + "plugin/read returned remote id `{actual_remote_plugin_id}` for requested id `{remote_plugin_id}`" + ); + } + Ok(RemotePluginExpectation { + plugin_id: summary.id, + remote_plugin_id: actual_remote_plugin_id, + plugin_name: summary.name, + marketplace_name: response.plugin.marketplace_name, + installed: summary.installed, + install_policy: summary.install_policy, + availability: summary.availability, + }) +} + +fn validate_initial_plugin(plugin: &RemotePluginExpectation, remote_plugin_id: &str) -> Result<()> { + if plugin.installed { + bail!( + "refusing to run: remote plugin `{remote_plugin_id}` is already installed; choose an initially uninstalled plugin" + ); + } + if plugin.availability != PluginAvailability::Available { + bail!( + "remote plugin `{remote_plugin_id}` is not available: {:?}", + plugin.availability + ); + } + if plugin.install_policy == PluginInstallPolicy::NotAvailable { + bail!("remote plugin `{remote_plugin_id}` is not available for install"); + } + Ok(()) +} + +struct MutationSequenceResult { + result: Result>, + uninstall_rpc_failed: bool, +} + +fn run_mutation_sequence( + client: &mut CodexClient, + capture_path: &Path, + expected: &RemotePluginExpectation, +) -> MutationSequenceResult { + let mut uninstall_rpc_failed = false; + let result = (|| { + install_remote_plugin(client, expected)?; + wait_for_installed_state( + client, + &expected.remote_plugin_id, + ExpectedInstalledState::Installed, + )?; + wait_for_remote_plugin_event( + capture_path, + &expected.remote_plugin_id, + "codex_plugin_installed", + )?; + + let uninstall_error = uninstall_remote_plugin(client, &expected.remote_plugin_id).err(); + uninstall_rpc_failed = uninstall_error.is_some(); + wait_for_installed_state( + client, + &expected.remote_plugin_id, + ExpectedInstalledState::Uninstalled, + ) + .map_err(|state_err| { + if let Some(err) = uninstall_error.as_ref() { + anyhow!("plugin/uninstall failed: {err:#}; final state check failed: {state_err:#}") + } else { + state_err + } + })?; + + let captured_events = + read_events_for_remote_plugin(capture_path, &expected.remote_plugin_id)?; + let events = validate_mutation_events( + captured_events, + PluginEventIdentity { + plugin_id: &expected.remote_plugin_id, + plugin_name: &expected.plugin_name, + marketplace_name: &expected.marketplace_name, + }, + )?; + if let Some(err) = uninstall_error { + return Err(err.context( + "plugin/uninstall reported an error after the backend became uninstalled", + )); + } + Ok(events) + })(); + + MutationSequenceResult { + result, + uninstall_rpc_failed, + } +} + +fn install_remote_plugin(client: &mut CodexClient, plugin: &RemotePluginExpectation) -> Result<()> { + let request_id = client.request_id(); + let _: PluginInstallResponse = client.send_request( + ClientRequest::PluginInstall { + request_id: request_id.clone(), + params: PluginInstallParams { + marketplace_path: None, + remote_marketplace_name: Some(plugin.marketplace_name.clone()), + plugin_name: plugin.remote_plugin_id.clone(), + }, + }, + request_id, + "plugin/install", + )?; + Ok(()) +} + +fn uninstall_remote_plugin(client: &mut CodexClient, remote_plugin_id: &str) -> Result<()> { + let request_id = client.request_id(); + let _: PluginUninstallResponse = client.send_request( + ClientRequest::PluginUninstall { + request_id: request_id.clone(), + params: PluginUninstallParams { + plugin_id: remote_plugin_id.to_string(), + }, + }, + request_id, + "plugin/uninstall", + )?; + Ok(()) +} + +fn wait_for_installed_state( + client: &mut CodexClient, + remote_plugin_id: &str, + expected_state: ExpectedInstalledState, +) -> Result { + let deadline = Instant::now() + STATE_TIMEOUT; + loop { + match read_remote_plugin(client, remote_plugin_id) { + Ok(plugin) if plugin.installed == expected_state.is_installed() => return Ok(plugin), + Ok(_) => {} + Err(err) if Instant::now() >= deadline => return Err(err), + Err(_) => {} + } + if Instant::now() >= deadline { + bail!( + "timed out waiting for remote plugin `{remote_plugin_id}` to become {expected_state:?}" + ); + } + thread::sleep(POLL_INTERVAL); + } +} + +enum RestorationStatus { + Clean, + LocalCleanupFailure(anyhow::Error), + Dirty(anyhow::Error), + Unknown(anyhow::Error), +} + +fn restore_uninstalled_state( + client: &mut CodexClient, + remote_plugin_id: &str, +) -> RestorationStatus { + let current = match read_remote_plugin(client, remote_plugin_id) { + Ok(current) => current, + Err(err) => return RestorationStatus::Unknown(err), + }; + if !current.installed { + return RestorationStatus::Clean; + } + + let uninstall_result = uninstall_remote_plugin(client, remote_plugin_id); + match wait_for_installed_state( + client, + remote_plugin_id, + ExpectedInstalledState::Uninstalled, + ) { + Ok(_) => match uninstall_result { + Ok(()) => RestorationStatus::Clean, + Err(err) => RestorationStatus::LocalCleanupFailure(err), + }, + Err(state_err) => { + let error = match uninstall_result { + Ok(()) => state_err, + Err(uninstall_err) => anyhow!( + "cleanup uninstall failed: {uninstall_err:#}; state verification failed: {state_err:#}" + ), + }; + RestorationStatus::Dirty(error) + } + } +} + +fn wait_for_remote_plugin_event( + path: &Path, + remote_plugin_id: &str, + event_type: &str, +) -> Result<()> { + let deadline = Instant::now() + CAPTURE_TIMEOUT; + loop { + let events = read_events_for_remote_plugin(path, remote_plugin_id)?; + if events.iter().any(|event| event["event_type"] == event_type) { + return Ok(()); + } + if Instant::now() >= deadline { + bail!("timed out waiting for `{event_type}` for remote plugin `{remote_plugin_id}`"); + } + thread::sleep(POLL_INTERVAL); + } +} + +fn print_dirty_recovery( + codex_bin: &Path, + config_overrides: &[String], + remote_plugin_id: &str, + err: &anyhow::Error, +) { + eprintln!( + "FAIL-DIRTY: remote plugin `{remote_plugin_id}` still appears installed after cleanup: {err:#}" + ); + print_recovery_command(codex_bin, config_overrides, remote_plugin_id); +} + +fn print_recovery_command(codex_bin: &Path, config_overrides: &[String], remote_plugin_id: &str) { + let test_client = std::env::current_exe() + .map(|path| path.display().to_string()) + .unwrap_or_else(|_| "codex-app-server-test-client".to_string()); + let mut command = format!( + "{} --codex-bin {}", + shell_quote(&test_client), + shell_quote(&codex_bin.display().to_string()) + ); + for override_kv in config_overrides { + command.push_str(&format!(" --config {}", shell_quote(override_kv))); + } + command.push_str(&format!( + " plugin-remote-uninstall --remote-plugin-id {} --confirm-account-mutation", + shell_quote(remote_plugin_id) + )); + eprintln!("Recovery command:"); + eprintln!(" {command}"); +} diff --git a/codex-rs/app-server-test-client/src/plugin_analytics_smoke.rs b/codex-rs/app-server-test-client/src/plugin_analytics_smoke.rs index 30b21d28f..701f6361a 100644 --- a/codex-rs/app-server-test-client/src/plugin_analytics_smoke.rs +++ b/codex-rs/app-server-test-client/src/plugin_analytics_smoke.rs @@ -27,7 +27,7 @@ use std::thread; use std::time::Duration; use std::time::Instant; -const ANALYTICS_CAPTURE_ENV_VAR: &str = "CODEX_ANALYTICS_EVENTS_CAPTURE_FILE"; +pub(super) const ANALYTICS_CAPTURE_ENV_VAR: &str = "CODEX_ANALYTICS_EVENTS_CAPTURE_FILE"; const TEST_USER_CONFIG_ENV_VAR: &str = "CODEX_APP_SERVER_TEST_USER_CONFIG_FILE"; const CAPTURE_READY_TIMEOUT: Duration = Duration::from_secs(5); const CAPTURE_TIMEOUT: Duration = Duration::from_secs(10); @@ -282,7 +282,7 @@ fn quoted(value: &str) -> Result { serde_json::to_string(value).context("serialize config string") } -fn prepare_capture_file(path: &Path) -> Result<()> { +pub(super) fn prepare_capture_file(path: &Path) -> Result<()> { let parent = path .parent() .context("capture file must have a parent directory")?;