diff --git a/md/reference/plugin-definition.md b/md/reference/plugin-definition.md index 9a705fa0..521ec5e3 100644 --- a/md/reference/plugin-definition.md +++ b/md/reference/plugin-definition.md @@ -53,7 +53,7 @@ crates = ["serde", "tokio"] # Only active in projects using serde OR tokio crates = ["*"] ``` -If omitted, the plugin applies to all projects. Plugin-level filtering is combined with skill group filtering using AND logic — both must match for skills to be available. +Plugin-level filtering is combined with skill group filtering using AND logic — both must match for skills to be available. ## `[[skills]]` groups @@ -119,7 +119,7 @@ executable = "rg" # the binary to run; if omitted and the crate has a singl args = ["--version"] # optional default args ``` -Symposium attempts `cargo binstall` first, falls back to `cargo install`, and caches the result under `~/.symposium/cache/binaries///bin/`. The chosen `executable` resolves to `/bin/`. +Symposium attempts `cargo binstall` first, falls back to `cargo install`, and caches the result under `~/.symposium/cache/binaries///bin/` (passing `--root` so the install doesn't pollute `~/.cargo/bin`). The chosen `executable` resolves to `/bin/`. Hooks that depend on this installation get `/bin/` prepended to `$PATH`, so scripts can invoke the binary by name. To install from a git repo instead of crates.io, set `git`: @@ -132,6 +132,17 @@ git = "https://github.com/example/tool" executable = "tool" # required for git sources (crates.io is not consulted) ``` +To install into the user's global cargo location (`~/.cargo/bin`) instead of a symposium-managed cache, set `global = true`. No `--root` is passed; `$PATH` is not augmented (the binary is expected to already be on `$PATH`). + +```toml +[[installations]] +name = "rg" +source = "cargo" +crate = "ripgrep" +executable = "rg" +global = true +``` + #### `github` ```toml @@ -296,6 +307,22 @@ script = "hooks/claude/rtk-rewrite.sh" Requirement installation is best-effort: failures are logged and dispatch continues. +### Hook environment + +Hooks are spawned with the following extras on top of the parent environment: + +| Variable | When set | Value | +|----------|----------|-------| +| `$SYMPOSIUM_DIR_` | Installation has a symposium-managed cache (scoped cargo, github) | Absolute path to the cache / clone directory. | +| `$SYMPOSIUM_` | Installation resolves to a runnable with an absolute path | Absolute path to the resolved executable / script. | +| `$PATH` | One or more dependencies contribute a runnable with an absolute path | Each runnable's parent dir is prepended, with the hook's `command` first. | + +`` is the installation name with non-alphanumeric characters replaced by `_` (e.g. `rtk-hooks` → `SYMPOSIUM_DIR_rtk_hooks`). Both the hook's `command` installation and every requirement (recursively, one level via installation-level requirements) contribute. + +Global cargo installs (`global = true`) don't set `$SYMPOSIUM_DIR_` or augment `$PATH` — the binary is expected to already be on the user's `$PATH` via `~/.cargo/bin`. + +> **`install_commands` runs before env vars are set.** The `$SYMPOSIUM_*` vars and the augmented `$PATH` are only available to the hook's spawned process. `install_commands` runs earlier, inside the symposium dispatch process, so it cannot reference its own (or any other) installation's env vars. Use absolute paths in `install_commands` instead. + ### Supported hook events | Hook event | Description | CLI usage | diff --git a/src/hook.rs b/src/hook.rs index c3d0e71b..a7bad27d 100644 --- a/src/hook.rs +++ b/src/hook.rs @@ -1,10 +1,10 @@ use std::{ io::{Read, Write}, - path::PathBuf, + path::{Path, PathBuf}, process::{Command, ExitCode, Stdio}, }; -use crate::installation::{Runnable, acquire_source, make_executable}; +use crate::installation::{AcquiredSource, acquire_source, make_executable}; use crate::plugins::{HookFormat, Installation}; use crate::{ config::Symposium, @@ -18,6 +18,9 @@ use crate::{ struct ResolvedHook { plugin_name: String, hook_name: String, + /// Directory containing the plugin's manifest. Relative `executable` / + /// `script` paths on no-source installations resolve against this. + plugin_dir: PathBuf, format: HookFormat, requirements: Vec, command: Installation, @@ -47,9 +50,16 @@ impl ResolvedHook { .map(|name| lookup(name)) .collect::>>()?; + let plugin_dir = parsed_plugin + .path + .parent() + .map(|p| p.to_path_buf()) + .unwrap_or_else(|| PathBuf::from(".")); + Ok(Self { plugin_name: parsed_plugin.plugin.name.clone(), hook_name: hook.name.clone(), + plugin_dir, format: hook.format.clone(), requirements, command, @@ -60,14 +70,132 @@ impl ResolvedHook { } } -/// Acquire an installation as a requirement: run its kind-specific source -/// step (if any), then any declared `install_commands`. Does NOT resolve to -/// a runnable — requirements are only ever "ensure on disk". -async fn install(sym: &Symposium, install: &Installation) -> anyhow::Result<()> { - if let Some(source) = &install.source { - acquire_source(sym, source, install.executable.as_deref()).await?; +/// Per-installation snapshot the dispatcher builds for the command and each +/// requirement. Drives env-var wiring for the spawned hook process: +/// `$SYMPOSIUM_DIR_`, `$SYMPOSIUM_`, and the `$PATH` prefix. +/// +/// One layer above [`AcquiredSource`]: that's the raw source-acquisition +/// result (where the bits landed + how to resolve names within them); +/// `AcquiredInstallation` is what we keep after layering on the installation's +/// `install_commands`, `executable`/`script` resolution, and the no-source +/// case where there's nothing to acquire at all. The dispatcher only cares +/// about the resolved form, so this is what flows from +/// [`acquire_installation`] into [`build_env`] and [`build_spawn_spec`]. +#[derive(Debug)] +struct AcquiredInstallation { + /// Installation name as declared in the manifest. Sanitized via + /// [`env_safe`] when used in env var keys. + name: String, + /// Cache or clone directory the source landed in. `None` for no-source + /// installations and for global cargo (which lives in the user's + /// `~/.cargo/bin`, outside symposium's management). + base: Option, + /// What this installation resolves to at spawn time, or `None` when the + /// installation has nothing runnable (pure setup). + runnable: Option, +} + +/// How `Command::new` should be invoked for an installation that resolved +/// to *something* runnable. +#[derive(Debug)] +enum AcquiredRunnable { + /// Symposium-resolved absolute path. Exposed as `$SYMPOSIUM_` and + /// its parent dir is prepended to `$PATH`. `is_script` chooses between + /// `Command::new(path)` and `Command::new("sh").arg(path)`. + Resolved { path: PathBuf, is_script: bool }, + /// Bare binary name, relying on `$PATH` lookup at spawn time (global + /// cargo). Not exposed in env vars and doesn't contribute to `$PATH` — + /// the installation is intentionally outside symposium's view. + PathLookup { name: String }, +} + +/// Acquire an installation: run its source step (if any), run +/// `install_commands`, and resolve its runnable using the installation's +/// own `executable`/`script` plus any hook-level overrides. +/// +/// `plugin_dir` is the directory containing the plugin's manifest; +/// relative `executable` / `script` paths on no-source installations +/// resolve against it. +async fn acquire_installation( + sym: &Symposium, + install: &Installation, + plugin_dir: &Path, + hook_executable: Option<&str>, + hook_script: Option<&str>, +) -> anyhow::Result { + let exec_choice = install.executable.as_deref().or(hook_executable); + let script_choice = install.script.as_deref().or(hook_script); + + let acquired: Option = match &install.source { + Some(source) => Some(acquire_source(sym, source, exec_choice).await?), + None => None, + }; + + run_install_commands(&install.install_commands).await?; + + // Anchor a no-source relative path against the plugin directory so the + // env vars and PATH augmentation always see absolute paths. Absolute + // paths pass through unchanged. + let anchor_to_plugin = |name: &str| -> PathBuf { + let p = PathBuf::from(name); + if p.is_absolute() { + p + } else { + plugin_dir.join(p) + } + }; + + let runnable: Option = match (&acquired, exec_choice, script_choice) { + // Unmanaged source (global cargo): bare name, $PATH lookup at spawn. + // Validation guarantees `exec_choice` is set for cargo + global. + (Some(a), _, _) if a.base.is_none() => { + let name = exec_choice + .or(a.resolved_executable.as_deref()) + .expect("global cargo validation enforces an executable name") + .to_string(); + Some(AcquiredRunnable::PathLookup { name }) + } + (Some(a), Some(name), None) => Some(AcquiredRunnable::Resolved { + path: a.resolve_executable(name), + is_script: false, + }), + (Some(a), None, Some(name)) => Some(AcquiredRunnable::Resolved { + path: a.resolve_script(name), + is_script: true, + }), + (Some(a), None, None) => { + a.resolved_executable + .as_deref() + .map(|name| AcquiredRunnable::Resolved { + path: a.resolve_executable(name), + is_script: false, + }) + } + (None, Some(name), None) => Some(AcquiredRunnable::Resolved { + path: anchor_to_plugin(name), + is_script: false, + }), + (None, None, Some(name)) => Some(AcquiredRunnable::Resolved { + path: anchor_to_plugin(name), + is_script: true, + }), + (None, None, None) => None, + (_, Some(_), Some(_)) => unreachable!("validation forbids both executable and script"), + }; + + if let Some(AcquiredRunnable::Resolved { + path, + is_script: false, + }) = &runnable + { + make_executable(path).ok(); } - run_install_commands(&install.install_commands).await + + Ok(AcquiredInstallation { + name: install.name.clone(), + base: acquired.as_ref().and_then(|a| a.base.clone()), + runnable, + }) } /// Run a list of post-install shell commands sequentially. Stops at the first @@ -86,87 +214,146 @@ async fn run_install_commands(commands: &[String]) -> anyhow::Result<()> { Ok(()) } -enum SpawnSpec { - Exec { path: PathBuf, args: Vec }, - Script { path: PathBuf, args: Vec }, +/// Sanitize an installation name for use as part of an env var name. +/// Replaces non-alphanumeric chars with underscore. +fn env_safe(name: &str) -> String { + name.chars() + .map(|c| if c.is_ascii_alphanumeric() { c } else { '_' }) + .collect() } -async fn build_spawn_spec(sym: &Symposium, hook: &ResolvedHook) -> anyhow::Result { - let installation = &hook.command; - // Validation guarantees only one slot is set across hook + installation. - let exec_choice = installation - .executable - .as_deref() - .or(hook.hook_executable.as_deref()); - let script_choice = installation - .script - .as_deref() - .or(hook.hook_script.as_deref()); - - // Acquire the source if any. - let acquired = match &installation.source { - Some(source) => Some(acquire_source(sym, source, exec_choice).await?), - None => None, - }; +/// Build the env vars (including augmented PATH) for the spawn. +/// +/// Iterates `acquired` in order; the parent directory of each absolute +/// `runnable` is prepended to `$PATH` so later entries take precedence over +/// earlier ones (the command's own parent ends up first since it's pushed +/// last and prepended). +fn build_env(acquired: &[AcquiredInstallation]) -> Vec<(String, String)> { + let mut env = Vec::new(); + let mut path_prefix: Vec = Vec::new(); + + for a in acquired { + let key = env_safe(&a.name); + if let Some(base) = &a.base { + env.push((format!("SYMPOSIUM_DIR_{key}"), base.display().to_string())); + } + if let Some(AcquiredRunnable::Resolved { path, .. }) = &a.runnable { + env.push((format!("SYMPOSIUM_{key}"), path.display().to_string())); + if let Some(parent) = path.parent() { + let parent_str = parent.display().to_string(); + if !parent_str.is_empty() { + path_prefix.push(parent_str); + } + } + } + } - // install_commands run after source acquisition. - run_install_commands(&installation.install_commands).await?; + // Command was pushed last into `acquired`; reverse so its bin dir wins + // PATH lookup over requirements' bin dirs. + path_prefix.reverse(); - let runnable = match (acquired, exec_choice, script_choice) { - (Some(a), Some(name), None) => Runnable::Exec(a.resolve_executable(name)), - (Some(a), None, Some(name)) => Runnable::Script(a.resolve_script(name)), - (Some(a), None, None) => { - // Cargo single-binary fallback: use the binary name resolved at - // acquisition time (from crates.io or the explicit hint). - if let Some(name) = a.resolved_executable.as_deref() { - Runnable::Exec(a.resolve_executable(name)) - } else { - anyhow::bail!( - "hook `{}`: command resolved to no executable or script", - hook.hook_name + if !path_prefix.is_empty() { + let existing = std::env::var("PATH").unwrap_or_default(); + let joined = if existing.is_empty() { + path_prefix.join(":") + } else { + format!("{}:{}", path_prefix.join(":"), existing) + }; + env.push(("PATH".to_string(), joined)); + } + + env +} + +enum SpawnSpec { + Exec { + path: PathBuf, + args: Vec, + env: Vec<(String, String)>, + }, + Script { + path: PathBuf, + args: Vec, + env: Vec<(String, String)>, + }, +} + +async fn build_spawn_spec(sym: &Symposium, hook: &ResolvedHook) -> anyhow::Result { + // Acquire requirements first so the command's PATH sees them. + let mut acquired: Vec = Vec::new(); + for requirement in &hook.requirements { + match acquire_installation(sym, requirement, &hook.plugin_dir, None, None).await { + Ok(a) => acquired.push(a), + Err(e) => { + tracing::error!( + name = %requirement.name, + error = %e, + "failed to install hook requirement" ); } } - (None, Some(name), None) => Runnable::Exec(PathBuf::from(name)), - (None, None, Some(name)) => Runnable::Script(PathBuf::from(name)), - (None, None, None) => anyhow::bail!( + } + + let command_acquired = acquire_installation( + sym, + &hook.command, + &hook.plugin_dir, + hook.hook_executable.as_deref(), + hook.hook_script.as_deref(), + ) + .await?; + + let (spawn_path, is_script) = match &command_acquired.runnable { + Some(AcquiredRunnable::Resolved { path, is_script }) => (path.clone(), *is_script), + Some(AcquiredRunnable::PathLookup { name }) => (PathBuf::from(name), false), + None => anyhow::bail!( "hook `{}`: command resolved to no executable or script", hook.hook_name ), - // Unreachable: validation rejects executable+script set together. - (_, Some(_), Some(_)) => unreachable!("validation forbids both executable and script"), }; - match runnable { - Runnable::Exec(path) => { - make_executable(&path).ok(); - Ok(SpawnSpec::Exec { - path, - args: hook.args.clone(), - }) - } - Runnable::Script(path) => Ok(SpawnSpec::Script { - path, + acquired.push(command_acquired); + let env = build_env(&acquired); + + if is_script { + Ok(SpawnSpec::Script { + path: spawn_path, args: hook.args.clone(), - }), + env, + }) + } else { + Ok(SpawnSpec::Exec { + path: spawn_path, + args: hook.args.clone(), + env, + }) } } fn spawn_from_spec(spec: SpawnSpec) -> std::io::Result { match spec { - SpawnSpec::Script { path, args } => Command::new("sh") - .arg(path) - .args(args) - .stdin(Stdio::piped()) - .stdout(Stdio::piped()) - .stderr(Stdio::piped()) - .spawn(), - SpawnSpec::Exec { path, args } => Command::new(path) - .args(args) - .stdin(Stdio::piped()) - .stdout(Stdio::piped()) - .stderr(Stdio::piped()) - .spawn(), + SpawnSpec::Script { path, args, env } => { + let mut cmd = Command::new("sh"); + cmd.arg(path).args(args); + for (k, v) in env { + cmd.env(k, v); + } + cmd.stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + } + SpawnSpec::Exec { path, args, env } => { + let mut cmd = Command::new(path); + cmd.args(args); + for (k, v) in env { + cmd.env(k, v); + } + cmd.stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + } } } @@ -375,12 +562,7 @@ pub async fn dispatch_plugin_hooks( "running plugin hook" ); - // Acquire each requirement (best-effort). - for requirement in &hook.requirements { - if let Err(e) = install(sym, requirement).await { - tracing::error!(name = %requirement.name, error = %e, "failed to install hook requirement"); - } - } + // Requirement acquisition + env wiring happens inside `build_spawn_spec`. // Determine stdin for the plugin based on its declared format let hook_agent = hook.format.as_agent(); @@ -576,6 +758,92 @@ fn dispatched_hooks_for_payload( mod tests { use super::*; + #[test] + fn env_safe_sanitizes_punctuation() { + assert_eq!(env_safe("rtk"), "rtk"); + assert_eq!(env_safe("rtk-hooks"), "rtk_hooks"); + assert_eq!(env_safe("a.b-c"), "a_b_c"); + assert_eq!(env_safe("name__req_0"), "name__req_0"); + } + + #[test] + fn build_env_sets_dir_and_name_vars() { + // [req (rtk), command (no-source)] order — command was pushed last, + // so PATH should put its bin dir first. + let acquired = vec![ + AcquiredInstallation { + name: "rtk".to_string(), + base: Some(PathBuf::from("/cache/rtk/1.0")), + runnable: Some(AcquiredRunnable::Resolved { + path: PathBuf::from("/cache/rtk/1.0/bin/rtk"), + is_script: false, + }), + }, + AcquiredInstallation { + name: "no-source".to_string(), + base: None, + runnable: Some(AcquiredRunnable::Resolved { + path: PathBuf::from("/usr/local/bin/tool"), + is_script: false, + }), + }, + ]; + let env: std::collections::HashMap<_, _> = build_env(&acquired).into_iter().collect(); + assert_eq!( + env.get("SYMPOSIUM_DIR_rtk").map(String::as_str), + Some("/cache/rtk/1.0") + ); + assert_eq!( + env.get("SYMPOSIUM_rtk").map(String::as_str), + Some("/cache/rtk/1.0/bin/rtk") + ); + // No source means no _DIR, but absolute runnable path → SYMPOSIUM_ set. + assert_eq!(env.get("SYMPOSIUM_DIR_no_source"), None); + assert_eq!( + env.get("SYMPOSIUM_no_source").map(String::as_str), + Some("/usr/local/bin/tool") + ); + // Command (pushed last) wins PATH lookup, so its parent comes first. + let path = env.get("PATH").expect("PATH set"); + assert!(path.starts_with("/usr/local/bin"), "PATH = {path}"); + assert!(path.contains("/cache/rtk/1.0/bin"), "PATH = {path}"); + } + + #[test] + fn build_env_no_runnable_no_vars() { + // Pure-setup installation: no runnable means no SYMPOSIUM_ + // and no PATH contribution. SYMPOSIUM_DIR_ still gets set + // when there's a managed base dir. + let acquired = vec![AcquiredInstallation { + name: "setup".to_string(), + base: Some(PathBuf::from("/cache/setup")), + runnable: None, + }]; + let env: std::collections::HashMap<_, _> = build_env(&acquired).into_iter().collect(); + assert_eq!( + env.get("SYMPOSIUM_DIR_setup").map(String::as_str), + Some("/cache/setup") + ); + assert!(env.get("SYMPOSIUM_setup").is_none()); + assert!(env.get("PATH").is_none()); + } + + #[test] + fn build_env_global_cargo_skips_env_and_path() { + // Global cargo: PathLookup runnable. Nothing exposed. + let acquired = vec![AcquiredInstallation { + name: "rg".to_string(), + base: None, + runnable: Some(AcquiredRunnable::PathLookup { + name: "rg".to_string(), + }), + }]; + let env: std::collections::HashMap<_, _> = build_env(&acquired).into_iter().collect(); + assert!(env.get("SYMPOSIUM_DIR_rg").is_none()); + assert!(env.get("SYMPOSIUM_rg").is_none()); + assert!(env.get("PATH").is_none()); + } + #[tokio::test] async fn builtin_pre_tool_use_returns_empty() { let tmp = tempfile::tempdir().unwrap(); diff --git a/src/installation.rs b/src/installation.rs index d48b8a22..2801241d 100644 --- a/src/installation.rs +++ b/src/installation.rs @@ -36,6 +36,17 @@ pub struct CargoSource { /// crates.io is not consulted to discover binary names. #[serde(default, skip_serializing_if = "Option::is_none")] pub git: Option, + /// Install into the user's global cargo location (`~/.cargo/bin`) instead + /// of a symposium-managed cache. The default (`false`) uses + /// `cargo install --root ` so binaries don't pollute the + /// global namespace; hook execution adds the cache `bin/` to `$PATH` so + /// scripts can still invoke them by name. + #[serde(default, skip_serializing_if = "is_false")] + pub global: bool, +} + +fn is_false(b: &bool) -> bool { + !*b } /// A directory of files acquired from a GitHub repository (or subtree). @@ -142,18 +153,27 @@ pub async fn query_crate_binaries( } /// Install a crate using cargo binstall (fast) or cargo install (fallback). +/// +/// `cache_dir` is `Some` for scoped installs (passed via `--root`) and `None` +/// for global installs (uses cargo's default location). pub(crate) async fn install_cargo_crate( crate_name: &str, version: &str, binary_name: Option, - cache_dir: PathBuf, + cache_dir: Option, git: Option, ) -> Result<()> { let crate_name = crate_name.to_string(); let version = version.to_string(); tokio::task::spawn_blocking(move || { - install_cargo_crate_sync(&crate_name, &version, binary_name, &cache_dir, git) + install_cargo_crate_sync( + &crate_name, + &version, + binary_name, + cache_dir.as_deref(), + git, + ) }) .await .context("Cargo install task panicked")? @@ -163,41 +183,54 @@ fn install_cargo_crate_sync( crate_name: &str, version: &str, binary_name: Option, - cache_dir: &Path, + cache_dir: Option<&Path>, git: Option, ) -> Result<()> { use std::fs; use std::process::Command; - if let Some(parent) = cache_dir.parent() - && parent.exists() - { - for entry in fs::read_dir(parent)? { - let entry = entry?; - let path = entry.path(); - if path != cache_dir && path.is_dir() { - fs::remove_dir_all(&path).ok(); + if let Some(cache_dir) = cache_dir { + if let Some(parent) = cache_dir.parent() + && parent.exists() + { + for entry in fs::read_dir(parent)? { + let entry = entry?; + let path = entry.path(); + if path != cache_dir && path.is_dir() { + fs::remove_dir_all(&path).ok(); + } } } + fs::create_dir_all(cache_dir)?; } - fs::create_dir_all(cache_dir)?; - - let crate_spec = format!("{}@{}", crate_name, version); + // Empty version → just the crate name. Avoids `cargo install rtk@` which + // cargo rejects. + let crate_spec = if version.is_empty() { + crate_name.to_string() + } else { + format!("{}@{}", crate_name, version) + }; + let cache_dir_str = cache_dir.map(|p| p.to_str().unwrap().to_string()); // Try cargo binstall first (faster, uses prebuilt binaries). tracing::info!("Attempting cargo binstall for {}", crate_spec); - let mut binstall_args = vec!["binstall", "--no-confirm", "--root"]; + let mut binstall_args: Vec<&str> = vec!["binstall", "--no-confirm"]; + if let Some(dir) = cache_dir_str.as_deref() { + binstall_args.push("--root"); + binstall_args.push(dir); + } if let Some(git) = &git { binstall_args.push("--git"); binstall_args.push(git); } - binstall_args.extend([cache_dir.to_str().unwrap(), &crate_spec]); - let binstall_result = Command::new("cargo").args(binstall_args).output(); + binstall_args.push(&crate_spec); + let binstall_result = Command::new("cargo").args(&binstall_args).output(); - let binary_path = binary_name - .as_ref() - .map(|bin| cache_dir.join("bin").join(platform_binary_exe(bin))); + let binary_path = match (cache_dir, binary_name.as_ref()) { + (Some(dir), Some(bin)) => Some(dir.join("bin").join(platform_binary_exe(bin))), + _ => None, + }; match binstall_result { Ok(output) if output.status.success() => { @@ -223,14 +256,18 @@ fn install_cargo_crate_sync( } tracing::info!("Falling back to cargo install for {}", crate_spec); - let mut args = vec!["install", "--root"]; + let mut args: Vec<&str> = vec!["install"]; + if let Some(dir) = cache_dir_str.as_deref() { + args.push("--root"); + args.push(dir); + } if let Some(git) = &git { args.push("--git"); args.push(git); } - args.extend([cache_dir.to_str().unwrap(), &crate_spec]); + args.push(&crate_spec); let install_result = Command::new("cargo") - .args(args) + .args(&args) .output() .context("Failed to run cargo install")?; @@ -263,16 +300,49 @@ fn git_cache_version(git_url: &str, version: Option<&str>) -> String { format!("{:016x}", hasher.finish()) } -/// Acquire a cargo installation: install if missing, return the cache dir -/// plus the resolved binary name (when discoverable from crates.io). +/// Outcome of acquiring a cargo source. +struct AcquiredCargo { + /// Symposium-managed cache dir, or `None` for global installs (no --root). + cache_dir: Option, + /// The resolved binary name, when known. + resolved_executable: Option, +} + +/// Acquire a cargo installation: install if missing. +/// +/// Three branches, in priority order: +/// - `global = true`: skip crates.io, install with no `--root` (binary lands +/// in the user's `$CARGO_HOME/bin`). Validation guarantees the caller has +/// set `executable`, so no inference needed. `cache_dir = None` is the +/// signal that the source is unmanaged. +/// - `git` source: skip crates.io, install with `--root ` and +/// `--git `. Validation guarantees `executable`. +/// - Plain crates.io: query for version + bin_names; auto-infer the binary +/// when the crate has exactly one. async fn acquire_cargo( sym: &Symposium, cargo: &CargoSource, executable_hint: Option<&str>, -) -> Result<(PathBuf, Option)> { - // For git sources, we don't consult crates.io. Cache key folds in the URL - // and the user's version (if any). The user must specify `executable` - // since we have no other way to pick a binary. +) -> Result { + if cargo.global { + // Validation requires `executable` for global cargo. + let resolved = executable_hint + .expect("validate_installation enforces `executable` for cargo + global") + .to_string(); + install_cargo_crate( + &cargo.crate_name, + cargo.version.as_deref().unwrap_or(""), + Some(resolved.clone()), + None, + cargo.git.clone(), + ) + .await?; + return Ok(AcquiredCargo { + cache_dir: None, + resolved_executable: Some(resolved), + }); + } + if let Some(git_url) = cargo.git.as_deref() { let resolved = match executable_hint { Some(name) => name.to_string(), @@ -283,19 +353,22 @@ async fn acquire_cargo( ), }; let cache_version = git_cache_version(git_url, cargo.version.as_deref()); - let cache_dir = get_binary_cache_dir(sym, &cargo.crate_name, &cache_version)?; - let probe = cache_dir.join("bin").join(platform_binary_exe(&resolved)); + let dir = get_binary_cache_dir(sym, &cargo.crate_name, &cache_version)?; + let probe = dir.join("bin").join(platform_binary_exe(&resolved)); if !probe.exists() { install_cargo_crate( &cargo.crate_name, cargo.version.as_deref().unwrap_or(""), Some(resolved.clone()), - cache_dir.clone(), + Some(dir.clone()), Some(git_url.to_string()), ) .await?; } - return Ok((cache_dir, Some(resolved))); + return Ok(AcquiredCargo { + cache_dir: Some(dir), + resolved_executable: Some(resolved), + }); } let (version, bin_names) = @@ -314,24 +387,26 @@ async fn acquire_cargo( }, }; - let cache_dir = get_binary_cache_dir(sym, &cargo.crate_name, &version)?; - let probe_path = resolved + let dir = get_binary_cache_dir(sym, &cargo.crate_name, &version)?; + let probe = resolved .as_ref() - .map(|n| cache_dir.join("bin").join(platform_binary_exe(n))); - - let already = probe_path.as_ref().map_or(false, |p| p.exists()); + .map(|n| dir.join("bin").join(platform_binary_exe(n))); + let already = probe.as_ref().map_or(false, |p| p.exists()); if !already { install_cargo_crate( &cargo.crate_name, &version, resolved.clone(), - cache_dir.clone(), + Some(dir.clone()), None, ) .await?; } - Ok((cache_dir, resolved)) + Ok(AcquiredCargo { + cache_dir: Some(dir), + resolved_executable: resolved, + }) } /// Acquire a github source: returns the cache directory containing the repo @@ -345,16 +420,24 @@ pub(crate) async fn acquire_github(sym: &Symposium, git: &GithubSource) -> Resul .await } -/// Acquired source — where files ended up on disk, plus the kind so -/// `executable` / `script` can be resolved correctly relative to it. -#[derive(Debug)] +/// Intermediate result of acquiring a `Source`: where the bits landed and +/// the layout-specific hooks needed to turn an `executable` / `script` name +/// into a concrete path. Consumed by `hook::acquire_installation`, which +/// wraps this together with the installation's name, `install_commands`, +/// and (for no-source installs) absolute path defaults to produce the +/// fully-resolved `AcquiredInstallation`. pub struct AcquiredSource { - pub base: PathBuf, - pub kind: AcquiredKind, - /// For cargo, the binary name we ended up with (from `executable_hint` - /// or by inferring the crate's sole binary). Used as the fallback when - /// the hook command supplies no explicit executable. `None` for github. + /// Where bits landed. `None` for global cargo (binary is in + /// `~/.cargo/bin`, which we don't manage). + pub base: Option, + /// For cargo, the binary name that was installed. Used as the fallback + /// when neither the installation nor the hook supplies an explicit + /// `executable`. `None` for github (which has no notion of "default + /// binary"). pub resolved_executable: Option, + /// Layout discriminator — cargo binaries live under `/bin/`, + /// github paths live under `/` directly. + pub kind: AcquiredKind, } #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -364,25 +447,37 @@ pub enum AcquiredKind { } impl AcquiredSource { - /// Path of an `executable` declared on installation/hook. + /// Resolve an `executable` name to an absolute path inside the cache. + /// Only valid for managed sources (`self.base.is_some()`); callers + /// should special-case unmanaged sources (global cargo) before calling. + /// Cargo applies the platform exe suffix; github does not. pub fn resolve_executable(&self, name: &str) -> PathBuf { + let base = self + .base + .as_ref() + .expect("resolve_executable called on unmanaged source"); match self.kind { - AcquiredKind::Cargo => self.base.join("bin").join(platform_binary_exe(name)), - AcquiredKind::Github => self.base.join(name.trim_start_matches("./")), + AcquiredKind::Cargo => base.join("bin").join(platform_binary_exe(name)), + AcquiredKind::Github => base.join(name.trim_start_matches("./")), } } - /// Path of a `script` declared on installation/hook. + /// Resolve a `script` name to an absolute path inside the cache. + /// Same managed-only constraint as `resolve_executable`. pub fn resolve_script(&self, name: &str) -> PathBuf { - match self.kind { - AcquiredKind::Cargo => self.base.join("bin").join(name.trim_start_matches("./")), - AcquiredKind::Github => self.base.join(name.trim_start_matches("./")), - } + let base = self + .base + .as_ref() + .expect("resolve_script called on unmanaged source"); + base.join(if matches!(self.kind, AcquiredKind::Cargo) { + format!("bin/{}", name.trim_start_matches("./")) + } else { + name.trim_start_matches("./").to_string() + }) } } -/// Acquire a source, downloading / installing as needed. The returned -/// `AcquiredSource` is used to resolve `executable` / `script` paths. +/// Acquire a source, downloading / installing as needed. /// /// `executable_hint` is only used for cargo (to pick which binary to install /// for multi-binary crates, or as the binary name when using a git source). @@ -393,30 +488,21 @@ pub async fn acquire_source( ) -> Result { match source { Source::Cargo(c) => { - let (base, resolved) = acquire_cargo(sym, c, executable_hint).await?; + let acquired = acquire_cargo(sym, c, executable_hint).await?; Ok(AcquiredSource { - base, + base: acquired.cache_dir, + resolved_executable: acquired.resolved_executable, kind: AcquiredKind::Cargo, - resolved_executable: resolved, }) } Source::Github(g) => Ok(AcquiredSource { - base: acquire_github(sym, g).await?, - kind: AcquiredKind::Github, + base: Some(acquire_github(sym, g).await?), resolved_executable: None, + kind: AcquiredKind::Github, }), } } -/// What an installation resolves to once acquired. -#[derive(Debug)] -pub enum Runnable { - /// Run as a binary: `path args...`. - Exec(PathBuf), - /// Run as a shell script: `sh path args...`. - Script(PathBuf), -} - /// Ensure a path is executable on Unix. No-op on other platforms. pub fn make_executable(path: &Path) -> Result<()> { #[cfg(unix)] diff --git a/src/plugins.rs b/src/plugins.rs index 233ff36f..ff992b4c 100644 --- a/src/plugins.rs +++ b/src/plugins.rs @@ -549,15 +549,22 @@ fn validate_installation(install: &Installation) -> Result<()> { install.name ); } - if let Some(Source::Cargo(c)) = &install.source - && c.git.is_some() - && install.executable.is_none() - { - bail!( - "installation `{}`: cargo source with `git` requires `executable` to be set \ - (crates.io is not consulted, so the binary name is unknown)", - install.name - ); + if let Some(Source::Cargo(c)) = &install.source { + if c.git.is_some() && install.executable.is_none() { + bail!( + "installation `{}`: cargo source with `git` requires `executable` to be set \ + (crates.io is not consulted, so the binary name is unknown)", + install.name + ); + } + if c.global && install.executable.is_none() { + bail!( + "installation `{}`: cargo source with `global = true` requires `executable` to \ + be set (the binary is spawned by name via `$PATH` lookup, so we don't infer \ + it from crates.io)", + install.name + ); + } } Ok(()) } @@ -2589,6 +2596,65 @@ mod tests { from_str(toml).expect("parse"); } + /// `global = true` on cargo source round-trips through validation. + #[test] + fn cargo_global_field_round_trips() { + let toml = indoc! {r#" + name = "p" + crates = ["*"] + + [[installations]] + name = "rg" + source = "cargo" + crate = "ripgrep" + executable = "rg" + global = true + + [[hooks]] + name = "h" + event = "PreToolUse" + command = "rg" + "#}; + let plugin = from_str(toml).expect("parse"); + let install = plugin + .installations + .iter() + .find(|i| i.name == "rg") + .unwrap(); + match &install.source { + Some(Source::Cargo(c)) => assert!(c.global), + _ => panic!("expected cargo source"), + } + } + + /// Cargo source with `global = true` and no `executable` is rejected at + /// parse time — we don't infer the binary from crates.io for global + /// installs, so the user must say what to spawn. + #[test] + fn cargo_global_without_executable_errors() { + let toml = indoc! {r#" + name = "p" + crates = ["*"] + + [[installations]] + name = "rg" + source = "cargo" + crate = "ripgrep" + global = true + + [[hooks]] + name = "h" + event = "PreToolUse" + command = "rg" + "#}; + let err = from_str(toml).unwrap_err(); + assert!( + err.to_string() + .contains("`global = true` requires `executable`"), + "got: {err}" + ); + } + /// `git` field on cargo source round-trips through validation. #[test] fn cargo_git_field_round_trips() { diff --git a/tests/fixtures/plugin-hooks0/dot-symposium/plugins/test-plugin/SYMPOSIUM.toml b/tests/fixtures/plugin-hooks0/dot-symposium/plugins/test-plugin/SYMPOSIUM.toml index 5819f749..76b3ace2 100644 --- a/tests/fixtures/plugin-hooks0/dot-symposium/plugins/test-plugin/SYMPOSIUM.toml +++ b/tests/fixtures/plugin-hooks0/dot-symposium/plugins/test-plugin/SYMPOSIUM.toml @@ -80,3 +80,19 @@ matcher = "Task" command = "bare-setup" script = "$TEST_DIR/dot-symposium/plugins/test-plugin/scripts/hook-supplied.sh" format = "symposium" + +# Env-var test: a no-source requirement with an absolute `executable` exposes +# `$SYMPOSIUM_helper` to the hook script. The script echoes the env var so the +# integration test can assert the wiring. +[[installations]] +name = "helper" +executable = "$TEST_DIR/helper-bin" +install_commands = ["echo helper-payload > $TEST_DIR/helper-bin"] + +[[hooks]] +name = "env-var-test" +event = "PreToolUse" +matcher = "TodoWrite" +requirements = ["helper"] +command = { script = "$TEST_DIR/dot-symposium/plugins/test-plugin/scripts/echo-helper-env.sh" } +format = "symposium" diff --git a/tests/fixtures/plugin-hooks0/dot-symposium/plugins/test-plugin/scripts/echo-helper-env.sh b/tests/fixtures/plugin-hooks0/dot-symposium/plugins/test-plugin/scripts/echo-helper-env.sh new file mode 100644 index 00000000..b4804564 --- /dev/null +++ b/tests/fixtures/plugin-hooks0/dot-symposium/plugins/test-plugin/scripts/echo-helper-env.sh @@ -0,0 +1,2 @@ +#!/bin/sh +printf '{"PreToolUse":{"additionalContext":"helper-env:%s"}}\n' "$SYMPOSIUM_helper" diff --git a/tests/plugin_dispatch.rs b/tests/plugin_dispatch.rs index 4c588fee..dbad3ff8 100644 --- a/tests/plugin_dispatch.rs +++ b/tests/plugin_dispatch.rs @@ -222,6 +222,43 @@ async fn hook_supplies_script_against_bare_installation() { .unwrap(); } +/// `$SYMPOSIUM_` is set in the hook's environment, pointing at the +/// requirement's resolved executable. The script echoes the env var; the +/// expected value is the absolute path the fixture's `executable` resolves to. +#[tokio::test(flavor = "multi_thread")] +async fn helper_env_var_is_set_for_hook() { + with_fixture( + TestMode::SimulationOnly, + &["plugin-hooks0"], + async |mut ctx| { + let result = ctx + .prompt_or_hook( + "ignored", + &[HookStep::PreToolUse { + tool_name: "TodoWrite".to_string(), + tool_input: json!({"todos": []}), + }], + HookAgent::Claude, + ) + .await?; + + // The fixture's `helper` installation has `executable = "$TEST_DIR/helper-bin"`, + // expanded to an absolute path before the script runs. We assert the + // env var made it through with the `/helper-bin` suffix. + assert!( + result.has_context_containing("helper-env:") + && result.has_context_containing("/helper-bin"), + "expected hook env to contain `$SYMPOSIUM_helper` ending in /helper-bin, \ + got: {:#?}", + result.outputs_for(HookEvent::PreToolUse), + ); + Ok(()) + }, + ) + .await + .unwrap(); +} + /// The `matcher` field filters hooks by tool name. Firing a tool no hook /// matches produces no `additionalContext` in the merged output. #[tokio::test(flavor = "multi_thread")]