From 92dbf6f932f4ee73aafaee7896b71ab0a18793d6 Mon Sep 17 00:00:00 2001 From: Ramon Nogueira Date: Mon, 29 Jun 2026 13:22:01 -0400 Subject: [PATCH] fix: update Slack trace reply on web handoff (#1630) * fix: update Slack trace reply on web handoff Co-authored-by: open-swe[bot] * fix: trigger web handoff on dashboard starts Co-authored-by: open-swe[bot] * fix: format web handoff as contextual fragment Co-authored-by: open-swe[bot] * fix: preserve trace_message_ts when overwriting Slack run mapping When store_slack_run_mapping is called without trace_message_ts (e.g. on follow-up Slack mentions), it was unconditionally overwriting the thread-level mapping and clobbering the timestamp captured from the initial trace reply. After that, _notify_slack_web_handoff could not find the original message, so a subsequent move to Web silently skipped the Slack trace update. Now, when trace_message_ts is not passed, the existing thread mapping is read first and its trace_message_ts is preserved. * style: ruff format --------- Co-authored-by: Ramon Nogueira <270434257+ramon-langchain@users.noreply.github.com> Co-authored-by: open-swe[bot] Co-authored-by: Johannes du Plessis <51395795+johannes117@users.noreply.github.com> Co-authored-by: Johannes du Plessis --- agent/dashboard/thread_api.py | 79 +++++++++- agent/middleware/check_message_queue.py | 12 +- agent/utils/dashboard_handoff.py | 11 ++ agent/utils/slack.py | 124 +++++++++++++++- agent/webhooks/slack.py | 1 + tests/test_dashboard_thread_api.py | 186 ++++++++++++++++++++++++ tests/test_dashboard_web_handoff.py | 115 ++++++++++++++- tests/test_slack_assistants_status.py | 24 +++ tests/test_slack_context.py | 127 ++++++++++++++++ 9 files changed, 661 insertions(+), 18 deletions(-) create mode 100644 agent/utils/dashboard_handoff.py diff --git a/agent/dashboard/thread_api.py b/agent/dashboard/thread_api.py index 891cae68..b611c7e4 100644 --- a/agent/dashboard/thread_api.py +++ b/agent/dashboard/thread_api.py @@ -17,8 +17,10 @@ from fastapi import HTTPException from langchain_core.messages.content import create_image_block from pydantic import BaseModel, ConfigDict, Field +from ..utils.dashboard_handoff import DASHBOARD_HANDOFF_INSTRUCTION from ..utils.langsmith import get_langsmith_trace_url from ..utils.sandbox import create_sandbox +from ..utils.slack import lookup_slack_thread_run_mapping, update_slack_trace_reply_for_web_handoff from ..utils.thread_ops import ( get_thread_active_status, langgraph_client, @@ -1073,6 +1075,17 @@ def _prefix_message_content(content: Any, prefix: str) -> Any: return content +def _prepend_message_content_block(content: Any, text: str) -> Any: + block = {"type": "text", "text": text} + if isinstance(content, str): + return [block, {"type": "text", "text": content}] + if isinstance(content, list): + return [block, *content] + if content is None: + return [block] + return content + + def _command_prompt_text(content: Any) -> str: if isinstance(content, str): return content.strip() @@ -1203,7 +1216,10 @@ async def _enrich_run_start_command( _validate_command_images(content, model_id=run_model) prefix = _attribution_prefix(metadata, login, email) if prefix: - _set_command_last_message_content(params, _prefix_message_content(content, prefix)) + content = _prefix_message_content(content, prefix) + if metadata.get("source") == "slack": + content = _prepend_message_content_block(content, DASHBOARD_HANDOFF_INSTRUCTION) + _set_command_last_message_content(params, content) metadata_update: dict[str, Any] = {"plan_mode": plan_mode_requested} if command_images and run_model and run_effort: overrides["agent_model_id"] = run_model @@ -1246,6 +1262,43 @@ async def _enrich_run_start_command( return command +def _slack_thread_context(metadata: dict[str, Any]) -> dict[str, Any] | None: + source_context = metadata.get("source_context") + if not isinstance(source_context, dict): + return None + slack_thread = source_context.get("slack_thread") + return slack_thread if isinstance(slack_thread, dict) else None + + +async def _notify_slack_web_handoff(thread_id: str, metadata: dict[str, Any], client: Any) -> None: + if metadata.get("source") != "slack": + return + slack_thread = _slack_thread_context(metadata) + if not slack_thread: + return + channel_id = slack_thread.get("channel_id") + thread_ts = slack_thread.get("thread_ts") + if not isinstance(channel_id, str) or not channel_id: + return + if not isinstance(thread_ts, str) or not thread_ts: + return + + trace_message_ts = slack_thread.get("trace_message_ts") + if not isinstance(trace_message_ts, str) or not trace_message_ts: + mapping = await lookup_slack_thread_run_mapping(client, channel_id, thread_ts) + if isinstance(mapping, dict): + candidate = mapping.get("trace_message_ts") + if isinstance(candidate, str) and candidate: + trace_message_ts = candidate + if not isinstance(trace_message_ts, str) or not trace_message_ts: + logger.info( + "Skipping Slack web handoff update for thread %s: missing trace message ts", thread_id + ) + return + + await update_slack_trace_reply_for_web_handoff(channel_id, trace_message_ts, thread_id) + + async def send_dashboard_message( thread_id: str, login: str, body: ThreadMessageBody, *, email: str | None = None ) -> dict[str, Any]: @@ -1261,6 +1314,7 @@ async def send_dashboard_message( prompt = f"{_attribution_prefix(metadata, login, email)}{body.content.strip()}" now_ms = _now_ms() chosen_model, chosen_effort = _normalize_model_choice(body.model_id, body.effort) + handoff_metadata = dict(metadata) metadata_update: dict[str, Any] = { "source": _DASHBOARD_SOURCE, "updated_at_ms": now_ms, @@ -1293,6 +1347,10 @@ async def send_dashboard_message( queued = await queue_message_for_thread(thread_id, queue_payload) if not queued: raise HTTPException(502, "failed to queue follow-up message") + try: + await _notify_slack_web_handoff(thread_id, handoff_metadata, client) + except Exception: + logger.exception("Failed to update Slack message for dashboard handoff on %s", thread_id) thread = await client.threads.get(thread_id) return _thread_summary( thread if isinstance(thread, dict) else {"thread_id": thread_id, "metadata": metadata} @@ -1812,11 +1870,20 @@ async def proxy_dashboard_thread_commands( async with httpx.AsyncClient(timeout=_PROXY_REQUEST_TIMEOUT) as client: response = await client.post(url, content=outgoing, headers=headers) - if ( - parsed.get("method") == "run.start" - and response.status_code in {200, 202, 204} - and response.content - ): + run_start_succeeded = parsed.get("method") == "run.start" and response.status_code in { + 200, + 202, + 204, + } + if run_start_succeeded and not creating: + try: + await _notify_slack_web_handoff(thread_id, metadata, langgraph_client()) + except Exception: + logger.exception( + "Failed to update Slack message for dashboard handoff on %s", thread_id + ) + + if run_start_succeeded and response.content: try: payload = json.loads(response.content) except json.JSONDecodeError: diff --git a/agent/middleware/check_message_queue.py b/agent/middleware/check_message_queue.py index fcbf89f8..43d3ce9f 100644 --- a/agent/middleware/check_message_queue.py +++ b/agent/middleware/check_message_queue.py @@ -18,19 +18,15 @@ from langgraph.store.base import BaseStore from langgraph_sdk import get_client from ..dashboard.options import model_supports_images +from ..utils.dashboard_handoff import ( # noqa: F401 + DASHBOARD_HANDOFF_INSTRUCTION, + DASHBOARD_HANDOFF_MARKER, +) from ..utils.http import DEFAULT_HTTP_TIMEOUT from ..utils.multimodal import fetch_image_block, vision_not_supported_warning 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.""" diff --git a/agent/utils/dashboard_handoff.py b/agent/utils/dashboard_handoff.py new file mode 100644 index 00000000..f99b5c9a --- /dev/null +++ b/agent/utils/dashboard_handoff.py @@ -0,0 +1,11 @@ +DASHBOARD_HANDOFF_OPEN_TAG = "" +DASHBOARD_HANDOFF_CLOSE_TAG = "" +DASHBOARD_HANDOFF_MARKER = DASHBOARD_HANDOFF_OPEN_TAG +DASHBOARD_HANDOFF_BODY = ( + "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." +) +DASHBOARD_HANDOFF_INSTRUCTION = ( + f"{DASHBOARD_HANDOFF_OPEN_TAG}\n{DASHBOARD_HANDOFF_BODY}\n{DASHBOARD_HANDOFF_CLOSE_TAG}" +) diff --git a/agent/utils/slack.py b/agent/utils/slack.py index 16f0a44d..b36b0953 100644 --- a/agent/utils/slack.py +++ b/agent/utils/slack.py @@ -346,6 +346,56 @@ async def post_slack_thread_reply_with_ts( return None, f"http_error: {type(exc).__name__}" +async def update_slack_message( + channel_id: str, + message_ts: str, + text: str, + *, + unfurl_links: bool = True, + unfurl_media: bool = True, + blocks: list[dict[str, Any]] | None = None, +) -> tuple[bool, str | None]: + """Update a Slack message and return success plus any Slack error.""" + if not SLACK_BOT_TOKEN: + return False, "missing_slack_bot_token" + + payload: dict[str, Any] = { + "channel": channel_id, + "ts": message_ts, + "text": text, + "unfurl_links": unfurl_links, + "unfurl_media": unfurl_media, + } + if blocks: + payload["blocks"] = blocks + + async with httpx.AsyncClient(timeout=DEFAULT_HTTP_TIMEOUT) as http_client: + try: + response = await http_client.post( + f"{SLACK_API_BASE_URL}/chat.update", + headers=_slack_headers(), + json=payload, + ) + if response.status_code == 429: + retry_after = response.headers.get("Retry-After") + logger.warning("Slack chat.update rate limited (retry-after=%s)", retry_after) + if retry_after: + return False, f"rate_limited: {retry_after}" + return False, "rate_limited" + response.raise_for_status() + data = response.json() + if not data.get("ok"): + error = data.get("error") + logger.warning("Slack chat.update failed: %s", error) + if error == "ratelimited": + return False, "rate_limited" + return False, error + return True, None + except httpx.HTTPError as exc: + logger.exception("Slack chat.update request failed") + return False, f"http_error: {type(exc).__name__}" + + async def post_slack_thread_reply(channel_id: str, thread_ts: str, text: str) -> bool: """Post a reply in a Slack thread.""" message_ts, _ = await post_slack_thread_reply_with_ts(channel_id, thread_ts, text) @@ -777,17 +827,24 @@ TRACE_REPLY_TIPS: tuple[str, ...] = ( "Ask me to search the web — I have a `web_search` tool for finding docs, examples, and GitHub repos mid-task.", "I can read, update, and create Linear issues directly — useful for filing follow-up tickets or linking work back to a project.", ) +TRACE_REPLY_WEB_HANDOFF_NOTICE = ( + "Conversation moved to Web — use the `Open in Web` link above for follow-ups." +) -def _format_trace_reply(trace_url: str | None, dashboard_url: str | None) -> str: - """Format the initial trace reply with a randomly selected tip.""" - tip = random.choice(TRACE_REPLY_TIPS) +def _format_trace_reply( + trace_url: str | None, dashboard_url: str | None, *, moved_to_web: bool = False +) -> str: + """Format the initial trace reply with status text.""" links = [] if trace_url: links.append(f"<{trace_url}|View trace>") if dashboard_url: links.append(f"<{dashboard_url}|Open in Web>") head = f"{' • '.join(links)}\n" if links else "" + if moved_to_web: + return f"{head}_{TRACE_REPLY_WEB_HANDOFF_NOTICE}_" + tip = random.choice(TRACE_REPLY_TIPS) return f"{head}_Tip: {tip}_" @@ -807,6 +864,29 @@ async def post_slack_trace_reply( return message_ts +async def update_slack_trace_reply_for_web_handoff( + channel_id: str, message_ts: str, thread_id: str +) -> bool: + """Update the initial Slack trace reply after a dashboard handoff.""" + trace_url = get_langsmith_trace_url(thread_id) + dashboard_url = dashboard_thread_url(thread_id) + ok, error = await update_slack_message( + channel_id, + message_ts, + _format_trace_reply(trace_url, dashboard_url, moved_to_web=True), + unfurl_links=False, + unfurl_media=False, + ) + if not ok: + logger.warning( + "Failed to update Slack trace reply for web handoff: channel=%s ts=%s error=%s", + channel_id, + message_ts, + error, + ) + return ok + + _SLACK_RUN_MAP_NAMESPACE = "slack_run_map" _THREAD_RUN_KEY_PREFIX = "thread:" _MESSAGE_RUN_KEY_PREFIX = "message:" @@ -830,12 +910,21 @@ async def store_slack_run_mapping( *, message_ts: str | None = None, triggering_user_id: str | None = None, + trace_message_ts: str | None = None, ) -> None: """Persist Slack thread/message to LangGraph run mapping.""" namespace = (_SLACK_RUN_MAP_NAMESPACE, channel_id) + if not trace_message_ts: + existing = await lookup_slack_thread_run_mapping(langgraph_client, channel_id, thread_ts) + if isinstance(existing, dict): + candidate = existing.get("trace_message_ts") + if isinstance(candidate, str) and candidate: + trace_message_ts = candidate value: dict[str, Any] = {"run_id": run_id, "thread_ts": thread_ts} if triggering_user_id: value["triggering_user_id"] = triggering_user_id + if trace_message_ts: + value["trace_message_ts"] = trace_message_ts try: await langgraph_client.store.put_item( namespace, f"{_THREAD_RUN_KEY_PREFIX}{thread_ts}", value @@ -876,12 +965,16 @@ async def store_slack_message_run_mapping( ) return triggering_user_id: str | None = None + trace_message_ts: str | None = None if isinstance(item, dict): value = item.get("value") if isinstance(value, dict): candidate = value.get("triggering_user_id") if isinstance(candidate, str) and candidate: triggering_user_id = candidate + candidate = value.get("trace_message_ts") + if isinstance(candidate, str) and candidate: + trace_message_ts = candidate await store_slack_run_mapping( langgraph_client, channel_id, @@ -889,6 +982,7 @@ async def store_slack_message_run_mapping( run_id, message_ts=message_ts, triggering_user_id=triggering_user_id, + trace_message_ts=trace_message_ts, ) except Exception: logger.exception( @@ -898,6 +992,30 @@ async def store_slack_message_run_mapping( ) +async def lookup_slack_thread_run_mapping( + langgraph_client: LangGraphClient, + channel_id: str, + thread_ts: str, +) -> dict[str, Any] | None: + """Return the stored mapping value for a Slack thread, or None.""" + namespace = (_SLACK_RUN_MAP_NAMESPACE, channel_id) + try: + item = await langgraph_client.store.get_item( + namespace, f"{_THREAD_RUN_KEY_PREFIX}{thread_ts}" + ) + except Exception: + logger.exception( + "Failed to look up Slack thread run mapping for channel=%s thread=%s", + channel_id, + thread_ts, + ) + return None + if not item: + return None + value = item.get("value") + return value if isinstance(value, dict) else None + + async def lookup_slack_run_mapping( langgraph_client: LangGraphClient, channel_id: str, diff --git a/agent/webhooks/slack.py b/agent/webhooks/slack.py index b587a86e..3632765b 100644 --- a/agent/webhooks/slack.py +++ b/agent/webhooks/slack.py @@ -337,6 +337,7 @@ async def _process_slack_mention_impl( run_id, message_ts=trace_message_ts, triggering_user_id=user_id, + trace_message_ts=trace_message_ts, ) else: webapp.logger.info( diff --git a/tests/test_dashboard_thread_api.py b/tests/test_dashboard_thread_api.py index e90582af..8e8fa9af 100644 --- a/tests/test_dashboard_thread_api.py +++ b/tests/test_dashboard_thread_api.py @@ -498,6 +498,98 @@ async def test_enrich_run_start_command_attributes_non_owner_message(monkeypatch assert last["content"] == "@teammate: fix the bug" +async def test_enrich_run_start_command_adds_web_handoff_for_slack_thread(monkeypatch) -> None: + class FakeThreads: + async def update(self, *, thread_id: str, metadata: dict[str, object]) -> None: + pass + + class FakeClient: + threads = FakeThreads() + + async def fake_get_profile(login: str) -> dict[str, object]: + return {} + + async def fake_ensure_token(login: str) -> None: + pass + + async def fake_resolve_email(login: str, profile: dict[str, object]) -> str: + return f"{login}@example.com" + + monkeypatch.setattr(thread_api, "langgraph_client", lambda: FakeClient()) + monkeypatch.setattr(thread_api, "get_profile", fake_get_profile) + monkeypatch.setattr(thread_api, "_ensure_dashboard_github_token", fake_ensure_token) + monkeypatch.setattr(thread_api, "_resolve_run_email", fake_resolve_email) + + command = { + "method": "run.start", + "params": {"input": {"messages": [{"role": "user", "content": "continue here"}]}}, + } + + enriched = await thread_api._enrich_run_start_command( + "tid", + "teammate", + command, + metadata={"source": "slack", "github_login": "owner"}, + email="teammate@example.com", + ) + + content = enriched["params"]["input"]["messages"][-1]["content"] + assert content[0] == {"type": "text", "text": thread_api.DASHBOARD_HANDOFF_INSTRUCTION} + assert content[1] == {"type": "text", "text": "@teammate: continue here"} + assert content[0]["text"].startswith("\n") + assert content[0]["text"].endswith("\n") + + +async def test_enrich_run_start_command_adds_web_handoff_before_image_blocks(monkeypatch) -> None: + class FakeThreads: + async def update(self, *, thread_id: str, metadata: dict[str, object]) -> None: + pass + + class FakeClient: + threads = FakeThreads() + + async def fake_get_profile(login: str) -> dict[str, object]: + return {} + + async def fake_ensure_token(login: str) -> None: + pass + + async def fake_resolve_email(login: str, profile: dict[str, object]) -> str: + return f"{login}@example.com" + + monkeypatch.setattr(thread_api, "langgraph_client", lambda: FakeClient()) + monkeypatch.setattr(thread_api, "get_profile", fake_get_profile) + monkeypatch.setattr(thread_api, "_ensure_dashboard_github_token", fake_ensure_token) + monkeypatch.setattr(thread_api, "_resolve_run_email", fake_resolve_email) + + command = { + "method": "run.start", + "params": { + "input": { + "messages": [ + { + "role": "user", + "content": [{"type": "text", "text": "continue here"}], + } + ] + } + }, + } + + enriched = await thread_api._enrich_run_start_command( + "tid", + "teammate", + command, + metadata={"source": "slack", "github_login": "owner"}, + email="teammate@example.com", + ) + + content = enriched["params"]["input"]["messages"][-1]["content"] + assert content[0] == {"type": "text", "text": thread_api.DASHBOARD_HANDOFF_INSTRUCTION} + assert content[1] == {"type": "text", "text": "@teammate:"} + assert content[2] == {"type": "text", "text": "continue here"} + + async def test_enrich_run_start_command_does_not_attribute_owner_message(monkeypatch) -> None: class FakeThreads: async def update(self, *, thread_id: str, metadata: dict[str, object]) -> None: @@ -602,6 +694,100 @@ async def test_enrich_run_start_command_allowlists_client_configurable(monkeypat assert updates[-1]["model"] == _VISION_MODEL +async def test_proxy_run_start_from_slack_thread_updates_trace_reply(monkeypatch) -> None: + captured: dict[str, object] = {} + + class FakeThreads: + async def get(self, thread_id: str) -> dict[str, object]: + assert thread_id == "tid" + return { + "thread_id": "tid", + "metadata": { + "source": "slack", + "github_login": "octocat", + "source_context": { + "slack_thread": { + "channel_id": "C1", + "thread_ts": "123.45", + "trace_message_ts": "123.46", + } + }, + }, + "status": "idle", + } + + async def update(self, *, thread_id: str, metadata: dict[str, object]) -> None: + captured.setdefault("updates", []).append(metadata) + + class FakeClient: + threads = FakeThreads() + + class FakeResponse: + status_code = 200 + content = b'{"run_id":"run-1"}' + headers = {"content-type": "application/json"} + + class FakeAsyncClient: + def __init__(self, *a: object, **kw: object) -> None: + pass + + async def __aenter__(self) -> "FakeAsyncClient": + return self + + async def __aexit__(self, *a: object) -> None: + pass + + async def post(self, url: str, *, content: bytes, headers: dict[str, str]) -> FakeResponse: + captured["url"] = url + captured["outgoing"] = json.loads(content) + return FakeResponse() + + async def fake_get_profile(login: str) -> dict[str, object]: + return {} + + async def fake_ensure_token(login: str) -> None: + pass + + async def fake_resolve_email(login: str, profile: dict[str, object]) -> str: + return f"{login}@example.com" + + async def fake_update_trace_reply(channel_id: str, message_ts: str, thread_id: str) -> bool: + captured["handoff_update"] = { + "channel_id": channel_id, + "message_ts": message_ts, + "thread_id": thread_id, + } + return True + + monkeypatch.setattr(thread_api, "langgraph_client", lambda: FakeClient()) + monkeypatch.setattr(thread_api, "get_profile", fake_get_profile) + monkeypatch.setattr(thread_api, "_ensure_dashboard_github_token", fake_ensure_token) + monkeypatch.setattr(thread_api, "_resolve_run_email", fake_resolve_email) + monkeypatch.setattr(thread_api.httpx, "AsyncClient", FakeAsyncClient) + monkeypatch.setattr( + thread_api, "update_slack_trace_reply_for_web_handoff", fake_update_trace_reply + ) + + status, body, _ = await thread_api.proxy_dashboard_thread_commands( + "tid", + "octocat", + b'{"method":"run.start","params":{"input":{"messages":[{"role":"user","content":"continue here"}]}}}', + ) + + assert status == 200 + assert body == b'{"run_id":"run-1"}' + outgoing = captured["outgoing"] + assert isinstance(outgoing, dict) + content = outgoing["params"]["input"]["messages"][-1]["content"] + assert content[0] == {"type": "text", "text": thread_api.DASHBOARD_HANDOFF_INSTRUCTION} + assert content[1] == {"type": "text", "text": "continue here"} + assert captured["handoff_update"] == { + "channel_id": "C1", + "message_ts": "123.46", + "thread_id": "tid", + } + + async def test_proxy_commands_rejects_non_object_body(monkeypatch) -> None: class FakeThreads: async def get(self, thread_id: str) -> dict[str, object]: diff --git a/tests/test_dashboard_web_handoff.py b/tests/test_dashboard_web_handoff.py index 5d30841b..c1d896f5 100644 --- a/tests/test_dashboard_web_handoff.py +++ b/tests/test_dashboard_web_handoff.py @@ -30,10 +30,25 @@ class _FakeRuns: return {"run_id": "run-1"} +class _FakeStore: + def __init__( + self, items: dict[tuple[tuple[str, ...], str], dict[str, Any]] | None = None + ) -> None: + self.items = items or {} + + async def get_item(self, namespace: tuple[str, ...], key: str) -> dict[str, Any] | None: + return self.items.get((namespace, key)) + + class _FakeClient: - def __init__(self, metadata: dict[str, Any]) -> None: + def __init__( + self, + metadata: dict[str, Any], + store_items: dict[tuple[tuple[str, ...], str], dict[str, Any]] | None = None, + ) -> None: self.threads = _FakeThreads(metadata) self.runs = _FakeRuns() + self.store = _FakeStore(store_items) async def _inactive_thread(thread_id: str) -> bool: @@ -162,6 +177,104 @@ async def test_dashboard_followup_on_busy_thread_queues_dashboard_handoff( assert queued_messages == [{"text": "continue in web", "source": "dashboard"}] +@pytest.mark.asyncio +async def test_dashboard_followup_on_busy_slack_thread_updates_trace_reply( + monkeypatch: pytest.MonkeyPatch, +) -> None: + metadata = { + "source": "slack", + "github_login": "octocat", + "triggering_user_email": "octocat@example.com", + "source_context": { + "slack_thread": { + "channel_id": "C1", + "thread_ts": "123.45", + "trace_message_ts": "123.46", + } + }, + } + client = _FakeClient(metadata) + queued_messages: list[object] = [] + handoff_updates: list[dict[str, str]] = [] + + async def fake_queue_message_for_thread(thread_id: str, message_content: object) -> bool: + queued_messages.append(message_content) + return True + + async def fake_update_trace_reply(channel_id: str, message_ts: str, thread_id: str) -> bool: + handoff_updates.append( + {"channel_id": channel_id, "message_ts": message_ts, "thread_id": thread_id} + ) + return True + + monkeypatch.setattr(thread_api, "langgraph_client", lambda: client) + monkeypatch.setattr(thread_api, "get_thread_active_status", _active_thread) + monkeypatch.setattr(thread_api, "queue_message_for_thread", fake_queue_message_for_thread) + monkeypatch.setattr( + thread_api, "update_slack_trace_reply_for_web_handoff", fake_update_trace_reply + ) + + await thread_api.send_dashboard_message( + "thread-1", + "octocat", + thread_api.ThreadMessageBody(content="continue in web"), + email="octocat@example.com", + ) + + assert queued_messages == [{"text": "continue in web", "source": "dashboard"}] + assert handoff_updates == [ + {"channel_id": "C1", "message_ts": "123.46", "thread_id": "thread-1"} + ] + + +@pytest.mark.asyncio +async def test_dashboard_followup_uses_stored_trace_reply_timestamp( + monkeypatch: pytest.MonkeyPatch, +) -> None: + metadata = { + "source": "slack", + "github_login": "octocat", + "triggering_user_email": "octocat@example.com", + "source_context": {"slack_thread": {"channel_id": "C1", "thread_ts": "123.45"}}, + } + client = _FakeClient( + metadata, + { + (("slack_run_map", "C1"), "thread:123.45"): { + "value": {"run_id": "run-1", "thread_ts": "123.45", "trace_message_ts": "123.46"} + } + }, + ) + handoff_updates: list[dict[str, str]] = [] + + async def fake_queue_message_for_thread(thread_id: str, message_content: object) -> bool: + return True + + async def fake_update_trace_reply(channel_id: str, message_ts: str, thread_id: str) -> bool: + handoff_updates.append( + {"channel_id": channel_id, "message_ts": message_ts, "thread_id": thread_id} + ) + return True + + monkeypatch.setattr(thread_api, "langgraph_client", lambda: client) + monkeypatch.setattr(thread_api, "get_thread_active_status", _active_thread) + monkeypatch.setattr(thread_api, "queue_message_for_thread", fake_queue_message_for_thread) + monkeypatch.setattr( + thread_api, "update_slack_trace_reply_for_web_handoff", fake_update_trace_reply + ) + + await thread_api.send_dashboard_message( + "thread-1", + "octocat", + thread_api.ThreadMessageBody(content="continue in web"), + email="octocat@example.com", + ) + + assert handoff_updates == [ + {"channel_id": "C1", "message_ts": "123.46", "thread_id": "thread-1"} + ] + + @pytest.mark.asyncio async def test_dashboard_followup_on_busy_thread_queues_images( monkeypatch: pytest.MonkeyPatch, diff --git a/tests/test_slack_assistants_status.py b/tests/test_slack_assistants_status.py index 0e75f00b..4bdea0e1 100644 --- a/tests/test_slack_assistants_status.py +++ b/tests/test_slack_assistants_status.py @@ -149,6 +149,30 @@ async def test_post_slack_thread_reply_does_not_call_set_status( assert client_cm.post.call_args.args[0].endswith("/chat.postMessage") +@pytest.mark.asyncio +async def test_update_slack_message_calls_chat_update( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr(slack_utils, "SLACK_BOT_TOKEN", "xoxb-test") + + client_cm = _async_client_cm(_ok_response()) + with patch.object(slack_utils.httpx, "AsyncClient", return_value=client_cm): + result = await slack_utils.update_slack_message( + "C1", "1.1", "moved", unfurl_links=False, unfurl_media=False + ) + + assert result == (True, None) + assert client_cm.post.await_count == 1 + assert client_cm.post.call_args.args[0].endswith("/chat.update") + assert client_cm.post.call_args.kwargs["json"] == { + "channel": "C1", + "ts": "1.1", + "text": "moved", + "unfurl_links": False, + "unfurl_media": False, + } + + @pytest.mark.asyncio async def test_post_slack_thread_reply_with_ts_returns_missing_token_error( monkeypatch: pytest.MonkeyPatch, diff --git a/tests/test_slack_context.py b/tests/test_slack_context.py index 5354a481..090e316a 100644 --- a/tests/test_slack_context.py +++ b/tests/test_slack_context.py @@ -45,6 +45,22 @@ class _FakeClient: self.threads = threads_client +class _FakeSlackMappingStore: + def __init__(self) -> None: + self.items: dict[tuple[tuple[str, ...], str], dict] = {} + + async def put_item(self, namespace: tuple[str, ...], key: str, value: dict) -> None: + self.items[(namespace, key)] = {"value": value} + + async def get_item(self, namespace: tuple[str, ...], key: str) -> dict | None: + return self.items.get((namespace, key)) + + +class _FakeSlackMappingClient: + def __init__(self) -> None: + self.store = _FakeSlackMappingStore() + + def test_generate_thread_id_from_slack_thread_is_deterministic() -> None: channel_id = "C12345" thread_ts = "1730900000.123456" @@ -54,6 +70,63 @@ def test_generate_thread_id_from_slack_thread_is_deterministic() -> None: assert len(first) == 36 +@pytest.mark.asyncio +async def test_slack_run_mapping_preserves_trace_message_ts() -> None: + client = _FakeSlackMappingClient() + + await slack_utils.store_slack_run_mapping( + client, + "C123", + "1.0", + "run-1", + message_ts="1.1", + triggering_user_id="U123", + trace_message_ts="1.1", + ) + await slack_utils.store_slack_message_run_mapping(client, "C123", "1.0", "1.2") + + thread_mapping = await slack_utils.lookup_slack_thread_run_mapping(client, "C123", "1.0") + message_mapping = await slack_utils.lookup_slack_run_mapping(client, "C123", "1.2") + assert thread_mapping is not None + assert thread_mapping["trace_message_ts"] == "1.1" + assert thread_mapping["triggering_user_id"] == "U123" + assert message_mapping is not None + assert message_mapping["trace_message_ts"] == "1.1" + assert message_mapping["message_ts"] == "1.2" + + +@pytest.mark.asyncio +async def test_slack_run_mapping_preserves_trace_message_ts_on_followup_mention() -> None: + """A subsequent Slack mention without trace_message_ts must not clobber the stored timestamp.""" + client = _FakeSlackMappingClient() + + # First mention stores the trace message ts. + await slack_utils.store_slack_run_mapping( + client, + "C123", + "1.0", + "run-1", + message_ts="1.1", + triggering_user_id="U123", + trace_message_ts="1.1", + ) + + # Follow-up mention (non-first) stores a new run_id without trace_message_ts. + await slack_utils.store_slack_run_mapping( + client, + "C123", + "1.0", + "run-2", + triggering_user_id="U456", + ) + + thread_mapping = await slack_utils.lookup_slack_thread_run_mapping(client, "C123", "1.0") + assert thread_mapping is not None + assert thread_mapping["run_id"] == "run-2" + assert thread_mapping["trace_message_ts"] == "1.1" + assert thread_mapping["triggering_user_id"] == "U456" + + def test_select_slack_context_messages_uses_thread_start_when_no_prior_mention() -> None: bot_user_id = "UBOT" messages = [ @@ -248,6 +321,60 @@ def test_post_slack_trace_reply_includes_trace_link_and_tip( assert posted[0]["unfurl_media"] is False +def test_format_trace_reply_can_show_web_handoff_notice() -> None: + text = slack_utils._format_trace_reply( + "https://smith/x", "https://app.example.com/agents/thread-id", moved_to_web=True + ) + + head, _, notice_line = text.partition("\n") + assert ( + head + == " • " + ) + assert "Conversation moved to Web" in notice_line + assert "Tip:" not in notice_line + + +@pytest.mark.asyncio +async def test_update_slack_trace_reply_for_web_handoff_updates_existing_message( + monkeypatch: pytest.MonkeyPatch, +) -> None: + updated: list[dict] = [] + + async def fake_update_slack_message( + channel_id: str, + message_ts: str, + text: str, + *, + unfurl_links: bool = True, + unfurl_media: bool = True, + ) -> tuple[bool, str | None]: + updated.append( + { + "channel_id": channel_id, + "message_ts": message_ts, + "text": text, + "unfurl_links": unfurl_links, + "unfurl_media": unfurl_media, + } + ) + return True, None + + monkeypatch.setenv("DASHBOARD_BASE_URL", "https://app.example.com") + monkeypatch.setattr(slack_utils, "update_slack_message", fake_update_slack_message) + monkeypatch.setattr(slack_utils, "get_langsmith_trace_url", lambda thread_id: "https://smith/x") + + ok = await slack_utils.update_slack_trace_reply_for_web_handoff("C123", "1.1", "thread-id") + + assert ok is True + assert len(updated) == 1 + assert updated[0]["channel_id"] == "C123" + assert updated[0]["message_ts"] == "1.1" + assert "Conversation moved to Web" in updated[0]["text"] + assert updated[0]["unfurl_links"] is False + assert updated[0]["unfurl_media"] is False + + def test_post_slack_trace_reply_can_skip_web_link( monkeypatch: pytest.MonkeyPatch, ) -> None: