Clarify and enforce reviewer-vs-merger role boundaries (#483) #486

Merged
sysadmin merged 9 commits from feat/issue-483-role-boundaries into master 2026-07-09 08:09:47 -05:00
17 changed files with 369 additions and 62 deletions
+12 -1
View File
@@ -22,9 +22,12 @@ launched with exactly one static execution profile:
| Namespace (MCP server name) | Profile (role) | Typical use |
|-----------------------------|----------------|-------------|
| `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 |
| `gitea-merger` | a merger profile | merge PRs after approval and verification |
| `gitea-reconciler` | a reconciler profile | close already-landed open PRs after ancestry proof (#304 profile; #310 close tool) |
Review and merge are separate workflow roles. A reviewer approval is not merge authorization.
Properties:
- **One process, one credential.** Each namespace authenticates as exactly
@@ -106,6 +109,14 @@ syntax to the client):
"GITEA_MCP_CONFIG": "<path-to-profiles.json>",
"GITEA_MCP_PROFILE": "<reviewer-profile-name>"
}
},
"gitea-merger": {
"command": "<path-to>/venv/bin/python3",
"args": ["<path-to>/mcp_server.py"],
"env": {
"GITEA_MCP_CONFIG": "<path-to-profiles.json>",
"GITEA_MCP_PROFILE": "<merger-profile-name>"
}
}
}
}
+2 -1
View File
@@ -311,7 +311,8 @@ To make Gitea MCP profile activation and runtime identity state explicit, the fo
### 2. Dual MCP Namespaces Recommendation
For security-sensitive or high-risk tasks, the preferred safety model uses separate, isolated MCP server instances (namespaces/sessions) launched with static profiles:
- `gitea-author`: Exposes tools configured with author permissions; cannot perform approvals or merges.
- `gitea-reviewer`: Exposes tools configured with reviewer permissions; used for PR reviews and merges.
- `gitea-reviewer`: Exposes tools configured with reviewer permissions; used for PR reviews. Review and merge are separate workflow roles. A reviewer approval is not merge authorization.
- `gitea-merger`: Exposes tools configured with merger permissions; used for PR merges.
This layout maintains physical separation of credentials and prevents privilege escalation within a single session.
This is the model accepted in #139; deployment details, rationale, and client
setup live in
+5 -5
View File
@@ -687,10 +687,10 @@ loop and do **not** substitute WebFetch/Playwright/manual base64.
### Merge a PR
- **Profile:** merger (allowed to merge; must **not** be the PR author).
- **Steps:** confirm eligibility; require explicit confirmation
(`MERGE PR <n>`); optionally pin head SHA / changed-file set; merge only when
Gitea reports the PR mergeable (branch-protection checks satisfied). No force,
no ignore-checks. Verify that remote master contains the merge commit or the expected squashed changes (do not assume a "closed" PR succeeded without verifying the actual landed changes).
- **Steps:**
- Merger workflow starts only after formal approval at current head. Reviewer workflow ends with review decision and separate merger handoff.
- Confirm eligibility; require explicit confirmation (`MERGE PR <n>`); optionally pin head SHA / changed-file set; merge only when Gitea reports the PR mergeable (branch-protection checks satisfied). No force, no ignore-checks. Verify that remote master contains the merge commit or the expected squashed changes (do not assume a "closed" PR succeeded without verifying the actual landed changes).
- Review and merge are separate workflow roles. A reviewer approval is not merge authorization.
- **Prompt:** `Use any eligible merger profile to merge PR #N if checks pass and
it is mergeable. Confirm with "MERGE PR N". Do not force-merge.`
@@ -779,7 +779,7 @@ an author.
|---|---|---|---|---|
| Review PR (`review_pr`) | reviewer (e.g. `sysadmin` / `prgs-reviewer`) | read, gated review verdicts | commits, pushes, file edits, author comments, merge without eligibility | active profile is an author profile — stop immediately; do **not** switch to author-side fixes unless the operator explicitly re-tasks |
| Address PR change requests (`address_pr_change_requests`) | author (e.g. `jcwalker3` / `prgs-author`) | commit/push fixes to the PR branch, PR comment summarizing fixes | review verdicts, approve, request-changes, merge | active profile lacks branch push |
| Merge PR (`merge_pr`) | reviewer/merger | gated merge after eligibility + approval | merging own PR, merging without pinned head match | active profile is an author profile, or any merge gate fails |
| Merge PR (`merge_pr`) | merger (e.g. `sysadmin` / `prgs-merger`) | gated merge after eligibility + approval | merging own PR, merging without pinned head match, reviewing PRs | active profile is an author or reviewer-only profile, or any merge gate fails |
| Comment on issue discussion (`comment_issue`) | any profile with `gitea.issue.comment` | issue thread comments | review verdicts, closing via comment | permission missing (`gitea.pr.comment` does **not** imply it) |
| Comment on PR (`comment_pr`) | any profile with `gitea.pr.comment` | PR thread comments | review verdicts | permission missing |
| Author implementation (`create_branch`/`push_branch`/`create_pr`) | author | branch, commit, push, open PR | self-review, self-merge | profile lacks the author permissions |
+12 -3
View File
@@ -16,12 +16,21 @@ bind an authenticated Gitea identity to an allowed operation set.
- **Allowed:** branch create/push, PR create, issue comment/create/close, repo commit, read.
- **Forbidden:** PR approve, merge, request_changes.
### Reviewer / merger
### Reviewer
- **Profile:** `prgs-reviewer`
- **Typical identity:** `sysadmin`
- **Allowed:** PR review/approve/merge/request_changes, issue comment, read.
- **Forbidden:** branch push, PR create, repo commit.
- **Allowed:** PR review/approve/request_changes, issue comment, read.
- **Forbidden:** branch push, PR create, repo commit, PR merge.
Review and merge are separate workflow roles. A reviewer approval is not merge authorization.
### Merger
- **Profile:** `prgs-merger`
- **Typical identity:** `sysadmin`
- **Allowed:** PR merge, issue comment, read.
- **Forbidden:** branch push, PR create, repo commit, PR approve, PR review, PR request_changes.
## Configuration
+15
View File
@@ -29,6 +29,7 @@ from validation_status_vocabulary import assess_validation_status_vocabulary
FINAL_REPORT_TASK_KINDS = frozenset({
"review_pr",
"merge_pr",
"reconcile_already_landed",
"author_issue",
"work_issue",
@@ -40,6 +41,8 @@ FINAL_REPORT_TASK_KINDS = frozenset({
_TASK_KIND_ALIASES = {
"review": "review_pr",
"review-merge-pr": "review_pr",
"merge": "merge_pr",
"merge_pr": "merge_pr",
"reconcile-landed-pr": "reconcile_already_landed",
"reconcile_already_landed": "reconcile_already_landed",
"work-issue": "work_issue",
@@ -48,6 +51,7 @@ _TASK_KIND_ALIASES = {
_HANDOFF_ROLE_BY_TASK = {
"review_pr": "review",
"merge_pr": "merger",
"reconcile_already_landed": None,
"author_issue": "author",
"work_issue": "author",
@@ -1362,6 +1366,17 @@ _RULES_BY_TASK: dict[str, list[Callable[..., list[dict[str, str]]]]] = {
*_SHARED_CLEANUP_PROOF_RULES,
_rule_reviewer_stale_head_proof,
],
"merge_pr": [
_rule_shared_controller_handoff,
_rule_shared_email_disclosure,
*_SHARED_ISSUE_LOCK_RULES,
_rule_reviewer_legacy_workspace_mutations,
_rule_reviewer_vague_mutations_none,
_rule_reviewer_mutation_categories,
_rule_reviewer_git_fetch_readonly,
_rule_reviewer_linked_issue,
_rule_reviewer_stale_head_proof,
],
"reconcile_already_landed": [
_rule_reconcile_controller_handoff,
_rule_shared_state_handoff_next_action,
+15 -3
View File
@@ -52,8 +52,19 @@
"execution_profile": "example-reviewer",
"audit_label": "example-reviewer",
"auth": { "type": "keychain", "id": "example-gitea-reviewer-token" },
"allowed_operations": ["read", "review", "comment", "issue.comment", "approve", "request_changes", "merge"],
"forbidden_operations": ["branch", "commit", "push", "open_pr"]
"allowed_operations": ["read", "review", "comment", "issue.comment", "approve", "request_changes"],
"forbidden_operations": ["branch", "commit", "push", "open_pr", "merge"]
},
"example-merger": {
"enabled": true,
"context": "example-context",
"role": "merger",
"username": "reviewer-user",
"execution_profile": "example-merger",
"audit_label": "example-merger",
"auth": { "type": "keychain", "id": "example-gitea-reviewer-token" },
"allowed_operations": ["read", "comment", "issue.comment", "merge"],
"forbidden_operations": ["branch", "commit", "push", "open_pr", "approve", "review", "request_changes"]
}
},
"projects": {
@@ -63,7 +74,8 @@
"default_owner": "Example-Org",
"default_repo": "Example-Repo",
"default_author_profile": "example-author",
"default_reviewer_profile": "example-reviewer"
"default_reviewer_profile": "example-reviewer",
"default_merger_profile": "example-merger"
}
},
"rules": {
+58 -24
View File
@@ -137,13 +137,13 @@ def verify_mutation_authority(remote: str | None, host: str | None = None,
)
# Reviewer/author role pivot boundary: only an authorized pivot
# (recorded by gitea_activate_profile) may cross author reviewer.
if (required_role == "reviewer"
# (recorded by gitea_activate_profile) may cross author -> reviewer/merger.
if (required_role in ("reviewer", "merger")
and "author" in str(data.get("initial_profile")).lower()
and "reviewer" in str(active_profile).lower()):
and ("reviewer" in str(active_profile).lower() or "merger" in str(active_profile).lower())):
if not data.get("role_pivot_authorized"):
raise RuntimeError(
"Attempted reviewer mutation from author session without "
f"Attempted {required_role} mutation from author session without "
"authorized role pivot (fail closed)"
)
@@ -562,7 +562,7 @@ def _enforce_branches_only_author_mutation(worktree_path: str | None = None) ->
Reviewer, merger, and reconciler roles are exempt: reconciler ``close_pr``
is a Gitea metadata mutation and must not require ``GITEA_AUTHOR_WORKTREE``
(#468). Non-author namespaces use dedicated workspace env vars (#510).
(#468, #483). Non-author namespaces use dedicated workspace env vars (#510).
The exemption honours BOTH the effective workspace role and the actual
profile role (#540). ``comment_issue`` preflight stamps
@@ -2279,13 +2279,23 @@ def gitea_check_pr_eligibility(
# ("gitea.pr.merge") always match each other and never cross services.
allowed = profile["allowed_operations"]
forbidden = profile["forbidden_operations"]
op_ok, op_reason = gitea_config.check_operation(action, allowed, forbidden)
active_role = _role_kind(allowed, forbidden)
if action == "merge" and active_role == "reviewer":
op_ok = False
op_reason = "reviewer-cannot-merge"
else:
op_ok, op_reason = gitea_config.check_operation(action, allowed, forbidden)
if not op_ok:
if op_reason == "no-allowed-operations":
reasons.append(
"profile has no configured allowed operations (fail closed)")
elif op_reason == "forbidden":
reasons.append(f"profile forbids '{action}'")
elif op_reason == "reviewer-cannot-merge":
reasons.append(
"Reviewer profile cannot merge. Use a merger profile/session after formal approval at current head."
)
elif op_reason == "invalid-forbidden-entry":
reasons.append(
"profile has an unrecognized forbidden operation entry "
@@ -2309,21 +2319,23 @@ def gitea_check_pr_eligibility(
missing_perm)
req_profile = None
role_noun = "reviewer"
if action in ("approve", "request_changes", "review"):
req_profile = "A profile with reviewer role permissions (allowing approve/merge/review, and forbidding author operations)"
req_profile = "A profile with reviewer role permissions (allowing approve/review, and forbidding author operations)"
elif action == "merge":
req_profile = "A profile with reviewer role permissions and explicit merge permission"
req_profile = "A profile with merger role permissions (allowing merge, and forbidding author operations)"
role_noun = "merger"
result["required_profile"] = req_profile
switching_supported = gitea_config.is_runtime_switching_enabled()
if switching_supported:
result["fixable_by_profile_switch"] = True
result["requires_different_namespace"] = False
safe_step = f"Switch to a reviewer profile by calling gitea_activate_profile with a profile that allows {action}."
safe_step = f"Switch to a {role_noun} profile by calling gitea_activate_profile with a profile that allows {action}."
else:
result["fixable_by_profile_switch"] = False
result["requires_different_namespace"] = True
safe_step = "Switch to the reviewer MCP session (e.g. gitea-reviewer) which has reviewer permissions configured, or ask the operator to update GITEA_MCP_PROFILE to a reviewer profile."
safe_step = f"Switch to the {role_noun} MCP session (e.g. gitea-{role_noun}) which has {role_noun} permissions configured, or ask the operator to update GITEA_MCP_PROFILE to a {role_noun} profile."
result["safe_next_step"] = safe_step
return result
@@ -4039,6 +4051,14 @@ def gitea_merge_pr(
_verify_role_mutation_workspace(
remote, worktree_path=worktree_path, task="merge_pr"
)
allowed = get_profile()["allowed_operations"]
forbidden = get_profile()["forbidden_operations"]
active_role = _role_kind(allowed, forbidden)
if active_role == "reviewer":
raise RuntimeError(
"Reviewer profile cannot merge. Use a merger profile/session "
"after formal approval at current head."
)
workflow_blockers = _review_workflow_load_gate_reasons()
do = (do or "").strip().lower()
result = {
@@ -4055,6 +4075,10 @@ def gitea_merge_pr(
"merge_result": None,
"merge_commit": None,
"reasons": [],
"active_profile": get_profile()["profile_name"],
"role_kind": active_role,
"merge_capability_source": "profile" if "gitea.pr.merge" in allowed else "config",
"explicit_operator_auth": confirmation,
}
reasons = result["reasons"]
if workflow_blockers:
@@ -4224,7 +4248,7 @@ def gitea_merge_pr(
# A profile/identity flip or side-channel override between preflight
# and merge fails closed here.
try:
verify_mutation_authority(remote, host, required_role="reviewer",
verify_mutation_authority(remote, host, required_role="merger",
active_identity=auth_user)
except RuntimeError as e:
reasons.append(str(e))
@@ -6543,26 +6567,28 @@ def _role_kind(allowed, forbidden) -> str:
"""Classify the active profile from its normalized operations.
'reconciler' can close already-landed PRs without review/author powers;
'author' can create PRs / push branches; 'reviewer' can approve/merge;
'reconciler' can close already-landed PRs via ``gitea.pr.close`` without
review/author powers; 'mixed' can do both (a config smell the
reviewer-deadlock invariant forbids it in v2 configs); 'limited' can do
neither.
'author' can create PRs / push branches; 'reviewer' can approve;
'merger' can merge PRs;
'mixed' can do multiple (a config smell); 'limited' can do neither.
"""
if reconciler_profile.is_reconciler_profile(allowed, forbidden):
return "reconciler"
def can(op):
return gitea_config.check_operation(op, allowed, forbidden)[0]
review = can("gitea.pr.approve") or can("gitea.pr.merge")
can_merge = can("gitea.pr.merge")
can_approve = can("gitea.pr.approve")
author = can("gitea.pr.create") or can("gitea.branch.push")
reconciler = (
can("gitea.pr.close")
and not review
and not can_approve
and not can_merge
and not author
)
if review and author:
if (can_approve or can_merge) and author:
return "mixed"
if review:
if can_merge:
return "merger"
if can_approve:
return "reviewer"
if reconciler:
return "reconciler"
@@ -7080,12 +7106,20 @@ def mcp_get_control_plane_guide(
"gitea.issue.comment (separate from gitea.pr.comment).")
elif role == "reviewer":
guidance.append(
"Reviewer profile: review/approve/merge may proceed ONLY after "
"Reviewer profile: review/approve may proceed ONLY after "
"gitea_check_pr_eligibility passes and the PR head SHA is "
"pinned (expected_head_sha); the PR author must be a different "
"user, and merging additionally requires explicit operator "
"authorization plus the 'MERGE PR <n>' confirmation. "
"PR comments do not imply issue comments.")
"user. Reviewer profiles cannot merge PRs. "
"PR comments do not imply issue comments. "
"Review and merge are separate workflow roles. A reviewer approval is not merge authorization.")
elif role == "merger":
guidance.append(
"Merger profile: merge may proceed ONLY after formal approval at "
"current head, gitea_check_pr_eligibility passes, and the PR head "
"SHA is pinned (expected_head_sha); the PR author must be a different "
"user, and merging requires explicit operator authorization plus the "
"'MERGE PR <n>' confirmation. "
"Review and merge are separate workflow roles. A reviewer approval is not merge authorization.")
elif role == "mixed":
guidance.append(
"WARNING: this profile allows both authoring and "
+15 -3
View File
@@ -2243,6 +2243,18 @@ HANDOFF_ROLE_FIELDS = {
("Linked issue status", ("linked issue status", "linked issue")),
("Cleanup status", ("cleanup status", "cleanup")),
) + HANDOFF_REVIEW_MUTATION_FIELDS,
"merger": (
("Selected PR", ("selected pr",)),
("Pinned reviewed head", ("pinned reviewed head", "pinned head")),
("Active profile", ("active profile",)),
("Role kind", ("role kind",)),
("Merge capability source", ("merge capability source",)),
("Explicit operator authorization", ("explicit operator authorization", "operator authorization", "authorization")),
("Expected head SHA", ("expected head sha", "expected head")),
("Approval at current head", ("approval at current head", "approval")),
("Merge result", ("merge result",)),
("Cleanup status", ("cleanup status", "cleanup")),
) + HANDOFF_REVIEW_MUTATION_FIELDS,
"author": (
("Selected issue", ("selected issue",)),
("Issue lock proof", ("issue lock proof", "lock before diff")),
@@ -2417,8 +2429,8 @@ def assess_controller_handoff(report_text, role=None, local_edits=False):
field for field in required
if field[0] not in ("Workspace mutations", "Mutations")
]
if role == "review":
# Issue #320: reviewer handoffs use the precise mutation categories
if role in ("review", "merger"):
# Issue #320: reviewer and merger handoffs use the precise mutation categories
# in HANDOFF_REVIEW_MUTATION_FIELDS instead of the legacy ambiguous
# "Workspace mutations" field, which is rejected below.
required = [
@@ -2431,7 +2443,7 @@ def assess_controller_handoff(report_text, role=None, local_edits=False):
"downgraded": True,
"missing_fields": [],
"reasons": [
"review handoff must not include legacy "
"review or merger handoff must not include legacy "
"'Workspace mutations' field; report the precise "
"mutation categories instead (issue #320)"
],
@@ -1,4 +1,4 @@
# Template: merge a PR (eligible reviewer only)
# Template: merge a PR (eligible merger only)
Copy, fill the `<...>` fields, and paste as the task prompt.
@@ -14,8 +14,9 @@ Rules (llm-project-workflow):
every gitea-tools call (e.g. `remote=prgs org=Scaled-Tech-Consulting
repo=Gitea-Tools`). A bare `remote=prgs` can resolve to the wrong default repo
and is blocked when it disagrees with the local git remote URL.
- Only an eligible, NON-author reviewer merges. If authenticated user == PR
- Only an eligible, NON-author merger merges. If authenticated user == PR
author → STOP.
- Review and merge are separate workflow roles. A reviewer approval is not merge authorization.
- Do not merge unless the PR is open, mergeable, and its checks/review pass.
- No force-merge, no bypassing branch protections.
- If the PR is closed but `merged=false`, STOP and run reconciliation. Do not clean up.
@@ -63,10 +64,12 @@ Then run the cleanup template (worktree-cleanup.md) in a reconciler session:
- fetch/prune; confirm main checkout is clean and current (0 0).
Handoff: end with a section titled exactly `Controller Handoff` per SKILL.md
§K (long form — a merge is always high-risk), including the review/merge role
fields (Selected PR, Reviewer eligibility, Pinned reviewed head, Review
§K (long form — a merge is always high-risk), including the merger role
fields (Selected PR, Merger eligibility, Pinned reviewed head, Review
decision, Merge result, Linked issue status, Cleanup status) plus: merge
commit, PR metadata state/merged flag/hash, remote master hash, and the
post-merge verification method used & verification results. Reports missing
the handoff are downgraded (review_proofs.assess_controller_handoff).
Review and merge are separate workflow roles. A reviewer approval is not merge authorization.
```
@@ -110,8 +110,9 @@ Steps:
- base branch unchanged
- no undismissed REQUEST_CHANGES / blocking review state left unaccounted
If anything moved → STOP, re-pin, re-validate before any verdict.
10. Post the review verdict: approve only if scope is clean and checks pass;
otherwise request changes with specifics. Never merge from this review step.
10. Post the review verdict: approve only if scope is clean and checks pass;
otherwise request changes with specifics. Never merge from this review step.
Review and merge are separate workflow roles. A reviewer approval is not merge authorization.
Include a "Review Metadata" block (attribution only — docs/llm-agent-sha.md):
Review Metadata:
@@ -128,6 +129,8 @@ Pinned reviewed head, Review decision, Merge result, Linked issue status,
Cleanup status. If you could not merge, name the exact gate. Reports missing
the handoff are downgraded (review_proofs.assess_controller_handoff).
Review and merge are separate workflow roles. A reviewer approval is not merge authorization.
Baseline failure proof (#533): if a validation command exits non-zero, do NOT
call it a clean pass. Only label a failure "baseline"/"pre-existing" with
pre-merge proof — the failure reproduced on the PR's pre-merge base commit, or a
+1 -1
View File
@@ -66,7 +66,7 @@ TASK_CAPABILITY_MAP: dict[str, dict[str, str]] = {
},
"merge_pr": {
"permission": "gitea.pr.merge",
"role": "reviewer",
"role": "merger",
},
"adopt_merger_pr_lease": {
"permission": "gitea.pr.comment",
+34
View File
@@ -435,6 +435,40 @@ class ValidateConfigTests(unittest.TestCase):
with open(example, encoding="utf-8") as fh:
self.assertEqual(gitea_config.validate_config(json.load(fh)), [])
def test_repo_example_file_reviewer_vs_merger_boundaries(self):
example_path = __import__("pathlib").Path(__file__).resolve().parent.parent \
/ "gitea-mcp.v2-contexts.example.json"
with open(example_path, encoding="utf-8") as fh:
cfg = json.load(fh)
# 1. Config includes separate reviewer and merger examples
self.assertIn("example-reviewer", cfg["profiles"])
self.assertIn("example-merger", cfg["profiles"])
reviewer = cfg["profiles"]["example-reviewer"]
merger = cfg["profiles"]["example-merger"]
# 2. Example reviewer config does not include merge capability
reviewer_allowed = reviewer.get("allowed_operations") or []
reviewer_forbidden = reviewer.get("forbidden_operations") or []
self.assertNotIn("merge", reviewer_allowed)
self.assertNotIn("gitea.pr.merge", reviewer_allowed)
self.assertIn("merge", reviewer_forbidden)
# 3. Example reviewer can resolve review task, cannot resolve merge task
# 4. Example merger can resolve merge task
rev_review_ok, _ = gitea_config.check_operation("gitea.pr.review", reviewer_allowed, reviewer_forbidden)
rev_merge_ok, _ = gitea_config.check_operation("gitea.pr.merge", reviewer_allowed, reviewer_forbidden)
merg_allowed = merger.get("allowed_operations") or []
merg_forbidden = merger.get("forbidden_operations") or []
merg_merge_ok, _ = gitea_config.check_operation("gitea.pr.merge", merg_allowed, merg_forbidden)
self.assertTrue(rev_review_ok, "example-reviewer must allow review")
self.assertFalse(rev_merge_ok, "example-reviewer must not allow merge")
self.assertTrue(merg_merge_ok, "example-merger must allow merge")
def test_broken_contexts_config_reports_problems(self):
data = contexts_config()
data["profiles"]["prgs-author"]["context"] = "nope"
+92
View File
@@ -594,8 +594,100 @@ class TestEntryPoint(unittest.TestCase):
def test_supported_task_kinds_include_review_and_reconcile(self):
self.assertIn("review_pr", FINAL_REPORT_TASK_KINDS)
self.assertIn("merge_pr", FINAL_REPORT_TASK_KINDS)
self.assertIn("reconcile_already_landed", FINAL_REPORT_TASK_KINDS)
class TestMergePrRules(unittest.TestCase):
def test_clean_merger_report_passes(self):
lines = [
"## Controller Handoff",
"",
"- Task: merge PR #203",
"- Repo: Scaled-Tech-Consulting/Gitea-Tools",
"- Role: merger",
"- Identity: sysadmin / prgs-merger",
"- Issue/PR: #182 / PR #203",
"- Branch/SHA: feat/x @ 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
"- Files changed: review_proofs.py",
"- Validation: pytest in branches/review-203",
"- Mutations: merge completed",
"- File edits by reviewer: none",
"- Worktree/index mutations: none",
"- Git ref mutations: git fetch prgs master",
"- MCP/Gitea mutations: merge completed",
"- Review mutations: none",
"- Merge mutations: merged PR #203",
"- Cleanup mutations: none",
"- External-state mutations: none",
"- Read-only diagnostics: metadata check",
"- Current status: PR merged",
"- Blockers: none",
"- Next: none",
"- Safety: review and merge are separate roles",
"- Selected PR: #203",
"- Pinned reviewed head: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
"- Reviewed head SHA: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
"- Final live head SHA before approval: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
"- Final live head SHA before merge: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
"- Push occurred during validation: no",
"- Active profile: prgs-merger",
"- Role kind: merger",
"- Merge capability source: profile",
"- Explicit operator authorization: MERGE PR 203",
"- Expected head SHA: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
"- Approval at current head: yes",
"- Merge result: PR merged successfully",
"- Cleanup status: complete",
]
report = "\n".join(lines)
result = assess_final_report_validator(
report,
"merge_pr",
)
self.assertEqual(result["grade"], "A")
self.assertFalse(result["blocked"])
def test_missing_merger_fields_blocks(self):
lines = [
"## Controller Handoff",
"",
"- Task: merge PR #203",
"- Repo: Scaled-Tech-Consulting/Gitea-Tools",
"- Role: merger",
"- Identity: sysadmin / prgs-merger",
"- Issue/PR: #182 / PR #203",
"- Branch/SHA: feat/x @ 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
"- Files changed: review_proofs.py",
"- Validation: pytest in branches/review-203",
"- Mutations: merge completed",
"- File edits by reviewer: none",
"- Worktree/index mutations: none",
"- Git ref mutations: git fetch prgs master",
"- MCP/Gitea mutations: merge completed",
"- Review mutations: none",
"- Merge mutations: merged PR #203",
"- Cleanup mutations: none",
"- External-state mutations: none",
"- Read-only diagnostics: metadata check",
"- Current status: PR merged",
"- Blockers: none",
"- Next: none",
"- Safety: review and merge are separate roles",
"- Selected PR: #203",
"- Pinned reviewed head: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
"- Reviewed head SHA: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
"- Final live head SHA before approval: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
"- Final live head SHA before merge: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
"- Push occurred during validation: no",
]
report = "\n".join(lines)
result = assess_final_report_validator(
report,
"merge_pr",
)
self.assertTrue(result["downgraded"])
if __name__ == "__main__":
unittest.main()
+17 -5
View File
@@ -861,6 +861,19 @@ class TestMergePR(unittest.TestCase):
# -- identity / profile / eligibility fail-closed -------------------------
@patch("mcp_server.api_request")
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
def test_reviewer_profile_cannot_merge(self, _auth, mock_api):
env = {"GITEA_PROFILE_NAME": "prgs-reviewer",
"GITEA_ALLOWED_OPERATIONS": "read,approve"}
with patch.dict(os.environ, env, clear=True):
with self.assertRaises(RuntimeError) as ctx:
gitea_merge_pr(
pr_number=8, confirmation=self._confirm(8), remote="prgs"
)
self.assertIn("Reviewer profile cannot merge. Use a merger profile/session after formal approval at current head.", str(ctx.exception))
mock_api.assert_not_called()
@patch("mcp_server.api_request")
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
def test_self_author_cannot_merge(self, _auth, mock_api):
@@ -902,14 +915,13 @@ class TestMergePR(unittest.TestCase):
@patch("mcp_server.api_request")
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
def test_profile_without_merge_permission_blocks(self, _auth, mock_api):
mock_api.side_effect = [{"login": "merger-bot"}, self._pr("author-bot")]
env = {"GITEA_PROFILE_NAME": "gitea-reviewer",
"GITEA_ALLOWED_OPERATIONS": "read,review,approve"}
with patch.dict(os.environ, env, clear=True):
r = gitea_merge_pr(
pr_number=8, confirmation=self._confirm(8), remote="prgs")
self.assertFalse(r["performed"])
self.assertIn("profile is not allowed to merge", r["reasons"])
with self.assertRaises(RuntimeError) as ctx:
gitea_merge_pr(
pr_number=8, confirmation=self._confirm(8), remote="prgs")
self.assertIn("Reviewer profile cannot merge. Use a merger profile/session after formal approval at current head.", str(ctx.exception))
self._assert_no_merge_call(mock_api)
# -- PR state / mergeability ----------------------------------------------
+18 -2
View File
@@ -33,8 +33,15 @@ REVIEWER_ENV = {
"GITEA_PROFILE_NAME": "reviewer-test",
"GITEA_ALLOWED_OPERATIONS":
"gitea.read,gitea.pr.review,gitea.pr.comment,gitea.pr.approve,"
"gitea.pr.request_changes,gitea.pr.merge",
"GITEA_FORBIDDEN_OPERATIONS": "gitea.pr.create,gitea.branch.push",
"gitea.pr.request_changes",
"GITEA_FORBIDDEN_OPERATIONS": "gitea.pr.create,gitea.branch.push,gitea.pr.merge",
}
MERGER_ENV = {
"GITEA_PROFILE_NAME": "merger-test",
"GITEA_ALLOWED_OPERATIONS":
"gitea.read,gitea.pr.comment,gitea.pr.merge",
"GITEA_FORBIDDEN_OPERATIONS": "gitea.pr.create,gitea.branch.push,gitea.pr.approve",
}
EXPECTED_SKILLS = [
@@ -88,6 +95,15 @@ class TestControlPlaneGuide(GuideTestBase):
self.assertIn("eligibility", blob)
self.assertIn("pinned", blob)
@patch("mcp_server.api_request", return_value={"login": "merger-bot"})
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
def test_merger_profile_guidance(self, _auth, _api):
with patch.dict(os.environ, MERGER_ENV, clear=True):
g = mcp_get_control_plane_guide(remote="prgs")
self.assertEqual(g["profile"]["role_kind"], "merger")
blob = " ".join(g["guidance"]).lower()
self.assertIn("merge", blob)
@patch("mcp_server.get_auth_header", return_value=None)
def test_unresolved_identity_instructs_stop(self, _auth):
with patch.dict(os.environ, AUTHOR_ENV, clear=True):
+15 -4
View File
@@ -44,9 +44,19 @@ CONFIG_RESOLVER = {
"role": "reviewer",
"username": "reviewer-user",
"auth": {"type": "env", "name": "GITEA_TOKEN_REVIEWER"},
"allowed_operations": ["gitea.read", "gitea.pr.review", "gitea.pr.approve", "gitea.pr.merge", "gitea.issue.comment"],
"forbidden_operations": ["gitea.pr.create", "gitea.branch.push"],
"allowed_operations": ["gitea.read", "gitea.pr.review", "gitea.pr.approve", "gitea.issue.comment"],
"forbidden_operations": ["gitea.pr.create", "gitea.branch.push", "gitea.pr.merge"],
"execution_profile": "reviewer-profile"
},
"merger-profile": {
"enabled": True,
"context": "ctx",
"role": "merger",
"username": "merger-user",
"auth": {"type": "env", "name": "GITEA_TOKEN_MERGER"},
"allowed_operations": ["gitea.read", "gitea.pr.merge", "gitea.issue.comment"],
"forbidden_operations": ["gitea.pr.create", "gitea.branch.push", "gitea.pr.approve"],
"execution_profile": "merger-profile"
}
},
"rules": {
@@ -83,6 +93,7 @@ class TestResolveTaskCapability(unittest.TestCase):
"GITEA_MCP_PROFILE": profile,
"GITEA_TOKEN_AUTHOR": "author-pass",
"GITEA_TOKEN_REVIEWER": "reviewer-pass",
"GITEA_TOKEN_MERGER": "merger-pass",
}
@patch("mcp_server.api_request", return_value={"login": "author-user"})
@@ -231,9 +242,9 @@ class TestResolveTaskCapability(unittest.TestCase):
with patch.dict(os.environ, self._env("author-profile")):
res = mcp_server.gitea_resolve_task_capability(task="merge_pr", remote="prgs")
self.assertFalse(res["allowed_in_current_session"])
self.assertIn("reviewer", res["exact_safe_next_action"].lower())
self.assertIn("merger", res["exact_safe_next_action"].lower())
self.assertEqual(res["required_operation_permission"], "gitea.pr.merge")
self.assertEqual(res["required_role_kind"], "reviewer")
self.assertEqual(res["required_role_kind"], "merger")
@patch("mcp_server.api_request")
def test_self_review_blocked_structured(self, mock_api):
+46 -4
View File
@@ -43,9 +43,19 @@ CONFIG_SWITCHING_DISABLED = {
"role": "reviewer",
"username": "reviewer-user",
"auth": {"type": "env", "name": "GITEA_TOKEN_REVIEWER"},
"allowed_operations": ["gitea.read", "gitea.pr.approve", "gitea.pr.merge"],
"forbidden_operations": ["gitea.pr.create", "gitea.branch.push"],
"allowed_operations": ["gitea.read", "gitea.pr.review", "gitea.pr.approve"],
"forbidden_operations": ["gitea.pr.create", "gitea.branch.push", "gitea.pr.merge"],
"execution_profile": "reviewer-profile"
},
"merger-profile": {
"enabled": True,
"context": "ctx",
"role": "merger",
"username": "merger-user",
"auth": {"type": "env", "name": "GITEA_TOKEN_MERGER"},
"allowed_operations": ["gitea.read", "gitea.pr.merge"],
"forbidden_operations": ["gitea.pr.create", "gitea.branch.push", "gitea.pr.approve"],
"execution_profile": "merger-profile"
}
},
"rules": {
@@ -90,6 +100,7 @@ class TestRuntimeClarity(unittest.TestCase):
"GITEA_MCP_REVEAL_ENDPOINTS": reveal,
"GITEA_TOKEN_AUTHOR": "author-pass",
"GITEA_TOKEN_REVIEWER": "reviewer-pass",
"GITEA_TOKEN_MERGER": "merger-pass",
}
# -------------------------------------------------------------------------
@@ -125,7 +136,7 @@ class TestRuntimeClarity(unittest.TestCase):
t for t in caps["task_capabilities"] if t["task"] == "merge_pr"
)
self.assertFalse(merge_entry["allowed_in_current_session"])
self.assertIn("reviewer-profile", merge_entry["matching_configured_profiles"])
self.assertIn("merger-profile", merge_entry["matching_configured_profiles"])
@patch(
"mcp_server.assess_preflight_status",
@@ -144,6 +155,30 @@ class TestRuntimeClarity(unittest.TestCase):
caps = ctx["session_capabilities"]
self.assertFalse(caps["can_author_prs"])
self.assertFalse(caps["can_create_issues"])
self.assertTrue(caps["can_review_prs"])
self.assertFalse(caps["can_merge_prs"])
merge_entry = next(
t for t in caps["task_capabilities"] if t["task"] == "merge_pr"
)
self.assertFalse(merge_entry["allowed_in_current_session"])
@patch(
"mcp_server.assess_preflight_status",
return_value={"preflight_ready": True, "preflight_block_reasons": []},
)
@patch("mcp_server.api_request", return_value={"login": "merger-user"})
@patch("mcp_server.get_auth_header", return_value="token merger-pass")
def test_get_runtime_context_merger(self, _auth, _api, _preflight):
with patch.dict(os.environ, self._env("merger-profile"), clear=True):
ctx = mcp_server.gitea_get_runtime_context(remote="dadeschools")
self.assertEqual(ctx["active_profile"], "merger-profile")
self.assertEqual(ctx["authenticated_username"], "merger-user")
self.assertTrue(ctx["review_merge_allowed"])
self.assertEqual(ctx["suggested_fix"], "none")
self.assertEqual(ctx["safe_next_action"], "None; ready for operations.")
caps = ctx["session_capabilities"]
self.assertFalse(caps["can_author_prs"])
self.assertFalse(caps["can_create_issues"])
self.assertFalse(caps["can_review_prs"])
self.assertTrue(caps["can_merge_prs"])
merge_entry = next(
@@ -160,7 +195,7 @@ class TestRuntimeClarity(unittest.TestCase):
with patch.dict(os.environ, self._env("author-profile", reveal="0"), clear=True):
res = mcp_server.gitea_list_profiles()
profiles = res["profiles"]
self.assertEqual(len(profiles), 2)
self.assertEqual(len(profiles), 3)
author_prof = next(p for p in profiles if p["name"] == "author-profile")
self.assertTrue(author_prof["is_active"])
@@ -176,6 +211,13 @@ class TestRuntimeClarity(unittest.TestCase):
self.assertEqual(reviewer_prof["base_url"], "<redacted>")
self.assertEqual(reviewer_prof["identity_status"], "credentials present")
merger_prof = next(p for p in profiles if p["name"] == "merger-profile")
self.assertFalse(merger_prof["is_active"])
self.assertEqual(merger_prof["role_kind"], "merger")
self.assertEqual(merger_prof["auth"]["name"], "<redacted>")
self.assertEqual(merger_prof["base_url"], "<redacted>")
self.assertEqual(merger_prof["identity_status"], "credentials present")
@patch("mcp_server.api_request", return_value={"login": "author-user"})
@patch("mcp_server.get_auth_header", return_value="token author-pass")
def test_list_profiles_revealed_under_opt_in(self, _auth, _api):