diff --git a/lib/merge.py b/lib/merge.py index 49cc61b..d2eeb32 100644 --- a/lib/merge.py +++ b/lib/merge.py @@ -444,15 +444,30 @@ async def _merge_no_ff_external(branch: str) -> tuple[bool, str]: Mechanics: 1. Fetch origin/main + origin/{branch} - 2. Detached worktree at origin/main, git merge --no-ff origin/{branch} - with verbose message: "Merge external GitHub PR #{N}: {branch_slug}" - 3. Force-push the merge commit as origin/{branch} (replacing the branch tip). - Dispatch's existing ff-push to main then produces the merge commit on main. + 2. Worktree on local branch _merged-{slug} from origin/main + 3. git merge --no-ff origin/{branch} with verbose message: + "Merge external GitHub PR #{N}: {branch_slug}" + 4. Push merge commit to origin/_merged/{branch} (synthetic audit ref) + 5. ff-push merge_sha → origin/main directly (function owns the push, NOT + dispatch — see sentinel return below) The merge commit M has parents [main_sha, branch_sha]. M is a fast-forward - descendant of main_sha (via first-parent chain), so the ff-push to main + descendant of main_sha (via first-parent chain), so the push to main works without --force. + Synthetic branch (Ship review Apr 28): we deliberately do NOT force-push + the contributor's gh-pr-N/* branch. Force-pushing it would rewrite the + branch tip with a merge commit the contributor didn't author, showing as + a confusing bot force-push in Forgejo's PR UI. The synthetic _merged/* + audit ref lets us track the merge commit without touching the contributor's + branch. Mirrors the _clean/* synthetic branch pattern in cherry-pick. + + Sentinel return: function pushes merge_sha → main itself (dispatch's ff-push + can't, since origin/{branch} is unchanged and not a descendant of main). + Returns a "merged --no-ff" sentinel string that dispatch detects to skip + its ff-push step and route directly to PR-close + mark_merged + audit. + The full 40-char merge SHA is in the return string for dispatch to extract. + Conflict handling: same auto-resolve pattern as cherry-pick — entity-only conflicts take main's version (--ours = current worktree HEAD = main), other conflicts abort and return False with detail. @@ -465,7 +480,10 @@ async def _merge_no_ff_external(branch: str) -> tuple[bool, str]: gh_pr_num = m.group(1) branch_slug = m.group(2) - worktree_path = f"/tmp/teleo-merge-{branch.replace('/', '-')}" + slug = branch.replace("/", "-") + worktree_path = f"/tmp/teleo-merge-{slug}" + local_branch = f"_merged-{slug}" # local working branch in worktree + audit_ref = f"_merged/{branch}" # remote synthetic ref (preserves hierarchy) # Fetch latest state — separate calls (long branch names break combined refspec) rc, out = await _git("fetch", "origin", "main", timeout=15) @@ -487,7 +505,9 @@ async def _merge_no_ff_external(branch: str) -> tuple[bool, str]: logger.info("External PR branch %s is descendant of main but has new content — proceeding", branch) async with _bare_repo_lock: - rc, out = await _git("worktree", "add", "--detach", worktree_path, "origin/main") + # Clean up any stale local branch from a prior failed run + await _git("branch", "-D", local_branch) + rc, out = await _git("worktree", "add", "-b", local_branch, worktree_path, "origin/main") if rc != 0: return False, f"worktree add failed: {out}" @@ -533,30 +553,45 @@ async def _merge_no_ff_external(branch: str) -> tuple[bool, str]: await _git("merge", "--abort", cwd=worktree_path) return False, f"merge conflict on PR #{gh_pr_num}: {conflict_detail}" - # Force-push the merge commit as origin/{branch}, replacing the contributor's - # branch tip. Dispatch then ff-pushes origin/{branch} → main, producing the - # merge commit on main. --force-with-lease guards against concurrent updates. - rc, expected_sha = await _git("rev-parse", f"origin/{branch}") + # Capture the merge commit SHA before any pushes + rc, merge_sha = await _git("rev-parse", "HEAD", cwd=worktree_path) if rc != 0: - return False, f"rev-parse origin/{branch} failed: {expected_sha}" - expected_sha = expected_sha.strip().split("\n")[0] + return False, f"rev-parse merge HEAD failed: {merge_sha}" + merge_sha = merge_sha.strip().split("\n")[0] + # Push to synthetic audit ref _merged/{branch} (does not touch contributor's + # gh-pr-N/* branch). Force-with-lease against expected nothing — the audit + # ref is per-PR and shouldn't pre-exist on a fresh merge. If it does (rare: + # a prior failed attempt), force-push is fine since the ref is bot-owned. rc, out = await _git( - "push", - f"--force-with-lease={branch}:{expected_sha}", - "origin", - f"HEAD:{branch}", - cwd=worktree_path, - timeout=30, + "push", "--force", "origin", f"HEAD:refs/heads/{audit_ref}", + cwd=worktree_path, timeout=30, ) if rc != 0: - return False, f"push rejected: {out}" + return False, f"push to audit ref {audit_ref} failed: {out}" - return True, f"merged --no-ff (external PR #{gh_pr_num})" + # ff-push the merge commit to main. This is a true fast-forward (M is a + # descendant of origin/main via its first parent), so no --force needed. + # Forgejo's branch protection allows ff-push to main from authorized users. + rc, out = await _git( + "push", "origin", f"{merge_sha}:main", + cwd=worktree_path, timeout=30, + ) + if rc != 0: + # Roll back audit ref if main push failed — keeps state consistent. + await _git("push", "--delete", "origin", f"refs/heads/{audit_ref}", + cwd=worktree_path, timeout=15) + return False, f"ff-push to main failed: {out}" + + # Sentinel return: "merged --no-ff" prefix triggers dispatch's external-PR + # close path (skips ff-push, does PR-close + mark_merged + audit). + # Full 40-char merge SHA in the message so dispatch can parse it for audit. + return True, f"merged --no-ff (external PR #{gh_pr_num}, M={merge_sha}, audit_ref={audit_ref})" finally: async with _bare_repo_lock: await _git("worktree", "remove", "--force", worktree_path) + await _git("branch", "-D", local_branch) from .frontmatter import ( @@ -922,6 +957,50 @@ async def _merge_domain_queue(conn, domain: str) -> tuple[int, int]: succeeded += 1 continue + # External GitHub PR (gh-pr-*): _merge_no_ff_external already pushed + # the merge commit to origin/main + the synthetic _merged/{branch} + # audit ref. Skip dispatch's ff-push (would fail — origin/{branch} is + # the contributor's untouched branch, not a descendant of main). + # Just close PR + mark_merged + audit, parsing merge SHA from sentinel. + if pick_msg.startswith("merged --no-ff"): + m = re.search(r"M=([a-f0-9]{40})", pick_msg) + merge_sha = m.group(1) if m else None + m_ref = re.search(r"audit_ref=(\S+?)\)", pick_msg) + audit_ref = m_ref.group(1) if m_ref else None + m_pr = re.search(r"external PR #(\d+)", pick_msg) + gh_pr_num = m_pr.group(1) if m_pr else None + + leo_token = get_agent_token("leo") + comment_body = ( + f"Merged via --no-ff into main.\n" + f"Merge commit: `{merge_sha}`\n" + f"Audit ref: `{audit_ref}`\n" + f"Branch: `{branch}` (preserved unchanged)" + ) + await forgejo_api("POST", repo_path(f"issues/{pr_num}/comments"), + {"body": comment_body}) + result = await forgejo_api("PATCH", repo_path(f"pulls/{pr_num}"), + {"state": "closed"}, token=leo_token) + if result is None: + logger.error("PR #%d: Forgejo close failed (no-ff path), skipping DB update", pr_num) + failed += 1 + continue + mark_merged(conn, pr_num) + db.audit(conn, "merge", "merged", json.dumps({ + "pr": pr_num, "branch": branch, "method": "no-ff", + "merge_commit_sha": merge_sha, + "audit_ref": audit_ref, + "github_pr": gh_pr_num, + })) + # NOTE: do NOT _delete_remote_branch(branch) here. The contributor's + # gh-pr-N/* branch is the mirror of their fork PR head — leaving it + # in place lets sync-mirror keep the GitHub PR <-> Forgejo PR link + # observable. The synthetic _merged/{branch} ref carries the merge. + logger.info("PR #%d merged via --no-ff (M=%s)", pr_num, + merge_sha[:8] if merge_sha else "?") + succeeded += 1 + continue + # Local ff-push: cherry-picked branch is a descendant of origin/main. # Regular push = fast-forward. Non-ff rejected by default (same safety). # --force-with-lease removed: Forgejo categorically blocks it on protected branches.