From 4cba2672fcb4d5a01bb5319a3cf572fba030e1eb Mon Sep 17 00:00:00 2001 From: Lev Velykoivanenko Date: Sun, 27 Sep 2026 21:41:11 +0200 Subject: [PATCH 1/2] fix(guard): redirect only commands a task covers A task claimed every typed command sharing its signature, which stops at the first flag. `test-doc` (`cargo test --doc --workspace`) claimed `cargo test -p X --test Y`, and `build` and `check` claimed every crate-scoped build and check, so agents could not run one crate. A bare word after `--` also dropped the signature, leaving `lint` unguarded. A typed command now redirects to a task only when each word it adds before `--` appears in the task's run, and its words after `--` equal the task's as a group. A task with nothing or a template past its signature keeps prefix matching. App launches keep prefix matching. Closes #195 Co-authored-by: Claude --- crates/devkit-ports/src/guard/mod.rs | 2 +- crates/devkit-ports/src/guard/sig.rs | 77 +++++++++++++++++++++++++++- tests/harness_guard.rs | 63 +++++++++++++++++++++++ 3 files changed, 140 insertions(+), 2 deletions(-) diff --git a/crates/devkit-ports/src/guard/mod.rs b/crates/devkit-ports/src/guard/mod.rs index de42e66c..c12d5b66 100644 --- a/crates/devkit-ports/src/guard/mod.rs +++ b/crates/devkit-ports/src/guard/mod.rs @@ -404,7 +404,7 @@ fn best_task(n: &Normalized, p: &Project, min_sig: usize) -> Option { if task.guard.is_none() && s.len() < min_sig { return None; } - if !sig::matches(&s, &n.argv) { + if !sig::matches(&s, &n.argv) || !sig::within(&cfg.argv, &s, &n.argv) { return None; } tasks::redirect_worth_it( diff --git a/crates/devkit-ports/src/guard/sig.rs b/crates/devkit-ports/src/guard/sig.rs index 23d173fb..c08b0def 100644 --- a/crates/devkit-ports/src/guard/sig.rs +++ b/crates/devkit-ports/src/guard/sig.rs @@ -33,7 +33,9 @@ pub fn signature(config_argv: &[String]) -> Option> { if sig.is_empty() { return None; } - let bare_after = config_argv[cut..] + // Words after `--` go to whatever the command hands them to, not a verb. + let (options, _) = split_trailing(&config_argv[cut..]); + let bare_after = options .iter() .any(|w| !w.starts_with('-') && !is_template(w)); if bare_after { @@ -65,6 +67,30 @@ pub fn matches(sig: &[String], typed: &[String]) -> bool { basename(&typed[0]) == basename(&sig[0]) && sig[1..] == typed[1..sig.len()] } +/// Whether `typed`, already matching `sig`, asks for nothing `config_argv` +/// does not do: each word it adds before `--` appears in the config's, and its +/// words after `--` equal the config's. A config with nothing or a template +/// past its signature leaves the rest open. +pub fn within(config_argv: &[String], sig: &[String], typed: &[String]) -> bool { + let tail = &config_argv[sig.len()..]; + if tail.is_empty() || tail.iter().any(|w| is_template(w)) { + return true; + } + let (options, trailing) = split_trailing(tail); + let (typed_options, typed_trailing) = split_trailing(&typed[sig.len()..]); + typed_options.iter().all(|w| options.contains(w)) + && typed_trailing.is_none_or(|t| trailing == Some(t)) +} + +/// `words` split at the first `--` into what comes before it and, when there +/// is one, what comes after it. +fn split_trailing(words: &[String]) -> (&[String], Option<&[String]>) { + match words.iter().position(|w| w == "--") { + Some(i) => (&words[..i], Some(&words[i + 1..])), + None => (words, None), + } +} + #[cfg(test)] mod tests { use super::*; @@ -143,4 +169,53 @@ mod tests { fn the_command_word_matches_by_basename() { assert!(matches(&v(&["vite"]), &v(&["./node_modules/.bin/vite"]))); } + + #[test] + fn bare_words_after_a_double_dash_keep_the_signature() { + assert_eq!( + sig(&["cargo", "clippy", "--workspace", "--", "-D", "warnings"]), + Some(v(&["cargo", "clippy"])) + ); + } + + fn is_within(config: &[&str], typed: &[&str]) -> bool { + let config = v(config); + within(&config, &signature(&config).unwrap(), &v(typed)) + } + + #[test] + fn a_typed_command_is_within_a_config_that_does_everything_it_asks() { + let build = ["cargo", "build", "--workspace", "--locked"]; + assert!(is_within(&build, &["cargo", "build"])); + assert!(is_within(&build, &[ + "cargo", + "build", + "--locked", + "--workspace" + ])); + assert!(!is_within(&build, &["cargo", "build", "-p", "x"])); + assert!(!is_within(&build, &["cargo", "build", "--release"])); + } + + #[test] + fn words_after_a_double_dash_compare_as_one_group() { + let lint = ["cargo", "clippy", "--workspace", "--", "-D", "warnings"]; + assert!(is_within(&lint, &[ + "cargo", "clippy", "--", "-D", "warnings" + ])); + assert!(is_within(&lint, &["cargo", "clippy", "--workspace"])); + assert!(!is_within(&lint, &["cargo", "clippy", "--", "-D"])); + assert!(!is_within(&lint, &[ + "cargo", "clippy", "--", "warnings", "-D" + ])); + assert!(!is_within(&lint, &["cargo", "clippy", "-D", "warnings"])); + } + + #[test] + fn a_config_with_nothing_or_a_template_past_its_signature_leaves_the_rest_open() { + assert!(is_within(&["vite"], &["vite", "build"])); + assert!(is_within(&["nitro", "dev", "--port", "{{ port }}"], &[ + "nitro", "dev", "--host", "0.0.0.0" + ])); + } } diff --git a/tests/harness_guard.rs b/tests/harness_guard.rs index 71c858b7..ae6643b7 100644 --- a/tests/harness_guard.rs +++ b/tests/harness_guard.rs @@ -381,6 +381,69 @@ guard = true assert!(stdout.contains("devrun task commit --arg msg="), "{stdout}"); } +const WORKSPACE_TASKS: &str = r#" +[tasks.build] +run = ["cargo", "build", "--workspace", "--locked"] +guard = true + +[tasks.check] +run = ["cargo", "check", "--workspace", "--all-targets", "--all-features", "--locked"] +guard = true + +[tasks.test] +run = ["cargo", "nextest", "run", "--workspace", "--all-features", "--locked", "--no-fail-fast"] +guard = true + +[tasks.test-doc] +run = ["cargo", "test", "--doc", "--workspace", "--all-features", "--locked"] +guard = true + +[tasks.lint] +run = ["cargo", "clippy", "--workspace", "--all-targets", "--all-features", "--locked", "--", "-D", "warnings"] +guard = true +"#; + +#[test] +fn a_command_asking_for_more_than_a_task_does_is_not_redirected() { + let home = tempfile::tempdir().unwrap(); + let proj = project(&(GUARDED.to_string() + WORKSPACE_TASKS)); + for typed in [ + "cargo test -p devkit-locks --test registry", + "cargo build -p devkit-locks", + "cargo check -p devkit-locks", + ] { + let out = run_hook(proj.path(), home.path(), &claude_payload(typed)); + assert!( + !denied(&out), + "{typed}: {}", + String::from_utf8_lossy(&out.stdout) + ); + } +} + +#[test] +fn a_command_within_a_task_is_redirected_to_it() { + let home = tempfile::tempdir().unwrap(); + let proj = project(&(GUARDED.to_string() + WORKSPACE_TASKS)); + for (typed, task) in [ + ("cargo test --workspace --doc", "test-doc"), + ("cargo build", "build"), + ("cargo nextest run --workspace", "test"), + ( + "cargo clippy --workspace --all-targets -- -D warnings", + "lint", + ), + ] { + let out = run_hook(proj.path(), home.path(), &claude_payload(typed)); + assert!(denied(&out), "{typed} was allowed"); + let stdout = String::from_utf8_lossy(&out.stdout); + assert!( + stdout.contains(&format!("devrun task {task}`")), + "{typed}: {stdout}" + ); + } +} + #[test] fn the_cwd_names_the_app_for_a_catalog_hit() { let home = tempfile::tempdir().unwrap(); From a5cda1e2d969b913f30d257c21361d6f7e726442 Mon Sep 17 00:00:00 2001 From: Lev Velykoivanenko Date: Sun, 27 Sep 2026 21:46:47 +0200 Subject: [PATCH 2/2] fix(guard): break task ties by omitted words Tasks tied on signature length fell through to name order, so `cargo build` named `build` over `build-release` only because it sorts first. Tasks with a `release` name ahead of the plain build would take it instead. Ties now go to the task whose run the typed command leaves the fewest words out of, before the app hint and name order are consulted. Co-authored-by: Claude --- crates/devkit-ports/src/guard/mod.rs | 63 ++++++++++++++++++++++++---- crates/devkit-ports/src/guard/sig.rs | 9 ++++ 2 files changed, 63 insertions(+), 9 deletions(-) diff --git a/crates/devkit-ports/src/guard/mod.rs b/crates/devkit-ports/src/guard/mod.rs index c12d5b66..7fa1c3ed 100644 --- a/crates/devkit-ports/src/guard/mod.rs +++ b/crates/devkit-ports/src/guard/mod.rs @@ -370,6 +370,8 @@ fn searched_app(n: &Normalized, p: &Project) -> Option { /// A task whose signature the typed segment matches. struct TaskHit<'a> { sig_len: usize, + /// Words of the task's `run` the typed segment leaves out. + omitted: usize, name: &'a str, /// The app the task is scoped to, which is what a hint resolves against. app: Option<&'a str>, @@ -378,12 +380,15 @@ struct TaskHit<'a> { /// The task this segment retypes, if redirecting to it would change the /// process. /// -/// Ranked by signature length, then by hint resolution among the tasks tied at -/// that length, then by name. Length first because a longer signature is the -/// more specific claim on the command; the hint next because a tie means -/// several tasks retype the same command and only the app the agent is working -/// in says which was meant; name last so a `HashMap`'s iteration order cannot -/// make the message name a different task from one call to the next. +/// Ranked by signature length, then by how few of the task's words the segment +/// leaves out, then by hint resolution among the tasks still tied, then by +/// name. Length first because a longer signature is the more specific claim on +/// the command; omissions next because `cargo build` is closer to a task +/// running `cargo build --workspace` than to one adding `--release`; the hint +/// next because a remaining tie means several tasks retype the same command and +/// only the app the agent is working in says which was meant; name last so a +/// `HashMap`'s iteration order cannot make the message name a different task +/// from one call to the next. /// /// `min_sig` is the shortest signature allowed to claim this segment; the /// caller raises it to exclude a bare-program task from a command the catalog @@ -415,14 +420,21 @@ fn best_task(n: &Normalized, p: &Project, min_sig: usize) -> Option { ) .then_some(TaskHit { sig_len: s.len(), + omitted: sig::omitted(&cfg.argv, &s, &n.argv), name: name.as_str(), app: task.app.as_deref(), }) }) .collect(); - hits.sort_by(|a, b| b.sig_len.cmp(&a.sig_len).then_with(|| a.name.cmp(b.name))); - let best = hits.first()?.sig_len; - let tied = &hits[..hits.iter().take_while(|h| h.sig_len == best).count()]; + hits.sort_by(|a, b| { + b.sig_len + .cmp(&a.sig_len) + .then_with(|| a.omitted.cmp(&b.omitted)) + .then_with(|| a.name.cmp(b.name)) + }); + let rank = |h: &TaskHit<'_>| (h.sig_len, h.omitted); + let best = rank(hits.first()?); + let tied = &hits[..hits.iter().take_while(|h| rank(h) == best).count()]; // `Scope::Catalog`, because the rung is hint *resolution*: a lone candidate // named without one would let a single app-scoped task beat an appless one @@ -1008,6 +1020,39 @@ mod tests { } } + #[test] + fn a_tie_goes_to_the_task_that_adds_least_to_the_command() { + let p = project(|c| { + for (name, run) in [ + ("release", vec![ + "cargo", + "build", + "--workspace", + "--release", + "--locked", + ]), + ("workspace-build", vec![ + "cargo", + "build", + "--workspace", + "--locked", + ]), + ] { + c.tasks.insert(name.into(), TaskConfig { + run: run.iter().map(|s| (*s).into()).collect(), + guard: Some(true), + ..Default::default() + }); + } + }); + let d = decide_with("cargo build", &BTreeMap::new(), Some(&p)); + assert!( + reason(&d).contains("devrun task workspace-build"), + "{}", + reason(&d) + ); + } + /// Two tasks that retype the same command, each scoped to a different app. fn two_tasks_tied_on_one_command(cwd_rel: Option<&str>) -> Project { let mut p = project(|c| { diff --git a/crates/devkit-ports/src/guard/sig.rs b/crates/devkit-ports/src/guard/sig.rs index c08b0def..5afb4183 100644 --- a/crates/devkit-ports/src/guard/sig.rs +++ b/crates/devkit-ports/src/guard/sig.rs @@ -82,6 +82,15 @@ pub fn within(config_argv: &[String], sig: &[String], typed: &[String]) -> bool && typed_trailing.is_none_or(|t| trailing == Some(t)) } +/// How many words past `sig` in `config_argv` the typed command leaves out. +pub fn omitted(config_argv: &[String], sig: &[String], typed: &[String]) -> usize { + let typed_rest = &typed[sig.len()..]; + config_argv[sig.len()..] + .iter() + .filter(|w| !typed_rest.contains(w)) + .count() +} + /// `words` split at the first `--` into what comes before it and, when there /// is one, what comes after it. fn split_trailing(words: &[String]) -> (&[String], Option<&[String]>) {