diff --git a/services/hackbot-api/app/phabricator_webhook.py b/services/hackbot-api/app/phabricator_webhook.py
index b17b436e57..02979a604b 100644
--- a/services/hackbot-api/app/phabricator_webhook.py
+++ b/services/hackbot-api/app/phabricator_webhook.py
@@ -10,7 +10,8 @@
import logging
from dataclasses import dataclass
-from typing import TYPE_CHECKING
+from typing import TYPE_CHECKING, Literal
+from xml.sax.saxutils import escape
if TYPE_CHECKING:
from phabricator_client import PhabricatorClient
@@ -28,6 +29,8 @@
class HackbotMention:
comment: str
author_phid: str
+ comment_id: int
+ comment_type: Literal["regular", "inline"]
def triggering_transaction_phids(payload: dict) -> list[str]:
@@ -60,33 +63,45 @@ def find_hackbot_mentions(
continue
if transaction.get("type") not in _COMMENT_TYPES:
continue
+
author_phid = transaction.get("authorPHID")
- if not author_phid:
- continue
- if bot_phid and author_phid == bot_phid:
+ if not author_phid or (bot_phid and author_phid == bot_phid):
continue
+
for comment in transaction.get("comments") or []:
- comment_text = (comment.get("content") or {}).get("raw") or ""
- if token in comment_text:
- matches.append(
- HackbotMention(
- comment=comment_text,
- author_phid=author_phid,
- )
+ comment_text = comment["content"]["raw"]
+ if token not in comment_text:
+ continue
+
+ matches.append(
+ HackbotMention(
+ comment=comment_text,
+ author_phid=author_phid,
+ comment_id=comment["id"],
+ comment_type=(
+ "inline" if transaction["type"] == "inline" else "regular"
+ ),
)
- break
+ )
+ break
return matches
-def _join_comments(comments: list[str]) -> str:
- """Combine one or more triggering comments into the agent's ``comment`` input.
+def _format_comment(mention: HackbotMention) -> str:
+ """Render one triggering comment as a service-generated XML element.
+
+ For example, an inline comment is rendered as::
- A lone comment is passed through unchanged; multiple are numbered so the
- agent can tell them apart and address each.
+
+ @hackbot fix this
+
"""
- if len(comments) == 1:
- return comments[0]
- return "\n\n".join(f"[comment {i}]\n{c}" for i, c in enumerate(comments, 1))
+ attributes = [
+ f'comment_id="{mention.comment_id}"',
+ f'type="{mention.comment_type}"',
+ ]
+ body = "\n".join(f" {line}" for line in escape(mention.comment).splitlines())
+ return f" \n{body}\n "
async def resolve_revision(
@@ -135,10 +150,10 @@ async def detect_mention_and_revision(
bot_phid=webhook.bot_phid,
token=webhook.mention_token,
)
- comments: list[str] = []
+ authorized_mentions: list[HackbotMention] = []
for mention in mentions:
if await authorizer.is_authorized(mention.author_phid):
- comments.append(mention.comment)
+ authorized_mentions.append(mention)
else:
log.warning(
"Ignoring %s mention from non-editbugs user %s on %s",
@@ -146,7 +161,7 @@ async def detect_mention_and_revision(
mention.author_phid,
object_phid,
)
- if not comments:
+ if not authorized_mentions:
log.warning(
"No actionable %s mention found in triggering transactions %s on %s",
webhook.mention_token,
@@ -154,7 +169,7 @@ async def detect_mention_and_revision(
object_phid,
)
return None
- comment = _join_comments(comments)
+ comment = "\n\n".join(_format_comment(mention) for mention in authorized_mentions)
revision_id, bug_id = await resolve_revision(client, object_phid)
if revision_id is None:
diff --git a/services/hackbot-api/tests/test_webhooks.py b/services/hackbot-api/tests/test_webhooks.py
index bc408f0a78..90074f3b84 100644
--- a/services/hackbot-api/tests/test_webhooks.py
+++ b/services/hackbot-api/tests/test_webhooks.py
@@ -20,7 +20,7 @@
)
from app.phabricator_webhook import (
HackbotMention,
- _join_comments,
+ _format_comment,
detect_mention_and_revision,
find_hackbot_mentions,
resolve_revision,
@@ -63,12 +63,19 @@ def test_signature_unconfigured_secret(monkeypatch):
# --- mention detection / loop prevention ---
-def _comment_txn(phid: str, author: str, raw: str, txn_type: str = "comment") -> dict:
+def _comment_txn(
+ phid: str,
+ author: str,
+ raw: str,
+ txn_type: str = "comment",
+ *,
+ comment_id: int = 1,
+) -> dict:
return {
"phid": phid,
"type": txn_type,
"authorPHID": author,
- "comments": [{"content": {"raw": raw}}],
+ "comments": [{"id": comment_id, "content": {"raw": raw}}],
}
@@ -76,7 +83,7 @@ def test_find_mention_matches():
txns = [_comment_txn("PHID-XACT-1", "PHID-USER-a", "hey @hackbot please fix")]
assert find_hackbot_mentions(
txns, {"PHID-XACT-1"}, bot_phid="PHID-USER-bot", token="@hackbot"
- ) == [HackbotMention("hey @hackbot please fix", "PHID-USER-a")]
+ ) == [HackbotMention("hey @hackbot please fix", "PHID-USER-a", 1, "regular")]
def test_find_mention_no_token():
@@ -122,20 +129,50 @@ def test_find_mention_ignores_non_comment_type():
def test_find_mention_matches_inline_comment():
txns = [
- _comment_txn("PHID-XACT-1", "PHID-USER-a", "@hackbot here", txn_type="inline")
+ _comment_txn(
+ "PHID-XACT-1",
+ "PHID-USER-a",
+ "@hackbot here",
+ txn_type="inline",
+ )
]
assert find_hackbot_mentions(
txns, {"PHID-XACT-1"}, bot_phid="PHID-USER-bot", token="@hackbot"
- ) == [HackbotMention("@hackbot here", "PHID-USER-a")]
+ ) == [
+ HackbotMention(
+ "@hackbot here",
+ "PHID-USER-a",
+ 1,
+ "inline",
+ )
+ ]
def test_find_mention_collects_all_inline_matches():
# A review with several inline @hackbot comments (each its own transaction)
# yields all of them, in order; comments without the token are skipped.
txns = [
- _comment_txn("PHID-XACT-1", "PHID-USER-a", "@hackbot fix this", "inline"),
- _comment_txn("PHID-XACT-2", "PHID-USER-a", "no mention here", "inline"),
- _comment_txn("PHID-XACT-3", "PHID-USER-a", "@hackbot and this too", "inline"),
+ _comment_txn(
+ "PHID-XACT-1",
+ "PHID-USER-a",
+ "@hackbot fix this",
+ "inline",
+ comment_id=1,
+ ),
+ _comment_txn(
+ "PHID-XACT-2",
+ "PHID-USER-a",
+ "no mention here",
+ "inline",
+ comment_id=2,
+ ),
+ _comment_txn(
+ "PHID-XACT-3",
+ "PHID-USER-a",
+ "@hackbot and this too",
+ "inline",
+ comment_id=3,
+ ),
]
assert find_hackbot_mentions(
txns,
@@ -143,8 +180,18 @@ def test_find_mention_collects_all_inline_matches():
bot_phid="PHID-USER-bot",
token="@hackbot",
) == [
- HackbotMention("@hackbot fix this", "PHID-USER-a"),
- HackbotMention("@hackbot and this too", "PHID-USER-a"),
+ HackbotMention(
+ "@hackbot fix this",
+ "PHID-USER-a",
+ 1,
+ "inline",
+ ),
+ HackbotMention(
+ "@hackbot and this too",
+ "PHID-USER-a",
+ 3,
+ "inline",
+ ),
]
@@ -156,23 +203,67 @@ def test_find_mention_one_per_transaction_ignores_comment_versions():
"type": "inline",
"authorPHID": "PHID-USER-a",
"comments": [
- {"content": {"raw": "@hackbot v1"}},
- {"content": {"raw": "@hackbot v2 edited"}},
+ {"id": 456, "content": {"raw": "@hackbot v1"}},
+ {"id": 456, "content": {"raw": "@hackbot v2 edited"}},
],
}
assert find_hackbot_mentions(
[txn], {"PHID-XACT-1"}, bot_phid="PHID-USER-bot", token="@hackbot"
- ) == [HackbotMention("@hackbot v1", "PHID-USER-a")]
+ ) == [
+ HackbotMention(
+ "@hackbot v1",
+ "PHID-USER-a",
+ 456,
+ "inline",
+ )
+ ]
-def test_join_comments_single_passthrough():
- assert _join_comments(["only one"]) == "only one"
+def test_format_comment_renders_regular_comment_as_xml():
+ mention = HackbotMention("only one", "PHID-USER-a", 123, "regular")
+ assert _format_comment(mention) == (
+ ' \n only one\n '
+ )
-def test_join_comments_numbers_multiple():
- joined = _join_comments(["first", "second"])
- assert "[comment 1]\nfirst" in joined
- assert "[comment 2]\nsecond" in joined
+def test_format_comment_renders_inline_comment_as_xml():
+ mention = HackbotMention(
+ "fix this",
+ "PHID-USER-a",
+ 456,
+ "inline",
+ )
+ assert _format_comment(mention) == (
+ ' \n fix this\n '
+ )
+
+
+def test_format_comments_renders_mixed_comments_in_order():
+ mentions = [
+ HackbotMention("first", "PHID-USER-a", 1, "regular"),
+ HackbotMention(
+ "second",
+ "PHID-USER-a",
+ 2,
+ "inline",
+ ),
+ ]
+ formatted = "\n\n".join(_format_comment(mention) for mention in mentions)
+ assert formatted == (
+ ' \n first\n \n\n'
+ ' \n'
+ " second\n"
+ " "
+ )
+
+
+def test_format_comment_escapes_comment_body():
+ mention = HackbotMention("@hackbot & explain", "PHID-USER-a", 1, "regular")
+ assert _format_comment(mention) == (
+ ' \n'
+ " @hackbot <fix> & explain\n"
+ " "
+ )
# --- revision resolution ---
@@ -254,7 +345,49 @@ async def test_detect_mention_accepts_editbugs_member(monkeypatch):
["PHID-XACT-1"],
authorizer=PhabricatorAuthorizer(client, AUTHORIZED_GROUP_PHID),
)
- assert result == ("@hackbot please fix", 42, 12345)
+ assert result == (
+ ' \n'
+ " @hackbot please fix\n"
+ " ",
+ 42,
+ 12345,
+ )
+ client.search_transactions.assert_awaited_once_with("PHID-DREV-x")
+
+
+async def test_detect_mention_formats_inline_comment(monkeypatch):
+ client = _FakeClient(
+ {"id": 42, "fields": {"bugzilla.bug-id": "12345"}},
+ members={"PHID-USER-authorized"},
+ )
+ transactions = [
+ _comment_txn(
+ "PHID-XACT-1",
+ "PHID-USER-authorized",
+ "@hackbot please fix",
+ "inline",
+ )
+ ]
+ monkeypatch.setattr(
+ client,
+ "search_transactions",
+ AsyncMock(return_value=transactions),
+ )
+
+ result = await detect_mention_and_revision(
+ client,
+ settings.webhook,
+ "PHID-DREV-x",
+ ["PHID-XACT-1"],
+ authorizer=PhabricatorAuthorizer(client, AUTHORIZED_GROUP_PHID),
+ )
+ assert result == (
+ ' \n'
+ " @hackbot please fix\n"
+ " ",
+ 42,
+ 12345,
+ )
# --- payload parsing ---