diff --git a/src/commands/cleanup.rs b/src/commands/cleanup.rs index de7114b..7f7865e 100644 --- a/src/commands/cleanup.rs +++ b/src/commands/cleanup.rs @@ -301,6 +301,41 @@ fn delete_local_branch( } } +/// Whether deleting `branch`'s remote would orphan open child PRs. +/// +/// GitHub closes a PR when its base branch is deleted, so if any open PR still +/// targets `branch` as its base we must NOT delete it — warn and return true to +/// skip the deletion. On a query error we conservatively skip too, rather than +/// risk silently closing a child PR. +fn remote_deletion_blocked_by_children(branch: &str) -> bool { + match github::open_prs_with_base(branch) { + Ok(children) if !children.is_empty() => { + output::warn(&format!( + "Not deleting origin/{branch}: {} open PR(s) still target it as base:", + children.len() + )); + for child in &children { + output::warn(&format!(" #{} ({})", child.number, child.head_branch)); + } + output::action( + "gw sync # run on each child to restack onto main, then re-run gw cleanup", + ); + true + } + Ok(_) => false, + Err(e) => { + output::warn(&format!("Could not check for dependent PRs: {e}")); + output::warn(&format!( + "Not deleting origin/{branch} to avoid closing a child PR." + )); + output::action(&format!( + "git push origin --delete {branch} # if you're sure nothing depends on it" + )); + true + } + } +} + /// Handle remote branch deletion fn handle_remote_branch(branch: &str, pr_info: &Option, verbose: bool) { let remote_exists = match git::remote_branch_exists(branch) { @@ -330,6 +365,12 @@ fn handle_remote_branch(branch: &str, pr_info: &Option, verbose: // Remote branch still exists match pr_info { Some(pr) if matches!(pr.state, PrState::Merged { .. }) => { + // Don't delete a branch that open PRs still use as their base — + // GitHub would close those child PRs. Skip the remote deletion + // (local cleanup already happened) and let the user restack first. + if remote_deletion_blocked_by_children(branch) { + return; + } // PR merged but remote branch exists - delete it output::info("PR merged, deleting remote branch..."); match github::delete_remote_branch(branch) { diff --git a/src/github/client.rs b/src/github/client.rs index 73166aa..76c8fb2 100644 --- a/src/github/client.rs +++ b/src/github/client.rs @@ -7,7 +7,7 @@ use std::process::Command; use crate::error::{GwError, Result}; -use super::parser::parse_pr_json; +use super::parser::{parse_pr_json, parse_pr_list_json}; use super::types::{MergeMethod, PrInfo, PrState, RawPrData}; /// Output from a command execution @@ -146,6 +146,38 @@ impl GitHubClient { Ok(Some(pr_info)) } + /// List the open PRs that target `base` as their base branch. + /// + /// Used before deleting a branch so we don't delete the base of an open + /// stacked PR (which GitHub would close). Returns an empty vec when none. + pub fn open_prs_with_base(&self, base: &str) -> Result> { + let output = self.executor.execute( + "gh", + &[ + "pr", + "list", + "--base", + base, + "--state", + "open", + "--json", + "number,title,url,state,baseRefName,headRefName,mergeCommit", + ], + )?; + + if !output.success { + return Err(GwError::GitCommandFailed(format!( + "gh pr list failed: {}", + output.stderr.trim() + ))); + } + + parse_pr_list_json(&output.stdout)? + .into_iter() + .map(|raw| self.convert_raw_to_pr_info(raw)) + .collect() + } + /// Delete a remote branch pub fn delete_remote_branch(&self, branch: &str) -> Result<()> { let output = self @@ -296,6 +328,11 @@ pub fn delete_remote_branch(branch: &str) -> Result<()> { GitHubClient::new().delete_remote_branch(branch) } +/// List the open PRs that target `base` as their base branch. +pub fn open_prs_with_base(base: &str) -> Result> { + GitHubClient::new().open_prs_with_base(base) +} + /// Add a comment to a PR pub fn add_pr_comment(pr_number: u64, comment: &str) -> Result<()> { GitHubClient::new().add_pr_comment(pr_number, comment) diff --git a/src/github/mod.rs b/src/github/mod.rs index f9ecd00..0ffead0 100644 --- a/src/github/mod.rs +++ b/src/github/mod.rs @@ -38,7 +38,8 @@ pub use types::{MergeMethod, PrInfo, PrState, RawPrData}; // Re-export client pub use client::{ CommandExecutor, CommandOutput, GitHubClient, RealCommandExecutor, add_pr_comment, - delete_remote_branch, get_pr_for_branch, is_gh_authenticated, is_gh_available, update_pr_base, + delete_remote_branch, get_pr_for_branch, is_gh_authenticated, is_gh_available, + open_prs_with_base, update_pr_base, }; // Re-export mock (for testing in other modules) diff --git a/src/github/parser.rs b/src/github/parser.rs index 2a313df..a919a0c 100644 --- a/src/github/parser.rs +++ b/src/github/parser.rs @@ -61,7 +61,19 @@ pub fn parse_pr_json(json: &str) -> Result { let parsed: GhPrJson = serde_json::from_str(json) .map_err(|e| GwError::Other(format!("Failed to parse PR JSON: {e}. Raw: {json}")))?; - Ok(RawPrData { + Ok(raw_from(parsed)) +} + +/// Parse a JSON array from `gh pr list --json ...` into raw PR records. +pub fn parse_pr_list_json(json: &str) -> Result> { + let parsed: Vec = serde_json::from_str(json) + .map_err(|e| GwError::Other(format!("Failed to parse PR list JSON: {e}. Raw: {json}")))?; + + Ok(parsed.into_iter().map(raw_from).collect()) +} + +fn raw_from(parsed: GhPrJson) -> RawPrData { + RawPrData { number: parsed.number, title: parsed.title, url: parsed.url, @@ -69,7 +81,7 @@ pub fn parse_pr_json(json: &str) -> Result { base_branch: parsed.base_ref_name, head_branch: parsed.head_ref_name, merge_commit: parsed.merge_commit.and_then(|m| m.oid), - }) + } } #[cfg(test)] @@ -168,4 +180,22 @@ mod tests { assert_eq!(pr.number, 49); assert_eq!(pr.title, "t"); } + + #[test] + fn test_parse_pr_list_json() { + let json = r#"[ + {"number":10,"title":"child one","state":"OPEN","baseRefName":"feature/base","headRefName":"feature/child-1"}, + {"number":11,"title":"child two","state":"OPEN","baseRefName":"feature/base","headRefName":"feature/child-2"} + ]"#; + let prs = parse_pr_list_json(json).unwrap(); + assert_eq!(prs.len(), 2); + assert_eq!(prs[0].number, 10); + assert_eq!(prs[0].head_branch, "feature/child-1"); + assert_eq!(prs[1].base_branch, "feature/base"); + } + + #[test] + fn test_parse_pr_list_json_empty() { + assert!(parse_pr_list_json("[]").unwrap().is_empty()); + } } diff --git a/src/github/tests.rs b/src/github/tests.rs index 086d4af..1070c82 100644 --- a/src/github/tests.rs +++ b/src/github/tests.rs @@ -6,10 +6,62 @@ //! - Error handling (auth errors, network errors) //! - Edge cases (Japanese titles, special characters) -use super::client::GitHubClient; +use super::client::{CommandOutput, GitHubClient}; use super::mock::{MockScenarioBuilder, fixtures}; use super::types::{MergeMethod, PrState}; +/// Args for the `gh pr list --base --state open` query, matching +/// `GitHubClient::open_prs_with_base`. +fn pr_list_args(base: &str) -> Vec { + [ + "pr", + "list", + "--base", + base, + "--state", + "open", + "--json", + "number,title,url,state,baseRefName,headRefName,mergeCommit", + ] + .iter() + .map(|s| s.to_string()) + .collect() +} + +#[test] +fn test_open_prs_with_base_lists_children() { + let executor = MockScenarioBuilder::new().gh_available().build(); + let args = pr_list_args("feature/base"); + let arg_refs: Vec<&str> = args.iter().map(String::as_str).collect(); + let json = r#"[ + {"number":10,"title":"child one","state":"OPEN","baseRefName":"feature/base","headRefName":"feature/child-1"}, + {"number":11,"title":"child two","state":"OPEN","baseRefName":"feature/base","headRefName":"feature/child-2"} + ]"#; + executor.on_command("gh", &arg_refs, CommandOutput::success(json)); + let client = GitHubClient::with_executor(executor); + + let prs = client.open_prs_with_base("feature/base").unwrap(); + assert_eq!(prs.len(), 2); + assert_eq!(prs[0].number, 10); + assert_eq!(prs[0].head_branch, "feature/child-1"); +} + +#[test] +fn test_open_prs_with_base_empty() { + let executor = MockScenarioBuilder::new().gh_available().build(); + let args = pr_list_args("feature/base"); + let arg_refs: Vec<&str> = args.iter().map(String::as_str).collect(); + executor.on_command("gh", &arg_refs, CommandOutput::success("[]")); + let client = GitHubClient::with_executor(executor); + + assert!( + client + .open_prs_with_base("feature/base") + .unwrap() + .is_empty() + ); +} + // ============================================================================= // gh CLI availability tests // =============================================================================