fix: resolve conflicts for issue 339
This commit is contained in:
@@ -99,12 +99,16 @@ class TestCapabilityStopTerminal(unittest.TestCase):
|
||||
self.assertIn("Cannot perform reviewer task", str(ctx.exception))
|
||||
self.assertIn("review_pr", str(ctx.exception))
|
||||
|
||||
@patch("mcp_server.api_get_all", return_value=[])
|
||||
@patch("mcp_server.api_fetch_page")
|
||||
@patch("mcp_server.api_request", return_value={"login": "jcwalker3"})
|
||||
@patch("mcp_server.get_auth_header", return_value="token author-pass")
|
||||
def test_list_prs_allowed_after_author_task_clears_stale_denial(
|
||||
self, _auth, _api, _get_all,
|
||||
self, _auth, _api, mock_fetch,
|
||||
):
|
||||
mock_fetch.return_value = ([], {
|
||||
"page": 1, "per_page": 50, "returned_count": 0,
|
||||
"has_more": False, "next_page": None, "is_final_page": True,
|
||||
})
|
||||
with patch.dict(os.environ, self._env()):
|
||||
denied = mcp_server.gitea_resolve_task_capability(
|
||||
task="review_pr", remote="prgs"
|
||||
@@ -120,7 +124,7 @@ class TestCapabilityStopTerminal(unittest.TestCase):
|
||||
self.assertFalse(capability_stop_terminal.is_active())
|
||||
|
||||
prs = mcp_server.gitea_list_prs(remote="prgs")
|
||||
self.assertEqual(prs, [])
|
||||
self.assertEqual(prs["prs"], [])
|
||||
|
||||
@patch("mcp_server.api_request", return_value={"login": "jcwalker3"})
|
||||
@patch("mcp_server.get_auth_header", return_value="token author-pass")
|
||||
|
||||
@@ -59,6 +59,22 @@ def test_registration_doc_preserves_boundaries():
|
||||
assert phrase in text
|
||||
|
||||
|
||||
def test_registration_doc_pins_glitchtip_filing_boundary():
|
||||
text = " ".join(_doc_text().split())
|
||||
for phrase in (
|
||||
"GlitchTip-to-Gitea filing is a separate library-only orchestrator in "
|
||||
"`mcp-control-plane`",
|
||||
"real GlitchTip read path",
|
||||
"dedup before any Gitea create action",
|
||||
"create/link/skip decisions",
|
||||
"fail closed on mutation-audit failure before any Gitea create, link, "
|
||||
"or comment mutation",
|
||||
"must use a separate write-boundary server/profile",
|
||||
"must not be exposed by `glitchtip-mcp`",
|
||||
):
|
||||
assert phrase in text
|
||||
|
||||
|
||||
def test_registration_doc_separates_jenkins_trigger_from_read_surface():
|
||||
text = _doc_text()
|
||||
assert "jenkins_trigger_build" in text
|
||||
@@ -100,3 +116,25 @@ def test_related_docs_keep_jenkins_trigger_off_read_surface():
|
||||
assert "jenkins-write-mcp" in text
|
||||
assert "jenkins_mcp.write_server" in text
|
||||
assert "jenkins-mcp" in text
|
||||
|
||||
|
||||
def test_glitchtip_filing_contract_doc_covers_issue_153_acceptance():
|
||||
text = (
|
||||
REPO_ROOT / "docs" / "architecture" /
|
||||
"glitchtip-to-gitea-workflow-design.md"
|
||||
).read_text(encoding="utf-8")
|
||||
flattened = " ".join(text.split())
|
||||
for phrase in (
|
||||
"**Status:** Contract for the library-only implementation in "
|
||||
"`Scaled-Tech-Consulting/mcp-control-plane` (#57)",
|
||||
"**Issue:** #153",
|
||||
"not exposed through `glitchtip-mcp`",
|
||||
"real GlitchTip read path",
|
||||
"must not use mocked GlitchTip issue data",
|
||||
"separate write-boundary server/profile",
|
||||
"fail-closed mutation audit before create/link/comment actions",
|
||||
"Deduplication before create",
|
||||
"Create, link, and skip decisions must be represented in tests",
|
||||
"Mutation audit fails closed",
|
||||
):
|
||||
assert phrase in flattened
|
||||
|
||||
@@ -0,0 +1,101 @@
|
||||
"""Git ref mutations must be reported, never hidden as diagnostics (#297).
|
||||
|
||||
``git fetch`` updates remote-tracking refs even though it edits no files.
|
||||
Reports that ran fetch (or any ref-updating command) must carry a
|
||||
``Git ref mutations`` entry and may not claim ``Git/worktree mutations:
|
||||
None`` or classify the fetch as read-only diagnostics.
|
||||
"""
|
||||
import sys
|
||||
import unittest
|
||||
|
||||
sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.parent))
|
||||
|
||||
import review_proofs
|
||||
|
||||
|
||||
def _assess(report, commands):
|
||||
return review_proofs.assess_git_ref_mutation_report(
|
||||
report, command_log=commands)
|
||||
|
||||
|
||||
class TestGitRefMutationDetection(unittest.TestCase):
|
||||
def test_fetch_only_review_with_proper_ledger_passes(self):
|
||||
report = "\n".join([
|
||||
"Git ref mutations: fetched prgs/master",
|
||||
"Review decision: APPROVE",
|
||||
])
|
||||
res = _assess(report, ["git fetch prgs master"])
|
||||
self.assertTrue(res["proven"], res["reasons"])
|
||||
self.assertEqual(res["ref_mutating_commands"], ["git fetch prgs master"])
|
||||
|
||||
def test_fetch_without_ref_mutation_line_is_blocked(self):
|
||||
report = "Review decision: APPROVE\nMutations: None\n"
|
||||
res = _assess(report, ["git fetch prgs master"])
|
||||
self.assertFalse(res["proven"])
|
||||
self.assertTrue(
|
||||
any("git ref mutations" in r.lower() for r in res["reasons"]))
|
||||
|
||||
def test_fetch_with_remote_branch_requires_fetched_target_in_report(self):
|
||||
report = "Git ref mutations: some refs updated\n"
|
||||
res = _assess(report, ["git fetch prgs master"])
|
||||
self.assertFalse(res["proven"])
|
||||
self.assertTrue(
|
||||
any("prgs/master" in r for r in res["reasons"]))
|
||||
|
||||
def test_git_worktree_mutations_none_rejected_when_fetch_occurred(self):
|
||||
report = "\n".join([
|
||||
"Git ref mutations: fetched prgs/master",
|
||||
"Git/worktree mutations: None",
|
||||
])
|
||||
res = _assess(report, ["git fetch prgs master"])
|
||||
self.assertFalse(res["proven"])
|
||||
self.assertTrue(
|
||||
any("git/worktree mutations" in r.lower() for r in res["reasons"]))
|
||||
|
||||
def test_fetch_classified_as_read_only_diagnostics_rejected(self):
|
||||
report = "\n".join([
|
||||
"Git ref mutations: fetched prgs/master",
|
||||
"Read-only diagnostics: git fetch prgs master, git status",
|
||||
])
|
||||
res = _assess(report, ["git fetch prgs master"])
|
||||
self.assertFalse(res["proven"])
|
||||
self.assertTrue(
|
||||
any("read-only" in r.lower() for r in res["reasons"]))
|
||||
|
||||
def test_worktree_creation_is_not_a_ref_mutation(self):
|
||||
report = "Worktree mutations: created branches/review-pr9\n"
|
||||
res = _assess(report, ["git worktree add branches/review-pr9 prgs/master"])
|
||||
self.assertTrue(res["proven"], res["reasons"])
|
||||
self.assertEqual(res["ref_mutating_commands"], [])
|
||||
|
||||
def test_merge_command_counts_as_ref_mutation(self):
|
||||
report = "Review decision: APPROVE\n"
|
||||
res = _assess(report, ["git merge --no-ff feat/x"])
|
||||
self.assertFalse(res["proven"])
|
||||
self.assertIn("git merge --no-ff feat/x", res["ref_mutating_commands"])
|
||||
|
||||
def test_cleanup_branch_delete_counts_as_ref_mutation(self):
|
||||
report = "Cleanup mutations: deleted branch feat/x\n"
|
||||
res = _assess(report, ["git branch -d feat/x"])
|
||||
self.assertFalse(res["proven"])
|
||||
self.assertIn("git branch -d feat/x", res["ref_mutating_commands"])
|
||||
|
||||
def test_no_mutation_blocked_handoff_passes(self):
|
||||
report = "\n".join([
|
||||
"Git/worktree mutations: None",
|
||||
"Current status: blocked handoff, no mutations performed",
|
||||
])
|
||||
res = _assess(report, ["git status --porcelain", "git log --oneline -1"])
|
||||
self.assertTrue(res["proven"], res["reasons"])
|
||||
self.assertEqual(res["ref_mutating_commands"], [])
|
||||
|
||||
def test_command_log_dict_entries_supported(self):
|
||||
report = "Git ref mutations: fetched prgs/master\n"
|
||||
res = _assess(report, [{"command": "git fetch prgs master"}])
|
||||
self.assertTrue(res["proven"], res["reasons"])
|
||||
self.assertEqual(
|
||||
res["ref_mutating_commands"], ["git fetch prgs master"])
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
@@ -0,0 +1,153 @@
|
||||
"""Tests for gitea_list_prs pagination metadata (#340)."""
|
||||
import os
|
||||
import sys
|
||||
import unittest
|
||||
from unittest.mock import patch
|
||||
|
||||
sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.parent))
|
||||
|
||||
import gitea_auth # noqa: E402
|
||||
from mcp_server import gitea_list_prs # noqa: E402
|
||||
from review_proofs import assess_list_prs_pagination_proof # noqa: E402
|
||||
|
||||
FAKE_AUTH = "Basic dGVzdDp0ZXN0"
|
||||
URL = "https://gitea.example.com/api/v1/repos/o/r/pulls?state=open"
|
||||
|
||||
SAMPLE_PR = {
|
||||
"number": 1,
|
||||
"title": "PR 1",
|
||||
"state": "open",
|
||||
"head": {"ref": "branch1"},
|
||||
"base": {"ref": "master"},
|
||||
"mergeable": True,
|
||||
"updated_at": "2026-07-05T12:00:00Z",
|
||||
}
|
||||
|
||||
|
||||
class TestApiFetchPage(unittest.TestCase):
|
||||
@patch("gitea_auth.api_request")
|
||||
def test_final_page_metadata(self, mock_req):
|
||||
mock_req.return_value = [SAMPLE_PR]
|
||||
items, meta = gitea_auth.api_fetch_page(URL, FAKE_AUTH, page=1, limit=50)
|
||||
self.assertEqual(len(items), 1)
|
||||
self.assertFalse(meta["has_more"])
|
||||
self.assertTrue(meta["is_final_page"])
|
||||
self.assertIsNone(meta["next_page"])
|
||||
|
||||
@patch("gitea_auth.api_request")
|
||||
def test_full_page_has_more(self, mock_req):
|
||||
mock_req.return_value = [{"id": i} for i in range(2)]
|
||||
items, meta = gitea_auth.api_fetch_page(URL, FAKE_AUTH, page=1, limit=2)
|
||||
self.assertEqual(len(items), 2)
|
||||
self.assertTrue(meta["has_more"])
|
||||
self.assertFalse(meta["is_final_page"])
|
||||
self.assertEqual(meta["next_page"], 2)
|
||||
|
||||
|
||||
class TestGiteaListPrsPagination(unittest.TestCase):
|
||||
@patch("mcp_server.api_fetch_page")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_explicit_page_returns_has_more(self, _auth, mock_fetch):
|
||||
mock_fetch.return_value = (
|
||||
[SAMPLE_PR],
|
||||
{
|
||||
"page": 1,
|
||||
"per_page": 50,
|
||||
"returned_count": 1,
|
||||
"has_more": False,
|
||||
"next_page": None,
|
||||
"is_final_page": True,
|
||||
},
|
||||
)
|
||||
with patch.dict(os.environ, {}, clear=True):
|
||||
result = gitea_list_prs(page=1, per_page=50)
|
||||
self.assertEqual(len(result["prs"]), 1)
|
||||
self.assertFalse(result["pagination"]["inventory_complete"])
|
||||
self.assertTrue(result["pagination"]["is_final_page"])
|
||||
|
||||
@patch("mcp_server.api_fetch_page")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_multi_page_fetch_all_marks_inventory_complete(self, _auth, mock_fetch):
|
||||
mock_fetch.side_effect = [
|
||||
(
|
||||
[dict(SAMPLE_PR, number=i) for i in (1, 2)],
|
||||
{
|
||||
"page": 1,
|
||||
"per_page": 2,
|
||||
"returned_count": 2,
|
||||
"has_more": True,
|
||||
"next_page": 2,
|
||||
"is_final_page": False,
|
||||
},
|
||||
),
|
||||
(
|
||||
[dict(SAMPLE_PR, number=3)],
|
||||
{
|
||||
"page": 2,
|
||||
"per_page": 2,
|
||||
"returned_count": 1,
|
||||
"has_more": False,
|
||||
"next_page": None,
|
||||
"is_final_page": True,
|
||||
},
|
||||
),
|
||||
]
|
||||
with patch.dict(os.environ, {}, clear=True):
|
||||
result = gitea_list_prs(per_page=2)
|
||||
self.assertEqual([p["number"] for p in result["prs"]], [1, 2, 3])
|
||||
self.assertTrue(result["pagination"]["inventory_complete"])
|
||||
self.assertEqual(result["pagination"]["pages_fetched"], 2)
|
||||
self.assertEqual(result["pagination"]["total_count"], 3)
|
||||
|
||||
|
||||
class TestListPrsPaginationProof(unittest.TestCase):
|
||||
def test_wrapped_final_page_passes_completeness_claim(self):
|
||||
response = {
|
||||
"prs": [],
|
||||
"pagination": {
|
||||
"page": 1,
|
||||
"per_page": 50,
|
||||
"returned_count": 0,
|
||||
"has_more": False,
|
||||
"next_page": None,
|
||||
"is_final_page": True,
|
||||
"inventory_complete": True,
|
||||
"pages_fetched": 1,
|
||||
"total_count": 0,
|
||||
},
|
||||
}
|
||||
result = assess_list_prs_pagination_proof(
|
||||
response, inventory_complete_claimed=True
|
||||
)
|
||||
self.assertTrue(result["proven"])
|
||||
|
||||
def test_bare_list_blocks_completeness_claim(self):
|
||||
result = assess_list_prs_pagination_proof(
|
||||
[{"number": 1}], inventory_complete_claimed=True
|
||||
)
|
||||
self.assertFalse(result["proven"])
|
||||
self.assertIn("pagination metadata", " ".join(result["reasons"]))
|
||||
|
||||
def test_single_page_without_complete_blocks_claim(self):
|
||||
response = {
|
||||
"prs": [{"number": 1}],
|
||||
"pagination": {
|
||||
"page": 1,
|
||||
"per_page": 50,
|
||||
"returned_count": 1,
|
||||
"has_more": True,
|
||||
"next_page": 2,
|
||||
"is_final_page": False,
|
||||
"inventory_complete": False,
|
||||
"pages_fetched": 1,
|
||||
"total_count": None,
|
||||
},
|
||||
}
|
||||
result = assess_list_prs_pagination_proof(
|
||||
response, inventory_complete_claimed=True
|
||||
)
|
||||
self.assertFalse(result["proven"])
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
@@ -1,4 +1,4 @@
|
||||
"""Doc-contract checks for task-specific LLM workflow split (#333)."""
|
||||
"""Doc-contract checks for task-specific LLM workflow split (#333, #334)."""
|
||||
from pathlib import Path
|
||||
|
||||
REPO_ROOT = Path(__file__).resolve().parent.parent
|
||||
@@ -46,15 +46,33 @@ def test_skill_is_router_not_monolithic_review_body():
|
||||
assert "gitea_submit_pr_review" not in SKILL
|
||||
|
||||
|
||||
def test_skill_still_declares_controller_handoff_contract():
|
||||
assert "## Controller Handoff" in SKILL
|
||||
assert "assess_controller_handoff" in SKILL
|
||||
assert "## Global LLM Worktree Rule" in SKILL
|
||||
|
||||
|
||||
def test_review_merge_workflow_contract():
|
||||
text = (SKILL_DIR / "workflows" / "review-merge-pr.md").read_text(encoding="utf-8")
|
||||
assert "canonical: true" in text
|
||||
assert "## 0. Load the canonical workflow first" in text
|
||||
assert "## 26A. Terminal review mutation hard-stop" in text
|
||||
assert "## 11A. Skipped PRs are read-only" in text
|
||||
assert "## 35. Duplicate request-changes prevention" in text
|
||||
assert "## 37. Controller handoff schema" in text
|
||||
assert "INVENTORY_PAGINATION_UNPROVEN" in text
|
||||
assert "ALREADY_LANDED_RECONCILE_REQUIRED" in text
|
||||
|
||||
|
||||
def test_review_merge_final_report_schema_exists():
|
||||
path = SKILL_DIR / "schemas" / "review-merge-final-report.md"
|
||||
text = path.read_text(encoding="utf-8")
|
||||
assert "Controller Handoff" in text
|
||||
assert "Candidate head SHA:" in text
|
||||
assert "Terminal review mutation:" in text
|
||||
assert "Workspace mutations" in text
|
||||
|
||||
|
||||
def test_reconcile_landed_workflow_contract():
|
||||
text = (SKILL_DIR / "workflows" / "reconcile-landed-pr.md").read_text(encoding="utf-8")
|
||||
assert "canonical: true" in text
|
||||
@@ -97,4 +115,10 @@ def test_merge_pr_template_references_workflow():
|
||||
|
||||
def test_start_issue_template_references_work_issue_workflow():
|
||||
text = (SKILL_DIR / "templates" / "start-issue.md").read_text(encoding="utf-8")
|
||||
assert "workflows/work-issue.md" in text
|
||||
assert "workflows/work-issue.md" in text
|
||||
|
||||
|
||||
def test_create_issue_final_report_verifier_exported():
|
||||
from review_proofs import assess_create_issue_final_report
|
||||
|
||||
assert callable(assess_create_issue_final_report)
|
||||
+45
-21
@@ -48,6 +48,21 @@ import mcp_server
|
||||
|
||||
FAKE_AUTH = "Basic dGVzdDp0ZXN0"
|
||||
|
||||
_NO_BLOCKER_FEEDBACK = {
|
||||
"success": True,
|
||||
"has_blocking_change_requests": False,
|
||||
"review_feedback_stale": False,
|
||||
}
|
||||
|
||||
|
||||
def _mark_request_changes_ready(pr_number=8, **kwargs):
|
||||
"""Mark a request_changes decision ready with the #332 duplicate-
|
||||
suppression feedback fetch stubbed to 'no existing blocker'."""
|
||||
with patch("mcp_server.gitea_get_pr_review_feedback",
|
||||
return_value=dict(_NO_BLOCKER_FEEDBACK)):
|
||||
return gitea_mark_final_review_decision(
|
||||
pr_number, "request_changes", **kwargs)
|
||||
|
||||
|
||||
def _formal_review(reviewer, verdict, sha="abc123", review_id=1):
|
||||
return {
|
||||
@@ -400,40 +415,49 @@ class TestMirrorRefs(unittest.TestCase):
|
||||
# ---------------------------------------------------------------------------
|
||||
class TestListPRs(unittest.TestCase):
|
||||
|
||||
@patch("mcp_server.api_get_all")
|
||||
@patch("mcp_server.api_fetch_page")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_list_prs(self, _auth, mock_api):
|
||||
mock_api.return_value = [
|
||||
{
|
||||
def test_list_prs(self, _auth, mock_fetch):
|
||||
mock_fetch.return_value = (
|
||||
[{
|
||||
"number": 1, "title": "PR 1", "state": "open",
|
||||
"head": {"ref": "branch1"}, "base": {"ref": "main"},
|
||||
"html_url": "http://url1", "mergeable": True,
|
||||
"updated_at": "2026-07-05T12:00:00Z"
|
||||
}
|
||||
]
|
||||
}],
|
||||
{
|
||||
"page": 1, "per_page": 50, "returned_count": 1,
|
||||
"has_more": False, "next_page": None, "is_final_page": True,
|
||||
},
|
||||
)
|
||||
with patch.dict(os.environ, {}, clear=True):
|
||||
result = gitea_list_prs()
|
||||
self.assertEqual(len(result), 1)
|
||||
self.assertEqual(result[0]["number"], 1)
|
||||
self.assertEqual(result[0]["head"], "branch1")
|
||||
self.assertNotIn("url", result[0])
|
||||
self.assertEqual(len(result["prs"]), 1)
|
||||
self.assertEqual(result["prs"][0]["number"], 1)
|
||||
self.assertEqual(result["prs"][0]["head"], "branch1")
|
||||
self.assertNotIn("url", result["prs"][0])
|
||||
self.assertTrue(result["pagination"]["inventory_complete"])
|
||||
# Staleness fields for queue reconciliation (#166)
|
||||
self.assertEqual(result[0]["updated_at"], "2026-07-05T12:00:00Z")
|
||||
self.assertEqual(result["prs"][0]["updated_at"], "2026-07-05T12:00:00Z")
|
||||
|
||||
@patch("mcp_server.api_get_all")
|
||||
@patch("mcp_server.api_fetch_page")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_list_prs_reveal_opt_in_includes_url(self, _auth, mock_api):
|
||||
mock_api.return_value = [
|
||||
{
|
||||
def test_list_prs_reveal_opt_in_includes_url(self, _auth, mock_fetch):
|
||||
mock_fetch.return_value = (
|
||||
[{
|
||||
"number": 1, "title": "PR 1", "state": "open",
|
||||
"head": {"ref": "branch1"}, "base": {"ref": "main"},
|
||||
"html_url": "http://url1", "mergeable": True,
|
||||
"updated_at": "2026-07-05T12:00:00Z"
|
||||
}
|
||||
]
|
||||
}],
|
||||
{
|
||||
"page": 1, "per_page": 50, "returned_count": 1,
|
||||
"has_more": False, "next_page": None, "is_final_page": True,
|
||||
},
|
||||
)
|
||||
with patch.dict(os.environ, {"GITEA_MCP_REVEAL_ENDPOINTS": "1"}, clear=True):
|
||||
result = gitea_list_prs()
|
||||
self.assertEqual(result[0]["url"], "http://url1")
|
||||
self.assertEqual(result["prs"][0]["url"], "http://url1")
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -1830,7 +1854,7 @@ class TestSubmitPrReview(unittest.TestCase):
|
||||
@patch("mcp_server.api_request")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_request_changes_succeeds_when_eligible(self, _auth, mock_api):
|
||||
gitea_mark_final_review_decision(8, "request_changes", remote="prgs")
|
||||
_mark_request_changes_ready(remote="prgs")
|
||||
mock_api.side_effect = [
|
||||
{"login": "reviewer-bot"}, self._pr("author-bot"),
|
||||
{"id": 9, "state": "REQUEST_CHANGES"},
|
||||
@@ -1853,7 +1877,7 @@ class TestSubmitPrReview(unittest.TestCase):
|
||||
self.assertEqual(post_calls[0].args[3]["event"], "REQUEST_CHANGES")
|
||||
|
||||
def test_request_changes_blocked_without_eligibility(self):
|
||||
gitea_mark_final_review_decision(8, "request_changes", remote="prgs")
|
||||
_mark_request_changes_ready(remote="prgs")
|
||||
with patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) as _a, \
|
||||
patch("mcp_server.api_request") as mock_api:
|
||||
mock_api.side_effect = [{"login": "reviewer-bot"}, self._pr("author-bot")]
|
||||
@@ -2135,7 +2159,7 @@ class TestSubmitPrReview(unittest.TestCase):
|
||||
reason="operator approved correcting mistaken approve",
|
||||
operator_authorized=True,
|
||||
)
|
||||
gitea_mark_final_review_decision(8, "request_changes", remote="prgs")
|
||||
_mark_request_changes_ready(remote="prgs")
|
||||
second = gitea_submit_pr_review(
|
||||
pr_number=8, action="request_changes", remote="prgs",
|
||||
final_review_decision_ready=True,
|
||||
|
||||
@@ -16,38 +16,50 @@ import gitea_config
|
||||
|
||||
FAKE_AUTH = "Basic dGVzdDp0ZXN0"
|
||||
|
||||
|
||||
def _final_page_fetch(items):
|
||||
return (items, {
|
||||
"page": 1,
|
||||
"per_page": 50,
|
||||
"returned_count": len(items),
|
||||
"has_more": False,
|
||||
"next_page": None,
|
||||
"is_final_page": True,
|
||||
})
|
||||
|
||||
|
||||
class TestPRQueueInventory(unittest.TestCase):
|
||||
|
||||
@patch("mcp_server.api_get_all")
|
||||
@patch("mcp_server.api_fetch_page")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
@patch("mcp_server.get_profile")
|
||||
def test_author_profile_can_list_open_prs(self, mock_get_profile, _auth, mock_get_all):
|
||||
def test_author_profile_can_list_open_prs(self, mock_get_profile, _auth, mock_fetch):
|
||||
mock_get_profile.return_value = {
|
||||
"profile_name": "gitea-author",
|
||||
"allowed_operations": ["read", "gitea.pr.comment"],
|
||||
"forbidden_operations": [],
|
||||
"base_url": None,
|
||||
}
|
||||
mock_get_all.return_value = [
|
||||
mock_fetch.return_value = _final_page_fetch([
|
||||
{"number": 1, "title": "PR 1", "state": "open", "head": {"ref": "branch1", "sha": "abc1"}, "base": {"ref": "master"}, "mergeable": True}
|
||||
]
|
||||
])
|
||||
result = gitea_list_prs(state="open", remote="prgs")
|
||||
self.assertEqual(len(result), 1)
|
||||
self.assertEqual(result[0]["number"], 1)
|
||||
self.assertEqual(len(result["prs"]), 1)
|
||||
self.assertEqual(result["prs"][0]["number"], 1)
|
||||
|
||||
@patch("mcp_server.api_get_all")
|
||||
@patch("mcp_server.api_fetch_page")
|
||||
@patch("mcp_server.api_request")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
@patch("mcp_server.get_profile")
|
||||
@patch("gitea_config.is_runtime_switching_enabled", return_value=True)
|
||||
def test_author_profile_can_produce_pending_reviewer_queue_report(self, mock_switch, mock_get_profile, _auth, mock_api, mock_get_all):
|
||||
def test_author_profile_can_produce_pending_reviewer_queue_report(self, mock_switch, mock_get_profile, _auth, mock_api, mock_fetch):
|
||||
mock_get_profile.return_value = {
|
||||
"profile_name": "gitea-author",
|
||||
"allowed_operations": ["read"],
|
||||
"forbidden_operations": [],
|
||||
"base_url": None,
|
||||
}
|
||||
mock_get_all.return_value = [
|
||||
mock_fetch.return_value = _final_page_fetch([
|
||||
{
|
||||
"number": 1,
|
||||
"title": "PR 1",
|
||||
@@ -72,7 +84,7 @@ class TestPRQueueInventory(unittest.TestCase):
|
||||
"body": "",
|
||||
"updated_at": "2026-07-05T11:00:00Z"
|
||||
}
|
||||
]
|
||||
])
|
||||
mock_api.return_value = {"login": "jcwalker3"} # whoami
|
||||
|
||||
result = gitea_review_pr(pr_number=1, event="APPROVE", remote="prgs")
|
||||
@@ -86,18 +98,18 @@ class TestPRQueueInventory(unittest.TestCase):
|
||||
# updated_at surfaced for reconciliation (#166)
|
||||
self.assertIn("Updated: 2026-07-05T10:00:00Z", result["message"])
|
||||
|
||||
@patch("mcp_server.api_get_all")
|
||||
@patch("mcp_server.api_fetch_page")
|
||||
@patch("mcp_server.api_request")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
@patch("mcp_server.get_profile")
|
||||
def test_author_profile_never_calls_mutation_tools_during_inventory(self, mock_get_profile, _auth, mock_api, mock_get_all):
|
||||
def test_author_profile_never_calls_mutation_tools_during_inventory(self, mock_get_profile, _auth, mock_api, mock_fetch):
|
||||
mock_get_profile.return_value = {
|
||||
"profile_name": "gitea-author",
|
||||
"allowed_operations": ["read"],
|
||||
"forbidden_operations": [],
|
||||
"base_url": None,
|
||||
}
|
||||
mock_get_all.return_value = []
|
||||
mock_fetch.return_value = _final_page_fetch([])
|
||||
mock_api.return_value = {"login": "jcwalker3"}
|
||||
|
||||
gitea_review_pr(pr_number=1, event="APPROVE", remote="prgs")
|
||||
@@ -105,20 +117,20 @@ class TestPRQueueInventory(unittest.TestCase):
|
||||
method = call.args[0]
|
||||
self.assertEqual(method, "GET")
|
||||
|
||||
@patch("mcp_server.api_get_all")
|
||||
@patch("mcp_server.api_fetch_page")
|
||||
@patch("mcp_server.api_request")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
@patch("mcp_server.get_profile")
|
||||
def test_reviewer_profile_can_proceed_from_inventory_to_review_gates(self, mock_get_profile, _auth, mock_api, mock_get_all):
|
||||
def test_reviewer_profile_can_proceed_from_inventory_to_review_gates(self, mock_get_profile, _auth, mock_api, mock_fetch):
|
||||
mock_get_profile.return_value = {
|
||||
"profile_name": "gitea-reviewer",
|
||||
"allowed_operations": ["read", "approve", "review"],
|
||||
"forbidden_operations": [],
|
||||
"base_url": None,
|
||||
}
|
||||
mock_get_all.return_value = [
|
||||
mock_fetch.return_value = _final_page_fetch([
|
||||
{"number": 1, "title": "PR 1", "state": "open", "head": {"ref": "branch1", "sha": "abc1"}, "base": {"ref": "master"}, "mergeable": True, "user": {"login": "other_user"}}
|
||||
]
|
||||
])
|
||||
# mock_api: inventory whoami, eligibility whoami, eligibility PR,
|
||||
# POST review (#244: state + visible-verdict GET reviews).
|
||||
mock_api.side_effect = [
|
||||
@@ -152,19 +164,19 @@ class TestPRQueueInventory(unittest.TestCase):
|
||||
self.assertIn("Repository:", result["message"])
|
||||
self.assertIn("Successfully submitted review", result["message"])
|
||||
|
||||
@patch("mcp_server.api_get_all")
|
||||
@patch("mcp_server.api_fetch_page")
|
||||
@patch("mcp_server.api_request")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
@patch("mcp_server.get_profile")
|
||||
@patch("gitea_config.is_runtime_switching_enabled", return_value=False)
|
||||
def test_static_runtime_rejects_or_hides_profile_activation(self, mock_switch, mock_get_profile, _auth, mock_api, mock_get_all):
|
||||
def test_static_runtime_rejects_or_hides_profile_activation(self, mock_switch, mock_get_profile, _auth, mock_api, mock_fetch):
|
||||
mock_get_profile.return_value = {
|
||||
"profile_name": "gitea-author",
|
||||
"allowed_operations": ["read"],
|
||||
"forbidden_operations": [],
|
||||
"base_url": None,
|
||||
}
|
||||
mock_get_all.return_value = []
|
||||
mock_fetch.return_value = _final_page_fetch([])
|
||||
mock_api.return_value = {"login": "jcwalker3"}
|
||||
|
||||
result = gitea_review_pr(pr_number=1, event="APPROVE", remote="prgs")
|
||||
@@ -200,11 +212,11 @@ class TestPRQueueInventory(unittest.TestCase):
|
||||
self.assertFalse(result["eligible"])
|
||||
self.assertEqual(mock_api.call_count, 0)
|
||||
|
||||
@patch("mcp_server.api_get_all")
|
||||
@patch("mcp_server.api_fetch_page")
|
||||
@patch("mcp_server.api_request")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
@patch("mcp_server.get_profile")
|
||||
def test_inventory_failure_output_is_redacted_no_raw_exception_leak(self, mock_get_profile, _auth, mock_api, mock_get_all):
|
||||
def test_inventory_failure_output_is_redacted_no_raw_exception_leak(self, mock_get_profile, _auth, mock_api, mock_fetch):
|
||||
"""Inventory failure path must use redaction and not leak raw exception text."""
|
||||
mock_get_profile.return_value = {
|
||||
"profile_name": "gitea-author",
|
||||
@@ -213,7 +225,7 @@ class TestPRQueueInventory(unittest.TestCase):
|
||||
"base_url": None,
|
||||
}
|
||||
# Exception contains something that should be redacted or not leaked raw
|
||||
mock_get_all.side_effect = Exception("connection error token supersecrettoken123 at https://internal.example/repo")
|
||||
mock_fetch.side_effect = Exception("connection error token supersecrettoken123 at https://internal.example/repo")
|
||||
mock_api.return_value = {"login": "jcwalker3"}
|
||||
|
||||
result = gitea_review_pr(pr_number=42, event="APPROVE", remote="prgs")
|
||||
@@ -228,11 +240,11 @@ class TestPRQueueInventory(unittest.TestCase):
|
||||
# uses project's redaction pattern (token prefix -> [REDACTED])
|
||||
# even if url leaks, the exception is redacted per pattern
|
||||
|
||||
@patch("mcp_server.api_get_all")
|
||||
@patch("mcp_server.api_fetch_page")
|
||||
@patch("mcp_server.api_request")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
@patch("mcp_server.get_profile")
|
||||
def test_inventory_failure_output_redacts_real_service_urls(self, mock_get_profile, _auth, mock_api, mock_get_all):
|
||||
def test_inventory_failure_output_redacts_real_service_urls(self, mock_get_profile, _auth, mock_api, mock_fetch):
|
||||
"""Failure paths must fully redact real service hostnames and URLs from inventory output."""
|
||||
mock_get_profile.return_value = {
|
||||
"profile_name": "gitea-author",
|
||||
@@ -240,7 +252,7 @@ class TestPRQueueInventory(unittest.TestCase):
|
||||
"forbidden_operations": [],
|
||||
"base_url": None,
|
||||
}
|
||||
mock_get_all.side_effect = Exception("failed to connect to https://gitea.prgs.cc/api/v1/repos/x")
|
||||
mock_fetch.side_effect = Exception("failed to connect to https://gitea.prgs.cc/api/v1/repos/x")
|
||||
mock_api.return_value = {"login": "jcwalker3"}
|
||||
|
||||
result = gitea_review_pr(pr_number=42, event="APPROVE", remote="prgs")
|
||||
@@ -250,11 +262,11 @@ class TestPRQueueInventory(unittest.TestCase):
|
||||
self.assertIn("[REDACTED_URL]", msg)
|
||||
self.assertNotIn("gitea.prgs.cc", msg)
|
||||
|
||||
@patch("mcp_server.api_get_all")
|
||||
@patch("mcp_server.api_fetch_page")
|
||||
@patch("mcp_server.api_request")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
@patch("mcp_server.get_profile")
|
||||
def test_author_profiles_can_perform_read_only_pr_inventory(self, mock_get_profile, _auth, mock_api, mock_get_all):
|
||||
def test_author_profiles_can_perform_read_only_pr_inventory(self, mock_get_profile, _auth, mock_api, mock_fetch):
|
||||
"""Author profiles with read can perform read-only PR inventory (report produced, no mutations)."""
|
||||
mock_get_profile.return_value = {
|
||||
"profile_name": "gitea-author",
|
||||
@@ -262,9 +274,9 @@ class TestPRQueueInventory(unittest.TestCase):
|
||||
"forbidden_operations": [],
|
||||
"base_url": None,
|
||||
}
|
||||
mock_get_all.return_value = [
|
||||
mock_fetch.return_value = _final_page_fetch([
|
||||
{"number": 7, "title": "Some PR", "state": "open", "head": {"ref": "f", "sha": "def"}, "base": {"ref": "master"}, "mergeable": True, "user": {"login": "someone"}, "labels": [], "body": ""}
|
||||
]
|
||||
])
|
||||
mock_api.return_value = {"login": "jcwalker3"}
|
||||
|
||||
result = gitea_review_pr(pr_number=7, event="APPROVE", remote="prgs")
|
||||
@@ -278,12 +290,12 @@ class TestPRQueueInventory(unittest.TestCase):
|
||||
self.assertEqual(call.args[0], "GET")
|
||||
|
||||
@patch("mcp_server._local_git_remote_url")
|
||||
@patch("mcp_server.api_get_all")
|
||||
@patch("mcp_server.api_fetch_page")
|
||||
@patch("mcp_server.api_request")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
@patch("mcp_server.get_profile")
|
||||
def test_empty_inventory_runs_trust_gate_and_blocks_without_trusted_empty(
|
||||
self, mock_get_profile, _auth, mock_api, mock_get_all, mock_local_url
|
||||
self, mock_get_profile, _auth, mock_api, mock_fetch, mock_local_url
|
||||
):
|
||||
mock_get_profile.return_value = {
|
||||
"profile_name": "gitea-author",
|
||||
@@ -291,7 +303,7 @@ class TestPRQueueInventory(unittest.TestCase):
|
||||
"forbidden_operations": [],
|
||||
"base_url": None,
|
||||
}
|
||||
mock_get_all.return_value = []
|
||||
mock_fetch.return_value = _final_page_fetch([])
|
||||
mock_api.return_value = {"login": "jcwalker3"}
|
||||
mock_local_url.return_value = None
|
||||
|
||||
@@ -309,12 +321,12 @@ class TestPRQueueInventory(unittest.TestCase):
|
||||
self.assertNotIn("Open PRs found: 0 (trusted_empty)", msg)
|
||||
|
||||
@patch("mcp_server._local_git_remote_url")
|
||||
@patch("mcp_server.api_get_all")
|
||||
@patch("mcp_server.api_fetch_page")
|
||||
@patch("mcp_server.api_request")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
@patch("mcp_server.get_profile")
|
||||
def test_empty_inventory_trusted_empty_when_gate_passes(
|
||||
self, mock_get_profile, _auth, mock_api, mock_get_all, mock_local_url
|
||||
self, mock_get_profile, _auth, mock_api, mock_fetch, mock_local_url
|
||||
):
|
||||
mock_get_profile.return_value = {
|
||||
"profile_name": "gitea-author",
|
||||
@@ -322,7 +334,7 @@ class TestPRQueueInventory(unittest.TestCase):
|
||||
"forbidden_operations": [],
|
||||
"base_url": None,
|
||||
}
|
||||
mock_get_all.return_value = []
|
||||
mock_fetch.return_value = _final_page_fetch([])
|
||||
mock_api.return_value = {"login": "jcwalker3"}
|
||||
mock_local_url.return_value = (
|
||||
"https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools.git"
|
||||
@@ -341,11 +353,11 @@ class TestPRQueueInventory(unittest.TestCase):
|
||||
self.assertIn("Open PRs found: 0 (trusted_empty)", msg)
|
||||
|
||||
@patch("mcp_server._local_git_remote_url")
|
||||
@patch("mcp_server.api_get_all")
|
||||
@patch("mcp_server.api_fetch_page")
|
||||
@patch("mcp_server.api_request")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
@patch("mcp_server.get_profile")
|
||||
def test_author_profiles_cannot_approve_request_changes_merge_or_bypass_gates(self, mock_get_profile, _auth, mock_api, mock_get_all, mock_local_url):
|
||||
def test_author_profiles_cannot_approve_request_changes_merge_or_bypass_gates(self, mock_get_profile, _auth, mock_api, mock_fetch, mock_local_url):
|
||||
"""Author profiles still cannot approve, request_changes, merge, or bypass gates even with inventory."""
|
||||
mock_local_url.return_value = (
|
||||
"https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools.git"
|
||||
@@ -357,7 +369,7 @@ class TestPRQueueInventory(unittest.TestCase):
|
||||
"forbidden_operations": [],
|
||||
"base_url": None,
|
||||
}
|
||||
mock_get_all.return_value = []
|
||||
mock_fetch.return_value = _final_page_fetch([])
|
||||
mock_api.return_value = {"login": "jcwalker3"}
|
||||
|
||||
result = gitea_review_pr(pr_number=1, event=event, remote="prgs")
|
||||
@@ -377,10 +389,10 @@ class TestPRQueueInventory(unittest.TestCase):
|
||||
# Tests use direct list/view calls + inventory path to ensure fields and
|
||||
# live data are available for detection without trusting prior handoffs.
|
||||
|
||||
@patch("mcp_server.api_get_all")
|
||||
@patch("mcp_server.api_fetch_page")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
@patch("mcp_server.get_profile")
|
||||
def test_r5_s1_stale_prior_handoff_merged_but_live_pr_open(self, mock_get_profile, _auth, mock_get_all):
|
||||
def test_r5_s1_stale_prior_handoff_merged_but_live_pr_open(self, mock_get_profile, _auth, mock_fetch):
|
||||
"""Scenario 1: stale prior handoff says merged but live PR list says open."""
|
||||
mock_get_profile.return_value = {
|
||||
"profile_name": "gitea-author",
|
||||
@@ -388,17 +400,17 @@ class TestPRQueueInventory(unittest.TestCase):
|
||||
"forbidden_operations": [],
|
||||
"base_url": None,
|
||||
}
|
||||
mock_get_all.return_value = [{
|
||||
mock_fetch.return_value = _final_page_fetch([{
|
||||
"number": 99, "title": "Stale merge claim", "state": "open",
|
||||
"head": {"ref": "f99", "sha": "sha99"}, "base": {"ref": "master"},
|
||||
"mergeable": True, "user": {"login": "other"},
|
||||
"labels": [], "body": "Fixes #200",
|
||||
"updated_at": "2026-07-05T22:00:00Z"
|
||||
}]
|
||||
}])
|
||||
prs = gitea_list_prs(state="open", remote="prgs")
|
||||
self.assertEqual(len(prs), 1)
|
||||
self.assertEqual(prs[0]["state"], "open")
|
||||
self.assertIn("updated_at", prs[0]) # enables staleness check vs prior "merged"
|
||||
self.assertEqual(len(prs["prs"]), 1)
|
||||
self.assertEqual(prs["prs"][0]["state"], "open")
|
||||
self.assertIn("updated_at", prs["prs"][0]) # enables staleness check vs prior "merged"
|
||||
|
||||
@patch("mcp_server.api_request")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
@@ -424,11 +436,11 @@ class TestPRQueueInventory(unittest.TestCase):
|
||||
self.assertIsNotNone(pr.get("closed_at"))
|
||||
# live closed would contradict prior "open" handoff
|
||||
|
||||
@patch("mcp_server.api_get_all")
|
||||
@patch("mcp_server.api_fetch_page")
|
||||
@patch("mcp_server.api_request")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
@patch("mcp_server.get_profile")
|
||||
def test_r5_s3_list_prs_and_view_pr_disagree(self, mock_get_profile, _auth, mock_api, mock_get_all):
|
||||
def test_r5_s3_list_prs_and_view_pr_disagree(self, mock_get_profile, _auth, mock_api, mock_fetch):
|
||||
"""Scenario 3: gitea_list_prs and gitea_view_pr disagree on state/details."""
|
||||
mock_get_profile.return_value = {
|
||||
"profile_name": "gitea-author",
|
||||
@@ -437,12 +449,12 @@ class TestPRQueueInventory(unittest.TestCase):
|
||||
"base_url": None,
|
||||
}
|
||||
# list says open
|
||||
mock_get_all.return_value = [{
|
||||
mock_fetch.return_value = _final_page_fetch([{
|
||||
"number": 77, "title": "Disagree", "state": "open",
|
||||
"head": {"ref": "f77", "sha": "s77"}, "base": {"ref": "master"},
|
||||
"mergeable": True, "user": {"login": "x"},
|
||||
"labels": [], "body": "", "updated_at": "2026-07-05T23:00:00Z"
|
||||
}]
|
||||
}])
|
||||
# view says closed (simulating inconsistency)
|
||||
mock_api.return_value = {
|
||||
"number": 77, "title": "Disagree", "state": "closed",
|
||||
@@ -453,7 +465,7 @@ class TestPRQueueInventory(unittest.TestCase):
|
||||
}
|
||||
listed = gitea_list_prs(state="open", remote="prgs")
|
||||
viewed = gitea_view_pr(pr_number=77, remote="prgs")
|
||||
self.assertEqual(listed[0]["state"], "open")
|
||||
self.assertEqual(listed["prs"][0]["state"], "open")
|
||||
self.assertEqual(viewed["state"], "closed")
|
||||
# caller must reconcile; mismatch is hard blocker per rules
|
||||
|
||||
@@ -483,11 +495,11 @@ class TestPRQueueInventory(unittest.TestCase):
|
||||
self.assertIsNone(pr.get("merge_commit_sha"))
|
||||
# post-merge view must surface missing commit for detection
|
||||
|
||||
@patch("mcp_server.api_get_all")
|
||||
@patch("mcp_server.api_fetch_page")
|
||||
@patch("mcp_server.api_request")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
@patch("mcp_server.get_profile")
|
||||
def test_r5_s5_linked_issue_remains_open_after_claimed_merge(self, mock_get_profile, _auth, mock_api, mock_get_all):
|
||||
def test_r5_s5_linked_issue_remains_open_after_claimed_merge(self, mock_get_profile, _auth, mock_api, mock_fetch):
|
||||
"""Scenario 5: linked issue remains open after claimed merge."""
|
||||
mock_get_profile.return_value = {
|
||||
"profile_name": "gitea-author",
|
||||
@@ -496,12 +508,12 @@ class TestPRQueueInventory(unittest.TestCase):
|
||||
"base_url": None,
|
||||
}
|
||||
# PR claims merge, links #300
|
||||
mock_get_all.return_value = [{
|
||||
mock_fetch.return_value = _final_page_fetch([{
|
||||
"number": 300, "title": "PR for #300", "state": "closed",
|
||||
"head": {"ref": "f300", "sha": "s3"}, "base": {"ref": "master"},
|
||||
"mergeable": True, "user": {"login": "u"},
|
||||
"labels": [], "body": "Fixes #300", "updated_at": "2026-07-05T20:20:00Z"
|
||||
}]
|
||||
}])
|
||||
# But linked issue (via list_issues or view) still open - here we use view_pr body to confirm link
|
||||
# For this test we assert the PR data shows link, caller must check issue state live
|
||||
mock_api.return_value = {
|
||||
|
||||
@@ -24,11 +24,15 @@ from review_proofs import ( # noqa: E402
|
||||
ISSUE_SELECTION_CONTINUATION_EXPLICIT,
|
||||
ISSUE_SELECTION_REPRESENTED_BY_OPEN_PR,
|
||||
assess_author_pr_report,
|
||||
assess_reviewer_baseline_validation_proof,
|
||||
assess_capability_evidence,
|
||||
assess_capability_proof,
|
||||
assess_contradictory_no_pr_claim,
|
||||
assess_continuation_mode_report,
|
||||
assess_controller_handoff,
|
||||
assess_create_issue_final_report,
|
||||
assess_create_issue_mode_isolation,
|
||||
assess_create_issue_workflow_source,
|
||||
assess_edited_pr_inventory_coverage,
|
||||
assess_empty_queue_report,
|
||||
assess_fresh_issue_selection,
|
||||
@@ -2042,5 +2046,240 @@ class TestIssueFilingFinalReport(unittest.TestCase):
|
||||
self.assertTrue(result["downgraded"])
|
||||
|
||||
|
||||
class TestCreateIssueFinalReport(unittest.TestCase):
|
||||
"""#337: create-issue final-report verifier coverage."""
|
||||
|
||||
ISSUE_TITLE = "Extract create-issue workflow from monolithic LLM project skill"
|
||||
CAPABILITIES = {
|
||||
"create_issue": {
|
||||
"requested_task": "create_issue",
|
||||
"required_operation_permission": "gitea.issue.create",
|
||||
"allowed_in_current_session": True,
|
||||
},
|
||||
}
|
||||
CLOSEST = [{"number": 333, "title": "Split monolithic workflow", "state": "closed"}]
|
||||
|
||||
def _good_report(self):
|
||||
return "\n".join([
|
||||
"Task mode: create-issue",
|
||||
"Workflow source: skills/llm-project-workflow/workflows/create-issue.md",
|
||||
f"Updated issue #337 — `{self.ISSUE_TITLE}` (remaining verifier AC).",
|
||||
"Duplicate check: searched 54 open issues.",
|
||||
"Closest existing: #333 — Split monolithic workflow (partial overlap).",
|
||||
"Why new issue justified: dedicated create-issue extraction.",
|
||||
"Only mutation: issue creation (gitea.issue.create).",
|
||||
"Confirm no labels, comments, PRs, reviews, merges, or closes.",
|
||||
"## Controller Handoff",
|
||||
"- Task: create-issue",
|
||||
"- Repo: Scaled-Tech-Consulting/Gitea-Tools",
|
||||
"- Role: author",
|
||||
"- Identity: jcwalker3 / prgs-author",
|
||||
"- Active profile: prgs-author",
|
||||
"- Runtime context: prgs-author on prgs",
|
||||
"- Requested issue task: extract create-issue workflow verifier",
|
||||
"- Workflow source: workflows/create-issue.md @ fd396df",
|
||||
"- Capability proof: create_issue allowed (gitea.issue.create)",
|
||||
"- Duplicate search terms: create-issue, workflow extract",
|
||||
"- Duplicate search pagination proof: 54 open issues < limit 500",
|
||||
"- Duplicates found: #333 partial only",
|
||||
"- Issues created: none (implementation PR for remaining AC)",
|
||||
"- Issues commented: none",
|
||||
"- Issues edited: none",
|
||||
"- Issues skipped as duplicates: none",
|
||||
"- File edits by issue creator: review_proofs.py, tests",
|
||||
"- Worktree/index mutations: feat/issue-337 worktree created",
|
||||
"- Git ref mutations: fetch prgs/master",
|
||||
"- MCP/Gitea mutations: gitea_mark_issue start on #337",
|
||||
"- Issue mutations: claimed #337",
|
||||
"- Label/assignment/milestone mutations: none",
|
||||
"- External-state mutations: none",
|
||||
"- Read-only diagnostics: whoami, list issues",
|
||||
"- Blockers: none",
|
||||
"- Current status: implementing verifier",
|
||||
"- Safe next action: open PR",
|
||||
"- Safety statement: no review/merge/repo main-checkout edits",
|
||||
"- Issue/PR: #337",
|
||||
"- Branch/SHA: feat/issue-337-create-issue-verifier",
|
||||
"- Files changed: review_proofs.py, tests",
|
||||
"- Validation: pytest tests/test_review_proofs.py",
|
||||
"- Mutations: create_issue capability resolved; mark_issue only",
|
||||
"- Next: open PR",
|
||||
"- Safety: no review/merge",
|
||||
])
|
||||
|
||||
def test_complete_create_issue_report_earns_a(self):
|
||||
result = assess_create_issue_final_report(
|
||||
self._good_report(),
|
||||
issue_number=337,
|
||||
issue_title=self.ISSUE_TITLE,
|
||||
action="updated",
|
||||
mutations=["create_issue"],
|
||||
resolved_capabilities=self.CAPABILITIES,
|
||||
issues_searched=54,
|
||||
closest_matches=self.CLOSEST,
|
||||
performed_mutations=["create_issue"],
|
||||
)
|
||||
self.assertEqual(result["grade"], "A")
|
||||
self.assertFalse(result["downgraded"])
|
||||
|
||||
def test_missing_workflow_source_downgrades(self):
|
||||
report = self._good_report().replace("workflows/create-issue.md", "SKILL.md only")
|
||||
result = assess_create_issue_workflow_source(report)
|
||||
self.assertTrue(result["downgraded"])
|
||||
|
||||
def test_work_issue_field_in_handoff_downgrades(self):
|
||||
report = self._good_report().replace(
|
||||
"- Safety statement:",
|
||||
"- Selected issue: #123\n- Safety statement:",
|
||||
)
|
||||
result = assess_create_issue_mode_isolation(report)
|
||||
self.assertTrue(result["downgraded"])
|
||||
self.assertTrue(
|
||||
any("Selected issue" in r for r in result["reasons"])
|
||||
)
|
||||
|
||||
def test_review_merge_field_in_handoff_downgrades(self):
|
||||
report = self._good_report().replace(
|
||||
"- Safety statement:",
|
||||
"- Review decision: APPROVE\n- Safety statement:",
|
||||
)
|
||||
result = assess_create_issue_mode_isolation(report)
|
||||
self.assertTrue(result["downgraded"])
|
||||
|
||||
def test_legacy_workspace_mutations_without_precise_categories_downgrades(self):
|
||||
report = self._good_report().replace(
|
||||
"- File edits by issue creator:",
|
||||
"- Workspace mutations: none\n- File edits by issue creator:",
|
||||
)
|
||||
lines = report.splitlines()
|
||||
cleaned = [
|
||||
line for line in lines
|
||||
if not line.strip().startswith("- File edits by issue creator:")
|
||||
]
|
||||
report = "\n".join(cleaned)
|
||||
result = assess_create_issue_mode_isolation(report)
|
||||
self.assertTrue(result["downgraded"])
|
||||
|
||||
|
||||
class TestReviewerBaselineValidationProof(unittest.TestCase):
|
||||
"""Issue #325: baseline validation must use branches/ worktrees."""
|
||||
|
||||
ROOT = "/repo/Gitea-Tools"
|
||||
|
||||
def _good_baseline_proof(self, **overrides):
|
||||
proof = {
|
||||
"worktree_path": f"{self.ROOT}/branches/baseline-master-pr280",
|
||||
"baseline_target_sha": PINNED,
|
||||
"pr_head_sha": OTHER,
|
||||
"baseline_failures": ["tests/test_foo.py::test_bar"],
|
||||
"pr_failures": ["tests/test_foo.py::test_bar"],
|
||||
"failure_signatures_match": True,
|
||||
"clean_before": True,
|
||||
"clean_after": True,
|
||||
}
|
||||
proof.update(overrides)
|
||||
return proof
|
||||
|
||||
def test_main_checkout_test_run_blocks(self):
|
||||
result = assess_reviewer_baseline_validation_proof(
|
||||
validation_runs=[
|
||||
{
|
||||
"command": "venv/bin/pytest tests/",
|
||||
"working_directory": self.ROOT,
|
||||
"project_root": self.ROOT,
|
||||
}
|
||||
],
|
||||
project_root=self.ROOT,
|
||||
)
|
||||
self.assertFalse(result["proven"])
|
||||
self.assertTrue(result["block"])
|
||||
self.assertTrue(result["violations"])
|
||||
|
||||
def test_branches_worktree_test_run_passes(self):
|
||||
result = assess_reviewer_baseline_validation_proof(
|
||||
validation_runs=[
|
||||
{
|
||||
"command": "venv/bin/pytest tests/",
|
||||
"working_directory": f"{self.ROOT}/branches/review-pr-280",
|
||||
"project_root": self.ROOT,
|
||||
}
|
||||
],
|
||||
project_root=self.ROOT,
|
||||
)
|
||||
self.assertTrue(result["proven"])
|
||||
|
||||
def test_preexisting_claim_without_baseline_proof_fails(self):
|
||||
result = assess_reviewer_baseline_validation_proof(
|
||||
report_text=(
|
||||
"Validation: full-suite failures are pre-existing on master."
|
||||
),
|
||||
project_root=self.ROOT,
|
||||
)
|
||||
self.assertFalse(result["proven"])
|
||||
self.assertTrue(result["claims_preexisting"])
|
||||
|
||||
def test_preexisting_claim_with_complete_baseline_proof_passes(self):
|
||||
result = assess_reviewer_baseline_validation_proof(
|
||||
report_text="Failures are pre-existing on master.",
|
||||
baseline_proof=self._good_baseline_proof(),
|
||||
project_root=self.ROOT,
|
||||
)
|
||||
self.assertTrue(result["proven"])
|
||||
|
||||
def test_mismatched_failures_block_preexisting_claim(self):
|
||||
result = assess_reviewer_baseline_validation_proof(
|
||||
report_text="Same as master.",
|
||||
baseline_proof=self._good_baseline_proof(
|
||||
failure_signatures_match=False,
|
||||
pr_failures=["tests/test_other.py::test_other"],
|
||||
),
|
||||
project_root=self.ROOT,
|
||||
)
|
||||
self.assertFalse(result["proven"])
|
||||
|
||||
def test_report_parsed_main_checkout_cwd_blocks(self):
|
||||
result = assess_reviewer_baseline_validation_proof(
|
||||
report_text=(
|
||||
"- Validation command: venv/bin/pytest tests/\n"
|
||||
f"- Working directory: {self.ROOT}\n"
|
||||
),
|
||||
project_root=self.ROOT,
|
||||
)
|
||||
self.assertFalse(result["proven"])
|
||||
self.assertTrue(result["block"])
|
||||
|
||||
def test_build_final_report_downgrades_main_checkout_validation(self):
|
||||
report = build_final_report(
|
||||
checkout_proof=_good_checkout(),
|
||||
inventory=_good_inventory(),
|
||||
validation=_good_validation(),
|
||||
contamination=_good_contamination(),
|
||||
identity_eligible=True,
|
||||
merge_performed=False,
|
||||
issue_status_verified=True,
|
||||
capability_evidence=_good_capability_evidence(),
|
||||
sweep=_good_sweep(),
|
||||
live_state=_good_live_state(),
|
||||
role_boundary=_good_role_boundary(),
|
||||
review_mutation=_good_review_mutation(),
|
||||
controller_handoff=_good_handoff(),
|
||||
capability_proof=_good_capability_proof(),
|
||||
sweep_proof=_good_secret_sweep(),
|
||||
worktree_proof={
|
||||
"worktree_path": "/repo/branches/review-feat-issue-224",
|
||||
"porcelain_status": "",
|
||||
"pr_scope_files": ["docs/wiki/Repositories.md"],
|
||||
"scratch_used": True,
|
||||
},
|
||||
report_text=(
|
||||
"- Validation command: venv/bin/pytest tests/\n"
|
||||
f"- Working directory: {self.ROOT}\n"
|
||||
),
|
||||
project_root=self.ROOT,
|
||||
)
|
||||
self.assertNotEqual(report["grade"], "A")
|
||||
self.assertFalse(report["baseline_validation_proven"])
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
|
||||
@@ -0,0 +1,187 @@
|
||||
"""Tests for the session hard-stop after a terminal review mutation (#332).
|
||||
|
||||
Extends the single-terminal-decision machinery (#211): after a live
|
||||
REQUEST_CHANGES the run must stop; after an APPROVE only the merge sequence
|
||||
for that same PR may continue; a different PR can never be reviewed or
|
||||
merged in the same run; duplicate REQUEST_CHANGES at an unchanged head is
|
||||
suppressed.
|
||||
"""
|
||||
import os
|
||||
import unittest
|
||||
from unittest.mock import patch
|
||||
|
||||
import mcp_server
|
||||
|
||||
|
||||
def _lock(mutations=None, correction=False):
|
||||
return {
|
||||
"task": "review_pr",
|
||||
"remote": "prgs",
|
||||
"session_pid": os.getpid(),
|
||||
"session_profile": "prgs-reviewer",
|
||||
"session_profile_lock": "",
|
||||
"final_review_decision_ready": False,
|
||||
"ready_pr_number": None,
|
||||
"ready_action": None,
|
||||
"ready_expected_head_sha": None,
|
||||
"ready_remote": None,
|
||||
"ready_org": None,
|
||||
"ready_repo": None,
|
||||
"live_mutations": list(mutations or []),
|
||||
"correction_authorized": correction,
|
||||
"correction_reason": None,
|
||||
}
|
||||
|
||||
|
||||
def _seed(mutations=None, correction=False):
|
||||
mcp_server._save_review_decision_lock(_lock(mutations, correction))
|
||||
|
||||
|
||||
APPROVED_A = {"pr_number": 5, "action": "approve", "review_id": 1,
|
||||
"review_state": "approve"}
|
||||
RC_A = {"pr_number": 5, "action": "request_changes", "review_id": 2,
|
||||
"review_state": "request_changes"}
|
||||
|
||||
|
||||
class TestTerminalHardStopReasons(unittest.TestCase):
|
||||
def tearDown(self):
|
||||
mcp_server._save_review_decision_lock(None)
|
||||
|
||||
def test_no_lock_no_reasons(self):
|
||||
mcp_server._save_review_decision_lock(None)
|
||||
self.assertEqual(
|
||||
mcp_server.terminal_review_hard_stop_reasons(5, "merge"), [])
|
||||
|
||||
def test_no_terminal_mutation_no_reasons(self):
|
||||
_seed()
|
||||
self.assertEqual(
|
||||
mcp_server.terminal_review_hard_stop_reasons(5, "merge"), [])
|
||||
|
||||
def test_request_changes_blocks_everything(self):
|
||||
_seed([RC_A])
|
||||
for op in ("merge", "mark_ready", "review"):
|
||||
for pr in (5, 6):
|
||||
reasons = mcp_server.terminal_review_hard_stop_reasons(pr, op)
|
||||
self.assertTrue(reasons, f"{op}/{pr}")
|
||||
self.assertIn("request_changes on PR #5", reasons[0])
|
||||
self.assertIn("#332", reasons[0])
|
||||
|
||||
def test_approve_allows_same_pr_merge_only(self):
|
||||
_seed([APPROVED_A])
|
||||
self.assertEqual(
|
||||
mcp_server.terminal_review_hard_stop_reasons(5, "merge"), [])
|
||||
self.assertTrue(
|
||||
mcp_server.terminal_review_hard_stop_reasons(6, "merge"))
|
||||
self.assertTrue(
|
||||
mcp_server.terminal_review_hard_stop_reasons(6, "mark_ready"))
|
||||
self.assertTrue(
|
||||
mcp_server.terminal_review_hard_stop_reasons(6, "review"))
|
||||
|
||||
def test_correction_reopens_review_path_only(self):
|
||||
_seed([RC_A], correction=True)
|
||||
self.assertEqual(
|
||||
mcp_server.terminal_review_hard_stop_reasons(5, "mark_ready"), [])
|
||||
self.assertEqual(
|
||||
mcp_server.terminal_review_hard_stop_reasons(5, "review"), [])
|
||||
# correction never opens a cross-PR merge
|
||||
self.assertTrue(
|
||||
mcp_server.terminal_review_hard_stop_reasons(6, "merge"))
|
||||
|
||||
|
||||
class TestMergeHardStopWiring(unittest.TestCase):
|
||||
def tearDown(self):
|
||||
mcp_server._save_review_decision_lock(None)
|
||||
|
||||
def test_merge_blocked_after_request_changes(self):
|
||||
_seed([RC_A])
|
||||
result = mcp_server.gitea_merge_pr(
|
||||
pr_number=6, confirmation="MERGE PR 6", remote="prgs")
|
||||
self.assertFalse(result["performed"])
|
||||
self.assertTrue(
|
||||
any("#332" in r for r in result["reasons"]), result["reasons"])
|
||||
|
||||
def test_merge_of_other_pr_blocked_after_approve(self):
|
||||
_seed([APPROVED_A])
|
||||
result = mcp_server.gitea_merge_pr(
|
||||
pr_number=6, confirmation="MERGE PR 6", remote="prgs")
|
||||
self.assertFalse(result["performed"])
|
||||
self.assertTrue(
|
||||
any("#332" in r for r in result["reasons"]), result["reasons"])
|
||||
|
||||
|
||||
class TestMarkFinalHardStopWiring(unittest.TestCase):
|
||||
def tearDown(self):
|
||||
mcp_server._save_review_decision_lock(None)
|
||||
|
||||
def test_mark_ready_blocked_after_terminal_mutation(self):
|
||||
_seed([RC_A])
|
||||
result = mcp_server.gitea_mark_final_review_decision(
|
||||
pr_number=6, action="approve", remote="prgs")
|
||||
self.assertFalse(result["marked_ready"])
|
||||
self.assertTrue(
|
||||
any("#332" in r for r in result["reasons"]), result["reasons"])
|
||||
|
||||
|
||||
def _feedback(blocking, stale=False, success=True):
|
||||
return {
|
||||
"success": success,
|
||||
"has_blocking_change_requests": blocking,
|
||||
"review_feedback_stale": stale,
|
||||
"current_head_sha": "abc123",
|
||||
}
|
||||
|
||||
|
||||
class TestDuplicateRequestChangesSuppression(unittest.TestCase):
|
||||
def tearDown(self):
|
||||
mcp_server._save_review_decision_lock(None)
|
||||
|
||||
def test_duplicate_request_changes_blocked_at_same_head(self):
|
||||
_seed()
|
||||
with patch.object(mcp_server, "gitea_get_pr_review_feedback",
|
||||
return_value=_feedback(blocking=True, stale=False)):
|
||||
result = mcp_server.gitea_mark_final_review_decision(
|
||||
pr_number=6, action="request_changes", remote="prgs")
|
||||
self.assertFalse(result["marked_ready"])
|
||||
self.assertTrue(
|
||||
any("duplicate" in r for r in result["reasons"]),
|
||||
result["reasons"])
|
||||
|
||||
def test_request_changes_allowed_when_prior_blocker_stale(self):
|
||||
_seed()
|
||||
with patch.object(mcp_server, "gitea_get_pr_review_feedback",
|
||||
return_value=_feedback(blocking=True, stale=True)):
|
||||
result = mcp_server.gitea_mark_final_review_decision(
|
||||
pr_number=6, action="request_changes", remote="prgs")
|
||||
self.assertTrue(result["marked_ready"], result.get("reasons"))
|
||||
|
||||
def test_request_changes_allowed_when_no_blocker(self):
|
||||
_seed()
|
||||
with patch.object(mcp_server, "gitea_get_pr_review_feedback",
|
||||
return_value=_feedback(blocking=False)):
|
||||
result = mcp_server.gitea_mark_final_review_decision(
|
||||
pr_number=6, action="request_changes", remote="prgs")
|
||||
self.assertTrue(result["marked_ready"], result.get("reasons"))
|
||||
|
||||
def test_request_changes_fails_closed_when_feedback_unavailable(self):
|
||||
_seed()
|
||||
with patch.object(mcp_server, "gitea_get_pr_review_feedback",
|
||||
return_value=_feedback(blocking=False,
|
||||
success=False)):
|
||||
result = mcp_server.gitea_mark_final_review_decision(
|
||||
pr_number=6, action="request_changes", remote="prgs")
|
||||
self.assertFalse(result["marked_ready"])
|
||||
self.assertTrue(
|
||||
any("could not verify" in r for r in result["reasons"]),
|
||||
result["reasons"])
|
||||
|
||||
def test_approve_does_not_fetch_feedback(self):
|
||||
_seed()
|
||||
with patch.object(mcp_server, "gitea_get_pr_review_feedback",
|
||||
side_effect=AssertionError("must not be called")):
|
||||
result = mcp_server.gitea_mark_final_review_decision(
|
||||
pr_number=6, action="approve", remote="prgs")
|
||||
self.assertTrue(result["marked_ready"], result.get("reasons"))
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
Reference in New Issue
Block a user