diff --git a/agent-team/slack/agent-team-manifest.json b/agent-team/slack/agent-team-manifest.json index 6345972..7b0ead9 100644 --- a/agent-team/slack/agent-team-manifest.json +++ b/agent-team/slack/agent-team-manifest.json @@ -26,7 +26,8 @@ "groups:history", "im:history", "app_mentions:read", - "commands" + "commands", + "reactions:write" ] } }, diff --git a/agent-team/tests/test_slack_listener.py b/agent-team/tests/test_slack_listener.py index 9b7cadc..f135c4a 100644 --- a/agent-team/tests/test_slack_listener.py +++ b/agent-team/tests/test_slack_listener.py @@ -687,3 +687,194 @@ def test_handle_event_swallows_sqlite_error_from_submit_answer( # Must not raise; returns None. assert listener.handle_event(_interactive_payload("q1")) is None + + +# --------------------------------------------------------------------------- +# 👍 reaction on received messages (after AUTHZ-01) — best-effort. +# --------------------------------------------------------------------------- + + +class _RecordingReactor: + """Records (channel, ts) reaction-add calls; optionally raises to test swallow.""" + + def __init__(self, *, boom: bool = False) -> None: + self.calls: list[tuple[str, str]] = [] + self._boom = boom + + def __call__(self, channel: str, ts: str) -> None: + self.calls.append((channel, ts)) + if self._boom: + raise RuntimeError("missing_scope: reactions:write not granted") + + +def _listener_with_reactor( + db_path: Path, + enqueue: Any, + reactor: Any, + *, + owner_ids: set[str] | None = frozenset({OWNER_ID}), +) -> SlackListener: + return SlackListener( + SlackTransport(channel="C123"), + db_path, + enqueue, + owner_ids=set(owner_ids) if owner_ids else None, + reactor=reactor, + ) + + +def test_reaction_added_to_thread_reply_after_authz(db_path: Path) -> None: + """An accepted thread-reply answer gets a 👍 on the reply message (event ts).""" + _seed_open_question(db_path, question_id="q1", channel_ref=SEED_CHANNEL_REF) + queue = RecordingQueue() + reactor = _RecordingReactor() + listener = _listener_with_reactor(db_path, queue, reactor) + + reply = _events_api_reply(text="approve", thread_ts=SEED_CHANNEL_REF) + outcome = listener.handle_event(reply) + + assert outcome is not None and outcome.accepted is True + # Reacted to the REPLY message: channel + the event ts (not the thread_ts). + assert reactor.calls == [("C123", "1700000001.000200")] + + +def test_reaction_not_added_for_non_owner(db_path: Path) -> None: + """AUTHZ-01 runs first: a non-owner message is rejected AND gets no reaction.""" + _seed_open_question(db_path, question_id="q1", channel_ref=SEED_CHANNEL_REF) + queue = RecordingQueue() + reactor = _RecordingReactor() + listener = _listener_with_reactor(db_path, queue, reactor) + + outcome = listener.handle_event( + _events_api_reply(sender_id="U_INTRUDER", thread_ts=SEED_CHANNEL_REF) + ) + + assert outcome is None + assert queue.jobs == [] + # The owner check returned before _maybe_react ran: NO reaction attempted. + assert reactor.calls == [] + assert _row_status(db_path, "q1") == "open" + + +def test_reaction_error_is_swallowed(db_path: Path) -> None: + """A reactor failure (e.g. missing reactions:write scope) never breaks handling.""" + _seed_open_question(db_path, question_id="q1", channel_ref=SEED_CHANNEL_REF) + queue = RecordingQueue() + reactor = _RecordingReactor(boom=True) + listener = _listener_with_reactor(db_path, queue, reactor) + + # Must not raise; the answer is still accepted + enqueued despite the reaction + # failing (the reaction silently no-ops until the scope is granted). + outcome = listener.handle_event( + _events_api_reply(text="approve", thread_ts=SEED_CHANNEL_REF) + ) + + assert outcome is not None and outcome.accepted is True + assert len(queue.jobs) == 1 + assert reactor.calls == [("C123", "1700000001.000200")] + + +def test_no_reactor_configured_is_quiet_noop(db_path: Path) -> None: + """With no reactor injected, an accepted answer simply does not react.""" + _seed_open_question(db_path, question_id="q1", channel_ref=SEED_CHANNEL_REF) + queue = RecordingQueue() + listener = _listener(db_path, queue, owner_ids={OWNER_ID}) # no reactor + + outcome = listener.handle_event( + _events_api_reply(text="approve", thread_ts=SEED_CHANNEL_REF) + ) + assert outcome is not None and outcome.accepted is True + + +def test_reaction_skipped_for_slash_or_interactive(db_path: Path) -> None: + """No reactable message ts on slash/interactive payloads => no reaction.""" + _seed_open_question(db_path, question_id="q1", thread_id="t1", turn=0) + queue = RecordingQueue() + reactor = _RecordingReactor() + listener = _listener_with_reactor(db_path, queue, reactor) + + # A block_actions interactive payload (no inner event ts) is accepted but has + # no reactable message, so no reaction is attempted. + outcome = listener.handle_event(_interactive_payload("q1", value="approve")) + assert outcome is not None and outcome.accepted is True + assert reactor.calls == [] + + +# --------------------------------------------------------------------------- +# /new-task — one-thread-per-task root "📥 Task received" ack post. +# --------------------------------------------------------------------------- + + +def test_new_task_posts_root_and_passes_root_ts(db_path: Path) -> None: + """A /new-task posts the root ack and forwards its ts to the callback. + + The "📥 Task received" message IS the slash command's acknowledgement (no + reaction), and its ``ts`` is threaded into start via the callback so every + later question/notification lands in one Slack thread. + """ + posted: list[dict[str, Any]] = [] + + def _poster(message: dict[str, Any]) -> dict[str, Any]: + posted.append(message) + return {"ts": "1700000000.ROOT"} + + seen: list[tuple[str, str, str]] = [] + + def _cb(task_text: str, via: str, root_ts: str) -> str: + seen.append((task_text, via, root_ts)) + return "thread-abc" + + listener = SlackListener( + SlackTransport(channel="C_TASK", poster=_poster), + db_path, + RecordingQueue(), + owner_ids={OWNER_ID}, + new_task_callback=_cb, + ) + + payload = { + "type": "slash_commands", + "command": "/new-task", + "text": "Add OAuth to the admin portal", + "user_id": OWNER_ID, + } + assert listener.handle_event(payload) is None + + # The root ack was posted to the channel with the 📥 prefix + description. + assert len(posted) == 1 + assert posted[0]["channel"] == "C_TASK" + assert posted[0]["text"].startswith("📥 Task received:") + assert "Add OAuth to the admin portal" in posted[0]["text"] + # The callback received the description, via, AND the captured root ts. + assert seen == [("Add OAuth to the admin portal", "slack", "1700000000.ROOT")] + + +def test_new_task_root_post_failure_degrades_to_empty_root_ts(db_path: Path) -> None: + """A failed root ack still starts the task (un-threaded): root_ts == ''.""" + + def _boom_poster(_message: dict[str, Any]) -> dict[str, Any]: + raise RuntimeError("slack down") + + seen: list[tuple[str, str, str]] = [] + + def _cb(task_text: str, via: str, root_ts: str) -> str: + seen.append((task_text, via, root_ts)) + return "thread-abc" + + listener = SlackListener( + SlackTransport(channel="C_TASK", poster=_boom_poster), + db_path, + RecordingQueue(), + owner_ids={OWNER_ID}, + new_task_callback=_cb, + ) + + payload = { + "type": "slash_commands", + "command": "/new-task", + "text": "do the thing", + "user_id": OWNER_ID, + } + assert listener.handle_event(payload) is None + # Task still started, with an empty root_ts (top-level questions). + assert seen == [("do the thing", "slack", "")] diff --git a/agent-team/tests/test_slack_live.py b/agent-team/tests/test_slack_live.py index 6d3fbf8..5911c89 100644 --- a/agent-team/tests/test_slack_live.py +++ b/agent-team/tests/test_slack_live.py @@ -38,11 +38,16 @@ class _FakeWebClient: response if response is not None else {"ts": "169.1", "ok": True} ) self.calls: list[dict[str, Any]] = [] + self.reaction_calls: list[dict[str, Any]] = [] def chat_postMessage(self, **kwargs: Any) -> dict[str, Any]: self.calls.append(kwargs) return self.response + def reactions_add(self, **kwargs: Any) -> dict[str, Any]: + self.reaction_calls.append(kwargs) + return {"ok": True} + class _DataResponse: """A ``slack_sdk.SlackResponse``-like object exposing the payload via ``.data``.""" @@ -161,6 +166,50 @@ def test_callback_id_dropped_metadata_carries_question_id() -> None: assert "blocks" in kwargs +def test_poster_forwards_thread_ts_to_chat_post_message() -> None: + """One-thread-per-task: thread_ts IS a postMessage param and is forwarded.""" + from agent_team.transport.slack_live import build_slack_poster as _bp + + client = _FakeWebClient() + transport = SlackTransport("C123", poster=_bp(client=client)) + + transport.post_question( + thread_id="task-7", + question_id="q-42", + turn=1, + question_set=_question_set(), + deadline="2026-06-18T00:00:00Z", + thread_ts="1700000000.ROOT", + ) + + assert client.calls[0]["thread_ts"] == "1700000000.ROOT" + + +def test_reactor_calls_reactions_add_with_thumbsup() -> None: + """build_slack_reactor performs reactions.add(channel, ts, name='thumbsup').""" + from agent_team.transport.slack_live import build_slack_reactor + + client = _FakeWebClient() + reactor = build_slack_reactor(client=client) + + reactor("C123", "1700000001.000200") + + assert client.reaction_calls == [ + {"channel": "C123", "timestamp": "1700000001.000200", "name": "thumbsup"} + ] + + +def test_reactor_missing_token_raises_runtime_error( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """No token + no client => the same loud RuntimeError as the poster path.""" + from agent_team.transport.slack_live import build_slack_reactor + + monkeypatch.delenv("SLACK_BOT_TOKEN", raising=False) + with pytest.raises(RuntimeError): + build_slack_reactor() + + def test_convenience_transport_factory() -> None: """``build_live_slack_transport`` wires the live poster onto a transport.""" client = _FakeWebClient()