feat: infer repo from git remote (origin) when not passed#235
feat: infer repo from git remote (origin) when not passed#235sachiniyer merged 6 commits intomainfrom
Conversation
Make the repo positional argument optional across all commands that accept it (bugs list, scans list, rules create/propose/list, rules requests list). When omitted, the CLI checks git remotes in order: upstream, then origin. If a recognisable GitHub URL is found, the owner/repo slug is extracted automatically. If neither a repo argument nor a usable git remote is available, a clear error message tells the user to pass the repo explicitly. Co-Authored-By: Sachin Iyer <siyer@detail.dev>
Original prompt from Sachin
|
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Co-Authored-By: Sachin Iyer <siyer@detail.dev>
| pub fn infer_repo_from_git_remote() -> Result<String> { | ||
| // Make sure we are inside a git work-tree. | ||
| let in_git = Command::new("git") | ||
| .args(["rev-parse", "--is-inside-work-tree"]) | ||
| .output() | ||
| .context("Failed to run git")?; | ||
|
|
||
| if !in_git.status.success() { | ||
| bail!( | ||
| "Not inside a git repository. Please pass a repo argument explicitly \ | ||
| (e.g. owner/repo)." | ||
| ); | ||
| } | ||
|
|
||
| // Try upstream first, then origin. | ||
| for remote in &["upstream", "origin"] { | ||
| if let Some(url) = get_remote_url(remote) { | ||
| if let Some(owner_repo) = parse_github_remote_url(&url) { | ||
| return Ok(owner_repo); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| bail!( | ||
| "Could not infer repository from git remotes. \ | ||
| Please pass a repo argument explicitly (e.g. owner/repo)." | ||
| ) | ||
| } |
There was a problem hiding this comment.
📝 Info: Synchronous process spawn inside async context
infer_repo_from_git_remote() at src/utils/git.rs:30-32 uses std::process::Command::output() which blocks the current thread. This is called from async handler functions (e.g., src/commands/bugs.rs:285). For a single-threaded CLI application this is acceptable and not a bug, but if the runtime were ever changed to multi-threaded with concurrent tasks, this could starve the executor. For now this is fine given the CLI's architecture.
Was this helpful? React with 👍 or 👎 to provide feedback.
| pub fn resolve_repo_arg(explicit: Option<&str>) -> Result<String> { | ||
| explicit.map_or_else(infer_repo_from_git_remote, |r| Ok(r.to_string())) | ||
| } |
There was a problem hiding this comment.
📝 Info: Consistent transformation across all affected commands
Verified that all commands that previously took a required repo: String positional argument have been updated to repo: Option<String> with the resolve_repo_arg call pattern. The transformation is applied consistently in: bugs list (src/commands/bugs.rs:82,285), rules create (src/commands/rules.rs:24,121), rules propose (src/commands/rules.rs:43,256), rules list (src/commands/rules.rs:54,149), rules requests list (src/commands/rules.rs:84,278), and scans list (src/commands/scans.rs:15,41). Commands that don't take a repo argument (e.g. bugs show, bugs close, rules show, rules persist, rules requests show) are correctly left unchanged.
Was this helpful? React with 👍 or 👎 to provide feedback.
| fn parse_github_remote_url(url: &str) -> Option<String> { | ||
| // HTTPS: https://github.com/owner/repo or https://github.com/owner/repo.git | ||
| if let Some(rest) = url | ||
| .strip_prefix("https://github.com/") | ||
| .or_else(|| url.strip_prefix("http://github.com/")) | ||
| { | ||
| let rest = rest.trim_end_matches(".git").trim_end_matches('/'); | ||
| let parts: Vec<&str> = rest.splitn(3, '/').collect(); | ||
| if parts.len() >= 2 && !parts[0].is_empty() && !parts[1].is_empty() { | ||
| return Some(format!("{}/{}", parts[0], parts[1])); | ||
| } | ||
| } | ||
|
|
||
| // SSH: git@github.com:owner/repo.git | ||
| if let Some(rest) = url.strip_prefix("git@github.com:") { | ||
| let rest = rest.trim_end_matches(".git").trim_end_matches('/'); | ||
| let parts: Vec<&str> = rest.splitn(3, '/').collect(); | ||
| if parts.len() >= 2 && !parts[0].is_empty() && !parts[1].is_empty() { | ||
| return Some(format!("{}/{}", parts[0], parts[1])); | ||
| } | ||
| } | ||
|
|
||
| // SSH with ssh:// scheme: ssh://git@github.com/owner/repo.git | ||
| if let Some(rest) = url.strip_prefix("ssh://git@github.com/") { | ||
| let rest = rest.trim_end_matches(".git").trim_end_matches('/'); | ||
| let parts: Vec<&str> = rest.splitn(3, '/').collect(); | ||
| if parts.len() >= 2 && !parts[0].is_empty() && !parts[1].is_empty() { | ||
| return Some(format!("{}/{}", parts[0], parts[1])); | ||
| } | ||
| } | ||
|
|
||
| None |
There was a problem hiding this comment.
📝 Info: GitHub-only URL parsing may surprise users with other Git hosts
The parse_github_remote_url function only recognizes GitHub URLs (github.com). If a user's upstream or origin remote points to GitLab, Bitbucket, or a self-hosted GitHub Enterprise instance (e.g. github.mycompany.com), the inference silently fails and falls through to the error message at src/utils/git.rs:89-92. The error message says "Could not infer repository from git remotes" which is reasonable, but doesn't hint that only GitHub URLs are supported. This is a design choice rather than a bug, but could cause confusion for GitHub Enterprise users whose URLs differ only in the hostname.
Was this helpful? React with 👍 or 👎 to provide feedback.
Co-Authored-By: Sachin Iyer <siyer@detail.dev>
| .strip_prefix("https://github.com/") | ||
| .or_else(|| url.strip_prefix("http://github.com/")) | ||
| { | ||
| let rest = rest.trim_end_matches('/').trim_end_matches(".git"); |
There was a problem hiding this comment.
📝 Info: trim_end_matches order handles .git/ URLs correctly
The order trim_end_matches('/').trim_end_matches(".git") at src/utils/git.rs:18 is intentional — commit a4c8c9b explicitly swapped the order to handle URLs ending in .git/. First stripping trailing slashes then stripping .git correctly normalizes repo.git/ → repo.git → repo. The reverse order would fail on .git/ since .git wouldn't be a suffix when / is still present. Note that trim_end_matches with a &str pattern removes repeatedly, so repo.git.git → repo, which is benign for real-world URLs.
Was this helpful? React with 👍 or 👎 to provide feedback.
…:then Co-Authored-By: Sachin Iyer <siyer@detail.dev>
| const PREFIXES: &[&str] = &[ | ||
| "https://github.com/", | ||
| "http://github.com/", | ||
| "git@github.com:", | ||
| "ssh://git@github.com/", | ||
| ]; |
There was a problem hiding this comment.
📝 Info: SSH URLs with non-standard ports are not handled
The parse_github_remote_url function matches against a fixed set of prefixes (src/utils/git.rs:10-15). An SSH URL with an explicit port like ssh://git@github.com:22/owner/repo.git would not match the prefix ssh://git@github.com/ due to the :22 port segment. This is a reasonable limitation since GitHub doesn't use non-standard ports, and the error message from infer_repo_from_git_remote clearly directs the user to pass the repo argument explicitly. Not a bug, just an edge case to be aware of.
Was this helpful? React with 👍 or 👎 to provide feedback.
Testing ResultsTested the CLI locally after building from the feature branch. Devin session Results (4/5 passed, 1 untestable)
Test 1 outputTest 3 output (from /tmp)Test 4 output (from CLI repo with proxy remote)Test 5 output |
Updated Testing Results — All 5 tests passedRe-tested inference happy path by cloning a fresh copy and using a Results (5/5 passed)
Test 2 output (inference happy path — no repo argument passed) |
Co-Authored-By: Sachin Iyer <siyer@detail.dev>
| if let Some(url) = get_remote_url("origin") { | ||
| if let Some(owner_repo) = parse_github_remote_url(&url) { | ||
| return Ok(owner_repo); | ||
| } | ||
| } |
There was a problem hiding this comment.
📝 Info: Only the 'origin' remote is checked for inference
The infer_repo_from_git_remote function at src/utils/git.rs:62 only checks the origin remote. If a user's GitHub remote is configured under a different name (e.g., upstream, github), inference will fail with a generic error. This is a reasonable default — origin is by far the most common convention — and the error message clearly instructs the user to pass the argument explicitly. Worth noting in documentation if users report confusion.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const PREFIXES: &[&str] = &[ | ||
| "https://github.com/", | ||
| "http://github.com/", | ||
| "git@github.com:", | ||
| "ssh://git@github.com/", | ||
| ]; | ||
|
|
||
| let rest = PREFIXES.iter().find_map(|p| url.strip_prefix(p))?; |
There was a problem hiding this comment.
📝 Info: URL prefix matching is case-sensitive
The prefix list at src/utils/git.rs:10-15 uses case-sensitive matching via strip_prefix. A URL like https://GitHub.com/owner/repo would not match. In practice, git normalizes remote URLs to lowercase hostnames, so this is unlikely to be an issue. If it ever becomes one, the fix would be to lowercase the URL before matching.
Was this helpful? React with 👍 or 👎 to provide feedback.
Co-Authored-By: Sachin Iyer <siyer@detail.dev>
## Summary Makes the `repo` positional argument **optional** across all commands that accept it (`bugs list`, `scans list`, `rules create/propose/list`, `rules requests list`). When omitted, the CLI infers `owner/repo` from the current git repository's `origin` remote. If `origin` does not exist or does not point to a recognised GitHub URL, the CLI errors with a message telling the user to pass the repo explicitly. New module: `src/utils/git.rs` with: - `parse_github_remote_url` — extracts `owner/repo` from HTTPS, SSH-colon, and `ssh://` GitHub URLs (uses a prefix list + `find_map` + `bool::then`) - `infer_repo_from_git_remote` — runs `git remote get-url origin` and parses the result - `resolve_repo_arg` — bridge: uses explicit arg if provided, otherwise infers 15 unit tests cover URL parsing edge cases (`.git` suffix, trailing slashes, non-GitHub URLs, malformed input, extra path segments). ## Review & Testing Checklist for Human - [ ] **Optional positional arg behaviour in clap**: `repo` is now `Option<String>` as a positional (no `#[arg(long)]`). Confirm that flag arguments (e.g. `--status`, `--limit`) are not accidentally consumed as the repo positional when repo is omitted. Try: `detail bugs list --status pending --limit 5` (no repo). - [ ] **Error message clarity**: All inference failures (not in a git repo, no origin remote, non-GitHub origin) now produce the same error: *"Could not infer repository from git remotes. Please pass a repo argument explicitly (e.g. owner/repo)."* Verify this is acceptable vs. having distinct messages. - [ ] **Manual end-to-end test**: `cd` into a repo with a GitHub origin, run `detail bugs list` (no repo arg) and confirm it infers correctly. Then run from a non-git directory and confirm the error message appears. ### Notes - Only the `origin` remote is checked. This matches the most common setup (`git clone` creates `origin`). - GitHub-only: non-GitHub remotes (GitLab, Bitbucket, self-hosted) will fall through to the error. This is intentional. - The `is_silent` pattern matching in `lib.rs` uses `..` wildcards, so no changes were needed there despite the type change. - URL prefix matching is case-sensitive; git normalises remote URLs to lowercase hostnames so this is unlikely to be an issue in practice. - `git remote get-url origin` is called via `std::process::Command` (blocking) on the tokio runtime thread. This is a fast local operation. Link to Devin session: https://app.devin.ai/sessions/9cd8d459be13437587ce77146aa4ce17 Requested by: @sachiniyer --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: Sachin Iyer <siyer@detail.dev>
Summary
Makes the
repopositional argument optional across all commands that accept it (bugs list,scans list,rules create/propose/list,rules requests list). When omitted, the CLI infersowner/repofrom the current git repository'soriginremote.If
origindoes not exist or does not point to a recognised GitHub URL, the CLI errors with a message telling the user to pass the repo explicitly.New module:
src/utils/git.rswith:parse_github_remote_url— extractsowner/repofrom HTTPS, SSH-colon, andssh://GitHub URLs (uses a prefix list +find_map+bool::then)infer_repo_from_git_remote— runsgit remote get-url originand parses the resultresolve_repo_arg— bridge: uses explicit arg if provided, otherwise infers15 unit tests cover URL parsing edge cases (
.gitsuffix, trailing slashes, non-GitHub URLs, malformed input, extra path segments).Review & Testing Checklist for Human
repois nowOption<String>as a positional (no#[arg(long)]). Confirm that flag arguments (e.g.--status,--limit) are not accidentally consumed as the repo positional when repo is omitted. Try:detail bugs list --status pending --limit 5(no repo).cdinto a repo with a GitHub origin, rundetail bugs list(no repo arg) and confirm it infers correctly. Then run from a non-git directory and confirm the error message appears.Notes
originremote is checked. This matches the most common setup (git clonecreatesorigin).is_silentpattern matching inlib.rsuses..wildcards, so no changes were needed there despite the type change.git remote get-url originis called viastd::process::Command(blocking) on the tokio runtime thread. This is a fast local operation.Link to Devin session: https://app.devin.ai/sessions/9cd8d459be13437587ce77146aa4ce17
Requested by: @sachiniyer