diff --git a/robot-repo-automaton/src/fixer.rs b/robot-repo-automaton/src/fixer.rs index f04df5fc..2717d648 100644 --- a/robot-repo-automaton/src/fixer.rs +++ b/robot-repo-automaton/src/fixer.rs @@ -72,7 +72,10 @@ 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, policy and boundary checks still run, but eligible + /// fixes are only reported and no files or commits are changed. pub fn new(repo_path: PathBuf, dry_run: bool) -> Self { // Keep one root representation for ignore rules and index paths. // Invalid roots are retained so apply's boundary check reports the error. @@ -80,7 +83,12 @@ impl Fixer { Fixer { repo_path, dry_run } } - /// Apply a fix for a detected issue + /// Apply a fix after checking the exclusion registry and repository boundary. + /// + /// Policy rejections and action-level non-applicability are represented in + /// [`FixResult`]; failures that prevent the operation itself may be returned + /// as errors. In dry-run mode, eligible fixes report their intended changes + /// without writing them. 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 @@ -379,10 +387,10 @@ impl Fixer { }) } - /// Modify a file with safety checks and rollback support + /// Modify a text file without exposing partially written content. /// - /// Reads the modification specification from the fix, applies it to the file, - /// and rolls back if the modification produces invalid content. + /// Binary files are rejected. For supported structured formats, the complete + /// result is parsed before the original file is atomically replaced. fn apply_modify( &self, target_path: &Path, @@ -502,13 +510,12 @@ impl Fixer { }) } - /// Create a file with template expansion + /// Create a file from explicit fallback content or a built-in template. /// - /// Supports template variables: - /// - `gitbot-fleet` - Repository name - /// - `hyperpolymath` - Repository owner - /// - `{{LICENSE}}` - License identifier - /// - `{{YEAR}}` - Current year + /// Existing or ignored targets are not written, and publication uses + /// no-clobber semantics. Template expansion replaces the literal + /// `gitbot-fleet` name and the `{{LICENSE}}`, `{{YEAR}}`, `{{AUTHOR}}` and + /// `{{EMAIL}}` placeholders. fn apply_create( &self, target_path: &Path, @@ -604,7 +611,9 @@ impl Fixer { }) } - /// Disable a workflow (rename to .disabled) + /// Disable a file by renaming it with a `yml.disabled` extension. + /// + /// The rename does not replace an existing destination. fn apply_disable( &self, target_path: &Path, @@ -671,7 +680,9 @@ impl Fixer { false } - /// Get template content for a file creation + /// Return explicit fallback content or the built-in template for `target`. + /// + /// Targets without either source return an empty string. fn get_template_content(&self, target: &str, fix: &Fix) -> String { // If the fix has explicit content in the fallback field, use it if let Some(ref fallback) = fix.fallback { @@ -687,7 +698,7 @@ impl Fixer { } } - /// Expand template variables in content + /// Expand the repository-specific and fixed metadata placeholders in a template. fn expand_template(&self, content: &str) -> String { let repo_name = self .repo_path @@ -705,7 +716,11 @@ impl Fixer { .replace("{{EMAIL}}", "j.d.a.jewell@open.ac.uk") } - /// Commit changes to the repository + /// Stage the listed paths and commit the resulting repository index. + /// + /// Deleted paths are removed from the index, while paths outside the canonical + /// repository root are ignored. Dry-run mode performs the exclusion check but + /// neither stages nor commits changes. 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 @@ -761,7 +776,11 @@ impl Fixer { Ok(()) } - /// Apply multiple fixes and commit + /// Apply each issue/fix pair and commit files reported as modified. + /// + /// No commit is created when nothing changes or dry-run mode is enabled. The + /// separate `issues` slice is retained for caller compatibility and is not + /// consulted; each tuple in `fixes` supplies its associated issue. pub fn apply_and_commit( &self, _issues: &[DetectedIssue], @@ -889,12 +908,14 @@ fn resolve_from_existing_ancestor(path: &Path) -> Result { } } +/// Return whether the character at `index` follows an odd run of backslashes. fn is_escaped(value: &str, index: usize) -> bool { value[..index].bytes().rev() .take_while(|byte| *byte == b'\\') .count() % 2 == 1 } +/// Replace each escaped colon with a literal colon, preserving other backslashes. 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 19ce0702..9f1757db 100644 --- a/robot-repo-automaton/src/hypatia.rs +++ b/robot-repo-automaton/src/hypatia.rs @@ -237,7 +237,11 @@ impl CicdHyperAClient { self.load_local_ruleset(ruleset_id) } - /// Load rules from local verisimdb-data recipes directory. + /// Load a local ruleset from the first configured recipes directory that exists. + /// + /// Data-root environment variables take precedence over `REPOS_BASE`, the + /// default estate directory and legacy locations. Built-in RSR rules are used + /// when that selected directory yields no valid recipes. fn load_local_ruleset(&self, ruleset_id: &str) -> crate::Result { let mut recipes_dirs = Vec::new(); for key in ["HYPATIA_DATA", "VERISIMDB_DATA"] { @@ -259,6 +263,10 @@ impl CicdHyperAClient { self.load_recipes_from(ruleset_id, &recipes_dirs) } + /// Build a ruleset from JSON recipes in the first existing candidate directory. + /// + /// Unreadable entries and malformed or incomplete recipes are skipped. If no + /// rules remain, the built-in RSR rules are returned instead. fn load_recipes_from(&self, ruleset_id: &str, recipes_dirs: &[PathBuf]) -> crate::Result { let recipes_dir = recipes_dirs.iter().find(|d| d.is_dir()); @@ -615,7 +623,10 @@ impl CicdHyperAClient { } } -/// Convert a verisimdb-data recipe JSON to a Rule. +/// Convert a verisimdb-data recipe into a rule. +/// +/// A string `id` and either `file_glob` or `pattern` are required; recipes +/// missing those fields are ignored. 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 544c7b43..23c21668 100644 --- a/robot-repo-automaton/src/main.rs +++ b/robot-repo-automaton/src/main.rs @@ -744,9 +744,9 @@ fn cmd_catalog(path: &Path, severity_filter: Option<&str>) -> anyhow::Result<()> /// Base directory holding local repo checkouts. /// -/// Override with `REPOS_BASE`; otherwise defaults to the canonical estate tree. -/// Legacy checkout locations can also be selected through `REPOS_BASE`; the -/// former literal `/var$REPOS_DIR` path did not expand a shell variable in Rust. +/// Uses a non-empty `REPOS_BASE` value when set; otherwise returns +/// `$HOME/developer/hyper-repos`, relative to the current directory when no home +/// directory is available. fn repos_base() -> PathBuf { if let Ok(base) = std::env::var("REPOS_BASE") { if !base.is_empty() { @@ -761,7 +761,9 @@ fn repos_base() -> PathBuf { /// Resolve a repo argument to a local path. /// -/// Accepts either a local path or a GitHub owner/name format. +/// Existing paths are returned directly. Other values are resolved relative to +/// [`repos_base`], including nested `owner/name` paths, and an error lists the +/// attempted location when neither exists. fn resolve_repo_path(repo: &str) -> anyhow::Result { let path = PathBuf::from(repo); if path.exists() {