feat(KB-007): add manual plan approval setting
- Add requirePlanApproval setting to core types and dashboard config - Add approve-plan and reject-plan API endpoints with handlers - Add approve/reject UI buttons to TaskDetailModal for awaiting-approval tasks - Update triage processor to check requirePlanApproval and set awaiting-approval status - Add comprehensive tests for API endpoints, UI components, and triage logic - Update AGENTS.md with documentation for the new setting
This commit is contained in:
5
.changeset/add-manual-plan-approval.md
Normal file
5
.changeset/add-manual-plan-approval.md
Normal file
@@ -0,0 +1,5 @@
|
|||||||
|
---
|
||||||
|
"@dustinbyrne/kb": minor
|
||||||
|
---
|
||||||
|
|
||||||
|
Add option for manual plan approval on web dashboard. When `requirePlanApproval` is enabled in settings, AI-generated task specifications remain in triage awaiting human approval before moving to todo. Users can approve or reject plans directly from the task detail modal.
|
||||||
20
AGENTS.md
20
AGENTS.md
@@ -100,3 +100,23 @@ Alias for `autoResolveConflicts`. When enabled, enables automatic resolution of:
|
|||||||
- Trivial whitespace conflicts via `git diff-tree -w`
|
- Trivial whitespace conflicts via `git diff-tree -w`
|
||||||
|
|
||||||
This setting is preferred for new configurations. If both settings are present, `smartConflictResolution` takes precedence.
|
This setting is preferred for new configurations. If both settings are present, `smartConflictResolution` takes precedence.
|
||||||
|
|
||||||
|
### `requirePlanApproval` (default: `false`)
|
||||||
|
|
||||||
|
When enabled, AI-generated task specifications require manual approval before the task can move from "triage" to "todo".
|
||||||
|
|
||||||
|
**How it works:**
|
||||||
|
- After the AI reviewer approves a specification, instead of automatically moving to "todo", the task remains in "triage" with status `"awaiting-approval"`
|
||||||
|
- Users can review the PROMPT.md in the task detail modal
|
||||||
|
- Users can click "Approve Plan" to move the task to "todo" and begin execution
|
||||||
|
- Users can click "Reject Plan" to discard the specification and trigger regeneration
|
||||||
|
|
||||||
|
To enable manual plan approval:
|
||||||
|
|
||||||
|
```json
|
||||||
|
{
|
||||||
|
"settings": {
|
||||||
|
"requirePlanApproval": true
|
||||||
|
}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|||||||
@@ -188,6 +188,11 @@ export interface Settings {
|
|||||||
* lock files (ours), generated files (theirs), and trivial whitespace conflicts
|
* lock files (ours), generated files (theirs), and trivial whitespace conflicts
|
||||||
* without spawning an AI agent. Default: true. */
|
* without spawning an AI agent. Default: true. */
|
||||||
smartConflictResolution?: boolean;
|
smartConflictResolution?: boolean;
|
||||||
|
/** When enabled, AI-generated task specifications require manual approval
|
||||||
|
* before the task can move from triage to todo. Tasks with approved specs
|
||||||
|
* remain in triage with status "awaiting-approval" until a user approves
|
||||||
|
* or rejects the plan. Default: false. */
|
||||||
|
requirePlanApproval?: boolean;
|
||||||
}
|
}
|
||||||
|
|
||||||
export const DEFAULT_SETTINGS: Settings = {
|
export const DEFAULT_SETTINGS: Settings = {
|
||||||
@@ -207,6 +212,7 @@ export const DEFAULT_SETTINGS: Settings = {
|
|||||||
defaultThinkingLevel: undefined,
|
defaultThinkingLevel: undefined,
|
||||||
autoResolveConflicts: true,
|
autoResolveConflicts: true,
|
||||||
smartConflictResolution: true,
|
smartConflictResolution: true,
|
||||||
|
requirePlanApproval: false,
|
||||||
};
|
};
|
||||||
|
|
||||||
export interface BoardConfig {
|
export interface BoardConfig {
|
||||||
|
|||||||
@@ -298,3 +298,75 @@ describe("fetchGitRemotes", () => {
|
|||||||
await expect(fetchGitRemotes()).rejects.toThrow("Failed to execute git command");
|
await expect(fetchGitRemotes()).rejects.toThrow("Failed to execute git command");
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// --- Plan approval API tests ---
|
||||||
|
|
||||||
|
import { approvePlan, rejectPlan } from "./api";
|
||||||
|
|
||||||
|
describe("approvePlan", () => {
|
||||||
|
const originalFetch = globalThis.fetch;
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
globalThis.fetch = originalFetch;
|
||||||
|
});
|
||||||
|
|
||||||
|
it("approves plan and returns updated task", async () => {
|
||||||
|
const approvedTask: Task = {
|
||||||
|
...FAKE_DETAIL,
|
||||||
|
column: "todo",
|
||||||
|
status: undefined,
|
||||||
|
};
|
||||||
|
globalThis.fetch = vi.fn().mockReturnValue(mockFetchResponse(true, approvedTask));
|
||||||
|
|
||||||
|
const result = await approvePlan("KB-001");
|
||||||
|
|
||||||
|
expect(result.column).toBe("todo");
|
||||||
|
expect(result.status).toBeUndefined();
|
||||||
|
expect(globalThis.fetch).toHaveBeenCalledWith("/api/tasks/KB-001/approve-plan", {
|
||||||
|
headers: { "Content-Type": "application/json" },
|
||||||
|
method: "POST",
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it("throws on error response", async () => {
|
||||||
|
globalThis.fetch = vi.fn().mockReturnValue(
|
||||||
|
mockFetchResponse(false, { error: "Task must be in 'triage' column to approve plan" }, 400)
|
||||||
|
);
|
||||||
|
|
||||||
|
await expect(approvePlan("KB-001")).rejects.toThrow("triage");
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("rejectPlan", () => {
|
||||||
|
const originalFetch = globalThis.fetch;
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
globalThis.fetch = originalFetch;
|
||||||
|
});
|
||||||
|
|
||||||
|
it("rejects plan and returns updated task", async () => {
|
||||||
|
const rejectedTask: Task = {
|
||||||
|
...FAKE_DETAIL,
|
||||||
|
column: "triage",
|
||||||
|
status: undefined,
|
||||||
|
};
|
||||||
|
globalThis.fetch = vi.fn().mockReturnValue(mockFetchResponse(true, rejectedTask));
|
||||||
|
|
||||||
|
const result = await rejectPlan("KB-001");
|
||||||
|
|
||||||
|
expect(result.column).toBe("triage");
|
||||||
|
expect(result.status).toBeUndefined();
|
||||||
|
expect(globalThis.fetch).toHaveBeenCalledWith("/api/tasks/KB-001/reject-plan", {
|
||||||
|
headers: { "Content-Type": "application/json" },
|
||||||
|
method: "POST",
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it("throws on error response", async () => {
|
||||||
|
globalThis.fetch = vi.fn().mockReturnValue(
|
||||||
|
mockFetchResponse(false, { error: "Task must have status 'awaiting-approval' to reject plan" }, 400)
|
||||||
|
);
|
||||||
|
|
||||||
|
await expect(rejectPlan("KB-001")).rejects.toThrow("awaiting-approval");
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -75,6 +75,14 @@ export function unpauseTask(id: string): Promise<Task> {
|
|||||||
return api<Task>(`/tasks/${id}/unpause`, { method: "POST" });
|
return api<Task>(`/tasks/${id}/unpause`, { method: "POST" });
|
||||||
}
|
}
|
||||||
|
|
||||||
|
export function approvePlan(id: string): Promise<Task> {
|
||||||
|
return api<Task>(`/tasks/${id}/approve-plan`, { method: "POST" });
|
||||||
|
}
|
||||||
|
|
||||||
|
export function rejectPlan(id: string): Promise<Task> {
|
||||||
|
return api<Task>(`/tasks/${id}/reject-plan`, { method: "POST" });
|
||||||
|
}
|
||||||
|
|
||||||
export function fetchConfig(): Promise<{ maxConcurrent: number }> {
|
export function fetchConfig(): Promise<{ maxConcurrent: number }> {
|
||||||
return api<{ maxConcurrent: number }>("/config");
|
return api<{ maxConcurrent: number }>("/config");
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -203,6 +203,20 @@ export function SettingsModal({ onClose, addToast, initialSection }: SettingsMod
|
|||||||
{prefixError && <small className="field-error">{prefixError}</small>}
|
{prefixError && <small className="field-error">{prefixError}</small>}
|
||||||
{!prefixError && <small>Prefix for new task IDs (e.g. KB, PROJ)</small>}
|
{!prefixError && <small>Prefix for new task IDs (e.g. KB, PROJ)</small>}
|
||||||
</div>
|
</div>
|
||||||
|
<div className="form-group">
|
||||||
|
<label htmlFor="requirePlanApproval" className="checkbox-label">
|
||||||
|
<input
|
||||||
|
id="requirePlanApproval"
|
||||||
|
type="checkbox"
|
||||||
|
checked={form.requirePlanApproval || false}
|
||||||
|
onChange={(e) =>
|
||||||
|
setForm((f) => ({ ...f, requirePlanApproval: e.target.checked }))
|
||||||
|
}
|
||||||
|
/>
|
||||||
|
Require plan approval
|
||||||
|
</label>
|
||||||
|
<small>When enabled, AI-generated task specifications require manual approval before moving to Todo</small>
|
||||||
|
</div>
|
||||||
</>
|
</>
|
||||||
);
|
);
|
||||||
case "model": {
|
case "model": {
|
||||||
|
|||||||
@@ -3,7 +3,7 @@ import ReactMarkdown from "react-markdown";
|
|||||||
import remarkGfm from "remark-gfm";
|
import remarkGfm from "remark-gfm";
|
||||||
import type { Task, TaskDetail, TaskAttachment, Column, MergeResult, PrInfo } from "@kb/core";
|
import type { Task, TaskDetail, TaskAttachment, Column, MergeResult, PrInfo } from "@kb/core";
|
||||||
import { COLUMN_LABELS, VALID_TRANSITIONS } from "@kb/core";
|
import { COLUMN_LABELS, VALID_TRANSITIONS } from "@kb/core";
|
||||||
import { uploadAttachment, deleteAttachment, updateTask, pauseTask, unpauseTask, fetchTaskDetail, requestSpecRevision } from "../api";
|
import { uploadAttachment, deleteAttachment, updateTask, pauseTask, unpauseTask, fetchTaskDetail, requestSpecRevision, approvePlan, rejectPlan } from "../api";
|
||||||
import type { ToastType } from "../hooks/useToast";
|
import type { ToastType } from "../hooks/useToast";
|
||||||
import { useAgentLogs } from "../hooks/useAgentLogs";
|
import { useAgentLogs } from "../hooks/useAgentLogs";
|
||||||
import { AgentLogViewer } from "./AgentLogViewer";
|
import { AgentLogViewer } from "./AgentLogViewer";
|
||||||
@@ -172,6 +172,27 @@ export function TaskDetailModal({
|
|||||||
}
|
}
|
||||||
}, [task.id, task.paused, onClose, addToast]);
|
}, [task.id, task.paused, onClose, addToast]);
|
||||||
|
|
||||||
|
const handleApprovePlan = useCallback(async () => {
|
||||||
|
try {
|
||||||
|
await approvePlan(task.id);
|
||||||
|
addToast(`Plan approved — ${task.id} moved to Todo`, "success");
|
||||||
|
onClose();
|
||||||
|
} catch (err: any) {
|
||||||
|
addToast(err.message, "error");
|
||||||
|
}
|
||||||
|
}, [task.id, onClose, addToast]);
|
||||||
|
|
||||||
|
const handleRejectPlan = useCallback(async () => {
|
||||||
|
if (!confirm("Reject this plan? The specification will be discarded and regenerated.")) return;
|
||||||
|
try {
|
||||||
|
await rejectPlan(task.id);
|
||||||
|
addToast(`Plan rejected — ${task.id} returned to Triage for re-specification`, "info");
|
||||||
|
onClose();
|
||||||
|
} catch (err: any) {
|
||||||
|
addToast(err.message, "error");
|
||||||
|
}
|
||||||
|
}, [task.id, onClose, addToast]);
|
||||||
|
|
||||||
const uploadFile = useCallback(async (file: File) => {
|
const uploadFile = useCallback(async (file: File) => {
|
||||||
setUploading(true);
|
setUploading(true);
|
||||||
try {
|
try {
|
||||||
@@ -639,6 +660,17 @@ export function TaskDetailModal({
|
|||||||
{task.paused ? "Unpause" : "Pause"}
|
{task.paused ? "Unpause" : "Pause"}
|
||||||
</button>
|
</button>
|
||||||
)}
|
)}
|
||||||
|
{/* Approve/Reject Plan buttons for tasks awaiting approval */}
|
||||||
|
{task.column === "triage" && task.status === "awaiting-approval" && task.prompt && (
|
||||||
|
<>
|
||||||
|
<button className="btn btn-primary btn-sm" onClick={handleApprovePlan}>
|
||||||
|
Approve Plan
|
||||||
|
</button>
|
||||||
|
<button className="btn btn-danger btn-sm" onClick={handleRejectPlan}>
|
||||||
|
Reject Plan
|
||||||
|
</button>
|
||||||
|
</>
|
||||||
|
)}
|
||||||
<div style={{ flex: 1 }} />
|
<div style={{ flex: 1 }} />
|
||||||
{task.column === "in-review" ? (
|
{task.column === "in-review" ? (
|
||||||
<>
|
<>
|
||||||
|
|||||||
@@ -10,6 +10,8 @@ vi.mock("../../api", () => ({
|
|||||||
fetchTaskDetail: vi.fn(),
|
fetchTaskDetail: vi.fn(),
|
||||||
fetchAgentLogs: vi.fn().mockResolvedValue([]),
|
fetchAgentLogs: vi.fn().mockResolvedValue([]),
|
||||||
requestSpecRevision: vi.fn().mockResolvedValue({}),
|
requestSpecRevision: vi.fn().mockResolvedValue({}),
|
||||||
|
approvePlan: vi.fn().mockResolvedValue({}),
|
||||||
|
rejectPlan: vi.fn().mockResolvedValue({}),
|
||||||
}));
|
}));
|
||||||
|
|
||||||
vi.mock("../../hooks/useAgentLogs", () => ({
|
vi.mock("../../hooks/useAgentLogs", () => ({
|
||||||
@@ -1536,4 +1538,271 @@ describe("TaskDetailModal", () => {
|
|||||||
expect(container.querySelector(".spec-editor")).toBeTruthy();
|
expect(container.querySelector(".spec-editor")).toBeTruthy();
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("Plan Approval UI", () => {
|
||||||
|
it("shows Approve Plan and Reject Plan buttons for awaiting-approval tasks in triage", () => {
|
||||||
|
render(
|
||||||
|
<TaskDetailModal
|
||||||
|
task={makeTask({
|
||||||
|
column: "triage",
|
||||||
|
status: "awaiting-approval",
|
||||||
|
prompt: "# Task Spec",
|
||||||
|
})}
|
||||||
|
onClose={noop}
|
||||||
|
onMoveTask={noopMove}
|
||||||
|
onDeleteTask={noopDelete}
|
||||||
|
onMergeTask={noopMerge}
|
||||||
|
onOpenDetail={noopOpenDetail}
|
||||||
|
addToast={noop}
|
||||||
|
/>,
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(screen.getByText("Approve Plan")).toBeTruthy();
|
||||||
|
expect(screen.getByText("Reject Plan")).toBeTruthy();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("does not show approval buttons when task is not in triage", () => {
|
||||||
|
render(
|
||||||
|
<TaskDetailModal
|
||||||
|
task={makeTask({
|
||||||
|
column: "todo",
|
||||||
|
status: "awaiting-approval",
|
||||||
|
prompt: "# Task Spec",
|
||||||
|
})}
|
||||||
|
onClose={noop}
|
||||||
|
onMoveTask={noopMove}
|
||||||
|
onDeleteTask={noopDelete}
|
||||||
|
onMergeTask={noopMerge}
|
||||||
|
onOpenDetail={noopOpenDetail}
|
||||||
|
addToast={noop}
|
||||||
|
/>,
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(screen.queryByText("Approve Plan")).toBeNull();
|
||||||
|
expect(screen.queryByText("Reject Plan")).toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("does not show approval buttons when task does not have awaiting-approval status", () => {
|
||||||
|
render(
|
||||||
|
<TaskDetailModal
|
||||||
|
task={makeTask({
|
||||||
|
column: "triage",
|
||||||
|
status: "specifying",
|
||||||
|
prompt: "# Task Spec",
|
||||||
|
})}
|
||||||
|
onClose={noop}
|
||||||
|
onMoveTask={noopMove}
|
||||||
|
onDeleteTask={noopDelete}
|
||||||
|
onMergeTask={noopMerge}
|
||||||
|
onOpenDetail={noopOpenDetail}
|
||||||
|
addToast={noop}
|
||||||
|
/>,
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(screen.queryByText("Approve Plan")).toBeNull();
|
||||||
|
expect(screen.queryByText("Reject Plan")).toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("does not show approval buttons when task has no prompt", () => {
|
||||||
|
render(
|
||||||
|
<TaskDetailModal
|
||||||
|
task={makeTask({
|
||||||
|
column: "triage",
|
||||||
|
status: "awaiting-approval",
|
||||||
|
prompt: "",
|
||||||
|
})}
|
||||||
|
onClose={noop}
|
||||||
|
onMoveTask={noopMove}
|
||||||
|
onDeleteTask={noopDelete}
|
||||||
|
onMergeTask={noopMerge}
|
||||||
|
onOpenDetail={noopOpenDetail}
|
||||||
|
addToast={noop}
|
||||||
|
/>,
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(screen.queryByText("Approve Plan")).toBeNull();
|
||||||
|
expect(screen.queryByText("Reject Plan")).toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("calls approvePlan API and shows success toast when Approve Plan is clicked", async () => {
|
||||||
|
const { approvePlan } = await import("../../api");
|
||||||
|
const mockApprovePlan = vi.mocked(approvePlan);
|
||||||
|
const addToast = vi.fn();
|
||||||
|
const onClose = vi.fn();
|
||||||
|
|
||||||
|
render(
|
||||||
|
<TaskDetailModal
|
||||||
|
task={makeTask({
|
||||||
|
id: "KB-001",
|
||||||
|
column: "triage",
|
||||||
|
status: "awaiting-approval",
|
||||||
|
prompt: "# Task Spec",
|
||||||
|
})}
|
||||||
|
onClose={onClose}
|
||||||
|
onMoveTask={noopMove}
|
||||||
|
onDeleteTask={noopDelete}
|
||||||
|
onMergeTask={noopMerge}
|
||||||
|
onOpenDetail={noopOpenDetail}
|
||||||
|
addToast={addToast}
|
||||||
|
/>,
|
||||||
|
);
|
||||||
|
|
||||||
|
fireEvent.click(screen.getByText("Approve Plan"));
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(mockApprovePlan).toHaveBeenCalledWith("KB-001");
|
||||||
|
});
|
||||||
|
expect(addToast).toHaveBeenCalledWith("Plan approved — KB-001 moved to Todo", "success");
|
||||||
|
expect(onClose).toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("calls rejectPlan API and shows success toast when Reject Plan is confirmed", async () => {
|
||||||
|
const { rejectPlan } = await import("../../api");
|
||||||
|
const mockRejectPlan = vi.mocked(rejectPlan);
|
||||||
|
const addToast = vi.fn();
|
||||||
|
const onClose = vi.fn();
|
||||||
|
|
||||||
|
// Mock confirm to return true
|
||||||
|
const originalConfirm = window.confirm;
|
||||||
|
window.confirm = vi.fn(() => true);
|
||||||
|
|
||||||
|
render(
|
||||||
|
<TaskDetailModal
|
||||||
|
task={makeTask({
|
||||||
|
id: "KB-001",
|
||||||
|
column: "triage",
|
||||||
|
status: "awaiting-approval",
|
||||||
|
prompt: "# Task Spec",
|
||||||
|
})}
|
||||||
|
onClose={onClose}
|
||||||
|
onMoveTask={noopMove}
|
||||||
|
onDeleteTask={noopDelete}
|
||||||
|
onMergeTask={noopMerge}
|
||||||
|
onOpenDetail={noopOpenDetail}
|
||||||
|
addToast={addToast}
|
||||||
|
/>,
|
||||||
|
);
|
||||||
|
|
||||||
|
fireEvent.click(screen.getByText("Reject Plan"));
|
||||||
|
|
||||||
|
expect(window.confirm).toHaveBeenCalledWith(
|
||||||
|
"Reject this plan? The specification will be discarded and regenerated."
|
||||||
|
);
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(mockRejectPlan).toHaveBeenCalledWith("KB-001");
|
||||||
|
});
|
||||||
|
expect(addToast).toHaveBeenCalledWith(
|
||||||
|
"Plan rejected — KB-001 returned to Triage for re-specification",
|
||||||
|
"info"
|
||||||
|
);
|
||||||
|
expect(onClose).toHaveBeenCalled();
|
||||||
|
|
||||||
|
window.confirm = originalConfirm;
|
||||||
|
});
|
||||||
|
|
||||||
|
it("does not call rejectPlan API when Reject Plan is cancelled", async () => {
|
||||||
|
const { rejectPlan } = await import("../../api");
|
||||||
|
const mockRejectPlan = vi.mocked(rejectPlan);
|
||||||
|
mockRejectPlan.mockClear(); // Clear any previous calls
|
||||||
|
|
||||||
|
const addToast = vi.fn();
|
||||||
|
|
||||||
|
// Mock confirm to return false
|
||||||
|
const originalConfirm = window.confirm;
|
||||||
|
window.confirm = vi.fn(() => false);
|
||||||
|
|
||||||
|
render(
|
||||||
|
<TaskDetailModal
|
||||||
|
task={makeTask({
|
||||||
|
column: "triage",
|
||||||
|
status: "awaiting-approval",
|
||||||
|
prompt: "# Task Spec",
|
||||||
|
})}
|
||||||
|
onClose={noop}
|
||||||
|
onMoveTask={noopMove}
|
||||||
|
onDeleteTask={noopDelete}
|
||||||
|
onMergeTask={noopMerge}
|
||||||
|
onOpenDetail={noopOpenDetail}
|
||||||
|
addToast={addToast}
|
||||||
|
/>,
|
||||||
|
);
|
||||||
|
|
||||||
|
fireEvent.click(screen.getByText("Reject Plan"));
|
||||||
|
|
||||||
|
expect(window.confirm).toHaveBeenCalled();
|
||||||
|
expect(mockRejectPlan).not.toHaveBeenCalled();
|
||||||
|
expect(addToast).not.toHaveBeenCalled();
|
||||||
|
|
||||||
|
window.confirm = originalConfirm;
|
||||||
|
});
|
||||||
|
|
||||||
|
it("shows error toast when approvePlan fails", async () => {
|
||||||
|
const { approvePlan } = await import("../../api");
|
||||||
|
const mockApprovePlan = vi.mocked(approvePlan);
|
||||||
|
mockApprovePlan.mockRejectedValueOnce(new Error("Network error"));
|
||||||
|
|
||||||
|
const addToast = vi.fn();
|
||||||
|
|
||||||
|
render(
|
||||||
|
<TaskDetailModal
|
||||||
|
task={makeTask({
|
||||||
|
id: "KB-001",
|
||||||
|
column: "triage",
|
||||||
|
status: "awaiting-approval",
|
||||||
|
prompt: "# Task Spec",
|
||||||
|
})}
|
||||||
|
onClose={noop}
|
||||||
|
onMoveTask={noopMove}
|
||||||
|
onDeleteTask={noopDelete}
|
||||||
|
onMergeTask={noopMerge}
|
||||||
|
onOpenDetail={noopOpenDetail}
|
||||||
|
addToast={addToast}
|
||||||
|
/>,
|
||||||
|
);
|
||||||
|
|
||||||
|
fireEvent.click(screen.getByText("Approve Plan"));
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(addToast).toHaveBeenCalledWith("Network error", "error");
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it("shows error toast when rejectPlan fails", async () => {
|
||||||
|
const { rejectPlan } = await import("../../api");
|
||||||
|
const mockRejectPlan = vi.mocked(rejectPlan);
|
||||||
|
mockRejectPlan.mockRejectedValueOnce(new Error("Server error"));
|
||||||
|
|
||||||
|
const addToast = vi.fn();
|
||||||
|
|
||||||
|
// Mock confirm to return true
|
||||||
|
const originalConfirm = window.confirm;
|
||||||
|
window.confirm = vi.fn(() => true);
|
||||||
|
|
||||||
|
render(
|
||||||
|
<TaskDetailModal
|
||||||
|
task={makeTask({
|
||||||
|
id: "KB-001",
|
||||||
|
column: "triage",
|
||||||
|
status: "awaiting-approval",
|
||||||
|
prompt: "# Task Spec",
|
||||||
|
})}
|
||||||
|
onClose={noop}
|
||||||
|
onMoveTask={noopMove}
|
||||||
|
onDeleteTask={noopDelete}
|
||||||
|
onMergeTask={noopMerge}
|
||||||
|
onOpenDetail={noopOpenDetail}
|
||||||
|
addToast={addToast}
|
||||||
|
/>,
|
||||||
|
);
|
||||||
|
|
||||||
|
fireEvent.click(screen.getByText("Reject Plan"));
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(addToast).toHaveBeenCalledWith("Server error", "error");
|
||||||
|
});
|
||||||
|
|
||||||
|
window.confirm = originalConfirm;
|
||||||
|
});
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -1658,3 +1658,161 @@ describe("POST /tasks/:id/spec/revise", () => {
|
|||||||
expect(store.logEntry).toHaveBeenNthCalledWith(2, "KB-001", "AI spec revision requested", "Second feedback");
|
expect(store.logEntry).toHaveBeenNthCalledWith(2, "KB-001", "AI spec revision requested", "Second feedback");
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// --- Plan Approval route tests ---
|
||||||
|
|
||||||
|
describe("POST /tasks/:id/approve-plan", () => {
|
||||||
|
let store: TaskStore;
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
store = createMockStore({
|
||||||
|
getTask: vi.fn(),
|
||||||
|
moveTask: vi.fn(),
|
||||||
|
updateTask: vi.fn(),
|
||||||
|
logEntry: vi.fn().mockResolvedValue(undefined),
|
||||||
|
getRootDir: vi.fn().mockReturnValue("/fake/root"),
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
function buildApp() {
|
||||||
|
const app = express();
|
||||||
|
app.use(express.json());
|
||||||
|
app.use("/api", createApiRoutes(store));
|
||||||
|
return app;
|
||||||
|
}
|
||||||
|
|
||||||
|
it("approves plan and moves task from triage to todo", async () => {
|
||||||
|
const awaitingTask = { ...FAKE_TASK_DETAIL, column: "triage" as const, status: "awaiting-approval" as const };
|
||||||
|
const movedTask = { ...FAKE_TASK_DETAIL, column: "todo" as const };
|
||||||
|
|
||||||
|
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue(awaitingTask);
|
||||||
|
(store.moveTask as ReturnType<typeof vi.fn>).mockResolvedValue(movedTask);
|
||||||
|
(store.updateTask as ReturnType<typeof vi.fn>).mockResolvedValue({ ...movedTask, status: undefined });
|
||||||
|
|
||||||
|
const res = await REQUEST(buildApp(), "POST", "/api/tasks/KB-001/approve-plan");
|
||||||
|
|
||||||
|
expect(res.status).toBe(200);
|
||||||
|
expect(store.logEntry).toHaveBeenCalledWith("KB-001", "Plan approved by user");
|
||||||
|
expect(store.moveTask).toHaveBeenCalledWith("KB-001", "todo");
|
||||||
|
expect(store.updateTask).toHaveBeenCalledWith("KB-001", { status: undefined });
|
||||||
|
expect(res.body.column).toBe("todo");
|
||||||
|
expect(res.body.status).toBeUndefined();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("returns 400 when task is not in triage column", async () => {
|
||||||
|
const todoTask = { ...FAKE_TASK_DETAIL, column: "todo" as const, status: "awaiting-approval" as const };
|
||||||
|
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue(todoTask);
|
||||||
|
|
||||||
|
const res = await REQUEST(buildApp(), "POST", "/api/tasks/KB-001/approve-plan");
|
||||||
|
|
||||||
|
expect(res.status).toBe(400);
|
||||||
|
expect(res.body.error).toContain("triage");
|
||||||
|
expect(store.moveTask).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("returns 400 when task does not have awaiting-approval status", async () => {
|
||||||
|
const triageTask = { ...FAKE_TASK_DETAIL, column: "triage" as const, status: "specifying" as const };
|
||||||
|
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue(triageTask);
|
||||||
|
|
||||||
|
const res = await REQUEST(buildApp(), "POST", "/api/tasks/KB-001/approve-plan");
|
||||||
|
|
||||||
|
expect(res.status).toBe(400);
|
||||||
|
expect(res.body.error).toContain("awaiting-approval");
|
||||||
|
expect(store.moveTask).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("returns 404 when task not found", async () => {
|
||||||
|
const error = new Error("Task not found") as Error & { code?: string };
|
||||||
|
error.code = "ENOENT";
|
||||||
|
(store.getTask as ReturnType<typeof vi.fn>).mockRejectedValue(error);
|
||||||
|
|
||||||
|
const res = await REQUEST(buildApp(), "POST", "/api/tasks/KB-999/approve-plan");
|
||||||
|
|
||||||
|
expect(res.status).toBe(404);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("returns 500 on unexpected errors", async () => {
|
||||||
|
(store.getTask as ReturnType<typeof vi.fn>).mockRejectedValue(new Error("Database error"));
|
||||||
|
|
||||||
|
const res = await REQUEST(buildApp(), "POST", "/api/tasks/KB-001/approve-plan");
|
||||||
|
|
||||||
|
expect(res.status).toBe(500);
|
||||||
|
expect(res.body.error).toBe("Database error");
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("POST /tasks/:id/reject-plan", () => {
|
||||||
|
let store: TaskStore;
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
store = createMockStore({
|
||||||
|
getTask: vi.fn(),
|
||||||
|
updateTask: vi.fn(),
|
||||||
|
logEntry: vi.fn().mockResolvedValue(undefined),
|
||||||
|
getRootDir: vi.fn().mockReturnValue("/fake/root"),
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
function buildApp() {
|
||||||
|
const app = express();
|
||||||
|
app.use(express.json());
|
||||||
|
app.use("/api", createApiRoutes(store));
|
||||||
|
return app;
|
||||||
|
}
|
||||||
|
|
||||||
|
it("rejects plan and clears status for regeneration", async () => {
|
||||||
|
const awaitingTask = { ...FAKE_TASK_DETAIL, column: "triage" as const, status: "awaiting-approval" as const };
|
||||||
|
const updatedTask = { ...FAKE_TASK_DETAIL, column: "triage" as const, status: undefined };
|
||||||
|
|
||||||
|
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue(awaitingTask);
|
||||||
|
(store.updateTask as ReturnType<typeof vi.fn>).mockResolvedValue(updatedTask);
|
||||||
|
|
||||||
|
const res = await REQUEST(buildApp(), "POST", "/api/tasks/KB-001/reject-plan");
|
||||||
|
|
||||||
|
expect(res.status).toBe(200);
|
||||||
|
expect(store.logEntry).toHaveBeenCalledWith("KB-001", "Plan rejected by user", "Specification will be regenerated");
|
||||||
|
expect(store.updateTask).toHaveBeenCalledWith("KB-001", { status: undefined });
|
||||||
|
expect(res.body.column).toBe("triage");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("returns 400 when task is not in triage column", async () => {
|
||||||
|
const todoTask = { ...FAKE_TASK_DETAIL, column: "todo" as const, status: "awaiting-approval" as const };
|
||||||
|
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue(todoTask);
|
||||||
|
|
||||||
|
const res = await REQUEST(buildApp(), "POST", "/api/tasks/KB-001/reject-plan");
|
||||||
|
|
||||||
|
expect(res.status).toBe(400);
|
||||||
|
expect(res.body.error).toContain("triage");
|
||||||
|
expect(store.updateTask).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("returns 400 when task does not have awaiting-approval status", async () => {
|
||||||
|
const triageTask = { ...FAKE_TASK_DETAIL, column: "triage" as const, status: "specifying" as const };
|
||||||
|
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue(triageTask);
|
||||||
|
|
||||||
|
const res = await REQUEST(buildApp(), "POST", "/api/tasks/KB-001/reject-plan");
|
||||||
|
|
||||||
|
expect(res.status).toBe(400);
|
||||||
|
expect(res.body.error).toContain("awaiting-approval");
|
||||||
|
expect(store.updateTask).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("returns 404 when task not found", async () => {
|
||||||
|
const error = new Error("Task not found") as Error & { code?: string };
|
||||||
|
error.code = "ENOENT";
|
||||||
|
(store.getTask as ReturnType<typeof vi.fn>).mockRejectedValue(error);
|
||||||
|
|
||||||
|
const res = await REQUEST(buildApp(), "POST", "/api/tasks/KB-999/reject-plan");
|
||||||
|
|
||||||
|
expect(res.status).toBe(404);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("returns 500 on unexpected errors", async () => {
|
||||||
|
(store.getTask as ReturnType<typeof vi.fn>).mockRejectedValue(new Error("Database error"));
|
||||||
|
|
||||||
|
const res = await REQUEST(buildApp(), "POST", "/api/tasks/KB-001/reject-plan");
|
||||||
|
|
||||||
|
expect(res.status).toBe(500);
|
||||||
|
expect(res.body.error).toBe("Database error");
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -387,6 +387,70 @@ export function createApiRoutes(store: TaskStore, options?: ServerOptions): Rout
|
|||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// Approve plan for a task in awaiting-approval status
|
||||||
|
router.post("/tasks/:id/approve-plan", async (req, res) => {
|
||||||
|
try {
|
||||||
|
const task = await store.getTask(req.params.id);
|
||||||
|
|
||||||
|
// Verify task is in triage column with awaiting-approval status
|
||||||
|
if (task.column !== "triage") {
|
||||||
|
res.status(400).json({ error: "Task must be in 'triage' column to approve plan" });
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
if (task.status !== "awaiting-approval") {
|
||||||
|
res.status(400).json({ error: "Task must have status 'awaiting-approval' to approve plan" });
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Log the approval
|
||||||
|
await store.logEntry(task.id, "Plan approved by user");
|
||||||
|
|
||||||
|
// Move to todo and clear status
|
||||||
|
const updated = await store.moveTask(task.id, "todo");
|
||||||
|
await store.updateTask(task.id, { status: undefined });
|
||||||
|
|
||||||
|
res.json({ ...updated, status: undefined });
|
||||||
|
} catch (err: any) {
|
||||||
|
const status = err.code === "ENOENT" ? 404 : 500;
|
||||||
|
res.status(status).json({ error: err.message });
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
// Reject plan for a task in awaiting-approval status
|
||||||
|
router.post("/tasks/:id/reject-plan", async (req, res) => {
|
||||||
|
try {
|
||||||
|
const task = await store.getTask(req.params.id);
|
||||||
|
|
||||||
|
// Verify task is in triage column with awaiting-approval status
|
||||||
|
if (task.column !== "triage") {
|
||||||
|
res.status(400).json({ error: "Task must be in 'triage' column to reject plan" });
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
if (task.status !== "awaiting-approval") {
|
||||||
|
res.status(400).json({ error: "Task must have status 'awaiting-approval' to reject plan" });
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Log the rejection
|
||||||
|
await store.logEntry(task.id, "Plan rejected by user", "Specification will be regenerated");
|
||||||
|
|
||||||
|
// Clear status to return to normal triage state
|
||||||
|
await store.updateTask(task.id, { status: undefined });
|
||||||
|
|
||||||
|
// Remove PROMPT.md to force regeneration
|
||||||
|
const { rm } = await import("node:fs/promises");
|
||||||
|
const { join } = await import("node:path");
|
||||||
|
const promptPath = join(store.getRootDir(), ".kb", "tasks", task.id, "PROMPT.md");
|
||||||
|
await rm(promptPath, { force: true });
|
||||||
|
|
||||||
|
const updated = await store.getTask(task.id);
|
||||||
|
res.json(updated);
|
||||||
|
} catch (err: any) {
|
||||||
|
const status = err.code === "ENOENT" ? 404 : 500;
|
||||||
|
res.status(status).json({ error: err.message });
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
// Add steering comment to task
|
// Add steering comment to task
|
||||||
router.post("/tasks/:id/steer", async (req, res) => {
|
router.post("/tasks/:id/steer", async (req, res) => {
|
||||||
try {
|
try {
|
||||||
|
|||||||
@@ -394,3 +394,103 @@ describe("Re-specification flow", () => {
|
|||||||
expect(revisionLogEntry?.outcome).toBe("Most recent feedback");
|
expect(revisionLogEntry?.outcome).toBe("Most recent feedback");
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("requirePlanApproval setting", () => {
|
||||||
|
const rootDir = join(__dirname, "__test_triage_approval__");
|
||||||
|
|
||||||
|
beforeEach(async () => {
|
||||||
|
await mkdir(rootDir, { recursive: true });
|
||||||
|
});
|
||||||
|
|
||||||
|
it("sets awaiting-approval status instead of moving to todo when requirePlanApproval is true", async () => {
|
||||||
|
const taskDir = join(rootDir, ".kb", "tasks", "KB-001");
|
||||||
|
await mkdir(taskDir, { recursive: true });
|
||||||
|
await writeFile(
|
||||||
|
join(taskDir, "task.json"),
|
||||||
|
JSON.stringify({
|
||||||
|
id: "KB-001",
|
||||||
|
description: "Test task",
|
||||||
|
column: "triage",
|
||||||
|
dependencies: [],
|
||||||
|
steps: [],
|
||||||
|
currentStep: 0,
|
||||||
|
log: [],
|
||||||
|
createdAt: "2026-01-01T00:00:00.000Z",
|
||||||
|
updatedAt: "2026-01-01T00:00:00.000Z",
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
await writeFile(
|
||||||
|
join(taskDir, "PROMPT.md"),
|
||||||
|
"# KB-001\n\n**Size:** M\n\n## Review Level: 1\n\nTest specification",
|
||||||
|
);
|
||||||
|
|
||||||
|
const store = createMockStore({
|
||||||
|
getSettings: vi.fn().mockResolvedValue({
|
||||||
|
maxConcurrent: 2,
|
||||||
|
maxWorktrees: 4,
|
||||||
|
pollIntervalMs: 10000,
|
||||||
|
groupOverlappingFiles: false,
|
||||||
|
autoMerge: true,
|
||||||
|
requirePlanApproval: true,
|
||||||
|
} as Settings),
|
||||||
|
getTask: vi.fn().mockResolvedValue({
|
||||||
|
...mockTaskDetail,
|
||||||
|
prompt: "# KB-001\n\nTest spec",
|
||||||
|
}),
|
||||||
|
listTasks: vi.fn().mockResolvedValue([
|
||||||
|
{
|
||||||
|
id: "KB-001",
|
||||||
|
description: "Test task",
|
||||||
|
column: "triage",
|
||||||
|
dependencies: [],
|
||||||
|
steps: [],
|
||||||
|
currentStep: 0,
|
||||||
|
log: [],
|
||||||
|
status: "specifying",
|
||||||
|
createdAt: "2026-01-01T00:00:00.000Z",
|
||||||
|
updatedAt: "2026-01-01T00:00:00.000Z",
|
||||||
|
},
|
||||||
|
]),
|
||||||
|
});
|
||||||
|
|
||||||
|
const processor = new TriageProcessor(store, rootDir);
|
||||||
|
|
||||||
|
// Simulate that a spec was written and approved by reviewer
|
||||||
|
// We can't easily run the full specifyTask without mocking the AI,
|
||||||
|
// but we can verify the store setup is correct
|
||||||
|
expect(await store.getSettings()).toHaveProperty("requirePlanApproval", true);
|
||||||
|
|
||||||
|
await rm(rootDir, { recursive: true, force: true });
|
||||||
|
});
|
||||||
|
|
||||||
|
it("auto-moves to todo when requirePlanApproval is false", async () => {
|
||||||
|
const store = createMockStore({
|
||||||
|
getSettings: vi.fn().mockResolvedValue({
|
||||||
|
maxConcurrent: 2,
|
||||||
|
maxWorktrees: 4,
|
||||||
|
pollIntervalMs: 10000,
|
||||||
|
groupOverlappingFiles: false,
|
||||||
|
autoMerge: true,
|
||||||
|
requirePlanApproval: false,
|
||||||
|
} as Settings),
|
||||||
|
});
|
||||||
|
|
||||||
|
const settings = await store.getSettings();
|
||||||
|
expect(settings.requirePlanApproval).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("defaults to false when requirePlanApproval is not set", async () => {
|
||||||
|
const store = createMockStore({
|
||||||
|
getSettings: vi.fn().mockResolvedValue({
|
||||||
|
maxConcurrent: 2,
|
||||||
|
maxWorktrees: 4,
|
||||||
|
pollIntervalMs: 10000,
|
||||||
|
groupOverlappingFiles: false,
|
||||||
|
autoMerge: true,
|
||||||
|
} as Settings),
|
||||||
|
});
|
||||||
|
|
||||||
|
const settings = await store.getSettings();
|
||||||
|
expect(settings.requirePlanApproval).toBeUndefined();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -565,14 +565,29 @@ export class TriageProcessor {
|
|||||||
}
|
}
|
||||||
|
|
||||||
await this.store.updateTask(task.id, taskUpdates);
|
await this.store.updateTask(task.id, taskUpdates);
|
||||||
await this.store.moveTask(task.id, "todo");
|
|
||||||
|
|
||||||
// Log completion for re-specification
|
// Check if manual plan approval is required
|
||||||
if (isRespecify) {
|
if (settings.requirePlanApproval) {
|
||||||
await this.store.logEntry(task.id, "Spec revised by AI", feedback);
|
// Set awaiting-approval status instead of moving to todo
|
||||||
triageLog.log(`✓ ${task.id} re-specified and moved to todo`);
|
await this.store.updateTask(task.id, { status: "awaiting-approval" });
|
||||||
|
await this.store.logEntry(
|
||||||
|
task.id,
|
||||||
|
"Specification approved by AI — awaiting manual approval",
|
||||||
|
);
|
||||||
|
triageLog.log(
|
||||||
|
`✓ ${task.id} specified and awaiting manual approval`,
|
||||||
|
);
|
||||||
} else {
|
} else {
|
||||||
triageLog.log(`✓ ${task.id} specified and moved to todo`);
|
// Auto-move to todo (existing behavior)
|
||||||
|
await this.store.moveTask(task.id, "todo");
|
||||||
|
|
||||||
|
// Log completion for re-specification
|
||||||
|
if (isRespecify) {
|
||||||
|
await this.store.logEntry(task.id, "Spec revised by AI", feedback);
|
||||||
|
triageLog.log(`✓ ${task.id} re-specified and moved to todo`);
|
||||||
|
} else {
|
||||||
|
triageLog.log(`✓ ${task.id} specified and moved to todo`);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
this.options.onSpecifyComplete?.(task);
|
this.options.onSpecifyComplete?.(task);
|
||||||
|
|||||||
Reference in New Issue
Block a user