Skip to content

refactor(mcp): reduce complexity of find_merge_base in src/mcp.rs #1115

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
github-actions wants to merge 1 commit into
base: main
Choose a base branch
from
+145 −105
Draft

refactor(mcp): reduce complexity of find_merge_base in src/mcp.rs #1115

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
250 changes: 145 additions & 105 deletions src/mcp.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,138 @@ fn generate_short_id() -> String {
// Re-export from tools module
use crate::safeoutputs::{ALWAYS_ON_TOOLS, DEBUG_ONLY_TOOLS};

// ============================================================================
// Git merge-base helpers (used by find_merge_base)
// ============================================================================

/// Discover the set of remote refs to try as a merge-base, starting with
/// the symbolic default branch and falling back to common names.
async fn build_merge_base_candidates(git_dir: &std::path::Path) -> Vec<String> {
use tokio::process::Command;

let mut candidates: Vec<String> = Vec::new();

let symbolic_output = Command::new("git")
.args(["symbolic-ref", "refs/remotes/origin/HEAD"])
.current_dir(git_dir)
.output()
.await
.ok();

if let Some(out) = symbolic_output.filter(|o| o.status.success()) {
let refname = String::from_utf8_lossy(&out.stdout).trim().to_string();
// e.g. "refs/remotes/origin/main" → "origin/main"
if let Some(branch) = refname.strip_prefix("refs/remotes/") {
candidates.push(branch.to_string());
}
}

for name in &["origin/main", "origin/master"] {
if !candidates.iter().any(|c| c == *name) {
candidates.push(name.to_string());
}
}

candidates
}

/// Try each candidate ref in order. Returns the merge-base SHA on the first
/// success, plus a flag indicating whether at least one remote ref was found
/// to exist (even if `git merge-base` failed for it).
async fn try_candidates_for_merge_base(
git_dir: &std::path::Path,
candidates: &[String],
) -> (Option<String>, bool) {
use tokio::process::Command;

let mut found_remote_ref = false;

for remote_ref in candidates {
let output = Command::new("git")
.args(["merge-base", "HEAD", remote_ref])
.current_dir(git_dir)
.output()
.await
.ok();

if let Some(out) = output {
if out.status.success() {
let base = String::from_utf8_lossy(&out.stdout).trim().to_string();
if !base.is_empty() {
return (Some(base), found_remote_ref);
}
}
// Check if the ref exists even though merge-base failed (diverged branches)
let ref_check = Command::new("git")
.args(["rev-parse", "--verify", remote_ref])
.current_dir(git_dir)
.output()
.await
.ok();
if ref_check.is_some_and(|o| o.status.success()) {
found_remote_ref = true;
}
}
}

(None, found_remote_ref)
}

/// For single-commit repositories, fall back to using the root commit as the
/// diff base. Returns the root SHA only when the repository has exactly one
/// commit total (to avoid masking real errors on normal repos).
async fn try_root_commit_fallback(git_dir: &std::path::Path) -> Option<String> {
use tokio::process::Command;

let root_output = Command::new("git")
.args(["rev-list", "--max-parents=0", "HEAD"])
.current_dir(git_dir)
.output()
.await
.ok();

let out = root_output.filter(|o| o.status.success())?;
let output_str = String::from_utf8_lossy(&out.stdout).to_string();
let roots: Vec<&str> = output_str.trim().lines().collect();

// Only proceed when there is exactly one root commit to examine.
// (Most repos have one root regardless of total commit count, so roots.len() == 1
// alone is not sufficient to conclude this is a single-commit repo.)
if roots.len() != 1 {
return None;
}

// Verify the repo truly has only one commit total before using the root.
let count_output = Command::new("git")
.args(["rev-list", "--count", "HEAD"])
.current_dir(git_dir)
.output()
.await
.ok();

let commit_count = count_output
.filter(|o| o.status.success())
.and_then(|o| {
String::from_utf8_lossy(&o.stdout)
.trim()
.parse::<u64>()
.ok()
})
.unwrap_or(0);

if commit_count <= 1 {
let sha = roots[0].to_string();
warn!(
"Could not find merge-base with origin; using root commit {} \
(single-commit repository)",
sha
);
Some(sha)
} else {
None
}
}

// ============================================================================
// SafeOutputs MCP Server
// ============================================================================
Expand Down Expand Up @@ -432,122 +564,30 @@ impl SafeOutputs {
/// 2. Common default branch names: `origin/main`, `origin/master`
/// 3. Root commit via `git rev-list --max-parents=0 HEAD` (handles single-commit repos)
async fn find_merge_base(git_dir: &std::path::Path) -> Result<String, McpError> {
use tokio::process::Command;

// First, try to discover the actual default branch from origin/HEAD
let symbolic_output = Command::new("git")
.args(["symbolic-ref", "refs/remotes/origin/HEAD"])
.current_dir(git_dir)
.output()
.await
.ok();

let mut candidates: Vec<String> = Vec::new();

if let Some(out) = symbolic_output.filter(|o| o.status.success()) {
let refname = String::from_utf8_lossy(&out.stdout).trim().to_string();
// e.g. "refs/remotes/origin/main" → "origin/main"
if let Some(branch) = refname.strip_prefix("refs/remotes/") {
candidates.push(branch.to_string());
}
}

// Always try common defaults as fallbacks
for name in &["origin/main", "origin/master"] {
if !candidates.iter().any(|c| c == *name) {
candidates.push(name.to_string());
}
}

let mut found_remote_ref = false;
for remote_ref in &candidates {
let output = Command::new("git")
.args(["merge-base", "HEAD", remote_ref])
.current_dir(git_dir)
.output()
.await
.ok();
let candidates = build_merge_base_candidates(git_dir).await;
let (base_sha, found_remote_ref) =
try_candidates_for_merge_base(git_dir, &candidates).await;

if let Some(out) = output {
if out.status.success() {
let base = String::from_utf8_lossy(&out.stdout).trim().to_string();
if !base.is_empty() {
return Ok(base);
}
}
// Check if the ref exists even though merge-base failed (diverged branches)
let ref_check = Command::new("git")
.args(["rev-parse", "--verify", remote_ref])
.current_dir(git_dir)
.output()
.await
.ok();
if ref_check.is_some_and(|o| o.status.success()) {
found_remote_ref = true;
}
}
if let Some(sha) = base_sha {
return Ok(sha);
}

// Fallback: find the root commit. Only valid for single-commit repos where HEAD~1
// doesn't exist. Verify the repo truly has only one commit total before
// using the root commit — most repos have exactly one root commit regardless
// of total commits (roots.len() == 1 would match virtually any repo).
let root_output = Command::new("git")
.args(["rev-list", "--max-parents=0", "HEAD"])
.current_dir(git_dir)
.output()
.await
.ok();

if let Some(out) = root_output.filter(|o| o.status.success()) {
let output_str = String::from_utf8_lossy(&out.stdout).to_string();
let roots: Vec<&str> = output_str.trim().lines().collect();

if roots.len() == 1 {
// Check total commit count to distinguish single-commit repos
// from normal repos that happen to have one root
let count_output = Command::new("git")
.args(["rev-list", "--count", "HEAD"])
.current_dir(git_dir)
.output()
.await
.ok();

let commit_count = count_output
.filter(|o| o.status.success())
.and_then(|o| {
String::from_utf8_lossy(&o.stdout)
.trim()
.parse::<u64>()
.ok()
})
.unwrap_or(0);

if commit_count <= 1 {
let sha = roots[0].to_string();
warn!(
"Could not find merge-base with origin; using root commit {} \
(single-commit repository)",
sha
);
return Ok(sha);
}
}
if let Some(sha) = try_root_commit_fallback(git_dir).await {
return Ok(sha);
}

if found_remote_ref {
Err(anyhow_to_mcp_error(anyhow::anyhow!(
Err(anyhow_to_mcp_error(if found_remote_ref {
anyhow::anyhow!(
"Cannot determine diff base: remote tracking branch exists but shares no \
common ancestry with HEAD (orphan or force-pushed branch). \
Ensure HEAD is based on the target branch."
)))
)
} else {
Err(anyhow_to_mcp_error(anyhow::anyhow!(
anyhow::anyhow!(
"Cannot determine diff base: no remote tracking branch found. \
This can happen with shallow clones (fetchDepth: 1). \
Ensure the pipeline fetches full history or push a tracking branch."
)))
}
)
}))
}

#[tool(
Expand Down