From 5cc5a664417a6b0c2108e2ee9d60296e8a67f070 Mon Sep 17 00:00:00 2001 From: Maksim Soltan Date: Tue, 14 Jul 2026 10:20:43 -0700 Subject: [PATCH 1/8] fix: preserve Cargo namespace argument ownership Implements [[tasks/meta-cargo-command-forwarding]] --- src/main.rs | 310 ++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 290 insertions(+), 20 deletions(-) diff --git a/src/main.rs b/src/main.rs index b7beca9..83be7db 100644 --- a/src/main.rs +++ b/src/main.rs @@ -462,8 +462,15 @@ fn main() -> Result<()> { log::debug!("cli.json = {}", cli.json); - // Check for orphaned nested meta repo and warn the user - check_and_warn_orphan(); + // Cargo/Rust namespace help must not inspect workspace configuration. + // Other command paths retain the existing orphan warning behavior. + let skip_orphan_check = matches!( + cli.command.as_ref(), + Some(Commands::External(args)) if is_cargo_namespace_help(args, cli.help) + ); + if !skip_orphan_check { + check_and_warn_orphan(); + } // Discover plugins early to handle --help requests and plugin listing let mut subprocess_plugins = SubprocessPluginManager::new(); @@ -532,17 +539,18 @@ fn main() -> Result<()> { handle_command_dispatch(args.command, &cli, &subprocess_plugins, true) } Some(Commands::External(args)) => { - // clap doesn't capture global flags that appear after an external - // subcommand name. Extract long-form global flags here so they - // work in both positions (before and after the subcommand). + // clap captures every token after an external subcommand name. + // Apply namespace-specific ownership before routing the command. let mut args = args; extract_global_flags(&mut args, &mut cli); // Keep root plugin help fast and plugin-aware, but let nested help // requests reach the matched plugin command implementation. if let Some(first) = args.first() { - let wants_help = args.iter().any(|a| a == "--help" || a == "-h"); + let wants_help = cli.help || contains_help_before_separator(&args); let is_bare = args.len() == 1; + let is_cargo_namespace = matches!(first.as_str(), "cargo" | "rust"); + let is_prefix_meta_help = cli.help && is_cargo_namespace; let is_root_help = wants_help && args.len() == 2 && matches!(args.get(1).map(String::as_str), Some("--help" | "-h")); @@ -554,7 +562,10 @@ fn main() -> Result<()> { .collect(); let is_promoted = promoted_commands.contains(&first.to_string()); - if is_root_help || (is_bare && !is_promoted) { + if is_prefix_meta_help + || is_root_help + || (is_bare && (!is_promoted || is_cargo_namespace)) + { if let Some(help_text) = subprocess_plugins.get_plugin_help(first) { println!("{help_text}"); return Ok(()); @@ -785,10 +796,11 @@ fn handle_command_dispatch( return Ok(()); } - // No config found — degraded legacy path with warning + // No config found — worktree paths are still authoritative for + // plugin dispatch, but config-backed tags/dependencies are unavailable. if cli.verbose { eprintln!( - "{} No .meta config found for worktree '{}'. Tags, plugins, and dependency features unavailable.", + "{} No .meta config found for worktree '{}'. Tags and dependency features unavailable.", "warning:".yellow().bold(), task_name ); @@ -801,10 +813,10 @@ fn handle_command_dispatch( let exclude_opt = none_if_empty(exclude_filters); let config = loop_lib::LoopConfig { - directories, + directories: directories.clone(), ignore: vec![], - include_filters: include_opt, - exclude_filters: exclude_opt, + include_filters: include_opt.clone(), + exclude_filters: exclude_opt.clone(), verbose: cli.verbose, silent: cli.silent, parallel, // Use the determined parallel mode, not hardcoded false @@ -817,7 +829,36 @@ fn handle_command_dispatch( root_dir: None, // Worktree paths don't use "." convention }; - run(&config, &command_str)?; + let subprocess_options = PluginRequestOptions { + json_output: cli.json, + verbose: cli.verbose, + parallel, + dry_run, + silent: cli.silent, + recursive, + depth, + include_filters: include_opt, + exclude_filters: exclude_opt, + strict: cli.strict, + }; + + if plugins.execute( + &command_str, + &command_args, + &directories, + subprocess_options, + )? { + if cli.verbose { + println!( + "{}", + "Command handled by subprocess plugin (worktree without config).".green() + ); + } + } else if is_explicit_exec { + run(&config, &command_str)?; + } else { + unrecognized_command_error(&command_args, &command_str, plugins); + } return Ok(()); } } @@ -1245,18 +1286,72 @@ fn handle_plugin_command( // === Helpers === -/// Extract meta-only global flags from external subcommand args. +/// Whether an external command explicitly enters Cargo's namespace. +fn is_cargo_namespace(args: &[String]) -> bool { + matches!(args.first().map(String::as_str), Some("cargo" | "rust")) +} + +/// Whether a help flag appears before the command's `--` separator. +fn contains_help_before_separator(args: &[String]) -> bool { + args.iter() + .take_while(|arg| arg.as_str() != "--") + .any(|arg| matches!(arg.as_str(), "--help" | "-h")) +} + +/// Cargo/Rust help paths that must not inspect Meta workspace configuration. +fn is_cargo_namespace_help(args: &[String], prefix_help: bool) -> bool { + is_cargo_namespace(args) + && (prefix_help || args.len() == 1 || contains_help_before_separator(args)) +} + +/// Extract Meta-owned global flags from external subcommand args. /// /// clap's `external_subcommand` captures all tokens after the first unrecognized -/// subcommand, including global flags like `--json`. This function pulls them -/// out and applies them to the CLI struct so they work regardless of position. +/// subcommand, including global flags like `--json`. For non-Cargo namespaces, +/// this function pulls Meta-owned flags out and applies them to the CLI struct. /// -/// Only extracts flags that are meta-global and NOT reused by plugin subcommands. -/// Flags like `--dry-run` and `--parallel` are left in args because plugin -/// subcommands (e.g. `worktree prune --dry-run`, `worktree exec --parallel`) -/// define their own versions and need to see them. +/// Cargo and Rust explicitly own all options after their namespace. The sole +/// compatibility exception is postfix `--recursive` for build, test, and clean, +/// which continues to select nested Meta projects. No arguments after a `--` +/// separator are inspected. Other namespaces retain the existing extraction +/// behavior for Meta-only global flags. fn extract_global_flags(args: &mut Vec, cli: &mut Cli) { + if is_cargo_namespace(args) { + let supports_recursive_compat = matches!( + args.get(1).map(String::as_str), + Some("build" | "test" | "clean") + ); + + if supports_recursive_compat { + let mut after_separator = false; + args.retain(|arg| { + if after_separator { + return true; + } + if arg == "--" { + after_separator = true; + return true; + } + if arg == "--recursive" { + cli.recursive = true; + return false; + } + true + }); + } + return; + } + + let mut after_separator = false; args.retain(|arg| { + if after_separator { + return true; + } + if arg == "--" { + after_separator = true; + return true; + } + match arg.as_str() { "--json" => { cli.json = true; @@ -1423,6 +1518,181 @@ mod tests { use std::io::Write; use tempfile::NamedTempFile; + fn empty_cli() -> Cli { + Cli::try_parse_from(["meta"]).unwrap() + } + + fn strings(values: &[&str]) -> Vec { + values.iter().map(|value| (*value).to_string()).collect() + } + + #[test] + fn test_cargo_recursive_compatibility_is_narrow() { + for namespace in ["cargo", "rust"] { + for subcommand in ["build", "test", "clean"] { + let mut cli = empty_cli(); + let mut args = strings(&[namespace, subcommand, "--all", "--recursive"]); + + extract_global_flags(&mut args, &mut cli); + + assert!(cli.recursive, "{namespace} {subcommand}"); + assert_eq!(args, strings(&[namespace, subcommand, "--all"])); + } + } + + for subcommand in ["update", "nextest"] { + let mut cli = empty_cli(); + let mut args = strings(&["cargo", subcommand, "--recursive"]); + let expected = args.clone(); + + extract_global_flags(&mut args, &mut cli); + + assert!(!cli.recursive, "cargo {subcommand}"); + assert_eq!(args, expected); + } + } + + #[test] + fn test_cargo_postfix_flags_and_separator_payload_are_cargo_owned() { + let mut cli = empty_cli(); + let mut args = strings(&[ + "cargo", + "test", + "--verbose", + "--json", + "--", + "--recursive", + "--help", + "--silent", + "--primary", + "--strict", + ]); + let expected = args.clone(); + + extract_global_flags(&mut args, &mut cli); + + assert_eq!(args, expected); + assert!(!cli.recursive); + assert!(!cli.verbose); + assert!(!cli.json); + assert!(!cli.silent); + assert!(!cli.primary); + assert!(!cli.strict); + assert!(!contains_help_before_separator(&args)); + } + + #[test] + fn test_non_cargo_extraction_stops_at_separator() { + let mut cli = empty_cli(); + let mut args = strings(&[ + "git", + "status", + "--verbose", + "--recursive", + "--", + "--json", + "--silent", + "--primary", + "--strict", + ]); + + extract_global_flags(&mut args, &mut cli); + + assert_eq!( + args, + strings(&[ + "git", + "status", + "--", + "--json", + "--silent", + "--primary", + "--strict", + ]) + ); + assert!(cli.verbose); + assert!(cli.recursive); + assert!(!cli.json); + assert!(!cli.silent); + assert!(!cli.primary); + assert!(!cli.strict); + } + + #[test] + fn test_non_cargo_global_flag_extraction_remains_compatible() { + let mut cli = empty_cli(); + let mut args = strings(&[ + "git", + "status", + "--json", + "--verbose", + "--silent", + "--primary", + "--recursive", + "--strict", + ]); + + extract_global_flags(&mut args, &mut cli); + + assert_eq!(args, strings(&["git", "status"])); + assert!(cli.json); + assert!(cli.verbose); + assert!(cli.silent); + assert!(cli.primary); + assert!(cli.recursive); + assert!(cli.strict); + } + + #[test] + fn test_cargo_help_classification_stops_at_separator() { + assert!(is_cargo_namespace_help(&strings(&["cargo"]), false)); + assert!(is_cargo_namespace_help(&strings(&["rust"]), false)); + assert!(is_cargo_namespace_help( + &strings(&["cargo", "check", "--help"]), + false + )); + assert!(is_cargo_namespace_help( + &strings(&["rust", "nextest", "-h"]), + false + )); + assert!(is_cargo_namespace_help(&strings(&["cargo", "check"]), true)); + + assert!(!is_cargo_namespace_help( + &strings(&["cargo", "test", "--", "--help"]), + false + )); + assert!(!is_cargo_namespace_help( + &strings(&["cargo", "check"]), + false + )); + assert!(!is_cargo_namespace_help( + &strings(&["git", "status", "--help"]), + false + )); + } + + #[test] + fn test_meta_controls_before_cargo_namespace_remain_global() { + let cli = Cli::try_parse_from([ + "meta", + "--dry-run", + "--verbose", + "cargo", + "check", + "--verbose", + ]) + .unwrap(); + + assert!(cli.dry_run); + assert!(cli.verbose); + match cli.command { + Some(Commands::External(args)) => { + assert_eq!(args, strings(&["cargo", "check", "--verbose"])); + } + _ => panic!("expected external Cargo command"), + } + } + #[test] fn test_parse_meta_config_valid_simple_format() { let mut file = NamedTempFile::new().unwrap(); From ff494b9de75eed3740ee36b5eb53bc199eb3a43c Mon Sep 17 00:00:00 2001 From: Maksim Soltan Date: Tue, 14 Jul 2026 10:32:46 -0700 Subject: [PATCH 2/8] fix: avoid duplicate Cargo project filtering Implements [[tasks/meta-cargo-command-forwarding]] --- src/subprocess_plugins.rs | 116 +++++++++++++++++++++++++++++++++++++- 1 file changed, 115 insertions(+), 1 deletion(-) diff --git a/src/subprocess_plugins.rs b/src/subprocess_plugins.rs index ed8a2f9..aea6328 100644 --- a/src/subprocess_plugins.rs +++ b/src/subprocess_plugins.rs @@ -27,6 +27,34 @@ pub struct SubprocessPluginManager { verbose: bool, } +/// Options used after a plugin has returned an execution plan. +/// +/// The Rust plugin's Cargo/Rust namespace implementation applies +/// include/exclude filters while planning so it can distinguish a selected +/// scope with no Cargo projects from an unfiltered scope. Reapplying those +/// string filters to its normalized Windows paths can drop commands whose +/// original path spelling used a short name or different casing. Other and +/// older plugins continue to rely on the host execution layer for filtering. +fn options_for_plan_execution( + plugin: &PluginInfo, + command: &str, + options: &PluginRequestOptions, +) -> PluginRequestOptions { + let mut execution_options = options.clone(); + let namespace = command.split_whitespace().next().unwrap_or_default(); + let owns_namespace_root = plugin + .commands + .iter() + .any(|registered| registered == namespace); + + if plugin.name == "rust" && matches!(namespace, "cargo" | "rust") && owns_namespace_root { + execution_options.include_filters = None; + execution_options.exclude_filters = None; + } + + execution_options +} + impl Default for SubprocessPluginManager { fn default() -> Self { Self::new() @@ -313,7 +341,8 @@ impl SubprocessPluginManager { match serde_json::from_str::(&stdout_str) { Ok(response) => { // Plugin returned an execution plan - execute it via loop_lib - self.execute_plan(&response.plan, options) + let execution_options = options_for_plan_execution(&plugin.info, command, options); + self.execute_plan(&response.plan, &execution_options) } Err(_) => { // Couldn't parse as our protocol - print output as-is (legacy behavior) @@ -673,6 +702,19 @@ fn is_executable(path: &Path) -> bool { mod tests { use super::*; + fn plugin_info(name: &str, commands: &[&str]) -> PluginInfo { + PluginInfo { + name: name.to_string(), + version: "1.0.0".to_string(), + commands: commands + .iter() + .map(|command| (*command).to_string()) + .collect(), + description: None, + help: None, + } + } + #[test] fn test_plugin_manager_new() { let manager = SubprocessPluginManager::new(); @@ -741,6 +783,78 @@ mod tests { assert!(options.exclude_filters.is_none()); } + #[test] + fn test_rust_plan_execution_does_not_reapply_directory_filters() { + let options = PluginRequestOptions { + json_output: true, + parallel: true, + dry_run: true, + include_filters: Some(vec!["included".to_string()]), + exclude_filters: Some(vec!["excluded".to_string()]), + ..Default::default() + }; + let plugin = plugin_info("rust", &["cargo", "rust"]); + + for namespace in ["cargo", "rust"] { + let execution = options_for_plan_execution(&plugin, namespace, &options); + + assert!(execution.include_filters.is_none()); + assert!(execution.exclude_filters.is_none()); + assert!(execution.json_output); + assert!(execution.parallel); + assert!(execution.dry_run); + } + + // Planning still receives the original options unchanged. + assert_eq!( + options.include_filters.as_deref(), + Some(&["included".to_string()][..]) + ); + assert_eq!( + options.exclude_filters.as_deref(), + Some(&["excluded".to_string()][..]) + ); + } + + #[test] + fn test_other_plugin_plans_keep_directory_filters() { + let options = PluginRequestOptions { + include_filters: Some(vec!["included".to_string()]), + exclude_filters: Some(vec!["excluded".to_string()]), + ..Default::default() + }; + + for (plugin, command) in [ + (plugin_info("git", &["git status"]), "git status"), + (plugin_info("cargo-wrapper", &["cargo"]), "cargo"), + ] { + let execution = options_for_plan_execution(&plugin, command, &options); + + assert_eq!(execution.include_filters, options.include_filters); + assert_eq!(execution.exclude_filters, options.exclude_filters); + } + + let non_namespace = + options_for_plan_execution(&plugin_info("rust", &["cargo", "rust"]), "build", &options); + assert_eq!(non_namespace.include_filters, options.include_filters); + assert_eq!(non_namespace.exclude_filters, options.exclude_filters); + } + + #[test] + fn test_legacy_rust_plugin_exact_commands_keep_directory_filters() { + let options = PluginRequestOptions { + include_filters: Some(vec!["included".to_string()]), + exclude_filters: Some(vec!["excluded".to_string()]), + ..Default::default() + }; + let legacy = plugin_info("rust", &["cargo build", "cargo test"]); + + let execution = options_for_plan_execution(&legacy, "cargo build", &options); + + assert_eq!(execution.include_filters, options.include_filters); + assert_eq!(execution.exclude_filters, options.exclude_filters); + } + #[test] fn test_handles_command_matching() { let mut manager = SubprocessPluginManager::new(); From 39d65dd4fa15f3eb40096bbb19bbf78954678334 Mon Sep 17 00:00:00 2001 From: Maksim Soltan Date: Tue, 14 Jul 2026 11:37:22 -0700 Subject: [PATCH 3/8] fix: harden Cargo namespace routing Implements [[tasks/meta-cargo-command-forwarding]] --- src/main.rs | 180 ++++++++++++++++++++++++++++++++++---- src/subprocess_plugins.rs | 59 +++++++++++-- 2 files changed, 215 insertions(+), 24 deletions(-) diff --git a/src/main.rs b/src/main.rs index 83be7db..d231132 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1291,6 +1291,94 @@ fn is_cargo_namespace(args: &[String]) -> bool { matches!(args.first().map(String::as_str), Some("cargo" | "rust")) } +/// Locate Cargo's subcommand after its leading global options. +/// +/// Cargo accepts an optional rustup `+toolchain` selector followed by global +/// flags before the command name. This parser recognizes only that documented +/// leading grammar and returns `None` for terminal, malformed, or unknown +/// forms, keeping ambiguous `--recursive` tokens Cargo-owned. It never scans +/// beyond Cargo's `--` separator. +fn cargo_subcommand_index(args: &[String]) -> Option { + if !is_cargo_namespace(args) { + return None; + } + + let separator = args + .iter() + .position(|arg| arg == "--") + .unwrap_or(args.len()); + let mut index = 1; + + if index < separator + && args[index] + .strip_prefix('+') + .is_some_and(|toolchain| !toolchain.is_empty()) + { + index += 1; + } + + while index < separator { + let argument = args[index].as_str(); + match argument { + // Global modifiers that do not consume a value. + "--locked" | "--offline" | "--frozen" | "--verbose" | "-q" | "--quiet" => { + index += 1; + } + // These modes exit without dispatching a Cargo subcommand. + "-V" | "--version" | "--list" | "--explain" | "-h" | "--help" => { + return None; + } + // Global options whose value is the following token. + "--color" | "--config" | "-C" => { + if index + 1 >= separator || args[index + 1].starts_with('-') { + return None; + } + index += 2; + } + "-Z" => { + if index + 1 >= separator + || args[index + 1].starts_with('-') + || args[index + 1] == "script" + { + return None; + } + index += 2; + } + _ if argument.starts_with("--color=") || argument.starts_with("--config=") => { + index += 1; + } + _ if argument.starts_with("--explain=") => return None, + _ if argument.strip_prefix('-').is_some_and(|flags| { + !flags.is_empty() && flags.chars().all(|flag| flag == 'v') + }) => + { + index += 1; + } + _ if argument + .strip_prefix("-C") + .is_some_and(|path| !path.is_empty()) => + { + index += 1; + } + _ if argument + .strip_prefix("-Z") + .is_some_and(|flag| !flag.is_empty()) => + { + if argument == "-Zscript" { + return None; + } + index += 1; + } + // Unknown leading options remain Cargo-owned rather than causing + // Meta to search later tokens for a familiar command name. + _ if argument.starts_with('-') => return None, + _ => return Some(index), + } + } + + None +} + /// Whether a help flag appears before the command's `--` separator. fn contains_help_before_separator(args: &[String]) -> bool { args.iter() @@ -1317,26 +1405,24 @@ fn is_cargo_namespace_help(args: &[String], prefix_help: bool) -> bool { /// behavior for Meta-only global flags. fn extract_global_flags(args: &mut Vec, cli: &mut Cli) { if is_cargo_namespace(args) { - let supports_recursive_compat = matches!( - args.get(1).map(String::as_str), - Some("build" | "test" | "clean") - ); - - if supports_recursive_compat { - let mut after_separator = false; + let subcommand_index = cargo_subcommand_index(args); + let supports_recursive_compat = subcommand_index + .is_some_and(|index| matches!(args[index].as_str(), "build" | "test" | "clean")); + + if let (true, Some(subcommand_index)) = (supports_recursive_compat, subcommand_index) { + let separator = args + .iter() + .position(|arg| arg == "--") + .unwrap_or(args.len()); + let mut index = 0; args.retain(|arg| { - if after_separator { - return true; - } - if arg == "--" { - after_separator = true; - return true; - } - if arg == "--recursive" { + let remove = + index > subcommand_index && index < separator && arg.as_str() == "--recursive"; + index += 1; + if remove { cli.recursive = true; - return false; } - true + !remove }); } return; @@ -1552,6 +1638,66 @@ mod tests { } } + #[test] + fn test_cargo_recursive_compatibility_finds_the_actual_subcommand() { + for (input, expected) in [ + ( + &["cargo", "--locked", "clean", "--recursive"][..], + &["cargo", "--locked", "clean"][..], + ), + ( + &["rust", "+nightly", "--offline", "test", "--recursive"][..], + &["rust", "+nightly", "--offline", "test"][..], + ), + ( + &["cargo", "--color", "always", "build", "--recursive"][..], + &["cargo", "--color", "always", "build"][..], + ), + ( + &[ + "cargo", + "-Zunstable-options", + "-C", + "crate", + "clean", + "--recursive", + ][..], + &["cargo", "-Zunstable-options", "-C", "crate", "clean"][..], + ), + ] { + let mut cli = empty_cli(); + let mut args = strings(input); + + extract_global_flags(&mut args, &mut cli); + + assert!(cli.recursive, "{input:?}"); + assert_eq!(args, strings(expected), "{input:?}"); + } + } + + #[test] + fn test_cargo_subcommand_detection_is_conservative() { + for input in [ + &["cargo", "--config", "clean", "update", "--recursive"][..], + &["cargo", "--config", "--locked", "clean", "--recursive"][..], + &["cargo", "--color", "--locked", "clean", "--recursive"][..], + &["cargo", "-C", "--locked", "clean", "--recursive"][..], + &["cargo", "-Z", "--locked", "clean", "--recursive"][..], + &["cargo", "--locked", "--", "clean", "--recursive"][..], + &["cargo", "--recursive", "clean"][..], + &["cargo", "-Zscript", "clean", "--recursive"][..], + ] { + let mut cli = empty_cli(); + let mut args = strings(input); + let expected = args.clone(); + + extract_global_flags(&mut args, &mut cli); + + assert!(!cli.recursive, "{input:?}"); + assert_eq!(args, expected, "{input:?}"); + } + } + #[test] fn test_cargo_postfix_flags_and_separator_payload_are_cargo_owned() { let mut cli = empty_cli(); diff --git a/src/subprocess_plugins.rs b/src/subprocess_plugins.rs index aea6328..b2d13b0 100644 --- a/src/subprocess_plugins.rs +++ b/src/subprocess_plugins.rs @@ -47,7 +47,7 @@ fn options_for_plan_execution( .iter() .any(|registered| registered == namespace); - if plugin.name == "rust" && matches!(namespace, "cargo" | "rust") && owns_namespace_root { + if is_rust_namespace_command(plugin, command) && owns_namespace_root { execution_options.include_filters = None; execution_options.exclude_filters = None; } @@ -55,6 +55,16 @@ fn options_for_plan_execution( execution_options } +/// Whether trusted Rust-plugin routing explicitly entered a Cargo namespace. +/// +/// This uses plugin identity and the matched command rather than inspecting a +/// returned shell plan. It also covers legacy multi-word Rust registrations so +/// Cargo remains authoritative when older plugins are installed. +fn is_rust_namespace_command(plugin: &PluginInfo, command: &str) -> bool { + let namespace = command.split_whitespace().next().unwrap_or_default(); + plugin.name == "rust" && matches!(namespace, "cargo" | "rust") +} + impl Default for SubprocessPluginManager { fn default() -> Self { Self::new() @@ -342,7 +352,8 @@ impl SubprocessPluginManager { Ok(response) => { // Plugin returned an execution plan - execute it via loop_lib let execution_options = options_for_plan_execution(&plugin.info, command, options); - self.execute_plan(&response.plan, &execution_options) + let expand_loop_aliases = !is_rust_namespace_command(&plugin.info, command); + self.execute_plan(&response.plan, &execution_options, expand_loop_aliases) } Err(_) => { // Couldn't parse as our protocol - print output as-is (legacy behavior) @@ -353,8 +364,20 @@ impl SubprocessPluginManager { } /// Execute an execution plan via loop_lib - fn execute_plan(&self, plan: &ExecutionPlan, options: &PluginRequestOptions) -> Result { - use loop_lib::{run_commands, DirCommand, LoopConfig}; + fn execute_plan( + &self, + plan: &ExecutionPlan, + options: &PluginRequestOptions, + expand_loop_aliases: bool, + ) -> Result { + use loop_lib::{run_commands, run_commands_without_loop_aliases, DirCommand, LoopConfig}; + + let run_plan_commands: fn(&LoopConfig, &[DirCommand]) -> Result<()> = if expand_loop_aliases + { + run_commands + } else { + run_commands_without_loop_aliases + }; // Phase 1: Run pre_commands sequentially (setup tasks like SSH ControlMaster) if !plan.pre_commands.is_empty() { @@ -388,7 +411,7 @@ impl SubprocessPluginManager { }; // Ignore failures for pre_commands (e.g., SSH socket already exists) // The main commands will fail if setup was actually needed - if let Err(e) = run_commands(&pre_config, &[cmd]) { + if let Err(e) = run_plan_commands(&pre_config, &[cmd]) { if options.verbose { eprintln!("Pre-command failed (continuing): {e}"); } @@ -428,7 +451,7 @@ impl SubprocessPluginManager { root_dir, }; - run_commands(&config, &commands)?; + run_plan_commands(&config, &commands)?; } // Phase 3: Run post_commands sequentially (cleanup tasks) @@ -456,7 +479,7 @@ impl SubprocessPluginManager { cmd: post_cmd.cmd.clone(), env: post_cmd.env.clone(), }; - if let Err(e) = run_commands(&post_config, &[cmd]) { + if let Err(e) = run_plan_commands(&post_config, &[cmd]) { if options.verbose { eprintln!("Post-command failed: {e}"); } @@ -783,6 +806,28 @@ mod tests { assert!(options.exclude_filters.is_none()); } + #[test] + fn test_only_rust_namespace_commands_disable_loop_aliases() { + let rust = plugin_info("rust", &["cargo", "rust"]); + for command in ["cargo", "cargo clean", "rust", "rust test"] { + assert!(is_rust_namespace_command(&rust, command), "{command}"); + } + + // Legacy exact registrations still represent the same Cargo authority + // boundary even though they do not own a namespace root. + let legacy_rust = plugin_info("rust", &["cargo build", "rust test"]); + assert!(is_rust_namespace_command(&legacy_rust, "cargo build")); + assert!(is_rust_namespace_command(&legacy_rust, "rust test")); + + for (plugin, command) in [ + (plugin_info("git", &["git status"]), "git status"), + (plugin_info("cargo-wrapper", &["cargo"]), "cargo"), + (plugin_info("rust", &["build"]), "build"), + ] { + assert!(!is_rust_namespace_command(&plugin, command), "{command}"); + } + } + #[test] fn test_rust_plan_execution_does_not_reapply_directory_filters() { let options = PluginRequestOptions { From 07d2793b81ad2e35b47076afb8ece901d4498ebd Mon Sep 17 00:00:00 2001 From: Maksim Soltan Date: Tue, 14 Jul 2026 12:55:09 -0700 Subject: [PATCH 4/8] fix: close Cargo forwarding review gaps Implements [[tasks/meta-cargo-command-forwarding]] Related: [[tasks/harmony-677]], [[incidents/inc-085-meta-subcommand-help-text]], [[context/immutable/patterns]] --- src/main.rs | 201 +++++++++++++++++++++++++++++------- src/subprocess_plugins.rs | 28 +++-- tests/command_forwarding.rs | 118 +++++++++++++++++++++ 3 files changed, 306 insertions(+), 41 deletions(-) create mode 100644 tests/command_forwarding.rs diff --git a/src/main.rs b/src/main.rs index d231132..4268f54 100644 --- a/src/main.rs +++ b/src/main.rs @@ -544,13 +544,19 @@ fn main() -> Result<()> { let mut args = args; extract_global_flags(&mut args, &mut cli); + let is_cargo_namespace = is_cargo_namespace(&args); + let has_forwarded_help = contains_help_before_separator(&args); + if cli.help && !is_cargo_namespace && !has_forwarded_help { + print_help_with_plugins(&subprocess_plugins, false); + return Ok(()); + } + // Keep root plugin help fast and plugin-aware, but let nested help // requests reach the matched plugin command implementation. if let Some(first) = args.first() { - let wants_help = cli.help || contains_help_before_separator(&args); let is_bare = args.len() == 1; - let is_cargo_namespace = matches!(first.as_str(), "cargo" | "rust"); let is_prefix_meta_help = cli.help && is_cargo_namespace; + let wants_help = is_prefix_meta_help || has_forwarded_help; let is_root_help = wants_help && args.len() == 2 && matches!(args.get(1).map(String::as_str), Some("--help" | "-h")); @@ -961,10 +967,11 @@ fn handle_command_dispatch( strict: cli.strict, }; - if plugins.execute( + if plugins.execute_with_root( &command_str, &command_args, &project_paths, + Some(meta_dir), subprocess_options, )? { log::info!("Command was handled by subprocess plugin"); @@ -1291,6 +1298,50 @@ fn is_cargo_namespace(args: &[String]) -> bool { matches!(args.first().map(String::as_str), Some("cargo" | "rust")) } +/// Result of parsing a documented Cargo short-option cluster. +enum CargoShortOption { + Complete, + NeedsValue, + Terminal, +} + +/// Parse Cargo's supported leading short options conservatively. +/// +/// `v` may repeat, while `q` cannot repeat or combine with `v`. `C` and `Z` +/// consume the rest of the cluster as their value, or the following token when +/// they end the cluster. Help and version are terminal, and unknown flags +/// reject the whole cluster. +fn cargo_short_option(argument: &str) -> Option { + let flags = argument.strip_prefix('-')?; + if flags.is_empty() || flags.starts_with('-') { + return None; + } + + let mut saw_quiet = false; + let mut saw_verbose = false; + for (offset, flag) in flags.char_indices() { + match flag { + 'v' if !saw_quiet => saw_verbose = true, + 'q' if !saw_quiet && !saw_verbose => saw_quiet = true, + 'h' | 'V' => return Some(CargoShortOption::Terminal), + 'C' | 'Z' => { + return Some(if offset + flag.len_utf8() < flags.len() { + CargoShortOption::Complete + } else { + CargoShortOption::NeedsValue + }); + } + _ => return None, + } + } + + Some(CargoShortOption::Complete) +} + +fn is_cargo_color(value: &str) -> bool { + matches!(value, "auto" | "always" | "never") +} + /// Locate Cargo's subcommand after its leading global options. /// /// Cargo accepts an optional rustup `+toolchain` selector followed by global @@ -1328,50 +1379,41 @@ fn cargo_subcommand_index(args: &[String]) -> Option { "-V" | "--version" | "--list" | "--explain" | "-h" | "--help" => { return None; } - // Global options whose value is the following token. - "--color" | "--config" | "-C" => { - if index + 1 >= separator || args[index + 1].starts_with('-') { + "--color" => { + if index + 1 >= separator || !is_cargo_color(&args[index + 1]) { return None; } index += 2; } - "-Z" => { - if index + 1 >= separator - || args[index + 1].starts_with('-') - || args[index + 1] == "script" - { + // Global options whose value is the following token. + "--config" => { + if index + 1 >= separator || args[index + 1].starts_with('-') { return None; } index += 2; } - _ if argument.starts_with("--color=") || argument.starts_with("--config=") => { - index += 1; - } - _ if argument.starts_with("--explain=") => return None, - _ if argument.strip_prefix('-').is_some_and(|flags| { - !flags.is_empty() && flags.chars().all(|flag| flag == 'v') - }) => - { - index += 1; - } _ if argument - .strip_prefix("-C") - .is_some_and(|path| !path.is_empty()) => + .strip_prefix("--color=") + .is_some_and(is_cargo_color) => { index += 1; } - _ if argument - .strip_prefix("-Z") - .is_some_and(|flag| !flag.is_empty()) => - { - if argument == "-Zscript" { - return None; + _ if argument.starts_with("--color=") => return None, + _ if argument.starts_with("--config=") => index += 1, + _ if argument.starts_with("--explain=") => return None, + _ if argument.starts_with('-') => match cargo_short_option(argument) { + Some(CargoShortOption::Complete) => index += 1, + Some(CargoShortOption::NeedsValue) => { + if index + 1 >= separator + || args[index + 1].is_empty() + || args[index + 1].starts_with('-') + { + return None; + } + index += 2; } - index += 1; - } - // Unknown leading options remain Cargo-owned rather than causing - // Meta to search later tokens for a familiar command name. - _ if argument.starts_with('-') => return None, + Some(CargoShortOption::Terminal) | None => return None, + }, _ => return Some(index), } } @@ -1675,6 +1717,88 @@ mod tests { } } + #[test] + fn test_cargo_color_values_are_validated_before_recursive_compatibility() { + for color in ["auto", "always", "never"] { + let attached = format!("--color={color}"); + for mut args in [ + strings(&["cargo", "--color", color, "build", "--recursive"]), + vec![ + "cargo".to_string(), + attached.clone(), + "build".to_string(), + "--recursive".to_string(), + ], + ] { + let mut cli = empty_cli(); + let mut expected = args.clone(); + expected.pop(); + + extract_global_flags(&mut args, &mut cli); + + assert!(cli.recursive, "{args:?}"); + assert_eq!(args, expected); + } + } + + for input in [ + &["cargo", "--color", "sometimes", "build", "--recursive"][..], + &["cargo", "--color=sometimes", "build", "--recursive"][..], + &["cargo", "--color", "", "build", "--recursive"][..], + &["cargo", "--color=", "build", "--recursive"][..], + &["cargo", "--color"][..], + ] { + let mut cli = empty_cli(); + let mut args = strings(input); + let expected = args.clone(); + + extract_global_flags(&mut args, &mut cli); + + assert!(!cli.recursive, "{input:?}"); + assert_eq!(args, expected, "{input:?}"); + } + } + + #[test] + fn test_cargo_script_mode_continues_to_the_manifest_path() { + for (input, expected_index) in [ + (&["cargo", "-Z", "script", "./tool.rs"][..], 3), + (&["cargo", "-Zscript", "./tool.rs"][..], 2), + ] { + let mut args = strings(input); + assert_eq!(cargo_subcommand_index(&args), Some(expected_index)); + + args.push("--recursive".to_string()); + let expected = args.clone(); + let mut cli = empty_cli(); + + extract_global_flags(&mut args, &mut cli); + + assert!(!cli.recursive, "{input:?}"); + assert_eq!(args, expected, "{input:?}"); + } + } + + #[test] + fn test_cargo_short_option_clusters_preserve_recursive_compatibility() { + for input in [ + &["cargo", "-vC.", "build", "--recursive"][..], + &["cargo", "-qZunstable-options", "clean", "--recursive"][..], + &["cargo", "-vC", ".", "test", "--recursive"][..], + &["cargo", "-qZ", "unstable-options", "build", "--recursive"][..], + ] { + let mut cli = empty_cli(); + let mut args = strings(input); + let mut expected = args.clone(); + expected.pop(); + + extract_global_flags(&mut args, &mut cli); + + assert!(cli.recursive, "{input:?}"); + assert_eq!(args, expected, "{input:?}"); + } + } + #[test] fn test_cargo_subcommand_detection_is_conservative() { for input in [ @@ -1685,7 +1809,14 @@ mod tests { &["cargo", "-Z", "--locked", "clean", "--recursive"][..], &["cargo", "--locked", "--", "clean", "--recursive"][..], &["cargo", "--recursive", "clean"][..], - &["cargo", "-Zscript", "clean", "--recursive"][..], + &["cargo", "-vX", "clean", "--recursive"][..], + &["cargo", "-qZ", "--locked", "clean", "--recursive"][..], + &["cargo", "-vC", "--locked", "clean", "--recursive"][..], + &["cargo", "-Vv", "clean", "--recursive"][..], + &["cargo", "-vh", "clean", "--recursive"][..], + &["cargo", "-qq", "build", "--recursive"][..], + &["cargo", "-vq", "clean", "--recursive"][..], + &["cargo", "-qv", "clean", "--recursive"][..], ] { let mut cli = empty_cli(); let mut args = strings(input); diff --git a/src/subprocess_plugins.rs b/src/subprocess_plugins.rs index b2d13b0..c5f893d 100644 --- a/src/subprocess_plugins.rs +++ b/src/subprocess_plugins.rs @@ -240,6 +240,18 @@ impl SubprocessPluginManager { args: &[String], projects: &[String], options: PluginRequestOptions, + ) -> Result { + self.execute_with_root(command, args, projects, None, options) + } + + /// Execute a command while preserving the caller's actual Meta root. + pub fn execute_with_root( + &self, + command: &str, + args: &[String], + projects: &[String], + root_dir: Option<&Path>, + options: PluginRequestOptions, ) -> Result { let cmd_parts: Vec<&str> = command.split_whitespace().collect(); if cmd_parts.is_empty() { @@ -274,7 +286,7 @@ impl SubprocessPluginManager { } if let Some((plugin, matched_cmd)) = best_match { - return self.execute_plugin(plugin, matched_cmd, args, projects, &options); + return self.execute_plugin(plugin, matched_cmd, args, projects, root_dir, &options); } Ok(false) @@ -287,6 +299,7 @@ impl SubprocessPluginManager { command: &str, args: &[String], projects: &[String], + root_dir: Option<&Path>, options: &PluginRequestOptions, ) -> Result { // Extract the remaining args after the matched command @@ -353,7 +366,12 @@ impl SubprocessPluginManager { // Plugin returned an execution plan - execute it via loop_lib let execution_options = options_for_plan_execution(&plugin.info, command, options); let expand_loop_aliases = !is_rust_namespace_command(&plugin.info, command); - self.execute_plan(&response.plan, &execution_options, expand_loop_aliases) + self.execute_plan( + &response.plan, + &execution_options, + root_dir, + expand_loop_aliases, + ) } Err(_) => { // Couldn't parse as our protocol - print output as-is (legacy behavior) @@ -368,6 +386,7 @@ impl SubprocessPluginManager { &self, plan: &ExecutionPlan, options: &PluginRequestOptions, + root_dir: Option<&Path>, expand_loop_aliases: bool, ) -> Result { use loop_lib::{run_commands, run_commands_without_loop_aliases, DirCommand, LoopConfig}; @@ -431,9 +450,6 @@ impl SubprocessPluginManager { }) .collect(); - // The first command's directory is the meta root (should display as ".") - let root_dir = commands.first().map(|c| PathBuf::from(&c.dir)); - let config = LoopConfig { directories: vec![], ignore: vec![], @@ -448,7 +464,7 @@ impl SubprocessPluginManager { spawn_stagger_ms: plan.spawn_stagger_ms.unwrap_or(0), env: None, max_parallel: plan.max_parallel, - root_dir, + root_dir: root_dir.map(Path::to_path_buf), }; run_plan_commands(&config, &commands)?; diff --git a/tests/command_forwarding.rs b/tests/command_forwarding.rs new file mode 100644 index 0000000..b429946 --- /dev/null +++ b/tests/command_forwarding.rs @@ -0,0 +1,118 @@ +#![cfg(unix)] + +use std::fs; +use std::os::unix::fs::PermissionsExt; +use std::path::Path; +use std::process::Command; +use tempfile::tempdir; + +fn write_executable(path: &Path, contents: &str) { + fs::write(path, contents).unwrap(); + let mut permissions = fs::metadata(path).unwrap().permissions(); + permissions.set_mode(0o755); + fs::set_permissions(path, permissions).unwrap(); +} + +#[test] +fn prefix_help_does_not_execute_non_cargo_plugin() { + let temp = tempdir().unwrap(); + let plugin_dir = temp.path().join("bin"); + fs::create_dir(&plugin_dir).unwrap(); + let marker = temp.path().join("plugin-executed"); + + write_executable( + &plugin_dir.join("meta-git"), + r#"#!/bin/sh +if [ "$1" = "--meta-plugin-info" ]; then + printf '%s\n' '{"name":"git","version":"1.0.0","commands":["git"]}' + exit 0 +fi +if [ "$1" = "--meta-plugin-exec" ]; then + IFS= read -r request || : + : > "$META_TEST_MARKER" + printf '%s\n' '{"plan":{"commands":[]}}' + exit 0 +fi +exit 1 +"#, + ); + + let run = |args: &[&str]| { + Command::new(assert_cmd::cargo::cargo_bin!("meta")) + .current_dir(temp.path()) + .env("PATH", &plugin_dir) + .env("HOME", temp.path()) + .env("META_DATA_DIR", temp.path().join("meta-data")) + .env("META_TEST_MARKER", &marker) + .args(args) + .output() + .unwrap() + }; + + for args in [ + &["--help", "git", "pull"][..], + &["--help", "git", "clone", "https://example.invalid/repo.git"][..], + ] { + let output = run(args); + assert!(output.status.success(), "args: {args:?}"); + assert!(String::from_utf8_lossy(&output.stdout).contains("Usage:")); + assert!(!marker.exists(), "prefix help executed {args:?}"); + } + + fs::write(temp.path().join(".meta"), r#"{"projects":{}}"#).unwrap(); + let output = run(&["--help", "git", "pull"]); + assert!(output.status.success()); + assert!(String::from_utf8_lossy(&output.stdout).contains("Usage:")); + assert!(!marker.exists(), "prefix help executed in a Meta workspace"); +} + +#[test] +fn child_only_plugin_plan_keeps_the_actual_meta_root_label() { + let temp = tempdir().unwrap(); + let plugin_dir = temp.path().join("bin"); + let child = temp.path().join("child"); + fs::create_dir(&plugin_dir).unwrap(); + fs::create_dir(&child).unwrap(); + fs::write( + temp.path().join(".meta"), + r#"{"projects":{"child":{"repo":"https://example.invalid/child.git","path":"child"}}}"#, + ) + .unwrap(); + + write_executable( + &plugin_dir.join("meta-rust"), + r#"#!/bin/sh +if [ "$1" = "--meta-plugin-info" ]; then + printf '%s\n' '{"name":"rust","version":"1.0.0","commands":["cargo","rust"]}' + exit 0 +fi +if [ "$1" = "--meta-plugin-exec" ]; then + IFS= read -r request || : + printf '{"plan":{"commands":[{"dir":"%s","cmd":"printf child-output"}],"parallel":false}}\n' "$META_TEST_CHILD" + exit 0 +fi +exit 1 +"#, + ); + + let data_dir = temp.path().join("meta-data"); + fs::create_dir(&data_dir).unwrap(); + let output = Command::new(assert_cmd::cargo::cargo_bin!("meta")) + .current_dir(temp.path()) + .env("PATH", &plugin_dir) + .env("HOME", temp.path()) + .env("META_DATA_DIR", data_dir) + .env("META_TEST_CHILD", &child) + .args(["--sequential", "--include", "child", "cargo", "check"]) + .output() + .unwrap(); + + assert!( + output.status.success(), + "stderr: {}", + String::from_utf8_lossy(&output.stderr) + ); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!(stdout.contains("✓ child"), "stdout: {stdout}"); + assert!(!stdout.contains("✓ . (child)"), "stdout: {stdout}"); +} From 7e3b7e2b3ea14803a38334c603014a25e2764a79 Mon Sep 17 00:00:00 2001 From: Maksim Soltan Date: Tue, 14 Jul 2026 14:18:30 -0700 Subject: [PATCH 5/8] revert: remove Cargo policy from host [[tasks/meta-cargo-command-forwarding]] --- src/main.rs | 589 ++---------------------------------- src/subprocess_plugins.rs | 197 +----------- tests/command_forwarding.rs | 118 -------- 3 files changed, 32 insertions(+), 872 deletions(-) delete mode 100644 tests/command_forwarding.rs diff --git a/src/main.rs b/src/main.rs index 4268f54..b7beca9 100644 --- a/src/main.rs +++ b/src/main.rs @@ -462,15 +462,8 @@ fn main() -> Result<()> { log::debug!("cli.json = {}", cli.json); - // Cargo/Rust namespace help must not inspect workspace configuration. - // Other command paths retain the existing orphan warning behavior. - let skip_orphan_check = matches!( - cli.command.as_ref(), - Some(Commands::External(args)) if is_cargo_namespace_help(args, cli.help) - ); - if !skip_orphan_check { - check_and_warn_orphan(); - } + // Check for orphaned nested meta repo and warn the user + check_and_warn_orphan(); // Discover plugins early to handle --help requests and plugin listing let mut subprocess_plugins = SubprocessPluginManager::new(); @@ -539,24 +532,17 @@ fn main() -> Result<()> { handle_command_dispatch(args.command, &cli, &subprocess_plugins, true) } Some(Commands::External(args)) => { - // clap captures every token after an external subcommand name. - // Apply namespace-specific ownership before routing the command. + // clap doesn't capture global flags that appear after an external + // subcommand name. Extract long-form global flags here so they + // work in both positions (before and after the subcommand). let mut args = args; extract_global_flags(&mut args, &mut cli); - let is_cargo_namespace = is_cargo_namespace(&args); - let has_forwarded_help = contains_help_before_separator(&args); - if cli.help && !is_cargo_namespace && !has_forwarded_help { - print_help_with_plugins(&subprocess_plugins, false); - return Ok(()); - } - // Keep root plugin help fast and plugin-aware, but let nested help // requests reach the matched plugin command implementation. if let Some(first) = args.first() { + let wants_help = args.iter().any(|a| a == "--help" || a == "-h"); let is_bare = args.len() == 1; - let is_prefix_meta_help = cli.help && is_cargo_namespace; - let wants_help = is_prefix_meta_help || has_forwarded_help; let is_root_help = wants_help && args.len() == 2 && matches!(args.get(1).map(String::as_str), Some("--help" | "-h")); @@ -568,10 +554,7 @@ fn main() -> Result<()> { .collect(); let is_promoted = promoted_commands.contains(&first.to_string()); - if is_prefix_meta_help - || is_root_help - || (is_bare && (!is_promoted || is_cargo_namespace)) - { + if is_root_help || (is_bare && !is_promoted) { if let Some(help_text) = subprocess_plugins.get_plugin_help(first) { println!("{help_text}"); return Ok(()); @@ -802,11 +785,10 @@ fn handle_command_dispatch( return Ok(()); } - // No config found — worktree paths are still authoritative for - // plugin dispatch, but config-backed tags/dependencies are unavailable. + // No config found — degraded legacy path with warning if cli.verbose { eprintln!( - "{} No .meta config found for worktree '{}'. Tags and dependency features unavailable.", + "{} No .meta config found for worktree '{}'. Tags, plugins, and dependency features unavailable.", "warning:".yellow().bold(), task_name ); @@ -819,10 +801,10 @@ fn handle_command_dispatch( let exclude_opt = none_if_empty(exclude_filters); let config = loop_lib::LoopConfig { - directories: directories.clone(), + directories, ignore: vec![], - include_filters: include_opt.clone(), - exclude_filters: exclude_opt.clone(), + include_filters: include_opt, + exclude_filters: exclude_opt, verbose: cli.verbose, silent: cli.silent, parallel, // Use the determined parallel mode, not hardcoded false @@ -835,36 +817,7 @@ fn handle_command_dispatch( root_dir: None, // Worktree paths don't use "." convention }; - let subprocess_options = PluginRequestOptions { - json_output: cli.json, - verbose: cli.verbose, - parallel, - dry_run, - silent: cli.silent, - recursive, - depth, - include_filters: include_opt, - exclude_filters: exclude_opt, - strict: cli.strict, - }; - - if plugins.execute( - &command_str, - &command_args, - &directories, - subprocess_options, - )? { - if cli.verbose { - println!( - "{}", - "Command handled by subprocess plugin (worktree without config).".green() - ); - } - } else if is_explicit_exec { - run(&config, &command_str)?; - } else { - unrecognized_command_error(&command_args, &command_str, plugins); - } + run(&config, &command_str)?; return Ok(()); } } @@ -967,11 +920,10 @@ fn handle_command_dispatch( strict: cli.strict, }; - if plugins.execute_with_root( + if plugins.execute( &command_str, &command_args, &project_paths, - Some(meta_dir), subprocess_options, )? { log::info!("Command was handled by subprocess plugin"); @@ -1293,193 +1245,18 @@ fn handle_plugin_command( // === Helpers === -/// Whether an external command explicitly enters Cargo's namespace. -fn is_cargo_namespace(args: &[String]) -> bool { - matches!(args.first().map(String::as_str), Some("cargo" | "rust")) -} - -/// Result of parsing a documented Cargo short-option cluster. -enum CargoShortOption { - Complete, - NeedsValue, - Terminal, -} - -/// Parse Cargo's supported leading short options conservatively. -/// -/// `v` may repeat, while `q` cannot repeat or combine with `v`. `C` and `Z` -/// consume the rest of the cluster as their value, or the following token when -/// they end the cluster. Help and version are terminal, and unknown flags -/// reject the whole cluster. -fn cargo_short_option(argument: &str) -> Option { - let flags = argument.strip_prefix('-')?; - if flags.is_empty() || flags.starts_with('-') { - return None; - } - - let mut saw_quiet = false; - let mut saw_verbose = false; - for (offset, flag) in flags.char_indices() { - match flag { - 'v' if !saw_quiet => saw_verbose = true, - 'q' if !saw_quiet && !saw_verbose => saw_quiet = true, - 'h' | 'V' => return Some(CargoShortOption::Terminal), - 'C' | 'Z' => { - return Some(if offset + flag.len_utf8() < flags.len() { - CargoShortOption::Complete - } else { - CargoShortOption::NeedsValue - }); - } - _ => return None, - } - } - - Some(CargoShortOption::Complete) -} - -fn is_cargo_color(value: &str) -> bool { - matches!(value, "auto" | "always" | "never") -} - -/// Locate Cargo's subcommand after its leading global options. -/// -/// Cargo accepts an optional rustup `+toolchain` selector followed by global -/// flags before the command name. This parser recognizes only that documented -/// leading grammar and returns `None` for terminal, malformed, or unknown -/// forms, keeping ambiguous `--recursive` tokens Cargo-owned. It never scans -/// beyond Cargo's `--` separator. -fn cargo_subcommand_index(args: &[String]) -> Option { - if !is_cargo_namespace(args) { - return None; - } - - let separator = args - .iter() - .position(|arg| arg == "--") - .unwrap_or(args.len()); - let mut index = 1; - - if index < separator - && args[index] - .strip_prefix('+') - .is_some_and(|toolchain| !toolchain.is_empty()) - { - index += 1; - } - - while index < separator { - let argument = args[index].as_str(); - match argument { - // Global modifiers that do not consume a value. - "--locked" | "--offline" | "--frozen" | "--verbose" | "-q" | "--quiet" => { - index += 1; - } - // These modes exit without dispatching a Cargo subcommand. - "-V" | "--version" | "--list" | "--explain" | "-h" | "--help" => { - return None; - } - "--color" => { - if index + 1 >= separator || !is_cargo_color(&args[index + 1]) { - return None; - } - index += 2; - } - // Global options whose value is the following token. - "--config" => { - if index + 1 >= separator || args[index + 1].starts_with('-') { - return None; - } - index += 2; - } - _ if argument - .strip_prefix("--color=") - .is_some_and(is_cargo_color) => - { - index += 1; - } - _ if argument.starts_with("--color=") => return None, - _ if argument.starts_with("--config=") => index += 1, - _ if argument.starts_with("--explain=") => return None, - _ if argument.starts_with('-') => match cargo_short_option(argument) { - Some(CargoShortOption::Complete) => index += 1, - Some(CargoShortOption::NeedsValue) => { - if index + 1 >= separator - || args[index + 1].is_empty() - || args[index + 1].starts_with('-') - { - return None; - } - index += 2; - } - Some(CargoShortOption::Terminal) | None => return None, - }, - _ => return Some(index), - } - } - - None -} - -/// Whether a help flag appears before the command's `--` separator. -fn contains_help_before_separator(args: &[String]) -> bool { - args.iter() - .take_while(|arg| arg.as_str() != "--") - .any(|arg| matches!(arg.as_str(), "--help" | "-h")) -} - -/// Cargo/Rust help paths that must not inspect Meta workspace configuration. -fn is_cargo_namespace_help(args: &[String], prefix_help: bool) -> bool { - is_cargo_namespace(args) - && (prefix_help || args.len() == 1 || contains_help_before_separator(args)) -} - -/// Extract Meta-owned global flags from external subcommand args. +/// Extract meta-only global flags from external subcommand args. /// /// clap's `external_subcommand` captures all tokens after the first unrecognized -/// subcommand, including global flags like `--json`. For non-Cargo namespaces, -/// this function pulls Meta-owned flags out and applies them to the CLI struct. +/// subcommand, including global flags like `--json`. This function pulls them +/// out and applies them to the CLI struct so they work regardless of position. /// -/// Cargo and Rust explicitly own all options after their namespace. The sole -/// compatibility exception is postfix `--recursive` for build, test, and clean, -/// which continues to select nested Meta projects. No arguments after a `--` -/// separator are inspected. Other namespaces retain the existing extraction -/// behavior for Meta-only global flags. +/// Only extracts flags that are meta-global and NOT reused by plugin subcommands. +/// Flags like `--dry-run` and `--parallel` are left in args because plugin +/// subcommands (e.g. `worktree prune --dry-run`, `worktree exec --parallel`) +/// define their own versions and need to see them. fn extract_global_flags(args: &mut Vec, cli: &mut Cli) { - if is_cargo_namespace(args) { - let subcommand_index = cargo_subcommand_index(args); - let supports_recursive_compat = subcommand_index - .is_some_and(|index| matches!(args[index].as_str(), "build" | "test" | "clean")); - - if let (true, Some(subcommand_index)) = (supports_recursive_compat, subcommand_index) { - let separator = args - .iter() - .position(|arg| arg == "--") - .unwrap_or(args.len()); - let mut index = 0; - args.retain(|arg| { - let remove = - index > subcommand_index && index < separator && arg.as_str() == "--recursive"; - index += 1; - if remove { - cli.recursive = true; - } - !remove - }); - } - return; - } - - let mut after_separator = false; args.retain(|arg| { - if after_separator { - return true; - } - if arg == "--" { - after_separator = true; - return true; - } - match arg.as_str() { "--json" => { cli.json = true; @@ -1646,330 +1423,6 @@ mod tests { use std::io::Write; use tempfile::NamedTempFile; - fn empty_cli() -> Cli { - Cli::try_parse_from(["meta"]).unwrap() - } - - fn strings(values: &[&str]) -> Vec { - values.iter().map(|value| (*value).to_string()).collect() - } - - #[test] - fn test_cargo_recursive_compatibility_is_narrow() { - for namespace in ["cargo", "rust"] { - for subcommand in ["build", "test", "clean"] { - let mut cli = empty_cli(); - let mut args = strings(&[namespace, subcommand, "--all", "--recursive"]); - - extract_global_flags(&mut args, &mut cli); - - assert!(cli.recursive, "{namespace} {subcommand}"); - assert_eq!(args, strings(&[namespace, subcommand, "--all"])); - } - } - - for subcommand in ["update", "nextest"] { - let mut cli = empty_cli(); - let mut args = strings(&["cargo", subcommand, "--recursive"]); - let expected = args.clone(); - - extract_global_flags(&mut args, &mut cli); - - assert!(!cli.recursive, "cargo {subcommand}"); - assert_eq!(args, expected); - } - } - - #[test] - fn test_cargo_recursive_compatibility_finds_the_actual_subcommand() { - for (input, expected) in [ - ( - &["cargo", "--locked", "clean", "--recursive"][..], - &["cargo", "--locked", "clean"][..], - ), - ( - &["rust", "+nightly", "--offline", "test", "--recursive"][..], - &["rust", "+nightly", "--offline", "test"][..], - ), - ( - &["cargo", "--color", "always", "build", "--recursive"][..], - &["cargo", "--color", "always", "build"][..], - ), - ( - &[ - "cargo", - "-Zunstable-options", - "-C", - "crate", - "clean", - "--recursive", - ][..], - &["cargo", "-Zunstable-options", "-C", "crate", "clean"][..], - ), - ] { - let mut cli = empty_cli(); - let mut args = strings(input); - - extract_global_flags(&mut args, &mut cli); - - assert!(cli.recursive, "{input:?}"); - assert_eq!(args, strings(expected), "{input:?}"); - } - } - - #[test] - fn test_cargo_color_values_are_validated_before_recursive_compatibility() { - for color in ["auto", "always", "never"] { - let attached = format!("--color={color}"); - for mut args in [ - strings(&["cargo", "--color", color, "build", "--recursive"]), - vec![ - "cargo".to_string(), - attached.clone(), - "build".to_string(), - "--recursive".to_string(), - ], - ] { - let mut cli = empty_cli(); - let mut expected = args.clone(); - expected.pop(); - - extract_global_flags(&mut args, &mut cli); - - assert!(cli.recursive, "{args:?}"); - assert_eq!(args, expected); - } - } - - for input in [ - &["cargo", "--color", "sometimes", "build", "--recursive"][..], - &["cargo", "--color=sometimes", "build", "--recursive"][..], - &["cargo", "--color", "", "build", "--recursive"][..], - &["cargo", "--color=", "build", "--recursive"][..], - &["cargo", "--color"][..], - ] { - let mut cli = empty_cli(); - let mut args = strings(input); - let expected = args.clone(); - - extract_global_flags(&mut args, &mut cli); - - assert!(!cli.recursive, "{input:?}"); - assert_eq!(args, expected, "{input:?}"); - } - } - - #[test] - fn test_cargo_script_mode_continues_to_the_manifest_path() { - for (input, expected_index) in [ - (&["cargo", "-Z", "script", "./tool.rs"][..], 3), - (&["cargo", "-Zscript", "./tool.rs"][..], 2), - ] { - let mut args = strings(input); - assert_eq!(cargo_subcommand_index(&args), Some(expected_index)); - - args.push("--recursive".to_string()); - let expected = args.clone(); - let mut cli = empty_cli(); - - extract_global_flags(&mut args, &mut cli); - - assert!(!cli.recursive, "{input:?}"); - assert_eq!(args, expected, "{input:?}"); - } - } - - #[test] - fn test_cargo_short_option_clusters_preserve_recursive_compatibility() { - for input in [ - &["cargo", "-vC.", "build", "--recursive"][..], - &["cargo", "-qZunstable-options", "clean", "--recursive"][..], - &["cargo", "-vC", ".", "test", "--recursive"][..], - &["cargo", "-qZ", "unstable-options", "build", "--recursive"][..], - ] { - let mut cli = empty_cli(); - let mut args = strings(input); - let mut expected = args.clone(); - expected.pop(); - - extract_global_flags(&mut args, &mut cli); - - assert!(cli.recursive, "{input:?}"); - assert_eq!(args, expected, "{input:?}"); - } - } - - #[test] - fn test_cargo_subcommand_detection_is_conservative() { - for input in [ - &["cargo", "--config", "clean", "update", "--recursive"][..], - &["cargo", "--config", "--locked", "clean", "--recursive"][..], - &["cargo", "--color", "--locked", "clean", "--recursive"][..], - &["cargo", "-C", "--locked", "clean", "--recursive"][..], - &["cargo", "-Z", "--locked", "clean", "--recursive"][..], - &["cargo", "--locked", "--", "clean", "--recursive"][..], - &["cargo", "--recursive", "clean"][..], - &["cargo", "-vX", "clean", "--recursive"][..], - &["cargo", "-qZ", "--locked", "clean", "--recursive"][..], - &["cargo", "-vC", "--locked", "clean", "--recursive"][..], - &["cargo", "-Vv", "clean", "--recursive"][..], - &["cargo", "-vh", "clean", "--recursive"][..], - &["cargo", "-qq", "build", "--recursive"][..], - &["cargo", "-vq", "clean", "--recursive"][..], - &["cargo", "-qv", "clean", "--recursive"][..], - ] { - let mut cli = empty_cli(); - let mut args = strings(input); - let expected = args.clone(); - - extract_global_flags(&mut args, &mut cli); - - assert!(!cli.recursive, "{input:?}"); - assert_eq!(args, expected, "{input:?}"); - } - } - - #[test] - fn test_cargo_postfix_flags_and_separator_payload_are_cargo_owned() { - let mut cli = empty_cli(); - let mut args = strings(&[ - "cargo", - "test", - "--verbose", - "--json", - "--", - "--recursive", - "--help", - "--silent", - "--primary", - "--strict", - ]); - let expected = args.clone(); - - extract_global_flags(&mut args, &mut cli); - - assert_eq!(args, expected); - assert!(!cli.recursive); - assert!(!cli.verbose); - assert!(!cli.json); - assert!(!cli.silent); - assert!(!cli.primary); - assert!(!cli.strict); - assert!(!contains_help_before_separator(&args)); - } - - #[test] - fn test_non_cargo_extraction_stops_at_separator() { - let mut cli = empty_cli(); - let mut args = strings(&[ - "git", - "status", - "--verbose", - "--recursive", - "--", - "--json", - "--silent", - "--primary", - "--strict", - ]); - - extract_global_flags(&mut args, &mut cli); - - assert_eq!( - args, - strings(&[ - "git", - "status", - "--", - "--json", - "--silent", - "--primary", - "--strict", - ]) - ); - assert!(cli.verbose); - assert!(cli.recursive); - assert!(!cli.json); - assert!(!cli.silent); - assert!(!cli.primary); - assert!(!cli.strict); - } - - #[test] - fn test_non_cargo_global_flag_extraction_remains_compatible() { - let mut cli = empty_cli(); - let mut args = strings(&[ - "git", - "status", - "--json", - "--verbose", - "--silent", - "--primary", - "--recursive", - "--strict", - ]); - - extract_global_flags(&mut args, &mut cli); - - assert_eq!(args, strings(&["git", "status"])); - assert!(cli.json); - assert!(cli.verbose); - assert!(cli.silent); - assert!(cli.primary); - assert!(cli.recursive); - assert!(cli.strict); - } - - #[test] - fn test_cargo_help_classification_stops_at_separator() { - assert!(is_cargo_namespace_help(&strings(&["cargo"]), false)); - assert!(is_cargo_namespace_help(&strings(&["rust"]), false)); - assert!(is_cargo_namespace_help( - &strings(&["cargo", "check", "--help"]), - false - )); - assert!(is_cargo_namespace_help( - &strings(&["rust", "nextest", "-h"]), - false - )); - assert!(is_cargo_namespace_help(&strings(&["cargo", "check"]), true)); - - assert!(!is_cargo_namespace_help( - &strings(&["cargo", "test", "--", "--help"]), - false - )); - assert!(!is_cargo_namespace_help( - &strings(&["cargo", "check"]), - false - )); - assert!(!is_cargo_namespace_help( - &strings(&["git", "status", "--help"]), - false - )); - } - - #[test] - fn test_meta_controls_before_cargo_namespace_remain_global() { - let cli = Cli::try_parse_from([ - "meta", - "--dry-run", - "--verbose", - "cargo", - "check", - "--verbose", - ]) - .unwrap(); - - assert!(cli.dry_run); - assert!(cli.verbose); - match cli.command { - Some(Commands::External(args)) => { - assert_eq!(args, strings(&["cargo", "check", "--verbose"])); - } - _ => panic!("expected external Cargo command"), - } - } - #[test] fn test_parse_meta_config_valid_simple_format() { let mut file = NamedTempFile::new().unwrap(); diff --git a/src/subprocess_plugins.rs b/src/subprocess_plugins.rs index c5f893d..ed8a2f9 100644 --- a/src/subprocess_plugins.rs +++ b/src/subprocess_plugins.rs @@ -27,44 +27,6 @@ pub struct SubprocessPluginManager { verbose: bool, } -/// Options used after a plugin has returned an execution plan. -/// -/// The Rust plugin's Cargo/Rust namespace implementation applies -/// include/exclude filters while planning so it can distinguish a selected -/// scope with no Cargo projects from an unfiltered scope. Reapplying those -/// string filters to its normalized Windows paths can drop commands whose -/// original path spelling used a short name or different casing. Other and -/// older plugins continue to rely on the host execution layer for filtering. -fn options_for_plan_execution( - plugin: &PluginInfo, - command: &str, - options: &PluginRequestOptions, -) -> PluginRequestOptions { - let mut execution_options = options.clone(); - let namespace = command.split_whitespace().next().unwrap_or_default(); - let owns_namespace_root = plugin - .commands - .iter() - .any(|registered| registered == namespace); - - if is_rust_namespace_command(plugin, command) && owns_namespace_root { - execution_options.include_filters = None; - execution_options.exclude_filters = None; - } - - execution_options -} - -/// Whether trusted Rust-plugin routing explicitly entered a Cargo namespace. -/// -/// This uses plugin identity and the matched command rather than inspecting a -/// returned shell plan. It also covers legacy multi-word Rust registrations so -/// Cargo remains authoritative when older plugins are installed. -fn is_rust_namespace_command(plugin: &PluginInfo, command: &str) -> bool { - let namespace = command.split_whitespace().next().unwrap_or_default(); - plugin.name == "rust" && matches!(namespace, "cargo" | "rust") -} - impl Default for SubprocessPluginManager { fn default() -> Self { Self::new() @@ -240,18 +202,6 @@ impl SubprocessPluginManager { args: &[String], projects: &[String], options: PluginRequestOptions, - ) -> Result { - self.execute_with_root(command, args, projects, None, options) - } - - /// Execute a command while preserving the caller's actual Meta root. - pub fn execute_with_root( - &self, - command: &str, - args: &[String], - projects: &[String], - root_dir: Option<&Path>, - options: PluginRequestOptions, ) -> Result { let cmd_parts: Vec<&str> = command.split_whitespace().collect(); if cmd_parts.is_empty() { @@ -286,7 +236,7 @@ impl SubprocessPluginManager { } if let Some((plugin, matched_cmd)) = best_match { - return self.execute_plugin(plugin, matched_cmd, args, projects, root_dir, &options); + return self.execute_plugin(plugin, matched_cmd, args, projects, &options); } Ok(false) @@ -299,7 +249,6 @@ impl SubprocessPluginManager { command: &str, args: &[String], projects: &[String], - root_dir: Option<&Path>, options: &PluginRequestOptions, ) -> Result { // Extract the remaining args after the matched command @@ -364,14 +313,7 @@ impl SubprocessPluginManager { match serde_json::from_str::(&stdout_str) { Ok(response) => { // Plugin returned an execution plan - execute it via loop_lib - let execution_options = options_for_plan_execution(&plugin.info, command, options); - let expand_loop_aliases = !is_rust_namespace_command(&plugin.info, command); - self.execute_plan( - &response.plan, - &execution_options, - root_dir, - expand_loop_aliases, - ) + self.execute_plan(&response.plan, options) } Err(_) => { // Couldn't parse as our protocol - print output as-is (legacy behavior) @@ -382,21 +324,8 @@ impl SubprocessPluginManager { } /// Execute an execution plan via loop_lib - fn execute_plan( - &self, - plan: &ExecutionPlan, - options: &PluginRequestOptions, - root_dir: Option<&Path>, - expand_loop_aliases: bool, - ) -> Result { - use loop_lib::{run_commands, run_commands_without_loop_aliases, DirCommand, LoopConfig}; - - let run_plan_commands: fn(&LoopConfig, &[DirCommand]) -> Result<()> = if expand_loop_aliases - { - run_commands - } else { - run_commands_without_loop_aliases - }; + fn execute_plan(&self, plan: &ExecutionPlan, options: &PluginRequestOptions) -> Result { + use loop_lib::{run_commands, DirCommand, LoopConfig}; // Phase 1: Run pre_commands sequentially (setup tasks like SSH ControlMaster) if !plan.pre_commands.is_empty() { @@ -430,7 +359,7 @@ impl SubprocessPluginManager { }; // Ignore failures for pre_commands (e.g., SSH socket already exists) // The main commands will fail if setup was actually needed - if let Err(e) = run_plan_commands(&pre_config, &[cmd]) { + if let Err(e) = run_commands(&pre_config, &[cmd]) { if options.verbose { eprintln!("Pre-command failed (continuing): {e}"); } @@ -450,6 +379,9 @@ impl SubprocessPluginManager { }) .collect(); + // The first command's directory is the meta root (should display as ".") + let root_dir = commands.first().map(|c| PathBuf::from(&c.dir)); + let config = LoopConfig { directories: vec![], ignore: vec![], @@ -464,10 +396,10 @@ impl SubprocessPluginManager { spawn_stagger_ms: plan.spawn_stagger_ms.unwrap_or(0), env: None, max_parallel: plan.max_parallel, - root_dir: root_dir.map(Path::to_path_buf), + root_dir, }; - run_plan_commands(&config, &commands)?; + run_commands(&config, &commands)?; } // Phase 3: Run post_commands sequentially (cleanup tasks) @@ -495,7 +427,7 @@ impl SubprocessPluginManager { cmd: post_cmd.cmd.clone(), env: post_cmd.env.clone(), }; - if let Err(e) = run_plan_commands(&post_config, &[cmd]) { + if let Err(e) = run_commands(&post_config, &[cmd]) { if options.verbose { eprintln!("Post-command failed: {e}"); } @@ -741,19 +673,6 @@ fn is_executable(path: &Path) -> bool { mod tests { use super::*; - fn plugin_info(name: &str, commands: &[&str]) -> PluginInfo { - PluginInfo { - name: name.to_string(), - version: "1.0.0".to_string(), - commands: commands - .iter() - .map(|command| (*command).to_string()) - .collect(), - description: None, - help: None, - } - } - #[test] fn test_plugin_manager_new() { let manager = SubprocessPluginManager::new(); @@ -822,100 +741,6 @@ mod tests { assert!(options.exclude_filters.is_none()); } - #[test] - fn test_only_rust_namespace_commands_disable_loop_aliases() { - let rust = plugin_info("rust", &["cargo", "rust"]); - for command in ["cargo", "cargo clean", "rust", "rust test"] { - assert!(is_rust_namespace_command(&rust, command), "{command}"); - } - - // Legacy exact registrations still represent the same Cargo authority - // boundary even though they do not own a namespace root. - let legacy_rust = plugin_info("rust", &["cargo build", "rust test"]); - assert!(is_rust_namespace_command(&legacy_rust, "cargo build")); - assert!(is_rust_namespace_command(&legacy_rust, "rust test")); - - for (plugin, command) in [ - (plugin_info("git", &["git status"]), "git status"), - (plugin_info("cargo-wrapper", &["cargo"]), "cargo"), - (plugin_info("rust", &["build"]), "build"), - ] { - assert!(!is_rust_namespace_command(&plugin, command), "{command}"); - } - } - - #[test] - fn test_rust_plan_execution_does_not_reapply_directory_filters() { - let options = PluginRequestOptions { - json_output: true, - parallel: true, - dry_run: true, - include_filters: Some(vec!["included".to_string()]), - exclude_filters: Some(vec!["excluded".to_string()]), - ..Default::default() - }; - let plugin = plugin_info("rust", &["cargo", "rust"]); - - for namespace in ["cargo", "rust"] { - let execution = options_for_plan_execution(&plugin, namespace, &options); - - assert!(execution.include_filters.is_none()); - assert!(execution.exclude_filters.is_none()); - assert!(execution.json_output); - assert!(execution.parallel); - assert!(execution.dry_run); - } - - // Planning still receives the original options unchanged. - assert_eq!( - options.include_filters.as_deref(), - Some(&["included".to_string()][..]) - ); - assert_eq!( - options.exclude_filters.as_deref(), - Some(&["excluded".to_string()][..]) - ); - } - - #[test] - fn test_other_plugin_plans_keep_directory_filters() { - let options = PluginRequestOptions { - include_filters: Some(vec!["included".to_string()]), - exclude_filters: Some(vec!["excluded".to_string()]), - ..Default::default() - }; - - for (plugin, command) in [ - (plugin_info("git", &["git status"]), "git status"), - (plugin_info("cargo-wrapper", &["cargo"]), "cargo"), - ] { - let execution = options_for_plan_execution(&plugin, command, &options); - - assert_eq!(execution.include_filters, options.include_filters); - assert_eq!(execution.exclude_filters, options.exclude_filters); - } - - let non_namespace = - options_for_plan_execution(&plugin_info("rust", &["cargo", "rust"]), "build", &options); - assert_eq!(non_namespace.include_filters, options.include_filters); - assert_eq!(non_namespace.exclude_filters, options.exclude_filters); - } - - #[test] - fn test_legacy_rust_plugin_exact_commands_keep_directory_filters() { - let options = PluginRequestOptions { - include_filters: Some(vec!["included".to_string()]), - exclude_filters: Some(vec!["excluded".to_string()]), - ..Default::default() - }; - let legacy = plugin_info("rust", &["cargo build", "cargo test"]); - - let execution = options_for_plan_execution(&legacy, "cargo build", &options); - - assert_eq!(execution.include_filters, options.include_filters); - assert_eq!(execution.exclude_filters, options.exclude_filters); - } - #[test] fn test_handles_command_matching() { let mut manager = SubprocessPluginManager::new(); diff --git a/tests/command_forwarding.rs b/tests/command_forwarding.rs deleted file mode 100644 index b429946..0000000 --- a/tests/command_forwarding.rs +++ /dev/null @@ -1,118 +0,0 @@ -#![cfg(unix)] - -use std::fs; -use std::os::unix::fs::PermissionsExt; -use std::path::Path; -use std::process::Command; -use tempfile::tempdir; - -fn write_executable(path: &Path, contents: &str) { - fs::write(path, contents).unwrap(); - let mut permissions = fs::metadata(path).unwrap().permissions(); - permissions.set_mode(0o755); - fs::set_permissions(path, permissions).unwrap(); -} - -#[test] -fn prefix_help_does_not_execute_non_cargo_plugin() { - let temp = tempdir().unwrap(); - let plugin_dir = temp.path().join("bin"); - fs::create_dir(&plugin_dir).unwrap(); - let marker = temp.path().join("plugin-executed"); - - write_executable( - &plugin_dir.join("meta-git"), - r#"#!/bin/sh -if [ "$1" = "--meta-plugin-info" ]; then - printf '%s\n' '{"name":"git","version":"1.0.0","commands":["git"]}' - exit 0 -fi -if [ "$1" = "--meta-plugin-exec" ]; then - IFS= read -r request || : - : > "$META_TEST_MARKER" - printf '%s\n' '{"plan":{"commands":[]}}' - exit 0 -fi -exit 1 -"#, - ); - - let run = |args: &[&str]| { - Command::new(assert_cmd::cargo::cargo_bin!("meta")) - .current_dir(temp.path()) - .env("PATH", &plugin_dir) - .env("HOME", temp.path()) - .env("META_DATA_DIR", temp.path().join("meta-data")) - .env("META_TEST_MARKER", &marker) - .args(args) - .output() - .unwrap() - }; - - for args in [ - &["--help", "git", "pull"][..], - &["--help", "git", "clone", "https://example.invalid/repo.git"][..], - ] { - let output = run(args); - assert!(output.status.success(), "args: {args:?}"); - assert!(String::from_utf8_lossy(&output.stdout).contains("Usage:")); - assert!(!marker.exists(), "prefix help executed {args:?}"); - } - - fs::write(temp.path().join(".meta"), r#"{"projects":{}}"#).unwrap(); - let output = run(&["--help", "git", "pull"]); - assert!(output.status.success()); - assert!(String::from_utf8_lossy(&output.stdout).contains("Usage:")); - assert!(!marker.exists(), "prefix help executed in a Meta workspace"); -} - -#[test] -fn child_only_plugin_plan_keeps_the_actual_meta_root_label() { - let temp = tempdir().unwrap(); - let plugin_dir = temp.path().join("bin"); - let child = temp.path().join("child"); - fs::create_dir(&plugin_dir).unwrap(); - fs::create_dir(&child).unwrap(); - fs::write( - temp.path().join(".meta"), - r#"{"projects":{"child":{"repo":"https://example.invalid/child.git","path":"child"}}}"#, - ) - .unwrap(); - - write_executable( - &plugin_dir.join("meta-rust"), - r#"#!/bin/sh -if [ "$1" = "--meta-plugin-info" ]; then - printf '%s\n' '{"name":"rust","version":"1.0.0","commands":["cargo","rust"]}' - exit 0 -fi -if [ "$1" = "--meta-plugin-exec" ]; then - IFS= read -r request || : - printf '{"plan":{"commands":[{"dir":"%s","cmd":"printf child-output"}],"parallel":false}}\n' "$META_TEST_CHILD" - exit 0 -fi -exit 1 -"#, - ); - - let data_dir = temp.path().join("meta-data"); - fs::create_dir(&data_dir).unwrap(); - let output = Command::new(assert_cmd::cargo::cargo_bin!("meta")) - .current_dir(temp.path()) - .env("PATH", &plugin_dir) - .env("HOME", temp.path()) - .env("META_DATA_DIR", data_dir) - .env("META_TEST_CHILD", &child) - .args(["--sequential", "--include", "child", "cargo", "check"]) - .output() - .unwrap(); - - assert!( - output.status.success(), - "stderr: {}", - String::from_utf8_lossy(&output.stderr) - ); - let stdout = String::from_utf8_lossy(&output.stdout); - assert!(stdout.contains("✓ child"), "stdout: {stdout}"); - assert!(!stdout.contains("✓ . (child)"), "stdout: {stdout}"); -} From 77c716c3e6220cd49a0550134f16902f89da03f3 Mon Sep 17 00:00:00 2001 From: Maksim Soltan Date: Tue, 14 Jul 2026 14:16:25 -0700 Subject: [PATCH 6/8] fix: make plugin plan execution policy generic [[tasks/meta-cargo-command-forwarding]] --- src/main.rs | 117 ++++++++++++++-- src/subprocess_plugins.rs | 116 +++++++++++---- tests/plugin_plan_policy.rs | 273 ++++++++++++++++++++++++++++++++++++ 3 files changed, 472 insertions(+), 34 deletions(-) create mode 100644 tests/plugin_plan_policy.rs diff --git a/src/main.rs b/src/main.rs index b7beca9..dd8ca25 100644 --- a/src/main.rs +++ b/src/main.rs @@ -538,12 +538,17 @@ fn main() -> Result<()> { let mut args = args; extract_global_flags(&mut args, &mut cli); + let has_forwarded_help = contains_help_before_separator(&args); + if cli.help && !has_forwarded_help { + print_help_with_plugins(&subprocess_plugins, false); + return Ok(()); + } + // Keep root plugin help fast and plugin-aware, but let nested help // requests reach the matched plugin command implementation. if let Some(first) = args.first() { - let wants_help = args.iter().any(|a| a == "--help" || a == "-h"); let is_bare = args.len() == 1; - let is_root_help = wants_help + let is_root_help = has_forwarded_help && args.len() == 2 && matches!(args.get(1).map(String::as_str), Some("--help" | "-h")); @@ -561,7 +566,7 @@ fn main() -> Result<()> { } } - if wants_help { + if has_forwarded_help { let command_str = args.join(" "); let options = PluginRequestOptions { json_output: cli.json, @@ -785,10 +790,11 @@ fn handle_command_dispatch( return Ok(()); } - // No config found — degraded legacy path with warning + // No config found — worktree paths are still authoritative for + // plugin dispatch, but config-backed tags/dependencies are unavailable. if cli.verbose { eprintln!( - "{} No .meta config found for worktree '{}'. Tags, plugins, and dependency features unavailable.", + "{} No .meta config found for worktree '{}'. Tags and dependency features unavailable.", "warning:".yellow().bold(), task_name ); @@ -801,10 +807,10 @@ fn handle_command_dispatch( let exclude_opt = none_if_empty(exclude_filters); let config = loop_lib::LoopConfig { - directories, + directories: directories.clone(), ignore: vec![], - include_filters: include_opt, - exclude_filters: exclude_opt, + include_filters: include_opt.clone(), + exclude_filters: exclude_opt.clone(), verbose: cli.verbose, silent: cli.silent, parallel, // Use the determined parallel mode, not hardcoded false @@ -817,7 +823,36 @@ fn handle_command_dispatch( root_dir: None, // Worktree paths don't use "." convention }; - run(&config, &command_str)?; + let subprocess_options = PluginRequestOptions { + json_output: cli.json, + verbose: cli.verbose, + parallel, + dry_run, + silent: cli.silent, + recursive, + depth, + include_filters: include_opt, + exclude_filters: exclude_opt, + strict: cli.strict, + }; + + if plugins.execute( + &command_str, + &command_args, + &directories, + subprocess_options, + )? { + if cli.verbose { + println!( + "{}", + "Command handled by subprocess plugin (worktree without config).".green() + ); + } + } else if is_explicit_exec { + run(&config, &command_str)?; + } else { + unrecognized_command_error(&command_args, &command_str, plugins); + } return Ok(()); } } @@ -920,10 +955,11 @@ fn handle_command_dispatch( strict: cli.strict, }; - if plugins.execute( + if plugins.execute_with_root( &command_str, &command_args, &project_paths, + Some(meta_dir), subprocess_options, )? { log::info!("Command was handled by subprocess plugin"); @@ -1256,7 +1292,16 @@ fn handle_plugin_command( /// subcommands (e.g. `worktree prune --dry-run`, `worktree exec --parallel`) /// define their own versions and need to see them. fn extract_global_flags(args: &mut Vec, cli: &mut Cli) { + let mut after_separator = false; args.retain(|arg| { + if after_separator { + return true; + } + if arg == "--" { + after_separator = true; + return true; + } + match arg.as_str() { "--json" => { cli.json = true; @@ -1287,6 +1332,13 @@ fn extract_global_flags(args: &mut Vec, cli: &mut Cli) { }); } +/// Whether a help flag appears before the command's `--` separator. +fn contains_help_before_separator(args: &[String]) -> bool { + args.iter() + .take_while(|arg| arg.as_str() != "--") + .any(|arg| matches!(arg.as_str(), "--help" | "-h")) +} + /// Check whether a project's tags match a comma-separated tag filter string. fn matches_tag_filter(tags: &[String], filter: &str) -> bool { let requested: Vec<&str> = filter.split(',').map(|s| s.trim()).collect(); @@ -1423,6 +1475,51 @@ mod tests { use std::io::Write; use tempfile::NamedTempFile; + fn empty_cli() -> Cli { + Cli::try_parse_from(["meta"]).unwrap() + } + + fn strings(values: &[&str]) -> Vec { + values.iter().map(|value| (*value).to_string()).collect() + } + + #[test] + fn test_external_global_extraction_stops_at_separator() { + let mut cli = empty_cli(); + let mut args = strings(&[ + "tool", + "run", + "--verbose", + "--recursive", + "--", + "--json", + "--strict", + "--recursive", + ]); + + extract_global_flags(&mut args, &mut cli); + + assert!(cli.verbose); + assert!(cli.recursive); + assert!(!cli.json); + assert!(!cli.strict); + assert_eq!( + args, + strings(&["tool", "run", "--", "--json", "--strict", "--recursive"]) + ); + } + + #[test] + fn test_forwarded_help_classification_stops_at_separator() { + assert!(contains_help_before_separator(&strings(&[ + "tool", "run", "--help", "--", "payload" + ]))); + assert!(contains_help_before_separator(&strings(&["tool", "-h"]))); + assert!(!contains_help_before_separator(&strings(&[ + "tool", "run", "--", "--help" + ]))); + } + #[test] fn test_parse_meta_config_valid_simple_format() { let mut file = NamedTempFile::new().unwrap(); diff --git a/src/subprocess_plugins.rs b/src/subprocess_plugins.rs index ed8a2f9..2655bfa 100644 --- a/src/subprocess_plugins.rs +++ b/src/subprocess_plugins.rs @@ -10,8 +10,8 @@ use std::process::{Command, Stdio}; #[allow(unused_imports)] pub use meta_plugin_protocol::{ - ExecutionPlan, PlanResponse as PluginResponse, PlannedCommand, PluginHelp, PluginInfo, - PluginRequest, PluginRequestOptions, + ExecutionPlan, PlanExecutionPolicy, PlanResponse as PluginResponse, PlannedCommand, PluginHelp, + PluginInfo, PluginRequest, PluginRequestOptions, HOST_CAPABILITY_PLAN_EXECUTION_POLICY_V1, }; /// A discovered subprocess plugin @@ -27,6 +27,19 @@ pub struct SubprocessPluginManager { verbose: bool, } +/// Apply the execution behavior selected by a plugin plan. +fn options_for_plan_execution( + policy: PlanExecutionPolicy, + options: &PluginRequestOptions, +) -> PluginRequestOptions { + let mut execution_options = options.clone(); + if !policy.apply_host_filters { + execution_options.include_filters = None; + execution_options.exclude_filters = None; + } + execution_options +} + impl Default for SubprocessPluginManager { fn default() -> Self { Self::new() @@ -202,6 +215,18 @@ impl SubprocessPluginManager { args: &[String], projects: &[String], options: PluginRequestOptions, + ) -> Result { + self.execute_with_root(command, args, projects, None, options) + } + + /// Execute a command while preserving the caller's actual Meta root. + pub fn execute_with_root( + &self, + command: &str, + args: &[String], + projects: &[String], + root_dir: Option<&Path>, + options: PluginRequestOptions, ) -> Result { let cmd_parts: Vec<&str> = command.split_whitespace().collect(); if cmd_parts.is_empty() { @@ -236,7 +261,7 @@ impl SubprocessPluginManager { } if let Some((plugin, matched_cmd)) = best_match { - return self.execute_plugin(plugin, matched_cmd, args, projects, &options); + return self.execute_plugin(plugin, matched_cmd, args, projects, root_dir, &options); } Ok(false) @@ -249,6 +274,7 @@ impl SubprocessPluginManager { command: &str, args: &[String], projects: &[String], + root_dir: Option<&Path>, options: &PluginRequestOptions, ) -> Result { // Extract the remaining args after the matched command @@ -262,6 +288,7 @@ impl SubprocessPluginManager { args: remaining_args, projects: projects.to_vec(), cwd: std::env::current_dir()?.to_string_lossy().to_string(), + host_capabilities: vec![HOST_CAPABILITY_PLAN_EXECUTION_POLICY_V1.to_string()], options: options.clone(), }; @@ -313,7 +340,7 @@ impl SubprocessPluginManager { match serde_json::from_str::(&stdout_str) { Ok(response) => { // Plugin returned an execution plan - execute it via loop_lib - self.execute_plan(&response.plan, options) + self.execute_plan(&response.plan, options, root_dir, response.execution_policy) } Err(_) => { // Couldn't parse as our protocol - print output as-is (legacy behavior) @@ -324,8 +351,22 @@ impl SubprocessPluginManager { } /// Execute an execution plan via loop_lib - fn execute_plan(&self, plan: &ExecutionPlan, options: &PluginRequestOptions) -> Result { - use loop_lib::{run_commands, DirCommand, LoopConfig}; + fn execute_plan( + &self, + plan: &ExecutionPlan, + options: &PluginRequestOptions, + root_dir: Option<&Path>, + policy: PlanExecutionPolicy, + ) -> Result { + use loop_lib::{run_commands, run_commands_without_loop_aliases, DirCommand, LoopConfig}; + + let execution_options = options_for_plan_execution(policy, options); + let run_plan_commands: fn(&LoopConfig, &[DirCommand]) -> Result<()> = + if policy.expand_loop_aliases { + run_commands + } else { + run_commands_without_loop_aliases + }; // Phase 1: Run pre_commands sequentially (setup tasks like SSH ControlMaster) if !plan.pre_commands.is_empty() { @@ -337,7 +378,7 @@ impl SubprocessPluginManager { let pre_config = LoopConfig { directories: vec![], ignore: vec![], - verbose: options.verbose, + verbose: execution_options.verbose, silent: true, // Pre-commands run silently unless verbose add_aliases_to_global_looprc: false, include_filters: None, @@ -359,8 +400,8 @@ impl SubprocessPluginManager { }; // Ignore failures for pre_commands (e.g., SSH socket already exists) // The main commands will fail if setup was actually needed - if let Err(e) = run_commands(&pre_config, &[cmd]) { - if options.verbose { + if let Err(e) = run_plan_commands(&pre_config, &[cmd]) { + if execution_options.verbose { eprintln!("Pre-command failed (continuing): {e}"); } } @@ -379,27 +420,24 @@ impl SubprocessPluginManager { }) .collect(); - // The first command's directory is the meta root (should display as ".") - let root_dir = commands.first().map(|c| PathBuf::from(&c.dir)); - let config = LoopConfig { directories: vec![], ignore: vec![], - verbose: options.verbose, - silent: options.silent, + verbose: execution_options.verbose, + silent: execution_options.silent, add_aliases_to_global_looprc: false, - include_filters: options.include_filters.clone(), - exclude_filters: options.exclude_filters.clone(), - parallel: plan.parallel.unwrap_or(options.parallel), - dry_run: options.dry_run, - json_output: options.json_output, + include_filters: execution_options.include_filters.clone(), + exclude_filters: execution_options.exclude_filters.clone(), + parallel: plan.parallel.unwrap_or(execution_options.parallel), + dry_run: execution_options.dry_run, + json_output: execution_options.json_output, spawn_stagger_ms: plan.spawn_stagger_ms.unwrap_or(0), env: None, max_parallel: plan.max_parallel, - root_dir, + root_dir: root_dir.map(Path::to_path_buf), }; - run_commands(&config, &commands)?; + run_plan_commands(&config, &commands)?; } // Phase 3: Run post_commands sequentially (cleanup tasks) @@ -407,7 +445,7 @@ impl SubprocessPluginManager { let post_config = LoopConfig { directories: vec![], ignore: vec![], - verbose: options.verbose, + verbose: execution_options.verbose, silent: true, add_aliases_to_global_looprc: false, include_filters: None, @@ -427,8 +465,8 @@ impl SubprocessPluginManager { cmd: post_cmd.cmd.clone(), env: post_cmd.env.clone(), }; - if let Err(e) = run_commands(&post_config, &[cmd]) { - if options.verbose { + if let Err(e) = run_plan_commands(&post_config, &[cmd]) { + if execution_options.verbose { eprintln!("Post-command failed: {e}"); } } @@ -714,6 +752,7 @@ mod tests { args: vec!["--verbose".to_string()], projects: vec!["project1".to_string(), "project2".to_string()], cwd: "/home/user/workspace".to_string(), + host_capabilities: vec![HOST_CAPABILITY_PLAN_EXECUTION_POLICY_V1.to_string()], options: PluginRequestOptions { json_output: true, verbose: false, @@ -727,6 +766,10 @@ mod tests { assert!(json.contains("\"command\":\"git status\"")); assert!(json.contains("\"json_output\":true")); assert!(json.contains("\"parallel\":true")); + assert_eq!( + request.host_capabilities, + vec![HOST_CAPABILITY_PLAN_EXECUTION_POLICY_V1.to_string()] + ); } #[test] @@ -741,6 +784,29 @@ mod tests { assert!(options.exclude_filters.is_none()); } + #[test] + fn test_plan_execution_policy_controls_host_filters() { + let options = PluginRequestOptions { + include_filters: Some(vec!["selected".to_string()]), + exclude_filters: Some(vec!["ignored".to_string()]), + ..Default::default() + }; + + let legacy = options_for_plan_execution(PlanExecutionPolicy::default(), &options); + assert_eq!(legacy.include_filters, options.include_filters); + assert_eq!(legacy.exclude_filters, options.exclude_filters); + + let plugin_owned = options_for_plan_execution( + PlanExecutionPolicy { + expand_loop_aliases: false, + apply_host_filters: false, + }, + &options, + ); + assert!(plugin_owned.include_filters.is_none()); + assert!(plugin_owned.exclude_filters.is_none()); + } + #[test] fn test_handles_command_matching() { let mut manager = SubprocessPluginManager::new(); @@ -1085,6 +1151,7 @@ mod tests { args: vec![], projects: vec!["proj1".to_string()], cwd: "/workspace".to_string(), + host_capabilities: vec![], options: PluginRequestOptions { json_output: false, verbose: false, @@ -1105,6 +1172,7 @@ mod tests { args: vec!["--release".to_string()], projects: vec![], cwd: ".".to_string(), + host_capabilities: vec![], options: PluginRequestOptions { json_output: true, verbose: true, diff --git a/tests/plugin_plan_policy.rs b/tests/plugin_plan_policy.rs new file mode 100644 index 0000000..d5a438b --- /dev/null +++ b/tests/plugin_plan_policy.rs @@ -0,0 +1,273 @@ +#![cfg(unix)] + +use std::ffi::OsString; +use std::fs; +use std::os::unix::fs::PermissionsExt; +use std::path::{Path, PathBuf}; +use std::process::Command; +use tempfile::tempdir; + +fn write_executable(path: &Path, contents: &str) { + fs::write(path, contents).unwrap(); + let mut permissions = fs::metadata(path).unwrap().permissions(); + permissions.set_mode(0o755); + fs::set_permissions(path, permissions).unwrap(); +} + +fn path_with_plugin(plugin_dir: &Path) -> OsString { + let mut paths = vec![plugin_dir.to_path_buf()]; + if let Some(existing) = std::env::var_os("PATH") { + paths.extend(std::env::split_paths(&existing)); + } + std::env::join_paths(paths).unwrap() +} + +fn meta_command(current_dir: &Path, plugin_dir: &Path, home: &Path) -> Command { + let mut command = Command::new(env!("CARGO_BIN_EXE_meta")); + command + .current_dir(current_dir) + .env("PATH", path_with_plugin(plugin_dir)) + .env("HOME", home) + .env("META_DATA_DIR", home.join("meta-data")); + command +} + +fn create_plugin_dir(root: &Path) -> PathBuf { + let plugin_dir = root.join("bin"); + fs::create_dir(&plugin_dir).unwrap(); + plugin_dir +} + +#[test] +fn prefix_help_does_not_execute_plugin_commands() { + let temp = tempdir().unwrap(); + let plugin_dir = create_plugin_dir(temp.path()); + let marker = temp.path().join("plugin-executed"); + + write_executable( + &plugin_dir.join("meta-tool"), + r#"#!/bin/sh +if [ "$1" = "--meta-plugin-info" ]; then + printf '%s\n' '{"name":"tool","version":"1.0.0","commands":["tool"]}' + exit 0 +fi +if [ "$1" = "--meta-plugin-exec" ]; then + IFS= read -r request || : + : > "$META_TEST_MARKER" + printf '%s\n' '{"plan":{"commands":[]}}' + exit 0 +fi +exit 1 +"#, + ); + + for args in [ + &["--help", "tool", "run"][..], + &["--help", "tool", "run", "payload"][..], + ] { + let output = meta_command(temp.path(), &plugin_dir, temp.path()) + .env("META_TEST_MARKER", &marker) + .args(args) + .output() + .unwrap(); + assert!(output.status.success(), "args: {args:?}"); + assert!(String::from_utf8_lossy(&output.stdout).contains("Usage:")); + assert!(!marker.exists(), "prefix help executed {args:?}"); + } + + fs::write(temp.path().join(".meta"), r#"{"projects":{}}"#).unwrap(); + let output = meta_command(temp.path(), &plugin_dir, temp.path()) + .env("META_TEST_MARKER", &marker) + .args(["--help", "tool", "run"]) + .output() + .unwrap(); + assert!(output.status.success()); + assert!(!marker.exists(), "prefix help executed in a Meta workspace"); +} + +#[test] +fn separator_payload_and_policy_capability_reach_the_plugin_unchanged() { + let temp = tempdir().unwrap(); + let plugin_dir = create_plugin_dir(temp.path()); + let request_path = temp.path().join("plugin-request.json"); + fs::write(temp.path().join(".meta"), r#"{"projects":{}}"#).unwrap(); + + write_executable( + &plugin_dir.join("meta-tool"), + r#"#!/bin/sh +if [ "$1" = "--meta-plugin-info" ]; then + printf '%s\n' '{"name":"tool","version":"1.0.0","commands":["tool"]}' + exit 0 +fi +if [ "$1" = "--meta-plugin-exec" ]; then + IFS= read -r request || : + printf '%s\n' "$request" > "$META_TEST_REQUEST" + printf '%s\n' '{"plan":{"commands":[]}}' + exit 0 +fi +exit 1 +"#, + ); + + let output = meta_command(temp.path(), &plugin_dir, temp.path()) + .env("META_TEST_REQUEST", &request_path) + .args(["tool", "run", "--", "--help", "--recursive", "--json"]) + .output() + .unwrap(); + assert!( + output.status.success(), + "stderr: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let request: serde_json::Value = + serde_json::from_slice(&fs::read(&request_path).unwrap()).unwrap(); + assert_eq!(request["command"], "tool"); + assert_eq!( + request["args"], + serde_json::json!(["run", "--", "--help", "--recursive", "--json"]) + ); + assert_eq!( + request["host_capabilities"], + serde_json::json!(["plan-execution-policy-v1"]) + ); + assert_eq!(request["options"]["recursive"], false); + assert_eq!(request["options"]["json_output"], false); +} + +#[test] +fn no_config_worktree_still_dispatches_to_plugins() { + let temp = tempdir().unwrap(); + let plugin_dir = create_plugin_dir(temp.path()); + let task_dir = temp.path().join(".worktrees").join("synthetic-task"); + let repo_dir = task_dir.join("repo"); + let marker = temp.path().join("plugin-executed"); + fs::create_dir_all(&repo_dir).unwrap(); + fs::write( + repo_dir.join(".git"), + format!( + "gitdir: {}\n", + temp.path() + .join("source") + .join(".git") + .join("worktrees") + .join("synthetic-task") + .display() + ), + ) + .unwrap(); + + write_executable( + &plugin_dir.join("meta-tool"), + r#"#!/bin/sh +if [ "$1" = "--meta-plugin-info" ]; then + printf '%s\n' '{"name":"tool","version":"1.0.0","commands":["tool"]}' + exit 0 +fi +if [ "$1" = "--meta-plugin-exec" ]; then + IFS= read -r request || : + : > "$META_TEST_MARKER" + printf '%s\n' '{"plan":{"commands":[]}}' + exit 0 +fi +exit 1 +"#, + ); + + let output = meta_command(&repo_dir, &plugin_dir, temp.path()) + .env("META_TEST_MARKER", &marker) + .args(["tool", "run"]) + .output() + .unwrap(); + assert!( + output.status.success(), + "stderr: {}", + String::from_utf8_lossy(&output.stderr) + ); + assert!(marker.exists(), "worktree command bypassed the plugin"); +} + +#[test] +fn child_only_plan_uses_the_actual_meta_root_for_output_labels() { + let temp = tempdir().unwrap(); + let plugin_dir = create_plugin_dir(temp.path()); + let child = temp.path().join("child"); + fs::create_dir(&child).unwrap(); + fs::write( + temp.path().join(".meta"), + r#"{"projects":{"child":{"repo":"https://example.invalid/child.git","path":"child"}}}"#, + ) + .unwrap(); + + write_executable( + &plugin_dir.join("meta-tool"), + r#"#!/bin/sh +if [ "$1" = "--meta-plugin-info" ]; then + printf '%s\n' '{"name":"tool","version":"1.0.0","commands":["tool"]}' + exit 0 +fi +if [ "$1" = "--meta-plugin-exec" ]; then + IFS= read -r request || : + printf '{"plan":{"commands":[{"dir":"%s","cmd":"printf child-output"}],"parallel":false}}\n' "$META_TEST_CHILD" + exit 0 +fi +exit 1 +"#, + ); + + let output = meta_command(temp.path(), &plugin_dir, temp.path()) + .env("META_TEST_CHILD", &child) + .args(["--sequential", "--include", "child", "tool", "check"]) + .output() + .unwrap(); + assert!( + output.status.success(), + "stderr: {}", + String::from_utf8_lossy(&output.stderr) + ); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!(stdout.contains("✓ child"), "stdout: {stdout}"); + assert!(!stdout.contains("✓ . (child)"), "stdout: {stdout}"); +} + +#[test] +fn plan_execution_policy_controls_aliases_and_host_filters() { + let temp = tempdir().unwrap(); + let plugin_dir = create_plugin_dir(temp.path()); + let marker = temp.path().join("policy-marker"); + fs::write(temp.path().join(".meta"), r#"{"projects":{}}"#).unwrap(); + fs::write( + temp.path().join(".looprc"), + r#"{"aliases":{"true":"false"}}"#, + ) + .unwrap(); + + write_executable( + &plugin_dir.join("meta-tool"), + r#"#!/bin/sh +if [ "$1" = "--meta-plugin-info" ]; then + printf '%s\n' '{"name":"tool","version":"1.0.0","commands":["tool"]}' + exit 0 +fi +if [ "$1" = "--meta-plugin-exec" ]; then + IFS= read -r request || : + printf '{"plan":{"commands":[{"dir":"%s","cmd":"true && printf policy-ok > policy-marker"}],"parallel":false},"execution_policy":{"expand_loop_aliases":false,"apply_host_filters":false}}\n' "$META_TEST_ROOT" + exit 0 +fi +exit 1 +"#, + ); + + let output = meta_command(temp.path(), &plugin_dir, temp.path()) + .env("META_TEST_ROOT", temp.path()) + .args(["--sequential", "--include", "does-not-match", "tool", "run"]) + .output() + .unwrap(); + assert!( + output.status.success(), + "stdout: {}\nstderr: {}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + assert_eq!(fs::read_to_string(marker).unwrap(), "policy-ok"); +} From dfe2fd863329b8e619ae4a4e8d39e88f8c3ff1f5 Mon Sep 17 00:00:00 2001 From: Maksim Soltan Date: Tue, 14 Jul 2026 14:37:40 -0700 Subject: [PATCH 7/8] fix: preserve generic plugin help boundaries [[tasks/meta-cargo-command-forwarding]] --- src/main.rs | 12 ++++++++-- src/subprocess_plugins.rs | 14 +++++++---- tests/plugin_plan_policy.rs | 46 +++++++++++++++++++++++++++++++++++++ 3 files changed, 65 insertions(+), 7 deletions(-) diff --git a/src/main.rs b/src/main.rs index dd8ca25..a140001 100644 --- a/src/main.rs +++ b/src/main.rs @@ -462,8 +462,16 @@ fn main() -> Result<()> { log::debug!("cli.json = {}", cli.json); - // Check for orphaned nested meta repo and warn the user - check_and_warn_orphan(); + // External help is metadata-only. Let the matched plugin answer it before + // workspace discovery so malformed or absent config cannot hide help. + let external_help_request = matches!( + cli.command.as_ref(), + Some(Commands::External(args)) + if cli.help || contains_help_before_separator(args) + ); + if !external_help_request { + check_and_warn_orphan(); + } // Discover plugins early to handle --help requests and plugin listing let mut subprocess_plugins = SubprocessPluginManager::new(); diff --git a/src/subprocess_plugins.rs b/src/subprocess_plugins.rs index 2655bfa..574e559 100644 --- a/src/subprocess_plugins.rs +++ b/src/subprocess_plugins.rs @@ -13,6 +13,10 @@ pub use meta_plugin_protocol::{ ExecutionPlan, PlanExecutionPolicy, PlanResponse as PluginResponse, PlannedCommand, PluginHelp, PluginInfo, PluginRequest, PluginRequestOptions, HOST_CAPABILITY_PLAN_EXECUTION_POLICY_V1, }; +use meta_plugin_protocol::{ + PlanResponseWithPolicy as HostPluginResponse, + PluginRequestWithCapabilities as HostPluginRequest, +}; /// A discovered subprocess plugin #[derive(Debug, Clone)] @@ -283,7 +287,7 @@ impl SubprocessPluginManager { let cmd_word_count = command.split_whitespace().count(); let remaining_args: Vec = args.iter().skip(cmd_word_count).cloned().collect(); - let request = PluginRequest { + let request = HostPluginRequest { command: command.to_string(), args: remaining_args, projects: projects.to_vec(), @@ -337,7 +341,7 @@ impl SubprocessPluginManager { } // Parse the plugin response - match serde_json::from_str::(&stdout_str) { + match serde_json::from_str::(&stdout_str) { Ok(response) => { // Plugin returned an execution plan - execute it via loop_lib self.execute_plan(&response.plan, options, root_dir, response.execution_policy) @@ -747,7 +751,7 @@ mod tests { #[test] fn test_plugin_request_serialization() { - let request = PluginRequest { + let request = HostPluginRequest { command: "git status".to_string(), args: vec!["--verbose".to_string()], projects: vec!["project1".to_string(), "project2".to_string()], @@ -1146,7 +1150,7 @@ mod tests { #[test] fn test_plugin_request_with_dry_run() { - let request = PluginRequest { + let request = HostPluginRequest { command: "git status".to_string(), args: vec![], projects: vec!["proj1".to_string()], @@ -1167,7 +1171,7 @@ mod tests { #[test] fn test_plugin_request_all_options_enabled() { - let request = PluginRequest { + let request = HostPluginRequest { command: "build".to_string(), args: vec!["--release".to_string()], projects: vec![], diff --git a/tests/plugin_plan_policy.rs b/tests/plugin_plan_policy.rs index d5a438b..8df4dbc 100644 --- a/tests/plugin_plan_policy.rs +++ b/tests/plugin_plan_policy.rs @@ -85,6 +85,52 @@ exit 1 assert!(!marker.exists(), "prefix help executed in a Meta workspace"); } +#[test] +fn nested_plugin_help_skips_workspace_config_and_reaches_plugin() { + let temp = tempdir().unwrap(); + let plugin_dir = create_plugin_dir(temp.path()); + let request_path = temp.path().join("plugin-request.json"); + fs::write(temp.path().join(".meta.yaml"), "projects: [malformed").unwrap(); + + write_executable( + &plugin_dir.join("meta-tool"), + r#"#!/bin/sh +if [ "$1" = "--meta-plugin-info" ]; then + printf '%s\n' '{"name":"tool","version":"1.0.0","commands":["tool"]}' + exit 0 +fi +if [ "$1" = "--meta-plugin-exec" ]; then + IFS= read -r request || : + printf '%s\n' "$request" > "$META_TEST_REQUEST" + printf '%s\n' 'tool-owned nested help' + exit 0 +fi +exit 1 +"#, + ); + + let output = meta_command(temp.path(), &plugin_dir, temp.path()) + .env("META_TEST_REQUEST", &request_path) + .args(["tool", "run", "--help"]) + .output() + .unwrap(); + assert!( + output.status.success(), + "stderr: {}", + String::from_utf8_lossy(&output.stderr) + ); + assert_eq!( + String::from_utf8_lossy(&output.stdout).trim(), + "tool-owned nested help" + ); + + let request: serde_json::Value = + serde_json::from_slice(&fs::read(&request_path).unwrap()).unwrap(); + assert_eq!(request["command"], "tool"); + assert_eq!(request["args"], serde_json::json!(["run", "--help"])); + assert_eq!(request["projects"], serde_json::json!([])); +} + #[test] fn separator_payload_and_policy_capability_reach_the_plugin_unchanged() { let temp = tempdir().unwrap(); From 2634ecbc83b0e5d11182be358724e095154c0638 Mon Sep 17 00:00:00 2001 From: Maksim Soltan Date: Tue, 14 Jul 2026 14:42:55 -0700 Subject: [PATCH 8/8] fix: honor plugin-owned bare help metadata [[tasks/meta-cargo-command-forwarding]] --- src/main.rs | 16 +++++---- src/subprocess_plugins.rs | 23 +++++++++++- tests/plugin_plan_policy.rs | 71 +++++++++++++++++++++++++++++++++++++ 3 files changed, 103 insertions(+), 7 deletions(-) diff --git a/src/main.rs b/src/main.rs index a140001..6df8fdc 100644 --- a/src/main.rs +++ b/src/main.rs @@ -462,21 +462,24 @@ fn main() -> Result<()> { log::debug!("cli.json = {}", cli.json); + // Discover plugins early to handle --help requests and plugin listing + let mut subprocess_plugins = SubprocessPluginManager::new(); + subprocess_plugins.discover_plugins(cli.verbose)?; + // External help is metadata-only. Let the matched plugin answer it before // workspace discovery so malformed or absent config cannot hide help. let external_help_request = matches!( cli.command.as_ref(), Some(Commands::External(args)) - if cli.help || contains_help_before_separator(args) + if cli.help + || contains_help_before_separator(args) + || (args.len() == 1 + && subprocess_plugins.is_bare_help_command(&args[0])) ); if !external_help_request { check_and_warn_orphan(); } - // Discover plugins early to handle --help requests and plugin listing - let mut subprocess_plugins = SubprocessPluginManager::new(); - subprocess_plugins.discover_plugins(cli.verbose)?; - // Handle --help flag at top level if cli.help && cli.command.is_none() { print_help_with_plugins(&subprocess_plugins, false); @@ -567,7 +570,8 @@ fn main() -> Result<()> { .collect(); let is_promoted = promoted_commands.contains(&first.to_string()); - if is_root_help || (is_bare && !is_promoted) { + let declared_bare_help = subprocess_plugins.is_bare_help_command(first); + if is_root_help || (is_bare && (!is_promoted || declared_bare_help)) { if let Some(help_text) = subprocess_plugins.get_plugin_help(first) { println!("{help_text}"); return Ok(()); diff --git a/src/subprocess_plugins.rs b/src/subprocess_plugins.rs index 574e559..fa680c8 100644 --- a/src/subprocess_plugins.rs +++ b/src/subprocess_plugins.rs @@ -4,7 +4,7 @@ //! This approach provides better isolation, language flexibility, and simpler debugging. use anyhow::{Context, Result}; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; @@ -28,9 +28,16 @@ pub struct SubprocessPlugin { /// Manager for subprocess-based plugins pub struct SubprocessPluginManager { plugins: HashMap, + bare_help_commands: HashSet, verbose: bool, } +#[derive(Default, serde::Deserialize)] +struct PluginDiscoveryExtensions { + #[serde(default)] + bare_help_commands: Vec, +} + /// Apply the execution behavior selected by a plugin plan. fn options_for_plan_execution( policy: PlanExecutionPolicy, @@ -54,6 +61,7 @@ impl SubprocessPluginManager { pub fn new() -> Self { Self { plugins: HashMap::new(), + bare_help_commands: HashSet::new(), verbose: false, } } @@ -161,6 +169,8 @@ impl SubprocessPluginManager { Ok(info) => info, Err(_) => return Ok(()), // Not a valid plugin, skip silently }; + let extensions: PluginDiscoveryExtensions = + serde_json::from_slice(&output.stdout).unwrap_or_default(); if self.verbose { println!( @@ -173,6 +183,12 @@ impl SubprocessPluginManager { // Don't override if already loaded (first one wins) if !self.plugins.contains_key(&info.name) { + self.bare_help_commands.extend( + extensions + .bare_help_commands + .into_iter() + .filter(|command| info.commands.contains(command)), + ); self.plugins.insert( info.name.clone(), SubprocessPlugin { @@ -189,6 +205,11 @@ impl SubprocessPluginManager { Ok(()) } + /// Whether a plugin declares a bare command root as metadata-only help. + pub fn is_bare_help_command(&self, command: &str) -> bool { + self.bare_help_commands.contains(command) + } + /// Check if any plugin handles the given command #[allow(dead_code)] pub fn handles_command(&self, command: &str) -> bool { diff --git a/tests/plugin_plan_policy.rs b/tests/plugin_plan_policy.rs index 8df4dbc..4f9e3c6 100644 --- a/tests/plugin_plan_policy.rs +++ b/tests/plugin_plan_policy.rs @@ -131,6 +131,77 @@ exit 1 assert_eq!(request["projects"], serde_json::json!([])); } +#[test] +fn declared_bare_help_is_metadata_only_without_reclassifying_other_commands() { + let temp = tempdir().unwrap(); + let plugin_dir = create_plugin_dir(temp.path()); + let marker = temp.path().join("plugin-executed"); + fs::write(temp.path().join(".meta.yaml"), "projects: [malformed").unwrap(); + + write_executable( + &plugin_dir.join("meta-suite"), + r#"#!/bin/sh +if [ "$1" = "--meta-plugin-info" ]; then + printf '%s\n' '{"name":"suite","version":"1.0.0","commands":["tool"],"bare_help_commands":["tool"],"help":{"usage":"meta tool "}}' + exit 0 +fi +if [ "$1" = "--meta-plugin-exec" ]; then + IFS= read -r request || : + : > "$META_TEST_MARKER" + printf '%s\n' '{"plan":{"commands":[]}}' + exit 0 +fi +exit 1 +"#, + ); + + let output = meta_command(temp.path(), &plugin_dir, temp.path()) + .env("META_TEST_MARKER", &marker) + .arg("tool") + .output() + .unwrap(); + assert!( + output.status.success(), + "stderr: {}", + String::from_utf8_lossy(&output.stderr) + ); + assert!(String::from_utf8_lossy(&output.stdout).contains("meta tool ")); + assert!( + !marker.exists(), + "declared bare help executed the plugin command" + ); + + fs::remove_file(temp.path().join(".meta.yaml")).unwrap(); + fs::write(temp.path().join(".meta"), r#"{"projects":{}}"#).unwrap(); + write_executable( + &plugin_dir.join("meta-runner"), + r#"#!/bin/sh +if [ "$1" = "--meta-plugin-info" ]; then + printf '%s\n' '{"name":"runner","version":"1.0.0","commands":["action"]}' + exit 0 +fi +if [ "$1" = "--meta-plugin-exec" ]; then + IFS= read -r request || : + : > "$META_TEST_MARKER" + printf '%s\n' '{"plan":{"commands":[]}}' + exit 0 +fi +exit 1 +"#, + ); + + let output = meta_command(temp.path(), &plugin_dir, temp.path()) + .env("META_TEST_MARKER", &marker) + .arg("action") + .output() + .unwrap(); + assert!(output.status.success()); + assert!( + marker.exists(), + "unlisted promoted command was reclassified as help" + ); +} + #[test] fn separator_payload_and_policy_capability_reach_the_plugin_unchanged() { let temp = tempdir().unwrap();