diff --git a/crates/devkit-ports/src/guard/mod.rs b/crates/devkit-ports/src/guard/mod.rs index de42e66c..67219e3d 100644 --- a/crates/devkit-ports/src/guard/mod.rs +++ b/crates/devkit-ports/src/guard/mod.rs @@ -111,7 +111,8 @@ fn args_match(patterns: &[String], args: &[Value]) -> Match { } /// Decide over an analysis. Every invocation is checked, nested ones -/// included; findings keep invocation order, then rule name order. +/// included; findings keep invocation order, then rule name order, and each +/// distinct finding appears once however many times a loop repeats it. pub fn decide( analysis: &Analysis, rules: &BTreeMap, @@ -141,8 +142,8 @@ pub fn decide( message: rule.reason.clone(), }; match rule.action { - RuleAction::Block => verdict.blocks.push(finding), - RuleAction::Warn => verdict.warnings.push(finding), + RuleAction::Block => push_once(&mut verdict.blocks, finding), + RuleAction::Warn => push_once(&mut verdict.warnings, finding), } } Match::Possible => undetermined = true, @@ -150,10 +151,12 @@ pub fn decide( } } if undetermined { - verdict.warnings.push(Finding { + push_once(&mut verdict.warnings, Finding { rule: None, severity: Severity::Warning, - message: format!("`{typed}` could not be fully resolved, so devkit could not tell whether a `[harness.commands]` rule applies; it was allowed."), + message: format!( + "`{typed}` could not be fully resolved, so devkit could not tell whether a `[harness.commands]` rule applies; it was allowed." + ), }); } let Some(project) = project else { continue }; @@ -162,18 +165,20 @@ pub fn decide( let program = basename(&known.argv[0]).to_string(); let normalized = norm_view(&known); if let Some(message) = project_hit(&inv.typed, &normalized, &program, project) { - verdict.blocks.push(Finding { + push_once(&mut verdict.blocks, Finding { rule: None, severity: Severity::Error, message, }); } } - None if !project.config.tasks.is_empty() || !project.catalog.is_empty() => { - verdict.warnings.push(Finding { + None if project_could_claim(inv.program.known(), project) => { + push_once(&mut verdict.warnings, Finding { rule: None, severity: Severity::Info, - message: format!("`{typed}` could not be fully resolved, so devkit did not check it against the project's tasks and apps."), + message: format!( + "`{typed}` could not be fully resolved, so devkit did not check it against the project's tasks and apps." + ), }) } None => {} @@ -182,6 +187,30 @@ pub fn decide( verdict } +fn push_once(findings: &mut Vec, finding: Finding) { + if !findings.contains(&finding) { + findings.push(finding); + } +} + +/// Whether a task, an app or the catalog could claim an invocation of +/// `program` once its arguments resolve. An unresolved program word could be +/// anything, so it counts whenever the project has a task or app at all. +fn project_could_claim(program: Option<&str>, p: &Project) -> bool { + let Some(program) = program.map(basename) else { + return !p.config.tasks.is_empty() || !p.catalog.is_empty(); + }; + let runs_program = |run: Option| run.is_some_and(|k| basename(&k.argv[0]) == program); + catalog::is_known_program(program) + || p.config + .tasks + .values() + .any(|t| runs_program(configured_task(&t.run))) + || p.catalog + .values() + .any(|a| runs_program(configured(&a.launch))) +} + /// Kept for callers holding a bash command string: the first block, if any. pub fn decide_with( command: &str, diff --git a/tests/harness_guard.rs b/tests/harness_guard.rs index 71c858b7..6e7e647a 100644 --- a/tests/harness_guard.rs +++ b/tests/harness_guard.rs @@ -485,3 +485,99 @@ fn the_declared_harness_picks_the_envelope() { ); assert!(v["agent_message"].is_string()); } + +/// The context lines an allowed command came back with, none when silent. +fn notes(out: &Output) -> Vec { + assert!( + !denied(out), + "stderr: {}", + String::from_utf8_lossy(&out.stderr) + ); + if out.stdout.iter().all(u8::is_ascii_whitespace) { + return Vec::new(); + } + let v: serde_json::Value = serde_json::from_slice(&out.stdout).expect("stdout is JSON"); + v["hookSpecificOutput"]["additionalContext"] + .as_str() + .expect("additionalContext") + .lines() + .map(str::to_string) + .collect() +} + +const CARGO_BUILD_TASK: &str = r#" +[harness] +enforce_commands = true + +[harness.commands.bun-only] +programs = ["node"] +args = ["server.js"] +reason = "This workspace is bun-only." + +[tasks.build] +run = ["cargo", "build"] + +[apps.web] +base_port = 3000 +path = "apps/web" +launch = ["nitro", "dev"] +"#; + +const PROJECT_NOTE: &str = "devkit did not check it against the project's tasks and apps."; + +#[test] +fn a_loop_repeats_no_unresolved_note() { + let home = tempfile::tempdir().unwrap(); + let proj = project(CARGO_BUILD_TASK); + let out = run_hook( + proj.path(), + home.path(), + &claude_payload(r#"for p in a b c; do cargo build "$(date)"; node "$y"; done"#), + ); + let notes = notes(&out); + let mut distinct = notes.clone(); + distinct.sort(); + distinct.dedup(); + assert_eq!(notes.len(), distinct.len(), "{notes:#?}"); + assert!( + notes + .iter() + .any(|n| n.starts_with("`cargo build \"$(date)\"`") && n.ends_with(PROJECT_NOTE)), + "{notes:#?}" + ); + assert!( + notes + .iter() + .any(|n| n.contains("`[harness.commands]` rule")), + "{notes:#?}" + ); +} + +#[test] +fn an_unresolved_program_no_task_or_app_runs_gets_no_project_note() { + let home = tempfile::tempdir().unwrap(); + let proj = project(CARGO_BUILD_TASK); + for command in [r#"printf %s "$q""#, r#"jq -r .x <<<"$base""#] { + let out = run_hook(proj.path(), home.path(), &claude_payload(command)); + assert_eq!(notes(&out), Vec::::new(), "{command}"); + } +} + +#[test] +fn an_unresolved_program_a_task_or_app_could_run_keeps_its_project_note() { + let home = tempfile::tempdir().unwrap(); + let proj = project(CARGO_BUILD_TASK); + for command in [ + r#"cargo build "$x""#, + r#"nitro "$x""#, + r#"vite "$x""#, + r#""$tool" build"#, + ] { + let out = run_hook(proj.path(), home.path(), &claude_payload(command)); + let notes = notes(&out); + assert!( + notes.iter().any(|n| n.ends_with(PROJECT_NOTE)), + "{command}: {notes:#?}" + ); + } +}