Add GitHub PR comment actions and handlers to hackbot

docs/hackbot/actions.md

@@ -35,20 +35,22 @@ record in its `config.py` and passes them to `actions_server_for`, which builds
 carrying only those. `bug-fix`, for instance, allows `phabricator.submit_patch` on a fresh
 triage run but swaps it for `phabricator.update_patch` on a follow-up.
 
-| Action type                 | Records the intent to…                     | Params                                                          |
-| --------------------------- | ------------------------------------------ | --------------------------------------------------------------- |
-| `bugzilla.update_bug`       | Change a bug's fields                      | `bug_id`, `changes`                                             |
-| `bugzilla.add_comment`      | Comment on a bug                           | `bug_id`, `text`, `is_private`                                  |
-| `bugzilla.add_attachment`   | Attach a file to a bug                     | `bug_id`, + a `file` attachment                                 |
-| `bugzilla.create_bug`       | File a new bug                             | the new bug's fields, `ref`                                     |
-| `phabricator.submit_patch`  | Deliver a fix as a **new** revision        | `bug_id`, `title`, `summary`, `test_plan`, `parent_revision_id` |
-| `phabricator.update_patch`  | Add a new diff to an **existing** revision | `revision_id`, `title`, `summary`, `test_plan`                  |
-| `phabricator.add_comment`   | Reply on a revision without changing code  | `revision_id`, `text`                                           |
-| `testrail.submit_test_plan` | Submit a generated test plan to TestRail   | the validated feature + test cases                              |
-| `slack.post_message`        | Post a message to Slack                    | `channel`, `text`                                               |
-| `email.send`                | Email a report about the run               | `to`, `subject`, `body_markdown`                                |
-| `feedback.report_problem`   | Report a problem to the Hackbot developers | `severity`, `title`, `message`                                  |
-| `agents.start_run`          | Start another agent on this run's output   | `agent`                                                         |
+| Action type                  | Records the intent to…                     | Params                                                          |
+| ---------------------------- | ------------------------------------------ | --------------------------------------------------------------- |
+| `bugzilla.update_bug`        | Change a bug's fields                      | `bug_id`, `changes`                                             |
+| `bugzilla.add_comment`       | Comment on a bug                           | `bug_id`, `text`, `is_private`                                  |
+| `bugzilla.add_attachment`    | Attach a file to a bug                     | `bug_id`, + a `file` attachment                                 |
+| `bugzilla.create_bug`        | File a new bug                             | the new bug's fields, `ref`                                     |
+| `phabricator.submit_patch`   | Deliver a fix as a **new** revision        | `bug_id`, `title`, `summary`, `test_plan`, `parent_revision_id` |
+| `phabricator.update_patch`   | Add a new diff to an **existing** revision | `revision_id`, `title`, `summary`, `test_plan`                  |
+| `phabricator.add_comment`    | Reply on a revision without changing code  | `revision_id`, `text`                                           |
+| `github.add_comment`         | Comment on a pull request conversation     | `repo`, `pull_number`, `text`                                   |
+| `github.add_inline_comments` | Review a pull request with inline comments | `repo`, `pull_number`, `comments`, `summary`                    |
+| `testrail.submit_test_plan`  | Submit a generated test plan to TestRail   | the validated feature + test cases                              |
+| `slack.post_message`         | Post a message to Slack                    | `channel`, `text`                                               |
+| `email.send`                 | Email a report about the run               | `to`, `subject`, `body_markdown`                                |
+| `feedback.report_problem`    | Report a problem to the Hackbot developers | `severity`, `title`, `message`                                  |
+| `agents.start_run`           | Start another agent on this run's output   | `agent`                                                         |
 
 All but `testrail.submit_test_plan` take a **`reasoning`** argument — a free-text audit trail
 stored on the action and shown in the UI beside the proposed change. Each tool returns the
@@ -165,6 +167,18 @@ applied at its last member's index, once every earlier dependency has resolved,
 group with a member another pending action references is excluded (that action could run
 before the group is applied).
 
+## GitHub review grouping
+
+When general and inline comments for the same GitHub PR are pending in the same
+apply pass, they are submitted together as one review. General comment text and
+existing review summaries are joined with blank lines in action order. General
+comments without inline comments still post to the PR conversation.
+
+Only actions selected for that apply pass are grouped. Already-applied actions
+are skipped, and groups containing an action referenced by another pending
+action are excluded. Each grouped action retains its own status; general
+comments return the review URL as their `comment_url`.
+
 ## Where to look
 
 | Concern                          | File                                                                                                                       |

libs/hackbot-runtime/hackbot_runtime/actions/__init__.py

@@ -13,6 +13,7 @@
     bugzilla,
     email,
     feedback,
+    github,
     phabricator,
     slack,
     testrail,
@@ -31,6 +32,7 @@
     "bugzilla",
     "email",
     "feedback",
+    "github",
     "phabricator",
     "slack",
     "testrail",

libs/hackbot-runtime/hackbot_runtime/actions/claude_sdk.py

@@ -15,6 +15,7 @@
 from hackbot_runtime.actions import agents as _agents
 from hackbot_runtime.actions import bugzilla as _bugzilla
 from hackbot_runtime.actions import feedback as _feedback
+from hackbot_runtime.actions import github as _github
 from hackbot_runtime.actions import phabricator as _phabricator
 from hackbot_runtime.actions import slack as _slack
 from hackbot_runtime.actions import testrail as _testrail
@@ -50,6 +51,7 @@ def actions_server_for(
         + _agents.TOOLS
         + _bugzilla.TOOLS
         + _phabricator.TOOLS
+        + _github.TOOLS
         + _testrail.TOOLS
         + _slack.TOOLS
         + _try_server.TOOLS

libs/hackbot-runtime/hackbot_runtime/actions/github.py

@@ -0,0 +1,153 @@
+"""GitHub-domain recordable actions.
+
+Mirrors ``actions/phabricator.py``'s shape: the tools record an intended comment
+(nothing is posted to GitHub here) and return a short confirmation string. See
+hackbot-api's ``app/action_handlers/github_handler.py`` for the apply side.
+"""
+
+from typing import Annotated, Any
+
+from agent_tools.registry import ToolError, tool, tools_in
+from pydantic import (
+    BaseModel,
+    Field,
+    StringConstraints,
+    ValidationError,
+    model_validator,
+)
+
+from hackbot_runtime.actions.recorder import ActionsRecorder, confirmation
+
+ADD_COMMENT_ACTION_TYPE = "github.add_comment"
+ADD_INLINE_COMMENTS_ACTION_TYPE = "github.add_inline_comments"
+
+_Text = Annotated[str, StringConstraints(strip_whitespace=True, min_length=1)]
+
+
+class InlineComment(BaseModel):
+    path: Annotated[
+        _Text, Field(description="Path of the file in the repository, e.g. src/a.py.")
+    ]
+    line: Annotated[
+        int,
+        Field(
+            ge=1,
+            description=(
+                "Line in the new version of the file. It must appear in the pull "
+                "request's diff, as a changed line or the context around one."
+            ),
+        ),
+    ]
+    body: Annotated[_Text, Field(description="Comment body (Markdown supported).")]
+    start_line: Annotated[
+        int | None,
+        Field(
+            ge=1,
+            description="First line of a multi-line comment ending at `line`.",
+        ),
+    ] = None
+
+    @model_validator(mode="after")
+    def _start_line_before_line(self) -> InlineComment:
+        if self.start_line is not None and self.start_line >= self.line:
+            raise ValueError("start_line must be before line")
+        return self
+
+
+# The tool adapters pass the agent's arguments through unvalidated, so these
+# models are the enforcement; the tool signatures below are the agent's schema.
+class _PullRequest(BaseModel):
+    # GitHub's own rules for owner and repository names.
+    repo: Annotated[str, Field(pattern=r"^[A-Za-z0-9-]+/[A-Za-z0-9._-]+$")]
+    pull_number: Annotated[int, Field(ge=1)]
+
+
+class _Comment(_PullRequest):
+    text: _Text
+
+
+class _InlineComments(_PullRequest):
+    comments: Annotated[list[InlineComment], Field(min_length=1)]
+    summary: _Text | None = None
+
+
+def _validated(model: type[BaseModel], **values: Any) -> dict[str, Any]:
+    try:
+        validated = model.model_validate(values)
+    except ValidationError as exc:
+        raise ToolError(
+            "invalid GitHub action",
+            payload={
+                "error": "invalid GitHub action",
+                "details": exc.errors(include_url=False),
+            },
+        ) from exc
+    return validated.model_dump(mode="json", exclude_none=True)
+
+
+_Repo = Annotated[
+    str,
+    Field(description='The repository, as "owner/name" (e.g. mozilla/bugbug).'),
+]
+_PullNumber = Annotated[int, Field(description="The pull request number.")]
+_Reasoning = Annotated[
+    str, Field(description="Why you are recording this comment (for audit log).")
+]
+
+
+@tool
+async def add_comment(
+    recorder: ActionsRecorder,
+    repo: _Repo,
+    pull_number: _PullNumber,
+    text: Annotated[str, Field(description="Comment body (Markdown supported).")],
+    reasoning: _Reasoning,
+) -> str:
+    """Record an intended comment on a GitHub pull request's conversation.
+
+    Use this to reply on a pull request, for example to answer a question, or for
+    feedback that is not about particular lines; for those, use
+    ``add_inline_comments``. Recorded into the run summary for human review;
+    nothing is posted to GitHub during the run.
+    """
+    params = _validated(_Comment, repo=repo, pull_number=pull_number, text=text)
+    action = recorder.record(ADD_COMMENT_ACTION_TYPE, params, reasoning=reasoning)
+    return confirmation(action)
+
+
+@tool
+async def add_inline_comments(
+    recorder: ActionsRecorder,
+    repo: _Repo,
+    pull_number: _PullNumber,
+    comments: Annotated[
+        list[InlineComment],
+        Field(min_length=1, description="The line comments, at least one."),
+    ],
+    reasoning: _Reasoning,
+    summary: Annotated[
+        str | None,
+        Field(description="Optional overall comment shown above the line comments."),
+    ] = None,
+) -> str:
+    """Record an intended review of a GitHub pull request: comments on its lines.
+
+    The comments are posted together as one review, so pass every line comment
+    for the pull request in a single call; each call becomes a separate review.
+    Recorded into the run summary for human review; nothing is posted to GitHub
+    during the run.
+    """
+    params = _validated(
+        _InlineComments,
+        repo=repo,
+        pull_number=pull_number,
+        comments=comments,
+        summary=summary,
+    )
+    action = recorder.record(
+        ADD_INLINE_COMMENTS_ACTION_TYPE, params, reasoning=reasoning
+    )
+    return confirmation(action)
+
+
+TOOLS = tools_in(__name__)

libs/hackbot-runtime/tests/test_github_actions.py

@@ -0,0 +1,124 @@
+"""Tests for the GitHub recording tools (PR comments and inline review comments)."""
+
+import pytest
+from agent_tools.registry import ToolError
+from hackbot_runtime.actions import ActionsRecorder, github
+
+REPO = "mozilla-mobile/firefox-ios"
+
+
+async def test_add_comment_records_the_pull_request_and_text():
+    rec = ActionsRecorder()
+    await github.add_comment(
+        rec, repo=REPO, pull_number=7, text="  Here is the answer. ", reasoning="r"
+    )
+    [action] = rec.actions
+    assert action["type"] == "github.add_comment"
+    assert action["params"] == {
+        "repo": REPO,
+        "pull_number": 7,
+        "text": "Here is the answer.",
+    }
+    assert action["reasoning"] == "r"
+
+
+async def test_add_inline_comments_records_one_action_for_all_comments():
+    rec = ActionsRecorder()
+    await github.add_inline_comments(
+        rec,
+        repo=REPO,
+        pull_number=7,
+        comments=[
+            {"path": "a.py", "line": 3, "body": "Off by one?"},
+            {"path": "b.py", "line": 12, "start_line": 10, "body": "Extract this."},
+        ],
+        summary="Two things.",
+        reasoning="r",
+    )
+    [action] = rec.actions
+    assert action["type"] == "github.add_inline_comments"
+    assert action["params"] == {
+        "repo": REPO,
+        "pull_number": 7,
+        "comments": [
+            {"path": "a.py", "line": 3, "body": "Off by one?"},
+            {"path": "b.py", "line": 12, "start_line": 10, "body": "Extract this."},
+        ],
+        "summary": "Two things.",
+    }
+
+
+async def test_add_inline_comments_summary_is_optional():
+    rec = ActionsRecorder()
+    await github.add_inline_comments(
+        rec,
+        repo=REPO,
+        pull_number=7,
+        comments=[{"path": "a.py", "line": 3, "body": "Nit."}],
+        reasoning="r",
+    )
+    assert "summary" not in rec.actions[0]["params"]
+
+
+@pytest.mark.parametrize(
+    ("repo", "pull_number"),
+    [("firefox-ios", 7), ("mozilla-mobile/firefox-ios/x", 7), (REPO, 0)],
+    ids=["no-owner", "extra-segment", "pull-number"],
+)
+async def test_a_malformed_pull_request_is_refused(repo, pull_number):
+    rec = ActionsRecorder()
+    with pytest.raises(ToolError):
+        await github.add_comment(
+            rec, repo=repo, pull_number=pull_number, text="x", reasoning="r"
+        )
+    with pytest.raises(ToolError):
+        await github.add_inline_comments(
+            rec,
+            repo=repo,
+            pull_number=pull_number,
+            comments=[{"path": "a.py", "line": 1, "body": "x"}],
+            reasoning="r",
+        )
+    assert rec.actions == []
+
+
+async def test_a_blank_comment_is_refused():
+    rec = ActionsRecorder()
+    with pytest.raises(ToolError):
+        await github.add_comment(rec, repo=REPO, pull_number=7, text=" ", reasoning="r")
+    assert rec.actions == []
+
+
+@pytest.mark.parametrize(
+    "comments",
+    [
+        [],
+        [{"path": "a.py", "line": 0, "body": "x"}],
+        [{"path": "a.py", "line": 3, "body": " "}],
+        [{"path": "", "line": 3, "body": "x"}],
+        [{"path": "a.py", "line": 3, "start_line": 3, "body": "x"}],
+    ],
+    ids=["none", "line-zero", "blank-body", "blank-path", "start-not-before-end"],
+)
+async def test_invalid_inline_comments_are_refused(comments):
+    rec = ActionsRecorder()
+    with pytest.raises(ToolError) as exc:
+        await github.add_inline_comments(
+            rec, repo=REPO, pull_number=7, comments=comments, reasoning="r"
+        )
+    assert exc.value.payload["details"]
+    assert rec.actions == []
+
+
+def test_tool_schemas():
+    tools = {t.name: t for t in github.TOOLS}
+    assert set(tools) == {"add_comment", "add_inline_comments"}
+    assert set(tools["add_comment"].input_schema["required"]) == {
+        "repo",
+        "pull_number",
+        "text",
+        "reasoning",
+    }
+    inline = tools["add_inline_comments"].input_schema
+    assert set(inline["required"]) == {"repo", "pull_number", "comments", "reasoning"}
+    assert "summary" in inline["properties"]