From 2a6c9385bb483565aa48bf22aefb49aa3b122ef8 Mon Sep 17 00:00:00 2001 From: Alex Fournier Date: Tue, 23 Jun 2026 12:35:59 -0700 Subject: [PATCH 01/20] feat: add dynamic plugin lifecycle CLI management Signed-off-by: Alex Fournier --- crates/cli/src/config.rs | 78 +++- crates/cli/src/main.rs | 11 +- crates/cli/src/plugins.rs | 3 +- crates/cli/src/plugins/config_io.rs | 153 ++++++- crates/cli/src/plugins/lifecycle.rs | 355 +++++++++++++++ crates/cli/src/plugins/lifecycle/render.rs | 253 +++++++++++ crates/cli/src/plugins/lifecycle/state.rs | 278 ++++++++++++ crates/cli/src/plugins/lifecycle/target.rs | 29 ++ crates/cli/tests/coverage/main_tests.rs | 49 ++- .../tests/coverage/plugins_lifecycle_tests.rs | 410 ++++++++++++++++++ crates/cli/tests/coverage/plugins_tests.rs | 17 +- crates/core/src/plugin/dynamic/registry.rs | 22 + 12 files changed, 1639 insertions(+), 19 deletions(-) create mode 100644 crates/cli/src/plugins/lifecycle.rs create mode 100644 crates/cli/src/plugins/lifecycle/render.rs create mode 100644 crates/cli/src/plugins/lifecycle/state.rs create mode 100644 crates/cli/src/plugins/lifecycle/target.rs create mode 100644 crates/cli/tests/coverage/plugins_lifecycle_tests.rs diff --git a/crates/cli/src/config.rs b/crates/cli/src/config.rs index 9c4d4c7c..a9df1d3b 100644 --- a/crates/cli/src/config.rs +++ b/crates/cli/src/config.rs @@ -202,6 +202,20 @@ pub(crate) struct PluginsCommand { pub(crate) enum PluginsSubcommand { /// Interactively create or edit built-in plugin configuration in `plugins.toml`. Edit(PluginsEditCommand), + /// Register a manifest-backed dynamic plugin in `plugins.toml`. + Add(PluginsAddCommand), + /// Validate a manifest-backed dynamic plugin by path or installed ID. + Validate(PluginsValidateCommand), + /// List discovered dynamic plugins from the resolved host config. + List(PluginsListCommand), + /// Inspect one discovered dynamic plugin by canonical ID. + Inspect(PluginsInspectCommand), + /// Mark a registered dynamic plugin enabled in desired state. + Enable(PluginsEnableCommand), + /// Mark a registered dynamic plugin disabled in desired state. + Disable(PluginsDisableCommand), + /// Tombstone a registered dynamic plugin and remove its host discovery reference. + Remove(PluginsRemoveCommand), } /// Args for `nemo-relay pricing`. @@ -298,7 +312,7 @@ pub(crate) struct PricingResolveCommand { .args(["user", "project", "global"]) .multiple(false) ))] -pub(crate) struct PluginsEditCommand { +pub(crate) struct PluginsScopeArgs { /// Edit the user config at `$XDG_CONFIG_HOME/nemo-relay/plugins.toml`. #[arg(long)] pub(crate) user: bool, @@ -310,6 +324,61 @@ pub(crate) struct PluginsEditCommand { pub(crate) global: bool, } +/// Args for `nemo-relay plugins edit`. +#[derive(Debug, Clone, Default, Args)] +pub(crate) struct PluginsEditCommand { + #[command(flatten)] + pub(crate) scope: PluginsScopeArgs, +} + +/// Args for `nemo-relay plugins add`. +#[derive(Debug, Clone, Default, Args)] +pub(crate) struct PluginsAddCommand { + #[command(flatten)] + pub(crate) scope: PluginsScopeArgs, + /// Path to a plugin directory or explicit `relay-plugin.toml`. + pub(crate) path: PathBuf, +} + +/// Args for `nemo-relay plugins validate`. +#[derive(Debug, Clone, Args)] +pub(crate) struct PluginsValidateCommand { + /// Canonical plugin ID or a local plugin directory / `relay-plugin.toml` path. + pub(crate) target: String, +} + +/// Args for `nemo-relay plugins list`. +#[derive(Debug, Clone, Default, Args)] +pub(crate) struct PluginsListCommand {} + +/// Args for `nemo-relay plugins inspect`. +#[derive(Debug, Clone, Args)] +pub(crate) struct PluginsInspectCommand { + /// Canonical plugin ID. + pub(crate) id: String, +} + +/// Args for `nemo-relay plugins enable`. +#[derive(Debug, Clone, Args)] +pub(crate) struct PluginsEnableCommand { + /// Canonical plugin ID. + pub(crate) id: String, +} + +/// Args for `nemo-relay plugins disable`. +#[derive(Debug, Clone, Args)] +pub(crate) struct PluginsDisableCommand { + /// Canonical plugin ID. + pub(crate) id: String, +} + +/// Args for `nemo-relay plugins remove`. +#[derive(Debug, Clone, Args)] +pub(crate) struct PluginsRemoveCommand { + /// Canonical plugin ID. + pub(crate) id: String, +} + #[derive(Debug, Clone, Default, Args)] pub(crate) struct ServerArgs { /// Path to an explicit config file (disables auto-discovery of workspace/global/system) @@ -623,6 +692,13 @@ pub(crate) fn resolve_server_config(args: &ServerArgs) -> Result, +) -> Result { + load_shared_config(explicit) +} + /// Resolves transparent `run` configuration and switches the gateway to an ephemeral bind address. /// /// Explicit run arguments override inherited top-level server flags, which override shared config. diff --git a/crates/cli/src/main.rs b/crates/cli/src/main.rs index 0c216ffc..0c713878 100644 --- a/crates/cli/src/main.rs +++ b/crates/cli/src/main.rs @@ -84,7 +84,7 @@ async fn run_command(command: Command, server: &ServerArgs) -> Result run_config(command).await, - Command::Plugins(command) => run_plugins(command), + Command::Plugins(command) => run_plugins(command, server), Command::Pricing(command) => run_pricing(command), Command::Doctor(command) => run_doctor(command).await, Command::Agents(command) => doctor::run_agents(command.json).await, @@ -101,9 +101,16 @@ async fn run_config(command: ConfigCommand) -> Result Ok(ExitCode::SUCCESS) } -fn run_plugins(command: PluginsCommand) -> Result { +fn run_plugins(command: PluginsCommand, server: &ServerArgs) -> Result { match command.command { PluginsSubcommand::Edit(command) => plugins::edit(command)?, + PluginsSubcommand::Add(command) => plugins::lifecycle::add(command, server)?, + PluginsSubcommand::Validate(command) => plugins::lifecycle::validate(command, server)?, + PluginsSubcommand::List(command) => plugins::lifecycle::list(command, server)?, + PluginsSubcommand::Inspect(command) => plugins::lifecycle::inspect(command, server)?, + PluginsSubcommand::Enable(command) => plugins::lifecycle::enable(command, server)?, + PluginsSubcommand::Disable(command) => plugins::lifecycle::disable(command, server)?, + PluginsSubcommand::Remove(command) => plugins::lifecycle::remove(command, server)?, } Ok(ExitCode::SUCCESS) } diff --git a/crates/cli/src/plugins.rs b/crates/cli/src/plugins.rs index 30ced81b..708fc94d 100644 --- a/crates/cli/src/plugins.rs +++ b/crates/cli/src/plugins.rs @@ -22,6 +22,7 @@ use crate::error::CliError; pub(crate) mod config_io; mod editor_model; +pub(crate) mod lifecycle; use self::config_io::*; use self::editor_model::*; @@ -91,7 +92,7 @@ fn print_save_success(path: &Path) { pub(crate) fn edit(command: PluginsEditCommand) -> Result<(), CliError> { ensure_tty()?; - let scope = target_scope(&command)?; + let scope = target_scope(&command.scope)?; let path = target_path(scope)?; let mut config = read_plugin_config(&path)?; ensure_observability_component(&mut config)?; diff --git a/crates/cli/src/plugins/config_io.rs b/crates/cli/src/plugins/config_io.rs index 1928d47d..10b86f08 100644 --- a/crates/cli/src/plugins/config_io.rs +++ b/crates/cli/src/plugins/config_io.rs @@ -6,13 +6,15 @@ use std::path::{Path, PathBuf}; use console::style; +use nemo_relay::plugin::dynamic::DynamicPluginManifest; use nemo_relay::plugin::{ConfigPolicy, PluginConfig, validate_plugin_config}; use nemo_relay_adaptive::plugin_component::register_adaptive_component; use nemo_relay_pii_redaction::component::register_pii_redaction_component; +use serde::Serialize; use serde_json::{Map, Value}; use crate::config::{ - PluginsEditCommand, global_plugin_config_path, project_plugin_config_path, + PluginsScopeArgs, global_plugin_config_path, project_plugin_config_path, user_plugin_config_path, }; use crate::error::CliError; @@ -24,7 +26,7 @@ pub(crate) enum TargetScope { Global, } -pub(crate) fn target_scope(command: &PluginsEditCommand) -> Result { +pub(crate) fn target_scope(command: &PluginsScopeArgs) -> Result { let selected = [command.user, command.project, command.global] .into_iter() .filter(|selected| *selected) @@ -43,6 +45,13 @@ pub(crate) fn target_scope(command: &PluginsEditCommand) -> Result, +} + pub(crate) fn target_path(scope: TargetScope) -> Result { match scope { TargetScope::User => user_plugin_config_path().ok_or_else(|| { @@ -95,6 +104,146 @@ pub(crate) fn write_plugin_config(path: &Path, config: &PluginConfig) -> Result< Ok(()) } +pub(crate) fn append_dynamic_plugin_reference( + path: &Path, + manifest_ref: &str, +) -> Result<(), CliError> { + let mut root = read_plugin_toml_root(path)?; + + let root_table = root + .as_table_mut() + .expect("root plugin TOML is always a table"); + let plugins = root_table + .entry("plugins") + .or_insert_with(|| toml::Value::Table(toml::map::Map::new())) + .as_table_mut() + .ok_or_else(|| { + CliError::Config(format!( + "invalid plugin TOML in {}: [plugins] must be a table", + path.display() + )) + })?; + let dynamic = plugins + .entry("dynamic") + .or_insert_with(|| toml::Value::Array(Vec::new())) + .as_array_mut() + .ok_or_else(|| { + CliError::Config(format!( + "invalid plugin TOML in {}: plugins.dynamic must be an array of tables", + path.display() + )) + })?; + dynamic.push( + toml::Value::try_from(DynamicPluginReferenceEntry { + manifest: manifest_ref.to_owned(), + config: Map::new(), + }) + .map_err(|error| { + CliError::Config(format!( + "could not serialize dynamic plugin reference for {}: {error}", + path.display() + )) + })?, + ); + + write_plugin_toml_root(path, &root)?; + Ok(()) +} + +pub(crate) fn remove_dynamic_plugin_reference( + path: &Path, + plugin_id: &str, +) -> Result { + if !path.exists() { + return Ok(false); + } + + let mut root = read_plugin_toml_root(path)?; + let Some(root_table) = root.as_table_mut() else { + return Ok(false); + }; + let Some(toml::Value::Table(plugins)) = root_table.get_mut("plugins") else { + return Ok(false); + }; + let Some(toml::Value::Array(dynamic_entries)) = plugins.get_mut("dynamic") else { + return Ok(false); + }; + + let original_len = dynamic_entries.len(); + let mut retained = Vec::with_capacity(original_len); + for entry in dynamic_entries.drain(..) { + let manifest_ref = entry + .as_table() + .and_then(|entry| entry.get("manifest")) + .and_then(toml::Value::as_str) + .map(|manifest| resolve_manifest_ref(path, manifest)); + + let keep = match manifest_ref { + Some(manifest_ref) => { + let (manifest, _) = DynamicPluginManifest::load_from_path(&manifest_ref) + .map_err(|error| CliError::Config(error.to_string()))?; + manifest.plugin.id.trim() != plugin_id + } + None => true, + }; + + if keep { + retained.push(entry); + } + } + + let removed = retained.len() != original_len; + *dynamic_entries = retained; + if dynamic_entries.is_empty() { + plugins.remove("dynamic"); + } + if plugins.is_empty() { + root_table.remove("plugins"); + } + if removed { + write_plugin_toml_root(path, &root)?; + } + Ok(removed) +} + +fn read_plugin_toml_root(path: &Path) -> Result { + if path.exists() { + let raw = std::fs::read_to_string(path)?; + raw.parse::() + .map(toml::Value::Table) + .map_err(|error| { + CliError::Config(format!( + "invalid plugin TOML in {}: {error}", + path.display() + )) + }) + } else { + Ok(toml::Value::Table(toml::map::Map::new())) + } +} + +fn write_plugin_toml_root(path: &Path, root: &toml::Value) -> Result<(), CliError> { + let rendered = toml::to_string_pretty(root) + .map_err(|error| CliError::Config(format!("could not render plugin TOML: {error}")))?; + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent)?; + } + std::fs::write(path, rendered)?; + Ok(()) +} + +fn resolve_manifest_ref(source: &Path, manifest: &str) -> PathBuf { + let manifest = PathBuf::from(manifest); + if manifest.is_absolute() { + manifest + } else { + source + .parent() + .map(|parent| parent.join(&manifest)) + .unwrap_or(manifest) + } +} + pub(super) fn print_preview(config: &PluginConfig) -> Result<(), CliError> { println!(); println!( diff --git a/crates/cli/src/plugins/lifecycle.rs b/crates/cli/src/plugins/lifecycle.rs new file mode 100644 index 00000000..a1f11064 --- /dev/null +++ b/crates/cli/src/plugins/lifecycle.rs @@ -0,0 +1,355 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +use std::collections::HashMap; +use std::path::PathBuf; + +use nemo_relay::plugin::dynamic::{ + DynamicPluginCheckState, DynamicPluginManifest, DynamicPluginRecord, + DynamicPluginValidationStatus, +}; + +use crate::config::{ + PluginsAddCommand, PluginsDisableCommand, PluginsEnableCommand, PluginsInspectCommand, + PluginsListCommand, PluginsRemoveCommand, PluginsValidateCommand, ResolvedConfig, + ResolvedDynamicPluginConfig, ServerArgs, resolve_plugins_config, +}; +use crate::error::CliError; + +use super::config_io::{ + append_dynamic_plugin_reference, remove_dynamic_plugin_reference, target_scope, +}; + +mod render; +mod state; +mod target; + +use self::render::{render_inspect, render_list, render_validation_summary}; +use self::state::{ + RegistryScope, ScopedRegistry, collect_records, find_record_by_id, load_scoped_registries, + scoped_paths_for_add, +}; +use self::target::PluginTarget; + +pub(crate) fn add(command: PluginsAddCommand, server: &ServerArgs) -> Result<(), CliError> { + let resolved = resolve_plugins_config(server.config.as_ref())?; + let mut scopes = load_and_hydrate_scopes(server.config.as_ref(), &resolved)?; + let (manifest, manifest_ref) = load_manifest_for_action("add", &command.path)?; + let plugin_id = manifest.plugin.id.trim().to_owned(); + if let Some(existing) = find_record_by_id(&scopes, &plugin_id)? + && !existing.record.is_tombstoned() + { + return Err(CliError::Config(format!( + "dynamic plugin '{}' is already registered in the {} lifecycle scope", + plugin_id, + existing.scope.label() + ))); + } + + if server.config.is_some() && scope_flags_selected(&command.scope) { + return Err(CliError::Config( + "--config cannot be combined with --user, --project, or --global for `plugins add`" + .into(), + )); + } + + let (plugins_toml_path, state_path, scope) = + scoped_paths_for_add(target_scope(&command.scope)?, server.config.as_ref())?; + let scope_index = ensure_scope(&mut scopes, scope, plugins_toml_path.clone(), state_path); + let record = validated_record_from_manifest(manifest, manifest_ref.clone())?; + + scopes[scope_index] + .registry + .add(record) + .map_err(|error| CliError::Config(error.to_string()))?; + append_dynamic_plugin_reference(&plugins_toml_path, &manifest_ref)?; + if let Err(error) = scopes[scope_index].save() { + let _ = remove_dynamic_plugin_reference(&plugins_toml_path, &plugin_id); + return Err(error); + } + + println!("Added dynamic plugin {}", plugin_id); + println!("scope: {}", scope.label()); + println!("manifest: {}", manifest_ref); + println!("plugins_toml: {}", plugins_toml_path.display()); + println!("state_path: {}", scopes[scope_index].state_path.display()); + println!("desired.enabled: false"); + Ok(()) +} + +pub(crate) fn validate( + command: PluginsValidateCommand, + server: &ServerArgs, +) -> Result<(), CliError> { + match PluginTarget::parse(&command.target) { + PluginTarget::Path(path) => { + let (manifest, manifest_ref) = load_manifest_for_action("validate", &path)?; + println!( + "{}", + render_validation_summary(&manifest, &manifest_ref, None, None) + ); + Ok(()) + } + PluginTarget::Id(plugin_id) => { + let resolved = resolve_plugins_config(server.config.as_ref())?; + let host_config_by_id = host_config_by_id(&resolved); + let mut scopes = load_and_hydrate_scopes(server.config.as_ref(), &resolved)?; + let entry = find_record_by_id(&scopes, &plugin_id)?.ok_or_else(|| { + CliError::Config(format!( + "dynamic plugin '{}' is not registered; run `nemo-relay plugins add `", + plugin_id + )) + })?; + let manifest_ref = manifest_ref_from_record(&entry.record)?; + let (manifest, manifest_ref) = load_manifest_for_action("validate", &manifest_ref)?; + scopes[entry.scope_index] + .registry + .update_validation_status( + &plugin_id, + DynamicPluginValidationStatus { + manifest: DynamicPluginCheckState::Valid, + compatibility: DynamicPluginCheckState::Valid, + integrity: DynamicPluginCheckState::Unknown, + environment: DynamicPluginCheckState::Unknown, + authenticity: DynamicPluginCheckState::Unknown, + policy_satisfied: DynamicPluginCheckState::Unknown, + checked_at: None, + message: Some("validated by CLI".into()), + }, + ) + .map_err(|error| CliError::Config(error.to_string()))?; + scopes[entry.scope_index].save()?; + let refreshed = find_record_by_id(&scopes, &plugin_id)? + .expect("validated registry record should still exist"); + println!( + "{}", + render_validation_summary( + &manifest, + &manifest_ref, + Some(&refreshed), + host_config_by_id.get(&plugin_id), + ) + ); + Ok(()) + } + } +} + +pub(crate) fn list(command: PluginsListCommand, server: &ServerArgs) -> Result<(), CliError> { + let _ = command; + let resolved = resolve_plugins_config(server.config.as_ref())?; + let host_config_by_id = host_config_by_id(&resolved); + let scopes = load_and_hydrate_scopes(server.config.as_ref(), &resolved)?; + let records = collect_records(&scopes, false); + if records.is_empty() { + println!("No dynamic plugins registered."); + return Ok(()); + } + println!("{}", render_list(&records, &host_config_by_id)); + Ok(()) +} + +pub(crate) fn inspect(command: PluginsInspectCommand, server: &ServerArgs) -> Result<(), CliError> { + let resolved = resolve_plugins_config(server.config.as_ref())?; + let host_config_by_id = host_config_by_id(&resolved); + let scopes = load_and_hydrate_scopes(server.config.as_ref(), &resolved)?; + let entry = find_record_by_id(&scopes, &command.id)?.ok_or_else(|| { + CliError::Config(format!( + "dynamic plugin '{}' is not registered; run `nemo-relay plugins add `", + command.id + )) + })?; + let manifest_ref = manifest_ref_from_record(&entry.record)?; + let (manifest, manifest_ref) = load_manifest_for_action("inspect", &manifest_ref)?; + println!( + "{}", + render_inspect( + &entry, + &manifest, + &manifest_ref, + host_config_by_id.get(&command.id), + ) + ); + Ok(()) +} + +pub(crate) fn enable(command: PluginsEnableCommand, server: &ServerArgs) -> Result<(), CliError> { + mutate_enabled_state(command.id, server, true) +} + +pub(crate) fn disable(command: PluginsDisableCommand, server: &ServerArgs) -> Result<(), CliError> { + mutate_enabled_state(command.id, server, false) +} + +pub(crate) fn remove(command: PluginsRemoveCommand, server: &ServerArgs) -> Result<(), CliError> { + let resolved = resolve_plugins_config(server.config.as_ref())?; + let mut scopes = load_and_hydrate_scopes(server.config.as_ref(), &resolved)?; + let entry = find_record_by_id(&scopes, &command.id)?.ok_or_else(|| { + CliError::Config(format!( + "dynamic plugin '{}' is not registered; run `nemo-relay plugins add `", + command.id + )) + })?; + + scopes[entry.scope_index] + .registry + .remove(&command.id) + .map_err(|error| CliError::Config(error.to_string()))?; + let removed_reference = remove_dynamic_plugin_reference(&entry.plugins_toml_path, &command.id)?; + scopes[entry.scope_index].save()?; + + println!("Removed dynamic plugin {}", command.id); + println!("scope: {}", entry.scope.label()); + println!( + "plugins_toml: {}", + if removed_reference { + entry.plugins_toml_path.display().to_string() + } else { + "".into() + } + ); + println!("state_path: {}", entry.state_path.display()); + println!("status: tombstoned"); + Ok(()) +} + +fn mutate_enabled_state( + plugin_id: String, + server: &ServerArgs, + enabled: bool, +) -> Result<(), CliError> { + let resolved = resolve_plugins_config(server.config.as_ref())?; + let mut scopes = load_and_hydrate_scopes(server.config.as_ref(), &resolved)?; + let entry = find_record_by_id(&scopes, &plugin_id)?.ok_or_else(|| { + CliError::Config(format!( + "dynamic plugin '{}' is not registered; run `nemo-relay plugins add `", + plugin_id + )) + })?; + if enabled { + scopes[entry.scope_index] + .registry + .enable(&plugin_id) + .map_err(|error| CliError::Config(error.to_string()))?; + } else { + scopes[entry.scope_index] + .registry + .disable(&plugin_id) + .map_err(|error| CliError::Config(error.to_string()))?; + } + scopes[entry.scope_index].save()?; + + println!( + "{} dynamic plugin {}", + if enabled { "Enabled" } else { "Disabled" }, + plugin_id + ); + println!("scope: {}", entry.scope.label()); + println!("state_path: {}", entry.state_path.display()); + Ok(()) +} + +fn load_and_hydrate_scopes( + explicit: Option<&PathBuf>, + resolved: &ResolvedConfig, +) -> Result, CliError> { + let mut scopes = load_scoped_registries(explicit)?; + for plugin in &resolved.dynamic_plugins { + if find_record_by_id(&scopes, &plugin.plugin_id)?.is_some() { + continue; + } + let scope_index = scopes + .iter() + .position(|scope| scope.plugins_toml_path == plugin.source) + .ok_or_else(|| { + CliError::Config(format!( + "dynamic plugin '{}' resolved from {} but no matching lifecycle scope exists", + plugin.plugin_id, + plugin.source.display() + )) + })?; + let (manifest, manifest_ref) = load_manifest_for_action("hydrate", &plugin.manifest_ref)?; + scopes[scope_index] + .registry + .add(validated_record_from_manifest(manifest, manifest_ref)?) + .map_err(|error| CliError::Config(error.to_string()))?; + } + Ok(scopes) +} + +fn validated_record_from_manifest( + manifest: DynamicPluginManifest, + manifest_ref: String, +) -> Result { + let mut record = manifest + .into_record(Some(manifest_ref)) + .map_err(|error| CliError::Config(error.to_string()))?; + record.status.validation = DynamicPluginValidationStatus { + manifest: DynamicPluginCheckState::Valid, + compatibility: DynamicPluginCheckState::Valid, + integrity: DynamicPluginCheckState::Unknown, + environment: DynamicPluginCheckState::Unknown, + authenticity: DynamicPluginCheckState::Unknown, + policy_satisfied: DynamicPluginCheckState::Unknown, + checked_at: None, + message: Some("validated by CLI".into()), + }; + Ok(record) +} + +fn host_config_by_id(resolved: &ResolvedConfig) -> HashMap { + resolved + .dynamic_plugins + .iter() + .cloned() + .map(|plugin| (plugin.plugin_id.clone(), plugin)) + .collect() +} + +fn load_manifest_for_action( + action: &str, + path: impl Into, +) -> Result<(DynamicPluginManifest, String), CliError> { + let path = path.into(); + DynamicPluginManifest::load_from_path(&path) + .map_err(|error| CliError::Config(format!("dynamic plugin {action} failed: {error}"))) +} + +fn manifest_ref_from_record(record: &DynamicPluginRecord) -> Result { + record.source.manifest_ref.clone().ok_or_else(|| { + CliError::Config(format!( + "dynamic plugin '{}' has no manifest_ref in lifecycle state", + record.metadata.id + )) + }) +} + +fn ensure_scope( + scopes: &mut Vec, + scope: RegistryScope, + plugins_toml_path: PathBuf, + state_path: PathBuf, +) -> usize { + if let Some(index) = scopes.iter().position(|existing| { + existing.scope == scope + && existing.plugins_toml_path == plugins_toml_path + && existing.state_path == state_path + }) { + return index; + } + scopes.push(ScopedRegistry { + scope, + plugins_toml_path, + state_path, + registry: nemo_relay::plugin::dynamic::DynamicPluginRegistry::new(), + }); + scopes.len() - 1 +} + +fn scope_flags_selected(scope: &crate::config::PluginsScopeArgs) -> bool { + scope.user || scope.project || scope.global +} + +#[cfg(test)] +#[path = "../../tests/coverage/plugins_lifecycle_tests.rs"] +mod tests; diff --git a/crates/cli/src/plugins/lifecycle/render.rs b/crates/cli/src/plugins/lifecycle/render.rs new file mode 100644 index 00000000..fbbda488 --- /dev/null +++ b/crates/cli/src/plugins/lifecycle/render.rs @@ -0,0 +1,253 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +use std::collections::HashMap; + +use nemo_relay::plugin::dynamic::{ + DynamicPluginCheckState, DynamicPluginCompatibility, DynamicPluginKind, + DynamicPluginLoadContract, DynamicPluginManifest, DynamicPluginRecord, + DynamicPluginRuntimeState, +}; +use serde_json::Value; + +use crate::config::{DynamicPluginHostConfigStatus, ResolvedDynamicPluginConfig}; + +use super::state::ScopedDynamicPluginRecord; + +pub(super) fn render_list( + records: &[ScopedDynamicPluginRecord], + host_config_by_id: &HashMap, +) -> String { + let mut lines = Vec::with_capacity(records.len() + 1); + lines.push("ID\tSCOPE\tENABLED\tSTATE\tVALIDATION\tHOST CONFIG".into()); + for entry in records { + let host_config_status = host_config_by_id + .get(&entry.record.metadata.id) + .map(|plugin| host_config_status_label(plugin.host_config_status())) + .unwrap_or("missing"); + lines.push(format!( + "{}\t{}\t{}\t{}\t{}\t{}", + entry.record.metadata.id, + entry.scope.label(), + bool_label(entry.record.spec.enabled), + runtime_state_label(entry.record.status.runtime.state), + check_state_label(entry.record.status.validation.manifest), + host_config_status + )); + } + lines.join("\n") +} + +pub(super) fn render_inspect( + entry: &ScopedDynamicPluginRecord, + manifest: &DynamicPluginManifest, + manifest_ref: &str, + host_config: Option<&ResolvedDynamicPluginConfig>, +) -> String { + let record = &entry.record; + let mut lines = vec![ + format!("id: {}", record.metadata.id), + format!("scope: {}", entry.scope.label()), + format!("kind: {}", manifest_kind_label(record.metadata.kind)), + format!( + "name: {}", + record.metadata.name.as_deref().unwrap_or("") + ), + format!( + "version: {}", + record.metadata.version.as_deref().unwrap_or("") + ), + format!("manifest: {manifest_ref}"), + format!("plugins_toml: {}", entry.plugins_toml_path.display()), + format!("state_path: {}", entry.state_path.display()), + format!("desired.present: {}", bool_label(record.spec.present)), + format!("desired.enabled: {}", bool_label(record.spec.enabled)), + format!("generation: {}", record.metadata.generation), + format!("reconciled: {}", bool_label(record.is_reconciled())), + format!( + "host_config: {}", + host_config + .map(|plugin| host_config_status_label(plugin.host_config_status())) + .unwrap_or("missing") + ), + ]; + + match &record.compatibility { + DynamicPluginCompatibility::Worker(compatibility) => { + lines.push(format!("compat.relay: {}", compatibility.relay)); + lines.push(format!( + "compat.worker_protocol: {}", + compatibility.worker_protocol + )); + } + DynamicPluginCompatibility::RustDynamic(compatibility) => { + lines.push(format!("compat.relay: {}", compatibility.relay)); + lines.push(format!("compat.native_api: {}", compatibility.native_api)); + } + } + + match &record.load { + DynamicPluginLoadContract::Worker(load) => { + lines.push(format!("load.runtime: {:?}", load.runtime).to_lowercase()); + lines.push(format!("load.entrypoint: {}", load.entrypoint)); + } + DynamicPluginLoadContract::RustDynamic(load) => { + lines.push(format!("load.library: {}", load.library)); + lines.push(format!("load.symbol: {}", load.symbol)); + } + } + + lines.extend(render_status(record)); + lines.push(format!( + "capabilities: {}", + manifest + .capabilities + .items + .iter() + .map(|item| format!("{item:?}").to_lowercase()) + .collect::>() + .join(", ") + )); + lines.push(format!( + "host_config_json: {}", + host_config + .map(|plugin| plugin.config.clone()) + .filter(|config| !config.is_empty()) + .map(|config| { + serde_json::to_string_pretty(&Value::Object(config)) + .expect("host config serializes") + }) + .unwrap_or_else(|| "".into()) + )); + lines.join("\n") +} + +pub(super) fn render_validation_summary( + manifest: &DynamicPluginManifest, + manifest_ref: &str, + entry: Option<&ScopedDynamicPluginRecord>, + host_config: Option<&ResolvedDynamicPluginConfig>, +) -> String { + let mut lines = vec![ + format!("Dynamic plugin '{}' is valid.", manifest.plugin.id), + format!("kind: {}", manifest_kind_label(manifest.plugin.kind)), + format!("manifest: {manifest_ref}"), + ]; + if let Some(entry) = entry { + lines.push(format!("scope: {}", entry.scope.label())); + lines.push(format!("state_path: {}", entry.state_path.display())); + lines.push(format!( + "desired.enabled: {}", + bool_label(entry.record.spec.enabled) + )); + lines.push(format!( + "host_config: {}", + host_config + .map(|plugin| host_config_status_label(plugin.host_config_status())) + .unwrap_or("missing") + )); + } + lines.join("\n") +} + +fn render_status(record: &DynamicPluginRecord) -> Vec { + let mut lines = vec![ + format!( + "status.validation.manifest: {}", + check_state_label(record.status.validation.manifest) + ), + format!( + "status.validation.compatibility: {}", + check_state_label(record.status.validation.compatibility) + ), + format!( + "status.validation.integrity: {}", + check_state_label(record.status.validation.integrity) + ), + format!( + "status.validation.environment: {}", + check_state_label(record.status.validation.environment) + ), + format!( + "status.validation.authenticity: {}", + check_state_label(record.status.validation.authenticity) + ), + format!( + "status.validation.policy_satisfied: {}", + check_state_label(record.status.validation.policy_satisfied) + ), + format!( + "status.runtime.state: {}", + runtime_state_label(record.status.runtime.state) + ), + format!( + "status.runtime.observed_generation: {}", + record.status.runtime.observed_generation + ), + ]; + if let Some(value) = record.status.validation.checked_at.as_deref() { + lines.push(format!("status.validation.checked_at: {value}")); + } + if let Some(value) = record.status.validation.message.as_deref() { + lines.push(format!("status.validation.message: {value}")); + } + if let Some(value) = record.status.runtime.started_at.as_deref() { + lines.push(format!("status.runtime.started_at: {value}")); + } + if let Some(value) = record.status.runtime.updated_at.as_deref() { + lines.push(format!("status.runtime.updated_at: {value}")); + } + if let Some(value) = record.status.runtime.message.as_deref() { + lines.push(format!("status.runtime.message: {value}")); + } + if let Some(value) = record.status.startup_class { + lines.push(format!("status.startup_class: {:?}", value).to_lowercase()); + } + if let Some(value) = record.status.attestation_mode { + lines.push(format!("status.attestation_mode: {:?}", value).to_lowercase()); + } + if let Some(error) = record.status.last_error.as_ref() { + lines.push(format!( + "status.last_error: {}:{} {}", + format!("{:?}", error.phase).to_lowercase(), + error.code, + error.message + )); + } + lines +} + +fn host_config_status_label(status: DynamicPluginHostConfigStatus) -> &'static str { + match status { + DynamicPluginHostConfigStatus::Absent => "absent", + DynamicPluginHostConfigStatus::Present => "present", + } +} + +fn check_state_label(state: DynamicPluginCheckState) -> &'static str { + match state { + DynamicPluginCheckState::Unknown => "unknown", + DynamicPluginCheckState::Valid => "valid", + DynamicPluginCheckState::Invalid => "invalid", + } +} + +fn runtime_state_label(state: DynamicPluginRuntimeState) -> &'static str { + match state { + DynamicPluginRuntimeState::Stopped => "stopped", + DynamicPluginRuntimeState::Starting => "starting", + DynamicPluginRuntimeState::Running => "running", + DynamicPluginRuntimeState::Failed => "failed", + } +} + +fn manifest_kind_label(kind: DynamicPluginKind) -> &'static str { + match kind { + DynamicPluginKind::RustDynamic => "rust_dynamic", + DynamicPluginKind::Worker => "worker", + } +} + +fn bool_label(value: bool) -> &'static str { + if value { "true" } else { "false" } +} diff --git a/crates/cli/src/plugins/lifecycle/state.rs b/crates/cli/src/plugins/lifecycle/state.rs new file mode 100644 index 00000000..74a38930 --- /dev/null +++ b/crates/cli/src/plugins/lifecycle/state.rs @@ -0,0 +1,278 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +use std::path::{Path, PathBuf}; + +use nemo_relay::plugin::dynamic::{DynamicPluginRecord, DynamicPluginRegistry}; +use serde::{Deserialize, Serialize}; + +use crate::config::{ + global_plugin_config_path, project_plugin_config_path, user_config_dir, user_plugin_config_path, +}; +use crate::error::CliError; + +use super::super::config_io::TargetScope; + +const DYNAMIC_PLUGIN_STATE_FILENAME: &str = "dynamic-plugins.json"; +const DYNAMIC_PLUGIN_STATE_SCHEMA_VERSION: u32 = 1; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(super) enum RegistryScope { + User, + Project, + Global, + Explicit, +} + +impl RegistryScope { + pub(super) fn label(self) -> &'static str { + match self { + Self::User => "user", + Self::Project => "project", + Self::Global => "global", + Self::Explicit => "explicit", + } + } +} + +#[derive(Debug)] +pub(super) struct ScopedRegistry { + pub(super) scope: RegistryScope, + pub(super) plugins_toml_path: PathBuf, + pub(super) state_path: PathBuf, + pub(super) registry: DynamicPluginRegistry, +} + +#[derive(Debug, Clone)] +pub(super) struct ScopedDynamicPluginRecord { + pub(super) scope_index: usize, + pub(super) scope: RegistryScope, + pub(super) plugins_toml_path: PathBuf, + pub(super) state_path: PathBuf, + pub(super) record: DynamicPluginRecord, +} + +#[derive(Debug, Clone, Default, Serialize, Deserialize)] +struct PersistedDynamicPluginRegistry { + #[serde(default = "default_state_schema_version")] + schema_version: u32, + #[serde(default)] + records: Vec, +} + +const fn default_state_schema_version() -> u32 { + DYNAMIC_PLUGIN_STATE_SCHEMA_VERSION +} + +impl ScopedRegistry { + pub(super) fn save(&self) -> Result<(), CliError> { + let rendered = serde_json::to_vec_pretty(&PersistedDynamicPluginRegistry { + schema_version: DYNAMIC_PLUGIN_STATE_SCHEMA_VERSION, + records: self.registry.cloned_records(true), + }) + .map_err(|error| { + CliError::Config(format!( + "could not serialize dynamic plugin registry state {}: {error}", + self.state_path.display() + )) + })?; + if let Some(parent) = self.state_path.parent() { + std::fs::create_dir_all(parent)?; + } + let mut rendered = rendered; + rendered.push(b'\n'); + std::fs::write(&self.state_path, rendered)?; + Ok(()) + } +} + +pub(super) fn load_scoped_registries( + explicit: Option<&PathBuf>, +) -> Result, CliError> { + scoped_registry_layouts(explicit)? + .into_iter() + .map(|(scope, plugins_toml_path, state_path)| { + Ok(ScopedRegistry { + scope, + plugins_toml_path, + registry: read_registry(&state_path)?, + state_path, + }) + }) + .collect() +} + +pub(super) fn scoped_paths_for_add( + scope: TargetScope, + explicit: Option<&PathBuf>, +) -> Result<(PathBuf, PathBuf, RegistryScope), CliError> { + if let Some(explicit) = explicit { + let parent = explicit.parent().ok_or_else(|| { + CliError::Config(format!( + "explicit config path {} has no parent directory", + explicit.display() + )) + })?; + return Ok(( + parent.join("plugins.toml"), + parent.join(DYNAMIC_PLUGIN_STATE_FILENAME), + RegistryScope::Explicit, + )); + } + + let plugins_toml_path = match scope { + TargetScope::User => user_plugin_config_path().ok_or_else(|| { + CliError::Config( + "cannot determine user config directory; set HOME or XDG_CONFIG_HOME".into(), + ) + })?, + TargetScope::Project => { + let cwd = std::env::current_dir()?; + project_plugin_config_path(&cwd) + } + TargetScope::Global => global_plugin_config_path(), + }; + let state_path = sibling_state_path(&plugins_toml_path); + let scope = match scope { + TargetScope::User => RegistryScope::User, + TargetScope::Project => RegistryScope::Project, + TargetScope::Global => RegistryScope::Global, + }; + Ok((plugins_toml_path, state_path, scope)) +} + +pub(super) fn collect_records( + scopes: &[ScopedRegistry], + include_tombstoned: bool, +) -> Vec { + let mut records = Vec::new(); + for (scope_index, scope) in scopes.iter().enumerate() { + for record in scope.registry.cloned_records(include_tombstoned) { + records.push(ScopedDynamicPluginRecord { + scope_index, + scope: scope.scope, + plugins_toml_path: scope.plugins_toml_path.clone(), + state_path: scope.state_path.clone(), + record, + }); + } + } + records.sort_by(|left, right| left.record.metadata.id.cmp(&right.record.metadata.id)); + records +} + +pub(super) fn find_record_by_id( + scopes: &[ScopedRegistry], + plugin_id: &str, +) -> Result, CliError> { + let mut live = Vec::new(); + let mut tombstoned = Vec::new(); + for record in collect_records(scopes, true) + .into_iter() + .filter(|record| record.record.metadata.id == plugin_id) + { + if record.record.is_tombstoned() { + tombstoned.push(record); + } else { + live.push(record); + } + } + + if live.len() > 1 { + return Err(CliError::Config(format!( + "dynamic plugin '{}' is configured in multiple lifecycle scopes; inspect {}", + plugin_id, + live.iter() + .map(|record| record.scope.label()) + .collect::>() + .join(", ") + ))); + } + if let Some(record) = live.into_iter().next() { + return Ok(Some(record)); + } + if tombstoned.len() > 1 { + return Err(CliError::Config(format!( + "dynamic plugin '{}' has multiple tombstoned lifecycle records; inspect {}", + plugin_id, + tombstoned + .iter() + .map(|record| record.scope.label()) + .collect::>() + .join(", ") + ))); + } + Ok(tombstoned.into_iter().next()) +} + +fn scoped_registry_layouts( + explicit: Option<&PathBuf>, +) -> Result, CliError> { + if let Some(explicit) = explicit { + let parent = explicit.parent().ok_or_else(|| { + CliError::Config(format!( + "explicit config path {} has no parent directory", + explicit.display() + )) + })?; + let plugins_toml_path = parent.join("plugins.toml"); + return Ok(vec![( + RegistryScope::Explicit, + plugins_toml_path.clone(), + sibling_state_path(&plugins_toml_path), + )]); + } + + let mut layouts = vec![( + RegistryScope::Global, + global_plugin_config_path(), + sibling_state_path(&global_plugin_config_path()), + )]; + if let Ok(cwd) = std::env::current_dir() { + let plugins_toml_path = project_plugin_config_path(&cwd); + layouts.push(( + RegistryScope::Project, + plugins_toml_path.clone(), + sibling_state_path(&plugins_toml_path), + )); + } + if let Some(user_dir) = user_config_dir() { + let plugins_toml_path = user_dir.join("plugins.toml"); + layouts.push(( + RegistryScope::User, + plugins_toml_path.clone(), + sibling_state_path(&plugins_toml_path), + )); + } + Ok(layouts) +} + +fn read_registry(path: &Path) -> Result { + if !path.exists() { + return Ok(DynamicPluginRegistry::new()); + } + let raw = std::fs::read_to_string(path)?; + let state: PersistedDynamicPluginRegistry = serde_json::from_str(&raw).map_err(|error| { + CliError::Config(format!( + "invalid dynamic plugin registry state in {}: {error}", + path.display() + )) + })?; + if state.schema_version != DYNAMIC_PLUGIN_STATE_SCHEMA_VERSION { + return Err(CliError::Config(format!( + "unsupported dynamic plugin registry schema_version {} in {}; expected {}", + state.schema_version, + path.display(), + DYNAMIC_PLUGIN_STATE_SCHEMA_VERSION + ))); + } + DynamicPluginRegistry::from_records(state.records) + .map_err(|error| CliError::Config(error.to_string())) +} + +fn sibling_state_path(plugins_toml_path: &Path) -> PathBuf { + plugins_toml_path + .parent() + .map(|parent| parent.join(DYNAMIC_PLUGIN_STATE_FILENAME)) + .unwrap_or_else(|| PathBuf::from(DYNAMIC_PLUGIN_STATE_FILENAME)) +} diff --git a/crates/cli/src/plugins/lifecycle/target.rs b/crates/cli/src/plugins/lifecycle/target.rs new file mode 100644 index 00000000..ab1734b7 --- /dev/null +++ b/crates/cli/src/plugins/lifecycle/target.rs @@ -0,0 +1,29 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +use std::path::{Path, PathBuf}; + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(super) enum PluginTarget { + Path(PathBuf), + Id(String), +} + +impl PluginTarget { + pub(super) fn parse(target: &str) -> Self { + if looks_like_path(target) { + Self::Path(PathBuf::from(target)) + } else { + Self::Id(target.to_owned()) + } + } +} + +fn looks_like_path(target: &str) -> bool { + let path = Path::new(target); + path.exists() + || target.ends_with(".toml") + || target.contains(std::path::MAIN_SEPARATOR) + || target.contains('/') + || target.contains('\\') +} diff --git a/crates/cli/tests/coverage/main_tests.rs b/crates/cli/tests/coverage/main_tests.rs index 3948767a..2bda64a7 100644 --- a/crates/cli/tests/coverage/main_tests.rs +++ b/crates/cli/tests/coverage/main_tests.rs @@ -5,8 +5,9 @@ use clap::Parser; use super::*; use crate::config::{ - CompletionsCommand, PluginsCommand, PluginsEditCommand, PluginsSubcommand, PricingSubcommand, - PricingValidateCommand, + CompletionsCommand, PluginsCommand, PluginsEditCommand, PluginsInspectCommand, + PluginsListCommand, PluginsSubcommand, PluginsValidateCommand, PricingSubcommand, + PricingValidateCommand, ServerArgs, }; #[test] @@ -33,12 +34,50 @@ fn safe_dispatch_helpers_cover_completions_and_plugins_paths() { ExitCode::SUCCESS ); - let error = run_plugins(PluginsCommand { - command: PluginsSubcommand::Edit(PluginsEditCommand::default()), - }) + let error = run_plugins( + PluginsCommand { + command: PluginsSubcommand::Edit(PluginsEditCommand::default()), + }, + &ServerArgs::default(), + ) .unwrap_err() .to_string(); assert!(error.contains("interactive terminal") || error.contains("TTY")); + + assert_eq!( + run_plugins( + PluginsCommand { + command: PluginsSubcommand::List(PluginsListCommand::default()), + }, + &ServerArgs::default() + ) + .unwrap(), + ExitCode::SUCCESS + ); + + let inspect_error = run_plugins( + PluginsCommand { + command: PluginsSubcommand::Inspect(PluginsInspectCommand { + id: "missing.plugin".into(), + }), + }, + &ServerArgs::default(), + ) + .unwrap_err() + .to_string(); + assert!(inspect_error.contains("not registered")); + + let validate_error = run_plugins( + PluginsCommand { + command: PluginsSubcommand::Validate(PluginsValidateCommand { + target: "missing.plugin".into(), + }), + }, + &ServerArgs::default(), + ) + .unwrap_err() + .to_string(); + assert!(validate_error.contains("not registered")); } #[tokio::test] diff --git a/crates/cli/tests/coverage/plugins_lifecycle_tests.rs b/crates/cli/tests/coverage/plugins_lifecycle_tests.rs new file mode 100644 index 00000000..9a6bdf11 --- /dev/null +++ b/crates/cli/tests/coverage/plugins_lifecycle_tests.rs @@ -0,0 +1,410 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +use std::path::{Path, PathBuf}; + +use super::*; +use crate::config::{ + PluginsAddCommand, PluginsDisableCommand, PluginsEnableCommand, PluginsInspectCommand, + PluginsListCommand, PluginsRemoveCommand, PluginsScopeArgs, PluginsValidateCommand, ServerArgs, +}; + +struct CurrentDirGuard { + original: PathBuf, +} + +impl CurrentDirGuard { + fn enter(path: &Path) -> Self { + let original = std::env::current_dir().unwrap(); + std::env::set_current_dir(path).unwrap(); + Self { original } + } +} + +impl Drop for CurrentDirGuard { + fn drop(&mut self) { + std::env::set_current_dir(&self.original).unwrap(); + } +} + +fn write_dynamic_manifest(dir: &Path, plugin_id: &str) -> PathBuf { + let manifest_path = dir.join("relay-plugin.toml"); + std::fs::write( + &manifest_path, + format!( + r#" +manifest_version = 1 + +[plugin] +id = "{plugin_id}" +kind = "worker" + +[compat] +relay = "0.5" +worker_protocol = "1" + +[defaults] +enabled = false + +[capabilities] +items = ["plugin_worker"] + +[load] +runtime = "python" +entrypoint = "{plugin_id}.plugin:register" +"# + ), + ) + .unwrap(); + manifest_path +} + +#[test] +fn add_registers_dynamic_plugin_in_project_plugins_toml() { + let _lock = crate::test_support::ENV_TEST_LOCK + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + let temp = tempfile::tempdir().unwrap(); + let _cwd = CurrentDirGuard::enter(temp.path()); + let plugin_dir = temp.path().join("plugins").join("acme"); + std::fs::create_dir_all(&plugin_dir).unwrap(); + write_dynamic_manifest(&plugin_dir, "acme.guardrail"); + + add( + PluginsAddCommand { + scope: PluginsScopeArgs { + project: true, + ..PluginsScopeArgs::default() + }, + path: plugin_dir.clone(), + }, + &crate::config::ServerArgs::default(), + ) + .unwrap(); + + let plugins_toml = temp.path().join(".nemo-relay").join("plugins.toml"); + let rendered = std::fs::read_to_string(&plugins_toml).unwrap(); + assert!(rendered.contains("[[plugins.dynamic]]")); + assert!(rendered.contains("relay-plugin.toml")); + + let resolved = resolve_plugins_config(None).unwrap(); + assert_eq!(resolved.dynamic_plugins.len(), 1); + assert_eq!(resolved.dynamic_plugins[0].plugin_id, "acme.guardrail"); +} + +#[test] +fn add_rejects_duplicate_dynamic_plugin_ids() { + let _lock = crate::test_support::ENV_TEST_LOCK + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + let temp = tempfile::tempdir().unwrap(); + let _cwd = CurrentDirGuard::enter(temp.path()); + let plugin_dir = temp.path().join("plugins").join("acme"); + std::fs::create_dir_all(&plugin_dir).unwrap(); + write_dynamic_manifest(&plugin_dir, "acme.guardrail"); + + add( + PluginsAddCommand { + scope: PluginsScopeArgs { + project: true, + ..PluginsScopeArgs::default() + }, + path: plugin_dir.clone(), + }, + &crate::config::ServerArgs::default(), + ) + .unwrap(); + + let error = add( + PluginsAddCommand { + scope: PluginsScopeArgs { + project: true, + ..PluginsScopeArgs::default() + }, + path: plugin_dir, + }, + &crate::config::ServerArgs::default(), + ) + .unwrap_err() + .to_string(); + assert!(error.contains("already registered")); +} + +#[test] +fn list_and_inspect_render_discovered_dynamic_plugins() { + let _lock = crate::test_support::ENV_TEST_LOCK + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + let temp = tempfile::tempdir().unwrap(); + let _cwd = CurrentDirGuard::enter(temp.path()); + let plugin_dir = temp.path().join("plugins").join("acme"); + std::fs::create_dir_all(&plugin_dir).unwrap(); + write_dynamic_manifest(&plugin_dir, "acme.guardrail"); + + add( + PluginsAddCommand { + scope: PluginsScopeArgs { + project: true, + ..PluginsScopeArgs::default() + }, + path: plugin_dir, + }, + &crate::config::ServerArgs::default(), + ) + .unwrap(); + + let resolved = resolve_plugins_config(None).unwrap(); + let host_config_by_id = host_config_by_id(&resolved); + let scopes = load_and_hydrate_scopes(None, &resolved).unwrap(); + let records = collect_records(&scopes, false); + let list = render_list(&records, &host_config_by_id); + assert!(list.contains("acme.guardrail")); + assert!(list.contains("absent")); + assert!(list.contains("false")); + + let entry = find_record_by_id(&scopes, "acme.guardrail") + .unwrap() + .expect("plugin record"); + let (manifest, manifest_ref) = + DynamicPluginManifest::load_from_path(entry.record.source.manifest_ref.clone().unwrap()) + .map_err(|error| CliError::Config(error.to_string())) + .unwrap(); + let inspect = render_inspect( + &entry, + &manifest, + &manifest_ref, + host_config_by_id.get("acme.guardrail"), + ); + assert!(inspect.contains("id: acme.guardrail")); + assert!(inspect.contains("kind: worker")); + assert!(inspect.contains("host_config: absent")); + assert!(inspect.contains("load.entrypoint: acme.guardrail.plugin:register")); +} + +#[test] +fn validate_renders_summary_for_path_and_id_targets() { + let _lock = crate::test_support::ENV_TEST_LOCK + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + let temp = tempfile::tempdir().unwrap(); + let _cwd = CurrentDirGuard::enter(temp.path()); + let plugin_dir = temp.path().join("plugins").join("acme"); + std::fs::create_dir_all(&plugin_dir).unwrap(); + let manifest_path = write_dynamic_manifest(&plugin_dir, "acme.guardrail"); + + add( + PluginsAddCommand { + scope: PluginsScopeArgs { + project: true, + ..PluginsScopeArgs::default() + }, + path: plugin_dir, + }, + &crate::config::ServerArgs::default(), + ) + .unwrap(); + + let (manifest, manifest_ref) = DynamicPluginManifest::load_from_path(&manifest_path) + .map_err(|error| CliError::Config(error.to_string())) + .unwrap(); + let path_summary = render_validation_summary(&manifest, &manifest_ref, None, None); + assert!(path_summary.contains("Dynamic plugin 'acme.guardrail' is valid.")); + + let resolved = resolve_plugins_config(None).unwrap(); + let host_config_by_id = host_config_by_id(&resolved); + let scopes = load_and_hydrate_scopes(None, &resolved).unwrap(); + let id_summary = render_validation_summary( + &manifest, + &manifest_ref, + Some( + &find_record_by_id(&scopes, "acme.guardrail") + .unwrap() + .expect("plugin record"), + ), + host_config_by_id.get("acme.guardrail"), + ); + assert!(id_summary.contains("host_config: absent")); + assert!(id_summary.contains("desired.enabled: false")); + + let missing_validate = validate( + PluginsValidateCommand { + target: "missing.plugin".into(), + }, + &crate::config::ServerArgs::default(), + ) + .unwrap_err() + .to_string(); + assert!(missing_validate.contains("not registered")); + + let missing_inspect = inspect( + PluginsInspectCommand { + id: "missing.plugin".into(), + }, + &crate::config::ServerArgs::default(), + ) + .unwrap_err() + .to_string(); + assert!(missing_inspect.contains("not registered")); + + assert_eq!( + list( + PluginsListCommand::default(), + &crate::config::ServerArgs::default() + ) + .unwrap(), + () + ); +} + +#[test] +fn enable_disable_and_remove_persist_lifecycle_state() { + let _lock = crate::test_support::ENV_TEST_LOCK + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + let temp = tempfile::tempdir().unwrap(); + let _cwd = CurrentDirGuard::enter(temp.path()); + let plugin_dir = temp.path().join("plugins").join("acme"); + std::fs::create_dir_all(&plugin_dir).unwrap(); + write_dynamic_manifest(&plugin_dir, "acme.guardrail"); + let server = crate::config::ServerArgs::default(); + + add( + PluginsAddCommand { + scope: PluginsScopeArgs { + project: true, + ..PluginsScopeArgs::default() + }, + path: plugin_dir, + }, + &server, + ) + .unwrap(); + + enable( + PluginsEnableCommand { + id: "acme.guardrail".into(), + }, + &server, + ) + .unwrap(); + + let resolved = resolve_plugins_config(None).unwrap(); + let scopes = load_and_hydrate_scopes(None, &resolved).unwrap(); + let enabled = find_record_by_id(&scopes, "acme.guardrail") + .unwrap() + .expect("enabled record"); + assert!(enabled.record.spec.enabled); + + disable( + PluginsDisableCommand { + id: "acme.guardrail".into(), + }, + &server, + ) + .unwrap(); + let resolved = resolve_plugins_config(None).unwrap(); + let scopes = load_and_hydrate_scopes(None, &resolved).unwrap(); + let disabled = find_record_by_id(&scopes, "acme.guardrail") + .unwrap() + .expect("disabled record"); + assert!(!disabled.record.spec.enabled); + + remove( + PluginsRemoveCommand { + id: "acme.guardrail".into(), + }, + &server, + ) + .unwrap(); + let resolved = resolve_plugins_config(None).unwrap(); + assert!(resolved.dynamic_plugins.is_empty()); + let scopes = load_and_hydrate_scopes(None, &resolved).unwrap(); + let removed = find_record_by_id(&scopes, "acme.guardrail") + .unwrap() + .expect("removed record"); + assert!(removed.record.is_tombstoned()); +} + +#[test] +fn add_with_explicit_config_uses_sibling_plugins_and_state_files() { + let _lock = crate::test_support::ENV_TEST_LOCK + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + let temp = tempfile::tempdir().unwrap(); + let plugin_dir = temp.path().join("plugins").join("acme"); + let config_dir = temp.path().join("custom-config"); + std::fs::create_dir_all(&plugin_dir).unwrap(); + std::fs::create_dir_all(&config_dir).unwrap(); + write_dynamic_manifest(&plugin_dir, "acme.explicit"); + + let server = ServerArgs { + config: Some(config_dir.join("gateway.toml")), + ..ServerArgs::default() + }; + + add( + PluginsAddCommand { + scope: PluginsScopeArgs::default(), + path: plugin_dir, + }, + &server, + ) + .unwrap(); + + let plugins_toml = config_dir.join("plugins.toml"); + let state_path = config_dir.join("dynamic-plugins.json"); + assert!(plugins_toml.exists()); + assert!(state_path.exists()); + + let resolved = resolve_plugins_config(server.config.as_ref()).unwrap(); + assert_eq!(resolved.dynamic_plugins.len(), 1); + assert_eq!(resolved.dynamic_plugins[0].plugin_id, "acme.explicit"); + + let scopes = load_and_hydrate_scopes(server.config.as_ref(), &resolved).unwrap(); + let entry = find_record_by_id(&scopes, "acme.explicit") + .unwrap() + .expect("explicit-scope record"); + assert_eq!(entry.scope.label(), "explicit"); + assert_eq!(entry.plugins_toml_path, plugins_toml); + assert_eq!(entry.state_path, state_path); +} + +#[test] +fn hydrate_bootstraps_registry_records_from_existing_dynamic_plugin_refs() { + let _lock = crate::test_support::ENV_TEST_LOCK + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + let temp = tempfile::tempdir().unwrap(); + let _cwd = CurrentDirGuard::enter(temp.path()); + let plugin_dir = temp.path().join("plugins").join("acme"); + let config_dir = temp.path().join(".nemo-relay"); + std::fs::create_dir_all(&plugin_dir).unwrap(); + std::fs::create_dir_all(&config_dir).unwrap(); + let manifest_path = write_dynamic_manifest(&plugin_dir, "acme.bootstrap"); + + std::fs::write( + config_dir.join("plugins.toml"), + format!( + "[[plugins.dynamic]]\nmanifest = {:?}\n", + manifest_path.to_string_lossy() + ), + ) + .unwrap(); + + let resolved = resolve_plugins_config(None).unwrap(); + assert_eq!(resolved.dynamic_plugins.len(), 1); + + let scopes = load_and_hydrate_scopes(None, &resolved).unwrap(); + let entry = find_record_by_id(&scopes, "acme.bootstrap") + .unwrap() + .expect("hydrated record"); + assert_eq!(entry.scope.label(), "project"); + assert_eq!(entry.record.metadata.id, "acme.bootstrap"); + assert!(entry.record.spec.present); + assert!(!entry.record.spec.enabled); + let canonical_manifest_path = std::fs::canonicalize(&manifest_path).unwrap(); + assert_eq!( + entry.record.source.manifest_ref.as_deref(), + Some(canonical_manifest_path.to_string_lossy().as_ref()) + ); +} diff --git a/crates/cli/tests/coverage/plugins_tests.rs b/crates/cli/tests/coverage/plugins_tests.rs index 6c2f6b35..80992976 100644 --- a/crates/cli/tests/coverage/plugins_tests.rs +++ b/crates/cli/tests/coverage/plugins_tests.rs @@ -3,7 +3,8 @@ use super::*; use crate::config::{ - global_plugin_config_path, project_plugin_config_path, user_plugin_config_path, + PluginsScopeArgs, global_plugin_config_path, project_plugin_config_path, + user_plugin_config_path, }; use nemo_relay::config_editor::{EditorConfig, EditorSchema}; use nemo_relay::observability::plugin_component::{OBSERVABILITY_PLUGIN_KIND, ObservabilityConfig}; @@ -90,30 +91,30 @@ fn local_llm_guardrails_component_config(config_yaml: &str) -> serde_json::Map) -> Result { + let mut registry = Self::new(); + for mut record in records { + normalize_record_shape(&mut record); + validate_record_shape(&record)?; + let plugin_id = record.metadata.id.clone(); + if registry.records.contains_key(&plugin_id) { + return Err(PluginError::Conflict(format!( + "dynamic plugin '{plugin_id}' is duplicated in persisted registry state" + ))); + } + registry.records.insert(plugin_id, record); + } + Ok(registry) + } + /// Returns the registered record for `plugin_id`, if present. pub fn get(&self, plugin_id: &str) -> Option<&DynamicPluginRecord> { self.records.get(plugin_id) @@ -35,6 +52,11 @@ impl DynamicPluginRegistry { .collect() } + /// Clones records for serialization or higher-level projection. + pub fn cloned_records(&self, include_tombstoned: bool) -> Vec { + self.list(include_tombstoned).into_iter().cloned().collect() + } + /// Adds a new dynamic plugin record. /// /// This is a trusted internal control-plane API. Callers that start from an From 2940171450636d18d70b2650bb0fa6e65bfc7bb4 Mon Sep 17 00:00:00 2001 From: Alex Fournier Date: Tue, 23 Jun 2026 13:29:52 -0700 Subject: [PATCH 02/20] feat: complete dynamic plugin lifecycle CLI diagnostics Signed-off-by: Alex Fournier --- crates/cli/src/config.rs | 41 ++- crates/cli/src/error.rs | 30 ++ crates/cli/src/main.rs | 43 ++- crates/cli/src/plugins/lifecycle.rs | 267 ++++++++++++++---- crates/cli/src/plugins/lifecycle/json.rs | 188 ++++++++++++ crates/cli/src/plugins/lifecycle/render.rs | 28 +- crates/cli/tests/cli_tests.rs | 60 ++++ crates/cli/tests/coverage/main_tests.rs | 93 ++++-- .../tests/coverage/plugins_lifecycle_tests.rs | 145 ++++++++++ 9 files changed, 801 insertions(+), 94 deletions(-) create mode 100644 crates/cli/src/plugins/lifecycle/json.rs diff --git a/crates/cli/src/config.rs b/crates/cli/src/config.rs index a9df1d3b..a5bb43e8 100644 --- a/crates/cli/src/config.rs +++ b/crates/cli/src/config.rs @@ -197,6 +197,12 @@ pub(crate) struct PluginsCommand { pub(crate) command: PluginsSubcommand, } +#[derive(Debug, Clone, Copy)] +pub(crate) struct PluginJsonContext<'a> { + pub(crate) command: &'static str, + pub(crate) target: Option<&'a str>, +} + /// Plugin configuration subcommands. #[derive(Debug, Clone, Subcommand)] pub(crate) enum PluginsSubcommand { @@ -218,6 +224,26 @@ pub(crate) enum PluginsSubcommand { Remove(PluginsRemoveCommand), } +impl PluginsSubcommand { + pub(crate) fn json_context(&self) -> Option> { + match self { + Self::Validate(command) if command.json => Some(PluginJsonContext { + command: "plugins validate", + target: Some(command.target.as_str()), + }), + Self::List(command) if command.json => Some(PluginJsonContext { + command: "plugins list", + target: None, + }), + Self::Inspect(command) if command.json => Some(PluginJsonContext { + command: "plugins inspect", + target: Some(command.id.as_str()), + }), + _ => None, + } + } +} + /// Args for `nemo-relay pricing`. #[derive(Debug, Clone, Args)] pub(crate) struct PricingCommand { @@ -345,17 +371,30 @@ pub(crate) struct PluginsAddCommand { pub(crate) struct PluginsValidateCommand { /// Canonical plugin ID or a local plugin directory / `relay-plugin.toml` path. pub(crate) target: String, + /// Emit machine-readable JSON output. + #[arg(long)] + pub(crate) json: bool, } /// Args for `nemo-relay plugins list`. #[derive(Debug, Clone, Default, Args)] -pub(crate) struct PluginsListCommand {} +pub(crate) struct PluginsListCommand { + /// Include tombstoned dynamic plugin records in the output. + #[arg(long)] + pub(crate) all: bool, + /// Emit machine-readable JSON output. + #[arg(long)] + pub(crate) json: bool, +} /// Args for `nemo-relay plugins inspect`. #[derive(Debug, Clone, Args)] pub(crate) struct PluginsInspectCommand { /// Canonical plugin ID. pub(crate) id: String, + /// Emit machine-readable JSON output. + #[arg(long)] + pub(crate) json: bool, } /// Args for `nemo-relay plugins enable`. diff --git a/crates/cli/src/error.rs b/crates/cli/src/error.rs index 3c979678..aa0e0fa3 100644 --- a/crates/cli/src/error.rs +++ b/crates/cli/src/error.rs @@ -5,8 +5,17 @@ use axum::Json; use axum::http::StatusCode; use axum::response::{IntoResponse, Response}; use nemo_relay::error::FlowError; +use serde::Serialize; use serde_json::{Map, Value, json}; +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +#[serde(rename_all = "snake_case")] +pub(crate) enum PluginLifecycleFailureKind { + Failed, + NotFound, + Refused, +} + #[derive(Debug, thiserror::Error)] pub(crate) enum CliError { #[error("guardrail rejected: {0}")] @@ -27,6 +36,13 @@ pub(crate) enum CliError { Config(String), #[error("launcher error: {0}")] Launch(String), + #[error("{message}")] + PluginLifecycle { + command: &'static str, + target: Option, + kind: PluginLifecycleFailureKind, + message: String, + }, #[error("NeMo Relay runtime error: {0}")] Flow(#[from] nemo_relay::error::FlowError), #[error("openinference error: {0}")] @@ -41,6 +57,20 @@ impl CliError { _ => None, } } + + pub(crate) fn plugin_lifecycle( + &self, + ) -> Option<(&'static str, Option<&str>, PluginLifecycleFailureKind, &str)> { + match self { + Self::PluginLifecycle { + command, + target, + kind, + message, + } => Some((command, target.as_deref(), *kind, message.as_str())), + _ => None, + } + } } impl IntoResponse for CliError { diff --git a/crates/cli/src/main.rs b/crates/cli/src/main.rs index 0c713878..41ec0ee3 100644 --- a/crates/cli/src/main.rs +++ b/crates/cli/src/main.rs @@ -102,17 +102,40 @@ async fn run_config(command: ConfigCommand) -> Result } fn run_plugins(command: PluginsCommand, server: &ServerArgs) -> Result { - match command.command { - PluginsSubcommand::Edit(command) => plugins::edit(command)?, - PluginsSubcommand::Add(command) => plugins::lifecycle::add(command, server)?, - PluginsSubcommand::Validate(command) => plugins::lifecycle::validate(command, server)?, - PluginsSubcommand::List(command) => plugins::lifecycle::list(command, server)?, - PluginsSubcommand::Inspect(command) => plugins::lifecycle::inspect(command, server)?, - PluginsSubcommand::Enable(command) => plugins::lifecycle::enable(command, server)?, - PluginsSubcommand::Disable(command) => plugins::lifecycle::disable(command, server)?, - PluginsSubcommand::Remove(command) => plugins::lifecycle::remove(command, server)?, + let json_context = command + .command + .json_context() + .map(|context| (context.command, context.target.map(str::to_owned))); + let json = json_context.is_some(); + let result = match command.command { + PluginsSubcommand::Edit(command) => plugins::edit(command), + PluginsSubcommand::Add(command) => plugins::lifecycle::add(command, server), + PluginsSubcommand::Validate(command) => plugins::lifecycle::validate(command, server), + PluginsSubcommand::List(command) => plugins::lifecycle::list(command, server), + PluginsSubcommand::Inspect(command) => plugins::lifecycle::inspect(command, server), + PluginsSubcommand::Enable(command) => plugins::lifecycle::enable(command, server), + PluginsSubcommand::Disable(command) => plugins::lifecycle::disable(command, server), + PluginsSubcommand::Remove(command) => plugins::lifecycle::remove(command, server), + }; + match result { + Ok(()) => Ok(ExitCode::SUCCESS), + Err(error) => { + if let Some(exit_code) = plugins::lifecycle::render_plugin_error(&error, json)? { + Ok(exit_code) + } else if json { + let (json_command, json_target) = json_context + .as_ref() + .expect("json plugin command context should exist when json output is enabled"); + plugins::lifecycle::render_generic_plugin_json_error( + json_command, + json_target.as_deref(), + &error.to_string(), + ) + } else { + Err(error) + } + } } - Ok(ExitCode::SUCCESS) } fn run_pricing(command: PricingCommand) -> Result { diff --git a/crates/cli/src/plugins/lifecycle.rs b/crates/cli/src/plugins/lifecycle.rs index a1f11064..87dd5c0f 100644 --- a/crates/cli/src/plugins/lifecycle.rs +++ b/crates/cli/src/plugins/lifecycle.rs @@ -3,6 +3,7 @@ use std::collections::HashMap; use std::path::PathBuf; +use std::process::ExitCode; use nemo_relay::plugin::dynamic::{ DynamicPluginCheckState, DynamicPluginManifest, DynamicPluginRecord, @@ -14,16 +15,21 @@ use crate::config::{ PluginsListCommand, PluginsRemoveCommand, PluginsValidateCommand, ResolvedConfig, ResolvedDynamicPluginConfig, ServerArgs, resolve_plugins_config, }; -use crate::error::CliError; +use crate::error::{CliError, PluginLifecycleFailureKind}; use super::config_io::{ append_dynamic_plugin_reference, remove_dynamic_plugin_reference, target_scope, }; +mod json; mod render; mod state; mod target; +use self::json::{ + ValidateJsonContext, failure_envelope, generic_failure_envelope, inspect_success_envelope, + list_success_envelope, print_json, validate_success_envelope, +}; use self::render::{render_inspect, render_list, render_validation_summary}; use self::state::{ RegistryScope, ScopedRegistry, collect_records, find_record_by_id, load_scoped_registries, @@ -36,15 +42,17 @@ pub(crate) fn add(command: PluginsAddCommand, server: &ServerArgs) -> Result<(), let mut scopes = load_and_hydrate_scopes(server.config.as_ref(), &resolved)?; let (manifest, manifest_ref) = load_manifest_for_action("add", &command.path)?; let plugin_id = manifest.plugin.id.trim().to_owned(); - if let Some(existing) = find_record_by_id(&scopes, &plugin_id)? - && !existing.record.is_tombstoned() - { - return Err(CliError::Config(format!( - "dynamic plugin '{}' is already registered in the {} lifecycle scope", - plugin_id, - existing.scope.label() - ))); - } + let revived = match find_record_by_id(&scopes, &plugin_id)? { + Some(existing) if !existing.record.is_tombstoned() => { + return Err(CliError::Config(format!( + "dynamic plugin '{}' is already registered in the {} lifecycle scope", + plugin_id, + existing.scope.label() + ))); + } + Some(_) => true, + None => false, + }; if server.config.is_some() && scope_flags_selected(&command.scope) { return Err(CliError::Config( @@ -73,6 +81,7 @@ pub(crate) fn add(command: PluginsAddCommand, server: &ServerArgs) -> Result<(), println!("manifest: {}", manifest_ref); println!("plugins_toml: {}", plugins_toml_path.display()); println!("state_path: {}", scopes[scope_index].state_path.display()); + println!("revived: {}", if revived { "true" } else { "false" }); println!("desired.enabled: false"); Ok(()) } @@ -83,23 +92,38 @@ pub(crate) fn validate( ) -> Result<(), CliError> { match PluginTarget::parse(&command.target) { PluginTarget::Path(path) => { + if !path.exists() { + return Err(plugin_not_found( + "plugins validate", + Some(command.target.clone()), + format!("dynamic plugin target '{}' does not exist", command.target), + )); + } let (manifest, manifest_ref) = load_manifest_for_action("validate", &path)?; - println!( - "{}", - render_validation_summary(&manifest, &manifest_ref, None, None) - ); + if command.json { + print_json(&validate_success_envelope(ValidateJsonContext { + command: "plugins validate", + target: Some(command.target.as_str()), + target_kind: "path", + resolved_plugin_id: Some(manifest.plugin.id.as_str()), + manifest: &manifest, + manifest_ref: &manifest_ref, + entry: None, + host_config: None, + }))?; + } else { + println!( + "{}", + render_validation_summary(&manifest, &manifest_ref, None, None) + ); + } Ok(()) } PluginTarget::Id(plugin_id) => { let resolved = resolve_plugins_config(server.config.as_ref())?; let host_config_by_id = host_config_by_id(&resolved); let mut scopes = load_and_hydrate_scopes(server.config.as_ref(), &resolved)?; - let entry = find_record_by_id(&scopes, &plugin_id)?.ok_or_else(|| { - CliError::Config(format!( - "dynamic plugin '{}' is not registered; run `nemo-relay plugins add `", - plugin_id - )) - })?; + let entry = find_registered_entry(&scopes, "plugins validate", &plugin_id)?; let manifest_ref = manifest_ref_from_record(&entry.record)?; let (manifest, manifest_ref) = load_manifest_for_action("validate", &manifest_ref)?; scopes[entry.scope_index] @@ -121,31 +145,61 @@ pub(crate) fn validate( scopes[entry.scope_index].save()?; let refreshed = find_record_by_id(&scopes, &plugin_id)? .expect("validated registry record should still exist"); - println!( - "{}", - render_validation_summary( - &manifest, - &manifest_ref, - Some(&refreshed), - host_config_by_id.get(&plugin_id), - ) - ); + if command.json { + print_json(&validate_success_envelope(ValidateJsonContext { + command: "plugins validate", + target: Some(plugin_id.as_str()), + target_kind: "plugin_id", + resolved_plugin_id: Some(plugin_id.as_str()), + manifest: &manifest, + manifest_ref: &manifest_ref, + entry: Some(&refreshed), + host_config: host_config_by_id.get(&plugin_id), + }))?; + } else { + println!( + "{}", + render_validation_summary( + &manifest, + &manifest_ref, + Some(&refreshed), + host_config_by_id.get(&plugin_id), + ) + ); + } Ok(()) } } } pub(crate) fn list(command: PluginsListCommand, server: &ServerArgs) -> Result<(), CliError> { - let _ = command; let resolved = resolve_plugins_config(server.config.as_ref())?; let host_config_by_id = host_config_by_id(&resolved); let scopes = load_and_hydrate_scopes(server.config.as_ref(), &resolved)?; - let records = collect_records(&scopes, false); + let records = collect_records(&scopes, command.all); if records.is_empty() { - println!("No dynamic plugins registered."); + if command.json { + print_json(&list_success_envelope( + "plugins list", + None, + &records, + &host_config_by_id, + ))?; + } else { + println!("No dynamic plugins registered."); + } return Ok(()); } - println!("{}", render_list(&records, &host_config_by_id)); + if command.json { + print_json(&list_success_envelope( + "plugins list", + None, + &records, + &host_config_by_id, + ))?; + } else { + println!("{}", render_list(&records, &host_config_by_id)); + } Ok(()) } @@ -153,23 +207,29 @@ pub(crate) fn inspect(command: PluginsInspectCommand, server: &ServerArgs) -> Re let resolved = resolve_plugins_config(server.config.as_ref())?; let host_config_by_id = host_config_by_id(&resolved); let scopes = load_and_hydrate_scopes(server.config.as_ref(), &resolved)?; - let entry = find_record_by_id(&scopes, &command.id)?.ok_or_else(|| { - CliError::Config(format!( - "dynamic plugin '{}' is not registered; run `nemo-relay plugins add `", - command.id - )) - })?; + let entry = find_registered_entry(&scopes, "plugins inspect", &command.id)?; let manifest_ref = manifest_ref_from_record(&entry.record)?; let (manifest, manifest_ref) = load_manifest_for_action("inspect", &manifest_ref)?; - println!( - "{}", - render_inspect( + if command.json { + print_json(&inspect_success_envelope( + "plugins inspect", + command.id.as_str(), &entry, &manifest, &manifest_ref, host_config_by_id.get(&command.id), - ) - ); + ))?; + } else { + println!( + "{}", + render_inspect( + &entry, + &manifest, + &manifest_ref, + host_config_by_id.get(&command.id), + ) + ); + } Ok(()) } @@ -184,19 +244,18 @@ pub(crate) fn disable(command: PluginsDisableCommand, server: &ServerArgs) -> Re pub(crate) fn remove(command: PluginsRemoveCommand, server: &ServerArgs) -> Result<(), CliError> { let resolved = resolve_plugins_config(server.config.as_ref())?; let mut scopes = load_and_hydrate_scopes(server.config.as_ref(), &resolved)?; - let entry = find_record_by_id(&scopes, &command.id)?.ok_or_else(|| { - CliError::Config(format!( - "dynamic plugin '{}' is not registered; run `nemo-relay plugins add `", - command.id - )) - })?; + let entry = find_registered_entry(&scopes, "plugins remove", &command.id)?; + let original_plugins_toml = std::fs::read(&entry.plugins_toml_path).ok(); scopes[entry.scope_index] .registry .remove(&command.id) .map_err(|error| CliError::Config(error.to_string()))?; let removed_reference = remove_dynamic_plugin_reference(&entry.plugins_toml_path, &command.id)?; - scopes[entry.scope_index].save()?; + if let Err(error) = scopes[entry.scope_index].save() { + let _ = restore_plugins_toml(&entry.plugins_toml_path, original_plugins_toml.as_deref()); + return Err(error); + } println!("Removed dynamic plugin {}", command.id); println!("scope: {}", entry.scope.label()); @@ -220,12 +279,23 @@ fn mutate_enabled_state( ) -> Result<(), CliError> { let resolved = resolve_plugins_config(server.config.as_ref())?; let mut scopes = load_and_hydrate_scopes(server.config.as_ref(), &resolved)?; - let entry = find_record_by_id(&scopes, &plugin_id)?.ok_or_else(|| { - CliError::Config(format!( - "dynamic plugin '{}' is not registered; run `nemo-relay plugins add `", - plugin_id - )) - })?; + let command = if enabled { + "plugins enable" + } else { + "plugins disable" + }; + let entry = find_registered_entry(&scopes, command, &plugin_id)?; + if entry.record.is_tombstoned() { + return Err(plugin_refused( + command, + Some(plugin_id.clone()), + format!( + "dynamic plugin '{}' is tombstoned and cannot be {}d", + plugin_id, + if enabled { "enable" } else { "disable" } + ), + )); + } if enabled { scopes[entry.scope_index] .registry @@ -306,6 +376,23 @@ fn host_config_by_id(resolved: &ResolvedConfig) -> HashMap Result { + find_record_by_id(scopes, plugin_id)?.ok_or_else(|| { + plugin_not_found( + command, + Some(plugin_id.to_owned()), + format!( + "dynamic plugin '{}' is not registered; run `nemo-relay plugins add `", + plugin_id + ), + ) + }) +} + fn load_manifest_for_action( action: &str, path: impl Into, @@ -350,6 +437,72 @@ fn scope_flags_selected(scope: &crate::config::PluginsScopeArgs) -> bool { scope.user || scope.project || scope.global } +fn restore_plugins_toml(path: &std::path::Path, original: Option<&[u8]>) -> Result<(), CliError> { + match original { + Some(bytes) => std::fs::write(path, bytes)?, + None if path.exists() => std::fs::remove_file(path)?, + None => {} + } + Ok(()) +} + +pub(crate) fn render_plugin_error( + error: &CliError, + json: bool, +) -> Result, CliError> { + let Some((command, target, kind, message)) = error.plugin_lifecycle() else { + return Ok(None); + }; + + let exit_code = match kind { + PluginLifecycleFailureKind::Failed => ExitCode::from(1), + PluginLifecycleFailureKind::NotFound => ExitCode::from(2), + PluginLifecycleFailureKind::Refused => ExitCode::from(3), + }; + + if json { + print_json(&failure_envelope(command, target, kind, message))?; + } else { + eprintln!("{message}"); + } + Ok(Some(exit_code)) +} + +pub(crate) fn render_generic_plugin_json_error( + command: &'static str, + target: Option<&str>, + message: &str, +) -> Result { + print_json(&generic_failure_envelope(command, target, message))?; + Ok(ExitCode::from(1)) +} + +fn plugin_not_found( + command: &'static str, + target: Option, + message: impl Into, +) -> CliError { + CliError::PluginLifecycle { + command, + target, + kind: PluginLifecycleFailureKind::NotFound, + message: message.into(), + } +} + +fn plugin_refused( + command: &'static str, + target: Option, + message: impl Into, +) -> CliError { + CliError::PluginLifecycle { + command, + target, + kind: PluginLifecycleFailureKind::Refused, + message: message.into(), + } +} + #[cfg(test)] #[path = "../../tests/coverage/plugins_lifecycle_tests.rs"] mod tests; diff --git a/crates/cli/src/plugins/lifecycle/json.rs b/crates/cli/src/plugins/lifecycle/json.rs new file mode 100644 index 00000000..7aa32438 --- /dev/null +++ b/crates/cli/src/plugins/lifecycle/json.rs @@ -0,0 +1,188 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +use std::collections::HashMap; + +use nemo_relay::plugin::dynamic::DynamicPluginManifest; +use serde_json::{Value, json}; + +use crate::config::{DynamicPluginHostConfigStatus, ResolvedDynamicPluginConfig}; +use crate::error::{CliError, PluginLifecycleFailureKind}; + +use super::render::{check_state_label, manifest_kind_label, runtime_state_label}; +use super::state::ScopedDynamicPluginRecord; + +pub(super) struct ValidateJsonContext<'a> { + pub(super) command: &'static str, + pub(super) target: Option<&'a str>, + pub(super) target_kind: &'static str, + pub(super) resolved_plugin_id: Option<&'a str>, + pub(super) manifest: &'a DynamicPluginManifest, + pub(super) manifest_ref: &'a str, + pub(super) entry: Option<&'a ScopedDynamicPluginRecord>, + pub(super) host_config: Option<&'a ResolvedDynamicPluginConfig>, +} + +pub(super) fn print_json(value: &Value) -> Result<(), CliError> { + let rendered = serde_json::to_string_pretty(value).map_err(|error| { + CliError::Config(format!("could not serialize plugin JSON output: {error}")) + })?; + println!("{rendered}"); + Ok(()) +} + +pub(super) fn list_success_envelope( + command: &'static str, + target: Option<&str>, + records: &[ScopedDynamicPluginRecord], + host_config_by_id: &HashMap, +) -> Value { + success_envelope( + command, + target, + Value::Array( + records + .iter() + .map(|entry| { + let record = &entry.record; + json!({ + "id": record.metadata.id, + "name": record.metadata.name, + "kind": manifest_kind_label(record.metadata.kind), + "enabled": record.spec.enabled, + "tombstoned": record.is_tombstoned(), + "validation_state": check_state_label(record.status.validation.manifest), + "runtime_state": if record.is_tombstoned() { "tombstoned" } else { runtime_state_label(record.status.runtime.state) }, + "startup": record.status.startup_class.map(|value| format!("{value:?}").to_lowercase()), + "last_error": record.status.last_error.as_ref().map(|error| { + json!({ + "phase": format!("{:?}", error.phase).to_lowercase(), + "code": error.code, + "message": error.message, + }) + }), + "host_config": host_config_by_id + .get(&record.metadata.id) + .map(|plugin| host_config_status_label(plugin.host_config_status())) + .unwrap_or("missing"), + }) + }) + .collect(), + ), + ) +} + +pub(super) fn inspect_success_envelope( + command: &'static str, + target: &str, + entry: &ScopedDynamicPluginRecord, + manifest: &DynamicPluginManifest, + manifest_ref: &str, + host_config: Option<&ResolvedDynamicPluginConfig>, +) -> Value { + let record = &entry.record; + success_envelope( + command, + Some(target), + json!({ + "id": record.metadata.id, + "name": record.metadata.name, + "kind": manifest_kind_label(record.metadata.kind), + "scope": entry.scope.label(), + "manifest_ref": manifest_ref, + "plugins_toml_path": entry.plugins_toml_path, + "state_path": entry.state_path, + "load": record.load, + "compat": record.compatibility, + "capabilities": manifest.capabilities.items.iter().map(|item| format!("{item:?}").to_lowercase()).collect::>(), + "metadata": record.metadata, + "source": record.source, + "spec": record.spec, + "status": record.status, + "host_config_status": host_config.map(|plugin| host_config_status_label(plugin.host_config_status())).unwrap_or("missing"), + "host_config": host_config.map(|plugin| Value::Object(plugin.config.clone())).unwrap_or(Value::Null), + }), + ) +} + +pub(super) fn validate_success_envelope(context: ValidateJsonContext<'_>) -> Value { + let notes = context + .entry + .and_then(|entry| entry.record.status.validation.message.clone()) + .into_iter() + .collect::>(); + + success_envelope( + context.command, + context.target, + json!({ + "target_kind": context.target_kind, + "resolved_plugin_id": context.resolved_plugin_id.or(Some(context.manifest.plugin.id.as_str())), + "valid": true, + "errors": Vec::::new(), + "warnings": Vec::::new(), + "notes": notes, + "manifest_ref": context.manifest_ref, + "kind": manifest_kind_label(context.manifest.plugin.kind), + "desired_enabled": context.entry.map(|entry| entry.record.spec.enabled), + "host_config_status": context.host_config.map(|plugin| host_config_status_label(plugin.host_config_status())).unwrap_or("missing"), + }), + ) +} + +pub(super) fn failure_envelope( + command: &'static str, + target: Option<&str>, + kind: PluginLifecycleFailureKind, + message: &str, +) -> Value { + let code = match kind { + PluginLifecycleFailureKind::Failed => "operation_failed", + PluginLifecycleFailureKind::NotFound => "not_found", + PluginLifecycleFailureKind::Refused => "refused", + }; + with_schema(json!({ + "ok": false, + "command": command, + "target": target, + "error": { + "code": code, + "kind": kind, + "message": message, + "details": {} + }, + "warnings": [] + })) +} + +pub(super) fn generic_failure_envelope( + command: &'static str, + target: Option<&str>, + message: &str, +) -> Value { + failure_envelope(command, target, PluginLifecycleFailureKind::Failed, message) +} + +fn success_envelope(command: &'static str, target: Option<&str>, data: Value) -> Value { + with_schema(json!({ + "ok": true, + "command": command, + "target": target, + "warnings": [], + "data": data + })) +} + +fn with_schema(mut value: Value) -> Value { + if let Some(object) = value.as_object_mut() { + object.insert("schema_version".into(), json!(1)); + } + value +} + +fn host_config_status_label(status: DynamicPluginHostConfigStatus) -> &'static str { + match status { + DynamicPluginHostConfigStatus::Absent => "absent", + DynamicPluginHostConfigStatus::Present => "present", + } +} diff --git a/crates/cli/src/plugins/lifecycle/render.rs b/crates/cli/src/plugins/lifecycle/render.rs index fbbda488..061067e2 100644 --- a/crates/cli/src/plugins/lifecycle/render.rs +++ b/crates/cli/src/plugins/lifecycle/render.rs @@ -30,7 +30,7 @@ pub(super) fn render_list( entry.record.metadata.id, entry.scope.label(), bool_label(entry.record.spec.enabled), - runtime_state_label(entry.record.status.runtime.state), + lifecycle_state_label(&entry.record), check_state_label(entry.record.status.validation.manifest), host_config_status )); @@ -60,6 +60,18 @@ pub(super) fn render_inspect( format!("manifest: {manifest_ref}"), format!("plugins_toml: {}", entry.plugins_toml_path.display()), format!("state_path: {}", entry.state_path.display()), + format!( + "source.manifest_ref: {}", + record.source.manifest_ref.as_deref().unwrap_or("") + ), + format!( + "source.artifact_ref: {}", + record.source.artifact_ref.as_deref().unwrap_or("") + ), + format!( + "source.environment_ref: {}", + record.source.environment_ref.as_deref().unwrap_or("") + ), format!("desired.present: {}", bool_label(record.spec.present)), format!("desired.enabled: {}", bool_label(record.spec.enabled)), format!("generation: {}", record.metadata.generation), @@ -224,7 +236,7 @@ fn host_config_status_label(status: DynamicPluginHostConfigStatus) -> &'static s } } -fn check_state_label(state: DynamicPluginCheckState) -> &'static str { +pub(super) fn check_state_label(state: DynamicPluginCheckState) -> &'static str { match state { DynamicPluginCheckState::Unknown => "unknown", DynamicPluginCheckState::Valid => "valid", @@ -232,7 +244,7 @@ fn check_state_label(state: DynamicPluginCheckState) -> &'static str { } } -fn runtime_state_label(state: DynamicPluginRuntimeState) -> &'static str { +pub(super) fn runtime_state_label(state: DynamicPluginRuntimeState) -> &'static str { match state { DynamicPluginRuntimeState::Stopped => "stopped", DynamicPluginRuntimeState::Starting => "starting", @@ -241,7 +253,15 @@ fn runtime_state_label(state: DynamicPluginRuntimeState) -> &'static str { } } -fn manifest_kind_label(kind: DynamicPluginKind) -> &'static str { +fn lifecycle_state_label(record: &DynamicPluginRecord) -> &'static str { + if record.is_tombstoned() { + "tombstoned" + } else { + runtime_state_label(record.status.runtime.state) + } +} + +pub(super) fn manifest_kind_label(kind: DynamicPluginKind) -> &'static str { match kind { DynamicPluginKind::RustDynamic => "rust_dynamic", DynamicPluginKind::Worker => "worker", diff --git a/crates/cli/tests/cli_tests.rs b/crates/cli/tests/cli_tests.rs index 65917236..4c7eb3be 100644 --- a/crates/cli/tests/cli_tests.rs +++ b/crates/cli/tests/cli_tests.rs @@ -33,6 +33,38 @@ fn toml_basic_string(value: &str) -> String { format!("\"{escaped}\"") } +fn write_dynamic_plugin_manifest(dir: &std::path::Path, plugin_id: &str) { + std::fs::create_dir_all(dir).unwrap(); + std::fs::write( + dir.join("relay-plugin.toml"), + format!( + r#"manifest_version = 1 + +[plugin] +id = {plugin_id} +kind = "worker" + +[compat] +relay = "0.5" +worker_protocol = "1" + +[defaults] +enabled = false + +[capabilities] +items = ["plugin_worker"] + +[load] +runtime = "python" +entrypoint = {entrypoint} +"#, + plugin_id = toml_basic_string(plugin_id), + entrypoint = toml_basic_string(&format!("{plugin_id}.plugin:register")), + ), + ) + .unwrap(); +} + #[test] fn toml_basic_string_escapes_toml_control_characters() { assert_eq!( @@ -98,6 +130,34 @@ fn cli_doctor_json_emits_versioned_report() { assert!(parsed["agents"].is_array()); } +#[test] +fn cli_plugins_validate_json_emits_versioned_success_output() { + let temp = tempfile::tempdir().unwrap(); + let plugin_dir = temp.path().join("plugins").join("acme"); + write_dynamic_plugin_manifest(&plugin_dir, "acme.cli-json"); + + let output = Command::new(gateway_bin()) + .env("XDG_CONFIG_HOME", temp.path().join("xdg")) + .env("HOME", temp.path()) + .args(["plugins", "validate"]) + .arg(&plugin_dir) + .arg("--json") + .output() + .unwrap(); + + assert!( + output.status.success(), + "stderr was:\n{}", + String::from_utf8_lossy(&output.stderr) + ); + let parsed: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + assert_eq!(parsed["schema_version"], 1); + assert_eq!(parsed["ok"], true); + assert_eq!(parsed["command"], "plugins validate"); + assert_eq!(parsed["data"]["target_kind"], "path"); + assert_eq!(parsed["data"]["resolved_plugin_id"], "acme.cli-json"); +} + #[test] fn cli_completions_prints_script_for_requested_shell() { let output = Command::new(gateway_bin()) diff --git a/crates/cli/tests/coverage/main_tests.rs b/crates/cli/tests/coverage/main_tests.rs index 2bda64a7..e8d2dec4 100644 --- a/crates/cli/tests/coverage/main_tests.rs +++ b/crates/cli/tests/coverage/main_tests.rs @@ -55,29 +55,78 @@ fn safe_dispatch_helpers_cover_completions_and_plugins_paths() { ExitCode::SUCCESS ); - let inspect_error = run_plugins( - PluginsCommand { - command: PluginsSubcommand::Inspect(PluginsInspectCommand { - id: "missing.plugin".into(), - }), - }, - &ServerArgs::default(), - ) - .unwrap_err() - .to_string(); - assert!(inspect_error.contains("not registered")); + assert_eq!( + run_plugins( + PluginsCommand { + command: PluginsSubcommand::Inspect(PluginsInspectCommand { + id: "missing.plugin".into(), + json: false, + }), + }, + &ServerArgs::default(), + ) + .unwrap(), + ExitCode::from(2) + ); - let validate_error = run_plugins( - PluginsCommand { - command: PluginsSubcommand::Validate(PluginsValidateCommand { - target: "missing.plugin".into(), - }), - }, - &ServerArgs::default(), - ) - .unwrap_err() - .to_string(); - assert!(validate_error.contains("not registered")); + assert_eq!( + run_plugins( + PluginsCommand { + command: PluginsSubcommand::Validate(PluginsValidateCommand { + target: "missing.plugin".into(), + json: false, + }), + }, + &ServerArgs::default(), + ) + .unwrap(), + ExitCode::from(2) + ); + + assert_eq!( + run_plugins( + PluginsCommand { + command: PluginsSubcommand::List(PluginsListCommand { + all: false, + json: false, + }), + }, + &ServerArgs::default() + ) + .unwrap(), + ExitCode::SUCCESS + ); +} + +#[test] +fn safe_dispatch_plugin_json_errors_return_exit_codes() { + assert_eq!( + run_plugins( + PluginsCommand { + command: PluginsSubcommand::Inspect(PluginsInspectCommand { + id: "missing.plugin".into(), + json: true, + }), + }, + &ServerArgs::default(), + ) + .unwrap(), + ExitCode::from(2) + ); + + assert_eq!( + run_plugins( + PluginsCommand { + command: PluginsSubcommand::Validate(PluginsValidateCommand { + target: "missing.plugin".into(), + json: true, + }), + }, + &ServerArgs::default(), + ) + .unwrap(), + ExitCode::from(2) + ); } #[tokio::test] diff --git a/crates/cli/tests/coverage/plugins_lifecycle_tests.rs b/crates/cli/tests/coverage/plugins_lifecycle_tests.rs index 9a6bdf11..a9f118ea 100644 --- a/crates/cli/tests/coverage/plugins_lifecycle_tests.rs +++ b/crates/cli/tests/coverage/plugins_lifecycle_tests.rs @@ -8,6 +8,7 @@ use crate::config::{ PluginsAddCommand, PluginsDisableCommand, PluginsEnableCommand, PluginsInspectCommand, PluginsListCommand, PluginsRemoveCommand, PluginsScopeArgs, PluginsValidateCommand, ServerArgs, }; +use crate::error::PluginLifecycleFailureKind; struct CurrentDirGuard { original: PathBuf, @@ -178,6 +179,9 @@ fn list_and_inspect_render_discovered_dynamic_plugins() { assert!(inspect.contains("id: acme.guardrail")); assert!(inspect.contains("kind: worker")); assert!(inspect.contains("host_config: absent")); + assert!(inspect.contains("source.manifest_ref:")); + assert!(inspect.contains("source.artifact_ref: ")); + assert!(inspect.contains("source.environment_ref: ")); assert!(inspect.contains("load.entrypoint: acme.guardrail.plugin:register")); } @@ -229,6 +233,7 @@ fn validate_renders_summary_for_path_and_id_targets() { let missing_validate = validate( PluginsValidateCommand { target: "missing.plugin".into(), + json: false, }, &crate::config::ServerArgs::default(), ) @@ -239,6 +244,7 @@ fn validate_renders_summary_for_path_and_id_targets() { let missing_inspect = inspect( PluginsInspectCommand { id: "missing.plugin".into(), + json: false, }, &crate::config::ServerArgs::default(), ) @@ -323,6 +329,11 @@ fn enable_disable_and_remove_persist_lifecycle_state() { .unwrap() .expect("removed record"); assert!(removed.record.is_tombstoned()); + + let all_records = collect_records(&scopes, true); + let all_list = render_list(&all_records, &host_config_by_id(&resolved)); + assert!(all_list.contains("acme.guardrail")); + assert!(all_list.contains("tombstoned")); } #[test] @@ -408,3 +419,137 @@ fn hydrate_bootstraps_registry_records_from_existing_dynamic_plugin_refs() { Some(canonical_manifest_path.to_string_lossy().as_ref()) ); } + +#[test] +fn add_can_revive_tombstoned_records() { + let _lock = crate::test_support::ENV_TEST_LOCK + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + let temp = tempfile::tempdir().unwrap(); + let _cwd = CurrentDirGuard::enter(temp.path()); + let plugin_dir = temp.path().join("plugins").join("acme"); + std::fs::create_dir_all(&plugin_dir).unwrap(); + write_dynamic_manifest(&plugin_dir, "acme.revive"); + let server = crate::config::ServerArgs::default(); + + add( + PluginsAddCommand { + scope: PluginsScopeArgs { + project: true, + ..PluginsScopeArgs::default() + }, + path: plugin_dir.clone(), + }, + &server, + ) + .unwrap(); + + remove( + PluginsRemoveCommand { + id: "acme.revive".into(), + }, + &server, + ) + .unwrap(); + + add( + PluginsAddCommand { + scope: PluginsScopeArgs { + project: true, + ..PluginsScopeArgs::default() + }, + path: plugin_dir, + }, + &server, + ) + .unwrap(); + + let resolved = resolve_plugins_config(None).unwrap(); + let scopes = load_and_hydrate_scopes(None, &resolved).unwrap(); + let revived = find_record_by_id(&scopes, "acme.revive") + .unwrap() + .expect("revived record"); + assert!(!revived.record.is_tombstoned()); + assert!(revived.record.spec.present); +} + +#[test] +fn json_helpers_emit_stable_success_and_failure_shapes() { + let _lock = crate::test_support::ENV_TEST_LOCK + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + let temp = tempfile::tempdir().unwrap(); + let _cwd = CurrentDirGuard::enter(temp.path()); + let plugin_dir = temp.path().join("plugins").join("acme"); + std::fs::create_dir_all(&plugin_dir).unwrap(); + let manifest_path = write_dynamic_manifest(&plugin_dir, "acme.json"); + let server = ServerArgs::default(); + + add( + PluginsAddCommand { + scope: PluginsScopeArgs { + project: true, + ..PluginsScopeArgs::default() + }, + path: plugin_dir, + }, + &server, + ) + .unwrap(); + + let resolved = resolve_plugins_config(None).unwrap(); + let host_config_by_id = host_config_by_id(&resolved); + let scopes = load_and_hydrate_scopes(None, &resolved).unwrap(); + let records = collect_records(&scopes, false); + let entry = find_record_by_id(&scopes, "acme.json") + .unwrap() + .expect("json record"); + let (manifest, manifest_ref) = DynamicPluginManifest::load_from_path(&manifest_path) + .map_err(|error| CliError::Config(error.to_string())) + .unwrap(); + + let list_value = + json::list_success_envelope("plugins list", None, &records, &host_config_by_id); + assert_eq!(list_value["schema_version"], serde_json::json!(1)); + assert_eq!(list_value["ok"], serde_json::json!(true)); + assert_eq!(list_value["data"][0]["id"], serde_json::json!("acme.json")); + + let inspect_value = json::inspect_success_envelope( + "plugins inspect", + "acme.json", + &entry, + &manifest, + &manifest_ref, + host_config_by_id.get("acme.json"), + ); + assert_eq!(inspect_value["data"]["id"], serde_json::json!("acme.json")); + assert_eq!( + inspect_value["data"]["source"]["manifest_ref"], + serde_json::json!(manifest_ref) + ); + + let validate_value = json::validate_success_envelope(json::ValidateJsonContext { + command: "plugins validate", + target: Some("acme.json"), + target_kind: "plugin_id", + resolved_plugin_id: Some("acme.json"), + manifest: &manifest, + manifest_ref: &manifest_ref, + entry: Some(&entry), + host_config: host_config_by_id.get("acme.json"), + }); + assert_eq!( + validate_value["data"]["target_kind"], + serde_json::json!("plugin_id") + ); + assert_eq!(validate_value["data"]["valid"], serde_json::json!(true)); + + let failure = json::failure_envelope( + "plugins inspect", + Some("missing.plugin"), + PluginLifecycleFailureKind::NotFound, + "missing plugin", + ); + assert_eq!(failure["ok"], serde_json::json!(false)); + assert_eq!(failure["error"]["code"], serde_json::json!("not_found")); +} From 6b9be086dcae8d625ea3b6eb9449cddc9c3d7abd Mon Sep 17 00:00:00 2001 From: Alex Fournier Date: Tue, 23 Jun 2026 13:43:00 -0700 Subject: [PATCH 03/20] test: harden dynamic plugin lifecycle CLI coverage Signed-off-by: Alex Fournier --- crates/cli/tests/cli_tests.rs | 141 ++++++++++++++++++ .../tests/coverage/plugins_lifecycle_tests.rs | 16 ++ 2 files changed, 157 insertions(+) diff --git a/crates/cli/tests/cli_tests.rs b/crates/cli/tests/cli_tests.rs index 4c7eb3be..7ea1e8ba 100644 --- a/crates/cli/tests/cli_tests.rs +++ b/crates/cli/tests/cli_tests.rs @@ -158,6 +158,147 @@ fn cli_plugins_validate_json_emits_versioned_success_output() { assert_eq!(parsed["data"]["resolved_plugin_id"], "acme.cli-json"); } +#[test] +fn cli_plugins_list_json_emits_empty_versioned_success_output() { + let temp = tempfile::tempdir().unwrap(); + let output = Command::new(gateway_bin()) + .env("XDG_CONFIG_HOME", temp.path().join("xdg")) + .env("HOME", temp.path()) + .args(["plugins", "list", "--json"]) + .output() + .unwrap(); + + assert!(output.status.success()); + let parsed: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + assert_eq!(parsed["schema_version"], 1); + assert_eq!(parsed["ok"], true); + assert_eq!(parsed["command"], "plugins list"); + assert_eq!(parsed["data"], serde_json::json!([])); +} + +#[test] +fn cli_plugins_inspect_json_missing_plugin_emits_not_found_error() { + let temp = tempfile::tempdir().unwrap(); + let output = Command::new(gateway_bin()) + .env("XDG_CONFIG_HOME", temp.path().join("xdg")) + .env("HOME", temp.path()) + .args(["plugins", "inspect", "missing.plugin", "--json"]) + .output() + .unwrap(); + + assert_eq!(output.status.code(), Some(2)); + let parsed: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + assert_eq!(parsed["schema_version"], 1); + assert_eq!(parsed["ok"], false); + assert_eq!(parsed["command"], "plugins inspect"); + assert_eq!(parsed["error"]["code"], "not_found"); + assert_eq!(parsed["error"]["kind"], "not_found"); +} + +#[test] +fn cli_plugins_list_all_json_includes_tombstoned_records() { + let temp = tempfile::tempdir().unwrap(); + let cwd = temp.path().join("workdir"); + let plugin_dir = cwd.join("plugins").join("acme"); + std::fs::create_dir_all(&cwd).unwrap(); + write_dynamic_plugin_manifest(&plugin_dir, "acme.tombstoned"); + + let add = Command::new(gateway_bin()) + .current_dir(&cwd) + .env("XDG_CONFIG_HOME", temp.path().join("xdg")) + .env("HOME", temp.path()) + .args(["plugins", "add", "--project"]) + .arg(&plugin_dir) + .output() + .unwrap(); + assert!( + add.status.success(), + "stderr was:\n{}", + String::from_utf8_lossy(&add.stderr) + ); + + let remove = Command::new(gateway_bin()) + .current_dir(&cwd) + .env("XDG_CONFIG_HOME", temp.path().join("xdg")) + .env("HOME", temp.path()) + .args(["plugins", "remove", "acme.tombstoned"]) + .output() + .unwrap(); + assert!( + remove.status.success(), + "stderr was:\n{}", + String::from_utf8_lossy(&remove.stderr) + ); + + let list = Command::new(gateway_bin()) + .current_dir(&cwd) + .env("XDG_CONFIG_HOME", temp.path().join("xdg")) + .env("HOME", temp.path()) + .args(["plugins", "list", "--all", "--json"]) + .output() + .unwrap(); + + assert!( + list.status.success(), + "stderr was:\n{}", + String::from_utf8_lossy(&list.stderr) + ); + let parsed: serde_json::Value = serde_json::from_slice(&list.stdout).unwrap(); + assert_eq!(parsed["schema_version"], 1); + assert_eq!(parsed["ok"], true); + assert_eq!(parsed["command"], "plugins list"); + assert_eq!(parsed["data"][0]["id"], "acme.tombstoned"); + assert_eq!(parsed["data"][0]["tombstoned"], true); + assert_eq!(parsed["data"][0]["runtime_state"], "tombstoned"); +} + +#[test] +fn cli_plugins_inspect_json_emits_installed_plugin_details() { + let temp = tempfile::tempdir().unwrap(); + let cwd = temp.path().join("workdir"); + let plugin_dir = cwd.join("plugins").join("acme"); + std::fs::create_dir_all(&cwd).unwrap(); + write_dynamic_plugin_manifest(&plugin_dir, "acme.inspect-json"); + + let add = Command::new(gateway_bin()) + .current_dir(&cwd) + .env("XDG_CONFIG_HOME", temp.path().join("xdg")) + .env("HOME", temp.path()) + .args(["plugins", "add", "--project"]) + .arg(&plugin_dir) + .output() + .unwrap(); + assert!( + add.status.success(), + "stderr was:\n{}", + String::from_utf8_lossy(&add.stderr) + ); + + let inspect = Command::new(gateway_bin()) + .current_dir(&cwd) + .env("XDG_CONFIG_HOME", temp.path().join("xdg")) + .env("HOME", temp.path()) + .args(["plugins", "inspect", "acme.inspect-json", "--json"]) + .output() + .unwrap(); + + assert!( + inspect.status.success(), + "stderr was:\n{}", + String::from_utf8_lossy(&inspect.stderr) + ); + let parsed: serde_json::Value = serde_json::from_slice(&inspect.stdout).unwrap(); + assert_eq!(parsed["schema_version"], 1); + assert_eq!(parsed["ok"], true); + assert_eq!(parsed["command"], "plugins inspect"); + assert_eq!(parsed["target"], "acme.inspect-json"); + assert_eq!(parsed["data"]["id"], "acme.inspect-json"); + assert_eq!(parsed["data"]["kind"], "worker"); + assert_eq!(parsed["data"]["scope"], "project"); + assert_eq!(parsed["data"]["host_config_status"], "absent"); + assert!(parsed["data"]["source"]["manifest_ref"].is_string()); +} + #[test] fn cli_completions_prints_script_for_requested_shell() { let output = Command::new(gateway_bin()) diff --git a/crates/cli/tests/coverage/plugins_lifecycle_tests.rs b/crates/cli/tests/coverage/plugins_lifecycle_tests.rs index a9f118ea..9b41d379 100644 --- a/crates/cli/tests/coverage/plugins_lifecycle_tests.rs +++ b/crates/cli/tests/coverage/plugins_lifecycle_tests.rs @@ -334,6 +334,22 @@ fn enable_disable_and_remove_persist_lifecycle_state() { let all_list = render_list(&all_records, &host_config_by_id(&resolved)); assert!(all_list.contains("acme.guardrail")); assert!(all_list.contains("tombstoned")); + + let error = enable( + PluginsEnableCommand { + id: "acme.guardrail".into(), + }, + &server, + ) + .expect_err("tombstoned plugin should not enable"); + match error { + CliError::PluginLifecycle { + kind: PluginLifecycleFailureKind::Refused, + message, + .. + } => assert!(message.contains("tombstoned")), + other => panic!("unexpected tombstone enable error: {other}"), + } } #[test] From 90d89c7c7f1e85331cf6683dbe000863692f33ed Mon Sep 17 00:00:00 2001 From: Alex Fournier Date: Tue, 23 Jun 2026 13:55:00 -0700 Subject: [PATCH 04/20] refactor: clarify dynamic plugin lifecycle responses Signed-off-by: Alex Fournier --- crates/cli/src/plugins/lifecycle.rs | 24 +- crates/cli/src/plugins/lifecycle/json.rs | 188 ----------- crates/cli/src/plugins/lifecycle/responses.rs | 316 ++++++++++++++++++ .../tests/coverage/plugins_lifecycle_tests.rs | 42 ++- 4 files changed, 354 insertions(+), 216 deletions(-) delete mode 100644 crates/cli/src/plugins/lifecycle/json.rs create mode 100644 crates/cli/src/plugins/lifecycle/responses.rs diff --git a/crates/cli/src/plugins/lifecycle.rs b/crates/cli/src/plugins/lifecycle.rs index 87dd5c0f..c3679c4f 100644 --- a/crates/cli/src/plugins/lifecycle.rs +++ b/crates/cli/src/plugins/lifecycle.rs @@ -21,16 +21,16 @@ use super::config_io::{ append_dynamic_plugin_reference, remove_dynamic_plugin_reference, target_scope, }; -mod json; mod render; +mod responses; mod state; mod target; -use self::json::{ - ValidateJsonContext, failure_envelope, generic_failure_envelope, inspect_success_envelope, - list_success_envelope, print_json, validate_success_envelope, -}; use self::render::{render_inspect, render_list, render_validation_summary}; +use self::responses::{ + ValidateResponseInput, failure, generic_failure, inspect_success, list_success, + print_response_json, validate_success, +}; use self::state::{ RegistryScope, ScopedRegistry, collect_records, find_record_by_id, load_scoped_registries, scoped_paths_for_add, @@ -101,7 +101,7 @@ pub(crate) fn validate( } let (manifest, manifest_ref) = load_manifest_for_action("validate", &path)?; if command.json { - print_json(&validate_success_envelope(ValidateJsonContext { + print_response_json(&validate_success(ValidateResponseInput { command: "plugins validate", target: Some(command.target.as_str()), target_kind: "path", @@ -146,7 +146,7 @@ pub(crate) fn validate( let refreshed = find_record_by_id(&scopes, &plugin_id)? .expect("validated registry record should still exist"); if command.json { - print_json(&validate_success_envelope(ValidateJsonContext { + print_response_json(&validate_success(ValidateResponseInput { command: "plugins validate", target: Some(plugin_id.as_str()), target_kind: "plugin_id", @@ -179,7 +179,7 @@ pub(crate) fn list(command: PluginsListCommand, server: &ServerArgs) -> Result<( let records = collect_records(&scopes, command.all); if records.is_empty() { if command.json { - print_json(&list_success_envelope( + print_response_json(&list_success( "plugins list", None, &records, @@ -191,7 +191,7 @@ pub(crate) fn list(command: PluginsListCommand, server: &ServerArgs) -> Result<( return Ok(()); } if command.json { - print_json(&list_success_envelope( + print_response_json(&list_success( "plugins list", None, &records, @@ -211,7 +211,7 @@ pub(crate) fn inspect(command: PluginsInspectCommand, server: &ServerArgs) -> Re let manifest_ref = manifest_ref_from_record(&entry.record)?; let (manifest, manifest_ref) = load_manifest_for_action("inspect", &manifest_ref)?; if command.json { - print_json(&inspect_success_envelope( + print_response_json(&inspect_success( "plugins inspect", command.id.as_str(), &entry, @@ -461,7 +461,7 @@ pub(crate) fn render_plugin_error( }; if json { - print_json(&failure_envelope(command, target, kind, message))?; + print_response_json(&failure(command, target, kind, message))?; } else { eprintln!("{message}"); } @@ -473,7 +473,7 @@ pub(crate) fn render_generic_plugin_json_error( target: Option<&str>, message: &str, ) -> Result { - print_json(&generic_failure_envelope(command, target, message))?; + print_response_json(&generic_failure(command, target, message))?; Ok(ExitCode::from(1)) } diff --git a/crates/cli/src/plugins/lifecycle/json.rs b/crates/cli/src/plugins/lifecycle/json.rs deleted file mode 100644 index 7aa32438..00000000 --- a/crates/cli/src/plugins/lifecycle/json.rs +++ /dev/null @@ -1,188 +0,0 @@ -// SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. -// SPDX-License-Identifier: Apache-2.0 - -use std::collections::HashMap; - -use nemo_relay::plugin::dynamic::DynamicPluginManifest; -use serde_json::{Value, json}; - -use crate::config::{DynamicPluginHostConfigStatus, ResolvedDynamicPluginConfig}; -use crate::error::{CliError, PluginLifecycleFailureKind}; - -use super::render::{check_state_label, manifest_kind_label, runtime_state_label}; -use super::state::ScopedDynamicPluginRecord; - -pub(super) struct ValidateJsonContext<'a> { - pub(super) command: &'static str, - pub(super) target: Option<&'a str>, - pub(super) target_kind: &'static str, - pub(super) resolved_plugin_id: Option<&'a str>, - pub(super) manifest: &'a DynamicPluginManifest, - pub(super) manifest_ref: &'a str, - pub(super) entry: Option<&'a ScopedDynamicPluginRecord>, - pub(super) host_config: Option<&'a ResolvedDynamicPluginConfig>, -} - -pub(super) fn print_json(value: &Value) -> Result<(), CliError> { - let rendered = serde_json::to_string_pretty(value).map_err(|error| { - CliError::Config(format!("could not serialize plugin JSON output: {error}")) - })?; - println!("{rendered}"); - Ok(()) -} - -pub(super) fn list_success_envelope( - command: &'static str, - target: Option<&str>, - records: &[ScopedDynamicPluginRecord], - host_config_by_id: &HashMap, -) -> Value { - success_envelope( - command, - target, - Value::Array( - records - .iter() - .map(|entry| { - let record = &entry.record; - json!({ - "id": record.metadata.id, - "name": record.metadata.name, - "kind": manifest_kind_label(record.metadata.kind), - "enabled": record.spec.enabled, - "tombstoned": record.is_tombstoned(), - "validation_state": check_state_label(record.status.validation.manifest), - "runtime_state": if record.is_tombstoned() { "tombstoned" } else { runtime_state_label(record.status.runtime.state) }, - "startup": record.status.startup_class.map(|value| format!("{value:?}").to_lowercase()), - "last_error": record.status.last_error.as_ref().map(|error| { - json!({ - "phase": format!("{:?}", error.phase).to_lowercase(), - "code": error.code, - "message": error.message, - }) - }), - "host_config": host_config_by_id - .get(&record.metadata.id) - .map(|plugin| host_config_status_label(plugin.host_config_status())) - .unwrap_or("missing"), - }) - }) - .collect(), - ), - ) -} - -pub(super) fn inspect_success_envelope( - command: &'static str, - target: &str, - entry: &ScopedDynamicPluginRecord, - manifest: &DynamicPluginManifest, - manifest_ref: &str, - host_config: Option<&ResolvedDynamicPluginConfig>, -) -> Value { - let record = &entry.record; - success_envelope( - command, - Some(target), - json!({ - "id": record.metadata.id, - "name": record.metadata.name, - "kind": manifest_kind_label(record.metadata.kind), - "scope": entry.scope.label(), - "manifest_ref": manifest_ref, - "plugins_toml_path": entry.plugins_toml_path, - "state_path": entry.state_path, - "load": record.load, - "compat": record.compatibility, - "capabilities": manifest.capabilities.items.iter().map(|item| format!("{item:?}").to_lowercase()).collect::>(), - "metadata": record.metadata, - "source": record.source, - "spec": record.spec, - "status": record.status, - "host_config_status": host_config.map(|plugin| host_config_status_label(plugin.host_config_status())).unwrap_or("missing"), - "host_config": host_config.map(|plugin| Value::Object(plugin.config.clone())).unwrap_or(Value::Null), - }), - ) -} - -pub(super) fn validate_success_envelope(context: ValidateJsonContext<'_>) -> Value { - let notes = context - .entry - .and_then(|entry| entry.record.status.validation.message.clone()) - .into_iter() - .collect::>(); - - success_envelope( - context.command, - context.target, - json!({ - "target_kind": context.target_kind, - "resolved_plugin_id": context.resolved_plugin_id.or(Some(context.manifest.plugin.id.as_str())), - "valid": true, - "errors": Vec::::new(), - "warnings": Vec::::new(), - "notes": notes, - "manifest_ref": context.manifest_ref, - "kind": manifest_kind_label(context.manifest.plugin.kind), - "desired_enabled": context.entry.map(|entry| entry.record.spec.enabled), - "host_config_status": context.host_config.map(|plugin| host_config_status_label(plugin.host_config_status())).unwrap_or("missing"), - }), - ) -} - -pub(super) fn failure_envelope( - command: &'static str, - target: Option<&str>, - kind: PluginLifecycleFailureKind, - message: &str, -) -> Value { - let code = match kind { - PluginLifecycleFailureKind::Failed => "operation_failed", - PluginLifecycleFailureKind::NotFound => "not_found", - PluginLifecycleFailureKind::Refused => "refused", - }; - with_schema(json!({ - "ok": false, - "command": command, - "target": target, - "error": { - "code": code, - "kind": kind, - "message": message, - "details": {} - }, - "warnings": [] - })) -} - -pub(super) fn generic_failure_envelope( - command: &'static str, - target: Option<&str>, - message: &str, -) -> Value { - failure_envelope(command, target, PluginLifecycleFailureKind::Failed, message) -} - -fn success_envelope(command: &'static str, target: Option<&str>, data: Value) -> Value { - with_schema(json!({ - "ok": true, - "command": command, - "target": target, - "warnings": [], - "data": data - })) -} - -fn with_schema(mut value: Value) -> Value { - if let Some(object) = value.as_object_mut() { - object.insert("schema_version".into(), json!(1)); - } - value -} - -fn host_config_status_label(status: DynamicPluginHostConfigStatus) -> &'static str { - match status { - DynamicPluginHostConfigStatus::Absent => "absent", - DynamicPluginHostConfigStatus::Present => "present", - } -} diff --git a/crates/cli/src/plugins/lifecycle/responses.rs b/crates/cli/src/plugins/lifecycle/responses.rs new file mode 100644 index 00000000..46a5b6f8 --- /dev/null +++ b/crates/cli/src/plugins/lifecycle/responses.rs @@ -0,0 +1,316 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +//! Machine-readable response layer for dynamic plugin lifecycle commands. +//! +//! This module owns the versioned response contract for +//! `plugins list`, `plugins inspect`, `plugins validate`, and structured +//! lifecycle errors. Command logic lives in `lifecycle.rs`; this file only +//! turns already-resolved state into stable responses serialized as JSON. + +use std::collections::HashMap; + +use nemo_relay::plugin::dynamic::{DynamicPluginFailurePhase, DynamicPluginManifest}; +use serde::Serialize; +use serde_json::{Map, Value}; + +use crate::config::{DynamicPluginHostConfigStatus, ResolvedDynamicPluginConfig}; +use crate::error::{CliError, PluginLifecycleFailureKind}; + +use super::render::{check_state_label, manifest_kind_label, runtime_state_label}; +use super::state::ScopedDynamicPluginRecord; + +#[derive(Debug)] +pub(super) struct ValidateResponseInput<'a> { + pub(super) command: &'static str, + pub(super) target: Option<&'a str>, + pub(super) target_kind: &'static str, + pub(super) resolved_plugin_id: Option<&'a str>, + pub(super) manifest: &'a DynamicPluginManifest, + pub(super) manifest_ref: &'a str, + pub(super) entry: Option<&'a ScopedDynamicPluginRecord>, + pub(super) host_config: Option<&'a ResolvedDynamicPluginConfig>, +} + +#[derive(Debug, Serialize)] +pub(super) struct ResponseEnvelope { + schema_version: u32, + ok: bool, + command: &'static str, + target: Option, + warnings: Vec, + #[serde(skip_serializing_if = "Option::is_none")] + data: Option, + #[serde(skip_serializing_if = "Option::is_none")] + error: Option, +} + +#[derive(Debug, Serialize)] +pub(super) struct ResponseError { + code: &'static str, + kind: PluginLifecycleFailureKind, + message: String, + details: Map, +} + +#[derive(Debug, Serialize)] +pub(super) struct ListEntryResponse { + id: String, + name: Option, + kind: &'static str, + enabled: bool, + tombstoned: bool, + validation_state: &'static str, + runtime_state: String, + startup: Option, + last_error: Option, + host_config: &'static str, +} + +#[derive(Debug, Serialize)] +pub(super) struct LastErrorResponse { + phase: String, + code: String, + message: String, +} + +#[derive(Debug, Serialize)] +pub(super) struct InspectResponse { + id: String, + name: Option, + kind: &'static str, + scope: &'static str, + manifest_ref: String, + plugins_toml_path: String, + state_path: String, + load: Value, + compat: Value, + capabilities: Vec, + metadata: Value, + source: Value, + spec: Value, + status: Value, + host_config_status: &'static str, + host_config: Value, +} + +#[derive(Debug, Serialize)] +pub(super) struct ValidateResponse { + target_kind: &'static str, + resolved_plugin_id: String, + valid: bool, + errors: Vec, + warnings: Vec, + notes: Vec, + manifest_ref: String, + kind: &'static str, + desired_enabled: Option, + host_config_status: &'static str, +} + +pub(super) fn print_response_json(value: &T) -> Result<(), CliError> { + let rendered = serde_json::to_string_pretty(value).map_err(|error| { + CliError::Config(format!("could not serialize plugin JSON output: {error}")) + })?; + println!("{rendered}"); + Ok(()) +} + +pub(super) fn list_success( + command: &'static str, + target: Option<&str>, + records: &[ScopedDynamicPluginRecord], + host_config_by_id: &HashMap, +) -> ResponseEnvelope> { + success( + command, + target, + records + .iter() + .map(|entry| { + let record = &entry.record; + ListEntryResponse { + id: record.metadata.id.clone(), + name: record.metadata.name.clone(), + kind: manifest_kind_label(record.metadata.kind), + enabled: record.spec.enabled, + tombstoned: record.is_tombstoned(), + validation_state: check_state_label(record.status.validation.manifest), + runtime_state: if record.is_tombstoned() { + "tombstoned".into() + } else { + runtime_state_label(record.status.runtime.state).into() + }, + startup: record + .status + .startup_class + .map(|value| format!("{value:?}").to_lowercase()), + last_error: record + .status + .last_error + .as_ref() + .map(|error| LastErrorResponse { + phase: failure_phase_label(error.phase).into(), + code: error.code.clone(), + message: error.message.clone(), + }), + host_config: host_config_by_id + .get(&record.metadata.id) + .map(|plugin| host_config_status_label(plugin.host_config_status())) + .unwrap_or("missing"), + } + }) + .collect(), + ) +} + +pub(super) fn inspect_success( + command: &'static str, + target: &str, + entry: &ScopedDynamicPluginRecord, + manifest: &DynamicPluginManifest, + manifest_ref: &str, + host_config: Option<&ResolvedDynamicPluginConfig>, +) -> ResponseEnvelope { + let record = &entry.record; + success( + command, + Some(target), + InspectResponse { + id: record.metadata.id.clone(), + name: record.metadata.name.clone(), + kind: manifest_kind_label(record.metadata.kind), + scope: entry.scope.label(), + manifest_ref: manifest_ref.into(), + plugins_toml_path: entry.plugins_toml_path.display().to_string(), + state_path: entry.state_path.display().to_string(), + load: serde_json::to_value(&record.load) + .expect("dynamic plugin load contract serializes to JSON"), + compat: serde_json::to_value(&record.compatibility) + .expect("dynamic plugin compatibility serializes to JSON"), + capabilities: manifest + .capabilities + .items + .iter() + .map(|item| format!("{item:?}").to_lowercase()) + .collect(), + metadata: serde_json::to_value(&record.metadata) + .expect("dynamic plugin metadata serializes to JSON"), + source: serde_json::to_value(&record.source) + .expect("dynamic plugin source serializes to JSON"), + spec: serde_json::to_value(&record.spec) + .expect("dynamic plugin spec serializes to JSON"), + status: serde_json::to_value(&record.status) + .expect("dynamic plugin status serializes to JSON"), + host_config_status: host_config + .map(|plugin| host_config_status_label(plugin.host_config_status())) + .unwrap_or("missing"), + host_config: host_config + .map(|plugin| Value::Object(plugin.config.clone())) + .unwrap_or(Value::Null), + }, + ) +} + +pub(super) fn validate_success( + input: ValidateResponseInput<'_>, +) -> ResponseEnvelope { + let notes = input + .entry + .and_then(|entry| entry.record.status.validation.message.clone()) + .into_iter() + .collect::>(); + + success( + input.command, + input.target, + ValidateResponse { + target_kind: input.target_kind, + resolved_plugin_id: input + .resolved_plugin_id + .unwrap_or(input.manifest.plugin.id.as_str()) + .to_owned(), + valid: true, + errors: Vec::new(), + warnings: Vec::new(), + notes, + manifest_ref: input.manifest_ref.into(), + kind: manifest_kind_label(input.manifest.plugin.kind), + desired_enabled: input.entry.map(|entry| entry.record.spec.enabled), + host_config_status: input + .host_config + .map(|plugin| host_config_status_label(plugin.host_config_status())) + .unwrap_or("missing"), + }, + ) +} + +pub(super) fn failure( + command: &'static str, + target: Option<&str>, + kind: PluginLifecycleFailureKind, + message: &str, +) -> ResponseEnvelope { + ResponseEnvelope { + schema_version: 1, + ok: false, + command, + target: target.map(str::to_owned), + warnings: Vec::new(), + data: None, + error: Some(ResponseError { + code: failure_code(kind), + kind, + message: message.to_owned(), + details: Map::new(), + }), + } +} + +pub(super) fn generic_failure( + command: &'static str, + target: Option<&str>, + message: &str, +) -> ResponseEnvelope { + failure(command, target, PluginLifecycleFailureKind::Failed, message) +} + +fn success( + command: &'static str, + target: Option<&str>, + data: T, +) -> ResponseEnvelope { + ResponseEnvelope { + schema_version: 1, + ok: true, + command, + target: target.map(str::to_owned), + warnings: Vec::new(), + data: Some(data), + error: None, + } +} + +fn host_config_status_label(status: DynamicPluginHostConfigStatus) -> &'static str { + match status { + DynamicPluginHostConfigStatus::Absent => "absent", + DynamicPluginHostConfigStatus::Present => "present", + } +} + +fn failure_code(kind: PluginLifecycleFailureKind) -> &'static str { + match kind { + PluginLifecycleFailureKind::Failed => "operation_failed", + PluginLifecycleFailureKind::NotFound => "not_found", + PluginLifecycleFailureKind::Refused => "refused", + } +} + +fn failure_phase_label(phase: DynamicPluginFailurePhase) -> &'static str { + match phase { + DynamicPluginFailurePhase::Validation => "validation", + DynamicPluginFailurePhase::Activation => "activation", + DynamicPluginFailurePhase::Runtime => "runtime", + DynamicPluginFailurePhase::Policy => "policy", + } +} diff --git a/crates/cli/tests/coverage/plugins_lifecycle_tests.rs b/crates/cli/tests/coverage/plugins_lifecycle_tests.rs index 9b41d379..8822b567 100644 --- a/crates/cli/tests/coverage/plugins_lifecycle_tests.rs +++ b/crates/cli/tests/coverage/plugins_lifecycle_tests.rs @@ -524,48 +524,58 @@ fn json_helpers_emit_stable_success_and_failure_shapes() { .map_err(|error| CliError::Config(error.to_string())) .unwrap(); - let list_value = - json::list_success_envelope("plugins list", None, &records, &host_config_by_id); + let list_value = serde_json::to_value(responses::list_success( + "plugins list", + None, + &records, + &host_config_by_id, + )) + .unwrap(); assert_eq!(list_value["schema_version"], serde_json::json!(1)); assert_eq!(list_value["ok"], serde_json::json!(true)); assert_eq!(list_value["data"][0]["id"], serde_json::json!("acme.json")); - let inspect_value = json::inspect_success_envelope( + let inspect_value = serde_json::to_value(responses::inspect_success( "plugins inspect", "acme.json", &entry, &manifest, &manifest_ref, host_config_by_id.get("acme.json"), - ); + )) + .unwrap(); assert_eq!(inspect_value["data"]["id"], serde_json::json!("acme.json")); assert_eq!( inspect_value["data"]["source"]["manifest_ref"], serde_json::json!(manifest_ref) ); - let validate_value = json::validate_success_envelope(json::ValidateJsonContext { - command: "plugins validate", - target: Some("acme.json"), - target_kind: "plugin_id", - resolved_plugin_id: Some("acme.json"), - manifest: &manifest, - manifest_ref: &manifest_ref, - entry: Some(&entry), - host_config: host_config_by_id.get("acme.json"), - }); + let validate_value = serde_json::to_value(responses::validate_success( + responses::ValidateResponseInput { + command: "plugins validate", + target: Some("acme.json"), + target_kind: "plugin_id", + resolved_plugin_id: Some("acme.json"), + manifest: &manifest, + manifest_ref: &manifest_ref, + entry: Some(&entry), + host_config: host_config_by_id.get("acme.json"), + }, + )) + .unwrap(); assert_eq!( validate_value["data"]["target_kind"], serde_json::json!("plugin_id") ); assert_eq!(validate_value["data"]["valid"], serde_json::json!(true)); - let failure = json::failure_envelope( + let failure = serde_json::to_value(responses::failure( "plugins inspect", Some("missing.plugin"), PluginLifecycleFailureKind::NotFound, "missing plugin", - ); + )) + .unwrap(); assert_eq!(failure["ok"], serde_json::json!(false)); assert_eq!(failure["error"]["code"], serde_json::json!("not_found")); } From 086832fd358e700d06e3adfa7d6c9ce23b16bcaf Mon Sep 17 00:00:00 2001 From: Alex Fournier Date: Tue, 23 Jun 2026 14:01:30 -0700 Subject: [PATCH 05/20] refactor: harden dynamic plugin target parsing Signed-off-by: Alex Fournier --- crates/cli/src/plugins/lifecycle/target.rs | 108 +++++++++++++++++++-- 1 file changed, 102 insertions(+), 6 deletions(-) diff --git a/crates/cli/src/plugins/lifecycle/target.rs b/crates/cli/src/plugins/lifecycle/target.rs index ab1734b7..46ab28b8 100644 --- a/crates/cli/src/plugins/lifecycle/target.rs +++ b/crates/cli/src/plugins/lifecycle/target.rs @@ -11,19 +11,115 @@ pub(super) enum PluginTarget { impl PluginTarget { pub(super) fn parse(target: &str) -> Self { - if looks_like_path(target) { - Self::Path(PathBuf::from(target)) - } else { - Self::Id(target.to_owned()) + match classify_target_syntax(target) { + TargetSyntax::PathLike => Self::Path(PathBuf::from(target)), + TargetSyntax::PluginId => Self::Id(target.to_owned()), } } } -fn looks_like_path(target: &str) -> bool { +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum TargetSyntax { + PathLike, + PluginId, +} + +fn classify_target_syntax(target: &str) -> TargetSyntax { + if should_treat_target_as_path(target) { + TargetSyntax::PathLike + } else { + TargetSyntax::PluginId + } +} + +// CLI target parsing intentionally uses a conservative "path-like" heuristic rather than trying +// to validate every possible plugin ID. The goal is to treat explicit filesystem syntax as a path +// while keeping ordinary canonical IDs like `acme.worker` on the ID branch. +fn should_treat_target_as_path(target: &str) -> bool { let path = Path::new(target); - path.exists() + if path.exists() || path.is_absolute() { + return true; + } + + target == "." + || target == ".." + || target.starts_with("./") + || target.starts_with("../") || target.ends_with(".toml") || target.contains(std::path::MAIN_SEPARATOR) || target.contains('/') || target.contains('\\') } + +#[cfg(test)] +mod tests { + use std::path::PathBuf; + + use super::PluginTarget; + use tempfile::tempdir; + + #[test] + fn parse_treats_canonical_plugin_ids_as_ids() { + assert_eq!( + PluginTarget::parse("acme.worker"), + PluginTarget::Id("acme.worker".into()) + ); + assert_eq!( + PluginTarget::parse("acme.worker.v2"), + PluginTarget::Id("acme.worker.v2".into()) + ); + assert_eq!( + PluginTarget::parse("relay-plugin"), + PluginTarget::Id("relay-plugin".into()) + ); + } + + #[test] + fn parse_treats_manifest_filenames_as_paths() { + assert_eq!( + PluginTarget::parse("relay-plugin.toml"), + PluginTarget::Path(PathBuf::from("relay-plugin.toml")) + ); + } + + #[test] + fn parse_treats_relative_path_syntax_as_paths() { + assert_eq!( + PluginTarget::parse("./plugins/acme/relay-plugin.toml"), + PluginTarget::Path(PathBuf::from("./plugins/acme/relay-plugin.toml")) + ); + assert_eq!( + PluginTarget::parse("."), + PluginTarget::Path(PathBuf::from(".")) + ); + assert_eq!( + PluginTarget::parse(".."), + PluginTarget::Path(PathBuf::from("..")) + ); + assert_eq!( + PluginTarget::parse(r"plugins\acme\relay-plugin.toml"), + PluginTarget::Path(PathBuf::from(r"plugins\acme\relay-plugin.toml")) + ); + } + + #[test] + fn parse_treats_absolute_paths_as_paths_even_when_missing() { + let temp = tempdir().unwrap(); + let missing = temp.path().join("missing").join("relay-plugin.toml"); + assert_eq!( + PluginTarget::parse(missing.to_str().unwrap()), + PluginTarget::Path(missing) + ); + } + + #[test] + fn parse_treats_existing_filesystem_entries_as_paths() { + let temp = tempdir().unwrap(); + let existing = temp.path().join("acme.worker"); + std::fs::create_dir_all(&existing).unwrap(); + assert_eq!( + PluginTarget::parse(existing.to_str().unwrap()), + PluginTarget::Path(existing) + ); + } +} From 5e94508e993391e9d8078dfa616422fce94b3433 Mon Sep 17 00:00:00 2001 From: Alex Fournier Date: Tue, 23 Jun 2026 18:46:47 -0700 Subject: [PATCH 06/20] fix: address dynamic plugin lifecycle CLI review nits Signed-off-by: Alex Fournier --- crates/cli/src/plugins/config_io.rs | 36 ++- crates/cli/src/plugins/lifecycle.rs | 16 +- crates/cli/src/plugins/lifecycle/render.rs | 22 +- crates/cli/src/plugins/lifecycle/responses.rs | 6 +- crates/cli/src/plugins/lifecycle/state.rs | 40 ++- crates/cli/src/plugins/lifecycle/target.rs | 16 +- crates/cli/tests/coverage/main_tests.rs | 58 +++++ .../tests/coverage/plugins_lifecycle_tests.rs | 230 ++++++++++++++++-- 8 files changed, 365 insertions(+), 59 deletions(-) diff --git a/crates/cli/src/plugins/config_io.rs b/crates/cli/src/plugins/config_io.rs index 10b86f08..37e6bc1b 100644 --- a/crates/cli/src/plugins/config_io.rs +++ b/crates/cli/src/plugins/config_io.rs @@ -153,6 +153,7 @@ pub(crate) fn append_dynamic_plugin_reference( pub(crate) fn remove_dynamic_plugin_reference( path: &Path, plugin_id: &str, + target_manifest_ref: Option<&str>, ) -> Result { if !path.exists() { return Ok(false); @@ -162,15 +163,28 @@ pub(crate) fn remove_dynamic_plugin_reference( let Some(root_table) = root.as_table_mut() else { return Ok(false); }; - let Some(toml::Value::Table(plugins)) = root_table.get_mut("plugins") else { + let Some(plugins_value) = root_table.get_mut("plugins") else { return Ok(false); }; - let Some(toml::Value::Array(dynamic_entries)) = plugins.get_mut("dynamic") else { + let plugins = plugins_value.as_table_mut().ok_or_else(|| { + CliError::Config(format!( + "invalid plugin TOML in {}: [plugins] must be a table", + path.display() + )) + })?; + let Some(dynamic_value) = plugins.get_mut("dynamic") else { return Ok(false); }; + let dynamic_entries = dynamic_value.as_array_mut().ok_or_else(|| { + CliError::Config(format!( + "invalid plugin TOML in {}: plugins.dynamic must be an array of tables", + path.display() + )) + })?; let original_len = dynamic_entries.len(); let mut retained = Vec::with_capacity(original_len); + let target_manifest_ref = target_manifest_ref.map(PathBuf::from); for entry in dynamic_entries.drain(..) { let manifest_ref = entry .as_table() @@ -178,16 +192,16 @@ pub(crate) fn remove_dynamic_plugin_reference( .and_then(toml::Value::as_str) .map(|manifest| resolve_manifest_ref(path, manifest)); - let keep = match manifest_ref { - Some(manifest_ref) => { - let (manifest, _) = DynamicPluginManifest::load_from_path(&manifest_ref) - .map_err(|error| CliError::Config(error.to_string()))?; - manifest.plugin.id.trim() != plugin_id - } - None => true, - }; + let remove = manifest_ref.as_ref().is_some_and(|manifest_ref| { + target_manifest_ref + .as_ref() + .is_some_and(|target_manifest_ref| manifest_ref == target_manifest_ref) + || DynamicPluginManifest::load_from_path(manifest_ref) + .map(|(manifest, _)| manifest.plugin.id.trim() == plugin_id) + .unwrap_or(false) + }); - if keep { + if !remove { retained.push(entry); } } diff --git a/crates/cli/src/plugins/lifecycle.rs b/crates/cli/src/plugins/lifecycle.rs index c3679c4f..c95bdd61 100644 --- a/crates/cli/src/plugins/lifecycle.rs +++ b/crates/cli/src/plugins/lifecycle.rs @@ -65,6 +65,7 @@ pub(crate) fn add(command: PluginsAddCommand, server: &ServerArgs) -> Result<(), scoped_paths_for_add(target_scope(&command.scope)?, server.config.as_ref())?; let scope_index = ensure_scope(&mut scopes, scope, plugins_toml_path.clone(), state_path); let record = validated_record_from_manifest(manifest, manifest_ref.clone())?; + let original_plugins_toml = std::fs::read(&plugins_toml_path).ok(); scopes[scope_index] .registry @@ -72,7 +73,7 @@ pub(crate) fn add(command: PluginsAddCommand, server: &ServerArgs) -> Result<(), .map_err(|error| CliError::Config(error.to_string()))?; append_dynamic_plugin_reference(&plugins_toml_path, &manifest_ref)?; if let Err(error) = scopes[scope_index].save() { - let _ = remove_dynamic_plugin_reference(&plugins_toml_path, &plugin_id); + let _ = restore_plugins_toml(&plugins_toml_path, original_plugins_toml.as_deref()); return Err(error); } @@ -242,8 +243,11 @@ pub(crate) fn disable(command: PluginsDisableCommand, server: &ServerArgs) -> Re } pub(crate) fn remove(command: PluginsRemoveCommand, server: &ServerArgs) -> Result<(), CliError> { - let resolved = resolve_plugins_config(server.config.as_ref())?; - let mut scopes = load_and_hydrate_scopes(server.config.as_ref(), &resolved)?; + let mut scopes = load_scoped_registries(server.config.as_ref())?; + if find_record_by_id(&scopes, &command.id)?.is_none() { + let resolved = resolve_plugins_config(server.config.as_ref())?; + scopes = load_and_hydrate_scopes(server.config.as_ref(), &resolved)?; + } let entry = find_registered_entry(&scopes, "plugins remove", &command.id)?; let original_plugins_toml = std::fs::read(&entry.plugins_toml_path).ok(); @@ -251,7 +255,11 @@ pub(crate) fn remove(command: PluginsRemoveCommand, server: &ServerArgs) -> Resu .registry .remove(&command.id) .map_err(|error| CliError::Config(error.to_string()))?; - let removed_reference = remove_dynamic_plugin_reference(&entry.plugins_toml_path, &command.id)?; + let removed_reference = remove_dynamic_plugin_reference( + &entry.plugins_toml_path, + &command.id, + entry.record.source.manifest_ref.as_deref(), + )?; if let Err(error) = scopes[entry.scope_index].save() { let _ = restore_plugins_toml(&entry.plugins_toml_path, original_plugins_toml.as_deref()); return Err(error); diff --git a/crates/cli/src/plugins/lifecycle/render.rs b/crates/cli/src/plugins/lifecycle/render.rs index 061067e2..fa6431aa 100644 --- a/crates/cli/src/plugins/lifecycle/render.rs +++ b/crates/cli/src/plugins/lifecycle/render.rs @@ -123,11 +123,10 @@ pub(super) fn render_inspect( lines.push(format!( "host_config_json: {}", host_config - .map(|plugin| plugin.config.clone()) - .filter(|config| !config.is_empty()) + .map(redacted_host_config_json) + .filter(|config| !config.is_null()) .map(|config| { - serde_json::to_string_pretty(&Value::Object(config)) - .expect("host config serializes") + serde_json::to_string_pretty(&config).expect("host config serializes") }) .unwrap_or_else(|| "".into()) )); @@ -271,3 +270,18 @@ pub(super) fn manifest_kind_label(kind: DynamicPluginKind) -> &'static str { fn bool_label(value: bool) -> &'static str { if value { "true" } else { "false" } } + +pub(super) fn redacted_host_config_json(host_config: &ResolvedDynamicPluginConfig) -> Value { + if host_config.config.is_empty() { + return Value::Null; + } + + Value::Object( + host_config + .config + .keys() + .cloned() + .map(|key| (key, Value::String("".into()))) + .collect(), + ) +} diff --git a/crates/cli/src/plugins/lifecycle/responses.rs b/crates/cli/src/plugins/lifecycle/responses.rs index 46a5b6f8..85ff42b8 100644 --- a/crates/cli/src/plugins/lifecycle/responses.rs +++ b/crates/cli/src/plugins/lifecycle/responses.rs @@ -17,7 +17,9 @@ use serde_json::{Map, Value}; use crate::config::{DynamicPluginHostConfigStatus, ResolvedDynamicPluginConfig}; use crate::error::{CliError, PluginLifecycleFailureKind}; -use super::render::{check_state_label, manifest_kind_label, runtime_state_label}; +use super::render::{ + check_state_label, manifest_kind_label, redacted_host_config_json, runtime_state_label, +}; use super::state::ScopedDynamicPluginRecord; #[derive(Debug)] @@ -206,7 +208,7 @@ pub(super) fn inspect_success( .map(|plugin| host_config_status_label(plugin.host_config_status())) .unwrap_or("missing"), host_config: host_config - .map(|plugin| Value::Object(plugin.config.clone())) + .map(redacted_host_config_json) .unwrap_or(Value::Null), }, ) diff --git a/crates/cli/src/plugins/lifecycle/state.rs b/crates/cli/src/plugins/lifecycle/state.rs index 74a38930..1e2791f2 100644 --- a/crates/cli/src/plugins/lifecycle/state.rs +++ b/crates/cli/src/plugins/lifecycle/state.rs @@ -1,7 +1,9 @@ // SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 +use std::io::Write; use std::path::{Path, PathBuf}; +use std::time::{SystemTime, UNIX_EPOCH}; use nemo_relay::plugin::dynamic::{DynamicPluginRecord, DynamicPluginRegistry}; use serde::{Deserialize, Serialize}; @@ -66,7 +68,7 @@ const fn default_state_schema_version() -> u32 { impl ScopedRegistry { pub(super) fn save(&self) -> Result<(), CliError> { - let rendered = serde_json::to_vec_pretty(&PersistedDynamicPluginRegistry { + let mut rendered = serde_json::to_vec_pretty(&PersistedDynamicPluginRegistry { schema_version: DYNAMIC_PLUGIN_STATE_SCHEMA_VERSION, records: self.registry.cloned_records(true), }) @@ -76,12 +78,38 @@ impl ScopedRegistry { self.state_path.display() )) })?; - if let Some(parent) = self.state_path.parent() { - std::fs::create_dir_all(parent)?; - } - let mut rendered = rendered; rendered.push(b'\n'); - std::fs::write(&self.state_path, rendered)?; + let parent = self + .state_path + .parent() + .map(Path::to_path_buf) + .unwrap_or_else(|| PathBuf::from(".")); + std::fs::create_dir_all(&parent)?; + + let temp_path = parent.join(format!( + ".{}.{}.tmp", + self.state_path + .file_name() + .and_then(|name| name.to_str()) + .unwrap_or("dynamic-plugins"), + SystemTime::now() + .duration_since(UNIX_EPOCH) + .expect("system clock is after unix epoch") + .as_nanos() + )); + + let write_result = (|| -> Result<(), CliError> { + let mut file = std::fs::File::create(&temp_path)?; + file.write_all(&rendered)?; + file.sync_all()?; + std::fs::rename(&temp_path, &self.state_path)?; + Ok(()) + })(); + + if write_result.is_err() { + let _ = std::fs::remove_file(&temp_path); + } + write_result?; Ok(()) } } diff --git a/crates/cli/src/plugins/lifecycle/target.rs b/crates/cli/src/plugins/lifecycle/target.rs index 46ab28b8..0da4e156 100644 --- a/crates/cli/src/plugins/lifecycle/target.rs +++ b/crates/cli/src/plugins/lifecycle/target.rs @@ -37,7 +37,7 @@ fn classify_target_syntax(target: &str) -> TargetSyntax { // while keeping ordinary canonical IDs like `acme.worker` on the ID branch. fn should_treat_target_as_path(target: &str) -> bool { let path = Path::new(target); - if path.exists() || path.is_absolute() { + if path.is_absolute() { return true; } @@ -113,13 +113,19 @@ mod tests { } #[test] - fn parse_treats_existing_filesystem_entries_as_paths() { + fn parse_treats_existing_filesystem_entries_with_explicit_path_syntax_as_paths() { let temp = tempdir().unwrap(); - let existing = temp.path().join("acme.worker"); + let existing = temp.path().join("plugins").join("acme.worker"); std::fs::create_dir_all(&existing).unwrap(); assert_eq!( - PluginTarget::parse(existing.to_str().unwrap()), - PluginTarget::Path(existing) + PluginTarget::parse( + existing + .strip_prefix(temp.path()) + .unwrap() + .to_str() + .unwrap() + ), + PluginTarget::Path(PathBuf::from("plugins/acme.worker")) ); } } diff --git a/crates/cli/tests/coverage/main_tests.rs b/crates/cli/tests/coverage/main_tests.rs index e8d2dec4..437d3987 100644 --- a/crates/cli/tests/coverage/main_tests.rs +++ b/crates/cli/tests/coverage/main_tests.rs @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 use clap::Parser; +use std::ffi::OsString; use super::*; use crate::config::{ @@ -10,6 +11,57 @@ use crate::config::{ PricingValidateCommand, ServerArgs, }; +struct EnvScope { + _guard: std::sync::MutexGuard<'static, ()>, + values: Vec<(&'static str, Option)>, +} + +impl EnvScope { + fn hermetic(temp: &tempfile::TempDir) -> Self { + let xdg = temp.path().join("xdg"); + std::fs::create_dir_all(&xdg).unwrap(); + Self::set(&[ + ("HOME", Some(temp.path().as_os_str())), + ("XDG_CONFIG_HOME", Some(xdg.as_os_str())), + ]) + } + + fn set(values: &[(&'static str, Option<&std::ffi::OsStr>)]) -> Self { + let guard = crate::test_support::ENV_TEST_LOCK + .lock() + .unwrap_or_else(|error| error.into_inner()); + let previous = values + .iter() + .map(|(key, _)| (*key, std::env::var_os(key))) + .collect::>(); + for (key, value) in values { + unsafe { + match value { + Some(value) => std::env::set_var(key, value), + None => std::env::remove_var(key), + } + } + } + Self { + _guard: guard, + values: previous, + } + } +} + +impl Drop for EnvScope { + fn drop(&mut self) { + for (key, value) in self.values.drain(..) { + unsafe { + match value { + Some(value) => std::env::set_var(key, value), + None => std::env::remove_var(key), + } + } + } + } +} + #[test] fn completions_helper_reports_missing_shell_and_generates_requested_shell() { let error = generate_completions_to(None, &mut Vec::new()) @@ -25,6 +77,9 @@ fn completions_helper_reports_missing_shell_and_generates_requested_shell() { #[test] fn safe_dispatch_helpers_cover_completions_and_plugins_paths() { + let temp = tempfile::tempdir().unwrap(); + let _env = EnvScope::hermetic(&temp); + assert_eq!( run_completions(CompletionsCommand { shell: Some(clap_complete::Shell::Bash), @@ -100,6 +155,9 @@ fn safe_dispatch_helpers_cover_completions_and_plugins_paths() { #[test] fn safe_dispatch_plugin_json_errors_return_exit_codes() { + let temp = tempfile::tempdir().unwrap(); + let _env = EnvScope::hermetic(&temp); + assert_eq!( run_plugins( PluginsCommand { diff --git a/crates/cli/tests/coverage/plugins_lifecycle_tests.rs b/crates/cli/tests/coverage/plugins_lifecycle_tests.rs index 8822b567..82dcbcde 100644 --- a/crates/cli/tests/coverage/plugins_lifecycle_tests.rs +++ b/crates/cli/tests/coverage/plugins_lifecycle_tests.rs @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 use std::path::{Path, PathBuf}; +use std::{ffi::OsString, sync::MutexGuard}; use super::*; use crate::config::{ @@ -28,6 +29,57 @@ impl Drop for CurrentDirGuard { } } +struct EnvScope { + _guard: MutexGuard<'static, ()>, + values: Vec<(&'static str, Option)>, +} + +impl EnvScope { + fn hermetic(temp: &tempfile::TempDir) -> Self { + let xdg = temp.path().join("xdg"); + std::fs::create_dir_all(&xdg).unwrap(); + Self::set(&[ + ("HOME", Some(temp.path().as_os_str())), + ("XDG_CONFIG_HOME", Some(xdg.as_os_str())), + ]) + } + + fn set(values: &[(&'static str, Option<&std::ffi::OsStr>)]) -> Self { + let guard = crate::test_support::ENV_TEST_LOCK + .lock() + .unwrap_or_else(|error| error.into_inner()); + let previous = values + .iter() + .map(|(key, _)| (*key, std::env::var_os(key))) + .collect::>(); + for (key, value) in values { + unsafe { + match value { + Some(value) => std::env::set_var(key, value), + None => std::env::remove_var(key), + } + } + } + Self { + _guard: guard, + values: previous, + } + } +} + +impl Drop for EnvScope { + fn drop(&mut self) { + for (key, value) in self.values.drain(..) { + unsafe { + match value { + Some(value) => std::env::set_var(key, value), + None => std::env::remove_var(key), + } + } + } + } +} + fn write_dynamic_manifest(dir: &Path, plugin_id: &str) -> PathBuf { let manifest_path = dir.join("relay-plugin.toml"); std::fs::write( @@ -62,10 +114,8 @@ entrypoint = "{plugin_id}.plugin:register" #[test] fn add_registers_dynamic_plugin_in_project_plugins_toml() { - let _lock = crate::test_support::ENV_TEST_LOCK - .lock() - .unwrap_or_else(|poisoned| poisoned.into_inner()); let temp = tempfile::tempdir().unwrap(); + let _env = EnvScope::hermetic(&temp); let _cwd = CurrentDirGuard::enter(temp.path()); let plugin_dir = temp.path().join("plugins").join("acme"); std::fs::create_dir_all(&plugin_dir).unwrap(); @@ -95,10 +145,8 @@ fn add_registers_dynamic_plugin_in_project_plugins_toml() { #[test] fn add_rejects_duplicate_dynamic_plugin_ids() { - let _lock = crate::test_support::ENV_TEST_LOCK - .lock() - .unwrap_or_else(|poisoned| poisoned.into_inner()); let temp = tempfile::tempdir().unwrap(); + let _env = EnvScope::hermetic(&temp); let _cwd = CurrentDirGuard::enter(temp.path()); let plugin_dir = temp.path().join("plugins").join("acme"); std::fs::create_dir_all(&plugin_dir).unwrap(); @@ -133,10 +181,8 @@ fn add_rejects_duplicate_dynamic_plugin_ids() { #[test] fn list_and_inspect_render_discovered_dynamic_plugins() { - let _lock = crate::test_support::ENV_TEST_LOCK - .lock() - .unwrap_or_else(|poisoned| poisoned.into_inner()); let temp = tempfile::tempdir().unwrap(); + let _env = EnvScope::hermetic(&temp); let _cwd = CurrentDirGuard::enter(temp.path()); let plugin_dir = temp.path().join("plugins").join("acme"); std::fs::create_dir_all(&plugin_dir).unwrap(); @@ -187,10 +233,8 @@ fn list_and_inspect_render_discovered_dynamic_plugins() { #[test] fn validate_renders_summary_for_path_and_id_targets() { - let _lock = crate::test_support::ENV_TEST_LOCK - .lock() - .unwrap_or_else(|poisoned| poisoned.into_inner()); let temp = tempfile::tempdir().unwrap(); + let _env = EnvScope::hermetic(&temp); let _cwd = CurrentDirGuard::enter(temp.path()); let plugin_dir = temp.path().join("plugins").join("acme"); std::fs::create_dir_all(&plugin_dir).unwrap(); @@ -264,10 +308,8 @@ fn validate_renders_summary_for_path_and_id_targets() { #[test] fn enable_disable_and_remove_persist_lifecycle_state() { - let _lock = crate::test_support::ENV_TEST_LOCK - .lock() - .unwrap_or_else(|poisoned| poisoned.into_inner()); let temp = tempfile::tempdir().unwrap(); + let _env = EnvScope::hermetic(&temp); let _cwd = CurrentDirGuard::enter(temp.path()); let plugin_dir = temp.path().join("plugins").join("acme"); std::fs::create_dir_all(&plugin_dir).unwrap(); @@ -354,10 +396,8 @@ fn enable_disable_and_remove_persist_lifecycle_state() { #[test] fn add_with_explicit_config_uses_sibling_plugins_and_state_files() { - let _lock = crate::test_support::ENV_TEST_LOCK - .lock() - .unwrap_or_else(|poisoned| poisoned.into_inner()); let temp = tempfile::tempdir().unwrap(); + let _env = EnvScope::hermetic(&temp); let plugin_dir = temp.path().join("plugins").join("acme"); let config_dir = temp.path().join("custom-config"); std::fs::create_dir_all(&plugin_dir).unwrap(); @@ -398,10 +438,8 @@ fn add_with_explicit_config_uses_sibling_plugins_and_state_files() { #[test] fn hydrate_bootstraps_registry_records_from_existing_dynamic_plugin_refs() { - let _lock = crate::test_support::ENV_TEST_LOCK - .lock() - .unwrap_or_else(|poisoned| poisoned.into_inner()); let temp = tempfile::tempdir().unwrap(); + let _env = EnvScope::hermetic(&temp); let _cwd = CurrentDirGuard::enter(temp.path()); let plugin_dir = temp.path().join("plugins").join("acme"); let config_dir = temp.path().join(".nemo-relay"); @@ -438,10 +476,8 @@ fn hydrate_bootstraps_registry_records_from_existing_dynamic_plugin_refs() { #[test] fn add_can_revive_tombstoned_records() { - let _lock = crate::test_support::ENV_TEST_LOCK - .lock() - .unwrap_or_else(|poisoned| poisoned.into_inner()); let temp = tempfile::tempdir().unwrap(); + let _env = EnvScope::hermetic(&temp); let _cwd = CurrentDirGuard::enter(temp.path()); let plugin_dir = temp.path().join("plugins").join("acme"); std::fs::create_dir_all(&plugin_dir).unwrap(); @@ -491,10 +527,8 @@ fn add_can_revive_tombstoned_records() { #[test] fn json_helpers_emit_stable_success_and_failure_shapes() { - let _lock = crate::test_support::ENV_TEST_LOCK - .lock() - .unwrap_or_else(|poisoned| poisoned.into_inner()); let temp = tempfile::tempdir().unwrap(); + let _env = EnvScope::hermetic(&temp); let _cwd = CurrentDirGuard::enter(temp.path()); let plugin_dir = temp.path().join("plugins").join("acme"); std::fs::create_dir_all(&plugin_dir).unwrap(); @@ -549,6 +583,10 @@ fn json_helpers_emit_stable_success_and_failure_shapes() { inspect_value["data"]["source"]["manifest_ref"], serde_json::json!(manifest_ref) ); + assert_eq!( + inspect_value["data"]["host_config"], + serde_json::Value::Null + ); let validate_value = serde_json::to_value(responses::validate_success( responses::ValidateResponseInput { @@ -579,3 +617,141 @@ fn json_helpers_emit_stable_success_and_failure_shapes() { assert_eq!(failure["ok"], serde_json::json!(false)); assert_eq!(failure["error"]["code"], serde_json::json!("not_found")); } + +#[test] +fn remove_tolerates_unreadable_non_target_manifest_entries() { + let temp = tempfile::tempdir().unwrap(); + let _env = EnvScope::hermetic(&temp); + let _cwd = CurrentDirGuard::enter(temp.path()); + let plugin_dir = temp.path().join("plugins").join("acme"); + let broken_dir = temp.path().join("plugins").join("broken"); + std::fs::create_dir_all(&plugin_dir).unwrap(); + std::fs::create_dir_all(&broken_dir).unwrap(); + let manifest_path = write_dynamic_manifest(&plugin_dir, "acme.guardrail"); + let server = ServerArgs::default(); + + add( + PluginsAddCommand { + scope: PluginsScopeArgs { + project: true, + ..PluginsScopeArgs::default() + }, + path: plugin_dir, + }, + &server, + ) + .unwrap(); + + let plugins_toml = temp.path().join(".nemo-relay").join("plugins.toml"); + std::fs::write( + &plugins_toml, + format!( + "[[plugins.dynamic]]\nmanifest = {:?}\n\n[[plugins.dynamic]]\nmanifest = {:?}\n", + manifest_path.to_string_lossy(), + broken_dir.join("missing.toml").to_string_lossy() + ), + ) + .unwrap(); + + remove( + PluginsRemoveCommand { + id: "acme.guardrail".into(), + }, + &server, + ) + .unwrap(); + + let rendered = std::fs::read_to_string(&plugins_toml).unwrap(); + assert!(!rendered.contains("acme.guardrail")); + assert!(rendered.contains("missing.toml")); +} + +#[test] +fn remove_reports_malformed_dynamic_plugin_containers() { + let temp = tempfile::tempdir().unwrap(); + let _env = EnvScope::hermetic(&temp); + let plugins_toml = temp.path().join("plugins.toml"); + + std::fs::write(&plugins_toml, "[plugins]\ndynamic = \"oops\"\n").unwrap(); + let error = remove_dynamic_plugin_reference(&plugins_toml, "acme.guardrail", None) + .unwrap_err() + .to_string(); + assert!(error.contains("plugins.dynamic must be an array of tables")); + + std::fs::write(&plugins_toml, "plugins = \"oops\"\n").unwrap(); + let error = remove_dynamic_plugin_reference(&plugins_toml, "acme.guardrail", None) + .unwrap_err() + .to_string(); + assert!(error.contains("[plugins] must be a table")); +} + +#[test] +fn inspect_redacts_host_config_values() { + let temp = tempfile::tempdir().unwrap(); + let _env = EnvScope::hermetic(&temp); + let _cwd = CurrentDirGuard::enter(temp.path()); + let plugin_dir = temp.path().join("plugins").join("acme"); + std::fs::create_dir_all(&plugin_dir).unwrap(); + let manifest_path = write_dynamic_manifest(&plugin_dir, "acme.redacted"); + let server = ServerArgs::default(); + + add( + PluginsAddCommand { + scope: PluginsScopeArgs { + project: true, + ..PluginsScopeArgs::default() + }, + path: plugin_dir, + }, + &server, + ) + .unwrap(); + + let plugins_toml = temp.path().join(".nemo-relay").join("plugins.toml"); + std::fs::write( + &plugins_toml, + format!( + "[[plugins.dynamic]]\nmanifest = {:?}\nconfig = {{ api_key = \"secret-token\", region = \"us-west-2\" }}\n", + manifest_path.to_string_lossy() + ), + ) + .unwrap(); + + let resolved = resolve_plugins_config(None).unwrap(); + let host_config_by_id = host_config_by_id(&resolved); + let scopes = load_and_hydrate_scopes(None, &resolved).unwrap(); + let entry = find_record_by_id(&scopes, "acme.redacted") + .unwrap() + .expect("redacted record"); + let (manifest, manifest_ref) = DynamicPluginManifest::load_from_path(&manifest_path) + .map_err(|error| CliError::Config(error.to_string())) + .unwrap(); + + let inspect_output = render_inspect( + &entry, + &manifest, + &manifest_ref, + host_config_by_id.get("acme.redacted"), + ); + assert!(!inspect_output.contains("secret-token")); + assert!(inspect_output.contains("")); + + let inspect_value = serde_json::to_value(responses::inspect_success( + "plugins inspect", + "acme.redacted", + &entry, + &manifest, + &manifest_ref, + host_config_by_id.get("acme.redacted"), + )) + .unwrap(); + assert_eq!( + inspect_value["data"]["host_config"]["api_key"], + serde_json::json!("") + ); + assert_eq!( + inspect_value["data"]["host_config"]["region"], + serde_json::json!("") + ); + assert_eq!(inspect_value["data"]["host_config_status"], "present"); +} From a55a87af1201f24a255a2c40e765e1bae0ad6006 Mon Sep 17 00:00:00 2001 From: Alex Fournier Date: Tue, 23 Jun 2026 18:53:51 -0700 Subject: [PATCH 07/20] refactor: tighten dynamic plugin lifecycle text rendering Signed-off-by: Alex Fournier --- crates/cli/src/plugins/lifecycle.rs | 20 ++++++---- crates/cli/src/plugins/lifecycle/render.rs | 44 +++++++++++++++------- 2 files changed, 42 insertions(+), 22 deletions(-) diff --git a/crates/cli/src/plugins/lifecycle.rs b/crates/cli/src/plugins/lifecycle.rs index c95bdd61..38ea45be 100644 --- a/crates/cli/src/plugins/lifecycle.rs +++ b/crates/cli/src/plugins/lifecycle.rs @@ -26,7 +26,7 @@ mod responses; mod state; mod target; -use self::render::{render_inspect, render_list, render_validation_summary}; +use self::render::{render_add_result, render_inspect, render_list, render_validation_summary}; use self::responses::{ ValidateResponseInput, failure, generic_failure, inspect_success, list_success, print_response_json, validate_success, @@ -77,13 +77,17 @@ pub(crate) fn add(command: PluginsAddCommand, server: &ServerArgs) -> Result<(), return Err(error); } - println!("Added dynamic plugin {}", plugin_id); - println!("scope: {}", scope.label()); - println!("manifest: {}", manifest_ref); - println!("plugins_toml: {}", plugins_toml_path.display()); - println!("state_path: {}", scopes[scope_index].state_path.display()); - println!("revived: {}", if revived { "true" } else { "false" }); - println!("desired.enabled: false"); + println!( + "{}", + render_add_result( + &plugin_id, + scope.label(), + &manifest_ref, + &plugins_toml_path, + &scopes[scope_index].state_path, + revived, + ) + ); Ok(()) } diff --git a/crates/cli/src/plugins/lifecycle/render.rs b/crates/cli/src/plugins/lifecycle/render.rs index fa6431aa..bfcdabe5 100644 --- a/crates/cli/src/plugins/lifecycle/render.rs +++ b/crates/cli/src/plugins/lifecycle/render.rs @@ -19,17 +19,20 @@ pub(super) fn render_list( host_config_by_id: &HashMap, ) -> String { let mut lines = Vec::with_capacity(records.len() + 1); - lines.push("ID\tSCOPE\tENABLED\tSTATE\tVALIDATION\tHOST CONFIG".into()); + lines.push(format!( + "{:<32} {:<8} {:<7} {:<10} {:<10} {}", + "ID", "SCOPE", "ENABLED", "STATE", "VALIDATION", "HOST CONFIG" + )); for entry in records { let host_config_status = host_config_by_id .get(&entry.record.metadata.id) .map(|plugin| host_config_status_label(plugin.host_config_status())) .unwrap_or("missing"); lines.push(format!( - "{}\t{}\t{}\t{}\t{}\t{}", + "{:<32} {:<8} {:<7} {:<10} {:<10} {}", entry.record.metadata.id, entry.scope.label(), - bool_label(entry.record.spec.enabled), + entry.record.spec.enabled, lifecycle_state_label(&entry.record), check_state_label(entry.record.status.validation.manifest), host_config_status @@ -38,6 +41,26 @@ pub(super) fn render_list( lines.join("\n") } +pub(super) fn render_add_result( + plugin_id: &str, + scope_label: &str, + manifest_ref: &str, + plugins_toml_path: &std::path::Path, + state_path: &std::path::Path, + revived: bool, +) -> String { + [ + format!("Added dynamic plugin {plugin_id}"), + format!("scope: {scope_label}"), + format!("manifest: {manifest_ref}"), + format!("plugins_toml: {}", plugins_toml_path.display()), + format!("state_path: {}", state_path.display()), + format!("revived: {revived}"), + "desired.enabled: false".into(), + ] + .join("\n") +} + pub(super) fn render_inspect( entry: &ScopedDynamicPluginRecord, manifest: &DynamicPluginManifest, @@ -72,10 +95,10 @@ pub(super) fn render_inspect( "source.environment_ref: {}", record.source.environment_ref.as_deref().unwrap_or("") ), - format!("desired.present: {}", bool_label(record.spec.present)), - format!("desired.enabled: {}", bool_label(record.spec.enabled)), + format!("desired.present: {}", record.spec.present), + format!("desired.enabled: {}", record.spec.enabled), format!("generation: {}", record.metadata.generation), - format!("reconciled: {}", bool_label(record.is_reconciled())), + format!("reconciled: {}", record.is_reconciled()), format!( "host_config: {}", host_config @@ -147,10 +170,7 @@ pub(super) fn render_validation_summary( if let Some(entry) = entry { lines.push(format!("scope: {}", entry.scope.label())); lines.push(format!("state_path: {}", entry.state_path.display())); - lines.push(format!( - "desired.enabled: {}", - bool_label(entry.record.spec.enabled) - )); + lines.push(format!("desired.enabled: {}", entry.record.spec.enabled)); lines.push(format!( "host_config: {}", host_config @@ -267,10 +287,6 @@ pub(super) fn manifest_kind_label(kind: DynamicPluginKind) -> &'static str { } } -fn bool_label(value: bool) -> &'static str { - if value { "true" } else { "false" } -} - pub(super) fn redacted_host_config_json(host_config: &ResolvedDynamicPluginConfig) -> Value { if host_config.config.is_empty() { return Value::Null; From cd8c682cebb5a920bbb5b0e73570e26632bb9805 Mon Sep 17 00:00:00 2001 From: Alex Fournier Date: Tue, 23 Jun 2026 18:54:40 -0700 Subject: [PATCH 08/20] refactor: reuse shared plugins toml constant Signed-off-by: Alex Fournier --- crates/cli/src/config.rs | 2 +- crates/cli/src/plugins/lifecycle/state.rs | 9 +++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/crates/cli/src/config.rs b/crates/cli/src/config.rs index a5bb43e8..c489cd4c 100644 --- a/crates/cli/src/config.rs +++ b/crates/cli/src/config.rs @@ -830,7 +830,7 @@ fn apply_server_overrides(config: &mut GatewayConfig, args: &ServerArgs) -> Resu Ok(()) } -const PLUGINS_TOML: &str = "plugins.toml"; +pub(crate) const PLUGINS_TOML: &str = "plugins.toml"; // Loads config from the ordered shared locations, deep-merges TOML tables, maps the typed file // shape onto runtime structs, applies a sibling/discovered plugins.toml when present, then lets diff --git a/crates/cli/src/plugins/lifecycle/state.rs b/crates/cli/src/plugins/lifecycle/state.rs index 1e2791f2..60e6a961 100644 --- a/crates/cli/src/plugins/lifecycle/state.rs +++ b/crates/cli/src/plugins/lifecycle/state.rs @@ -9,7 +9,8 @@ use nemo_relay::plugin::dynamic::{DynamicPluginRecord, DynamicPluginRegistry}; use serde::{Deserialize, Serialize}; use crate::config::{ - global_plugin_config_path, project_plugin_config_path, user_config_dir, user_plugin_config_path, + PLUGINS_TOML, global_plugin_config_path, project_plugin_config_path, user_config_dir, + user_plugin_config_path, }; use crate::error::CliError; @@ -142,7 +143,7 @@ pub(super) fn scoped_paths_for_add( )) })?; return Ok(( - parent.join("plugins.toml"), + parent.join(PLUGINS_TOML), parent.join(DYNAMIC_PLUGIN_STATE_FILENAME), RegistryScope::Explicit, )); @@ -243,7 +244,7 @@ fn scoped_registry_layouts( explicit.display() )) })?; - let plugins_toml_path = parent.join("plugins.toml"); + let plugins_toml_path = parent.join(PLUGINS_TOML); return Ok(vec![( RegistryScope::Explicit, plugins_toml_path.clone(), @@ -265,7 +266,7 @@ fn scoped_registry_layouts( )); } if let Some(user_dir) = user_config_dir() { - let plugins_toml_path = user_dir.join("plugins.toml"); + let plugins_toml_path = user_dir.join(PLUGINS_TOML); layouts.push(( RegistryScope::User, plugins_toml_path.clone(), From cf6ddf74a2f2389f6057bc56d222e8f6b1262b61 Mon Sep 17 00:00:00 2001 From: Alex Fournier Date: Tue, 23 Jun 2026 18:55:24 -0700 Subject: [PATCH 09/20] refactor: hide dynamic plugin lifecycle state file Signed-off-by: Alex Fournier --- crates/cli/src/plugins/lifecycle.rs | 4 ++-- crates/cli/src/plugins/lifecycle/render.rs | 9 ++++++--- crates/cli/src/plugins/lifecycle/state.rs | 3 ++- crates/cli/tests/coverage/plugins_lifecycle_tests.rs | 2 +- 4 files changed, 11 insertions(+), 7 deletions(-) diff --git a/crates/cli/src/plugins/lifecycle.rs b/crates/cli/src/plugins/lifecycle.rs index 38ea45be..de54519c 100644 --- a/crates/cli/src/plugins/lifecycle.rs +++ b/crates/cli/src/plugins/lifecycle.rs @@ -279,7 +279,7 @@ pub(crate) fn remove(command: PluginsRemoveCommand, server: &ServerArgs) -> Resu "".into() } ); - println!("state_path: {}", entry.state_path.display()); + println!("lifecycle_state_path: {}", entry.state_path.display()); println!("status: tombstoned"); Ok(()) } @@ -327,7 +327,7 @@ fn mutate_enabled_state( plugin_id ); println!("scope: {}", entry.scope.label()); - println!("state_path: {}", entry.state_path.display()); + println!("lifecycle_state_path: {}", entry.state_path.display()); Ok(()) } diff --git a/crates/cli/src/plugins/lifecycle/render.rs b/crates/cli/src/plugins/lifecycle/render.rs index bfcdabe5..53da16df 100644 --- a/crates/cli/src/plugins/lifecycle/render.rs +++ b/crates/cli/src/plugins/lifecycle/render.rs @@ -54,7 +54,7 @@ pub(super) fn render_add_result( format!("scope: {scope_label}"), format!("manifest: {manifest_ref}"), format!("plugins_toml: {}", plugins_toml_path.display()), - format!("state_path: {}", state_path.display()), + format!("lifecycle_state_path: {}", state_path.display()), format!("revived: {revived}"), "desired.enabled: false".into(), ] @@ -82,7 +82,7 @@ pub(super) fn render_inspect( ), format!("manifest: {manifest_ref}"), format!("plugins_toml: {}", entry.plugins_toml_path.display()), - format!("state_path: {}", entry.state_path.display()), + format!("lifecycle_state_path: {}", entry.state_path.display()), format!( "source.manifest_ref: {}", record.source.manifest_ref.as_deref().unwrap_or("") @@ -169,7 +169,10 @@ pub(super) fn render_validation_summary( ]; if let Some(entry) = entry { lines.push(format!("scope: {}", entry.scope.label())); - lines.push(format!("state_path: {}", entry.state_path.display())); + lines.push(format!( + "lifecycle_state_path: {}", + entry.state_path.display() + )); lines.push(format!("desired.enabled: {}", entry.record.spec.enabled)); lines.push(format!( "host_config: {}", diff --git a/crates/cli/src/plugins/lifecycle/state.rs b/crates/cli/src/plugins/lifecycle/state.rs index 60e6a961..f26d85bd 100644 --- a/crates/cli/src/plugins/lifecycle/state.rs +++ b/crates/cli/src/plugins/lifecycle/state.rs @@ -16,7 +16,8 @@ use crate::error::CliError; use super::super::config_io::TargetScope; -const DYNAMIC_PLUGIN_STATE_FILENAME: &str = "dynamic-plugins.json"; +// Internal CLI-managed lifecycle state. This file is not intended to be user-edited. +const DYNAMIC_PLUGIN_STATE_FILENAME: &str = ".dynamic-plugins.json"; const DYNAMIC_PLUGIN_STATE_SCHEMA_VERSION: u32 = 1; #[derive(Debug, Clone, Copy, PartialEq, Eq)] diff --git a/crates/cli/tests/coverage/plugins_lifecycle_tests.rs b/crates/cli/tests/coverage/plugins_lifecycle_tests.rs index 82dcbcde..4bd711b2 100644 --- a/crates/cli/tests/coverage/plugins_lifecycle_tests.rs +++ b/crates/cli/tests/coverage/plugins_lifecycle_tests.rs @@ -419,7 +419,7 @@ fn add_with_explicit_config_uses_sibling_plugins_and_state_files() { .unwrap(); let plugins_toml = config_dir.join("plugins.toml"); - let state_path = config_dir.join("dynamic-plugins.json"); + let state_path = config_dir.join(".dynamic-plugins.json"); assert!(plugins_toml.exists()); assert!(state_path.exists()); From eaf621de4e80bf0d62b67e6ea53587ac78870062 Mon Sep 17 00:00:00 2001 From: Alex Fournier Date: Tue, 23 Jun 2026 19:05:03 -0700 Subject: [PATCH 10/20] refactor: trim lifecycle mutation command output Signed-off-by: Alex Fournier --- crates/cli/src/plugins/lifecycle.rs | 30 ++++------------------ crates/cli/src/plugins/lifecycle/render.rs | 20 --------------- 2 files changed, 5 insertions(+), 45 deletions(-) diff --git a/crates/cli/src/plugins/lifecycle.rs b/crates/cli/src/plugins/lifecycle.rs index de54519c..50467cf9 100644 --- a/crates/cli/src/plugins/lifecycle.rs +++ b/crates/cli/src/plugins/lifecycle.rs @@ -26,7 +26,7 @@ mod responses; mod state; mod target; -use self::render::{render_add_result, render_inspect, render_list, render_validation_summary}; +use self::render::{render_inspect, render_list, render_validation_summary}; use self::responses::{ ValidateResponseInput, failure, generic_failure, inspect_success, list_success, print_response_json, validate_success, @@ -77,17 +77,8 @@ pub(crate) fn add(command: PluginsAddCommand, server: &ServerArgs) -> Result<(), return Err(error); } - println!( - "{}", - render_add_result( - &plugin_id, - scope.label(), - &manifest_ref, - &plugins_toml_path, - &scopes[scope_index].state_path, - revived, - ) - ); + let _ = (scope, manifest_ref, plugins_toml_path, revived); + println!("Added dynamic plugin {}", plugin_id); Ok(()) } @@ -269,18 +260,8 @@ pub(crate) fn remove(command: PluginsRemoveCommand, server: &ServerArgs) -> Resu return Err(error); } + let _ = (entry, removed_reference); println!("Removed dynamic plugin {}", command.id); - println!("scope: {}", entry.scope.label()); - println!( - "plugins_toml: {}", - if removed_reference { - entry.plugins_toml_path.display().to_string() - } else { - "".into() - } - ); - println!("lifecycle_state_path: {}", entry.state_path.display()); - println!("status: tombstoned"); Ok(()) } @@ -321,13 +302,12 @@ fn mutate_enabled_state( } scopes[entry.scope_index].save()?; + let _ = entry; println!( "{} dynamic plugin {}", if enabled { "Enabled" } else { "Disabled" }, plugin_id ); - println!("scope: {}", entry.scope.label()); - println!("lifecycle_state_path: {}", entry.state_path.display()); Ok(()) } diff --git a/crates/cli/src/plugins/lifecycle/render.rs b/crates/cli/src/plugins/lifecycle/render.rs index 53da16df..178ea019 100644 --- a/crates/cli/src/plugins/lifecycle/render.rs +++ b/crates/cli/src/plugins/lifecycle/render.rs @@ -41,26 +41,6 @@ pub(super) fn render_list( lines.join("\n") } -pub(super) fn render_add_result( - plugin_id: &str, - scope_label: &str, - manifest_ref: &str, - plugins_toml_path: &std::path::Path, - state_path: &std::path::Path, - revived: bool, -) -> String { - [ - format!("Added dynamic plugin {plugin_id}"), - format!("scope: {scope_label}"), - format!("manifest: {manifest_ref}"), - format!("plugins_toml: {}", plugins_toml_path.display()), - format!("lifecycle_state_path: {}", state_path.display()), - format!("revived: {revived}"), - "desired.enabled: false".into(), - ] - .join("\n") -} - pub(super) fn render_inspect( entry: &ScopedDynamicPluginRecord, manifest: &DynamicPluginManifest, From b73b13ee86c87156dffe896c1c1496f62a651b2a Mon Sep 17 00:00:00 2001 From: Alex Fournier Date: Tue, 23 Jun 2026 19:12:53 -0700 Subject: [PATCH 11/20] fix: resolve lifecycle target manifest refs symmetrically Signed-off-by: Alex Fournier --- crates/cli/src/plugins/config_io.rs | 3 +- .../tests/coverage/plugins_lifecycle_tests.rs | 43 +++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/crates/cli/src/plugins/config_io.rs b/crates/cli/src/plugins/config_io.rs index 37e6bc1b..1e8a7da7 100644 --- a/crates/cli/src/plugins/config_io.rs +++ b/crates/cli/src/plugins/config_io.rs @@ -184,7 +184,8 @@ pub(crate) fn remove_dynamic_plugin_reference( let original_len = dynamic_entries.len(); let mut retained = Vec::with_capacity(original_len); - let target_manifest_ref = target_manifest_ref.map(PathBuf::from); + let target_manifest_ref = + target_manifest_ref.map(|manifest_ref| resolve_manifest_ref(path, manifest_ref)); for entry in dynamic_entries.drain(..) { let manifest_ref = entry .as_table() diff --git a/crates/cli/tests/coverage/plugins_lifecycle_tests.rs b/crates/cli/tests/coverage/plugins_lifecycle_tests.rs index 4bd711b2..d279749d 100644 --- a/crates/cli/tests/coverage/plugins_lifecycle_tests.rs +++ b/crates/cli/tests/coverage/plugins_lifecycle_tests.rs @@ -685,6 +685,49 @@ fn remove_reports_malformed_dynamic_plugin_containers() { assert!(error.contains("[plugins] must be a table")); } +#[test] +fn remove_matches_relative_target_manifest_refs_without_loading_manifest() { + let temp = tempfile::tempdir().unwrap(); + let _env = EnvScope::hermetic(&temp); + let _cwd = CurrentDirGuard::enter(temp.path()); + let config_dir = temp.path().join(".nemo-relay"); + let plugin_dir = temp.path().join("plugins").join("acme"); + std::fs::create_dir_all(&config_dir).unwrap(); + std::fs::create_dir_all(&plugin_dir).unwrap(); + + let manifest_path = plugin_dir.join("relay-plugin.toml"); + std::fs::write( + &manifest_path, + r#" +manifest_version = 1 + +[plugin] +id = "acme.guardrail" +kind = "worker" +"#, + ) + .unwrap(); + + let plugins_toml = config_dir.join("plugins.toml"); + std::fs::write( + &plugins_toml, + "[[plugins.dynamic]]\nmanifest = \"../plugins/acme/relay-plugin.toml\"\n", + ) + .unwrap(); + + std::fs::remove_file(&manifest_path).unwrap(); + + let removed = remove_dynamic_plugin_reference( + &plugins_toml, + "acme.guardrail", + Some("../plugins/acme/relay-plugin.toml"), + ) + .unwrap(); + assert!(removed); + let rendered = std::fs::read_to_string(&plugins_toml).unwrap(); + assert!(!rendered.contains("relay-plugin.toml")); +} + #[test] fn inspect_redacts_host_config_values() { let temp = tempfile::tempdir().unwrap(); From 3757b24213379a164d5a40efee0c33fbf39da4d2 Mon Sep 17 00:00:00 2001 From: Alex Fournier Date: Tue, 23 Jun 2026 19:30:54 -0700 Subject: [PATCH 12/20] fix: simplify lifecycle mutation output handling Signed-off-by: Alex Fournier --- crates/cli/src/plugins/lifecycle.rs | 11 ++-- crates/cli/tests/cli_tests.rs | 96 +++++++++++++++++++++++++++++ 2 files changed, 102 insertions(+), 5 deletions(-) diff --git a/crates/cli/src/plugins/lifecycle.rs b/crates/cli/src/plugins/lifecycle.rs index 50467cf9..f92320bf 100644 --- a/crates/cli/src/plugins/lifecycle.rs +++ b/crates/cli/src/plugins/lifecycle.rs @@ -77,8 +77,11 @@ pub(crate) fn add(command: PluginsAddCommand, server: &ServerArgs) -> Result<(), return Err(error); } - let _ = (scope, manifest_ref, plugins_toml_path, revived); - println!("Added dynamic plugin {}", plugin_id); + println!( + "{} dynamic plugin {}", + if revived { "Revived" } else { "Added" }, + plugin_id + ); Ok(()) } @@ -250,7 +253,7 @@ pub(crate) fn remove(command: PluginsRemoveCommand, server: &ServerArgs) -> Resu .registry .remove(&command.id) .map_err(|error| CliError::Config(error.to_string()))?; - let removed_reference = remove_dynamic_plugin_reference( + remove_dynamic_plugin_reference( &entry.plugins_toml_path, &command.id, entry.record.source.manifest_ref.as_deref(), @@ -260,7 +263,6 @@ pub(crate) fn remove(command: PluginsRemoveCommand, server: &ServerArgs) -> Resu return Err(error); } - let _ = (entry, removed_reference); println!("Removed dynamic plugin {}", command.id); Ok(()) } @@ -302,7 +304,6 @@ fn mutate_enabled_state( } scopes[entry.scope_index].save()?; - let _ = entry; println!( "{} dynamic plugin {}", if enabled { "Enabled" } else { "Disabled" }, diff --git a/crates/cli/tests/cli_tests.rs b/crates/cli/tests/cli_tests.rs index 7ea1e8ba..6910029c 100644 --- a/crates/cli/tests/cli_tests.rs +++ b/crates/cli/tests/cli_tests.rs @@ -299,6 +299,102 @@ fn cli_plugins_inspect_json_emits_installed_plugin_details() { assert!(parsed["data"]["source"]["manifest_ref"].is_string()); } +#[test] +fn cli_plugins_mutation_commands_emit_terse_confirmation_output() { + let temp = tempfile::tempdir().unwrap(); + let cwd = temp.path().join("workdir"); + let plugin_dir = cwd.join("plugins").join("acme"); + std::fs::create_dir_all(&cwd).unwrap(); + write_dynamic_plugin_manifest(&plugin_dir, "acme.mutate-output"); + + let add = Command::new(gateway_bin()) + .current_dir(&cwd) + .env("XDG_CONFIG_HOME", temp.path().join("xdg")) + .env("HOME", temp.path()) + .args(["plugins", "add", "--project"]) + .arg(&plugin_dir) + .output() + .unwrap(); + assert!( + add.status.success(), + "stderr was:\n{}", + String::from_utf8_lossy(&add.stderr) + ); + assert_eq!( + String::from_utf8_lossy(&add.stdout).trim(), + "Added dynamic plugin acme.mutate-output" + ); + + let enable = Command::new(gateway_bin()) + .current_dir(&cwd) + .env("XDG_CONFIG_HOME", temp.path().join("xdg")) + .env("HOME", temp.path()) + .args(["plugins", "enable", "acme.mutate-output"]) + .output() + .unwrap(); + assert!( + enable.status.success(), + "stderr was:\n{}", + String::from_utf8_lossy(&enable.stderr) + ); + assert_eq!( + String::from_utf8_lossy(&enable.stdout).trim(), + "Enabled dynamic plugin acme.mutate-output" + ); + + let disable = Command::new(gateway_bin()) + .current_dir(&cwd) + .env("XDG_CONFIG_HOME", temp.path().join("xdg")) + .env("HOME", temp.path()) + .args(["plugins", "disable", "acme.mutate-output"]) + .output() + .unwrap(); + assert!( + disable.status.success(), + "stderr was:\n{}", + String::from_utf8_lossy(&disable.stderr) + ); + assert_eq!( + String::from_utf8_lossy(&disable.stdout).trim(), + "Disabled dynamic plugin acme.mutate-output" + ); + + let remove = Command::new(gateway_bin()) + .current_dir(&cwd) + .env("XDG_CONFIG_HOME", temp.path().join("xdg")) + .env("HOME", temp.path()) + .args(["plugins", "remove", "acme.mutate-output"]) + .output() + .unwrap(); + assert!( + remove.status.success(), + "stderr was:\n{}", + String::from_utf8_lossy(&remove.stderr) + ); + assert_eq!( + String::from_utf8_lossy(&remove.stdout).trim(), + "Removed dynamic plugin acme.mutate-output" + ); + + let revive = Command::new(gateway_bin()) + .current_dir(&cwd) + .env("XDG_CONFIG_HOME", temp.path().join("xdg")) + .env("HOME", temp.path()) + .args(["plugins", "add", "--project"]) + .arg(&plugin_dir) + .output() + .unwrap(); + assert!( + revive.status.success(), + "stderr was:\n{}", + String::from_utf8_lossy(&revive.stderr) + ); + assert_eq!( + String::from_utf8_lossy(&revive.stdout).trim(), + "Revived dynamic plugin acme.mutate-output" + ); +} + #[test] fn cli_completions_prints_script_for_requested_shell() { let output = Command::new(gateway_bin()) From eebfff89b6e43e3b37e770309dffc74ab3e425d7 Mon Sep 17 00:00:00 2001 From: Alex Fournier Date: Tue, 23 Jun 2026 19:36:54 -0700 Subject: [PATCH 13/20] test: raise lifecycle CLI patch coverage Signed-off-by: Alex Fournier --- crates/cli/tests/cli_tests.rs | 50 +++++++++++++++++++ .../tests/coverage/plugins_lifecycle_tests.rs | 49 ++++++++++++++++++ 2 files changed, 99 insertions(+) diff --git a/crates/cli/tests/cli_tests.rs b/crates/cli/tests/cli_tests.rs index 6910029c..934f795d 100644 --- a/crates/cli/tests/cli_tests.rs +++ b/crates/cli/tests/cli_tests.rs @@ -395,6 +395,56 @@ fn cli_plugins_mutation_commands_emit_terse_confirmation_output() { ); } +#[test] +fn cli_plugins_enable_tombstoned_plugin_returns_refused_exit_code() { + let temp = tempfile::tempdir().unwrap(); + let cwd = temp.path().join("workdir"); + let plugin_dir = cwd.join("plugins").join("acme"); + std::fs::create_dir_all(&cwd).unwrap(); + write_dynamic_plugin_manifest(&plugin_dir, "acme.tombstone-enable"); + + let add = Command::new(gateway_bin()) + .current_dir(&cwd) + .env("XDG_CONFIG_HOME", temp.path().join("xdg")) + .env("HOME", temp.path()) + .args(["plugins", "add", "--project"]) + .arg(&plugin_dir) + .output() + .unwrap(); + assert!( + add.status.success(), + "stderr was:\n{}", + String::from_utf8_lossy(&add.stderr) + ); + + let remove = Command::new(gateway_bin()) + .current_dir(&cwd) + .env("XDG_CONFIG_HOME", temp.path().join("xdg")) + .env("HOME", temp.path()) + .args(["plugins", "remove", "acme.tombstone-enable"]) + .output() + .unwrap(); + assert!( + remove.status.success(), + "stderr was:\n{}", + String::from_utf8_lossy(&remove.stderr) + ); + + let enable = Command::new(gateway_bin()) + .current_dir(&cwd) + .env("XDG_CONFIG_HOME", temp.path().join("xdg")) + .env("HOME", temp.path()) + .args(["plugins", "enable", "acme.tombstone-enable"]) + .output() + .unwrap(); + assert_eq!(enable.status.code(), Some(3)); + assert!( + String::from_utf8_lossy(&enable.stderr).contains("tombstoned"), + "stderr was:\n{}", + String::from_utf8_lossy(&enable.stderr) + ); +} + #[test] fn cli_completions_prints_script_for_requested_shell() { let output = Command::new(gateway_bin()) diff --git a/crates/cli/tests/coverage/plugins_lifecycle_tests.rs b/crates/cli/tests/coverage/plugins_lifecycle_tests.rs index d279749d..6661b8d8 100644 --- a/crates/cli/tests/coverage/plugins_lifecycle_tests.rs +++ b/crates/cli/tests/coverage/plugins_lifecycle_tests.rs @@ -179,6 +179,36 @@ fn add_rejects_duplicate_dynamic_plugin_ids() { assert!(error.contains("already registered")); } +#[test] +fn add_rejects_scope_flags_when_explicit_config_is_set() { + let temp = tempfile::tempdir().unwrap(); + let _env = EnvScope::hermetic(&temp); + let plugin_dir = temp.path().join("plugins").join("acme"); + let config_dir = temp.path().join("custom-config"); + std::fs::create_dir_all(&plugin_dir).unwrap(); + std::fs::create_dir_all(&config_dir).unwrap(); + write_dynamic_manifest(&plugin_dir, "acme.explicit-conflict"); + + let server = ServerArgs { + config: Some(config_dir.join("gateway.toml")), + ..ServerArgs::default() + }; + + let error = add( + PluginsAddCommand { + scope: PluginsScopeArgs { + project: true, + ..PluginsScopeArgs::default() + }, + path: plugin_dir, + }, + &server, + ) + .unwrap_err() + .to_string(); + assert!(error.contains("--config cannot be combined")); +} + #[test] fn list_and_inspect_render_discovered_dynamic_plugins() { let temp = tempfile::tempdir().unwrap(); @@ -685,6 +715,25 @@ fn remove_reports_malformed_dynamic_plugin_containers() { assert!(error.contains("[plugins] must be a table")); } +#[test] +fn append_reports_malformed_dynamic_plugin_containers() { + let temp = tempfile::tempdir().unwrap(); + let _env = EnvScope::hermetic(&temp); + let plugins_toml = temp.path().join("plugins.toml"); + + std::fs::write(&plugins_toml, "plugins = \"oops\"\n").unwrap(); + let error = append_dynamic_plugin_reference(&plugins_toml, "/tmp/plugin/relay-plugin.toml") + .unwrap_err() + .to_string(); + assert!(error.contains("[plugins] must be a table")); + + std::fs::write(&plugins_toml, "[plugins]\ndynamic = \"oops\"\n").unwrap(); + let error = append_dynamic_plugin_reference(&plugins_toml, "/tmp/plugin/relay-plugin.toml") + .unwrap_err() + .to_string(); + assert!(error.contains("plugins.dynamic must be an array of tables")); +} + #[test] fn remove_matches_relative_target_manifest_refs_without_loading_manifest() { let temp = tempfile::tempdir().unwrap(); From f35f8d11186d759375ce1b90d0b3d432765c232a Mon Sep 17 00:00:00 2001 From: Alex Fournier Date: Tue, 23 Jun 2026 19:41:56 -0700 Subject: [PATCH 14/20] refactor: derive lifecycle labels with strum Signed-off-by: Alex Fournier --- Cargo.lock | 22 ++++++++++ crates/cli/src/plugins/lifecycle/render.rs | 40 +++++-------------- crates/cli/src/plugins/lifecycle/responses.rs | 8 ++-- crates/core/Cargo.toml | 1 + crates/core/src/plugin/dynamic.rs | 11 ++++- 5 files changed, 46 insertions(+), 36 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c28b4667..f49b3e6a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1348,6 +1348,7 @@ dependencies = [ "schemars", "serde", "serde_json", + "strum", "tempfile", "thiserror 2.0.18", "tokio", @@ -2535,6 +2536,27 @@ version = "0.11.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7da8b5736845d9f2fcb837ea5d9e2628564b3b043a70948a3f0b778838c5fb4f" +[[package]] +name = "strum" +version = "0.27.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "af23d6f6c1a224baef9d3f61e287d2761385a5b88fdab4eb4c6f11aeb54c4bcf" +dependencies = [ + "strum_macros", +] + +[[package]] +name = "strum_macros" +version = "0.27.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7695ce3845ea4b33927c055a39dc438a45b059f7c1b3d91d38d10355fb8cbca7" +dependencies = [ + "heck", + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "subtle" version = "2.6.1" diff --git a/crates/cli/src/plugins/lifecycle/render.rs b/crates/cli/src/plugins/lifecycle/render.rs index 178ea019..8e36ce9e 100644 --- a/crates/cli/src/plugins/lifecycle/render.rs +++ b/crates/cli/src/plugins/lifecycle/render.rs @@ -4,9 +4,8 @@ use std::collections::HashMap; use nemo_relay::plugin::dynamic::{ - DynamicPluginCheckState, DynamicPluginCompatibility, DynamicPluginKind, - DynamicPluginLoadContract, DynamicPluginManifest, DynamicPluginRecord, - DynamicPluginRuntimeState, + DynamicPluginCompatibility, DynamicPluginKind, DynamicPluginLoadContract, + DynamicPluginManifest, DynamicPluginRecord, }; use serde_json::Value; @@ -34,7 +33,7 @@ pub(super) fn render_list( entry.scope.label(), entry.record.spec.enabled, lifecycle_state_label(&entry.record), - check_state_label(entry.record.status.validation.manifest), + <&'static str>::from(entry.record.status.validation.manifest), host_config_status )); } @@ -168,31 +167,31 @@ fn render_status(record: &DynamicPluginRecord) -> Vec { let mut lines = vec![ format!( "status.validation.manifest: {}", - check_state_label(record.status.validation.manifest) + <&'static str>::from(record.status.validation.manifest) ), format!( "status.validation.compatibility: {}", - check_state_label(record.status.validation.compatibility) + <&'static str>::from(record.status.validation.compatibility) ), format!( "status.validation.integrity: {}", - check_state_label(record.status.validation.integrity) + <&'static str>::from(record.status.validation.integrity) ), format!( "status.validation.environment: {}", - check_state_label(record.status.validation.environment) + <&'static str>::from(record.status.validation.environment) ), format!( "status.validation.authenticity: {}", - check_state_label(record.status.validation.authenticity) + <&'static str>::from(record.status.validation.authenticity) ), format!( "status.validation.policy_satisfied: {}", - check_state_label(record.status.validation.policy_satisfied) + <&'static str>::from(record.status.validation.policy_satisfied) ), format!( "status.runtime.state: {}", - runtime_state_label(record.status.runtime.state) + <&'static str>::from(record.status.runtime.state) ), format!( "status.runtime.observed_generation: {}", @@ -238,28 +237,11 @@ fn host_config_status_label(status: DynamicPluginHostConfigStatus) -> &'static s } } -pub(super) fn check_state_label(state: DynamicPluginCheckState) -> &'static str { - match state { - DynamicPluginCheckState::Unknown => "unknown", - DynamicPluginCheckState::Valid => "valid", - DynamicPluginCheckState::Invalid => "invalid", - } -} - -pub(super) fn runtime_state_label(state: DynamicPluginRuntimeState) -> &'static str { - match state { - DynamicPluginRuntimeState::Stopped => "stopped", - DynamicPluginRuntimeState::Starting => "starting", - DynamicPluginRuntimeState::Running => "running", - DynamicPluginRuntimeState::Failed => "failed", - } -} - fn lifecycle_state_label(record: &DynamicPluginRecord) -> &'static str { if record.is_tombstoned() { "tombstoned" } else { - runtime_state_label(record.status.runtime.state) + record.status.runtime.state.into() } } diff --git a/crates/cli/src/plugins/lifecycle/responses.rs b/crates/cli/src/plugins/lifecycle/responses.rs index 85ff42b8..8545e04c 100644 --- a/crates/cli/src/plugins/lifecycle/responses.rs +++ b/crates/cli/src/plugins/lifecycle/responses.rs @@ -17,9 +17,7 @@ use serde_json::{Map, Value}; use crate::config::{DynamicPluginHostConfigStatus, ResolvedDynamicPluginConfig}; use crate::error::{CliError, PluginLifecycleFailureKind}; -use super::render::{ - check_state_label, manifest_kind_label, redacted_host_config_json, runtime_state_label, -}; +use super::render::{manifest_kind_label, redacted_host_config_json}; use super::state::ScopedDynamicPluginRecord; #[derive(Debug)] @@ -137,11 +135,11 @@ pub(super) fn list_success( kind: manifest_kind_label(record.metadata.kind), enabled: record.spec.enabled, tombstoned: record.is_tombstoned(), - validation_state: check_state_label(record.status.validation.manifest), + validation_state: record.status.validation.manifest.into(), runtime_state: if record.is_tombstoned() { "tombstoned".into() } else { - runtime_state_label(record.status.runtime.state).into() + <&'static str>::from(record.status.runtime.state).into() }, startup: record .status diff --git a/crates/core/Cargo.toml b/crates/core/Cargo.toml index 595bd577..5b9ca03c 100644 --- a/crates/core/Cargo.toml +++ b/crates/core/Cargo.toml @@ -83,6 +83,7 @@ schemars = { version = "0.8", optional = true } chrono = { version = "0.4", features = ["serde"] } bitflags = { version = "2", features = ["serde"] } thiserror = "2" +strum = { version = "0.27", features = ["derive"] } tokio = { version = "1", default-features = false, features = ["rt", "macros", "sync", "time"] } tokio-stream = { version = "0.1", default-features = false, features = ["sync"] } typed-builder = "0.23.2" diff --git a/crates/core/src/plugin/dynamic.rs b/crates/core/src/plugin/dynamic.rs index 16a5a693..0f041c83 100644 --- a/crates/core/src/plugin/dynamic.rs +++ b/crates/core/src/plugin/dynamic.rs @@ -10,6 +10,7 @@ use chrono::Utc; use serde::{Deserialize, Serialize}; +use strum::IntoStaticStr; /// Canonical identifier for one dynamic plugin record. pub type DynamicPluginId = String; @@ -81,9 +82,12 @@ pub enum DynamicPluginAttestationMode { } /// High-level verification state for one validation axis. -#[derive(Debug, Clone, Copy, Default, Serialize, Deserialize, PartialEq, Eq, Hash)] +#[derive( + Debug, Clone, Copy, Default, Serialize, Deserialize, PartialEq, Eq, Hash, IntoStaticStr, +)] #[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] #[serde(rename_all = "snake_case")] +#[strum(serialize_all = "snake_case")] pub enum DynamicPluginCheckState { /// No verification result is currently known. #[default] @@ -95,9 +99,12 @@ pub enum DynamicPluginCheckState { } /// Observed runtime state for a dynamic plugin. -#[derive(Debug, Clone, Copy, Default, Serialize, Deserialize, PartialEq, Eq, Hash)] +#[derive( + Debug, Clone, Copy, Default, Serialize, Deserialize, PartialEq, Eq, Hash, IntoStaticStr, +)] #[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] #[serde(rename_all = "snake_case")] +#[strum(serialize_all = "snake_case")] pub enum DynamicPluginRuntimeState { /// Not currently active. #[default] From ba501e63f449671cd50fc00cb7c2e9901875fe49 Mon Sep 17 00:00:00 2001 From: Alex Fournier Date: Wed, 24 Jun 2026 07:30:08 -0700 Subject: [PATCH 15/20] refactor: derive lifecycle scope labels with strum Signed-off-by: Alex Fournier --- ATTRIBUTIONS-Rust.md | 58 ++++++++++++++ Cargo.lock | 1 + crates/cli/Cargo.toml | 1 + crates/cli/src/plugins/lifecycle.rs | 3 +- crates/cli/src/plugins/lifecycle/render.rs | 6 +- crates/cli/src/plugins/lifecycle/responses.rs | 4 +- crates/cli/src/plugins/lifecycle/state.rs | 19 ++--- crates/cli/src/plugins/lifecycle/target.rs | 80 +------------------ .../plugins_lifecycle_target_tests.rs | 78 ++++++++++++++++++ .../tests/coverage/plugins_lifecycle_tests.rs | 4 +- 10 files changed, 153 insertions(+), 101 deletions(-) create mode 100644 crates/cli/tests/coverage/plugins_lifecycle_target_tests.rs diff --git a/ATTRIBUTIONS-Rust.md b/ATTRIBUTIONS-Rust.md index 9ccd2236..afca1c0c 100644 --- a/ATTRIBUTIONS-Rust.md +++ b/ATTRIBUTIONS-Rust.md @@ -33812,6 +33812,64 @@ SOFTWARE. ``` +## strum - 0.27.2 +**Repository URL**: https://github.com/Peternator7/strum +**License Type(s)**: MIT +### License: https://spdx.org/licenses/MIT.html +``` +MIT License + +Copyright (c) 2019 Peter Glotfelty + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. + +``` + +## strum_macros - 0.27.2 +**Repository URL**: https://github.com/Peternator7/strum +**License Type(s)**: MIT +### License: https://spdx.org/licenses/MIT.html +``` +MIT License + +Copyright (c) 2019 Peter Glotfelty + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. + +``` + ## subtle - 2.6.1 **Repository URL**: https://github.com/dalek-cryptography/subtle **License Type(s)**: BSD-3-Clause diff --git a/Cargo.lock b/Cargo.lock index f49b3e6a..066876da 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1406,6 +1406,7 @@ dependencies = [ "serde", "serde_json", "serde_yaml", + "strum", "tempfile", "thiserror 2.0.18", "tokio", diff --git a/crates/cli/Cargo.toml b/crates/cli/Cargo.toml index 6d6538c1..a494bde4 100644 --- a/crates/cli/Cargo.toml +++ b/crates/cli/Cargo.toml @@ -43,6 +43,7 @@ rustls = { version = "0.23", default-features = false, features = ["ring", "std" serde = { version = "1", features = ["derive"] } serde_json = "1" serde_yaml = "0.9" +strum = { version = "0.27", features = ["derive"] } thiserror = "2" tokio = { version = "1", features = ["macros", "net", "process", "rt-multi-thread", "signal", "sync", "time"] } tokio-tungstenite = { version = "0.27", default-features = false, features = ["connect", "rustls-tls-native-roots"] } diff --git a/crates/cli/src/plugins/lifecycle.rs b/crates/cli/src/plugins/lifecycle.rs index f92320bf..a89e9651 100644 --- a/crates/cli/src/plugins/lifecycle.rs +++ b/crates/cli/src/plugins/lifecycle.rs @@ -46,8 +46,7 @@ pub(crate) fn add(command: PluginsAddCommand, server: &ServerArgs) -> Result<(), Some(existing) if !existing.record.is_tombstoned() => { return Err(CliError::Config(format!( "dynamic plugin '{}' is already registered in the {} lifecycle scope", - plugin_id, - existing.scope.label() + plugin_id, existing.scope ))); } Some(_) => true, diff --git a/crates/cli/src/plugins/lifecycle/render.rs b/crates/cli/src/plugins/lifecycle/render.rs index 8e36ce9e..673b015d 100644 --- a/crates/cli/src/plugins/lifecycle/render.rs +++ b/crates/cli/src/plugins/lifecycle/render.rs @@ -30,7 +30,7 @@ pub(super) fn render_list( lines.push(format!( "{:<32} {:<8} {:<7} {:<10} {:<10} {}", entry.record.metadata.id, - entry.scope.label(), + entry.scope, entry.record.spec.enabled, lifecycle_state_label(&entry.record), <&'static str>::from(entry.record.status.validation.manifest), @@ -49,7 +49,7 @@ pub(super) fn render_inspect( let record = &entry.record; let mut lines = vec![ format!("id: {}", record.metadata.id), - format!("scope: {}", entry.scope.label()), + format!("scope: {}", entry.scope), format!("kind: {}", manifest_kind_label(record.metadata.kind)), format!( "name: {}", @@ -147,7 +147,7 @@ pub(super) fn render_validation_summary( format!("manifest: {manifest_ref}"), ]; if let Some(entry) = entry { - lines.push(format!("scope: {}", entry.scope.label())); + lines.push(format!("scope: {}", entry.scope)); lines.push(format!( "lifecycle_state_path: {}", entry.state_path.display() diff --git a/crates/cli/src/plugins/lifecycle/responses.rs b/crates/cli/src/plugins/lifecycle/responses.rs index 8545e04c..a136b3fa 100644 --- a/crates/cli/src/plugins/lifecycle/responses.rs +++ b/crates/cli/src/plugins/lifecycle/responses.rs @@ -79,7 +79,7 @@ pub(super) struct InspectResponse { id: String, name: Option, kind: &'static str, - scope: &'static str, + scope: String, manifest_ref: String, plugins_toml_path: String, state_path: String, @@ -180,7 +180,7 @@ pub(super) fn inspect_success( id: record.metadata.id.clone(), name: record.metadata.name.clone(), kind: manifest_kind_label(record.metadata.kind), - scope: entry.scope.label(), + scope: entry.scope.to_string(), manifest_ref: manifest_ref.into(), plugins_toml_path: entry.plugins_toml_path.display().to_string(), state_path: entry.state_path.display().to_string(), diff --git a/crates/cli/src/plugins/lifecycle/state.rs b/crates/cli/src/plugins/lifecycle/state.rs index f26d85bd..0eb21b31 100644 --- a/crates/cli/src/plugins/lifecycle/state.rs +++ b/crates/cli/src/plugins/lifecycle/state.rs @@ -7,6 +7,7 @@ use std::time::{SystemTime, UNIX_EPOCH}; use nemo_relay::plugin::dynamic::{DynamicPluginRecord, DynamicPluginRegistry}; use serde::{Deserialize, Serialize}; +use strum::Display; use crate::config::{ PLUGINS_TOML, global_plugin_config_path, project_plugin_config_path, user_config_dir, @@ -20,7 +21,8 @@ use super::super::config_io::TargetScope; const DYNAMIC_PLUGIN_STATE_FILENAME: &str = ".dynamic-plugins.json"; const DYNAMIC_PLUGIN_STATE_SCHEMA_VERSION: u32 = 1; -#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[derive(Display, Debug, Clone, Copy, PartialEq, Eq)] +#[strum(serialize_all = "snake_case")] pub(super) enum RegistryScope { User, Project, @@ -28,17 +30,6 @@ pub(super) enum RegistryScope { Explicit, } -impl RegistryScope { - pub(super) fn label(self) -> &'static str { - match self { - Self::User => "user", - Self::Project => "project", - Self::Global => "global", - Self::Explicit => "explicit", - } - } -} - #[derive(Debug)] pub(super) struct ScopedRegistry { pub(super) scope: RegistryScope, @@ -213,7 +204,7 @@ pub(super) fn find_record_by_id( "dynamic plugin '{}' is configured in multiple lifecycle scopes; inspect {}", plugin_id, live.iter() - .map(|record| record.scope.label()) + .map(|record| record.scope.to_string()) .collect::>() .join(", ") ))); @@ -227,7 +218,7 @@ pub(super) fn find_record_by_id( plugin_id, tombstoned .iter() - .map(|record| record.scope.label()) + .map(|record| record.scope.to_string()) .collect::>() .join(", ") ))); diff --git a/crates/cli/src/plugins/lifecycle/target.rs b/crates/cli/src/plugins/lifecycle/target.rs index 0da4e156..a423bb32 100644 --- a/crates/cli/src/plugins/lifecycle/target.rs +++ b/crates/cli/src/plugins/lifecycle/target.rs @@ -50,82 +50,6 @@ fn should_treat_target_as_path(target: &str) -> bool { || target.contains('/') || target.contains('\\') } - #[cfg(test)] -mod tests { - use std::path::PathBuf; - - use super::PluginTarget; - use tempfile::tempdir; - - #[test] - fn parse_treats_canonical_plugin_ids_as_ids() { - assert_eq!( - PluginTarget::parse("acme.worker"), - PluginTarget::Id("acme.worker".into()) - ); - assert_eq!( - PluginTarget::parse("acme.worker.v2"), - PluginTarget::Id("acme.worker.v2".into()) - ); - assert_eq!( - PluginTarget::parse("relay-plugin"), - PluginTarget::Id("relay-plugin".into()) - ); - } - - #[test] - fn parse_treats_manifest_filenames_as_paths() { - assert_eq!( - PluginTarget::parse("relay-plugin.toml"), - PluginTarget::Path(PathBuf::from("relay-plugin.toml")) - ); - } - - #[test] - fn parse_treats_relative_path_syntax_as_paths() { - assert_eq!( - PluginTarget::parse("./plugins/acme/relay-plugin.toml"), - PluginTarget::Path(PathBuf::from("./plugins/acme/relay-plugin.toml")) - ); - assert_eq!( - PluginTarget::parse("."), - PluginTarget::Path(PathBuf::from(".")) - ); - assert_eq!( - PluginTarget::parse(".."), - PluginTarget::Path(PathBuf::from("..")) - ); - assert_eq!( - PluginTarget::parse(r"plugins\acme\relay-plugin.toml"), - PluginTarget::Path(PathBuf::from(r"plugins\acme\relay-plugin.toml")) - ); - } - - #[test] - fn parse_treats_absolute_paths_as_paths_even_when_missing() { - let temp = tempdir().unwrap(); - let missing = temp.path().join("missing").join("relay-plugin.toml"); - assert_eq!( - PluginTarget::parse(missing.to_str().unwrap()), - PluginTarget::Path(missing) - ); - } - - #[test] - fn parse_treats_existing_filesystem_entries_with_explicit_path_syntax_as_paths() { - let temp = tempdir().unwrap(); - let existing = temp.path().join("plugins").join("acme.worker"); - std::fs::create_dir_all(&existing).unwrap(); - assert_eq!( - PluginTarget::parse( - existing - .strip_prefix(temp.path()) - .unwrap() - .to_str() - .unwrap() - ), - PluginTarget::Path(PathBuf::from("plugins/acme.worker")) - ); - } -} +#[path = "../../../tests/coverage/plugins_lifecycle_target_tests.rs"] +mod tests; diff --git a/crates/cli/tests/coverage/plugins_lifecycle_target_tests.rs b/crates/cli/tests/coverage/plugins_lifecycle_target_tests.rs new file mode 100644 index 00000000..6c0ff668 --- /dev/null +++ b/crates/cli/tests/coverage/plugins_lifecycle_target_tests.rs @@ -0,0 +1,78 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +use std::path::PathBuf; + +use super::PluginTarget; +use tempfile::tempdir; + +#[test] +fn parse_treats_canonical_plugin_ids_as_ids() { + assert_eq!( + PluginTarget::parse("acme.worker"), + PluginTarget::Id("acme.worker".into()) + ); + assert_eq!( + PluginTarget::parse("acme.worker.v2"), + PluginTarget::Id("acme.worker.v2".into()) + ); + assert_eq!( + PluginTarget::parse("relay-plugin"), + PluginTarget::Id("relay-plugin".into()) + ); +} + +#[test] +fn parse_treats_manifest_filenames_as_paths() { + assert_eq!( + PluginTarget::parse("relay-plugin.toml"), + PluginTarget::Path(PathBuf::from("relay-plugin.toml")) + ); +} + +#[test] +fn parse_treats_relative_path_syntax_as_paths() { + assert_eq!( + PluginTarget::parse("./plugins/acme/relay-plugin.toml"), + PluginTarget::Path(PathBuf::from("./plugins/acme/relay-plugin.toml")) + ); + assert_eq!( + PluginTarget::parse("."), + PluginTarget::Path(PathBuf::from(".")) + ); + assert_eq!( + PluginTarget::parse(".."), + PluginTarget::Path(PathBuf::from("..")) + ); + assert_eq!( + PluginTarget::parse(r"plugins\acme\relay-plugin.toml"), + PluginTarget::Path(PathBuf::from(r"plugins\acme\relay-plugin.toml")) + ); +} + +#[test] +fn parse_treats_absolute_paths_as_paths_even_when_missing() { + let temp = tempdir().unwrap(); + let missing = temp.path().join("missing").join("relay-plugin.toml"); + assert_eq!( + PluginTarget::parse(missing.to_str().unwrap()), + PluginTarget::Path(missing) + ); +} + +#[test] +fn parse_treats_existing_filesystem_entries_with_explicit_path_syntax_as_paths() { + let temp = tempdir().unwrap(); + let existing = temp.path().join("plugins").join("acme.worker"); + std::fs::create_dir_all(&existing).unwrap(); + assert_eq!( + PluginTarget::parse( + existing + .strip_prefix(temp.path()) + .unwrap() + .to_str() + .unwrap() + ), + PluginTarget::Path(PathBuf::from("plugins/acme.worker")) + ); +} diff --git a/crates/cli/tests/coverage/plugins_lifecycle_tests.rs b/crates/cli/tests/coverage/plugins_lifecycle_tests.rs index 6661b8d8..20db9e75 100644 --- a/crates/cli/tests/coverage/plugins_lifecycle_tests.rs +++ b/crates/cli/tests/coverage/plugins_lifecycle_tests.rs @@ -461,7 +461,7 @@ fn add_with_explicit_config_uses_sibling_plugins_and_state_files() { let entry = find_record_by_id(&scopes, "acme.explicit") .unwrap() .expect("explicit-scope record"); - assert_eq!(entry.scope.label(), "explicit"); + assert_eq!(entry.scope.to_string(), "explicit"); assert_eq!(entry.plugins_toml_path, plugins_toml); assert_eq!(entry.state_path, state_path); } @@ -493,7 +493,7 @@ fn hydrate_bootstraps_registry_records_from_existing_dynamic_plugin_refs() { let entry = find_record_by_id(&scopes, "acme.bootstrap") .unwrap() .expect("hydrated record"); - assert_eq!(entry.scope.label(), "project"); + assert_eq!(entry.scope.to_string(), "project"); assert_eq!(entry.record.metadata.id, "acme.bootstrap"); assert!(entry.record.spec.present); assert!(!entry.record.spec.enabled); From c9939484bb9fbce1089c3e092fa298dcb77b0379 Mon Sep 17 00:00:00 2001 From: Alex Fournier Date: Wed, 24 Jun 2026 07:41:40 -0700 Subject: [PATCH 16/20] refactor: derive lifecycle labels from enum display Signed-off-by: Alex Fournier --- crates/cli/src/config.rs | 4 +- crates/cli/src/error.rs | 4 +- crates/cli/src/plugins/lifecycle/render.rs | 48 +++++-------- crates/cli/src/plugins/lifecycle/responses.rs | 71 +++++++------------ crates/cli/src/plugins/lifecycle/state.rs | 3 +- crates/core/src/plugin/dynamic.rs | 20 ++++-- 6 files changed, 64 insertions(+), 86 deletions(-) diff --git a/crates/cli/src/config.rs b/crates/cli/src/config.rs index c489cd4c..9792a3dd 100644 --- a/crates/cli/src/config.rs +++ b/crates/cli/src/config.rs @@ -11,6 +11,7 @@ use nemo_relay::plugin::dynamic::DynamicPluginManifest; use nemo_relay::plugin::{PluginError, merge_plugin_config_documents}; use serde::{Deserialize, Serialize}; use serde_json::{Map, Value}; +use strum::Display; use crate::error::CliError; use crate::plugin_shim::PluginShimCommand; @@ -599,8 +600,9 @@ pub(crate) struct ResolvedDynamicPluginConfig { pub(crate) source: PathBuf, } -#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Display)] #[serde(rename_all = "snake_case")] +#[strum(serialize_all = "snake_case")] pub(crate) enum DynamicPluginHostConfigStatus { Absent, Present, diff --git a/crates/cli/src/error.rs b/crates/cli/src/error.rs index aa0e0fa3..0fd38e3d 100644 --- a/crates/cli/src/error.rs +++ b/crates/cli/src/error.rs @@ -7,9 +7,11 @@ use axum::response::{IntoResponse, Response}; use nemo_relay::error::FlowError; use serde::Serialize; use serde_json::{Map, Value, json}; +use strum::Display; -#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Display)] #[serde(rename_all = "snake_case")] +#[strum(serialize_all = "snake_case")] pub(crate) enum PluginLifecycleFailureKind { Failed, NotFound, diff --git a/crates/cli/src/plugins/lifecycle/render.rs b/crates/cli/src/plugins/lifecycle/render.rs index 673b015d..867e4927 100644 --- a/crates/cli/src/plugins/lifecycle/render.rs +++ b/crates/cli/src/plugins/lifecycle/render.rs @@ -4,12 +4,12 @@ use std::collections::HashMap; use nemo_relay::plugin::dynamic::{ - DynamicPluginCompatibility, DynamicPluginKind, DynamicPluginLoadContract, - DynamicPluginManifest, DynamicPluginRecord, + DynamicPluginCompatibility, DynamicPluginLoadContract, DynamicPluginManifest, + DynamicPluginRecord, }; use serde_json::Value; -use crate::config::{DynamicPluginHostConfigStatus, ResolvedDynamicPluginConfig}; +use crate::config::ResolvedDynamicPluginConfig; use super::state::ScopedDynamicPluginRecord; @@ -25,8 +25,8 @@ pub(super) fn render_list( for entry in records { let host_config_status = host_config_by_id .get(&entry.record.metadata.id) - .map(|plugin| host_config_status_label(plugin.host_config_status())) - .unwrap_or("missing"); + .map(|plugin| plugin.host_config_status().to_string()) + .unwrap_or_else(|| "missing".into()); lines.push(format!( "{:<32} {:<8} {:<7} {:<10} {:<10} {}", entry.record.metadata.id, @@ -50,7 +50,7 @@ pub(super) fn render_inspect( let mut lines = vec![ format!("id: {}", record.metadata.id), format!("scope: {}", entry.scope), - format!("kind: {}", manifest_kind_label(record.metadata.kind)), + format!("kind: {}", record.metadata.kind), format!( "name: {}", record.metadata.name.as_deref().unwrap_or("") @@ -81,8 +81,8 @@ pub(super) fn render_inspect( format!( "host_config: {}", host_config - .map(|plugin| host_config_status_label(plugin.host_config_status())) - .unwrap_or("missing") + .map(|plugin| plugin.host_config_status().to_string()) + .unwrap_or_else(|| "missing".into()) ), ]; @@ -102,7 +102,7 @@ pub(super) fn render_inspect( match &record.load { DynamicPluginLoadContract::Worker(load) => { - lines.push(format!("load.runtime: {:?}", load.runtime).to_lowercase()); + lines.push(format!("load.runtime: {}", load.runtime)); lines.push(format!("load.entrypoint: {}", load.entrypoint)); } DynamicPluginLoadContract::RustDynamic(load) => { @@ -118,7 +118,7 @@ pub(super) fn render_inspect( .capabilities .items .iter() - .map(|item| format!("{item:?}").to_lowercase()) + .map(ToString::to_string) .collect::>() .join(", ") )); @@ -143,7 +143,7 @@ pub(super) fn render_validation_summary( ) -> String { let mut lines = vec![ format!("Dynamic plugin '{}' is valid.", manifest.plugin.id), - format!("kind: {}", manifest_kind_label(manifest.plugin.kind)), + format!("kind: {}", manifest.plugin.kind), format!("manifest: {manifest_ref}"), ]; if let Some(entry) = entry { @@ -156,8 +156,8 @@ pub(super) fn render_validation_summary( lines.push(format!( "host_config: {}", host_config - .map(|plugin| host_config_status_label(plugin.host_config_status())) - .unwrap_or("missing") + .map(|plugin| plugin.host_config_status().to_string()) + .unwrap_or_else(|| "missing".into()) )); } lines.join("\n") @@ -214,29 +214,20 @@ fn render_status(record: &DynamicPluginRecord) -> Vec { lines.push(format!("status.runtime.message: {value}")); } if let Some(value) = record.status.startup_class { - lines.push(format!("status.startup_class: {:?}", value).to_lowercase()); + lines.push(format!("status.startup_class: {}", value)); } if let Some(value) = record.status.attestation_mode { - lines.push(format!("status.attestation_mode: {:?}", value).to_lowercase()); + lines.push(format!("status.attestation_mode: {}", value)); } if let Some(error) = record.status.last_error.as_ref() { lines.push(format!( "status.last_error: {}:{} {}", - format!("{:?}", error.phase).to_lowercase(), - error.code, - error.message + error.phase, error.code, error.message )); } lines } -fn host_config_status_label(status: DynamicPluginHostConfigStatus) -> &'static str { - match status { - DynamicPluginHostConfigStatus::Absent => "absent", - DynamicPluginHostConfigStatus::Present => "present", - } -} - fn lifecycle_state_label(record: &DynamicPluginRecord) -> &'static str { if record.is_tombstoned() { "tombstoned" @@ -245,13 +236,6 @@ fn lifecycle_state_label(record: &DynamicPluginRecord) -> &'static str { } } -pub(super) fn manifest_kind_label(kind: DynamicPluginKind) -> &'static str { - match kind { - DynamicPluginKind::RustDynamic => "rust_dynamic", - DynamicPluginKind::Worker => "worker", - } -} - pub(super) fn redacted_host_config_json(host_config: &ResolvedDynamicPluginConfig) -> Value { if host_config.config.is_empty() { return Value::Null; diff --git a/crates/cli/src/plugins/lifecycle/responses.rs b/crates/cli/src/plugins/lifecycle/responses.rs index a136b3fa..a72263a0 100644 --- a/crates/cli/src/plugins/lifecycle/responses.rs +++ b/crates/cli/src/plugins/lifecycle/responses.rs @@ -10,14 +10,16 @@ use std::collections::HashMap; -use nemo_relay::plugin::dynamic::{DynamicPluginFailurePhase, DynamicPluginManifest}; +use nemo_relay::plugin::dynamic::{ + DynamicPluginCheckState, DynamicPluginKind, DynamicPluginManifest, +}; use serde::Serialize; use serde_json::{Map, Value}; -use crate::config::{DynamicPluginHostConfigStatus, ResolvedDynamicPluginConfig}; +use crate::config::ResolvedDynamicPluginConfig; use crate::error::{CliError, PluginLifecycleFailureKind}; -use super::render::{manifest_kind_label, redacted_host_config_json}; +use super::render::redacted_host_config_json; use super::state::ScopedDynamicPluginRecord; #[derive(Debug)] @@ -57,14 +59,14 @@ pub(super) struct ResponseError { pub(super) struct ListEntryResponse { id: String, name: Option, - kind: &'static str, + kind: DynamicPluginKind, enabled: bool, tombstoned: bool, - validation_state: &'static str, + validation_state: DynamicPluginCheckState, runtime_state: String, startup: Option, last_error: Option, - host_config: &'static str, + host_config: String, } #[derive(Debug, Serialize)] @@ -78,8 +80,8 @@ pub(super) struct LastErrorResponse { pub(super) struct InspectResponse { id: String, name: Option, - kind: &'static str, - scope: String, + kind: DynamicPluginKind, + scope: super::state::RegistryScope, manifest_ref: String, plugins_toml_path: String, state_path: String, @@ -90,7 +92,7 @@ pub(super) struct InspectResponse { source: Value, spec: Value, status: Value, - host_config_status: &'static str, + host_config_status: String, host_config: Value, } @@ -103,9 +105,9 @@ pub(super) struct ValidateResponse { warnings: Vec, notes: Vec, manifest_ref: String, - kind: &'static str, + kind: DynamicPluginKind, desired_enabled: Option, - host_config_status: &'static str, + host_config_status: String, } pub(super) fn print_response_json(value: &T) -> Result<(), CliError> { @@ -132,32 +134,29 @@ pub(super) fn list_success( ListEntryResponse { id: record.metadata.id.clone(), name: record.metadata.name.clone(), - kind: manifest_kind_label(record.metadata.kind), + kind: record.metadata.kind, enabled: record.spec.enabled, tombstoned: record.is_tombstoned(), - validation_state: record.status.validation.manifest.into(), + validation_state: record.status.validation.manifest, runtime_state: if record.is_tombstoned() { "tombstoned".into() } else { <&'static str>::from(record.status.runtime.state).into() }, - startup: record - .status - .startup_class - .map(|value| format!("{value:?}").to_lowercase()), + startup: record.status.startup_class.map(|value| value.to_string()), last_error: record .status .last_error .as_ref() .map(|error| LastErrorResponse { - phase: failure_phase_label(error.phase).into(), + phase: error.phase.to_string(), code: error.code.clone(), message: error.message.clone(), }), host_config: host_config_by_id .get(&record.metadata.id) - .map(|plugin| host_config_status_label(plugin.host_config_status())) - .unwrap_or("missing"), + .map(|plugin| plugin.host_config_status().to_string()) + .unwrap_or_else(|| "missing".into()), } }) .collect(), @@ -179,8 +178,8 @@ pub(super) fn inspect_success( InspectResponse { id: record.metadata.id.clone(), name: record.metadata.name.clone(), - kind: manifest_kind_label(record.metadata.kind), - scope: entry.scope.to_string(), + kind: record.metadata.kind, + scope: entry.scope, manifest_ref: manifest_ref.into(), plugins_toml_path: entry.plugins_toml_path.display().to_string(), state_path: entry.state_path.display().to_string(), @@ -192,7 +191,7 @@ pub(super) fn inspect_success( .capabilities .items .iter() - .map(|item| format!("{item:?}").to_lowercase()) + .map(ToString::to_string) .collect(), metadata: serde_json::to_value(&record.metadata) .expect("dynamic plugin metadata serializes to JSON"), @@ -203,8 +202,8 @@ pub(super) fn inspect_success( status: serde_json::to_value(&record.status) .expect("dynamic plugin status serializes to JSON"), host_config_status: host_config - .map(|plugin| host_config_status_label(plugin.host_config_status())) - .unwrap_or("missing"), + .map(|plugin| plugin.host_config_status().to_string()) + .unwrap_or_else(|| "missing".into()), host_config: host_config .map(redacted_host_config_json) .unwrap_or(Value::Null), @@ -235,12 +234,12 @@ pub(super) fn validate_success( warnings: Vec::new(), notes, manifest_ref: input.manifest_ref.into(), - kind: manifest_kind_label(input.manifest.plugin.kind), + kind: input.manifest.plugin.kind, desired_enabled: input.entry.map(|entry| entry.record.spec.enabled), host_config_status: input .host_config - .map(|plugin| host_config_status_label(plugin.host_config_status())) - .unwrap_or("missing"), + .map(|plugin| plugin.host_config_status().to_string()) + .unwrap_or_else(|| "missing".into()), }, ) } @@ -291,13 +290,6 @@ fn success( } } -fn host_config_status_label(status: DynamicPluginHostConfigStatus) -> &'static str { - match status { - DynamicPluginHostConfigStatus::Absent => "absent", - DynamicPluginHostConfigStatus::Present => "present", - } -} - fn failure_code(kind: PluginLifecycleFailureKind) -> &'static str { match kind { PluginLifecycleFailureKind::Failed => "operation_failed", @@ -305,12 +297,3 @@ fn failure_code(kind: PluginLifecycleFailureKind) -> &'static str { PluginLifecycleFailureKind::Refused => "refused", } } - -fn failure_phase_label(phase: DynamicPluginFailurePhase) -> &'static str { - match phase { - DynamicPluginFailurePhase::Validation => "validation", - DynamicPluginFailurePhase::Activation => "activation", - DynamicPluginFailurePhase::Runtime => "runtime", - DynamicPluginFailurePhase::Policy => "policy", - } -} diff --git a/crates/cli/src/plugins/lifecycle/state.rs b/crates/cli/src/plugins/lifecycle/state.rs index 0eb21b31..f487692e 100644 --- a/crates/cli/src/plugins/lifecycle/state.rs +++ b/crates/cli/src/plugins/lifecycle/state.rs @@ -21,7 +21,8 @@ use super::super::config_io::TargetScope; const DYNAMIC_PLUGIN_STATE_FILENAME: &str = ".dynamic-plugins.json"; const DYNAMIC_PLUGIN_STATE_SCHEMA_VERSION: u32 = 1; -#[derive(Display, Debug, Clone, Copy, PartialEq, Eq)] +#[derive(Display, Debug, Clone, Copy, PartialEq, Eq, Serialize)] +#[serde(rename_all = "snake_case")] #[strum(serialize_all = "snake_case")] pub(super) enum RegistryScope { User, diff --git a/crates/core/src/plugin/dynamic.rs b/crates/core/src/plugin/dynamic.rs index 0f041c83..63971453 100644 --- a/crates/core/src/plugin/dynamic.rs +++ b/crates/core/src/plugin/dynamic.rs @@ -10,7 +10,7 @@ use chrono::Utc; use serde::{Deserialize, Serialize}; -use strum::IntoStaticStr; +use strum::{Display, IntoStaticStr}; /// Canonical identifier for one dynamic plugin record. pub type DynamicPluginId = String; @@ -25,9 +25,10 @@ pub use manifest::*; pub use registry::*; /// Plugin execution lane. -#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq, Hash)] +#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq, Hash, Display)] #[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] #[serde(rename_all = "snake_case")] +#[strum(serialize_all = "snake_case")] pub enum DynamicPluginKind { /// Trusted in-process native plugin. RustDynamic, @@ -36,18 +37,20 @@ pub enum DynamicPluginKind { } /// Managed runtime identity for worker-based plugins. -#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq, Hash)] +#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq, Hash, Display)] #[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] #[serde(rename_all = "snake_case")] +#[strum(serialize_all = "snake_case")] pub enum WorkerRuntime { /// Python worker runtime. Python, } /// Relay-enforced capability declared by a dynamic plugin. -#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq, Hash)] +#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq, Hash, Display)] #[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] #[serde(rename_all = "snake_case")] +#[strum(serialize_all = "snake_case")] pub enum DynamicPluginCapability { /// Trusted in-process native extension capability. PluginNative, @@ -58,9 +61,10 @@ pub enum DynamicPluginCapability { } /// Host policy startup classification for a plugin. -#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq, Hash)] +#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq, Hash, Display)] #[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] #[serde(rename_all = "snake_case")] +#[strum(serialize_all = "snake_case")] pub enum DynamicPluginStartupClass { /// Failure is tolerated and the host may start in degraded mode. Optional, @@ -69,9 +73,10 @@ pub enum DynamicPluginStartupClass { } /// Host attestation policy mode. -#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq, Hash)] +#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq, Hash, Display)] #[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] #[serde(rename_all = "snake_case")] +#[strum(serialize_all = "snake_case")] pub enum DynamicPluginAttestationMode { /// Integrity verification only. IntegrityOnly, @@ -118,9 +123,10 @@ pub enum DynamicPluginRuntimeState { } /// Recent failure phase for diagnostics and operator UX. -#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq, Hash)] +#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq, Hash, Display)] #[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] #[serde(rename_all = "snake_case")] +#[strum(serialize_all = "snake_case")] pub enum DynamicPluginFailurePhase { /// Failure occurred during validation. Validation, From 518648efe85e6459414371fc7e70581638dab1c4 Mon Sep 17 00:00:00 2001 From: Alex Fournier Date: Wed, 24 Jun 2026 09:00:43 -0700 Subject: [PATCH 17/20] refactor: reduce lifecycle renderer string churn Signed-off-by: Alex Fournier --- crates/cli/src/plugins/lifecycle/render.rs | 321 ++++++++++++++------- 1 file changed, 214 insertions(+), 107 deletions(-) diff --git a/crates/cli/src/plugins/lifecycle/render.rs b/crates/cli/src/plugins/lifecycle/render.rs index 867e4927..7fce652e 100644 --- a/crates/cli/src/plugins/lifecycle/render.rs +++ b/crates/cli/src/plugins/lifecycle/render.rs @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 use std::collections::HashMap; +use std::fmt::{self, Write as _}; use nemo_relay::plugin::dynamic::{ DynamicPluginCompatibility, DynamicPluginLoadContract, DynamicPluginManifest, @@ -17,27 +18,33 @@ pub(super) fn render_list( records: &[ScopedDynamicPluginRecord], host_config_by_id: &HashMap, ) -> String { - let mut lines = Vec::with_capacity(records.len() + 1); - lines.push(format!( - "{:<32} {:<8} {:<7} {:<10} {:<10} {}", - "ID", "SCOPE", "ENABLED", "STATE", "VALIDATION", "HOST CONFIG" - )); + let mut output = String::new(); + push_line( + &mut output, + format_args!( + "{:<32} {:<8} {:<7} {:<10} {:<10} {}", + "ID", "SCOPE", "ENABLED", "STATE", "VALIDATION", "HOST CONFIG" + ), + ); for entry in records { let host_config_status = host_config_by_id .get(&entry.record.metadata.id) .map(|plugin| plugin.host_config_status().to_string()) .unwrap_or_else(|| "missing".into()); - lines.push(format!( - "{:<32} {:<8} {:<7} {:<10} {:<10} {}", - entry.record.metadata.id, - entry.scope, - entry.record.spec.enabled, - lifecycle_state_label(&entry.record), - <&'static str>::from(entry.record.status.validation.manifest), - host_config_status - )); + push_line( + &mut output, + format_args!( + "{:<32} {:<8} {:<7} {:<10} {:<10} {}", + entry.record.metadata.id, + entry.scope, + entry.record.spec.enabled, + lifecycle_state_label(&entry.record), + <&'static str>::from(entry.record.status.validation.manifest), + host_config_status + ), + ); } - lines.join("\n") + finish_output(output) } pub(super) fn render_inspect( @@ -47,92 +54,147 @@ pub(super) fn render_inspect( host_config: Option<&ResolvedDynamicPluginConfig>, ) -> String { let record = &entry.record; - let mut lines = vec![ - format!("id: {}", record.metadata.id), - format!("scope: {}", entry.scope), - format!("kind: {}", record.metadata.kind), - format!( + let mut output = String::new(); + + push_line(&mut output, format_args!("id: {}", record.metadata.id)); + push_line(&mut output, format_args!("scope: {}", entry.scope)); + push_line(&mut output, format_args!("kind: {}", record.metadata.kind)); + push_line( + &mut output, + format_args!( "name: {}", record.metadata.name.as_deref().unwrap_or("") ), - format!( + ); + push_line( + &mut output, + format_args!( "version: {}", record.metadata.version.as_deref().unwrap_or("") ), - format!("manifest: {manifest_ref}"), - format!("plugins_toml: {}", entry.plugins_toml_path.display()), - format!("lifecycle_state_path: {}", entry.state_path.display()), - format!( + ); + push_line(&mut output, format_args!("manifest: {manifest_ref}")); + push_line( + &mut output, + format_args!("plugins_toml: {}", entry.plugins_toml_path.display()), + ); + push_line( + &mut output, + format_args!("lifecycle_state_path: {}", entry.state_path.display()), + ); + push_line( + &mut output, + format_args!( "source.manifest_ref: {}", record.source.manifest_ref.as_deref().unwrap_or("") ), - format!( + ); + push_line( + &mut output, + format_args!( "source.artifact_ref: {}", record.source.artifact_ref.as_deref().unwrap_or("") ), - format!( + ); + push_line( + &mut output, + format_args!( "source.environment_ref: {}", record.source.environment_ref.as_deref().unwrap_or("") ), - format!("desired.present: {}", record.spec.present), - format!("desired.enabled: {}", record.spec.enabled), - format!("generation: {}", record.metadata.generation), - format!("reconciled: {}", record.is_reconciled()), - format!( + ); + push_line( + &mut output, + format_args!("desired.present: {}", record.spec.present), + ); + push_line( + &mut output, + format_args!("desired.enabled: {}", record.spec.enabled), + ); + push_line( + &mut output, + format_args!("generation: {}", record.metadata.generation), + ); + push_line( + &mut output, + format_args!("reconciled: {}", record.is_reconciled()), + ); + push_line( + &mut output, + format_args!( "host_config: {}", host_config .map(|plugin| plugin.host_config_status().to_string()) .unwrap_or_else(|| "missing".into()) ), - ]; + ); match &record.compatibility { DynamicPluginCompatibility::Worker(compatibility) => { - lines.push(format!("compat.relay: {}", compatibility.relay)); - lines.push(format!( - "compat.worker_protocol: {}", - compatibility.worker_protocol - )); + push_line( + &mut output, + format_args!("compat.relay: {}", compatibility.relay), + ); + push_line( + &mut output, + format_args!("compat.worker_protocol: {}", compatibility.worker_protocol), + ); } DynamicPluginCompatibility::RustDynamic(compatibility) => { - lines.push(format!("compat.relay: {}", compatibility.relay)); - lines.push(format!("compat.native_api: {}", compatibility.native_api)); + push_line( + &mut output, + format_args!("compat.relay: {}", compatibility.relay), + ); + push_line( + &mut output, + format_args!("compat.native_api: {}", compatibility.native_api), + ); } } match &record.load { DynamicPluginLoadContract::Worker(load) => { - lines.push(format!("load.runtime: {}", load.runtime)); - lines.push(format!("load.entrypoint: {}", load.entrypoint)); + push_line(&mut output, format_args!("load.runtime: {}", load.runtime)); + push_line( + &mut output, + format_args!("load.entrypoint: {}", load.entrypoint), + ); } DynamicPluginLoadContract::RustDynamic(load) => { - lines.push(format!("load.library: {}", load.library)); - lines.push(format!("load.symbol: {}", load.symbol)); + push_line(&mut output, format_args!("load.library: {}", load.library)); + push_line(&mut output, format_args!("load.symbol: {}", load.symbol)); } } - lines.extend(render_status(record)); - lines.push(format!( - "capabilities: {}", - manifest - .capabilities - .items - .iter() - .map(ToString::to_string) - .collect::>() - .join(", ") - )); - lines.push(format!( - "host_config_json: {}", - host_config - .map(redacted_host_config_json) - .filter(|config| !config.is_null()) - .map(|config| { - serde_json::to_string_pretty(&config).expect("host config serializes") - }) - .unwrap_or_else(|| "".into()) - )); - lines.join("\n") + render_status(&mut output, record); + push_line( + &mut output, + format_args!( + "capabilities: {}", + manifest + .capabilities + .items + .iter() + .map(ToString::to_string) + .collect::>() + .join(", ") + ), + ); + push_line( + &mut output, + format_args!( + "host_config_json: {}", + host_config + .map(redacted_host_config_json) + .filter(|config| !config.is_null()) + .map(|config| { + serde_json::to_string_pretty(&config).expect("host config serializes") + }) + .unwrap_or_else(|| "".into()) + ), + ); + + finish_output(output) } pub(super) fn render_validation_summary( @@ -141,91 +203,136 @@ pub(super) fn render_validation_summary( entry: Option<&ScopedDynamicPluginRecord>, host_config: Option<&ResolvedDynamicPluginConfig>, ) -> String { - let mut lines = vec![ - format!("Dynamic plugin '{}' is valid.", manifest.plugin.id), - format!("kind: {}", manifest.plugin.kind), - format!("manifest: {manifest_ref}"), - ]; + let mut output = String::new(); + push_line( + &mut output, + format_args!("Dynamic plugin '{}' is valid.", manifest.plugin.id), + ); + push_line(&mut output, format_args!("kind: {}", manifest.plugin.kind)); + push_line(&mut output, format_args!("manifest: {manifest_ref}")); if let Some(entry) = entry { - lines.push(format!("scope: {}", entry.scope)); - lines.push(format!( - "lifecycle_state_path: {}", - entry.state_path.display() - )); - lines.push(format!("desired.enabled: {}", entry.record.spec.enabled)); - lines.push(format!( - "host_config: {}", - host_config - .map(|plugin| plugin.host_config_status().to_string()) - .unwrap_or_else(|| "missing".into()) - )); + push_line(&mut output, format_args!("scope: {}", entry.scope)); + push_line( + &mut output, + format_args!("lifecycle_state_path: {}", entry.state_path.display()), + ); + push_line( + &mut output, + format_args!("desired.enabled: {}", entry.record.spec.enabled), + ); + push_line( + &mut output, + format_args!( + "host_config: {}", + host_config + .map(|plugin| plugin.host_config_status().to_string()) + .unwrap_or_else(|| "missing".into()) + ), + ); } - lines.join("\n") + finish_output(output) } -fn render_status(record: &DynamicPluginRecord) -> Vec { - let mut lines = vec![ - format!( +fn render_status(output: &mut String, record: &DynamicPluginRecord) { + push_line( + output, + format_args!( "status.validation.manifest: {}", <&'static str>::from(record.status.validation.manifest) ), - format!( + ); + push_line( + output, + format_args!( "status.validation.compatibility: {}", <&'static str>::from(record.status.validation.compatibility) ), - format!( + ); + push_line( + output, + format_args!( "status.validation.integrity: {}", <&'static str>::from(record.status.validation.integrity) ), - format!( + ); + push_line( + output, + format_args!( "status.validation.environment: {}", <&'static str>::from(record.status.validation.environment) ), - format!( + ); + push_line( + output, + format_args!( "status.validation.authenticity: {}", <&'static str>::from(record.status.validation.authenticity) ), - format!( + ); + push_line( + output, + format_args!( "status.validation.policy_satisfied: {}", <&'static str>::from(record.status.validation.policy_satisfied) ), - format!( + ); + push_line( + output, + format_args!( "status.runtime.state: {}", <&'static str>::from(record.status.runtime.state) ), - format!( + ); + push_line( + output, + format_args!( "status.runtime.observed_generation: {}", record.status.runtime.observed_generation ), - ]; + ); if let Some(value) = record.status.validation.checked_at.as_deref() { - lines.push(format!("status.validation.checked_at: {value}")); + push_line( + output, + format_args!("status.validation.checked_at: {value}"), + ); } if let Some(value) = record.status.validation.message.as_deref() { - lines.push(format!("status.validation.message: {value}")); + push_line(output, format_args!("status.validation.message: {value}")); } if let Some(value) = record.status.runtime.started_at.as_deref() { - lines.push(format!("status.runtime.started_at: {value}")); + push_line(output, format_args!("status.runtime.started_at: {value}")); } if let Some(value) = record.status.runtime.updated_at.as_deref() { - lines.push(format!("status.runtime.updated_at: {value}")); + push_line(output, format_args!("status.runtime.updated_at: {value}")); } if let Some(value) = record.status.runtime.message.as_deref() { - lines.push(format!("status.runtime.message: {value}")); + push_line(output, format_args!("status.runtime.message: {value}")); } if let Some(value) = record.status.startup_class { - lines.push(format!("status.startup_class: {}", value)); + push_line(output, format_args!("status.startup_class: {}", value)); } if let Some(value) = record.status.attestation_mode { - lines.push(format!("status.attestation_mode: {}", value)); + push_line(output, format_args!("status.attestation_mode: {}", value)); } if let Some(error) = record.status.last_error.as_ref() { - lines.push(format!( - "status.last_error: {}:{} {}", - error.phase, error.code, error.message - )); + push_line( + output, + format_args!( + "status.last_error: {}:{} {}", + error.phase, error.code, error.message + ), + ); } - lines +} + +fn push_line(output: &mut String, args: fmt::Arguments<'_>) { + output.write_fmt(args).expect("writing to string succeeds"); + output.push('\n'); +} + +fn finish_output(mut output: String) -> String { + output.pop(); + output } fn lifecycle_state_label(record: &DynamicPluginRecord) -> &'static str { From 63f460d788119af92f6c8b6ad238ab4b167e33f2 Mon Sep 17 00:00:00 2001 From: Will Killian Date: Wed, 24 Jun 2026 14:04:16 -0400 Subject: [PATCH 18/20] refactor: simplify lifecycle output rendering Signed-off-by: Will Killian --- crates/cli/src/plugins/lifecycle.rs | 213 ++++++++++- crates/cli/src/plugins/lifecycle/render.rs | 359 ------------------ crates/cli/src/plugins/lifecycle/responses.rs | 86 ++--- crates/cli/tests/coverage/launcher_tests.rs | 1 + .../tests/coverage/plugins_lifecycle_tests.rs | 180 +++++++-- 5 files changed, 386 insertions(+), 453 deletions(-) delete mode 100644 crates/cli/src/plugins/lifecycle/render.rs diff --git a/crates/cli/src/plugins/lifecycle.rs b/crates/cli/src/plugins/lifecycle.rs index a89e9651..6c6cfc96 100644 --- a/crates/cli/src/plugins/lifecycle.rs +++ b/crates/cli/src/plugins/lifecycle.rs @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 use std::collections::HashMap; +use std::fmt; use std::path::PathBuf; use std::process::ExitCode; @@ -9,6 +10,7 @@ use nemo_relay::plugin::dynamic::{ DynamicPluginCheckState, DynamicPluginManifest, DynamicPluginRecord, DynamicPluginValidationStatus, }; +use serde_json::Value; use crate::config::{ PluginsAddCommand, PluginsDisableCommand, PluginsEnableCommand, PluginsInspectCommand, @@ -21,19 +23,17 @@ use super::config_io::{ append_dynamic_plugin_reference, remove_dynamic_plugin_reference, target_scope, }; -mod render; mod responses; mod state; mod target; -use self::render::{render_inspect, render_list, render_validation_summary}; use self::responses::{ - ValidateResponseInput, failure, generic_failure, inspect_success, list_success, + ValidateResponseInput, failure, generic_failure, inspect_data, inspect_success, list_success, print_response_json, validate_success, }; use self::state::{ - RegistryScope, ScopedRegistry, collect_records, find_record_by_id, load_scoped_registries, - scoped_paths_for_add, + RegistryScope, ScopedDynamicPluginRecord, ScopedRegistry, collect_records, find_record_by_id, + load_scoped_registries, scoped_paths_for_add, }; use self::target::PluginTarget; @@ -112,7 +112,12 @@ pub(crate) fn validate( } else { println!( "{}", - render_validation_summary(&manifest, &manifest_ref, None, None) + PluginValidationSummaryView { + manifest: &manifest, + manifest_ref: &manifest_ref, + entry: None, + host_config: None, + } ); } Ok(()) @@ -157,12 +162,12 @@ pub(crate) fn validate( } else { println!( "{}", - render_validation_summary( - &manifest, - &manifest_ref, - Some(&refreshed), - host_config_by_id.get(&plugin_id), - ) + PluginValidationSummaryView { + manifest: &manifest, + manifest_ref: &manifest_ref, + entry: Some(&refreshed), + host_config: host_config_by_id.get(&plugin_id), + } ); } Ok(()) @@ -196,7 +201,13 @@ pub(crate) fn list(command: PluginsListCommand, server: &ServerArgs) -> Result<( &host_config_by_id, ))?; } else { - println!("{}", render_list(&records, &host_config_by_id)); + println!( + "{}", + PluginListView { + records: &records, + host_config_by_id: &host_config_by_id, + } + ); } Ok(()) } @@ -220,12 +231,12 @@ pub(crate) fn inspect(command: PluginsInspectCommand, server: &ServerArgs) -> Re } else { println!( "{}", - render_inspect( - &entry, - &manifest, - &manifest_ref, - host_config_by_id.get(&command.id), - ) + PluginInspectView { + entry: &entry, + manifest: &manifest, + manifest_ref: &manifest_ref, + host_config: host_config_by_id.get(&command.id), + } ); } Ok(()) @@ -495,6 +506,170 @@ fn plugin_refused( } } +struct PluginListView<'a> { + records: &'a [ScopedDynamicPluginRecord], + host_config_by_id: &'a HashMap, +} + +impl fmt::Display for PluginListView<'_> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let rows = self + .records + .iter() + .map(|entry| PluginListRow { + id: entry.record.metadata.id.as_str(), + scope: entry.scope.to_string(), + enabled: entry.record.spec.enabled.to_string(), + state: lifecycle_state_label(&entry.record).into(), + validation: <&'static str>::from(entry.record.status.validation.manifest).into(), + host_config: host_config_status( + self.host_config_by_id.get(&entry.record.metadata.id), + ), + }) + .collect::>(); + let widths = PluginListWidths::from_rows(&rows); + + write!( + f, + "{: { + id: &'a str, + scope: String, + enabled: String, + state: String, + validation: String, + host_config: String, +} + +struct PluginListWidths { + id: usize, + scope: usize, + enabled: usize, + state: usize, + validation: usize, +} + +impl PluginListWidths { + fn from_rows(rows: &[PluginListRow<'_>]) -> Self { + Self { + id: column_width("ID", rows.iter().map(|row| row.id)), + scope: column_width("SCOPE", rows.iter().map(|row| row.scope.as_str())), + enabled: column_width("ENABLED", rows.iter().map(|row| row.enabled.as_str())), + state: column_width("STATE", rows.iter().map(|row| row.state.as_str())), + validation: column_width("VALIDATION", rows.iter().map(|row| row.validation.as_str())), + } + } +} + +fn column_width<'a>(header: &'static str, values: impl Iterator) -> usize { + values + .map(str::len) + .chain(std::iter::once(header.len())) + .max() + .unwrap_or(header.len()) +} + +struct PluginInspectView<'a> { + entry: &'a ScopedDynamicPluginRecord, + manifest: &'a DynamicPluginManifest, + manifest_ref: &'a str, + host_config: Option<&'a ResolvedDynamicPluginConfig>, +} + +impl fmt::Display for PluginInspectView<'_> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let view = inspect_data( + self.entry, + self.manifest, + self.manifest_ref, + self.host_config, + ); + let yaml = serde_yaml::to_string(&view).map_err(|_| fmt::Error)?; + write!(f, "{}", yaml.trim_end()) + } +} + +struct PluginValidationSummaryView<'a> { + manifest: &'a DynamicPluginManifest, + manifest_ref: &'a str, + entry: Option<&'a ScopedDynamicPluginRecord>, + host_config: Option<&'a ResolvedDynamicPluginConfig>, +} + +impl fmt::Display for PluginValidationSummaryView<'_> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + writeln!(f, "Dynamic plugin '{}' is valid.", self.manifest.plugin.id)?; + writeln!(f, "kind: {}", self.manifest.plugin.kind)?; + if let Some(entry) = self.entry { + writeln!(f, "manifest: {}", self.manifest_ref)?; + writeln!(f, "scope: {}", entry.scope)?; + writeln!(f, "lifecycle_state_path: {}", entry.state_path.display())?; + writeln!(f, "desired.enabled: {}", entry.record.spec.enabled)?; + write!(f, "host_config: {}", host_config_status(self.host_config))?; + } else { + write!(f, "manifest: {}", self.manifest_ref)?; + } + Ok(()) + } +} + +fn lifecycle_state_label(record: &DynamicPluginRecord) -> &'static str { + if record.is_tombstoned() { + "tombstoned" + } else { + record.status.runtime.state.into() + } +} + +fn host_config_status(host_config: Option<&ResolvedDynamicPluginConfig>) -> String { + host_config + .map(|plugin| plugin.host_config_status().to_string()) + .unwrap_or_else(|| "missing".into()) +} + +fn redacted_host_config_json(host_config: &ResolvedDynamicPluginConfig) -> Value { + Value::Object( + host_config + .config + .keys() + .cloned() + .map(|key| (key, Value::String("".into()))) + .collect(), + ) +} + #[cfg(test)] #[path = "../../tests/coverage/plugins_lifecycle_tests.rs"] mod tests; diff --git a/crates/cli/src/plugins/lifecycle/render.rs b/crates/cli/src/plugins/lifecycle/render.rs deleted file mode 100644 index 7fce652e..00000000 --- a/crates/cli/src/plugins/lifecycle/render.rs +++ /dev/null @@ -1,359 +0,0 @@ -// SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. -// SPDX-License-Identifier: Apache-2.0 - -use std::collections::HashMap; -use std::fmt::{self, Write as _}; - -use nemo_relay::plugin::dynamic::{ - DynamicPluginCompatibility, DynamicPluginLoadContract, DynamicPluginManifest, - DynamicPluginRecord, -}; -use serde_json::Value; - -use crate::config::ResolvedDynamicPluginConfig; - -use super::state::ScopedDynamicPluginRecord; - -pub(super) fn render_list( - records: &[ScopedDynamicPluginRecord], - host_config_by_id: &HashMap, -) -> String { - let mut output = String::new(); - push_line( - &mut output, - format_args!( - "{:<32} {:<8} {:<7} {:<10} {:<10} {}", - "ID", "SCOPE", "ENABLED", "STATE", "VALIDATION", "HOST CONFIG" - ), - ); - for entry in records { - let host_config_status = host_config_by_id - .get(&entry.record.metadata.id) - .map(|plugin| plugin.host_config_status().to_string()) - .unwrap_or_else(|| "missing".into()); - push_line( - &mut output, - format_args!( - "{:<32} {:<8} {:<7} {:<10} {:<10} {}", - entry.record.metadata.id, - entry.scope, - entry.record.spec.enabled, - lifecycle_state_label(&entry.record), - <&'static str>::from(entry.record.status.validation.manifest), - host_config_status - ), - ); - } - finish_output(output) -} - -pub(super) fn render_inspect( - entry: &ScopedDynamicPluginRecord, - manifest: &DynamicPluginManifest, - manifest_ref: &str, - host_config: Option<&ResolvedDynamicPluginConfig>, -) -> String { - let record = &entry.record; - let mut output = String::new(); - - push_line(&mut output, format_args!("id: {}", record.metadata.id)); - push_line(&mut output, format_args!("scope: {}", entry.scope)); - push_line(&mut output, format_args!("kind: {}", record.metadata.kind)); - push_line( - &mut output, - format_args!( - "name: {}", - record.metadata.name.as_deref().unwrap_or("") - ), - ); - push_line( - &mut output, - format_args!( - "version: {}", - record.metadata.version.as_deref().unwrap_or("") - ), - ); - push_line(&mut output, format_args!("manifest: {manifest_ref}")); - push_line( - &mut output, - format_args!("plugins_toml: {}", entry.plugins_toml_path.display()), - ); - push_line( - &mut output, - format_args!("lifecycle_state_path: {}", entry.state_path.display()), - ); - push_line( - &mut output, - format_args!( - "source.manifest_ref: {}", - record.source.manifest_ref.as_deref().unwrap_or("") - ), - ); - push_line( - &mut output, - format_args!( - "source.artifact_ref: {}", - record.source.artifact_ref.as_deref().unwrap_or("") - ), - ); - push_line( - &mut output, - format_args!( - "source.environment_ref: {}", - record.source.environment_ref.as_deref().unwrap_or("") - ), - ); - push_line( - &mut output, - format_args!("desired.present: {}", record.spec.present), - ); - push_line( - &mut output, - format_args!("desired.enabled: {}", record.spec.enabled), - ); - push_line( - &mut output, - format_args!("generation: {}", record.metadata.generation), - ); - push_line( - &mut output, - format_args!("reconciled: {}", record.is_reconciled()), - ); - push_line( - &mut output, - format_args!( - "host_config: {}", - host_config - .map(|plugin| plugin.host_config_status().to_string()) - .unwrap_or_else(|| "missing".into()) - ), - ); - - match &record.compatibility { - DynamicPluginCompatibility::Worker(compatibility) => { - push_line( - &mut output, - format_args!("compat.relay: {}", compatibility.relay), - ); - push_line( - &mut output, - format_args!("compat.worker_protocol: {}", compatibility.worker_protocol), - ); - } - DynamicPluginCompatibility::RustDynamic(compatibility) => { - push_line( - &mut output, - format_args!("compat.relay: {}", compatibility.relay), - ); - push_line( - &mut output, - format_args!("compat.native_api: {}", compatibility.native_api), - ); - } - } - - match &record.load { - DynamicPluginLoadContract::Worker(load) => { - push_line(&mut output, format_args!("load.runtime: {}", load.runtime)); - push_line( - &mut output, - format_args!("load.entrypoint: {}", load.entrypoint), - ); - } - DynamicPluginLoadContract::RustDynamic(load) => { - push_line(&mut output, format_args!("load.library: {}", load.library)); - push_line(&mut output, format_args!("load.symbol: {}", load.symbol)); - } - } - - render_status(&mut output, record); - push_line( - &mut output, - format_args!( - "capabilities: {}", - manifest - .capabilities - .items - .iter() - .map(ToString::to_string) - .collect::>() - .join(", ") - ), - ); - push_line( - &mut output, - format_args!( - "host_config_json: {}", - host_config - .map(redacted_host_config_json) - .filter(|config| !config.is_null()) - .map(|config| { - serde_json::to_string_pretty(&config).expect("host config serializes") - }) - .unwrap_or_else(|| "".into()) - ), - ); - - finish_output(output) -} - -pub(super) fn render_validation_summary( - manifest: &DynamicPluginManifest, - manifest_ref: &str, - entry: Option<&ScopedDynamicPluginRecord>, - host_config: Option<&ResolvedDynamicPluginConfig>, -) -> String { - let mut output = String::new(); - push_line( - &mut output, - format_args!("Dynamic plugin '{}' is valid.", manifest.plugin.id), - ); - push_line(&mut output, format_args!("kind: {}", manifest.plugin.kind)); - push_line(&mut output, format_args!("manifest: {manifest_ref}")); - if let Some(entry) = entry { - push_line(&mut output, format_args!("scope: {}", entry.scope)); - push_line( - &mut output, - format_args!("lifecycle_state_path: {}", entry.state_path.display()), - ); - push_line( - &mut output, - format_args!("desired.enabled: {}", entry.record.spec.enabled), - ); - push_line( - &mut output, - format_args!( - "host_config: {}", - host_config - .map(|plugin| plugin.host_config_status().to_string()) - .unwrap_or_else(|| "missing".into()) - ), - ); - } - finish_output(output) -} - -fn render_status(output: &mut String, record: &DynamicPluginRecord) { - push_line( - output, - format_args!( - "status.validation.manifest: {}", - <&'static str>::from(record.status.validation.manifest) - ), - ); - push_line( - output, - format_args!( - "status.validation.compatibility: {}", - <&'static str>::from(record.status.validation.compatibility) - ), - ); - push_line( - output, - format_args!( - "status.validation.integrity: {}", - <&'static str>::from(record.status.validation.integrity) - ), - ); - push_line( - output, - format_args!( - "status.validation.environment: {}", - <&'static str>::from(record.status.validation.environment) - ), - ); - push_line( - output, - format_args!( - "status.validation.authenticity: {}", - <&'static str>::from(record.status.validation.authenticity) - ), - ); - push_line( - output, - format_args!( - "status.validation.policy_satisfied: {}", - <&'static str>::from(record.status.validation.policy_satisfied) - ), - ); - push_line( - output, - format_args!( - "status.runtime.state: {}", - <&'static str>::from(record.status.runtime.state) - ), - ); - push_line( - output, - format_args!( - "status.runtime.observed_generation: {}", - record.status.runtime.observed_generation - ), - ); - if let Some(value) = record.status.validation.checked_at.as_deref() { - push_line( - output, - format_args!("status.validation.checked_at: {value}"), - ); - } - if let Some(value) = record.status.validation.message.as_deref() { - push_line(output, format_args!("status.validation.message: {value}")); - } - if let Some(value) = record.status.runtime.started_at.as_deref() { - push_line(output, format_args!("status.runtime.started_at: {value}")); - } - if let Some(value) = record.status.runtime.updated_at.as_deref() { - push_line(output, format_args!("status.runtime.updated_at: {value}")); - } - if let Some(value) = record.status.runtime.message.as_deref() { - push_line(output, format_args!("status.runtime.message: {value}")); - } - if let Some(value) = record.status.startup_class { - push_line(output, format_args!("status.startup_class: {}", value)); - } - if let Some(value) = record.status.attestation_mode { - push_line(output, format_args!("status.attestation_mode: {}", value)); - } - if let Some(error) = record.status.last_error.as_ref() { - push_line( - output, - format_args!( - "status.last_error: {}:{} {}", - error.phase, error.code, error.message - ), - ); - } -} - -fn push_line(output: &mut String, args: fmt::Arguments<'_>) { - output.write_fmt(args).expect("writing to string succeeds"); - output.push('\n'); -} - -fn finish_output(mut output: String) -> String { - output.pop(); - output -} - -fn lifecycle_state_label(record: &DynamicPluginRecord) -> &'static str { - if record.is_tombstoned() { - "tombstoned" - } else { - record.status.runtime.state.into() - } -} - -pub(super) fn redacted_host_config_json(host_config: &ResolvedDynamicPluginConfig) -> Value { - if host_config.config.is_empty() { - return Value::Null; - } - - Value::Object( - host_config - .config - .keys() - .cloned() - .map(|key| (key, Value::String("".into()))) - .collect(), - ) -} diff --git a/crates/cli/src/plugins/lifecycle/responses.rs b/crates/cli/src/plugins/lifecycle/responses.rs index a72263a0..506d7e02 100644 --- a/crates/cli/src/plugins/lifecycle/responses.rs +++ b/crates/cli/src/plugins/lifecycle/responses.rs @@ -19,8 +19,8 @@ use serde_json::{Map, Value}; use crate::config::ResolvedDynamicPluginConfig; use crate::error::{CliError, PluginLifecycleFailureKind}; -use super::render::redacted_host_config_json; use super::state::ScopedDynamicPluginRecord; +use super::{host_config_status, redacted_host_config_json}; #[derive(Debug)] pub(super) struct ValidateResponseInput<'a> { @@ -153,10 +153,7 @@ pub(super) fn list_success( code: error.code.clone(), message: error.message.clone(), }), - host_config: host_config_by_id - .get(&record.metadata.id) - .map(|plugin| plugin.host_config_status().to_string()) - .unwrap_or_else(|| "missing".into()), + host_config: host_config_status(host_config_by_id.get(&record.metadata.id)), } }) .collect(), @@ -171,46 +168,52 @@ pub(super) fn inspect_success( manifest_ref: &str, host_config: Option<&ResolvedDynamicPluginConfig>, ) -> ResponseEnvelope { - let record = &entry.record; success( command, Some(target), - InspectResponse { - id: record.metadata.id.clone(), - name: record.metadata.name.clone(), - kind: record.metadata.kind, - scope: entry.scope, - manifest_ref: manifest_ref.into(), - plugins_toml_path: entry.plugins_toml_path.display().to_string(), - state_path: entry.state_path.display().to_string(), - load: serde_json::to_value(&record.load) - .expect("dynamic plugin load contract serializes to JSON"), - compat: serde_json::to_value(&record.compatibility) - .expect("dynamic plugin compatibility serializes to JSON"), - capabilities: manifest - .capabilities - .items - .iter() - .map(ToString::to_string) - .collect(), - metadata: serde_json::to_value(&record.metadata) - .expect("dynamic plugin metadata serializes to JSON"), - source: serde_json::to_value(&record.source) - .expect("dynamic plugin source serializes to JSON"), - spec: serde_json::to_value(&record.spec) - .expect("dynamic plugin spec serializes to JSON"), - status: serde_json::to_value(&record.status) - .expect("dynamic plugin status serializes to JSON"), - host_config_status: host_config - .map(|plugin| plugin.host_config_status().to_string()) - .unwrap_or_else(|| "missing".into()), - host_config: host_config - .map(redacted_host_config_json) - .unwrap_or(Value::Null), - }, + inspect_data(entry, manifest, manifest_ref, host_config), ) } +pub(super) fn inspect_data( + entry: &ScopedDynamicPluginRecord, + manifest: &DynamicPluginManifest, + manifest_ref: &str, + host_config: Option<&ResolvedDynamicPluginConfig>, +) -> InspectResponse { + let record = &entry.record; + InspectResponse { + id: record.metadata.id.clone(), + name: record.metadata.name.clone(), + kind: record.metadata.kind, + scope: entry.scope, + manifest_ref: manifest_ref.into(), + plugins_toml_path: entry.plugins_toml_path.display().to_string(), + state_path: entry.state_path.display().to_string(), + load: serde_json::to_value(&record.load) + .expect("dynamic plugin load contract serializes to JSON"), + compat: serde_json::to_value(&record.compatibility) + .expect("dynamic plugin compatibility serializes to JSON"), + capabilities: manifest + .capabilities + .items + .iter() + .map(ToString::to_string) + .collect(), + metadata: serde_json::to_value(&record.metadata) + .expect("dynamic plugin metadata serializes to JSON"), + source: serde_json::to_value(&record.source) + .expect("dynamic plugin source serializes to JSON"), + spec: serde_json::to_value(&record.spec).expect("dynamic plugin spec serializes to JSON"), + status: serde_json::to_value(&record.status) + .expect("dynamic plugin status serializes to JSON"), + host_config_status: host_config_status(host_config), + host_config: host_config + .map(redacted_host_config_json) + .unwrap_or(Value::Null), + } +} + pub(super) fn validate_success( input: ValidateResponseInput<'_>, ) -> ResponseEnvelope { @@ -236,10 +239,7 @@ pub(super) fn validate_success( manifest_ref: input.manifest_ref.into(), kind: input.manifest.plugin.kind, desired_enabled: input.entry.map(|entry| entry.record.spec.enabled), - host_config_status: input - .host_config - .map(|plugin| plugin.host_config_status().to_string()) - .unwrap_or_else(|| "missing".into()), + host_config_status: host_config_status(input.host_config), }, ) } diff --git a/crates/cli/tests/coverage/launcher_tests.rs b/crates/cli/tests/coverage/launcher_tests.rs index 34235383..dd1bd619 100644 --- a/crates/cli/tests/coverage/launcher_tests.rs +++ b/crates/cli/tests/coverage/launcher_tests.rs @@ -1151,6 +1151,7 @@ async fn execute_live_run_reports_gateway_startup_error_when_health_check_fails( let resolved = ResolvedConfig { gateway: GatewayConfig::default(), agents: AgentConfigs::default(), + ..ResolvedConfig::default() }; let prepared = PreparedRun::new( CodingAgent::ClaudeCode, diff --git a/crates/cli/tests/coverage/plugins_lifecycle_tests.rs b/crates/cli/tests/coverage/plugins_lifecycle_tests.rs index 20db9e75..e5c26970 100644 --- a/crates/cli/tests/coverage/plugins_lifecycle_tests.rs +++ b/crates/cli/tests/coverage/plugins_lifecycle_tests.rs @@ -234,7 +234,11 @@ fn list_and_inspect_render_discovered_dynamic_plugins() { let host_config_by_id = host_config_by_id(&resolved); let scopes = load_and_hydrate_scopes(None, &resolved).unwrap(); let records = collect_records(&scopes, false); - let list = render_list(&records, &host_config_by_id); + let list = PluginListView { + records: &records, + host_config_by_id: &host_config_by_id, + } + .to_string(); assert!(list.contains("acme.guardrail")); assert!(list.contains("absent")); assert!(list.contains("false")); @@ -246,19 +250,30 @@ fn list_and_inspect_render_discovered_dynamic_plugins() { DynamicPluginManifest::load_from_path(entry.record.source.manifest_ref.clone().unwrap()) .map_err(|error| CliError::Config(error.to_string())) .unwrap(); - let inspect = render_inspect( - &entry, - &manifest, - &manifest_ref, - host_config_by_id.get("acme.guardrail"), + let inspect = PluginInspectView { + entry: &entry, + manifest: &manifest, + manifest_ref: &manifest_ref, + host_config: host_config_by_id.get("acme.guardrail"), + } + .to_string(); + let inspect_value: serde_yaml::Value = serde_yaml::from_str(&inspect).unwrap(); + assert_eq!( + inspect_value["metadata"]["id"].as_str(), + Some("acme.guardrail") + ); + assert_eq!(inspect_value["metadata"]["kind"].as_str(), Some("worker")); + assert_eq!(inspect_value["host_config_status"].as_str(), Some("absent")); + assert!( + inspect_value["source"]["manifest_ref"] + .as_str() + .unwrap() + .contains("relay-plugin.toml") + ); + assert_eq!( + inspect_value["load"]["entrypoint"].as_str(), + Some("acme.guardrail.plugin:register") ); - assert!(inspect.contains("id: acme.guardrail")); - assert!(inspect.contains("kind: worker")); - assert!(inspect.contains("host_config: absent")); - assert!(inspect.contains("source.manifest_ref:")); - assert!(inspect.contains("source.artifact_ref: ")); - assert!(inspect.contains("source.environment_ref: ")); - assert!(inspect.contains("load.entrypoint: acme.guardrail.plugin:register")); } #[test] @@ -285,22 +300,28 @@ fn validate_renders_summary_for_path_and_id_targets() { let (manifest, manifest_ref) = DynamicPluginManifest::load_from_path(&manifest_path) .map_err(|error| CliError::Config(error.to_string())) .unwrap(); - let path_summary = render_validation_summary(&manifest, &manifest_ref, None, None); + let path_summary = PluginValidationSummaryView { + manifest: &manifest, + manifest_ref: &manifest_ref, + entry: None, + host_config: None, + } + .to_string(); assert!(path_summary.contains("Dynamic plugin 'acme.guardrail' is valid.")); let resolved = resolve_plugins_config(None).unwrap(); let host_config_by_id = host_config_by_id(&resolved); let scopes = load_and_hydrate_scopes(None, &resolved).unwrap(); - let id_summary = render_validation_summary( - &manifest, - &manifest_ref, - Some( - &find_record_by_id(&scopes, "acme.guardrail") - .unwrap() - .expect("plugin record"), - ), - host_config_by_id.get("acme.guardrail"), - ); + let entry = find_record_by_id(&scopes, "acme.guardrail") + .unwrap() + .expect("plugin record"); + let id_summary = PluginValidationSummaryView { + manifest: &manifest, + manifest_ref: &manifest_ref, + entry: Some(&entry), + host_config: host_config_by_id.get("acme.guardrail"), + } + .to_string(); assert!(id_summary.contains("host_config: absent")); assert!(id_summary.contains("desired.enabled: false")); @@ -403,7 +424,12 @@ fn enable_disable_and_remove_persist_lifecycle_state() { assert!(removed.record.is_tombstoned()); let all_records = collect_records(&scopes, true); - let all_list = render_list(&all_records, &host_config_by_id(&resolved)); + let host_config_by_id = host_config_by_id(&resolved); + let all_list = PluginListView { + records: &all_records, + host_config_by_id: &host_config_by_id, + } + .to_string(); assert!(all_list.contains("acme.guardrail")); assert!(all_list.contains("tombstoned")); @@ -819,14 +845,23 @@ fn inspect_redacts_host_config_values() { .map_err(|error| CliError::Config(error.to_string())) .unwrap(); - let inspect_output = render_inspect( - &entry, - &manifest, - &manifest_ref, - host_config_by_id.get("acme.redacted"), - ); + let inspect_output = PluginInspectView { + entry: &entry, + manifest: &manifest, + manifest_ref: &manifest_ref, + host_config: host_config_by_id.get("acme.redacted"), + } + .to_string(); assert!(!inspect_output.contains("secret-token")); - assert!(inspect_output.contains("")); + let inspect_output: serde_yaml::Value = serde_yaml::from_str(&inspect_output).unwrap(); + assert_eq!( + inspect_output["host_config"]["api_key"].as_str(), + Some("") + ); + assert_eq!( + inspect_output["host_config"]["region"].as_str(), + Some("") + ); let inspect_value = serde_json::to_value(responses::inspect_success( "plugins inspect", @@ -847,3 +882,84 @@ fn inspect_redacts_host_config_values() { ); assert_eq!(inspect_value["data"]["host_config_status"], "present"); } + +#[test] +fn inspect_distinguishes_empty_host_config_from_missing_host_config() { + let temp = tempfile::tempdir().unwrap(); + let _env = EnvScope::hermetic(&temp); + let _cwd = CurrentDirGuard::enter(temp.path()); + let plugin_dir = temp.path().join("plugins").join("acme"); + std::fs::create_dir_all(&plugin_dir).unwrap(); + let manifest_path = write_dynamic_manifest(&plugin_dir, "acme.empty-config"); + let server = ServerArgs::default(); + + add( + PluginsAddCommand { + scope: PluginsScopeArgs { + project: true, + ..PluginsScopeArgs::default() + }, + path: plugin_dir, + }, + &server, + ) + .unwrap(); + + let plugins_toml = temp.path().join(".nemo-relay").join("plugins.toml"); + std::fs::write( + &plugins_toml, + format!( + "[[plugins.dynamic]]\nmanifest = {:?}\nconfig = {{}}\n", + manifest_path.to_string_lossy() + ), + ) + .unwrap(); + + let resolved = resolve_plugins_config(None).unwrap(); + let host_config_by_id = host_config_by_id(&resolved); + let scopes = load_and_hydrate_scopes(None, &resolved).unwrap(); + let entry = find_record_by_id(&scopes, "acme.empty-config") + .unwrap() + .expect("empty-config record"); + let (manifest, manifest_ref) = DynamicPluginManifest::load_from_path(&manifest_path) + .map_err(|error| CliError::Config(error.to_string())) + .unwrap(); + + let inspect_output = PluginInspectView { + entry: &entry, + manifest: &manifest, + manifest_ref: &manifest_ref, + host_config: host_config_by_id.get("acme.empty-config"), + } + .to_string(); + let inspect_output: serde_yaml::Value = serde_yaml::from_str(&inspect_output).unwrap(); + assert_eq!( + inspect_output["host_config_status"].as_str(), + Some("present") + ); + assert_eq!( + inspect_output["host_config"] + .as_mapping() + .expect("empty host config should render as an object") + .len(), + 0 + ); + + let inspect_value = serde_json::to_value(responses::inspect_success( + "plugins inspect", + "acme.empty-config", + &entry, + &manifest, + &manifest_ref, + host_config_by_id.get("acme.empty-config"), + )) + .unwrap(); + assert_eq!(inspect_value["data"]["host_config_status"], "present"); + assert_eq!( + inspect_value["data"]["host_config"] + .as_object() + .expect("empty host config should serialize as an object") + .len(), + 0 + ); +} From 44411df7cf553e0531a0914f682ad72fc191eaad Mon Sep 17 00:00:00 2001 From: Alex Fournier Date: Wed, 24 Jun 2026 11:05:25 -0700 Subject: [PATCH 19/20] test: fix stale launcher config initializers Signed-off-by: Alex Fournier --- crates/cli/tests/coverage/launcher_tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/cli/tests/coverage/launcher_tests.rs b/crates/cli/tests/coverage/launcher_tests.rs index f524643b..e04a6f0c 100644 --- a/crates/cli/tests/coverage/launcher_tests.rs +++ b/crates/cli/tests/coverage/launcher_tests.rs @@ -574,7 +574,7 @@ fn prepares_hermes_hook_environment() { }, ..AgentConfigs::default() }, - ..ResolvedConfig::default() + dynamic_plugins: Vec::new(), }; let prepared = PreparedRun::new( CodingAgent::Hermes, From 0cd15e92ef68bf3e229c0018847a10e7146f2c09 Mon Sep 17 00:00:00 2001 From: Alex Fournier Date: Wed, 24 Jun 2026 12:29:49 -0700 Subject: [PATCH 20/20] fix: restore lifecycle inspect and host config semantics Signed-off-by: Alex Fournier --- crates/cli/src/config.rs | 12 ++++--- crates/cli/src/plugins/lifecycle.rs | 34 +++++++++++++++++-- crates/cli/src/plugins/lifecycle/responses.rs | 10 +++--- 3 files changed, 44 insertions(+), 12 deletions(-) diff --git a/crates/cli/src/config.rs b/crates/cli/src/config.rs index 9792a3dd..33857ae0 100644 --- a/crates/cli/src/config.rs +++ b/crates/cli/src/config.rs @@ -597,6 +597,7 @@ pub(crate) struct ResolvedDynamicPluginConfig { pub(crate) plugin_id: String, pub(crate) manifest_ref: String, pub(crate) config: Map, + pub(crate) has_explicit_config: bool, pub(crate) source: PathBuf, } @@ -610,10 +611,10 @@ pub(crate) enum DynamicPluginHostConfigStatus { impl ResolvedDynamicPluginConfig { pub(crate) fn host_config_status(&self) -> DynamicPluginHostConfigStatus { - if self.config.is_empty() { - DynamicPluginHostConfigStatus::Absent - } else { + if self.has_explicit_config { DynamicPluginHostConfigStatus::Present + } else { + DynamicPluginHostConfigStatus::Absent } } } @@ -1054,7 +1055,7 @@ struct PluginTomlPluginsSection { struct FileDynamicPluginConfig { manifest: String, #[serde(default)] - config: Map, + config: Option>, } fn load_plugin_toml_config( @@ -1185,7 +1186,8 @@ fn resolve_dynamic_plugin_refs( resolved.push(ResolvedDynamicPluginConfig { plugin_id, manifest_ref, - config: dynamic.config, + has_explicit_config: dynamic.config.is_some(), + config: dynamic.config.unwrap_or_default(), source: source.to_path_buf(), }); } diff --git a/crates/cli/src/plugins/lifecycle.rs b/crates/cli/src/plugins/lifecycle.rs index 6c6cfc96..ce6160c9 100644 --- a/crates/cli/src/plugins/lifecycle.rs +++ b/crates/cli/src/plugins/lifecycle.rs @@ -7,8 +7,8 @@ use std::path::PathBuf; use std::process::ExitCode; use nemo_relay::plugin::dynamic::{ - DynamicPluginCheckState, DynamicPluginManifest, DynamicPluginRecord, - DynamicPluginValidationStatus, + DynamicPluginCheckState, DynamicPluginCompatibility, DynamicPluginLoadContract, + DynamicPluginManifest, DynamicPluginRecord, DynamicPluginValidationStatus, }; use serde_json::Value; @@ -660,6 +660,10 @@ fn host_config_status(host_config: Option<&ResolvedDynamicPluginConfig>) -> Stri } fn redacted_host_config_json(host_config: &ResolvedDynamicPluginConfig) -> Value { + if host_config.config.is_empty() && !host_config.has_explicit_config { + return Value::Null; + } + Value::Object( host_config .config @@ -670,6 +674,32 @@ fn redacted_host_config_json(host_config: &ResolvedDynamicPluginConfig) -> Value ) } +pub(super) fn inspect_load_data(record: &DynamicPluginRecord) -> Value { + match &record.load { + DynamicPluginLoadContract::Worker(load) => serde_json::json!({ + "runtime": load.runtime, + "entrypoint": load.entrypoint, + }), + DynamicPluginLoadContract::RustDynamic(load) => serde_json::json!({ + "library": load.library, + "symbol": load.symbol, + }), + } +} + +pub(super) fn inspect_compat_data(record: &DynamicPluginRecord) -> Value { + match &record.compatibility { + DynamicPluginCompatibility::Worker(compatibility) => serde_json::json!({ + "relay": compatibility.relay, + "worker_protocol": compatibility.worker_protocol, + }), + DynamicPluginCompatibility::RustDynamic(compatibility) => serde_json::json!({ + "relay": compatibility.relay, + "native_api": compatibility.native_api, + }), + } +} + #[cfg(test)] #[path = "../../tests/coverage/plugins_lifecycle_tests.rs"] mod tests; diff --git a/crates/cli/src/plugins/lifecycle/responses.rs b/crates/cli/src/plugins/lifecycle/responses.rs index 506d7e02..a01eecd4 100644 --- a/crates/cli/src/plugins/lifecycle/responses.rs +++ b/crates/cli/src/plugins/lifecycle/responses.rs @@ -20,7 +20,9 @@ use crate::config::ResolvedDynamicPluginConfig; use crate::error::{CliError, PluginLifecycleFailureKind}; use super::state::ScopedDynamicPluginRecord; -use super::{host_config_status, redacted_host_config_json}; +use super::{ + host_config_status, inspect_compat_data, inspect_load_data, redacted_host_config_json, +}; #[derive(Debug)] pub(super) struct ValidateResponseInput<'a> { @@ -190,10 +192,8 @@ pub(super) fn inspect_data( manifest_ref: manifest_ref.into(), plugins_toml_path: entry.plugins_toml_path.display().to_string(), state_path: entry.state_path.display().to_string(), - load: serde_json::to_value(&record.load) - .expect("dynamic plugin load contract serializes to JSON"), - compat: serde_json::to_value(&record.compatibility) - .expect("dynamic plugin compatibility serializes to JSON"), + load: inspect_load_data(record), + compat: inspect_compat_data(record), capabilities: manifest .capabilities .items