From 5e9dcdb21d2d71e02c10ec734f72971e1bf9adb9 Mon Sep 17 00:00:00 2001 From: Johannes du Plessis Date: Thu, 4 Jun 2026 10:01:10 -0700 Subject: [PATCH] fix: prioritize web replies after Slack handoff (#1406) --- agent/dashboard/thread_api.py | 14 ++- agent/middleware/check_message_queue.py | 14 +++ agent/middleware/ensure_no_empty_msg.py | 21 ++++ tests/test_check_message_queue.py | 55 +++++++++++ tests/test_dashboard_web_handoff.py | 122 ++++++++++++++++++++++++ tests/test_ensure_no_empty_msg.py | 24 +++++ 6 files changed, 242 insertions(+), 8 deletions(-) create mode 100644 tests/test_check_message_queue.py create mode 100644 tests/test_dashboard_web_handoff.py diff --git a/agent/dashboard/thread_api.py b/agent/dashboard/thread_api.py index da14944c..8d17b89f 100644 --- a/agent/dashboard/thread_api.py +++ b/agent/dashboard/thread_api.py @@ -377,14 +377,17 @@ async def send_dashboard_message( prompt = body.content.strip() now_ms = _now_ms() chosen_model, chosen_effort = _normalize_model_choice(body.model_id, body.effort) - metadata_update: dict[str, Any] = {"updated_at_ms": now_ms} + metadata_update: dict[str, Any] = {"source": _DASHBOARD_SOURCE, "updated_at_ms": now_ms} if chosen_model and chosen_effort: metadata_update["model"] = chosen_model metadata_update["effort"] = chosen_effort await client.threads.update(thread_id=thread_id, metadata=metadata_update) if await is_thread_active(thread_id): - queued = await queue_message_for_thread(thread_id, prompt) + queued = await queue_message_for_thread( + thread_id, + {"text": prompt, "source": _DASHBOARD_SOURCE}, + ) if not queued: raise HTTPException(502, "failed to queue follow-up message") thread = await client.threads.get(thread_id) @@ -394,19 +397,14 @@ async def send_dashboard_message( await _ensure_dashboard_github_token(login) profile = await get_profile(login) or {} - thread_source = _thread_source(metadata) configurable: dict[str, Any] = { "thread_id": thread_id, - "source": thread_source, + "source": _DASHBOARD_SOURCE, "github_login": login, "user_email": await _resolve_run_email(login, profile), } if owner and name: configurable["repo"] = {"owner": owner, "name": name} - source_context = metadata.get("source_context") - if isinstance(source_context, dict): - for key, value in source_context.items(): - configurable.setdefault(key, value) if chosen_model and chosen_effort: configurable["agent_model_id"] = chosen_model configurable["agent_effort"] = chosen_effort diff --git a/agent/middleware/check_message_queue.py b/agent/middleware/check_message_queue.py index a8825761..babdaeb7 100644 --- a/agent/middleware/check_message_queue.py +++ b/agent/middleware/check_message_queue.py @@ -19,6 +19,14 @@ from ..utils.multimodal import fetch_image_block logger = logging.getLogger(__name__) +DASHBOARD_HANDOFF_MARKER = "[Open SWE Web handoff]" +DASHBOARD_HANDOFF_INSTRUCTION = ( + f"{DASHBOARD_HANDOFF_MARKER} This follow-up was sent from Web. " + "The conversation has moved to Web, so answer in the dashboard stream with a normal " + "assistant message. Do not call slack_thread_reply unless a later Slack message explicitly " + "moves the conversation back to Slack." +) + class LinearNotifyState(AgentState): """Extended agent state for tracking Linear notifications.""" @@ -45,6 +53,10 @@ async def _build_blocks_from_payload( return blocks +def _is_dashboard_queued_message(content: object) -> bool: + return isinstance(content, dict) and content.get("source") == "dashboard" + + @before_model(state_schema=LinearNotifyState) async def check_message_queue_before_model( # noqa: PLR0911 state: LinearNotifyState, # noqa: ARG001 @@ -105,6 +117,8 @@ async def check_message_queue_before_model( # noqa: PLR0911 content_blocks: list[dict[str, Any]] = [] for msg in queued_messages: content = msg.get("content") + if _is_dashboard_queued_message(content): + content_blocks.append({"type": "text", "text": DASHBOARD_HANDOFF_INSTRUCTION}) if isinstance(content, dict) and ("text" in content or "image_urls" in content): logger.debug("Queued message contains text + image URLs") blocks = await _build_blocks_from_payload(content) diff --git a/agent/middleware/ensure_no_empty_msg.py b/agent/middleware/ensure_no_empty_msg.py index 19580cfa..6e5da553 100644 --- a/agent/middleware/ensure_no_empty_msg.py +++ b/agent/middleware/ensure_no_empty_msg.py @@ -6,6 +6,8 @@ from langchain_core.messages import AnyMessage, ToolMessage from langgraph.config import get_config from langgraph.runtime import Runtime +from .check_message_queue import DASHBOARD_HANDOFF_MARKER + _DASHBOARD_SOURCE = "dashboard" @@ -43,6 +45,24 @@ def check_if_no_op(messages: list[AnyMessage]) -> bool: return False +def _content_contains_text(content: object, text: str) -> bool: + if isinstance(content, str): + return text in content + if not isinstance(content, list): + return False + for block in content: + if isinstance(block, dict) and text in str(block.get("text", "")): + return True + return False + + +def _last_human_is_dashboard_handoff(state: AgentState) -> bool: + for msg in reversed(state["messages"]): + if msg.type == "human": + return _content_contains_text(msg.content, DASHBOARD_HANDOFF_MARKER) + return False + + def _is_dashboard_source() -> bool: try: config = get_config() @@ -85,6 +105,7 @@ def ensure_no_empty_msg(state: AgentState, runtime: Runtime) -> dict[str, Any] | check_if_model_messaged_user(messages_since_last_human) or check_if_confirming_completion(messages_since_last_human) or _is_dashboard_source() + or _last_human_is_dashboard_handoff(state) ): return None diff --git a/tests/test_check_message_queue.py b/tests/test_check_message_queue.py new file mode 100644 index 00000000..5ae2a132 --- /dev/null +++ b/tests/test_check_message_queue.py @@ -0,0 +1,55 @@ +from __future__ import annotations + +from typing import Any +from unittest.mock import MagicMock, patch + +import pytest + +from agent.middleware.check_message_queue import ( + DASHBOARD_HANDOFF_MARKER, + check_message_queue_before_model, +) + + +class _QueuedItem: + def __init__(self, value: dict[str, Any]) -> None: + self.value = value + + +class _FakeStore: + def __init__(self, value: dict[str, Any]) -> None: + self.value = value + self.deleted: list[tuple[tuple[str, ...], str]] = [] + + async def aget(self, namespace: tuple[str, ...], key: str) -> _QueuedItem: + return _QueuedItem(self.value) + + async def adelete(self, namespace: tuple[str, ...], key: str) -> None: + self.deleted.append((namespace, key)) + + +@pytest.mark.asyncio +async def test_check_message_queue_injects_dashboard_handoff_instruction() -> None: + store = _FakeStore( + { + "messages": [ + {"content": {"text": "continue in web", "source": "dashboard"}}, + ] + } + ) + + with ( + patch( + "agent.middleware.check_message_queue.get_config", + return_value={"configurable": {"thread_id": "thread-1"}}, + ), + patch("agent.middleware.check_message_queue.get_store", return_value=store), + ): + result = await check_message_queue_before_model.abefore_model({}, MagicMock()) + + assert result is not None + message = result["messages"][0] + assert message["role"] == "user" + assert DASHBOARD_HANDOFF_MARKER in message["content"][0]["text"] + assert message["content"][1] == {"type": "text", "text": "continue in web"} + assert store.deleted == [(("queue", "thread-1"), "pending_messages")] diff --git a/tests/test_dashboard_web_handoff.py b/tests/test_dashboard_web_handoff.py new file mode 100644 index 00000000..2bba563d --- /dev/null +++ b/tests/test_dashboard_web_handoff.py @@ -0,0 +1,122 @@ +from __future__ import annotations + +from typing import Any + +import pytest + +from agent.dashboard import thread_api + + +class _FakeThreads: + def __init__(self, metadata: dict[str, Any]) -> None: + self.metadata = metadata + self.updates: list[dict[str, Any]] = [] + + async def get(self, thread_id: str) -> dict[str, Any]: + return {"thread_id": thread_id, "metadata": self.metadata} + + async def update(self, *, thread_id: str, metadata: dict[str, Any]) -> None: + self.updates.append(metadata) + self.metadata.update(metadata) + + +class _FakeRuns: + def __init__(self) -> None: + self.created: list[dict[str, Any]] = [] + + async def create(self, *args: Any, **kwargs: Any) -> dict[str, str]: + self.created.append({"args": args, "kwargs": kwargs}) + return {"run_id": "run-1"} + + +class _FakeClient: + def __init__(self, metadata: dict[str, Any]) -> None: + self.threads = _FakeThreads(metadata) + self.runs = _FakeRuns() + + +async def _inactive_thread(thread_id: str) -> bool: + return False + + +async def _active_thread(thread_id: str) -> bool: + return True + + +async def _noop_token_check(login: str) -> None: + return None + + +async def _empty_profile(login: str) -> dict[str, Any]: + return {} + + +async def _run_email(login: str, profile: dict[str, Any]) -> str: + return "octocat@example.com" + + +@pytest.mark.asyncio +async def test_dashboard_followup_on_slack_thread_uses_dashboard_source( + monkeypatch: pytest.MonkeyPatch, +) -> None: + metadata = { + "source": "slack", + "github_login": "octocat", + "triggering_user_email": "octocat@example.com", + "repo_owner": "octo", + "repo_name": "repo", + "source_context": { + "slack_thread": {"channel_id": "C1", "thread_ts": "123.45"}, + }, + } + client = _FakeClient(metadata) + + monkeypatch.setattr(thread_api, "langgraph_client", lambda: client) + monkeypatch.setattr(thread_api, "is_thread_active", _inactive_thread) + monkeypatch.setattr(thread_api, "_ensure_dashboard_github_token", _noop_token_check) + monkeypatch.setattr(thread_api, "get_profile", _empty_profile) + monkeypatch.setattr(thread_api, "_resolve_run_email", _run_email) + + await thread_api.send_dashboard_message( + "thread-1", + "octocat", + thread_api.ThreadMessageBody(content="continue in web"), + email="octocat@example.com", + ) + + run_config = client.runs.created[0]["kwargs"]["config"]["configurable"] + assert client.threads.updates[0]["source"] == "dashboard" + assert run_config["source"] == "dashboard" + assert "slack_thread" not in run_config + assert run_config["repo"] == {"owner": "octo", "name": "repo"} + + +@pytest.mark.asyncio +async def test_dashboard_followup_on_busy_thread_queues_dashboard_handoff( + monkeypatch: pytest.MonkeyPatch, +) -> None: + metadata = { + "source": "slack", + "github_login": "octocat", + "triggering_user_email": "octocat@example.com", + } + client = _FakeClient(metadata) + queued_messages: list[object] = [] + + async def fake_queue_message_for_thread(thread_id: str, message_content: object) -> bool: + queued_messages.append(message_content) + return True + + monkeypatch.setattr(thread_api, "langgraph_client", lambda: client) + monkeypatch.setattr(thread_api, "is_thread_active", _active_thread) + monkeypatch.setattr(thread_api, "queue_message_for_thread", fake_queue_message_for_thread) + + await thread_api.send_dashboard_message( + "thread-1", + "octocat", + thread_api.ThreadMessageBody(content="continue in web"), + email="octocat@example.com", + ) + + assert client.threads.updates[0]["source"] == "dashboard" + assert queued_messages == [{"text": "continue in web", "source": "dashboard"}] diff --git a/tests/test_ensure_no_empty_msg.py b/tests/test_ensure_no_empty_msg.py index 10013703..8c99850a 100644 --- a/tests/test_ensure_no_empty_msg.py +++ b/tests/test_ensure_no_empty_msg.py @@ -2,6 +2,7 @@ from unittest.mock import MagicMock, patch from langchain_core.messages import AIMessage, HumanMessage, ToolMessage +from agent.middleware.check_message_queue import DASHBOARD_HANDOFF_INSTRUCTION from agent.middleware.ensure_no_empty_msg import ( check_if_confirming_completion, check_if_model_messaged_user, @@ -205,3 +206,26 @@ class TestEnsureNoEmptyMsgNotify: assert result is None assert not ai.tool_calls + + def test_skips_confirming_completion_for_dashboard_handoff(self) -> None: + ai = AIMessage(content="Done in web.") + state = { + "messages": [ + HumanMessage( + content=[ + {"type": "text", "text": DASHBOARD_HANDOFF_INSTRUCTION}, + {"type": "text", "text": "continue in web"}, + ] + ), + ai, + ] + } + + with patch( + "agent.middleware.ensure_no_empty_msg.get_config", + return_value={"configurable": {"source": "slack"}}, + ): + result = ensure_no_empty_msg.after_model(state, self._make_runtime()) + + assert result is None + assert not ai.tool_calls