From cfa7a7f45c1efbac9e97a0c7b073dea301971466 Mon Sep 17 00:00:00 2001 From: Brian Cooper Date: Thu, 26 Mar 2026 22:38:45 -0400 Subject: [PATCH 1/2] feat(plugin): implement install command and harden plugin resolution - Add install command with embedded plugin registry and package manager detection (Homebrew, AUR via yay/paru) - Scope PATH fallback to `omni-` prefixed binaries to prevent arbitrary binary execution - Add `${VAR}` env var expansion in manifest endpoint fields - Add changeset for minor version bump --- .changeset/plugin-install.md | 5 + src/main.rs | 11 +- src/plugin/discovery.rs | 13 ++- src/plugin/install.rs | 189 +++++++++++++++++++++++++++++++++++ src/plugin/manifest.rs | 40 +++++++- src/plugin/mod.rs | 2 + tests/plugin_integration.rs | 7 +- 7 files changed, 250 insertions(+), 17 deletions(-) create mode 100644 .changeset/plugin-install.md create mode 100644 src/plugin/install.rs diff --git a/.changeset/plugin-install.md b/.changeset/plugin-install.md new file mode 100644 index 0000000..977149f --- /dev/null +++ b/.changeset/plugin-install.md @@ -0,0 +1,5 @@ +--- +"@omnidotdev/cli": minor +--- + +Implement plugin install command with embedded registry, package manager detection (Homebrew, AUR), env var expansion in manifest endpoints, and scoped PATH fallback to `omni-` prefixed binaries diff --git a/src/main.rs b/src/main.rs index 19bf6c5..ce4dc3c 100644 --- a/src/main.rs +++ b/src/main.rs @@ -8,7 +8,7 @@ use omni_cli::{ Config, cli::{AuthCommands, Cli, Commands, ConfigCommands, SessionCommands, SynapseCommands}, core::session::SessionTarget, - plugin::{PluginDiscovery, PluginType, run_plugin_command}, + plugin::{PluginDiscovery, PluginType, install_plugin, run_plugin_command}, }; #[tokio::main] @@ -147,7 +147,7 @@ async fn run(cli: Cli) -> anyhow::Result<()> { Commands::Install { plugins } => { for name in &plugins { - println!("Installing plugin '{name}' is not yet implemented"); + install_plugin(name)?; } } @@ -388,14 +388,15 @@ fn handle_external_command(args: &[String]) -> anyhow::Result { if let Some(manifest) = &plugin.manifest { run_plugin_command(manifest, &remaining) } else { - // PATH-only fallback: run directly - let status = std::process::Command::new(name) + // PATH-only fallback: run omni- binary + let bin = format!("omni-{name}"); + let status = std::process::Command::new(&bin) .args(&remaining) .stdin(std::process::Stdio::inherit()) .stdout(std::process::Stdio::inherit()) .stderr(std::process::Stdio::inherit()) .status() - .map_err(|e| anyhow::anyhow!("failed to execute '{name}': {e}"))?; + .map_err(|e| anyhow::anyhow!("failed to execute '{bin}': {e}"))?; Ok(status.code().unwrap_or(1)) } } diff --git a/src/plugin/discovery.rs b/src/plugin/discovery.rs index 2a47dbd..d1b7df1 100644 --- a/src/plugin/discovery.rs +++ b/src/plugin/discovery.rs @@ -60,8 +60,9 @@ impl PluginDiscovery { })); } - // Fall back to PATH - if which::which(name).is_ok() { + // Fall back to PATH with `omni-` prefix convention (like git/cargo) + let prefixed = format!("omni-{name}"); + if which::which(&prefixed).is_ok() { return Ok(Some(DiscoveredPlugin { name: name.to_string(), manifest: None, @@ -169,14 +170,12 @@ endpoint = "https://runa.omni.dev/api" } #[test] - fn find_falls_back_to_path() { + fn find_does_not_fall_back_to_unprefixed_path() { let dir = tempfile::TempDir::new().unwrap(); let discovery = PluginDiscovery::new(dir.path().to_path_buf()); + // "ls" exists on PATH but "omni-ls" does not let plugin = discovery.find("ls").unwrap(); - assert!(plugin.is_some()); - let p = plugin.unwrap(); - assert_eq!(p.source, PluginSource::Path); - assert!(p.manifest.is_none()); + assert!(plugin.is_none()); } #[test] diff --git a/src/plugin/install.rs b/src/plugin/install.rs new file mode 100644 index 0000000..7660c94 --- /dev/null +++ b/src/plugin/install.rs @@ -0,0 +1,189 @@ +use std::path::Path; + +use crate::plugin::{PluginDiscovery, PluginManifest}; + +/// Embedded plugin registry (known Omni ecosystem plugins) +const REGISTRY: &[(&str, &str)] = &[( + "beacon", + include_str!("../../examples/plugins/beacon/plugin.toml"), +)]; + +/// Detect the available system package manager +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum PackageManager { + Homebrew, + Aur, +} + +impl PackageManager { + fn detect() -> Option { + if which::which("brew").is_ok() { + return Some(Self::Homebrew); + } + + // Prefer AUR helpers over raw pacman + if which::which("yay").is_ok() || which::which("paru").is_ok() { + return Some(Self::Aur); + } + + None + } + + fn install_cmd(self, package: &str) -> Vec { + match self { + Self::Homebrew => vec!["brew".into(), "install".into(), package.into()], + Self::Aur => { + let helper = if which::which("paru").is_ok() { + "paru" + } else { + "yay" + }; + vec![ + helper.into(), + "-S".into(), + "--noconfirm".into(), + package.into(), + ] + } + } + } + + fn package_field(self, manifest: &PluginManifest) -> Option<&str> { + let packages = manifest.packages.as_ref()?; + match self { + Self::Homebrew => packages.homebrew.as_deref(), + Self::Aur => packages.aur.as_deref(), + } + } +} + +/// Install a plugin by name +/// +/// # Errors +/// +/// Returns an error if the plugin is unknown, already installed, or installation fails +pub fn install_plugin(name: &str) -> anyhow::Result<()> { + let plugins_dir = PluginDiscovery::default_dir()?; + let target = plugins_dir.join(name); + + if target.exists() { + anyhow::bail!("plugin '{name}' is already installed"); + } + + // Look up in embedded registry + let manifest_toml = REGISTRY + .iter() + .find(|(n, _)| *n == name) + .map(|(_, toml)| *toml); + + let manifest_toml = manifest_toml.ok_or_else(|| anyhow::anyhow!("unknown plugin '{name}'"))?; + + let manifest: PluginManifest = toml::from_str(manifest_toml) + .map_err(|e| anyhow::anyhow!("invalid manifest for '{name}': {e}"))?; + + // Install binary via package manager if applicable + if manifest.bin.is_some() { + install_binary(name, &manifest)?; + } + + // Write manifest to plugins dir + write_manifest(&target, manifest_toml)?; + + println!("Installed plugin '{name}'"); + Ok(()) +} + +/// Install the binary for a plugin via system package manager +fn install_binary(name: &str, manifest: &PluginManifest) -> anyhow::Result<()> { + // Check if binary is already available + if let Some(ref bin) = manifest.bin { + if which::which(bin).is_ok() { + println!("'{bin}' is already on PATH, skipping binary install"); + return Ok(()); + } + } + + let pm = PackageManager::detect().ok_or_else(|| { + anyhow::anyhow!("no supported package manager found (brew, yay, or paru required)") + })?; + + let package = pm.package_field(manifest).ok_or_else(|| { + anyhow::anyhow!("plugin '{name}' has no package hint for the detected package manager") + })?; + + println!("Installing '{package}' via {pm:?}..."); + + let cmd = pm.install_cmd(package); + let status = std::process::Command::new(&cmd[0]) + .args(&cmd[1..]) + .stdin(std::process::Stdio::inherit()) + .stdout(std::process::Stdio::inherit()) + .stderr(std::process::Stdio::inherit()) + .status() + .map_err(|e| anyhow::anyhow!("failed to run package manager: {e}"))?; + + if !status.success() { + anyhow::bail!("package manager exited with status {status}"); + } + + Ok(()) +} + +/// Write a plugin manifest to the plugins directory +fn write_manifest(target: &Path, manifest_toml: &str) -> anyhow::Result<()> { + std::fs::create_dir_all(target)?; + std::fs::write(target.join("plugin.toml"), manifest_toml)?; + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn registry_contains_known_plugins() { + for (name, toml_str) in REGISTRY { + let manifest: PluginManifest = toml::from_str(toml_str) + .unwrap_or_else(|e| panic!("invalid manifest for '{name}': {e}")); + assert_eq!(manifest.name, *name); + } + } + + #[test] + fn detect_package_manager_returns_some_on_ci_or_dev() { + // Just verify it doesn't panic + let _ = PackageManager::detect(); + } + + #[test] + fn homebrew_install_cmd() { + let cmd = PackageManager::Homebrew.install_cmd("omnidotdev/tap/foo"); + assert_eq!(cmd, vec!["brew", "install", "omnidotdev/tap/foo"]); + } + + #[test] + fn install_already_installed_errors() { + let dir = tempfile::TempDir::new().unwrap(); + let target = dir.path().join("beacon"); + std::fs::create_dir_all(&target).unwrap(); + std::fs::write(target.join("plugin.toml"), "").unwrap(); + + // We can't easily test install_plugin since it uses default_dir(), + // but we can verify the manifest writes correctly + let result = write_manifest(&dir.path().join("new-plugin"), "name = \"test\""); + assert!(result.is_ok()); + assert!(dir.path().join("new-plugin/plugin.toml").exists()); + } + + #[test] + fn write_manifest_creates_dir_and_file() { + let dir = tempfile::TempDir::new().unwrap(); + let target = dir.path().join("test-plugin"); + write_manifest(&target, "name = \"test\"").unwrap(); + assert!(target.join("plugin.toml").exists()); + assert_eq!( + std::fs::read_to_string(target.join("plugin.toml")).unwrap(), + "name = \"test\"" + ); + } +} diff --git a/src/plugin/manifest.rs b/src/plugin/manifest.rs index 9dfa0a2..44dfb23 100644 --- a/src/plugin/manifest.rs +++ b/src/plugin/manifest.rs @@ -1,6 +1,7 @@ use std::collections::HashMap; use std::path::Path; +use regex::Regex; use serde::Deserialize; /// Plugin type determines how the CLI delegates to the plugin @@ -54,16 +55,32 @@ pub struct PluginManifest { impl PluginManifest { /// Read and parse a plugin manifest from a TOML file /// + /// Expands `${VAR}` references in the `endpoint` field + /// /// # Errors /// /// Returns an error if the file cannot be read or parsed pub fn from_file(path: &Path) -> anyhow::Result { let contents = std::fs::read_to_string(path)?; - let manifest: Self = toml::from_str(&contents)?; + let mut manifest: Self = toml::from_str(&contents)?; + + if let Some(ref ep) = manifest.endpoint { + manifest.endpoint = Some(expand_env_vars(ep)); + } + Ok(manifest) } } +/// Expand `${VAR}` references using environment variables +fn expand_env_vars(input: &str) -> String { + let re = Regex::new(r"\$\{([^}]+)\}").expect("valid regex"); + re.replace_all(input, |caps: ®ex::Captures| { + std::env::var(&caps[1]).unwrap_or_default() + }) + .into_owned() +} + #[cfg(test)] mod tests { use super::*; @@ -166,6 +183,27 @@ type = "invalid" assert!(result.is_err()); } + #[test] + fn expand_env_vars_substitutes_known_vars() { + // HOME is always set in test environments + let result = super::expand_env_vars("${HOME}/plugins"); + assert!(!result.contains("${HOME}")); + assert!(result.ends_with("/plugins")); + assert!(result.len() > "/plugins".len()); + } + + #[test] + fn expand_env_vars_missing_var_becomes_empty() { + let result = super::expand_env_vars("${NONEXISTENT_VAR_12345}/api"); + assert_eq!(result, "/api"); + } + + #[test] + fn expand_env_vars_no_vars_unchanged() { + let result = super::expand_env_vars("https://example.com/api"); + assert_eq!(result, "https://example.com/api"); + } + #[test] fn missing_required_fields_returns_error() { let bad = r#" diff --git a/src/plugin/mod.rs b/src/plugin/mod.rs index 4e0bb97..fa71a07 100644 --- a/src/plugin/mod.rs +++ b/src/plugin/mod.rs @@ -1,7 +1,9 @@ pub mod discovery; pub mod exec; +pub mod install; pub mod manifest; pub use discovery::{DiscoveredPlugin, PluginDiscovery, PluginSource}; pub use exec::{resolve_bin_and_args, run_plugin_command}; +pub use install::install_plugin; pub use manifest::{CommandDef, PackageHints, PluginManifest, PluginType}; diff --git a/tests/plugin_integration.rs b/tests/plugin_integration.rs index 65866fb..c0a4219 100644 --- a/tests/plugin_integration.rs +++ b/tests/plugin_integration.rs @@ -68,12 +68,11 @@ bin = "echo" } #[test] -fn path_fallback_when_no_manifest() { +fn path_fallback_requires_omni_prefix() { let dir = TempDir::new().unwrap(); let discovery = PluginDiscovery::new(dir.path().to_path_buf()); - // "echo" should be found on PATH + // "echo" exists on PATH but "omni-echo" does not, so no match let plugin = discovery.find("echo").unwrap(); - assert!(plugin.is_some()); - assert!(plugin.unwrap().manifest.is_none()); + assert!(plugin.is_none()); } From d864f291785ff8262f6e6bb7336a37b005f29830 Mon Sep 17 00:00:00 2001 From: Brian Cooper Date: Thu, 26 Mar 2026 23:19:14 -0400 Subject: [PATCH 2/2] feat(plugin): fetch manifests from remote registry with embedded fallback - Add registry client that fetches plugin manifests from the remote plugin registry API (configurable via OMNI_PLUGIN_REGISTRY_URL) - Fall back to embedded registry when remote is unavailable - Add Serialize derives to manifest types for TOML round-tripping - Make install_plugin async to support remote fetch --- src/main.rs | 2 +- src/plugin/install.rs | 64 ++++++++++++++----- src/plugin/manifest.rs | 10 +-- src/plugin/mod.rs | 1 + src/plugin/registry.rs | 138 +++++++++++++++++++++++++++++++++++++++++ 5 files changed, 193 insertions(+), 22 deletions(-) create mode 100644 src/plugin/registry.rs diff --git a/src/main.rs b/src/main.rs index ce4dc3c..bf4e499 100644 --- a/src/main.rs +++ b/src/main.rs @@ -147,7 +147,7 @@ async fn run(cli: Cli) -> anyhow::Result<()> { Commands::Install { plugins } => { for name in &plugins { - install_plugin(name)?; + install_plugin(name).await?; } } diff --git a/src/plugin/install.rs b/src/plugin/install.rs index 7660c94..cd881ea 100644 --- a/src/plugin/install.rs +++ b/src/plugin/install.rs @@ -1,8 +1,8 @@ use std::path::Path; -use crate::plugin::{PluginDiscovery, PluginManifest}; +use crate::plugin::{PluginDiscovery, PluginManifest, registry}; -/// Embedded plugin registry (known Omni ecosystem plugins) +/// Embedded plugin registry (offline fallback) const REGISTRY: &[(&str, &str)] = &[( "beacon", include_str!("../../examples/plugins/beacon/plugin.toml"), @@ -57,12 +57,43 @@ impl PackageManager { } } +/// Resolve a plugin manifest by trying the remote registry first, then the +/// embedded fallback. Returns the manifest and TOML string to write to disk. +async fn resolve_manifest(name: &str) -> anyhow::Result<(PluginManifest, String)> { + let base_url = registry::registry_url(); + + match registry::fetch_plugin(&base_url, name).await { + Ok(manifest) => { + let toml_str = toml::to_string_pretty(&manifest) + .map_err(|e| anyhow::anyhow!("failed to serialize manifest: {e}"))?; + Ok((manifest, toml_str)) + } + Err(e) => { + tracing::debug!("remote registry unavailable, using embedded fallback: {e}"); + + let toml_str = REGISTRY + .iter() + .find(|(n, _)| *n == name) + .map(|(_, toml)| *toml) + .ok_or_else(|| anyhow::anyhow!("unknown plugin '{name}'"))?; + + let manifest: PluginManifest = toml::from_str(toml_str) + .map_err(|e| anyhow::anyhow!("invalid manifest for '{name}': {e}"))?; + + Ok((manifest, toml_str.to_string())) + } + } +} + /// Install a plugin by name /// +/// Fetches the manifest from the remote plugin registry, falling back to +/// the embedded registry if unavailable. +/// /// # Errors /// /// Returns an error if the plugin is unknown, already installed, or installation fails -pub fn install_plugin(name: &str) -> anyhow::Result<()> { +pub async fn install_plugin(name: &str) -> anyhow::Result<()> { let plugins_dir = PluginDiscovery::default_dir()?; let target = plugins_dir.join(name); @@ -70,16 +101,7 @@ pub fn install_plugin(name: &str) -> anyhow::Result<()> { anyhow::bail!("plugin '{name}' is already installed"); } - // Look up in embedded registry - let manifest_toml = REGISTRY - .iter() - .find(|(n, _)| *n == name) - .map(|(_, toml)| *toml); - - let manifest_toml = manifest_toml.ok_or_else(|| anyhow::anyhow!("unknown plugin '{name}'"))?; - - let manifest: PluginManifest = toml::from_str(manifest_toml) - .map_err(|e| anyhow::anyhow!("invalid manifest for '{name}': {e}"))?; + let (manifest, toml_str) = resolve_manifest(name).await?; // Install binary via package manager if applicable if manifest.bin.is_some() { @@ -87,7 +109,7 @@ pub fn install_plugin(name: &str) -> anyhow::Result<()> { } // Write manifest to plugins dir - write_manifest(&target, manifest_toml)?; + write_manifest(&target, &toml_str)?; println!("Installed plugin '{name}'"); Ok(()) @@ -168,8 +190,7 @@ mod tests { std::fs::create_dir_all(&target).unwrap(); std::fs::write(target.join("plugin.toml"), "").unwrap(); - // We can't easily test install_plugin since it uses default_dir(), - // but we can verify the manifest writes correctly + // Verify the manifest writes correctly let result = write_manifest(&dir.path().join("new-plugin"), "name = \"test\""); assert!(result.is_ok()); assert!(dir.path().join("new-plugin/plugin.toml").exists()); @@ -186,4 +207,15 @@ mod tests { "name = \"test\"" ); } + + #[test] + fn manifest_round_trips_through_toml() { + for (_, toml_str) in REGISTRY { + let manifest: PluginManifest = toml::from_str(toml_str).unwrap(); + let serialized = toml::to_string_pretty(&manifest).unwrap(); + let reparsed: PluginManifest = toml::from_str(&serialized).unwrap(); + assert_eq!(manifest.name, reparsed.name); + assert_eq!(manifest.plugin_type, reparsed.plugin_type); + } + } } diff --git a/src/plugin/manifest.rs b/src/plugin/manifest.rs index 44dfb23..86192dd 100644 --- a/src/plugin/manifest.rs +++ b/src/plugin/manifest.rs @@ -2,10 +2,10 @@ use std::collections::HashMap; use std::path::Path; use regex::Regex; -use serde::Deserialize; +use serde::{Deserialize, Serialize}; /// Plugin type determines how the CLI delegates to the plugin -#[derive(Debug, Clone, PartialEq, Eq, Deserialize)] +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "lowercase")] pub enum PluginType { /// Delegates to an external binary @@ -17,7 +17,7 @@ pub enum PluginType { } /// A command exposed by a plugin -#[derive(Debug, Clone, Deserialize)] +#[derive(Debug, Clone, Serialize, Deserialize)] pub struct CommandDef { pub description: String, /// HTTP method for API plugins @@ -27,14 +27,14 @@ pub struct CommandDef { } /// System package manager hints for installation -#[derive(Debug, Clone, Deserialize)] +#[derive(Debug, Clone, Serialize, Deserialize)] pub struct PackageHints { pub aur: Option, pub homebrew: Option, } /// Plugin manifest parsed from plugin.toml -#[derive(Debug, Clone, Deserialize)] +#[derive(Debug, Clone, Serialize, Deserialize)] pub struct PluginManifest { pub name: String, pub version: String, diff --git a/src/plugin/mod.rs b/src/plugin/mod.rs index fa71a07..2061927 100644 --- a/src/plugin/mod.rs +++ b/src/plugin/mod.rs @@ -2,6 +2,7 @@ pub mod discovery; pub mod exec; pub mod install; pub mod manifest; +pub mod registry; pub use discovery::{DiscoveredPlugin, PluginDiscovery, PluginSource}; pub use exec::{resolve_bin_and_args, run_plugin_command}; diff --git a/src/plugin/registry.rs b/src/plugin/registry.rs new file mode 100644 index 0000000..652e1f6 --- /dev/null +++ b/src/plugin/registry.rs @@ -0,0 +1,138 @@ +use std::collections::HashMap; +use std::time::Duration; + +use serde::Deserialize; + +use crate::plugin::{CommandDef, PackageHints, PluginManifest, PluginType}; + +/// Default registry URL +const DEFAULT_REGISTRY_URL: &str = "https://manifold.omni.dev"; + +/// Response shape for GET /api/plugins/:name +#[derive(Deserialize)] +struct PluginResponse { + plugin: RegistryPlugin, +} + +/// Plugin as returned by the registry API +#[derive(Deserialize)] +struct RegistryPlugin { + name: String, + version: String, + description: String, + #[serde(rename = "type")] + plugin_type: String, + bin: Option, + endpoint: Option, + #[serde(default)] + commands: HashMap, + packages: Option, +} + +impl TryFrom for PluginManifest { + type Error = anyhow::Error; + + fn try_from(rp: RegistryPlugin) -> anyhow::Result { + let plugin_type = match rp.plugin_type.as_str() { + "bin" => PluginType::Bin, + "api" => PluginType::Api, + "launch" => PluginType::Launch, + other => anyhow::bail!("unknown plugin type: {other}"), + }; + + Ok(Self { + name: rp.name, + version: rp.version, + description: rp.description, + plugin_type, + bin: rp.bin, + endpoint: rp.endpoint, + commands: rp.commands, + packages: rp.packages, + }) + } +} + +/// Resolve the registry base URL from env or config +#[must_use] +pub fn registry_url() -> String { + std::env::var("OMNI_PLUGIN_REGISTRY_URL").unwrap_or_else(|_| DEFAULT_REGISTRY_URL.to_string()) +} + +/// Fetch a plugin manifest from the remote registry +/// +/// # Errors +/// +/// Returns an error if the request fails or the response is invalid +pub async fn fetch_plugin(base_url: &str, name: &str) -> anyhow::Result { + let url = format!("{base_url}/api/plugins/{name}"); + + let client = reqwest::Client::builder() + .timeout(Duration::from_secs(5)) + .build()?; + + let resp = client.get(&url).send().await?; + + if resp.status() == 404 { + anyhow::bail!("plugin '{name}' not found in registry"); + } + + if !resp.status().is_success() { + anyhow::bail!("registry returned HTTP {}", resp.status()); + } + + let body: PluginResponse = resp.json().await?; + body.plugin.try_into() +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn registry_plugin_converts_to_manifest() { + let rp = RegistryPlugin { + name: "test".into(), + version: "0.1.0".into(), + description: "a test plugin".into(), + plugin_type: "bin".into(), + bin: Some("test-bin".into()), + endpoint: None, + commands: HashMap::new(), + packages: Some(PackageHints { + aur: Some("test-aur".into()), + homebrew: Some("test/tap/test".into()), + }), + }; + + let manifest: PluginManifest = rp.try_into().unwrap(); + assert_eq!(manifest.name, "test"); + assert_eq!(manifest.plugin_type, PluginType::Bin); + assert_eq!(manifest.bin.as_deref(), Some("test-bin")); + assert!(manifest.packages.is_some()); + } + + #[test] + fn invalid_plugin_type_errors() { + let rp = RegistryPlugin { + name: "bad".into(), + version: "0.1.0".into(), + description: "bad".into(), + plugin_type: "invalid".into(), + bin: None, + endpoint: None, + commands: HashMap::new(), + packages: None, + }; + + let result: Result = rp.try_into(); + assert!(result.is_err()); + } + + #[test] + fn default_registry_url_is_set() { + // When env var is not set, should return the default + let url = DEFAULT_REGISTRY_URL; + assert!(url.starts_with("https://")); + } +}