diff --git a/.changeset/fluffy-configs-move.md b/.changeset/fluffy-configs-move.md new file mode 100644 index 0000000..35fd69b --- /dev/null +++ b/.changeset/fluffy-configs-move.md @@ -0,0 +1,8 @@ +--- +"warpforge": patch +--- + +The workspace config has a new preferred home at `.warpforge/workspace.yaml`, +alongside the new `.warpforge/workflows/` directory. Existing config files in +the project root keep working exactly as before; only newly generated configs +land in the `.warpforge/` directory. diff --git a/.changeset/wide-pipelines-arrive.md b/.changeset/wide-pipelines-arrive.md new file mode 100644 index 0000000..b3a9692 --- /dev/null +++ b/.changeset/wide-pipelines-arrive.md @@ -0,0 +1,31 @@ +--- +"warpforge": minor +--- + +Adds configurable workflows: a task can now run as a pipeline of agent stages +instead of a single session. A workflow is a YAML file in your project's +`.warpforge/workflows/`, and it decides which agent and model runs each stage, +what each stage is told to do, what the reviewers see, and how many review +rounds are allowed. Two built-in templates ship with the app — "Implement + +review loop" and "Plan + implement + review loop" — and either can be copied +into a project in one click to customize. + +Pick a workflow in the New Task dialog and the daemon drives the run: it plans +(if the workflow asks for it), implements, then loops review and repair until +the reviewers approve or the round limit runs out. Reviewers can be several +different agents at once and return structured verdicts, and a repeat round +continues in the same reviewer's session so it verifies its own findings +instead of reviewing from scratch. + +The pipeline reports to the parent task as a timeline of stages, each with the +agents that ran it — click one to open that agent's own session. It also stops +for you when it needs to: a stage can ask a question, and running out of review +rounds asks whether to grant more, finish as is, or stop. Pipelines can be +paused between stages and resumed with extra guidance, survive a daemon restart +by parking at their last safe point, and never commit anything — a finished run +lands in Needs review for you to inspect. + +Reviewers can pin each finding to a line and a short code excerpt, so the +repair stage goes straight to the right place instead of searching, and the +summary a stage hands to the next one is its closing message rather than the +whole turn's tool narration. diff --git a/.warpforge/workflows/review-loop.yaml b/.warpforge/workflows/review-loop.yaml new file mode 100644 index 0000000..7385e87 --- /dev/null +++ b/.warpforge/workflows/review-loop.yaml @@ -0,0 +1,67 @@ +# Built-in workflow: implement → review ⇄ fix. +# +# This workflow ALWAYS starts with implementation. Omitting `implement` only +# selects Warpforge's built-in implementation settings/prompt; it never removes +# this fixed stage. Copy the file into /.warpforge/workflows/ (or use +# "Copy to project" in the New Task dialog) to customize it. +version: 1 +name: Implement + review loop +description: Implement the task, then loop reviews and fixes until reviewers approve. + +# Uncomment agent/model to pin this stage. When omitted, they fall back to the +# lead agent/model picked in the New Task dialog. +implement: + # agent: claude + # model: claude-opus-5 + prompt: | + You are the implementation stage of a workflow pipeline. Implement the task + completely: write the code, keep the change focused, and verify your work + with builds/tests where feasible. Your final message must summarize what + you did because it is passed to the reviewers. + + ## Task + {{task_prompt}} + +review: + max_rounds: 2 # review ⇄ fix iterations before asking you what to do + on_limit: ask # ask | finish — behaviour when the limit is hit + context: [prompt, implementer_summary, diff] + reviewers: + # Each reviewer may use a different agent/model. Omitted values use Lead. + - agent: codex + model: gpt-5.6-sol + # focus: correctness and edge cases + prompt: | + You are a code reviewer in workflow round {{round}}/{{max_rounds}}. + Review the implementation against the task. Do not edit files. Verify + claims against the diff and report only concrete, actionable problems. + + ## Task + {{task_prompt}} + + ## Implementer's summary + {{implementer_summary}} + + ## Working-copy diff + {{diff}} + +fix: + # Uncomment to override the implementation stage. Omitted values inherit it. + # agent: claude + # model: claude-opus-5 + prompt: | + You are the repair stage of workflow round {{round}}/{{max_rounds}}. + Address every reviewer finding: fix it or, if it is factually wrong, + explain why in your final summary. Do not change unrelated code. Verify the + result where feasible and summarize what changed for the next review. + + ## Task + {{task_prompt}} + + ## Findings to address + {{findings}} + + ## Current working-copy diff + {{diff}} + +# Warpforge appends the need_user_input/verdict JSON protocols to these prompts. diff --git a/CLAUDE.md b/CLAUDE.md index df13a84..93def38 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -12,7 +12,7 @@ Workspace orchestrator with TUI and desktop interfaces. Manages multiple dev pro - **PTY:** `portable-pty` — real TTY for spawned agents - **Terminal emulation:** `vt100` crate — parses ANSI into screen buffer - **CLI:** `clap` derive API -- **Config:** `.workspace.yaml` per project (serde_yaml), registry in `~/.warpforge/projects.json` +- **Config:** `.warpforge/workspace.yaml` per project (serde_yaml; legacy root-level `.warpforge.yaml` / `.wf.yaml` / `.workspace.yaml` still load), registry in `~/.warpforge/projects.json` - **Port isolation:** each project gets 100-port range (4000+), `${svc.port}` interpolation in env vars - **Daemon IPC:** WebSocket JSON-RPC (port 61814) @@ -40,7 +40,8 @@ main.rs — CLI entry (clap), subcommands: add, remove, list, ui app.rs — TUI event loop (tokio::select), AppState, InputMode (Navigate/Terminal), key handling agent.rs — AgentManager: PTY spawn via portable-pty, vt100 parser, input/output channels service.rs — ServiceManager: sh -c spawn, stdout/stderr log capture, process-group kill, port allocation -config.rs — .workspace.yaml parsing + auto-detect (package.json scripts, docker-compose) +config.rs — workspace config parsing + auto-detect (package.json scripts, docker-compose) +workflow_config.rs — .warpforge/workflows/*.yaml templates: schema, validation, prompt rendering registry.rs — ~/.warpforge/projects.json CRUD ports.rs — Port range allocation (4000+), ${svc.port} env interpolation tui/ @@ -54,6 +55,9 @@ tui/ - WebSocket server (port 61814) that desktop/TUI connect to - Manages agents, services, port-forwards, tasks, git operations - Key files: server.rs, actor.rs, task.rs, agents.rs, diff.rs +- **workflow.rs** — deterministic workflow pipeline: run state, stage prompts, + agent protocols. Actor glue is the workflow block at the end of actor.rs. + Templates are parsed by `src/workflow_config.rs`; see `docs/adr/0001`. **Desktop source (`desktop/src/`):** ``` @@ -112,6 +116,10 @@ lib/ — Utilities (38 files: sessionActivity, sessionTiming, e - **Agent setup:** detect/install agents (Claude, Codex), bootstrap wizard - **UI state:** Zustand store with localStorage persistence (panels, toggles, view) - **Orchestration:** planner → workers → reviewers pipeline (daemon-driven) +- **Workflows:** pick a configured pipeline (`.warpforge/workflows/*.yaml`) in + New Task; the daemon runs `plan? → implement → review ⇄ fix` as child tasks, + asks you at barriers (stage question, review limit), and supports + pause/resume. See `docs/adr/0001-workflow-pipelines.md` before changing it. ### Known Issues / TODO @@ -123,6 +131,11 @@ lib/ — Utilities (38 files: sessionActivity, sessionTiming, e ### Key Design Decisions +**Read `docs/adr/` before changing a subsystem it covers.** Those records hold +the rejected alternatives and the invariants that are not visible in the code — +each entry's *Invariants* section lists mistakes already made once. Add a new +numbered record when you make a decision a future reader could not infer. + - **Ratatui + vt100** for terminal-in-terminal: native cell-by-cell rendering with zero translation layer. Previous TypeScript/OpenTUI attempt couldn't render ANSI properly. - **vt100 + TerminalPane:** Agent PTY output → `vt100::Parser::process()` → `Screen` → iterate cells with colors/attributes → write directly to ratatui `Buffer`. Cursor rendered via `Modifier::REVERSED`. - **portable-pty:** Cross-platform PTY. Reader/writer in `spawn_blocking` tasks, input via `mpsc::unbounded_channel`. diff --git a/crates/warpforge-protocol/src/lib.rs b/crates/warpforge-protocol/src/lib.rs index 7d4189d..8348616 100644 --- a/crates/warpforge-protocol/src/lib.rs +++ b/crates/warpforge-protocol/src/lib.rs @@ -205,6 +205,13 @@ pub enum Method { /// after the model. Unknown option ids are logged and skipped. #[serde(default)] config_overrides: HashMap, + /// When set, run this task as a deterministic workflow pipeline: the + /// created task becomes the pipeline parent (no agent session of its + /// own) and the daemon drives plan? → implement → review ⇄ fix stages + /// as child tasks. The id comes from `workflow.list`. Mutually + /// exclusive with the orchestrator-chat mode. + #[serde(default)] + workflow: Option, }, #[serde(rename = "task.cancel")] TaskCancel { task_id: String }, @@ -432,6 +439,50 @@ pub enum Method { #[serde(rename = "orchestrate.saveConfig")] OrchestrateSaveConfig { config: OrchestratorConfigDto }, + // ── Workflows (deterministic pipeline templates) ── + /// List workflows selectable for a project: `.warpforge/workflows/*.yaml` + /// plus built-in templates (a project file overrides the built-in with the + /// same id). Returns `{ "workflows": [WorkflowMeta] }`. + #[serde(rename = "workflow.list")] + WorkflowList { project: String }, + /// Copy a built-in workflow into the project's `.warpforge/workflows/` + /// directory so it can be customized. Refuses to overwrite an existing + /// file. Returns `{ "path": … }`. + #[serde(rename = "workflow.eject")] + WorkflowEject { project: String, id: String }, + /// Soft-pause a running workflow pipeline: the current stage finishes its + /// turn, the next stage does not start. Errors when the pipeline is not + /// in a pausable state (already waiting for the user, or finished). + #[serde(rename = "workflow.pause")] + WorkflowPause { task: String }, + /// Resume a paused pipeline from its stage barrier. `note`, when set, is + /// delivered to the next stage as an extra "User guidance" block. + #[serde(rename = "workflow.resume")] + WorkflowResume { + task: String, + #[serde(default)] + note: Option, + }, + /// Answer a stage's pending `need_user_input` question. The message is + /// forwarded verbatim to the session that asked. Errors unless the + /// pipeline is waiting on a question. + #[serde(rename = "workflow.reply")] + WorkflowReply { task: String, message: String }, + /// Decide what an out-of-rounds pipeline does next. Errors unless the + /// pipeline is waiting on a limit decision. + #[serde(rename = "workflow.decide")] + WorkflowDecide { + task: String, + decision: WorkflowDecision, + /// For `extend`: how many extra review ⇄ fix rounds to grant (1..=5, + /// default 1). + #[serde(default)] + rounds: Option, + /// Optional extra guidance delivered to the next fix stage. + #[serde(default)] + note: Option, + }, + // ── Bootstrap wizard (desktop) ── /// Scan the repo, build the bootstrap prompt from the user's answers, and /// create a config-gen task. Returns `{ taskId }`. @@ -746,6 +797,9 @@ pub struct TaskInfo { /// on the wire lets clients present the child in its parent's context. #[serde(default, skip_serializing_if = "Option::is_none")] pub parent_task_id: Option, + /// Live workflow pipeline state for workflow parent tasks. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub workflow_run: Option, /// Explicit settle override (true = settled, false = not settled). /// `None` = derive from execution status only (no manual override). #[serde(default, skip_serializing_if = "Option::is_none")] @@ -789,6 +843,38 @@ pub struct SessionUsageCost { pub currency: String, } +/// One agent session referenced by an inline workflow timeline event. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +#[serde(rename_all = "camelCase")] +pub struct WorkflowEventAgent { + pub task_id: String, + pub label: String, + pub agent: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub model: Option, +} + +#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)] +#[serde(rename_all = "snake_case")] +pub enum WorkflowEventKind { + WorkflowStarted, + StageStarted, + AgentOutput, + ReviewResult, + Status, + WorkflowFinished, +} + +#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)] +#[serde(rename_all = "snake_case")] +pub enum WorkflowEventTone { + Info, + Running, + Success, + Warning, + Error, +} + #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] #[serde(tag = "kind", rename_all = "snake_case")] pub enum SessionUpdate { @@ -806,6 +892,20 @@ pub enum SessionUpdate { AgentText { text: String, }, + /// A durable, independently rendered entry in a workflow parent's + /// Conversation timeline. Unlike streamed AgentText chunks these records + /// never coalesce, and agent references remain clickable after completion. + WorkflowEvent { + event: WorkflowEventKind, + title: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + detail: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + stage: Option, + #[serde(default)] + agents: Vec, + tone: WorkflowEventTone, + }, AgentThought { text: String, }, @@ -1303,6 +1403,8 @@ pub enum OrchNodeKind { Implement, Review, Merge, + /// Workflow-pipeline repair stage (fix findings from a review round). + Fix, } #[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)] @@ -1315,6 +1417,125 @@ pub enum OrchNodeStatus { Skipped, } +/// One selectable workflow template, as returned by `workflow.list`. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +#[serde(rename_all = "camelCase")] +pub struct WorkflowMeta { + pub id: String, + /// Display name from the YAML; falls back to the id for invalid files. + pub name: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub description: Option, + pub source: WorkflowSource, + /// False when the file failed to parse or validate — such workflows are + /// listed (greyed out in the picker, `error` in the tooltip) but cannot + /// be selected. + pub valid: bool, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub error: Option, + /// Non-fatal issues (unknown keys, clamped values). + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub warnings: Vec, + /// Stage names for the picker tooltip, e.g. ["plan","implement","review×2","fix"]. + /// Empty for invalid files. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub stages: Vec, + /// Review ⇄ fix round limit. 0 for invalid files. + #[serde(default)] + pub max_rounds: u32, +} + +/// Where a workflow definition comes from. +#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)] +#[serde(rename_all = "lowercase")] +pub enum WorkflowSource { + Project, + Builtin, +} + +/// A `workflow.decide` choice after review rounds are exhausted. +#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)] +#[serde(rename_all = "lowercase")] +pub enum WorkflowDecision { + /// Grant extra review ⇄ fix rounds and continue. + Extend, + /// Finish as NeedsReview with the open findings in the summary. + Finish, + /// Stop the pipeline (parent becomes Interrupted). + Stop, +} + +/// Live state of a workflow pipeline, carried on the parent task and updated +/// via `task.updated` events. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +#[serde(rename_all = "camelCase")] +pub struct WorkflowRunInfo { + pub workflow_id: String, + pub workflow_name: String, + pub stage: WorkflowStage, + /// Current review round, 1-based; 0 until the first review starts. + pub round: u32, + /// Effective round limit: the YAML `max_rounds` plus user-granted + /// extensions. + pub max_rounds: u32, + /// Latest merged review verdict. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub verdict: Option, + /// Present while the pipeline waits for the user — the parent composer + /// opens on this, and it drives the attention ("Needs you") rail. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub waiting: Option, + /// A pause has been requested and takes effect when the running stage + /// finishes. Lets the UI show progress instead of an idle Pause button. + #[serde(default, skip_serializing_if = "std::ops::Not::not")] + pub pause_requested: bool, +} + +/// Pipeline position for display. +#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)] +#[serde(rename_all = "snake_case")] +pub enum WorkflowStage { + Plan, + Implement, + Review, + Fix, + Done, + Failed, +} + +#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)] +#[serde(rename_all = "snake_case")] +pub enum WorkflowVerdict { + Approve, + RequestChanges, +} + +/// Why a pipeline is suspended and what input unblocks it. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +#[serde(rename_all = "camelCase")] +pub struct WorkflowWaiting { + pub kind: WorkflowWaitKind, + /// Which stage asked (for `question`). + #[serde(default, skip_serializing_if = "Option::is_none")] + pub stage: Option, + /// The question text (for `question`), or a short findings summary (for + /// `limit`). + #[serde(default, skip_serializing_if = "Option::is_none")] + pub question: Option, +} + +#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)] +#[serde(rename_all = "snake_case")] +pub enum WorkflowWaitKind { + /// A stage asked `need_user_input` — answer with `workflow.reply`. + Question, + /// Review rounds exhausted with open findings — answer with + /// `workflow.decide`. + Limit, + /// Soft-paused — continue with `workflow.resume`. + Paused, +} + /// Orchestrator configuration DTO (wire format). #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] #[serde(rename_all = "camelCase")] @@ -1386,6 +1607,7 @@ mod tests { attachments: vec![], default_model: Some("opus".into()), config_overrides: Default::default(), + workflow: None, }, }; let json = serde_json::to_value(&req).unwrap(); @@ -1476,6 +1698,32 @@ mod tests { ); } + #[test] + fn workflow_event_keeps_agent_links_as_distinct_wire_records() { + let update = SessionUpdate::WorkflowEvent { + event: WorkflowEventKind::StageStarted, + title: "Implement started".into(), + detail: None, + stage: Some(WorkflowStage::Implement), + agents: vec![WorkflowEventAgent { + task_id: "t_impl".into(), + label: "implement".into(), + agent: "codex".into(), + model: Some("gpt-5.6-sol".into()), + }], + tone: WorkflowEventTone::Running, + }; + let value = serde_json::to_value(&update).unwrap(); + assert_eq!(value["kind"], "workflow_event"); + assert_eq!(value["event"], "stage_started"); + assert_eq!(value["stage"], "implement"); + assert_eq!(value["agents"][0]["taskId"], "t_impl"); + assert_eq!( + serde_json::from_value::(value).unwrap(), + update + ); + } + #[test] fn event_wire_shape() { let ev = Event::ServiceLog { diff --git a/desktop/src/components/ChatComposer.test.tsx b/desktop/src/components/ChatComposer.test.tsx new file mode 100644 index 0000000..1425e39 --- /dev/null +++ b/desktop/src/components/ChatComposer.test.tsx @@ -0,0 +1,134 @@ +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import type { TaskInfo, WorkflowRunInfo, WorkflowWaitKind } from "../protocol"; +import { ChatComposer } from "./ChatComposer"; + +const request = vi.fn<(...args: unknown[]) => Promise>(async () => ({})); +const workflowReply = vi.fn<(...args: unknown[]) => Promise>(async () => {}); +const workflowResume = vi.fn<(...args: unknown[]) => Promise>(async () => {}); +const workflowDecide = vi.fn<(...args: unknown[]) => Promise>(async () => {}); + +vi.mock("../daemon", () => ({ + daemon: { + request: (...args: unknown[]) => request(...(args as [])), + workflowDecide: (...args: unknown[]) => workflowDecide(...(args as [])), + workflowReply: (...args: unknown[]) => workflowReply(...(args as [])), + workflowResume: (...args: unknown[]) => workflowResume(...(args as [])), + }, +})); + +function task(workflowRun?: WorkflowRunInfo | null): TaskInfo { + return { + agent: "claude", + blockedReason: null, + createdAt: 1, + filesChanged: 0, + id: "t_1", + project: "warpforge", + prompt: "do it", + status: "running", + tags: [], + title: "", + updatedAt: 1, + workflowRun, + }; +} + +function run(waiting?: WorkflowWaitKind): WorkflowRunInfo { + return { + maxRounds: 2, + round: 1, + stage: waiting === "question" ? "plan" : "review", + waiting: waiting ? { kind: waiting } : null, + workflowId: "wf", + workflowName: "Review loop", + }; +} + +function renderComposer(t: TaskInfo) { + return render( + void>()} + task={t} + />, + ); +} + +async function send(text: string) { + const user = userEvent.setup(); + await user.type(screen.getByRole("textbox"), text); + await user.click(screen.getByRole("button", { name: /send/i })); +} + +describe("ChatComposer — workflow parents", () => { + beforeEach(() => vi.clearAllMocks()); + + it("prompts the agent session for a regular task", async () => { + renderComposer(task(null)); + await send("hello"); + expect(request).toHaveBeenCalledWith( + "session.prompt", + expect.objectContaining({ task_id: "t_1", text: "hello" }), + ); + expect(workflowReply).not.toHaveBeenCalled(); + }); + + it("routes a message to the asking stage when a question is pending", async () => { + renderComposer(task(run("question"))); + await send("Postgres"); + expect(workflowReply).toHaveBeenCalledWith("t_1", "Postgres"); + expect(request).not.toHaveBeenCalledWith("session.prompt", expect.anything()); + }); + + it("resumes a paused pipeline, passing the message as guidance", async () => { + renderComposer(task(run("paused"))); + await send("prefer the simpler fix"); + expect(workflowResume).toHaveBeenCalledWith("t_1", "prefer the simpler fix"); + }); + + it("turns a message at the review limit into one more round with guidance", async () => { + renderComposer(task(run("limit"))); + await send("focus on the parser"); + expect(workflowDecide).toHaveBeenCalledWith("t_1", "extend", { + note: "focus on the parser", + rounds: 1, + }); + }); + + it("disables input while the pipeline runs unattended and points at the stages", () => { + renderComposer(task(run())); + expect(screen.getByRole("textbox")).toBeDisabled(); + expect(screen.getByRole("textbox")).toHaveAttribute( + "placeholder", + expect.stringContaining("open a stage above"), + ); + }); + + it("leaves stopping a pipeline to WorkflowControls, not the composer", () => { + // Two destructive controls a few pixels apart is a trap; the labelled Stop + // in WorkflowControls owns this for pipelines. + renderComposer(task(run())); + expect(screen.queryByRole("button", { name: /^stop$/i })).not.toBeInTheDocument(); + }); + + it("keeps input disabled after the pipeline finishes, with an explanation", () => { + // The parent never has an agent session, so a message here could only fail + // with a raw daemon error. + renderComposer(task({ ...run(), stage: "done", waiting: null })); + const input = screen.getByRole("textbox"); + expect(input).toBeDisabled(); + expect(input).toHaveAttribute("placeholder", expect.stringContaining("has finished")); + }); + + it("never prompts the session-less parent, even with no barrier pending", async () => { + renderComposer(task({ ...run(), stage: "done", waiting: null })); + // Disabled input cannot submit, but the routing must refuse regardless. + expect(request).not.toHaveBeenCalledWith("session.prompt", expect.anything()); + }); +}); diff --git a/desktop/src/components/ChatComposer.tsx b/desktop/src/components/ChatComposer.tsx index 2c10122..3a73bae 100644 --- a/desktop/src/components/ChatComposer.tsx +++ b/desktop/src/components/ChatComposer.tsx @@ -1,6 +1,7 @@ import { forwardRef, memo, useCallback } from "react"; import { daemon } from "../daemon"; +import { useWorkflowSend } from "../hooks/useWorkflowSend"; import type { ContextUsage } from "../lib/sessionUsage"; import type { CommandInfo, ProjectFile, PromptSubmission, TaskInfo } from "../protocol"; import { AgentConfigBar } from "./AgentConfigBar"; @@ -22,18 +23,23 @@ export const ChatComposer = memo( { commands, contextUsage, files, filesLoading, imageSupported, onBeforeSend, task }, ref, ) { + const workflow = useWorkflowSend(task); + const onSend = useCallback( async (submission: PromptSubmission) => { onBeforeSend(); + if (await workflow.send(submission)) return; await daemon.request("session.prompt", { task_id: task.id, ...submission }); }, - [onBeforeSend, task.id], + [onBeforeSend, task.id, workflow], ); - const onCancel = useCallback( - () => void daemon.request("task.cancel", { task_id: task.id }), - [task.id], - ); + // For a workflow parent the daemon maps task.cancel onto stopping the + // whole pipeline, so the composer's stop button stays the terminal + // "kill it" affordance next to WorkflowControls' soft pause. + const onCancel = useCallback(async () => { + await daemon.request("task.cancel", { task_id: task.id }); + }, [task.id]); const isRunning = task.status === "running" || task.status === "queued"; @@ -45,9 +51,10 @@ export const ChatComposer = memo( files={files} filesLoading={filesLoading} imageSupported={imageSupported} - disabled={task.status === "done"} + disabled={task.status === "done" || workflow.disabled} onSend={onSend} - onCancel={isRunning ? onCancel : undefined} + onCancel={isRunning && !workflow.isWorkflow ? onCancel : undefined} + placeholder={workflow.placeholder} toolbar={ task.configOptions && task.configOptions.length > 0 ? ( diff --git a/desktop/src/components/ChatTranscript.tsx b/desktop/src/components/ChatTranscript.tsx index ab3d766..113210f 100644 --- a/desktop/src/components/ChatTranscript.tsx +++ b/desktop/src/components/ChatTranscript.tsx @@ -42,6 +42,7 @@ import { AgentActivityIndicator } from "./AgentActivityIndicator"; import { ChatComposer } from "./ChatComposer"; import type { ComposerHandle } from "./Composer"; import { MessageActions } from "./MessageActions"; +import { WorkflowControls } from "./WorkflowControls"; const CHAT_DRAW_DISTANCE_PX = 250; const CHAT_MAINTAIN_SCROLL_AT_END = { @@ -181,6 +182,7 @@ const TranscriptRow = memo(function TranscriptRow({ resolveFilePath={resolveFilePath} onOpenFile={onOpenFile} onOpenFileDiff={onOpenFileDiff} + onOpenTask={onOpenTask} project={project} /> {messageText && ( @@ -486,6 +488,7 @@ export function ChatTranscript({ )} + {task.workflowRun && } void | Promise; - onCancel?: () => void; + onCancel?: () => void | Promise; commands?: CommandInfo[]; files?: ProjectFile[]; filesLoading?: boolean; @@ -102,6 +102,7 @@ export const Composer = forwardRef< const [diffs, setDiffs] = useState([]); const [images, setImages] = useState([]); const [sending, setSending] = useState(false); + const [stopping, setStopping] = useState(false); const [error, setError] = useState(null); const [dragging, setDragging] = useState(false); const textRef = useRef(null); @@ -234,6 +235,19 @@ export const Composer = forwardRef< } } + async function stop() { + if (!onCancel || stopping) return; + setStopping(true); + setError(null); + try { + await onCancel(); + } catch (cause) { + setError(cause instanceof Error ? cause.message : "Task could not be stopped."); + } finally { + setStopping(false); + } + } + useLayoutEffect(() => { sendActionRef.current = () => void send(); }); @@ -405,9 +419,10 @@ export const Composer = forwardRef< {!hideSendButton && ( void stop()} + stopping={stopping} /> )} @@ -422,25 +437,33 @@ const ComposerActionButton = memo(function ComposerActionButton({ disabled, sendActionRef, onStop, + stopping, }: { action: "send" | "stop"; disabled: boolean; sendActionRef: React.RefObject<() => void>; onStop?: () => void; + stopping: boolean; }) { - const stopping = action === "stop"; + const stopAction = action === "stop"; return ( ); }); diff --git a/desktop/src/components/SessionRailCard.tsx b/desktop/src/components/SessionRailCard.tsx index 428b0c0..5db8509 100644 --- a/desktop/src/components/SessionRailCard.tsx +++ b/desktop/src/components/SessionRailCard.tsx @@ -260,7 +260,12 @@ const SessionRailCard = memo(function SessionRailCard({

)} {reason && ( -

+ // `reason` can now be free-form agent text (a pipeline's question), so + // it needs a tooltip when truncated. +

{reason}

)} diff --git a/desktop/src/components/TaskAgentSwitcher.test.tsx b/desktop/src/components/TaskAgentSwitcher.test.tsx index 4fa2898..ff62495 100644 --- a/desktop/src/components/TaskAgentSwitcher.test.tsx +++ b/desktop/src/components/TaskAgentSwitcher.test.tsx @@ -60,4 +60,48 @@ describe("TaskAgentSwitcher", () => { expect(onOpenTask).not.toHaveBeenCalled(); }); + + it("labels a workflow root and its stage sessions explicitly", async () => { + const user = userEvent.setup(); + const root = { + ...task("root", "codex", "running"), + orchestrationGraph: { + goal: "Implement + review loop", + id: "root", + nodes: [ + { + agent: "codex", + id: "implement", + kind: "implement" as const, + status: "running" as const, + taskId: "child", + }, + ], + }, + workflowRun: { + maxRounds: 2, + round: 0, + stage: "implement" as const, + workflowId: "review-loop", + workflowName: "Implement + review loop", + }, + }; + const [tree] = buildTaskForest([root, task("child", "codex", "running", "root")]); + + render( + void>()} + />, + ); + + expect(screen.getByRole("button", { name: /current: workflow/i })).toHaveTextContent( + "Stages 1", + ); + await user.click(screen.getByRole("button", { name: /current: workflow/i })); + expect( + await screen.findByRole("menuitem", { name: /implement · codex: running/i }), + ).toBeInTheDocument(); + }); }); diff --git a/desktop/src/components/TaskAgentSwitcher.tsx b/desktop/src/components/TaskAgentSwitcher.tsx index ef5c126..a09712d 100644 --- a/desktop/src/components/TaskAgentSwitcher.tsx +++ b/desktop/src/components/TaskAgentSwitcher.tsx @@ -28,7 +28,25 @@ export const TaskAgentSwitcher = memo(function TaskAgentSwitcher({ const members = useMemo(() => flattenTaskTree(tree), [tree]); const currentIndex = members.findIndex((member) => member.id === currentTaskId); const current = members[currentIndex] ?? tree.task; - const currentLabel = currentIndex === 0 ? "Lead" : agentDisplayName(current.agent); + const workflow = Boolean(tree.task.workflowRun); + const stageByTaskId = useMemo(() => { + const result = new Map(); + for (const node of tree.task.orchestrationGraph?.nodes ?? []) { + if (node.taskId) result.set(node.taskId, node.id); + } + return result; + }, [tree.task.orchestrationGraph?.nodes]); + const memberLabel = useCallback( + (member: (typeof members)[number], index: number) => { + if (index === 0) return workflow ? "Workflow" : "Lead"; + const stage = stageByTaskId.get(member.id); + return stage + ? `${stage} · ${agentDisplayName(member.agent)}` + : agentDisplayName(member.agent); + }, + [stageByTaskId, workflow], + ); + const currentLabel = memberLabel(current, currentIndex); const handleSelect = useCallback( (id: string) => { if (id !== currentTaskId) onOpenTask(id); @@ -48,12 +66,16 @@ export const TaskAgentSwitcher = memo(function TaskAgentSwitcher({ className="flex h-7 shrink-0 items-center gap-1.5 rounded px-2 text-xs text-muted-foreground hover:bg-secondary hover:text-foreground" > - Agents {members.length - 1} + + {workflow ? "Stages" : "Agents"} {members.length - 1} + · {currentIndex === 0 ? ( - Lead + + {workflow ? "Workflow" : "Lead"} + ) : ( - + {currentLabel} )} @@ -61,7 +83,8 @@ export const TaskAgentSwitcher = memo(function TaskAgentSwitcher({ {members.map((member, index) => { const selected = member.id === currentTaskId; - const label = index === 0 ? "Lead" : agentDisplayName(member.agent); + const label = memberLabel(member, index); + const stage = stageByTaskId.get(member.id); return ( {index === 0 ? ( - Lead + + {workflow ? "Workflow" : "Lead"} + ) : ( - + <> + {stage && {stage}} + + )} diff --git a/desktop/src/components/TaskComposeBar.test.tsx b/desktop/src/components/TaskComposeBar.test.tsx new file mode 100644 index 0000000..8c57ae4 --- /dev/null +++ b/desktop/src/components/TaskComposeBar.test.tsx @@ -0,0 +1,124 @@ +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import type { WorkflowMeta } from "../protocol"; +import { TaskComposeBar } from "./TaskComposeBar"; + +const onWorkflowChange = vi.fn<(v: string | null) => void>(); +const onEjectWorkflow = vi.fn<(id: string) => void>(); +const onOrchChatChange = vi.fn<(v: boolean) => void>(); + +const workflows: WorkflowMeta[] = [ + { + description: "Implement, then loop reviews and fixes.", + id: "review-loop", + maxRounds: 2, + name: "Review loop", + source: "builtin", + stages: ["implement", "review×2", "fix"], + valid: true, + }, + { + error: "`name` is required", + id: "broken", + name: "broken", + source: "project", + valid: false, + }, +]; + +function renderBar(props: Partial[0]> = {}) { + return render( + void>()} + onEjectWorkflow={onEjectWorkflow} + onOrchChatChange={onOrchChatChange} + onProjectChange={vi.fn<(v: string) => void>()} + onShareContextChange={vi.fn<(v: boolean) => void>()} + onUseWorktreeChange={vi.fn<(v: boolean) => void>()} + onWorkflowChange={onWorkflowChange} + orchChat={false} + project="warpforge" + projects={[ + { + agentTemplates: {}, + declaredServices: [], + name: "warpforge", + path: "/tmp/warpforge", + portRange: [4000, 4099], + }, + ]} + services={[]} + shareContext + useWorktree={false} + workflow={null} + workflows={workflows} + {...props} + />, + ); +} + +describe("TaskComposeBar — workflow picker", () => { + beforeEach(() => vi.clearAllMocks()); + + it("hides the picker when a project has no workflows", () => { + renderBar({ workflows: [] }); + expect(screen.queryByRole("button", { name: /workflow/i })).not.toBeInTheDocument(); + }); + + it("selects a workflow from the menu", async () => { + const user = userEvent.setup(); + renderBar(); + await user.click(screen.getByRole("button", { name: /workflow/i })); + await user.click(screen.getByRole("menuitem", { name: /implement → review×2 → fix/ })); + expect(onWorkflowChange).toHaveBeenCalledWith("review-loop"); + }); + + it("lists an invalid workflow with its error but does not select it", async () => { + const user = userEvent.setup(); + renderBar(); + await user.click(screen.getByRole("button", { name: /workflow/i })); + const broken = screen.getByRole("menuitem", { name: /`name` is required/ }); + expect(broken).toHaveAttribute("aria-disabled", "true"); + await user.click(broken); + expect(onWorkflowChange).not.toHaveBeenCalled(); + }); + + it("copies a built-in into the project without selecting it", async () => { + const user = userEvent.setup(); + renderBar(); + await user.click(screen.getByRole("button", { name: /workflow/i })); + await user.click(screen.getByRole("menuitem", { name: /copy review loop to this project/i })); + expect(onEjectWorkflow).toHaveBeenCalledWith("review-loop"); + expect(onWorkflowChange).not.toHaveBeenCalled(); + }); + + it("summarizes the selected workflow and labels the agent as the per-stage default", () => { + renderBar({ workflow: "review-loop" }); + expect(screen.getByText(/Stages: implement → review×2 → fix/)).toBeInTheDocument(); + expect(screen.getByText(/up to 2 review rounds/)).toBeInTheDocument(); + // Nothing is "led": the parent has no session, this is just the fallback. + expect(screen.getByText("Default agent")).toBeInTheDocument(); + }); + + it("locks the orchestrator toggle while a workflow is selected", () => { + renderBar({ workflow: "review-loop" }); + expect(screen.getByRole("button", { name: /orchestrator/i })).toBeDisabled(); + }); + + it("locks the workflow picker while the orchestrator is on", () => { + renderBar({ orchChat: true }); + expect(screen.getByRole("button", { name: /workflow/i })).toBeDisabled(); + }); +}); diff --git a/desktop/src/components/TaskComposeBar.tsx b/desktop/src/components/TaskComposeBar.tsx index 66d0d29..a23b064 100644 --- a/desktop/src/components/TaskComposeBar.tsx +++ b/desktop/src/components/TaskComposeBar.tsx @@ -1,8 +1,26 @@ -import { GitBranch, GitMerge, Share2 } from "lucide-react"; +import { + AlertTriangle, + Check, + ChevronDown, + Copy, + GitBranch, + GitMerge, + Route, + Share2, +} from "lucide-react"; +import { Fragment } from "react"; +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuLabel, + DropdownMenuSeparator, + DropdownMenuTrigger, +} from "@/components/ui/dropdown-menu"; import { cn } from "@/lib/utils"; -import type { AgentConfig, ProjectInfo, ServiceInfo } from "../protocol"; +import type { AgentConfig, ProjectInfo, ServiceInfo, WorkflowMeta } from "../protocol"; import { AgentBadge } from "./AgentBadge"; interface TaskComposeBarProps { @@ -15,12 +33,18 @@ interface TaskComposeBarProps { shareContext: boolean; useWorktree: boolean; orchChat: boolean; + /** Available workflow templates for the selected project. */ + workflows: WorkflowMeta[]; + /** Selected workflow id, or null for a plain single-agent task. */ + workflow: string | null; onProjectChange: (v: string) => void; onAgentChange: (v: string) => void; onShareContextChange: (v: boolean) => void; onUseWorktreeChange: (v: boolean) => void; onOrchChatChange: (v: boolean) => void; + onWorkflowChange: (v: string | null) => void; + onEjectWorkflow: (id: string) => void; } /** @@ -42,11 +66,15 @@ export function TaskComposeBar({ shareContext, useWorktree, orchChat, + workflows, + workflow, onProjectChange, onAgentChange, onShareContextChange, onUseWorktreeChange, onOrchChatChange, + onWorkflowChange, + onEjectWorkflow, }: TaskComposeBarProps) { const enabledAgents = agents.filter((a) => a.enabled); const agentChoices = @@ -54,6 +82,7 @@ export function TaskComposeBar({ const runningForProject = services.filter( (s) => s.project === project && s.status === "running" && s.allocatedPort > 0, ); + const selectedWorkflow = workflows.find((w) => w.id === workflow) ?? null; return (
@@ -71,7 +100,7 @@ export function TaskComposeBar({
- {orchChat ? "Lead agent" : "Agent"} + {orchChat ? "Lead agent" : workflow ? "Default agent" : "Agent"} {agentChoices.map((a) => ( onAgentChange(a.id)}> @@ -101,12 +130,34 @@ export function TaskComposeBar({ /> onOrchChatChange(!orchChat)} icon={} label="Orchestrator" - tooltip="Chat + sub-agents" + tooltip={workflow ? "Not available with a workflow selected" : "Chat + sub-agents"} + /> +
+ {selectedWorkflow && ( +

+ {selectedWorkflow.description ? `${selectedWorkflow.description} ` : ""} + {(selectedWorkflow.stages ?? []).length > 0 && + `Stages: ${(selectedWorkflow.stages ?? []).join(" \u2192 ")}${ + selectedWorkflow.maxRounds + ? ` (up to ${selectedWorkflow.maxRounds} review round${ + selectedWorkflow.maxRounds === 1 ? "" : "s" + })` + : "" + }. `} + {"Stages that don\u2019t name their own agent use the one selected above."} +

+ )} ); } @@ -180,3 +231,116 @@ function PillToggle({ ); } + +/** + * Workflow template picker. A workflow replaces the single-agent run with a + * daemon-driven pipeline, so it is mutually exclusive with the orchestrator + * chat. Invalid templates stay listed (with their parse error) so a typo in a + * project's YAML is visible here rather than silently missing. + */ +function WorkflowPicker({ + workflows, + selected, + disabled, + onSelect, + onEject, +}: { + workflows: WorkflowMeta[]; + selected: WorkflowMeta | null; + disabled?: boolean; + onSelect: (id: string | null) => void; + onEject: (id: string) => void; +}) { + if (workflows.length === 0) return null; + return ( + + + + + + onSelect(null)}> + + + + No workflow + + One agent works the task directly. + + + + + + + Pipelines + + {workflows.map((w) => ( + + onSelect(w.id)} + > + + + + + {w.name} + {w.source === "builtin" && ( + + built-in + + )} + {!w.valid && } + + + {w.valid ? (w.stages ?? []).join(" \u2192 ") : (w.error ?? "invalid workflow")} + + + + + {/* Ejecting is its own row: a nested button inside a menu item + never receives the click, because Radix closes the menu first. */} + {w.source === "builtin" && w.valid && ( + onEject(w.id)} + aria-label={`Copy ${w.name} to this project`} + > + + + Copy to project + + + )} + + ))} + + + ); +} diff --git a/desktop/src/components/WorkflowControls.test.tsx b/desktop/src/components/WorkflowControls.test.tsx new file mode 100644 index 0000000..da873ec --- /dev/null +++ b/desktop/src/components/WorkflowControls.test.tsx @@ -0,0 +1,166 @@ +import { render, screen, waitFor } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import type { TaskInfo, WorkflowRunInfo } from "../protocol"; +import { WorkflowControls } from "./WorkflowControls"; + +const workflowPause = vi.fn<(...args: unknown[]) => Promise>(async () => {}); +const workflowResume = vi.fn<(...args: unknown[]) => Promise>(async () => {}); +const workflowDecide = vi.fn<(...args: unknown[]) => Promise>(async () => {}); +const request = vi.fn<(...args: unknown[]) => Promise>(async () => ({})); + +vi.mock("../daemon", () => ({ + daemon: { + request: (...args: unknown[]) => request(...(args as [])), + workflowDecide: (...args: unknown[]) => workflowDecide(...(args as [])), + workflowPause: (...args: unknown[]) => workflowPause(...(args as [])), + workflowResume: (...args: unknown[]) => workflowResume(...(args as [])), + }, +})); + +function task(run: Partial): TaskInfo { + return { + agent: "claude", + blockedReason: null, + createdAt: 1, + filesChanged: 0, + id: "t_1", + project: "warpforge", + prompt: "do it", + status: "running", + tags: [], + title: "", + updatedAt: 1, + workflowRun: { + maxRounds: 2, + round: 1, + stage: "review", + workflowId: "wf", + workflowName: "Review loop", + ...run, + }, + }; +} + +describe("WorkflowControls", () => { + beforeEach(() => { + vi.clearAllMocks(); + workflowDecide.mockImplementation(async () => {}); + }); + + it("shows the pipeline position and pauses a running stage", async () => { + render(); + expect(screen.getByText("Review loop")).toBeInTheDocument(); + expect(screen.getByText("reviewing")).toBeInTheDocument(); + expect(screen.getByText("round 1/2")).toBeInTheDocument(); + + await userEvent.click(screen.getByRole("button", { name: /pause/i })); + expect(workflowPause).toHaveBeenCalledWith("t_1"); + }); + + it("offers Resume instead of Pause when paused", async () => { + render(); + expect(screen.queryByRole("button", { name: /pause/i })).not.toBeInTheDocument(); + + await userEvent.click(screen.getByRole("button", { name: /resume/i })); + expect(workflowResume).toHaveBeenCalledWith("t_1"); + }); + + it("offers extend / finish / stop when the review limit is reached", async () => { + render( + , + ); + expect(screen.getByText(/open findings: 2 high/)).toBeInTheDocument(); + expect(screen.getByText("Review limit reached")).toBeInTheDocument(); + expect(screen.getByText(/type it below and press Send/i)).toBeInTheDocument(); + // Pausing makes no sense while a decision is pending. + expect(screen.queryByRole("button", { name: /pause/i })).not.toBeInTheDocument(); + + const oneRound = screen.getByRole("button", { name: /1 more round/i }); + const twoRounds = screen.getByRole("button", { name: /2 more rounds/i }); + const finish = screen.getByRole("button", { name: /finish for review/i }); + const stop = screen.getByRole("button", { name: /^stop$/i }); + expect(oneRound).toHaveClass("bg-primary"); + expect(twoRounds).toHaveClass("bg-primary"); + expect(finish).toHaveClass("bg-primary"); + expect(stop).toHaveClass("text-destructive", "bg-destructive/15"); + + await userEvent.click(oneRound); + expect(workflowDecide).toHaveBeenCalledWith("t_1", "extend", { rounds: 1 }); + + await userEvent.click(twoRounds); + expect(workflowDecide).toHaveBeenCalledWith("t_1", "extend", { rounds: 2 }); + + await userEvent.click(finish); + expect(workflowDecide).toHaveBeenCalledWith("t_1", "finish"); + + await userEvent.click(stop); + expect(workflowDecide).toHaveBeenCalledWith("t_1", "stop"); + }); + + it("shows which limit action is in progress and locks competing decisions", async () => { + let finishRequest: (() => void) | undefined; + workflowDecide.mockImplementationOnce( + () => + new Promise((resolve) => { + finishRequest = resolve; + }), + ); + render( + , + ); + + await userEvent.click(screen.getByRole("button", { name: /finish for review/i })); + + expect(screen.getByRole("button", { name: /finishing/i })).toBeDisabled(); + expect(screen.getAllByRole("button").every((button) => button.hasAttribute("disabled"))).toBe( + true, + ); + + finishRequest?.(); + await waitFor(() => + expect(screen.getByRole("button", { name: /finish for review/i })).toBeEnabled(), + ); + }); + + it("hides Pause while a stage question is pending — the daemon rejects it", async () => { + render( + , + ); + expect(screen.queryByRole("button", { name: /pause/i })).not.toBeInTheDocument(); + expect(workflowPause).not.toHaveBeenCalled(); + }); + + it("shows a queued pause as progress instead of an idle button", () => { + render(); + const button = screen.getByRole("button", { name: /pausing/i }); + expect(button).toBeDisabled(); + }); + + it("drops the controls once the pipeline is finished", () => { + render(); + expect(screen.queryByRole("button", { name: /pause/i })).not.toBeInTheDocument(); + expect(screen.getByText("approved")).toBeInTheDocument(); + }); + + it("renders nothing for a task without a pipeline", () => { + const plain = { ...task({}), workflowRun: null }; + const { container } = render(); + expect(container).toBeEmptyDOMElement(); + }); + + it("hard-stops the parent workflow", async () => { + render(); + const stop = screen.getByRole("button", { name: /^stop$/i }); + expect(stop).toHaveClass("text-destructive", "bg-destructive/15"); + await userEvent.click(stop); + expect(request).toHaveBeenCalledWith("task.cancel", { task_id: "t_1" }); + }); +}); diff --git a/desktop/src/components/WorkflowControls.tsx b/desktop/src/components/WorkflowControls.tsx new file mode 100644 index 0000000..a395ee1 --- /dev/null +++ b/desktop/src/components/WorkflowControls.tsx @@ -0,0 +1,262 @@ +import { AlertTriangle, CircleCheckBig, Loader2, Pause, Play, Plus, Square } from "lucide-react"; +import { memo, useState } from "react"; +import { toast } from "sonner"; + +import { Button } from "@/components/ui/button"; +import { cn } from "@/lib/utils"; +import { workflowStageLabel } from "@/lib/workflow"; + +import { daemon } from "../daemon"; +import type { TaskInfo, WorkflowRunInfo } from "../protocol"; + +/** + * Pipeline status strip + controls for a workflow parent task. + * + * A workflow parent has no agent session of its own — the daemon drives its + * stages — so this bar (not the composer) is where the pipeline is steered. + * The composer takes over only when a stage asks a question; see + * `ChatComposer`. + */ +export const WorkflowControls = memo(function WorkflowControls({ task }: { task: TaskInfo }) { + const run = task.workflowRun; + const [busyAction, setBusyAction] = useState(null); + if (!run) return null; + + const waiting = run.waiting ?? null; + const finished = run.stage === "done" || run.stage === "failed"; + const busy = busyAction !== null; + + const act = async (label: string, fn: () => Promise) => { + setBusyAction(label); + try { + await fn(); + } catch (e) { + toast.error(`Could not ${label}`, { + description: e instanceof Error ? e.message : String(e), + }); + } finally { + setBusyAction(null); + } + }; + + return ( +
+
+ + + {!finished && (waiting === null || waiting.kind === "paused") && ( + <> + {waiting?.kind === "paused" ? ( + + ) : ( + + )} + + )} + {!finished && waiting?.kind !== "limit" && ( + + )} + +
+ + {waiting?.kind === "limit" && ( + + )} + + {waiting?.kind === "paused" && ( +

+ Paused before the {workflowStageLabel(run.stage)} stage. Type a message to resume with it + as guidance, or press Resume. +

+ )} +
+ ); +}); + +/** Buttons shown when review rounds ran out with findings still open. */ +function LimitDecision({ + task, + summary, + busyAction, + act, +}: { + task: TaskInfo; + summary: string; + busyAction: string | null; + act: (label: string, fn: () => Promise) => Promise; +}) { + const busy = busyAction !== null; + return ( +
+
+ +
+

Review limit reached

+

+ Reviewers still request changes{summary ? ` — ${summary}` : ""}. +

+

+ Continue the fix → review loop, finish with the current changes, or stop the workflow. +

+
+
+ +
+ + + + +
+ +

+ These buttons continue without guidance. To add guidance, type it below and press Send + instead — that runs one more round with your note. +

+
+ ); +} + +function StageIndicator({ run }: { run: WorkflowRunInfo }) { + const waiting = run.waiting ?? null; + return ( + + + {run.workflowName} + + · + + {waiting?.kind === "paused" + ? `paused before ${workflowStageLabel(run.stage)}` + : workflowStageLabel(run.stage)} + + {run.round > 0 && run.stage !== "done" && run.stage !== "failed" && ( + + round {run.round}/{run.maxRounds} + + )} + {run.verdict && ( + + {run.verdict === "approve" ? "approved" : "changes requested"} + + )} + + ); +} diff --git a/desktop/src/components/WorkflowEventLine.tsx b/desktop/src/components/WorkflowEventLine.tsx new file mode 100644 index 0000000..d36991b --- /dev/null +++ b/desktop/src/components/WorkflowEventLine.tsx @@ -0,0 +1,118 @@ +import { + CheckCircle2, + ChevronRight, + CircleDotDashed, + Workflow as WorkflowIcon, + XCircle, +} from "lucide-react"; + +import { cn } from "@/lib/utils"; +import type { SessionUpdate } from "@/protocol"; + +import { AgentBadge } from "./AgentBadge"; +import { CollapsibleMarkdown, Markdown } from "./Markdown"; + +type WorkflowEvent = Extract; + +export function WorkflowEventLine({ + compact, + onOpenTask, + update, +}: { + compact?: boolean; + onOpenTask?: (id: string) => void; + update: WorkflowEvent; +}) { + const Icon = + update.tone === "running" + ? CircleDotDashed + : update.tone === "success" + ? CheckCircle2 + : update.tone === "error" + ? XCircle + : WorkflowIcon; + const tone = { + error: "border-destructive/35 bg-destructive/[0.06] text-destructive", + info: "border-border bg-secondary/20 text-muted-foreground", + running: "border-primary/30 bg-primary/[0.06] text-primary", + success: "border-ok/30 bg-ok/[0.05] text-ok", + warning: "border-warn/35 bg-warn/[0.06] text-warn", + }[update.tone]; + const showAgentCards = update.event === "stage_started"; + + return ( +
+
+ + + {update.title} + + {!showAgentCards && + update.agents.map((agent) => ( + + ))} +
+ + {showAgentCards && update.agents.length > 0 && ( +
+ {update.agents.map((agent) => ( + + ))} +
+ )} + + {update.detail && + (compact ? ( + {update.detail} + ) : ( +
+ {update.detail} +
+ ))} +
+ ); +} diff --git a/desktop/src/daemon.ts b/desktop/src/daemon.ts index fa16247..098b87c 100644 --- a/desktop/src/daemon.ts +++ b/desktop/src/daemon.ts @@ -1061,13 +1061,53 @@ export class DaemonClient { return (result as { ok?: boolean })?.ok ?? false; } + // ── Workflow RPCs ── + + /** Workflows selectable for a project: its own files plus built-ins. */ + async workflowList(project: string): Promise { + const result = await this.request("workflow.list", { project }); + const workflows = (result as { workflows?: import("./protocol").WorkflowMeta[] })?.workflows; + return Array.isArray(workflows) ? workflows : []; + } + + /** Copy a built-in workflow into the project so it can be customized. */ + async workflowEject(project: string, id: string): Promise { + const result = await this.request("workflow.eject", { id, project }); + return (result as { path?: string })?.path ?? ""; + } + + /** Soft-pause a pipeline: the running stage finishes, the next won't start. */ + async workflowPause(task: string): Promise { + await this.request("workflow.pause", { task }); + } + + /** Resume a paused pipeline; `note` reaches the next stage as guidance. */ + async workflowResume(task: string, note?: string): Promise { + await this.request("workflow.resume", { note, task }); + } + + /** Answer a stage's pending question. */ + async workflowReply(task: string, message: string): Promise { + await this.request("workflow.reply", { message, task }); + } + + /** Decide what an out-of-rounds pipeline does next. */ + async workflowDecide( + task: string, + decision: import("./protocol").WorkflowDecision, + opts?: { rounds?: number; note?: string }, + ): Promise { + await this.request("workflow.decide", { + decision, + note: opts?.note, + rounds: opts?.rounds, + task, + }); + } + // ── Terminal PTY RPCs ── - async spawnTerminal( - project: string, - cols: number, - rows: number, - ): Promise { + async spawnTerminal(project: string, cols: number, rows: number): Promise { const result = await this.request("terminal.spawn", { project, command: 'exec "${SHELL:-/bin/sh}" -l', diff --git a/desktop/src/hooks/useDaemonEvents.tsx b/desktop/src/hooks/useDaemonEvents.tsx index b7338d2..7f97427 100644 --- a/desktop/src/hooks/useDaemonEvents.tsx +++ b/desktop/src/hooks/useDaemonEvents.tsx @@ -18,6 +18,16 @@ function attentionToastTitle(status: TaskStatus): string { return "Session interrupted"; } +/** The barrier a pipeline is parked at, or null when it needs nothing. */ +function waitingKind(task: TaskInfo): "question" | "limit" | null { + const kind = task.workflowRun?.waiting?.kind; + return kind === "question" || kind === "limit" ? kind : null; +} + +function workflowToastTitle(kind: "question" | "limit"): string { + return kind === "question" ? "Pipeline needs your answer" : "Pipeline is out of review rounds"; +} + export function useDaemonEvents() { const seenPermissionIds = useRef(new Set()); const notificationsReady = useRef(false); @@ -39,6 +49,34 @@ export function useDaemonEvents() { ui.openTask(taskId); ui.setAttentionOpen(false); }; + const notifyWorkflowWaiting = (task: TaskInfo, kind: "question" | "limit") => { + const toastId = `attention:workflow:${task.id}:${kind}`; + const question = task.workflowRun?.waiting?.question; + toast.custom( + (sonnerId) => ( + toast.dismiss(sonnerId)} + onOpen={() => { + openInChat(task.id); + toast.dismiss(sonnerId); + }} + /> + ), + { + action: null, + cancel: null, + description: null, + duration: 10_000, + icon: null, + id: toastId, + richColors: false, + unstyled: true, + }, + ); + }; const notifyTask = (task: TaskInfo) => { const toastId = `attention:${task.id}:${task.status}`; toast.custom( @@ -138,9 +176,19 @@ export function useDaemonEvents() { } if (event.event === "task.updated") { - const previous = daemon + const previousTask = daemon .getState() - .snapshot.tasks.find((task) => task.id === event.data.id)?.status; + .snapshot.tasks.find((task) => task.id === event.data.id); + const previous = previousTask?.status; + // A pipeline parking on the user is an attention state the coarse task + // status cannot express (it stays Idle), so it needs its own toast. + const wasWaiting = previousTask ? waitingKind(previousTask) : null; + const nowWaiting = waitingKind(event.data); + if (nowWaiting && nowWaiting !== wasWaiting) { + notifyWorkflowWaiting(event.data, nowWaiting); + } else if (!nowWaiting && wasWaiting) { + toast.dismiss(`attention:workflow:${event.data.id}:${wasWaiting}`); + } if (ATTENTION_STATUS.has(event.data.status) && previous !== event.data.status) { if (previous && ATTENTION_STATUS.has(previous)) { toast.dismiss(`attention:${event.data.id}:${previous}`); diff --git a/desktop/src/hooks/useWorkflowSend.test.ts b/desktop/src/hooks/useWorkflowSend.test.ts new file mode 100644 index 0000000..b42839f --- /dev/null +++ b/desktop/src/hooks/useWorkflowSend.test.ts @@ -0,0 +1,100 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import type { PromptSubmission, TaskInfo, WorkflowRunInfo } from "../protocol"; +import { useWorkflowSend } from "./useWorkflowSend"; + +const workflowReply = vi.fn<(...args: unknown[]) => Promise>(async () => {}); +const workflowResume = vi.fn<(...args: unknown[]) => Promise>(async () => {}); +const workflowDecide = vi.fn<(...args: unknown[]) => Promise>(async () => {}); + +vi.mock("../daemon", () => ({ + daemon: { + workflowDecide: (...args: unknown[]) => workflowDecide(...(args as [])), + workflowReply: (...args: unknown[]) => workflowReply(...(args as [])), + workflowResume: (...args: unknown[]) => workflowResume(...(args as [])), + }, +})); + +// The hook only reads props and returns callbacks, so calling it outside a +// renderer is enough to pin the routing table. +vi.mock("react", async () => { + const actual = await vi.importActual("react"); + return { ...actual, useCallback: (fn: unknown) => fn }; +}); + +function task(run: Partial | null): TaskInfo { + return { + agent: "claude", + blockedReason: null, + createdAt: 1, + filesChanged: 0, + id: "t_1", + project: "warpforge", + prompt: "do it", + status: "running", + tags: [], + title: "", + updatedAt: 1, + workflowRun: run + ? { + maxRounds: 2, + round: 1, + stage: "review", + workflowId: "wf", + workflowName: "Loop", + ...run, + } + : null, + }; +} + +const submission = (text: string): PromptSubmission => ({ attachments: [], text }); + +describe("useWorkflowSend", () => { + beforeEach(() => vi.clearAllMocks()); + + it("declines to handle a plain task so the caller prompts its session", async () => { + const send = useWorkflowSend(task(null)); + expect(send.isWorkflow).toBe(false); + expect(send.disabled).toBe(false); + expect(await send.send(submission("hello"))).toBe(false); + }); + + it("routes each barrier to its own RPC", async () => { + expect( + await useWorkflowSend(task({ waiting: { kind: "question" } })).send(submission("Postgres")), + ).toBe(true); + expect(workflowReply).toHaveBeenCalledWith("t_1", "Postgres"); + + await useWorkflowSend(task({ waiting: { kind: "paused" } })).send(submission("carry on")); + expect(workflowResume).toHaveBeenCalledWith("t_1", "carry on"); + + await useWorkflowSend(task({ waiting: { kind: "limit" } })).send(submission("focus here")); + expect(workflowDecide).toHaveBeenCalledWith("t_1", "extend", { + note: "focus here", + rounds: 1, + }); + }); + + it("swallows messages for a session-less parent rather than failing an RPC", async () => { + // A running or finished pipeline has no addressee; prompting the parent + // would surface a raw "no live or resumable agent session" error. + const handled = await Promise.all( + (["review", "done", "failed"] as const).map((stage) => + useWorkflowSend(task({ stage, waiting: null })).send(submission("hi")), + ), + ); + expect(handled).toEqual([true, true, true]); + expect(workflowReply).not.toHaveBeenCalled(); + expect(workflowResume).not.toHaveBeenCalled(); + expect(workflowDecide).not.toHaveBeenCalled(); + }); + + it("explains why the box is disabled, differently for running vs finished", () => { + expect(useWorkflowSend(task({ stage: "review" })).placeholder).toMatch(/open a stage above/); + expect(useWorkflowSend(task({ stage: "done" })).placeholder).toMatch(/has finished/); + expect(useWorkflowSend(task({ waiting: { kind: "question" } })).placeholder).toMatch( + /Answer the stage/, + ); + }); +}); diff --git a/desktop/src/hooks/useWorkflowSend.ts b/desktop/src/hooks/useWorkflowSend.ts new file mode 100644 index 0000000..70ebd25 --- /dev/null +++ b/desktop/src/hooks/useWorkflowSend.ts @@ -0,0 +1,88 @@ +import { useCallback } from "react"; + +import { daemon } from "../daemon"; +import type { PromptSubmission, TaskInfo } from "../protocol"; + +export interface WorkflowSend { + /** True when this task is a workflow parent — it has no agent session. */ + isWorkflow: boolean; + /** True while the pipeline runs unattended: a message has no addressee. */ + disabled: boolean; + /** `undefined` keeps the composer's own default placeholder. */ + placeholder: string | undefined; + /** + * Deliver a composed message. Returns true when it was handled as pipeline + * input, so callers must not also prompt the (nonexistent) parent session. + */ + send: (submission: PromptSubmission) => Promise; +} + +/** + * Routing for messages typed into a workflow parent's composer. + * + * The parent task has no ACP session of its own — the daemon drives its stages + * — so `session.prompt` would be rejected. A message is only meaningful at the + * barriers where the pipeline is waiting for a human, and each barrier has its + * own RPC. Shared by every composer that can be pointed at a task, so no + * surface can accidentally fall through to the raw prompt path. + */ +export function useWorkflowSend(task: TaskInfo): WorkflowSend { + const run = task.workflowRun ?? null; + const waiting = run?.waiting ?? null; + const finished = run?.stage === "done" || run?.stage === "failed"; + const disabled = !!run && !waiting; + + const send = useCallback( + async (submission: PromptSubmission): Promise => { + if (!run) return false; + const text = submission.text.trim(); + switch (waiting?.kind) { + case "question": + await daemon.workflowReply(task.id, text); + return true; + case "paused": + await daemon.workflowResume(task.id, text || undefined); + return true; + case "limit": + // Typed guidance rides along with one more round of fixes. + await daemon.workflowDecide(task.id, "extend", { + note: text || undefined, + rounds: 1, + }); + return true; + default: + // Nothing is listening: swallow rather than prompting a parent that + // has no session (which fails with a raw daemon error). + return true; + } + }, + [run, task.id, waiting], + ); + + return { + disabled, + isWorkflow: !!run, + placeholder: placeholderFor(waiting?.kind, !!run, finished), + send, + }; +} + +function placeholderFor( + kind: "question" | "limit" | "paused" | undefined, + isWorkflow: boolean, + finished: boolean, +): string | undefined { + switch (kind) { + case "question": + return "Answer the stage's question…"; + case "paused": + return "Add guidance for the next stage, then send to resume…"; + case "limit": + return "Add guidance for another fix round, or pick an option above…"; + default: + if (!isWorkflow) return undefined; + return finished + ? "This pipeline has finished — open a stage above to continue with that agent, or start a new task." + : "The pipeline is running — open a stage above to message that agent."; + } +} diff --git a/desktop/src/lib/attentionRail.test.ts b/desktop/src/lib/attentionRail.test.ts index 0a6d04f..797bb02 100644 --- a/desktop/src/lib/attentionRail.test.ts +++ b/desktop/src/lib/attentionRail.test.ts @@ -53,6 +53,20 @@ describe("taskStatusRank", () => { }); }); +function waitingRun( + kind: "question" | "limit" | "paused", + question?: string, +): NonNullable { + return { + maxRounds: 2, + round: 1, + stage: "review", + waiting: { kind, question }, + workflowId: "wf", + workflowName: "Review loop", + }; +} + describe("buildAttentionQueue", () => { it("orders permission > review > blocked > interrupted", () => { const tasks = [ @@ -88,6 +102,46 @@ describe("buildAttentionQueue", () => { }); }); +describe("buildAttentionQueue — workflow pipelines", () => { + it("queues a pipeline waiting on a question, using the question as the reason", () => { + const t = task("wf", { status: "running", workflowRun: waitingRun("question", "Which db?") }); + const [item] = buildAttentionQueue([t], {}); + expect(item.reason).toBe("Which db?"); + expect(item.task.id).toBe("wf"); + }); + + it("queues an exhausted review limit with its findings summary", () => { + const t = task("wf", { + status: "running", + workflowRun: waitingRun("limit", "open findings: 2 high"), + }); + const [item] = buildAttentionQueue([t], {}); + expect(item.reason).toBe("review limit reached — open findings: 2 high"); + }); + + it("leaves a user-initiated pause out of the queue", () => { + const t = task("wf", { status: "running", workflowRun: waitingRun("paused") }); + expect(buildAttentionQueue([t], {})).toEqual([]); + }); + + it("ranks a waiting pipeline under a permission but above needs_review", () => { + const tasks = [ + task("review", { status: "needs_review" }), + task("wf", { status: "running", workflowRun: waitingRun("question", "?") }), + task("perm", { status: "running" }), + ]; + const queue = buildAttentionQueue(tasks, { perm: [permUpdate()] }); + expect(queue.map((i) => i.task.id)).toEqual(["perm", "wf", "review"]); + }); + + it("still reports a pending permission on a workflow stage child", () => { + const t = task("wf", { status: "running", workflowRun: waitingRun("question", "?") }); + const [item] = buildAttentionQueue([t], { wf: [permUpdate("p", "Write file?")] }); + expect(item.reason).toBe("Write file?"); + expect(item.permission).toBeDefined(); + }); +}); + describe("selectRailTasks", () => { const attention = (ids: string[]): Map => new Map(ids.map((id) => [id, { priority: 1, reason: "test", task: task(id) }])); diff --git a/desktop/src/lib/attentionRail.ts b/desktop/src/lib/attentionRail.ts index f26fb4d..6f6f127 100644 --- a/desktop/src/lib/attentionRail.ts +++ b/desktop/src/lib/attentionRail.ts @@ -49,8 +49,21 @@ export function buildAttentionQueue( prunePermissionCache(new Set(tasks.map((task) => task.id))); for (const task of tasks) { const perm = latestPendingPermission(task.id, sessionUpdates[task.id]); + const waiting = task.workflowRun?.waiting ?? null; if (perm) { items.push({ permission: perm, priority: 0, reason: perm.title, task }); + } else if (waiting && waiting.kind !== "paused") { + // A pipeline that suspended itself is asking directly, so it ranks just + // under a permission prompt. A pause is user-initiated — the user knows + // it is waiting, so it stays out of the queue. + items.push({ + priority: 0.5, + reason: + waiting.kind === "question" + ? (waiting.question ?? "workflow needs your input") + : `review limit reached${waiting.question ? ` — ${waiting.question}` : ""}`, + task, + }); } else if (task.status === "needs_review") { items.push({ priority: 1, reason: "finished — review changes", task }); } else if (task.status === "blocked") { diff --git a/desktop/src/lib/sessionPreview.ts b/desktop/src/lib/sessionPreview.ts index 488cb4f..c0c510d 100644 --- a/desktop/src/lib/sessionPreview.ts +++ b/desktop/src/lib/sessionPreview.ts @@ -128,6 +128,11 @@ export function latestSessionPreview( continue; } + if (update.kind === "workflow_event") { + // A workflow parent's transcript is almost entirely these, so without a + // branch here a running pipeline shows no activity in the rail or tiles. + return { kind: "message", label: "Pipeline", text: update.title, truncated: false }; + } if (update.kind === "agent_text" || update.kind === "agent_thought") { const preview = textPreview(updates, index, update.kind, active, maxChars); if (preview) return preview; diff --git a/desktop/src/lib/snooze.test.ts b/desktop/src/lib/snooze.test.ts index 05bb8b1..e2194fc 100644 --- a/desktop/src/lib/snooze.test.ts +++ b/desktop/src/lib/snooze.test.ts @@ -8,13 +8,7 @@ import { snoozeTomorrowMorning, } from "./snooze"; -function localMs( - year: number, - month: number, - day: number, - hours: number, - minutes: number, -): number { +function localMs(year: number, month: number, day: number, hours: number, minutes: number): number { return new Date(year, month, day, hours, minutes, 0, 0).getTime(); } diff --git a/desktop/src/lib/taskGroups.test.ts b/desktop/src/lib/taskGroups.test.ts index 6fae8ac..f919565 100644 --- a/desktop/src/lib/taskGroups.test.ts +++ b/desktop/src/lib/taskGroups.test.ts @@ -11,6 +11,7 @@ import { resolveGroupTaskId, setTaskGroupPinned, taskGroupStatus, + taskNeedsAttention, treeLane, treeMatches, } from "./taskGroups"; @@ -120,6 +121,21 @@ describe("task orchestration groups", () => { expect(forest).toStrictEqual([]); }); + it("treats a workflow waiting for a decision as attention while its task is idle", () => { + const workflow = task("workflow", "idle"); + workflow.workflowRun = { + maxRounds: 2, + round: 2, + stage: "review", + verdict: "request_changes", + waiting: { kind: "limit", question: "open findings: 1 medium" }, + workflowId: "review-loop", + workflowName: "Review loop", + }; + + expect(taskNeedsAttention(workflow)).toBeTruthy(); + }); + it("handles self-referencing parent (cycle to self)", () => { const forest = buildTaskForest([task("self-ref", "running", "self-ref")]); // Self-referencing task becomes a root (cycle broken) diff --git a/desktop/src/lib/taskGroups.ts b/desktop/src/lib/taskGroups.ts index 78b6d48..2f550cc 100644 --- a/desktop/src/lib/taskGroups.ts +++ b/desktop/src/lib/taskGroups.ts @@ -63,8 +63,12 @@ export function taskLifecycle(task: TaskInfo, nowSeconds: number): TaskLifecycle } export function taskNeedsAttention(task: TaskInfo): boolean { + const waiting = task.workflowRun?.waiting ?? null; return ( - task.status === "needs_review" || task.status === "blocked" || task.status === "interrupted" + (!!waiting && waiting.kind !== "paused") || + task.status === "needs_review" || + task.status === "blocked" || + task.status === "interrupted" ); } diff --git a/desktop/src/lib/terminalWorkspace.ts b/desktop/src/lib/terminalWorkspace.ts index 3f2629e..9967865 100644 --- a/desktop/src/lib/terminalWorkspace.ts +++ b/desktop/src/lib/terminalWorkspace.ts @@ -155,7 +155,12 @@ export class TerminalWorkspace { return label; } - private attachTerminal(info: { id: string; project: string; command: string; startedAt: number }) { + private attachTerminal(info: { + id: string; + project: string; + command: string; + startedAt: number; + }) { if (this.entries.has(info.id)) return; if (this.exitedTombstones.has(info.id)) return; const controller = new TerminalController({ terminalId: info.id, project: info.project }); @@ -183,7 +188,10 @@ export class TerminalWorkspace { private getTerminalSignature(): string { const snap = daemon.getState().snapshot; const projectTerminals = snap.terminals.filter((t) => t.project === this.project); - return projectTerminals.map((t) => t.id).sort().join(","); + return projectTerminals + .map((t) => t.id) + .sort() + .join(","); } private reconcileFromSnapshot() { diff --git a/desktop/src/lib/updater.test.ts b/desktop/src/lib/updater.test.ts index 58460d0..87bc896 100644 --- a/desktop/src/lib/updater.test.ts +++ b/desktop/src/lib/updater.test.ts @@ -35,9 +35,7 @@ describe("DesktopUpdater", () => { }); it("explains that the update feed is unavailable before the first desktop release", async () => { - check.mockRejectedValueOnce( - new Error("Could not fetch a valid release JSON from the remote"), - ); + check.mockRejectedValueOnce(new Error("Could not fetch a valid release JSON from the remote")); const updater = new DesktopUpdater(); await updater.check(); diff --git a/desktop/src/lib/workflow.ts b/desktop/src/lib/workflow.ts new file mode 100644 index 0000000..51bb533 --- /dev/null +++ b/desktop/src/lib/workflow.ts @@ -0,0 +1,22 @@ +import type { WorkflowRunInfo } from "@/protocol"; + +export function workflowStageLabel(stage: WorkflowRunInfo["stage"]): string { + switch (stage) { + case "plan": + return "planning"; + case "implement": + return "implementing"; + case "review": + return "reviewing"; + case "fix": + return "fixing"; + case "done": + return "done"; + case "failed": + return "failed"; + default: + // A newer daemon may report a stage this build does not know; show the + // raw value rather than silently rendering nothing. + return stage; + } +} diff --git a/desktop/src/protocol.ts b/desktop/src/protocol.ts index 7024e18..7849cf4 100644 --- a/desktop/src/protocol.ts +++ b/desktop/src/protocol.ts @@ -192,8 +192,10 @@ export interface TaskInfo { configOptions?: ConfigOption[]; /** Path to the git worktree for this task, if isolated. */ worktree?: string | null; - /** Orchestration graph for parent orchestrator tasks. */ + /** Orchestration graph for parent orchestrator tasks, and for workflow parents. */ orchestrationGraph?: OrchGraphInfo | null; + /** Live pipeline state when this task is a workflow parent. */ + workflowRun?: WorkflowRunInfo | null; /** Task that spawned this sub-agent through the orchestrator MCP. */ parentTaskId?: string | null; /** Explicit settle override (true = settled, false = not settled). */ @@ -237,7 +239,7 @@ export interface OrchNodeInfo { result?: string | null; } -export type OrchNodeKind = "plan" | "implement" | "review" | "merge"; +export type OrchNodeKind = "plan" | "implement" | "review" | "fix" | "merge"; export type OrchNodeStatus = "pending" | "running" | "complete" | "failed" | "skipped"; @@ -256,6 +258,58 @@ export interface OrchReviewerPool { agent: string; } +// ── Workflow DTOs ────────────────────────────────────────────────────────── + +/** One selectable workflow template, from `workflow.list`. */ +export interface WorkflowMeta { + id: string; + name: string; + description?: string | null; + source: WorkflowSource; + /** False when the YAML failed to parse or validate — listed but unselectable. */ + valid: boolean; + error?: string | null; + /** Non-fatal issues (unknown keys, clamped values). */ + warnings?: string[]; + /** Stage names for the picker tooltip, e.g. ["plan","implement","review\u00d72","fix"]. */ + stages?: string[]; + maxRounds?: number; +} + +export type WorkflowSource = "project" | "builtin"; + +/** Live state of a workflow pipeline, carried on its parent task. */ +export interface WorkflowRunInfo { + workflowId: string; + workflowName: string; + stage: WorkflowStage; + /** Current review round, 1-based; 0 until the first review starts. */ + round: number; + /** Round limit including any user-granted extensions. */ + maxRounds: number; + verdict?: WorkflowVerdict | null; + /** Set while the pipeline waits for the user. */ + waiting?: WorkflowWaiting | null; + /** A pause is queued and takes effect when the running stage finishes. */ + pauseRequested?: boolean; +} + +export type WorkflowStage = "plan" | "implement" | "review" | "fix" | "done" | "failed"; + +export type WorkflowVerdict = "approve" | "request_changes"; + +export interface WorkflowWaiting { + kind: WorkflowWaitKind; + /** Which stage asked (for `question`). */ + stage?: WorkflowStage | null; + /** The question text, or a findings summary for `limit`. */ + question?: string | null; +} + +export type WorkflowWaitKind = "question" | "limit" | "paused"; + +export type WorkflowDecision = "extend" | "finish" | "stop"; + export type ToolCallStatus = "pending" | "in_progress" | "completed" | "failed"; export interface PlanEntry { @@ -287,10 +341,36 @@ export interface SessionUsageCost { currency: string; } +export interface WorkflowEventAgent { + taskId: string; + label: string; + agent: string; + model?: string | null; +} + +export type WorkflowEventKind = + | "workflow_started" + | "stage_started" + | "agent_output" + | "review_result" + | "status" + | "workflow_finished"; + +export type WorkflowEventTone = "info" | "running" | "success" | "warning" | "error"; + export type SessionUpdate = | { kind: "user_message"; text: string; attachments?: PromptAttachmentSummary[] } | { kind: "prompt_capabilities"; image: boolean; embedded_context: boolean } | { kind: "agent_text"; text: string } + | { + kind: "workflow_event"; + event: WorkflowEventKind; + title: string; + detail?: string | null; + stage?: WorkflowStage | null; + agents: WorkflowEventAgent[]; + tone: WorkflowEventTone; + } | { kind: "agent_thought"; text: string } | { kind: "tool_call"; diff --git a/desktop/src/views/Board.tsx b/desktop/src/views/Board.tsx index b08055a..2de2ebc 100644 --- a/desktop/src/views/Board.tsx +++ b/desktop/src/views/Board.tsx @@ -7,6 +7,7 @@ import { ChevronRight, GitBranch, Plus, + Route, Workflow, } from "lucide-react"; import { useEffect, useMemo, useState } from "react"; @@ -41,6 +42,7 @@ import { } from "@/lib/taskGroups"; import { taskLabel } from "@/lib/taskLabel"; import { cn } from "@/lib/utils"; +import { workflowStageLabel } from "@/lib/workflow"; import type { OrchNodeInfo, Snapshot, TaskInfo, TaskStatus } from "../protocol"; @@ -562,6 +564,7 @@ function TaskCard({
+ {task.project} {task.worktree && }
{nodes.map((node) => ( - + // `node.id` is a human label and repeats when a stage re-runs + // (restart-resume, or a same-session review round), so it is not + // a key on its own. + ))}
)} @@ -617,7 +623,12 @@ function NodeRow({ node }: { node: OrchNodeInfo }) { return (
- {node.kind} + + {node.id || node.kind} + {node.taskId && ( {node.taskId} @@ -709,6 +720,43 @@ function LifecycleBadge({ task, nowSeconds }: { task: TaskInfo; nowSeconds: numb ); } +/** + * Pipeline stage pill for workflow parent tasks. Amber while the pipeline + * waits on the user (a question or an exhausted review limit), muted + * otherwise — the surrounding StatusBadge already carries run/done state. + */ +function WorkflowBadge({ task }: { task: TaskInfo }) { + const run = task.workflowRun; + if (!run || run.stage === "done" || run.stage === "failed") return null; + const waiting = run.waiting ?? null; + const needsUser = !!waiting && waiting.kind !== "paused"; + const label = + waiting?.kind === "limit" + ? "review limit" + : waiting?.kind === "question" + ? "needs answer" + : waiting?.kind === "paused" + ? "paused" + : workflowStageLabel(run.stage); + return ( + 0 ? ` — round ${run.round}/${run.maxRounds}` : ""}`} + className={cn( + "inline-flex min-w-0 max-w-32 items-center gap-1 truncate rounded-full px-1.5 py-0.5 text-[10px] font-medium", + needsUser ? "bg-warn/12 text-warn" : "bg-primary/10 text-primary", + )} + > + + {label} + {run.round > 0 && !needsUser && ( + + {run.round}/{run.maxRounds} + + )} + + ); +} + function Empty() { return
No tasks
; } diff --git a/desktop/src/views/MissionControl.test.ts b/desktop/src/views/MissionControl.test.ts index 84e6285..638e5e9 100644 --- a/desktop/src/views/MissionControl.test.ts +++ b/desktop/src/views/MissionControl.test.ts @@ -146,6 +146,66 @@ describe("agent text streaming", () => { }); }); +describe("workflow conversation events", () => { + it("renders a persistent inline agent card and opens its child session", async () => { + const user = userEvent.setup(); + const onOpenTask = vi.fn<(id: string) => void>(); + render( + createElement(StreamLine, { + onOpenTask, + update: { + agents: [ + { + agent: "codex", + label: "implement", + model: "gpt-5.6-sol", + taskId: "t_impl", + }, + ], + event: "stage_started", + kind: "workflow_event", + stage: "implement", + title: "Implement started", + tone: "running", + }, + }), + ); + + expect(screen.getByText("Implement started")).toBeInTheDocument(); + expect(screen.getByText("gpt-5.6-sol")).toBeInTheDocument(); + await user.click(screen.getByRole("button", { name: "Open implement agent session" })); + expect(onOpenTask).toHaveBeenCalledWith("t_impl"); + }); + + it("renders an agent summary as the next independent history entry", () => { + render( + createElement(StreamLine, { + update: { + agents: [ + { + agent: "codex", + label: "implement", + taskId: "t_impl", + }, + ], + detail: "Implemented the parser and ran **all tests**.", + event: "agent_output", + kind: "workflow_event", + stage: "implement", + title: "Implement completed", + tone: "success", + }, + }), + ); + + expect(screen.getByText("Implement completed")).toBeInTheDocument(); + expect(screen.getByText(/Implemented the parser/)).toBeInTheDocument(); + expect( + screen.getByRole("button", { name: "Open implement agent session" }), + ).toBeInTheDocument(); + }); +}); + describe("incremental coalescing", () => { const stream: SessionUpdate[] = [ { kind: "user_message", text: "hi" }, diff --git a/desktop/src/views/MissionControl.tsx b/desktop/src/views/MissionControl.tsx index efe2fa0..e8d3ed7 100644 --- a/desktop/src/views/MissionControl.tsx +++ b/desktop/src/views/MissionControl.tsx @@ -58,8 +58,11 @@ import { BufferedMarkdown, CollapsibleMarkdown, Markdown } from "../components/M import { StatusBadge } from "../components/StatusBadge"; import { TaskAgentSwitcher } from "../components/TaskAgentSwitcher"; import { ThinkingBlock } from "../components/ThinkingBlock"; +import { WorkflowControls } from "../components/WorkflowControls"; +import { WorkflowEventLine } from "../components/WorkflowEventLine"; import type { DaemonState } from "../daemon"; import { daemon } from "../daemon"; +import { useWorkflowSend } from "../hooks/useWorkflowSend"; import type { CommandInfo, EditHunk, ProjectFile, SessionUpdate, TaskInfo } from "../protocol"; import { daemonQuery } from "../query"; import { useUi } from "../store/ui"; @@ -293,6 +296,8 @@ function FocusPane({ const capability = [...updates].reverse().find((update) => update.kind === "prompt_capabilities"); const imageSupported = capability?.kind === "prompt_capabilities" ? capability.image : false; const activity = sessionActivity(task, stream); + const workflowSend = useWorkflowSend(task); + const openTask = useUi((s) => s.openTask); return ( ))} @@ -435,6 +441,7 @@ function FocusPane({
+ {task.workflowRun && } { + // A workflow parent has no session — route to the pipeline instead. + if (await workflowSend.send(submission)) return; await daemon.request("session.prompt", { task_id: task.id, ...submission }); }} toolbar={ @@ -557,11 +566,14 @@ function PinnedStreamLine({ compact, taskId, resolved, + onOpenTask, }: { update: SessionUpdate; compact?: boolean; taskId?: string; resolved?: Record; + /** Lets a workflow stage card in a tile open that stage's session. */ + onOpenTask?: (id: string) => void; }) { if (update.kind === "agent_text" || update.kind === "agent_thought") { return ( @@ -575,10 +587,15 @@ function PinnedStreamLine({
); } - if (update.kind === "user_message") { - return ; - } - return ; + return ( + + ); } /** Shared renderer for one session-stream update. `compact` = focus pane @@ -703,6 +720,7 @@ export function StreamLine({ resolveFilePath, onOpenFile, onOpenFileDiff, + onOpenTask, project, thinkingActive, textStreaming, @@ -716,6 +734,8 @@ export function StreamLine({ resolveFilePath?: FileLinkResolver; onOpenFile?: (path: string) => void; onOpenFileDiff?: (path: string, hunks?: EditHunk[]) => void; + /** Opens a workflow stage/reviewer child from an inline timeline card. */ + onOpenTask?: (id: string) => void; /** Project root label retained after stripping the machine-specific prefix. */ project?: string; /** True only for the thought block currently receiving streamed deltas. */ @@ -779,6 +799,8 @@ export function StreamLine({ {update.text} ); + case "workflow_event": + return ; case "agent_thought": return compact ? ( s.openTask); const autoNameTasks = useUi((s) => s.autoNameTasks); const textGenAgentId = useUi((s) => s.textGenAgentId); @@ -53,6 +60,7 @@ export default function NewTaskDialog({ const [shareContext, setShareContext] = useState(true); const [useWorktree, setUseWorktree] = useState(false); const [orchChat, setOrchChat] = useState(false); + const [workflow, setWorkflow] = useState(null); const composerRef = useRef(null); const agent = enabledAgents.some((candidate) => candidate.id === selectedAgent) @@ -66,6 +74,20 @@ export default function NewTaskDialog({ setProject(nextProject); setSelectedAgent(enabledAgents[0]?.id ?? "claude"); setConfigPicks({}); + // Workflows are per-project files — a pick can't carry over. + setWorkflow(null); + }; + + // A workflow and the orchestrator chat are different engines for the same + // task, so picking one clears the other. + const changeWorkflow = (next: string | null) => { + setWorkflow(next); + if (next) setOrchChat(false); + }; + + const changeOrchChat = (next: boolean) => { + setOrchChat(next); + if (next) setWorkflow(null); }; const changeAgent = (nextAgent: string) => { @@ -99,6 +121,25 @@ export default function NewTaskDialog({ queryKey: ["fileList", "new", project], }); const projectFiles = Array.isArray(filesQuery.data) ? filesQuery.data : []; + const workflowsQuery = useQuery({ + enabled: !!project, + queryFn: () => daemon.workflowList(project), + queryKey: ["workflows", project], + }); + const workflows: WorkflowMeta[] = workflowsQuery.data ?? []; + const selectedWorkflow = workflows.find((w) => w.id === workflow) ?? null; + + const ejectWorkflow = async (id: string) => { + try { + const path = await daemon.workflowEject(project, id); + toast.success("Workflow copied to project", { description: path }); + await queryClient.invalidateQueries({ queryKey: ["workflows", project] }); + } catch (e) { + toast.error("Could not copy workflow", { + description: e instanceof Error ? e.message : String(e), + }); + } + }; const close = useCallback(() => onOpenChange(false), [onOpenChange]); @@ -125,24 +166,37 @@ export default function NewTaskDialog({ const pick = configPicks[opt.id]; if (pick != null) configOverrides[opt.id] = pick; } - const resp = await daemon.request("task.create", { - project, - prompt: submission.text.trim(), - attachments: submission.attachments, - agent, - tags: orchChat ? [...userTags, "orchestrator-chat"] : userTags, - include_runtime_context: shareContext, - worktree: orchChat ? false : useWorktree, - default_model: modelPick, - config_overrides: configOverrides, - }); + let resp: unknown; + try { + resp = await daemon.request("task.create", { + project, + prompt: submission.text.trim(), + attachments: submission.attachments, + agent, + tags: orchChat ? [...userTags, "orchestrator-chat"] : userTags, + include_runtime_context: shareContext, + worktree: orchChat ? false : useWorktree, + default_model: modelPick, + config_overrides: configOverrides, + workflow: workflow ?? undefined, + }); + } catch (e) { + // A workflow can fail validation daemon-side (edited YAML between the + // list and the send) — keep the dialog open so the prompt isn't lost. + toast.error("Could not start the task", { + description: e instanceof Error ? e.message : String(e), + }); + return; + } const taskId = (resp as { taskId?: string } | null)?.taskId ?? (resp as { result?: { taskId?: string } } | null)?.result?.taskId ?? null; if (taskId) { - toast.success("Task started", { - description: `${orchChat ? "Orchestrator" : "Agent"} session created for ${project}`, + toast.success(selectedWorkflow ? "Workflow started" : "Task started", { + description: selectedWorkflow + ? `${selectedWorkflow.name} pipeline running in ${project}` + : `${orchChat ? "Orchestrator" : "Agent"} session created for ${project}`, action: { label: "Open task", onClick: () => openTask(taskId), @@ -180,7 +234,9 @@ export default function NewTaskDialog({

New task

- One task = one agent session. The agent starts working immediately. + {selectedWorkflow + ? `One task = the ${selectedWorkflow.name} pipeline. Each stage runs as its own agent session.` + : "One task = one agent session. The agent starts working immediately."}
diff --git a/desktop/src/views/TaskDetail.tsx b/desktop/src/views/TaskDetail.tsx index a178d2d..c5f1586 100644 --- a/desktop/src/views/TaskDetail.tsx +++ b/desktop/src/views/TaskDetail.tsx @@ -372,7 +372,7 @@ export default function TaskDetail({ task, snapshot, onClose, onOpenTask, onOpen

Loading changes…

) ) : rightPanel === "subtasks" ? ( - + ) : ( (null); - const hasLiveResources = - liveCounts.services + liveCounts.portforwards + liveCounts.terminals > 0; + const hasLiveResources = liveCounts.services + liveCounts.portforwards + liveCounts.terminals > 0; const actorRejectedRemoval = error?.startsWith("conflict:") === true || error?.startsWith("internal:") === true; @@ -85,12 +84,7 @@ export function RemoveProjectDialog({ project, liveCounts, onCancel, onConfirm }
  • {countLabel(liveCounts.services, "running or starting service")}
    • )} {liveCounts.portforwards > 0 && ( -
  • - {countLabel( - liveCounts.portforwards, - "active or starting port-forward", - )} -
    • +
  • {countLabel(liveCounts.portforwards, "active or starting port-forward")}
    • )} {liveCounts.terminals > 0 && (
  • {countLabel(liveCounts.terminals, "live terminal")}
    • diff --git a/desktop/src/views/task-detail/SubtasksRail.tsx b/desktop/src/views/task-detail/SubtasksRail.tsx index a8017d8..04ced5f 100644 --- a/desktop/src/views/task-detail/SubtasksRail.tsx +++ b/desktop/src/views/task-detail/SubtasksRail.tsx @@ -6,20 +6,50 @@ import { AgentBadge } from "../../components/AgentBadge"; import { StatusBadge } from "../../components/StatusBadge"; import type { OrchNodeInfo, TaskInfo } from "../../protocol"; -function SubtaskRow({ node }: { node: OrchNodeInfo }) { - return ( -
    +function SubtaskRow({ + node, + onOpenTask, +}: { + node: OrchNodeInfo; + onOpenTask: (id: string) => void; +}) { + const content = ( + <> - {node.kind} + + {node.id || node.kind} + {node.taskId && ( {node.taskId} )} + + ); + return node.taskId ? ( + + ) : ( +
    + {content}
    ); } -export function SubtasksRail({ task }: { task: TaskInfo }) { +export function SubtasksRail({ + onOpenTask, + task, +}: { + onOpenTask: (id: string) => void; + task: TaskInfo; +}) { const nodes = task.orchestrationGraph?.nodes ?? []; const graph = task.orchestrationGraph; if (!graph) { @@ -38,7 +68,12 @@ export function SubtasksRail({ task }: { task: TaskInfo }) {
    {nodes.map((node) => ( - + // Labels repeat when a stage re-runs, so index-qualify the key. + ))}
    diff --git a/docs/adr/0001-workflow-pipelines.md b/docs/adr/0001-workflow-pipelines.md new file mode 100644 index 0000000..a8385b8 --- /dev/null +++ b/docs/adr/0001-workflow-pipelines.md @@ -0,0 +1,125 @@ +# 0001 — Workflow pipelines: deterministic engine, project-configured + +**Status:** accepted (2026-07-30) · introduced in `ephor/warpforge#21` + +## Context + +"Process" used to mean one **Orchestrator** toggle: a hardcoded prompt plus +three MCP tools handed to an LLM, which then decided for itself what to +delegate. There were no roles, no review, no iteration limit, and nothing a +user could configure. Making the process better meant editing a prompt string +in the binary. + +## Decisions + +**The engine is deterministic. The model never decides what happens next.** +Transitions live in `daemon/workflow.rs` + the workflow block of +`daemon/actor.rs`. *Rejected:* a manager agent driving the pipeline through +tools — it reintroduces exactly the "process is whatever the model felt like" +problem, and its failures are unreproducible. + +**A workflow is a YAML file in the project** (`.warpforge/workflows/*.yaml`), +not UI state or a DB row. It lives with the code it reviews, is diffable, and +travels with the repo for the whole team. Built-ins ship in the binary and can +be copied into a project to edit. + +**The pipeline shape is fixed** (`plan? → implement → review ⇄ fix`) and the +YAML configures those stages. *Rejected for the first iteration:* an arbitrary +stage DAG with activation conditions and gate expressions. The fixed shape +covers the real cases, and a DAG can be added later without changing the file +format for existing workflows. + +**Each stage is a normal child task with its own agent session.** It appears on +the board, its chat can be opened, and a human can intervene in it. *Rejected:* +one long session with role-switching prompts — the roles then share context and +a reviewer inherits the implementer's blind spots. + +**The parent task has no agent session.** It is the pipeline's record: a +timeline of stages, plus the place where the engine asks the user something. +Consequence: the parent's composer can only be an answer box at a barrier, and +**everything that routes composer input must go through `useWorkflowSend`** — +prompting the parent directly fails with a raw daemon error. + +**The user is asked at structured decision points, not in free chat.** A stage +can suspend the run with a `need_user_input` marker; exhausting the review +rounds asks extend/finish/stop. Control is by button, free text is payload with +one unambiguous addressee. *Rejected:* interpreting free text as commands — +that needs an LLM in the control path, which decision 1 rules out. + +**Repeat review rounds continue in the same reviewer's session** by default +(`review.reask`). The reviewer remembers its own findings and can verify each +one, and it costs a delta instead of a full re-read. The cost is anchoring +bias, mitigated in the prompt ("defending your earlier verdict is not a goal") +and by an explicit regression check. A dead session falls back to a fresh one +carrying the previous findings. + +**Agents talk to the engine through fenced JSON** (verdict, `need_user_input`), +appended to every stage prompt including custom ones. Machine-readable output +from a text model is the weakest link in the design, so every parse path has a +fallback rather than a hard failure. + +**A finished pipeline commits nothing.** It lands in `NeedsReview` for a human. +*Rejected:* an `on_success: commit` option — it makes an unattended pipeline +able to write history. + +**Pause is soft, at stage boundaries.** The running stage finishes its turn and +the next one does not start. This is what makes pause survive a daemon restart: +no barrier state depends on a live session. *Rejected for now:* interrupting a +turn mid-flight. + +## Invariants + +Break one of these and the failure is quiet. Each was a real bug found in +review. + +1. **`workflow_runs` take/put.** Methods `remove` the run and must re-insert on + *every* exit path, including early returns and error branches. A dropped run + leaves the parent stuck in "running" with no controls. +2. **`AcpHandle::prompt` is not a liveness check.** Its channel belongs to the + driver task, which outlives the child process, so it returns `Ok` for a dead + agent and the prompt vanishes. Ask `is_alive()` before treating a follow-up + as delivered — otherwise the same-session and dead-session fallbacks are + unreachable exactly when they are needed. +3. **A stage's output is its closing message** — the text after its last tool + call — not the whole turn. The full turn is only a parsing fallback. This + keeps tool narration out of reviewer prompts and stops a JSON block quoted + mid-turn from being read as the protocol payload. +4. **`request_changes` must never finish the run as a success.** A reviewer that + asks for changes but produces no parseable findings has its prose salvaged + into one finding. The "only low-severity notes remain" shortcut is valid + *only* when findings were actually parsed and all were low. +5. **A reviewer that cannot produce a verdict abstains.** Only "no reviewer + produced one" fails the run. Failing a complete implementation because one + agent wrote prose twice is not acceptable. +6. **A stage whose session fails to start must fail the run.** `start_session` + reports failure by blocking the child and inserting no handle, so no + `TurnEnded` will ever arrive and nothing else will notice. +7. **Stage sessions stay alive for the whole run** (same-session re-review needs + them) and must *all* be swept at finalize — `active_children` is not enough. +8. **The round counter increments only when a new review round starts.** + Re-entering the same round after a restart must not re-count it, or the run + reports "round 3/2" and jumps straight to the limit decision. +9. **Waits on a child's exit are bounded.** The daemon actor is single-threaded + and awaits handlers inline; an unkillable child would otherwise freeze every + project. +10. **Everything persisted in a run snapshot carries `serde(default)`.** A field + that is required on load turns every in-flight pipeline unreadable after an + upgrade. + +## Consequences + +- Roughly 3–4× the tokens and wall-clock of a single-agent run, and stages are + serial. Worth it for review-worthy changes, not for trivial ones. +- `max_rounds` counts **reviews**; a fix runs between them, so N rounds buy N−1 + repair attempts (default 3 = two attempts). +- Usefulness depends on how well agents follow the JSON protocols. The prompts + are load-bearing: treat them as code, not copy. +- Reviewers see the working-copy diff, so a dirty tree or an agent that commits + its own work distorts what they review. + +## Out of scope, deliberately + +Finding ledger and cross-round dedup, deterministic build/lint/test gates +(agents run their own checks), separate tester and final-acceptor roles, global +`~/.warpforge/workflows/`, template versioning, auto-commit. The richer +reference design these were pruned from is kept outside the repo. diff --git a/docs/adr/README.md b/docs/adr/README.md new file mode 100644 index 0000000..c4387e6 --- /dev/null +++ b/docs/adr/README.md @@ -0,0 +1,18 @@ +# Architecture decision records + +Decisions that are **not recoverable from the code**: what was chosen, what was +rejected, and which invariants a future change must not break. If something can +be learned by reading the code, it does not belong here — that duplication goes +stale and then misleads. + +- One file per decision or per coherent cluster of decisions, numbered: + `NNNN-short-slug.md`. +- Records are append-only. Don't rewrite a decision that changed — add a new + record and mark the old one `Superseded by NNNN`. +- Keep each one short enough to read in full before touching the subsystem. + The **Invariants** section is the part that prevents regressions; put real + effort there and name the module it applies to. + +| ADR | Subject | +| --- | --- | +| [0001](0001-workflow-pipelines.md) | Workflow pipelines: deterministic engine, project-configured | diff --git a/src/config.rs b/src/config.rs index f4d3054..b57a19b 100644 --- a/src/config.rs +++ b/src/config.rs @@ -89,8 +89,15 @@ pub fn sorted_services(config: &WorkspaceConfig) -> Vec { result } -/// Config file names in priority order: new → legacy. -const CONFIG_NAMES: &[&str] = &[".warpforge.yaml", ".wf.yaml", ".workspace.yaml"]; +/// Config file names in priority order: new → legacy. `.warpforge/` is the +/// preferred home for warpforge files (workspace config, workflows); the +/// root-level names keep working for existing projects. +const CONFIG_NAMES: &[&str] = &[ + ".warpforge/workspace.yaml", + ".warpforge.yaml", + ".wf.yaml", + ".workspace.yaml", +]; /// Load a project's config while preserving the distinction between a missing /// config and an existing file that could not be read or parsed. @@ -115,7 +122,9 @@ pub fn load_workspace_config(project_path: &Path) -> Option { try_load_workspace_config(project_path).ok().flatten() } -/// Return the first existing config file path, or the default `.warpforge.yaml`. +/// Return the first existing config file path, or the default +/// `.warpforge/workspace.yaml` for projects that have no config yet. Callers +/// writing to the returned path must create its parent directory first. pub fn find_config_file(project_path: &Path) -> std::path::PathBuf { for name in CONFIG_NAMES { let p = project_path.join(name); @@ -123,7 +132,7 @@ pub fn find_config_file(project_path: &Path) -> std::path::PathBuf { return p; } } - project_path.join(".warpforge.yaml") + project_path.join(CONFIG_NAMES[0]) } fn auto_detect(project_path: &Path) -> Option { @@ -219,13 +228,17 @@ fn auto_detect(project_path: &Path) -> Option { }) } -/// Generate a .warpforge.yaml file in the given directory. -/// If auto-detection finds services, pre-populates them. +/// Generate a `.warpforge/workspace.yaml` file in the given directory. +/// If auto-detection finds services, pre-populates them. Refuses to run when +/// any config (new or legacy location) already exists. pub fn generate_workspace_yaml(project_path: &Path) -> anyhow::Result<()> { - let target = project_path.join(".warpforge.yaml"); - if target.exists() { - anyhow::bail!(".warpforge.yaml already exists at {}", target.display()); + for name in CONFIG_NAMES { + let existing = project_path.join(name); + if existing.exists() { + anyhow::bail!("config already exists at {}", existing.display()); + } } + let target = project_path.join(CONFIG_NAMES[0]); let name = project_path .canonicalize() @@ -238,11 +251,11 @@ pub fn generate_workspace_yaml(project_path: &Path) -> anyhow::Result<()> { let content = if let Some(config) = auto_detect(project_path) { // Serialize detected config let yaml = serde_yaml::to_string(&config)?; - format!("# .warpforge.yaml — auto-detected by warpforge\n{yaml}") + format!("# .warpforge/workspace.yaml — auto-detected by warpforge\n{yaml}") } else { // Write template format!( - r#"# .warpforge.yaml — Warpforge project configuration + r#"# .warpforge/workspace.yaml — Warpforge project configuration name: {name} services: @@ -263,6 +276,9 @@ services: ) }; + if let Some(parent) = target.parent() { + fs::create_dir_all(parent).with_context(|| format!("creating {}", parent.display()))?; + } fs::write(&target, content)?; println!("Created {}", target.display()); Ok(()) diff --git a/src/daemon/acp.rs b/src/daemon/acp.rs index c6733c6..817b213 100644 --- a/src/daemon/acp.rs +++ b/src/daemon/acp.rs @@ -98,6 +98,7 @@ pub enum AcpCommand { #[derive(Clone)] pub struct AcpHandle { cmd_tx: mpsc::UnboundedSender, + exit_rx: watch::Receiver, image_capability: Arc, process: Arc, run_id: u64, @@ -128,6 +129,51 @@ impl AcpHandle { let _ = self.cmd_tx.send(AcpCommand::Cancel); } + /// Request cancellation and wait until the process monitor has reaped the + /// ACP child. Callers can use this as the acknowledgement boundary for a + /// hard-stop operation. + pub async fn cancel_and_wait(&self) -> Result<(), String> { + self.cancel(); + self.wait_for_exit_within(STOP_GRACE).await + } + + /// False once the process monitor has reaped the ACP child. `prompt()` is + /// NOT a liveness test — its channel belongs to the driver task, which + /// outlives the child — so anything that must know whether an agent is + /// still there has to ask this. + pub fn is_alive(&self) -> bool { + matches!(*self.exit_rx.borrow(), ChildState::Running) + } + + /// Wait for the process monitor's post-`child.wait()` notification, giving + /// up after `timeout`. The bound matters: callers await this inline in the + /// daemon actor, so an unkillable child (D-state on a stuck mount) would + /// otherwise freeze every project's RPCs and events, not just this task. + pub async fn wait_for_exit_within(&self, timeout: std::time::Duration) -> Result<(), String> { + match tokio::time::timeout(timeout, self.wait_for_exit()).await { + Ok(result) => result, + Err(_) => Err(format!( + "agent process did not exit within {}s of being killed", + timeout.as_secs() + )), + } + } + + /// Wait for the process monitor's post-`child.wait()` notification. + pub async fn wait_for_exit(&self) -> Result<(), String> { + let mut exit_rx = self.exit_rx.clone(); + loop { + if matches!(*exit_rx.borrow(), ChildState::Exited(_)) { + return Ok(()); + } + if exit_rx.changed().await.is_err() { + return Err( + "ACP process monitor closed before confirming process termination".to_string(), + ); + } + } + } + pub fn run_id(&self) -> u64 { self.run_id } @@ -190,6 +236,9 @@ impl FailureReporter { } } +/// How long a caller waits for a killed ACP child to be reaped before giving +/// up. Bounded so a stuck child cannot stall the single-threaded daemon actor. +pub const STOP_GRACE: std::time::Duration = std::time::Duration::from_secs(10); const STDERR_LINE_BYTES: usize = 512; const STDERR_TOTAL_BYTES: usize = 4096; @@ -759,10 +808,12 @@ pub fn spawn_acp_session( let driver_stderr_capture = Arc::clone(&stderr_capture); let driver_default_model = default_model.clone(); let driver_config_overrides = config_overrides; + let driver_exit_state = exit_rx.clone(); tokio::spawn(async move { const INITIALIZE_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(60); const RPC_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(15); let agent_name = sanitize_stderr(command_for_err.trim().as_bytes()); + let exit_rx = driver_exit_state; let mut driver_exit_rx = exit_rx.clone(); let init = match tokio::time::timeout( @@ -1182,6 +1233,7 @@ pub fn spawn_acp_session( Ok(AcpHandle { cmd_tx, + exit_rx, image_capability, process, run_id, @@ -2142,9 +2194,11 @@ mod tests { #[test] fn handle_enforces_negotiated_image_capability() { let (tx, mut rx) = mpsc::unbounded_channel(); + let (_exit_tx, exit_rx) = watch::channel(ChildState::Running); let capability = Arc::new(AtomicU8::new(1)); let handle = AcpHandle { cmd_tx: tx, + exit_rx, image_capability: Arc::clone(&capability), process: test_process_guard(), run_id: 1, diff --git a/src/daemon/actor.rs b/src/daemon/actor.rs index e266775..f0d4437 100644 --- a/src/daemon/actor.rs +++ b/src/daemon/actor.rs @@ -35,6 +35,9 @@ use super::acp::{spawn_acp_session, AcpHandle, AcpUpdate, PolicyCheck}; use super::store::Store; use super::task::{Task, TaskStatus}; use super::wire as wireconv; +use super::workflow::{ + self, RunState, StageKind, StageSignal, Verdict, WorkflowOutcome, WorkflowRun, +}; use super::worktree::WorktreeManager; use crate::policies::builtins::{BlastRadiusPolicy, SpawnBoundsPolicy}; use crate::policies::registry::PolicyRegistry; @@ -146,7 +149,8 @@ fn is_acp_replay_update(update: &wire::SessionUpdate) -> bool { match update { wire::SessionUpdate::UserMessage { .. } | wire::SessionUpdate::PermissionResolved { .. } - | wire::SessionUpdate::PromptCapabilities { .. } => false, + | wire::SessionUpdate::PromptCapabilities { .. } + | wire::SessionUpdate::WorkflowEvent { .. } => false, wire::SessionUpdate::AgentText { text } => { text != "Reconnecting to the saved agent session…" && !text.starts_with("⚠ No live agent session") @@ -569,6 +573,47 @@ pub enum Command { config_overrides: std::collections::HashMap, reply: oneshot::Sender, }, + /// Create a workflow-pipeline parent task and start its first stage. + /// Unlike `CreateTask` the parent gets no agent session of its own — the + /// daemon drives stages as child tasks. + CreateWorkflowTask { + project: String, + prompt: String, + agent: String, + tags: Vec, + worktree: bool, + workflow: String, + attachments: Vec, + default_model: Option, + include_runtime_context: bool, + config_overrides: std::collections::HashMap, + reply: oneshot::Sender>, + }, + /// Soft-pause a workflow pipeline at its next stage barrier. + WorkflowPause { + task: String, + reply: oneshot::Sender>, + }, + /// Resume a paused workflow pipeline. + WorkflowResume { + task: String, + note: Option, + reply: oneshot::Sender>, + }, + /// Answer a workflow stage's pending `need_user_input` question. + WorkflowReply { + task: String, + message: String, + reply: oneshot::Sender>, + }, + /// Decide what an out-of-rounds workflow pipeline does next. + WorkflowDecide { + task: String, + decision: wire::WorkflowDecision, + rounds: Option, + note: Option, + reply: oneshot::Sender>, + }, /// Drain an orchestrator task's inbox of finished sub-agent results. ReadInbox { parent_task_id: String, @@ -576,6 +621,7 @@ pub enum Command { }, CancelTask { id: String, + reply: oneshot::Sender>, }, /// Archive a task (set status to Done, hide from live views). ArchiveTask { @@ -1015,6 +1061,19 @@ impl DaemonHandle { .await; } + /// Hard-stop a task and wait until the actor has cancelled its session. + /// For workflow parents this also stops every active stage child. + pub async fn cancel_task(&self, id: &str) -> Result<(), String> { + let (tx, rx) = oneshot::channel(); + self.send(Command::CancelTask { + id: id.to_string(), + reply: tx, + }) + .await; + rx.await + .map_err(|_| "daemon stopped before the task was cancelled".to_string())? + } + pub async fn set_task_title(&self, id: &str, title: &str) { self.send(Command::SetTaskTitle { id: id.to_string(), @@ -1495,6 +1554,9 @@ pub struct Daemon { pending_wake: std::collections::HashSet, /// Stable first-seen timestamps for streamed frames of the same tool call. tool_call_starts: HashMap<(String, String), u64>, + /// Deterministic workflow pipelines keyed by parent task id. Finished runs + /// stay in the map so their state remains visible on the board. + workflow_runs: HashMap, } impl Daemon { @@ -1596,6 +1658,7 @@ impl Daemon { orchestrator_inbox: HashMap::new(), pending_wake: std::collections::HashSet::new(), tool_call_starts, + workflow_runs: HashMap::new(), }; let handle = DaemonHandle { cmd_tx, event_tx }; @@ -1629,6 +1692,9 @@ impl Daemon { let mut daemon = daemon; daemon.orch_tx = Some(orch_cmd_tx); daemon.orch_event_rx = None; // receiver moved to forwarder task + // Bring persisted workflow pipelines back: barrier states as-is, + // mid-stage runs paused at their last barrier. + daemon.restore_workflow_runs(); tokio::spawn(daemon.run(cmd_rx, agent_rx, service_rx, pf_rx, acp_rx, policy_rx)); @@ -1893,7 +1959,7 @@ impl Daemon { Some(ev) = agent_rx.recv() => self.handle_agent_event(ev), Some(ev) = service_rx.recv() => self.handle_service_event(ev), Some(ev) = pf_rx.recv() => self.handle_pf_event(ev), - Some((task_id, update)) = acp_rx.recv() => self.handle_acp_update(task_id, update), + Some((task_id, update)) = acp_rx.recv() => self.handle_acp_update(task_id, update).await, Some(check) = policy_rx.recv() => self.handle_policy_check(check).await, _ = config_poll.tick() => self.handle_config_changes().await, } @@ -2363,6 +2429,57 @@ impl Daemon { config_overrides, ); } + Command::CreateWorkflowTask { + project, + prompt, + agent, + tags, + worktree, + workflow, + attachments, + default_model, + include_runtime_context, + config_overrides, + reply, + } => { + let result = self + .workflow_create( + project, + prompt, + agent, + tags, + worktree, + workflow, + attachments, + default_model, + include_runtime_context, + config_overrides, + ) + .await; + let _ = reply.send(result); + } + Command::WorkflowPause { task, reply } => { + let _ = reply.send(self.workflow_pause(&task)); + } + Command::WorkflowResume { task, note, reply } => { + let _ = reply.send(self.workflow_resume(&task, note).await); + } + Command::WorkflowReply { + task, + message, + reply, + } => { + let _ = reply.send(self.workflow_reply(&task, message).await); + } + Command::WorkflowDecide { + task, + decision, + rounds, + note, + reply, + } => { + let _ = reply.send(self.workflow_decide(&task, decision, rounds, note).await); + } Command::ReadInbox { parent_task_id, reply, @@ -2681,19 +2798,40 @@ impl Daemon { } } } - Command::CancelTask { id } => { - if let Some(handle) = self.sessions.remove(&id) { - handle.cancel(); - } - self.pending_permissions.cleanup_task(&id); - if let Some(task) = self.tasks.get_mut(&id) { - task.set_status(TaskStatus::Idle); - let updated = task.clone(); - self.persist(&updated); - self.emit(Event::TaskUpdated(updated)); - } + Command::CancelTask { id, reply } => { + let result = if self.workflow_is_active(&id) { + // Stopping a workflow parent stops the whole pipeline. + self.workflow_finalize(&id, WorkflowOutcome::Stopped).await + } else { + let stop_result = match self.sessions.remove(&id) { + Some(handle) => handle.cancel_and_wait().await, + None => Ok(()), + }; + self.pending_permissions.cleanup_task(&id); + // A finished pipeline's parent keeps its terminal status: + // cancelling it must not rewrite NeedsReview back to Idle. + let finished_workflow = self + .workflow_runs + .get(&id) + .is_some_and(|run| !run.is_active()); + if let Some(task) = self.tasks.get_mut(&id).filter(|_| !finished_workflow) { + task.set_status(TaskStatus::Idle); + let updated = task.clone(); + self.persist(&updated); + self.emit(Event::TaskUpdated(updated)); + } + // Cancelling a stage child mid-run fails that stage. + self.workflow_child_gone(&id).await; + stop_result + }; + let _ = reply.send(result); } Command::ArchiveTask { id } => { + // Archiving a live workflow parent stops the pipeline first so + // no orphaned stage sessions keep running behind a Done task. + if self.workflow_is_active(&id) { + let _ = self.workflow_finalize(&id, WorkflowOutcome::Stopped).await; + } // Collect children that reference this task as parent so we // can archive them together with the leader. let child_ids: Vec = self @@ -2722,6 +2860,14 @@ impl Daemon { } } Command::DeleteTask { id } => { + if self.workflow_is_active(&id) { + let _ = self.workflow_finalize(&id, WorkflowOutcome::Stopped).await; + } + if self.workflow_runs.remove(&id).is_some() { + if let Some(store) = &self.store { + let _ = store.delete_workflow_run(&id); + } + } if let Some(handle) = self.sessions.remove(&id) { handle.cancel(); } @@ -2742,7 +2888,9 @@ impl Daemon { if let Some(store) = &self.store { let _ = store.delete_task(&id); } - self.emit(Event::TaskRemoved { id }); + self.emit(Event::TaskRemoved { id: id.clone() }); + // Deleting a stage child mid-run fails that stage. + self.workflow_child_gone(&id).await; } } Command::SetTaskTitle { id, title } => { @@ -3261,7 +3409,7 @@ impl Daemon { let entry = crate::registry::add_project(path, name).map_err(|e| format!("registry: {e}"))?; - // Generate .warpforge.yaml if none exists. + // Generate the workspace config if none exists. let config_file = crate::config::find_config_file(std::path::Path::new(&entry.path)); if !config_file.exists() { crate::config::generate_workspace_yaml(std::path::Path::new(&entry.path)).ok(); @@ -3662,9 +3810,16 @@ impl Daemon { } } - fn handle_acp_update(&mut self, task_id: String, update: AcpUpdate) { + async fn handle_acp_update(&mut self, task_id: String, update: AcpUpdate) { match update { AcpUpdate::SessionStarted { session_id } => { + // A hard stop removes the handle before awaiting process exit. + // Ignore an initialize reply that was already queued behind + // that stop; otherwise it would resurrect the cancelled child + // task as Running after task.cancel was acknowledged. + if !self.sessions.contains_key(&task_id) { + return; + } if let Some(task) = self.tasks.get_mut(&task_id) { task.attach_session(session_id); let updated = task.clone(); @@ -3777,6 +3932,7 @@ impl Daemon { // A clean turn end completes the node; a "disconnected" stop is // the agent process dying, which we treat as a failure. let success = stop_reason != "disconnected"; + let workflow_child = self.workflow_child_of(&task_id).is_some(); let update = wire::SessionUpdate::TurnEnded { stop_reason }; if self.should_skip_resume_replay(&task_id, &update) { return; @@ -3785,24 +3941,40 @@ impl Daemon { // Turn over: only NeedsReview if there are actually changes to // review; a pure Q&A turn goes Idle (waiting for the next // message) instead of falsely demanding a review. - if let Some(task) = self.tasks.get_mut(&task_id) { - if task.status == TaskStatus::Running { - let next = if task.files_changed > 0 { - TaskStatus::NeedsReview - } else { - TaskStatus::Idle - }; - task.set_status(next); - let updated = task.clone(); - self.persist(&updated); - self.emit(Event::TaskUpdated(updated)); + // + // Workflow children have different semantics: their output is + // consumed by the pipeline, so the workflow handler below owns + // their terminal/waiting status. File edits made by an + // implement/fix stage must not leak into NeedsReview here. + if !workflow_child { + if let Some(task) = self.tasks.get_mut(&task_id) { + if task.status == TaskStatus::Running { + let next = if task.files_changed > 0 { + TaskStatus::NeedsReview + } else { + TaskStatus::Idle + }; + task.set_status(next); + let updated = task.clone(); + self.persist(&updated); + self.emit(Event::TaskUpdated(updated)); + } } } let output = self.collect_agent_text(&task_id); self.notify_orch_finished(&task_id, success, output.clone()); - // Deliver to a parent if this was a sub-agent; and drain our own - // inbox if we are a parent that just went idle. - self.deliver_child_result(&task_id, success, output); + if workflow_child { + // A workflow stage finished — advance the pipeline. Parse + // only the latest turn's text: answered questions and + // superseded verdicts from earlier turns must not count. + // The legacy orchestrator inbox path does not apply here. + let text = self.collect_stage_text(&task_id); + self.workflow_stage_finished(&task_id, success, text).await; + } else { + // Deliver to a parent if this was a sub-agent; and drain our + // own inbox if we are a parent that just went idle. + self.deliver_child_result(&task_id, success, output); + } // If we are an orchestrator whose sub-agents finished mid-turn, // process them now that the turn is over. if self.pending_wake.remove(&task_id) { @@ -3829,7 +4001,19 @@ impl Daemon { self.emit(Event::TaskUpdated(updated)); } self.notify_orch_finished(&task_id, false, reason.clone()); - self.deliver_child_result(&task_id, false, reason); + if self.workflow_child_of(&task_id).is_some() { + self.workflow_stage_finished( + &task_id, + false, + StageText { + closing: reason.clone(), + full: reason, + }, + ) + .await; + } else { + self.deliver_child_result(&task_id, false, reason); + } } } } @@ -3928,6 +4112,58 @@ impl Daemon { .join("") } + /// Like [`collect_agent_text`], but only the text streamed since the last + /// user message — i.e. the output of the task's latest turn. The workflow + /// engine parses this: a `need_user_input` block answered two turns ago + /// must not be mistaken for a fresh question. + fn collect_last_turn_text(&self, task_id: &str) -> String { + self.collect_stage_text(task_id).full + } + + /// A finished stage's output, in the two shapes the pipeline needs. + /// + /// `closing` is the agent's final message — the text streamed after its + /// last tool call or file edit. That is what the stage prompts ask for + /// ("your final message should summarize what you did") and what a human + /// reads as the result, so it is what reviewers and fixers are handed. + /// `full` is every chunk of the turn, kept as a parsing fallback for an + /// agent that emits its protocol block before a trailing tool call. + fn collect_stage_text(&self, task_id: &str) -> StageText { + let Some(updates) = self + .store + .as_ref() + .and_then(|s| s.load_session_updates(task_id).ok()) + else { + return StageText::default(); + }; + let mut full: Vec = Vec::new(); + let mut closing: Vec = Vec::new(); + for update in updates { + match update { + // A new user message starts a fresh turn. + wire::SessionUpdate::UserMessage { .. } => { + full.clear(); + closing.clear(); + } + wire::SessionUpdate::AgentText { text } => { + full.push(text.clone()); + closing.push(text); + } + // Any work the agent does ends whatever it was narrating, so + // the closing message restarts after it. + wire::SessionUpdate::ToolCall { .. } + | wire::SessionUpdate::FileEdit { .. } + | wire::SessionUpdate::AgentThought { .. } + | wire::SessionUpdate::Plan { .. } => closing.clear(), + _ => {} + } + } + StageText { + closing: closing.join(""), + full: full.join(""), + } + } + /// Tell the orchestrator a dispatched task finished. No-op unless the task /// carries the "orchestrator" tag and an orchestrator is wired. fn notify_orch_finished(&self, task_id: &str, success: bool, result: String) { @@ -4145,6 +4381,1451 @@ impl Daemon { } } +// ─── Workflow pipeline engine (actor glue) ─────────────────────────────────── +// +// The deterministic `plan? → implement → review ⇄ fix` pipeline. Pure logic +// (state container, prompt building, verdict parsing, review merging) lives in +// `daemon/workflow.rs`; these methods are the side-effectful glue: they spawn +// stage child tasks, react to their turn ends, and narrate progress into the +// parent task's transcript. +// +// Borrow discipline: methods that mutate a run *and* call `&mut self` helpers +// temporarily remove the run from `workflow_runs` and re-insert it (the +// take/put pattern). Every exit path must re-insert. +impl Daemon { + fn workflow_is_active(&self, task_id: &str) -> bool { + self.workflow_runs + .get(task_id) + .is_some_and(WorkflowRun::is_active) + } + + /// The parent task id of an *active* pipeline this child belongs to. + /// Searches the runs (not the tasks map) so it also works for tasks that + /// were just removed. + fn workflow_child_of(&self, child_id: &str) -> Option { + self.workflow_runs + .values() + .find(|run| { + run.is_active() + && (run.active_children.contains_key(child_id) + || run.review_pending.contains_key(child_id)) + }) + .map(|run| run.parent_id.clone()) + } + + /// Deliver a follow-up into an existing stage session. Returns false when + /// the session is gone — checking `is_alive()` matters because + /// `AcpHandle::prompt` succeeds even for a dead child (its channel belongs + /// to the driver task, which outlives the process), and a prompt sent into + /// a corpse simply vanishes. + fn workflow_followup(&mut self, child_id: &str, text: String) -> bool { + let delivered = self + .sessions + .get(child_id) + .filter(|handle| handle.is_alive()) + .map(|handle| { + handle + .prompt(super::prompt::PreparedPrompt { + content: vec![super::prompt::PromptContent::Text(text.clone())], + summaries: vec![], + has_images: false, + }) + .is_ok() + }) + .unwrap_or(false); + if delivered { + self.emit_session( + child_id, + wire::SessionUpdate::UserMessage { + text, + attachments: vec![], + }, + ); + } + delivered + } + + /// Append one durable, independently rendered workflow entry to the + /// parent's Conversation. Structured events deliberately do not use + /// AgentText: transport coalescing is correct for streamed agent chunks, + /// but would glue unrelated workflow transitions into one Markdown blob. + #[allow(clippy::too_many_arguments)] + fn workflow_event( + &self, + parent_id: &str, + event: wire::WorkflowEventKind, + title: impl Into, + detail: Option, + stage: Option, + agents: Vec, + tone: wire::WorkflowEventTone, + ) { + self.emit_session( + parent_id, + wire::SessionUpdate::WorkflowEvent { + event, + title: title.into(), + detail, + stage: stage.map(StageKind::wire), + agents, + tone, + }, + ); + } + + /// Convenience wrapper for transitions that do not reference a particular + /// agent. Split the first paragraph into the card title and keep the rest + /// as Markdown detail. + fn workflow_timeline(&self, parent_id: &str, text: impl Into) { + let text = text.into(); + let text = text.trim(); + let (heading, detail) = text + .split_once("\n\n") + .map(|(heading, detail)| (heading, Some(detail.trim().to_string()))) + .unwrap_or((text, None)); + let title = heading.trim_start_matches('#').trim().replace("**", ""); + let lower = text.to_ascii_lowercase(); + let tone = if lower.contains("failed") || lower.contains("stopped") { + wire::WorkflowEventTone::Error + } else if lower.contains("changes requested") + || lower.contains("limit reached") + || lower.contains("needs your input") + { + wire::WorkflowEventTone::Warning + } else if lower.contains("approved") || lower.contains("finished") { + wire::WorkflowEventTone::Success + } else { + wire::WorkflowEventTone::Info + }; + let event = if lower.starts_with("workflow") + && (lower.contains("finished") || lower.contains("failed") || lower.contains("stopped")) + { + wire::WorkflowEventKind::WorkflowFinished + } else { + wire::WorkflowEventKind::Status + }; + self.workflow_event(parent_id, event, title, detail, None, Vec::new(), tone); + } + + /// Sync the parent task's `workflow_run` + `orchestration_graph` + /// projections from the run, persist, and broadcast; also persist the run. + fn workflow_sync(&mut self, run: &WorkflowRun) { + if let Some(task) = self.tasks.get_mut(&run.parent_id) { + // The coarse task status describes whether work is executing, while + // workflow_run.waiting carries the precise barrier reason. + let active_status = match run.state { + RunState::Running { .. } => Some(TaskStatus::Running), + RunState::AwaitingReply { .. } + | RunState::AwaitingLimitDecision + | RunState::Paused { .. } => Some(TaskStatus::Idle), + RunState::Done | RunState::Failed => None, + }; + if let Some(status) = active_status { + if task.status != status { + task.set_status(status); + } + } + task.workflow_run = Some(run.wire_info()); + task.orchestration_graph = Some(run.graph_info()); + task.updated_at = super::task::now_secs(); + let updated = task.clone(); + self.persist(&updated); + self.emit(Event::TaskUpdated(updated)); + } + if let Some(store) = &self.store { + if let Ok(json) = serde_json::to_string(run) { + let _ = store.save_workflow_run(&run.parent_id, &json); + } + } + } + + /// Workflow stage tasks are execution records, not independent changes to + /// review. Keep their lifecycle aligned with the stage state and emit the + /// update immediately so Board/Subtasks never retain a stale generic + /// turn-end status. + fn workflow_set_child_status(&mut self, child_id: &str, status: TaskStatus) { + if let Some(task) = self.tasks.get_mut(child_id) { + if task.status == status { + return; + } + task.set_status(status); + let updated = task.clone(); + self.persist(&updated); + self.emit(Event::TaskUpdated(updated)); + } + } + + /// `CreateWorkflowTask`: validate the workflow, create the parent task + /// (without an agent session), and start the first stage. + #[allow(clippy::too_many_arguments)] + async fn workflow_create( + &mut self, + project: String, + prompt: String, + agent: String, + tags: Vec, + use_worktree: bool, + workflow_id: String, + attachments: Vec, + default_model: Option, + include_runtime_context: bool, + config_overrides: HashMap, + ) -> Result { + let path = self + .project_path(&project) + .ok_or_else(|| format!("unknown project '{project}'"))?; + if self.store.is_none() { + // Stage results are read back out of the persisted transcript, so + // without a store every stage would look like it produced nothing. + return Err( + "workflows need the local database, which failed to open — check \ + ~/.warpforge and restart the daemon" + .to_string(), + ); + } + let loaded = + crate::workflow_config::load_workflow(std::path::Path::new(&path), &workflow_id) + .ok_or_else(|| format!("unknown workflow `{workflow_id}`"))?; + let warnings = loaded.warnings.clone(); + let spec = loaded + .spec + .map_err(|e| format!("workflow `{workflow_id}` is invalid: {e}"))?; + + let mut tags = tags; + tags.push(format!("workflow:{workflow_id}")); + let mut task = Task::new(&project, &prompt, &agent, tags); + if use_worktree { + let wt_mgr = self + .worktrees + .entry(project.clone()) + .or_insert_with(|| WorktreeManager::new(std::path::PathBuf::from(&path))); + match wt_mgr.create(&task.id, None).await { + Ok(wt) => task.worktree = Some(wt.path.to_string_lossy().to_string()), + Err(e) => eprintln!("[daemon] worktree creation failed: {e}"), + } + } + // The parent is "running" for the whole life of the pipeline. + task.set_status(TaskStatus::Running); + let resolved_model = default_model.or_else(|| { + self.configured_agents + .iter() + .find(|a| a.id == agent) + .and_then(|a| a.last_model.clone()) + }); + let parent_id = task.id.clone(); + self.tasks.insert(parent_id.clone(), task.clone()); + self.persist(&task); + self.emit(Event::TaskCreated(task)); + + let run = WorkflowRun::new( + parent_id.clone(), + project, + spec, + agent, + resolved_model, + attachments, + include_runtime_context, + config_overrides, + ); + self.workflow_event( + &parent_id, + wire::WorkflowEventKind::WorkflowStarted, + format!("Workflow started: {}", run.spec.name), + Some({ + let mut detail = format!( + "**Stages:** {} \n**Review limit:** {} round(s)", + run.spec.stage_summary().join(" → "), + run.effective_max_rounds(), + ); + // Warnings are otherwise only visible as a picker tooltip, so a + // clamped limit or an ignored key would silently shape the run. + if !warnings.is_empty() { + detail.push_str("\n\n**Workflow file warnings:**\n"); + for warning in &warnings { + detail.push_str(&format!("- {warning}\n")); + } + } + detail + }), + None, + Vec::new(), + wire::WorkflowEventTone::Info, + ); + let first = run.first_stage(); + self.workflow_runs.insert(parent_id.clone(), run); + self.workflow_spawn_stage(&parent_id, first).await; + Ok(parent_id) + } + + /// Spawn the child task(s) for a stage and mark the run as running it. + async fn workflow_spawn_stage(&mut self, parent_id: &str, stage: StageKind) { + let Some(mut run) = self.workflow_runs.remove(parent_id) else { + return; + }; + let Some(parent) = self.tasks.get(parent_id) else { + self.workflow_runs.insert(parent_id.to_string(), run); + return; + }; + let parent_prompt = parent.prompt.clone(); + let parent_title = parent.title.clone(); + let worktree = parent.worktree.clone(); + let project = run.project.clone(); + + if stage == StageKind::Review { + run.round += 1; + } + let guidance = match stage { + StageKind::Review => None, + _ => run.take_guidance(), + }; + // Review and fix stages see the current working-copy diff. + let diff = match stage { + StageKind::Review | StageKind::Fix => { + let dir = worktree.clone().or_else(|| self.project_path(&project)); + match dir { + Some(dir) => match super::diff::working_diff(&dir).await { + Ok(files) => Some(workflow::format_diff(&files)), + Err(e) => Some(format!("(diff unavailable: {e})")), + }, + None => None, + } + } + _ => None, + }; + let ctx = workflow::PromptCtx { + task_prompt: parent_prompt, + plan: run.plan_output.clone(), + implementer_summary: run.last_summary.as_deref().map(workflow::clip_summary), + diff, + findings: match stage { + StageKind::Fix => Some(workflow::format_findings(&run.open_findings)), + _ => None, + }, + prior_findings: match stage { + // On a repeat round `open_findings` still holds what the last + // review raised — the next reviewers must verify each item. + StageKind::Review if run.round > 1 && !run.open_findings.is_empty() => { + Some(workflow::format_findings(&run.open_findings)) + } + _ => None, + }, + round: run.round, + max_rounds: run.effective_max_rounds(), + guidance, + }; + // The dialog's attachments ride along with the very first stage only. + let attachments = if run.history.is_empty() { + std::mem::take(&mut run.attachments) + } else { + Vec::new() + }; + + run.state = RunState::Running { stage }; + match stage { + StageKind::Review => { + run.review_pending.clear(); + run.review_collected.clear(); + run.reasked.clear(); + let round_label = format!("round {}/{}", run.round, run.effective_max_rounds()); + // Repeat rounds follow up in the previous reviewers' live + // sessions (review.reask: same_session, the default): the + // reviewer remembers its own findings and verifies each one + // instead of re-reviewing from scratch. A dead session falls + // back to a fresh spawn whose prompt carries those findings. + let reuse_sessions = run.round > 1 + && run.spec.review.reask == crate::workflow_config::ReaskMode::SameSession; + let mut event_agents = Vec::with_capacity(run.spec.review.reviewers.len()); + let mut reused = 0usize; + for index in 0..run.spec.review.reviewers.len() { + let (agent, model) = run.stage_agent(stage, Some(index)); + let label = run.reviewer_label(index); + if reuse_sessions { + if let Some(prior_id) = run.prior_review_children.get(&index).cloned() { + let followup = workflow::build_rereview_prompt(&ctx); + if self.workflow_followup(&prior_id, followup) { + // The child was parked Done after its verdict; + // the generic mark_task_running refuses Done + // tasks, so flip it explicitly. + self.workflow_set_child_status(&prior_id, TaskStatus::Running); + run.review_pending.insert(prior_id.clone(), index); + run.active_children.insert(prior_id.clone(), stage); + run.record_stage( + stage, + &prior_id, + &agent, + format!("{label}, {round_label}"), + ); + event_agents.push(wire::WorkflowEventAgent { + task_id: prior_id, + label, + agent, + model, + }); + reused += 1; + continue; + } + } + } + let prompt = workflow::build_reviewer_prompt(&run.spec, index, &ctx); + let spawned = self.workflow_spawn_child( + &run.project, + parent_id, + &agent, + model.clone(), + prompt, + worktree.clone(), + format!("review · {parent_title}"), + Vec::new(), + run.include_runtime_context, + run.config_overrides.clone(), + ); + // A reviewer whose session never started is recorded as a + // failed node and excluded, exactly like one that dies + // mid-review — it must not sit in `review_pending` waiting + // for a TurnEnded that can never arrive. + let (child_id, started) = match spawned { + Ok(id) => (id, true), + Err(id) => (id, false), + }; + run.record_stage(stage, &child_id, &agent, format!("{label}, {round_label}")); + if started { + run.review_pending.insert(child_id.clone(), index); + run.active_children.insert(child_id.clone(), stage); + } else { + run.set_record_status(&child_id, wire::OrchNodeStatus::Failed); + } + event_agents.push(wire::WorkflowEventAgent { + task_id: child_id, + label, + agent, + model, + }); + } + // Remember this round's staffing for the next reask. + run.prior_review_children = run + .review_pending + .iter() + .map(|(child, index)| (*index, child.clone())) + .collect(); + let detail = if reused > 0 { + format!( + "{} reviewer(s) running; {reused} continuing their previous session to \ + verify their own findings.", + run.review_pending.len() + ) + } else { + format!("{} reviewer(s) running.", run.review_pending.len()) + }; + self.workflow_event( + parent_id, + wire::WorkflowEventKind::StageStarted, + format!("Review {round_label} started"), + Some(detail), + Some(stage), + event_agents, + wire::WorkflowEventTone::Running, + ); + } + _ => { + let (agent, model) = run.stage_agent(stage, None); + let prompt = match stage { + StageKind::Plan => workflow::build_plan_prompt(&run.spec, &ctx), + StageKind::Implement => workflow::build_implement_prompt(&run.spec, &ctx), + StageKind::Fix => workflow::build_fix_prompt(&run.spec, &ctx), + StageKind::Review => unreachable!(), + }; + let spawned = self.workflow_spawn_child( + &run.project, + parent_id, + &agent, + model.clone(), + prompt, + worktree.clone(), + format!("{} · {parent_title}", stage.label()), + attachments, + run.include_runtime_context, + run.config_overrides.clone(), + ); + let label = match stage { + StageKind::Fix => format!("{} (round {})", stage.label(), run.round), + _ => stage.label().to_string(), + }; + let child_id = match spawned { + Ok(id) => { + run.active_children.insert(id.clone(), stage); + run.record_stage(stage, &id, &agent, label.clone()); + id + } + Err(id) => { + // No session means no TurnEnded will ever arrive, so + // fail the pipeline here instead of hanging in + // "running" until the user cancels. + run.record_stage(stage, &id, &agent, label.clone()); + run.set_record_status(&id, wire::OrchNodeStatus::Failed); + let reason = self + .tasks + .get(&id) + .and_then(|t| t.blocked_reason.clone()) + .unwrap_or_else(|| { + "the agent session could not be started".to_string() + }); + self.workflow_runs.insert(parent_id.to_string(), run); + let _ = self + .workflow_finalize( + parent_id, + WorkflowOutcome::Error(format!( + "stage {} could not start: {reason}", + stage.label() + )), + ) + .await; + return; + } + }; + self.workflow_event( + parent_id, + wire::WorkflowEventKind::StageStarted, + format!("{} started", stage.title()), + None, + Some(stage), + vec![wire::WorkflowEventAgent { + task_id: child_id, + label, + agent, + model, + }], + wire::WorkflowEventTone::Running, + ); + } + } + self.workflow_sync(&run); + self.workflow_runs.insert(parent_id.to_string(), run); + } + + /// Create and start one stage child task. Children run in the parent's + /// directory (its worktree when isolated) but are NOT registered in the + /// worktree manager — the parent owns the worktree's lifecycle. + #[allow(clippy::too_many_arguments)] + #[allow(clippy::result_large_err)] + fn workflow_spawn_child( + &mut self, + project: &str, + parent_id: &str, + agent: &str, + model: Option, + prompt: String, + worktree: Option, + title: String, + attachments: Vec, + include_runtime_context: bool, + config_overrides: HashMap, + ) -> Result { + let mut task = Task::new(project, &prompt, agent, vec!["workflow-stage".to_string()]); + task.parent_task_id = Some(parent_id.to_string()); + task.worktree = worktree; + task.title = title; + let child_id = task.id.clone(); + self.tasks.insert(child_id.clone(), task.clone()); + self.persist(&task); + self.emit(Event::TaskCreated(task)); + self.start_session( + &child_id, + project, + agent, + &prompt, + include_runtime_context, + None, + attachments, + model, + config_overrides, + ); + // `start_session` reports prompt-preparation and spawn failures by + // blocking the child task and inserting no handle. Without a session + // there is no TurnEnded to advance the pipeline, so the caller must + // learn about it here or the parent hangs in "running" forever. + if self.sessions.contains_key(&child_id) { + Ok(child_id) + } else { + Err(child_id) + } + } + + /// A stage child's turn ended (or its session died). Advance the pipeline. + async fn workflow_stage_finished(&mut self, child_id: &str, success: bool, text: StageText) { + // Prefer the closing message: it is the agent's actual result, and + // reading it instead of the whole turn means a JSON block quoted + // mid-turn (while browsing a config file, say) cannot be mistaken for + // the protocol payload. Fall back to the full turn only when the + // payload genuinely is not in the closing message — an agent that + // emitted its block and then made one last tool call. + let closing_is_usable = !text.closing.trim().is_empty() + && (workflow::has_protocol_payload(&text.closing) + || !workflow::has_protocol_payload(&text.full)); + let output = if closing_is_usable { + text.closing.clone() + } else { + text.full.clone() + }; + let Some(parent_id) = self.workflow_child_of(child_id) else { + return; + }; + let Some(mut run) = self.workflow_runs.remove(&parent_id) else { + return; + }; + let stage = match run.active_children.get(child_id) { + Some(stage) => *stage, + None => { + self.workflow_runs.insert(parent_id.clone(), run); + return; + } + }; + // Only a running stage advances the pipeline: a turn that ends while + // we await a reply is the answered child continuing, handled below. + let running_stage = matches!(run.state, RunState::Running { stage: s } if s == stage); + let awaiting_this_child = + matches!(&run.state, RunState::AwaitingReply { child, .. } if child == child_id); + if !running_stage && !awaiting_this_child { + self.workflow_runs.insert(parent_id.clone(), run); + return; + } + + if !success { + self.workflow_child_failed(&parent_id, run, child_id, stage) + .await; + return; + } + + match stage { + StageKind::Review => { + self.workflow_review_finished(&parent_id, run, child_id, output) + .await; + } + StageKind::Plan | StageKind::Implement | StageKind::Fix => { + match workflow::parse_stage_signal(&output) { + StageSignal::Question(question) => { + run.state = RunState::AwaitingReply { + stage, + child: child_id.to_string(), + question: question.clone(), + }; + self.workflow_set_child_status(child_id, TaskStatus::Idle); + let event_agent = run + .history + .iter() + .rev() + .find(|record| record.task_id == child_id) + .map(|record| wire::WorkflowEventAgent { + task_id: record.task_id.clone(), + label: record.label.clone(), + agent: record.agent.clone(), + model: run.stage_agent(stage, None).1, + }); + self.workflow_event( + &parent_id, + wire::WorkflowEventKind::AgentOutput, + format!("{} needs your input", stage.title()), + Some(workflow::display_output(&output)), + Some(stage), + event_agent.into_iter().collect(), + wire::WorkflowEventTone::Warning, + ); + self.workflow_sync(&run); + self.workflow_runs.insert(parent_id.clone(), run); + } + StageSignal::Output => { + run.active_children.remove(child_id); + run.set_record_status(child_id, wire::OrchNodeStatus::Complete); + self.workflow_set_child_status(child_id, TaskStatus::Done); + let event_agent = run + .history + .iter() + .rev() + .find(|record| record.task_id == child_id) + .map(|record| wire::WorkflowEventAgent { + task_id: record.task_id.clone(), + label: record.label.clone(), + agent: record.agent.clone(), + model: run.stage_agent(stage, None).1, + }); + match stage { + StageKind::Plan => run.plan_output = Some(output.clone()), + _ => run.last_summary = Some(output.clone()), + } + self.workflow_event( + &parent_id, + wire::WorkflowEventKind::AgentOutput, + format!("{} completed", stage.title()), + Some(workflow::display_output(&output)), + Some(stage), + event_agent.into_iter().collect(), + wire::WorkflowEventTone::Success, + ); + let next = stage.successor().unwrap_or(StageKind::Review); + self.workflow_runs.insert(parent_id.clone(), run); + self.workflow_advance(&parent_id, next).await; + } + } + } + } + } + + /// A non-review stage child failed, or a reviewer died. Reviewers are + /// excluded from the verdict; any other stage failure fails the pipeline. + async fn workflow_child_failed( + &mut self, + parent_id: &str, + mut run: WorkflowRun, + child_id: &str, + stage: StageKind, + ) { + run.set_record_status(child_id, wire::OrchNodeStatus::Failed); + if stage == StageKind::Review { + let index = run.review_pending.remove(child_id); + run.active_children.remove(child_id); + run.reasked.remove(child_id); + let label = index + .map(|i| run.reviewer_label(i)) + .unwrap_or_else(|| "reviewer".to_string()); + let event_agent = run + .history + .iter() + .rev() + .find(|record| record.task_id == child_id) + .map(|record| wire::WorkflowEventAgent { + task_id: record.task_id.clone(), + label: record.label.clone(), + agent: record.agent.clone(), + model: index.and_then(|i| run.stage_agent(StageKind::Review, Some(i)).1), + }); + self.workflow_event( + parent_id, + wire::WorkflowEventKind::AgentOutput, + format!("{label} failed"), + Some("Excluded from this round's verdict.".to_string()), + Some(stage), + event_agent.into_iter().collect(), + wire::WorkflowEventTone::Error, + ); + if run.review_pending.is_empty() { + if run.review_collected.is_empty() { + self.workflow_runs.insert(parent_id.to_string(), run); + let _ = self + .workflow_finalize( + parent_id, + WorkflowOutcome::Error("all reviewers failed".to_string()), + ) + .await; + } else { + self.workflow_merge_reviews(parent_id, run).await; + } + } else { + self.workflow_sync(&run); + self.workflow_runs.insert(parent_id.to_string(), run); + } + return; + } + let reason = self + .tasks + .get(child_id) + .and_then(|t| t.blocked_reason.clone()) + .unwrap_or_else(|| "agent session ended unexpectedly".to_string()); + let event_agent = run + .history + .iter() + .rev() + .find(|record| record.task_id == child_id) + .map(|record| wire::WorkflowEventAgent { + task_id: record.task_id.clone(), + label: record.label.clone(), + agent: record.agent.clone(), + model: run.stage_agent(stage, None).1, + }); + self.workflow_event( + parent_id, + wire::WorkflowEventKind::AgentOutput, + format!("{} failed", stage.title()), + Some(reason.clone()), + Some(stage), + event_agent.into_iter().collect(), + wire::WorkflowEventTone::Error, + ); + self.workflow_runs.insert(parent_id.to_string(), run); + let _ = self + .workflow_finalize( + parent_id, + WorkflowOutcome::Error(format!("stage {} failed: {reason}", stage.label())), + ) + .await; + } + + /// One reviewer's turn ended: parse its verdict, re-ask once on garbage, + /// and merge the round when every reviewer has resolved. + async fn workflow_review_finished( + &mut self, + parent_id: &str, + mut run: WorkflowRun, + child_id: &str, + output: String, + ) { + let Some(index) = run.review_pending.get(child_id).copied() else { + self.workflow_runs.insert(parent_id.to_string(), run); + return; + }; + let label = run.reviewer_label(index); + match workflow::parse_review_verdict(&output, &label) { + Ok((verdict, findings)) => { + self.workflow_set_child_status(child_id, TaskStatus::Done); + let event_agent = run + .history + .iter() + .rev() + .find(|record| record.task_id == child_id) + .map(|record| wire::WorkflowEventAgent { + task_id: record.task_id.clone(), + label: record.label.clone(), + agent: record.agent.clone(), + model: run.stage_agent(StageKind::Review, Some(index)).1, + }); + self.workflow_event( + parent_id, + wire::WorkflowEventKind::ReviewResult, + format!( + "{label}: {}", + match verdict { + Verdict::Approve => "approved", + Verdict::RequestChanges => "changes requested", + }, + ), + Some(workflow::display_output(&output)), + Some(StageKind::Review), + event_agent.into_iter().collect(), + match verdict { + Verdict::Approve => wire::WorkflowEventTone::Success, + Verdict::RequestChanges => wire::WorkflowEventTone::Warning, + }, + ); + run.review_pending.remove(child_id); + run.active_children.remove(child_id); + run.set_record_status(child_id, wire::OrchNodeStatus::Complete); + run.review_collected.push((index, verdict, findings)); + if run.review_pending.is_empty() { + self.workflow_merge_reviews(parent_id, run).await; + } else { + self.workflow_sync(&run); + self.workflow_runs.insert(parent_id.to_string(), run); + } + } + Err(reason) => { + let event_agent = run + .history + .iter() + .rev() + .find(|record| record.task_id == child_id) + .map(|record| wire::WorkflowEventAgent { + task_id: record.task_id.clone(), + label: record.label.clone(), + agent: record.agent.clone(), + model: run.stage_agent(StageKind::Review, Some(index)).1, + }); + self.workflow_event( + parent_id, + wire::WorkflowEventKind::AgentOutput, + format!("{label}: invalid review response"), + Some(workflow::display_output(&output)), + Some(StageKind::Review), + event_agent.into_iter().collect(), + wire::WorkflowEventTone::Warning, + ); + let asked = run.reasked.entry(child_id.to_string()).or_insert(0); + if *asked < workflow::MAX_VERDICT_REASKS { + *asked += 1; + let reask = workflow::reask_verdict_prompt(&reason); + if self.workflow_followup(child_id, reask) { + self.mark_task_running(child_id); + self.workflow_timeline( + parent_id, + format!("{label} returned no parseable verdict — asking again."), + ); + self.workflow_runs.insert(parent_id.to_string(), run); + return; + } + // Dead session: fall through to the failure path. + } + self.workflow_runs.insert(parent_id.to_string(), run); + // Treat a reviewer that cannot produce a parseable verdict the + // same way as one whose process died: abstain from this round. + // Failing the whole pipeline because one agent wrote prose + // twice would throw away a complete implementation. + self.workflow_event( + parent_id, + wire::WorkflowEventKind::AgentOutput, + format!("{label} abstained"), + Some(format!( + "No parseable verdict after a retry ({reason}) — excluded from this \ + round's verdict." + )), + Some(StageKind::Review), + Vec::new(), + wire::WorkflowEventTone::Warning, + ); + let Some(mut run) = self.workflow_runs.remove(parent_id) else { + return; + }; + run.review_pending.remove(child_id); + run.active_children.remove(child_id); + run.reasked.remove(child_id); + run.set_record_status(child_id, wire::OrchNodeStatus::Failed); + self.workflow_set_child_status(child_id, TaskStatus::Idle); + if run.review_pending.is_empty() { + if run.review_collected.is_empty() { + self.workflow_runs.insert(parent_id.to_string(), run); + let _ = self + .workflow_finalize( + parent_id, + WorkflowOutcome::Error( + "no reviewer produced a usable verdict".to_string(), + ), + ) + .await; + } else { + self.workflow_merge_reviews(parent_id, run).await; + } + } else { + self.workflow_sync(&run); + self.workflow_runs.insert(parent_id.to_string(), run); + } + } + } + } + + /// All reviewers of a round resolved: merge, then approve / fix / limit. + async fn workflow_merge_reviews(&mut self, parent_id: &str, mut run: WorkflowRun) { + let (verdict, findings) = workflow::merge_reviews(&run.review_collected); + run.review_collected.clear(); + run.last_verdict = Some(verdict); + let (to_fix, low): (Vec<_>, Vec<_>) = + findings.into_iter().partition(|f| f.severity.goes_to_fix()); + run.deferred_findings.extend(low); + match verdict { + Verdict::Approve => { + self.workflow_timeline( + parent_id, + format!("Review round {}: **approved**.", run.round), + ); + run.open_findings.clear(); + self.workflow_runs.insert(parent_id.to_string(), run); + let _ = self + .workflow_finalize(parent_id, WorkflowOutcome::Success { limit_hit: false }) + .await; + } + Verdict::RequestChanges if to_fix.is_empty() => { + // Changes requested but every finding is low-severity — there + // is nothing for the fixer to do. Finish with notes. + self.workflow_timeline( + parent_id, + format!( + "Review round {}: changes requested, but only low-severity notes remain — finishing.", + run.round + ), + ); + run.open_findings.clear(); + self.workflow_runs.insert(parent_id.to_string(), run); + let _ = self + .workflow_finalize(parent_id, WorkflowOutcome::Success { limit_hit: false }) + .await; + } + Verdict::RequestChanges => { + run.open_findings = to_fix; + self.workflow_timeline( + parent_id, + format!( + "Review round {}: **changes requested** — {}.\n\n{}", + run.round, + workflow::summarize_findings(&run.open_findings), + workflow::format_findings(&run.open_findings), + ), + ); + if run.round < run.effective_max_rounds() { + self.workflow_runs.insert(parent_id.to_string(), run); + self.workflow_advance(parent_id, StageKind::Fix).await; + } else { + match run.spec.review.on_limit { + crate::workflow_config::OnLimit::Ask => { + run.state = RunState::AwaitingLimitDecision; + self.workflow_timeline( + parent_id, + format!( + "Review limit reached ({} rounds) with {}. What next — extend \ + the rounds, finish as is, or stop? You can add guidance for \ + the next fix attempt.", + run.effective_max_rounds(), + workflow::summarize_findings(&run.open_findings), + ), + ); + self.workflow_sync(&run); + self.workflow_runs.insert(parent_id.to_string(), run); + } + crate::workflow_config::OnLimit::Finish => { + self.workflow_runs.insert(parent_id.to_string(), run); + let _ = self + .workflow_finalize( + parent_id, + WorkflowOutcome::Success { limit_hit: true }, + ) + .await; + } + } + } + } + } + } + + /// Stage barrier: honour a pending pause request, otherwise start `next`. + async fn workflow_advance(&mut self, parent_id: &str, next: StageKind) { + let paused = { + let Some(run) = self.workflow_runs.get_mut(parent_id) else { + return; + }; + if run.pause_requested { + run.pause_requested = false; + run.state = RunState::Paused { next }; + true + } else { + false + } + }; + if paused { + self.workflow_timeline( + parent_id, + format!( + "Paused before stage **{}**. Resume to continue; you can add guidance.", + next.label() + ), + ); + let run = self.workflow_runs.get(parent_id).cloned(); + if let Some(run) = run { + self.workflow_sync(&run); + } + } else { + self.workflow_spawn_stage(parent_id, next).await; + } + } + + /// Kill the run's stage sessions, wait until their processes have exited, + /// and mark the still-active children Interrupted. Completed stages keep + /// their sessions alive during the run (same-session re-review follows up + /// in them), so the sweep covers every child the run ever spawned — not + /// just the active ones. + async fn workflow_stop_children(&mut self, run: &mut WorkflowRun) -> Result<(), String> { + let active: Vec = run.active_children.keys().cloned().collect(); + let mut handles = Vec::new(); + for child_id in run.all_children() { + if let Some(handle) = self.sessions.remove(&child_id) { + handle.cancel(); + handles.push(handle); + } + self.pending_permissions.cleanup_task(&child_id); + } + // Only in-flight stages get their record and task status rewritten; + // completed ones keep their Done/Complete state. + for child_id in active { + run.set_record_status(&child_id, wire::OrchNodeStatus::Skipped); + if let Some(task) = self.tasks.get_mut(&child_id) { + if task.status == TaskStatus::Running || task.status == TaskStatus::Queued { + task.set_status(TaskStatus::Interrupted); + let updated = task.clone(); + self.persist(&updated); + self.emit(Event::TaskUpdated(updated)); + } + } + } + // Signal every parallel reviewer before awaiting any one of them. + let mut stop_error = None; + for handle in handles { + if let Err(error) = handle.wait_for_exit_within(super::acp::STOP_GRACE).await { + stop_error.get_or_insert(error); + } + } + run.active_children.clear(); + run.review_pending.clear(); + match stop_error { + Some(error) => Err(error), + None => Ok(()), + } + } + + /// End the pipeline: stop children, write the summary, set the parent's + /// final status. + async fn workflow_finalize( + &mut self, + parent_id: &str, + outcome: WorkflowOutcome, + ) -> Result<(), String> { + let Some(mut run) = self.workflow_runs.remove(parent_id) else { + return Ok(()); + }; + if !run.is_active() { + self.workflow_runs.insert(parent_id.to_string(), run); + return Ok(()); + } + let stop_result = self.workflow_stop_children(&mut run).await; + + let mut summary = String::new(); + let rounds_used = run.round; + match &outcome { + WorkflowOutcome::Success { limit_hit } => { + run.state = RunState::Done; + summary.push_str(&format!( + "Workflow **{}** finished after {rounds_used} review round(s).", + run.spec.name + )); + if *limit_hit { + summary.push_str(&format!( + "\n\n⚠ Review limit reached with unresolved findings:\n{}", + workflow::format_findings(&run.open_findings) + )); + } + if !run.deferred_findings.is_empty() { + summary.push_str(&format!( + "\n\nLow-severity notes from review (not auto-fixed):\n{}", + workflow::format_findings(&run.deferred_findings) + )); + } + summary.push_str("\n\nReview the changes and commit when ready."); + } + WorkflowOutcome::Stopped => { + run.state = RunState::Failed; + summary.push_str(&format!("Workflow **{}** stopped.", run.spec.name)); + } + WorkflowOutcome::Error(reason) => { + run.state = RunState::Failed; + summary.push_str(&format!( + "Workflow **{}** failed: {reason}. Changes made so far remain in the \ + working copy.", + run.spec.name + )); + } + } + self.workflow_timeline(parent_id, summary); + + if let Some(task) = self.tasks.get_mut(parent_id) { + match &outcome { + WorkflowOutcome::Success { .. } => task.set_status(TaskStatus::NeedsReview), + WorkflowOutcome::Stopped => task.set_status(TaskStatus::Interrupted), + WorkflowOutcome::Error(reason) => { + task.blocked_reason = Some(reason.clone()); + task.set_status(TaskStatus::Blocked); + } + } + } + self.workflow_sync(&run); + self.workflow_runs.insert(parent_id.to_string(), run); + // The state transition succeeded even if a stage process was slow to + // die; surfacing teardown trouble as the RPC's error would make a + // completed decision look rejected. + if let Err(error) = stop_result { + self.workflow_timeline( + parent_id, + format!("Note: a stage agent did not shut down cleanly ({error})."), + ); + } + Ok(()) + } + + /// A stage child task was cancelled or deleted out from under the run. + async fn workflow_child_gone(&mut self, child_id: &str) { + if self.workflow_child_of(child_id).is_some() { + self.workflow_stage_finished(child_id, false, StageText::default()) + .await; + } + } + + // ── User-facing controls (workflow.pause / resume / reply / decide) ── + + fn workflow_pause(&mut self, parent_id: &str) -> Result<(), String> { + let Some(run) = self.workflow_runs.get_mut(parent_id) else { + return Err("no workflow pipeline on this task".to_string()); + }; + match run.state { + RunState::Running { .. } => { + if run.pause_requested { + return Err("pause already requested".to_string()); + } + run.pause_requested = true; + self.workflow_timeline( + parent_id, + "Pause requested — takes effect when the current stage finishes its turn.", + ); + let run = self.workflow_runs.get(parent_id).cloned(); + if let Some(run) = run { + self.workflow_sync(&run); + } + Ok(()) + } + RunState::Paused { .. } => Err("already paused".to_string()), + RunState::AwaitingReply { .. } | RunState::AwaitingLimitDecision => { + Err("the pipeline is already waiting for your input".to_string()) + } + RunState::Done | RunState::Failed => Err("the pipeline has finished".to_string()), + } + } + + async fn workflow_resume( + &mut self, + parent_id: &str, + note: Option, + ) -> Result<(), String> { + let next = { + let Some(run) = self.workflow_runs.get_mut(parent_id) else { + return Err("no workflow pipeline on this task".to_string()); + }; + let RunState::Paused { next } = run.state else { + return Err("the pipeline is not paused".to_string()); + }; + run.pause_requested = false; + if let Some(note) = note.filter(|n| !n.trim().is_empty()) { + self.emit_session( + parent_id, + wire::SessionUpdate::UserMessage { + text: note.clone(), + attachments: vec![], + }, + ); + let run = self.workflow_runs.get_mut(parent_id).unwrap(); + run.pending_guidance = Some(note); + } + next + }; + self.workflow_timeline(parent_id, "Resumed."); + self.workflow_spawn_stage(parent_id, next).await; + Ok(()) + } + + async fn workflow_reply(&mut self, parent_id: &str, message: String) -> Result<(), String> { + let (stage, child) = { + let Some(run) = self.workflow_runs.get(parent_id) else { + return Err("no workflow pipeline on this task".to_string()); + }; + match &run.state { + RunState::AwaitingReply { stage, child, .. } => (*stage, child.clone()), + _ => return Err("the pipeline is not waiting for an answer".to_string()), + } + }; + // Show the user's answer in the parent timeline either way. + self.emit_session( + parent_id, + wire::SessionUpdate::UserMessage { + text: message.clone(), + attachments: vec![], + }, + ); + if self.workflow_followup(&child, message.clone()) { + self.mark_task_running(&child); + if let Some(run) = self.workflow_runs.get_mut(parent_id) { + run.state = RunState::Running { stage }; + } + self.workflow_timeline( + parent_id, + format!("Answer delivered — stage **{}** continues.", stage.label()), + ); + let run = self.workflow_runs.get(parent_id).cloned(); + if let Some(run) = run { + self.workflow_sync(&run); + } + } else { + // The asking session is gone (daemon restarted, agent died). Re-run + // the stage with the question + answer as guidance instead. + let question = { + let run = self.workflow_runs.get_mut(parent_id).unwrap(); + let question = match &run.state { + RunState::AwaitingReply { question, .. } => question.clone(), + _ => String::new(), + }; + run.active_children.remove(&child); + run.set_record_status(&child, wire::OrchNodeStatus::Skipped); + run.pending_guidance = Some(format!( + "The previous attempt of this stage asked:\n> {question}\n\nUser's answer:\n{message}" + )); + question + }; + let _ = question; + self.workflow_timeline( + parent_id, + format!( + "The asking session is no longer alive — re-running stage **{}** with your \ + answer as guidance.", + stage.label() + ), + ); + self.workflow_spawn_stage(parent_id, stage).await; + } + Ok(()) + } + + async fn workflow_decide( + &mut self, + parent_id: &str, + decision: wire::WorkflowDecision, + rounds: Option, + note: Option, + ) -> Result<(), String> { + { + let Some(run) = self.workflow_runs.get(parent_id) else { + return Err("no workflow pipeline on this task".to_string()); + }; + if run.state != RunState::AwaitingLimitDecision { + return Err("the pipeline is not waiting for a limit decision".to_string()); + } + } + match decision { + wire::WorkflowDecision::Extend => { + let granted = rounds.unwrap_or(1).clamp(1, workflow::MAX_EXTEND_ROUNDS); + let guidance = note.filter(|note| !note.trim().is_empty()); + if let Some(message) = guidance.as_ref() { + self.emit_session( + parent_id, + wire::SessionUpdate::UserMessage { + text: message.clone(), + attachments: vec![], + }, + ); + } + { + let run = self.workflow_runs.get_mut(parent_id).unwrap(); + run.extra_rounds += granted; + run.pending_guidance = guidance; + // Asking for more rounds supersedes a pause requested + // while the last review was still running; otherwise the + // next stage would park immediately after we just said + // "continuing with a fix". + run.pause_requested = false; + } + self.workflow_timeline( + parent_id, + format!("You granted {granted} more review round(s) — continuing with a fix."), + ); + self.workflow_advance(parent_id, StageKind::Fix).await; + Ok(()) + } + wire::WorkflowDecision::Finish => { + self.workflow_timeline(parent_id, "You chose to finish with the open findings."); + self.workflow_finalize(parent_id, WorkflowOutcome::Success { limit_hit: true }) + .await?; + Ok(()) + } + wire::WorkflowDecision::Stop => { + self.workflow_finalize(parent_id, WorkflowOutcome::Stopped) + .await?; + Ok(()) + } + } + } + + /// Restore persisted runs after a daemon restart. Barrier states survive + /// as-is; a run caught mid-stage converts to `Paused` at its last barrier + /// (resume re-runs the interrupted stage from scratch). + fn restore_workflow_runs(&mut self) { + let rows = self + .store + .as_ref() + .and_then(|s| s.load_workflow_runs().ok()) + .unwrap_or_default(); + for (task_id, json) in rows { + let Ok(mut run) = serde_json::from_str::(&json) else { + // Leaving the row in place would re-fail on every start while + // the parent sits with no pipeline state and therefore no + // pause/resume/stop controls. Say so, once, and move on. + eprintln!("[daemon] dropping unreadable workflow run for task {task_id}"); + if let Some(store) = &self.store { + let _ = store.delete_workflow_run(&task_id); + } + if let Some(task) = self.tasks.get_mut(&task_id) { + task.blocked_reason = + Some("workflow state could not be restored after an upgrade".to_string()); + task.set_status(TaskStatus::Blocked); + let updated = task.clone(); + self.persist(&updated); + } + continue; + }; + if !self.tasks.contains_key(&task_id) { + continue; + } + if run.is_active() { + if let RunState::Running { stage } = run.state { + // Sessions died with the previous daemon: park at the + // barrier before the interrupted stage. + let children: Vec = run.active_children.keys().cloned().collect(); + for child in &children { + run.set_record_status(child, wire::OrchNodeStatus::Failed); + } + run.active_children.clear(); + run.review_pending.clear(); + run.review_collected.clear(); + // Re-running a review re-increments `round` on spawn, so + // give the interrupted round back — otherwise a restart + // during round 2 of 2 resumes as "round 3/2" and lands + // straight on the limit decision. + if stage == StageKind::Review { + run.round = run.round.saturating_sub(1); + } + run.state = RunState::Paused { next: stage }; + // The working copy may hold half-applied edits from the + // killed attempt; the re-run has to know that. + run.pending_guidance = Some( + "A previous attempt of this stage was interrupted by a daemon restart. \ + The working copy may already contain its partial changes — inspect the \ + current diff before assuming you are starting from scratch." + .to_string(), + ); + self.workflow_timeline( + &task_id, + format!( + "Daemon restarted while stage **{}** was running. The pipeline is \ + paused — resume to re-run that stage.", + stage.label() + ), + ); + } + // The store normalizes Running → Interrupted on load; a live + // pipeline parent is restored according to whether a stage is + // executing or the runner is parked at a barrier. + if let Some(task) = self.tasks.get_mut(&task_id) { + task.blocked_reason = None; + let status = match run.state { + RunState::Running { .. } => TaskStatus::Running, + RunState::AwaitingReply { .. } + | RunState::AwaitingLimitDecision + | RunState::Paused { .. } => TaskStatus::Idle, + RunState::Done | RunState::Failed => task.status.clone(), + }; + task.set_status(status); + } + } + if let Some(task) = self.tasks.get_mut(&task_id) { + task.workflow_run = Some(run.wire_info()); + task.orchestration_graph = Some(run.graph_info()); + let updated = task.clone(); + self.persist(&updated); + } + if let Some(store) = &self.store { + if let Ok(json) = serde_json::to_string(&run) { + let _ = store.save_workflow_run(&task_id, &json); + } + } + self.workflow_runs.insert(task_id, run); + } + } +} + +/// A finished stage's text, split into the agent's closing message and the +/// whole turn. See [`Daemon::collect_stage_text`]. +#[derive(Debug, Default, Clone)] +struct StageText { + closing: String, + full: String, +} + /// Create the default policy set for a new daemon. fn default_policies() -> PolicyRegistry { let mut reg = PolicyRegistry::new(); diff --git a/src/daemon/mod.rs b/src/daemon/mod.rs index d56ea20..7a21a48 100644 --- a/src/daemon/mod.rs +++ b/src/daemon/mod.rs @@ -20,6 +20,7 @@ pub mod sessions; pub mod store; pub mod task; pub mod wire; +pub mod workflow; pub mod worktree; #[allow(unused_imports)] @@ -326,7 +327,7 @@ mod tests { .await; let mut events = daemon.subscribe(); - daemon.send(Command::CancelTask { id: id.clone() }).await; + daemon.cancel_task(&id).await.expect("cancel accepted"); timeout(Duration::from_secs(1), async { loop { @@ -573,6 +574,722 @@ mod tests { ); } + // ── Workflow pipeline engine ── + + const WF_FIXTURE: &str = concat!( + env!("CARGO_MANIFEST_DIR"), + "/tests/fixtures/mock-acp-workflow.mjs" + ); + + /// A tempdir project with one workflow file at + /// `.warpforge/workflows/test.yaml` and a registered `demo` project entry. + fn workflow_project(yaml: &str) -> (tempfile::TempDir, Vec) { + let dir = tempfile::tempdir().unwrap(); + let wf_dir = dir.path().join(".warpforge/workflows"); + std::fs::create_dir_all(&wf_dir).unwrap(); + std::fs::write(wf_dir.join("test.yaml"), yaml).unwrap(); + let projects = vec![ProjectEntry { + name: "demo".into(), + path: dir.path().to_string_lossy().into_owned(), + added_at: "0".into(), + }]; + (dir, projects) + } + + /// Scripted mock-agent command sharing `state` across stage processes. + fn wf_agent(dir: &tempfile::TempDir, state: &str, script: &str) -> String { + format!( + "node {WF_FIXTURE} {} {script}", + dir.path().join(state).display() + ) + } + + async fn create_workflow_task(daemon: &DaemonHandle, agent: &str) -> String { + let (tx, rx) = tokio::sync::oneshot::channel(); + daemon + .send(Command::CreateWorkflowTask { + project: "demo".into(), + prompt: "do the thing".into(), + agent: agent.into(), + tags: vec![], + worktree: false, + workflow: "test".into(), + attachments: vec![], + default_model: None, + include_runtime_context: false, + config_overrides: std::collections::HashMap::new(), + reply: tx, + }) + .await; + rx.await.unwrap().expect("workflow task created") + } + + /// Drive the event stream until the parent task satisfies `pred`. + async fn wait_for_parent( + events: &mut tokio::sync::broadcast::Receiver, + parent_id: &str, + what: &str, + pred: impl Fn(&Task) -> bool, + ) -> Task { + timeout(Duration::from_secs(20), async { + loop { + if let Ok(Event::TaskUpdated(task)) = events.recv().await { + if task.id == parent_id && pred(&task) { + break task; + } + } + } + }) + .await + .unwrap_or_else(|_| panic!("timed out waiting for: {what}")) + } + + #[tokio::test] + async fn workflow_full_loop_reject_fix_approve() { + use warpforge_protocol as wire; + let (dir, projects) = workflow_project("name: placeholder\n"); + // Round 1 rejects with one high finding, round 2 approves. + let reviewer = wf_agent(&dir, "rev.state", "reject approve"); + std::fs::write( + dir.path().join(".warpforge/workflows/test.yaml"), + format!( + "name: Test flow\nreview:\n max_rounds: 2\n reviewers:\n - agent: {reviewer}\n" + ), + ) + .unwrap(); + let lead = wf_agent(&dir, "impl.state", "impl fix"); + + let store = Store::open_at(std::path::Path::new(":memory:")).ok(); + let daemon = Daemon::spawn(projects, store); + let mut events = daemon.subscribe(); + let parent_id = create_workflow_task(&daemon, &lead).await; + + let done = wait_for_parent(&mut events, &parent_id, "pipeline done", |t| { + t.workflow_run + .as_ref() + .is_some_and(|w| w.stage == wire::WorkflowStage::Done) + }) + .await; + + assert_eq!(done.status, TaskStatus::NeedsReview); + let run = done.workflow_run.unwrap(); + assert_eq!(run.round, 2, "reject → fix → approve takes two rounds"); + assert_eq!(run.verdict, Some(wire::WorkflowVerdict::Approve)); + assert!(run.waiting.is_none()); + // Graph: implement, review r1, fix, review r2 — all with task ids. + let graph = done.orchestration_graph.unwrap(); + assert_eq!(graph.nodes.len(), 4, "{:?}", graph.nodes); + assert!(graph.nodes.iter().all(|n| n.task_id.is_some())); + assert_eq!(graph.nodes[0].kind, wire::OrchNodeKind::Implement); + assert_eq!(graph.nodes[2].kind, wire::OrchNodeKind::Fix); + // Default reask mode: round 2 follows up in the SAME reviewer session, + // so both review nodes point at one task. + assert_eq!(graph.nodes[1].kind, wire::OrchNodeKind::Review); + assert_eq!(graph.nodes[3].kind, wire::OrchNodeKind::Review); + assert_eq!( + graph.nodes[1].task_id, graph.nodes[3].task_id, + "same_session re-review must continue the round-1 reviewer session" + ); + + // The parent conversation is a useful workflow narrative, not just a + // sequence of opaque/coalesced stage transitions. Agent cards and + // results remain independent, ordered history entries. + let snapshot = daemon.snapshot().await; + let workflow_events: Vec<_> = snapshot.session_history[&parent_id] + .iter() + .filter(|update| matches!(update, wire::SessionUpdate::WorkflowEvent { .. })) + .collect(); + assert!(matches!( + workflow_events.first(), + Some(wire::SessionUpdate::WorkflowEvent { + event: wire::WorkflowEventKind::WorkflowStarted, + .. + }) + )); + let implement_started = workflow_events + .iter() + .position(|update| { + matches!( + update, + wire::SessionUpdate::WorkflowEvent { + event: wire::WorkflowEventKind::StageStarted, + stage: Some(wire::WorkflowStage::Implement), + agents, + .. + } if agents.len() == 1 + ) + }) + .unwrap(); + let implement_summary = workflow_events + .iter() + .position(|update| { + matches!( + update, + wire::SessionUpdate::WorkflowEvent { + event: wire::WorkflowEventKind::AgentOutput, + detail: Some(detail), + .. + } if detail.contains("IMPL-DONE: implemented the change.") + ) + }) + .unwrap(); + let first_review = workflow_events + .iter() + .position(|update| { + matches!( + update, + wire::SessionUpdate::WorkflowEvent { + event: wire::WorkflowEventKind::StageStarted, + stage: Some(wire::WorkflowStage::Review), + .. + } + ) + }) + .unwrap(); + // The finding reaches the timeline through the round's merged-verdict + // entry. The reviewer's own card shows its prose, NOT the raw protocol + // JSON it was asked to emit — that is stripped for display. + let finding = workflow_events + .iter() + .position(|update| { + matches!( + update, + wire::SessionUpdate::WorkflowEvent { detail: Some(detail), .. } + if detail.contains("bug here") + ) + }) + .expect("the merged verdict lists the finding"); + assert!( + workflow_events.iter().all(|update| !matches!( + update, + wire::SessionUpdate::WorkflowEvent { detail: Some(detail), .. } + if detail.contains("\"verdict\"") + )), + "the machine protocol block must not leak into the parent's timeline" + ); + let fix_summary = workflow_events + .iter() + .position(|update| { + matches!( + update, + wire::SessionUpdate::WorkflowEvent { + event: wire::WorkflowEventKind::AgentOutput, + detail: Some(detail), + .. + } if detail.contains("FIX-DONE: addressed the findings.") + ) + }) + .unwrap(); + assert!(implement_started < implement_summary); + assert!(implement_summary < first_review); + assert!(first_review < finding); + assert!(finding < fix_summary); + assert!(workflow_events.iter().any(|update| matches!( + update, + wire::SessionUpdate::WorkflowEvent { + event: wire::WorkflowEventKind::ReviewResult, + title, + .. + } if title.contains("approved") + ))); + assert!( + snapshot + .tasks + .iter() + .filter(|task| task.parent_task_id.as_deref() == Some(&parent_id)) + .all(|task| task.status == wire::TaskStatus::Done), + "consumed workflow stages should be terminal, not idle/needs-review" + ); + assert_eq!( + snapshot + .tasks + .iter() + .filter(|task| task.parent_task_id.as_deref() == Some(&parent_id)) + .count(), + 3, + "implement, one reused reviewer session, fix" + ); + // Finalize must sweep every stage session — completed ones included — + // so no mock agent processes outlive the pipeline. + assert_no_stage_processes(&dir).await; + } + + /// Poll until no mock agent process whose command line references this + /// test's tempdir remains alive. Stage sessions are kept alive during a + /// run (same-session re-review follows up in them) and must all be killed + /// at finalize. + async fn assert_no_stage_processes(dir: &tempfile::TempDir) { + let needle = dir.path().to_string_lossy().into_owned(); + for _ in 0..50 { + let alive = std::process::Command::new("pgrep") + .args(["-f", &needle]) + .output() + .map(|out| out.status.success()) + .unwrap_or(false); + if !alive { + return; + } + tokio::time::sleep(Duration::from_millis(100)).await; + } + panic!("stage agent processes still alive after the pipeline finished"); + } + + /// Reviewers must receive the implementer's closing message, not the + /// whole turn's tool narration. + #[tokio::test] + async fn workflow_carries_the_closing_message_not_the_narration() { + use warpforge_protocol as wire; + let (dir, projects) = workflow_project("name: placeholder\n"); + let reviewer = wf_agent(&dir, "rev.state", "approve"); + std::fs::write( + dir.path().join(".warpforge/workflows/test.yaml"), + format!("name: Closing flow\nreview:\n reviewers:\n - agent: {reviewer}\n"), + ) + .unwrap(); + let lead = wf_agent(&dir, "impl.state", "noisy-impl"); + + let store = Store::open_at(std::path::Path::new(":memory:")).ok(); + let daemon = Daemon::spawn(projects, store); + let mut events = daemon.subscribe(); + let parent_id = create_workflow_task(&daemon, &lead).await; + + wait_for_parent(&mut events, &parent_id, "pipeline done", |t| { + t.workflow_run + .as_ref() + .is_some_and(|w| w.stage == wire::WorkflowStage::Done) + }) + .await; + + // The reviewer child's prompt is its task prompt. + let snapshot = daemon.snapshot().await; + let reviewer_prompt = snapshot + .tasks + .iter() + .find(|t| { + t.parent_task_id.as_deref() == Some(&parent_id) && t.title.starts_with("review") + }) + .map(|t| t.prompt.clone()) + .expect("a reviewer stage ran"); + assert!( + reviewer_prompt.contains("CLOSING: implemented the change"), + "the reviewer must see the closing message" + ); + assert!( + !reviewer_prompt.contains("NARRATION:"), + "tool narration must not be passed off as the implementer's summary" + ); + + // The parent's timeline shows the same closing text as the stage result. + let events: Vec<_> = snapshot.session_history[&parent_id] + .iter() + .filter_map(|update| match update { + wire::SessionUpdate::WorkflowEvent { detail, .. } => detail.clone(), + _ => None, + }) + .collect(); + assert!( + events.iter().any(|detail| detail.contains("CLOSING:")), + "{events:?}" + ); + assert!( + events.iter().all(|detail| !detail.contains("NARRATION:")), + "{events:?}" + ); + } + + #[tokio::test] + async fn workflow_fresh_reask_spawns_new_reviewers() { + use warpforge_protocol as wire; + let (dir, projects) = workflow_project("name: placeholder\n"); + let reviewer = wf_agent(&dir, "rev.state", "reject approve"); + std::fs::write( + dir.path().join(".warpforge/workflows/test.yaml"), + format!( + "name: Fresh flow\nreview:\n max_rounds: 2\n reask: fresh\n reviewers:\n - agent: {reviewer}\n" + ), + ) + .unwrap(); + let lead = wf_agent(&dir, "impl.state", "impl fix"); + + let store = Store::open_at(std::path::Path::new(":memory:")).ok(); + let daemon = Daemon::spawn(projects, store); + let mut events = daemon.subscribe(); + let parent_id = create_workflow_task(&daemon, &lead).await; + + let done = wait_for_parent(&mut events, &parent_id, "pipeline done", |t| { + t.workflow_run + .as_ref() + .is_some_and(|w| w.stage == wire::WorkflowStage::Done) + }) + .await; + assert_eq!(done.status, TaskStatus::NeedsReview); + let graph = done.orchestration_graph.unwrap(); + assert_eq!(graph.nodes.len(), 4, "{:?}", graph.nodes); + assert_ne!( + graph.nodes[1].task_id, graph.nodes[3].task_id, + "reask: fresh must staff round 2 with a new reviewer session" + ); + } + + /// With the default same_session reask, a reviewer whose session died + /// between rounds falls back to a fresh session — whose prompt carries the + /// previous round's findings for verification. + #[tokio::test] + async fn workflow_dead_reviewer_session_falls_back_to_fresh() { + use warpforge_protocol as wire; + let (dir, projects) = workflow_project("name: placeholder\n"); + // Round 1: reject, then the process exits. Round 2 (fresh fallback + // process) pops the next behavior: approve. + let reviewer = wf_agent(&dir, "rev.state", "reject-die approve"); + std::fs::write( + dir.path().join(".warpforge/workflows/test.yaml"), + format!( + "name: Fallback flow\nreview:\n max_rounds: 2\n reviewers:\n - agent: {reviewer}\n" + ), + ) + .unwrap(); + // A slow fix keeps round 2 far enough away for the reviewer process + // death (~100ms after its verdict) to be observed first. + let lead = wf_agent(&dir, "impl.state", "impl slow-fix"); + + let store = Store::open_at(std::path::Path::new(":memory:")).ok(); + let daemon = Daemon::spawn(projects, store); + let mut events = daemon.subscribe(); + let parent_id = create_workflow_task(&daemon, &lead).await; + + let done = wait_for_parent(&mut events, &parent_id, "pipeline done", |t| { + t.workflow_run + .as_ref() + .is_some_and(|w| w.stage == wire::WorkflowStage::Done) + }) + .await; + assert_eq!(done.status, TaskStatus::NeedsReview); + let run = done.workflow_run.unwrap(); + assert_eq!(run.verdict, Some(wire::WorkflowVerdict::Approve)); + let graph = done.orchestration_graph.unwrap(); + assert_eq!(graph.nodes.len(), 4, "{:?}", graph.nodes); + assert_ne!( + graph.nodes[1].task_id, graph.nodes[3].task_id, + "a dead reviewer session must be replaced by a fresh one" + ); + } + + #[tokio::test] + async fn workflow_plan_question_reply_flow() { + use warpforge_protocol as wire; + let (dir, projects) = workflow_project("name: placeholder\n"); + let reviewer = wf_agent(&dir, "rev.state", "approve"); + std::fs::write( + dir.path().join(".warpforge/workflows/test.yaml"), + format!("name: Q flow\nplan: {{}}\nreview:\n reviewers:\n - agent: {reviewer}\n"), + ) + .unwrap(); + // Plan turn 1 asks, the answered turn plans, then implement runs. + let lead = wf_agent(&dir, "lead.state", "question plan impl"); + + let store = Store::open_at(std::path::Path::new(":memory:")).ok(); + let daemon = Daemon::spawn(projects, store); + let mut events = daemon.subscribe(); + let parent_id = create_workflow_task(&daemon, &lead).await; + + let waiting = wait_for_parent(&mut events, &parent_id, "question", |t| { + t.workflow_run + .as_ref() + .and_then(|w| w.waiting.as_ref()) + .is_some_and(|w| w.kind == wire::WorkflowWaitKind::Question) + }) + .await; + assert_eq!(waiting.status, TaskStatus::Idle); + let question = waiting.workflow_run.unwrap().waiting.unwrap(); + assert_eq!(question.question.as_deref(), Some("Which database?")); + assert_eq!(question.stage, Some(wire::WorkflowStage::Plan)); + let asking_child = daemon + .tasks() + .await + .into_iter() + .find(|task| task.parent_task_id.as_deref() == Some(&parent_id)) + .expect("asking plan stage"); + assert_eq!(asking_child.status, TaskStatus::Idle); + + let (tx, rx) = tokio::sync::oneshot::channel(); + daemon + .send(Command::WorkflowReply { + task: parent_id.clone(), + message: "Postgres".into(), + reply: tx, + }) + .await; + rx.await.unwrap().expect("reply accepted"); + + let done = wait_for_parent(&mut events, &parent_id, "pipeline done", |t| { + t.workflow_run + .as_ref() + .is_some_and(|w| w.stage == wire::WorkflowStage::Done) + }) + .await; + assert_eq!(done.status, TaskStatus::NeedsReview); + assert_eq!(done.workflow_run.unwrap().round, 1); + } + + #[tokio::test] + async fn workflow_limit_asks_and_finishes_on_decision() { + use warpforge_protocol as wire; + let (dir, projects) = workflow_project("name: placeholder\n"); + let reviewer = wf_agent(&dir, "rev.state", "reject"); + std::fs::write( + dir.path().join(".warpforge/workflows/test.yaml"), + format!( + "name: Limit flow\nreview:\n max_rounds: 1\n on_limit: ask\n reviewers:\n - agent: {reviewer}\n" + ), + ) + .unwrap(); + let lead = wf_agent(&dir, "impl.state", "impl fix"); + + let store = Store::open_at(std::path::Path::new(":memory:")).ok(); + let daemon = Daemon::spawn(projects, store); + let mut events = daemon.subscribe(); + let parent_id = create_workflow_task(&daemon, &lead).await; + + let waiting = wait_for_parent(&mut events, &parent_id, "limit decision", |t| { + t.workflow_run + .as_ref() + .and_then(|w| w.waiting.as_ref()) + .is_some_and(|w| w.kind == wire::WorkflowWaitKind::Limit) + }) + .await; + assert_eq!(waiting.status, TaskStatus::Idle); + assert!( + daemon + .tasks() + .await + .iter() + .filter(|task| task.parent_task_id.as_deref() == Some(&parent_id)) + .all(|task| task.status == TaskStatus::Done), + "every stage has completed when the review-limit decision is shown" + ); + + // A pause is invalid while waiting on a decision. + let (tx, rx) = tokio::sync::oneshot::channel(); + daemon + .send(Command::WorkflowPause { + task: parent_id.clone(), + reply: tx, + }) + .await; + assert!(rx.await.unwrap().is_err()); + + let (tx, rx) = tokio::sync::oneshot::channel(); + daemon + .send(Command::WorkflowDecide { + task: parent_id.clone(), + decision: wire::WorkflowDecision::Finish, + rounds: None, + note: None, + reply: tx, + }) + .await; + rx.await.unwrap().expect("decision accepted"); + + let done = wait_for_parent(&mut events, &parent_id, "pipeline done", |t| { + t.workflow_run + .as_ref() + .is_some_and(|w| w.stage == wire::WorkflowStage::Done) + }) + .await; + assert_eq!(done.status, TaskStatus::NeedsReview); + assert_eq!( + done.workflow_run.unwrap().verdict, + Some(wire::WorkflowVerdict::RequestChanges) + ); + } + + #[tokio::test] + async fn workflow_pause_takes_effect_at_barrier_and_resumes() { + use warpforge_protocol as wire; + let (dir, projects) = workflow_project("name: placeholder\n"); + let reviewer = wf_agent(&dir, "rev.state", "approve"); + std::fs::write( + dir.path().join(".warpforge/workflows/test.yaml"), + format!("name: Pause flow\nreview:\n reviewers:\n - agent: {reviewer}\n"), + ) + .unwrap(); + // The implement turn takes ~600ms — enough for the pause to land. + let lead = wf_agent(&dir, "impl.state", "slow-impl fix"); + + let store = Store::open_at(std::path::Path::new(":memory:")).ok(); + let daemon = Daemon::spawn(projects, store); + let mut events = daemon.subscribe(); + let parent_id = create_workflow_task(&daemon, &lead).await; + + let (tx, rx) = tokio::sync::oneshot::channel(); + daemon + .send(Command::WorkflowPause { + task: parent_id.clone(), + reply: tx, + }) + .await; + rx.await.unwrap().expect("pause accepted while running"); + + let paused = wait_for_parent(&mut events, &parent_id, "paused at barrier", |t| { + t.workflow_run + .as_ref() + .and_then(|w| w.waiting.as_ref()) + .is_some_and(|w| w.kind == wire::WorkflowWaitKind::Paused) + }) + .await; + assert_eq!(paused.status, TaskStatus::Idle); + + let (tx, rx) = tokio::sync::oneshot::channel(); + daemon + .send(Command::WorkflowResume { + task: parent_id.clone(), + note: Some("carry on".into()), + reply: tx, + }) + .await; + rx.await.unwrap().expect("resume accepted"); + + let done = wait_for_parent(&mut events, &parent_id, "pipeline done", |t| { + t.workflow_run + .as_ref() + .is_some_and(|w| w.stage == wire::WorkflowStage::Done) + }) + .await; + assert_eq!(done.status, TaskStatus::NeedsReview); + } + + #[tokio::test] + async fn workflow_parent_cancel_stops_the_active_stage_before_acknowledging() { + use warpforge_protocol as wire; + let (dir, projects) = workflow_project("name: Cancel flow\n"); + let pid_path = dir.path().join("cancel.pid"); + let lead = format!( + "echo $$ > {}; exec {}", + pid_path.display(), + wf_agent(&dir, "cancel.state", "slow-impl") + ); + let daemon = Daemon::spawn( + projects, + Store::open_at(std::path::Path::new(":memory:")).ok(), + ); + let parent_id = create_workflow_task(&daemon, &lead).await; + + let pid = timeout(Duration::from_secs(2), async { + loop { + if let Ok(pid) = std::fs::read_to_string(&pid_path) { + let pid = pid.trim(); + if pid.parse::().is_ok() { + break pid.to_string(); + } + } + tokio::task::yield_now().await; + } + }) + .await + .expect("active stage should write its process id"); + + daemon + .cancel_task(&parent_id) + .await + .expect("workflow cancellation acknowledged"); + + let process_alive = tokio::process::Command::new("kill") + .args(["-0", &pid]) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .status() + .await + .is_ok_and(|status| status.success()); + assert!( + !process_alive, + "task.cancel acknowledged before ACP process {pid} exited" + ); + + let tasks = daemon.tasks().await; + let parent = tasks.iter().find(|task| task.id == parent_id).unwrap(); + assert_eq!(parent.status, TaskStatus::Interrupted); + assert_eq!( + parent.workflow_run.as_ref().map(|run| run.stage), + Some(wire::WorkflowStage::Failed) + ); + let child = tasks + .iter() + .find(|task| task.parent_task_id.as_deref() == Some(&parent_id)) + .expect("active workflow stage"); + assert_eq!(child.status, TaskStatus::Interrupted); + assert_eq!( + parent.orchestration_graph.as_ref().unwrap().nodes[0].status, + wire::OrchNodeStatus::Skipped + ); + } + + /// A daemon restart mid-stage parks the pipeline at its last barrier as + /// Paused; resume re-runs the interrupted stage and the run completes. + #[tokio::test] + async fn workflow_restart_converts_midstage_to_paused_and_resumes() { + use warpforge_protocol as wire; + let (dir, projects) = workflow_project("name: placeholder\n"); + let reviewer = wf_agent(&dir, "rev.state", "approve"); + std::fs::write( + dir.path().join(".warpforge/workflows/test.yaml"), + format!("name: Restart flow\nreview:\n reviewers:\n - agent: {reviewer}\n"), + ) + .unwrap(); + // Attempt 1 of implement is slow and dies with the daemon; the re-run + // after restart pops the next behavior and completes quickly. + let lead = wf_agent(&dir, "impl.state", "slow-impl impl fix"); + let db_path = dir.path().join("warpforge.db"); + + let daemon = Daemon::spawn(projects.clone(), Store::open_at(&db_path).ok()); + let parent_id = create_workflow_task(&daemon, &lead).await; + // Shut down while the implement turn is still in flight (~600ms). + daemon.shutdown().await; + + let daemon = Daemon::spawn(projects, Store::open_at(&db_path).ok()); + let mut events = daemon.subscribe(); + let restored = timeout(Duration::from_secs(5), async { + loop { + let tasks = daemon.tasks().await; + if let Some(task) = tasks.iter().find(|t| t.id == parent_id) { + if task + .workflow_run + .as_ref() + .and_then(|w| w.waiting.as_ref()) + .is_some_and(|w| w.kind == wire::WorkflowWaitKind::Paused) + { + break task.clone(); + } + } + tokio::time::sleep(Duration::from_millis(50)).await; + } + }) + .await + .expect("restored run should be paused at the implement barrier"); + assert_eq!(restored.status, TaskStatus::Idle); + assert_eq!( + restored.workflow_run.unwrap().stage, + wire::WorkflowStage::Implement + ); + + let (tx, rx) = tokio::sync::oneshot::channel(); + daemon + .send(Command::WorkflowResume { + task: parent_id.clone(), + note: None, + reply: tx, + }) + .await; + rx.await.unwrap().expect("resume accepted after restart"); + + let done = wait_for_parent(&mut events, &parent_id, "pipeline done", |t| { + t.workflow_run + .as_ref() + .is_some_and(|w| w.stage == wire::WorkflowStage::Done) + }) + .await; + assert_eq!(done.status, TaskStatus::NeedsReview); + } + /// Regression: when a stale ACP handle is in sessions and a prompt /// arrives, the daemon must detect the dead handle and trigger resume /// via the stored session_id rather than failing with "no live session". diff --git a/src/daemon/server.rs b/src/daemon/server.rs index 762ddb8..4a87a3a 100644 --- a/src/daemon/server.rs +++ b/src/daemon/server.rs @@ -418,7 +418,34 @@ async fn dispatch( attachments, default_model, config_overrides, + workflow, } => { + if let Some(workflow) = workflow { + let (tx, rx) = oneshot::channel(); + handle + .send(Command::CreateWorkflowTask { + project, + prompt, + agent, + tags, + worktree, + workflow, + attachments, + default_model, + include_runtime_context, + config_overrides, + reply: tx, + }) + .await; + let id = rx + .await + .unwrap_or_else(|_| Err("daemon closed".into())) + .map_err(|e| wire::RpcError { + code: wire::ErrorCode::InvalidRequest, + message: e, + })?; + return Ok(json!({ "taskId": id })); + } let id = handle .create_task( &project, @@ -584,7 +611,13 @@ async fn dispatch( Ok(json!({ "text": text })) } TaskCancel { task_id } => { - handle.send(Command::CancelTask { id: task_id }).await; + handle + .cancel_task(&task_id) + .await + .map_err(|message| wire::RpcError { + code: wire::ErrorCode::Internal, + message, + })?; Ok(json!(null)) } TaskArchive { task_id } => { @@ -831,6 +864,59 @@ async fn dispatch( let ok = rx.await.unwrap_or(false); Ok(json!({ "ok": ok })) } + // ── Workflows ── + WorkflowList { project } => { + let path = project_path(handle, &project).await?; + let workflows: Vec = + crate::workflow_config::list_workflows(std::path::Path::new(&path)) + .into_iter() + .map(workflow_meta) + .collect(); + Ok(json!({ "workflows": workflows })) + } + WorkflowEject { project, id } => { + let path = project_path(handle, &project).await?; + let target = crate::workflow_config::eject_builtin(std::path::Path::new(&path), &id) + .map_err(|e| wire::RpcError { + code: wire::ErrorCode::InvalidRequest, + message: format!("{e:#}"), + })?; + Ok(json!({ "path": target.to_string_lossy() })) + } + WorkflowPause { task } => { + workflow_control(handle, |reply| Command::WorkflowPause { task, reply }).await + } + WorkflowResume { task, note } => { + workflow_control(handle, |reply| Command::WorkflowResume { + task, + note, + reply, + }) + .await + } + WorkflowReply { task, message } => { + workflow_control(handle, |reply| Command::WorkflowReply { + task, + message, + reply, + }) + .await + } + WorkflowDecide { + task, + decision, + rounds, + note, + } => { + workflow_control(handle, |reply| Command::WorkflowDecide { + task, + decision, + rounds, + note, + reply, + }) + .await + } ProjectAdd { path, name } => { let entry = handle .add_project(&path, name.as_deref()) @@ -899,6 +985,12 @@ async fn dispatch( BootstrapWriteConfig { project, yaml } => { let path = project_path(handle, &project).await?; let target = crate::config::find_config_file(std::path::Path::new(&path)); + if let Some(parent) = target.parent() { + std::fs::create_dir_all(parent).map_err(|e| wire::RpcError { + code: wire::ErrorCode::Internal, + message: format!("create {}: {e}", parent.display()), + })?; + } std::fs::write(&target, yaml).map_err(|e| wire::RpcError { code: wire::ErrorCode::Internal, message: format!("write {}: {e}", target.display()), @@ -926,6 +1018,56 @@ fn validate_issues(yaml: &str) -> Vec { } } +/// Send a workflow control command and map its `Result<(), String>` reply to +/// an RPC response (`null` on success, `InvalidRequest` with the reason +/// otherwise — e.g. the pipeline is not in the state the control expects). +async fn workflow_control( + handle: &DaemonHandle, + build: impl FnOnce(oneshot::Sender>) -> Command, +) -> Result { + let (tx, rx) = oneshot::channel(); + handle.send(build(tx)).await; + rx.await + .unwrap_or_else(|_| Err("daemon closed".into())) + .map_err(|e| wire::RpcError { + code: wire::ErrorCode::InvalidRequest, + message: e, + })?; + Ok(json!(null)) +} + +/// Wire form of one workflow definition for the New Task picker. +fn workflow_meta(w: crate::workflow_config::LoadedWorkflow) -> wire::WorkflowMeta { + let source = match w.source { + crate::workflow_config::WorkflowSource::Project => wire::WorkflowSource::Project, + crate::workflow_config::WorkflowSource::Builtin => wire::WorkflowSource::Builtin, + }; + match w.spec { + Ok(spec) => wire::WorkflowMeta { + id: w.id, + name: spec.name.clone(), + description: spec.description.clone(), + source, + valid: true, + error: None, + warnings: w.warnings, + stages: spec.stage_summary(), + max_rounds: spec.review.max_rounds, + }, + Err(error) => wire::WorkflowMeta { + name: w.id.clone(), + id: w.id, + description: None, + source, + valid: false, + error: Some(error), + warnings: w.warnings, + stages: vec![], + max_rounds: 0, + }, + } +} + /// Resolve a registered project's directory, or an `InvalidRequest` error. async fn project_path(handle: &DaemonHandle, project: &str) -> Result { handle @@ -991,6 +1133,7 @@ fn method_is_mutation(method: &wire::Method) -> bool { | GitPushInfo { .. } | OrchestrateList {} | OrchestrateGetConfig {} + | WorkflowList { .. } | BootstrapFinalize { .. } | BootstrapReadConfig { .. } ) @@ -1091,6 +1234,94 @@ mod tests { assert!(saw_response, "expected a response with a taskId"); } + #[tokio::test] + async fn workflow_list_and_eject_over_websocket() { + type Ws = tokio_tungstenite::WebSocketStream< + tokio_tungstenite::MaybeTlsStream, + >; + async fn rpc( + ws: &mut Ws, + id: u64, + method: &str, + params: serde_json::Value, + ) -> serde_json::Value { + ws.send(Message::Text( + json!({ "id": id, "method": method, "params": params }).to_string(), + )) + .await + .unwrap(); + loop { + let msg = timeout(Duration::from_secs(2), ws.next()) + .await + .expect("frame") + .expect("some") + .expect("ok"); + if let Message::Text(t) = msg { + let v: serde_json::Value = serde_json::from_str(t.as_str()).unwrap(); + if v.get("id").and_then(|i| i.as_u64()) == Some(id) { + return v; + } + } + } + } + + let dir = tempfile::tempdir().unwrap(); + let projects = vec![ProjectEntry { + name: "demo".into(), + path: dir.path().to_string_lossy().into_owned(), + added_at: "0".into(), + }]; + let store = Store::open_at(std::path::Path::new(":memory:")).ok(); + let handle = Daemon::spawn(projects, store); + + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + tokio::spawn(run(listener, handle.clone(), String::new())); + let (mut ws, _) = tokio_tungstenite::connect_async(format!("ws://{addr}")) + .await + .unwrap(); + + // A fresh project sees exactly the built-in templates. + let v = rpc(&mut ws, 1, "workflow.list", json!({ "project": "demo" })).await; + let workflows = v["result"]["workflows"].as_array().unwrap(); + assert_eq!(workflows.len(), 2); + assert!(workflows + .iter() + .all(|w| w["source"] == "builtin" && w["valid"] == true)); + assert!(workflows.iter().any(|w| w["id"] == "review-loop")); + + // Ejecting copies the built-in into .warpforge/workflows/. + let v = rpc( + &mut ws, + 2, + "workflow.eject", + json!({ "project": "demo", "id": "review-loop" }), + ) + .await; + let path = v["result"]["path"].as_str().unwrap(); + assert!(std::path::Path::new(path).exists()); + + // The ejected copy now overrides the built-in… + let v = rpc(&mut ws, 3, "workflow.list", json!({ "project": "demo" })).await; + let workflows = v["result"]["workflows"].as_array().unwrap(); + let review = workflows.iter().find(|w| w["id"] == "review-loop").unwrap(); + assert_eq!(review["source"], "project"); + + // …and a second eject refuses to overwrite it. + let v = rpc( + &mut ws, + 4, + "workflow.eject", + json!({ "project": "demo", "id": "review-loop" }), + ) + .await; + assert!(v["error"]["message"].as_str().unwrap().contains("exists")); + + // Unknown projects are rejected. + let v = rpc(&mut ws, 5, "workflow.list", json!({ "project": "nope" })).await; + assert!(v.get("error").is_some()); + } + #[tokio::test] async fn spawn_terminal_streams_screen_over_websocket() { let projects = vec![ProjectEntry { diff --git a/src/daemon/store.rs b/src/daemon/store.rs index 5be0b9a..695b4c0 100644 --- a/src/daemon/store.rs +++ b/src/daemon/store.rs @@ -194,6 +194,11 @@ impl Store { id INTEGER PRIMARY KEY CHECK (id = 1), config_json TEXT NOT NULL ); + CREATE TABLE IF NOT EXISTS workflow_runs ( + task_id TEXT PRIMARY KEY, + run_json TEXT NOT NULL, + updated_at INTEGER NOT NULL + ); "#, )?; // Existing databases from before config selector persistence won't have @@ -312,6 +317,7 @@ impl Store { config_options: serde_json::from_str(&config_options_json).unwrap_or_default(), worktree: row.get(12)?, orchestration_graph: None, + workflow_run: None, parent_task_id: row.get(13)?, settled_override: row.get::<_, Option>(15)?.map(|v| v != 0), settled_at: row.get::<_, Option>(16)?, @@ -414,12 +420,44 @@ impl Store { Ok(()) } + /// Persist a workflow pipeline's full state (spec snapshot included) as + /// one JSON blob, replacing any previous snapshot for the task. + pub fn save_workflow_run(&self, task_id: &str, run_json: &str) -> Result<()> { + self.conn.execute( + "INSERT OR REPLACE INTO workflow_runs (task_id, run_json, updated_at) \ + VALUES (?1, ?2, strftime('%s','now'))", + rusqlite::params![task_id, run_json], + )?; + Ok(()) + } + + /// All persisted workflow runs as (task_id, run_json) pairs. + pub fn load_workflow_runs(&self) -> Result> { + let mut stmt = self + .conn + .prepare("SELECT task_id, run_json FROM workflow_runs")?; + let rows = stmt.query_map([], |row| Ok((row.get(0)?, row.get(1)?)))?; + Ok(rows.filter_map(|r| r.ok()).collect()) + } + + pub fn delete_workflow_run(&self, task_id: &str) -> Result<()> { + self.conn.execute( + "DELETE FROM workflow_runs WHERE task_id = ?1", + rusqlite::params![task_id], + )?; + Ok(()) + } + /// Delete a task and its session history permanently. pub fn delete_task(&self, id: &str) -> Result<()> { self.conn.execute( "DELETE FROM session_updates WHERE task_id = ?1", rusqlite::params![id], )?; + self.conn.execute( + "DELETE FROM workflow_runs WHERE task_id = ?1", + rusqlite::params![id], + )?; self.conn .execute("DELETE FROM tasks WHERE id = ?1", rusqlite::params![id])?; Ok(()) diff --git a/src/daemon/task.rs b/src/daemon/task.rs index 808b4b9..198d97c 100644 --- a/src/daemon/task.rs +++ b/src/daemon/task.rs @@ -64,6 +64,9 @@ pub struct Task { pub worktree: Option, /// Orchestration graph for parent orchestrator tasks. pub orchestration_graph: Option, + /// Live workflow pipeline state for workflow parent tasks. Derived from + /// the engine's run (not persisted with the task — the run itself is). + pub workflow_run: Option, /// When this task was spawned by an orchestrator agent as a sub-agent, the /// id of that orchestrator task. Its result is delivered back into the /// parent's inbox on completion. @@ -99,6 +102,7 @@ impl Task { config_options: Vec::new(), worktree: None, orchestration_graph: None, + workflow_run: None, parent_task_id: None, settled_override: None, settled_at: None, diff --git a/src/daemon/wire.rs b/src/daemon/wire.rs index 63d7cf0..180c66a 100644 --- a/src/daemon/wire.rs +++ b/src/daemon/wire.rs @@ -136,6 +136,7 @@ pub fn task_info(t: &Task) -> wire::TaskInfo { worktree: t.worktree.clone(), orchestration_graph: t.orchestration_graph.clone(), parent_task_id: t.parent_task_id.clone(), + workflow_run: t.workflow_run.clone(), settled_override: t.settled_override, settled_at: t.settled_at, snoozed_until: t.snoozed_until, diff --git a/src/daemon/workflow.rs b/src/daemon/workflow.rs new file mode 100644 index 0000000..392bd58 --- /dev/null +++ b/src/daemon/workflow.rs @@ -0,0 +1,1614 @@ +//! Deterministic workflow pipeline engine: run-state container and pure +//! helpers (stage prompts, verdict/marker parsing, review merging, context +//! formatting). +//! +//! The pipeline shape is fixed: `plan? → implement → review ⇄ fix`. This +//! module has no side effects — the actor glue that spawns stage sessions, +//! reacts to turn ends, and emits events lives in `actor.rs` and calls into +//! these helpers, so everything here is unit-testable in isolation. + +use serde::{Deserialize, Serialize}; +use std::collections::HashMap; +use warpforge_protocol as wire; + +use crate::workflow_config::{render_template, ReviewContextItem, WorkflowSpec}; + +/// Byte budget for the diff embedded into review/fix prompts. +pub const DIFF_CONTEXT_MAX_BYTES: usize = 200 * 1024; +/// Byte budget for the implementer-summary context section (tail wins). +pub const SUMMARY_CONTEXT_MAX_BYTES: usize = 16 * 1024; +/// Budget for a salvaged review body used as a stand-in finding. +const SALVAGED_FINDING_MAX_BYTES: usize = 4 * 1024; +/// How many times a reviewer is re-asked for a parseable verdict before the +/// pipeline fails. +pub const MAX_VERDICT_REASKS: u8 = 1; +/// Cap on rounds granted by a single `workflow.decide { extend }`. +pub const MAX_EXTEND_ROUNDS: u32 = 5; + +// ─── Stages and state ──────────────────────────────────────────────────────── + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum StageKind { + Plan, + Implement, + Review, + Fix, +} + +impl StageKind { + pub fn label(self) -> &'static str { + match self { + StageKind::Plan => "plan", + StageKind::Implement => "implement", + StageKind::Review => "review", + StageKind::Fix => "fix", + } + } + + pub fn title(self) -> &'static str { + match self { + StageKind::Plan => "Plan", + StageKind::Implement => "Implement", + StageKind::Review => "Review", + StageKind::Fix => "Fix", + } + } + + pub fn wire(self) -> wire::WorkflowStage { + match self { + StageKind::Plan => wire::WorkflowStage::Plan, + StageKind::Implement => wire::WorkflowStage::Implement, + StageKind::Review => wire::WorkflowStage::Review, + StageKind::Fix => wire::WorkflowStage::Fix, + } + } + + pub fn node_kind(self) -> wire::OrchNodeKind { + match self { + StageKind::Plan => wire::OrchNodeKind::Plan, + StageKind::Implement => wire::OrchNodeKind::Implement, + StageKind::Review => wire::OrchNodeKind::Review, + StageKind::Fix => wire::OrchNodeKind::Fix, + } + } + + /// The stage that follows a successfully completed one. Review is not a + /// simple successor — it branches on the merged verdict — so it has no + /// entry here. + pub fn successor(self) -> Option { + match self { + StageKind::Plan => Some(StageKind::Implement), + StageKind::Implement => Some(StageKind::Review), + StageKind::Fix => Some(StageKind::Review), + StageKind::Review => None, + } + } +} + +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case", tag = "state")] +pub enum RunState { + /// A stage's child session(s) are running. + Running { + stage: StageKind, + }, + /// A stage asked `need_user_input`; suspended until `workflow.reply`. + AwaitingReply { + stage: StageKind, + child: String, + question: String, + }, + /// Review rounds exhausted with open findings; suspended until + /// `workflow.decide`. + AwaitingLimitDecision, + /// Soft-paused at a stage barrier; `next` starts on `workflow.resume`. + Paused { + next: StageKind, + }, + Done, + Failed, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum Severity { + Critical, + High, + Medium, + Low, +} + +impl Severity { + pub fn label(self) -> &'static str { + match self { + Severity::Critical => "critical", + Severity::High => "high", + Severity::Medium => "medium", + Severity::Low => "low", + } + } + + /// Low-severity findings go to the final summary, not to the fixer. + pub fn goes_to_fix(self) -> bool { + !matches!(self, Severity::Low) + } + + /// Map an agent's severity word onto the four levels. Agents use a much + /// wider vocabulary than the protocol asks for, and the default matters: + /// an unknown word becomes `Medium`, which forces a repair round, so + /// opinion-shaped words must land in `Low` rather than fall through. + fn parse(s: &str) -> Severity { + match s.trim().to_ascii_lowercase().as_str() { + "critical" | "blocker" | "blocking" | "severe" | "fatal" => Severity::Critical, + "high" | "major" | "important" => Severity::High, + "low" | "minor" | "nit" | "nitpick" | "info" | "informational" | "suggestion" + | "style" | "cosmetic" | "trivial" | "optional" | "polish" | "note" => Severity::Low, + _ => Severity::Medium, + } + } +} + +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct Finding { + pub severity: Severity, + pub file: Option, + /// Line in the post-change file, when the reviewer pinned one down. + #[serde(default)] + pub line: Option, + /// A short verbatim excerpt of the offending code. More robust than a line + /// number — the fixer can search for it after the file has shifted. + #[serde(default)] + pub snippet: Option, + pub description: String, + /// Reviewer label, e.g. "reviewer 2 (codex)". + pub reviewer: String, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum Verdict { + Approve, + RequestChanges, +} + +impl Verdict { + pub fn wire(self) -> wire::WorkflowVerdict { + match self { + Verdict::Approve => wire::WorkflowVerdict::Approve, + Verdict::RequestChanges => wire::WorkflowVerdict::RequestChanges, + } + } +} + +/// How a pipeline ends. +#[derive(Debug, Clone, PartialEq)] +pub enum WorkflowOutcome { + /// Reviewers approved, or the user chose to finish with open findings. + Success { limit_hit: bool }, + /// Stopped by the user (task cancel, or `workflow.decide { stop }`). + Stopped, + /// Infrastructure or protocol failure. + Error(String), +} + +/// One spawned stage child, kept for the orchestration graph on the board. +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct StageRecord { + pub kind: StageKind, + pub task_id: String, + pub agent: String, + /// Display label, e.g. "review 1/2 (codex)". + pub label: String, + pub status: wire::OrchNodeStatus, +} + +// ─── The run ───────────────────────────────────────────────────────────────── + +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct WorkflowRun { + pub parent_id: String, + pub project: String, + pub spec: WorkflowSpec, + /// Lead agent / model picked in the New Task dialog — the fallback for + /// every stage that doesn't override them. + pub lead_agent: String, + pub lead_model: Option, + pub state: RunState, + /// 1-based review round; 0 until the first review starts. + pub round: u32, + /// Extra rounds granted by `workflow.decide { extend }`. + pub extra_rounds: u32, + /// Set by `workflow.pause` while a stage is running; takes effect at the + /// next stage barrier. + pub pause_requested: bool, + /// Free-text from resume/decide, delivered to the next spawned stage as a + /// "User guidance" block and then cleared. + pub pending_guidance: Option, + pub plan_output: Option, + /// Final text of the last implement/fix session. + pub last_summary: Option, + pub last_verdict: Option, + /// Findings of the latest review round that still need fixing. + #[serde(default)] + pub open_findings: Vec, + /// Low-severity findings accumulated for the final summary only. + #[serde(default)] + pub deferred_findings: Vec, + /// child task id → reviewer index, while a review stage is in flight. + #[serde(default)] + pub review_pending: HashMap, + /// (reviewer index, verdict, findings) collected this round. + #[serde(default)] + pub review_collected: Vec<(usize, Verdict, Vec)>, + /// child task id → verdict re-ask count. + #[serde(default)] + pub reasked: HashMap, + /// child task id → stage kind, for routing TurnEnded (single-child stages). + #[serde(default)] + pub active_children: HashMap, + /// reviewer index → child task id of the previous review round. With + /// `review.reask: same_session` the next round follows up in these + /// sessions instead of spawning fresh ones. + #[serde(default)] + pub prior_review_children: HashMap, + #[serde(default)] + pub history: Vec, + /// Attachments from the New Task dialog, delivered to the first stage. + #[serde(default)] + pub attachments: Vec, + /// Whether stage sessions get the project's runtime-context preamble, as + /// picked in the New Task dialog. + #[serde(default)] + pub include_runtime_context: bool, + /// Non-model session config picks from the dialog (reasoning effort, mode), + /// applied to every stage session. + #[serde(default)] + pub config_overrides: HashMap, +} + +impl WorkflowRun { + #[allow(clippy::too_many_arguments)] + pub fn new( + parent_id: String, + project: String, + spec: WorkflowSpec, + lead_agent: String, + lead_model: Option, + attachments: Vec, + include_runtime_context: bool, + config_overrides: HashMap, + ) -> Self { + Self { + parent_id, + project, + spec, + lead_agent, + lead_model, + state: RunState::Running { + stage: StageKind::Implement, // set properly by the first spawn + }, + round: 0, + extra_rounds: 0, + pause_requested: false, + pending_guidance: None, + plan_output: None, + last_summary: None, + last_verdict: None, + open_findings: Vec::new(), + deferred_findings: Vec::new(), + review_pending: HashMap::new(), + review_collected: Vec::new(), + prior_review_children: HashMap::new(), + reasked: HashMap::new(), + active_children: HashMap::new(), + history: Vec::new(), + attachments: Vec::new(), + include_runtime_context, + config_overrides, + } + .with_attachments(attachments) + } + + fn with_attachments(mut self, attachments: Vec) -> Self { + self.attachments = attachments; + self + } + + pub fn first_stage(&self) -> StageKind { + if self.spec.plan.is_some() { + StageKind::Plan + } else { + StageKind::Implement + } + } + + pub fn effective_max_rounds(&self) -> u32 { + self.spec.review.max_rounds + self.extra_rounds + } + + pub fn is_active(&self) -> bool { + !matches!(self.state, RunState::Done | RunState::Failed) + } + + /// Agent + model for a stage, applying the fallback chain: + /// stage override → (fix falls back to implement) → lead agent/model. + pub fn stage_agent( + &self, + kind: StageKind, + reviewer: Option, + ) -> (String, Option) { + let (agent, model) = match kind { + StageKind::Plan => { + let s = self.spec.plan.as_ref(); + ( + s.and_then(|s| s.agent.clone()), + s.and_then(|s| s.model.clone()), + ) + } + StageKind::Implement => ( + self.spec.implement.agent.clone(), + self.spec.implement.model.clone(), + ), + StageKind::Fix => ( + self.spec + .fix + .agent + .clone() + .or_else(|| self.spec.implement.agent.clone()), + self.spec + .fix + .model + .clone() + .or_else(|| self.spec.implement.model.clone()), + ), + StageKind::Review => { + let r = reviewer.and_then(|i| self.spec.review.reviewers.get(i)); + ( + r.and_then(|r| r.agent.clone()), + r.and_then(|r| r.model.clone()), + ) + } + }; + ( + agent.unwrap_or_else(|| self.lead_agent.clone()), + model.or_else(|| self.lead_model.clone()), + ) + } + + pub fn reviewer_label(&self, index: usize) -> String { + let total = self.spec.review.reviewers.len(); + let (agent, _) = self.stage_agent(StageKind::Review, Some(index)); + if total == 1 { + format!("reviewer ({agent})") + } else { + format!("reviewer {}/{total} ({agent})", index + 1) + } + } + + pub fn record_stage(&mut self, kind: StageKind, task_id: &str, agent: &str, label: String) { + self.history.push(StageRecord { + kind, + task_id: task_id.to_string(), + agent: agent.to_string(), + label, + status: wire::OrchNodeStatus::Running, + }); + } + + pub fn set_record_status(&mut self, task_id: &str, status: wire::OrchNodeStatus) { + if let Some(rec) = self.history.iter_mut().rev().find(|r| r.task_id == task_id) { + rec.status = status; + } + } + + /// Take the pending guidance (it is delivered to exactly one stage). + pub fn take_guidance(&mut self) -> Option { + self.pending_guidance.take() + } + + /// Every stage child this run ever spawned. Completed stages keep their + /// sessions alive during the run (same-session re-review needs them), so + /// final cleanup must sweep this full set, not just `active_children`. + pub fn all_children(&self) -> std::collections::HashSet { + self.history + .iter() + .map(|record| record.task_id.clone()) + .chain(self.active_children.keys().cloned()) + .collect() + } + + // ── Wire projections ── + + pub fn wire_info(&self) -> wire::WorkflowRunInfo { + let (stage, waiting) = match &self.state { + RunState::Running { stage } => (stage.wire(), None), + RunState::AwaitingReply { + stage, question, .. + } => ( + stage.wire(), + Some(wire::WorkflowWaiting { + kind: wire::WorkflowWaitKind::Question, + stage: Some(stage.wire()), + question: Some(question.clone()), + }), + ), + RunState::AwaitingLimitDecision => ( + wire::WorkflowStage::Review, + Some(wire::WorkflowWaiting { + kind: wire::WorkflowWaitKind::Limit, + stage: Some(wire::WorkflowStage::Review), + question: Some(summarize_findings(&self.open_findings)), + }), + ), + RunState::Paused { next } => ( + next.wire(), + Some(wire::WorkflowWaiting { + kind: wire::WorkflowWaitKind::Paused, + stage: Some(next.wire()), + question: None, + }), + ), + RunState::Done => (wire::WorkflowStage::Done, None), + RunState::Failed => (wire::WorkflowStage::Failed, None), + }; + wire::WorkflowRunInfo { + workflow_id: self.spec.id.clone(), + workflow_name: self.spec.name.clone(), + stage, + round: self.round, + max_rounds: self.effective_max_rounds(), + verdict: self.last_verdict.map(Verdict::wire), + waiting, + pause_requested: self.pause_requested, + } + } + + pub fn graph_info(&self) -> wire::OrchGraphInfo { + wire::OrchGraphInfo { + id: self.parent_id.clone(), + goal: self.spec.name.clone(), + nodes: self + .history + .iter() + .map(|rec| wire::OrchNodeInfo { + id: rec.label.clone(), + kind: rec.kind.node_kind(), + agent: rec.agent.clone(), + status: rec.status, + task_id: Some(rec.task_id.clone()), + result: None, + }) + .collect(), + } + } +} + +// ─── Output parsing ────────────────────────────────────────────────────────── + +/// The machine-readable signal at the end of a plan/implement/fix stage. +#[derive(Debug, Clone, PartialEq)] +pub enum StageSignal { + /// The stage needs an answer from the user before it can continue. + Question(String), + /// Normal completion; the stage's text output is its result. + Output, +} + +/// Scan a stage's output for the trailing `need_user_input` marker. +pub fn parse_stage_signal(text: &str) -> StageSignal { + if let Some(value) = extract_last_json_with_key(text, "need_user_input") { + if let Some(q) = value.get("need_user_input").and_then(|v| v.as_str()) { + let q = q.trim(); + // Ignore an agent that echoes the protocol example back at us + // instead of asking something. + if !q.is_empty() && q != "" { + return StageSignal::Question(q.to_string()); + } + } + } + StageSignal::Output +} + +/// Parse a reviewer's verdict from its output. `Err` is a human-readable +/// reason suitable for the re-ask prompt / failure message. +pub fn parse_review_verdict(text: &str, reviewer: &str) -> Result<(Verdict, Vec), String> { + let Some(value) = extract_last_json_with_key(text, "verdict") else { + return Err("no fenced JSON block with a `verdict` field found".to_string()); + }; + let verdict = match value.get("verdict").and_then(|v| v.as_str()) { + Some("approve") => Verdict::Approve, + Some("request_changes") => Verdict::RequestChanges, + Some(other) => { + return Err(format!( + "verdict must be \"approve\" or \"request_changes\", got \"{other}\"" + )) + } + None => return Err("the `verdict` field is not a string".to_string()), + }; + let mut findings = Vec::new(); + if let Some(items) = value.get("findings").and_then(|v| v.as_array()) { + for item in items { + let description = item + .get("description") + .or_else(|| item.get("title")) + .or_else(|| item.get("message")) + .and_then(|v| v.as_str()) + .unwrap_or_default() + .trim() + .to_string(); + if description.is_empty() { + continue; + } + findings.push(Finding { + severity: item + .get("severity") + .and_then(|v| v.as_str()) + .map(Severity::parse) + .unwrap_or(Severity::Medium), + file: item + .get("file") + .and_then(|v| v.as_str()) + .map(str::to_string) + .filter(|f| !f.is_empty()), + // Agents emit the line as a number or a string, and sometimes + // as a range ("42-45") — the first number is the anchor. + line: item + .get("line") + .and_then(|v| { + v.as_u64().or_else(|| { + v.as_str().and_then(|s| { + s.trim() + .split(|c: char| !c.is_ascii_digit()) + .find(|part| !part.is_empty()) + .and_then(|part| part.parse().ok()) + }) + }) + }) + .and_then(|n| u32::try_from(n).ok()) + .filter(|n| *n > 0), + snippet: item + .get("snippet") + .or_else(|| item.get("code")) + .and_then(|v| v.as_str()) + .map(|s| clip_snippet(s.trim())) + .filter(|s| !s.is_empty()), + description, + reviewer: reviewer.to_string(), + }); + } + } + // A reviewer that asks for changes but leaves `findings` empty (prose-only + // review, a differently-named key, entries without a description) must not + // read as "nothing to fix" — that path finishes the pipeline as a success + // and silently rubber-stamps the change. Salvage its own words instead so + // the repair stage still has something concrete to act on. + if verdict == Verdict::RequestChanges && findings.is_empty() { + let prose = strip_protocol_blocks(text).trim().to_string(); + let prose = if prose.len() > SALVAGED_FINDING_MAX_BYTES { + let end = floor_char_boundary(&prose, SALVAGED_FINDING_MAX_BYTES); + format!("{}\n[…truncated…]", &prose[..end]) + } else { + prose + }; + findings.push(Finding { + severity: Severity::Medium, + file: None, + line: None, + snippet: None, + description: if prose.is_empty() { + "Changes were requested without any detail. Re-check the diff against the task \ + and fix what looks wrong." + .to_string() + } else { + format!( + "Changes requested without a structured findings list — the reviewer's own \ + words follow:\n\n{prose}" + ) + }, + reviewer: reviewer.to_string(), + }); + } + + Ok((verdict, findings)) +} + +/// The last fenced code block in `text` that parses as a JSON object +/// containing `key`. Requiring the key matters: agents routinely end a reply +/// with an unrelated fenced block (a config snippet, a quoted diff), and +/// taking the last JSON object unconditionally would misread that as the +/// protocol payload. Accepts both ```json and bare ``` fences — agents are +/// inconsistent. +fn extract_last_json_with_key(text: &str, key: &str) -> Option { + let mut best: Option = None; + let mut rest = text; + while let Some(open) = rest.find("```") { + let after_open = &rest[open + 3..]; + // Skip the info string ("json", "JSON", …) up to the first newline. + let Some(nl) = after_open.find('\n') else { + break; + }; + let body_start = nl + 1; + let Some(close) = after_open[body_start..].find("```") else { + break; + }; + let body = &after_open[body_start..body_start + close]; + if let Ok(value) = serde_json::from_str::(body.trim()) { + if value.get(key).is_some() { + best = Some(value); + } + } + rest = &after_open[body_start + close + 3..]; + } + best +} + +/// Merge one round's reviewer results: approve only when everyone approves; +/// findings are concatenated in reviewer order. +pub fn merge_reviews(collected: &[(usize, Verdict, Vec)]) -> (Verdict, Vec) { + let mut sorted: Vec<_> = collected.iter().collect(); + sorted.sort_by_key(|(idx, _, _)| *idx); + let verdict = if sorted + .iter() + .all(|(_, verdict, _)| *verdict == Verdict::Approve) + { + Verdict::Approve + } else { + Verdict::RequestChanges + }; + let findings = sorted + .iter() + .flat_map(|(_, _, findings)| findings.iter().cloned()) + .collect(); + (verdict, findings) +} + +// ─── Context formatting ────────────────────────────────────────────────────── + +/// Render a working-copy diff into prompt text, truncated to +/// [`DIFF_CONTEXT_MAX_BYTES`] with an explicit truncation note. +pub fn format_diff(files: &[wire::FileDiff]) -> String { + if files.is_empty() { + return "(no changes in the working copy)".to_string(); + } + let mut out = String::new(); + let mut truncated = false; + 'files: for file in files { + let header = match (&file.status, &file.old_path) { + (wire::FileDiffStatus::Renamed, Some(old)) => { + format!("--- {old}\n+++ {} (renamed)\n", file.path) + } + (wire::FileDiffStatus::Added, _) => format!("+++ {} (added)\n", file.path), + (wire::FileDiffStatus::Deleted, _) => format!("--- {} (deleted)\n", file.path), + _ => format!("--- a/{p}\n+++ b/{p}\n", p = file.path), + }; + out.push_str(&header); + for hunk in &file.hunks { + out.push_str(&format!( + "@@ -{},{} +{},{} @@\n", + hunk.old_start, hunk.old_lines, hunk.new_start, hunk.new_lines + )); + for line in &hunk.lines { + out.push_str(line); + out.push('\n'); + } + if out.len() > DIFF_CONTEXT_MAX_BYTES { + truncated = true; + break 'files; + } + } + out.push('\n'); + } + if truncated { + out.truncate(floor_char_boundary(&out, DIFF_CONTEXT_MAX_BYTES)); + out.push_str("\n\n[diff truncated — full list of changed files:]\n"); + for file in files { + out.push_str(&format!("- {}\n", file.path)); + } + } + out +} + +/// Strip the trailing machine protocol block (the `verdict` / `need_user_input` +/// JSON fence) so the parent's timeline shows the agent's prose instead of the +/// wire format it was asked to emit. Falls back to the full text when nothing +/// matches, and always clips to the summary budget. +pub fn display_output(text: &str) -> String { + let trimmed = strip_protocol_blocks(text).trim().to_string(); + let trimmed = trimmed.as_str(); + if trimmed.is_empty() { + // The whole reply was the protocol block — show it rather than an + // empty card. + return clip_summary(text.trim()); + } + clip_summary(trimmed) +} + +/// Drop the trailing machine protocol fences from an agent reply. +fn strip_protocol_blocks(text: &str) -> &str { + let mut visible = text; + for key in ["verdict", "need_user_input"] { + if let Some(start) = protocol_block_start(visible, key) { + visible = &visible[..start]; + } + } + visible +} + +/// Byte offset of the fence that opens the last JSON block containing `key`. +fn protocol_block_start(text: &str, key: &str) -> Option { + let mut found = None; + let mut cursor = 0; + while let Some(open) = text[cursor..].find("```").map(|p| cursor + p) { + let after_open = open + 3; + let Some(nl) = text[after_open..].find('\n').map(|p| after_open + p + 1) else { + break; + }; + let Some(close) = text[nl..].find("```").map(|p| nl + p) else { + break; + }; + if serde_json::from_str::(text[nl..close].trim()) + .ok() + .and_then(|value| value.get(key).cloned()) + .is_some() + { + found = Some(open); + } + cursor = close + 3; + } + found +} + +/// Keep the tail of a long implementer summary (the conclusion matters most). +pub fn clip_summary(summary: &str) -> String { + if summary.len() <= SUMMARY_CONTEXT_MAX_BYTES { + return summary.to_string(); + } + let start = ceil_char_boundary(summary, summary.len() - SUMMARY_CONTEXT_MAX_BYTES); + format!("[…truncated…]\n{}", &summary[start..]) +} + +fn floor_char_boundary(s: &str, mut i: usize) -> usize { + if i >= s.len() { + return s.len(); + } + while i > 0 && !s.is_char_boundary(i) { + i -= 1; + } + i +} + +fn ceil_char_boundary(s: &str, mut i: usize) -> usize { + while i < s.len() && !s.is_char_boundary(i) { + i += 1; + } + i +} + +pub fn format_findings(findings: &[Finding]) -> String { + findings + .iter() + .enumerate() + .map(|(i, f)| { + let location = match (f.file.as_deref(), f.line) { + (Some(path), Some(line)) => format!(" `{path}:{line}`"), + (Some(path), None) => format!(" `{path}`"), + (None, _) => String::new(), + }; + let mut entry = format!( + "{}. [{}]{} — {} ({})", + i + 1, + f.severity.label(), + location, + f.description, + f.reviewer + ); + if let Some(snippet) = f.snippet.as_deref() { + // A verbatim excerpt survives line drift: the fixer can search + // for it even after the file has moved underneath the number. + entry.push_str(&format!("\n at: `{snippet}`")); + } + entry + }) + .collect::>() + .join("\n") +} + +/// Keep a code anchor short: a reviewer that pastes a whole function would +/// otherwise dominate the repair prompt. +fn clip_snippet(snippet: &str) -> String { + const MAX: usize = 200; + let one_line = snippet.replace('\n', " ").trim().to_string(); + if one_line.len() <= MAX { + return one_line; + } + let end = floor_char_boundary(&one_line, MAX); + format!("{}…", &one_line[..end]) +} + +/// True when `text` carries either machine protocol payload. Used to decide +/// whether an agent's closing message is the authoritative output or whether +/// the payload only appears earlier in the turn. +pub fn has_protocol_payload(text: &str) -> bool { + extract_last_json_with_key(text, "verdict").is_some() + || extract_last_json_with_key(text, "need_user_input").is_some() +} + +/// Short one-line findings summary for the limit-decision prompt. +pub fn summarize_findings(findings: &[Finding]) -> String { + let mut counts: [usize; 4] = [0; 4]; + for f in findings { + counts[match f.severity { + Severity::Critical => 0, + Severity::High => 1, + Severity::Medium => 2, + Severity::Low => 3, + }] += 1; + } + let parts: Vec = [ + (counts[0], "critical"), + (counts[1], "high"), + (counts[2], "medium"), + (counts[3], "low"), + ] + .iter() + .filter(|(n, _)| *n > 0) + .map(|(n, label)| format!("{n} {label}")) + .collect(); + if parts.is_empty() { + "no open findings".to_string() + } else { + format!("open findings: {}", parts.join(", ")) + } +} + +// ─── Prompt building ───────────────────────────────────────────────────────── + +/// Everything a stage prompt can draw on. Owned strings so the actor can +/// assemble it without borrow gymnastics. +#[derive(Debug, Default, Clone)] +pub struct PromptCtx { + pub task_prompt: String, + pub plan: Option, + pub implementer_summary: Option, + pub diff: Option, + /// Pre-formatted findings list (fix stage). + pub findings: Option, + /// Previous round's findings (repeat review rounds) — the reviewer must + /// verify each one instead of reviewing from scratch. + pub prior_findings: Option, + pub round: u32, + pub max_rounds: u32, + pub guidance: Option, +} + +/// Appended to every plan/implement/fix prompt — the question protocol the +/// engine's `parse_stage_signal` understands. +const QUESTION_PROTOCOL: &str = "This stage runs unattended, so stop for the user only when you \ +genuinely cannot proceed — a decision only they can make, or missing access. Otherwise pick the \ +most reasonable option, proceed, and record the assumption in your final message. When you do \ +need them, end your reply with exactly one fenced code block of this shape and stop:\n\ +```json\n{\"need_user_input\": \"\"}\n```\n\ +Otherwise, do not emit such a block."; + +/// Appended to every reviewer prompt — the verdict protocol the engine's +/// `parse_review_verdict` understands. +const VERDICT_PROTOCOL: &str = "Scope: judge correctness, regressions, and whether the task's \ +stated requirements are met. Style, naming, and improvements nobody asked for are `low` at most. \ +Report real problems only — do not invent findings to seem thorough. You may run read-only \ +verification (build, tests, linters) to check your reasoning, but do not edit files.\n\n\ +You MUST end your reply with exactly one fenced code block, and nothing after it, of this \ +shape:\n\ +```json\n{\"verdict\": \"request_changes\", \"findings\": [{\"severity\": \"high\", \"file\": \"src/example.rs\", \"line\": 42, \"snippet\": \"if attempts > max {\", \"description\": \"what is wrong and why\"}]}\n```\n\ +Anchor every finding you can: `line` is the line in the file AFTER the change \ +(each diff hunk header `@@ -old +new @@` gives you the new-file line its body \ +starts at), and `snippet` is one short verbatim line of the offending code. \ +Both are optional but they let the repair stage go straight to the right place \ +instead of searching.\n\ +When you approve, send `{\"verdict\": \"approve\", \"findings\": []}` — use `approve` only when no \ +critical, high, or medium problem remains. `findings` MUST be a JSON array (empty when there are \ +none) and every entry MUST carry a `description`: anything you only write in prose is invisible \ +to the pipeline. `severity` is critical, high, medium, or low, and `file` may be null. Note that \ +`low` findings are recorded for the human but are NEVER sent to the repair stage — use `medium` \ +or higher for anything you actually want fixed."; + +fn vars_from_ctx(ctx: &PromptCtx, focus: Option<&str>) -> HashMap<&'static str, String> { + let mut vars: HashMap<&'static str, String> = HashMap::new(); + vars.insert("task_prompt", ctx.task_prompt.clone()); + vars.insert("plan", ctx.plan.clone().unwrap_or_default()); + vars.insert( + "implementer_summary", + ctx.implementer_summary.clone().unwrap_or_default(), + ); + vars.insert("diff", ctx.diff.clone().unwrap_or_default()); + vars.insert("findings", ctx.findings.clone().unwrap_or_default()); + vars.insert("round", ctx.round.to_string()); + vars.insert("max_rounds", ctx.max_rounds.to_string()); + vars.insert("focus", focus.unwrap_or_default().to_string()); + vars +} + +fn push_section(out: &mut String, title: &str, body: &str) { + if body.trim().is_empty() { + return; + } + out.push_str("## "); + out.push_str(title); + out.push('\n'); + out.push_str(body.trim_end()); + out.push_str("\n\n"); +} + +fn finish_prompt(mut body: String, protocol: &str, guidance: Option<&str>) -> String { + if let Some(guidance) = guidance { + if !guidance.trim().is_empty() { + push_section(&mut body, "User guidance", guidance); + } + } + let body = body.trim_end(); + format!("{body}\n\n---\n{protocol}") +} + +pub fn build_plan_prompt(spec: &WorkflowSpec, ctx: &PromptCtx) -> String { + let body = match spec.plan.as_ref().and_then(|s| s.prompt.as_deref()) { + Some(custom) => render_template(custom, &vars_from_ctx(ctx, None)), + None => { + let mut out = String::from( + "You are the planning stage of a workflow pipeline. Explore the codebase as \ + needed and produce a concise implementation plan: the files to touch, the \ + approach, edge cases, and how to verify the result. Do NOT edit any files — \ + this stage is planning only. Your final message is handed to the implementer \ + verbatim, so end with the complete plan.\n\n", + ); + push_section(&mut out, "Task", &ctx.task_prompt); + out + } + }; + finish_prompt(body, QUESTION_PROTOCOL, ctx.guidance.as_deref()) +} + +pub fn build_implement_prompt(spec: &WorkflowSpec, ctx: &PromptCtx) -> String { + let body = match spec.implement.prompt.as_deref() { + Some(custom) => render_template(custom, &vars_from_ctx(ctx, None)), + None => { + let mut out = String::from( + "You are the implementation stage of a workflow pipeline. Implement the task \ + below completely: write the code, keep the change focused, and verify your \ + work (build/tests) where feasible. Your final message should summarize what \ + you did — it is handed to the reviewers.\n\n", + ); + push_section(&mut out, "Task", &ctx.task_prompt); + if let Some(plan) = ctx.plan.as_deref() { + push_section(&mut out, "Approved plan", plan); + } + out + } + }; + finish_prompt(body, QUESTION_PROTOCOL, ctx.guidance.as_deref()) +} + +pub fn build_reviewer_prompt(spec: &WorkflowSpec, reviewer: usize, ctx: &PromptCtx) -> String { + let config = &spec.review.reviewers[reviewer]; + let focus = config.focus.as_deref(); + let body = match config.prompt.as_deref() { + Some(custom) => render_template(custom, &vars_from_ctx(ctx, focus)), + None => { + let mut out = format!( + "You are a code reviewer in a workflow pipeline (round {}/{}). Review the \ + changes below against the task. Do NOT edit any files — review only. Judge \ + what is actually there: verify claims against the diff, and flag real \ + problems with concrete evidence.\n\n", + ctx.round, ctx.max_rounds + ); + if let Some(focus) = focus { + push_section(&mut out, "Your focus", focus); + } + for item in &spec.review.context { + match item { + ReviewContextItem::Prompt => push_section(&mut out, "Task", &ctx.task_prompt), + ReviewContextItem::Plan => { + if let Some(plan) = ctx.plan.as_deref() { + push_section(&mut out, "Approved plan", plan); + } + } + ReviewContextItem::ImplementerSummary => { + if let Some(summary) = ctx.implementer_summary.as_deref() { + push_section(&mut out, "Implementer's summary", summary); + } + } + ReviewContextItem::Diff => { + if let Some(diff) = ctx.diff.as_deref() { + push_section(&mut out, "Working-copy diff", diff); + } + } + } + } + out + } + }; + let mut body = body; + if let Some(prior) = ctx.prior_findings.as_deref() { + if !body.ends_with('\n') { + body.push('\n'); + } + body.push('\n'); + push_section( + &mut body, + "Previous round's findings — verify each one", + &format!( + "{prior}\n\nThis is a repeat review after a repair pass. For every finding \ + above, state whether it is actually resolved in the current diff — do not take \ + the fixer's summary on faith; reopen anything that is not. Then check the rest \ + of the changes for regressions the repair may have introduced, including code \ + that was fine last round." + ), + ); + } + // Reviewers get no guidance block — guidance targets implement/fix. + finish_prompt(body, VERDICT_PROTOCOL, None) +} + +/// Follow-up message sent into a reviewer's EXISTING session for a repeat +/// round (`review.reask: same_session`). The session already holds the task, +/// the original diff, and this reviewer's own reasoning, so the follow-up +/// carries only what changed since — plus the explicit anti-anchoring +/// instructions: verify every prior finding and re-check for regressions. +pub fn build_rereview_prompt(ctx: &PromptCtx) -> String { + let mut out = format!( + "The repair stage has addressed your review. This is review round {}/{}.\n\n", + ctx.round, ctx.max_rounds + ); + if let Some(summary) = ctx.implementer_summary.as_deref() { + push_section(&mut out, "Fixer's summary", summary); + } + if let Some(findings) = ctx.prior_findings.as_deref() { + push_section( + &mut out, + "Findings raised last round (all reviewers)", + findings, + ); + } + if let Some(diff) = ctx.diff.as_deref() { + push_section(&mut out, "Current working-copy diff", diff); + } + push_section( + &mut out, + "What to do", + "Go through each finding above and verify against the current diff that it is actually \ + resolved — do not take the fixer's summary on faith; reopen anything that is not. Then \ + check the changes for regressions the repair may have introduced, including code you \ + found fine last round. Defending your earlier verdict is not a goal — accuracy is.", + ); + finish_prompt(out, VERDICT_PROTOCOL, None) +} + +pub fn build_fix_prompt(spec: &WorkflowSpec, ctx: &PromptCtx) -> String { + let body = match spec.fix.prompt.as_deref() { + Some(custom) => render_template(custom, &vars_from_ctx(ctx, None)), + None => { + let mut out = format!( + "You are the repair stage of a workflow pipeline (round {}/{}). Reviewers \ + found the problems listed below. Address every finding — fix it or, when a \ + finding is factually wrong, explain why in your summary. Do not change \ + unrelated code. Your final message should summarize what you changed; it is \ + handed back to the reviewers.\n\n", + ctx.round, ctx.max_rounds + ); + push_section(&mut out, "Task", &ctx.task_prompt); + if let Some(findings) = ctx.findings.as_deref() { + push_section(&mut out, "Findings to address", findings); + } + if let Some(diff) = ctx.diff.as_deref() { + push_section(&mut out, "Current working-copy diff", diff); + } + out + } + }; + finish_prompt(body, QUESTION_PROTOCOL, ctx.guidance.as_deref()) +} + +/// Follow-up sent to a reviewer whose output had no parseable verdict. +pub fn reask_verdict_prompt(reason: &str) -> String { + format!( + "Your previous reply could not be parsed: {reason}. Reply with ONLY the fenced JSON \ + verdict block:\n```json\n{{\"verdict\": \"approve\" | \"request_changes\", \ + \"findings\": [{{\"severity\": \"…\", \"file\": \"…\", \"description\": \"…\"}}]}}\n```" + ) +} + +// ─── Tests ─────────────────────────────────────────────────────────────────── + +#[cfg(test)] +mod tests { + use super::*; + use crate::workflow_config::parse_workflow; + + fn spec(yaml: &str) -> WorkflowSpec { + parse_workflow("test", yaml).0.expect("valid spec") + } + + fn run_for(yaml: &str) -> WorkflowRun { + WorkflowRun::new( + "t_parent".into(), + "demo".into(), + spec(yaml), + "claude".into(), + Some("lead-model".into()), + vec![], + false, + HashMap::new(), + ) + } + + #[test] + fn stage_signal_detects_trailing_question() { + let text = "I looked around.\n```json\n{\"need_user_input\": \"Which database?\"}\n```\n"; + assert_eq!( + parse_stage_signal(text), + StageSignal::Question("Which database?".into()) + ); + assert_eq!( + parse_stage_signal("all done, no questions"), + StageSignal::Output + ); + // An empty question is not a question. + assert_eq!( + parse_stage_signal("```json\n{\"need_user_input\": \" \"}\n```"), + StageSignal::Output + ); + } + + #[test] + fn verdict_parsing_happy_path_and_aliases() { + let text = r#"Review done. +```json +{"verdict": "request_changes", "findings": [ + {"severity": "HIGH", "file": "src/a.rs", "description": "off-by-one"}, + {"severity": "nit", "description": "typo"}, + {"severity": "weird", "title": "fallback title"}, + {"description": ""} +]} +```"#; + let (verdict, findings) = parse_review_verdict(text, "reviewer 1 (claude)").unwrap(); + assert_eq!(verdict, Verdict::RequestChanges); + assert_eq!(findings.len(), 3, "empty description dropped"); + assert_eq!(findings[0].severity, Severity::High); + assert_eq!(findings[0].file.as_deref(), Some("src/a.rs")); + assert_eq!(findings[1].severity, Severity::Low); + assert_eq!(findings[2].severity, Severity::Medium); + assert_eq!(findings[2].description, "fallback title"); + assert!(findings.iter().all(|f| f.reviewer == "reviewer 1 (claude)")); + } + + #[test] + fn verdict_parsing_uses_last_json_block_and_bare_fences() { + let text = "```json\n{\"verdict\": \"approve\"}\n```\nwait, actually:\n```\n{\"verdict\": \"request_changes\", \"findings\": []}\n```"; + let (verdict, _) = parse_review_verdict(text, "r").unwrap(); + assert_eq!(verdict, Verdict::RequestChanges); + } + + #[test] + fn verdict_parsing_errors() { + assert!(parse_review_verdict("no block at all", "r") + .unwrap_err() + .contains("no fenced JSON")); + // A JSON block without a `verdict` key is not the protocol payload. + assert!( + parse_review_verdict("```json\n{\"findings\": []}\n```", "r") + .unwrap_err() + .contains("no fenced JSON") + ); + assert!( + parse_review_verdict("```json\n{\"verdict\": \"maybe\"}\n```", "r") + .unwrap_err() + .contains("\"maybe\"") + ); + // Non-JSON fenced blocks are skipped, not fatal. + let text = "```rust\nfn x() {}\n```\n```json\n{\"verdict\": \"approve\"}\n```"; + assert!(parse_review_verdict(text, "r").is_ok()); + } + + #[test] + fn verdict_parsing_reads_finding_anchors() { + let text = r#"```json +{"verdict": "request_changes", "findings": [ + {"severity": "high", "file": "src/a.rs", "line": 42, "snippet": "if attempts > max {", "description": "off-by-one"}, + {"severity": "high", "file": "src/b.rs", "line": "17-20", "code": "let x = 1;", "description": "string range"}, + {"severity": "low", "line": 0, "description": "no anchor"} +]} +```"#; + let (_, findings) = parse_review_verdict(text, "r").unwrap(); + assert_eq!(findings[0].line, Some(42)); + assert_eq!(findings[0].snippet.as_deref(), Some("if attempts > max {")); + // A range and a `code` alias both resolve. + assert_eq!(findings[1].line, Some(17)); + assert_eq!(findings[1].snippet.as_deref(), Some("let x = 1;")); + // Line 0 is not a location. + assert_eq!(findings[2].line, None); + assert_eq!(findings[2].snippet, None); + + // Anchors reach the repair prompt. + let rendered = format_findings(&findings); + assert!(rendered.contains("`src/a.rs:42`"), "{rendered}"); + assert!(rendered.contains("at: `if attempts > max {`"), "{rendered}"); + assert!(rendered.contains("`src/b.rs:17`"), "{rendered}"); + + // An oversized excerpt is clipped to one line. + let long = format!( + "```json\n{{\"verdict\": \"approve\", \"findings\": [{{\"description\": \"d\", \"snippet\": \"{}\"}}]}}\n```", + "x".repeat(400) + ); + let (_, findings) = parse_review_verdict(&long, "r").unwrap(); + let snippet = findings[0].snippet.as_deref().unwrap(); + assert!(snippet.len() <= 204, "{}", snippet.len()); + assert!(snippet.ends_with('…')); + } + + #[test] + fn protocol_payload_detection() { + assert!(has_protocol_payload( + "```json\n{\"verdict\": \"approve\"}\n```" + )); + assert!(has_protocol_payload( + "```json\n{\"need_user_input\": \"which db?\"}\n```" + )); + // An unrelated JSON block is not a protocol payload — this is what + // keeps a quoted config file from hijacking a stage's result. + assert!(!has_protocol_payload("```json\n{\"name\": \"pkg\"}\n```")); + assert!(!has_protocol_payload("just prose")); + } + + #[test] + fn merge_reviews_requires_unanimous_approve() { + let f = |desc: &str| Finding { + severity: Severity::High, + file: None, + line: None, + snippet: None, + description: desc.into(), + reviewer: "r".into(), + }; + let (verdict, findings) = merge_reviews(&[ + (1, Verdict::Approve, vec![f("b")]), + (0, Verdict::RequestChanges, vec![f("a")]), + ]); + assert_eq!(verdict, Verdict::RequestChanges); + // Findings ordered by reviewer index. + assert_eq!(findings[0].description, "a"); + assert_eq!(findings[1].description, "b"); + + let (verdict, _) = + merge_reviews(&[(0, Verdict::Approve, vec![]), (1, Verdict::Approve, vec![])]); + assert_eq!(verdict, Verdict::Approve); + } + + #[test] + fn stage_agent_fallback_chain() { + let run = run_for( + "name: X\nplan: {}\nimplement:\n agent: codex\n model: gpt-x\nreview:\n reviewers:\n - agent: opencode\n - {}\n", + ); + // plan has no override → lead agent + lead model. + assert_eq!( + run.stage_agent(StageKind::Plan, None), + ("claude".into(), Some("lead-model".into())) + ); + // implement overrides both. + assert_eq!( + run.stage_agent(StageKind::Implement, None), + ("codex".into(), Some("gpt-x".into())) + ); + // fix falls back to implement's overrides. + assert_eq!( + run.stage_agent(StageKind::Fix, None), + ("codex".into(), Some("gpt-x".into())) + ); + // reviewer 0 overrides agent, inherits lead model; reviewer 1 → lead. + assert_eq!( + run.stage_agent(StageKind::Review, Some(0)), + ("opencode".into(), Some("lead-model".into())) + ); + assert_eq!( + run.stage_agent(StageKind::Review, Some(1)), + ("claude".into(), Some("lead-model".into())) + ); + } + + #[test] + fn first_stage_respects_plan_presence() { + assert_eq!(run_for("name: X\n").first_stage(), StageKind::Implement); + assert_eq!( + run_for("name: X\nplan: {}\n").first_stage(), + StageKind::Plan + ); + } + + #[test] + fn default_prompts_include_context_and_protocols() { + let run = run_for("name: X\nplan: {}\nreview:\n reviewers:\n - focus: security only\n"); + let ctx = PromptCtx { + task_prompt: "Add rate limiting".into(), + plan: Some("1. do things".into()), + implementer_summary: Some("did things".into()), + diff: Some("--- a/x\n+++ b/x".into()), + findings: Some("1. [high] — bug (r)".into()), + prior_findings: None, + round: 1, + max_rounds: 2, + guidance: Some("prefer tower middleware".into()), + }; + + let plan = build_plan_prompt(&run.spec, &ctx); + assert!(plan.contains("Add rate limiting")); + assert!(plan.contains("need_user_input")); + assert!(plan.contains("User guidance")); + + let implement = build_implement_prompt(&run.spec, &ctx); + assert!(implement.contains("Approved plan")); + assert!(implement.contains("1. do things")); + + let review = build_reviewer_prompt(&run.spec, 0, &ctx); + assert!(review.contains("security only")); + assert!(review.contains("Working-copy diff")); + assert!(review.contains("\"verdict\"")); + assert!( + !review.contains("User guidance"), + "guidance must not leak to reviewers" + ); + + let fix = build_fix_prompt(&run.spec, &ctx); + assert!(fix.contains("Findings to address")); + assert!(fix.contains("round 1/2")); + } + + #[test] + fn custom_prompts_render_placeholders_and_keep_protocol() { + let run = run_for( + "name: X\nimplement:\n prompt: \"Do {{task_prompt}} now\"\nreview:\n reviewers:\n - prompt: \"{{focus}} check {{diff}} r{{round}}\"\n focus: perf\n", + ); + let ctx = PromptCtx { + task_prompt: "the thing".into(), + diff: Some("DIFF".into()), + round: 2, + max_rounds: 3, + ..Default::default() + }; + let implement = build_implement_prompt(&run.spec, &ctx); + assert!(implement.starts_with("Do the thing now")); + assert!( + implement.contains("need_user_input"), + "protocol always appended" + ); + + let review = build_reviewer_prompt(&run.spec, 0, &ctx); + assert!(review.starts_with("perf check DIFF r2")); + assert!(review.contains("\"verdict\"")); + } + + #[test] + fn repeat_round_reviewer_prompt_verifies_prior_findings() { + let run = run_for("name: X\nreview:\n reviewers:\n - focus: security\n"); + let base = PromptCtx { + task_prompt: "Add rate limiting".into(), + diff: Some("DIFF".into()), + round: 2, + max_rounds: 3, + ..Default::default() + }; + // Round 1 (no prior findings): no verification section. + let first = build_reviewer_prompt(&run.spec, 0, &base); + assert!(!first.contains("Previous round's findings")); + + let ctx = PromptCtx { + prior_findings: Some("1. [high] `a.rs` — bug here (reviewer)".into()), + ..base.clone() + }; + let repeat = build_reviewer_prompt(&run.spec, 0, &ctx); + assert!(repeat.contains("Previous round's findings — verify each one")); + assert!(repeat.contains("bug here")); + assert!(repeat.contains("regressions")); + + // The verification section is engine-owned: custom reviewer prompts + // get it appended too, without any template changes. + let custom = run_for("name: X\nreview:\n reviewers:\n - prompt: \"check {{diff}}\"\n"); + let repeat = build_reviewer_prompt(&custom.spec, 0, &ctx); + assert!(repeat.starts_with("check DIFF")); + assert!(repeat.contains("Previous round's findings — verify each one")); + assert!( + repeat.contains("\"verdict\""), + "protocol still appended last" + ); + } + + #[test] + fn rereview_followup_carries_delta_and_instructions() { + let ctx = PromptCtx { + task_prompt: "irrelevant — the session already has it".into(), + implementer_summary: Some("FIX-DONE: patched the parser".into()), + prior_findings: Some("1. [high] — off-by-one (reviewer)".into()), + diff: Some("THE-NEW-DIFF".into()), + round: 2, + max_rounds: 2, + ..Default::default() + }; + let followup = build_rereview_prompt(&ctx); + assert!(followup.starts_with("The repair stage has addressed your review")); + assert!(followup.contains("round 2/2")); + assert!(followup.contains("FIX-DONE: patched the parser")); + assert!(followup.contains("off-by-one")); + assert!(followup.contains("THE-NEW-DIFF")); + assert!(followup.contains("regressions")); + assert!( + followup.contains("\"verdict\""), + "verdict protocol reminder" + ); + // The session already has the task prompt — the follow-up must not + // re-send it. + assert!(!followup.contains("irrelevant — the session already has it")); + } + + #[test] + fn all_children_spans_history_and_active() { + let mut run = run_for("name: X\n"); + run.record_stage(StageKind::Implement, "t_impl", "claude", "implement".into()); + run.record_stage(StageKind::Review, "t_rev", "claude", "reviewer".into()); + run.active_children.insert("t_fix".into(), StageKind::Fix); + let all = run.all_children(); + assert_eq!(all.len(), 3); + for id in ["t_impl", "t_rev", "t_fix"] { + assert!(all.contains(id), "{id} missing"); + } + } + + #[test] + fn diff_formatting_and_truncation() { + let hunk = wire::Hunk { + old_start: 1, + old_lines: 1, + new_start: 1, + new_lines: 1, + lines: vec!["-old".into(), "+new".into()], + resolution: None, + }; + let small = vec![wire::FileDiff { + path: "src/a.rs".into(), + old_path: None, + status: wire::FileDiffStatus::Modified, + hunks: vec![hunk.clone()], + }]; + let text = format_diff(&small); + assert!(text.contains("--- a/src/a.rs")); + assert!(text.contains("+new")); + + let big_hunk = wire::Hunk { + lines: vec!["+x".to_string(); 300_000], + ..hunk + }; + let big = vec![ + wire::FileDiff { + path: "src/big.rs".into(), + old_path: None, + status: wire::FileDiffStatus::Modified, + hunks: vec![big_hunk], + }, + wire::FileDiff { + path: "src/other.rs".into(), + old_path: None, + status: wire::FileDiffStatus::Added, + hunks: vec![], + }, + ]; + let text = format_diff(&big); + assert!(text.len() < DIFF_CONTEXT_MAX_BYTES + 1024); + assert!(text.contains("[diff truncated")); + assert!( + text.contains("- src/other.rs"), + "file list survives truncation" + ); + + assert_eq!(format_diff(&[]), "(no changes in the working copy)"); + } + + #[test] + fn summary_clip_keeps_tail() { + let long = format!("{}THE-END", "x".repeat(SUMMARY_CONTEXT_MAX_BYTES * 2)); + let clipped = clip_summary(&long); + assert!(clipped.len() <= SUMMARY_CONTEXT_MAX_BYTES + 32); + assert!(clipped.ends_with("THE-END")); + assert!(clipped.starts_with("[…truncated…]")); + assert_eq!(clip_summary("short"), "short"); + } + + #[test] + fn wire_info_reflects_state() { + let mut run = run_for("name: My flow\nreview:\n max_rounds: 2\n"); + run.state = RunState::Running { + stage: StageKind::Implement, + }; + let info = run.wire_info(); + assert_eq!(info.stage, wire::WorkflowStage::Implement); + assert!(info.waiting.is_none()); + assert!(!info.pause_requested); + assert_eq!(info.max_rounds, 2); + + // A requested pause is visible before it takes effect, so the UI can + // show progress rather than an idle Pause button. + run.pause_requested = true; + assert!(run.wire_info().pause_requested); + run.pause_requested = false; + + run.extra_rounds = 2; + run.state = RunState::AwaitingReply { + stage: StageKind::Plan, + child: "t_c".into(), + question: "which db?".into(), + }; + let info = run.wire_info(); + assert_eq!(info.max_rounds, 4); + let waiting = info.waiting.unwrap(); + assert_eq!(waiting.kind, wire::WorkflowWaitKind::Question); + assert_eq!(waiting.question.as_deref(), Some("which db?")); + + run.open_findings = vec![Finding { + severity: Severity::High, + file: None, + line: None, + snippet: None, + description: "d".into(), + reviewer: "r".into(), + }]; + run.state = RunState::AwaitingLimitDecision; + let waiting = run.wire_info().waiting.unwrap(); + assert_eq!(waiting.kind, wire::WorkflowWaitKind::Limit); + assert_eq!(waiting.question.as_deref(), Some("open findings: 1 high")); + + run.state = RunState::Paused { + next: StageKind::Fix, + }; + let info = run.wire_info(); + assert_eq!(info.stage, wire::WorkflowStage::Fix); + assert_eq!(info.waiting.unwrap().kind, wire::WorkflowWaitKind::Paused); + } + + #[test] + fn graph_info_maps_history() { + let mut run = run_for("name: X\n"); + run.record_stage(StageKind::Implement, "t_1", "claude", "implement".into()); + run.set_record_status("t_1", wire::OrchNodeStatus::Complete); + run.record_stage( + StageKind::Review, + "t_2", + "codex", + "reviewer 1/2 (codex)".into(), + ); + let graph = run.graph_info(); + assert_eq!(graph.goal, "X"); + assert_eq!(graph.nodes.len(), 2); + assert_eq!(graph.nodes[0].status, wire::OrchNodeStatus::Complete); + assert_eq!(graph.nodes[0].kind, wire::OrchNodeKind::Implement); + assert_eq!(graph.nodes[1].task_id.as_deref(), Some("t_2")); + assert_eq!(graph.nodes[1].kind, wire::OrchNodeKind::Review); + } + + #[test] + fn run_serialization_roundtrip() { + let mut run = run_for("name: X\nplan: {}\n"); + run.state = RunState::Paused { + next: StageKind::Review, + }; + run.round = 2; + run.open_findings = vec![Finding { + severity: Severity::Critical, + file: Some("a.rs".into()), + line: Some(7), + snippet: Some("let x = 1;".into()), + description: "boom".into(), + reviewer: "r".into(), + }]; + let json = serde_json::to_string(&run).unwrap(); + let restored: WorkflowRun = serde_json::from_str(&json).unwrap(); + assert_eq!(restored.state, run.state); + assert_eq!(restored.round, 2); + assert_eq!(restored.spec, run.spec); + assert_eq!(restored.open_findings, run.open_findings); + } +} diff --git a/src/main.rs b/src/main.rs index 97cad53..3612a55 100644 --- a/src/main.rs +++ b/src/main.rs @@ -14,6 +14,7 @@ mod ports; mod registry; mod service; mod tui; +mod workflow_config; use anyhow::{anyhow, Context, Result}; use clap::{Parser, Subcommand}; @@ -40,7 +41,7 @@ enum Commands { Remove { name: String }, /// List registered projects List, - /// Generate .warpforge.yaml in the current (or given) directory + /// Generate .warpforge/workspace.yaml in the current (or given) directory Init { /// Directory to init (defaults to current directory) path: Option, @@ -48,7 +49,7 @@ enum Commands { #[arg(short, long)] add: bool, }, - /// Interactively generate a .warpforge.yaml with an agent. Registers the + /// Interactively generate a workspace config with an agent. Registers the /// project if needed, asks the daemon to run a bootstrap task, then lets you /// accept / edit / discard the proposed config. Bootstrap { @@ -102,8 +103,10 @@ async fn main() -> Result<()> { let config_file = config::find_config_file(std::path::Path::new(&entry.path)); if !config_file.exists() { match config::generate_workspace_yaml(std::path::Path::new(&entry.path)) { - Ok(_) => println!("Created .warpforge.yaml — edit it to configure services"), - Err(e) => println!("Note: could not create .warpforge.yaml: {}", e), + Ok(_) => println!( + "Created .warpforge/workspace.yaml — edit it to configure services" + ), + Err(e) => println!("Note: could not create .warpforge/workspace.yaml: {}", e), } } } @@ -154,7 +157,7 @@ async fn main() -> Result<()> { /// Interactive CLI bootstrap: register the project, ask a few runtime /// questions, run a config-gen task on the daemon, then review + write the -/// proposed `.warpforge.yaml`. The daemon owns the repo scan, prompt building, +/// proposed workspace config. The daemon owns the repo scan, prompt building, /// and validation (see `bootstrap.*` RPCs); this only drives the flow. async fn run_bootstrap(path: &str) -> Result<()> { use std::io::{self, Write}; @@ -250,7 +253,7 @@ async fn run_bootstrap(path: &str) -> Result<()> { .unwrap_or_default(); let yaml = bootstrap::extract_yaml_from_response(&response); - println!("\n── Proposed .warpforge.yaml ──\n{yaml}\n──────────────────────────────"); + println!("\n── Proposed workspace config ──\n{yaml}\n───────────────────────────────"); match bootstrap::validate_config_yaml(&yaml) { Ok((_, issues)) => { for issue in &issues { @@ -269,6 +272,10 @@ async fn run_bootstrap(path: &str) -> Result<()> { .starts_with('y') { let target = config::find_config_file(std::path::Path::new(path)); + if let Some(parent) = target.parent() { + std::fs::create_dir_all(parent) + .with_context(|| format!("creating {}", parent.display()))?; + } std::fs::write(&target, &yaml).with_context(|| format!("writing {}", target.display()))?; println!("Wrote {}", target.display()); } else { diff --git a/src/workflow_config.rs b/src/workflow_config.rs new file mode 100644 index 0000000..5826abd --- /dev/null +++ b/src/workflow_config.rs @@ -0,0 +1,1026 @@ +//! Workflow templates: `.warpforge/workflows/*.yaml` parsing and validation, +//! `{{placeholder}}` prompt-template rendering, and the built-in templates +//! shipped with the binary. +//! +//! The pipeline shape is fixed (`plan? → implement → review ⇄ fix`), so a +//! workflow file configures the fixed stages rather than declaring arbitrary +//! ones. +//! +//! This module is deliberately independent of daemon internals: the daemon's +//! workflow engine consumes [`WorkflowSpec`], and everything here is plain +//! sync code unit-testable in isolation. + +use anyhow::{bail, Context, Result}; +use serde::{Deserialize, Serialize}; +use std::collections::{HashMap, HashSet}; +use std::fs; +use std::path::{Path, PathBuf}; + +/// The only workflow file format version this build understands. +pub const SUPPORTED_VERSION: u64 = 1; +/// Hard cap on review ⇄ fix rounds a YAML may request (a human can still +/// extend a running pipeline past this — that is an explicit decision). +pub const MAX_ROUNDS_CAP: u32 = 5; +/// Default review rounds. A fix runs *between* rounds, so N rounds buy N-1 +/// repair attempts: 3 keeps a second attempt available when the first fix +/// misses, which is the common case. +pub const DEFAULT_MAX_ROUNDS: u32 = 3; +pub const MAX_REVIEWERS: usize = 4; + +/// Built-in templates, selectable everywhere and ejectable into a project. +/// A project file with the same id overrides (hides) the built-in. +pub const BUILTIN_WORKFLOWS: &[(&str, &str)] = &[ + ("review-loop", include_str!("workflows/review-loop.yaml")), + ( + "plan-review-loop", + include_str!("workflows/plan-review-loop.yaml"), + ), +]; + +// ─── Validated model ───────────────────────────────────────────────────────── + +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct WorkflowSpec { + /// File stem for project workflows, registry key for built-ins. + pub id: String, + pub name: String, + pub description: Option, + /// `Some` when the optional planning stage is enabled. + pub plan: Option, + pub implement: StageConfig, + pub review: ReviewConfig, + pub fix: StageConfig, +} + +/// Per-stage overrides. `None` falls back to the lead agent / model picked in +/// the New Task dialog (for `fix`: to the `implement` stage's values) and to +/// the built-in stage prompt. +#[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize)] +pub struct StageConfig { + pub agent: Option, + pub model: Option, + pub prompt: Option, +} + +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct ReviewConfig { + pub max_rounds: u32, + pub on_limit: OnLimit, + /// How repeat review rounds are staffed after a fix. + #[serde(default)] + pub reask: ReaskMode, + // These collections carry `serde(default)` because a `WorkflowSpec` is + // persisted inside a running pipeline's snapshot: a field that is required + // on load turns every in-flight run unreadable after an upgrade. + #[serde(default)] + pub context: Vec, + /// Always 1..=MAX_REVIEWERS entries; defaults to one all-`None` reviewer. + #[serde(default)] + pub reviewers: Vec, + /// True when the YAML set `context` explicitly. Only drives the warning + /// that the key is inert alongside custom reviewer prompts. + #[serde(default, skip_serializing)] + pub context_was_set: bool, +} + +/// Who reviews repeat rounds after a fix. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum ReaskMode { + /// Follow up in the same reviewer session: it remembers its own findings + /// and verifies each one is actually resolved, at the cost of some + /// anchoring bias. Falls back to a fresh session when the old one is gone + /// (daemon restart, agent death). + #[default] + SameSession, + /// Spawn fresh reviewer sessions every round. The previous round's + /// findings are still included in the prompt for verification. + Fresh, +} + +/// What the pipeline does when `max_rounds` is exhausted with open findings. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum OnLimit { + /// Suspend and ask the user (extend / finish / stop). + Ask, + /// Finish as NeedsReview with the open findings in the summary. + Finish, +} + +/// One piece of context assembled into a reviewer's prompt. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum ReviewContextItem { + /// The task prompt the user typed. + Prompt, + /// The plan stage output, when that stage ran. + Plan, + /// The final text of the last implement/fix session. + ImplementerSummary, + /// The working-copy diff. + Diff, +} + +#[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize)] +pub struct ReviewerConfig { + pub agent: Option, + pub model: Option, + /// Appended to the default reviewer prompt (available as `{{focus}}`). + pub focus: Option, + /// Full prompt override for this reviewer. + pub prompt: Option, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum WorkflowSource { + Project, + Builtin, +} + +/// A workflow as found on disk (or built in): invalid files are carried as +/// `Err` so pickers can list them greyed-out with the reason. +#[derive(Debug)] +pub struct LoadedWorkflow { + pub id: String, + pub source: WorkflowSource, + pub spec: Result, + pub warnings: Vec, +} + +impl WorkflowSpec { + /// Stage names for picker tooltips, e.g. `["plan", "implement", "review×2", "fix"]`. + pub fn stage_summary(&self) -> Vec { + let mut stages = Vec::new(); + if self.plan.is_some() { + stages.push("plan".to_string()); + } + stages.push("implement".to_string()); + stages.push(match self.review.reviewers.len() { + 1 => "review".to_string(), + n => format!("review×{n}"), + }); + stages.push("fix".to_string()); + stages + } +} + +// ─── Raw YAML shape ────────────────────────────────────────────────────────── + +/// Deserialize an optional mapping so that a bare `plan:` (YAML null) means +/// "present with defaults", while an absent key stays `None`. +fn nullable<'de, D, T>(deserializer: D) -> Result, D::Error> +where + D: serde::Deserializer<'de>, + T: Deserialize<'de> + Default, +{ + let value = Option::::deserialize(deserializer)?; + Ok(Some(value.unwrap_or_default())) +} + +/// Like [`nullable`], but `false` means "this stage is off". Deleting the key +/// is the canonical way to disable planning; `plan: false` is the obvious +/// guess, so accept it instead of failing with a type error. +fn nullable_or_false<'de, D, T>(deserializer: D) -> Result, D::Error> +where + D: serde::Deserializer<'de>, + T: Deserialize<'de> + Default, +{ + #[derive(Deserialize)] + #[serde(untagged)] + enum Toggle { + Off(bool), + On(T), + } + match Option::>::deserialize(deserializer)? { + None => Ok(Some(T::default())), + Some(Toggle::Off(false)) => Ok(None), + Some(Toggle::Off(true)) => Ok(Some(T::default())), + Some(Toggle::On(value)) => Ok(Some(value)), + } +} + +#[derive(Debug, Default, Deserialize)] +struct RawWorkflow { + version: Option, + name: Option, + description: Option, + #[serde(default, deserialize_with = "nullable_or_false")] + plan: Option, + #[serde(default, deserialize_with = "nullable")] + implement: Option, + #[serde(default, deserialize_with = "nullable")] + review: Option, + #[serde(default, deserialize_with = "nullable")] + fix: Option, +} + +#[derive(Debug, Default, Deserialize)] +struct RawStage { + agent: Option, + model: Option, + prompt: Option, +} + +#[derive(Debug, Default, Deserialize)] +struct RawReview { + max_rounds: Option, + on_limit: Option, + reask: Option, + context: Option>, + reviewers: Option>, +} + +#[derive(Debug, Default, Deserialize)] +struct RawReviewer { + agent: Option, + model: Option, + focus: Option, + prompt: Option, +} + +// ─── Parsing and validation ────────────────────────────────────────────────── + +/// Parse and validate one workflow file. Returns the spec (or a human-readable +/// error making the file invalid) plus non-fatal warnings either way. +pub fn parse_workflow(id: &str, yaml: &str) -> (Result, Vec) { + let mut warnings = Vec::new(); + let value: serde_yaml::Value = match serde_yaml::from_str(yaml) { + Ok(value) => value, + Err(e) => return (Err(format!("invalid YAML: {e}")), warnings), + }; + if !value.is_mapping() { + return ( + Err("workflow file must be a YAML mapping".to_string()), + warnings, + ); + } + collect_unknown_keys(&value, &mut warnings); + // Deserialize from the source text, not the `Value` above: only `from_str` + // reports the offending key path plus line and column, and that message is + // what the picker shows as the reason a workflow is rejected. + let raw: RawWorkflow = match serde_yaml::from_str(yaml) { + Ok(raw) => raw, + Err(e) => return (Err(format!("invalid workflow: {e}")), warnings), + }; + (build_spec(id, raw, &mut warnings), warnings) +} + +/// Unknown keys are forward-compatible warnings, not errors — `workflow.list` +/// surfaces them in picker tooltips. +fn collect_unknown_keys(value: &serde_yaml::Value, warnings: &mut Vec) { + const TOP: &[&str] = &[ + "version", + "name", + "description", + "plan", + "implement", + "review", + "fix", + ]; + const STAGE: &[&str] = &["agent", "model", "prompt"]; + const REVIEW: &[&str] = &["max_rounds", "on_limit", "reask", "context", "reviewers"]; + const REVIEWER: &[&str] = &["agent", "model", "focus", "prompt"]; + + check_keys(value, TOP, "top level", warnings); + for stage in ["plan", "implement", "fix"] { + if let Some(v) = value.get(stage) { + check_keys(v, STAGE, stage, warnings); + } + } + if let Some(review) = value.get("review") { + check_keys(review, REVIEW, "review", warnings); + if let Some(serde_yaml::Value::Sequence(reviewers)) = review.get("reviewers") { + for (i, reviewer) in reviewers.iter().enumerate() { + check_keys( + reviewer, + REVIEWER, + &format!("review.reviewers[{i}]"), + warnings, + ); + } + } + } +} + +fn check_keys( + value: &serde_yaml::Value, + known: &[&str], + location: &str, + warnings: &mut Vec, +) { + let serde_yaml::Value::Mapping(map) = value else { + return; + }; + for key in map.keys() { + if let Some(key) = key.as_str() { + if !known.contains(&key) { + warnings.push(format!("unknown key `{key}` in {location}")); + } + } + } +} + +fn build_spec( + id: &str, + raw: RawWorkflow, + warnings: &mut Vec, +) -> Result { + let version = raw.version.unwrap_or(SUPPORTED_VERSION); + if version != SUPPORTED_VERSION { + return Err(format!( + "unsupported workflow version {version} (this build supports {SUPPORTED_VERSION})" + )); + } + let name = raw.name.as_deref().map(str::trim).unwrap_or_default(); + if name.is_empty() { + return Err("`name` is required".to_string()); + } + + let plan = raw.plan.map(stage_config); + let implement = raw.implement.map(stage_config).unwrap_or_default(); + let fix = raw.fix.map(stage_config).unwrap_or_default(); + let review = build_review(raw.review.unwrap_or_default(), warnings)?; + + // A placeholder this workflow can never populate renders as an empty + // section, so scope the allow-lists: `{{plan}}` needs a plan stage and + // `{{focus}}` needs that reviewer to define one. + let has_plan = plan.is_some(); + let allow = |vars: &[&'static str], _focus: bool| -> Vec<&'static str> { + vars.iter() + .copied() + .filter(|name| match *name { + "plan" => has_plan, + _ => true, + }) + .collect() + }; + validate_prompt( + plan.as_ref().and_then(|s| s.prompt.as_deref()), + "plan", + &allow(VARS_PLAN, false), + )?; + validate_prompt( + implement.prompt.as_deref(), + "implement", + &allow(VARS_IMPLEMENT, false), + )?; + validate_prompt(fix.prompt.as_deref(), "fix", &allow(VARS_FIX, false))?; + for (i, reviewer) in review.reviewers.iter().enumerate() { + validate_prompt( + reviewer.prompt.as_deref(), + &format!("review.reviewers[{i}]"), + &allow(VARS_REVIEW, reviewer.focus.is_some()), + )?; + } + // `context` and `focus` only shape the BUILT-IN reviewer prompt: a custom + // prompt renders its own context. Silently ignoring them is the most + // confusing possible outcome for the first edit a user makes. + if review.reviewers.iter().all(|r| r.prompt.is_some()) { + if review.context_was_set { + warnings.push( + "review.context is ignored because every reviewer defines its own `prompt` — a \ + custom prompt decides what context it includes" + .to_string(), + ); + } + for (i, reviewer) in review.reviewers.iter().enumerate() { + let uses_focus = reviewer + .prompt + .as_deref() + .is_some_and(|p| extract_placeholders(p).iter().any(|v| v == "focus")); + if reviewer.focus.is_some() && !uses_focus { + warnings.push(format!( + "review.reviewers[{i}].focus is ignored because that reviewer's `prompt` \ + never uses {{{{focus}}}}" + )); + } + } + } + + Ok(WorkflowSpec { + id: id.to_string(), + name: name.to_string(), + description: raw + .description + .map(|d| d.trim().to_string()) + .filter(|d| !d.is_empty()), + plan, + implement, + review, + fix, + }) +} + +fn stage_config(raw: RawStage) -> StageConfig { + StageConfig { + agent: raw.agent, + model: raw.model, + prompt: raw.prompt, + } +} + +fn build_review(raw: RawReview, warnings: &mut Vec) -> Result { + let context_was_set = raw.context.is_some(); + let mut max_rounds = raw.max_rounds.unwrap_or(DEFAULT_MAX_ROUNDS); + if max_rounds == 0 { + return Err("review.max_rounds must be at least 1".to_string()); + } + if max_rounds > MAX_ROUNDS_CAP { + warnings.push(format!( + "review.max_rounds {max_rounds} exceeds the cap, clamped to {MAX_ROUNDS_CAP}" + )); + max_rounds = MAX_ROUNDS_CAP; + } + + let reask = match raw.reask.as_deref() { + None => ReaskMode::default(), + Some("same_session") => ReaskMode::SameSession, + Some("fresh") => ReaskMode::Fresh, + Some(other) => { + return Err(format!( + "review.reask must be `same_session` or `fresh`, got `{other}`" + )) + } + }; + + let on_limit = match raw.on_limit.as_deref() { + None => OnLimit::Ask, + Some("ask") => OnLimit::Ask, + Some("finish") => OnLimit::Finish, + Some(other) => { + return Err(format!( + "review.on_limit must be `ask` or `finish`, got `{other}`" + )) + } + }; + + let context = match raw.context { + None => vec![ + ReviewContextItem::Prompt, + ReviewContextItem::Plan, + ReviewContextItem::ImplementerSummary, + ReviewContextItem::Diff, + ], + Some(items) => { + if items.is_empty() { + warnings.push( + "review.context is empty — reviewers will only see their instructions" + .to_string(), + ); + } + let mut parsed = Vec::with_capacity(items.len()); + for item in &items { + parsed.push(match item.as_str() { + "prompt" => ReviewContextItem::Prompt, + "plan" => ReviewContextItem::Plan, + "implementer_summary" => ReviewContextItem::ImplementerSummary, + "diff" => ReviewContextItem::Diff, + other => { + return Err(format!( + "unknown review.context item `{other}` (expected prompt, plan, implementer_summary, diff)" + )) + } + }); + } + parsed + } + }; + + let reviewers = match raw.reviewers { + None => vec![ReviewerConfig::default()], + Some(list) => { + if list.is_empty() || list.len() > MAX_REVIEWERS { + return Err(format!( + "review.reviewers must list 1..{MAX_REVIEWERS} reviewers when set, got {}", + list.len() + )); + } + list.into_iter() + .map(|r| ReviewerConfig { + agent: r.agent, + model: r.model, + focus: r.focus, + prompt: r.prompt, + }) + .collect() + } + }; + + Ok(ReviewConfig { + max_rounds, + on_limit, + reask, + context, + reviewers, + context_was_set, + }) +} + +// ─── Prompt templates ──────────────────────────────────────────────────────── + +/// Placeholders each stage's custom prompt may use. An unknown placeholder is +/// a validation error — silently rendering `{{typo}}` verbatim into an agent +/// prompt would be far harder to notice. +pub const VARS_PLAN: &[&str] = &["task_prompt"]; +pub const VARS_IMPLEMENT: &[&str] = &["task_prompt", "plan"]; +pub const VARS_REVIEW: &[&str] = &[ + "task_prompt", + "plan", + "implementer_summary", + "diff", + "round", + "max_rounds", + "focus", +]; +pub const VARS_FIX: &[&str] = &[ + "task_prompt", + "plan", + "implementer_summary", + "diff", + "findings", + "round", + "max_rounds", +]; + +fn validate_prompt( + prompt: Option<&str>, + stage: &str, + allowed: &[&'static str], +) -> Result<(), String> { + let Some(prompt) = prompt else { + return Ok(()); + }; + for name in extract_placeholders(prompt) { + if !allowed.contains(&name.as_str()) { + return Err(format!( + "unknown placeholder {{{{{name}}}}} in {stage} prompt (allowed: {})", + allowed.join(", ") + )); + } + } + Ok(()) +} + +/// `{{name}}` occurrences as (start, end-exclusive, trimmed name). Only +/// identifier-shaped names count; other `{{…}}` text is left alone. +fn scan_placeholders(template: &str) -> Vec<(usize, usize, String)> { + let mut found = Vec::new(); + let bytes = template.as_bytes(); + let mut i = 0; + while let Some(open) = template[i..].find("{{").map(|p| i + p) { + let Some(close) = template[open + 2..].find("}}").map(|p| open + 2 + p) else { + break; + }; + let name = template[open + 2..close].trim(); + let is_ident = + !name.is_empty() && name.chars().all(|c| c.is_ascii_alphanumeric() || c == '_'); + if is_ident { + found.push((open, close + 2, name.to_string())); + i = close + 2; + } else { + // Skip just the opening braces so `{{ {{diff}}` still finds the + // inner placeholder. + i = open + 2; + } + if i >= bytes.len() { + break; + } + } + found +} + +/// Distinct placeholder names used in a template, in order of first use. +pub fn extract_placeholders(template: &str) -> Vec { + let mut seen = HashSet::new(); + scan_placeholders(template) + .into_iter() + .map(|(_, _, name)| name) + .filter(|name| seen.insert(name.clone())) + .collect() +} + +/// Substitute `{{name}}` placeholders. Names missing from `vars` are left +/// verbatim (validation has already rejected unknown names at load time). +/// Consumed by the workflow engine when it renders stage prompts. +#[allow(dead_code)] +pub fn render_template(template: &str, vars: &HashMap<&str, String>) -> String { + let mut out = String::with_capacity(template.len()); + let mut last = 0; + for (start, end, name) in scan_placeholders(template) { + if let Some(value) = vars.get(name.as_str()) { + out.push_str(&template[last..start]); + out.push_str(value); + last = end; + } + } + out.push_str(&template[last..]); + out +} + +// ─── Disk access ───────────────────────────────────────────────────────────── + +pub fn workflows_dir(project_path: &Path) -> PathBuf { + project_path.join(".warpforge").join("workflows") +} + +/// All workflows visible to a project: `.warpforge/workflows/*.{yaml,yml}` +/// (sorted by file name; on duplicate stems the first file wins) followed by +/// built-ins not overridden by a project file with the same id. +pub fn list_workflows(project_path: &Path) -> Vec { + let mut out: Vec = Vec::new(); + let mut seen: HashSet = HashSet::new(); + + let mut files: Vec = fs::read_dir(workflows_dir(project_path)) + .map(|entries| { + entries + .flatten() + .map(|entry| entry.path()) + .filter(|path| { + matches!( + path.extension().and_then(|e| e.to_str()), + Some("yaml") | Some("yml") + ) + }) + .collect() + }) + .unwrap_or_default(); + files.sort(); + + for path in files { + let Some(id) = path + .file_stem() + .and_then(|s| s.to_str()) + .map(str::to_string) + else { + continue; + }; + let file_name = path + .file_name() + .map(|n| n.to_string_lossy().into_owned()) + .unwrap_or_default(); + if !seen.insert(id.clone()) { + if let Some(existing) = out.iter_mut().find(|w| w.id == id) { + existing + .warnings + .push(format!("duplicate workflow file ignored: {file_name}")); + } + continue; + } + let (spec, warnings) = match fs::read_to_string(&path) { + Ok(text) => parse_workflow(&id, &text), + Err(e) => (Err(format!("reading {file_name}: {e}")), Vec::new()), + }; + out.push(LoadedWorkflow { + id, + source: WorkflowSource::Project, + spec, + warnings, + }); + } + + for (id, text) in BUILTIN_WORKFLOWS { + if seen.contains(*id) { + continue; + } + let (spec, warnings) = parse_workflow(id, text); + out.push(LoadedWorkflow { + id: (*id).to_string(), + source: WorkflowSource::Builtin, + spec, + warnings, + }); + } + + out +} + +/// Load one workflow by id: the project file wins over a built-in. +/// Consumed by the workflow engine when a task starts with a workflow. +#[allow(dead_code)] +pub fn load_workflow(project_path: &Path, id: &str) -> Option { + list_workflows(project_path) + .into_iter() + .find(|w| w.id == id) +} + +/// Copy a built-in workflow into the project's workflows directory so the +/// user can customize it. Refuses to overwrite an existing file. +pub fn eject_builtin(project_path: &Path, id: &str) -> Result { + let Some((_, text)) = BUILTIN_WORKFLOWS.iter().find(|(bid, _)| *bid == id) else { + bail!("no built-in workflow `{id}`"); + }; + let dir = workflows_dir(project_path); + fs::create_dir_all(&dir).with_context(|| format!("creating {}", dir.display()))?; + for ext in ["yaml", "yml"] { + let existing = dir.join(format!("{id}.{ext}")); + if existing.exists() { + bail!("{} already exists", existing.display()); + } + } + let target = dir.join(format!("{id}.yaml")); + fs::write(&target, text).with_context(|| format!("writing {}", target.display()))?; + Ok(target) +} + +// ─── Tests ─────────────────────────────────────────────────────────────────── + +#[cfg(test)] +mod tests { + use super::*; + + fn parse_ok(yaml: &str) -> (WorkflowSpec, Vec) { + let (spec, warnings) = parse_workflow("test", yaml); + (spec.expect("expected valid workflow"), warnings) + } + + fn parse_err(yaml: &str) -> String { + let (spec, _) = parse_workflow("test", yaml); + spec.expect_err("expected invalid workflow") + } + + #[test] + fn minimal_workflow_gets_defaults() { + let (spec, warnings) = parse_ok("name: Minimal\n"); + assert!(warnings.is_empty()); + assert_eq!(spec.name, "Minimal"); + assert!(spec.plan.is_none()); + assert_eq!(spec.implement, StageConfig::default()); + assert_eq!(spec.review.max_rounds, DEFAULT_MAX_ROUNDS); + assert_eq!(spec.review.on_limit, OnLimit::Ask); + assert_eq!(spec.review.reask, ReaskMode::SameSession); + assert_eq!(spec.review.reviewers, vec![ReviewerConfig::default()]); + assert_eq!( + spec.review.context, + vec![ + ReviewContextItem::Prompt, + ReviewContextItem::Plan, + ReviewContextItem::ImplementerSummary, + ReviewContextItem::Diff, + ] + ); + assert_eq!(spec.stage_summary(), vec!["implement", "review", "fix"]); + } + + #[test] + fn builtins_are_valid() { + for (id, text) in BUILTIN_WORKFLOWS { + let (spec, warnings) = parse_workflow(id, text); + let spec = spec.unwrap_or_else(|e| panic!("built-in `{id}` invalid: {e}")); + assert!( + warnings.is_empty(), + "built-in `{id}` has warnings: {warnings:?}" + ); + assert_eq!(spec.id, *id); + } + } + + #[test] + fn bare_plan_key_enables_stage() { + assert!(parse_ok("name: X\n").0.plan.is_none()); + assert_eq!( + parse_ok("name: X\nplan:\n").0.plan, + Some(StageConfig::default()) + ); + assert_eq!( + parse_ok("name: X\nplan: {}\n").0.plan, + Some(StageConfig::default()) + ); + let (spec, _) = parse_ok("name: X\nplan:\n agent: codex\n"); + assert_eq!(spec.plan.unwrap().agent.as_deref(), Some("codex")); + } + + #[test] + fn full_workflow_parses() { + let yaml = r#" +version: 1 +name: Feature loop +description: Cross-review with two models. +plan: + agent: claude +implement: + agent: claude + model: claude-fable-5 +review: + max_rounds: 3 + on_limit: finish + reask: fresh + context: [prompt, diff] + reviewers: + - agent: claude + model: claude-opus-5 + focus: correctness + - agent: codex + prompt: "Review this: {{diff}} for round {{round}}/{{max_rounds}}" +fix: + agent: claude +"#; + let (spec, warnings) = parse_ok(yaml); + assert!(warnings.is_empty(), "{warnings:?}"); + assert_eq!(spec.review.max_rounds, 3); + assert_eq!(spec.review.on_limit, OnLimit::Finish); + assert_eq!(spec.review.reask, ReaskMode::Fresh); + assert_eq!( + spec.review.context, + vec![ReviewContextItem::Prompt, ReviewContextItem::Diff] + ); + assert_eq!(spec.review.reviewers.len(), 2); + assert_eq!(spec.review.reviewers[1].agent.as_deref(), Some("codex")); + assert_eq!( + spec.stage_summary(), + vec!["plan", "implement", "review×2", "fix"] + ); + } + + #[test] + fn unknown_keys_warn_but_stay_valid() { + let yaml = "name: X\nfuture_thing: 1\nreview:\n gate: build\n reviewers:\n - agent: claude\n voice: loud\n"; + let (spec, warnings) = parse_workflow("test", yaml); + assert!(spec.is_ok()); + assert_eq!( + warnings, + vec![ + "unknown key `future_thing` in top level", + "unknown key `gate` in review", + "unknown key `voice` in review.reviewers[0]", + ] + ); + } + + #[test] + fn validation_errors() { + assert!(parse_err("services: {}\n").contains("`name` is required")); + assert!(parse_err("name: X\nversion: 2\n").contains("unsupported workflow version 2")); + assert!(parse_err("name: X\nreview:\n max_rounds: 0\n").contains("at least 1")); + assert!(parse_err("name: X\nreview:\n on_limit: retry\n").contains("`ask` or `finish`")); + assert!( + parse_err("name: X\nreview:\n reask: never\n").contains("`same_session` or `fresh`") + ); + assert!( + parse_err("name: X\nreview:\n context: [prompt, everything]\n") + .contains("unknown review.context item `everything`") + ); + assert!(parse_err("name: X\nreview:\n reviewers: []\n").contains("1..4 reviewers")); + let five = + "name: X\nreview:\n reviewers:\n - {}\n - {}\n - {}\n - {}\n - {}\n"; + assert!(parse_err(five).contains("1..4 reviewers")); + assert!(parse_workflow("test", ": : :") + .0 + .unwrap_err() + .contains("invalid YAML")); + } + + #[test] + fn max_rounds_clamped_with_warning() { + let (spec, warnings) = parse_ok("name: X\nreview:\n max_rounds: 9\n"); + assert_eq!(spec.review.max_rounds, MAX_ROUNDS_CAP); + assert_eq!(warnings.len(), 1); + assert!(warnings[0].contains("clamped to 5")); + } + + #[test] + fn unknown_placeholder_is_an_error() { + let err = parse_err("name: X\nimplement:\n prompt: \"Do {{taks_prompt}}\"\n"); + assert!( + err.contains("unknown placeholder {{taks_prompt}} in implement prompt"), + "{err}" + ); + // `{{diff}}` is a review/fix variable, not an implement one. + let err = parse_err("name: X\nimplement:\n prompt: \"See {{diff}}\"\n"); + assert!(err.contains("{{diff}}"), "{err}"); + // Reviewer prompts may use the review variable set, including {{focus}}. + let yaml = "name: X\nreview:\n reviewers:\n - prompt: \"{{focus}}: check {{diff}}\"\n"; + assert!(parse_workflow("test", yaml).0.is_ok()); + // {{plan}} is rejected when the workflow has no planning stage — it + // would render an empty section instead of a plan. + let err = parse_err("name: X\nimplement:\n prompt: \"Plan: {{plan}}\"\n"); + assert!(err.contains("{{plan}}"), "{err}"); + assert!(parse_workflow( + "test", + "name: X\nplan: {}\nimplement:\n prompt: \"Plan: {{plan}}\"\n" + ) + .0 + .is_ok()); + } + + #[test] + fn inert_reviewer_knobs_warn() { + // `context` and `focus` only shape the built-in reviewer prompt, so + // setting them next to a custom prompt must not pass silently. + let (spec, warnings) = parse_workflow( + "test", + "name: X\nreview:\n context: [prompt]\n reviewers:\n - focus: security\n prompt: \"check it\"\n", + ); + assert!(spec.is_ok()); + assert!( + warnings + .iter() + .any(|w| w.contains("review.context is ignored")), + "{warnings:?}" + ); + assert!( + warnings + .iter() + .any(|w| w.contains("focus is ignored") && w.contains("{{focus}}")), + "{warnings:?}" + ); + // A custom prompt that actually uses {{focus}} draws no warning. + let (_, warnings) = parse_workflow( + "test", + "name: X\nreview:\n reviewers:\n - focus: security\n prompt: \"{{focus}}\"\n", + ); + assert!(warnings.is_empty(), "{warnings:?}"); + } + + #[test] + fn plan_false_disables_the_stage() { + assert!(parse_ok("name: X\nplan: false\n").0.plan.is_none()); + assert!(parse_ok("name: X\nplan: true\n").0.plan.is_some()); + } + + #[test] + fn placeholder_extraction_and_rendering() { + assert_eq!( + extract_placeholders("{{a}} and {{ b }} and {{a}} but not {{a b}} or {{}}"), + vec!["a", "b"] + ); + let vars = HashMap::from([ + ("task_prompt", "fix the bug".to_string()), + ("round", "2".to_string()), + ]); + assert_eq!( + render_template( + "Task: {{task_prompt}} (round {{ round }}) {{unknown}}", + &vars + ), + "Task: fix the bug (round 2) {{unknown}}" + ); + assert_eq!(render_template("no placeholders", &vars), "no placeholders"); + } + + #[test] + fn listing_project_overrides_builtin_and_reports_invalid() { + let dir = tempfile::tempdir().unwrap(); + let wf_dir = workflows_dir(dir.path()); + fs::create_dir_all(&wf_dir).unwrap(); + // Overrides the built-in of the same id. + fs::write(wf_dir.join("review-loop.yaml"), "name: Mine\n").unwrap(); + // Invalid file still listed. + fs::write(wf_dir.join("broken.yaml"), "name: [\n").unwrap(); + // Non-workflow files ignored. + fs::write(wf_dir.join("notes.txt"), "hi").unwrap(); + + let listed = list_workflows(dir.path()); + let ids: Vec<(&str, WorkflowSource, bool)> = listed + .iter() + .map(|w| (w.id.as_str(), w.source, w.spec.is_ok())) + .collect(); + assert_eq!( + ids, + vec![ + ("broken", WorkflowSource::Project, false), + ("review-loop", WorkflowSource::Project, true), + ("plan-review-loop", WorkflowSource::Builtin, true), + ] + ); + let mine = load_workflow(dir.path(), "review-loop").unwrap(); + assert_eq!(mine.spec.unwrap().name, "Mine"); + } + + #[test] + fn listing_without_workflows_dir_returns_builtins() { + let dir = tempfile::tempdir().unwrap(); + let listed = list_workflows(dir.path()); + assert_eq!(listed.len(), BUILTIN_WORKFLOWS.len()); + assert!(listed.iter().all(|w| w.source == WorkflowSource::Builtin)); + } + + #[test] + fn duplicate_stems_warn_and_first_wins() { + let dir = tempfile::tempdir().unwrap(); + let wf_dir = workflows_dir(dir.path()); + fs::create_dir_all(&wf_dir).unwrap(); + fs::write(wf_dir.join("loop.yaml"), "name: From yaml\n").unwrap(); + fs::write(wf_dir.join("loop.yml"), "name: From yml\n").unwrap(); + + let listed = list_workflows(dir.path()); + let entry = listed.iter().find(|w| w.id == "loop").unwrap(); + assert_eq!(entry.spec.as_ref().unwrap().name, "From yaml"); + assert_eq!( + entry.warnings, + vec!["duplicate workflow file ignored: loop.yml"] + ); + } + + #[test] + fn eject_writes_once() { + let dir = tempfile::tempdir().unwrap(); + let path = eject_builtin(dir.path(), "review-loop").unwrap(); + assert!(path.ends_with(".warpforge/workflows/review-loop.yaml")); + let text = fs::read_to_string(&path).unwrap(); + assert!(parse_workflow("review-loop", &text).0.is_ok()); + // Second eject refuses to overwrite. + assert!(eject_builtin(dir.path(), "review-loop").is_err()); + assert!(eject_builtin(dir.path(), "nope").is_err()); + } +} diff --git a/src/workflows/plan-review-loop.yaml b/src/workflows/plan-review-loop.yaml new file mode 100644 index 0000000..6074709 --- /dev/null +++ b/src/workflows/plan-review-loop.yaml @@ -0,0 +1,87 @@ +# Built-in workflow: plan → implement → review ⇄ fix. +# +# Every stage is explicit here so this file is both runnable and a complete +# customization example. Agent/model omissions use the lead agent selected in +# the New Task dialog. +version: 1 +name: Plan + implement + review loop +description: Plan first, implement the plan, then loop reviews and fixes until approved. + +plan: + # agent: claude + # model: claude-opus-5 + prompt: | + You are the planning stage of a workflow pipeline. Explore the codebase as + needed and produce a concise implementation plan: files to touch, approach, + edge cases, and verification. Do not edit files. End with the complete plan + because it is passed verbatim to the implementer. + + ## Task + {{task_prompt}} + +implement: + # agent: codex + # model: gpt-5.6-sol + prompt: | + You are the implementation stage of a workflow pipeline. Implement the task + and approved plan completely: write the code, keep the change focused, and + verify your work with builds/tests where feasible. Your final message must + summarize what you did because it is passed to the reviewers. + + ## Task + {{task_prompt}} + + ## Approved plan + {{plan}} + +review: + max_rounds: 3 # review rounds; a fix runs between them, so 3 = two fix attempts + on_limit: ask # ask | finish — behaviour when the limit is hit + # reask: same_session # same_session | fresh — repeat rounds follow up in the + # same reviewer session (it verifies its own findings); `fresh` spawns new + # reviewers each round, with last round's findings included for verification. + reviewers: + - # agent: claude + # model: claude-opus-5 + # focus: correctness and edge cases # only used if you delete `prompt` below + prompt: | + You are a code reviewer in workflow round {{round}}/{{max_rounds}}. + Review the implementation against the task and approved plan. Do not + edit files. Verify claims against the diff and report only concrete, + actionable problems. + + ## Task + {{task_prompt}} + + ## Approved plan + {{plan}} + + ## Implementer's summary + {{implementer_summary}} + + ## Working-copy diff + {{diff}} + +fix: + # Omitted agent/model inherit the implementation stage; uncomment to pin. + # agent: codex + # model: gpt-5.6-sol + prompt: | + You are the repair stage of workflow round {{round}}/{{max_rounds}}. + Address every reviewer finding: fix it or, if it is factually wrong, + explain why in your final summary. Do not change unrelated code. Verify the + result where feasible and summarize what changed for the next review. + + ## Task + {{task_prompt}} + + ## Approved plan + {{plan}} + + ## Findings to address + {{findings}} + + ## Current working-copy diff + {{diff}} + +# Warpforge appends the need_user_input/verdict JSON protocols to these prompts. diff --git a/src/workflows/review-loop.yaml b/src/workflows/review-loop.yaml new file mode 100644 index 0000000..c7a8a92 --- /dev/null +++ b/src/workflows/review-loop.yaml @@ -0,0 +1,69 @@ +# Built-in workflow: implement → review ⇄ fix. +# +# This workflow ALWAYS starts with implementation. Omitting `implement` only +# selects Warpforge's built-in implementation settings/prompt; it never removes +# this fixed stage. Copy the file into /.warpforge/workflows/ (or use +# "Copy to project" in the New Task dialog) to customize it. +version: 1 +name: Implement + review loop +description: Implement the task, then loop reviews and fixes until reviewers approve. + +# Uncomment agent/model to pin this stage. When omitted, they fall back to the +# lead agent/model picked in the New Task dialog. +implement: + # agent: claude + # model: claude-opus-5 + prompt: | + You are the implementation stage of a workflow pipeline. Implement the task + completely: write the code, keep the change focused, and verify your work + with builds/tests where feasible. Your final message must summarize what + you did because it is passed to the reviewers. + + ## Task + {{task_prompt}} + +review: + max_rounds: 3 # review rounds; a fix runs between them, so 3 = two fix attempts + on_limit: ask # ask | finish — behaviour when the limit is hit + # reask: same_session # same_session | fresh — repeat rounds follow up in the + # same reviewer session (it verifies its own findings); `fresh` spawns new + # reviewers each round, with last round's findings included for verification. + reviewers: + # Each reviewer may use a different agent/model. Omitted values use Lead. + - # agent: codex + # model: gpt-5.6-sol + # focus: correctness and edge cases # only used if you delete `prompt` below + prompt: | + You are a code reviewer in workflow round {{round}}/{{max_rounds}}. + Review the implementation against the task. Do not edit files. Verify + claims against the diff and report only concrete, actionable problems. + + ## Task + {{task_prompt}} + + ## Implementer's summary + {{implementer_summary}} + + ## Working-copy diff + {{diff}} + +fix: + # Uncomment to override the implementation stage. Omitted values inherit it. + # agent: claude + # model: claude-opus-5 + prompt: | + You are the repair stage of workflow round {{round}}/{{max_rounds}}. + Address every reviewer finding: fix it or, if it is factually wrong, + explain why in your final summary. Do not change unrelated code. Verify the + result where feasible and summarize what changed for the next review. + + ## Task + {{task_prompt}} + + ## Findings to address + {{findings}} + + ## Current working-copy diff + {{diff}} + +# Warpforge appends the need_user_input/verdict JSON protocols to these prompts. diff --git a/tests/fixtures/mock-acp-workflow.mjs b/tests/fixtures/mock-acp-workflow.mjs new file mode 100644 index 0000000..a5f7645 --- /dev/null +++ b/tests/fixtures/mock-acp-workflow.mjs @@ -0,0 +1,114 @@ +// Scripted ACP agent for workflow-engine tests. +// +// Usage: node mock-acp-workflow.mjs [behavior...] +// +// Each session/prompt consumes the next behavior from the shared state file, +// so consecutive stage processes driven by the same agent command (implement → +// fix, review round 1 → review round 2) continue one script. Prompts past the +// end of the script repeat the last behavior. +import { readFileSync, writeFileSync } from "node:fs"; + +const [stateFile, ...script] = process.argv.slice(2); +let buf = ""; +const SID = "mock-session-workflow"; + +const send = (obj) => process.stdout.write(JSON.stringify(obj) + "\n"); +const update = (u) => + send({ jsonrpc: "2.0", method: "session/update", params: { sessionId: SID, update: u } }); +const text = (t) => + update({ sessionUpdate: "agent_message_chunk", content: { type: "text", text: t } }); +const endTurn = (id) => send({ jsonrpc: "2.0", id, result: { stopReason: "end_turn" } }); + +const nextBehavior = () => { + let index = 0; + try { + index = parseInt(readFileSync(stateFile, "utf8"), 10) || 0; + } catch {} + writeFileSync(stateFile, String(index + 1)); + return script[Math.min(index, script.length - 1)]; +}; + +process.stdin.on("data", (chunk) => { + buf += chunk; + let i; + while ((i = buf.indexOf("\n")) >= 0) { + const line = buf.slice(0, i).trim(); + buf = buf.slice(i + 1); + if (line) handle(JSON.parse(line)); + } +}); + +function handle(msg) { + if (msg.method === "initialize") { + send({ jsonrpc: "2.0", id: msg.id, result: { protocolVersion: 1, agentCapabilities: {} } }); + } else if (msg.method === "session/new") { + send({ jsonrpc: "2.0", id: msg.id, result: { sessionId: SID } }); + } else if (msg.method === "session/prompt") { + const behavior = nextBehavior(); + switch (behavior) { + case "impl": + text("IMPL-DONE: implemented the change."); + endTurn(msg.id); + break; + case "noisy-impl": + // Narration, then work, then the real closing message — only the last + // part is the summary the pipeline should carry forward. + text("NARRATION: let me look at the files first."); + update({ + sessionUpdate: "tool_call", + toolCallId: "t1", + title: "read src/lib.rs", + status: "completed", + }); + text("CLOSING: implemented the change and ran the tests."); + endTurn(msg.id); + break; + case "slow-impl": + text("IMPL-DONE: implemented the change (slowly)."); + setTimeout(() => endTurn(msg.id), 600); + break; + case "fix": + text("FIX-DONE: addressed the findings."); + endTurn(msg.id); + break; + case "plan": + text("PLAN: 1. edit a.rs 2. run tests"); + endTurn(msg.id); + break; + case "question": + text('Before I plan this:\n```json\n{"need_user_input": "Which database?"}\n```'); + endTurn(msg.id); + break; + case "approve": + text('Looks good.\n```json\n{"verdict": "approve", "findings": []}\n```'); + endTurn(msg.id); + break; + case "reject": + text( + 'Found problems.\n```json\n{"verdict": "request_changes", "findings": [{"severity": "high", "file": "a.rs", "description": "bug here"}]}\n```' + ); + endTurn(msg.id); + break; + case "reject-die": + // Reject, then die shortly after: simulates a reviewer whose session + // is gone by the time the next round wants to follow up in it. + text( + 'Found problems.\n```json\n{"verdict": "request_changes", "findings": [{"severity": "high", "file": "a.rs", "description": "bug here"}]}\n```' + ); + endTurn(msg.id); + setTimeout(() => process.exit(0), 100); + break; + case "slow-fix": + text("FIX-DONE: addressed the findings (slowly)."); + setTimeout(() => endTurn(msg.id), 600); + break; + case "garbage": + text("looks fine to me, ship it"); + endTurn(msg.id); + break; + default: + text(`unknown behavior: ${behavior}`); + endTurn(msg.id); + } + } +}