Merge commit '4243b60ca32d79f1ee5e6b0f10d7570e9dea2937' into HEAD
This commit is contained in:
@@ -0,0 +1,142 @@
|
|||||||
|
"""Already-landed open PR reconciliation gates (#310).
|
||||||
|
|
||||||
|
Reconciler workflows may close an open PR only when the PR head SHA is proven
|
||||||
|
an ancestor of a freshly fetched target branch. Arbitrary PR closure is denied.
|
||||||
|
"""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import subprocess
|
||||||
|
from typing import Any
|
||||||
|
|
||||||
|
from merged_cleanup_reconcile import extract_linked_issue, is_head_ancestor_of_ref
|
||||||
|
|
||||||
|
ELIGIBILITY_ALREADY_LANDED = "ALREADY_LANDED_RECONCILE_REQUIRED"
|
||||||
|
ELIGIBILITY_NOT_LANDED = "NOT_ALREADY_LANDED"
|
||||||
|
ELIGIBILITY_STALE_TARGET = "TARGET_BRANCH_UNVERIFIED"
|
||||||
|
ELIGIBILITY_PR_NOT_OPEN = "PR_NOT_OPEN"
|
||||||
|
|
||||||
|
|
||||||
|
def fetch_target_branch(project_root: str, remote: str, branch: str) -> dict[str, Any]:
|
||||||
|
"""Fetch *branch* from *remote* and return the resolved SHA."""
|
||||||
|
ref = f"{remote}/{branch}"
|
||||||
|
fetch = subprocess.run(
|
||||||
|
["git", "-C", project_root, "fetch", remote, branch],
|
||||||
|
capture_output=True,
|
||||||
|
text=True,
|
||||||
|
check=False,
|
||||||
|
)
|
||||||
|
if fetch.returncode != 0:
|
||||||
|
return {
|
||||||
|
"success": False,
|
||||||
|
"target_branch": branch,
|
||||||
|
"target_ref": ref,
|
||||||
|
"target_branch_sha": None,
|
||||||
|
"reasons": [
|
||||||
|
f"git fetch {remote} {branch} failed: "
|
||||||
|
f"{(fetch.stderr or fetch.stdout or '').strip()}"
|
||||||
|
],
|
||||||
|
}
|
||||||
|
|
||||||
|
rev = subprocess.run(
|
||||||
|
["git", "-C", project_root, "rev-parse", ref],
|
||||||
|
capture_output=True,
|
||||||
|
text=True,
|
||||||
|
check=False,
|
||||||
|
)
|
||||||
|
if rev.returncode != 0:
|
||||||
|
return {
|
||||||
|
"success": False,
|
||||||
|
"target_branch": branch,
|
||||||
|
"target_ref": ref,
|
||||||
|
"target_branch_sha": None,
|
||||||
|
"reasons": [
|
||||||
|
f"git rev-parse {ref} failed: "
|
||||||
|
f"{(rev.stderr or rev.stdout or '').strip()}"
|
||||||
|
],
|
||||||
|
}
|
||||||
|
|
||||||
|
return {
|
||||||
|
"success": True,
|
||||||
|
"target_branch": branch,
|
||||||
|
"target_ref": ref,
|
||||||
|
"target_branch_sha": (rev.stdout or "").strip(),
|
||||||
|
"reasons": [],
|
||||||
|
"git_fetch_command": f"git fetch {remote} {branch}",
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
def assess_already_landed_reconciliation(
|
||||||
|
*,
|
||||||
|
pr: dict[str, Any],
|
||||||
|
project_root: str,
|
||||||
|
remote: str,
|
||||||
|
target_branch: str,
|
||||||
|
target_fetch: dict[str, Any] | None = None,
|
||||||
|
) -> dict[str, Any]:
|
||||||
|
"""Return eligibility and proof for reconciling an open PR."""
|
||||||
|
pr_number = int(pr.get("number") or 0)
|
||||||
|
pr_state = (pr.get("state") or "").strip().lower()
|
||||||
|
head_sha = (pr.get("head") or {}).get("sha") if isinstance(pr.get("head"), dict) else None
|
||||||
|
if not head_sha and isinstance(pr.get("head"), str):
|
||||||
|
head_sha = None
|
||||||
|
head_ref = (pr.get("head") or {}).get("ref") if isinstance(pr.get("head"), dict) else pr.get("head")
|
||||||
|
base_ref = (pr.get("base") or {}).get("ref") if isinstance(pr.get("base"), dict) else pr.get("base")
|
||||||
|
title = pr.get("title") or ""
|
||||||
|
body = pr.get("body") or ""
|
||||||
|
|
||||||
|
fetch_result = target_fetch or fetch_target_branch(project_root, remote, target_branch)
|
||||||
|
linked_issue = extract_linked_issue(title, body)
|
||||||
|
|
||||||
|
result: dict[str, Any] = {
|
||||||
|
"pr_number": pr_number,
|
||||||
|
"pr_state": pr_state,
|
||||||
|
"candidate_head_sha": head_sha,
|
||||||
|
"head_ref": head_ref,
|
||||||
|
"base_ref": base_ref or target_branch,
|
||||||
|
"target_branch": target_branch,
|
||||||
|
"target_branch_sha": fetch_result.get("target_branch_sha"),
|
||||||
|
"linked_issue": linked_issue,
|
||||||
|
"git_ref_mutations": [],
|
||||||
|
"reasons": [],
|
||||||
|
}
|
||||||
|
if fetch_result.get("git_fetch_command"):
|
||||||
|
result["git_ref_mutations"].append(fetch_result["git_fetch_command"])
|
||||||
|
|
||||||
|
if pr_state != "open":
|
||||||
|
result["eligibility_class"] = ELIGIBILITY_PR_NOT_OPEN
|
||||||
|
result["ancestor_proof"] = None
|
||||||
|
result["close_allowed"] = False
|
||||||
|
result["reasons"].append(f"PR #{pr_number} state is {pr_state!r}, not open")
|
||||||
|
return result
|
||||||
|
|
||||||
|
if not fetch_result.get("success"):
|
||||||
|
result["eligibility_class"] = ELIGIBILITY_STALE_TARGET
|
||||||
|
result["ancestor_proof"] = None
|
||||||
|
result["close_allowed"] = False
|
||||||
|
result["reasons"].extend(fetch_result.get("reasons") or [])
|
||||||
|
return result
|
||||||
|
|
||||||
|
target_ref = fetch_result.get("target_ref") or f"{remote}/{target_branch}"
|
||||||
|
ancestor = is_head_ancestor_of_ref(project_root, head_sha, target_ref)
|
||||||
|
result["ancestor_proof"] = ancestor
|
||||||
|
|
||||||
|
if ancestor is None:
|
||||||
|
result["eligibility_class"] = ELIGIBILITY_STALE_TARGET
|
||||||
|
result["close_allowed"] = False
|
||||||
|
result["reasons"].append(
|
||||||
|
f"ancestor check failed for head {head_sha!r} against {target_ref}"
|
||||||
|
)
|
||||||
|
return result
|
||||||
|
|
||||||
|
if ancestor:
|
||||||
|
result["eligibility_class"] = ELIGIBILITY_ALREADY_LANDED
|
||||||
|
result["close_allowed"] = True
|
||||||
|
return result
|
||||||
|
|
||||||
|
result["eligibility_class"] = ELIGIBILITY_NOT_LANDED
|
||||||
|
result["close_allowed"] = False
|
||||||
|
result["reasons"].append(
|
||||||
|
f"PR head {head_sha} is not an ancestor of {target_ref}"
|
||||||
|
)
|
||||||
|
return result
|
||||||
@@ -23,6 +23,7 @@ launched with exactly one static execution profile:
|
|||||||
|-----------------------------|----------------|-------------|
|
|-----------------------------|----------------|-------------|
|
||||||
| `gitea-author` | an author profile | implement issues, push branches, open PRs, comment |
|
| `gitea-author` | an author profile | implement issues, push branches, open PRs, comment |
|
||||||
| `gitea-reviewer` | a reviewer profile | review, approve/request changes, merge |
|
| `gitea-reviewer` | a reviewer profile | review, approve/request changes, merge |
|
||||||
|
| `gitea-reconciler` | a reconciler profile | close already-landed open PRs after ancestry proof (#310) |
|
||||||
|
|
||||||
Properties:
|
Properties:
|
||||||
|
|
||||||
|
|||||||
@@ -226,6 +226,24 @@ explicit operator-directed closure of a contaminated PR). If `close_pr` ever
|
|||||||
resolves as unknown, agents must fail closed rather than fall back to the
|
resolves as unknown, agents must fail closed rather than fall back to the
|
||||||
edit path.
|
edit path.
|
||||||
|
|
||||||
|
## Reconciler profile for already-landed open PRs (#310)
|
||||||
|
|
||||||
|
Normal author and reviewer profiles must not gain broad PR-close authority.
|
||||||
|
Already-landed open PRs (head SHA is an ancestor of the target branch) need a
|
||||||
|
dedicated reconciler profile such as `prgs-reconciler` with:
|
||||||
|
|
||||||
|
- `gitea.read`
|
||||||
|
- `gitea.pr.comment`
|
||||||
|
- `gitea.issue.comment`
|
||||||
|
- `gitea.issue.close`
|
||||||
|
- `gitea.pr.close`
|
||||||
|
|
||||||
|
Use the `gitea-reconciler` MCP namespace (static profile launch) and the
|
||||||
|
`gitea_reconcile_already_landed_pr` tool. The resolver task is
|
||||||
|
`reconcile_already_landed_pr`. PR close is allowed only after live PR fetch,
|
||||||
|
fresh target-branch fetch, recorded target SHA, and ancestor proof. PRs whose
|
||||||
|
heads are not already landed cannot be closed through this path.
|
||||||
|
|
||||||
## Identity and fail-closed rules
|
## Identity and fail-closed rules
|
||||||
|
|
||||||
Before **any** mutating action, a workflow must know both:
|
Before **any** mutating action, a workflow must know both:
|
||||||
|
|||||||
@@ -169,6 +169,25 @@ To avoid the bottleneck of relaunching/restarting the MCP server to switch betwe
|
|||||||
* `mcp__gitea-reviewer__*` (for reviewing PRs, approving, requesting changes, merging)
|
* `mcp__gitea-reviewer__*` (for reviewing PRs, approving, requesting changes, merging)
|
||||||
* **Trust Model:** Separate tokens remain separate in the keychain/environment. Each instance operates under its own `GITEA_MCP_PROFILE` and enforces its own `allowed_operations`. A runtime `whoami` identity check is still performed independently, and self-review/self-merge checks remain strictly mandatory. The dual-server pattern is a operational convenience and never a security bypass.
|
* **Trust Model:** Separate tokens remain separate in the keychain/environment. Each instance operates under its own `GITEA_MCP_PROFILE` and enforces its own `allowed_operations`. A runtime `whoami` identity check is still performed independently, and self-review/self-merge checks remain strictly mandatory. The dual-server pattern is a operational convenience and never a security bypass.
|
||||||
* **Reviewer-Identity PR Creation Deadlock:** Reviewer/merge identities must not create PRs or push branches. Doing so makes the reviewer identity the PR author in Gitea, blocking subsequent independent review and causing a review deadlock. Normally, PRs must be created by the author/work identity (`gitea-author`), leaving the reviewer identity (`gitea-reviewer`) clean and available for independent review and merge.
|
* **Reviewer-Identity PR Creation Deadlock:** Reviewer/merge identities must not create PRs or push branches. Doing so makes the reviewer identity the PR author in Gitea, blocking subsequent independent review and causing a review deadlock. Normally, PRs must be created by the author/work identity (`gitea-author`), leaving the reviewer identity (`gitea-reviewer`) clean and available for independent review and merge.
|
||||||
|
* **Reconciler namespace (#310):** Register a third static instance for
|
||||||
|
already-landed PR cleanup when review queues block on open PRs whose heads
|
||||||
|
already landed on `master`:
|
||||||
|
|
||||||
|
```json
|
||||||
|
"gitea-reconciler": {
|
||||||
|
"command": "/path/to/Gitea-Tools/venv/bin/python3",
|
||||||
|
"args": ["/path/to/Gitea-Tools/mcp_server.py"],
|
||||||
|
"env": {
|
||||||
|
"GITEA_MCP_CONFIG": "/path/to/.config/gitea-tools/profiles.json",
|
||||||
|
"GITEA_MCP_PROFILE": "prgs-reconciler"
|
||||||
|
}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
The reconciler profile grants `gitea.pr.close` only for
|
||||||
|
`gitea_reconcile_already_landed_pr` after ancestry proof — not for normal
|
||||||
|
review or author workflows.
|
||||||
|
|
||||||
* **Fallback:** If the dual-profile MCP launcher pattern is not supported or configured in the client, the LLM must relaunch or restart the client/MCP with the correct profile environment variable before claiming or working on any tasks.
|
* **Fallback:** If the dual-profile MCP launcher pattern is not supported or configured in the client, the LLM must relaunch or restart the client/MCP with the correct profile environment variable before claiming or working on any tasks.
|
||||||
|
|
||||||
## Setup runbook — interactive menu
|
## Setup runbook — interactive menu
|
||||||
|
|||||||
+194
-2
@@ -476,6 +476,7 @@ import task_capability_map # noqa: E402
|
|||||||
import review_proofs # noqa: E402
|
import review_proofs # noqa: E402
|
||||||
import agent_temp_artifacts
|
import agent_temp_artifacts
|
||||||
import issue_lock_worktree # noqa: E402
|
import issue_lock_worktree # noqa: E402
|
||||||
|
import already_landed_reconcile # noqa: E402
|
||||||
import issue_claim_heartbeat # noqa: E402
|
import issue_claim_heartbeat # noqa: E402
|
||||||
import merged_cleanup_reconcile # noqa: E402
|
import merged_cleanup_reconcile # noqa: E402
|
||||||
import review_merge_state_machine # noqa: E402
|
import review_merge_state_machine # noqa: E402
|
||||||
@@ -3428,6 +3429,186 @@ def gitea_reconcile_merged_cleanups(
|
|||||||
return {"success": True, "performed": True, **report}
|
return {"success": True, "performed": True, **report}
|
||||||
|
|
||||||
|
|
||||||
|
@mcp.tool()
|
||||||
|
def gitea_reconcile_already_landed_pr(
|
||||||
|
pr_number: int,
|
||||||
|
close_pr: bool = False,
|
||||||
|
close_linked_issue: bool = False,
|
||||||
|
post_comment: bool = False,
|
||||||
|
comment_body: str = "",
|
||||||
|
target_branch: str = "master",
|
||||||
|
remote: str = "dadeschools",
|
||||||
|
host: str | None = None,
|
||||||
|
org: str | None = None,
|
||||||
|
repo: str | None = None,
|
||||||
|
) -> dict:
|
||||||
|
"""Reconcile an open PR whose head is already on the target branch (#310).
|
||||||
|
|
||||||
|
Read-only assessment always runs. PR close requires ``gitea.pr.close`` and
|
||||||
|
proof that the pinned PR head SHA is an ancestor of a freshly fetched
|
||||||
|
target branch. Arbitrary PR closure is denied.
|
||||||
|
|
||||||
|
Args:
|
||||||
|
pr_number: Open PR to reconcile.
|
||||||
|
close_pr: When True, close the PR after already-landed proof passes.
|
||||||
|
close_linked_issue: When True, close a linked issue after live fetch
|
||||||
|
when the issue is open and ``gitea.issue.close`` is allowed.
|
||||||
|
post_comment: When True, post ``comment_body`` on the PR thread.
|
||||||
|
comment_body: PR comment text when ``post_comment`` is True.
|
||||||
|
target_branch: Target branch used for ancestry proof (default master).
|
||||||
|
remote: Known instance — 'dadeschools' or 'prgs'.
|
||||||
|
host: Override the Gitea host.
|
||||||
|
org: Override the owner/organization.
|
||||||
|
repo: Override the repository name.
|
||||||
|
|
||||||
|
Returns:
|
||||||
|
dict with eligibility proof, optional mutations, and recovery guidance.
|
||||||
|
"""
|
||||||
|
read_block = _profile_operation_gate("gitea.read")
|
||||||
|
if read_block:
|
||||||
|
return {
|
||||||
|
"success": False,
|
||||||
|
"performed": False,
|
||||||
|
"reasons": read_block,
|
||||||
|
"permission_report": _permission_block_report("gitea.read"),
|
||||||
|
}
|
||||||
|
|
||||||
|
if close_pr:
|
||||||
|
close_block = _profile_operation_gate("gitea.pr.close")
|
||||||
|
if close_block:
|
||||||
|
return {
|
||||||
|
"success": False,
|
||||||
|
"performed": False,
|
||||||
|
"pr_number": pr_number,
|
||||||
|
"required_permission": "gitea.pr.close",
|
||||||
|
"reasons": close_block,
|
||||||
|
"permission_report": _permission_block_report("gitea.pr.close"),
|
||||||
|
"safe_next_action": (
|
||||||
|
"Launch the prgs-reconciler MCP namespace or configure a "
|
||||||
|
"profile with gitea.pr.close for already-landed cleanup."
|
||||||
|
),
|
||||||
|
}
|
||||||
|
|
||||||
|
h, o, r = _resolve(remote, host, org, repo)
|
||||||
|
auth = _auth(h)
|
||||||
|
base = repo_api_url(h, o, r)
|
||||||
|
pr_url = f"{base}/pulls/{pr_number}"
|
||||||
|
pr = api_request("GET", pr_url, auth)
|
||||||
|
|
||||||
|
assessment = already_landed_reconcile.assess_already_landed_reconciliation(
|
||||||
|
pr=pr,
|
||||||
|
project_root=PROJECT_ROOT,
|
||||||
|
remote=remote,
|
||||||
|
target_branch=target_branch,
|
||||||
|
)
|
||||||
|
|
||||||
|
result: dict = {
|
||||||
|
"success": True,
|
||||||
|
"performed": False,
|
||||||
|
"pr_number": pr_number,
|
||||||
|
"pr_state": assessment.get("pr_state"),
|
||||||
|
"candidate_head_sha": assessment.get("candidate_head_sha"),
|
||||||
|
"target_branch": assessment.get("target_branch"),
|
||||||
|
"target_branch_sha": assessment.get("target_branch_sha"),
|
||||||
|
"ancestor_proof": assessment.get("ancestor_proof"),
|
||||||
|
"eligibility_class": assessment.get("eligibility_class"),
|
||||||
|
"linked_issue": assessment.get("linked_issue"),
|
||||||
|
"close_allowed": assessment.get("close_allowed"),
|
||||||
|
"git_ref_mutations": assessment.get("git_ref_mutations") or [],
|
||||||
|
"reasons": list(assessment.get("reasons") or []),
|
||||||
|
"pr_closed": False,
|
||||||
|
"issue_closed": False,
|
||||||
|
"pr_comment_posted": False,
|
||||||
|
}
|
||||||
|
|
||||||
|
if not assessment.get("close_allowed"):
|
||||||
|
result["success"] = False
|
||||||
|
result["safe_next_action"] = (
|
||||||
|
"Do not close this PR via the reconciler path until "
|
||||||
|
"already-landed proof passes."
|
||||||
|
)
|
||||||
|
return result
|
||||||
|
|
||||||
|
verify_preflight_purity(remote)
|
||||||
|
|
||||||
|
if post_comment and comment_body.strip():
|
||||||
|
comment_block = _profile_operation_gate("gitea.pr.comment")
|
||||||
|
if comment_block:
|
||||||
|
result["success"] = False
|
||||||
|
result["reasons"].extend(comment_block)
|
||||||
|
result["permission_report"] = _permission_block_report(
|
||||||
|
"gitea.pr.comment"
|
||||||
|
)
|
||||||
|
return result
|
||||||
|
comment_url = f"{base}/issues/{pr_number}/comments"
|
||||||
|
with _audited(
|
||||||
|
"comment_pr",
|
||||||
|
host=h,
|
||||||
|
remote=remote,
|
||||||
|
org=o,
|
||||||
|
repo=r,
|
||||||
|
pr_number=pr_number,
|
||||||
|
request_metadata={"source": "reconcile_already_landed_pr"},
|
||||||
|
):
|
||||||
|
api_request("POST", comment_url, auth, {"body": comment_body.strip()})
|
||||||
|
result["pr_comment_posted"] = True
|
||||||
|
result["performed"] = True
|
||||||
|
|
||||||
|
if close_pr:
|
||||||
|
patch_url = f"{base}/pulls/{pr_number}"
|
||||||
|
request_metadata = {
|
||||||
|
"source": "reconcile_already_landed_pr",
|
||||||
|
"required_permission": "gitea.pr.close",
|
||||||
|
"candidate_head_sha": assessment.get("candidate_head_sha"),
|
||||||
|
"target_branch_sha": assessment.get("target_branch_sha"),
|
||||||
|
"ancestor_proof": assessment.get("ancestor_proof"),
|
||||||
|
}
|
||||||
|
with _audited(
|
||||||
|
"close_pr",
|
||||||
|
host=h,
|
||||||
|
remote=remote,
|
||||||
|
org=o,
|
||||||
|
repo=r,
|
||||||
|
pr_number=pr_number,
|
||||||
|
request_metadata=request_metadata,
|
||||||
|
):
|
||||||
|
closed = api_request("PATCH", patch_url, auth, {"state": "closed"})
|
||||||
|
result["pr_closed"] = True
|
||||||
|
result["performed"] = True
|
||||||
|
result["pr_state"] = closed.get("state", "closed")
|
||||||
|
|
||||||
|
linked_issue = assessment.get("linked_issue")
|
||||||
|
if close_linked_issue and linked_issue:
|
||||||
|
issue_block = _profile_operation_gate("gitea.issue.close")
|
||||||
|
if issue_block:
|
||||||
|
result["reasons"].extend(issue_block)
|
||||||
|
result["issue_close_blocked"] = True
|
||||||
|
return result
|
||||||
|
issue_url = f"{base}/issues/{linked_issue}"
|
||||||
|
issue = api_request("GET", issue_url, auth)
|
||||||
|
result["linked_issue_live_status"] = issue.get("state")
|
||||||
|
if (issue.get("state") or "").lower() == "open":
|
||||||
|
with _audited(
|
||||||
|
"close_issue",
|
||||||
|
host=h,
|
||||||
|
remote=remote,
|
||||||
|
org=o,
|
||||||
|
repo=r,
|
||||||
|
issue_number=linked_issue,
|
||||||
|
request_metadata={"source": "reconcile_already_landed_pr"},
|
||||||
|
):
|
||||||
|
api_request(
|
||||||
|
"PATCH",
|
||||||
|
issue_url,
|
||||||
|
auth,
|
||||||
|
{"state": "closed"},
|
||||||
|
)
|
||||||
|
result["issue_closed"] = True
|
||||||
|
result["performed"] = True
|
||||||
|
|
||||||
|
return result
|
||||||
|
|
||||||
|
|
||||||
@mcp.tool()
|
@mcp.tool()
|
||||||
def gitea_close_issue(
|
def gitea_close_issue(
|
||||||
issue_number: int,
|
issue_number: int,
|
||||||
@@ -3938,17 +4119,26 @@ def _role_kind(allowed, forbidden) -> str:
|
|||||||
"""Classify the active profile from its normalized operations.
|
"""Classify the active profile from its normalized operations.
|
||||||
|
|
||||||
'author' can create PRs / push branches; 'reviewer' can approve/merge;
|
'author' can create PRs / push branches; 'reviewer' can approve/merge;
|
||||||
'mixed' can do both (a config smell — the reviewer-deadlock invariant
|
'reconciler' can close already-landed PRs via ``gitea.pr.close`` without
|
||||||
forbids it in v2 configs); 'limited' can do neither.
|
review/author powers; 'mixed' can do both (a config smell — the
|
||||||
|
reviewer-deadlock invariant forbids it in v2 configs); 'limited' can do
|
||||||
|
neither.
|
||||||
"""
|
"""
|
||||||
def can(op):
|
def can(op):
|
||||||
return gitea_config.check_operation(op, allowed, forbidden)[0]
|
return gitea_config.check_operation(op, allowed, forbidden)[0]
|
||||||
review = can("gitea.pr.approve") or can("gitea.pr.merge")
|
review = can("gitea.pr.approve") or can("gitea.pr.merge")
|
||||||
author = can("gitea.pr.create") or can("gitea.branch.push")
|
author = can("gitea.pr.create") or can("gitea.branch.push")
|
||||||
|
reconciler = (
|
||||||
|
can("gitea.pr.close")
|
||||||
|
and not review
|
||||||
|
and not author
|
||||||
|
)
|
||||||
if review and author:
|
if review and author:
|
||||||
return "mixed"
|
return "mixed"
|
||||||
if review:
|
if review:
|
||||||
return "reviewer"
|
return "reviewer"
|
||||||
|
if reconciler:
|
||||||
|
return "reconciler"
|
||||||
if author:
|
if author:
|
||||||
return "author"
|
return "author"
|
||||||
return "limited"
|
return "limited"
|
||||||
@@ -4728,6 +4918,7 @@ _RUNTIME_CAPABILITY_TASKS = (
|
|||||||
"review_pr",
|
"review_pr",
|
||||||
"merge_pr",
|
"merge_pr",
|
||||||
"close_issue",
|
"close_issue",
|
||||||
|
"reconcile_already_landed_pr",
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
@@ -4779,6 +4970,7 @@ def _build_runtime_task_capabilities(
|
|||||||
"review_pr": "can_review_prs",
|
"review_pr": "can_review_prs",
|
||||||
"merge_pr": "can_merge_prs",
|
"merge_pr": "can_merge_prs",
|
||||||
"close_issue": "can_close_issues",
|
"close_issue": "can_close_issues",
|
||||||
|
"reconcile_already_landed_pr": "can_reconcile_already_landed_prs",
|
||||||
}
|
}
|
||||||
for task in _RUNTIME_CAPABILITY_TASKS:
|
for task in _RUNTIME_CAPABILITY_TASKS:
|
||||||
permission = task_capability_map.required_permission(task)
|
permission = task_capability_map.required_permission(task)
|
||||||
|
|||||||
@@ -31,6 +31,8 @@ REVIEWER_DEFAULT_FORBIDDEN = ["branch", "commit", "push", "open_pr"]
|
|||||||
def infer_role(name, execution_profile):
|
def infer_role(name, execution_profile):
|
||||||
"""Return the unambiguous role for a legacy profile name, or None."""
|
"""Return the unambiguous role for a legacy profile name, or None."""
|
||||||
haystack = f"{name} {execution_profile or ''}".lower()
|
haystack = f"{name} {execution_profile or ''}".lower()
|
||||||
|
if "reconciler" in haystack:
|
||||||
|
return "reconciler"
|
||||||
has_author = "author" in haystack
|
has_author = "author" in haystack
|
||||||
has_reviewer = "reviewer" in haystack
|
has_reviewer = "reviewer" in haystack
|
||||||
if has_author == has_reviewer:
|
if has_author == has_reviewer:
|
||||||
|
|||||||
@@ -74,6 +74,14 @@ AUTHOR_TASKS = frozenset({
|
|||||||
"comment_pr",
|
"comment_pr",
|
||||||
"address_pr_change_requests",
|
"address_pr_change_requests",
|
||||||
"delete_branch",
|
"delete_branch",
|
||||||
|
"work_issue",
|
||||||
|
"work-issue",
|
||||||
|
})
|
||||||
|
|
||||||
|
RECONCILER_TASKS = frozenset({
|
||||||
|
"reconcile_already_landed_pr",
|
||||||
|
"reconcile_already_landed",
|
||||||
|
"reconcile-landed-pr",
|
||||||
})
|
})
|
||||||
|
|
||||||
TASK_REQUIRED_ROLE = {
|
TASK_REQUIRED_ROLE = {
|
||||||
@@ -94,6 +102,11 @@ TASK_REQUIRED_ROLE = {
|
|||||||
"pr-queue-cleanup": "reviewer",
|
"pr-queue-cleanup": "reviewer",
|
||||||
"request_changes_pr": "reviewer",
|
"request_changes_pr": "reviewer",
|
||||||
"approve_pr": "reviewer",
|
"approve_pr": "reviewer",
|
||||||
|
"work_issue": "author",
|
||||||
|
"work-issue": "author",
|
||||||
|
"reconcile_already_landed_pr": "reconciler",
|
||||||
|
"reconcile_already_landed": "reconciler",
|
||||||
|
"reconcile-landed-pr": "reconciler",
|
||||||
# #309: reconciler tasks close already-landed PRs/issues only.
|
# #309: reconciler tasks close already-landed PRs/issues only.
|
||||||
"reconcile_close_landed_pr": "reconciler",
|
"reconcile_close_landed_pr": "reconciler",
|
||||||
"reconcile_close_landed_issue": "reconciler",
|
"reconcile_close_landed_issue": "reconciler",
|
||||||
|
|||||||
@@ -104,6 +104,10 @@ TASK_CAPABILITY_MAP: dict[str, dict[str, str]] = {
|
|||||||
"permission": "gitea.read",
|
"permission": "gitea.read",
|
||||||
"role": "author",
|
"role": "author",
|
||||||
},
|
},
|
||||||
|
"reconcile_already_landed_pr": {
|
||||||
|
"permission": "gitea.pr.close",
|
||||||
|
"role": "reconciler",
|
||||||
|
},
|
||||||
# #309: dedicated reconciler path for already-landed open PRs. Exact
|
# #309: dedicated reconciler path for already-landed open PRs. Exact
|
||||||
# close capabilities only — never review/approve/request_changes/merge.
|
# close capabilities only — never review/approve/request_changes/merge.
|
||||||
"reconcile_close_landed_pr": {
|
"reconcile_close_landed_pr": {
|
||||||
|
|||||||
@@ -0,0 +1,131 @@
|
|||||||
|
"""Tests for already-landed PR reconciliation gates (#310)."""
|
||||||
|
import os
|
||||||
|
import sys
|
||||||
|
import unittest
|
||||||
|
from unittest.mock import patch
|
||||||
|
|
||||||
|
sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.parent))
|
||||||
|
|
||||||
|
import already_landed_reconcile
|
||||||
|
|
||||||
|
|
||||||
|
class TestAssessAlreadyLandedReconciliation(unittest.TestCase):
|
||||||
|
def test_open_pr_already_landed_allows_close(self):
|
||||||
|
with patch(
|
||||||
|
"already_landed_reconcile.is_head_ancestor_of_ref",
|
||||||
|
return_value=True,
|
||||||
|
):
|
||||||
|
assessment = already_landed_reconcile.assess_already_landed_reconciliation(
|
||||||
|
pr={
|
||||||
|
"number": 278,
|
||||||
|
"state": "open",
|
||||||
|
"title": "Fix thing (Closes #263)",
|
||||||
|
"body": "",
|
||||||
|
"head": {"ref": "feat/x", "sha": "abc123"},
|
||||||
|
"base": {"ref": "master"},
|
||||||
|
},
|
||||||
|
project_root="/tmp/repo",
|
||||||
|
remote="prgs",
|
||||||
|
target_branch="master",
|
||||||
|
target_fetch={
|
||||||
|
"success": True,
|
||||||
|
"target_branch": "master",
|
||||||
|
"target_ref": "prgs/master",
|
||||||
|
"target_branch_sha": "deadbeef",
|
||||||
|
"git_fetch_command": "git fetch prgs master",
|
||||||
|
},
|
||||||
|
)
|
||||||
|
self.assertTrue(assessment["close_allowed"])
|
||||||
|
self.assertEqual(
|
||||||
|
assessment["eligibility_class"],
|
||||||
|
already_landed_reconcile.ELIGIBILITY_ALREADY_LANDED,
|
||||||
|
)
|
||||||
|
self.assertEqual(assessment["linked_issue"], 263)
|
||||||
|
self.assertTrue(assessment["ancestor_proof"])
|
||||||
|
|
||||||
|
def test_not_landed_pr_denies_close(self):
|
||||||
|
with patch(
|
||||||
|
"already_landed_reconcile.is_head_ancestor_of_ref",
|
||||||
|
return_value=False,
|
||||||
|
):
|
||||||
|
assessment = already_landed_reconcile.assess_already_landed_reconciliation(
|
||||||
|
pr={
|
||||||
|
"number": 99,
|
||||||
|
"state": "open",
|
||||||
|
"title": "WIP",
|
||||||
|
"body": "",
|
||||||
|
"head": {"ref": "feat/y", "sha": "fff111"},
|
||||||
|
"base": {"ref": "master"},
|
||||||
|
},
|
||||||
|
project_root="/tmp/repo",
|
||||||
|
remote="prgs",
|
||||||
|
target_branch="master",
|
||||||
|
target_fetch={
|
||||||
|
"success": True,
|
||||||
|
"target_branch": "master",
|
||||||
|
"target_ref": "prgs/master",
|
||||||
|
"target_branch_sha": "deadbeef",
|
||||||
|
},
|
||||||
|
)
|
||||||
|
self.assertFalse(assessment["close_allowed"])
|
||||||
|
self.assertEqual(
|
||||||
|
assessment["eligibility_class"],
|
||||||
|
already_landed_reconcile.ELIGIBILITY_NOT_LANDED,
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_stale_target_branch_denies_close(self):
|
||||||
|
assessment = already_landed_reconcile.assess_already_landed_reconciliation(
|
||||||
|
pr={
|
||||||
|
"number": 99,
|
||||||
|
"state": "open",
|
||||||
|
"title": "WIP",
|
||||||
|
"body": "",
|
||||||
|
"head": {"ref": "feat/y", "sha": "fff111"},
|
||||||
|
"base": {"ref": "master"},
|
||||||
|
},
|
||||||
|
project_root="/tmp/repo",
|
||||||
|
remote="prgs",
|
||||||
|
target_branch="master",
|
||||||
|
target_fetch={
|
||||||
|
"success": False,
|
||||||
|
"target_branch": "master",
|
||||||
|
"target_ref": "prgs/master",
|
||||||
|
"target_branch_sha": None,
|
||||||
|
"reasons": ["git fetch failed"],
|
||||||
|
},
|
||||||
|
)
|
||||||
|
self.assertFalse(assessment["close_allowed"])
|
||||||
|
self.assertEqual(
|
||||||
|
assessment["eligibility_class"],
|
||||||
|
already_landed_reconcile.ELIGIBILITY_STALE_TARGET,
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_closed_pr_denies_close(self):
|
||||||
|
assessment = already_landed_reconcile.assess_already_landed_reconciliation(
|
||||||
|
pr={
|
||||||
|
"number": 50,
|
||||||
|
"state": "closed",
|
||||||
|
"title": "Done",
|
||||||
|
"body": "",
|
||||||
|
"head": {"ref": "feat/z", "sha": "aaa"},
|
||||||
|
"base": {"ref": "master"},
|
||||||
|
},
|
||||||
|
project_root="/tmp/repo",
|
||||||
|
remote="prgs",
|
||||||
|
target_branch="master",
|
||||||
|
target_fetch={
|
||||||
|
"success": True,
|
||||||
|
"target_branch": "master",
|
||||||
|
"target_ref": "prgs/master",
|
||||||
|
"target_branch_sha": "deadbeef",
|
||||||
|
},
|
||||||
|
)
|
||||||
|
self.assertFalse(assessment["close_allowed"])
|
||||||
|
self.assertEqual(
|
||||||
|
assessment["eligibility_class"],
|
||||||
|
already_landed_reconcile.ELIGIBILITY_PR_NOT_OPEN,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
if __name__ == "__main__":
|
||||||
|
unittest.main()
|
||||||
@@ -0,0 +1,149 @@
|
|||||||
|
"""Tests for gitea_reconcile_already_landed_pr MCP tool (#310)."""
|
||||||
|
import sys
|
||||||
|
import unittest
|
||||||
|
from unittest.mock import patch
|
||||||
|
|
||||||
|
sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.parent))
|
||||||
|
|
||||||
|
import mcp_server
|
||||||
|
from mcp_server import gitea_reconcile_already_landed_pr
|
||||||
|
|
||||||
|
RECONCILER_PROFILE = {
|
||||||
|
"profile_name": "prgs-reconciler",
|
||||||
|
"allowed_operations": [
|
||||||
|
"gitea.read",
|
||||||
|
"gitea.pr.comment",
|
||||||
|
"gitea.issue.comment",
|
||||||
|
"gitea.issue.close",
|
||||||
|
"gitea.pr.close",
|
||||||
|
],
|
||||||
|
"forbidden_operations": [
|
||||||
|
"gitea.pr.approve",
|
||||||
|
"gitea.pr.merge",
|
||||||
|
"gitea.pr.create",
|
||||||
|
"gitea.branch.push",
|
||||||
|
],
|
||||||
|
"audit_label": "prgs-reconciler",
|
||||||
|
}
|
||||||
|
|
||||||
|
AUTHOR_PROFILE = {
|
||||||
|
"profile_name": "prgs-author",
|
||||||
|
"allowed_operations": [
|
||||||
|
"gitea.read",
|
||||||
|
"gitea.pr.create",
|
||||||
|
"gitea.issue.comment",
|
||||||
|
],
|
||||||
|
"forbidden_operations": ["gitea.pr.close", "gitea.pr.approve", "gitea.pr.merge"],
|
||||||
|
"audit_label": "prgs-author",
|
||||||
|
}
|
||||||
|
|
||||||
|
OPEN_LANDED_PR = {
|
||||||
|
"number": 278,
|
||||||
|
"title": "Already landed (Closes #263)",
|
||||||
|
"body": "",
|
||||||
|
"state": "open",
|
||||||
|
"head": {"ref": "feat/issue-263", "sha": "2a544b78d1371925761d16f1c451bb3b2984470e"},
|
||||||
|
"base": {"ref": "master"},
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
class TestReconcileAlreadyLandedPrTool(unittest.TestCase):
|
||||||
|
def setUp(self):
|
||||||
|
self.mock_api = patch("mcp_server.api_request").start()
|
||||||
|
self.mock_auth = patch(
|
||||||
|
"mcp_server.get_auth_header", return_value="token test"
|
||||||
|
).start()
|
||||||
|
patch("mcp_server.verify_preflight_purity").start()
|
||||||
|
patch("gitea_audit.audit_enabled", return_value=False).start()
|
||||||
|
mcp_server._IDENTITY_CACHE.clear()
|
||||||
|
|
||||||
|
def tearDown(self):
|
||||||
|
patch.stopall()
|
||||||
|
mcp_server._IDENTITY_CACHE.clear()
|
||||||
|
|
||||||
|
def _set_profile(self, profile):
|
||||||
|
patch("mcp_server.get_profile", return_value=profile).start()
|
||||||
|
|
||||||
|
def test_close_blocked_without_pr_close_capability(self):
|
||||||
|
self._set_profile(AUTHOR_PROFILE)
|
||||||
|
res = gitea_reconcile_already_landed_pr(
|
||||||
|
pr_number=278,
|
||||||
|
close_pr=True,
|
||||||
|
remote="prgs",
|
||||||
|
)
|
||||||
|
self.assertFalse(res["success"])
|
||||||
|
self.assertFalse(res["performed"])
|
||||||
|
self.assertEqual(res["required_permission"], "gitea.pr.close")
|
||||||
|
self.mock_api.assert_not_called()
|
||||||
|
|
||||||
|
def test_not_landed_pr_close_denied_after_live_fetch(self):
|
||||||
|
self._set_profile(RECONCILER_PROFILE)
|
||||||
|
self.mock_api.return_value = OPEN_LANDED_PR
|
||||||
|
with patch(
|
||||||
|
"mcp_server.already_landed_reconcile.assess_already_landed_reconciliation",
|
||||||
|
return_value={
|
||||||
|
"pr_state": "open",
|
||||||
|
"candidate_head_sha": OPEN_LANDED_PR["head"]["sha"],
|
||||||
|
"target_branch": "master",
|
||||||
|
"target_branch_sha": "deadbeef",
|
||||||
|
"ancestor_proof": False,
|
||||||
|
"eligibility_class": "NOT_ALREADY_LANDED",
|
||||||
|
"linked_issue": 263,
|
||||||
|
"close_allowed": False,
|
||||||
|
"git_ref_mutations": ["git fetch prgs master"],
|
||||||
|
"reasons": ["not ancestor"],
|
||||||
|
},
|
||||||
|
):
|
||||||
|
res = gitea_reconcile_already_landed_pr(
|
||||||
|
pr_number=278,
|
||||||
|
close_pr=True,
|
||||||
|
remote="prgs",
|
||||||
|
)
|
||||||
|
self.assertFalse(res["success"])
|
||||||
|
self.assertFalse(res.get("pr_closed"))
|
||||||
|
patch_calls = [
|
||||||
|
c for c in self.mock_api.call_args_list if c.args[0] == "PATCH"
|
||||||
|
]
|
||||||
|
self.assertEqual(patch_calls, [])
|
||||||
|
|
||||||
|
def test_already_landed_close_allowed_with_capability(self):
|
||||||
|
self._set_profile(RECONCILER_PROFILE)
|
||||||
|
|
||||||
|
def api_side_effect(method, url, auth, payload=None):
|
||||||
|
if method == "GET" and "/pulls/278" in url:
|
||||||
|
return dict(OPEN_LANDED_PR)
|
||||||
|
if method == "PATCH" and "/pulls/278" in url:
|
||||||
|
closed = dict(OPEN_LANDED_PR)
|
||||||
|
closed["state"] = "closed"
|
||||||
|
return closed
|
||||||
|
return {}
|
||||||
|
|
||||||
|
self.mock_api.side_effect = api_side_effect
|
||||||
|
with patch(
|
||||||
|
"mcp_server.already_landed_reconcile.assess_already_landed_reconciliation",
|
||||||
|
return_value={
|
||||||
|
"pr_state": "open",
|
||||||
|
"candidate_head_sha": OPEN_LANDED_PR["head"]["sha"],
|
||||||
|
"target_branch": "master",
|
||||||
|
"target_branch_sha": "e017d80256217f17b0f421cd051cfa5cbcf5ae0f",
|
||||||
|
"ancestor_proof": True,
|
||||||
|
"eligibility_class": "ALREADY_LANDED_RECONCILE_REQUIRED",
|
||||||
|
"linked_issue": 263,
|
||||||
|
"close_allowed": True,
|
||||||
|
"git_ref_mutations": ["git fetch prgs master"],
|
||||||
|
"reasons": [],
|
||||||
|
},
|
||||||
|
):
|
||||||
|
res = gitea_reconcile_already_landed_pr(
|
||||||
|
pr_number=278,
|
||||||
|
close_pr=True,
|
||||||
|
remote="prgs",
|
||||||
|
)
|
||||||
|
self.assertTrue(res["success"])
|
||||||
|
self.assertTrue(res["performed"])
|
||||||
|
self.assertTrue(res["pr_closed"])
|
||||||
|
self.assertEqual(res["eligibility_class"], "ALREADY_LANDED_RECONCILE_REQUIRED")
|
||||||
|
|
||||||
|
|
||||||
|
if __name__ == "__main__":
|
||||||
|
unittest.main()
|
||||||
@@ -275,6 +275,18 @@ class TestResolveTaskCapability(unittest.TestCase):
|
|||||||
self.assertEqual(res["required_role_kind"], "author")
|
self.assertEqual(res["required_role_kind"], "author")
|
||||||
self.assertIn("author", res["exact_safe_next_action"].lower())
|
self.assertIn("author", res["exact_safe_next_action"].lower())
|
||||||
|
|
||||||
|
@patch("mcp_server.api_request", return_value={"login": "author-user"})
|
||||||
|
@patch("mcp_server.get_auth_header", return_value="token author-pass")
|
||||||
|
def test_resolve_reconcile_already_landed_pr_requires_close(self, _auth, _api):
|
||||||
|
with patch.dict(os.environ, self._env("author-profile")):
|
||||||
|
res = mcp_server.gitea_resolve_task_capability(
|
||||||
|
task="reconcile_already_landed_pr", remote="prgs"
|
||||||
|
)
|
||||||
|
self.assertEqual(res["requested_task"], "reconcile_already_landed_pr")
|
||||||
|
self.assertEqual(res["required_operation_permission"], "gitea.pr.close")
|
||||||
|
self.assertEqual(res["required_role_kind"], "reconciler")
|
||||||
|
self.assertFalse(res["allowed_in_current_session"])
|
||||||
|
|
||||||
@patch("mcp_server.api_request", return_value={"login": "author-user"})
|
@patch("mcp_server.api_request", return_value={"login": "author-user"})
|
||||||
@patch("mcp_server.get_auth_header", return_value="token author-pass")
|
@patch("mcp_server.get_auth_header", return_value="token author-pass")
|
||||||
def test_resolve_pr_queue_cleanup_author_profile_blocked(self, _auth, _api):
|
def test_resolve_pr_queue_cleanup_author_profile_blocked(self, _auth, _api):
|
||||||
|
|||||||
@@ -52,6 +52,22 @@ CONFIG = {
|
|||||||
],
|
],
|
||||||
"execution_profile": "prgs-reviewer",
|
"execution_profile": "prgs-reviewer",
|
||||||
},
|
},
|
||||||
|
"prgs-reconciler": {
|
||||||
|
"enabled": True,
|
||||||
|
"context": "ctx",
|
||||||
|
"role": "reconciler",
|
||||||
|
"username": "sysadmin",
|
||||||
|
"auth": {"type": "env", "name": "GITEA_TOKEN_RECONCILER"},
|
||||||
|
"allowed_operations": [
|
||||||
|
"gitea.read", "gitea.pr.comment", "gitea.issue.comment",
|
||||||
|
"gitea.issue.close", "gitea.pr.close",
|
||||||
|
],
|
||||||
|
"forbidden_operations": [
|
||||||
|
"gitea.pr.approve", "gitea.pr.merge", "gitea.pr.create",
|
||||||
|
"gitea.branch.push",
|
||||||
|
],
|
||||||
|
"execution_profile": "prgs-reconciler",
|
||||||
|
},
|
||||||
},
|
},
|
||||||
"rules": {"allow_runtime_switching": False},
|
"rules": {"allow_runtime_switching": False},
|
||||||
}
|
}
|
||||||
@@ -88,6 +104,7 @@ class TestRoleSessionRouter(unittest.TestCase):
|
|||||||
"GITEA_MCP_PROFILE": profile,
|
"GITEA_MCP_PROFILE": profile,
|
||||||
"GITEA_TOKEN_AUTHOR": "author-pass",
|
"GITEA_TOKEN_AUTHOR": "author-pass",
|
||||||
"GITEA_TOKEN_REVIEWER": "reviewer-pass",
|
"GITEA_TOKEN_REVIEWER": "reviewer-pass",
|
||||||
|
"GITEA_TOKEN_RECONCILER": "reconciler-pass",
|
||||||
}
|
}
|
||||||
|
|
||||||
def test_reviewer_task_under_author_profile_wrong_role_stop(self):
|
def test_reviewer_task_under_author_profile_wrong_role_stop(self):
|
||||||
@@ -197,6 +214,24 @@ class TestRoleSessionRouter(unittest.TestCase):
|
|||||||
)
|
)
|
||||||
self.assertFalse(route["downstream_allowed"])
|
self.assertFalse(route["downstream_allowed"])
|
||||||
|
|
||||||
|
@patch("mcp_server.api_request", return_value={"login": "sysadmin"})
|
||||||
|
@patch("mcp_server.get_auth_header", return_value="token reconciler-pass")
|
||||||
|
def test_reconciler_task_under_reconciler_profile_allowed(self, _auth, _api):
|
||||||
|
with patch.dict(os.environ, self._env("prgs-reconciler")):
|
||||||
|
route = mcp_server.gitea_route_task_session(
|
||||||
|
task_type="reconcile_already_landed_pr", remote="prgs"
|
||||||
|
)
|
||||||
|
self.assertEqual(route["route_result"], role_session_router.ROUTE_ALLOWED)
|
||||||
|
self.assertTrue(route["downstream_allowed"])
|
||||||
|
|
||||||
|
def test_reconciler_task_under_author_profile_wrong_role_stop(self):
|
||||||
|
with patch.dict(os.environ, self._env("prgs-author")):
|
||||||
|
route = mcp_server.gitea_route_task_session(
|
||||||
|
task_type="reconcile_already_landed_pr", remote="prgs"
|
||||||
|
)
|
||||||
|
self.assertEqual(route["route_result"], role_session_router.ROUTE_WRONG_ROLE)
|
||||||
|
self.assertFalse(route["downstream_allowed"])
|
||||||
|
|
||||||
def test_activate_profile_blocked_in_static_mode(self):
|
def test_activate_profile_blocked_in_static_mode(self):
|
||||||
with patch.dict(os.environ, self._env("prgs-author")):
|
with patch.dict(os.environ, self._env("prgs-author")):
|
||||||
result = mcp_server.gitea_activate_profile(
|
result = mcp_server.gitea_activate_profile(
|
||||||
|
|||||||
Reference in New Issue
Block a user