From c63af0af2f0407a30634c97cf4a304e0dec0c629 Mon Sep 17 00:00:00 2001 From: ayoubdiourin7 Date: Sun, 20 Sep 2026 20:12:26 +0200 Subject: [PATCH] Deduplicate Slack button clicks Use the Slack message identity as the run dedupe key. Fixes #6732 --- services/hackbot-api/app/routers/slack.py | 22 +++- services/hackbot-api/app/slack_webhook.py | 8 ++ .../hackbot-api/tests/test_slack_webhook.py | 113 ++++++++++++++---- 3 files changed, 120 insertions(+), 23 deletions(-) diff --git a/services/hackbot-api/app/routers/slack.py b/services/hackbot-api/app/routers/slack.py index 0badd9e4e3..c5e7c0746e 100644 --- a/services/hackbot-api/app/routers/slack.py +++ b/services/hackbot-api/app/routers/slack.py @@ -11,7 +11,7 @@ from app.auth import require_slack_signature from app.routers.webhooks import get_hackbot_client -from app.slack_webhook import BlockActionsEvent +from app.slack_webhook import BlockActionsEvent, Container log = logging.getLogger(__name__) @@ -47,11 +47,29 @@ async def slack_interactions(request: Request) -> Response: match action.value.type: case "start_agent_run": client = get_hackbot_client() - await client.trigger_run( + run = await client.trigger_run( action.value.agent_name, action.value.params, + dedupe_key=button_dedupe_key(event.container), + ) + log.info( + "Slack action by %s on %s/%s -> run %s (%s)", + event.user.id, + event.container.channel_id, + event.container.message_ts, + run.run_id, + run.status, ) case _: raise ValueError("Unsupported action type: %s" % action.value.type) return Response(status_code=status.HTTP_200_OK) + + +def button_dedupe_key(container: Container) -> str: + """Key a run on the message the button sits on, so the button works once. + + Channel plus ``ts`` is how Slack identifies a message; ``ts`` alone is only + unique within a channel. + """ + return f"slack:{container.channel_id}:{container.message_ts}" diff --git a/services/hackbot-api/app/slack_webhook.py b/services/hackbot-api/app/slack_webhook.py index ec05a64b19..023772d127 100644 --- a/services/hackbot-api/app/slack_webhook.py +++ b/services/hackbot-api/app/slack_webhook.py @@ -37,6 +37,13 @@ class Message(BaseModel): ts: str +class Container(BaseModel): + """The message the clicked element is on, identified by channel and ``ts``.""" + + channel_id: str + message_ts: str + + class ActionValue(BaseModel): type: Literal["start_agent_run"] agent_name: str @@ -65,6 +72,7 @@ class BlockActionsEvent(BaseModel): type: Literal["block_actions"] user: User + container: Container channel: Channel | None = None message: Message | None = None actions: list[Action] = [] diff --git a/services/hackbot-api/tests/test_slack_webhook.py b/services/hackbot-api/tests/test_slack_webhook.py index c4b8e85d9c..8d6920212b 100644 --- a/services/hackbot-api/tests/test_slack_webhook.py +++ b/services/hackbot-api/tests/test_slack_webhook.py @@ -1,44 +1,115 @@ +"""The Slack interactions receiver: a click on a hackbot button starts a run once.""" + import json +import pytest from app.auth import require_slack_signature from app.main import app from app.routers import slack +from app.routers.slack import button_dedupe_key +from app.slack_webhook import Container +from hackbot_client import RunRef, RunStatus +from pydantic import ValidationError class _FakeHackbotClient: + """Records every trigger_run call the route makes.""" + def __init__(self): self.calls = [] - async def trigger_run(self, agent_name, inputs): - self.calls.append((agent_name, inputs)) + async def trigger_run(self, agent_name, inputs, *, dedupe_key=None): + self.calls.append((agent_name, inputs, dedupe_key)) + return RunRef( + run_id="d3d5f21d-d716-4bb0-a812-8c9ef3e2f1c6", + agent=agent_name, + status=RunStatus.pending, + ) -def test_start_agent_run_action_triggers_run(client, monkeypatch): - api_client = _FakeHackbotClient() +@pytest.fixture +def api_client(client, monkeypatch): + fake = _FakeHackbotClient() app.dependency_overrides[require_slack_signature] = lambda: None - monkeypatch.setattr(slack, "get_hackbot_client", lambda: api_client) - payload = { + monkeypatch.setattr(slack, "get_hackbot_client", lambda: fake) + return fake + + +def _click( + *, + user_id: str = "U123", + channel_id: str = "C0FFEE", + message_ts: str = "1758300000.000100", + action_id: str = "start", + value: dict | None = None, +) -> dict: + """A `block_actions` delivery for one click.""" + if value is None: + value = { + "type": "start_agent_run", + "agent_name": "bug-fix", + "params": {"bug_id": 123}, + } + return { "type": "block_actions", - "user": {"id": "U123", "username": "user"}, - "actions": [ - { - "action_id": "start", - "value": json.dumps( - { - "type": "start_agent_run", - "agent_name": "bug-fix", - "params": {"bug_id": 123}, - } - ), - } - ], + "user": {"id": user_id, "username": "user"}, + "container": { + "type": "message", + "channel_id": channel_id, + "message_ts": message_ts, + "is_ephemeral": False, + }, + "channel": {"id": channel_id}, + "message": {"ts": message_ts}, + "actions": [{"action_id": action_id, "value": json.dumps(value)}], "trigger_id": "trigger-123", } - response = client.post( + +def _post(client, payload: dict): + return client.post( "/webhooks/slack/interactions", data={"payload": json.dumps(payload)}, ) + +def test_button_dedupe_key_is_the_message_identity(): + container = Container(channel_id="C0FFEE", message_ts="1758300000.000100") + assert button_dedupe_key(container) == "slack:C0FFEE:1758300000.000100" + + +def test_start_agent_run_action_triggers_run_keyed_on_the_message(client, api_client): + response = _post(client, _click()) + assert response.status_code == 200 - assert api_client.calls == [("bug-fix", {"bug_id": 123})] + assert api_client.calls == [ + ("bug-fix", {"bug_id": 123}, "slack:C0FFEE:1758300000.000100") + ] + + +def test_repeated_clicks_on_one_button_carry_the_same_key(client, api_client): + _post(client, _click(user_id="U123")) + _post(client, _click(user_id="U123")) + _post(client, _click(user_id="U999")) + + keys = {key for _, _, key in api_client.calls} + assert keys == {"slack:C0FFEE:1758300000.000100"} + + +def test_buttons_on_different_messages_carry_different_keys(client, api_client): + _post(client, _click(channel_id="C0FFEE", message_ts="1758300000.000100")) + _post(client, _click(channel_id="C0FFEE", message_ts="1758300000.000200")) + _post(client, _click(channel_id="CBEEF0", message_ts="1758300000.000100")) + + keys = [key for _, _, key in api_client.calls] + assert len(set(keys)) == 3 + + +def test_click_without_a_message_container_is_rejected(client, api_client): + payload = _click() + payload["container"] = {"type": "view", "view_id": "V123"} + + with pytest.raises(ValidationError): + _post(client, payload) + + assert api_client.calls == []