open-swe/tests/test_plan_review.py
Adam Moussa b3b0274403
feat: Re-land deferred upstream features on modular webhooks (#80) (#128)
* fix(webhooks): fall back to vision model for Slack/Linear image threads

Re-land upstream #1626 onto the modular webhook structure. When a
Slack mention or Linear issue carries images but the resolved model is
text-only, fall back to a vision-capable model instead of dropping the
images. Re-points default_vision_model_pair at the fork's image-capable
models (Opus 4.8 default, else any supports_images model) rather than
upstream's openai:/anthropic: provider filter.

Refs #80, upstream #1626

* fix(slack): persist trace_message_ts so web-handoff updates the trace reply

Re-land upstream #1630 onto the modular structure. The first-mention
store_slack_run_mapping call did not pass trace_message_ts, so it was
never persisted (nothing to preserve from on first mention) and
_notify_slack_web_handoff always skipped the trace-reply update on web
handoff. Pass it through and cover it with a test.

Refs #80, upstream #1630

* feat(slack): include channel context in Slack prompts

Re-land upstream #1633 onto the modular structure. Fetch cached Slack
channel metadata once per event (_get_slack_channel_context) and thread
it through the docs-plz gate, repo resolution, and process_slack_mention
so prompts carry the channel name and a clearly-marked untrusted
channel description. Avoids duplicate conversations.info calls.

Refs #80, upstream #1633

* feat(tools): add slack_start_new_thread breakout tool

Re-land upstream #1638 onto the modular structure. Adds the
slack_start_new_thread tool (posts a top-level Slack message and
dispatches a fresh agent run for a broken-out task via the durable
dispatch_agent_run contract), wires it into the agent tool list and
tools/__init__, adds prompt guidance, and excludes it from plan mode so
it can't bypass the approval flow. Tool imports only live modules.

Refs #80, upstream #1638

* feat(plan): notify Slack on plan approval

Re-land upstream #1632 onto the modular structure. When a plan is
approved via the dashboard approve endpoint, post a thread reply to the
originating Slack thread noting the comment count and approver, after
the follow-up run is dispatched. Slack post failures never break
approval. Adapted to the fork's approve_plan (no plan_markdown read).

Refs #80, upstream #1632

* feat(plan): publish plans from sandbox files

Re-land upstream #1635 onto the modular structure, completing the
partially-ported change so dev is internally consistent. save_plan now
takes a plan_file_path, reads the agent-authored Markdown file from
/workspace/plans/ (validating extension/location/UTF-8/size) and
publishes it, instead of taking a plan_markdown string. Removes
write_file/edit_file from PLAN_MODE_EXCLUDED_TOOLS so the agent can
author the plan file, updates enter_plan_mode/reject_plan guidance and
the e2e fake LLM. Skips the #1610-only update_plan hunk (not on dev).

Refs #80, upstream #1635

* fix(security): SSRF-harden server-side image fetch + stop logging raw image URLs

INJ-01 (high): fetch_image_block used follow_redirects=True with no per-hop
revalidation and discarded the resolved-IP pin, so an attacker-authored Slack/
Linear image URL could 302-redirect the fetch to an internal host / cloud
metadata endpoint (blind SSRF), and DNS-rebinding could bypass the one-shot
is_url_safe check. Route image fetches through the same per-hop resolve+pin+
revalidate loop the http_request tool uses, lifted into url_safety as the shared
request_with_safe_redirects. Also strip the per-host Slack/Linear bearer token
on redirect so it can't be replayed to a redirect target.

SC-1 (low): linear.py logged full image URLs (which can carry signed tokens) at
DEBUG; multimodal logged them at INFO on every fetch. Log host-only.

Sink lived in multimodal.py (unchanged by the feature work) but PR #128 widened
its reach by no longer dropping images for text-only models. Fixing on the base
branch so #130/#129 inherit it on rebase. Adds fetch_image_block SSRF regression
tests (redirect-to-internal blocked; auth stripped on redirect).
2026-07-08 18:32:43 -04:00

348 lines
12 KiB
Python

from __future__ import annotations
from typing import Any
import pytest
def test_dashboard_plan_url_uses_plan_path(monkeypatch: pytest.MonkeyPatch) -> None:
monkeypatch.setenv("DASHBOARD_BASE_URL", "https://example.test")
from agent.utils.dashboard_links import dashboard_plan_url
assert dashboard_plan_url("abc-123") == "https://example.test/agents/abc-123/plan"
def test_dashboard_plan_url_none_without_thread() -> None:
from agent.utils.dashboard_links import dashboard_plan_url
assert dashboard_plan_url("") is None
def test_format_comments_numbers_and_skips_blank() -> None:
from agent.dashboard.plan_api import _format_comments
text = _format_comments(
[
{"author": "alice", "body": "add a docstring"},
{"author": "bob", "body": "looks good"},
{"author": "carol", "body": " "}, # blank → skipped
]
)
assert "1. alice: add a docstring" in text
assert "2. bob: looks good" in text
assert "carol" not in text
def test_format_comments_empty() -> None:
from agent.dashboard.plan_api import _format_comments
assert _format_comments([]) == ""
def test_plan_comment_helpers_exported() -> None:
from agent.dashboard import plan_store
assert plan_store.PLAN_COMMENTS_NAMESPACE == ["plan", "comments"]
assert callable(plan_store.add_plan_comment)
assert callable(plan_store.list_plan_comments)
assert callable(plan_store.delete_plan_comment)
assert callable(plan_store.clear_plan_comments)
def _fake_client(store: Any) -> Any:
return type("C", (), {"store": store})()
async def test_list_plan_comments_swallows_errors_by_default(
monkeypatch: pytest.MonkeyPatch,
) -> None:
from agent.dashboard import plan_store
class _Store:
async def search_items(self, *a: Any, **k: Any) -> Any:
raise RuntimeError("boom")
monkeypatch.setattr(plan_store, "_client", lambda: _fake_client(_Store()))
assert await plan_store.list_plan_comments("t") == []
async def test_list_plan_comments_raises_with_flag(monkeypatch: pytest.MonkeyPatch) -> None:
from agent.dashboard import plan_store
class _Store:
async def search_items(self, *a: Any, **k: Any) -> Any:
raise RuntimeError("boom")
monkeypatch.setattr(plan_store, "_client", lambda: _fake_client(_Store()))
with pytest.raises(RuntimeError):
await plan_store.list_plan_comments("t", raise_on_error=True)
async def test_clear_plan_comments_deletes_each(monkeypatch: pytest.MonkeyPatch) -> None:
from agent.dashboard import plan_store
deleted: list[str] = []
class _Store:
async def search_items(self, *a: Any, **k: Any) -> Any:
return {"items": [{"value": {"id": "a"}}, {"value": {"id": "b"}}]}
async def delete_item(self, _ns: Any, key: str) -> None:
deleted.append(key)
monkeypatch.setattr(plan_store, "_client", lambda: _fake_client(_Store()))
await plan_store.clear_plan_comments("t")
assert deleted == ["a", "b"]
async def test_save_plan_requires_run_context() -> None:
from agent.tools.save_plan import save_plan
# No LangGraph run context → no thread_id → graceful error, not a crash.
result = await save_plan("/workspace/plans/2026-07-08-test-plan.md")
assert result["success"] is False
assert "thread_id" in result["error"]
async def test_save_plan_rejects_empty_path() -> None:
from agent.tools.save_plan import save_plan
result = await save_plan(" ")
assert result["success"] is False
assert "empty" in result["error"]
async def test_save_plan_rejects_non_markdown_path() -> None:
from agent.tools.save_plan import save_plan
result = await save_plan("/workspace/plans/plan.txt")
assert result["success"] is False
assert "Markdown" in result["error"]
async def test_save_plan_rejects_markdown_outside_plans_dir() -> None:
from agent.tools.save_plan import save_plan
result = await save_plan("/workspace/plan.md")
assert result["success"] is False
assert "/workspace/plans" in result["error"]
async def test_save_plan_reads_markdown_file_from_sandbox(
monkeypatch: pytest.MonkeyPatch,
) -> None:
import importlib
save_plan_tool = importlib.import_module("agent.tools.save_plan")
saved: dict[str, Any] = {}
reads: list[tuple[str, int, int]] = []
class _Backend:
async def aread(self, file_path: str, offset: int = 0, limit: int = 2000) -> dict[str, Any]:
reads.append((file_path, offset, limit))
return {"file_data": {"encoding": "utf-8", "content": "# Plan\n\nDo it.\n"}}
async def fake_backend(thread_id: str) -> _Backend:
assert thread_id == "thread-1"
return _Backend()
async def fake_save_content(
thread_id: str, *, markdown: str, status: str, plan_file_path: str | None = None
) -> None:
saved.update(
thread_id=thread_id, markdown=markdown, status=status, plan_file_path=plan_file_path
)
monkeypatch.setattr(
save_plan_tool,
"get_config",
lambda: {"configurable": {"thread_id": "thread-1"}},
)
monkeypatch.setattr(save_plan_tool, "get_sandbox_backend", fake_backend)
monkeypatch.setattr(save_plan_tool, "save_plan_content", fake_save_content)
result = await save_plan_tool.save_plan("/workspace/plans/2026-07-08-test-plan.md")
assert result == {"success": True, "path": "/workspace/plans/2026-07-08-test-plan.md"}
assert reads == [
("/workspace/plans/2026-07-08-test-plan.md", 0, save_plan_tool._MAX_PLAN_LINES)
]
assert saved == {
"thread_id": "thread-1",
"markdown": "# Plan\n\nDo it.",
"status": "ready",
"plan_file_path": "/workspace/plans/2026-07-08-test-plan.md",
}
def test_plan_routes_registered() -> None:
from agent.webapp import app
paths = set()
for route in app.routes:
if hasattr(route, "path"):
paths.add(route.path)
included = getattr(route, "original_router", None)
if included is not None:
paths.update(r.path for r in included.routes if hasattr(r, "path"))
assert "/dashboard/api/plan/{thread_id}" in paths
assert "/dashboard/api/plan/{thread_id}/approve" in paths
assert "/dashboard/api/plan/{thread_id}/reject" in paths
assert "/dashboard/api/plan/{thread_id}/comments" in paths
assert "/dashboard/api/plan/{thread_id}/comments/{comment_id}" in paths
assert "/dashboard/api/plan/yjs/{thread_id}" not in paths
def test_save_plan_exported_and_wired() -> None:
from agent.tools import save_plan
assert callable(save_plan)
def test_plan_status_constants() -> None:
from agent.dashboard import plan_store
assert plan_store.PLAN_STATUS_READY == "ready"
assert plan_store.PLAN_STATUS_PLANNING == "planning"
assert plan_store.PLAN_STATUS_APPROVED == "approved"
assert plan_store.PLAN_STATUS_REVISING == "revising"
def test_plan_file_path_for_thread_uses_plans_dir_and_slug() -> None:
from agent.dashboard import plan_store
path = plan_store.plan_file_path_for_thread("Thread ABC/123")
assert path.startswith("/workspace/plans/")
assert path.endswith("-thread-abc-123.md")
def test_http_request_excluded_in_plan_mode() -> None:
from agent.server import PLAN_MODE_EXCLUDED_TOOLS
assert "http_request" in PLAN_MODE_EXCLUDED_TOOLS
def test_file_edit_tools_available_in_plan_mode_for_plan_file() -> None:
from agent.server import PLAN_MODE_EXCLUDED_TOOLS
assert "write_file" not in PLAN_MODE_EXCLUDED_TOOLS
assert "edit_file" not in PLAN_MODE_EXCLUDED_TOOLS
class _FakeReq:
def __init__(self, tools: list[Any], state: dict[str, Any]) -> None:
self.tools = tools
self.state = state
def override(self, **kw: Any) -> _FakeReq:
return _FakeReq(kw.get("tools", self.tools), self.state)
def _names(req: _FakeReq) -> set[str]:
return {t["name"] for t in req.tools}
def test_plan_mode_middleware_initial_always_filters() -> None:
from agent.middleware import PlanModeMiddleware
mw = PlanModeMiddleware(excluded=frozenset({"write_file"}), initial=True)
req = _FakeReq([{"name": "read_file"}, {"name": "write_file"}], {})
assert _names(mw._filter(req)) == {"read_file"}
def test_plan_mode_middleware_self_activation_via_state() -> None:
from agent.middleware import PlanModeMiddleware
mw = PlanModeMiddleware(excluded=frozenset({"write_file"}), initial=False)
# Plan mode not yet active: nothing filtered.
off = _FakeReq([{"name": "read_file"}, {"name": "write_file"}], {})
assert _names(mw._filter(off)) == {"read_file", "write_file"}
# After enter_plan_mode sets state: the next request is filtered.
on = _FakeReq([{"name": "read_file"}, {"name": "write_file"}], {"plan_mode": True})
assert _names(mw._filter(on)) == {"read_file"}
def test_plan_approved_slack_text_mentions_comments_actor_and_start() -> None:
from agent.dashboard.plan_api import _plan_approved_slack_text
text = _plan_approved_slack_text(3, "Alice")
assert text == "Plan approved with 3 comments by Alice\nbeginning implementation"
async def test_approve_plan_posts_slack_approval_notice(monkeypatch: pytest.MonkeyPatch) -> None:
from agent.dashboard import plan_api
metadata = {
"github_login": "alice",
"source_context": {"slack_thread": {"channel_id": "C1", "thread_ts": "1700000000.0001"}},
}
posted: dict[str, Any] = {}
async def fake_thread_metadata(thread_id: str) -> dict[str, Any]:
return metadata
def fake_user_owns_thread(md: dict, sub: str, email: str | None) -> bool:
return True
async def fake_list_plan_comments(
thread_id: str, *, raise_on_error: bool = False
) -> list[dict]:
return [{"author": "bob", "body": "tweak this"}]
async def fake_set_plan_status(thread_id: str, status: str, *, plan_mode: bool) -> None:
posted["status"] = {"status": status, "plan_mode": plan_mode}
async def fake_dispatch_followup(
thread_id: str, md: dict, text: str, *, plan_mode: bool
) -> None:
posted["dispatch"] = {"text": text, "plan_mode": plan_mode}
async def fake_post_slack_thread_reply(channel_id: str, thread_ts: str, text: str) -> bool:
posted["slack"] = {"channel_id": channel_id, "thread_ts": thread_ts, "text": text}
return True
monkeypatch.setattr(plan_api, "_thread_metadata", fake_thread_metadata)
monkeypatch.setattr(plan_api, "_user_owns_thread", fake_user_owns_thread)
monkeypatch.setattr(plan_api, "list_plan_comments", fake_list_plan_comments)
monkeypatch.setattr(plan_api, "set_plan_status", fake_set_plan_status)
monkeypatch.setattr(plan_api, "_dispatch_followup", fake_dispatch_followup)
monkeypatch.setattr(plan_api, "post_slack_thread_reply", fake_post_slack_thread_reply)
result = await plan_api.approve_plan("tid", session={"sub": "u1", "name": "Alice"})
assert result == {"status": plan_api.PLAN_STATUS_APPROVED}
assert posted["dispatch"]["plan_mode"] is False
assert posted["slack"] == {
"channel_id": "C1",
"thread_ts": "1700000000.0001",
"text": "Plan approved with 1 comments by Alice\nbeginning implementation",
}
async def test_set_plan_status_preserves_plan_file_path(monkeypatch: pytest.MonkeyPatch) -> None:
from agent.dashboard import plan_store
existing = {
"markdown": "# Plan",
"status": "ready",
"plan_file_path": "/workspace/plans/foo.md",
}
saved: dict[str, Any] = {}
class _Store:
async def get_item(self, *a: Any, **k: Any) -> Any:
return {"value": existing}
async def put_item(self, namespace: Any, key: str, value: Any, *a: Any, **k: Any) -> None:
saved.update(value)
async def fake_merge(thread_id: str, metadata: dict[str, Any]) -> None:
return None
monkeypatch.setattr(plan_store, "_client", lambda: _fake_client(_Store()))
monkeypatch.setattr(plan_store, "_merge_thread_metadata", fake_merge)
await plan_store.set_plan_status("t", plan_store.PLAN_STATUS_REVISING, plan_mode=True)
assert saved["plan_file_path"] == "/workspace/plans/foo.md"
assert saved["status"] == plan_store.PLAN_STATUS_REVISING