fix: revert review_pr.py to GiteaClient (tea v0.14.1 is interactive-only) (#70)

AGENTS.md

@@ -291,12 +291,12 @@ The `tea` Gitea CLI tool is used for Gitea API interactions in CI scripts. It is
 - `TeaCLI.list_branches()` — Branch listing
 
 **Scripts using tea (via `gitea_cli.py`):**
-- `scripts/ci/review_pr.py` — Posts PR reviews via `tea pulls review`
 - `scripts/ci/publish.py` — Creates Gitea releases via `tea releases create`
 - `scripts/ci/notify_failure.py` — Creates issues via `tea issues create` (falls back to `GiteaClient` if tea not installed)
 - `scripts/configure_repo.py` — Creates labels via `tea labels create` (falls back to `GiteaClient` if tea fails; branch protection still uses `GiteaClient` since tea only supports basic protect/unprotect)
 
 **Operations still using `GiteaClient` (not supported by tea):**
+- PR reviews (`review_pr.py`) — tea v0.14.1 only supports interactive reviews
 - Wiki page management (`sync_wiki.py`)
 - Commit status checks (`auto_merge.py`)
 - Runner discovery (`discover_runners.py`)
@@ -309,8 +309,8 @@ Scripts have different import requirements. Workflows must set `PYTHONPATH` acco
 
 | PYTHONPATH | When to use | Example scripts |
 |------------|-------------|-----------------|
-| `src` | Script imports from `gitea_runner_manager` | `auto_merge.py`, `pr_review.py`, `sync_wiki.py`, `post_merge.py`, `classify_changes.py`, `discover_runners.py`, `doc_coverage.py` |
+| `src` | Script imports from `gitea_runner_manager` | `auto_merge.py`, `pr_review.py`, `review_pr.py`, `sync_wiki.py`, `post_merge.py`, `classify_changes.py`, `discover_runners.py`, `doc_coverage.py` |
-| `.:src` | Script imports from both `gitea_runner_manager` and `scripts.gitea_cli` | `review_pr.py`, `publish.py`, `notify_failure.py`, `configure_repo.py` |
+| `.:src` | Script imports from both `gitea_runner_manager` and `scripts.gitea_cli` | `publish.py`, `notify_failure.py`, `configure_repo.py` |
 | `.` | Script imports from other `scripts.ci.*` modules | `release.py` (imports `classify_changes.has_user_facing_changes`) |
 | (none) | Script has no GRM or cross-script imports | `detect_release_commit.py`, `distribute_molecule.py`, `molecule_ci_guard.py`, `push_badges.py`, `validate_commit_msg.py` |
 

scripts/ci/review_pr.py

@@ -3,9 +3,14 @@
 
 Used by the GRM workflow to post structured PR reviews.  The review body
 is provided via --body and inline comments via a JSON file
-(--comments-json) or stdin (--comments-stdin).  This script uses the
+(--comments-json) or stdin (--comments-stdin).
-``tea`` Gitea CLI for posting the review — the actual review analysis is
+
-performed by the agent before invoking this tool.
+.. note::
+   The ``tea`` CLI v0.14.1 only supports interactive reviews (no
+   ``--approve``/``--comment`` flags), so this script uses
+   ``GiteaClient`` (direct HTTP API) for posting reviews. When a newer
+   version of tea adds non-interactive review support, this can be
+   switched to use ``TeaCLI.review_pr()``.
 
 Usage:
   REPO_TOKEN=<token> python3 scripts/review_pr.py <pr_number> <repo> \
@@ -36,8 +41,10 @@ from typing import Any
 import click
 from dotenv import load_dotenv  # pyright: ignore[reportMissingImports,reportUnknownVariableType]
 
+from gitea_runner_manager.api_clients import GiteaClient
+from gitea_runner_manager.config import GITEA_API_URL
+from gitea_runner_manager.exceptions import APIError
 from gitea_runner_manager.i18n import _
-from scripts.gitea_cli import TeaCLI, TeaCLIError
 
 load_dotenv(override=True)
 
@@ -105,7 +112,8 @@ def main(
     if not token:
         raise click.ClickException(_("ERROR: REPO_TOKEN is not set."))
 
-    tea = TeaCLI(repo=repo)
+    owner, repo_name = repo.split("/")
+    client = GiteaClient(GITEA_API_URL, token, owner, repo_name)
 
     comments = parse_comments(comments_json, comments_stdin)
 
@@ -129,13 +137,21 @@ def main(
             )
 
     try:
-        tea.review_pr(repo, int(pr_number), event=event, body=body)
+        review = client.create_review(pr_number, event=event, body=body, comments=comments)
-    except TeaCLIError as e:
+    except APIError as e:
-        raise click.ClickException(_("Failed to post review: {error}", error=str(e))) from None
+        raise click.ClickException(
+            _(
+                "Failed to post review: HTTP {status} — {message}",
+                status=e.status,
+                message=e.message,
+            )
+        ) from None
 
+    review_id = review.get("id", "?")
     click.echo(
         _(
-            "Review posted on PR #{pr_number} with event '{event}' ({num_comments} inline comments).",
+            "Review #{review_id} posted on PR #{pr_number} with event '{event}' ({num_comments} inline comments).",
+            review_id=review_id,
             pr_number=pr_number,
             event=event,
             num_comments=len(comments),

tests/unit/test_review_pr.py

@@ -1,5 +1,6 @@
 """Unit tests for scripts/ci/review_pr.py."""
 
+import http
 import json
 from unittest.mock import MagicMock, patch
 
@@ -7,8 +8,8 @@ import click
 import pytest
 from click.testing import CliRunner
 
+from gitea_runner_manager.exceptions import APIError
 from scripts.ci.review_pr import main, parse_comments
-from scripts.gitea_cli import TeaCLIError
 
 
 class TestParseComments:
@@ -59,25 +60,27 @@ class TestParseComments:
 
 class TestMain:
     @patch.dict("os.environ", {"REPO_TOKEN": "tok"})
-    @patch("scripts.ci.review_pr.TeaCLI")
+    @patch("scripts.ci.review_pr.GiteaClient")
-    def test_successful_comment_review(self, mock_tea_cls: MagicMock) -> None:
+    def test_successful_comment_review(self, mock_client_cls: MagicMock) -> None:
-        mock_tea = MagicMock()
+        mock_client = MagicMock()
-        mock_tea_cls.return_value = mock_tea
+        mock_client.create_review.return_value = {"id": 42}
+        mock_client_cls.return_value = mock_client
         runner = CliRunner()
         result = runner.invoke(
             main,
             ["5", "owner/repo", "--event", "COMMENT", "--body", "LGTM"],
         )
         assert result.exit_code == 0
-        assert "Review posted" in result.output
+        assert "Review #42" in result.output
-        mock_tea.review_pr.assert_called_once_with("owner/repo", 5, event="COMMENT", body="LGTM")
+        mock_client.create_review.assert_called_once_with("5", event="COMMENT", body="LGTM", comments=[])
 
     @patch.dict("os.environ", {"REPO_TOKEN": "tok"})
-    @patch("scripts.ci.review_pr.TeaCLI")
+    @patch("scripts.ci.review_pr.GiteaClient")
-    def test_successful_approve_review(self, mock_tea_cls: MagicMock) -> None:
+    def test_successful_approve_review(self, mock_client_cls: MagicMock) -> None:
         """APPROVE requires --checklist-confirmed and substantive body."""
-        mock_tea = MagicMock()
+        mock_client = MagicMock()
-        mock_tea_cls.return_value = mock_tea
+        mock_client.create_review.return_value = {"id": 7}
+        mock_client_cls.return_value = mock_client
         runner = CliRunner()
         result = runner.invoke(
             main,
@@ -92,19 +95,20 @@ class TestMain:
             ],
         )
         assert result.exit_code == 0
-        mock_tea.review_pr.assert_called_once_with(
+        assert "Review #7" in result.output
-            "owner/repo",
+        mock_client.create_review.assert_called_once_with(
-            5,
+            "5",
             event="APPROVE",
             body="All 10 checklist categories verified. Architecture OK, tests pass.",
+            comments=[],
         )
 
     @patch.dict("os.environ", {"REPO_TOKEN": "tok"})
-    @patch("scripts.ci.review_pr.TeaCLI")
+    @patch("scripts.ci.review_pr.GiteaClient")
-    def test_approve_without_checklist_confirmed_fails(self, mock_tea_cls: MagicMock) -> None:
+    def test_approve_without_checklist_confirmed_fails(self, mock_client_cls: MagicMock) -> None:
         """APPROVE without --checklist-confirmed is rejected."""
-        mock_tea = MagicMock()
+        mock_client = MagicMock()
-        mock_tea_cls.return_value = mock_tea
+        mock_client_cls.return_value = mock_client
         runner = CliRunner()
         result = runner.invoke(
             main,
@@ -112,14 +116,14 @@ class TestMain:
         )
         assert result.exit_code != 0
         assert "checklist" in result.output.lower()
-        mock_tea.review_pr.assert_not_called()
+        mock_client.create_review.assert_not_called()
 
     @patch.dict("os.environ", {"REPO_TOKEN": "tok"})
-    @patch("scripts.ci.review_pr.TeaCLI")
+    @patch("scripts.ci.review_pr.GiteaClient")
-    def test_approve_with_trivial_body_fails(self, mock_tea_cls: MagicMock) -> None:
+    def test_approve_with_trivial_body_fails(self, mock_client_cls: MagicMock) -> None:
         """APPROVE with trivial body (< 20 chars) and no comments is rejected."""
-        mock_tea = MagicMock()
+        mock_client = MagicMock()
-        mock_tea_cls.return_value = mock_tea
+        mock_client_cls.return_value = mock_client
         runner = CliRunner()
         result = runner.invoke(
             main,
@@ -127,30 +131,32 @@ class TestMain:
         )
         assert result.exit_code != 0
         assert "substantive" in result.output.lower()
-        mock_tea.review_pr.assert_not_called()
+        mock_client.create_review.assert_not_called()
 
     @patch.dict("os.environ", {"REPO_TOKEN": "tok"})
-    @patch("scripts.ci.review_pr.TeaCLI")
+    @patch("scripts.ci.review_pr.GiteaClient")
-    def test_successful_with_inline_comments(self, mock_tea_cls: MagicMock, tmp_path) -> None:
+    def test_successful_with_inline_comments(self, mock_client_cls: MagicMock, tmp_path) -> None:
         comments = [{"path": "a.py", "body": "fix", "new_position": 1}]
         f = tmp_path / "comments.json"
         f.write_text(json.dumps(comments))
-        mock_tea = MagicMock()
+        mock_client = MagicMock()
-        mock_tea_cls.return_value = mock_tea
+        mock_client.create_review.return_value = {"id": 9}
+        mock_client_cls.return_value = mock_client
         runner = CliRunner()
         result = runner.invoke(
             main,
             ["5", "owner/repo", "--comments-json", str(f)],
         )
         assert result.exit_code == 0
-        mock_tea.review_pr.assert_called_once_with("owner/repo", 5, event="COMMENT", body="")
+        mock_client.create_review.assert_called_once_with("5", event="COMMENT", body="", comments=comments)
 
     @patch.dict("os.environ", {"REPO_TOKEN": "tok"})
-    @patch("scripts.ci.review_pr.TeaCLI")
+    @patch("scripts.ci.review_pr.GiteaClient")
-    def test_successful_with_stdin_comments(self, mock_tea_cls: MagicMock) -> None:
+    def test_successful_with_stdin_comments(self, mock_client_cls: MagicMock) -> None:
         comments = [{"path": "a.py", "body": "fix", "new_position": 1}]
-        mock_tea = MagicMock()
+        mock_client = MagicMock()
-        mock_tea_cls.return_value = mock_tea
+        mock_client.create_review.return_value = {"id": 11}
+        mock_client_cls.return_value = mock_client
         runner = CliRunner()
         result = runner.invoke(
             main,
@@ -158,7 +164,7 @@ class TestMain:
             input=json.dumps(comments),
         )
         assert result.exit_code == 0
-        mock_tea.review_pr.assert_called_once_with("owner/repo", 5, event="COMMENT", body="")
+        mock_client.create_review.assert_called_once_with("5", event="COMMENT", body="", comments=comments)
 
     @patch.dict("os.environ", {"REPO_TOKEN": ""}, clear=True)
     def test_missing_token_exits(self) -> None:
@@ -168,44 +174,44 @@ class TestMain:
         assert "REPO_TOKEN" in result.output
 
     @patch.dict("os.environ", {"REPO_TOKEN": "tok"})
-    @patch("scripts.ci.review_pr.TeaCLI")
+    @patch("scripts.ci.review_pr.GiteaClient")
-    def test_no_body_or_comments_for_comment_event(self, mock_tea_cls: MagicMock) -> None:
+    def test_no_body_or_comments_for_comment_event(self, mock_client_cls: MagicMock) -> None:
-        mock_tea = MagicMock()
+        mock_client = MagicMock()
-        mock_tea_cls.return_value = mock_tea
+        mock_client_cls.return_value = mock_client
         runner = CliRunner()
         result = runner.invoke(main, ["5", "owner/repo", "--event", "COMMENT"])
         assert result.exit_code == 1
         assert "required" in result.output
-        mock_tea.review_pr.assert_not_called()
+        mock_client.create_review.assert_not_called()
 
     @patch.dict("os.environ", {"REPO_TOKEN": "tok"})
-    @patch("scripts.ci.review_pr.TeaCLI")
+    @patch("scripts.ci.review_pr.GiteaClient")
-    def test_no_body_or_comments_for_request_changes(self, mock_tea_cls: MagicMock) -> None:
+    def test_no_body_or_comments_for_request_changes(self, mock_client_cls: MagicMock) -> None:
-        mock_tea = MagicMock()
+        mock_client = MagicMock()
-        mock_tea_cls.return_value = mock_tea
+        mock_client_cls.return_value = mock_client
         runner = CliRunner()
         result = runner.invoke(main, ["5", "owner/repo", "--event", "REQUEST_CHANGES"])
         assert result.exit_code == 1
         assert "required" in result.output
-        mock_tea.review_pr.assert_not_called()
+        mock_client.create_review.assert_not_called()
 
     @patch.dict("os.environ", {"REPO_TOKEN": "tok"})
-    @patch("scripts.ci.review_pr.TeaCLI")
+    @patch("scripts.ci.review_pr.GiteaClient")
-    def test_tea_error_raises_click(self, mock_tea_cls: MagicMock) -> None:
+    def test_api_error_raises_click(self, mock_client_cls: MagicMock) -> None:
-        mock_tea = MagicMock()
+        mock_client = MagicMock()
-        mock_tea.review_pr.side_effect = TeaCLIError("tea command failed")
+        mock_client.create_review.side_effect = APIError(http.HTTPStatus.INTERNAL_SERVER_ERROR, "server error")
-        mock_tea_cls.return_value = mock_tea
+        mock_client_cls.return_value = mock_client
         runner = CliRunner()
         result = runner.invoke(main, ["5", "owner/repo", "--body", "x"])
         assert result.exit_code == 1
-        assert "Failed to post review" in result.output
+        assert "HTTP" in result.output
 
     @patch.dict("os.environ", {"REPO_TOKEN": "tok"})
-    @patch("scripts.ci.review_pr.TeaCLI")
+    @patch("scripts.ci.review_pr.GiteaClient")
-    def test_invalid_event_choice(self, mock_tea_cls: MagicMock) -> None:
+    def test_invalid_event_choice(self, mock_client_cls: MagicMock) -> None:
-        mock_tea = MagicMock()
+        mock_client = MagicMock()
-        mock_tea_cls.return_value = mock_tea
+        mock_client_cls.return_value = mock_client
         runner = CliRunner()
         result = runner.invoke(main, ["5", "owner/repo", "--event", "Bogus"])
         assert result.exit_code != 0
-        mock_tea.review_pr.assert_not_called()
+        mock_client.create_review.assert_not_called()