Skip to content

Commit fbc06e9

Browse files
feat(rhodibot): check a repository against the canon from the command line (#549)
Fourth and last step of the canon-sourced rules (#543, #545, #547). The three pieces before this one read the rules from the canon, decide which criteria apply, and classify what was found. Nothing ran them. This adds `rhodibot canon`: ```sh rhodibot canon --owner hyperpolymath --repo ubicity rhodibot canon --path /path/to/checkout --format json rhodibot canon --owner O --repo R --fail-on missing ``` **Advisory by default** — exits 0 whatever it finds. The canon designates hypatia's `rsr-conformance` as the single normative checker, so a gate here would be a second opinion claiming an authority it does not have. `--fail-on` is opt-in. ## Two details that carry their weight - `GitHubClient::tree_paths` asks once for the whole file list and **refuses a truncated response**. A partial tree is missing files, a missing file reads as an absent one, and the report would manufacture findings against a repository that has the file. - `get_file_content_if_present` distinguishes 404 from everything else: an absent `rsr-profile.a2ml` means "declares no capabilities", a 500 means "could not read the declaration". Treating the second as the first would silently shrink the check to the universal criteria and report a cleaner scorecard than the truth. `canon/report.rs` assembles the report (counts, findings worst-first, retired locations quoted) and serialises to JSON. `canon/local.rs` reads a checkout, preferring `git ls-files` over a walk — a walked list includes build output, and a `CODE_OF_CONDUCT.md` inside `target/` is not the repository's code of conduct. ## The pilot re-run through the tool Unauthenticated, against the five repositories of the advisory pilot: | repository | at path | elsewhere | deprecated | absent | |---|---|---|---|---| | gitbot-fleet | 17 | 3 | 0 | 2 | | ubicity | 7 | 2 | 6 | 7 | | awesome-nickel | 8 | 1 | 7 | 6 | | julia-professional-registry | 8 | 1 | 7 | 6 | | nesy-solver | 12 | 4 | 3 | 3 | **110 questions, 52 satisfied, 58 findings** (11 relocated, 23 at the retired `6a2/`, 24 absent). 22 of the canon's 74 criteria apply to every one of them: 26 are gated on a capability none declares, 26 more ask content questions rather than naming files. The 23 files under `.machine_readable/6a2/` are still 23, re-derived from criterion 3.1.1's own sentence about the retirement rather than from a hardcoded path, with the same per-repository breakdown as the hand run. Both of the hand run's false positives (criterion 1.2.2) now come back satisfied. ## Two findings the earlier scope could not see - `2.1.3` asks for `SECURITY.md`; `ubicity` has `SECURITY.adoc`. The verdict is `absent` and the report now says what is there instead. Same for `CODE_OF_CONDUCT.md`/`CODE_OF_CONDUCT.adoc` at `2.1.4`. If `.adoc` is the estate's convention, the canon is where that belongs. - `2.2.1` is `relocated` for all five: the `.well-known/` files live inside a `www/` publication bundle, not at the repository root the criterion names. The origin-versus-repository question is worth an answer. ## Verification - `cargo test --locked` — 135 lib + 10 lockstep + 50 integration, all pass - `cargo clippy --locked --all-targets -- -D warnings` — clean - new wiremock tests cover the tree listing (directories excluded), the truncated tree refusal, and 404-versus-500 on the optional file read - `scripts/check-canon-drift.sh` — exit 0 against upstream `main` Signed-off-by: Jonathan D.A. Jewell <6759885+hyperpolymath@users.noreply.github.com>
1 parent c80fb93 commit fbc06e9

6 files changed

Lines changed: 1311 additions & 3 deletions

File tree

‎bots/rhodibot/src/canon.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,9 @@
3939
//! The failure mode to avoid is the quiet one: a rule set that silently shrinks
4040
//! and reports every repository as compliant.
4141
42+
pub mod local;
4243
pub mod profile;
44+
pub mod report;
4345
pub mod requirement;
4446
pub mod verdict;
4547

‎bots/rhodibot/src/canon/local.rs‎

Lines changed: 256 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,256 @@
1+
// SPDX-License-Identifier: MPL-2.0
2+
3+
//! Reading a repository that is already on disk.
4+
//!
5+
//! The same checks run against a local checkout as against the API, which is
6+
//! useful for three things: a repository too large for GitHub to list in one
7+
//! response, a private repository with no token to hand, and checking before
8+
//! pushing rather than after.
9+
//!
10+
//! What "the repository's files" means here is the **tracked** set, taken from
11+
//! `git ls-files` when the directory is a work tree. A walk of the working
12+
//! directory would count build output and editor scratch files, and one of
13+
//! those can easily satisfy a criterion by accident -- a `CODE_OF_CONDUCT.md`
14+
//! inside `target/` is not the repository's code of conduct. Where there is no
15+
//! git to ask, the walk is the fallback, and it skips `.git` because those are
16+
//! not files anybody commits.
17+
18+
use std::path::Path;
19+
use std::process::Command;
20+
21+
use anyhow::{Context, Result, ensure};
22+
23+
/// The two paths the canon's baseline allows for a profile, in preference
24+
/// order: both are acceptable, and the first is the estate's.
25+
const PROFILE_PATHS: [&str; 2] = [
26+
".machine_readable/rsr-profile.a2ml",
27+
"machine-readable/rsr-profile.a2ml",
28+
];
29+
30+
/// A local checkout, read.
31+
#[derive(Debug)]
32+
pub struct LocalSource {
33+
/// The repository's tracked files, relative to its root.
34+
pub files: Vec<String>,
35+
/// The text of its profile, when it has one.
36+
pub profile: Option<String>,
37+
/// Whether the file list came from git or from a walk.
38+
from_git: bool,
39+
}
40+
41+
impl LocalSource {
42+
/// Whether the file list came from git rather than a walk.
43+
///
44+
/// Worth reporting: a walked list includes untracked files, so a criterion
45+
/// can be satisfied by something the repository does not actually carry.
46+
pub fn from_git(&self) -> bool {
47+
self.from_git
48+
}
49+
50+
/// Kept private so a `LocalSource` can only come from `read`, which is the
51+
/// only thing that knows whether the list is the tracked set.
52+
fn new(files: Vec<String>, profile: Option<String>, from_git: bool) -> Self {
53+
Self {
54+
files,
55+
profile,
56+
from_git,
57+
}
58+
}
59+
}
60+
61+
/// Read a checkout: its files, and its profile if it has one.
62+
pub fn read(root: &Path) -> Result<LocalSource> {
63+
ensure!(
64+
root.is_dir(),
65+
"{} is not a directory; --path expects a checkout",
66+
root.display()
67+
);
68+
69+
let git_files = tracked_files(root);
70+
let from_git = git_files.is_some();
71+
let files = match git_files {
72+
Some(files) => files,
73+
None => {
74+
let mut files = Vec::new();
75+
walk(root, root, &mut files).with_context(|| format!("walking {}", root.display()))?;
76+
files.sort();
77+
files
78+
}
79+
};
80+
81+
Ok(LocalSource::new(files, profile(root)?, from_git))
82+
}
83+
84+
/// The repository's tracked files, when git can be asked.
85+
fn tracked_files(root: &Path) -> Option<Vec<String>> {
86+
let output = Command::new("git")
87+
.arg("-C")
88+
.arg(root)
89+
.arg("ls-files")
90+
.output()
91+
.ok()?;
92+
93+
if !output.status.success() {
94+
return None;
95+
}
96+
97+
let listing = String::from_utf8(output.stdout).ok()?;
98+
let mut files: Vec<String> = listing
99+
.lines()
100+
.map(str::trim)
101+
.filter(|line| !line.is_empty())
102+
.map(str::to_string)
103+
.collect();
104+
files.sort();
105+
files.dedup();
106+
Some(files)
107+
}
108+
109+
/// Every file under `directory`, as a path relative to `root`.
110+
fn walk(root: &Path, directory: &Path, files: &mut Vec<String>) -> Result<()> {
111+
for entry in
112+
std::fs::read_dir(directory).with_context(|| format!("reading {}", directory.display()))?
113+
{
114+
let entry = entry?;
115+
let path = entry.path();
116+
let name = entry.file_name();
117+
let name = name.to_string_lossy();
118+
119+
if path.is_dir() {
120+
if name == ".git" {
121+
continue;
122+
}
123+
walk(root, &path, files)?;
124+
continue;
125+
}
126+
127+
let relative = path
128+
.strip_prefix(root)
129+
.with_context(|| format!("{} is not under {}", path.display(), root.display()))?;
130+
files.push(relative.to_string_lossy().replace('\\', "/"));
131+
}
132+
Ok(())
133+
}
134+
135+
/// The repository's profile text, if it has one.
136+
fn profile(root: &Path) -> Result<Option<String>> {
137+
for candidate in PROFILE_PATHS {
138+
let path = root.join(candidate);
139+
match std::fs::read_to_string(&path) {
140+
Ok(text) => return Ok(Some(text)),
141+
Err(error) if error.kind() == std::io::ErrorKind::NotFound => continue,
142+
Err(error) => {
143+
// Present but unreadable is not absent: reading it as "no
144+
// profile" would silently shrink the check to the universal
145+
// criteria and report a cleaner scorecard than the truth.
146+
return Err(error).with_context(|| format!("reading {}", path.display()));
147+
}
148+
}
149+
}
150+
Ok(None)
151+
}
152+
153+
#[cfg(test)]
154+
mod tests {
155+
use super::*;
156+
157+
/// A scratch directory with a unique name, removed on drop.
158+
struct Scratch(std::path::PathBuf);
159+
160+
impl Scratch {
161+
fn new(label: &str) -> Self {
162+
let path =
163+
std::env::temp_dir().join(format!("rhodibot-local-{}-{label}", std::process::id()));
164+
let _ = std::fs::remove_dir_all(&path);
165+
std::fs::create_dir_all(&path).expect("the scratch directory is creatable");
166+
Self(path)
167+
}
168+
169+
fn write(&self, relative: &str, contents: &str) -> &Self {
170+
let path = self.0.join(relative);
171+
std::fs::create_dir_all(path.parent().expect("has a parent"))
172+
.expect("parent directories are creatable");
173+
std::fs::write(&path, contents).expect("the file is writable");
174+
self
175+
}
176+
177+
fn path(&self) -> &Path {
178+
&self.0
179+
}
180+
}
181+
182+
impl Drop for Scratch {
183+
fn drop(&mut self) {
184+
let _ = std::fs::remove_dir_all(&self.0);
185+
}
186+
}
187+
188+
#[test]
189+
fn a_walk_skips_dot_git_and_finds_the_rest() {
190+
let scratch = Scratch::new("walk");
191+
scratch.write("README.adoc", "hi");
192+
scratch.write(".machine_readable/descriptiles/STATE.a2ml", "state");
193+
// A file inside .git is not a file the repository carries.
194+
scratch.write(".git/objects/deadbeef", "not yours");
195+
196+
let files = file_list_for(&scratch);
197+
assert_eq!(
198+
files,
199+
vec![
200+
".machine_readable/descriptiles/STATE.a2ml".to_string(),
201+
"README.adoc".to_string()
202+
]
203+
);
204+
}
205+
206+
#[test]
207+
fn a_profile_is_read_from_either_allowed_path() {
208+
let estate = Scratch::new("profile-estate");
209+
estate.write(
210+
".machine_readable/rsr-profile.a2ml",
211+
"[rsr-profile]\ncapabilities = [\"bash\"]\n",
212+
);
213+
let source = read(estate.path()).expect("reads");
214+
assert!(
215+
source
216+
.profile
217+
.as_deref()
218+
.expect("a profile")
219+
.contains("capabilities")
220+
);
221+
222+
let alternate = Scratch::new("profile-alt");
223+
alternate.write(
224+
"machine-readable/rsr-profile.a2ml",
225+
"[rsr-profile]\ncapabilities = [\"bash\"]\n",
226+
);
227+
assert!(read(alternate.path()).expect("reads").profile.is_some());
228+
}
229+
230+
#[test]
231+
fn no_profile_is_none_rather_than_an_error() {
232+
let scratch = Scratch::new("no-profile");
233+
scratch.write("README.adoc", "hi");
234+
let source = read(scratch.path()).expect("reads");
235+
assert_eq!(source.profile, None, "a repository may declare nothing");
236+
}
237+
238+
#[test]
239+
fn a_path_that_is_not_a_directory_is_refused() {
240+
let error = read(Path::new("/nonexistent-rhodibot-check")).expect_err("not a directory");
241+
assert!(
242+
format!("{error:#}").contains("not a directory"),
243+
"{error:#}"
244+
);
245+
}
246+
247+
/// The walk's own answer, bypassing git: the scratch directory is not a
248+
/// work tree, so this is what `read` produces, but asserting on it directly
249+
/// keeps the test meaningful on a machine where /tmp is inside a repository.
250+
fn file_list_for(scratch: &Scratch) -> Vec<String> {
251+
let mut files = Vec::new();
252+
walk(scratch.path(), scratch.path(), &mut files).expect("the walk succeeds");
253+
files.sort();
254+
files
255+
}
256+
}

0 commit comments

Comments
 (0)