From b9d979e10a5f418614ce452774536c653fd97118 Mon Sep 17 00:00:00 2001 From: Vitor Py Date: Mon, 5 Jan 2026 20:45:27 +0100 Subject: [PATCH] Implement stacked PR merge support with conflict detection Add automatic detection and merging of stacked pull requests with comprehensive conflict checking and user confirmation. When merging a PR that's part of a stack, the CLI now: - Auto-detects stack membership via stack_id field - Displays preview of all PRs to be merged (current + below) - Checks cumulative patches for conflicts before merging - Prompts user for confirmation before executing merge - Provides clear error messages for conflicts API changes: - Add stack fields to Pull: stack_id, change_id, parent_change_id - Add MergeCheckRequest/Response types for conflict detection - Add merge_check() method to TangledClient CLI changes: - Refactor merge() to detect and handle stacked PRs - Add helper functions for stack ordering and conflict checking - Maintain backward compatibility for non-stacked PRs --- crates/tangled-api/src/client.rs | 55 ++++ crates/tangled-api/src/lib.rs | 4 +- crates/tangled-cli/src/commands/pr.rs | 380 +++++++++++++++++++++++--- 3 files changed, 393 insertions(+), 46 deletions(-) diff --git a/crates/tangled-api/src/client.rs b/crates/tangled-api/src/client.rs index 7855ef7..7c37ffd 100644 --- a/crates/tangled-api/src/client.rs +++ b/crates/tangled-api/src/client.rs @@ -1237,6 +1237,28 @@ impl TangledClient { Ok(()) } + pub async fn merge_check( + &self, + repo_did: &str, + repo_name: &str, + branch: &str, + patch: &str, + pds_base: &str, + access_jwt: &str, + ) -> Result { + let sa = self.service_auth_token(pds_base, access_jwt).await?; + + let req = MergeCheckRequest { + did: repo_did.to_string(), + name: repo_name.to_string(), + branch: branch.to_string(), + patch: patch.to_string(), + }; + + self.post_json("sh.tangled.repo.mergeCheck", &req, Some(&sa)) + .await + } + pub async fn update_repo_spindle( &self, did: &str, @@ -1373,6 +1395,13 @@ pub struct Pull { pub patch: String, #[serde(rename = "createdAt")] pub created_at: String, + // Stack support fields + #[serde(skip_serializing_if = "Option::is_none")] + pub stack_id: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub change_id: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub parent_change_id: Option, } #[derive(Debug, Clone)] @@ -1382,6 +1411,32 @@ pub struct PullRecord { pub pull: Pull, } +// Merge check types for stacked diff conflict detection +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct MergeCheckRequest { + pub did: String, + pub name: String, + pub branch: String, + pub patch: String, +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct MergeCheckResponse { + pub is_conflicted: bool, + #[serde(default)] + pub conflicts: Vec, + #[serde(skip_serializing_if = "Option::is_none")] + pub message: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub error: Option, +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct ConflictInfo { + pub filename: String, + pub reason: String, +} + #[derive(Debug, Clone)] pub struct RepoRecord { pub did: String, diff --git a/crates/tangled-api/src/lib.rs b/crates/tangled-api/src/lib.rs index 661c21a..28bac29 100644 --- a/crates/tangled-api/src/lib.rs +++ b/crates/tangled-api/src/lib.rs @@ -2,6 +2,6 @@ pub mod client; pub use client::TangledClient; pub use client::{ - CreateRepoOptions, DefaultBranch, Issue, IssueRecord, Language, Languages, Pull, PullRecord, - RepoRecord, Repository, Secret, + ConflictInfo, CreateRepoOptions, DefaultBranch, Issue, IssueRecord, Language, Languages, + MergeCheckRequest, MergeCheckResponse, Pull, PullRecord, RepoRecord, Repository, Secret, }; diff --git a/crates/tangled-cli/src/commands/pr.rs b/crates/tangled-cli/src/commands/pr.rs index a869bf6..a989fe0 100644 --- a/crates/tangled-cli/src/commands/pr.rs +++ b/crates/tangled-cli/src/commands/pr.rs @@ -179,59 +179,34 @@ async fn merge(args: PrMergeArgs) -> Result<()> { .or_else(|| std::env::var("TANGLED_PDS_BASE").ok()) .unwrap_or_else(|| "https://bsky.social".into()); - // Get the PR to find the target repo + // Get the PR let pds_client = tangled_api::TangledClient::new(&pds); let pull = pds_client .get_pull_record(&did, &rkey, Some(session.access_jwt.as_str())) .await?; - // Parse the target repo AT-URI to get did and name - let target_repo = &pull.target.repo; - // Format: at://did:plc:.../sh.tangled.repo/rkey - let parts: Vec<&str> = target_repo.strip_prefix("at://").unwrap_or(target_repo).split('/').collect(); - if parts.len() < 2 { - return Err(anyhow!("Invalid target repo AT-URI: {}", target_repo)); - } - let repo_did = parts[0]; + // Parse target repo info + let (repo_did, repo_name) = parse_target_repo_info(&pull, &pds_client, &session).await?; - // Get repo info to find the name - // Parse rkey from target repo AT-URI - let repo_rkey = if parts.len() >= 4 { - parts[3] + // Check if PR is part of a stack + if let Some(stack_id) = &pull.stack_id { + merge_stacked_pr( + &pds_client, + &session, + &pull, + &did, + &rkey, + &repo_did, + &repo_name, + stack_id, + &pds, + ) + .await?; } else { - return Err(anyhow!("Invalid target repo AT-URI: {}", target_repo)); - }; - - #[derive(serde::Deserialize)] - struct Rec { - name: String, + // Single PR merge (existing logic) + merge_single_pr(&session, &did, &rkey, &repo_did, &repo_name, &pds).await?; } - #[derive(serde::Deserialize)] - struct GetRes { - value: Rec, - } - let params = [ - ("repo", repo_did.to_string()), - ("collection", "sh.tangled.repo".to_string()), - ("rkey", repo_rkey.to_string()), - ]; - let repo_rec: GetRes = pds_client - .get_json("com.atproto.repo.getRecord", ¶ms, Some(session.access_jwt.as_str())) - .await?; - - // Call merge on the default Tangled API base (tngl.sh) - let api = tangled_api::TangledClient::default(); - api.merge_pull( - &did, - &rkey, - repo_did, - &repo_rec.value.name, - &pds, - &session.access_jwt, - ) - .await?; - println!("Merged PR {}:{}", did, rkey); Ok(()) } @@ -259,3 +234,320 @@ fn parse_record_id<'a>(id: &'a str, default_did: &'a str) -> Result<(String, Str } Ok((default_did.to_string(), id.to_string())) } + +// Helper functions for stacked PR merge support + +async fn merge_single_pr( + session: &tangled_config::session::Session, + did: &str, + rkey: &str, + repo_did: &str, + repo_name: &str, + pds: &str, +) -> Result<()> { + let api = tangled_api::TangledClient::default(); + api.merge_pull(did, rkey, repo_did, repo_name, pds, &session.access_jwt) + .await?; + + println!("Merged PR {}:{}", did, rkey); + Ok(()) +} + +async fn merge_stacked_pr( + pds_client: &tangled_api::TangledClient, + session: &tangled_config::session::Session, + current_pull: &tangled_api::Pull, + current_did: &str, + current_rkey: &str, + repo_did: &str, + repo_name: &str, + stack_id: &str, + pds: &str, +) -> Result<()> { + // Step 1: Get full stack + println!("🔍 Detecting stack..."); + let stack = get_stack_pulls(pds_client, &session.did, stack_id, &session.access_jwt).await?; + + if stack.is_empty() { + return Err(anyhow!("Stack is empty")); + } + + // Step 2: Find substack (current PR and all below it) + let substack = find_substack(&stack, current_pull.change_id.as_deref())?; + + println!( + "✓ Detected PR is part of stack (stack has {} total PRs)", + stack.len() + ); + println!(); + println!("The following {} PR(s) will be merged:", substack.len()); + + for (idx, pr) in substack.iter().enumerate() { + let marker = if pr.rkey == current_rkey { + " (current)" + } else { + "" + }; + println!(" [{}] {}: {}{}", idx + 1, pr.rkey, pr.pull.title, marker); + } + println!(); + + // Step 3: Check for conflicts + println!("✓ Checking for conflicts..."); + let api = tangled_api::TangledClient::default(); + let conflicts = check_stack_conflicts( + &api, + repo_did, + repo_name, + ¤t_pull.target.branch, + &substack, + pds, + &session.access_jwt, + ) + .await?; + + if !conflicts.is_empty() { + println!("✗ Cannot merge: conflicts detected"); + println!(); + for (pr_rkey, conflict_resp) in conflicts { + println!( + " PR {}: Conflicts in {} file(s)", + pr_rkey, + conflict_resp.conflicts.len() + ); + for conflict in conflict_resp.conflicts { + println!(" - {}: {}", conflict.filename, conflict.reason); + } + } + return Err(anyhow!("Stack has merge conflicts")); + } + + println!("✓ All PRs can be merged cleanly"); + println!(); + + // Step 4: Confirmation prompt + if !prompt_confirmation(&format!("Merge {} pull request(s)?", substack.len()))? { + println!("Merge cancelled."); + return Ok(()); + } + + // Step 5: Merge the stack (backend handles combined patch) + println!("Merging {} PR(s)...", substack.len()); + + // Use the current PR's merge endpoint - backend will handle the stack + api.merge_pull( + current_did, + current_rkey, + repo_did, + repo_name, + pds, + &session.access_jwt, + ) + .await?; + + println!("✓ Successfully merged {} pull request(s)", substack.len()); + + Ok(()) +} + +async fn get_stack_pulls( + client: &tangled_api::TangledClient, + user_did: &str, + stack_id: &str, + bearer: &str, +) -> Result> { + // List all user's PRs and filter by stack_id + let all_pulls = client.list_pulls(user_did, None, Some(bearer)).await?; + + let mut stack_pulls: Vec<_> = all_pulls + .into_iter() + .filter(|p| p.pull.stack_id.as_deref() == Some(stack_id)) + .collect(); + + // Order by parent relationships (top to bottom) + order_stack(&mut stack_pulls)?; + + Ok(stack_pulls) +} + +fn order_stack(pulls: &mut Vec) -> Result<()> { + if pulls.is_empty() { + return Ok(()); + } + + // Build parent map: parent_change_id -> pull + let mut change_id_map: std::collections::HashMap = + std::collections::HashMap::new(); + let mut parent_map: std::collections::HashMap = + std::collections::HashMap::new(); + + for (idx, pr) in pulls.iter().enumerate() { + if let Some(cid) = &pr.pull.change_id { + change_id_map.insert(cid.clone(), idx); + } + if let Some(pcid) = &pr.pull.parent_change_id { + parent_map.insert(pcid.clone(), idx); + } + } + + // Find top of stack (not a parent of any other PR) + let mut top_idx = None; + for (idx, pr) in pulls.iter().enumerate() { + if let Some(cid) = &pr.pull.change_id { + if !parent_map.contains_key(cid) { + top_idx = Some(idx); + break; + } + } + } + + let top_idx = top_idx.ok_or_else(|| anyhow!("Could not find top of stack"))?; + + // Walk down the stack to build ordered list + let mut ordered = Vec::new(); + let mut current_idx = top_idx; + let mut visited = std::collections::HashSet::new(); + + loop { + if visited.contains(¤t_idx) { + return Err(anyhow!("Circular dependency in stack")); + } + visited.insert(current_idx); + ordered.push(current_idx); + + // Find child (PR that has this PR as parent) + let current_parent = &pulls[current_idx].pull.parent_change_id; + if current_parent.is_none() { + break; + } + + let next_idx = change_id_map.get(current_parent.as_ref().unwrap()); + + if let Some(&next) = next_idx { + current_idx = next; + } else { + break; + } + } + + // Reorder pulls based on ordered indices + let original = pulls.clone(); + pulls.clear(); + for idx in ordered { + pulls.push(original[idx].clone()); + } + + Ok(()) +} + +fn find_substack<'a>( + stack: &'a [tangled_api::PullRecord], + current_change_id: Option<&str>, +) -> Result> { + let change_id = current_change_id.ok_or_else(|| anyhow!("PR has no change_id"))?; + + let position = stack + .iter() + .position(|p| p.pull.change_id.as_deref() == Some(change_id)) + .ok_or_else(|| anyhow!("PR not found in stack"))?; + + // Return from current position to end (including current) + Ok(stack[position..].iter().collect()) +} + +async fn check_stack_conflicts( + api: &tangled_api::TangledClient, + repo_did: &str, + repo_name: &str, + target_branch: &str, + substack: &[&tangled_api::PullRecord], + pds: &str, + access_jwt: &str, +) -> Result> { + let mut conflicts = Vec::new(); + let mut cumulative_patch = String::new(); + + // Check each PR in order (bottom to top of substack) + for pr in substack.iter().rev() { + cumulative_patch.push_str(&pr.pull.patch); + cumulative_patch.push('\n'); + + let check = api + .merge_check( + repo_did, + repo_name, + target_branch, + &cumulative_patch, + pds, + access_jwt, + ) + .await?; + + if check.is_conflicted { + conflicts.push((pr.rkey.clone(), check)); + } + } + + Ok(conflicts) +} + +fn prompt_confirmation(message: &str) -> Result { + use std::io::{self, Write}; + + print!("{} [y/N]: ", message); + io::stdout().flush()?; + + let mut input = String::new(); + io::stdin().read_line(&mut input)?; + + Ok(matches!( + input.trim().to_lowercase().as_str(), + "y" | "yes" + )) +} + +async fn parse_target_repo_info( + pull: &tangled_api::Pull, + pds_client: &tangled_api::TangledClient, + session: &tangled_config::session::Session, +) -> Result<(String, String)> { + let target_repo = &pull.target.repo; + let parts: Vec<&str> = target_repo + .strip_prefix("at://") + .unwrap_or(target_repo) + .split('/') + .collect(); + + if parts.len() < 4 { + return Err(anyhow!("Invalid target repo AT-URI: {}", target_repo)); + } + + let repo_did = parts[0].to_string(); + let repo_rkey = parts[3]; + + // Get repo name + #[derive(serde::Deserialize)] + struct Rec { + name: String, + } + #[derive(serde::Deserialize)] + struct GetRes { + value: Rec, + } + + let params = [ + ("repo", repo_did.clone()), + ("collection", "sh.tangled.repo".to_string()), + ("rkey", repo_rkey.to_string()), + ]; + + let repo_rec: GetRes = pds_client + .get_json( + "com.atproto.repo.getRecord", + ¶ms, + Some(&session.access_jwt), + ) + .await?; + + Ok((repo_did, repo_rec.value.name)) +} -- 2.51.2