fix: check Forgejo close return value in 2 merge.py paths to prevent ghost PRs
Both the "already merged" path and _handle_permanent_conflicts closed PRs on Forgejo without checking the return value. On API failure, the DB update would proceed anyway, creating ghost PRs (DB=closed/merged, Forgejo=open). Now both paths check for None return and skip DB updates on failure — same pattern as close_pr in pr_state.py. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
28b25329b3
commit
0ce7412396
1 changed files with 12 additions and 5 deletions
17
lib/merge.py
17
lib/merge.py
|
|
@ -746,12 +746,16 @@ async def _merge_domain_queue(conn, domain: str) -> tuple[int, int]:
|
||||||
# The branch ref still points at old commits (not a descendant of main),
|
# The branch ref still points at old commits (not a descendant of main),
|
||||||
# so pushing branch_sha:main would fail as non-fast-forward.
|
# so pushing branch_sha:main would fail as non-fast-forward.
|
||||||
if pick_msg in ("already merged (all commits empty)", "already up to date"):
|
if pick_msg in ("already merged (all commits empty)", "already up to date"):
|
||||||
mark_merged(conn, pr_num)
|
|
||||||
db.audit(conn, "merge", "merged", json.dumps({"pr": pr_num, "branch": branch, "note": "content already on main"}))
|
|
||||||
leo_token = get_agent_token("leo")
|
leo_token = get_agent_token("leo")
|
||||||
await forgejo_api("POST", repo_path(f"issues/{pr_num}/comments"),
|
await forgejo_api("POST", repo_path(f"issues/{pr_num}/comments"),
|
||||||
{"body": f"Content already on main — closing.\nBranch: `{branch}`"})
|
{"body": f"Content already on main — closing.\nBranch: `{branch}`"})
|
||||||
await forgejo_api("PATCH", repo_path(f"pulls/{pr_num}"), {"state": "closed"}, token=leo_token)
|
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 (already-merged 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, "note": "content already on main"}))
|
||||||
await _delete_remote_branch(branch)
|
await _delete_remote_branch(branch)
|
||||||
logger.info("PR #%d already merged (content on main), closed", pr_num)
|
logger.info("PR #%d already merged (content on main), closed", pr_num)
|
||||||
succeeded += 1
|
succeeded += 1
|
||||||
|
|
@ -964,12 +968,15 @@ async def _handle_permanent_conflicts(conn) -> int:
|
||||||
branch = row["branch"]
|
branch = row["branch"]
|
||||||
domain = row["domain"] or "unknown"
|
domain = row["domain"] or "unknown"
|
||||||
|
|
||||||
# Close PR on Forgejo
|
# Close PR on Forgejo — check return to prevent ghost PR
|
||||||
await forgejo_api(
|
close_result = await forgejo_api(
|
||||||
"PATCH",
|
"PATCH",
|
||||||
repo_path(f"pulls/{pr_number}"),
|
repo_path(f"pulls/{pr_number}"),
|
||||||
body={"state": "closed"},
|
body={"state": "closed"},
|
||||||
)
|
)
|
||||||
|
if close_result is None:
|
||||||
|
logger.error("PR #%d: Forgejo close failed (permanent conflict), skipping", pr_number)
|
||||||
|
continue
|
||||||
await forgejo_api(
|
await forgejo_api(
|
||||||
"POST",
|
"POST",
|
||||||
repo_path(f"issues/{pr_number}/comments"),
|
repo_path(f"issues/{pr_number}/comments"),
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue