diff --git a/robot-repo-automaton/src/fixer.rs b/robot-repo-automaton/src/fixer.rs index afc2c04b..3b482ace 100644 --- a/robot-repo-automaton/src/fixer.rs +++ b/robot-repo-automaton/src/fixer.rs @@ -72,12 +72,18 @@ const BINARY_EXTENSIONS: &[&str] = &[ ]; impl Fixer { - /// Create a new fixer for a repository + /// Create a fixer rooted at `repo_path`. + /// + /// When `dry_run` is true, fixes report the changes they would make without + /// modifying the working tree or creating commits. pub fn new(repo_path: PathBuf, dry_run: bool) -> Self { Fixer { repo_path, dry_run } } - /// Apply a fix for a detected issue + /// Apply one fix after enforcing the exclusion registry and repository boundary. + /// + /// Policy and boundary rejections are returned as unsuccessful [`FixResult`]s; + /// failures while performing an allowed operation are returned as errors. pub fn apply(&self, issue: &DetectedIssue, fix: &Fix) -> Result { // EXCLUSION REGISTRY GUARD: refuse the write if the target repo, // origin, or target path is on the estate-wide denylist. In dry-run @@ -337,7 +343,7 @@ impl Fixer { Ok(result) } - /// Delete a file + /// Delete a file, treating an already absent target as a successful no-op. fn apply_delete( &self, target_path: &Path, @@ -376,10 +382,9 @@ impl Fixer { }) } - /// Modify a file with safety checks and rollback support + /// Modify a non-binary file after validating the resulting source where supported. /// - /// Reads the modification specification from the fix, applies it to the file, - /// and rolls back if the modification produces invalid content. + /// Invalid modifications are rejected before the original file is replaced. fn apply_modify( &self, target_path: &Path, @@ -499,13 +504,10 @@ impl Fixer { }) } - /// Create a file with template expansion + /// Create an absent, non-ignored file from fallback content or a built-in template. /// - /// Supports template variables: - /// - `gitbot-fleet` - Repository name - /// - `hyperpolymath` - Repository owner - /// - `{{LICENSE}}` - License identifier - /// - `{{YEAR}}` - Current year + /// Empty expanded content is rejected, and publication does not overwrite a file + /// that appears concurrently. fn apply_create( &self, target_path: &Path, @@ -601,7 +603,7 @@ impl Fixer { }) } - /// Disable a workflow (rename to .disabled) + /// Disable a workflow by renaming it to a `.yml.disabled` path without overwriting. fn apply_disable( &self, target_path: &Path, @@ -684,7 +686,8 @@ impl Fixer { } } - /// Expand template variables in content + /// Replace the `gitbot-fleet` token with the repository name and expand the + /// licence, year, author and email placeholders. fn expand_template(&self, content: &str) -> String { let repo_name = self .repo_path @@ -702,7 +705,10 @@ impl Fixer { .replace("{{EMAIL}}", "j.d.a.jewell@open.ac.uk") } - /// Commit changes to the repository + /// Stage the listed repository paths and commit the resulting index to `HEAD`. + /// + /// Existing staged changes are included in the commit. Dry-run mode leaves both + /// the index and `HEAD` unchanged. pub fn commit(&self, message: &str, files: &[PathBuf]) -> Result<()> { // EXCLUSION REGISTRY GUARD: a commit is a write action even though // apply() has already checked each file individually, because some @@ -758,7 +764,10 @@ impl Fixer { Ok(()) } - /// Apply multiple fixes and commit + /// Apply each issue/fix pair and commit all successfully modified paths together. + /// + /// No commit is created in dry-run mode or when no fix reports a modified path. + /// The `_issues` slice is not consulted; processing is driven by `fixes`. pub fn apply_and_commit( &self, _issues: &[DetectedIssue], @@ -881,12 +890,14 @@ fn resolve_from_existing_ancestor(path: &Path) -> Result { } } +/// Return whether the byte at `index` follows an odd-length run of backslashes. fn is_escaped(value: &str, index: usize) -> bool { value[..index].bytes().rev() .take_while(|byte| *byte == b'\\') .count() % 2 == 1 } +/// Remove one escaping backslash from each `\:` sequence, preserving other escapes. fn unescape_colons(value: &str) -> String { let mut output = String::with_capacity(value.len()); let mut characters = value.chars().peekable(); diff --git a/robot-repo-automaton/src/hypatia.rs b/robot-repo-automaton/src/hypatia.rs index 0e2b8781..e10dd1fd 100644 --- a/robot-repo-automaton/src/hypatia.rs +++ b/robot-repo-automaton/src/hypatia.rs @@ -601,7 +601,10 @@ impl CicdHyperAClient { } } -/// Convert a verisim-data recipe JSON to a Rule. +/// Convert a verisim-data recipe into a rule. +/// +/// Returns `None` when the recipe lacks a string `id`, or when it supplies +/// neither a string `file_glob` nor a string content `pattern`. fn recipe_to_rule(recipe: &serde_json::Value) -> Option { let id = recipe.get("id")?.as_str()?.to_string(); let name = recipe.get("name").and_then(|v| v.as_str()).unwrap_or(&id).to_string(); diff --git a/robot-repo-automaton/src/main.rs b/robot-repo-automaton/src/main.rs index 494e4385..a0d8609e 100644 --- a/robot-repo-automaton/src/main.rs +++ b/robot-repo-automaton/src/main.rs @@ -742,9 +742,11 @@ fn cmd_catalog(path: &Path, severity_filter: Option<&str>) -> anyhow::Result<()> Ok(()) } -/// Base directory holding local repo checkouts. +/// Return the base directory holding local repository checkouts. /// -/// Override with `REPOS_BASE`; otherwise defaults to the canonical estate tree. +/// A non-empty `REPOS_BASE` takes precedence; otherwise this uses +/// `/developer/hyper-repos`, or `./developer/hyper-repos` when no home +/// directory is available. fn repos_base() -> PathBuf { if let Ok(base) = std::env::var("REPOS_BASE") { if !base.is_empty() { @@ -757,9 +759,11 @@ fn repos_base() -> PathBuf { .join("hyper-repos") } -/// Resolve a repo argument to a local path. +/// Resolve a repository argument to an existing local path. /// -/// Accepts either a local path or a GitHub owner/name format. +/// The argument is checked as supplied before being resolved relative to +/// [`repos_base`]. Returns an error containing both attempted locations when +/// neither exists. fn resolve_repo_path(repo: &str) -> anyhow::Result { let path = PathBuf::from(repo); if path.exists() {