fix(mcp): delete remote cr/* branches on merge, harden merged detection, add prune#60
Merged
Conversation
…ntracts
- ContentrainConfig.remoteBranchCleanup?: boolean (default true) gates all
remote cr/* branch mutations (post-merge delete, branch delete, pruning)
- MergeResult.remote reports the post-merge source-branch cleanup outcome
(deleted / skipped / warning) without ever failing the merge itself
- RepoProvider.mergeBranch gains an optional { removeSourceBranch } opt so
drivers that own cleanup separately (Studio) can keep the old behavior
…ction Review-mode saves push their cr/* branch to origin, but nothing ever deleted the remote copy after a merge — one branch leaked per save+merge cycle (121 stale branches in a month in the field), rendering as phantom pending reviews in Studio. - mergeBranch now deletes the merged branch's remote copy, best-effort: failures surface as remote.warning, never as a failed merge. Config-gated by remoteBranchCleanup (default on). Inherited by contentrain_merge, the CLI merge/diff commands, the serve approve endpoints and LocalProvider.mergeBranch for free. - contentrain_branch_delete and LocalProvider.deleteBranch remove the remote copy too; branch_delete falls back to a remote-only delete when the local ref is already gone. - GitHub/GitLab providers delete the source branch after a successful API merge (opt out per call with removeSourceBranch: false). - Merged detection (isMerged/cleanupMergedBranches/checkBranchHealth) now falls back to patch-id equivalence (git cherry) so a base-history rewrite no longer flips merged branches to unmerged. The fallback only runs when it can change the health verdict, with bounded concurrency and monotonic verdict caching, so the hot pre-write gate stays at one subprocess. - Fixes a latent no-op: merge-base --is-ancestor signals via exit code with empty stderr, which simple-git reports as success — the fast-forward guards in the transaction layer never fired. Replaced with rev-list --count (and cat-file -e with -t in the prune path). - contentrain_doctor gains an offline-safe 'Remote branches' check (ls-remote, 5s cap); contentrain_branch_list accepts remote: true; contentrain_submit lazily prunes up to 20 merged remote leftovers. - New branch-lifecycle exports: deleteRemoteBranch, listRemoteCrBranches, pruneMergedRemoteBranches, isRefMerged, classifyMergedBranches. Tests: net-new bare-remote fixture; remote-branches suite (14), workflow-remote MCP-harness suite (6, single beforeAll fixture), mergeBranch transaction suite (4), branch-lifecycle classification suite (12 — cleanupMergedBranches/checkBranchHealth had zero coverage before), GitHub branch-ops suite (6, net-new), GitLab merge cleanup cases, doctor remote check cases. Full suite: 636/637 (1 pre-bump load-flake, timeout raised) + 18/18 end-to-end checks against a real bare remote.
- new 'contentrain prune' command: previews merged cr/* leftovers on the remote (--dry-run), confirms, then drains them in batch alongside the local retention cleanup; --json mutates only with --yes; non-zero exit when remote deletions fail - merge/diff commands and the serve approve/reject endpoints surface the remote cleanup outcome (deleted / warning) from the MCP layer; reject and interactive delete now remove the pushed copy of a discarded draft - doctor renders the new Remote branches check with zero changes (generic checks loop) Tests: prune command suite (6, mocked MCP layer per merge.test.ts pattern); diff/serve integration mocks extended for the new branch-lifecycle surface. CLI suite: 154/154.
✅ Deploy Preview for contentrain-ai ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
In
workflow: "review"with a git remote, every content save+merge cycle leaked exactly one remotecr/*branch.contentrain_content_savepushedcr/{op}/{model}/{ts}to origin;contentrain_mergemerged it and deleted only the local branch. Nothing in the toolchain ever rangit push --delete.From one field project: ~121 stale remote branches in a month, 13 new ones in a single working day. Studio's Pending Changes panel lists
origin/cr/*by existence, so every leftover rendered as a phantom pending review. The branch-pressure limits andcontentrain doctoronly counted local branches, so the remote pile stayed invisible to the tooling meant to catch it.Fix
Cut the leak at the source —
mergeBranchnow deletes the merged branch's remote copy. Because every local merge path funnels through this one helper, the behavior is inherited for free bycontentrain_merge, the CLImerge/diffcommands, the Serve UI approve endpoints, andLocalProvider.mergeBranch(the Studio local path). Best-effort and config-gated: a failure surfaces asremote.warning, never as a failed merge. Opt out withremoteBranchCleanup: false.Cover the sibling delete surfaces —
contentrain_branch_delete,LocalProvider.deleteBranch, the Serve reject endpoint, and interactivediffdelete all remove the remote copy too.branch_deletealso does a remote-only delete when the local ref is already gone (for branches leaked before this shipped).Remote providers — GitHub/GitLab delete the source branch after a successful API merge (opt out per call with
removeSourceBranch: false).Drain the existing backlog — new
contentrain pruneCLI command (--dry-run/--yes/--json) batch-deletes merged remote leftovers alongside the local retention cleanup;contentrain_submitlazily prunes up to 20 merged remote branches per run so review workflows self-heal.Make the remote visible —
contentrain doctorgains an offline-safe "Remote branches" check (authoritativels-remote, 5s cap);contentrain_branch_listacceptsremote: true.Harden merged-branch detection — the reporter hit a case where, after a base-history rewrite (rebase/squash/force-push), merged branches flip to "unmerged" by ancestry. Detection now falls back to patch-id equivalence (
git cherry) when ancestry breaks. The fallback only runs when it could actually change the health verdict, with bounded concurrency and monotonic verdict caching, so the per-write branch-health gate still costs onegit branch --mergedin the normal case.Latent bug found along the way
git merge-base --is-ancestorsignals via exit code with empty stderr, and simple-git reports exit-code-only failures as success. Every--is-ancestorguard in the codebase silently always-passed — including the fast-forward safety checks in the transaction layer. Replaced withrev-list --count(andcat-file -e→-tin the prune path).Config
One new key,
remoteBranchCleanup?: boolean(defaulttrue), gating all remote mutations. Read-only remote visibility (doctor, branch listing) is never gated. Also documented the previously-undocumentedbranchWarnLimit/branchBlockLimitand fixed thebranchRetentiondescription.Tests
git init --bare+ file:// remote — push/ls-remote/push-delete work fully offline)remote-branches.test.ts(14): deleteRemoteBranch / listRemoteCrBranches / pruneMergedRemoteBranches — happy, not-found, no-remote, disabled, offline, dry-run, max-capworkflow-remote.test.ts(6): the three branch tools through the real MCP InMemoryTransport harness (singlebeforeAllfixture — these tools had zero prior coverage)mergeBranchtransaction suite (4): local+remote delete, gate off, offline warning, no-remotebranch-lifecycle.test.tsclassification suite (12): ancestry, rewrite scenario, cap, retention, health thresholds —cleanupMergedBranches/checkBranchHealthhad zero prior coverageFull suite: 636/637 (the one miss was a load-flake on a real-git test; timeout raised, passes 14/14 standalone). CLI: 154/154. Plus an 18/18 end-to-end check driving the real MCP server + real CLI against a real bare remote (save→merge→delete, gate off, remote-only delete, doctor, prune, offline).
Out of scope
Studio-side "Pending Changes" filtering lives in the separate Studio repo. This PR cuts the root cause; the Studio ancestry/tree-diff filter is a follow-up there.
🤖 Generated with Claude Code