From 01ab675c152fad61e1c331cda7d0c9259f604eee Mon Sep 17 00:00:00 2001 From: CheckPickerUpper <194722943+CheckPickerUpper@users.noreply.github.com> Date: Fri, 28 Aug 2026 13:54:30 -0400 Subject: [PATCH 1/2] fix: reject ambiguous repository aliases --- src/error.rs | 14 +++ src/reportal_config/reportal_config_root.rs | 117 ++++++++++++++++++-- 2 files changed, 121 insertions(+), 10 deletions(-) diff --git a/src/error.rs b/src/error.rs index a41517d..a195a0e 100644 --- a/src/error.rs +++ b/src/error.rs @@ -37,6 +37,20 @@ pub enum ReportalError { alias: String, }, + /// A repository alias collides with another repository's + /// canonical key or declared alias. + #[error( + "Repo alias conflict: '{conflicting_value}' is declared by repo '{repository_name}' but also by {conflicting_entity_description}" + )] + RepoAliasConflict { + /// The repository declaring the conflicting alias. + repository_name: String, + /// The alias or canonical key claimed more than once. + conflicting_value: String, + /// Human-readable description of the other repository claim. + conflicting_entity_description: String, + }, + /// The editor process failed to launch. #[error("Failed to open editor: {reason}")] EditorLaunchFailure { diff --git a/src/reportal_config/reportal_config_root.rs b/src/reportal_config/reportal_config_root.rs index b506121..7210d39 100644 --- a/src/reportal_config/reportal_config_root.rs +++ b/src/reportal_config/reportal_config_root.rs @@ -79,6 +79,7 @@ impl ReportalConfig { /// default config cannot be written, `ConfigParseFailure` if /// the TOML is malformed, `WorkspaceHasDanglingRepo` if a /// workspace references an unknown repo alias, or + /// `RepoAliasConflict` if repository aliases are ambiguous, or /// `WorkspaceAliasConflict` if any workspace's name or alias /// collides with another entry. pub fn load_or_initialize() -> Result { @@ -103,8 +104,8 @@ impl ReportalConfig { Ok(parsed_config) } - /// Validates that every workspace alias is globally unambiguous - /// across both the workspace and repo namespaces. + /// Validates that repository and workspace aliases are globally + /// unambiguous across their supported namespaces. /// /// The resolver in `resolve_canonical_key` returns the first /// match it finds, so a config where two entries declare the @@ -115,9 +116,11 @@ impl ReportalConfig { /// /// # Errors /// - /// Returns [`ReportalError::WorkspaceAliasConflict`] at the - /// first collision encountered. + /// Returns [`ReportalError::RepoAliasConflict`] or + /// [`ReportalError::WorkspaceAliasConflict`] at the first + /// collision encountered. pub fn validate_alias_collisions(&self) -> Result<(), ReportalError> { + self.validate_repo_alias_collisions()?; for (workspace_name, workspace_entry) in &self.workspaces { self.check_workspace_canonical_name_repo_collision(workspace_name)?; for declared_alias in workspace_entry.aliases() { @@ -130,6 +133,58 @@ impl ReportalConfig { Ok(()) } + /// Rejects repository aliases that are repeated by another repo + /// or collide with a peer repository's canonical key. A repo may + /// list its own canonical key as a redundant alias because that + /// does not make lookup ambiguous; every other collision is + /// rejected before the resolver can depend on map ordering. + /// + /// # Errors + /// + /// Returns [`ReportalError::RepoAliasConflict`] when a declared + /// repository alias has more than one owner. + fn validate_repo_alias_collisions(&self) -> Result<(), ReportalError> { + let mut declared_alias_owners: BTreeMap = BTreeMap::new(); + for (repository_name, repository_entry) in &self.repos { + for declared_alias in repository_entry.aliases() { + self.validate_repository_alias_ownership( + repository_name, + declared_alias, + &mut declared_alias_owners, + )?; + } + } + Ok(()) + } + + /// Checks one repository alias against peer canonical keys and + /// aliases, keeping the outer validation loop shallow. + fn validate_repository_alias_ownership( + &self, + repository_name: &str, + declared_alias: &str, + declared_alias_owners: &mut BTreeMap, + ) -> Result<(), ReportalError> { + if self.repos.contains_key(declared_alias) && declared_alias != repository_name { + return Err(ReportalError::RepoAliasConflict { + repository_name: repository_name.to_owned(), + conflicting_value: declared_alias.to_owned(), + conflicting_entity_description: format!( + "repo '{declared_alias}' as its canonical key" + ), + }); + } + if let Some(previous_owner) = declared_alias_owners.get(declared_alias) { + return Err(ReportalError::RepoAliasConflict { + repository_name: repository_name.to_owned(), + conflicting_value: declared_alias.to_owned(), + conflicting_entity_description: format!("repo '{previous_owner}' as an alias"), + }); + } + declared_alias_owners.insert(declared_alias.to_owned(), repository_name.to_owned()); + Ok(()) + } + /// Rejects a workspace whose canonical name collides with a /// repo's canonical key or declared alias. /// @@ -583,12 +638,10 @@ impl ReportalConfig { repos: BTreeMap::new(), commands: BTreeMap::new(), workspaces: BTreeMap::new(), - ai_tools: BTreeMap::from([ - ( - "aider".to_owned(), - AiToolEntry::with_executable("aider".to_owned()), - ), - ]), + ai_tools: BTreeMap::from([( + "aider".to_owned(), + AiToolEntry::with_executable("aider".to_owned()), + )]), } } } @@ -777,6 +830,50 @@ mod tests { Ok(()) } + #[test] + fn duplicate_repository_aliases_are_rejected() { + let mut config = ReportalConfig::create_default(); + config + .repos + .insert("alpha".to_owned(), make_repo(vec!["shared".to_owned()])); + config + .repos + .insert("bravo".to_owned(), make_repo(vec!["shared".to_owned()])); + + let outcome = config.validate_alias_collisions(); + assert!( + outcome.is_err(), + "two repositories declaring the same alias must be rejected, got {outcome:?}", + ); + } + + #[test] + fn repository_alias_matching_peer_canonical_key_is_rejected() { + let mut config = ReportalConfig::create_default(); + config + .repos + .insert("alpha".to_owned(), make_repo(vec!["bravo".to_owned()])); + config.repos.insert("bravo".to_owned(), make_repo(vec![])); + + let outcome = config.validate_alias_collisions(); + assert!( + outcome.is_err(), + "a repository alias matching a peer canonical key must be rejected, got {outcome:?}", + ); + } + + #[test] + fn unique_repository_alias_still_resolves() -> Result<(), ReportalError> { + let mut config = ReportalConfig::create_default(); + config + .repos + .insert("alpha".to_owned(), make_repo(vec!["a".to_owned()])); + + config.validate_alias_collisions()?; + assert_eq!(config.get_repo("a")?.raw_path(), "C:/fake"); + Ok(()) + } + fn make_inline_only_workspace() -> WorkspaceEntry { WorkspaceEntry { repos: vec![WorkspaceMember::InlinePath { From fc72ee78f6df7018bb600ef1bd3ebac2dd93fcf9 Mon Sep 17 00:00:00 2001 From: CheckPickerUpper <194722943+CheckPickerUpper@users.noreply.github.com> Date: Fri, 28 Aug 2026 14:11:58 -0400 Subject: [PATCH 2/2] chore: keep alias validation diff focused --- src/reportal_config/reportal_config_root.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/reportal_config/reportal_config_root.rs b/src/reportal_config/reportal_config_root.rs index 7210d39..2d2140e 100644 --- a/src/reportal_config/reportal_config_root.rs +++ b/src/reportal_config/reportal_config_root.rs @@ -638,10 +638,12 @@ impl ReportalConfig { repos: BTreeMap::new(), commands: BTreeMap::new(), workspaces: BTreeMap::new(), - ai_tools: BTreeMap::from([( - "aider".to_owned(), - AiToolEntry::with_executable("aider".to_owned()), - )]), + ai_tools: BTreeMap::from([ + ( + "aider".to_owned(), + AiToolEntry::with_executable("aider".to_owned()), + ), + ]), } } }