fix: strengthen reconciler merged cleanup tests and runbook (#523)
Make the preflight bypass test actually exercise purity (not test short-circuit), cover unmerged/open-head fail-closed cleanup, and document post-merge cleanup ownership for prgs-reconciler so authors are not required for that path.
This commit is contained in:
@@ -738,6 +738,27 @@ merger handoff.
|
|||||||
- **Prompt (normal):** `After verifying master contains the merge of PR #N using post-merge file-presence verification, close issue #M and delete the merged branch. Include verification details in the report.`
|
- **Prompt (normal):** `After verifying master contains the merge of PR #N using post-merge file-presence verification, close issue #M and delete the merged branch. Include verification details in the report.`
|
||||||
- **Prompt (reconcile):** `Reconcile closed-not-merged PR #N by verifying if its content landed on master.`
|
- **Prompt (reconcile):** `Reconcile closed-not-merged PR #N by verifying if its content landed on master.`
|
||||||
|
|
||||||
|
### Post-merge merged cleanup ownership (#523)
|
||||||
|
|
||||||
|
Post-merge **local worktree / remote branch cleanup** is **reconciler** work, not
|
||||||
|
author work. Do not switch from `prgs-reconciler` to `prgs-author` only to run
|
||||||
|
`gitea_reconcile_merged_cleanups`.
|
||||||
|
|
||||||
|
- **Profile:** `prgs-reconciler` (task `reconcile_merged_cleanups` /
|
||||||
|
`reconciliation_cleanup`).
|
||||||
|
- **Namespace:** reconciler MCP server; stable control checkout is allowed for
|
||||||
|
this role (branches-only author guard does not apply).
|
||||||
|
- **Steps:**
|
||||||
|
1. `gitea_whoami` + `gitea_resolve_task_capability(task="reconcile_merged_cleanups")`.
|
||||||
|
2. Dry-run first: `gitea_reconcile_merged_cleanups(dry_run=True)`.
|
||||||
|
3. Execute only after audit/authorization gates when remote branch delete or
|
||||||
|
worktree removal is required (`dry_run=False`, `execute_confirmed=True`,
|
||||||
|
and `gitea.branch.delete` when deleting remotes).
|
||||||
|
- **Fail closed:** unmerged/open heads, mismatched worktrees, and non-merged
|
||||||
|
closed PRs must not be cleaned.
|
||||||
|
- **Reports:** label cleanup actions as reconciler cleanup (not author mutation).
|
||||||
|
- **Prompt:** `As prgs-reconciler, dry-run then execute gitea_reconcile_merged_cleanups for recently merged PRs without switching to prgs-author.`
|
||||||
|
|
||||||
### Stop on blocker
|
### Stop on blocker
|
||||||
|
|
||||||
- **Any profile.** If a required gate cannot be satisfied — identity
|
- **Any profile.** If a required gate cannot be satisfied — identity
|
||||||
|
|||||||
@@ -2,13 +2,14 @@
|
|||||||
import os
|
import os
|
||||||
import sys
|
import sys
|
||||||
import unittest
|
import unittest
|
||||||
from unittest.mock import patch, MagicMock
|
from unittest.mock import patch
|
||||||
|
|
||||||
sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.parent))
|
sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.parent))
|
||||||
|
|
||||||
import mcp_server
|
import mcp_server
|
||||||
import task_capability_map
|
import task_capability_map
|
||||||
from audit_reconciliation_mode import clear_phase
|
from audit_reconciliation_mode import clear_phase
|
||||||
|
from task_capability_map import required_role
|
||||||
|
|
||||||
RECONCILER_PROFILE = {
|
RECONCILER_PROFILE = {
|
||||||
"profile_name": "prgs-reconciler",
|
"profile_name": "prgs-reconciler",
|
||||||
@@ -16,10 +17,33 @@ RECONCILER_PROFILE = {
|
|||||||
"role": "reconciler",
|
"role": "reconciler",
|
||||||
"username": "sysadmin",
|
"username": "sysadmin",
|
||||||
"base_url": "https://gitea.prgs.cc",
|
"base_url": "https://gitea.prgs.cc",
|
||||||
"allowed_operations": ["gitea.read", "gitea.pr.close", "gitea.pr.comment", "gitea.issue.comment"],
|
"allowed_operations": [
|
||||||
|
"gitea.read",
|
||||||
|
"gitea.pr.close",
|
||||||
|
"gitea.pr.comment",
|
||||||
|
"gitea.issue.comment",
|
||||||
|
],
|
||||||
"forbidden_operations": [],
|
"forbidden_operations": [],
|
||||||
}
|
}
|
||||||
|
|
||||||
|
AUTHOR_PROFILE = {
|
||||||
|
"profile_name": "prgs-author",
|
||||||
|
"context": "prgs",
|
||||||
|
"role": "author",
|
||||||
|
"username": "jcwalker3",
|
||||||
|
"base_url": "https://gitea.prgs.cc",
|
||||||
|
"allowed_operations": [
|
||||||
|
"gitea.read",
|
||||||
|
"gitea.pr.create",
|
||||||
|
"gitea.branch.create",
|
||||||
|
"gitea.branch.push",
|
||||||
|
"gitea.repo.commit",
|
||||||
|
],
|
||||||
|
"forbidden_operations": [],
|
||||||
|
}
|
||||||
|
|
||||||
|
CONTROL_ROOT = "/Users/jasonwalker/Development/Gitea-Tools"
|
||||||
|
|
||||||
|
|
||||||
class TestReconcilerCleanupIntegration(unittest.TestCase):
|
class TestReconcilerCleanupIntegration(unittest.TestCase):
|
||||||
def setUp(self):
|
def setUp(self):
|
||||||
@@ -32,6 +56,7 @@ class TestReconcilerCleanupIntegration(unittest.TestCase):
|
|||||||
mcp_server._preflight_capability_violation = False
|
mcp_server._preflight_capability_violation = False
|
||||||
mcp_server._preflight_whoami_violation_files = []
|
mcp_server._preflight_whoami_violation_files = []
|
||||||
mcp_server._preflight_capability_violation_files = []
|
mcp_server._preflight_capability_violation_files = []
|
||||||
|
mcp_server._preflight_capability_baseline_porcelain = ""
|
||||||
self.mock_api = patch("gitea_auth.api_request").start()
|
self.mock_api = patch("gitea_auth.api_request").start()
|
||||||
self.mock_auth = patch(
|
self.mock_auth = patch(
|
||||||
"mcp_server.get_auth_header", return_value="token test"
|
"mcp_server.get_auth_header", return_value="token test"
|
||||||
@@ -42,15 +67,26 @@ class TestReconcilerCleanupIntegration(unittest.TestCase):
|
|||||||
clear_phase()
|
clear_phase()
|
||||||
mcp_server._IDENTITY_CACHE.clear()
|
mcp_server._IDENTITY_CACHE.clear()
|
||||||
|
|
||||||
|
def test_capability_map_routes_merged_cleanup_to_reconciler(self):
|
||||||
|
self.assertEqual(required_role("reconcile_merged_cleanups"), "reconciler")
|
||||||
|
self.assertEqual(required_role("reconciliation_cleanup"), "reconciler")
|
||||||
|
self.assertEqual(
|
||||||
|
task_capability_map.required_permission("reconcile_merged_cleanups"),
|
||||||
|
"gitea.read",
|
||||||
|
)
|
||||||
|
|
||||||
@patch.dict(os.environ, {"GITEA_PROFILE_NAME": "prgs-reconciler"}, clear=True)
|
@patch.dict(os.environ, {"GITEA_PROFILE_NAME": "prgs-reconciler"}, clear=True)
|
||||||
@patch("mcp_server.get_profile", return_value=RECONCILER_PROFILE)
|
@patch("mcp_server.get_profile", return_value=RECONCILER_PROFILE)
|
||||||
def test_reconciler_can_run_reconcile_merged_cleanups_directly(self, _profile):
|
def test_reconciler_can_run_reconcile_merged_cleanups_directly(self, _profile):
|
||||||
mcp_server.record_preflight_check("whoami")
|
mcp_server.record_preflight_check("whoami")
|
||||||
mcp_server.record_preflight_check("capability", resolved_role="reconciler", resolved_task="reconcile_merged_cleanups")
|
mcp_server.record_preflight_check(
|
||||||
|
"capability",
|
||||||
# Mock closed PRs and open PRs returned by API
|
resolved_role="reconciler",
|
||||||
|
resolved_task="reconcile_merged_cleanups",
|
||||||
|
)
|
||||||
|
|
||||||
self.mock_api.side_effect = [
|
self.mock_api.side_effect = [
|
||||||
[ # closed pulls
|
[ # closed pulls page
|
||||||
{
|
{
|
||||||
"number": 100,
|
"number": 100,
|
||||||
"title": "merged feature",
|
"title": "merged feature",
|
||||||
@@ -59,53 +95,168 @@ class TestReconcilerCleanupIntegration(unittest.TestCase):
|
|||||||
"merged_at": "2026-07-06T12:00:00Z",
|
"merged_at": "2026-07-06T12:00:00Z",
|
||||||
"merge_commit_sha": "deadbeef",
|
"merge_commit_sha": "deadbeef",
|
||||||
"head": {"ref": "feat/issue-100", "sha": "cafebabe"},
|
"head": {"ref": "feat/issue-100", "sha": "cafebabe"},
|
||||||
}
|
},
|
||||||
|
{
|
||||||
|
# closed but NOT merged — must not become a cleanup entry
|
||||||
|
"number": 101,
|
||||||
|
"title": "abandoned",
|
||||||
|
"body": "Closes #101",
|
||||||
|
"merged": False,
|
||||||
|
"merged_at": None,
|
||||||
|
"head": {"ref": "feat/issue-101", "sha": "badcafe"},
|
||||||
|
},
|
||||||
],
|
],
|
||||||
[], # open pulls
|
[], # open pulls
|
||||||
]
|
]
|
||||||
|
|
||||||
with patch("mcp_server._remote_branch_exists", return_value=True):
|
with patch("mcp_server._remote_branch_exists", return_value=True):
|
||||||
with patch("mcp_server.merged_cleanup_reconcile.is_head_ancestor_of_ref", return_value=True):
|
with patch(
|
||||||
|
"mcp_server.merged_cleanup_reconcile.is_head_ancestor_of_ref",
|
||||||
|
return_value=True,
|
||||||
|
):
|
||||||
result = mcp_server.gitea_reconcile_merged_cleanups(
|
result = mcp_server.gitea_reconcile_merged_cleanups(
|
||||||
dry_run=True,
|
dry_run=True,
|
||||||
remote="prgs",
|
remote="prgs",
|
||||||
)
|
)
|
||||||
self.assertTrue(result["success"])
|
self.assertTrue(result["success"])
|
||||||
self.assertTrue(result["dry_run"])
|
self.assertTrue(result["dry_run"])
|
||||||
self.assertEqual(result["entries"][0]["issue_number"], 100)
|
entry_numbers = [e["issue_number"] for e in result["entries"]]
|
||||||
|
self.assertEqual(entry_numbers, [100])
|
||||||
|
self.assertNotIn(101, entry_numbers)
|
||||||
|
|
||||||
|
@patch.dict(
|
||||||
|
os.environ,
|
||||||
|
{
|
||||||
|
"GITEA_PROFILE_NAME": "prgs-reconciler",
|
||||||
|
# Force preflight purity to evaluate (disable test short-circuit).
|
||||||
|
"GITEA_TEST_PORCELAIN": "",
|
||||||
|
},
|
||||||
|
clear=True,
|
||||||
|
)
|
||||||
|
@patch("mcp_server.get_profile", return_value=RECONCILER_PROFILE)
|
||||||
|
def test_reconciler_role_bypasses_branches_only_mutation_guard(self, _profile):
|
||||||
|
mcp_server.record_preflight_check("whoami")
|
||||||
|
mcp_server.record_preflight_check(
|
||||||
|
"capability",
|
||||||
|
resolved_role="reconciler",
|
||||||
|
resolved_task="reconcile_merged_cleanups",
|
||||||
|
)
|
||||||
|
|
||||||
|
with patch("mcp_server.PROJECT_ROOT", CONTROL_ROOT):
|
||||||
|
with patch(
|
||||||
|
"mcp_server.root_checkout_guard.resolve_remote_master_sha",
|
||||||
|
return_value="abc123",
|
||||||
|
):
|
||||||
|
with patch(
|
||||||
|
"mcp_server.issue_lock_worktree.read_worktree_git_state",
|
||||||
|
return_value={
|
||||||
|
"current_branch": "master",
|
||||||
|
"head_sha": "abc123",
|
||||||
|
"porcelain_status": "",
|
||||||
|
},
|
||||||
|
):
|
||||||
|
try:
|
||||||
|
mcp_server.verify_preflight_purity(
|
||||||
|
remote="prgs",
|
||||||
|
task="reconcile_merged_cleanups",
|
||||||
|
)
|
||||||
|
except RuntimeError as e:
|
||||||
|
self.fail(
|
||||||
|
"verify_preflight_purity raised RuntimeError "
|
||||||
|
f"unexpectedly for reconciler: {e}"
|
||||||
|
)
|
||||||
|
|
||||||
|
@patch.dict(
|
||||||
|
os.environ,
|
||||||
|
{"GITEA_PROFILE_NAME": "prgs-author", "GITEA_TEST_PORCELAIN": ""},
|
||||||
|
clear=True,
|
||||||
|
)
|
||||||
|
@patch("mcp_server.get_profile", return_value=AUTHOR_PROFILE)
|
||||||
|
def test_author_role_remains_blocked_on_root_checkout(self, _profile):
|
||||||
|
mcp_server.record_preflight_check("whoami")
|
||||||
|
mcp_server.record_preflight_check(
|
||||||
|
"capability",
|
||||||
|
resolved_role="author",
|
||||||
|
resolved_task="reconcile_merged_cleanups",
|
||||||
|
)
|
||||||
|
|
||||||
|
with patch("mcp_server.PROJECT_ROOT", CONTROL_ROOT):
|
||||||
|
with patch(
|
||||||
|
"mcp_server.root_checkout_guard.resolve_remote_master_sha",
|
||||||
|
return_value="abc123",
|
||||||
|
):
|
||||||
|
with patch(
|
||||||
|
"mcp_server.issue_lock_worktree.read_worktree_git_state",
|
||||||
|
return_value={
|
||||||
|
"current_branch": "master",
|
||||||
|
"head_sha": "abc123",
|
||||||
|
"porcelain_status": "",
|
||||||
|
},
|
||||||
|
):
|
||||||
|
with self.assertRaises(RuntimeError) as ctx:
|
||||||
|
mcp_server.verify_preflight_purity(
|
||||||
|
remote="prgs",
|
||||||
|
task="reconcile_merged_cleanups",
|
||||||
|
)
|
||||||
|
message = str(ctx.exception)
|
||||||
|
self.assertTrue(
|
||||||
|
"stable control checkout" in message
|
||||||
|
or "author mutation blocked" in message
|
||||||
|
or "branches/" in message,
|
||||||
|
msg=message,
|
||||||
|
)
|
||||||
|
|
||||||
@patch.dict(os.environ, {"GITEA_PROFILE_NAME": "prgs-reconciler"}, clear=True)
|
@patch.dict(os.environ, {"GITEA_PROFILE_NAME": "prgs-reconciler"}, clear=True)
|
||||||
@patch("mcp_server.get_profile", return_value=RECONCILER_PROFILE)
|
@patch("mcp_server.get_profile", return_value=RECONCILER_PROFILE)
|
||||||
def test_reconciler_role_bypasses_branches_only_mutation_guard(self, _profile):
|
def test_open_unmerged_pr_heads_are_not_safe_cleanup_targets(self, _profile):
|
||||||
# We simulate verify_preflight_purity under reconciler role.
|
"""AC5: active/unmerged author work must remain blocked from cleanup."""
|
||||||
# It should succeed without raising RuntimeError even if CWD/workspace is the process root.
|
|
||||||
mcp_server.record_preflight_check("whoami")
|
mcp_server.record_preflight_check("whoami")
|
||||||
mcp_server.record_preflight_check("capability", resolved_role="reconciler", resolved_task="reconcile_merged_cleanups")
|
mcp_server.record_preflight_check(
|
||||||
|
"capability",
|
||||||
# Calling verify_preflight_purity directly
|
resolved_role="reconciler",
|
||||||
try:
|
resolved_task="reconcile_merged_cleanups",
|
||||||
mcp_server.verify_preflight_purity(remote="prgs", task="reconcile_merged_cleanups")
|
)
|
||||||
except RuntimeError as e:
|
|
||||||
self.fail(f"verify_preflight_purity raised RuntimeError unexpectedly: {e}")
|
|
||||||
|
|
||||||
@patch.dict(os.environ, {"GITEA_PROFILE_NAME": "prgs-author", "GITEA_TEST_PORCELAIN": ""}, clear=True)
|
open_pr = {
|
||||||
@patch("mcp_server.get_profile", return_value={
|
"number": 200,
|
||||||
"profile_name": "prgs-author",
|
"title": "active work",
|
||||||
"context": "prgs",
|
"body": "Closes #200",
|
||||||
"role": "author",
|
"merged": False,
|
||||||
"username": "jcwalker3",
|
"state": "open",
|
||||||
"base_url": "https://gitea.prgs.cc",
|
"head": {"ref": "feat/issue-200", "sha": "livehead1"},
|
||||||
"allowed_operations": ["gitea.read", "gitea.pr.create", "gitea.branch.create", "gitea.branch.push", "gitea.repo.commit"],
|
}
|
||||||
"forbidden_operations": [],
|
# Same head branch still open on another PR while a closed/merged PR
|
||||||
})
|
# used the same branch name historically — open head must block delete.
|
||||||
def test_author_role_remains_blocked_on_root_checkout(self, _profile):
|
closed_merged = {
|
||||||
mcp_server.record_preflight_check("whoami")
|
"number": 199,
|
||||||
mcp_server.record_preflight_check("capability", resolved_role="author", resolved_task="reconcile_merged_cleanups")
|
"title": "older merge same branch name",
|
||||||
|
"body": "Closes #199",
|
||||||
with patch("mcp_server.PROJECT_ROOT", "/Users/jasonwalker/Development/Gitea-Tools"):
|
"merged": True,
|
||||||
with self.assertRaises(RuntimeError) as ctx:
|
"merged_at": "2026-07-01T12:00:00Z",
|
||||||
mcp_server.verify_preflight_purity(remote="prgs", task="reconcile_merged_cleanups")
|
"merge_commit_sha": "deadbeef",
|
||||||
self.assertIn("author mutation blocked: workspace is the stable control checkout", str(ctx.exception))
|
"head": {"ref": "feat/issue-200", "sha": "oldhead99"},
|
||||||
|
}
|
||||||
|
self.mock_api.side_effect = [
|
||||||
|
[closed_merged], # closed
|
||||||
|
[open_pr], # open
|
||||||
|
]
|
||||||
|
|
||||||
|
with patch("mcp_server._remote_branch_exists", return_value=True):
|
||||||
|
with patch(
|
||||||
|
"mcp_server.merged_cleanup_reconcile.is_head_ancestor_of_ref",
|
||||||
|
return_value=True,
|
||||||
|
):
|
||||||
|
result = mcp_server.gitea_reconcile_merged_cleanups(
|
||||||
|
dry_run=True,
|
||||||
|
remote="prgs",
|
||||||
|
)
|
||||||
|
self.assertTrue(result["success"])
|
||||||
|
self.assertEqual(len(result["entries"]), 1)
|
||||||
|
remote_assessment = result["entries"][0].get("remote_branch") or {}
|
||||||
|
self.assertFalse(
|
||||||
|
remote_assessment.get("safe_to_delete_remote"),
|
||||||
|
msg=f"open head must block remote delete: {remote_assessment}",
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
if __name__ == "__main__":
|
if __name__ == "__main__":
|
||||||
|
|||||||
Reference in New Issue
Block a user