Skip to content

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

Draft
github-actions[bot] wants to merge 1 commit into
mainfrom
refactor/reduce-complexity-find-merge-base-67f10a313d2babc4
Draft

refactor(mcp): reduce complexity of find_merge_base in src/mcp.rs#1115
github-actions[bot] wants to merge 1 commit into
mainfrom
refactor/reduce-complexity-find-merge-base-67f10a313d2babc4

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

What was complex

find_merge_base in src/mcp.rs had a cognitive complexity of 18 (against a threshold of 10). The function handled three distinct concerns in a single deeply-nested body:

  1. Candidate discovery — running git symbolic-ref refs/remotes/origin/HEAD and building the candidate list
  2. Merge-base probing — looping through candidates with git merge-base, tracking whether any ref existed
  3. Single-commit fallback — running git rev-list --max-parents=0 and --count HEAD with extra nesting to validate the "truly single-commit repo" condition

The four nested control-flow levels (loop → if-let → if-success → if-non-empty) plus the two-level fallback block pushed the complexity well above threshold.

What changed

Extracted three focused module-level async helper functions, placed in a new // Git merge-base helpers section:

Helper Responsibility
build_merge_base_candidates Runs git symbolic-ref, parses the result, appends common defaults
try_candidates_for_merge_base Iterates candidates, tries git merge-base, tracks found_remote_ref
try_root_commit_fallback Single-commit edge-case: checks root + total commit count, warns and returns SHA only when count ≤ 1

find_merge_base itself is now a four-line sequencing function:

async fn find_merge_base(git_dir: &std::path::Path) -> Result<String, McpError> {
    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(sha) = base_sha { return Ok(sha); }
    if let Some(sha) = try_root_commit_fallback(git_dir).await { return Ok(sha); }
    Err(anyhow_to_mcp_error(if found_remote_ref { ... } else { ... }))
}

Before / After complexity

Function Before After
find_merge_base 18 ~4 (no warning at threshold 10)
build_merge_base_candidates ~5
try_candidates_for_merge_base ~7
try_root_commit_fallback ~5

Verification

  • All 2000+ tests pass (cargo test)
  • cargo clippy --all-targets --all-features — clean, no new warnings
  • find_merge_base no longer appears in the cognitive-complexity report at threshold 10
  • No behaviour changes: all three git-command sequences, the found_remote_ref tracking, and both error messages are preserved verbatim in the extracted helpers

Warning

Firewall blocked 2 domains

The following domains were blocked by the firewall during workflow execution:

  • spsprodeus21.vssps.visualstudio.com
  • spsprodweu4.vssps.visualstudio.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "spsprodeus21.vssps.visualstudio.com"
    - "spsprodweu4.vssps.visualstudio.com"

See Network Configuration for more information.

Generated by Cyclomatic Complexity Reducer · 984.3 AIC · ⌖ 38.5 AIC · ⊞ 36.3K ·

…pers

Extract three focused helper functions to break up the monolithic
find_merge_base method (cognitive complexity: 18 → ~4):

- build_merge_base_candidates: discovers candidate remote refs by running
  'git symbolic-ref refs/remotes/origin/HEAD' and appending common
  defaults (origin/main, origin/master) as fallbacks.

- try_candidates_for_merge_base: iterates the candidates, tries
  'git merge-base HEAD <ref>' for each, and tracks whether any ref
  exists in the repo even when merge-base itself fails.

- try_root_commit_fallback: handles the single-commit-repo edge case —
  runs 'git rev-list --max-parents=0 HEAD' and 'git rev-list --count HEAD'
  and returns the root SHA only when the repo has exactly one commit.

find_merge_base itself is now a concise four-step sequencing function
that delegates each phase to a named helper, making the overall
strategy immediately readable.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants