Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions bots/finishingbot/src/analyzers/claims.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
);
}
Expand Down Expand Up @@ -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"),
);
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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"),
);
}
Expand Down
1 change: 1 addition & 0 deletions bots/finishingbot/src/analyzers/license.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
);
}
Expand Down
79 changes: 79 additions & 0 deletions bots/finishingbot/src/analyzers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Finding> {
let cfg = Config::default();
let mut out = Vec::new();
let analyzers: Vec<Box<dyn Analyzer>> = 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()));
}
}
1 change: 1 addition & 0 deletions bots/finishingbot/src/analyzers/placeholder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
);
}
Expand Down
5 changes: 5 additions & 0 deletions bots/finishingbot/src/analyzers/release.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
);
}
Expand Down Expand Up @@ -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"),
);
}
Expand Down Expand Up @@ -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"),
);
}
Expand Down Expand Up @@ -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"),
);
}
Expand Down Expand Up @@ -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"),
);
}
Expand Down
1 change: 1 addition & 0 deletions bots/finishingbot/src/analyzers/scm_files.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 9 additions & 2 deletions bots/finishingbot/src/analyzers/testing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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"));
}
}

Expand All @@ -158,6 +158,7 @@ impl TestingAnalyzer {
Severity::Error,
"Rust project missing benches/ directory for criterion benchmarks.",
)
.with_file(benches_dir.clone())

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Point directory-level rules at a trackable file

When a PR introduces a Rust project by adding Cargo.toml without a benchmark, this error is now scoped to benches, but the grid matches finding paths against git diff --name-only and Git never reports directories. Thus TEST-RU-001 can never be classified as part of the change and is always downgraded to standing; the same problem affects the newly assigned directory paths for empty benchmarks, fuzzing, release artifacts, tests, and workflow directories. Use a concrete expected file such as a benchmark source, or the tracked manifest that activates the rule.

Useful? React with 👍 / 👎.

.with_suggestion("Create benches/ directory with criterion benchmark files"),
);
} else {
Expand All @@ -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"),
);
}
Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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)"),
);
}
Expand Down
3 changes: 3 additions & 0 deletions bots/finishingbot/src/analyzers/tooling.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down
3 changes: 3 additions & 0 deletions bots/finishingbot/src/analyzers/v1_readiness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
3 changes: 3 additions & 0 deletions bots/finishingbot/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion bots/glambot/README.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading
Loading