diff --git a/bots/finishingbot/src/analyzers/claims.rs b/bots/finishingbot/src/analyzers/claims.rs index efbc7010..81a034ff 100644 --- a/bots/finishingbot/src/analyzers/claims.rs +++ b/bots/finishingbot/src/analyzers/claims.rs @@ -100,6 +100,7 @@ impl ClaimsAnalyzer { Severity::Error, "No README file found in repository root", ) + .with_file(path.join("README.adoc")) .with_suggestion("Create a README.md or README.adoc describing the project"), ); } @@ -186,6 +187,7 @@ impl ClaimsAnalyzer { Severity::Warning, "No CHANGELOG file found for release tracking", ) + .with_file(path.join("CHANGELOG.adoc")) .with_suggestion("Create a CHANGELOG.md following Keep a Changelog format"), ); } @@ -404,6 +406,7 @@ impl ClaimsAnalyzer { Severity::Warning, "No test files or test directories found in repository", ) + .with_file(path.join("tests")) .with_suggestion("Add tests to verify code functionality"), ); } else { @@ -434,6 +437,7 @@ impl ClaimsAnalyzer { Severity::Info, "No continuous integration configuration found", ) + .with_file(path.join(".github/workflows")) .with_suggestion("Add CI/CD configuration to automatically run tests"), ); } diff --git a/bots/finishingbot/src/analyzers/license.rs b/bots/finishingbot/src/analyzers/license.rs index 65cb6c7e..4ffbf853 100644 --- a/bots/finishingbot/src/analyzers/license.rs +++ b/bots/finishingbot/src/analyzers/license.rs @@ -113,6 +113,7 @@ impl LicenseAnalyzer { Severity::Error, "No LICENSE file found in repository root", ) + .with_file(path.join("LICENSE")) .with_suggestion("Add a LICENSE file with your chosen license"), ); } diff --git a/bots/finishingbot/src/analyzers/mod.rs b/bots/finishingbot/src/analyzers/mod.rs index bb2a4622..2e108a0f 100644 --- a/bots/finishingbot/src/analyzers/mod.rs +++ b/bots/finishingbot/src/analyzers/mod.rs @@ -264,3 +264,82 @@ impl AuditResult { all } } + +#[cfg(test)] +mod scope_tests { + //! Every finding must carry a path. The fork-governance verdict grid grades + //! a finding with no path as part of the change (SPEC §3.0), so a + //! repository-level rule with no `file` would show on every pull request + //! whatever it touches. A repo-level rule names the file that would fix it. + use super::*; + use tempfile::TempDir; + + /// Findings that genuinely have no file to name. Anything else lacking a + /// path is a regression. + const NO_MEANINGFUL_FILE: &[&str] = &["TEST-001"]; + + fn all_findings(dir: &Path) -> Vec { + let cfg = Config::default(); + let mut out = Vec::new(); + let analyzers: Vec> = vec![ + Box::new(license::LicenseAnalyzer::default()), + Box::new(placeholder::PlaceholderAnalyzer::default()), + Box::new(claims::ClaimsAnalyzer::default()), + Box::new(release::ReleaseAnalyzer::default()), + Box::new(scm_files::ScmFilesAnalyzer::default()), + Box::new(testing::TestingAnalyzer::default()), + Box::new(tooling::ToolingAnalyzer::default()), + Box::new(v1_readiness::V1ReadinessAnalyzer::default()), + ]; + for a in &analyzers { + out.extend(a.analyze(dir, &cfg).unwrap().findings); + } + out + } + + fn assert_all_pathed(dir: &Path) -> usize { + let findings = all_findings(dir); + let unpathed: Vec<&str> = findings + .iter() + .filter(|f| f.file.is_none() && !NO_MEANINGFUL_FILE.contains(&f.id.as_str())) + .map(|f| f.id.as_str()) + .collect(); + assert!(unpathed.is_empty(), "findings without a path: {:?}", unpathed); + findings.len() + } + + #[test] + fn empty_repo_findings_all_carry_a_path() { + let dir = TempDir::new().unwrap(); + // Negative control: the empty repo really does produce repo-level findings. + assert!(assert_all_pathed(dir.path()) > 5); + } + + #[test] + fn rust_repo_findings_all_carry_a_path() { + let dir = TempDir::new().unwrap(); + std::fs::write( + dir.path().join("Cargo.toml"), + "[package]\nname = \"x\"\nversion = \"0.1.0\"\n", + ) + .unwrap(); + assert!(assert_all_pathed(dir.path()) > 5); + } + + #[test] + fn deno_repo_findings_all_carry_a_path() { + let dir = TempDir::new().unwrap(); + std::fs::write(dir.path().join("deno.json"), "{}").unwrap(); + assert!(assert_all_pathed(dir.path()) > 5); + } + + #[test] + fn missing_file_rule_names_the_expected_path() { + let dir = TempDir::new().unwrap(); + let f = all_findings(dir.path()) + .into_iter() + .find(|f| f.id == "LIC-001") + .expect("LIC-001 on an empty repo"); + assert_eq!(f.file.as_deref(), Some(dir.path().join("LICENSE").as_path())); + } +} diff --git a/bots/finishingbot/src/analyzers/placeholder.rs b/bots/finishingbot/src/analyzers/placeholder.rs index 313659d5..f5281488 100644 --- a/bots/finishingbot/src/analyzers/placeholder.rs +++ b/bots/finishingbot/src/analyzers/placeholder.rs @@ -94,6 +94,7 @@ impl Analyzer for PlaceholderAnalyzer { config.placeholders.max_allowed ), ) + .with_file(Path::new(".finishing-bot/config.yml").to_path_buf()) .with_suggestion(&format!("Remove at least {} placeholders before release", excess)), ); } diff --git a/bots/finishingbot/src/analyzers/release.rs b/bots/finishingbot/src/analyzers/release.rs index a3056d4a..16637abc 100644 --- a/bots/finishingbot/src/analyzers/release.rs +++ b/bots/finishingbot/src/analyzers/release.rs @@ -123,6 +123,7 @@ impl ReleaseAnalyzer { Severity::Error, &format!("Version numbers are inconsistent:\n {}", version_list.join("\n ")), ) + .with_file(path.join(&inconsistent[0].0)) .with_suggestion("Ensure all version files have the same version number"), ); } @@ -186,6 +187,7 @@ impl ReleaseAnalyzer { Severity::Warning, &format!("Debug file found: {}", pattern), ) + .with_file(path.join(pattern)) .with_suggestion("Remove debug files before release"), ); } @@ -261,6 +263,7 @@ impl ReleaseAnalyzer { Severity::Warning, "No SECURITY.md file found", ) + .with_file(path.join("SECURITY.md")) .with_suggestion("Add a SECURITY.md describing how to report vulnerabilities"), ); } @@ -312,6 +315,7 @@ impl ReleaseAnalyzer { Severity::Warning, &format!("No hash files found in {}", dir), ) + .with_file(release_path.clone()) .with_suggestion("Generate SHA256/SHA512 hashes for release artifacts"), ); } @@ -489,6 +493,7 @@ impl ReleaseAnalyzer { Severity::Info, "No GitHub release workflow found", ) + .with_file(release_workflow.clone()) .with_suggestion("Consider adding .github/workflows/release.yml for automated releases"), ); } diff --git a/bots/finishingbot/src/analyzers/scm_files.rs b/bots/finishingbot/src/analyzers/scm_files.rs index a7e89fe2..08879e58 100644 --- a/bots/finishingbot/src/analyzers/scm_files.rs +++ b/bots/finishingbot/src/analyzers/scm_files.rs @@ -76,6 +76,7 @@ impl Analyzer for ScmFilesAnalyzer { Severity::Error, &format!("Required SCM file {} not found ({}).", filename, description), ) + .with_file(correct_path.clone()) .with_suggestion(&format!( "Create .machine_readable/{} with proper metadata and content. See hyperpolymath SCM file specs.", filename diff --git a/bots/finishingbot/src/analyzers/testing.rs b/bots/finishingbot/src/analyzers/testing.rs index 6b7f56b4..43886197 100644 --- a/bots/finishingbot/src/analyzers/testing.rs +++ b/bots/finishingbot/src/analyzers/testing.rs @@ -117,7 +117,7 @@ impl TestingAnalyzer { result.add(Finding::new( "TEST-AG-001", "Missing Agda source", Severity::Error, "The proof layout contains no nonempty .agda source files.", - ).with_suggestion("Provide actual proof sources; an empty directory is not verification infrastructure")); + ).with_file(path.join("verification/proofs")).with_suggestion("Provide actual proof sources; an empty directory is not verification infrastructure")); } let has_entrypoint = [ ".github/workflows/agda.yml", @@ -142,7 +142,7 @@ impl TestingAnalyzer { result.add(Finding::new( "TEST-AG-002", "Agda runner not statically identified", Severity::Warning, "No direct single-line Agda source check was identified in the known workflow or recipe locations. Dynamic runners need separate review; no proof was executed.", - ).with_suggestion("Provide or document the real Agda check entrypoint instead of adding Rust or Deno scaffolding")); + ).with_file(path.join(".github/workflows/agda.yml")).with_suggestion("Provide or document the real Agda check entrypoint instead of adding Rust or Deno scaffolding")); } } @@ -158,6 +158,7 @@ impl TestingAnalyzer { Severity::Error, "Rust project missing benches/ directory for criterion benchmarks.", ) + .with_file(benches_dir.clone()) .with_suggestion("Create benches/ directory with criterion benchmark files"), ); } else { @@ -177,6 +178,7 @@ impl TestingAnalyzer { Severity::Warning, "benches/ directory exists but contains no .rs files.", ) + .with_file(benches_dir.clone()) .with_suggestion("Add benchmark files to benches/ directory"), ); } @@ -195,6 +197,7 @@ impl TestingAnalyzer { Severity::Error, "Rust project missing fuzz/ or .clusterfuzzlite/ for fuzzing.", ) + .with_file(fuzz_dir.clone()) .with_suggestion("Add ClusterFuzzLite configuration for automated fuzzing"), ); } else { @@ -229,6 +232,7 @@ impl TestingAnalyzer { Severity::Error, "No .github/workflows/stress-test.yml found.", ) + .with_file(stress_workflow.clone()) .with_suggestion("Add stress test workflow to test under high load"), ); } else { @@ -266,6 +270,7 @@ impl TestingAnalyzer { Severity::Error, "Deno project missing bench/ directory or bench.ts file.", ) + .with_file(bench_dir.clone()) .with_suggestion("Create bench/ directory with Deno.bench() benchmarks"), ); } else { @@ -282,6 +287,7 @@ impl TestingAnalyzer { Severity::Error, "No .github/workflows/stress-test.yml found.", ) + .with_file(stress_workflow.clone()) .with_suggestion("Add stress test workflow for load testing"), ); } else { @@ -310,6 +316,7 @@ impl TestingAnalyzer { Severity::Warning, "No tests/ directory or *_test.ts files found.", ) + .with_file(tests_dir.clone()) .with_suggestion("Add test files following Deno conventions (*_test.ts)"), ); } diff --git a/bots/finishingbot/src/analyzers/tooling.rs b/bots/finishingbot/src/analyzers/tooling.rs index 87b79ea9..c066e75c 100644 --- a/bots/finishingbot/src/analyzers/tooling.rs +++ b/bots/finishingbot/src/analyzers/tooling.rs @@ -36,6 +36,7 @@ impl Analyzer for ToolingAnalyzer { Severity::Error, ".tool-versions file not found for asdf/mise version management.", ) + .with_file(tool_versions.clone()) .with_suggestion("Create .tool-versions file with required tool versions (rust, just, etc.)"), ); } else { @@ -57,6 +58,7 @@ impl Analyzer for ToolingAnalyzer { Severity::Warning, ".editorconfig not found for consistent code formatting.", ) + .with_file(editorconfig.clone()) .with_suggestion("Add .editorconfig for consistent editor settings"), ); } else { @@ -73,6 +75,7 @@ impl Analyzer for ToolingAnalyzer { Severity::Error, ".github/workflows/ directory not found.", ) + .with_file(workflows_dir.clone()) .with_suggestion("Add CI/CD workflows for automated testing and quality checks"), ); } else { diff --git a/bots/finishingbot/src/analyzers/v1_readiness.rs b/bots/finishingbot/src/analyzers/v1_readiness.rs index 7bfecd32..13f80cd2 100644 --- a/bots/finishingbot/src/analyzers/v1_readiness.rs +++ b/bots/finishingbot/src/analyzers/v1_readiness.rs @@ -69,6 +69,7 @@ impl V1ReadinessAnalyzer { Severity::Error, &format!("Required file {} not found ({}).", filename, description), ) + .with_file(file_path.clone()) .with_suggestion(&format!("Create {} file", filename)), ); } else { @@ -88,6 +89,7 @@ impl V1ReadinessAnalyzer { Severity::Warning, "SECURITY.md not found for responsible disclosure.", ) + .with_file(security_file.clone()) .with_suggestion("Add SECURITY.md with security policy and contact information"), ); } else { @@ -173,6 +175,7 @@ impl V1ReadinessAnalyzer { Severity::Warning, &format!("{} file not found ({}).", filename, description), ) + .with_file(file_path.clone()) .with_suggestion(&format!("Add {} for community health", filename)), ); } else { diff --git a/bots/finishingbot/src/main.rs b/bots/finishingbot/src/main.rs index 111ab1cf..9091b241 100644 --- a/bots/finishingbot/src/main.rs +++ b/bots/finishingbot/src/main.rs @@ -262,7 +262,10 @@ fn handle_fleet_mode( fn init_logging(level: &str) { let filter = EnvFilter::try_new(level).unwrap_or_else(|_| EnvFilter::new("info")); + // Logs go to stderr: stdout carries the JSON report that the fleet driver + // parses, and a log line there makes the report unreadable. let layer = fmt::layer() + .with_writer(std::io::stderr) .with_target(true) .with_span_events(FmtSpan::CLOSE) .with_ansi(true); diff --git a/bots/glambot/README.adoc b/bots/glambot/README.adoc index 553f8af1..626b3a31 100644 --- a/bots/glambot/README.adoc +++ b/bots/glambot/README.adoc @@ -115,7 +115,7 @@ Shells out to the `git-seo` CLI tool for comprehensive forge-based SEO scoring ( |=== | Finding ID | Description | Severity -| GS-001 | git-seo not installed | Info +| GS-001 | _Retired as a finding_: git-seo not being installed is a property of the machine running glambot, not of the repository, so it is logged (`warn`) and the analyzer reports nothing | n/a | GS-002 | git-seo analysis failed | Warning | GS-003 | No git remote found | Info | GS-100 | Overall SEO score | Varies diff --git a/bots/glambot/src/analyzers/git_seo_integration.rs b/bots/glambot/src/analyzers/git_seo_integration.rs index dfbb1810..ed66117c 100644 --- a/bots/glambot/src/analyzers/git_seo_integration.rs +++ b/bots/glambot/src/analyzers/git_seo_integration.rs @@ -20,26 +20,24 @@ impl Default for GitSeoAnalyzer { } } -impl Analyzer for GitSeoAnalyzer { - fn name(&self) -> &str { - "Git-SEO" - } - - fn analyze(&self, path: &Path, config: &Config) -> Result { +impl GitSeoAnalyzer { + /// Analyze with an explicit git-seo availability (testable seam). + fn analyze_with( + &self, + path: &Path, + config: &Config, + git_seo_installed: bool, + ) -> Result { let mut result = AnalysisResult::new(); let start = std::time::Instant::now(); - // Check if git-seo is installed - if !is_git_seo_installed() { - result.add( - Finding::new( - "GS-001", - "git-seo not installed", - Severity::Info, - "Install git-seo for comprehensive SEO analysis: https://github.com/hyperpolymath/git-seo", - ) - .with_suggestion("Install: git clone https://github.com/hyperpolymath/git-seo && cd git-seo && julia --project=. -e 'using Pkg; Pkg.instantiate()'"), - ); + // git-seo missing is a property of the machine running glambot, not of + // the repository under review, so it must never become a finding: a + // finding is graded as the change (an info finding turns the lane + // "minor" on every pull request). The analyzer did not run, which is + // logged loudly instead; install hint: https://github.com/hyperpolymath/git-seo + if !git_seo_installed { + warn!("git-seo is not installed; the Git-SEO analyzer did not run (no findings reported, not a clean result)"); result.duration_ms = start.elapsed().as_millis() as u64; return Ok(result); } @@ -75,6 +73,7 @@ impl Analyzer for GitSeoAnalyzer { Severity::Info, "Repository has no git remote. git-seo requires a forge URL (GitHub/GitLab/Bitbucket).", ) + .with_file(path.join(".git/config")) .with_suggestion("Add remote: git remote add origin "), ); } @@ -82,6 +81,16 @@ impl Analyzer for GitSeoAnalyzer { result.duration_ms = start.elapsed().as_millis() as u64; Ok(result) } +} + +impl Analyzer for GitSeoAnalyzer { + fn name(&self) -> &str { + "Git-SEO" + } + + fn analyze(&self, path: &Path, config: &Config) -> Result { + self.analyze_with(path, config, is_git_seo_installed()) + } fn fix(&self, path: &Path, config: &Config, findings: &[Finding]) -> Result> { let mut fixes = Vec::new(); @@ -321,30 +330,41 @@ mod tests { Config::default() } - #[test] - fn test_repo_without_git_remote() { + fn git_repo_without_remote() -> TempDir { let dir = TempDir::new().unwrap(); - // Initialize a git repo without a remote std::process::Command::new("git") .arg("init") .arg(dir.path()) .output() .unwrap(); + dir + } - let analyzer = GitSeoAnalyzer::default(); - let config = default_config(); - let result = analyzer.analyze(dir.path(), &config).unwrap(); - - // Should find no remote (GS-003) or git-seo not installed (GS-001) - let ids: Vec<&str> = result - .findings - .iter() - .map(|f| f.id.as_str()) - .collect(); + #[test] + fn test_repo_without_git_remote() { + let dir = git_repo_without_remote(); + let result = GitSeoAnalyzer::default() + .analyze_with(dir.path(), &default_config(), true) + .unwrap(); + let gs003: Vec<_> = result.findings.iter().filter(|f| f.id == "GS-003").collect(); + assert_eq!(gs003.len(), 1, "a repo with no remote reports GS-003"); + assert_eq!( + gs003[0].file.as_deref(), + Some(dir.path().join(".git/config").as_path()), + "GS-003 names the file that would fix it so the grid can scope it" + ); + } + + #[test] + fn missing_git_seo_is_not_a_finding() { + let dir = git_repo_without_remote(); + let result = GitSeoAnalyzer::default() + .analyze_with(dir.path(), &default_config(), false) + .unwrap(); assert!( - ids.contains(&"GS-001") || ids.contains(&"GS-003"), - "Should report GS-001 (no git-seo) or GS-003 (no remote), got: {:?}", - ids + result.findings.is_empty(), + "tool-availability advice is not a repo finding, got: {:?}", + result.findings.iter().map(|f| f.id.as_str()).collect::>() ); } diff --git a/bots/glambot/src/analyzers/machine.rs b/bots/glambot/src/analyzers/machine.rs index ba066a10..487eb451 100644 --- a/bots/glambot/src/analyzers/machine.rs +++ b/bots/glambot/src/analyzers/machine.rs @@ -324,6 +324,7 @@ impl MachineAnalyzer { Severity::Info, "No robots.txt found. Consider adding one for crawler control.", ) + .with_file(path.join("robots.txt")) .with_suggestion("Add robots.txt to repository root if hosting documentation"), ); } else { @@ -344,6 +345,7 @@ impl MachineAnalyzer { "No .machine_readable/ directory found. SCM files (STATE.scm, META.scm, ECOSYSTEM.scm) should be placed here.", ) .fixable() + .with_file(mr_dir.clone()) .with_suggestion("Create .machine_readable/ directory with STATE.scm, META.scm, ECOSYSTEM.scm"), ); return; @@ -365,6 +367,7 @@ impl MachineAnalyzer { Severity::Info, &format!("{} not found in .machine_readable/ directory.", scm_file), ) + .with_file(mr_dir.join(scm_file)) .with_suggestion(&format!("Add {} to .machine_readable/", scm_file)), ); } diff --git a/bots/glambot/src/analyzers/mod.rs b/bots/glambot/src/analyzers/mod.rs index 718d606a..129bdeb7 100644 --- a/bots/glambot/src/analyzers/mod.rs +++ b/bots/glambot/src/analyzers/mod.rs @@ -217,3 +217,36 @@ impl AuditResult { all } } + +#[cfg(test)] +mod scope_tests { + //! A finding with no path is graded as part of the change by the + //! fork-governance verdict grid (SPEC §3.0), so repository-level rules + //! must name the file that would fix them. + use super::*; + use tempfile::TempDir; + + #[test] + fn empty_repo_findings_all_carry_a_path() { + let dir = TempDir::new().unwrap(); + let cfg = Config::default(); + let mut findings = Vec::new(); + let analyzers: Vec> = vec![ + Box::new(visual::VisualAnalyzer::default()), + Box::new(accessibility::AccessibilityAnalyzer::default()), + Box::new(seo::SeoAnalyzer::default()), + Box::new(machine::MachineAnalyzer::default()), + ]; + for a in &analyzers { + findings.extend(a.analyze(dir.path(), &cfg).unwrap().findings); + } + // Negative control: the empty repo really does produce repo-level findings. + assert!(findings.len() >= 3, "got {:?}", findings.iter().map(|f| &f.id).collect::>()); + let unpathed: Vec<&str> = findings + .iter() + .filter(|f| f.file.is_none()) + .map(|f| f.id.as_str()) + .collect(); + assert!(unpathed.is_empty(), "findings without a path: {:?}", unpathed); + } +} diff --git a/bots/glambot/src/analyzers/visual.rs b/bots/glambot/src/analyzers/visual.rs index e2ee6da5..e8768087 100644 --- a/bots/glambot/src/analyzers/visual.rs +++ b/bots/glambot/src/analyzers/visual.rs @@ -138,6 +138,7 @@ impl VisualAnalyzer { Severity::Error, "No README file found in repository root.", ) + .with_file(path.join("README.adoc")) .with_suggestion("Add a README.md or README.adoc file"), ); } @@ -272,6 +273,7 @@ impl VisualAnalyzer { Severity::Info, "No logo file found. Consider adding branding.", ) + .with_file(path.join("logo.svg")) .with_suggestion("Add logo.png or logo.svg to repository root or docs/ directory"), ); } diff --git a/bots/glambot/src/main.rs b/bots/glambot/src/main.rs index d826e986..50b71e1b 100644 --- a/bots/glambot/src/main.rs +++ b/bots/glambot/src/main.rs @@ -134,8 +134,11 @@ fn main() -> ExitCode { fn init_logging(level: &str) { let filter = EnvFilter::try_new(level).unwrap_or_else(|_| EnvFilter::new("info")); + // Logs go to stderr: stdout carries the JSON report that the fleet driver + // parses, and a log line there makes the report unreadable. fmt() .with_target(true) + .with_writer(std::io::stderr) .with_ansi(true) .with_env_filter(filter) .init(); diff --git a/bots/glambot/tests/integration.rs b/bots/glambot/tests/integration.rs index 19326966..b5390e3b 100644 --- a/bots/glambot/tests/integration.rs +++ b/bots/glambot/tests/integration.rs @@ -223,3 +223,21 @@ fn test_analysis_result_methods() { assert!(result.has_errors(), "has_errors() should return true"); assert!(result.files_checked > 0, "Should have checked some files"); } + +/// The fleet driver parses stdout as the JSON report, so nothing else may be +/// written there: log lines belong on stderr. +#[test] +fn json_audit_stdout_is_only_the_report() { + let dir = TempDir::new().unwrap(); + // Exactly the invocation scripts/fleet-report-back.sh uses. + let out = std::process::Command::new(env!("CARGO_BIN_EXE_glambot")) + .arg("--path") + .arg(dir.path()) + .args(["--format", "json", "audit"]) + .output() + .unwrap(); + let text = String::from_utf8(out.stdout).unwrap(); + let parsed: serde_json::Value = + serde_json::from_str(&text).unwrap_or_else(|e| panic!("stdout is not pure JSON ({e}): {text}")); + assert!(parsed.is_object()); +} diff --git a/docs/REPORT-BACK.adoc b/docs/REPORT-BACK.adoc index 450fde15..25ebc21f 100644 --- a/docs/REPORT-BACK.adoc +++ b/docs/REPORT-BACK.adoc @@ -42,7 +42,7 @@ adopts it. build is a warning here and an `error` row later, never a dead run. | `tests/fleet-report-back-test.sh` -| 48 checks over stubbed adapters whose JSON shapes were taken from live runs: +| Checks over stubbed adapters whose JSON shapes were taken from live runs: enrolment rules, every adapter's translation, path relativisation, the failure modes, envelope shape, `--plan`, the summary, and (with `STANDARDS_DIR` set) the real renderer turning the rows non-orange. @@ -90,6 +90,18 @@ reason), where the grid renders it grey. `severity` is the fleet's own word are repository-relative with no `./`, so the grid's change-scope (delta) grading can match them against the diff. +*Every finding must carry a path.* The canon grades a finding with no path as +part of the change (standards `3-practice/fork-governance/SPEC.adoc` §3.0: +absence of a location is not evidence of absence of a defect, and the +alternative lets a reporter escape the gate by omitting a field). A +repository-level rule that omits its path therefore shows on every pull request +whatever it touches. The path of such a finding is the file the rule is about: +the expected path for a missing-file rule (`docs/status/ROADMAP.adoc`, +`.github/SECURITY.md`), the manifest or config that would fix a whole-repo +property. A pull request that does not touch that file then reports the finding +as standing, as the canon intends. The driver prints a `WARNING` for any member +that emits a path-less finding; rhodibot and seambot already comply. + == 4. What each member reports, and how [cols="1,2,3",options="header"] diff --git a/scripts/fleet-report-back.sh b/scripts/fleet-report-back.sh index 60290a26..3cdea75a 100755 --- a/scripts/fleet-report-back.sh +++ b/scripts/fleet-report-back.sh @@ -618,6 +618,14 @@ fi for m in "${members[@]+"${members[@]}"}"; do run_member "$m" echo "fleet-report-back: $m -> $(jq -r '.status + (if .status == "ran" then " (\(.findings | length) finding(s))" else ": " + (.reason // "") end)' "$out/$m.json")" + # A path-less finding is graded as part of EVERY change (standards + # fork-governance SPEC section 3.0: absence of a location is not evidence of + # absence of a defect), so a repository-level rule that omits its path makes + # the row identical on every pull request. Say so, loudly, at the source. + unpathed=$(jq -r '[.findings[] | select((.file // "") == "")] | length' "$out/$m.json") + if [ "$unpathed" -gt 0 ]; then + echo "fleet-report-back: WARNING: $m reported $unpathed finding(s) with no path; the grid grades those as part of every change. The adapter must name the file each rule is about (the expected path, for a missing-file rule)." >&2 + fi done # ── summary ────────────────────────────────────────────────────────────────── diff --git a/tests/fleet-report-back-test.sh b/tests/fleet-report-back-test.sh index 6c035fb1..35115936 100755 --- a/tests/fleet-report-back-test.sh +++ b/tests/fleet-report-back-test.sh @@ -149,6 +149,8 @@ check "seambot: check name becomes the rule id" "SEAM-register-exists|error|spec check "finishingbot: analyzers are flattened" 2 "$(field "$TMP/o1/finishingbot.json" '.findings | length')" check "finishingbot: ./ prefix is stripped" config/tools.yml "$(field "$TMP/o1/finishingbot.json" '.findings[] | select(.rule_id == "LIC-002") | .file')" check "finishingbot: a repository-level finding has an empty path" "" "$(field "$TMP/o1/finishingbot.json" '.findings[] | select(.rule_id == "REL-001") | .file')" +check "driver: warns loudly about a finding with no path" 1 "$(grep -c 'WARNING: finishingbot reported 1 finding(s) with no path' "$TMP/o1.log")" +check "driver: does not warn about members whose findings all carry a path" 0 "$(grep -c 'WARNING: glambot' "$TMP/o1.log")" check "glambot: line numbers survive" 7 "$(field "$TMP/o1/glambot.json" '.findings[0].line')" check "accessibilitybot: absolute paths become repository-relative" src/index.html "$(field "$TMP/o1/accessibilitybot.json" '.findings[0].file')" check "a default title is used when the manifest has none" "glambot (presentation quality)" "$(field "$TMP/o1/glambot.json" .title)"