Skip to content

Commit 2b08b9a

Browse files
committed
Fetching
1 parent 38af639 commit 2b08b9a

File tree

3 files changed

+173
-22
lines changed

3 files changed

+173
-22
lines changed

AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525

2626
## Learned while implementing PR local diff sourcing
2727

28-
- `fetch` now builds PR change blocks from local git (`git diff -w <base_sha>...<head_sha>`) instead of GitHub file patches, so PR review startup must fail early when the base commit is missing locally.
28+
- `fetch` builds PR change blocks from local git (`git diff -w <base_sha>...<head_sha>`) instead of GitHub file patches; when commits are missing locally it attempts a targeted `git fetch` of base/head refs before failing.
2929

3030
## Project Overview
3131

cli/src/commands/fetch.rs

Lines changed: 170 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,34 +1,21 @@
11
use anyhow::Result;
2+
use regex::Regex;
3+
use std::process::Command;
24

35
use crate::commands::diff::{ensure_git_commit_available, get_pr_review_files};
46
use crate::github::client::GitHubClient;
5-
use crate::github::types::FetchResponse;
7+
use crate::github::types::{FetchResponse, PrRef};
68

79
pub async fn run(url: &str) -> Result<()> {
810
let client = GitHubClient::new()?;
911
let pr_ref = GitHubClient::parse_pr_url(url)?;
1012

1113
// Fetch PR metadata and viewer in parallel
1214
let (pr, viewer) = tokio::try_join!(client.get_pr(&pr_ref), client.get_viewer())?;
15+
let remote = detect_repo_remote(&pr_ref)?.unwrap_or_else(|| "origin".to_string());
1316

14-
ensure_git_commit_available(&pr.base_sha).map_err(|err| {
15-
anyhow::anyhow!(
16-
"Missing PR base commit {} locally (base branch '{}'). Run `git fetch origin {}` and retry. {}",
17-
pr.base_sha,
18-
pr.base_ref,
19-
pr.base_ref,
20-
err
21-
)
22-
})?;
23-
24-
ensure_git_commit_available(&pr.head_sha).map_err(|err| {
25-
anyhow::anyhow!(
26-
"Missing PR head commit {} locally. Run `gh pr checkout {}` and retry. {}",
27-
pr.head_sha,
28-
pr.number,
29-
err
30-
)
31-
})?;
17+
ensure_base_commit_available(&pr_ref, &remote, &pr.base_sha, &pr.base_ref)?;
18+
ensure_head_commit_available(&pr_ref, &remote, pr.number, &pr.head_sha, &pr.head_ref)?;
3219

3320
// Fetch change blocks from local git using the PR commit range.
3421
let files = get_pr_review_files(&pr.base_sha, &pr.head_sha, true)?;
@@ -48,3 +35,167 @@ pub async fn run(url: &str) -> Result<()> {
4835

4936
Ok(())
5037
}
38+
39+
fn ensure_base_commit_available(
40+
pr_ref: &PrRef,
41+
remote: &str,
42+
base_sha: &str,
43+
base_ref: &str,
44+
) -> Result<()> {
45+
if ensure_git_commit_available(base_sha).is_ok() {
46+
return Ok(());
47+
}
48+
49+
fetch_remote_ref(remote, base_ref)?;
50+
if ensure_git_commit_available(base_sha).is_ok() {
51+
return Ok(());
52+
}
53+
54+
Err(anyhow::anyhow!(
55+
"Missing PR base commit {} locally after fetching {} from remote '{}' for {}/{}. \
56+
Run `git fetch {} {}` and retry.",
57+
base_sha,
58+
base_ref,
59+
remote,
60+
pr_ref.owner,
61+
pr_ref.repo,
62+
remote,
63+
base_ref
64+
))
65+
}
66+
67+
fn ensure_head_commit_available(
68+
pr_ref: &PrRef,
69+
remote: &str,
70+
pr_number: u64,
71+
head_sha: &str,
72+
head_ref: &str,
73+
) -> Result<()> {
74+
if ensure_git_commit_available(head_sha).is_ok() {
75+
return Ok(());
76+
}
77+
78+
// Try branch ref first, then GitHub's pull/<n>/head ref for forked PRs.
79+
let _ = fetch_remote_ref(remote, head_ref);
80+
let pull_ref = format!("pull/{pr_number}/head");
81+
let _ = fetch_remote_ref(remote, &pull_ref);
82+
83+
if ensure_git_commit_available(head_sha).is_ok() {
84+
return Ok(());
85+
}
86+
87+
Err(anyhow::anyhow!(
88+
"Missing PR head commit {} locally after fetching '{}' and '{}' from remote '{}' for {}/{}. \
89+
Run `gh pr checkout {}` and retry.",
90+
head_sha,
91+
head_ref,
92+
pull_ref,
93+
remote,
94+
pr_ref.owner,
95+
pr_ref.repo,
96+
pr_number
97+
))
98+
}
99+
100+
fn fetch_remote_ref(remote: &str, git_ref: &str) -> Result<()> {
101+
let output = Command::new("git")
102+
.args(["fetch", "--no-tags", remote, git_ref])
103+
.output()?;
104+
105+
if !output.status.success() {
106+
return Err(anyhow::anyhow!(
107+
"git fetch --no-tags {} {} failed: {}",
108+
remote,
109+
git_ref,
110+
String::from_utf8_lossy(&output.stderr).trim()
111+
));
112+
}
113+
114+
Ok(())
115+
}
116+
117+
fn detect_repo_remote(pr_ref: &PrRef) -> Result<Option<String>> {
118+
let output = Command::new("git").args(["remote", "-v"]).output()?;
119+
if !output.status.success() {
120+
return Ok(None);
121+
}
122+
123+
let lines = String::from_utf8(output.stdout)?;
124+
let remote_line_re = Regex::new(r"^(\S+)\s+(\S+)\s+\(fetch\)$")?;
125+
126+
for line in lines.lines() {
127+
if let Some(caps) = remote_line_re.captures(line) {
128+
let remote_name = caps[1].to_string();
129+
let remote_url = caps[2].to_string();
130+
if remote_points_to_repo(&remote_url, &pr_ref.owner, &pr_ref.repo) {
131+
return Ok(Some(remote_name));
132+
}
133+
}
134+
}
135+
136+
Ok(None)
137+
}
138+
139+
fn remote_points_to_repo(remote_url: &str, owner: &str, repo: &str) -> bool {
140+
let ssh_re = Regex::new(r"^git@github\.com:([^/]+)/([^/]+?)(?:\.git)?$").ok();
141+
let https_re = Regex::new(r"^https?://github\.com/([^/]+)/([^/]+?)(?:\.git)?$").ok();
142+
143+
let mut parsed = None;
144+
if let Some(re) = ssh_re
145+
&& let Some(caps) = re.captures(remote_url)
146+
{
147+
parsed = Some((caps[1].to_string(), caps[2].to_string()));
148+
}
149+
if parsed.is_none()
150+
&& let Some(re) = https_re
151+
&& let Some(caps) = re.captures(remote_url)
152+
{
153+
parsed = Some((caps[1].to_string(), caps[2].to_string()));
154+
}
155+
156+
match parsed {
157+
Some((remote_owner, remote_repo)) => remote_owner == owner && remote_repo == repo,
158+
None => false,
159+
}
160+
}
161+
162+
#[cfg(test)]
163+
mod tests {
164+
use super::*;
165+
166+
#[test]
167+
fn remote_points_to_repo_matches_https_url() {
168+
assert!(remote_points_to_repo(
169+
"https://github.com/owner/repo.git",
170+
"owner",
171+
"repo"
172+
));
173+
}
174+
175+
#[test]
176+
fn remote_points_to_repo_matches_ssh_url() {
177+
assert!(remote_points_to_repo(
178+
"git@github.com:owner/repo.git",
179+
"owner",
180+
"repo"
181+
));
182+
}
183+
184+
#[test]
185+
fn remote_points_to_repo_rejects_non_matching_repo() {
186+
assert!(!remote_points_to_repo(
187+
"https://github.com/owner/other.git",
188+
"owner",
189+
"repo"
190+
));
191+
}
192+
193+
#[test]
194+
fn remote_points_to_repo_rejects_non_github_url() {
195+
assert!(!remote_points_to_repo(
196+
"https://gitlab.com/owner/repo.git",
197+
"owner",
198+
"repo"
199+
));
200+
}
201+
}

doc/neo_reviewer.txt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -180,8 +180,8 @@ These user commands are created when you call `setup()`.
180180

181181
Diff hunks are generated from local git using the PR base/head commits
182182
(`git diff -w base...head`), so whitespace-only changes are excluded.
183-
If the base commit is missing locally, review startup fails and prompts
184-
you to fetch the base branch.
183+
If required commits are missing locally, neo-reviewer first attempts to
184+
fetch the needed refs automatically before failing.
185185

186186
Examples: >
187187
:ReviewPR " Current branch PR

0 commit comments

Comments
 (0)