mirror of
https://github.com/Sea-Haven-Industries/open-swe.git
synced 2026-09-30 13:53:15 +00:00
Some checks are pending
CI / Lint (push) Waiting to run
CI / Format check (push) Waiting to run
CI / Unit tests (push) Waiting to run
CI / Playwright E2E (push) Waiting to run
CI / Docker build smoke (push) Waiting to run
CI / Triage ledger up to date (push) Waiting to run
CI / ui bun.lock in sync (push) Waiting to run
* feat: port plan-review & workflow-approval UX (#135)
Port six upstream commits onto dev:
- c03a6be7 (already ported): keep plan guidance high-level
- 546042a4: add workflow approval UI with diff preview, approval URLs,
web review links, and polling for approval status during active runs
- 216cf181: remove workflow token elevation; approved pushes pass
through directly without proxy token rewriting
- 3dbc0282: preserve plan redirects after login by accepting relative
same-origin redirect_to values and rejecting blocked paths
- bb104d93: submit plan comments with cmd+enter
- 90cb6caa: terse Slack replies, shared content via save_plan outside
plan mode (PLAN_STATUS_SHARED), reject shared-content mutations
Refs: #135
* fix: restore login page render and clear CI lint/format
The plan-review port removed the authRedirectUrl import from login.tsx
but left its call site, crashing the login page at runtime (blank page,
no 'Sign in to open-swe'). Pass the relative path straight to loginUrl,
matching the plan route and the backend relative-redirect handling.
Also drop an unused os import in the guard test and reformat
workflow_push_guard.py to satisfy ruff.
* fix: carry workflows:write on the standing proxy token
Complete the half-ported upstream 216cf181 cascade. The port dropped
_run_with_workflow_token from the guard but missed the paired github_app
change, so an approved .github/workflows push ran with the base token
(no workflows:write) and GitHub 403'd it.
Add workflows:write to BASE_RUNTIME_PROXY_TOKEN_PERMISSIONS and delete the
now-orphaned WORKFLOW_RUNTIME_PROXY_TOKEN_PERMISSIONS constant; update the
github_app and proxy_auth tests to match. The HITL approval gate in
workflow_push_guard.py is unchanged — this only lets the standing token
push once a human approves.
* fix: restore transient workflow-token elevation (revert standing workflows:write)
The standing GitHub-App proxy token (BASE_RUNTIME_PROXY_TOKEN_PERMISSIONS) is
ALWAYS-ON, so carrying workflows:write on it made the fork's HITL workflow-push
guard the sole control over unapproved workflow pushes. The guard's git-push
parser has gaps (obfuscated-expansion push, `gh api` REST contents PUT,
fully-qualified cross-branch refspecs); with a permanently workflows-scoped
token those gaps become live unapproved-workflow-push exploits (1 critical, 2
high — security review BLOCK on #159).
Restore dev's transient-elevation model:
- Drop workflows:write from BASE_RUNTIME_PROXY_TOKEN_PERMISSIONS; re-add the
WORKFLOW_RUNTIME_PROXY_TOKEN_PERMISSIONS constant (base + workflows:write).
- Re-introduce _run_with_workflow_token in the guard: it mints the
workflows-scoped token via refresh_proxy_token around the approved,
guard-normalized fixed_command, then downscopes to RUNTIME then BASE in a
finally. Route the approval branch through it.
- Restore the dev token/elevation tests.
The standing token no longer carries workflows:write, so the three parser
bypasses hit GitHub 403 again; an approved push still succeeds because the
elevation grants workflows:write only around the normalized command. Keeps all
of #159's diff-preview / approval-URL / Slack-card guard additions.
* fix: reject protocol-relative path from sanitizeAuthRedirect (open redirect)
sanitizeAuthRedirect returned parsed.pathname+search+hash, which `new URL` can
resolve to a protocol-relative `//host` (e.g. input `/..//evil.com` normalizes
same-origin, passing the origin check, but yields a path starting with `//`).
ClientRedirect / login.tsx feed that path to window.location.replace, so it
navigates cross-origin — an open redirect. Reject any resolved path that is not
a single-leading-slash path (`^/[^/]`), falling back to the default. Adds
coverage for `/..//evil.com`, `/.//evil.com`, and `//evil.com`.
* fix: log SECURITY error when workflow-token downscope fails
The elevate->push->downscope finally block was silent on failure. If both
refresh_proxy_token calls fail, the sandbox retains workflows:write for the
rest of the run with no signal. Log a SECURITY error on the partial and full
downscope-failure paths so the retention is observable.
Addresses the GPT-4.1 cross-family review of the token-scope remediation.
---------
Co-authored-by: amoussa1229 <166072409+amoussa1229@users.noreply.github.com>
Co-authored-by: Adam Moussa <adam@seahavenind.com>
758 lines
26 KiB
Python
758 lines
26 KiB
Python
from __future__ import annotations
|
|
|
|
from typing import Any
|
|
|
|
import pytest
|
|
|
|
|
|
async def _async_noop(*args: Any, **kwargs: Any) -> None:
|
|
return None
|
|
|
|
|
|
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,
|
|
plan_mode: bool | None = True,
|
|
) -> None:
|
|
saved.update(
|
|
thread_id=thread_id,
|
|
markdown=markdown,
|
|
status=status,
|
|
plan_file_path=plan_file_path,
|
|
plan_mode=plan_mode,
|
|
)
|
|
|
|
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": "shared",
|
|
"plan_file_path": "/workspace/plans/2026-07-08-test-plan.md",
|
|
"plan_mode": None,
|
|
}
|
|
|
|
|
|
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_get_plan_content(
|
|
thread_id: str, *, raise_on_error: bool = False
|
|
) -> dict[str, Any] | None:
|
|
return {"markdown": "# Plan", "status": "ready"}
|
|
|
|
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 = None
|
|
) -> 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, "get_plan_content", fake_get_plan_content)
|
|
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_approve_plan_is_idempotent_when_already_approved(
|
|
monkeypatch: pytest.MonkeyPatch,
|
|
) -> None:
|
|
from fastapi import HTTPException
|
|
|
|
from agent.dashboard import plan_api
|
|
|
|
calls: list[str] = []
|
|
|
|
async def fake_thread_metadata(thread_id: str) -> dict[str, Any]:
|
|
return {"github_login": "alice"}
|
|
|
|
async def fake_get_plan_content(
|
|
thread_id: str, *, raise_on_error: bool = False
|
|
) -> dict[str, Any] | None:
|
|
return {"markdown": "# Plan", "status": plan_api.PLAN_STATUS_APPROVED}
|
|
|
|
async def fake_dispatch_followup(*a: Any, **k: Any) -> None:
|
|
calls.append("dispatch")
|
|
|
|
async def fake_set_plan_status(*a: Any, **k: Any) -> None:
|
|
calls.append("set_status")
|
|
|
|
monkeypatch.setattr(plan_api, "_thread_metadata", fake_thread_metadata)
|
|
monkeypatch.setattr(plan_api, "_user_owns_thread", lambda *a, **k: True)
|
|
monkeypatch.setattr(plan_api, "get_plan_content", fake_get_plan_content)
|
|
monkeypatch.setattr(plan_api, "_dispatch_followup", fake_dispatch_followup)
|
|
monkeypatch.setattr(plan_api, "set_plan_status", fake_set_plan_status)
|
|
|
|
with pytest.raises(HTTPException) as exc:
|
|
await plan_api.approve_plan("tid", session={"sub": "u1", "name": "Alice"})
|
|
|
|
assert exc.value.status_code == 409
|
|
# A repeat approve must neither dispatch a second run nor re-persist state.
|
|
assert calls == []
|
|
|
|
|
|
async def test_approve_plan_slack_notice_counts_only_substantive_comments(
|
|
monkeypatch: pytest.MonkeyPatch,
|
|
) -> None:
|
|
from agent.dashboard import plan_api
|
|
|
|
posted: dict[str, Any] = {}
|
|
|
|
async def fake_thread_metadata(thread_id: str) -> dict[str, Any]:
|
|
return {
|
|
"github_login": "alice",
|
|
"source_context": {"slack_thread": {"channel_id": "C1", "thread_ts": "1700000000.1"}},
|
|
}
|
|
|
|
async def fake_get_plan_content(
|
|
thread_id: str, *, raise_on_error: bool = False
|
|
) -> dict[str, Any] | None:
|
|
return {"markdown": "# Plan", "status": "ready"}
|
|
|
|
async def fake_list_plan_comments(
|
|
thread_id: str, *, raise_on_error: bool = False
|
|
) -> list[dict]:
|
|
# Two real comments plus an empty and a whitespace-only one that
|
|
# _format_comments filters out — the notice must count only the two.
|
|
return [
|
|
{"author": "bob", "body": "tweak this"},
|
|
{"author": "sys", "body": ""},
|
|
{"author": "sys", "body": " "},
|
|
{"author": "cara", "body": "and this"},
|
|
]
|
|
|
|
async def fake_post_slack_thread_reply(channel_id: str, thread_ts: str, text: str) -> bool:
|
|
posted["text"] = text
|
|
return True
|
|
|
|
monkeypatch.setattr(plan_api, "_thread_metadata", fake_thread_metadata)
|
|
monkeypatch.setattr(plan_api, "_user_owns_thread", lambda *a, **k: True)
|
|
monkeypatch.setattr(plan_api, "get_plan_content", fake_get_plan_content)
|
|
monkeypatch.setattr(plan_api, "list_plan_comments", fake_list_plan_comments)
|
|
monkeypatch.setattr(plan_api, "set_plan_status", _async_noop)
|
|
monkeypatch.setattr(plan_api, "_dispatch_followup", _async_noop)
|
|
monkeypatch.setattr(plan_api, "post_slack_thread_reply", fake_post_slack_thread_reply)
|
|
|
|
await plan_api.approve_plan("tid", session={"sub": "u1", "name": "Alice"})
|
|
|
|
assert posted["text"] == "Plan approved with 2 comments by Alice\nbeginning implementation"
|
|
|
|
|
|
async def test_approve_plan_dispatches_before_persisting_approved(
|
|
monkeypatch: pytest.MonkeyPatch,
|
|
) -> None:
|
|
from agent.dashboard import plan_api
|
|
|
|
order: list[str] = []
|
|
|
|
async def fake_thread_metadata(thread_id: str) -> dict[str, Any]:
|
|
return {"github_login": "alice"}
|
|
|
|
async def fake_get_plan_content(
|
|
thread_id: str, *, raise_on_error: bool = False
|
|
) -> dict[str, Any] | None:
|
|
return {"markdown": "# Plan", "status": "ready"}
|
|
|
|
async def fake_list_plan_comments(
|
|
thread_id: str, *, raise_on_error: bool = False
|
|
) -> list[dict]:
|
|
return []
|
|
|
|
async def fake_dispatch_followup(*a: Any, **k: Any) -> None:
|
|
order.append("dispatch")
|
|
|
|
async def fake_set_plan_status(*a: Any, **k: Any) -> None:
|
|
order.append("set_status")
|
|
|
|
monkeypatch.setattr(plan_api, "_thread_metadata", fake_thread_metadata)
|
|
monkeypatch.setattr(plan_api, "_user_owns_thread", lambda *a, **k: True)
|
|
monkeypatch.setattr(plan_api, "get_plan_content", fake_get_plan_content)
|
|
monkeypatch.setattr(plan_api, "list_plan_comments", fake_list_plan_comments)
|
|
monkeypatch.setattr(plan_api, "_dispatch_followup", fake_dispatch_followup)
|
|
monkeypatch.setattr(plan_api, "set_plan_status", fake_set_plan_status)
|
|
monkeypatch.setattr(plan_api, "_maybe_post_plan_approved_to_slack", _async_noop)
|
|
|
|
await plan_api.approve_plan("tid", session={"sub": "u1", "name": "Alice"})
|
|
|
|
# Dispatch must precede the APPROVED write so a dispatch failure leaves the
|
|
# plan re-approvable rather than stuck approved-but-undispatched.
|
|
assert order == ["dispatch", "set_status"]
|
|
|
|
|
|
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
|
|
|
|
|
|
# --- manual plan editing -------------------------------------------------
|
|
|
|
|
|
async def test_save_plan_content_clear_comments_flag(monkeypatch: pytest.MonkeyPatch) -> None:
|
|
from agent.dashboard import plan_store
|
|
|
|
cleared: list[str] = []
|
|
|
|
class _Store:
|
|
async def put_item(self, *a: Any, **k: Any) -> None:
|
|
return None
|
|
|
|
async def fake_clear(thread_id: str) -> None:
|
|
cleared.append(thread_id)
|
|
|
|
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, "clear_plan_comments", fake_clear)
|
|
monkeypatch.setattr(plan_store, "_merge_thread_metadata", fake_merge)
|
|
|
|
# A manual edit keeps reviewer comments; the agent's republish clears them.
|
|
await plan_store.save_plan_content("t", markdown="x", clear_comments=False)
|
|
assert cleared == []
|
|
await plan_store.save_plan_content("t", markdown="x")
|
|
assert cleared == ["t"]
|
|
|
|
|
|
def _patch_update_plan_deps(
|
|
monkeypatch: pytest.MonkeyPatch,
|
|
*,
|
|
metadata: dict[str, Any],
|
|
owner: bool,
|
|
content: dict[str, Any],
|
|
saved: dict[str, Any],
|
|
sandbox: dict[str, Any],
|
|
) -> None:
|
|
from agent.dashboard import plan_api
|
|
|
|
async def fake_meta(thread_id: str) -> dict[str, Any]:
|
|
return metadata
|
|
|
|
async def fake_get_content(thread_id: str) -> dict[str, Any]:
|
|
return content
|
|
|
|
async def fake_save(
|
|
thread_id: str, *, markdown: str, status: str, clear_comments: bool = True
|
|
) -> None:
|
|
saved.update(markdown=markdown, status=status, clear_comments=clear_comments)
|
|
|
|
async def fake_write(thread_id: str, c: str) -> str:
|
|
sandbox["content"] = c
|
|
return "plan.md"
|
|
|
|
monkeypatch.setattr(plan_api, "_thread_metadata", fake_meta)
|
|
monkeypatch.setattr(plan_api, "_user_owns_thread", lambda *a, **k: owner)
|
|
monkeypatch.setattr(plan_api, "get_plan_content", fake_get_content)
|
|
monkeypatch.setattr(plan_api, "save_plan_content", fake_save)
|
|
monkeypatch.setattr(plan_api, "write_plan_to_sandbox", fake_write)
|
|
|
|
|
|
async def test_update_plan_owner_saves_and_mirrors_sandbox(
|
|
monkeypatch: pytest.MonkeyPatch,
|
|
) -> None:
|
|
from agent.dashboard import plan_api
|
|
|
|
saved: dict[str, Any] = {}
|
|
sandbox: dict[str, Any] = {}
|
|
_patch_update_plan_deps(
|
|
monkeypatch,
|
|
metadata={"plan_status": "ready"},
|
|
owner=True,
|
|
content={"markdown": "old", "status": "ready"},
|
|
saved=saved,
|
|
sandbox=sandbox,
|
|
)
|
|
|
|
result = await plan_api.update_plan(
|
|
"tid",
|
|
plan_api.PlanUpdate(markdown=" # Edited plan "),
|
|
session={"sub": "u1"},
|
|
)
|
|
|
|
assert result == {"status": "ready", "markdown": "# Edited plan"}
|
|
assert saved["markdown"] == "# Edited plan"
|
|
assert saved["status"] == "ready"
|
|
assert saved["clear_comments"] is False
|
|
assert sandbox["content"] == "# Edited plan"
|
|
|
|
|
|
async def test_update_plan_rejects_non_owner(monkeypatch: pytest.MonkeyPatch) -> None:
|
|
from fastapi import HTTPException
|
|
|
|
from agent.dashboard import plan_api
|
|
|
|
_patch_update_plan_deps(
|
|
monkeypatch,
|
|
metadata={},
|
|
owner=False,
|
|
content={},
|
|
saved={},
|
|
sandbox={},
|
|
)
|
|
with pytest.raises(HTTPException, match="only the plan owner"):
|
|
await plan_api.update_plan("tid", plan_api.PlanUpdate(markdown="x"), session={"sub": "u1"})
|
|
|
|
|
|
async def test_update_plan_rejects_empty_markdown(monkeypatch: pytest.MonkeyPatch) -> None:
|
|
from fastapi import HTTPException
|
|
|
|
from agent.dashboard import plan_api
|
|
|
|
_patch_update_plan_deps(
|
|
monkeypatch,
|
|
metadata={},
|
|
owner=True,
|
|
content={},
|
|
saved={},
|
|
sandbox={},
|
|
)
|
|
with pytest.raises(HTTPException, match="plan markdown cannot be empty"):
|
|
await plan_api.update_plan(
|
|
"tid", plan_api.PlanUpdate(markdown=" "), session={"sub": "u1"}
|
|
)
|
|
|
|
|
|
async def test_update_plan_rejects_approved_plan(monkeypatch: pytest.MonkeyPatch) -> None:
|
|
from fastapi import HTTPException
|
|
|
|
from agent.dashboard import plan_api
|
|
|
|
_patch_update_plan_deps(
|
|
monkeypatch,
|
|
metadata={},
|
|
owner=True,
|
|
content={"status": "approved"},
|
|
saved={},
|
|
sandbox={},
|
|
)
|
|
with pytest.raises(HTTPException, match="cannot edit a approved plan"):
|
|
await plan_api.update_plan("tid", plan_api.PlanUpdate(markdown="x"), session={"sub": "u1"})
|
|
|
|
|
|
async def test_update_plan_rejects_cancelled_plan(monkeypatch: pytest.MonkeyPatch) -> None:
|
|
from fastapi import HTTPException
|
|
|
|
from agent.dashboard import plan_api
|
|
|
|
_patch_update_plan_deps(
|
|
monkeypatch,
|
|
metadata={},
|
|
owner=True,
|
|
content={"status": "cancelled"},
|
|
saved={},
|
|
sandbox={},
|
|
)
|
|
with pytest.raises(HTTPException, match="cannot edit a cancelled plan"):
|
|
await plan_api.update_plan("tid", plan_api.PlanUpdate(markdown="x"), session={"sub": "u1"})
|
|
|
|
|
|
async def test_approve_plan_reads_published_content_strictly(
|
|
monkeypatch: pytest.MonkeyPatch,
|
|
) -> None:
|
|
"""Approve reads plan + comments with raise_on_error; failure aborts."""
|
|
from agent.dashboard import plan_api
|
|
|
|
metadata = {"github_login": "alice"}
|
|
|
|
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_get_plan_content(
|
|
thread_id: str, *, raise_on_error: bool = False
|
|
) -> dict[str, Any] | None:
|
|
if raise_on_error:
|
|
raise RuntimeError("store is down")
|
|
return None
|
|
|
|
async def fake_list_plan_comments(
|
|
thread_id: str, *, raise_on_error: bool = False
|
|
) -> list[dict]:
|
|
return []
|
|
|
|
monkeypatch.setattr(plan_api, "_thread_metadata", fake_thread_metadata)
|
|
monkeypatch.setattr(plan_api, "_user_owns_thread", fake_user_owns_thread)
|
|
monkeypatch.setattr(plan_api, "get_plan_content", fake_get_plan_content)
|
|
monkeypatch.setattr(plan_api, "list_plan_comments", fake_list_plan_comments)
|
|
|
|
with pytest.raises(RuntimeError, match="store is down"):
|
|
await plan_api.approve_plan("tid", session={"sub": "u1", "name": "Alice"})
|
|
|
|
|
|
async def test_approve_plan_hands_edited_plan_to_agent(
|
|
monkeypatch: pytest.MonkeyPatch,
|
|
) -> None:
|
|
"""When the owner edited the plan, the edited markdown is dispatched."""
|
|
from agent.dashboard import plan_api
|
|
|
|
metadata = {"github_login": "alice"}
|
|
dispatched: 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_get_plan_content(
|
|
thread_id: str, *, raise_on_error: bool = False
|
|
) -> dict[str, Any] | None:
|
|
return {"markdown": "# Owner edits", "status": "ready"}
|
|
|
|
async def fake_list_plan_comments(
|
|
thread_id: str, *, raise_on_error: bool = False
|
|
) -> list[dict]:
|
|
return [{"author": "bob", "body": "looks good"}]
|
|
|
|
async def fake_set_plan_status(
|
|
thread_id: str, status: str, *, plan_mode: bool | None = None
|
|
) -> None:
|
|
dispatched["status_set"] = status
|
|
|
|
async def fake_dispatch_followup(
|
|
thread_id: str, md: dict, text: str, *, plan_mode: bool
|
|
) -> None:
|
|
dispatched["text"] = text
|
|
dispatched["plan_mode"] = plan_mode
|
|
|
|
monkeypatch.setattr(plan_api, "_thread_metadata", fake_thread_metadata)
|
|
monkeypatch.setattr(plan_api, "_user_owns_thread", fake_user_owns_thread)
|
|
monkeypatch.setattr(plan_api, "get_plan_content", fake_get_plan_content)
|
|
monkeypatch.setattr(plan_api, "list_plan_comments", fake_list_plan_comments)
|
|
monkeypatch.setattr(plan_api, "set_plan_status", fake_set_plan_status)
|
|
|
|
async def fake_maybe_post_slack(*a: Any, **k: Any) -> None:
|
|
return None
|
|
|
|
monkeypatch.setattr(plan_api, "_dispatch_followup", fake_dispatch_followup)
|
|
monkeypatch.setattr(plan_api, "_maybe_post_plan_approved_to_slack", fake_maybe_post_slack)
|
|
|
|
await plan_api.approve_plan("tid", session={"sub": "u1", "name": "Alice"})
|
|
|
|
assert "# Owner edits" in dispatched["text"]
|
|
assert "looks good" in dispatched["text"]
|
|
|
|
|
|
def test_plan_update_route_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
|