mirror of
https://github.com/Sea-Haven-Industries/open-swe.git
synced 2026-10-03 04:33:27 +00:00
fix: prioritize web replies after Slack handoff (#1406)
This commit is contained in:
parent
2070a770c2
commit
5e9dcdb21d
6 changed files with 242 additions and 8 deletions
|
|
@ -377,14 +377,17 @@ async def send_dashboard_message(
|
||||||
prompt = body.content.strip()
|
prompt = body.content.strip()
|
||||||
now_ms = _now_ms()
|
now_ms = _now_ms()
|
||||||
chosen_model, chosen_effort = _normalize_model_choice(body.model_id, body.effort)
|
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:
|
if chosen_model and chosen_effort:
|
||||||
metadata_update["model"] = chosen_model
|
metadata_update["model"] = chosen_model
|
||||||
metadata_update["effort"] = chosen_effort
|
metadata_update["effort"] = chosen_effort
|
||||||
await client.threads.update(thread_id=thread_id, metadata=metadata_update)
|
await client.threads.update(thread_id=thread_id, metadata=metadata_update)
|
||||||
|
|
||||||
if await is_thread_active(thread_id):
|
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:
|
if not queued:
|
||||||
raise HTTPException(502, "failed to queue follow-up message")
|
raise HTTPException(502, "failed to queue follow-up message")
|
||||||
thread = await client.threads.get(thread_id)
|
thread = await client.threads.get(thread_id)
|
||||||
|
|
@ -394,19 +397,14 @@ async def send_dashboard_message(
|
||||||
|
|
||||||
await _ensure_dashboard_github_token(login)
|
await _ensure_dashboard_github_token(login)
|
||||||
profile = await get_profile(login) or {}
|
profile = await get_profile(login) or {}
|
||||||
thread_source = _thread_source(metadata)
|
|
||||||
configurable: dict[str, Any] = {
|
configurable: dict[str, Any] = {
|
||||||
"thread_id": thread_id,
|
"thread_id": thread_id,
|
||||||
"source": thread_source,
|
"source": _DASHBOARD_SOURCE,
|
||||||
"github_login": login,
|
"github_login": login,
|
||||||
"user_email": await _resolve_run_email(login, profile),
|
"user_email": await _resolve_run_email(login, profile),
|
||||||
}
|
}
|
||||||
if owner and name:
|
if owner and name:
|
||||||
configurable["repo"] = {"owner": owner, "name": 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:
|
if chosen_model and chosen_effort:
|
||||||
configurable["agent_model_id"] = chosen_model
|
configurable["agent_model_id"] = chosen_model
|
||||||
configurable["agent_effort"] = chosen_effort
|
configurable["agent_effort"] = chosen_effort
|
||||||
|
|
|
||||||
|
|
@ -19,6 +19,14 @@ from ..utils.multimodal import fetch_image_block
|
||||||
|
|
||||||
logger = logging.getLogger(__name__)
|
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):
|
class LinearNotifyState(AgentState):
|
||||||
"""Extended agent state for tracking Linear notifications."""
|
"""Extended agent state for tracking Linear notifications."""
|
||||||
|
|
@ -45,6 +53,10 @@ async def _build_blocks_from_payload(
|
||||||
return blocks
|
return blocks
|
||||||
|
|
||||||
|
|
||||||
|
def _is_dashboard_queued_message(content: object) -> bool:
|
||||||
|
return isinstance(content, dict) and content.get("source") == "dashboard"
|
||||||
|
|
||||||
|
|
||||||
@before_model(state_schema=LinearNotifyState)
|
@before_model(state_schema=LinearNotifyState)
|
||||||
async def check_message_queue_before_model( # noqa: PLR0911
|
async def check_message_queue_before_model( # noqa: PLR0911
|
||||||
state: LinearNotifyState, # noqa: ARG001
|
state: LinearNotifyState, # noqa: ARG001
|
||||||
|
|
@ -105,6 +117,8 @@ async def check_message_queue_before_model( # noqa: PLR0911
|
||||||
content_blocks: list[dict[str, Any]] = []
|
content_blocks: list[dict[str, Any]] = []
|
||||||
for msg in queued_messages:
|
for msg in queued_messages:
|
||||||
content = msg.get("content")
|
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):
|
if isinstance(content, dict) and ("text" in content or "image_urls" in content):
|
||||||
logger.debug("Queued message contains text + image URLs")
|
logger.debug("Queued message contains text + image URLs")
|
||||||
blocks = await _build_blocks_from_payload(content)
|
blocks = await _build_blocks_from_payload(content)
|
||||||
|
|
|
||||||
|
|
@ -6,6 +6,8 @@ from langchain_core.messages import AnyMessage, ToolMessage
|
||||||
from langgraph.config import get_config
|
from langgraph.config import get_config
|
||||||
from langgraph.runtime import Runtime
|
from langgraph.runtime import Runtime
|
||||||
|
|
||||||
|
from .check_message_queue import DASHBOARD_HANDOFF_MARKER
|
||||||
|
|
||||||
_DASHBOARD_SOURCE = "dashboard"
|
_DASHBOARD_SOURCE = "dashboard"
|
||||||
|
|
||||||
|
|
||||||
|
|
@ -43,6 +45,24 @@ def check_if_no_op(messages: list[AnyMessage]) -> bool:
|
||||||
return False
|
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:
|
def _is_dashboard_source() -> bool:
|
||||||
try:
|
try:
|
||||||
config = get_config()
|
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)
|
check_if_model_messaged_user(messages_since_last_human)
|
||||||
or check_if_confirming_completion(messages_since_last_human)
|
or check_if_confirming_completion(messages_since_last_human)
|
||||||
or _is_dashboard_source()
|
or _is_dashboard_source()
|
||||||
|
or _last_human_is_dashboard_handoff(state)
|
||||||
):
|
):
|
||||||
return None
|
return None
|
||||||
|
|
||||||
|
|
|
||||||
55
tests/test_check_message_queue.py
Normal file
55
tests/test_check_message_queue.py
Normal file
|
|
@ -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")]
|
||||||
122
tests/test_dashboard_web_handoff.py
Normal file
122
tests/test_dashboard_web_handoff.py
Normal file
|
|
@ -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"}]
|
||||||
|
|
@ -2,6 +2,7 @@ from unittest.mock import MagicMock, patch
|
||||||
|
|
||||||
from langchain_core.messages import AIMessage, HumanMessage, ToolMessage
|
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 (
|
from agent.middleware.ensure_no_empty_msg import (
|
||||||
check_if_confirming_completion,
|
check_if_confirming_completion,
|
||||||
check_if_model_messaged_user,
|
check_if_model_messaged_user,
|
||||||
|
|
@ -205,3 +206,26 @@ class TestEnsureNoEmptyMsgNotify:
|
||||||
|
|
||||||
assert result is None
|
assert result is None
|
||||||
assert not ai.tool_calls
|
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
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue