Merge pull request 'feat: require issue type and workflow status labels (Closes #513)' (#518) from codex/issue-513-workflow-labels into master

tests/test_create_issue.py

@@ -103,6 +103,37 @@ class TestAPIPayload(unittest.TestCase):
             self.assertEqual(payload["title"], "My Title")
             self.assertEqual(payload["body"], "My Body")
 
+    @patch("create_issue.get_credentials", return_value=FAKE_CREDS)
+    def test_applies_workflow_labels_by_name(self, _cred):
+        def api_side_effect(method, url, auth, payload=None):
+            if method == "GET" and url.endswith("/labels"):
+                return [
+                    {"id": 1, "name": "type:feature"},
+                    {"id": 2, "name": "status:ready"},
+                ]
+            if method == "POST" and url.endswith("/issues"):
+                return {"number": 7, "html_url": "http://x/7"}
+            if method == "PUT" and url.endswith("/issues/7/labels"):
+                return [{"name": "type:feature"}, {"name": "status:ready"}]
+            return {}
+
+        with patch("create_issue.api_request", side_effect=api_side_effect) as mock_api:
+            rc = create_issue.main([
+                "--title", "My Title",
+                "--type-label", "feature",
+                "--status-label", "ready",
+            ])
+        self.assertEqual(rc, 0)
+        put_call = next(c for c in mock_api.call_args_list if c[0][0] == "PUT")
+        self.assertEqual(put_call[0][3], {"labels": [1, 2]})
+
+    @patch("create_issue.get_credentials", return_value=FAKE_CREDS)
+    def test_require_workflow_labels_blocks_missing_labels(self, _cred):
+        with patch("create_issue.api_request") as mock_api:
+            rc = create_issue.main(["--title", "My Title", "--require-workflow-labels"])
+        self.assertEqual(rc, 1)
+        mock_api.assert_not_called()
+
     @patch("create_issue.get_credentials", return_value=FAKE_CREDS)
     def test_url_construction(self, _cred):
         with patch("create_issue.api_request",

tests/test_issue_workflow_labels.py

@@ -0,0 +1,73 @@
+"""Tests for canonical issue workflow label policy (#513)."""
+
+from __future__ import annotations
+
+import sys
+import unittest
+from pathlib import Path
+
+sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
+
+import issue_workflow_labels as labels  # noqa: E402
+
+
+class TestIssueWorkflowLabelValidation(unittest.TestCase):
+    def test_valid_labeled_issue(self):
+        result = labels.assess_issue_labels(["type:feature", "status:ready"])
+        self.assertTrue(result["valid"])
+        self.assertEqual(result["type_labels"], ["type:feature"])
+        self.assertEqual(result["status_labels"], ["status:ready"])
+
+    def test_issue_missing_type_label(self):
+        result = labels.assess_issue_labels(["status:ready"])
+        self.assertFalse(result["valid"])
+        self.assertIn("issue is missing a type:* label", result["errors"])
+
+    def test_issue_missing_status_label(self):
+        result = labels.assess_issue_labels(["type:bug"])
+        self.assertFalse(result["valid"])
+        self.assertIn("issue is missing a status:* label", result["errors"])
+
+    def test_discussion_issue_missing_type_discussion(self):
+        result = labels.assess_issue_labels(
+            ["type:feature", "status:triage"],
+            discussion=True,
+        )
+        self.assertFalse(result["valid"])
+        self.assertIn("discussion issue is missing type:discussion", result["errors"])
+
+    def test_multiple_conflicting_status_labels(self):
+        result = labels.assess_issue_labels(
+            ["type:feature", "status:ready", "status:blocked"]
+        )
+        self.assertFalse(result["valid"])
+        self.assertTrue(
+            any("multiple active status:* labels" in err for err in result["errors"])
+        )
+
+
+class TestIssueWorkflowStatusTransitions(unittest.TestCase):
+    def test_status_transition_removes_old_status_label(self):
+        result = labels.transition_status_labels(
+            ["type:feature", "status:ready", "workflow"],
+            "in-progress",
+        )
+        self.assertEqual(result, ["type:feature", "workflow", "status:in-progress"])
+
+    def test_pr_open_transition_applies_status_pr_open(self):
+        result = labels.transition_status_labels(
+            ["type:feature", "status:in-progress"],
+            "pr-open",
+        )
+        self.assertEqual(result, ["type:feature", "status:pr-open"])
+
+    def test_duplicate_transition_applies_status_duplicate(self):
+        result = labels.transition_status_labels(
+            ["type:process", "status:triage"],
+            "duplicate",
+        )
+        self.assertEqual(result, ["type:process", "status:duplicate"])
+
+
+if __name__ == "__main__":
+    unittest.main()

tests/test_issue_write_tool_gates.py

@@ -164,13 +164,14 @@ class TestAllowedIssueWritesProceed(IssueWriteGateBase):
             result = mcp_server.gitea_close_issue(issue_number=9, remote="prgs")
         self.assertTrue(result.get("success"))
 
+    @patch("mcp_server.api_get_all", return_value=[{"id": 10, "name": "status:in-progress"}])
     @patch("mcp_server.api_request")
     @patch("mcp_server.get_auth_header", return_value="token author-pass")
-    def test_resolver_allowed_mark_issue_proceeds(self, _auth, mock_api):
+    def test_resolver_allowed_mark_issue_proceeds(self, _auth, mock_api, _labels):
         def api_side_effect(method, url, auth, payload=None):
-            if method == "GET" and url.endswith("/labels?limit=100"):
-                return [{"id": 10, "name": "status:in-progress"}]
-            if method == "POST" and "/issues/9/labels" in url:
+            if method == "GET" and "/issues/9" in url:
+                return {"labels": []}
+            if method == "PUT" and "/issues/9/labels" in url:
                 return [{"name": "status:in-progress"}]
             if method == "POST" and "/issues/9/comments" in url:
                 return {"id": 501}
@@ -209,4 +210,4 @@ class TestCreateIssueMissingPermissionReport(IssueWriteGateBase):
 
 
 if __name__ == "__main__":
-    unittest.main()
+    unittest.main()

tests/test_manage_labels.py

@@ -11,6 +11,7 @@ from unittest.mock import MagicMock, call, patch
 
 sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.parent))
 import manage_labels  # noqa: E402
+import issue_workflow_labels  # noqa: E402
 
 
 FAKE_AUTH = "Basic dGVzdDp0ZXN0"  # base64("test:test")
@@ -129,6 +130,11 @@ class TestConstants(unittest.TestCase):
         names = [l["name"] for l in manage_labels.LABELS]
         self.assertIn("status:in-progress", names)
 
+    def test_canonical_workflow_labels_are_defined(self):
+        names = {l["name"] for l in manage_labels.LABELS}
+        for spec in issue_workflow_labels.CANONICAL_LABEL_SPECS:
+            self.assertIn(spec.name, names)
+
     def test_all_mapped_labels_are_defined(self):
         defined = {l["name"] for l in manage_labels.LABELS}
         for issue, names in manage_labels.MAPPING.items():

tests/test_mcp_server.py

@@ -91,6 +91,16 @@ def _visible_approval_reviews(reviewer="reviewer-bot", sha="abc123"):
 
 _DEFAULT_LEASE_SESSION = "mcp-test-reviewer-lease"
 _NO_PR_WORK_LEASE_BLOCK = {"block": False, "reasons": [], "mutation_allowed": True}
+WORKFLOW_REPO_LABELS = [
+    {"id": 1, "name": "type:feature"},
+    {"id": 2, "name": "status:in-progress"},
+    {"id": 3, "name": "status:ready"},
+    {"id": 4, "name": "status:pr-open"},
+]
+
+
+def _issue_with_labels(*names):
+    return {"labels": [{"name": name} for name in names]}
 
 
 def _reviewer_lease_comment(
@@ -305,6 +315,22 @@ class TestCreateIssue(unittest.TestCase):
         self.assertIn("gitea.prgs.cc", url)
         self.assertIn("Scaled-Tech-Consulting", url)
 
+    @patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
+           return_value=(True, []))
+    @patch("mcp_server.api_request")
+    @patch("mcp_server.api_get_all", return_value=[])
+    @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
+    def test_create_issue_required_workflow_labels_blocks(self, _auth, _get_all, mock_api, _role):
+        with patch.dict(os.environ, ISSUE_WRITE_ENV, clear=True):
+            result = gitea_create_issue(
+                title="Test issue",
+                require_workflow_labels=True,
+            )
+        self.assertFalse(result["success"])
+        self.assertFalse(result["performed"])
+        self.assertIn("issue is missing a type:* label", result["reasons"])
+        mock_api.assert_not_called()
+
 
 # ---------------------------------------------------------------------------
 # Create PR
@@ -315,13 +341,24 @@ class TestCreatePR(unittest.TestCase):
         "mcp_server.issue_duplicate_context_fetcher",
         return_value=([], [], {"status": "not_claimed"}),
     )
+    @patch("mcp_server.api_get_all", return_value=WORKFLOW_REPO_LABELS)
     @patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
            return_value=(True, []))
     @patch("mcp_server.api_request")
     @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
-    def test_creates_pr(self, _auth, mock_api, _role, _dup_fetcher):
+    def test_creates_pr(self, _auth, mock_api, _role, _labels, _dup_fetcher):
         worktree = os.path.realpath(os.getcwd())
-        mock_api.return_value = {"number": 3, "html_url": "https://example.com/pulls/3"}
+
+        def api_side_effect(method, url, auth, payload=None):
+            if method == "GET" and "/issues/123" in url:
+                return _issue_with_labels("type:feature", "status:in-progress")
+            if method == "POST" and url.endswith("/pulls"):
+                return {"number": 3, "html_url": "https://example.com/pulls/3"}
+            if method == "PUT" and url.endswith("/issues/123/labels"):
+                return [{"name": "type:feature"}, {"name": "status:pr-open"}]
+            return {}
+
+        mock_api.side_effect = api_side_effect
         with tempfile.TemporaryDirectory() as lock_dir:
             env = {**CREATE_PR_ENV, "GITEA_ISSUE_LOCK_DIR": lock_dir}
             with patch.dict(os.environ, env, clear=True):
@@ -334,22 +371,37 @@ class TestCreatePR(unittest.TestCase):
                 )
         self.assertEqual(result["number"], 3)
         self.assertNotIn("url", result)
-        payload = mock_api.call_args[0][3]
+        post_call = next(c for c in mock_api.call_args_list if c[0][0] == "POST")
+        payload = post_call[0][3]
         self.assertEqual(payload["head"], "feat/x")
         self.assertEqual(payload["base"], "main")
         self.assertIn("Closes #123", payload["title"])
+        put_call = next(c for c in mock_api.call_args_list if c[0][0] == "PUT")
+        self.assertEqual(put_call[0][3], {"labels": [1, 4]})
+        self.assertTrue(result["issue_status_transition"]["performed"])
 
     @patch(
         "mcp_server.issue_duplicate_context_fetcher",
         return_value=([], [], {"status": "not_claimed"}),
     )
+    @patch("mcp_server.api_get_all", return_value=WORKFLOW_REPO_LABELS)
     @patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
            return_value=(True, []))
     @patch("mcp_server.api_request")
     @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
-    def test_create_pr_reveal_opt_in_includes_url(self, _auth, mock_api, _role, _dup_fetcher):
+    def test_create_pr_reveal_opt_in_includes_url(self, _auth, mock_api, _role, _labels, _dup_fetcher):
         worktree = os.path.realpath(os.getcwd())
-        mock_api.return_value = {"number": 3, "html_url": "https://example.com/pulls/3"}
+
+        def api_side_effect(method, url, auth, payload=None):
+            if method == "GET" and "/issues/123" in url:
+                return _issue_with_labels("type:feature", "status:in-progress")
+            if method == "POST" and url.endswith("/pulls"):
+                return {"number": 3, "html_url": "https://example.com/pulls/3"}
+            if method == "PUT" and url.endswith("/issues/123/labels"):
+                return [{"name": "type:feature"}, {"name": "status:pr-open"}]
+            return {}
+
+        mock_api.side_effect = api_side_effect
         with tempfile.TemporaryDirectory() as lock_dir:
             env = {**CREATE_PR_ENV, "GITEA_ISSUE_LOCK_DIR": lock_dir, "GITEA_MCP_REVEAL_ENDPOINTS": "1"}
             with patch.dict(os.environ, env, clear=True):
@@ -362,6 +414,38 @@ class TestCreatePR(unittest.TestCase):
                 )
         self.assertIn("pulls/3", result["url"])
 
+    @patch(
+        "mcp_server.issue_duplicate_context_fetcher",
+        return_value=([], [], {"status": "not_claimed"}),
+    )
+    @patch("mcp_server.api_get_all", return_value=[{"id": 1, "name": "type:feature"}])
+    @patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
+           return_value=(True, []))
+    @patch("mcp_server.api_request")
+    @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
+    def test_create_pr_blocks_when_pr_open_status_label_missing(
+        self, _auth, mock_api, _role, _labels, _dup_fetcher
+    ):
+        worktree = os.path.realpath(os.getcwd())
+        mock_api.return_value = _issue_with_labels("type:feature", "status:in-progress")
+        with tempfile.TemporaryDirectory() as lock_dir:
+            env = {**CREATE_PR_ENV, "GITEA_ISSUE_LOCK_DIR": lock_dir}
+            with patch.dict(os.environ, env, clear=True):
+                _bind_test_lock(issue_number=123, branch_name="feat/x", worktree_path=worktree)
+                result = gitea_create_pr(
+                    title="feat: X Closes #123",
+                    head="feat/x",
+                    base="main",
+                    worktree_path=worktree,
+                )
+        self.assertFalse(result["success"])
+        self.assertFalse(result["performed"])
+        self.assertIn("status:pr-open", result["reasons"][0])
+        self.assertFalse(
+            any(c[0][0] == "POST" and c[0][1].endswith("/pulls")
+                for c in mock_api.call_args_list)
+        )
+
     @patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
            return_value=(True, []))
     @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
@@ -486,26 +570,33 @@ class TestViewIssue(unittest.TestCase):
 # ---------------------------------------------------------------------------
 class TestMarkIssue(unittest.TestCase):
 
+    @patch("mcp_server.api_get_all", return_value=WORKFLOW_REPO_LABELS)
     @patch("mcp_server.api_request")
     @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
-    def test_start_adds_label(self, _auth, mock_api):
-        # labels, add label, post claim heartbeat comment
-        mock_api.side_effect = [
-            [{"id": 10, "name": "status:in-progress"}],
-            [{"name": "status:in-progress"}],
-            {"id": 99},
-        ]
+    def test_start_adds_label(self, _auth, mock_api, _labels):
+        def api_side_effect(method, url, auth, payload=None):
+            if method == "GET" and "/issues/5" in url:
+                return _issue_with_labels("type:feature", "status:ready")
+            if method == "PUT" and url.endswith("/issues/5/labels"):
+                return [{"name": "type:feature"}, {"name": "status:in-progress"}]
+            if method == "POST" and url.endswith("/issues/5/comments"):
+                return {"id": 99}
+            return {}
+
+        mock_api.side_effect = api_side_effect
         with patch.dict(os.environ, ISSUE_WRITE_ENV, clear=True):
             result = gitea_mark_issue(issue_number=5, action="start")
         self.assertTrue(result["success"])
         self.assertIn("claimed", result["message"])
         self.assertTrue(result.get("heartbeat_posted"))
+        put_call = next(c for c in mock_api.call_args_list if c[0][0] == "PUT")
+        self.assertEqual(put_call[0][3], {"labels": [1, 2]})
 
+    @patch("mcp_server.api_get_all", return_value=WORKFLOW_REPO_LABELS)
     @patch("mcp_server.api_request")
     @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
-    def test_done_removes_label(self, _auth, mock_api):
+    def test_done_removes_label(self, _auth, mock_api, _labels):
         mock_api.side_effect = [
-            [{"id": 10, "name": "status:in-progress"}],
             None,
         ]
         with patch.dict(os.environ, ISSUE_WRITE_ENV, clear=True):
@@ -517,10 +608,10 @@ class TestMarkIssue(unittest.TestCase):
         with self.assertRaises(ValueError):
             gitea_mark_issue(issue_number=5, action="pause")
 
+    @patch("mcp_server.api_get_all", return_value=[])
     @patch("mcp_server.api_request")
     @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
-    def test_missing_label_raises(self, _auth, mock_api):
-        mock_api.return_value = []  # no labels exist
+    def test_missing_label_raises(self, _auth, mock_api, _labels):
         with patch.dict(os.environ, ISSUE_WRITE_ENV, clear=True):
             with self.assertRaises(RuntimeError):
                 gitea_mark_issue(issue_number=5, action="start")
@@ -3765,13 +3856,24 @@ class TestIssueLocking(unittest.TestCase):
                     )
         self.assertIn("lock provenance", str(ctx.exception).lower())
 
+    @patch("mcp_server.api_get_all", return_value=WORKFLOW_REPO_LABELS)
     @patch("mcp_server.api_request")
     @patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
            return_value=(True, []))
     @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
-    def test_create_pr_honors_scratch_worktree_lock(self, _auth, _role, mock_api):
+    def test_create_pr_honors_scratch_worktree_lock(self, _auth, _role, mock_api, _labels):
         scratch = os.path.realpath("/tmp/gitea-tools-author-scratch/issue-249-e2e")
-        mock_api.return_value = {"number": 250, "html_url": "https://example/pr/250"}
+
+        def api_side_effect(method, url, auth, payload=None):
+            if method == "GET" and "/issues/249" in url:
+                return _issue_with_labels("type:feature", "status:in-progress")
+            if method == "POST" and url.endswith("/pulls"):
+                return {"number": 250, "html_url": "https://example/pr/250"}
+            if method == "PUT" and url.endswith("/issues/249/labels"):
+                return [{"name": "type:feature"}, {"name": "status:pr-open"}]
+            return {}
+
+        mock_api.side_effect = api_side_effect
         _bind_test_lock(
             issue_number=249,
             branch_name="feat/issue-249-issue-lock-scratch-worktree",
@@ -3786,6 +3888,7 @@ class TestIssueLocking(unittest.TestCase):
                 worktree_path=scratch,
             )
         self.assertEqual(res["number"], 250)
+        self.assertTrue(res["issue_status_transition"]["performed"])
 
 
 # ---------------------------------------------------------------------------