fix: return structured conflict-fix push assessment errors (Closes #519)
Merge PR #520 — structured conflict-fix push assessment errors (Closes #519).
This commit was merged in pull request #520.
This commit is contained in:
+108
-4
@@ -2885,6 +2885,85 @@ def gitea_get_pr_review_feedback(
|
||||
}
|
||||
|
||||
|
||||
def _fetch_pr_lease_comments_safe(
|
||||
pr_number: int,
|
||||
*,
|
||||
remote: str,
|
||||
host: str | None,
|
||||
org: str | None,
|
||||
repo: str | None,
|
||||
limit: int = 100,
|
||||
require_open: bool = False,
|
||||
) -> dict:
|
||||
"""Fetch PR thread comments with structured fail-closed errors (#519)."""
|
||||
h, o, r = _resolve(remote, host, org, repo)
|
||||
auth = _auth(h)
|
||||
resolved_repo = f"{o}/{r}"
|
||||
pr_url = f"{repo_api_url(h, o, r)}/pulls/{pr_number}"
|
||||
try:
|
||||
pr = api_request("GET", pr_url, auth)
|
||||
except RuntimeError as exc:
|
||||
return {
|
||||
"success": False,
|
||||
"comments": [],
|
||||
"reasons": [
|
||||
"PR lookup failed before conflict-fix push assessment "
|
||||
f"(pr_number={pr_number}, repo={resolved_repo}, remote={remote}): "
|
||||
f"{_redact(str(exc))}"
|
||||
],
|
||||
"pr_lookup": "failed",
|
||||
"resolved_repo": resolved_repo,
|
||||
"remote": remote,
|
||||
"pr_number": pr_number,
|
||||
}
|
||||
pr_state = (pr.get("state") or "").strip().lower()
|
||||
if require_open and pr_state != "open":
|
||||
return {
|
||||
"success": False,
|
||||
"comments": [],
|
||||
"reasons": [
|
||||
f"PR #{pr_number} on {resolved_repo} is not open "
|
||||
f"(state={pr_state or 'unknown'})"
|
||||
],
|
||||
"pr_lookup": "not_open",
|
||||
"resolved_repo": resolved_repo,
|
||||
"remote": remote,
|
||||
"pr_number": pr_number,
|
||||
"head_sha": (pr.get("head") or {}).get("sha"),
|
||||
}
|
||||
api = f"{repo_api_url(h, o, r)}/issues/{pr_number}/comments"
|
||||
try:
|
||||
comments = api_request("GET", api, auth)
|
||||
except RuntimeError as exc:
|
||||
return {
|
||||
"success": False,
|
||||
"comments": [],
|
||||
"reasons": [
|
||||
"PR comment fetch failed during conflict-fix push assessment "
|
||||
f"(pr_number={pr_number}, issue_index={pr_number}, "
|
||||
f"repo={resolved_repo}, remote={remote}): "
|
||||
f"{_redact(str(exc))}"
|
||||
],
|
||||
"pr_lookup": "ok",
|
||||
"resolved_repo": resolved_repo,
|
||||
"remote": remote,
|
||||
"pr_number": pr_number,
|
||||
"head_sha": (pr.get("head") or {}).get("sha"),
|
||||
}
|
||||
if not isinstance(comments, list):
|
||||
comments = []
|
||||
return {
|
||||
"success": True,
|
||||
"comments": list(comments[:limit]),
|
||||
"reasons": [],
|
||||
"pr_lookup": "ok",
|
||||
"resolved_repo": resolved_repo,
|
||||
"remote": remote,
|
||||
"pr_number": pr_number,
|
||||
"head_sha": (pr.get("head") or {}).get("sha"),
|
||||
}
|
||||
|
||||
|
||||
def _list_pr_lease_comments(
|
||||
pr_number: int,
|
||||
*,
|
||||
@@ -2894,7 +2973,15 @@ def _list_pr_lease_comments(
|
||||
repo: str | None,
|
||||
limit: int = 100,
|
||||
) -> list[dict]:
|
||||
"""Fetch PR/issue thread comments used for reviewer/conflict-fix leases."""
|
||||
"""Fetch PR/issue thread comments used for reviewer/conflict-fix leases.
|
||||
|
||||
Intentionally does **not** pre-fetch the PR via GET /pulls/{n}. Shared
|
||||
callers (merge approval feedback, reviewer lease gates) depend on a single
|
||||
comments GET so mock sequences and fail-open #485 non-list handling stay
|
||||
stable. Structured PR-lookup failures belong only to
|
||||
:func:`_fetch_pr_lease_comments_safe` used by conflict-fix push assessment
|
||||
(#519).
|
||||
"""
|
||||
h, o, r = _resolve(remote, host, org, repo)
|
||||
auth = _auth(h)
|
||||
api = f"{repo_api_url(h, o, r)}/issues/{pr_number}/comments"
|
||||
@@ -8031,22 +8118,39 @@ def gitea_assess_conflict_fix_push(
|
||||
"reasons": read_block,
|
||||
"permission_report": _permission_block_report("gitea.read"),
|
||||
}
|
||||
comments = _list_pr_lease_comments(
|
||||
fetched = _fetch_pr_lease_comments_safe(
|
||||
pr_number,
|
||||
remote=remote,
|
||||
host=host,
|
||||
org=org,
|
||||
repo=repo,
|
||||
require_open=True,
|
||||
)
|
||||
return pr_work_lease.assess_conflict_fix_push(
|
||||
if not fetched["success"]:
|
||||
return {
|
||||
"push_allowed": False,
|
||||
"block": True,
|
||||
"reasons": fetched["reasons"],
|
||||
"pr_lookup": fetched.get("pr_lookup"),
|
||||
"resolved_repo": fetched.get("resolved_repo"),
|
||||
"remote": fetched.get("remote"),
|
||||
"pr_number": pr_number,
|
||||
"assessment_failed": True,
|
||||
}
|
||||
assessment = pr_work_lease.assess_conflict_fix_push(
|
||||
pr_number=pr_number,
|
||||
comments=comments,
|
||||
comments=fetched["comments"],
|
||||
branch_head_before=branch_head_before,
|
||||
branch_head_after=branch_head_after,
|
||||
worktree_path=worktree_path,
|
||||
push_cwd=push_cwd,
|
||||
is_fast_forward=is_fast_forward,
|
||||
)
|
||||
assessment["pr_lookup"] = fetched.get("pr_lookup")
|
||||
assessment["resolved_repo"] = fetched.get("resolved_repo")
|
||||
assessment["remote"] = fetched.get("remote")
|
||||
assessment["live_pr_head_sha"] = fetched.get("head_sha")
|
||||
return assessment
|
||||
|
||||
|
||||
@mcp.tool()
|
||||
|
||||
Reference in New Issue
Block a user