mirror of
https://github.com/Sea-Haven-Industries/open-swe.git
synced 2026-09-30 05:43:14 +00:00
feat: publish plans from sandbox files (#1635)
* feat: publish plans from sandbox files Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * fix: avoid fixed plan filenames Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * fix: virtualize local sandbox file paths Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * fix: preserve plan_file_path across set_plan_status set_plan_status was rewriting the content record with only markdown and status, dropping plan_file_path. After a reject, the owner's dashboard edit would mirror to a different file than the agent's original, and the next save_plan could republish the stale file. Preserve plan_file_path when updating status. --------- Co-authored-by: Ramon Nogueira <270434257+ramon-langchain@users.noreply.github.com> Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com>
This commit is contained in:
Ramon Nogueira
GitHub
parent
c03a6be7a8
commit
ae04b72b41
12 changed files with 327 additions and 87 deletions
|
|
@ -33,6 +33,7 @@ from .plan_store import (
|
|||
delete_plan_comment,
|
||||
get_plan_content,
|
||||
list_plan_comments,
|
||||
plan_file_path_for_thread,
|
||||
save_plan_content,
|
||||
set_plan_status,
|
||||
write_plan_to_sandbox,
|
||||
|
|
@ -103,7 +104,7 @@ async def update_plan(
|
|||
"""Owner-only manual edit of the plan markdown.
|
||||
|
||||
Re-publishes the edited plan as ``ready`` (and mirrors it into the sandbox
|
||||
``plan.md``) while preserving reviewer comments, so the owner can refine the
|
||||
plan file) while preserving reviewer comments, so the owner can refine the
|
||||
plan before approving it."""
|
||||
metadata = await _thread_metadata(thread_id)
|
||||
if not _user_owns_thread(metadata, session["sub"], session.get("email")):
|
||||
|
|
@ -115,10 +116,18 @@ async def update_plan(
|
|||
status = content.get("status") or metadata.get("plan_status") or "planning"
|
||||
if status in (PLAN_STATUS_APPROVED, PLAN_STATUS_CANCELLED):
|
||||
raise HTTPException(409, f"cannot edit a {status} plan")
|
||||
await save_plan_content(
|
||||
thread_id, markdown=markdown, status=PLAN_STATUS_READY, clear_comments=False
|
||||
plan_file_path = content.get("plan_file_path")
|
||||
plan_file_path = (
|
||||
plan_file_path if isinstance(plan_file_path, str) else plan_file_path_for_thread(thread_id)
|
||||
)
|
||||
await write_plan_to_sandbox(thread_id, markdown)
|
||||
await save_plan_content(
|
||||
thread_id,
|
||||
markdown=markdown,
|
||||
status=PLAN_STATUS_READY,
|
||||
clear_comments=False,
|
||||
plan_file_path=plan_file_path,
|
||||
)
|
||||
await write_plan_to_sandbox(thread_id, markdown, plan_file_path=plan_file_path)
|
||||
return {"status": PLAN_STATUS_READY, "markdown": markdown}
|
||||
|
||||
|
||||
|
|
@ -210,7 +219,8 @@ async def reject_plan(thread_id: str, session: dict[str, Any] = _SESSION_DEP) ->
|
|||
await set_plan_status(thread_id, PLAN_STATUS_REVISING, plan_mode=True)
|
||||
text = (
|
||||
"The plan needs changes before implementation. Address this reviewer "
|
||||
"feedback and publish an updated plan with the save_plan tool:\n\n"
|
||||
"feedback in the existing Markdown file under /workspace/plans/, then "
|
||||
"publish an updated plan with the save_plan tool:\n\n"
|
||||
f"{feedback or '(no specific comments were left)'}"
|
||||
)
|
||||
await _dispatch_followup(thread_id, metadata, text, plan_mode=True)
|
||||
|
|
|
|||
|
|
@ -1,8 +1,7 @@
|
|||
"""Persistence for the plan-review feature.
|
||||
|
||||
The plan lives in two places:
|
||||
- the agent's sandbox, as a real ``plan.md`` file (written by the ``save_plan``
|
||||
tool — the source artifact the agent produces and can re-read), and
|
||||
- the agent's sandbox, as a real Markdown file the agent creates and edits, and
|
||||
- the LangGraph store, as the published snapshot the dashboard renders.
|
||||
|
||||
Reviewers leave whole-document comments, stored one item per comment under
|
||||
|
|
@ -13,6 +12,7 @@ store operations (no CRDT/WebSocket).
|
|||
from __future__ import annotations
|
||||
|
||||
import logging
|
||||
import re
|
||||
import uuid
|
||||
from datetime import UTC, datetime
|
||||
from typing import Any
|
||||
|
|
@ -24,8 +24,8 @@ logger = logging.getLogger(__name__)
|
|||
PLAN_CONTENT_NAMESPACE = ["plan", "content"]
|
||||
PLAN_COMMENTS_NAMESPACE = ["plan", "comments"]
|
||||
|
||||
# The plan is mirrored into the sandbox as a real file the agent can re-read.
|
||||
PLAN_FILE_PATH = "plan.md"
|
||||
# Plans are mirrored into the sandbox outside cloned repositories.
|
||||
PLAN_FILE_DIRECTORY = "/workspace/plans"
|
||||
|
||||
# Plan lifecycle, stored on both the content record and the thread metadata.
|
||||
PLAN_STATUS_PLANNING = "planning"
|
||||
|
|
@ -35,6 +35,12 @@ PLAN_STATUS_APPROVED = "approved"
|
|||
PLAN_STATUS_CANCELLED = "cancelled"
|
||||
|
||||
|
||||
def plan_file_path_for_thread(thread_id: str) -> str:
|
||||
date = datetime.now(UTC).strftime("%Y-%m-%d")
|
||||
slug = re.sub(r"[^a-zA-Z0-9-]+", "-", thread_id).strip("-").lower()[:48]
|
||||
return f"{PLAN_FILE_DIRECTORY}/{date}-{slug or 'plan'}.md"
|
||||
|
||||
|
||||
def _client() -> Any:
|
||||
return get_client()
|
||||
|
||||
|
|
@ -46,12 +52,22 @@ def _item_value(item: Any) -> dict[str, Any] | None:
|
|||
return value if isinstance(value, dict) else None
|
||||
|
||||
|
||||
async def _stored_plan_file_path(client: Any, thread_id: str) -> str | None:
|
||||
try:
|
||||
value = _item_value(await client.store.get_item(PLAN_CONTENT_NAMESPACE, thread_id)) or {}
|
||||
except Exception:
|
||||
return None
|
||||
path = value.get("plan_file_path")
|
||||
return path if isinstance(path, str) and path else None
|
||||
|
||||
|
||||
async def save_plan_content(
|
||||
thread_id: str,
|
||||
*,
|
||||
markdown: str,
|
||||
status: str = PLAN_STATUS_READY,
|
||||
clear_comments: bool = True,
|
||||
plan_file_path: str | None = None,
|
||||
) -> None:
|
||||
"""Publish the plan markdown + status for the dashboard to render.
|
||||
|
||||
|
|
@ -60,10 +76,15 @@ async def save_plan_content(
|
|||
and be fed back to the agent on the next approve/reject. A manual owner edit
|
||||
passes ``clear_comments=False`` so reviewer feedback survives the edit."""
|
||||
client = _client()
|
||||
if plan_file_path is None:
|
||||
plan_file_path = await _stored_plan_file_path(client, thread_id)
|
||||
record = {"markdown": markdown, "status": status}
|
||||
if plan_file_path:
|
||||
record["plan_file_path"] = plan_file_path
|
||||
await client.store.put_item(
|
||||
PLAN_CONTENT_NAMESPACE,
|
||||
thread_id,
|
||||
{"markdown": markdown, "status": status},
|
||||
record,
|
||||
)
|
||||
if clear_comments:
|
||||
try:
|
||||
|
|
@ -74,18 +95,24 @@ async def save_plan_content(
|
|||
await _merge_thread_metadata(thread_id, {"plan_status": status, "plan_mode": True})
|
||||
|
||||
|
||||
async def write_plan_to_sandbox(thread_id: str, content: str) -> str:
|
||||
"""Write ``plan.md`` into the thread's sandbox. Best-effort: a missing sandbox
|
||||
must not block publishing the plan to the review page."""
|
||||
async def write_plan_to_sandbox(
|
||||
thread_id: str, content: str, *, plan_file_path: str | None = None
|
||||
) -> str:
|
||||
"""Mirror the dashboard plan edit into the thread's sandbox.
|
||||
|
||||
Best-effort: a missing sandbox must not block publishing the plan to the
|
||||
review page.
|
||||
"""
|
||||
path = plan_file_path or plan_file_path_for_thread(thread_id)
|
||||
try:
|
||||
from ..utils.sandbox_state import get_sandbox_backend
|
||||
|
||||
backend = await get_sandbox_backend(thread_id)
|
||||
await backend.awrite(PLAN_FILE_PATH, content)
|
||||
return PLAN_FILE_PATH
|
||||
await backend.awrite(path, content)
|
||||
return path
|
||||
except Exception:
|
||||
logger.warning("Could not write plan.md to sandbox for %s", thread_id, exc_info=True)
|
||||
return PLAN_FILE_PATH
|
||||
logger.warning("Could not write plan file to sandbox for %s", thread_id, exc_info=True)
|
||||
return path
|
||||
|
||||
|
||||
async def get_plan_content(
|
||||
|
|
@ -110,10 +137,14 @@ async def set_plan_status(thread_id: str, status: str, *, plan_mode: bool | None
|
|||
"""Update the plan lifecycle status on both the content record and metadata."""
|
||||
existing = await get_plan_content(thread_id) or {}
|
||||
client = _client()
|
||||
record: dict[str, Any] = {"markdown": existing.get("markdown", ""), "status": status}
|
||||
plan_file_path = existing.get("plan_file_path")
|
||||
if isinstance(plan_file_path, str) and plan_file_path:
|
||||
record["plan_file_path"] = plan_file_path
|
||||
await client.store.put_item(
|
||||
PLAN_CONTENT_NAMESPACE,
|
||||
thread_id,
|
||||
{"markdown": existing.get("markdown", ""), "status": status},
|
||||
record,
|
||||
)
|
||||
metadata: dict[str, Any] = {"plan_status": status}
|
||||
if plan_mode is not None:
|
||||
|
|
|
|||
|
|
@ -24,5 +24,6 @@ def create_local_sandbox(sandbox_id: str | None = None):
|
|||
|
||||
return LocalShellBackend(
|
||||
root_dir=root_dir,
|
||||
virtual_mode=True,
|
||||
inherit_env=True,
|
||||
)
|
||||
|
|
|
|||
|
|
@ -1,11 +1,12 @@
|
|||
"""Plan-mode tool gating.
|
||||
|
||||
Hides the mutating tools whenever plan mode is active — either when the run
|
||||
starts in plan mode (the per-thread ``plan_mode`` carried in configurable, e.g.
|
||||
a reject re-dispatch) OR after the model calls ``enter_plan_mode`` mid-run, which
|
||||
sets ``plan_mode`` in the run state. Installed unconditionally so self-activation
|
||||
actually restricts the *next* model turn (the tool list is recomputed on every
|
||||
model call), rather than only affecting a future run.
|
||||
Hides tools that mutate external systems whenever plan mode is active — either
|
||||
when the run starts in plan mode (the per-thread ``plan_mode`` carried in
|
||||
configurable, e.g. a reject re-dispatch) OR after the model calls
|
||||
``enter_plan_mode`` mid-run, which sets ``plan_mode`` in the run state. Installed
|
||||
unconditionally so self-activation actually restricts the *next* model turn (the
|
||||
tool list is recomputed on every model call), rather than only affecting a future
|
||||
run.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
|
@ -37,7 +38,7 @@ def _tool_name(tool: BaseTool | dict[str, Any] | Any) -> str | None:
|
|||
|
||||
|
||||
class PlanModeMiddleware(AgentMiddleware):
|
||||
"""Strip mutating tools from each model request while plan mode is active."""
|
||||
"""Strip disallowed tools from each model request while plan mode is active."""
|
||||
|
||||
state_schema = PlanModeState
|
||||
|
||||
|
|
|
|||
|
|
@ -99,7 +99,7 @@ PLAN_MODE_GUIDANCE_SECTION = """---
|
|||
|
||||
### Plan Mode
|
||||
|
||||
If a task would genuinely benefit from a structured plan before any code — complex, many files, or multiple valid approaches — call the `enter_plan_mode` tool. This is NOT triggered by the word "plan" in the request; use judgment. Once in plan mode, stay read-only, research the code, save your plan with `save_plan` (it writes `plan.md` and publishes a review page), and share the plan-review link with the user, who approves before you implement.
|
||||
If a task would genuinely benefit from a structured plan before any code — complex, many files, or multiple valid approaches — call the `enter_plan_mode` tool. This is NOT triggered by the word "plan" in the request; use judgment. Once in plan mode, stay read-only for the target repo, research the code, create/edit your plan as a dated Markdown file under `/workspace/plans/` (for example, `/workspace/plans/YYYY-MM-DD-short-task-slug.md`), publish it with `save_plan`, and share the plan-review link with the user, who approves before you implement.
|
||||
|
||||
Plan-review link for this conversation: {plan_review_url}"""
|
||||
|
||||
|
|
@ -109,15 +109,15 @@ PLAN_MODE_SECTION = """---
|
|||
|
||||
**Plan mode is enabled for this run. This supersedes any instruction telling you to edit code, commit, push, or open a pull request.**
|
||||
|
||||
You are in a read-only research-and-planning phase. Your single deliverable is a clear, reviewable implementation plan saved with `save_plan` — NOT code changes. Share the plan-review link below with the user right after entering plan mode and again when the plan is ready.
|
||||
You are in a read-only research-and-planning phase for the target repo. Your single deliverable is a clear, reviewable implementation plan saved as a Markdown file outside any repo and published with `save_plan` — NOT code changes. Share the plan-review link below with the user right after entering plan mode and again when the plan is ready.
|
||||
|
||||
**Plan-review link:** {plan_url}
|
||||
|
||||
**You MUST NOT** edit/create/delete files, run state-changing `execute` commands (no `git commit`/`push`/`checkout -b`, installs, code generators, or file-rewriting formatters), commit, push, open/update a PR, call `request_pr_review`, or mutate Linear/external systems. The `task` subagent is disabled here (subagents wouldn't inherit these restrictions) — research directly.
|
||||
**You MUST NOT** edit/create/delete files inside the target repo, run state-changing `execute` commands except creating `/workspace/plans` (no `git commit`/`push`/`checkout -b`, installs, code generators, or file-rewriting formatters), commit, push, open/update a PR, call `request_pr_review`, or mutate Linear/external systems. The `task` subagent is disabled here (subagents wouldn't inherit these restrictions) — research directly.
|
||||
|
||||
**You MAY (read-only):** clone and read the repo (`read_file`, `ls`, `glob`, `grep`, read-only `execute` like `git clone`/`status`/`log`/`diff`, `cat`, `rg`), research with `web_search`/`fetch_url`, and ask clarifying questions via `slack_thread_reply` / `linear_comment`.
|
||||
**You MAY:** clone and read the repo (`read_file`, `ls`, `glob`, `grep`, read-only `execute` like `git clone`/`status`/`log`/`diff`, `cat`, `rg`), research with `web_search`/`fetch_url`, ask clarifying questions via `slack_thread_reply` / `linear_comment`, use `execute` only if needed to create `/workspace/plans`, and use `write_file` / `edit_file` only to create or revise the plan file outside any repo under `/workspace/plans/`.
|
||||
|
||||
**Workflow:** explore the relevant code enough to choose a sound approach, clarify ambiguity, then save ONE concise recommended plan with `save_plan` (pass the full Markdown as `plan_markdown`) using this structure. Keep it high level: focus on desired behavior, architecture boundaries, product decisions, tradeoffs, rollout/migration concerns, and verification. Avoid file/function-level details and exhaustive file lists unless a specific implementation detail is unusually tricky, risky, or controversial. Aim for about one page or less unless the task truly requires more.
|
||||
**Workflow:** explore the relevant code enough to choose a sound approach, clarify ambiguity, choose a dated, descriptive plan path like `/workspace/plans/YYYY-MM-DD-short-task-slug.md`, create it with ONE recommended plan, refine it with normal file-editing tools if needed, then publish it with `save_plan` by passing that exact `plan_file_path`. Keep it high level: focus on desired behavior, architecture boundaries, product decisions, tradeoffs, rollout/migration concerns, and verification. Avoid file/function-level details and exhaustive file lists unless a specific implementation detail is unusually tricky, risky, or controversial. Aim for about one page or less unless the task truly requires more. Use this structure:
|
||||
|
||||
```
|
||||
## Plan: <short title>
|
||||
|
|
|
|||
|
|
@ -529,18 +529,19 @@ DEFAULT_RECURSION_LIMIT = 9_999
|
|||
# signal via notify_step_limit_reached rather than dying silently.
|
||||
MODEL_CALL_RECURSION_LIMIT = 5_000
|
||||
|
||||
# Mutating tools hidden from the model while plan mode is active so it can only
|
||||
# research and propose a plan. `execute` stays available; plan-mode shell
|
||||
# discipline (no mutating commands) is instructed via the system prompt rather
|
||||
# than enforced. `http_request` is excluded because it can POST/PUT/PATCH/DELETE
|
||||
# to external services — read-only web research goes through `web_search` /
|
||||
# `fetch_url`. `task` is excluded because the general-purpose subagent is built
|
||||
# with its own filesystem/PR/Linear tools and does not inherit this exclusion, so
|
||||
# delegating to it would bypass the read-only intent.
|
||||
# Mutating external tools hidden from the model while plan mode is active so it
|
||||
# can only research and propose a plan. File edit tools stay available so the
|
||||
# agent can draft and revise a plan under `/workspace/plans/`; prompt guidance
|
||||
# restricts them to that plan file outside cloned repositories. `execute` stays available;
|
||||
# plan-mode shell discipline (no mutating commands) is instructed via the system
|
||||
# prompt rather than enforced. `http_request` is excluded because it can
|
||||
# POST/PUT/PATCH/DELETE to external services — read-only web research goes
|
||||
# through `web_search` / `fetch_url`. `task` is excluded because the
|
||||
# general-purpose subagent is built with its own filesystem/PR/Linear tools and
|
||||
# does not inherit this exclusion, so delegating to it would bypass the read-only
|
||||
# intent.
|
||||
PLAN_MODE_EXCLUDED_TOOLS: frozenset[str] = frozenset(
|
||||
{
|
||||
"write_file",
|
||||
"edit_file",
|
||||
"task",
|
||||
"http_request",
|
||||
"open_pull_request",
|
||||
|
|
|
|||
|
|
@ -15,10 +15,11 @@ from ..dashboard.plan_store import PLAN_STATUS_PLANNING, set_plan_status
|
|||
logger = logging.getLogger(__name__)
|
||||
|
||||
_ENTERED_MESSAGE = (
|
||||
"Plan mode is active. Stay read-only: research the codebase, then record a "
|
||||
"concise, high-level plan with the `save_plan` tool (it publishes the plan to "
|
||||
"the review page) and share the plan-review link in the source channel. Do not "
|
||||
"edit files, commit, push, or open a PR — wait for the user to approve the plan."
|
||||
"Plan mode is active. Stay read-only for the target repo: research the codebase, "
|
||||
"create or edit a dated, concise plan file under `/workspace/plans/`, then publish "
|
||||
"it with the `save_plan` tool and share the plan-review link in the source channel. "
|
||||
"Do not edit repo files, commit, push, or open a PR — wait for the user to approve "
|
||||
"the plan."
|
||||
)
|
||||
|
||||
|
||||
|
|
@ -31,11 +32,12 @@ async def enter_plan_mode(tool_call_id: Annotated[str, InjectedToolCallId]) -> C
|
|||
NOT triggered by the word "plan" appearing in the request; use your
|
||||
judgment about whether planning is genuinely warranted.
|
||||
|
||||
Once activated, stay read-only: research the codebase, then record a concise,
|
||||
high-level plan with the ``save_plan`` tool (it publishes the plan to the
|
||||
review page) and share the plan-review link with the user. Do not edit files,
|
||||
commit, push, or open a PR — the user reviews the plan and approves it before
|
||||
you implement.
|
||||
Once activated, stay read-only for the target repo: research the codebase,
|
||||
create or edit a dated, concise Markdown plan outside any repo (for example,
|
||||
``/workspace/plans/YYYY-MM-DD-short-task-slug.md``), then publish it with
|
||||
the ``save_plan`` tool and share the plan-review link with the user. Do not
|
||||
edit repo files, commit, push, or open a PR — the user reviews the plan and
|
||||
approves it before you implement.
|
||||
"""
|
||||
thread_id = _thread_id_from_config()
|
||||
if thread_id:
|
||||
|
|
|
|||
|
|
@ -1,35 +1,37 @@
|
|||
"""Tool: ``save_plan``. Record the implementation plan for review.
|
||||
"""Tool: ``save_plan``. Publish the sandbox plan file for review.
|
||||
|
||||
Writes the plan as a real ``plan.md`` file in the sandbox (the artifact the
|
||||
agent produces and can re-read) and publishes it to the plan-review page, where
|
||||
the user and collaborators read it, comment inline, and approve or request
|
||||
changes. Available in plan mode (it does not modify the repository under review).
|
||||
Reads the Markdown plan file the agent created in the sandbox and publishes it to
|
||||
the plan-review page, where the user and collaborators read it, comment inline,
|
||||
and approve or request changes. Available in plan mode (it does not modify the
|
||||
repository under review).
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import logging
|
||||
from collections.abc import Mapping
|
||||
from typing import Any
|
||||
|
||||
from langgraph.config import get_config
|
||||
|
||||
from ..dashboard.plan_store import (
|
||||
PLAN_STATUS_READY,
|
||||
save_plan_content,
|
||||
write_plan_to_sandbox,
|
||||
)
|
||||
from ..dashboard.plan_store import PLAN_FILE_DIRECTORY, PLAN_STATUS_READY, save_plan_content
|
||||
from ..utils.sandbox_state import get_sandbox_backend
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
_MAX_PLAN_LINES = 20_000
|
||||
_MARKDOWN_EXTENSIONS = (".md", ".markdown")
|
||||
|
||||
async def save_plan(plan_markdown: str) -> dict[str, Any]:
|
||||
"""Write your implementation plan as a markdown file and publish it for review.
|
||||
|
||||
Use this in plan mode once your plan is ready. The plan is saved as
|
||||
``plan.md`` in the sandbox and published to the plan-review page linked in
|
||||
the conversation, where the user (the owner) and any collaborators can read
|
||||
it, leave inline comments, and then approve it or request changes. Call it
|
||||
again to overwrite the plan with a revised version when addressing feedback.
|
||||
async def save_plan(plan_file_path: str) -> dict[str, Any]:
|
||||
"""Publish a Markdown plan file from the sandbox for review.
|
||||
|
||||
Use this in plan mode once your plan is ready. First create a Markdown file
|
||||
under ``/workspace/plans/`` using a dated, descriptive filename, then pass
|
||||
that file path here. The file contents are published to the plan-review page
|
||||
linked in the conversation, where the user (the owner) and any collaborators
|
||||
can read it, leave inline comments, and then approve it or request changes.
|
||||
Call it again to publish a revised file when addressing feedback.
|
||||
|
||||
Write the plan in standard Markdown — headings, bullet/numbered lists, and
|
||||
fenced code blocks all render. Keep it concise and high level, focusing on
|
||||
|
|
@ -37,14 +39,21 @@ async def save_plan(plan_markdown: str) -> dict[str, Any]:
|
|||
details unless they are unusually tricky or controversial.
|
||||
|
||||
Args:
|
||||
plan_markdown: The full plan, as a Markdown document.
|
||||
plan_file_path: Path to the Markdown plan file in the sandbox.
|
||||
|
||||
Returns:
|
||||
``{success: True, path}`` on success, or ``{success: False, error}``.
|
||||
"""
|
||||
content = plan_markdown.strip()
|
||||
if not content:
|
||||
return {"success": False, "error": "plan_markdown cannot be empty"}
|
||||
if not isinstance(plan_file_path, str):
|
||||
return {"success": False, "error": "plan_file_path must be a string"}
|
||||
path = plan_file_path.strip()
|
||||
if not path:
|
||||
return {"success": False, "error": "plan_file_path cannot be empty"}
|
||||
if not _is_markdown_path(path):
|
||||
return {
|
||||
"success": False,
|
||||
"error": f"plan_file_path must point to a Markdown file in {PLAN_FILE_DIRECTORY}",
|
||||
}
|
||||
|
||||
try:
|
||||
config = get_config()
|
||||
|
|
@ -56,14 +65,52 @@ async def save_plan(plan_markdown: str) -> dict[str, Any]:
|
|||
return {"success": False, "error": "no thread_id in run config"}
|
||||
|
||||
try:
|
||||
path = await _save(str(thread_id), content)
|
||||
content = (await _read_plan_file(str(thread_id), path)).strip()
|
||||
if not content:
|
||||
return {"success": False, "error": "plan file cannot be empty"}
|
||||
await _save(str(thread_id), content, path)
|
||||
except Exception as exc: # noqa: BLE001
|
||||
logger.exception("save_plan failed for thread %s", thread_id)
|
||||
return {"success": False, "error": f"failed to save plan: {exc}"}
|
||||
return {"success": True, "path": path}
|
||||
|
||||
|
||||
async def _save(thread_id: str, content: str) -> str:
|
||||
sandbox_path = await write_plan_to_sandbox(thread_id, content)
|
||||
await save_plan_content(thread_id, markdown=content, status=PLAN_STATUS_READY)
|
||||
return sandbox_path
|
||||
async def _save(thread_id: str, content: str, path: str) -> None:
|
||||
await save_plan_content(
|
||||
thread_id, markdown=content, status=PLAN_STATUS_READY, plan_file_path=path
|
||||
)
|
||||
|
||||
|
||||
async def _read_plan_file(thread_id: str, path: str) -> str:
|
||||
backend = await get_sandbox_backend(thread_id)
|
||||
result = await backend.aread(path, offset=0, limit=_MAX_PLAN_LINES)
|
||||
error = _value(result, "error")
|
||||
if error:
|
||||
raise ValueError(error)
|
||||
file_data = _value(result, "file_data")
|
||||
if file_data is None:
|
||||
raise ValueError("plan file could not be read")
|
||||
encoding = _value(file_data, "encoding")
|
||||
if encoding is not None and encoding != "utf-8":
|
||||
raise ValueError("plan file must be UTF-8 text")
|
||||
content = _value(file_data, "content")
|
||||
if not isinstance(content, str):
|
||||
raise ValueError("plan file content was not text")
|
||||
if content.count("\n") + 1 >= _MAX_PLAN_LINES:
|
||||
raise ValueError("plan file is too large")
|
||||
return content
|
||||
|
||||
|
||||
def _value(value: Any, key: str) -> Any:
|
||||
if isinstance(value, Mapping):
|
||||
return value.get(key)
|
||||
return getattr(value, key, None)
|
||||
|
||||
|
||||
def _is_markdown_path(path: str) -> bool:
|
||||
if "\x00" in path or not path.startswith(f"{PLAN_FILE_DIRECTORY}/"):
|
||||
return False
|
||||
filename = path.removeprefix(f"{PLAN_FILE_DIRECTORY}/")
|
||||
if not filename or "/" in filename:
|
||||
return False
|
||||
return filename.lower().endswith(_MARKDOWN_EXTENSIONS)
|
||||
|
|
|
|||
|
|
@ -136,6 +136,8 @@ def _step_reply(messages: list[BaseMessage]) -> AIMessage:
|
|||
|
||||
|
||||
# --- plan-mode flow --------------------------------------------------------
|
||||
PLAN_FILE_PATH = "/workspace/plans/2026-06-29-greet-helper.md"
|
||||
|
||||
PLAN_MARKDOWN = """## Plan: Add greet() helper
|
||||
|
||||
### Overview
|
||||
|
|
@ -185,11 +187,28 @@ def _step_plan_research(_messages: list[BaseMessage]) -> AIMessage:
|
|||
)
|
||||
|
||||
|
||||
def _step_write_plan(_messages: list[BaseMessage]) -> AIMessage:
|
||||
return AIMessage(
|
||||
content="Writing the plan file for review.",
|
||||
tool_calls=[
|
||||
{
|
||||
"name": "write_file",
|
||||
"args": {"file_path": PLAN_FILE_PATH, "content": PLAN_MARKDOWN},
|
||||
"id": "call-write-plan",
|
||||
}
|
||||
],
|
||||
)
|
||||
|
||||
|
||||
def _step_save_plan(_messages: list[BaseMessage]) -> AIMessage:
|
||||
return AIMessage(
|
||||
content="Saving the plan for review.",
|
||||
tool_calls=[
|
||||
{"name": "save_plan", "args": {"plan_markdown": PLAN_MARKDOWN}, "id": "call-save-plan"}
|
||||
{
|
||||
"name": "save_plan",
|
||||
"args": {"plan_file_path": PLAN_FILE_PATH},
|
||||
"id": "call-save-plan",
|
||||
}
|
||||
],
|
||||
)
|
||||
|
||||
|
|
@ -220,6 +239,7 @@ def build_plan_script() -> list[Any]:
|
|||
_step_enter_plan,
|
||||
_step_plan_link,
|
||||
_step_plan_research,
|
||||
_step_write_plan,
|
||||
_step_save_plan,
|
||||
_step_plan_complete,
|
||||
_step_plan_end,
|
||||
|
|
|
|||
|
|
@ -2,8 +2,9 @@ import agent.integrations.local as local_mod
|
|||
|
||||
|
||||
class _StubLocalShellBackend:
|
||||
def __init__(self, *, root_dir, inherit_env):
|
||||
def __init__(self, *, root_dir, virtual_mode, inherit_env):
|
||||
self.root_dir = root_dir
|
||||
self.virtual_mode = virtual_mode
|
||||
self.inherit_env = inherit_env
|
||||
|
||||
|
||||
|
|
@ -16,6 +17,7 @@ def test_create_local_sandbox_creates_missing_root_dir(monkeypatch, tmp_path):
|
|||
|
||||
assert root.is_dir()
|
||||
assert backend.root_dir == str(root)
|
||||
assert backend.virtual_mode is True
|
||||
assert backend.inherit_env is True
|
||||
|
||||
|
||||
|
|
@ -27,3 +29,4 @@ def test_create_local_sandbox_defaults_to_cwd(monkeypatch, tmp_path):
|
|||
backend = local_mod.create_local_sandbox()
|
||||
|
||||
assert backend.root_dir == str(tmp_path)
|
||||
assert backend.virtual_mode is True
|
||||
|
|
|
|||
|
|
@ -24,8 +24,6 @@ def test_plan_mode_prompt_absent_by_default() -> None:
|
|||
def test_plan_mode_excluded_tools_cover_mutating_tools() -> None:
|
||||
excluded = server.PLAN_MODE_EXCLUDED_TOOLS
|
||||
for tool in (
|
||||
"write_file",
|
||||
"edit_file",
|
||||
"task",
|
||||
"open_pull_request",
|
||||
"request_pr_review",
|
||||
|
|
@ -34,8 +32,10 @@ def test_plan_mode_excluded_tools_cover_mutating_tools() -> None:
|
|||
"linear_delete_issue",
|
||||
):
|
||||
assert tool in excluded
|
||||
# Read-only tools must stay available.
|
||||
# Read-only tools and plan-file editing tools must stay available.
|
||||
assert "read_file" not in excluded
|
||||
assert "write_file" not in excluded
|
||||
assert "edit_file" not in excluded
|
||||
assert "execute" not in excluded
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -108,12 +108,12 @@ 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("## Plan")
|
||||
result = await save_plan("/workspace/plans/2026-06-29-test-plan.md")
|
||||
assert result["success"] is False
|
||||
assert "thread_id" in result["error"]
|
||||
|
||||
|
||||
async def test_save_plan_rejects_empty_markdown() -> None:
|
||||
async def test_save_plan_rejects_empty_path() -> None:
|
||||
from agent.tools.save_plan import save_plan
|
||||
|
||||
result = await save_plan(" ")
|
||||
|
|
@ -121,6 +121,70 @@ async def test_save_plan_rejects_empty_markdown() -> None:
|
|||
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-06-29-test-plan.md")
|
||||
|
||||
assert result == {"success": True, "path": "/workspace/plans/2026-06-29-test-plan.md"}
|
||||
assert reads == [
|
||||
("/workspace/plans/2026-06-29-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-06-29-test-plan.md",
|
||||
}
|
||||
|
||||
|
||||
def test_plan_routes_registered() -> None:
|
||||
from agent.webapp import app
|
||||
|
||||
|
|
@ -150,12 +214,27 @@ def test_plan_status_constants() -> None:
|
|||
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
|
||||
|
|
@ -192,6 +271,34 @@ def test_plan_mode_middleware_self_activation_via_state() -> None:
|
|||
# --- manual plan editing -------------------------------------------------
|
||||
|
||||
|
||||
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
|
||||
|
||||
|
||||
async def test_save_plan_content_clear_comments_flag(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
from agent.dashboard import plan_store
|
||||
|
||||
|
|
@ -236,13 +343,24 @@ def _patch_update_plan_deps(
|
|||
return content
|
||||
|
||||
async def fake_save(
|
||||
thread_id: str, *, markdown: str, status: str, clear_comments: bool = True
|
||||
thread_id: str,
|
||||
*,
|
||||
markdown: str,
|
||||
status: str,
|
||||
clear_comments: bool = True,
|
||||
plan_file_path: str | None = None,
|
||||
) -> None:
|
||||
saved.update(markdown=markdown, status=status, clear_comments=clear_comments)
|
||||
saved.update(
|
||||
markdown=markdown,
|
||||
status=status,
|
||||
clear_comments=clear_comments,
|
||||
plan_file_path=plan_file_path,
|
||||
)
|
||||
|
||||
async def fake_write(thread_id: str, c: str) -> str:
|
||||
async def fake_write(thread_id: str, c: str, *, plan_file_path: str | None = None) -> str:
|
||||
sandbox["content"] = c
|
||||
return "plan.md"
|
||||
sandbox["plan_file_path"] = plan_file_path
|
||||
return plan_file_path or "/workspace/plans/fallback.md"
|
||||
|
||||
monkeypatch.setattr(plan_api, "_thread_metadata", fake_meta)
|
||||
monkeypatch.setattr(plan_api, "_user_owns_thread", lambda *a, **k: owner)
|
||||
|
|
@ -262,7 +380,11 @@ async def test_update_plan_owner_saves_and_mirrors_sandbox(
|
|||
monkeypatch,
|
||||
metadata={"plan_status": "ready"},
|
||||
owner=True,
|
||||
content={"markdown": "old", "status": "ready"},
|
||||
content={
|
||||
"markdown": "old",
|
||||
"status": "ready",
|
||||
"plan_file_path": "/workspace/plans/2026-06-29-existing.md",
|
||||
},
|
||||
saved=saved,
|
||||
sandbox=sandbox,
|
||||
)
|
||||
|
|
@ -273,7 +395,9 @@ async def test_update_plan_owner_saves_and_mirrors_sandbox(
|
|||
assert result == {"status": "ready", "markdown": "# New\n\ndo x"}
|
||||
assert saved["status"] == "ready"
|
||||
assert saved["clear_comments"] is False
|
||||
assert saved["plan_file_path"] == "/workspace/plans/2026-06-29-existing.md"
|
||||
assert sandbox["content"] == "# New\n\ndo x"
|
||||
assert sandbox["plan_file_path"] == "/workspace/plans/2026-06-29-existing.md"
|
||||
|
||||
|
||||
async def test_update_plan_rejects_non_owner(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue