From 817a47b62fe5c863c0109146021d288e89a4afaa Mon Sep 17 00:00:00 2001 From: Jakob Stender Guldberg Date: Fri, 28 Aug 2026 07:02:23 +0200 Subject: [PATCH 1/5] feat(pr-url): accept pull/merge request URLs in the repository field The repository path field (desktop app, web mode) and `diffcore analyze --repo` now take a PR/MR URL. Diffcore clones into ~/.diffcore/cache/repos/// (DIFFCORE_REPO_CACHE_DIR overrides), fetches the provider's pull-request ref namespace, and resolves to a base..head pair matching the provider's "Files changed" view. Supported via git refs alone, no API token: GitHub.com/GHE, GitLab SaaS/self-managed, Gitea/Forgejo/Codeberg/Gitee/Gogs, Bitbucket Data Center, Azure DevOps (Services/visualstudio.com/Server), Gerrit and SourceForge. Bitbucket Cloud, Launchpad, CodeCommit, Phabricator, SourceHut and Radicle publish no PR refs; their URLs parse but fail with a message naming the provider instead of a generic parse error. Detection keys off URL shape rather than hostname, so self-hosted instances work on any host, port or context path. Clone and fetch shell out to the git CLI: libgit2 is built here without network transports, and the CLI inherits the user's credential helper and SSH agent for private repositories. Base is reported as the fork point so a two-dot base..head diff equals the provider's view. Providers drop the merge ref once a PR lands, so for merged PRs the pre-merge tip is recovered from the merge commit that landed it; squash- and rebase-merged PRs leave no such commit and remain unresolvable without a provider API. See docs/pr-url-providers.md for the full provider table. --- Cargo.lock | 1 + README.md | 7 + crates/diffcore-cli/src/main.rs | 36 +- crates/diffcore-core/Cargo.toml | 1 + crates/diffcore-core/src/lib.rs | 1 + crates/diffcore-core/src/pr_url.rs | 904 ++++++++++++++++++++++++ crates/diffcore-tauri/src/commands.rs | 13 + crates/diffcore-tauri/src/main.rs | 1 + crates/diffcore-tauri/src/web_server.rs | 1 + crates/diffcore-tauri/ui/src/App.tsx | 61 +- crates/diffcore-tauri/ui/src/types.ts | 12 + docs/pr-url-providers.md | 85 +++ 12 files changed, 1109 insertions(+), 14 deletions(-) create mode 100644 crates/diffcore-core/src/pr_url.rs create mode 100644 docs/pr-url-providers.md diff --git a/Cargo.lock b/Cargo.lock index 70a21b4..e12eb57 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1265,6 +1265,7 @@ dependencies = [ "tree-sitter-scala", "tree-sitter-swift", "tree-sitter-typescript", + "url", "wiremock", ] diff --git a/README.md b/README.md index aa53625..fbdb21e 100644 --- a/README.md +++ b/README.md @@ -108,6 +108,13 @@ diffcore analyze --base main --refine --refine-model gpt-4o # Analyze a different repo diffcore analyze --base main --repo /path/to/repo +# Analyze a pull/merge request by URL — clones into ~/.diffcore/cache/repos and +# resolves base/head from the provider's PR refs. Works for GitHub, GitLab, +# Gitea/Forgejo, Bitbucket Data Center, Azure DevOps and Gerrit; see +# docs/pr-url-providers.md for the full list. +diffcore analyze --repo https://github.com/BurntSushi/ripgrep/pull/2900 +diffcore analyze --repo https://gitlab.com/gitlab-org/cli/-/merge_requests/976 + # Open a flow group in an external diff tool diffcore launch --tool bcompare --group group_1 --input review.json ``` diff --git a/crates/diffcore-cli/src/main.rs b/crates/diffcore-cli/src/main.rs index b3c2583..adf7795 100644 --- a/crates/diffcore-cli/src/main.rs +++ b/crates/diffcore-cli/src/main.rs @@ -26,6 +26,7 @@ use diffcore_core::llm; use diffcore_core::llm::refinement; use diffcore_core::output::{self, build_analysis_output}; use diffcore_core::pipeline; +use diffcore_core::pr_url; use diffcore_core::rank; use diffcore_core::types::AnalysisOutput; @@ -111,7 +112,9 @@ struct AnalyzeArgs { #[arg(long)] no_cache: bool, - /// Path to the git repository (defaults to current directory) + /// Path to the git repository, or a pull/merge request URL + /// (GitHub, GitLab, Gitea/Forgejo, Bitbucket DC, Azure DevOps, Gerrit). + /// URLs are cloned into ~/.diffcore/cache/repos and resolved to base/head refs. #[arg(long, default_value = ".")] repo: PathBuf, } @@ -316,8 +319,34 @@ fn main() { } } +/// When `--repo` is a pull/merge request URL, clone (or reuse) the repository in +/// the diffcore cache, fetch the PR refs, and rewrite `--repo`/`--base`/`--head` +/// to point at the resolved local checkout. +fn resolve_pr_url_args(args: &mut AnalyzeArgs) -> Result<(), Box> { + let Some(pr) = args.repo.to_str().and_then(pr_url::parse) else { + return Ok(()); + }; + info!( + "Resolving {} #{} on {} ({})", + pr.provider.unit(), + pr.number, + pr.host, + pr.provider.name() + ); + let resolved = pr_url::resolve(&pr)?; + info!("Using cached checkout at {}", resolved.path); + args.repo = PathBuf::from(&resolved.path); + args.base.get_or_insert(resolved.base); + args.head.get_or_insert(resolved.head); + // The checkout is detached at the PR head; never mix in working-tree state. + args.include_uncommitted = false; + args.no_include_uncommitted = true; + Ok(()) +} + /// Run analysis and return the output (without writing or LLM steps). -fn run_analyze_and_return(args: AnalyzeArgs) -> Result> { +fn run_analyze_and_return(mut args: AnalyzeArgs) -> Result> { + resolve_pr_url_args(&mut args)?; let repo_path = std::fs::canonicalize(&args.repo)?; let repo = Repository::discover(&repo_path).map_err(|e| format!("Not a git repository: {}", e))?; @@ -404,7 +433,8 @@ fn run_analyze_and_return(args: AnalyzeArgs) -> Result Result<(), Box> { +fn run_analyze(mut args: AnalyzeArgs) -> Result<(), Box> { + resolve_pr_url_args(&mut args)?; // Resolve repo path let repo_path = std::fs::canonicalize(&args.repo)?; let repo = diff --git a/crates/diffcore-core/Cargo.toml b/crates/diffcore-core/Cargo.toml index 537ab18..f2cbc75 100644 --- a/crates/diffcore-core/Cargo.toml +++ b/crates/diffcore-core/Cargo.toml @@ -17,6 +17,7 @@ tree-sitter-rust = "0.24.1" tree-sitter-java = "0.23.5" toml = "0.8" glob = "0.3" +url = "2" reqwest = { version = "0.12", default-features = false, features = ["json", "http2", "charset", "rustls-tls-native-roots", "macos-system-configuration"] } tokio = { version = "1", features = ["full"] } async-trait = "0.1" diff --git a/crates/diffcore-core/src/lib.rs b/crates/diffcore-core/src/lib.rs index 1be85a5..3fecfb1 100644 --- a/crates/diffcore-core/src/lib.rs +++ b/crates/diffcore-core/src/lib.rs @@ -23,6 +23,7 @@ pub mod llm; pub mod logging; pub mod output; pub mod pipeline; +pub mod pr_url; pub mod query_engine; pub mod manifest; pub mod rank; diff --git a/crates/diffcore-core/src/pr_url.rs b/crates/diffcore-core/src/pr_url.rs new file mode 100644 index 0000000..2975823 --- /dev/null +++ b/crates/diffcore-core/src/pr_url.rs @@ -0,0 +1,904 @@ +//! Parse pull-request / merge-request URLs and resolve them to local git refs. +//! +//! The desktop app's repository field and the CLI's `--repo` flag both accept a +//! PR/MR URL in place of a filesystem path. Resolution clones (or reuses) a +//! cached mirror under `~/.diffcore/cache/repos/` and fetches the provider's +//! pull-request ref namespace, so no API token is needed for public repos. +//! +//! Provider ref namespaces (see `docs/pr-url-providers.md` for the full table): +//! +//! | Provider | web URL | git refs | +//! |-------------------------|------------------------------------------------------|---------------------------------| +//! | GitHub / GHE | `/{owner}/{repo}/pull/{n}` | `refs/pull/{n}/{head,merge}` | +//! | GitLab (SaaS + self) | `/{ns...}/{repo}/-/merge_requests/{n}` | `refs/merge-requests/{n}/{head,merge}` | +//! | Gitea/Forgejo/Codeberg/Gitea-likes | `/{owner}/{repo}/pulls/{n}` | `refs/pull/{n}/head` | +//! | Bitbucket Data Center | `/projects/{KEY}/repos/{repo}/pull-requests/{n}` | `refs/pull-requests/{n}/{from,merge}` | +//! | Azure DevOps | `/{org}/{project}/_git/{repo}/pullrequest/{n}` | `refs/pull/{n}/{merge,head}` | +//! | Gerrit | `/c/{project}/+/{change}[/{patchset}]` | `refs/changes/{nn}/{change}/{ps}` | +//! | SourceForge (Allura) | `/p/{project}/{repo}/merge-requests/{n}` | `refs/merge-requests/{n}/head` | +//! | Bitbucket Cloud | `/{workspace}/{repo}/pull-requests/{n}` | none — API only | +//! | Launchpad | `/~{user}/{proj}/+git/{repo}/+merge/{n}` | none — API only | + +use std::path::{Path, PathBuf}; +use std::process::Command; + +use url::Url; + +#[derive(Debug, thiserror::Error)] +pub enum PrUrlError { + #[error("not a recognised pull/merge request URL: {0}")] + Unrecognised(String), + #[error( + "{0} does not publish pull-request refs over git — clone the repository and pick the source/target branches manually" + )] + NoGitRefs(&'static str), + #[error("{0} #{1} not found on {2} (private repo? try `git ls-remote {2}`)")] + NotFound(&'static str, u64, String), + #[error("`git {0}` failed: {1}")] + Git(String, String), + #[error("cannot locate a cache directory — set DIFFCORE_REPO_CACHE_DIR or HOME")] + NoCacheDir, + #[error("io error running git: {0}")] + Io(#[from] std::io::Error), +} + +/// Forge families that share a pull-request URL shape and ref namespace. +#[derive(Debug, Clone, Copy, PartialEq, Eq, serde::Serialize)] +pub enum Provider { + /// github.com and GitHub Enterprise Server. + #[serde(rename = "github")] + GitHub, + /// gitlab.com and self-managed GitLab (CE/EE). + #[serde(rename = "gitlab")] + GitLab, + /// Gitea, Forgejo, Codeberg, Gogs, Gitee — same `/pulls/{n}` shape. + #[serde(rename = "gitea")] + Gitea, + /// Bitbucket Data Center / Server (on-prem). + #[serde(rename = "bitbucket-server")] + BitbucketServer, + /// bitbucket.org (Cloud) — no PR refs over git. + #[serde(rename = "bitbucket-cloud")] + BitbucketCloud, + /// Azure DevOps Services and Azure DevOps Server / TFS. + #[serde(rename = "azure-devops")] + AzureDevOps, + /// Gerrit changes (the review unit, equivalent to a PR). + #[serde(rename = "gerrit")] + Gerrit, + /// SourceForge / Apache Allura merge requests. + #[serde(rename = "sourceforge")] + SourceForge, + /// Launchpad merge proposals — no PR refs over git. + #[serde(rename = "launchpad")] + Launchpad, +} + +impl Provider { + pub fn name(self) -> &'static str { + match self { + Provider::GitHub => "GitHub", + Provider::GitLab => "GitLab", + Provider::Gitea => "Gitea/Forgejo", + Provider::BitbucketServer => "Bitbucket Data Center", + Provider::BitbucketCloud => "Bitbucket Cloud", + Provider::AzureDevOps => "Azure DevOps", + Provider::Gerrit => "Gerrit", + Provider::SourceForge => "SourceForge", + Provider::Launchpad => "Launchpad", + } + } + + /// What the provider calls a change request, for user-facing messages. + pub fn unit(self) -> &'static str { + match self { + Provider::GitLab | Provider::SourceForge => "merge request", + Provider::Gerrit => "change", + Provider::Launchpad => "merge proposal", + _ => "pull request", + } + } +} + +/// A parsed pull/merge request URL. +#[derive(Debug, Clone, PartialEq, Eq, serde::Serialize)] +pub struct PrUrl { + pub provider: Provider, + /// Host, including port when non-default (e.g. `gitea.internal:3000`). + pub host: String, + /// Namespace/owner path (`rust-lang`, `gitlab-org/security`, `myorg/myproject`). + pub owner: String, + pub repo: String, + /// PR / MR / change number. + pub number: u64, + /// Gerrit patchset, when pinned in the URL. + pub patchset: Option, + /// HTTPS clone URL for the repository. + pub clone_url: String, +} + +/// Parse a PR/MR URL. Returns `None` for anything that is not an http(s) URL +/// matching a known forge layout — callers treat that as a filesystem path. +pub fn parse(input: &str) -> Option { + let raw = input.trim().trim_end_matches('/'); + if !(raw.starts_with("http://") || raw.starts_with("https://")) { + return None; + } + let mut url = Url::parse(raw).ok()?; + url.set_query(None); + url.set_fragment(None); + + let scheme = url.scheme().to_string(); + let mut host = url.host_str()?.to_ascii_lowercase(); + if let Some(port) = url.port() { + host = format!("{host}:{port}"); + } + let segs: Vec = url + .path_segments()? + .filter(|s| !s.is_empty()) + .map(|s| s.to_string()) + .collect(); + if segs.is_empty() { + return None; + } + + // api.github.com/repos/{owner}/{repo}/pulls/{n} — rewrite to the web shape. + let (host, segs) = if host == "api.github.com" && segs.first().map(String::as_str) == Some("repos") + { + ("github.com".to_string(), segs[1..].to_vec()) + } else { + (host, segs) + }; + + let s: Vec<&str> = segs.iter().map(String::as_str).collect(); + let base = format!("{scheme}://{host}"); + + // Azure DevOps: .../{org}/{project}/_git/{repo}/pullrequest/{n} + if let Some(i) = s.iter().position(|x| *x == "_git") { + let repo = s.get(i + 1)?; + if s.get(i + 2).copied() != Some("pullrequest") { + return None; + } + let number = parse_number(s.get(i + 3)?)?; + let owner = s[..i].join("/"); + if owner.is_empty() { + return None; + } + return Some(PrUrl { + provider: Provider::AzureDevOps, + clone_url: format!("{base}/{owner}/_git/{repo}"), + host, + owner, + repo: (*repo).to_string(), + number, + patchset: None, + }); + } + + // Bitbucket Data Center: [ctx]/projects/{KEY}/repos/{repo}/pull-requests/{n} + if let Some(i) = s.iter().position(|x| *x == "projects") { + if s.get(i + 2).copied() == Some("repos") + && s.get(i + 4).copied() == Some("pull-requests") + { + let key = s.get(i + 1)?; + let repo = s.get(i + 3)?; + let number = parse_number(s.get(i + 5)?)?; + let ctx = s[..i].join("/"); + let prefix = if ctx.is_empty() { + base.clone() + } else { + format!("{base}/{ctx}") + }; + return Some(PrUrl { + provider: Provider::BitbucketServer, + clone_url: format!("{prefix}/scm/{key}/{repo}.git"), + host, + owner: (*key).to_string(), + repo: (*repo).to_string(), + number, + patchset: None, + }); + } + } + + // Gerrit: /c/{project...}/+/{change}[/{patchset}] + if s.first().copied() == Some("c") { + if let Some(i) = s.iter().position(|x| *x == "+") { + let project = s[1..i].join("/"); + let number = parse_number(s.get(i + 1)?)?; + let patchset = s.get(i + 2).and_then(|p| parse_number(p)); + let repo = project.rsplit('/').next().unwrap_or(&project).to_string(); + let owner = project + .rsplit_once('/') + .map(|(o, _)| o.to_string()) + .unwrap_or_default(); + return Some(PrUrl { + provider: Provider::Gerrit, + clone_url: format!("{base}/{project}"), + host, + owner, + repo, + number, + patchset, + }); + } + } + + // SourceForge / Allura: /p/{project}/{repo}/merge-requests/{n} + if s.first().copied() == Some("p") && s.get(3).copied() == Some("merge-requests") { + let project = s.get(1)?; + let repo = s.get(2)?; + let number = parse_number(s.get(4)?)?; + return Some(PrUrl { + provider: Provider::SourceForge, + clone_url: format!("https://git.code.sf.net/p/{project}/{repo}"), + host, + owner: (*project).to_string(), + repo: (*repo).to_string(), + number, + patchset: None, + }); + } + + // Launchpad: /~{user}/{project}/+git/{repo}/+merge/{n} + if let Some(i) = s.iter().position(|x| *x == "+merge") { + let number = parse_number(s.get(i + 1)?)?; + let repo = s.get(i.checked_sub(1)?)?; + let owner = s[..i - 1].join("/"); + return Some(PrUrl { + provider: Provider::Launchpad, + clone_url: format!("https://git.launchpad.net/{}", s[..i].join("/")), + host, + owner, + repo: (*repo).to_string(), + number, + patchset: None, + }); + } + + // Remaining forges all share `{namespace...}/{repo}//{n}`. + let (provider, marker) = [ + (Provider::BitbucketCloud, "pull-requests"), + (Provider::GitLab, "merge_requests"), + (Provider::Gitea, "pulls"), + (Provider::GitHub, "pull"), + ] + .into_iter() + .find(|(_, m)| s.contains(m))?; + + // `api.github.com/repos/{o}/{r}/pulls/{n}` shares Gitea's `pulls` marker. + let provider = match (provider, host.as_str()) { + (Provider::Gitea, "github.com") => Provider::GitHub, + (p, _) => p, + }; + + let i = s.iter().position(|x| *x == marker)?; + let number = parse_number(s.get(i + 1)?)?; + // GitLab inserts a `/-/` separator between the project path and the route. + let path_end = if i > 0 && s[i - 1] == "-" { i - 1 } else { i }; + if path_end < 2 { + return None; + } + let repo = s[path_end - 1]; + let owner = s[..path_end - 1].join("/"); + + Some(PrUrl { + provider, + clone_url: format!("{base}/{owner}/{repo}.git"), + host, + owner, + repo: repo.to_string(), + number, + patchset: None, + }) +} + +/// Strip a `.diff` / `.patch` suffix and parse the remainder as a number. +fn parse_number(seg: &str) -> Option { + seg.trim_end_matches(".diff") + .trim_end_matches(".patch") + .parse() + .ok() +} + +impl PrUrl { + /// `git ls-remote` glob covering every ref the provider publishes for this PR. + /// + /// `None` means the provider exposes no PR refs over git at all. + pub fn ref_glob(&self) -> Option { + let n = self.number; + Some(match self.provider { + Provider::GitHub | Provider::Gitea | Provider::AzureDevOps => { + format!("refs/pull/{n}/*") + } + Provider::GitLab => format!("refs/merge-requests/{n}/*"), + Provider::BitbucketServer => format!("refs/pull-requests/{n}/*"), + Provider::SourceForge => format!("refs/merge-requests/{n}/*"), + // refs/changes/{last two digits, zero padded}/{change}/{patchset} + Provider::Gerrit => format!("refs/changes/{:02}/{n}/*", n % 100), + Provider::BitbucketCloud | Provider::Launchpad => return None, + }) + } + + /// Pick the head ref (the PR's tip) and, when published, the merge ref + /// (whose first parent is the target branch at merge-preview time). + fn pick_refs(&self, refs: &[String]) -> Option<(String, Option)> { + if self.provider == Provider::Gerrit { + // Highest patchset wins unless the URL pinned one. + let want = self.patchset.map(|p| format!("/{p}")); + let head = match &want { + Some(suffix) => refs.iter().find(|r| r.ends_with(suffix))?.clone(), + None => refs + .iter() + .max_by_key(|r| { + r.rsplit('/').next().and_then(|p| p.parse::().ok()).unwrap_or(0) + })? + .clone(), + }; + return Some((head, None)); + } + let head = refs + .iter() + .find(|r| r.ends_with("/head") || r.ends_with("/from"))? + .clone(); + let merge = refs.iter().find(|r| r.ends_with("/merge")).cloned(); + Some((head, merge)) + } + + /// Directory this repo is cached in. + fn cache_dir(&self) -> Result { + let root = std::env::var_os("DIFFCORE_REPO_CACHE_DIR") + .map(PathBuf::from) + .or_else(|| { + std::env::var_os("HOME") + .map(|h| PathBuf::from(h).join(".diffcore").join("cache").join("repos")) + }) + .ok_or(PrUrlError::NoCacheDir)?; + Ok(root + .join(slug(&self.host)) + .join(slug(&self.owner)) + .join(slug(&self.repo))) + } +} + +/// Filesystem-safe path component. +fn slug(s: &str) -> String { + s.chars() + .map(|c| if c.is_ascii_alphanumeric() || c == '.' || c == '-' || c == '_' { c } else { '-' }) + .collect() +} + +/// A PR URL resolved to a usable local repository + revision pair. +#[derive(Debug, Clone, serde::Serialize)] +pub struct ResolvedPr { + /// Local working tree to analyze. + pub path: String, + /// Base revision — the merge base (fork point) of the PR against its target + /// branch, so `base..head` matches the provider's "Files changed" view. + pub base: String, + /// Head revision — a local branch `pr-{n}` at the PR's tip commit. HEAD in + /// the checkout stays detached, so re-resolving the same PR can update it. + pub head: String, + pub provider: Provider, + pub number: u64, +} + +/// Clone (or reuse) the repository behind `pr` and fetch its PR refs. +/// +/// Uses the `git` CLI rather than git2 because libgit2 is built here without +/// network transports, and because shelling out inherits the user's existing +/// credential helpers and SSH agent for private repositories. +pub fn resolve(pr: &PrUrl) -> Result { + let glob = pr + .ref_glob() + .ok_or(PrUrlError::NoGitRefs(pr.provider.name()))?; + + let listing = git(Path::new("."), &["ls-remote", &pr.clone_url, &glob])?; + let refs: Vec = listing + .lines() + .filter_map(|l| l.split_once('\t').map(|(_, r)| r.trim().to_string())) + .collect(); + let (head_ref, merge_ref) = pr.pick_refs(&refs).ok_or_else(|| { + PrUrlError::NotFound(pr.provider.unit(), pr.number, pr.clone_url.clone()) + })?; + + let dir = pr.cache_dir()?; + if !dir.join(".git").exists() { + if let Some(parent) = dir.parent() { + std::fs::create_dir_all(parent)?; + } + // Full clone: libgit2 (which does the actual diffing) has no partial-clone + // promisor support, so a `--filter=blob:none` clone fails on missing blobs. + // The cost is paid once per repository and then cached. + git(Path::new("."), &["clone", &pr.clone_url, &lossy(&dir)])?; + } + + let local_head = format!("refs/heads/pr-{}", pr.number); + let local_merge = format!("refs/diffcore/pr-{}/merge", pr.number); + let mut args = vec![ + "fetch".to_string(), + "--force".to_string(), + "origin".to_string(), + format!("+{head_ref}:{local_head}"), + ]; + if let Some(m) = &merge_ref { + args.push(format!("+{m}:{local_merge}")); + } + let argv: Vec<&str> = args.iter().map(String::as_str).collect(); + git(&dir, &argv)?; + git(&dir, &["checkout", "--force", "--detach", &local_head])?; + + // Target-branch tip: the merge ref's first parent when the provider publishes + // one, otherwise reconstructed from the default branch. + let target = match merge_ref { + Some(_) => format!("{local_merge}^1"), + None => target_from_default_branch(&dir, &local_head)?, + }; + // Report the fork point, not the target tip, so a plain two-dot `base..head` + // diff equals what the provider shows under "Files changed". Abbreviated so + // the UI's branch selector shows something readable. + let fork_point = git(&dir, &["merge-base", &target, &local_head])?; + let base = git(&dir, &["rev-parse", "--short", fork_point.trim()])? + .trim() + .to_string(); + + Ok(ResolvedPr { + path: lossy(&dir), + base, + head: format!("pr-{}", pr.number), + provider: pr.provider, + number: pr.number, + }) +} + +/// Best guess at the PR's target-branch tip when no merge ref is published. +/// +/// Providers drop the merge ref once a PR lands, and by then the head is an +/// ancestor of the default branch, so `merge-base(default, head)` collapses to +/// `head` and the diff comes out empty. Recover the pre-merge tip from the merge +/// commit that landed the PR. +/// +/// ponytail: squash- and rebase-merged PRs leave no merge commit, so their base +/// is unrecoverable from git alone and the diff still comes out empty — needs a +/// provider API call to fix. +fn target_from_default_branch(dir: &Path, head: &str) -> Result { + let default = default_branch(dir)?; + let already_merged = git(dir, &["merge-base", "--is-ancestor", head, &default]).is_ok(); + if already_merged { + let range = format!("{head}..{default}"); + let merges = git(dir, &["rev-list", "--ancestry-path", "--merges", &range])?; + // Oldest merge on the ancestry path is the one that landed this PR. + if let Some(landing) = merges.split_whitespace().next_back() { + if let Ok(parent) = git(dir, &["rev-parse", &format!("{landing}^1")]) { + return Ok(parent.trim().to_string()); + } + } + } + Ok(default) +} + +/// Resolve the remote's default branch, for providers without a merge ref. +fn default_branch(dir: &Path) -> Result { + if let Ok(r) = git(dir, &["symbolic-ref", "--short", "refs/remotes/origin/HEAD"]) { + let r = r.trim(); + if !r.is_empty() { + return Ok(r.to_string()); + } + } + Ok("origin/HEAD".to_string()) +} + +fn lossy(p: &Path) -> String { + p.to_string_lossy().into_owned() +} + +/// Run git, failing loudly. `GIT_TERMINAL_PROMPT=0` keeps a private repo from +/// hanging on an interactive credential prompt with no terminal attached. +fn git(dir: &Path, args: &[&str]) -> Result { + let out = Command::new("git") + .args(args) + .current_dir(dir) + .env("GIT_TERMINAL_PROMPT", "0") + .output()?; + if !out.status.success() { + return Err(PrUrlError::Git( + args.join(" "), + String::from_utf8_lossy(&out.stderr).trim().to_string(), + )); + } + Ok(String::from_utf8_lossy(&out.stdout).into_owned()) +} + +#[cfg(test)] +mod tests { + use super::*; + + /// `resolve` and `cache_dir` both read DIFFCORE_REPO_CACHE_DIR, which is + /// process-global — serialize the tests that set it. + static CACHE_ENV: std::sync::Mutex<()> = std::sync::Mutex::new(()); + + fn run(dir: &Path, args: &[&str]) -> String { + match git(dir, args) { + Ok(out) => out.trim().to_string(), + Err(e) => unreachable!("git {args:?} in {dir:?}: {e}"), + } + } + + fn write(dir: &Path, name: &str, body: &str) { + if let Err(e) = std::fs::write(dir.join(name), body) { + unreachable!("write {name}: {e}"); + } + } + + fn p(u: &str) -> PrUrl { + match parse(u) { + Some(pr) => pr, + None => unreachable!("expected {u} to parse"), + } + } + + #[test] + fn parses_every_supported_provider() { + let cases: &[(&str, Provider, &str, &str, u64, &str)] = &[ + ( + "https://github.com/rust-lang/rust/pull/12345", + Provider::GitHub, + "rust-lang", + "rust", + 12345, + "https://github.com/rust-lang/rust.git", + ), + // Sub-routes and patch suffixes are common copy-paste shapes. + ( + "https://github.com/rust-lang/rust/pull/12345/files", + Provider::GitHub, + "rust-lang", + "rust", + 12345, + "https://github.com/rust-lang/rust.git", + ), + ( + "https://github.com/rust-lang/rust/pull/12345.diff", + Provider::GitHub, + "rust-lang", + "rust", + 12345, + "https://github.com/rust-lang/rust.git", + ), + ( + "https://api.github.com/repos/rust-lang/rust/pulls/12345", + Provider::GitHub, + "rust-lang", + "rust", + 12345, + "https://github.com/rust-lang/rust.git", + ), + ( + "https://ghe.corp.internal/platform/api/pull/7", + Provider::GitHub, + "platform", + "api", + 7, + "https://ghe.corp.internal/platform/api.git", + ), + ( + "https://gitlab.com/gitlab-org/gitlab/-/merge_requests/999", + Provider::GitLab, + "gitlab-org", + "gitlab", + 999, + "https://gitlab.com/gitlab-org/gitlab.git", + ), + // Nested subgroups, and the pre-11.0 URL shape without `/-/`. + ( + "https://gitlab.com/a/b/c/repo/-/merge_requests/4", + Provider::GitLab, + "a/b/c", + "repo", + 4, + "https://gitlab.com/a/b/c/repo.git", + ), + ( + "https://gitlab.example.com/team/repo/merge_requests/4", + Provider::GitLab, + "team", + "repo", + 4, + "https://gitlab.example.com/team/repo.git", + ), + ( + "https://codeberg.org/forgejo/forgejo/pulls/321", + Provider::Gitea, + "forgejo", + "forgejo", + 321, + "https://codeberg.org/forgejo/forgejo.git", + ), + ( + "http://gitea.internal:3000/ops/infra/pulls/8", + Provider::Gitea, + "ops", + "infra", + 8, + "http://gitea.internal:3000/ops/infra.git", + ), + ( + "https://gitee.com/openharmony/docs/pulls/55", + Provider::Gitea, + "openharmony", + "docs", + 55, + "https://gitee.com/openharmony/docs.git", + ), + ( + "https://bitbucket.org/atlassian/stash/pull-requests/42", + Provider::BitbucketCloud, + "atlassian", + "stash", + 42, + "https://bitbucket.org/atlassian/stash.git", + ), + ( + "https://bb.corp.com/projects/PLAT/repos/api/pull-requests/17/overview", + Provider::BitbucketServer, + "PLAT", + "api", + 17, + "https://bb.corp.com/scm/PLAT/api.git", + ), + // Bitbucket DC behind a context path. + ( + "https://corp.com/bitbucket/projects/PLAT/repos/api/pull-requests/17", + Provider::BitbucketServer, + "PLAT", + "api", + 17, + "https://corp.com/bitbucket/scm/PLAT/api.git", + ), + ( + "https://dev.azure.com/contoso/Payments/_git/gateway/pullrequest/88", + Provider::AzureDevOps, + "contoso/Payments", + "gateway", + 88, + "https://dev.azure.com/contoso/Payments/_git/gateway", + ), + ( + "https://contoso.visualstudio.com/Payments/_git/gateway/pullrequest/88", + Provider::AzureDevOps, + "Payments", + "gateway", + 88, + "https://contoso.visualstudio.com/Payments/_git/gateway", + ), + ( + "https://gerrit.googlesource.com/c/gerrit/+/400123", + Provider::Gerrit, + "", + "gerrit", + 400123, + "https://gerrit.googlesource.com/gerrit", + ), + ( + "https://sourceforge.net/p/mingw/mingw-org-wsl/merge-requests/3", + Provider::SourceForge, + "mingw", + "mingw-org-wsl", + 3, + "https://git.code.sf.net/p/mingw/mingw-org-wsl", + ), + ( + "https://code.launchpad.net/~user/proj/+git/repo/+merge/456", + Provider::Launchpad, + "~user/proj/+git", + "repo", + 456, + "https://git.launchpad.net/~user/proj/+git/repo", + ), + ]; + + for (url, provider, owner, repo, number, clone_url) in cases { + let got = p(url); + assert_eq!( + (got.provider, got.owner.as_str(), got.repo.as_str(), got.number, got.clone_url.as_str()), + (*provider, *owner, *repo, *number, *clone_url), + "parsing {url}" + ); + } + } + + #[test] + fn gerrit_url_pins_patchset_and_shards_ref() { + let pinned = p("https://gerrit.example.org/c/platform/build/+/1234/7"); + assert_eq!(pinned.patchset, Some(7)); + assert_eq!(pinned.repo, "build"); + assert_eq!(pinned.owner, "platform"); + // Gerrit sharding: last two digits of the change number, zero padded. + assert_eq!(pinned.ref_glob().as_deref(), Some("refs/changes/34/1234/*")); + let single_digit = p("https://gerrit.example.org/c/proj/+/7"); + assert_eq!(single_digit.ref_glob().as_deref(), Some("refs/changes/07/7/*")); + } + + #[test] + fn ref_globs_match_provider_namespaces() { + assert_eq!( + p("https://github.com/o/r/pull/1").ref_glob().as_deref(), + Some("refs/pull/1/*") + ); + assert_eq!( + p("https://gitlab.com/o/r/-/merge_requests/2").ref_glob().as_deref(), + Some("refs/merge-requests/2/*") + ); + assert_eq!( + p("https://bb.corp.com/projects/P/repos/r/pull-requests/3").ref_glob().as_deref(), + Some("refs/pull-requests/3/*") + ); + assert_eq!( + p("https://dev.azure.com/o/p/_git/r/pullrequest/4").ref_glob().as_deref(), + Some("refs/pull/4/*") + ); + // No git-visible refs — callers must surface a "use branches" error. + assert_eq!(p("https://bitbucket.org/w/r/pull-requests/5").ref_glob(), None); + assert_eq!( + p("https://code.launchpad.net/~u/p/+git/r/+merge/6").ref_glob(), + None + ); + } + + #[test] + fn picks_head_and_merge_refs() { + let gh = p("https://github.com/o/r/pull/1"); + let refs = vec!["refs/pull/1/head".to_string(), "refs/pull/1/merge".to_string()]; + assert_eq!( + gh.pick_refs(&refs), + Some(("refs/pull/1/head".into(), Some("refs/pull/1/merge".into()))) + ); + // Conflicted PRs have no merge ref; head alone must still resolve. + assert_eq!( + gh.pick_refs(&["refs/pull/1/head".to_string()]), + Some(("refs/pull/1/head".into(), None)) + ); + assert_eq!(gh.pick_refs(&[]), None); + + // Bitbucket DC names the head ref `from`. + let bb = p("https://bb.corp.com/projects/P/repos/r/pull-requests/3"); + assert_eq!( + bb.pick_refs(&["refs/pull-requests/3/from".to_string()]), + Some(("refs/pull-requests/3/from".into(), None)) + ); + + // Gerrit: newest patchset unless the URL pinned one. + let g = p("https://gerrit.example.org/c/proj/+/1234"); + let ps: Vec = ["1", "2", "10"] + .iter() + .map(|n| format!("refs/changes/34/1234/{n}")) + .collect(); + assert_eq!(g.pick_refs(&ps), Some(("refs/changes/34/1234/10".into(), None))); + let pinned = p("https://gerrit.example.org/c/proj/+/1234/2"); + assert_eq!(pinned.pick_refs(&ps), Some(("refs/changes/34/1234/2".into(), None))); + } + + #[test] + fn rejects_non_pr_input() { + for input in [ + "/home/me/projects/repo", + "~/code/repo", + "C:\\code\\repo", + "git@github.com:o/r.git", + "https://github.com/rust-lang/rust", + "https://github.com/rust-lang/rust/issues/123", + "https://gitlab.com/o/r/-/issues/5", + "https://example.com", + "not a url at all", + "", + ] { + assert!(parse(input).is_none(), "{input} should not parse as a PR URL"); + } + } + + #[test] + fn cache_dir_is_sandboxed_and_filesystem_safe() { + let _guard = CACHE_ENV.lock(); + let tmp = tempfile::tempdir().unwrap_or_else(|e| unreachable!("tempdir: {e}")); + std::env::set_var("DIFFCORE_REPO_CACHE_DIR", tmp.path()); + let dir = p("http://gitea.internal:3000/a/b/c/pulls/1") + .cache_dir() + .unwrap_or_else(|e| unreachable!("cache_dir: {e}")); + std::env::remove_var("DIFFCORE_REPO_CACHE_DIR"); + assert!(dir.starts_with(tmp.path())); + assert_eq!( + dir.strip_prefix(tmp.path()).ok(), + Some(Path::new("gitea.internal-3000/a-b/c")) + ); + } + + /// End-to-end `resolve` against a local origin publishing GitHub-shaped PR + /// refs. Exercises clone, fetch, merge-ref parent lookup and fork-point + /// resolution without touching the network. + #[test] + fn resolve_clones_fetches_and_reports_the_fork_point() { + let _guard = CACHE_ENV.lock(); + let tmp = tempfile::tempdir().unwrap_or_else(|e| unreachable!("tempdir: {e}")); + let origin = tmp.path().join("origin"); + if let Err(e) = std::fs::create_dir_all(&origin) { + unreachable!("mkdir origin: {e}"); + } + + run(&origin, &["init", "-q", "-b", "main"]); + run(&origin, &["config", "user.email", "test@diffcore.invalid"]); + run(&origin, &["config", "user.name", "diffcore test"]); + write(&origin, "shared.txt", "base\n"); + run(&origin, &["add", "-A"]); + run(&origin, &["commit", "-qm", "base"]); + let fork_point = run(&origin, &["rev-parse", "HEAD"]); + + run(&origin, &["checkout", "-q", "-b", "feature"]); + write(&origin, "feature.txt", "pr work\n"); + run(&origin, &["add", "-A"]); + run(&origin, &["commit", "-qm", "pr work"]); + let pr_head = run(&origin, &["rev-parse", "HEAD"]); + + // The target branch moves on after the fork, so the target tip and the + // fork point differ — a two-dot diff from the tip would be wrong. + run(&origin, &["checkout", "-q", "main"]); + write(&origin, "unrelated.txt", "moved on\n"); + run(&origin, &["add", "-A"]); + run(&origin, &["commit", "-qm", "main moves on"]); + let main_tip = run(&origin, &["rev-parse", "HEAD"]); + + // Provider-published refs: head plus a merge preview whose first parent + // is the target branch tip. + let merge = run( + &origin, + &[ + "commit-tree", + "-p", + &main_tip, + "-p", + &pr_head, + "-m", + "merge preview", + &format!("{main_tip}^{{tree}}"), + ], + ); + run(&origin, &["update-ref", "refs/pull/1/head", &pr_head]); + run(&origin, &["update-ref", "refs/pull/1/merge", &merge]); + + std::env::set_var("DIFFCORE_REPO_CACHE_DIR", tmp.path().join("cache")); + let pr = PrUrl { + provider: Provider::GitHub, + host: "example.test".to_string(), + owner: "o".to_string(), + repo: "r".to_string(), + number: 1, + patchset: None, + clone_url: origin.to_string_lossy().into_owned(), + }; + let resolved = resolve(&pr).unwrap_or_else(|e| unreachable!("resolve: {e}")); + // Second call must reuse the existing clone rather than re-cloning. + let again = resolve(&pr).unwrap_or_else(|e| unreachable!("re-resolve: {e}")); + std::env::remove_var("DIFFCORE_REPO_CACHE_DIR"); + + assert_eq!(resolved.path, again.path); + let dir = PathBuf::from(&resolved.path); + assert_eq!( + run(&dir, &["rev-parse", &resolved.base]), + fork_point, + "base must be the fork point" + ); + assert_ne!(run(&dir, &["rev-parse", &resolved.base]), main_tip); + assert_eq!(run(&dir, &["rev-parse", &resolved.head]), pr_head); + // What the provider shows under "Files changed": the PR's file only. + assert_eq!( + run( + &dir, + &[ + "diff", + "--name-only", + &format!("{}..{}", resolved.base, resolved.head) + ] + ), + "feature.txt" + ); + } +} diff --git a/crates/diffcore-tauri/src/commands.rs b/crates/diffcore-tauri/src/commands.rs index 2b72d00..65c2cf9 100644 --- a/crates/diffcore-tauri/src/commands.rs +++ b/crates/diffcore-tauri/src/commands.rs @@ -29,6 +29,7 @@ use diffcore_core::llm::refinement; use diffcore_core::llm::schema::{Pass1Response, Pass2Response, RefinementResponse}; use diffcore_core::output::{self, build_analysis_output}; use diffcore_core::pipeline; +use diffcore_core::pr_url; use diffcore_core::rank; use diffcore_core::types::AnalysisOutput; @@ -1590,6 +1591,18 @@ pub fn get_repo_info(repo_path: String) -> Result { }) } +/// Resolve a pull/merge request URL into a local checkout plus base/head refs. +/// +/// The repository field accepts a PR URL from any supported forge; the app calls +/// this first, then re-runs its normal path-based flow against the returned path. +/// Cloning and fetching happen synchronously and may take a while on first use. +#[cfg_attr(feature = "desktop", tauri::command)] +pub fn resolve_pr_url(url: String) -> Result { + let pr = pr_url::parse(&url) + .ok_or_else(|| CommandError::Git(format!("Not a pull/merge request URL: {}", url)))?; + pr_url::resolve(&pr).map_err(|e| CommandError::Git(e.to_string())) +} + /// Check whether LLM access is configured and available. /// /// This includes API-key-based providers plus subscription-backed Codex/Claude CLIs. diff --git a/crates/diffcore-tauri/src/main.rs b/crates/diffcore-tauri/src/main.rs index 1fbc18c..74a892a 100644 --- a/crates/diffcore-tauri/src/main.rs +++ b/crates/diffcore-tauri/src/main.rs @@ -73,6 +73,7 @@ fn main() { commands::list_worktrees, commands::get_branch_status, commands::get_repo_info, + commands::resolve_pr_url, commands::check_api_key, commands::get_llm_settings, commands::save_llm_settings, diff --git a/crates/diffcore-tauri/src/web_server.rs b/crates/diffcore-tauri/src/web_server.rs index d7d3d63..6c57ad5 100644 --- a/crates/diffcore-tauri/src/web_server.rs +++ b/crates/diffcore-tauri/src/web_server.rs @@ -343,6 +343,7 @@ fn dispatch_sync(cmd: &str, args: &mut Args, app: &AppState) -> Result ok(commands::list_worktrees(req(args, "repoPath")?)?), "get_branch_status" => ok(commands::get_branch_status(req(args, "repoPath")?)?), "get_repo_info" => ok(commands::get_repo_info(req(args, "repoPath")?)?), + "resolve_pr_url" => ok(commands::resolve_pr_url(req(args, "url")?)?), "check_api_key" => ok(commands::check_api_key(opt(args, "repoPath")?)?), "get_llm_settings" => ok(commands::get_llm_settings(opt(args, "repoPath")?)?), "save_llm_settings" => ok(commands::save_llm_settings( diff --git a/crates/diffcore-tauri/ui/src/App.tsx b/crates/diffcore-tauri/ui/src/App.tsx index ffc73bd..5e081a8 100644 --- a/crates/diffcore-tauri/ui/src/App.tsx +++ b/crates/diffcore-tauri/ui/src/App.tsx @@ -7,6 +7,7 @@ import type { Pass1GroupAnnotation, Pass2Response, RepoInfo, + ResolvedPr, BranchInfo, LlmSettings, LlmProvider, @@ -84,6 +85,14 @@ function isApiProvider(provider: string): boolean { return provider === "anthropic" || provider === "openai" || provider === "gemini"; } +/** Coarse check — the backend decides whether a URL is actually a PR/MR we support. */ +function isPrUrl(value: string): boolean { + return /^https?:\/\//i.test(value.trim()); +} + +/** Explicit analysis target, for when React state has not committed yet. */ +type AnalyzeTarget = { repoPath: string; base: string; head: string | null }; + function TruncatedText({ text, maxChars = 320, @@ -414,7 +423,7 @@ export default function App() { // Load repo info when repo path changes useEffect(() => { - if (repoPath) { + if (repoPath && !isPrUrl(repoPath)) { loadRepoInfo(repoPath); } else { setRepoInfo(null); @@ -424,7 +433,7 @@ export default function App() { // Watch the repo's HEAD for changes made outside the app (git pull/checkout/merge in a // terminal). The watcher lives in Rust and emits "git-head-changed"; see the listener below. useEffect(() => { - if (!IS_TAURI || !repoPath) return; + if (!IS_TAURI || !repoPath || isPrUrl(repoPath)) return; tauriInvoke("watch_git_head", { repoPath }).catch(() => {}); return () => { tauriInvoke("unwatch_git_head", {}).catch(() => {}); @@ -790,8 +799,11 @@ export default function App() { [handleSelectFile], ); - const runAnalysis = useCallback(async () => { - if (!repoPath) return; + const runAnalysis = useCallback(async (target?: AnalyzeTarget) => { + const path = target?.repoPath ?? repoPath; + const base = target?.base ?? baseRef; + const head = target?.head ?? headRef; + if (!path) return; setLoading(true); setError(null); // Reset LLM state on new analysis @@ -827,9 +839,9 @@ export default function App() { let result: AnalysisOutput; if (HAS_BACKEND) { result = await tauriInvoke("analyze", { - repoPath, - base: baseRef || "main", - head: headRef || null, + repoPath: path, + base: base || "main", + head: head || null, range: null, staged: false, unstaged: false, @@ -855,7 +867,7 @@ export default function App() { } // Check for cached refinement and auto-apply if found if (HAS_BACKEND) { - tauriInvoke("get_cached_refinement", { repoPath: repoPath || null }).then((cached) => { + tauriInvoke("get_cached_refinement", { repoPath: path || null }).then((cached) => { if (cached) { applyRefinementResult(cached, { fromCache: true }); } @@ -871,6 +883,33 @@ export default function App() { } }, [repoPath, baseRef, headRef, handleSelectGroup, closeActivityStream]); + /** Analyze whatever is in the repository field — a local path, or a PR/MR URL + * that we first clone and resolve to a base/head pair. */ + const submitRepoInput = useCallback(async () => { + const value = repoPath.trim(); + if (!value || loading) return; + if (!HAS_BACKEND || !isPrUrl(value)) { + runAnalysis(); + return; + } + setLoading(true); + setError(null); + let resolved: ResolvedPr; + try { + resolved = await tauriInvoke("resolve_pr_url", { url: value }); + } catch (e) { + setLoading(false); + setError(String(e)); + repoInputRef.current?.focus(); + repoInputRef.current?.select(); + return; + } + setRepoPath(resolved.path); + setBaseRef(resolved.base); + setHeadRef(resolved.head); + await runAnalysis({ repoPath: resolved.path, base: resolved.base, head: resolved.head }); + }, [repoPath, loading, runAnalysis]); + const recommendedSubscriptionProvider: SubscriptionProvider | null = llmSettings?.codex_authenticated ? "codex" : llmSettings?.claude_authenticated @@ -2959,13 +2998,13 @@ export default function App() { ref={repoInputRef} className="input repo-input" type="text" - placeholder="Repository path..." + placeholder="Repository path or pull/merge request URL..." value={repoPath} onChange={(e) => setRepoPath(e.target.value)} onKeyDown={(e) => { if (e.key === "Enter" && repoPath && !loading) { (e.target as HTMLInputElement).blur(); - runAnalysis(); + submitRepoInput(); } }} /> @@ -3055,7 +3094,7 @@ export default function App() {