mirror of
https://github.com/Sea-Haven-Industries/open-swe.git
synced 2026-10-03 13:53:27 +00:00
fix: drop multitask_strategy=enqueue and forward image_urls on slack busy path (#1251)
The Slack mention path already routes mid-run messages through the store-based queue + check_message_queue_before_model middleware (the same path Linear uses), so multitask_strategy="enqueue" on runs.create was a leftover no-op — that line is only reached when is_thread_active returned False. The busy-path payload was also hardcoding image_urls=[] which silently dropped any images attached to mid-run Slack mentions; forward the resolved image_urls so the middleware can rebuild image blocks like Linear does.
This commit is contained in:
parent
88d3659d00
commit
bb836b58bc
2 changed files with 111 additions and 5 deletions
|
|
@ -733,7 +733,7 @@ async def process_linear_issue( # noqa: PLR0912, PLR0915
|
||||||
|
|
||||||
|
|
||||||
async def process_slack_mention(event_data: dict[str, Any], repo_config: dict[str, str]) -> None:
|
async def process_slack_mention(event_data: dict[str, Any], repo_config: dict[str, str]) -> None:
|
||||||
"""Process a Slack app mention by creating or interrupting a thread run."""
|
"""Process a Slack app mention by creating a run or queuing a mid-run message."""
|
||||||
channel_id = event_data.get("channel_id", "")
|
channel_id = event_data.get("channel_id", "")
|
||||||
thread_ts = event_data.get("thread_ts", "")
|
thread_ts = event_data.get("thread_ts", "")
|
||||||
event_ts = event_data.get("event_ts", "")
|
event_ts = event_data.get("event_ts", "")
|
||||||
|
|
@ -871,7 +871,7 @@ async def process_slack_mention(event_data: dict[str, Any], repo_config: dict[st
|
||||||
"Thread %s is active, queuing Slack message for middleware pickup",
|
"Thread %s is active, queuing Slack message for middleware pickup",
|
||||||
thread_id,
|
thread_id,
|
||||||
)
|
)
|
||||||
queued_payload = {"text": prompt, "image_urls": []}
|
queued_payload = {"text": prompt, "image_urls": image_urls}
|
||||||
queued = await queue_message_for_thread(
|
queued = await queue_message_for_thread(
|
||||||
thread_id=thread_id,
|
thread_id=thread_id,
|
||||||
message_content=queued_payload,
|
message_content=queued_payload,
|
||||||
|
|
@ -889,7 +889,6 @@ async def process_slack_mention(event_data: dict[str, Any], repo_config: dict[st
|
||||||
input={"messages": [{"role": "user", "content": content_blocks}]},
|
input={"messages": [{"role": "user", "content": content_blocks}]},
|
||||||
config={"configurable": configurable, "metadata": _AGENT_VERSION_METADATA},
|
config={"configurable": configurable, "metadata": _AGENT_VERSION_METADATA},
|
||||||
if_not_exists="create",
|
if_not_exists="create",
|
||||||
multitask_strategy="enqueue",
|
|
||||||
)
|
)
|
||||||
logger.info(
|
logger.info(
|
||||||
"Slack LangGraph run %s created for thread %s",
|
"Slack LangGraph run %s created for thread %s",
|
||||||
|
|
|
||||||
|
|
@ -416,7 +416,7 @@ def test_get_slack_repo_config_repo_name_only_space_syntax(
|
||||||
assert repo == {"owner": "langchain-ai", "name": "open-swe"}
|
assert repo == {"owner": "langchain-ai", "name": "open-swe"}
|
||||||
|
|
||||||
|
|
||||||
def test_process_slack_mention_creates_thread_followup_run_with_enqueue(
|
def test_process_slack_mention_creates_thread_followup_run(
|
||||||
monkeypatch: pytest.MonkeyPatch,
|
monkeypatch: pytest.MonkeyPatch,
|
||||||
) -> None:
|
) -> None:
|
||||||
captured: dict[str, object] = {}
|
captured: dict[str, object] = {}
|
||||||
|
|
@ -540,9 +540,116 @@ def test_process_slack_mention_creates_thread_followup_run_with_enqueue(
|
||||||
assert run_create["graph"] == "agent"
|
assert run_create["graph"] == "agent"
|
||||||
kwargs = run_create["kwargs"]
|
kwargs = run_create["kwargs"]
|
||||||
assert kwargs["if_not_exists"] == "create"
|
assert kwargs["if_not_exists"] == "create"
|
||||||
assert kwargs["multitask_strategy"] == "enqueue"
|
assert "multitask_strategy" not in kwargs
|
||||||
assert kwargs["config"]["configurable"]["slack_thread"]["thread_ts"] == thread_ts
|
assert kwargs["config"]["configurable"]["slack_thread"]["thread_ts"] == thread_ts
|
||||||
prompt_block = kwargs["input"]["messages"][0]["content"][0]
|
prompt_block = kwargs["input"]["messages"][0]["content"][0]
|
||||||
assert prompt_block["text"].count("## Slack Thread") == 1
|
assert prompt_block["text"].count("## Slack Thread") == 1
|
||||||
assert f"Thread TS: {thread_ts}" in prompt_block["text"]
|
assert f"Thread TS: {thread_ts}" in prompt_block["text"]
|
||||||
assert "## Latest Mention Request\ncontinue on the branch" in prompt_block["text"]
|
assert "## Latest Mention Request\ncontinue on the branch" in prompt_block["text"]
|
||||||
|
|
||||||
|
|
||||||
|
def test_process_slack_mention_queues_active_thread_message(
|
||||||
|
monkeypatch: pytest.MonkeyPatch,
|
||||||
|
) -> None:
|
||||||
|
captured: dict[str, object] = {}
|
||||||
|
|
||||||
|
async def fake_add_slack_reaction(channel_id: str, message_ts: str, emoji: str) -> bool:
|
||||||
|
captured["reaction"] = {
|
||||||
|
"channel_id": channel_id,
|
||||||
|
"message_ts": message_ts,
|
||||||
|
"emoji": emoji,
|
||||||
|
}
|
||||||
|
return True
|
||||||
|
|
||||||
|
async def fake_get_slack_user_info(user_id: str) -> dict:
|
||||||
|
return {
|
||||||
|
"profile": {
|
||||||
|
"email": "mason@example.com",
|
||||||
|
"display_name": "Mason",
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
async def fake_fetch_slack_thread_messages(channel_id: str, thread_ts: str) -> list[dict]:
|
||||||
|
return [
|
||||||
|
{"ts": "1700000000.000100", "text": "<@UBOT> first request", "user": "U123"},
|
||||||
|
{
|
||||||
|
"ts": "1700000000.000200",
|
||||||
|
"text": "<@UBOT> include this screenshot https://example.com/image.png",
|
||||||
|
"user": "U123",
|
||||||
|
},
|
||||||
|
]
|
||||||
|
|
||||||
|
async def fake_get_slack_user_names(user_ids: list[str]) -> dict[str, str]:
|
||||||
|
captured["user_ids"] = user_ids
|
||||||
|
return {"U123": "Mason"}
|
||||||
|
|
||||||
|
async def fake_resolve_slack_links_in_context(
|
||||||
|
context_messages: list[dict], user_names_by_id: dict[str, str]
|
||||||
|
) -> tuple[str, list[str]]:
|
||||||
|
captured["context_messages"] = context_messages
|
||||||
|
return "", []
|
||||||
|
|
||||||
|
async def fake_fetch_image_block(image_url: str, http_client: object) -> None:
|
||||||
|
captured["image_url"] = image_url
|
||||||
|
return None
|
||||||
|
|
||||||
|
async def fake_is_thread_active(thread_id: str) -> bool:
|
||||||
|
captured["active_thread_id"] = thread_id
|
||||||
|
return True
|
||||||
|
|
||||||
|
async def fake_queue_message_for_thread(thread_id: str, message_content: object) -> bool:
|
||||||
|
captured["queued"] = {"thread_id": thread_id, "message_content": message_content}
|
||||||
|
return True
|
||||||
|
|
||||||
|
async def fake_post_slack_trace_reply(*args, **kwargs) -> None:
|
||||||
|
raise AssertionError("trace reply should not be posted for queued mid-run Slack messages")
|
||||||
|
|
||||||
|
class _FakeRunsClient:
|
||||||
|
async def create(self, *args, **kwargs) -> None:
|
||||||
|
raise AssertionError("run should not be created for active Slack threads")
|
||||||
|
|
||||||
|
class _FakeThreadsClientForProcess:
|
||||||
|
async def update(self, *, thread_id: str, metadata: dict) -> None:
|
||||||
|
captured["metadata_update"] = {"thread_id": thread_id, "metadata": metadata}
|
||||||
|
|
||||||
|
class _FakeLangGraphClientForProcess:
|
||||||
|
runs = _FakeRunsClient()
|
||||||
|
threads = _FakeThreadsClientForProcess()
|
||||||
|
|
||||||
|
monkeypatch.setattr(webapp, "SLACK_BOT_USERNAME", "open-swe")
|
||||||
|
monkeypatch.setattr(webapp, "add_slack_reaction", fake_add_slack_reaction)
|
||||||
|
monkeypatch.setattr(webapp, "get_slack_user_info", fake_get_slack_user_info)
|
||||||
|
monkeypatch.setattr(webapp, "fetch_slack_thread_messages", fake_fetch_slack_thread_messages)
|
||||||
|
monkeypatch.setattr(webapp, "get_slack_user_names", fake_get_slack_user_names)
|
||||||
|
monkeypatch.setattr(
|
||||||
|
webapp, "resolve_slack_links_in_context", fake_resolve_slack_links_in_context
|
||||||
|
)
|
||||||
|
monkeypatch.setattr(webapp, "fetch_image_block", fake_fetch_image_block)
|
||||||
|
monkeypatch.setattr(webapp, "is_thread_active", fake_is_thread_active)
|
||||||
|
monkeypatch.setattr(webapp, "queue_message_for_thread", fake_queue_message_for_thread)
|
||||||
|
monkeypatch.setattr(webapp, "post_slack_trace_reply", fake_post_slack_trace_reply)
|
||||||
|
monkeypatch.setattr(webapp, "get_client", lambda url: _FakeLangGraphClientForProcess())
|
||||||
|
|
||||||
|
thread_ts = "1700000000.000100"
|
||||||
|
event_ts = "1700000000.000200"
|
||||||
|
expected_thread_id = generate_thread_id_from_slack_thread("C123", thread_ts)
|
||||||
|
|
||||||
|
asyncio.run(
|
||||||
|
webapp.process_slack_mention(
|
||||||
|
{
|
||||||
|
"channel_id": "C123",
|
||||||
|
"thread_ts": thread_ts,
|
||||||
|
"event_ts": event_ts,
|
||||||
|
"user_id": "U123",
|
||||||
|
"text": "<@UBOT> include this screenshot https://example.com/image.png",
|
||||||
|
"bot_user_id": "UBOT",
|
||||||
|
},
|
||||||
|
{"owner": "langchain-ai", "name": "open-swe"},
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|
||||||
|
assert captured["active_thread_id"] == expected_thread_id
|
||||||
|
assert captured["queued"]["thread_id"] == expected_thread_id
|
||||||
|
queued_payload = captured["queued"]["message_content"]
|
||||||
|
assert queued_payload["image_urls"] == ["https://example.com/image.png"]
|
||||||
|
assert "## Latest Mention Request\ninclude this screenshot" in queued_payload["text"]
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue