From 62747aa58471aaef9f4b1866efbd1cff077a396f Mon Sep 17 00:00:00 2001 From: Jerome Iveson Date: Thu, 2 Jul 2026 16:55:19 +0100 Subject: [PATCH] fix(slack): delete stale progress messages --- plugins/platforms/slack/adapter.py | 28 +++++++++ tests/gateway/test_run_cleanup_progress.py | 49 ++++++++++++++- tests/gateway/test_slack.py | 69 ++++++++++++++++++++++ 3 files changed, 144 insertions(+), 2 deletions(-) diff --git a/plugins/platforms/slack/adapter.py b/plugins/platforms/slack/adapter.py index b98cfec5a8e4..8a4d04eb3aa4 100644 --- a/plugins/platforms/slack/adapter.py +++ b/plugins/platforms/slack/adapter.py @@ -2495,6 +2495,34 @@ class SlackAdapter(BasePlatformAdapter): ) return SendResult(success=False, error=str(e)) + async def delete_message(self, chat_id: str, message_id: str) -> bool: + """Delete a Slack message previously sent by this bot. + + Used by gateway progress cleanup so temporary "Working"/tool-progress + bubbles do not remain after a successful final response. + """ + if not self._app: + return False + try: + response = await self._get_client(chat_id).chat_delete(channel=chat_id, ts=message_id) + if hasattr(response, "get") and response.get("ok") is False: + logger.debug( + "[Slack] chat.delete returned ok=false for message %s in channel %s: %s", + message_id, + chat_id, + response.get("error", "unknown"), + ) + return False + return True + except Exception as e: # pragma: no cover - best-effort cleanup + logger.debug( + "[Slack] Failed to delete message %s in channel %s: %s", + message_id, + chat_id, + e, + ) + return False + async def send_typing(self, chat_id: str, metadata=None) -> None: """Show a typing/status indicator using assistant.threads.setStatus. diff --git a/tests/gateway/test_run_cleanup_progress.py b/tests/gateway/test_run_cleanup_progress.py index b71c8b2712f5..8fa62ff6aa10 100644 --- a/tests/gateway/test_run_cleanup_progress.py +++ b/tests/gateway/test_run_cleanup_progress.py @@ -166,7 +166,13 @@ def _make_runner(adapter): return runner -def _install_fakes(monkeypatch, agent_cls, *, cleanup_on: bool): +def _install_fakes( + monkeypatch, + agent_cls, + *, + cleanup_on: bool, + cleanup_platform: Platform = Platform.TELEGRAM, +): """Wire up the module stubs every _run_agent test needs.""" monkeypatch.setenv("HERMES_TOOL_PROGRESS_MODE", "all") @@ -187,7 +193,7 @@ def _install_fakes(monkeypatch, agent_cls, *, cleanup_on: bool): cfg = { "display": { "platforms": { - "telegram": {"cleanup_progress": True}, + cleanup_platform.value: {"cleanup_progress": True}, } } } if cleanup_on else {} @@ -320,6 +326,45 @@ async def test_cleanup_registers_callback_and_deletes_on_success(monkeypatch, tm assert entry["chat_id"] == "-1001" +@pytest.mark.asyncio +async def test_slack_cleanup_flag_deletes_progress_bubbles(monkeypatch, tmp_path): + """Slack's per-platform cleanup flag uses the same post-delivery cleanup path.""" + adapter = CleanupCaptureAdapter(Platform.SLACK) + runner = _make_runner(adapter) + gateway_run = _install_fakes( + monkeypatch, + ProgressAgent, + cleanup_on=True, + cleanup_platform=Platform.SLACK, + ) + monkeypatch.setattr(gateway_run, "_hermes_home", tmp_path) + + source = SessionSource(platform=Platform.SLACK, chat_id="D123") + session_key = "agent:main:slack:dm:D123" + + result = await runner._run_agent( + message="hello", + context_prompt="", + history=[], + source=source, + session_id="sess-slack", + session_key=session_key, + ) + + assert result["final_response"] == "done" + cb = adapter.pop_post_delivery_callback(session_key) + assert callable(cb) + await _fire_post_delivery_cb(cb) + for _ in range(20): + await asyncio.sleep(0.01) + if adapter.deleted: + break + + assert len(adapter.deleted) >= 1, f"deleted={adapter.deleted} sent={adapter.sent}" + for entry in adapter.deleted: + assert entry["chat_id"] == "D123" + + @pytest.mark.asyncio async def test_cleanup_skipped_on_failed_run(monkeypatch, tmp_path): """Failed runs skip cleanup registration — breadcrumbs stay.""" diff --git a/tests/gateway/test_slack.py b/tests/gateway/test_slack.py index 30071775d8fd..5cb322c848f6 100644 --- a/tests/gateway/test_slack.py +++ b/tests/gateway/test_slack.py @@ -4043,6 +4043,75 @@ class TestEditMessage: assert len(kwargs["text"]) <= adapter.MAX_MESSAGE_LENGTH +# --------------------------------------------------------------------------- +# TestDeleteMessage +# --------------------------------------------------------------------------- + + +class TestDeleteMessage: + """Verify that delete_message() calls Slack's chat.delete API safely.""" + + @pytest.mark.asyncio + async def test_delete_message_calls_chat_delete(self, adapter): + adapter._app.client.chat_delete = AsyncMock(return_value={"ok": True}) + + result = await adapter.delete_message("C123", "1234.5678") + + assert result is True + adapter._app.client.chat_delete.assert_awaited_once_with( + channel="C123", + ts="1234.5678", + ) + + @pytest.mark.asyncio + async def test_delete_message_uses_workspace_specific_client(self, adapter): + workspace_client = MagicMock() + workspace_client.chat_delete = AsyncMock(return_value={"ok": True}) + adapter._channel_team["C999"] = "T999" + adapter._team_clients["T999"] = workspace_client + + result = await adapter.delete_message("C999", "1712345678.000100") + + assert result is True + workspace_client.chat_delete.assert_awaited_once_with( + channel="C999", + ts="1712345678.000100", + ) + adapter._app.client.chat_delete.assert_not_called() + + @pytest.mark.asyncio + async def test_delete_message_returns_false_when_not_connected(self, adapter): + adapter._app = None + + assert await adapter.delete_message("C123", "1234.5678") is False + + @pytest.mark.asyncio + async def test_delete_message_is_best_effort_on_api_error(self, adapter): + adapter._app.client.chat_delete = AsyncMock(side_effect=RuntimeError("missing_scope")) + + result = await adapter.delete_message("C123", "1234.5678") + + assert result is False + adapter._app.client.chat_delete.assert_awaited_once_with( + channel="C123", + ts="1234.5678", + ) + + @pytest.mark.asyncio + async def test_delete_message_returns_false_when_slack_response_not_ok(self, adapter): + adapter._app.client.chat_delete = AsyncMock( + return_value={"ok": False, "error": "cant_delete_message"}, + ) + + result = await adapter.delete_message("C123", "1234.5678") + + assert result is False + adapter._app.client.chat_delete.assert_awaited_once_with( + channel="C123", + ts="1234.5678", + ) + + # --------------------------------------------------------------------------- # TestEditMessageStreamingPipeline # ---------------------------------------------------------------------------