diff --git a/crates/gitlawb-node/src/git/issues.rs b/crates/gitlawb-node/src/git/issues.rs index 664e441f..7fdb8a47 100644 --- a/crates/gitlawb-node/src/git/issues.rs +++ b/crates/gitlawb-node/src/git/issues.rs @@ -141,19 +141,10 @@ pub fn resolve_issue_id(repo_path: &Path, id_or_prefix: &str) -> Result Result = output.lines().filter(|l| !l.trim().is_empty()).collect(); - - match matches.len() { - 0 => Ok(None), - 1 => { - // Strip the "refs/gitlawb/issues/" prefix to get the bare ID. - let full_id = matches[0] - .trim() - .strip_prefix("refs/gitlawb/issues/") - .unwrap_or(matches[0].trim()) - .to_string(); - Ok(Some(full_id)) + let matches: Vec<&str> = output + .lines() + .map(str::trim) + .filter(|l| !l.is_empty()) + .collect(); + + // An exact id wins over longer ids that share it as a prefix, but only if its + // object exists: for-each-ref also lists a ref whose blob is missing, so a + // dangling exact ref falls through to the prefix rules below. + let exact = match matches.iter().find(|m| **m == exact_ref) { + Some(m) => { + let check = Command::new("git") + .args(["cat-file", "-e", m]) + .current_dir(repo_path) + .output() + .context("failed to run git cat-file -e")?; + check.status.success().then_some(*m) } - _ => anyhow::bail!( - "ambiguous issue prefix '{}': matches {} issues", - id_or_prefix, - matches.len() - ), - } + None => None, + }; + let found = match exact { + Some(m) => m, + None => match matches.as_slice() { + [] => return Ok(None), + [m] => *m, + _ => anyhow::bail!( + "ambiguous issue prefix '{}': matches {} issues", + id_or_prefix, + matches.len() + ), + }, + }; + // Strip the "refs/gitlawb/issues/" prefix to get the bare ID. + let full_id = found + .strip_prefix("refs/gitlawb/issues/") + .unwrap_or(found) + .to_string(); + Ok(Some(full_id)) } /// Close an issue by updating its status to "closed" in the git ref. @@ -267,6 +277,43 @@ mod tests { assert_eq!(resolved, Some(full_id.to_string())); } + #[test] + fn test_resolve_exact_id_wins_over_longer_id() { + let dir = TempDir::new().unwrap(); + init_repo(&dir); + create_issue(dir.path(), "abc12345", r#"{"status":"open"}"#).unwrap(); + create_issue(dir.path(), "abc12345-0000", r#"{"status":"open"}"#).unwrap(); + let resolved = resolve_issue_id(dir.path(), "abc12345").unwrap(); + assert_eq!(resolved, Some("abc12345".to_string())); + } + + #[test] + fn test_resolve_dangling_exact_id_does_not_win_over_longer_id() { + let dir = TempDir::new().unwrap(); + init_repo(&dir); + create_issue(dir.path(), "abc12345", r#"{"status":"open","n":1}"#).unwrap(); + create_issue(dir.path(), "abc12345-0000", r#"{"status":"open","n":2}"#).unwrap(); + // Dangle the exact ref: the ref stays listed by for-each-ref, but its blob is gone. + let out = Command::new("git") + .args(["rev-parse", "refs/gitlawb/issues/abc12345"]) + .current_dir(dir.path()) + .output() + .unwrap(); + let hash = String::from_utf8(out.stdout).unwrap(); + let hash = hash.trim(); + std::fs::remove_file( + dir.path() + .join(".git/objects") + .join(&hash[..2]) + .join(&hash[2..]), + ) + .unwrap(); + + let result = resolve_issue_id(dir.path(), "abc12345"); + assert!(result.unwrap_err().to_string().contains("ambiguous")); + assert!(close_issue(dir.path(), "abc12345").is_err()); + } + #[test] fn test_resolve_prefix_matches_unique() { let dir = TempDir::new().unwrap();