From 96cceb741dd3073b7e42a73dcc8de9d2f9dde23b Mon Sep 17 00:00:00 2001 From: Ramon Nogueira Date: Mon, 29 Jun 2026 15:23:40 -0400 Subject: [PATCH] feat: notify Slack on plan approval (#1632) * feat: notify Slack on plan approval Co-authored-by: open-swe[bot] * fix: post Slack approval notice after successful dispatch Move the _maybe_post_plan_approved_to_slack call until after _dispatch_followup succeeds so the Slack thread is not told implementation is beginning before the LangGraph run is created. Addresses PR review comment. --------- Co-authored-by: Ramon Nogueira <270434257+ramon-langchain@users.noreply.github.com> Co-authored-by: open-swe[bot] Co-authored-by: Johannes du Plessis --- agent/dashboard/plan_api.py | 54 +++++++++++++++++++++++++++++- tests/test_plan_review.py | 65 +++++++++++++++++++++++++++++++++++++ 2 files changed, 118 insertions(+), 1 deletion(-) diff --git a/agent/dashboard/plan_api.py b/agent/dashboard/plan_api.py index 3dbec608..1dcbb34a 100644 --- a/agent/dashboard/plan_api.py +++ b/agent/dashboard/plan_api.py @@ -22,6 +22,7 @@ from langgraph_sdk import get_client from pydantic import BaseModel from ..dispatch import dispatch_agent_run +from ..utils.slack import post_slack_thread_reply from .oauth import require_same_origin_for_mutations, require_session from .plan_store import ( PLAN_STATUS_APPROVED, @@ -178,7 +179,8 @@ async def approve_plan(thread_id: str, session: dict[str, Any] = _SESSION_DEP) - # strictly so a transient failure can't silently drop the edit. content = await get_plan_content(thread_id, raise_on_error=True) or {} plan_markdown = str(content.get("markdown", "")).strip() - feedback = _format_comments(await list_plan_comments(thread_id, raise_on_error=True)) + comments = await list_plan_comments(thread_id, raise_on_error=True) + feedback = _format_comments(comments) await set_plan_status(thread_id, PLAN_STATUS_APPROVED, plan_mode=False) if plan_markdown: text = ( @@ -191,6 +193,11 @@ async def approve_plan(thread_id: str, session: dict[str, Any] = _SESSION_DEP) - if feedback: text += "\n\nAlso take this reviewer feedback into account:\n\n" + feedback await _dispatch_followup(thread_id, metadata, text, plan_mode=False) + await _maybe_post_plan_approved_to_slack( + metadata, + comment_count=len(comments), + actor=_approval_actor_name(session), + ) return {"status": PLAN_STATUS_APPROVED} @@ -210,6 +217,51 @@ async def reject_plan(thread_id: str, session: dict[str, Any] = _SESSION_DEP) -> return {"status": PLAN_STATUS_REVISING} +def _approval_actor_name(session: dict[str, Any]) -> str: + actor = session.get("name") or session.get("sub") or "User" + return str(actor).strip() or "User" + + +def _slack_thread_from_metadata(metadata: dict[str, Any]) -> tuple[str, str] | None: + source_context = metadata.get("source_context") + if not isinstance(source_context, dict): + return None + slack_thread = source_context.get("slack_thread") + if not isinstance(slack_thread, dict): + return None + channel_id = slack_thread.get("channel_id") + thread_ts = slack_thread.get("thread_ts") + if not isinstance(channel_id, str) or not channel_id.strip(): + return None + if not isinstance(thread_ts, str) or not thread_ts.strip(): + return None + return channel_id.strip(), thread_ts.strip() + + +def _plan_approved_slack_text(comment_count: int, actor: str) -> str: + return f"Plan approved with {comment_count} comments by {actor}\nbeginning implementation" + + +async def _maybe_post_plan_approved_to_slack( + metadata: dict[str, Any], *, comment_count: int, actor: str +) -> None: + slack_thread = _slack_thread_from_metadata(metadata) + if slack_thread is None: + return + channel_id, thread_ts = slack_thread + try: + ok = await post_slack_thread_reply( + channel_id, + thread_ts, + _plan_approved_slack_text(comment_count, actor), + ) + except Exception: + logger.warning("Could not post plan approval Slack reply", exc_info=True) + return + if not ok: + logger.warning("Could not post plan approval Slack reply to %s/%s", channel_id, thread_ts) + + def _format_comments(comments: list[dict[str, Any]]) -> str: lines: list[str] = [] index = 1 diff --git a/tests/test_plan_review.py b/tests/test_plan_review.py index 5175e68a..b308100e 100644 --- a/tests/test_plan_review.py +++ b/tests/test_plan_review.py @@ -39,6 +39,15 @@ def test_format_comments_empty() -> None: assert _format_comments([]) == "" +def test_plan_approved_slack_text_mentions_comments_actor_and_start() -> None: + from agent.dashboard.plan_api import _plan_approved_slack_text + + assert ( + _plan_approved_slack_text(2, "Alice") + == "Plan approved with 2 comments by Alice\nbeginning implementation" + ) + + def test_plan_comment_helpers_exported() -> None: from agent.dashboard import plan_store @@ -352,6 +361,62 @@ async def test_approve_plan_dispatches_published_markdown( assert dispatched["plan_mode"] is False +async def test_approve_plan_posts_slack_approval_notice( + monkeypatch: pytest.MonkeyPatch, +) -> None: + from agent.dashboard import plan_api + + posted: dict[str, Any] = {} + dispatched: dict[str, Any] = {} + + async def fake_meta(thread_id: str) -> dict[str, Any]: + return { + "plan_status": "ready", + "source_context": {"slack_thread": {"channel_id": "C1", "thread_ts": "123.45"}}, + } + + async def fake_get_content(thread_id: str, *, raise_on_error: bool = False) -> dict[str, Any]: + return {"markdown": "# Plan", "status": "ready"} + + async def fake_list(thread_id: str, *, raise_on_error: bool = False) -> list[dict[str, Any]]: + return [ + {"author": "alice", "body": "looks good"}, + {"author": "bob", "body": "add a test"}, + ] + + async def fake_set_status(thread_id: str, status: str, *, plan_mode: Any = None) -> None: + return None + + async def fake_post(channel_id: str, thread_ts: str, text: str) -> bool: + posted.update(channel_id=channel_id, thread_ts=thread_ts, text=text) + return True + + async def fake_dispatch( + thread_id: str, metadata: dict[str, Any], text: str, *, plan_mode: bool + ) -> None: + dispatched.update(text=text, plan_mode=plan_mode) + + monkeypatch.setattr(plan_api, "_thread_metadata", fake_meta) + monkeypatch.setattr(plan_api, "_user_owns_thread", lambda *a, **k: True) + monkeypatch.setattr(plan_api, "get_plan_content", fake_get_content) + monkeypatch.setattr(plan_api, "list_plan_comments", fake_list) + monkeypatch.setattr(plan_api, "set_plan_status", fake_set_status) + monkeypatch.setattr(plan_api, "post_slack_thread_reply", fake_post) + monkeypatch.setattr(plan_api, "_dispatch_followup", fake_dispatch) + + result = await plan_api.approve_plan( + "t1", session={"sub": "alice", "email": None, "name": "Alice Example"} + ) + + assert result["status"] == "approved" + assert posted == { + "channel_id": "C1", + "thread_ts": "123.45", + "text": "Plan approved with 2 comments by Alice Example\nbeginning implementation", + } + assert dispatched["plan_mode"] is False + + async def test_approve_plan_aborts_when_plan_read_fails( monkeypatch: pytest.MonkeyPatch, ) -> None: