feat(agent-team): 👍-acknowledge received Slack answers (reactions:write)
WS Slack-UX Feature 2. When the inbound listener acts on an answer in a task thread, it adds a 👍 reaction to that reply so the human sees the machine received it. - SlackListener gains an optional reactor seam; handle_event reacts to the inbound reply message (channel + event ts) AFTER the AUTHZ-01 owner check passes — a non-owner message is rejected and never reacted to. Best-effort: any reaction failure (notably a missing scope) is swallowed and never breaks handle_event or the listen loop. - /new-task is NOT reacted to (a slash command has no reactable message); its "📥 Task received" root post is the acknowledgement. - build_slack_reactor wraps WebClient.reactions_add(name="thumbsup"); the default listener factory wires it best-effort from SLACK_BOT_TOKEN. - Adds reactions:write to the bot scopes in agent-team-manifest.json. NOTE: the new reactions:write scope requires Adam to re-apply the manifest to app A0BCC7TTU66 and reinstall the app. Until then reactions.add returns missing_scope, which the listener swallows (the reaction silently no-ops) — answer handling is unaffected. AUTHZ-01 and the first-answer-wins CAS remain unchanged.
This commit is contained in:
parent
3847e43ba3
commit
9bfb5f1534
3 changed files with 242 additions and 1 deletions
|
|
@ -26,7 +26,8 @@
|
|||
"groups:history",
|
||||
"im:history",
|
||||
"app_mentions:read",
|
||||
"commands"
|
||||
"commands",
|
||||
"reactions:write"
|
||||
]
|
||||
}
|
||||
},
|
||||
|
|
|
|||
|
|
@ -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", "")]
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
|
|
|||
Reference in a new issue