From ed305bbe00016cd4f0cc672d99b43660fe1027fb Mon Sep 17 00:00:00 2001 From: damocles Date: Wed, 29 Jul 2026 18:44:28 +0200 Subject: [PATCH] hive-forge: shell out to git for origin-remote inference instead of hand-parsing config --- Cargo.lock | 1 - hive-forge/Cargo.toml | 3 -- hive-forge/src/client.rs | 99 ++++++++-------------------------------- 3 files changed, 18 insertions(+), 85 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 99d06fef..cbe1df47 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1689,7 +1689,6 @@ dependencies = [ "reqwest 0.13.1", "serde", "serde_json", - "tempfile", "time", "url", ] diff --git a/hive-forge/Cargo.toml b/hive-forge/Cargo.toml index 23f75d32..746d6b14 100644 --- a/hive-forge/Cargo.toml +++ b/hive-forge/Cargo.toml @@ -39,8 +39,5 @@ reqwest = { workspace = true, features = [ serde = { workspace = true } serde_json = { workspace = true } -[dev-dependencies] -tempfile = "3" - [lints] workspace = true diff --git a/hive-forge/src/client.rs b/hive-forge/src/client.rs index 69f02340..eb391550 100644 --- a/hive-forge/src/client.rs +++ b/hive-forge/src/client.rs @@ -237,56 +237,25 @@ impl Client { } /// Infer `owner/name` from the `origin` remote of the git checkout -/// containing the current working directory. Walks upward from `$PWD` -/// looking for a `.git` directory (so it works from any subdirectory of -/// a checkout, not just the root), reads its `config` file for the -/// `[remote "origin"]` section's `url`, and extracts the repo path. -/// Returns `None` (never an error) on any failure — an unreadable cwd, -/// no `.git` directory found, no `origin` remote, or a URL that doesn't -/// parse into `owner/name` — so callers can fall through to the next -/// resolution step. +/// containing the current working directory. Shells out to +/// `git remote get-url origin` rather than hand-parsing `.git/config` — +/// git already knows how to find the enclosing repo from any +/// subdirectory, honours `include`/worktrees/whatever config shape is +/// actually on disk, and is one process spawn cheaper to trust than a +/// bespoke parser. Returns `None` (never an error) on any failure — cwd +/// isn't inside a repo, no `origin` remote, `git` isn't on `PATH`, or +/// the URL doesn't parse into `owner/name` — so callers fall through to +/// the next resolution step. fn infer_repo_from_cwd() -> Option { - let cwd = std::env::current_dir().ok()?; - let git_dir = find_git_dir(&cwd)?; - let config = std::fs::read_to_string(git_dir.join("config")).ok()?; - let url = origin_url(&config)?; - owner_repo_from_git_url(&url) -} - -/// Walk upward from `start` looking for a `.git` directory. Plain -/// checkouts only — a `.git` *file* (submodules, linked worktrees) -/// isn't followed, since agents don't use either shape here. -fn find_git_dir(start: &std::path::Path) -> Option { - let mut dir = start; - loop { - let candidate = dir.join(".git"); - if candidate.is_dir() { - return Some(candidate); - } - dir = dir.parent()?; + let output = std::process::Command::new("git") + .args(["remote", "get-url", "origin"]) + .output() + .ok()?; + if !output.status.success() { + return None; } -} - -/// Extract the `[remote "origin"]` section's `url` value from a git -/// config file's contents. Minimal line-based INI parsing — just -/// enough for what `git init`/`git remote add` actually write, not a -/// general config parser. -fn origin_url(config: &str) -> Option { - let mut in_origin = false; - for line in config.lines() { - let trimmed = line.trim(); - if let Some(section) = trimmed.strip_prefix('[').and_then(|s| s.strip_suffix(']')) { - in_origin = section == "remote \"origin\""; - continue; - } - if in_origin - && let Some(rest) = trimmed.strip_prefix("url") - && let Some(value) = rest.trim_start().strip_prefix('=') - { - return Some(value.trim().to_owned()); - } - } - None + let url = String::from_utf8(output.stdout).ok()?; + owner_repo_from_git_url(url.trim()) } /// Extract `owner/name` from a git remote URL. Handles the shapes this @@ -478,7 +447,7 @@ fn check_status(resp: Response, op: &str) -> Result { #[cfg(test)] mod tests { - use super::{find_git_dir, origin_url, owner_repo_from_git_url, split_repo}; + use super::{owner_repo_from_git_url, split_repo}; #[test] fn owner_repo_from_git_url_strips_scheme_and_creds() { @@ -519,38 +488,6 @@ mod tests { assert_eq!(owner_repo_from_git_url("https://forge.example"), None); } - #[test] - fn origin_url_finds_url_in_origin_section_only() { - let config = "[core]\n\trepositoryformatversion = 0\n[remote \"upstream\"]\n\turl = https://wrong/repo.git\n[remote \"origin\"]\n\turl = https://forge.pr1ma.darkest.space/hyperhive/hyperhive.git\n\tfetch = +refs/heads/*:refs/remotes/origin/*\n"; - assert_eq!( - origin_url(config), - Some("https://forge.pr1ma.darkest.space/hyperhive/hyperhive.git".to_owned()) - ); - } - - #[test] - fn origin_url_none_without_origin_remote() { - let config = "[core]\n\trepositoryformatversion = 0\n[remote \"upstream\"]\n\turl = https://wrong/repo.git\n"; - assert_eq!(origin_url(config), None); - } - - #[test] - fn find_git_dir_walks_up_from_subdirectory() { - let root = tempfile::tempdir().unwrap(); - std::fs::create_dir(root.path().join(".git")).unwrap(); - let nested = root.path().join("a").join("b"); - std::fs::create_dir_all(&nested).unwrap(); - assert_eq!(find_git_dir(&nested), Some(root.path().join(".git"))); - } - - #[test] - fn find_git_dir_none_when_no_ancestor_has_one() { - let root = tempfile::tempdir().unwrap(); - let nested = root.path().join("a"); - std::fs::create_dir_all(&nested).unwrap(); - assert_eq!(find_git_dir(&nested), None); - } - #[test] fn split_repo_accepts_owner_name() { assert_eq!(