diff --git a/Cargo.lock b/Cargo.lock index 7118566..61733df 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -45,6 +45,7 @@ name = "bitbygit-tui" version = "0.1.0" dependencies = [ "bitbygit-core", + "bitbygit-gh", "bitbygit-git", "bitbygit-store", "crossterm", diff --git a/crates/bitbygit-core/src/lib.rs b/crates/bitbygit-core/src/lib.rs index 01c4241..a698ca7 100644 --- a/crates/bitbygit-core/src/lib.rs +++ b/crates/bitbygit-core/src/lib.rs @@ -39,6 +39,9 @@ pub enum OperationRequest { Rebase { base: String, }, + OpenPullRequest { + base: Option, + }, PromptSequence { requests: Vec, }, @@ -63,6 +66,7 @@ pub enum OperationKind { CreateBranch, MergeFastForward, Rebase, + OpenPullRequest, } impl OperationKind { @@ -84,6 +88,7 @@ impl OperationKind { Self::CreateBranch => "create branch", Self::MergeFastForward => "merge", Self::Rebase => "rebase", + Self::OpenPullRequest => "open pull request", } } @@ -106,6 +111,7 @@ impl OperationKind { Self::CreateBranch => "create_branch", Self::MergeFastForward => "merge_ff_only", Self::Rebase => "rebase", + Self::OpenPullRequest => "open_pull_request", } } } diff --git a/crates/bitbygit-core/src/prompt_parser.rs b/crates/bitbygit-core/src/prompt_parser.rs index 162ebc5..5a17a43 100644 --- a/crates/bitbygit-core/src/prompt_parser.rs +++ b/crates/bitbygit-core/src/prompt_parser.rs @@ -3,8 +3,8 @@ use std::fmt; use crate::OperationRequest; -const PROMPT_EXAMPLES: &str = "branches, checkout , branch , branch from , merge , rebase , commit -m \"message\", fetch, push, pull, or pull --rebase"; -pub const UNSUPPORTED_PROMPT_MESSAGE: &str = "Unsupported prompt. Try: branches, checkout , branch , branch from , merge , rebase , commit -m \"message\", fetch, push, pull, or pull --rebase"; +const PROMPT_EXAMPLES: &str = "branches, checkout , branch , branch from , merge , rebase , open pr, commit -m \"message\", fetch, push, pull, or pull --rebase"; +pub const UNSUPPORTED_PROMPT_MESSAGE: &str = "Unsupported prompt. Try: branches, checkout , branch , branch from , merge , rebase , open pr, commit -m \"message\", fetch, push, pull, or pull --rebase"; const SHELL_SYNTAX_MESSAGE: &str = "Shell-style prompt syntax is not supported. Use one guarded prompt at a time."; const RAW_GIT_MESSAGE: &str = @@ -168,6 +168,7 @@ fn parse_single_prompt(input: &str) -> Result Ok(OperationRequest::Branches), "fetch" => Ok(OperationRequest::Fetch), "push" => Ok(OperationRequest::Push), + "open pr" | "open pull request" => Ok(OperationRequest::OpenPullRequest { base: None }), "pull" => Ok(OperationRequest::Pull { rebase: false }), "pull --rebase" | "pull rebase" => Ok(OperationRequest::Pull { rebase: true }), _ if lower.starts_with("checkout ") => parse_one_arg_prompt(trimmed, "checkout") @@ -178,6 +179,10 @@ fn parse_single_prompt(input: &str) -> Result { parse_one_arg_prompt(trimmed, "rebase").map(|base| OperationRequest::Rebase { base }) } + _ if lower.starts_with("open pr to ") => parse_open_pull_request_prompt(trimmed, "open pr"), + _ if lower.starts_with("open pull request to ") => { + parse_open_pull_request_prompt(trimmed, "open pull request") + } _ if lower.starts_with("branch ") => parse_branch_prompt(trimmed), _ if lower == "commit" || lower.starts_with("commit ") => { parse_commit_prompt(trimmed).map(|message| OperationRequest::Commit { message }) @@ -186,6 +191,30 @@ fn parse_single_prompt(input: &str) -> Result Result { + let Some(rest) = input.get(command.len()..) else { + return Err(parse_error("Expected: open pr or open pr to ")); + }; + let rest = rest.trim_start(); + let Some(base) = rest + .get(..2) + .filter(|prefix| prefix.eq_ignore_ascii_case("to")) + .and_then(|_prefix| rest.get(2..)) + .filter(|base| base.starts_with(char::is_whitespace)) + else { + return Err(parse_error("Expected: open pr or open pr to ")); + }; + Ok(OperationRequest::OpenPullRequest { + base: Some(parse_branch_arg( + base.trim(), + "Expected: open pr or open pr to ", + )?), + }) +} + fn parse_one_arg_prompt(input: &str, command: &str) -> Result { let Some(rest) = input.get(command.len()..) else { return Err(parse_error(format!("Expected: {command} "))); @@ -304,6 +333,23 @@ mod tests { ("branches", OperationRequest::Branches), ("fetch", OperationRequest::Fetch), ("push", OperationRequest::Push), + ("open pr", OperationRequest::OpenPullRequest { base: None }), + ( + "open pull request", + OperationRequest::OpenPullRequest { base: None }, + ), + ( + "open pull request to develop", + OperationRequest::OpenPullRequest { + base: Some("develop".to_owned()), + }, + ), + ( + "OPEN PR TO release", + OperationRequest::OpenPullRequest { + base: Some("release".to_owned()), + }, + ), ("pull", OperationRequest::Pull { rebase: false }), ("pull --rebase", OperationRequest::Pull { rebase: true }), ("pull rebase", OperationRequest::Pull { rebase: true }), diff --git a/crates/bitbygit-gh/src/lib.rs b/crates/bitbygit-gh/src/lib.rs index 49d636c..32d7973 100644 --- a/crates/bitbygit-gh/src/lib.rs +++ b/crates/bitbygit-gh/src/lib.rs @@ -14,6 +14,9 @@ pub const AUTHENTICATE_GH_GUIDANCE: &str = pub struct GitHub { cwd: PathBuf, executable: PathBuf, + #[cfg(test)] + executable_args: Vec, + repository: Option, } impl GitHub { @@ -25,9 +28,32 @@ impl GitHub { Self { cwd: cwd.into(), executable: executable.into(), + #[cfg(test)] + executable_args: Vec::new(), + repository: None, } } + pub fn with_executable_and_repository( + cwd: impl Into, + executable: impl Into, + repository: impl Into, + ) -> Self { + Self { + cwd: cwd.into(), + executable: executable.into(), + #[cfg(test)] + executable_args: Vec::new(), + repository: Some(repository.into()), + } + } + + #[cfg(test)] + fn with_executable_args(mut self, executable_args: Vec) -> Self { + self.executable_args = executable_args; + self + } + pub fn setup_status(&self) -> Result { match self.run_status(vec!["--version".to_owned()]) { Ok(()) => {} @@ -50,12 +76,95 @@ impl GitHub { pub fn repository(&self) -> Result { self.ensure_ready()?; + self.repository_after_ready() + } + + pub fn existing_pull_requests(&self, head: &str) -> Result, GhError> { + self.ensure_ready()?; + let repository = self.repository_after_ready()?; + let (owner, branch) = match head.split_once(':') { + Some((owner, branch)) => (owner, branch), + None => ( + repository + .name_with_owner + .split_once('/') + .map_or("", |(owner, _)| owner), + head, + ), + }; + if owner.is_empty() || branch.is_empty() { + return Err(GhError::InvalidInput { + name: "pull request head", + }); + } + let output = self.run_output(vec![ + "api".to_owned(), + "--method".to_owned(), + "GET".to_owned(), + "--paginate".to_owned(), + "--slurp".to_owned(), + format!("repos/{}/pulls", repository.name_with_owner), + "-f".to_owned(), + "state=open".to_owned(), + "-f".to_owned(), + format!("head={owner}:{branch}"), + "-f".to_owned(), + "per_page=100".to_owned(), + "--hostname".to_owned(), + "github.com".to_owned(), + ])?; + let pages: Vec> = parse_json(&output, "pull request query")?; + Ok(pages.into_iter().flatten().map(PullRequest::from).collect()) + } + + pub fn branch_exists(&self, branch: &str) -> Result { + Ok(self.branch_reference(branch)?.is_some()) + } + + pub fn branch_oid(&self, branch: &str) -> Result, GhError> { + Ok(self + .branch_reference(branch)? + .map(|reference| reference.object.sha)) + } + + fn branch_reference(&self, branch: &str) -> Result, GhError> { + self.ensure_ready()?; + if branch.trim().is_empty() { + return Err(GhError::InvalidInput { name: "branch" }); + } + let repository = self.repository_after_ready()?; let output = self.run_output(vec![ - "repo".to_owned(), - "view".to_owned(), + "api".to_owned(), + "--method".to_owned(), + "GET".to_owned(), + "--paginate".to_owned(), + "--slurp".to_owned(), + format!( + "repos/{}/git/matching-refs/heads/{}", + repository.name_with_owner, + url_path_component(branch) + ), + "--hostname".to_owned(), + "github.com".to_owned(), + ])?; + let pages: Vec> = parse_json(&output, "branch query")?; + let expected = format!("refs/heads/{branch}"); + Ok(pages + .into_iter() + .flatten() + .find(|reference| reference.name == expected)) + } + + fn repository_after_ready(&self) -> Result { + let mut args = vec!["repo".to_owned(), "view".to_owned()]; + if let Some(repository) = &self.repository { + args.push(repository.clone()); + } + args.extend([ "--json".to_owned(), "nameWithOwner,defaultBranchRef".to_owned(), - ])?; + ]); + let output = self.run_output(args)?; let repository: RepositoryResponse = parse_json(&output, "repository")?; let default_branch = repository .default_branch_ref @@ -70,30 +179,13 @@ impl GitHub { }) } - pub fn existing_pull_requests(&self, head: &str) -> Result, GhError> { - self.ensure_ready()?; - let output = self.run_output(vec![ - "pr".to_owned(), - "list".to_owned(), - "--head".to_owned(), - head.to_owned(), - "--state".to_owned(), - "open".to_owned(), - "--limit".to_owned(), - "0".to_owned(), - "--json".to_owned(), - "number,url,title,baseRefName,headRefName".to_owned(), - ])?; - parse_json(&output, "pull request query") - } - pub fn create_pull_request( &self, request: &CreatePullRequest, ) -> Result { self.ensure_ready()?; request.validate()?; - let output = self.run_output(vec![ + let output = self.run_output(self.with_repository(vec![ "pr".to_owned(), "create".to_owned(), "--title".to_owned(), @@ -104,7 +196,7 @@ impl GitHub { request.base.clone(), "--head".to_owned(), request.head.clone(), - ])?; + ]))?; let url = output .lines() .map(str::trim) @@ -125,6 +217,14 @@ impl GitHub { } } + fn with_repository(&self, mut args: Vec) -> Vec { + if let Some(repository) = &self.repository { + args.push("--repo".to_owned()); + args.push(repository.clone()); + } + args + } + fn run_status(&self, args: Vec) -> Result<(), GhError> { let output = self.command(&args).output().map_err(|source| { if source.kind() == io::ErrorKind::NotFound { @@ -164,8 +264,10 @@ impl GitHub { let mut command = Command::new(&self.executable); command .current_dir(&self.cwd) - .env("GH_PROMPT_DISABLED", "1") - .args(args); + .env("GH_PROMPT_DISABLED", "1"); + #[cfg(test)] + command.args(&self.executable_args); + command.args(args); command } } @@ -193,15 +295,70 @@ pub struct Repository { pub default_branch: String, } -#[derive(Debug, Clone, PartialEq, Eq, Deserialize)] +#[derive(Debug, Clone, PartialEq, Eq)] pub struct PullRequest { pub number: u64, pub url: String, pub title: String, - #[serde(rename = "baseRefName")] pub base_ref_name: String, - #[serde(rename = "headRefName")] pub head_ref_name: String, + pub head_repository: Option, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct PullRequestRepository { + pub name_with_owner: String, +} + +#[derive(Deserialize)] +struct RestPullRequest { + number: u64, + html_url: String, + title: String, + base: RestPullRequestBranch, + head: RestPullRequestBranch, +} + +#[derive(Deserialize)] +struct RestPullRequestBranch { + #[serde(rename = "ref")] + reference: String, + repo: Option, +} + +#[derive(Deserialize)] +struct RestPullRequestRepository { + full_name: String, +} + +#[derive(Deserialize)] +struct RestReference { + #[serde(rename = "ref")] + name: String, + object: RestReferenceObject, +} + +#[derive(Deserialize)] +struct RestReferenceObject { + sha: String, +} + +impl From for PullRequest { + fn from(pull_request: RestPullRequest) -> Self { + Self { + number: pull_request.number, + url: pull_request.html_url, + title: pull_request.title, + base_ref_name: pull_request.base.reference, + head_ref_name: pull_request.head.reference, + head_repository: pull_request + .head + .repo + .map(|repository| PullRequestRepository { + name_with_owner: repository.full_name, + }), + } + } } #[derive(Debug, Clone, PartialEq, Eq)] @@ -297,11 +454,29 @@ fn parse_json Deserialize<'de>>( serde_json::from_str(output).map_err(|_| GhError::InvalidOutput { operation }) } +fn url_path_component(value: &str) -> String { + const HEX: &[u8; 16] = b"0123456789ABCDEF"; + + let mut encoded = String::with_capacity(value.len()); + for byte in value.bytes() { + match byte { + b'A'..=b'Z' | b'a'..=b'z' | b'0'..=b'9' | b'-' | b'.' | b'_' | b'~' => { + encoded.push(char::from(byte)); + } + _ => { + encoded.push('%'); + encoded.push(char::from(HEX[usize::from(byte >> 4)])); + encoded.push(char::from(HEX[usize::from(byte & 0x0f)])); + } + } + } + encoded +} + #[cfg(all(test, unix))] mod tests { use super::*; use std::fs; - use std::os::unix::fs::PermissionsExt; use std::sync::atomic::{AtomicUsize, Ordering}; static NEXT_FAKE_ID: AtomicUsize = AtomicUsize::new(0); @@ -381,11 +556,13 @@ mod tests { } #[test] - fn queries_repository_and_all_existing_pull_requests() -> Result<(), Box> { + fn decodes_rest_existing_pull_request_responses() -> Result<(), Box> { let fake = FakeGh::new( - "case \"$1:$2\" in\n--version:*) exit 0 ;;\nauth:status) exit 0 ;;\nrepo:view) printf '%s\\n' '{\"nameWithOwner\":\"octo/repo\",\"defaultBranchRef\":{\"name\":\"main\"}}' ;;\npr:list) printf '%s\\n' '[{\"number\":42,\"url\":\"https://github.com/octo/repo/pull/42\",\"title\":\"Existing PR\",\"baseRefName\":\"main\",\"headRefName\":\"feature\"}]' ;;\nesac", + "case \"$1:$2\" in\n--version:*) exit 0 ;;\nauth:status) exit 0 ;;\nrepo:view) [ \"$3\" = github.com/octo/repo ] && [ \"$4\" = --json ] && [ \"$5\" = nameWithOwner,defaultBranchRef ] || exit 1; printf '%s\\n' '{\"nameWithOwner\":\"octo/repo\",\"defaultBranchRef\":{\"name\":\"main\"}}' ;;\napi:--method) [ \"$3\" = GET ] && [ \"$4\" = --paginate ] && [ \"$5\" = --slurp ] && [ \"$6\" = repos/octo/repo/pulls ] && [ \"$7\" = -f ] && [ \"$8\" = state=open ] && [ \"$9\" = -f ] && [ \"${10}\" = head=octo:feature ] && [ \"${11}\" = -f ] && [ \"${12}\" = per_page=100 ] && [ \"${13}\" = --hostname ] && [ \"${14}\" = github.com ] || exit 1; printf '%s\\n' '[[{\"number\":42,\"html_url\":\"https://github.com/octo/repo/pull/42\",\"title\":\"Existing PR\",\"base\":{\"ref\":\"main\"},\"head\":{\"ref\":\"feature\",\"repo\":{\"full_name\":\"octo/repo\"}}}]]' ;;\n*) exit 1 ;;\nesac", )?; - let github = fake.github(); + let github = + GitHub::with_executable_and_repository(&fake.path, "/bin/sh", "github.com/octo/repo") + .with_executable_args(vec![fake.executable.display().to_string()]); let repository = github.repository()?; let pull_requests = github.existing_pull_requests("feature")?; @@ -395,10 +572,47 @@ mod tests { assert_eq!(pull_requests.len(), 1); assert_eq!(pull_requests[0].number, 42); assert_eq!(pull_requests[0].url, "https://github.com/octo/repo/pull/42"); + assert_eq!(pull_requests[0].base_ref_name, "main"); + assert_eq!(pull_requests[0].head_ref_name, "feature"); + assert_eq!( + pull_requests[0] + .head_repository + .as_ref() + .map(|repository| repository.name_with_owner.as_str()), + Some("octo/repo") + ); + assert!(fake.invocations()?.contains( + "repo\u{1f}view\u{1f}github.com/octo/repo\u{1f}--json\u{1f}nameWithOwner,defaultBranchRef\u{1f}\n" + )); assert!(fake.invocations()?.contains( - "pr\u{1f}list\u{1f}--head\u{1f}feature\u{1f}--state\u{1f}open\u{1f}--limit\u{1f}0\u{1f}--json\u{1f}number,url,title,baseRefName,headRefName\u{1f}\n" + "api\u{1f}--method\u{1f}GET\u{1f}--paginate\u{1f}--slurp\u{1f}repos/octo/repo/pulls\u{1f}-f\u{1f}state=open\u{1f}-f\u{1f}head=octo:feature\u{1f}-f\u{1f}per_page=100\u{1f}--hostname\u{1f}github.com\u{1f}\n" )); - assert_eq!(fake.prompt_values()?, "1\n1\n1\n1\n1\n1\n"); + assert_eq!(fake.prompt_values()?, "1\n1\n1\n1\n1\n1\n1\n"); + Ok(()) + } + + #[test] + fn checks_encoded_base_branch_reference() -> Result<(), Box> { + let fake = FakeGh::new( + "case \"$1:$2\" in\n--version:*) exit 0 ;;\nauth:status) exit 0 ;;\nrepo:view) printf '%s\\n' '{\"nameWithOwner\":\"octo/repo\",\"defaultBranchRef\":{\"name\":\"main\"}}' ;;\napi:--method) [ \"$6\" = repos/octo/repo/git/matching-refs/heads/release%2Fnext ] || exit 1; printf '%s\\n' '[[{\"ref\":\"refs/heads/release/next\",\"object\":{\"sha\":\"abc123\"}}]]' ;;\n*) exit 1 ;;\nesac", + )?; + + assert!(fake.github().branch_exists("release/next")?); + assert_eq!( + fake.github().branch_oid("release/next")?.as_deref(), + Some("abc123") + ); + Ok(()) + } + + #[test] + fn rest_queries_pin_github_com_when_gh_host_conflicts() -> Result<(), Box> { + let fake = FakeGh::new( + "GH_HOST=example.com\ncase \"$1:$2\" in\n--version:*) exit 0 ;;\nauth:status) exit 0 ;;\nrepo:view) printf '%s\\n' '{\"nameWithOwner\":\"octo/repo\",\"defaultBranchRef\":{\"name\":\"main\"}}' ;;\napi:--method) case \"$6\" in\n*/pulls) [ \"${13}\" = --hostname ] && [ \"${14}\" = github.com ] || exit 1; printf '%s\\n' '[[]]' ;;\n*/git/matching-refs/heads/*) [ \"$7\" = --hostname ] && [ \"$8\" = github.com ] || exit 1; printf '%s\\n' '[[]]' ;;\n*) exit 1 ;;\nesac ;;\n*) exit 1 ;;\nesac", + )?; + + assert!(fake.github().existing_pull_requests("feature")?.is_empty()); + assert!(!fake.github().branch_exists("main")?); Ok(()) } @@ -428,6 +642,34 @@ mod tests { Ok(()) } + #[test] + fn fake_gh_executes_reliably_in_parallel() -> Result<(), Box> { + let threads = (0..32) + .map(|_| { + std::thread::spawn(|| -> Result<(), String> { + let fake = FakeGh::new( + "case \"$1:$2\" in\n--version:*) exit 0 ;;\nauth:status) exit 0 ;;\nesac\nexit 1", + ) + .map_err(|error| error.to_string())?; + if fake.github().setup_status().map_err(|error| error.to_string())? + != GhSetupStatus::Ready + { + return Err("fake GitHub CLI was not ready".to_owned()); + } + Ok(()) + }) + }) + .collect::>(); + + for thread in threads { + thread + .join() + .map_err(|_| io::Error::other("fake GitHub CLI thread panicked"))? + .map_err(io::Error::other)?; + } + Ok(()) + } + struct FakeGh { path: PathBuf, executable: PathBuf, @@ -438,8 +680,13 @@ mod tests { impl FakeGh { fn new(body: &str) -> Result> { let id = NEXT_FAKE_ID.fetch_add(1, Ordering::Relaxed); - let path = - std::env::temp_dir().join(format!("bitbygit-fake-gh-{}-{id}", std::process::id())); + let nonce = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH)? + .as_nanos(); + let path = std::env::temp_dir().join(format!( + "bitbygit-fake-gh-{}-{id}-{nonce}", + std::process::id() + )); if path.exists() { fs::remove_dir_all(&path)?; } @@ -456,9 +703,6 @@ mod tests { prompt_values.display(), ), )?; - let mut permissions = fs::metadata(&executable)?.permissions(); - permissions.set_mode(0o755); - fs::set_permissions(&executable, permissions)?; Ok(Self { path, executable, @@ -468,7 +712,8 @@ mod tests { } fn github(&self) -> GitHub { - GitHub::with_executable(&self.path, &self.executable) + GitHub::with_executable(&self.path, "/bin/sh") + .with_executable_args(vec![self.executable.display().to_string()]) } fn invocations(&self) -> Result { diff --git a/crates/bitbygit-git/src/lib.rs b/crates/bitbygit-git/src/lib.rs index 53635b3..657dd20 100644 --- a/crates/bitbygit-git/src/lib.rs +++ b/crates/bitbygit-git/src/lib.rs @@ -13,6 +13,7 @@ use std::os::unix::ffi::OsStringExt; const ZERO_OID: &str = "0000000000000000000000000000000000000000"; const EMPTY_TREE_OID: &str = "4b825dc642cb6eb9a060e54bf8d69288fbee4904"; +const SSH_OPTIONS: &str = "-oBatchMode=yes -oNumberOfPasswordPrompts=0 -oKbdInteractiveAuthentication=no -oStrictHostKeyChecking=yes"; const COMMIT_HOOKS: &[&str] = &[ "pre-commit", "prepare-commit-msg", @@ -23,11 +24,25 @@ const COMMIT_HOOKS: &[&str] = &[ #[derive(Debug, Clone)] pub struct Git { cwd: PathBuf, + ssh_executable: Option, } impl Git { pub fn new(cwd: impl Into) -> Self { - Self { cwd: cwd.into() } + Self { + cwd: cwd.into(), + ssh_executable: None, + } + } + + pub fn with_ssh_executable( + cwd: impl Into, + ssh_executable: impl Into, + ) -> Self { + Self { + cwd: cwd.into(), + ssh_executable: Some(ssh_executable.into()), + } } pub fn repository(&self) -> Result { @@ -279,24 +294,21 @@ impl Git { } pub fn remote_push_urls(&self, remote: &str) -> Result, GitError> { - match self.run_args(vec![ + let output = self.run_args(vec![ "remote".to_owned(), "get-url".to_owned(), "--push".to_owned(), "--all".to_owned(), "--".to_owned(), remote.to_owned(), - ]) { - Ok(output) => Ok(output - .stdout - .lines() - .map(str::trim) - .filter(|line| !line.is_empty()) - .map(ToOwned::to_owned) - .collect()), - Err(GitError::GitFailed { status, .. }) if status.code() == Some(2) => Ok(Vec::new()), - Err(error) => Err(error), - } + ])?; + Ok(output + .stdout + .lines() + .map(str::trim) + .filter(|line| !line.is_empty()) + .map(ToOwned::to_owned) + .collect()) } pub fn pull(&self) -> Result { @@ -359,6 +371,41 @@ impl Git { Ok(Some((remote, branch))) } + pub fn push_target(&self, branch: &str) -> Result, GitError> { + let output = self.run_args(vec![ + "for-each-ref".to_owned(), + "--format=%(push:remotename)%00%(push:short)".to_owned(), + "--count=1".to_owned(), + "--".to_owned(), + format!("refs/heads/{branch}"), + ])?; + let Some((remote, target)) = output.stdout.trim().split_once('\0') else { + return self.upstream_push_target(branch); + }; + if !remote.is_empty() && target.is_empty() { + let push_default = self.config_value(["config", "--get", "push.default"])?; + let upstream = self.upstream_push_target(branch)?; + if push_default.as_deref().unwrap_or("simple") == "simple" { + return Ok(upstream.map(|(upstream_remote, upstream_branch)| { + let push_branch = if upstream_remote == remote { + upstream_branch + } else { + branch.to_owned() + }; + (remote.to_owned(), push_branch) + })); + } + return Ok(None); + } + let Some(push_branch) = target.strip_prefix(&format!("{remote}/")) else { + return self.upstream_push_target(branch); + }; + if remote.is_empty() || push_branch.is_empty() { + return self.upstream_push_target(branch); + } + Ok(Some((remote.to_owned(), push_branch.to_owned()))) + } + pub fn remote_tracking_oid( &self, remote: &str, @@ -826,11 +873,13 @@ impl Git { .env("GIT_ASKPASS", "") .env("SSH_ASKPASS", "") .env("SSH_ASKPASS_REQUIRE", "never"); - if env::var_os("GIT_SSH_COMMAND").is_none() { - command.env( - "GIT_SSH_COMMAND", - "ssh -oBatchMode=yes -oNumberOfPasswordPrompts=0 -oKbdInteractiveAuthentication=no -oStrictHostKeyChecking=yes", - ); + if self.ssh_executable.is_some() || env::var_os("GIT_SSH_COMMAND").is_none() { + let ssh_executable = self + .ssh_executable + .as_ref() + .map(|path| shell_quote(&path.to_string_lossy())) + .unwrap_or_else(|| "ssh".to_owned()); + command.env("GIT_SSH_COMMAND", format!("{ssh_executable} {SSH_OPTIONS}")); } let output = command .args(&args) @@ -1597,6 +1646,10 @@ fn split_once_byte(value: &[u8], delimiter: u8) -> Option<(&[u8], &[u8])> { Some((&value[..index], &value[index + 1..])) } +fn shell_quote(value: &str) -> String { + format!("'{}'", value.replace('\'', "'\\''")) +} + #[cfg(unix)] fn path_from_bytes(value: &[u8]) -> PathBuf { PathBuf::from(OsString::from_vec(value.to_vec())) @@ -2546,6 +2599,182 @@ mod tests { Ok(()) } + #[test] + fn remote_push_urls_applies_push_instead_of_rewrites() -> Result<(), Box> { + let repo = TempRepo::new()?; + repo.run(["init", "-b", "main"])?; + repo.run([ + "remote", + "add", + "origin", + "https://github.com/upstream/repo.git", + ])?; + repo.run([ + "config", + "--add", + "url.https://github.com/fork/repo.git.pushInsteadOf", + "https://github.com/upstream/repo.git", + ])?; + assert_eq!( + Git::new(repo.path()).remote_push_urls("origin")?, + vec!["https://github.com/fork/repo.git"] + ); + Ok(()) + } + + #[test] + fn remote_push_urls_accepts_leading_hyphen_remote() -> Result<(), Box> { + let repo = TempRepo::new()?; + repo.run(["init", "-b", "main"])?; + repo.run([ + "remote", + "add", + "--", + "-fork", + "https://github.com/fork/repo.git", + ])?; + + assert_eq!( + Git::new(repo.path()).remote_push_urls("-fork")?, + vec!["https://github.com/fork/repo.git"] + ); + Ok(()) + } + + #[test] + fn push_target_uses_default_simple_in_triangular_workflow() -> Result<(), Box> { + let repo = TempRepo::new()?; + repo.run(["init", "-b", "feature"])?; + repo.run(["config", "user.email", "bitbygit@example.invalid"])?; + repo.run(["config", "user.name", "bitbygit test"])?; + repo.run(["commit", "--allow-empty", "-m", "initial"])?; + repo.run([ + "remote", + "add", + "upstream", + "https://github.com/upstream/repo.git", + ])?; + repo.run(["remote", "add", "fork", "https://github.com/fork/repo.git"])?; + repo.run(["update-ref", "refs/remotes/upstream/main", "HEAD"])?; + repo.run(["update-ref", "refs/remotes/fork/feature", "HEAD"])?; + repo.run(["config", "branch.feature.remote", "upstream"])?; + repo.run(["config", "branch.feature.merge", "refs/heads/main"])?; + repo.run(["config", "branch.feature.pushRemote", "fork"])?; + + assert_eq!( + Git::new(repo.path()).push_target("feature")?, + Some(("fork".to_owned(), "feature".to_owned())) + ); + Ok(()) + } + + #[cfg(unix)] + #[test] + fn remote_url_head_oid_ignores_configured_ssh_command() -> Result<(), Box> { + use std::os::unix::fs::PermissionsExt; + + let repo = TempRepo::new()?; + repo.run(["init", "-b", "main"])?; + let marker = repo.path().join("configured-ssh-ran"); + let configured_ssh = repo.path().join("configured-ssh"); + fs::write( + &configured_ssh, + format!("#!/bin/sh\ntouch '{}'\nexit 1\n", marker.display()), + )?; + let mut permissions = fs::metadata(&configured_ssh)?.permissions(); + permissions.set_mode(0o755); + fs::set_permissions(&configured_ssh, permissions)?; + repo.run_args(&[ + "config", + "core.sshCommand", + &configured_ssh.display().to_string(), + ])?; + + let result = + Git::new(repo.path()).remote_url_head_oid("ssh://git@127.0.0.1:1/repo.git", "main"); + + assert!(result.is_err()); + assert!(!marker.exists()); + Ok(()) + } + + #[cfg(unix)] + #[test] + fn fetch_preserves_inherited_ssh_command() -> Result<(), Box> { + use std::os::unix::fs::PermissionsExt; + + if let Some(repo) = env::var_os("BITBYGIT_TEST_INHERITED_SSH_REPO") { + Git::new(repo).fetch_remote_branch("origin", "main")?; + return Ok(()); + } + + let remote = TempRepo::new()?; + remote.run(["init", "--bare"])?; + let repo = TempRepo::new()?; + repo.run(["init", "-b", "main"])?; + repo.run(["config", "user.email", "bitbygit@example.invalid"])?; + repo.run(["config", "user.name", "bitbygit test"])?; + repo.run(["commit", "--allow-empty", "-m", "initial"])?; + let remote_path = remote.path().to_string_lossy().into_owned(); + repo.run_args(&["remote", "add", "origin", &remote_path])?; + repo.run(["push", "origin", "main"])?; + repo.run([ + "remote", + "set-url", + "origin", + "ssh://git@127.0.0.1/repo.git", + ])?; + + let inherited_marker = repo.path().join("inherited-ssh-ran"); + let inherited_ssh = repo.path().join("inherited-ssh"); + fs::write( + &inherited_ssh, + "#!/bin/sh\ntouch \"$BITBYGIT_TEST_INHERITED_SSH_MARKER\"\nexec git-upload-pack \"$BITBYGIT_TEST_INHERITED_SSH_REMOTE\"\n", + )?; + let mut permissions = fs::metadata(&inherited_ssh)?.permissions(); + permissions.set_mode(0o755); + fs::set_permissions(&inherited_ssh, permissions)?; + + let configured_marker = repo.path().join("configured-ssh-ran"); + let configured_ssh = repo.path().join("configured-ssh"); + fs::write( + &configured_ssh, + format!( + "#!/bin/sh\ntouch '{}'\nexit 1\n", + configured_marker.display() + ), + )?; + let mut permissions = fs::metadata(&configured_ssh)?.permissions(); + permissions.set_mode(0o755); + fs::set_permissions(&configured_ssh, permissions)?; + repo.run_args(&[ + "config", + "core.sshCommand", + &configured_ssh.display().to_string(), + ])?; + + let output = Command::new(env::current_exe()?) + .args([ + "--exact", + "tests::fetch_preserves_inherited_ssh_command", + "--nocapture", + ]) + .env("BITBYGIT_TEST_INHERITED_SSH_REPO", repo.path()) + .env("BITBYGIT_TEST_INHERITED_SSH_REMOTE", remote.path()) + .env("BITBYGIT_TEST_INHERITED_SSH_MARKER", &inherited_marker) + .env("GIT_SSH_COMMAND", &inherited_ssh) + .output()?; + + assert!( + output.status.success(), + "child test failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + assert!(inherited_marker.exists()); + assert!(!configured_marker.exists()); + Ok(()) + } + #[cfg(unix)] #[test] fn reads_non_utf8_repository_root() -> Result<(), Box> { diff --git a/crates/bitbygit-tui/Cargo.toml b/crates/bitbygit-tui/Cargo.toml index 117de06..6d15710 100644 --- a/crates/bitbygit-tui/Cargo.toml +++ b/crates/bitbygit-tui/Cargo.toml @@ -8,6 +8,7 @@ rust-version.workspace = true [dependencies] bitbygit-core = { path = "../bitbygit-core" } +bitbygit-gh = { path = "../bitbygit-gh" } bitbygit-git = { path = "../bitbygit-git" } bitbygit-store = { path = "../bitbygit-store" } crossterm = "0.28" diff --git a/crates/bitbygit-tui/src/lib.rs b/crates/bitbygit-tui/src/lib.rs index 9270d51..c076f9d 100644 --- a/crates/bitbygit-tui/src/lib.rs +++ b/crates/bitbygit-tui/src/lib.rs @@ -21,6 +21,7 @@ use bitbygit_core::{ RiskLevel, prompt_parser::{ParsedPrompt, parse_prompt}, }; +use bitbygit_gh::{CreatePullRequest, GhError, GitHub}; use bitbygit_git::{ BranchInfo, BranchKind, BranchState, BranchTarget, ChangeKind, Git, GitError, GitOutput, Head, HeadTarget, StatusEntry, StatusEntryType, @@ -675,6 +676,22 @@ enum PendingPayload { base: BranchTarget, target: HeadTarget, }, + OpenPullRequest { + branch: String, + upstream: String, + remote: String, + upstream_branch: String, + remote_urls: Vec, + target: HeadTarget, + base: String, + title: String, + repository: String, + github_repository: String, + head_github_repository: String, + head_repository: String, + head: String, + github_executable: Option, + }, } fn prompt_accepts_modifiers(modifiers: KeyModifiers) -> bool { @@ -1187,6 +1204,65 @@ fn rebase_plan(current: &str, base: &BranchTarget) -> OperationPlan { ) } +fn open_pull_request_plan( + request: OperationRequest, + remote: &str, + head: &str, + base: &str, + title: &str, + repository: &str, + existing_url: Option<&str>, +) -> OperationPlan { + let target = existing_url.map(ToOwned::to_owned).unwrap_or_else(|| { + format!( + "https://github.com/{repository}/compare/{}...{}?expand=1", + compare_url_ref(base), + compare_url_ref(head) + ) + }); + let summary = if existing_url.is_some() { + format!("surface existing pull request for {head}") + } else { + format!("open pull request from {head} to {base}") + }; + OperationPlan::new( + request, + "Open pull request plan", + vec![ + OperationStep::new(OperationKind::OpenPullRequest, RiskLevel::Medium, summary) + .with_detail("provider: GitHub") + .with_detail(format!("remote: {remote}")) + .with_detail(format!("head: {head}")) + .with_detail(format!("base: {base}")) + .with_detail(format!("title: {title}")) + .with_detail(format!("target: {target}")) + .with_detail( + "revalidate branch, upstream, remote, and pull request state before execution", + ), + ], + "Press y to open or surface the pull request or n to cancel.", + ) +} + +fn compare_url_ref(reference: &str) -> String { + const HEX: &[u8; 16] = b"0123456789ABCDEF"; + + let mut encoded = String::with_capacity(reference.len()); + for byte in reference.bytes() { + match byte { + b'A'..=b'Z' | b'a'..=b'z' | b'0'..=b'9' | b'-' | b'.' | b'_' | b'~' | b'/' => { + encoded.push(char::from(byte)); + } + _ => { + encoded.push('%'); + encoded.push(char::from(HEX[usize::from(byte >> 4)])); + encoded.push(char::from(HEX[usize::from(byte & 0x0f)])); + } + } + } + encoded +} + fn prompt_sequence_plan( requests: &[OperationRequest], first: &PreparedOperation, @@ -1344,6 +1420,9 @@ fn prompt_sequence_request_preview( RiskLevel::High, format!("rebase current branch onto {base}"), )), + OperationRequest::OpenPullRequest { .. } => { + Err("open pull request is only available as a single prompt".to_owned()) + } OperationRequest::RefreshStatus | OperationRequest::ViewDiff { .. } | OperationRequest::StagePaths { .. } @@ -1435,12 +1514,18 @@ fn short_oid(oid: &str) -> String { #[derive(Debug, Clone)] struct OperationPlanner { repo_root: PathBuf, + github_executable: Option, + #[cfg(test)] + ssh_executable: Option, } impl OperationPlanner { fn current() -> Self { Self { repo_root: current_dir(), + github_executable: None, + #[cfg(test)] + ssh_executable: None, } } @@ -1448,6 +1533,8 @@ impl OperationPlanner { fn new(repo_root: impl Into) -> Self { Self { repo_root: repo_root.into(), + github_executable: None, + ssh_executable: None, } } @@ -1490,6 +1577,7 @@ impl OperationPlanner { } OperationRequest::Merge { branch } => self.plan_merge(branch), OperationRequest::Rebase { base } => self.plan_rebase(base), + OperationRequest::OpenPullRequest { base } => self.plan_open_pull_request(base), OperationRequest::PromptSequence { .. } => { Err("Prompt sequences are handled by prompt submission.".to_owned()) } @@ -1818,6 +1906,124 @@ impl OperationPlanner { )) } + fn plan_open_pull_request( + &self, + requested_base: Option, + ) -> Result { + let git = self.git(); + let status = git + .status() + .map_err(|error| format!("Unable to prepare pull request plan: {error}"))?; + let branch = branch_name(&status.branch)?; + let Some(upstream) = status.branch.upstream.clone() else { + return Err("Open pull request blocked: current branch has no upstream. Push it with `push` first.".to_owned()); + }; + let (remote, upstream_branch) = git + .push_target(&branch) + .map_err(|error| format!("Unable to prepare pull request target: {error}"))? + .ok_or_else(|| { + "Open pull request blocked: unable to resolve the current branch push target." + .to_owned() + })?; + let remote_urls = git + .remote_push_urls(&remote) + .map_err(|error| format!("Unable to prepare pull request remote: {error}"))?; + let push_url = single_pull_request_push_url(&remote, &remote_urls)?; + let head_github_repository = github_repository_from_push_url(push_url).ok_or_else(|| { + format!( + "Open pull request blocked: remote {remote} does not identify a GitHub repository." + ) + })?; + let target = git + .head_target() + .map_err(|error| format!("Unable to snapshot pull request branch: {error}"))?; + let local_oid = target.oid.as_deref().ok_or_else(|| { + "Open pull request blocked: current branch needs a commit before opening a pull request." + .to_owned() + })?; + let github_repository = github_base_repository(&git, &remote, &head_github_repository)?; + let github = self.github(&github_repository); + let repository = github.repository().map_err(open_pull_request_gh_error)?; + let head_github = self.github(&head_github_repository); + let canonical_head_repository = + if head_github_repository.eq_ignore_ascii_case(&github_repository) { + repository.clone() + } else { + head_github + .repository() + .map_err(open_pull_request_gh_error)? + }; + let remote_oid = head_github + .branch_oid(&upstream_branch) + .map_err(open_pull_request_gh_error)?; + if remote_oid.as_deref() != Some(local_oid) { + return Err("Open pull request blocked: current branch is not pushed to its upstream. Push it with `push` first.".to_owned()); + } + let head_repository = canonical_head_repository.name_with_owner; + let canonical_head_github_repository = format!("github.com/{head_repository}"); + let canonical_github_repository = format!("github.com/{}", repository.name_with_owner); + let head = pull_request_head( + &canonical_head_github_repository, + &canonical_github_repository, + &upstream_branch, + )?; + let base = requested_base.unwrap_or(repository.default_branch); + if canonical_head_github_repository.eq_ignore_ascii_case(&canonical_github_repository) + && base == upstream_branch + { + return Err( + "Open pull request blocked: base branch must differ from the pull request head." + .to_owned(), + ); + } + if !github + .branch_exists(&base) + .map_err(open_pull_request_gh_error)? + { + return Err(format!( + "Open pull request blocked: base branch {base} was not found in {}.", + repository.name_with_owner + )); + } + let existing = github + .existing_pull_requests(&head) + .map_err(open_pull_request_gh_error)?; + let existing_url = + matching_pull_request(&existing, &base, &upstream_branch, &head_repository) + .map(|pull_request| pull_request.url.as_str()); + let title = branch.clone(); + let request = OperationRequest::OpenPullRequest { + base: Some(base.clone()), + }; + Ok(PreparedOperation::new( + open_pull_request_plan( + request, + &remote, + &head, + &base, + &title, + &repository.name_with_owner, + existing_url, + ), + ExecutionContext::from_payload(PendingPayload::OpenPullRequest { + branch, + upstream, + remote, + upstream_branch, + remote_urls, + target, + base, + title, + repository: repository.name_with_owner, + github_repository, + head_github_repository, + head_repository, + head, + github_executable: self.github_executable.clone(), + }), + )) + } + fn ensure_clean_branch_worktree(&self, action: &str) -> Result<(), String> { self.git() .ensure_clean_worktree(&action.to_ascii_lowercase()) @@ -1842,8 +2048,25 @@ impl OperationPlanner { } fn git(&self) -> Git { + #[cfg(test)] + if let Some(executable) = &self.ssh_executable { + return Git::with_ssh_executable(self.repo_root.clone(), executable); + } Git::new(self.repo_root.clone()) } + + fn github(&self, repository: &str) -> GitHub { + match &self.github_executable { + Some(executable) => { + GitHub::with_executable_and_repository(&self.repo_root, executable, repository) + } + None => GitHub::with_executable_and_repository(&self.repo_root, "gh", repository), + } + } +} + +fn open_pull_request_gh_error(error: GhError) -> String { + format!("Open pull request blocked: {error}") } fn single_push_url<'a>(remote: &str, urls: &'a [String]) -> Result<&'a str, String> { @@ -1858,6 +2081,143 @@ fn single_push_url<'a>(remote: &str, urls: &'a [String]) -> Result<&'a str, Stri } } +fn single_pull_request_push_url<'a>(remote: &str, urls: &'a [String]) -> Result<&'a str, String> { + match urls { + [url] => Ok(url), + [] => Err(format!( + "Open pull request blocked: remote {remote} has no push URL." + )), + _ => Err(format!( + "Open pull request blocked: remote {remote} has multiple push URLs." + )), + } +} + +fn github_repository_from_push_url(url: &str) -> Option { + let (host, path) = if let Some((scheme, rest)) = url.split_once("://") { + if !scheme.eq_ignore_ascii_case("https") && !scheme.eq_ignore_ascii_case("ssh") { + return None; + } + let (authority, path) = rest.split_once('/')?; + ( + authority + .rsplit_once('@') + .map_or(authority, |(_, host)| host), + path, + ) + } else { + let (authority, path) = url.split_once(':')?; + if authority.contains('/') { + return None; + } + ( + authority + .rsplit_once('@') + .map_or(authority, |(_, host)| host), + path, + ) + }; + if !host.eq_ignore_ascii_case("github.com") { + return None; + } + let mut components = path.trim_end_matches('/').split('/'); + let owner = components.next()?; + let repository = components.next()?; + let repository = repository.strip_suffix(".git").unwrap_or(repository); + if host.is_empty() + || owner.is_empty() + || repository.is_empty() + || components.next().is_some() + || owner.contains(['?', '#']) + || repository.contains(['?', '#']) + { + return None; + } + Some(format!("github.com/{owner}/{repository}")) +} + +fn github_base_repository( + git: &Git, + push_remote: &str, + head_repository: &str, +) -> Result { + let remotes = git + .remotes() + .map_err(|error| format!("Unable to prepare pull request base remote: {error}"))?; + if let Some(fetch_repository) = remotes + .iter() + .find(|remote| remote.name == push_remote) + .and_then(|remote| remote.fetch_url.as_deref()) + .and_then(github_repository_from_push_url) + .filter(|repository| !repository.eq_ignore_ascii_case(head_repository)) + { + return Ok(fetch_repository); + } + let upstream = remotes.into_iter().find(|remote| remote.name == "upstream"); + let Some(upstream) = upstream else { + return Ok(head_repository.to_owned()); + }; + let url = upstream + .fetch_url + .as_deref() + .or(upstream.push_url.as_deref()) + .ok_or_else(|| { + "Open pull request blocked: upstream remote has no URL; configure the base repository or remove the remote." + .to_owned() + })?; + github_repository_from_push_url(url).ok_or_else(|| { + "Open pull request blocked: upstream remote does not identify a GitHub repository." + .to_owned() + }) +} + +fn pull_request_head( + head_repository: &str, + base_repository: &str, + branch: &str, +) -> Result { + if head_repository.eq_ignore_ascii_case(base_repository) { + return Ok(branch.to_owned()); + } + let (head_host, head_name) = head_repository.split_once('/').ok_or_else(|| { + "Open pull request blocked: source remote does not identify a GitHub repository.".to_owned() + })?; + let (base_host, _) = base_repository.split_once('/').ok_or_else(|| { + "Open pull request blocked: upstream remote does not identify a GitHub repository." + .to_owned() + })?; + if !head_host.eq_ignore_ascii_case(base_host) { + return Err( + "Open pull request blocked: source and upstream remotes use different GitHub hosts." + .to_owned(), + ); + } + let (owner, _) = head_name.split_once('/').ok_or_else(|| { + "Open pull request blocked: source remote does not identify a GitHub repository.".to_owned() + })?; + Ok(format!("{owner}:{branch}")) +} + +fn matching_pull_request<'a>( + pull_requests: &'a [bitbygit_gh::PullRequest], + base: &str, + branch: &str, + head_repository: &str, +) -> Option<&'a bitbygit_gh::PullRequest> { + pull_requests.iter().find(|pull_request| { + pull_request.base_ref_name == base + && pull_request.head_ref_name == branch + && pull_request + .head_repository + .as_ref() + .is_some_and(|pull_request_head_repository| { + pull_request_head_repository + .name_with_owner + .eq_ignore_ascii_case(head_repository) + }) + }) +} + fn validate_push_plan( git: &Git, branch: &str, @@ -1932,6 +2292,55 @@ fn status_visible_len(area: Rect) -> usize { area.height.saturating_sub(2).max(1) as usize } +fn validate_open_pull_request_plan( + git: &Git, + branch: &str, + expected_upstream: &str, + remote: &str, + upstream_branch: &str, + remote_urls: &[String], + target: &HeadTarget, +) -> Result<(), StepExecutionError> { + if git.head_target().map_err(StepExecutionError::Git)? != *target { + return Err(StepExecutionError::Blocked( + "Open pull request blocked: branch target changed since the plan was shown.".to_owned(), + )); + } + let status = git.status().map_err(StepExecutionError::Git)?; + if branch_name(&status.branch).map_err(StepExecutionError::Blocked)? != branch { + return Err(StepExecutionError::Blocked( + "Open pull request blocked: current branch changed since the plan was shown." + .to_owned(), + )); + } + if status.branch.upstream.as_deref() != Some(expected_upstream) { + return Err(StepExecutionError::Blocked( + "Open pull request blocked: upstream changed since the plan was shown.".to_owned(), + )); + } + let current_target = git.push_target(branch).map_err(StepExecutionError::Git)?; + if current_target + .as_ref() + .map(|(name, branch)| (name.as_str(), branch.as_str())) + != Some((remote, upstream_branch)) + { + return Err(StepExecutionError::Blocked( + "Open pull request blocked: push target changed since the plan was shown.".to_owned(), + )); + } + let current_remote_urls = git + .remote_push_urls(remote) + .map_err(StepExecutionError::Git)?; + if current_remote_urls != remote_urls { + return Err(StepExecutionError::Blocked( + "Open pull request blocked: remote URLs changed since the plan was shown.".to_owned(), + )); + } + single_pull_request_push_url(remote, ¤t_remote_urls) + .map_err(StepExecutionError::Blocked)?; + Ok(()) +} + fn status_file_visible_len(area: Rect) -> usize { status_visible_len(area).saturating_sub(1) } @@ -2087,6 +2496,8 @@ impl ExecutionContext { struct PlanExecutor { repo_root: PathBuf, audit: AuditDestination, + #[cfg(test)] + ssh_executable: Option, } impl PlanExecutor { @@ -2094,6 +2505,8 @@ impl PlanExecutor { Self { repo_root: current_dir(), audit: AuditDestination::Environment, + #[cfg(test)] + ssh_executable: None, } } @@ -2102,6 +2515,20 @@ impl PlanExecutor { Self { repo_root: repo_root.into(), audit: AuditDestination::Paths(paths), + ssh_executable: None, + } + } + + #[cfg(test)] + fn with_audit_paths_and_ssh( + repo_root: impl Into, + paths: StorePaths, + ssh_executable: impl Into, + ) -> Self { + Self { + repo_root: repo_root.into(), + audit: AuditDestination::Paths(paths), + ssh_executable: Some(ssh_executable.into()), } } @@ -2193,6 +2620,7 @@ impl PlanExecutor { OperationKind::CreateBranch => self.run_create_branch_step(plan, context, &git), OperationKind::MergeFastForward => self.run_merge_step(plan, context, &git), OperationKind::Rebase => self.run_rebase_step(plan, context, &git), + OperationKind::OpenPullRequest => self.run_open_pull_request_step(plan, context, &git), OperationKind::RefreshStatus | OperationKind::ViewDiff => { Err(StepExecutionError::Unsupported(format!( "{} step is not executable by the typed operation executor", @@ -2203,6 +2631,10 @@ impl PlanExecutor { } fn git(&self) -> Git { + #[cfg(test)] + if let Some(executable) = &self.ssh_executable { + return Git::with_ssh_executable(self.repo_root.clone(), executable); + } Git::new(self.repo_root.clone()) } @@ -2468,6 +2900,180 @@ impl PlanExecutor { } git_output(git.rebase_onto(target, head)) } + + fn run_open_pull_request_step( + &self, + plan: &OperationPlan, + context: &ExecutionContext, + git: &Git, + ) -> StepRunResult { + let OperationRequest::OpenPullRequest { + base: Some(requested_base), + } = &plan.request + else { + return Err(StepExecutionError::Unsupported( + "open pull request step requires a typed pull request request".to_owned(), + )); + }; + let PendingPayload::OpenPullRequest { + branch, + upstream, + remote, + upstream_branch, + remote_urls, + target, + base, + title, + repository, + github_repository, + head_github_repository, + head_repository, + head, + github_executable, + } = typed_payload(context, OperationKind::OpenPullRequest)? + else { + return Err(mismatched_context(OperationKind::OpenPullRequest)); + }; + if base != requested_base { + return Err(StepExecutionError::Unsupported( + "open pull request request does not match its typed execution context".to_owned(), + )); + } + validate_open_pull_request_plan( + git, + branch, + upstream, + remote, + upstream_branch, + remote_urls, + target, + )?; + if !github_base_repository(git, remote, head_github_repository) + .map_err(StepExecutionError::Blocked)? + .eq_ignore_ascii_case(github_repository) + { + return Err(StepExecutionError::Blocked( + "Open pull request blocked: GitHub base repository changed since the plan was shown." + .to_owned(), + )); + } + let github = match github_executable { + Some(executable) => GitHub::with_executable_and_repository( + &self.repo_root, + executable, + github_repository, + ), + None => { + GitHub::with_executable_and_repository(&self.repo_root, "gh", github_repository) + } + }; + let current_repository = github.repository().map_err(StepExecutionError::GitHub)?; + if !current_repository + .name_with_owner + .eq_ignore_ascii_case(repository) + { + return Err(StepExecutionError::Blocked( + "Open pull request blocked: GitHub repository changed since the plan was shown." + .to_owned(), + )); + } + let head_github = match github_executable { + Some(executable) => GitHub::with_executable_and_repository( + &self.repo_root, + executable, + head_github_repository, + ), + None => GitHub::with_executable_and_repository( + &self.repo_root, + "gh", + head_github_repository, + ), + }; + let current_head_repository = + if head_github_repository.eq_ignore_ascii_case(github_repository) { + current_repository.clone() + } else { + head_github + .repository() + .map_err(StepExecutionError::GitHub)? + }; + if !current_head_repository + .name_with_owner + .eq_ignore_ascii_case(head_repository) + { + return Err(StepExecutionError::Blocked( + "Open pull request blocked: GitHub head repository changed since the plan was shown." + .to_owned(), + )); + } + let local_oid = target.oid.as_deref().ok_or_else(|| { + StepExecutionError::Blocked( + "Open pull request blocked: planned branch has no commit.".to_owned(), + ) + })?; + let remote_oid = head_github + .branch_oid(upstream_branch) + .map_err(StepExecutionError::GitHub)?; + if remote_oid.as_deref() != Some(local_oid) { + return Err(StepExecutionError::Blocked( + "Open pull request blocked: current branch is no longer pushed to its upstream." + .to_owned(), + )); + } + let current_head = pull_request_head( + &format!("github.com/{}", current_head_repository.name_with_owner), + &format!("github.com/{}", current_repository.name_with_owner), + upstream_branch, + ) + .map_err(StepExecutionError::Blocked)?; + if current_head != *head { + return Err(StepExecutionError::Blocked( + "Open pull request blocked: GitHub head changed since the plan was shown." + .to_owned(), + )); + } + let existing = github + .existing_pull_requests(head) + .map_err(StepExecutionError::GitHub)?; + if let Some(existing) = + matching_pull_request(&existing, base, upstream_branch, head_repository) + { + return Ok(ExecutionOutput::PullRequest { + url: existing.url.clone(), + existing: true, + }); + } + if !github + .branch_exists(base) + .map_err(StepExecutionError::GitHub)? + { + return Err(StepExecutionError::Blocked(format!( + "Open pull request blocked: base branch {base} no longer exists in {repository}." + ))); + } + match github.create_pull_request(&CreatePullRequest { + title: title.clone(), + body: String::new(), + base: base.clone(), + head: head.clone(), + }) { + Ok(created) => Ok(ExecutionOutput::PullRequest { + url: created.url, + existing: false, + }), + Err(error) => match github.existing_pull_requests(head) { + Ok(existing) => { + matching_pull_request(&existing, base, upstream_branch, head_repository) + .map(|pull_request| ExecutionOutput::PullRequest { + url: pull_request.url.clone(), + existing: true, + }) + .ok_or(StepExecutionError::GitHub(error)) + } + Err(_) => Err(StepExecutionError::GitHub(error)), + }, + } + } } #[derive(Debug, Clone)] @@ -2496,10 +3102,15 @@ impl PromptSequenceExecutor { let total_steps = sequence.total_steps(); let planner = OperationPlanner { repo_root: self.repo_root.clone(), + github_executable: None, + #[cfg(test)] + ssh_executable: None, }; let executor = PlanExecutor { repo_root: self.repo_root.clone(), audit: self.audit.clone(), + #[cfg(test)] + ssh_executable: None, }; let mut step_results = Vec::with_capacity(total_steps); @@ -2787,11 +3398,13 @@ type StepRunResult = Result; enum ExecutionOutput { Git(GitOutput), Branches(Vec), + PullRequest { url: String, existing: bool }, } #[derive(Debug)] enum StepExecutionError { Git(GitError), + GitHub(GhError), Blocked(String), Unsupported(String), } @@ -2800,6 +3413,7 @@ impl StepExecutionError { fn audit_message(&self) -> String { match self { Self::Git(error) => sanitized_git_error(error), + Self::GitHub(error) => sanitized_github_error(error), Self::Blocked(message) => format!("operation blocked: {message}"), Self::Unsupported(message) => message.clone(), } @@ -2810,6 +3424,7 @@ impl std::fmt::Display for StepExecutionError { fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { Self::Git(error) => write!(formatter, "{error}"), + Self::GitHub(error) => write!(formatter, "{error}"), Self::Blocked(message) | Self::Unsupported(message) => write!(formatter, "{message}"), } } @@ -2891,6 +3506,13 @@ fn mismatched_context(kind: OperationKind) -> StepExecutionError { fn success_message(kind: OperationKind, action: &str, output: &ExecutionOutput) -> String { match output { ExecutionOutput::Branches(branches) => branch_list_message(branches), + ExecutionOutput::PullRequest { url, existing } => { + if *existing { + format!("Pull request already open: {url}") + } else { + format!("Pull request created: {url}") + } + } ExecutionOutput::Git(output) => { let output = git_output_text(output); if should_show_git_output(kind) && !output.is_empty() { @@ -2913,6 +3535,9 @@ fn success_with_audit_error_message( ExecutionOutput::Branches(_) => { format!("{action} succeeded, but audit finalization failed: {audit_error}\n{success}") } + ExecutionOutput::PullRequest { .. } => { + format!("{success}, but audit finalization failed: {audit_error}") + } ExecutionOutput::Git(output) => { let output = git_output_text(output); if should_show_git_output(kind) && !output.is_empty() { @@ -3124,6 +3749,19 @@ fn sanitized_git_error(error: &GitError) -> String { } } +fn sanitized_github_error(error: &GhError) -> String { + match error { + GhError::MissingCli | GhError::NotAuthenticated | GhError::InvalidInput { .. } => { + error.to_string() + } + GhError::CommandFailed { status } => format!("GitHub CLI failed with status {status}"), + GhError::Io { .. } => "GitHub CLI failed before execution".to_owned(), + GhError::InvalidOutput { operation } => { + format!("GitHub CLI returned invalid output for {operation}") + } + } +} + #[cfg(test)] mod tests { use super::*; @@ -3558,27 +4196,793 @@ mod tests { } #[test] - fn operation_plans_use_guardrail_risk_levels() { - let branch = BranchTarget { - name: "feature/auth".to_owned(), - reference: "refs/heads/feature/auth".to_owned(), - oid: "abcdef1234567890".to_owned(), - kind: BranchKind::Local, + fn open_pull_request_plan_previews_target_and_creates_pull_request() + -> Result<(), Box> { + let repo = pushed_branch_repo("open-pr-create")?; + let fake_gh = fake_gh("open-pr-create", false)?; + let planner = OperationPlanner { + repo_root: repo.clone(), + github_executable: Some(fake_gh.clone()), + ssh_executable: Some(test_ssh_command()?), }; - assert_eq!(fetch_plan().confirmation.risk_level, RiskLevel::Low); + let operation = planner + .plan_request(OperationRequest::OpenPullRequest { base: None }) + .map_err(std::io::Error::other)?; + + let preview = operation.plan.preview_text(); + assert!(preview.contains("provider: GitHub")); + assert!(preview.contains("remote: origin")); + assert!(preview.contains("head: feature/open-pr")); + assert!(preview.contains("base: main")); + assert!(preview.contains("title: feature/open-pr")); + assert!(preview.contains("https://github.com/octo/repo/compare/main...feature/open-pr")); assert_eq!( - stage_paths_plan(vec!["file.txt".to_owned()]) - .confirmation - .risk_level, - RiskLevel::Low + operation.plan.confirmation.requirement, + ConfirmationRequirement::VisiblePlan ); + let paths = isolated_store_paths("open-pr-create-audit")?; + let execution = + PlanExecutor::with_audit_paths_and_ssh(&repo, paths.clone(), test_ssh_command()?) + .execute(&operation.plan, operation.context); + + assert!(execution.succeeded(), "{}", execution.message()); assert_eq!( - commit_plan("message", 1).confirmation.risk_level, - RiskLevel::Medium + execution.message(), + "Pull request created: https://github.com/octo/repo/pull/43" ); - assert_eq!( - push_plan("feature/auth", "origin/feature/auth", 1) + let entries = LocalStore::open(paths)?.list_audit_entries()?; + assert_eq!(entries[0].operation, "open_pull_request"); + assert!( + entries + .iter() + .all(|entry| !entry.message.contains("pull/43")) + ); + Ok(()) + } + + #[cfg(unix)] + #[test] + fn private_https_pull_request_uses_authenticated_api_not_repository_credential_helpers() + -> Result<(), Box> { + let repo = pushed_branch_repo("open-pr-private-https")?; + git_stdout( + &repo, + &[ + "remote", + "set-url", + "origin", + "https://github.com/octo/repo.git", + ], + )?; + let marker = repo.join("credential-helper-ran"); + git_stdout( + &repo, + &[ + "config", + "credential.helper", + &format!("!touch '{}'", marker.display()), + ], + )?; + let planner = OperationPlanner { + repo_root: repo.clone(), + github_executable: Some(fake_gh("open-pr-private-https", false)?), + ssh_executable: Some(test_ssh_command()?), + }; + + let operation = planner + .plan_request(OperationRequest::OpenPullRequest { base: None }) + .map_err(std::io::Error::other)?; + + assert!(operation.plan.preview_text().contains("base: main")); + assert!(!marker.exists()); + Ok(()) + } + + #[test] + fn pull_request_uses_differently_named_tracked_remote_branch() -> Result<(), Box> { + let repo = pushed_branch_repo("open-pr-different-upstream-branch")?; + git_stdout_with_ssh( + &repo, + &["push", "origin", "HEAD:review-feature"], + &test_ssh_command()?, + )?; + git_stdout_with_ssh( + &repo, + &["push", "origin", "--delete", "feature/open-pr"], + &test_ssh_command()?, + )?; + git_stdout( + &repo, + &[ + "branch", + "--set-upstream-to=origin/review-feature", + "feature/open-pr", + ], + )?; + let fake_gh = fake_gh_with_pull_requests( + "open-pr-different-upstream-branch", + "[{\"number\":42,\"html_url\":\"https://github.com/octo/repo/pull/42\",\"title\":\"Existing PR\",\"base\":{\"ref\":\"main\"},\"head\":{\"ref\":\"review-feature\",\"repo\":{\"full_name\":\"octo/repo\"}}}]", + true, + )?; + let planner = OperationPlanner { + repo_root: repo.clone(), + github_executable: Some(fake_gh.clone()), + ssh_executable: Some(test_ssh_command()?), + }; + + let operation = planner + .plan_request(OperationRequest::OpenPullRequest { base: None }) + .map_err(std::io::Error::other)?; + + let preview = operation.plan.preview_text(); + assert!(preview.contains("head: review-feature")); + assert!(preview.contains("title: feature/open-pr")); + assert!(preview.contains("target: https://github.com/octo/repo/pull/42")); + let execution = PlanExecutor::with_audit_paths_and_ssh( + &repo, + isolated_store_paths("open-pr-different-upstream-branch-audit")?, + test_ssh_command()?, + ) + .execute(&operation.plan, operation.context); + + assert!(execution.succeeded(), "{}", execution.message()); + assert!(execution.message().contains("Pull request already open")); + let invocations = std::fs::read_to_string(fake_gh.with_file_name("invocations"))?; + assert!(invocations.contains("head=octo:review-feature")); + assert!(!invocations.contains("head=octo:feature/open-pr")); + assert!(!invocations.contains("pr:create")); + Ok(()) + } + + #[test] + fn pull_request_head_revalidation_failure_does_not_persist_remote_credentials() + -> Result<(), Box> { + let repo = pushed_branch_repo("open-pr-credential-audit")?; + git_stdout( + &repo, + &[ + "remote", + "set-url", + "origin", + "ssh://abc123@github.com/octo/repo.git", + ], + )?; + let fake_gh = fake_gh("open-pr-credential-audit", false)?; + let planner = OperationPlanner { + repo_root: repo.clone(), + github_executable: Some(fake_gh.clone()), + ssh_executable: Some(test_ssh_command()?), + }; + let operation = planner + .plan_request(OperationRequest::OpenPullRequest { base: None }) + .map_err(std::io::Error::other)?; + std::fs::write(fake_gh.with_file_name("head-missing"), "")?; + let paths = isolated_store_paths("open-pr-credential-audit")?; + + let execution = + PlanExecutor::with_audit_paths_and_ssh(&repo, paths.clone(), test_ssh_command()?) + .execute(&operation.plan, operation.context); + + assert!(!execution.succeeded()); + assert!(execution.message().contains("no longer pushed")); + let entries = LocalStore::open(paths)?.list_audit_entries()?; + assert_eq!(entries.len(), 2); + assert_eq!(entries[1].result, "error"); + assert!(entries[1].message.contains("no longer pushed")); + assert!(!entries[1].message.contains("abc123")); + assert!(!entries[1].message.contains("github.com/octo/repo")); + Ok(()) + } + + #[test] + fn existing_pull_request_is_surfaced_without_creation() -> Result<(), Box> { + let repo = pushed_branch_repo("open-pr-existing")?; + let fake_gh = fake_gh("open-pr-existing", true)?; + let planner = OperationPlanner { + repo_root: repo.clone(), + github_executable: Some(fake_gh.clone()), + ssh_executable: Some(test_ssh_command()?), + }; + let operation = planner + .plan_request(OperationRequest::OpenPullRequest { base: None }) + .map_err(std::io::Error::other)?; + + assert!( + operation + .plan + .preview_text() + .contains("target: https://github.com/octo/repo/pull/42") + ); + let execution = PlanExecutor::with_audit_paths_and_ssh( + &repo, + isolated_store_paths("open-pr-existing-audit")?, + test_ssh_command()?, + ) + .execute(&operation.plan, operation.context); + + assert!(execution.succeeded(), "{}", execution.message()); + assert_eq!( + execution.message(), + "Pull request already open: https://github.com/octo/repo/pull/42" + ); + assert!( + !std::fs::read_to_string(fake_gh.with_file_name("invocations"))?.contains("pr:create") + ); + Ok(()) + } + + #[test] + fn existing_pull_request_repository_match_is_case_insensitive() -> Result<(), Box> { + let repo = pushed_branch_repo("open-pr-existing-mixed-case")?; + git_stdout( + &repo, + &[ + "remote", + "set-url", + "origin", + "ssh://git@github.com/OCTO/REPO.git", + ], + )?; + let fake_gh = fake_gh("open-pr-existing-mixed-case", true)?; + let planner = OperationPlanner { + repo_root: repo.clone(), + github_executable: Some(fake_gh.clone()), + ssh_executable: Some(test_ssh_command()?), + }; + + let operation = planner + .plan_request(OperationRequest::OpenPullRequest { base: None }) + .map_err(std::io::Error::other)?; + + assert!(operation.plan.preview_text().contains("pull/42")); + let execution = PlanExecutor::with_audit_paths_and_ssh( + &repo, + isolated_store_paths("open-pr-existing-mixed-case-audit")?, + test_ssh_command()?, + ) + .execute(&operation.plan, operation.context); + assert!(execution.succeeded(), "{}", execution.message()); + assert!(execution.message().contains("already open")); + assert!( + !std::fs::read_to_string(fake_gh.with_file_name("invocations"))?.contains("pr:create") + ); + Ok(()) + } + + #[test] + fn renamed_head_repository_surfaces_existing_pull_request() -> Result<(), Box> { + let repo = pushed_branch_repo("open-pr-renamed-head")?; + git_stdout( + &repo, + &[ + "remote", + "set-url", + "origin", + "ssh://git@github.com/octo/old-repo.git", + ], + )?; + let fake_gh = fake_gh("open-pr-renamed-head", true)?; + let planner = OperationPlanner { + repo_root: repo.clone(), + github_executable: Some(fake_gh.clone()), + ssh_executable: Some(test_ssh_command()?), + }; + + let operation = planner + .plan_request(OperationRequest::OpenPullRequest { base: None }) + .map_err(std::io::Error::other)?; + + assert!(operation.plan.preview_text().contains("pull/42")); + let execution = PlanExecutor::with_audit_paths_and_ssh( + &repo, + isolated_store_paths("open-pr-renamed-head-audit")?, + test_ssh_command()?, + ) + .execute(&operation.plan, operation.context); + assert!(execution.succeeded(), "{}", execution.message()); + assert!(execution.message().contains("already open")); + assert!( + !std::fs::read_to_string(fake_gh.with_file_name("invocations"))?.contains("pr:create") + ); + Ok(()) + } + + #[test] + fn open_pull_request_plan_rejects_missing_base_branch() -> Result<(), Box> { + let repo = pushed_branch_repo("open-pr-missing-base")?; + let fake_gh = fake_gh("open-pr-missing-base", false)?; + let planner = OperationPlanner { + repo_root: repo, + github_executable: Some(fake_gh), + ssh_executable: Some(test_ssh_command()?), + }; + + let error = match planner.plan_request(OperationRequest::OpenPullRequest { + base: Some("missing".to_owned()), + }) { + Ok(_) => return Err(std::io::Error::other("missing base must fail closed").into()), + Err(error) => error, + }; + + assert!(error.contains("base branch missing was not found")); + Ok(()) + } + + #[test] + fn open_pull_request_execution_rejects_deleted_base_branch() -> Result<(), Box> { + let repo = pushed_branch_repo("open-pr-deleted-base")?; + let fake_gh = fake_gh("open-pr-deleted-base", false)?; + let planner = OperationPlanner { + repo_root: repo.clone(), + github_executable: Some(fake_gh.clone()), + ssh_executable: Some(test_ssh_command()?), + }; + let operation = planner + .plan_request(OperationRequest::OpenPullRequest { base: None }) + .map_err(std::io::Error::other)?; + std::fs::write(fake_gh.with_file_name("base-missing"), "")?; + + let execution = PlanExecutor::with_audit_paths_and_ssh( + &repo, + isolated_store_paths("open-pr-deleted-base-audit")?, + test_ssh_command()?, + ) + .execute(&operation.plan, operation.context); + + assert!(!execution.succeeded()); + assert!( + execution + .message() + .contains("base branch main no longer exists") + ); + assert!( + !std::fs::read_to_string(fake_gh.with_file_name("invocations"))?.contains("pr:create") + ); + Ok(()) + } + + #[cfg(unix)] + #[test] + fn open_pull_request_plan_never_launches_ext_transport() -> Result<(), Box> { + let repo = pushed_branch_repo("open-pr-ext-transport")?; + let marker = repo.join("ext-ran"); + let push_url = format!("ext::touch {}", marker.display()); + git_stdout(&repo, &["remote", "set-url", "--push", "origin", &push_url])?; + git_stdout(&repo, &["config", "protocol.ext.allow", "always"])?; + + let error = match OperationPlanner::new(&repo) + .plan_request(OperationRequest::OpenPullRequest { base: None }) + { + Ok(_) => return Err(std::io::Error::other("ext transport must fail closed").into()), + Err(error) => error, + }; + + assert!(error.contains("does not identify a GitHub repository")); + assert!(!marker.exists()); + Ok(()) + } + + #[test] + fn pull_request_for_other_base_is_not_surfaced_after_create_failure() + -> Result<(), Box> { + let repo = pushed_branch_repo("open-pr-other-base")?; + let fake_gh = fake_gh_with_pull_requests( + "open-pr-other-base", + "[{\"number\":42,\"html_url\":\"https://github.com/octo/repo/pull/42\",\"title\":\"Existing PR\",\"base\":{\"ref\":\"main\"},\"head\":{\"ref\":\"feature/open-pr\",\"repo\":{\"full_name\":\"octo/repo\"}}}]", + false, + )?; + let planner = OperationPlanner { + repo_root: repo.clone(), + github_executable: Some(fake_gh.clone()), + ssh_executable: Some(test_ssh_command()?), + }; + let operation = planner + .plan_request(OperationRequest::OpenPullRequest { + base: Some("release".to_owned()), + }) + .map_err(std::io::Error::other)?; + + assert!( + operation + .plan + .preview_text() + .contains("https://github.com/octo/repo/compare/release...feature/open-pr") + ); + let execution = PlanExecutor::with_audit_paths_and_ssh( + &repo, + isolated_store_paths("open-pr-other-base-audit")?, + test_ssh_command()?, + ) + .execute(&operation.plan, operation.context); + + assert!(!execution.succeeded()); + assert!(!execution.message().contains("pull/42")); + assert!( + std::fs::read_to_string(fake_gh.with_file_name("invocations"))? + .contains("--base release") + ); + Ok(()) + } + + #[test] + fn pull_request_from_another_fork_is_not_surfaced() -> Result<(), Box> { + let repo = pushed_branch_repo("open-pr-fork-collision")?; + let fake_gh = fake_gh_with_pull_requests( + "open-pr-fork-collision", + "[{\"number\":42,\"html_url\":\"https://github.com/bob/repo/pull/42\",\"title\":\"Other fork PR\",\"base\":{\"ref\":\"main\"},\"head\":{\"ref\":\"feature/open-pr\",\"repo\":{\"full_name\":\"bob/repo\"}}}]", + true, + )?; + let planner = OperationPlanner { + repo_root: repo.clone(), + github_executable: Some(fake_gh.clone()), + ssh_executable: Some(test_ssh_command()?), + }; + let operation = planner + .plan_request(OperationRequest::OpenPullRequest { base: None }) + .map_err(std::io::Error::other)?; + + assert!(!operation.plan.preview_text().contains("pull/42")); + let execution = PlanExecutor::with_audit_paths_and_ssh( + &repo, + isolated_store_paths("open-pr-fork-collision-audit")?, + test_ssh_command()?, + ) + .execute(&operation.plan, operation.context); + + assert!(execution.succeeded(), "{}", execution.message()); + assert_eq!( + execution.message(), + "Pull request created: https://github.com/octo/repo/pull/43" + ); + assert!( + std::fs::read_to_string(fake_gh.with_file_name("invocations"))?.contains("pr:create") + ); + Ok(()) + } + + #[test] + fn pull_request_gh_commands_target_upstream_for_a_tracked_fork() -> Result<(), Box> { + let repo = pushed_branch_repo("open-pr-tracked-fork")?; + git_stdout(&repo, &["remote", "rename", "origin", "fork"])?; + git_stdout( + &repo, + &[ + "remote", + "add", + "upstream", + "https://github.com/upstream/repo.git", + ], + )?; + let fake_gh = fake_gh("open-pr-tracked-fork", false)?; + let planner = OperationPlanner { + repo_root: repo.clone(), + github_executable: Some(fake_gh.clone()), + ssh_executable: Some(test_ssh_command()?), + }; + + let operation = planner + .plan_request(OperationRequest::OpenPullRequest { base: None }) + .map_err(std::io::Error::other)?; + assert!(operation.plan.preview_text().contains("remote: fork")); + assert!( + operation + .plan + .preview_text() + .contains("https://github.com/upstream/repo/compare/main...octo%3Afeature/open-pr"), + "{}", + operation.plan.preview_text() + ); + let execution = PlanExecutor::with_audit_paths_and_ssh( + &repo, + isolated_store_paths("open-pr-tracked-fork-audit")?, + test_ssh_command()?, + ) + .execute(&operation.plan, operation.context); + + assert!(execution.succeeded(), "{}", execution.message()); + let invocations = std::fs::read_to_string(fake_gh.with_file_name("invocations"))?; + let repo_views = invocations + .lines() + .filter(|line| line.starts_with("repo:view")) + .collect::>(); + assert!( + repo_views + .iter() + .any(|invocation| invocation.contains("view github.com/upstream/repo")) + ); + assert!( + repo_views + .iter() + .any(|invocation| invocation.contains("view github.com/octo/repo")) + ); + assert!( + repo_views + .iter() + .all(|invocation| !invocation.contains("--repo")) + ); + for invocation in invocations + .lines() + .filter(|line| line.starts_with("api:--method") && line.contains("/pulls")) + { + assert!(invocation.contains("repos/upstream/repo/pulls")); + assert!(invocation.contains("head=octo:feature/open-pr")); + } + for invocation in invocations + .lines() + .filter(|line| line.starts_with("pr:create")) + { + assert!(invocation.contains("--repo github.com/upstream/repo")); + assert!(invocation.contains("octo:feature/open-pr")); + } + Ok(()) + } + + #[test] + fn pull_request_targets_fetch_repository_for_a_split_remote() -> Result<(), Box> { + let repo = pushed_branch_repo("open-pr-split-remote")?; + git_stdout( + &repo, + &[ + "remote", + "set-url", + "origin", + "https://github.com/upstream/repo.git", + ], + )?; + git_stdout( + &repo, + &[ + "config", + "--add", + "url.https://github.com/octo/repo.git.pushInsteadOf", + "https://github.com/upstream/repo.git", + ], + )?; + let fake_gh = fake_gh("open-pr-split-remote", false)?; + let planner = OperationPlanner { + repo_root: repo.clone(), + github_executable: Some(fake_gh.clone()), + ssh_executable: Some(test_ssh_command()?), + }; + + let operation = planner + .plan_request(OperationRequest::OpenPullRequest { base: None }) + .map_err(std::io::Error::other)?; + + let preview = operation.plan.preview_text(); + assert!(preview.contains("remote: origin")); + assert!(preview.contains("head: octo:feature/open-pr")); + assert!(preview.contains("https://github.com/upstream/repo/compare/main")); + let execution = PlanExecutor::with_audit_paths_and_ssh( + &repo, + isolated_store_paths("open-pr-split-remote-audit")?, + test_ssh_command()?, + ) + .execute(&operation.plan, operation.context); + + assert!(execution.succeeded(), "{}", execution.message()); + let invocations = std::fs::read_to_string(fake_gh.with_file_name("invocations"))?; + assert!(invocations.contains("repos/upstream/repo/pulls")); + assert!(invocations.contains("head=octo:feature/open-pr")); + assert!(invocations.contains("--repo github.com/upstream/repo")); + Ok(()) + } + + #[test] + fn pull_request_uses_default_simple_in_triangular_workflow() -> Result<(), Box> { + let repo = pushed_branch_repo("open-pr-triangular-fork")?; + git_stdout(&repo, &["remote", "rename", "origin", "fork"])?; + git_stdout( + &repo, + &[ + "remote", + "add", + "upstream", + "https://github.com/upstream/repo.git", + ], + )?; + git_stdout( + &repo, + &["update-ref", "refs/remotes/upstream/main", "HEAD^"], + )?; + git_stdout( + &repo, + &["config", "branch.feature/open-pr.remote", "upstream"], + )?; + git_stdout( + &repo, + &["config", "branch.feature/open-pr.merge", "refs/heads/main"], + )?; + git_stdout( + &repo, + &["config", "branch.feature/open-pr.pushRemote", "fork"], + )?; + let fake_gh = fake_gh("open-pr-triangular-fork", false)?; + let planner = OperationPlanner { + repo_root: repo.clone(), + github_executable: Some(fake_gh.clone()), + ssh_executable: Some(test_ssh_command()?), + }; + + let operation = planner + .plan_request(OperationRequest::OpenPullRequest { base: None }) + .map_err(std::io::Error::other)?; + + let preview = operation.plan.preview_text(); + assert!(preview.contains("remote: fork")); + assert!(preview.contains("head: octo:feature/open-pr")); + assert!(preview.contains("https://github.com/upstream/repo/compare/main")); + let execution = PlanExecutor::with_audit_paths_and_ssh( + &repo, + isolated_store_paths("open-pr-triangular-fork-audit")?, + test_ssh_command()?, + ) + .execute(&operation.plan, operation.context); + assert!(execution.succeeded(), "{}", execution.message()); + let invocations = std::fs::read_to_string(fake_gh.with_file_name("invocations"))?; + assert!(invocations.contains("head=octo:feature/open-pr")); + assert!(invocations.contains("pr:create")); + Ok(()) + } + + #[test] + fn pull_request_accepts_suffixless_head_and_upstream_remotes() -> Result<(), Box> { + let repo = pushed_branch_repo("open-pr-suffixless-remotes")?; + git_stdout(&repo, &["remote", "rename", "origin", "fork"])?; + git_stdout( + &repo, + &[ + "remote", + "set-url", + "fork", + "ssh://git@github.com/octo/repo", + ], + )?; + git_stdout( + &repo, + &["remote", "add", "upstream", "git@github.com:upstream/repo"], + )?; + let fake_gh = fake_gh("open-pr-suffixless-remotes", false)?; + let planner = OperationPlanner { + repo_root: repo.clone(), + github_executable: Some(fake_gh), + ssh_executable: Some(test_ssh_command()?), + }; + + let operation = planner + .plan_request(OperationRequest::OpenPullRequest { base: None }) + .map_err(std::io::Error::other)?; + + assert!( + operation + .plan + .preview_text() + .contains("https://github.com/upstream/repo/compare/main...octo%3Afeature/open-pr") + ); + Ok(()) + } + + #[test] + fn pull_request_allows_matching_branch_names_across_fork_and_upstream() + -> Result<(), Box> { + let repo = pushed_branch_repo("open-pr-matching-fork-branch")?; + git_stdout(&repo, &["branch", "-m", "main"])?; + git_stdout_with_ssh( + &repo, + &["push", "-u", "origin", "main"], + &test_ssh_command()?, + )?; + git_stdout( + &repo, + &[ + "remote", + "add", + "upstream", + "https://github.com/upstream/repo.git", + ], + )?; + let fake_gh = fake_gh("open-pr-matching-fork-branch", false)?; + let planner = OperationPlanner { + repo_root: repo.clone(), + github_executable: Some(fake_gh), + ssh_executable: Some(test_ssh_command()?), + }; + + let operation = planner + .plan_request(OperationRequest::OpenPullRequest { base: None }) + .map_err(std::io::Error::other)?; + + assert!( + operation + .plan + .preview_text() + .contains("https://github.com/upstream/repo/compare/main...octo%3Amain") + ); + Ok(()) + } + + #[test] + fn open_pull_request_plan_encodes_compare_ref_names() { + let plan = open_pull_request_plan( + OperationRequest::OpenPullRequest { + base: Some("release#candidate".to_owned()), + }, + "origin", + "feature%ready", + "release#candidate", + "feature%ready", + "octo/repo", + None, + ); + + assert!(plan.preview_text().contains( + "https://github.com/octo/repo/compare/release%23candidate...feature%25ready?expand=1" + )); + } + + #[test] + fn open_pull_request_blocks_missing_upstream_and_matching_base() -> Result<(), Box> { + let repo = isolated_git_repo("open-pr-invalid-state")?; + configure_git_identity(&repo)?; + std::fs::write(repo.join("file.txt"), "base\n")?; + git_stdout(&repo, &["add", "file.txt"])?; + git_stdout(&repo, &["commit", "-m", "initial"])?; + let planner = OperationPlanner::new(&repo); + + let error = match planner.plan_request(OperationRequest::OpenPullRequest { base: None }) { + Ok(_) => return Err(std::io::Error::other("missing upstream must fail closed").into()), + Err(error) => error, + }; + assert!(error.contains("has no upstream")); + + let remote = isolated_bare_git_repo("open-pr-invalid-state-remote")?; + let branch = git_stdout(&repo, &["branch", "--show-current"])? + .trim() + .to_owned(); + add_github_remote(&repo, "origin", &remote)?; + git_stdout_with_ssh( + &repo, + &["push", "-u", "origin", branch.as_str()], + &test_ssh_command()?, + )?; + let fake_gh = fake_gh("open-pr-invalid-state", false)?; + let planner = OperationPlanner { + repo_root: repo, + github_executable: Some(fake_gh), + ssh_executable: Some(test_ssh_command()?), + }; + let error = + match planner.plan_request(OperationRequest::OpenPullRequest { base: Some(branch) }) { + Ok(_) => return Err(std::io::Error::other("base=head must fail closed").into()), + Err(error) => error, + }; + assert!(error.contains("base branch must differ")); + Ok(()) + } + + #[test] + fn operation_plans_use_guardrail_risk_levels() { + let branch = BranchTarget { + name: "feature/auth".to_owned(), + reference: "refs/heads/feature/auth".to_owned(), + oid: "abcdef1234567890".to_owned(), + kind: BranchKind::Local, + }; + + assert_eq!(fetch_plan().confirmation.risk_level, RiskLevel::Low); + assert_eq!( + stage_paths_plan(vec!["file.txt".to_owned()]) + .confirmation + .risk_level, + RiskLevel::Low + ); + assert_eq!( + commit_plan("message", 1).confirmation.risk_level, + RiskLevel::Medium + ); + assert_eq!( + push_plan("feature/auth", "origin/feature/auth", 1) .confirmation .risk_level, RiskLevel::Medium @@ -4327,6 +5731,135 @@ mod tests { Ok(root) } + fn pushed_branch_repo(name: &str) -> Result> { + let repo = isolated_git_repo(name)?; + let remote = isolated_bare_git_repo(&format!("{name}-remote"))?; + configure_git_identity(&repo)?; + std::fs::write(repo.join("file.txt"), "base\n")?; + git_stdout(&repo, &["add", "file.txt"])?; + git_stdout(&repo, &["commit", "-m", "initial"])?; + git_stdout(&repo, &["checkout", "-b", "feature/open-pr"])?; + std::fs::write(repo.join("file.txt"), "feature\n")?; + git_stdout(&repo, &["add", "file.txt"])?; + git_stdout(&repo, &["commit", "-m", "feature"])?; + add_github_remote(&repo, "origin", &remote)?; + git_stdout_with_ssh( + &repo, + &["push", "-u", "origin", "feature/open-pr"], + &test_ssh_command()?, + )?; + Ok(repo) + } + + fn add_github_remote( + repo: &std::path::Path, + name: &str, + bare_remote: &std::path::Path, + ) -> Result<(), Box> { + let remote_url = "ssh://git@github.com/octo/repo.git"; + let test_bare_key = format!("remote.{name}.testbare"); + git_stdout(repo, &["remote", "add", name, remote_url])?; + git_stdout( + repo, + &[ + "config", + test_bare_key.as_str(), + &bare_remote.display().to_string(), + ], + )?; + Ok(()) + } + + fn git_stdout_with_ssh( + repo: &std::path::Path, + args: &[&str], + ssh_executable: &std::path::Path, + ) -> Result> { + let output = std::process::Command::new("git") + .current_dir(repo) + .env("GIT_SSH_COMMAND", ssh_executable) + .args(args) + .output()?; + if !output.status.success() { + return Err(std::io::Error::other(format!( + "git command failed: {}", + String::from_utf8_lossy(&output.stderr) + )) + .into()); + } + Ok(String::from_utf8(output.stdout)?) + } + + fn test_ssh_command() -> Result> { + use std::os::unix::fs::PermissionsExt; + use std::sync::OnceLock; + + static COMMAND: OnceLock> = OnceLock::new(); + match COMMAND.get_or_init(|| { + (|| -> Result> { + let root = isolated_temp_root("fake-ssh")?; + std::fs::create_dir_all(&root)?; + let executable = root.join("ssh"); + std::fs::write( + &executable, + "#!/bin/sh\ntarget=$(git config --get-regexp '^remote\\..*\\.testbare$' | cut -d' ' -f2-)\ncase \"$*\" in\n*git-receive-pack*) exec git-receive-pack \"$target\" ;;\n*) exec git-upload-pack \"$target\" ;;\nesac\n", + )?; + let mut permissions = std::fs::metadata(&executable)?.permissions(); + permissions.set_mode(0o755); + std::fs::set_permissions(&executable, permissions)?; + Ok(executable) + })() + .map_err(|error| error.to_string()) + }) { + Ok(command) => Ok(command.clone()), + Err(error) => Err(std::io::Error::other(error.clone()).into()), + } + } + + fn fake_gh(name: &str, existing: bool) -> Result> { + let pull_requests = if existing { + "[{\"number\":42,\"html_url\":\"https://github.com/octo/repo/pull/42\",\"title\":\"Existing PR\",\"base\":{\"ref\":\"main\"},\"head\":{\"ref\":\"feature/open-pr\",\"repo\":{\"full_name\":\"octo/repo\"}}}]" + } else { + "[]" + }; + fake_gh_with_pull_requests(name, pull_requests, true) + } + + fn fake_gh_with_pull_requests( + name: &str, + pull_requests: &str, + create_succeeds: bool, + ) -> Result> { + use std::os::unix::fs::PermissionsExt; + + let root = isolated_temp_root(&format!("fake-gh-{name}"))?; + std::fs::create_dir_all(&root)?; + let executable = root.join("gh"); + let invocations = root.join("invocations"); + let base_missing = root.join("base-missing"); + let head_missing = root.join("head-missing"); + let create_response = if create_succeeds { + "printf '%s\\n' 'https://github.com/octo/repo/pull/43' ;;" + } else { + "exit 1 ;;" + }; + std::fs::write( + &executable, + format!( + "#!/bin/sh\nprintf '%s:%s %s\\n' \"$1\" \"$2\" \"$*\" >> '{}'\nrepository=octo/repo\nfor arg in \"$@\"; do\n case \"$arg\" in\n github.com/*/*) repository=${{arg#github.com/}} ;;\n esac\ndone\nif [ \"$repository\" = octo/old-repo ]; then repository=octo/repo; fi\ncase \"$1:$2\" in\n--version:*) exit 0 ;;\nauth:status) exit 0 ;;\nrepo:view) case \"$*\" in *--repo*) exit 1 ;; esac; printf '{{\"nameWithOwner\":\"%s\",\"defaultBranchRef\":{{\"name\":\"main\"}}}}\\n' \"$repository\" ;;\napi:--method)\n case \"$6\" in\n */git/matching-refs/heads/*)\n ref=${{6##*/heads/}}\n ref=$(printf '%s' \"$ref\" | sed 's/%2F/\\//g')\n if [ \"$ref\" = missing ] || {{ [ \"$ref\" = main ] && [ -f '{}' ]; }} || {{ [ \"$ref\" != main ] && [ \"$ref\" != release ] && [ -f '{}' ]; }}; then\n printf '%s\\n' '[[]]'\n else\n oid=$(git rev-parse HEAD)\n printf '[[{{\"ref\":\"refs/heads/%s\",\"object\":{{\"sha\":\"%s\"}}}}]]\\n' \"$ref\" \"$oid\"\n fi ;;\n */pulls) case \"$*\" in *--head*|*'--limit 0'*) exit 1 ;; esac; printf '%s\\n' '[{}]' ;;\n *) exit 1 ;;\n esac ;;\npr:create) {}\n*) exit 1 ;;\nesac\n", + invocations.display(), + base_missing.display(), + head_missing.display(), + pull_requests, + create_response + ), + )?; + let mut permissions = std::fs::metadata(&executable)?.permissions(); + permissions.set_mode(0o755); + std::fs::set_permissions(&executable, permissions)?; + Ok(executable) + } + fn git_stdout(repo: &std::path::Path, args: &[&str]) -> Result> { let output = std::process::Command::new("git") .arg("-C")