Skip to content

fix(plans): /revise route now dispatches plan_update prompt#75

Merged
pufit merged 1 commit into
mainfrom
pufit/fix-revise-route-plan-update
May 15, 2026
Merged

fix(plans): /revise route now dispatches plan_update prompt#75
pufit merged 1 commit into
mainfrom
pufit/fix-revise-route-plan-update

Conversation

@pufit
Copy link
Copy Markdown
Member

@pufit pufit commented May 15, 2026

Summary

The web UI's Revise button silently no-op'd. The HTTP route /api/plans/{plan_id}/revise still instructed the planner to call plan_propose, which now refuses with "Task X already has a pending or implementing plan. Skip it." whenever a pending plan exists for the task — i.e. every revise attempt. The request returned 200 {\"status\": \"revision_requested\"}, so the failure was invisible.

Fix: dispatch the plan_update prompt (mirroring the MCP plan_revise tool) and extract a shared request_plan_revision() helper so the two surfaces can't drift apart again.

Changes

  • New nerve/agent/plan_service.py — single source of truth for revision dispatch. Exposes request_plan_revision(db, engine, plan_id, feedback) plus PlanNotFound / TaskNotFound / PlanNotPending exceptions. Prompt template is a module-level constant.
  • HTTP route nerve/gateway/routes/plans.py — thin wrapper around the helper; maps exceptions to 400 (empty feedback), 404 (plan/task missing), 409 (non-pending). Previously every error path returned 200 with feedback silently stored on the plan.
  • MCP tool nerve/agent/tools.py::plan_revise — same thin-wrapper treatment.
  • FrontendplanStore.revisePlan now returns Promise<boolean>, sets actionError on failure, and parses FastAPI's {detail: ...} JSON envelope. PlanDetailPage renders the error inline next to the textarea; the form only collapses on success.

Test plan

  • tests/test_plan_revise.py — 17 new tests covering the helper (prompt content, feedback persistence, session routing, fallback to cron:task-planner, all three exceptions, empty-feedback ValueError), the MCP tool wrapper, and the HTTP route via FastAPI TestClient.
  • Full suite green: 564 passed in 33s
  • npm run build clean
  • Manual smoke test post-merge against one of the 85 pending plans:
    curl -X POST http://localhost:8900/api/plans/<some-pending-plan>/revise \
         -H \"Content-Type: application/json\" \
         -d '{\"feedback\":\"smoke test — be more specific about edge cases\"}'
    # Expect: original plan superseded + v2 pending within ~60s

Out of scope

The dead supersede-loop in _plan_propose_impl (tools.py:1131-1133) is now unreachable from the revise path. Harmless dead code; separate cleanup PR.

Generated by Nerve

The HTTP /api/plans/{plan_id}/revise route was still telling the
planner to call plan_propose, which refuses when a pending plan
already exists for the task. Result: clicking Revise in the UI
silently no-op'd while the request returned 200.

Extract a shared request_plan_revision helper in nerve.agent.plan_service
so the HTTP route and MCP plan_revise tool can't drift apart on the
prompt template again. The route now also rejects non-pending plans
with a real 409 and surfaces validation errors with 400/404, letting
the UI display a meaningful message instead of dropping the request.

- new: nerve/agent/plan_service.py (PlanNotFound/TaskNotFound/PlanNotPending)
- HTTP and MCP surfaces become thin wrappers around the helper
- planStore.ts gains actionError state; PlanDetailPage shows it inline
- tests/test_plan_revise.py covers helper + tool + HTTP route paths
@pufit pufit merged commit 7c62ea0 into main May 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant