skillpack/verify/
mod.rs

1//! The `skillpack verify` subcommand: load the generated distribution files
2//! and run discovery + invocation checks against them.
3//!
4//! Design §5.2. `verify` works even on hand-written plugin files (not just
5//! `init` output) — see §4.2 — so the loader is tolerant of missing pieces and
6//! each check degrades gracefully.
7
8pub mod discovery;
9pub mod fix;
10pub mod invocation;
11pub mod result;
12pub mod schema;
13
14use anyhow::Result;
15
16use self::invocation::InvocationInput;
17use self::result::CheckResult;
18
19// Re-export the pieces the rest of the crate touches.
20pub use self::result::VerifyReport;
21
22/// Where the invocation stage should look for skill text. Passed in so the
23/// dispatcher owns the single `find_skill_file` call.
24///
25/// `root` is where the skill/manifest files live (the project root for the
26/// `verify` subcommand, the temp dir for `init`'s pre-commit gate).
27/// `spawn_root` is the real project root the CLI spawns from — it must be
28/// separate from `root` so the pre-commit gate can spawn the real CLI in its
29/// source tree while still verifying the rendered files (design §5.3 + §6.3).
30#[derive(Debug, Clone)]
31pub struct VerifyInput {
32    pub root: std::path::PathBuf,
33    /// The real project root the documented CLI runs in. For the `verify`
34    /// subcommand this equals `root`; for `init`'s pre-commit gate it's the
35    /// project root while `root` is the temp dir holding the rendered files.
36    pub spawn_root: std::path::PathBuf,
37    pub cli_command: Option<Vec<String>>,
38    /// The repo URL `git remote get-url origin` produced at introspection
39    /// (cached on `ProjectProfile.repo_url`, threaded here so `discovery`'s
40    /// URL-drift check stays free of a subprocess spawn — see module doc).
41    /// `None` when no git origin is configured.
42    pub repo_url: Option<String>,
43    /// The kebab-coerced project name (`coerce_kebab(&profile.name)`,
44    /// already computed for rendering). Threaded so `discovery`'s
45    /// `discovery.skill.name_drift` check can compare the SKILL.md `name:`
46    /// frontmatter against the canonical value the template renders — without
47    /// `discovery` itself calling `coerce_kebab` or building a `ProjectProfile`.
48    /// `None` only when introspection couldn't derive a name at all.
49    pub profile_name: Option<String>,
50    /// Stdin bytes to feed the CLI during `verify` spawns. For interactive
51    /// CLIs that block on stdin. `None` uses `/dev/null` (default).
52    pub verify_stdin: Option<String>,
53}
54
55/// Run the full verify suite against `root`, returning the aggregate report.
56pub fn run(input: &VerifyInput) -> Result<VerifyReport> {
57    let root = &input.root;
58    let mut report = VerifyReport::default();
59
60    // Discovery checks (pure, file reads only + the threaded repo_url +
61    // profile_name for the plugin.json / SKILL.md drift sub-checks).
62    for check in discovery::run(root, &input.repo_url, &input.profile_name)? {
63        report.push(check);
64    }
65
66    // Invocation checks run against EVERY skill that documents a CLI, so a
67    // multi-skill pack can't hide drift in a secondary skill. The primary
68    // (first) CLI spawns from the introspected `cli_command` (which may be a
69    // resolved absolute path); each secondary skill derives its program from
70    // its own documented invocation and must be on PATH to be spawnable.
71    let skill_files = discovery::find_skill_files(root);
72    let mut spawned_primary = false;
73    // The Claude target emits the same SKILL.md at BOTH `skills/<name>/` and
74    // the native `.claude/skills/<name>/`. They are one skill, not two —
75    // dedupe by skill directory name so invocation checks run once per skill.
76    // (Discovery still checks both copies structurally; this only avoids a
77    // redundant double spawn of the same CLI.)
78    let mut seen_skill_dirs = std::collections::HashSet::new();
79    for skill_path in &skill_files {
80        if let Some(dir) = skill_path.parent().and_then(|p| p.file_name()) {
81            if !seen_skill_dirs.insert(dir.to_string_lossy().to_string()) {
82                continue;
83            }
84        }
85        let skill_md = match std::fs::read_to_string(skill_path) {
86            Ok(s) => s,
87            Err(e) => {
88                // Path exists (find_skill_files returned it) — read failure is
89                // non-missing (permissions, non-UTF8, EBUSY). Discovery's
90                // `check_one_skill_md` would abort verify on the same file;
91                // surface a WARN here so the maintainer sees the read failure.
92                report.push(CheckResult::warn(
93                    "invocation.read_failed",
94                    "skills a verify can spawn should be readable",
95                    format!("{}: read failed ({}); invocation drift check skipped for this skill", discovery::rel_unix(root, skill_path), e),
96                    "To fix: check file permissions, ensure UTF-8 encoding (no Latin-1), and re-run.",
97                ));
98                continue;
99            }
100        };
101        // A pure-library skill (or one with no documented CLI) still goes
102        // through `invocation::run`, which emits its "Skipped: pure-library
103        // project" result — silently `continue`-ing here would drop that
104        // signal from the report.
105        let is_cli = invocation::extract_documented_invocation(&skill_md).is_some();
106        let cmd = if !is_cli {
107            None
108        } else if !spawned_primary {
109            spawned_primary = true;
110            input.cli_command.clone()
111        } else {
112            // Secondary skill: derive the command from its own invocation. If
113            // the binary is not on PATH (expected on many machines, since
114            // introspection only resolved the primary), warn and skip rather
115            // than false-fail a legitimately multi-CLI pack.
116            match invocation::command_from_documented(&skill_md) {
117                Some(c) if crate::introspect::which_on_path(&c[0]).is_some() => Some(c),
118                Some(c) => {
119                    report.push(CheckResult::warn(
120                        "invocation.secondary_not_runnable",
121                        "every documented CLI can be spawned for drift checks",
122                        format!("secondary skill documents CLI `{}`, which is not on PATH; its drift checks were skipped", c[0]),
123                        "To fix: install/build the secondary CLI so it is on PATH, then re-run verify.",
124                    ));
125                    continue;
126                }
127                None => {
128                    report.push(CheckResult::warn(
129                        "invocation.secondary_unparseable",
130                        "every documented CLI can be spawned for drift checks",
131                        format!("could not derive a command from {}'s documented invocation; its drift checks were skipped", discovery::rel_unix(root, skill_path)),
132                        "To fix: document the CLI with a plain command line in the `## Invocation` section.",
133                    ));
134                    continue;
135                }
136            }
137        };
138        let inv = InvocationInput::new(
139            root,
140            &input.spawn_root,
141            &skill_md,
142            cmd.as_deref(),
143            input.verify_stdin.as_deref(),
144        );
145        invocation::run(&inv, &mut report)?;
146    }
147
148    Ok(report)
149}
150
151/// How `verify` presents its results (Improvement B). The human format is the
152/// default; `json` is for CI gating / scripting and uses the machine-readable
153/// `check_id`s already on each [`CheckResult`].
154#[derive(Debug, Clone, Copy, PartialEq, Eq, clap::ValueEnum)]
155pub enum OutputFormat {
156    Human,
157    Json,
158    Sarif,
159    /// GitHub Actions workflow commands: one `::error`/`::warning` annotation
160    /// per failed/warned check, so CI failures surface inline on the PR diff.
161    Github,
162    /// JUnit XML for CI dashboards that consume xUnit-style reports (GitLab
163    /// CI, Jenkins, CircleCI). Errors map to `<failure>`; warnings are
164    /// attached as `<system-out>` notes on a passing case.
165    Junit,
166}
167
168/// Pretty-print a report as the human-facing output (design §5.2 step 4).
169/// Returns a single string the CLI writes to stdout.
170pub fn render(report: &VerifyReport) -> String {
171    use self::result::Severity;
172    let mut out = String::new();
173    let (pass, warn, fail, _skip) = report.counts();
174    for r in &report.results {
175        let glyph = match r.severity {
176            Severity::Pass => "✓",
177            Severity::Warn => "!",
178            Severity::Error => "✗",
179            Severity::Skipped => "·",
180        };
181        out.push_str(&format!(
182            "{} {}: {}\n",
183            glyph,
184            r.severity.as_str(),
185            r.check_name
186        ));
187        if !r.message.is_empty() {
188            out.push_str(&format!("    {}\n", r.message));
189        }
190        if let Some(s) = &r.suggestion {
191            out.push_str(&format!("    {s}\n"));
192        }
193    }
194
195    out.push_str(&format!(
196        "\n{pass} passed, {warn} warning(s), {fail} failed, discoverability score {}/100",
197        report.discoverability_score()
198    ));
199    out.push_str(if fail > 0 {
200        ": verify FAILED\n"
201    } else {
202        ": verify OK\n"
203    });
204    out
205}
206
207/// Render the report as a stable JSON object for CI / scripting. Shape:
208/// `{ "ok": bool, "discoverability_score": u8, "counts": {pass,warn,fail,skip},
209/// "results": [ {check_id, check_name, severity, message, suggestion?,
210/// location?} ... ] }`. The score weights Pass=1.0, Warn=0.5, Error=0;
211/// Skipped excluded from the denominator.
212pub fn render_json(report: &VerifyReport) -> String {
213    let (pass, warn, fail, skip) = report.counts();
214    let results: Vec<_> = report
215        .results
216        .iter()
217        .map(|r| {
218            let mut o = serde_json::json!({
219                "check_id": r.check_id,
220                "check_name": r.check_name,
221                "severity": r.severity.as_str(),
222                "message": r.message,
223            });
224            if let Some(s) = &r.suggestion {
225                o["suggestion"] = serde_json::Value::String(s.clone());
226            }
227            if let Some((file, line)) = &r.location {
228                let mut loc = serde_json::Map::new();
229                loc.insert("file".to_string(), serde_json::Value::String(file.clone()));
230                if let Some(n) = line {
231                    loc.insert("line".to_string(), serde_json::Value::from(*n));
232                }
233                o["location"] = serde_json::Value::Object(loc);
234            }
235            o
236        })
237        .collect();
238    let body = serde_json::json!({
239        "ok": !report.has_critical_failure(),
240        "discoverability_score": report.discoverability_score(),
241        "counts": {
242            "pass": pass,
243            "warn": warn,
244            "fail": fail,
245            "skip": skip,
246        },
247        "results": results,
248    });
249    serde_json::to_string_pretty(&body).expect("verify report serializes to JSON")
250}
251
252/// Render the report as GitHub Actions workflow commands (`::error` /
253/// `::warning`). Emitted to stdout so a CI step like
254/// `skillpack verify --format github` annotates the PR diff inline. Each
255/// `Error` maps to `::error`, each `Warn` to `::warning`; `file`/`line` are
256/// threaded from the result's `location` (absent when the check has no file
257/// location). Newlines in messages are flattened to spaces — a workflow
258/// command is a single line.
259pub fn render_github_annotations(report: &VerifyReport) -> String {
260    use self::result::Severity;
261
262    let mut out = String::new();
263    for r in &report.results {
264        let kind = match r.severity {
265            Severity::Error => "error",
266            Severity::Warn => "warning",
267            _ => continue,
268        };
269        // Build the property list as `key=value` pairs joined by commas, then
270        // escape the message. A workflow command is a single line: `::kind
271        // key=value,key=value::message`. The old code emitted a bare leading
272        // comma when a result had no file location (`::error,title=...`),
273        // which is malformed; properties are now only emitted when present.
274        let mut props: Vec<String> = Vec::new();
275        if let Some((file, line)) = &r.location {
276            props.push(format!("file={}", gh_escape(file)));
277            if let Some(n) = line {
278                props.push(format!("line={n}"));
279            }
280        }
281        // title is the human check label; the machine check_id stays available
282        // in the JSON/SARIF formats for pipelines that need it.
283        props.push(format!("title={}", gh_escape(&r.check_name)));
284
285        // Flatten newlines to spaces and %-escape the message so a raw `%` or
286        // embedded control char can't break the command grammar.
287        let mut message = r.message.replace(['\r', '\n'], " ");
288        if let Some(s) = &r.suggestion {
289            message.push(' ');
290            message.push_str(&s.replace(['\r', '\n'], " "));
291        }
292        message = gh_escape(&message);
293
294        out.push_str(&format!("::{kind} {}::{message}\n", props.join(",")));
295    }
296    out
297}
298
299/// Escape a value for a GitHub workflow command (properties and message).
300/// Per the Actions toolkit spec, `%`, `\r` and `\n` always need escaping, and
301/// `:`/`,` additionally need escaping inside a property *value* (they are the
302/// key/value and property separators). Escaping them in the message too is
303/// harmless (GitHub decodes uniformly).
304fn gh_escape(value: &str) -> String {
305    value
306        .replace('%', "%25")
307        .replace('\r', "%0D")
308        .replace('\n', "%0A")
309        .replace(':', "%3A")
310        .replace(',', "%2C")
311}
312
313/// Render the report as JUnit XML (xUnit-style) for CI dashboards that
314/// consume test-report artifacts (GitLab CI, Jenkins, CircleCI). Each result
315/// becomes a `<testcase name="<check_id>">`; `Error` severities are
316/// `<failure>` elements (counted in `failures`), `Warn` severities attach the
317/// message + suggestion as a `<system-out>` note on an otherwise-passing case,
318/// and `Skipped` emit a `<skipped/>` marker.
319pub fn render_junit(report: &VerifyReport) -> String {
320    use self::result::Severity;
321
322    let mut failures = 0usize;
323    let mut cases = String::new();
324    for r in &report.results {
325        cases.push_str(&format!(
326            "    <testcase name=\"{}\" classname=\"skillpack.verify\">",
327            xml_escape(&r.check_id)
328        ));
329        match r.severity {
330            Severity::Error => {
331                failures += 1;
332                let mut body = r.message.clone();
333                if let Some(s) = &r.suggestion {
334                    body.push_str("\nSuggestion: ");
335                    body.push_str(s);
336                }
337                cases.push_str(&format!(
338                    "<failure message=\"{}\">{}</failure>",
339                    xml_escape(&r.message),
340                    xml_escape(&body)
341                ));
342            }
343            Severity::Warn => {
344                let mut body = r.message.clone();
345                if let Some(s) = &r.suggestion {
346                    body.push_str("\nSuggestion: ");
347                    body.push_str(s);
348                }
349                cases.push_str(&format!("<system-out>{}</system-out>", xml_escape(&body)));
350            }
351            Severity::Pass => {}
352            Severity::Skipped => cases.push_str("<skipped/>"),
353        }
354        cases.push_str("</testcase>\n");
355    }
356
357    let total = report.results.len();
358    format!(
359        "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n\
360         <testsuites tests=\"{total}\" failures=\"{failures}\">\n\
361         \x20 <testsuite name=\"skillpack verify\" tests=\"{total}\" failures=\"{failures}\">\n\
362         {cases}  </testsuite>\n\
363         </testsuites>\n"
364    )
365}
366
367/// Escape a string for XML text/attribute content. Covers the five XML
368/// predefined entities; control characters are not escaped (verify messages
369/// are already newline-safe and lack NULs).
370fn xml_escape(s: &str) -> String {
371    s.replace('&', "&amp;")
372        .replace('<', "&lt;")
373        .replace('>', "&gt;")
374        .replace('"', "&quot;")
375        .replace('\'', "&apos;")
376}
377
378/// Render the report as SARIF 2.1.0 for GitHub Code Scanning upload-sarif.
379/// Only `Warn` and `Error` results are emitted (SARIF reports failures, not
380/// passes). Each result maps `ruleId` → `check_id`, `level` → `"warning"` or
381/// `"error"`, `message` → CheckResult message, `locations` → file + line
382/// when available. Pass/Skipped results are omitted.
383pub fn render_sarif(report: &VerifyReport) -> String {
384    use self::result::Severity;
385
386    let results: Vec<_> = report
387        .results
388        .iter()
389        .filter(|r| matches!(r.severity, Severity::Warn | Severity::Error))
390        .map(|r| {
391            let level = match r.severity {
392                Severity::Warn => "warning",
393                Severity::Error => "error",
394                _ => "none",
395            };
396            let mut result = serde_json::json!({
397                "ruleId": r.check_id,
398                "level": level,
399                "message": { "text": r.message },
400            });
401
402            // suggestion → rule metadata, appended to the message.
403            if let Some(s) = &r.suggestion {
404                result["message"]["text"] =
405                    serde_json::Value::String(format!("{}\nSuggestion: {s}", r.message));
406            }
407
408            if let Some((file, line)) = &r.location {
409                let mut region = serde_json::Map::new();
410                if let Some(n) = line {
411                    region.insert("startLine".to_string(), serde_json::Value::from(*n));
412                }
413                let mut phys_loc = serde_json::json!({
414                    "artifactLocation": { "uri": file }
415                });
416                if !region.is_empty() {
417                    phys_loc["region"] = serde_json::Value::Object(region);
418                }
419                result["locations"] = serde_json::json!([phys_loc]);
420            }
421
422            result
423        })
424        .collect();
425
426    let body = serde_json::json!({
427        "$schema": "https://json.schemastore.org/sarif-2.1.0.json",
428        "version": "2.1.0",
429        "runs": [{
430            "tool": {
431                "driver": {
432                    "name": "skillpack",
433                    "informationUri": "https://github.com/nordicnode/skillpack"
434                }
435            },
436            "results": results
437        }]
438    });
439
440    serde_json::to_string_pretty(&body).expect("verify report serializes to SARIF JSON")
441}
442
443#[cfg(test)]
444mod tests {
445    use super::*;
446    use crate::verify::result::{CheckResult, Severity};
447
448    fn warn_with_location(loc: Option<(String, Option<usize>)>) -> CheckResult {
449        CheckResult {
450            check_id: "discovery.skill.when_to_use".to_string(),
451            check_name: "SKILL.md has non-empty `when_to_use` trigger phrases".to_string(),
452            severity: Severity::Warn,
453            message: "when_to_use is missing".to_string(),
454            suggestion: Some("list 2-5 trigger verbs".to_string()),
455            location: loc,
456        }
457    }
458
459    #[test]
460    fn github_annotations_have_no_leading_comma_without_location() {
461        let report = VerifyReport {
462            results: vec![warn_with_location(None)],
463        };
464        let out = render_github_annotations(&report);
465        assert!(
466            !out.contains("::warning,"),
467            "must not emit a leading comma before properties, got: {out}"
468        );
469        assert!(
470            out.starts_with("::warning title="),
471            "title should be the first property, got: {out}"
472        );
473    }
474
475    #[test]
476    fn github_annotations_include_file_and_line_when_present() {
477        let report = VerifyReport {
478            results: vec![warn_with_location(Some((
479                "skills/foo/SKILL.md".to_string(),
480                Some(3),
481            )))],
482        };
483        let out = render_github_annotations(&report);
484        assert!(
485            out.contains("file=skills/foo/SKILL.md,line=3"),
486            "got: {out}"
487        );
488    }
489
490    #[test]
491    fn junit_counts_failures_and_escapes_xml() {
492        let mut report = VerifyReport::default();
493        report.push(CheckResult {
494            check_id: "a&b".to_string(),
495            check_name: "name".to_string(),
496            severity: Severity::Error,
497            message: "msg <x>".to_string(),
498            suggestion: Some("s&s".to_string()),
499            location: None,
500        });
501        report.push(CheckResult::pass("c", "name", "ok"));
502        report.push(CheckResult::skipped("d", "name", "skip"));
503
504        let out = render_junit(&report);
505        assert!(
506            out.starts_with("<?xml version=\"1.0\" encoding=\"UTF-8\"?>"),
507            "got: {out}"
508        );
509        assert!(out.contains("tests=\"3\" failures=\"1\""), "got: {out}");
510        assert!(out.contains("name=\"a&amp;b\""), "got: {out}");
511        assert!(
512            out.contains("<failure message=\"msg &lt;x&gt;\">"),
513            "got: {out}"
514        );
515        assert!(
516            out.contains("s&amp;s"),
517            "suggestion must be escaped, got: {out}"
518        );
519        assert!(out.contains("<skipped/>"), "got: {out}");
520    }
521
522    #[test]
523    fn github_annotations_escape_percent_and_flatten_newlines() {
524        let mut r = warn_with_location(None);
525        r.message = "100% broken\nsecond line".to_string();
526        let report = VerifyReport { results: vec![r] };
527        let out = render_github_annotations(&report);
528        assert!(
529            !out.contains("100% broken"),
530            "raw % must be escaped, got: {out}"
531        );
532        assert!(out.contains("100%25"), "got: {out}");
533        assert_eq!(
534            out.matches('\n').count(),
535            1,
536            "only the line terminator may remain, got: {out:?}"
537        );
538    }
539}
skillpack/verify/mod.rs

skillpack/verify/
mod.rs
