feat: executable custom workflows + visual graph editor + interpreter cutover foundation#1363
Conversation
Add a workflows table (migration 103) storing WorkflowIr graphs plus editor layout, with CRUD on TaskStore (create/list/get/update/delete) that validates the IR via parseWorkflowIr on write. IDs (WF-001…) use a monotonic __meta counter that never reuses across deletes.
Add compileWorkflowToSteps + validateLinearity: walk the linear main path of a WorkflowIr, emit prompt/script/gate user nodes as ordered WorkflowStep inputs, skip the execute/review seams, and use the merge seam as the pre-/post-merge boundary. Non-linear graphs (branching beyond canonical seam success/failure) throw WorkflowCompileError so they route to the deferred interpreter instead of mis-executing.
Selecting a workflow compiles it, materializes WorkflowStep rows (tagged and hidden from the step manager), and writes their ids into the task's existing enabledWorkflowSteps — the executor's read path is untouched. Re-selection replaces prior steps with no orphans; non-linear graphs abort before any write. New tasks inherit a project default workflow (settings.defaultWorkflowId) ahead of legacy default-on steps. Adds task_workflow_selection table (migration 104). No scheduler/executor/merger changes.
Add register-workflow-routes: GET/POST/PATCH/DELETE /workflows, POST /workflows/:id/compile (200 steps or 422 for non-linear graphs), PUT/GET /tasks/:taskId/workflow selection, and GET/PUT /project/default-workflow. Handlers are thin pass-throughs to the TaskStore; malformed IR -> 400, unknown id -> 404, non-compilable graph -> 422.
Add fetch/create/update/delete/compile workflow client wrappers plus select/fetch task workflow and get/set project default, mirroring the existing WorkflowStep client conventions (api/withProjectId/dedupe).
Add a React Flow (@xyflow/react) based WorkflowNodeEditor: a lazy-loaded modal with a workflow list, a node palette (prompt/script/gate/merge-boundary), drag-to-connect edges, a per-node inspector, and save with compile-validation that surfaces non-linear graphs as a banner. Pure irToFlow/flowToIr mapping round-trips the v1 IR plus editor layout. Reachable via a Graph editor button in the Workflow Steps manager; mounted in AppModals behind a new modal-manager flag. Adds a vendor-reactflow Vite chunk and a feature changeset.
…ings (U8) Add WorkflowSelector: a per-task picker in the task detail workflow tab that applies a workflow (selection returns the resulting enabledWorkflowSteps so the controlled steps list refreshes in place), and a ProjectDefaultWorkflowField in Project General settings for the default new tasks inherit.
Consolidate the duplicated step-materialization loop; validate by compiling before any mutation so a non-linear graph still aborts with nothing written.
- selectTaskWorkflow: compile once, materialize the new steps and repoint the task before deleting prior rows, so a mid-flight failure never leaves the task referencing deleted step ids - createTaskWithReservedId: inherit the project default workflow like createTask (imports/reserved-id creations were skipping it) - setDefaultWorkflowId: use null (updateSettings' delete sentinel) to clear - parseWorkflowLayout: reject JSON arrays - editor inspector: gate-mode default display now matches the compiler (script/gate block by default, prompt is advisory)
…er (CU-U1) Non-seam prompt/script nodes now dispatch to an injected WorkflowCustomNodeRunner instead of throwing; gate nodes support an executable (prompt/script-backed) form alongside the original context-gate contract. WorkflowGraphExecutor accepts the runner via deps.
…ifecycle (CU-U2) Loads a task's selected workflow, runs the graph with injected legacy seams (execute/review/merge) and a custom-node runner, and maps the terminal outcome to completed/failed/fell-back. Any interpreter-level error falls back so the caller can run the legacy pipeline — a task is never stranded. Covered with fake seams: lifecycle ordering, failure routing, gate blocking, fallback reasons, diagnostics isolation. Includes the interpreter-cutover plan doc.
…he flag (CU-U3, CU-U4) Real engine seams: execute delegates to the legacy implementation phase via a completion interceptor that stops execute() at the implementation-complete boundary (no double review/merge); review performs the in-review handoff; merge resolves through ProjectEngine.onMerge over the same serialized merge queue (wired via a late-bound setMergeRequester, mirroring setMergeEnqueuer). Custom graph nodes run on the proven WorkflowStep machinery (readonly tool policy, verdict parsing). Adds a 'planning' seam to the vocabulary (no-op for pre-specified tasks; custom planning is a prompt node today). Entry point: execute() routes graph-selected tasks through the runner when experimentalFeatures.workflowGraphExecutor is on, with process-wide routing claims (FN-4811 posture), duplicate-dispatch dropping, pre-run errors falling back to legacy, and mid-run errors parking the task in review (never re-running the implementation, never stranding the task). Flag off by default: all 587 executor tests pass unchanged.
- runner: report visited nodes on mid-run interpreter errors - handleGraphFailure: clear completed-task watchdog + untrack stuck detector - getDefaultWorkflowId: use getSettingsFast (drop per-create listWorkflowSteps read) - db: index workflows(createdAt) in base schema + migration 103 - store: hoist workflow-definition type imports; mapping: single editorKind call - executor: drop backwards TaskDetail->Task cast in execute seam
- liveness: graph-routed tasks count as executing in getExecutingTaskIds/ isTaskActive and are skipped by recoverCompletedTask + the completed-task resume fast-path — recovery can no longer drive a parallel lifecycle (P0) - handleGraphFailure: sets status 'failed' (self-healing revival exemption, prevents FN-5704-style re-run loop) and leaves paused tasks untouched - execute seam distinguishes pause/abort from implementation failure - merge seam: 30-minute timeout so a wedged queue cannot strand the run - awaitAbortInFlightTaskWork: defensive interceptor/routing cleanup - deleteWorkflowDefinition: cascades to selections, materialized steps, affected tasks' enabledWorkflowSteps, and the project default
Prompt nodes now support an executor kind: model (provider/model override), agent (adopts a named agent's model + custom instructions), skill (prompt becomes a skill invocation), and cli (named project script with the prompt in FUSION_NODE_PROMPT — raw commands still never accepted). Per-node maxRetries overrides the executor-wide default (capped at 10). Await-input nodes pause the task with status 'awaiting-user-input' and the question as pausedReason; on unpause the newest steering comment is consumed as the answer and exposed in graph context.
Inspector for prompt nodes: executor picker (model via CustomModelDropdown, agent, skill from discovered skills, CLI named script), auto-approve toggle, per-node max retries, and a wait-for-user-input mode with a User input palette preset. Task card shows a 'Needs input' badge for awaiting-user-input status; the task modal workflow tab shows the node's question as a banner.
…-first-use approval CLI prompt nodes now accept a raw cliCommand (any command + args), not just named scripts. A raw command must be explicitly approved by the user before it runs: an unapproved command pauses the task (status awaiting-cli-approval) with the command shown; the user approves via POST /tasks/:id/workflow/approve-cli, which records the exact command string in settings.approvedWorkflowCliCommands and resumes. Named scripts (settings.scripts) still never require approval. Adds POST /tasks/:id/workflow/input to answer await-input nodes (records a steering comment + resumes).
Node inspector: CLI executor toggles between a raw command textarea and a named script. Task modal workflow tab: the await-input banner is now interactive (reply textarea + Submit & resume), and a new awaiting-cli-approval banner shows the pending command with an Approve & run action.
A CLI node can set cliSkipApproval to bypass the trust-on-first-use pause and run its command immediately. Exposed as a checkbox in the node inspector.
Ship read-only built-in workflows surfaced in the workflow list and selectable like any workflow: 'Coding' (the existing execute->review->merge pipeline as a graph), 'Quick fix' (no review), 'Review-heavy' (extra security gate), and 'Compound engineering' (plan -> implement -> review -> code-review gate -> merge -> document, invoking ce-plan/ce-code-review/ce-compound skills). Built-ins lead the list, resolve by id for selection, and reject edit/delete.
New-task modal and the quick-add card (expanded) now expose a workflow picker; the chosen workflow is applied to the new task via selectTaskWorkflow right after creation, non-blocking on failure.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds persisted workflow graphs, a linearity-validating compiler, built-in workflows, DB migrations and TaskStore selection/materialization, dashboard API/routes and React Flow editor, UI workflow selectors and paused-task banners, and an engine graph interpreter/runner with merge routing and agent tools. ChangesExecutable custom workflows
Sequence Diagram(s)sequenceDiagram
participant UI as Dashboard Editor (rgba(66,135,245,0.5))
participant API as API Routes (rgba(99,99,99,0.5))
participant Core as Core Store/Compiler (rgba(46,204,113,0.5))
participant Eng as Engine Executor (rgba(155,89,182,0.5))
UI->>API: Save / Compile / Select workflow
API->>Core: Persist / Validate / Compile IR
Core-->>API: Definition / Steps / Selection
Eng->>Core: Load selection / definition
Eng->>Eng: Interpret graph (seams / custom nodes)
Eng-->>Core: Update task state (pause / approve / resume)
UI->>API: Approve CLI / Submit input
API->>Core: Resume task via store
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
Greptile SummaryThis PR adds executable custom workflow graphs (React Flow editor, IR→steps compiler, per-task/project-default selection), a flag-gated
Confidence Score: 5/5Safe to merge: the flag is off by default so production behavior is unchanged, and the issues identified in earlier review rounds are all addressed in this commit. All previously flagged issues (steering-comment bypass in runAwaitInputNode, infinite re-pause loop, merge-seam failure for autoMerge-off projects, arbitrary CLI command injection in /approve-cli) are resolved. The interpreter cutover is fully flag-gated and the fallback path is exercised on every non-graph task. Remaining comments are non-blocking quality concerns that do not affect correctness on the current code paths. No files require special attention before merging. packages/core/src/store.ts has the orphaned-step and CLI-approval-revocation concerns, but neither blocks correctness of the happy path.
|
| Filename | Overview |
|---|---|
| packages/engine/src/executor.ts | Adds graph routing claim, WorkflowGraphTaskRunner wiring, createGraphSeams, handleGraphFailure, runImplementationPhase, and runAwaitInputNode. Previous P1s addressed; runAwaitInputNode now uses epoch watermarks. |
| packages/engine/src/project-engine.ts | manualMergeResolvers upgraded to per-task list; requestInterpreterMerge added with noOp: true for autoMerge-off tasks. Logic is sound. |
| packages/engine/src/workflow-graph-task-runner.ts | New file; side-effect tracking via wrapped seams enables safe fell-back vs failed disposition. Clean. |
| packages/dashboard/src/routes/register-workflow-routes.ts | /approve-cli derives command from task.pausedReason only; /input preserves pausedReason for runAwaitInputNode. Input validation present throughout. |
| packages/core/src/store.ts | Adds workflow CRUD, selectTaskWorkflow with task lock, approveWorkflowCliCommand (no revocation). Partial materializeWorkflowSteps failure leaves orphaned steps. |
| packages/core/src/workflow-compiler.ts | validateLinearity rejects branching and enforces execute->review->merge seam order; compileWorkflowToSteps is correct and pure. |
| packages/core/src/db.ts | Migrations 103/104/105 add workflows and task_workflow_selection tables; migration 105 cleans orphaned selection rows. Schema is additive and rollback-safe. |
| packages/engine/src/workflow-graph-executor.ts | Per-node maxRetries override added; runCustomNode injected via deps. Cycle detection and edge traversal correct. |
Sequence Diagram
sequenceDiagram
participant Sched as Scheduler
participant Exec as TaskExecutor
participant Runner as WorkflowGraphTaskRunner
participant GExec as WorkflowGraphExecutor
participant Seams as LegacySeams
participant Store as TaskStore
participant PE as ProjectEngine
Sched->>Exec: execute(task)
Exec->>Exec: graphRouting.add(task.id)
Exec->>Runner: runner.run(task, settings)
Runner->>Store: getWorkflowDefinition(id)
Runner->>GExec: executor.run(task, settings, ir)
GExec->>Seams: execute seam
Seams->>Exec: runImplementationPhase(task)
Note over Exec: agent session runs, fn_task_done fires interceptor
Exec-->>Seams: taskDone: true
GExec->>Seams: review seam
Seams->>Store: handoffTaskToReview
GExec->>Seams: merge seam
Seams->>PE: requestInterpreterMerge(taskId)
alt autoMerge eligible
PE-->>Seams: merged: true
Seams-->>GExec: outcome: success
else autoMerge off
PE-->>Seams: merged: false, noOp: true
Seams-->>GExec: outcome: success, merge-noop
Note over Store: task stays in-review for human merge
end
GExec-->>Runner: executed: true, outcome: success
Runner-->>Exec: disposition: completed
Exec->>Exec: graphRouting.delete(task.id)
Reviews (12): Last reviewed commit: "Update packages/engine/src/project-engin..." | Re-trigger Greptile
There was a problem hiding this comment.
Actionable comments posted: 1
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (25)
packages/core/src/db.ts-404-411 (1)
404-411:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftAdd task-owned cleanup semantics to
task_workflow_selection.
task_workflow_selectionis modeled as per-task state, buttaskIdis not tied totasks(id). Deleting a task can therefore leave behind orphaned workflow-selection rows and stale materializedstepIds, which breaks the new persistence contract.Also applies to: 4081-4091
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/db.ts` around lines 404 - 411, The task_workflow_selection table models per-task state but taskId is not tied to tasks(id), leaving orphan rows and stale stepIds when a task is deleted; update the schema so taskId references tasks(id) with ON DELETE CASCADE (or add a corresponding FK + cascade constraint for existing schema) to ensure rows in task_workflow_selection are removed when their task is deleted, and apply the same change for the other affected DDL region (the similar create/alter statements around lines 4081-4091) so all task-owned cleanup semantics are enforced.packages/core/src/store.ts-3713-3716 (1)
3713-3716:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftDon't materialize compiled step rows before the owning write is safe.
These paths persist hidden
workflow_stepsrows first and only later create/update the owning task or selection. Any later failure on the task path leaves orphaned compiled steps that notask_workflow_selectionrow points to, so they can never be reclaimed._createTaskInternal()still has post-insert failure paths, andselectTaskWorkflow()can also fail after materialization whenupdateTask()rejects. Compile first, but either defermaterializeWorkflowSteps()until the owner write is guaranteed or delete the created ids in a catch/finally.Also applies to: 3876-3879, 11239-11253, 11257-11266, 11275-11287
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/store.ts` around lines 3713 - 3716, The code materializes compiled workflow step rows (via materializeDefaultWorkflowSteps/materializeWorkflowSteps) before the owning task/selection write is guaranteed, which can leave orphaned workflow_steps if the subsequent task/selection insert/update fails; change the flow so you first compile steps but defer calling materializeDefaultWorkflowSteps/materializeWorkflowSteps until after the owner row is successfully written (the paths around _createTaskInternal(), selectTaskWorkflow(), and updateTask()), or if deferring is not possible then ensure any created step ids (resolvedWorkflowSteps/pendingWorkflowSelection) are deleted in a catch/finally immediately after a failed owner write to avoid orphans.packages/core/src/store.ts-10931-10941 (1)
10931-10941:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMake workflow-definition ID allocation atomic across TaskStore instances.
withConfigLock()only serializes callers inside one process. Two stores can both read the samenextWorkflowDefinitionIdvalue here, both returnWF-00N, and oneINSERT INTO workflowsthen fails on the primary key. This counter needs a SQLite write transaction so allocation is serialized cross-process.Possible fix
private nextWorkflowDefinitionId(): string { - const row = this.db.prepare("SELECT value FROM __meta WHERE key = 'nextWorkflowDefinitionId'").get() as - | { value: string } - | undefined; - const next = row ? parseInt(row.value, 10) || 1 : 1; - this.db - .prepare( - "INSERT INTO __meta (key, value) VALUES ('nextWorkflowDefinitionId', ?) ON CONFLICT(key) DO UPDATE SET value = excluded.value", - ) - .run(String(next + 1)); - return `WF-${String(next).padStart(3, "0")}`; + return this.db.transactionImmediate(() => { + const row = this.db.prepare("SELECT value FROM __meta WHERE key = 'nextWorkflowDefinitionId'").get() as + | { value: string } + | undefined; + const next = row ? parseInt(row.value, 10) || 1 : 1; + this.db + .prepare( + "INSERT INTO __meta (key, value) VALUES ('nextWorkflowDefinitionId', ?) ON CONFLICT(key) DO UPDATE SET value = excluded.value", + ) + .run(String(next + 1)); + return `WF-${String(next).padStart(3, "0")}`; + }); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/store.ts` around lines 10931 - 10941, The nextWorkflowDefinitionId() currently reads and then writes __meta non-atomically; wrap the read/increment/write in a SQLite write transaction so allocation is serialized across processes: inside nextWorkflowDefinitionId() start a write transaction (e.g., BEGIN IMMEDIATE or use the library's transaction helper), SELECT the current value for key 'nextWorkflowDefinitionId', compute next, UPDATE or UPSERT the new value, commit, and then return the formatted ID; ensure the entire read+update is in the same transaction so two TaskStore instances cannot allocate the same WF-ID.packages/core/src/store.ts-11121-11145 (1)
11121-11145:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftSerialize task workflow selection mutations under one task lock.
These code paths mutate
tasks.enabledWorkflowSteps,task_workflow_selection, andworkflow_stepsacross multiple awaits, but there is no outerwithTaskLock(taskId, ...)protecting the whole sequence. Concurrent select/clear/delete calls on the same task can interleave and leave the task pointing at removed step ids or leavetask_workflow_selectionout of sync withenabledWorkflowSteps. This needs an internal unlocked helper or equivalent so the entire task-scoped mutation runs as one serialized unit.Also applies to: 11225-11301
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/store.ts` around lines 11121 - 11145, The sequence that deletes workflow_steps rows, removes the task_workflow_selection row, and calls updateTask must be executed under a per-task lock to prevent races; wrap the entire per-row logic (the JSON.parse of row.stepIds, the DELETEs against workflow_steps and task_workflow_selection, and the await this.updateTask call) in withTaskLock(row.taskId, async () => { ... }) or extract an internal helper (e.g., clearTaskWorkflowSelectionUnlocked(taskId, stepIds)) that performs those mutations and invoke it only while holding withTaskLock; apply the same change to the similar block referenced at the 11225-11301 region so enabledWorkflowSteps, task_workflow_selection, and workflow_steps are mutated atomically for each task.packages/core/src/workflow-compiler.ts-77-122 (1)
77-122:⚠️ Potential issue | 🟠 Major | ⚡ Quick winEnforce a single canonical seam sequence before compiling.
validateLinearity()currently accepts any linear arrangement of seam nodes. That means graphs such asstart -> merge -> prompt -> review -> end, or graphs with twomergeseams, will still compile even though this compiler treats seams as a fixedexecute -> review -> mergepipeline. In those casescompileWorkflowToSteps()will assign phases and skip seams inconsistently with the runtime contract.Suggested validation shape
export function validateLinearity(ir: WorkflowIr): WorkflowCompileError | null { const nodesById = new Map(ir.nodes.map((node) => [node.id, node])); + const expectedSeamOrder = ["execute", "review", "merge"] as const; + const seenSeams = new Set<string>(); for (const edge of ir.edges) { if (!nodesById.has(edge.from)) return new WorkflowCompileError(`edge references unknown node '${edge.from}'`); if (!nodesById.has(edge.to)) return new WorkflowCompileError(`edge references unknown node '${edge.to}'`); } @@ const visited = new Set<string>(); let cursor: string | undefined = startNode.id; + let nextExpectedSeamIndex = 0; while (cursor && !visited.has(cursor)) { visited.add(cursor); + const node = nodesById.get(cursor); + if (!node) break; + const seam = seamOf(node); + if (seam) { + if (seenSeams.has(seam)) { + return new WorkflowCompileError(`seam '${seam}' appears more than once`); + } + while (expectedSeamOrder[nextExpectedSeamIndex] !== seam && nextExpectedSeamIndex < expectedSeamOrder.length) { + nextExpectedSeamIndex += 1; + } + if (expectedSeamOrder[nextExpectedSeamIndex] !== seam) { + return new WorkflowCompileError("seams must follow the execute -> review -> merge order"); + } + seenSeams.add(seam); + nextExpectedSeamIndex += 1; + } if (cursor === endNode.id) break; cursor = mainEdge(outgoing.get(cursor) ?? [])?.to; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/workflow-compiler.ts` around lines 77 - 122, The validator accepts any placement/number of seams but the compiler expects a single canonical seam pipeline (execute -> review -> merge); update validateLinearity (in workflow-compiler.ts) to ensure at most one instance of each seam role in the correct order and connectivity: exactly one execute seam that success-targets the review seam, exactly one review seam that success-targets the merge seam, and exactly one merge seam whose failure edge (if present) targets endNode; also reject graphs with additional seam nodes, seams out of order, or multiple seams of the same role so compileWorkflowToSteps can safely assign phases.packages/dashboard/app/components/WorkflowSelector.tsx-104-116 (1)
104-116:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReset the displayed default workflow when the project context changes.
valuesurvives acrossprojectIdchanges here. Because the error path on Lines 110-112 is silent, a failed fetch leaves the previous project's default workflow visible indefinitely, which is misleading and can drive writes against the wrong baseline.Suggested fix
useEffect(() => { let cancelled = false; + setValue(null); fetchProjectDefaultWorkflow(projectId) .then((res) => { if (!cancelled) setValue(res.workflowId); }) .catch(() => { - /* default is optional; ignore load failures */ + if (!cancelled) setValue(null); + /* default is optional; ignore load failures */ });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/dashboard/app/components/WorkflowSelector.tsx` around lines 104 - 116, When projectId changes the previous `value` can remain if `fetchProjectDefaultWorkflow(projectId)` fails; update the `useEffect` that calls `fetchProjectDefaultWorkflow` to immediately clear/reset `value` (via `setValue(undefined|null|''` as appropriate) when `projectId` changes and also in the `.catch()` handler so the selector doesn't show the prior project's workflow; keep the existing `cancelled` cleanup logic and only call `setValue(res.workflowId)` when not cancelled.packages/dashboard/app/components/WorkflowSelector.tsx-35-49 (1)
35-49:⚠️ Potential issue | 🟠 Major | ⚡ Quick winClear stale workflow options before refetching project-scoped data.
On Line 37 you start a new load, but the previous
workflowsarray stays in state. IfprojectIdchanges and the new fetch fails, the selector re-enables with the old project's workflow IDs, so the nextonChangecan submit an out-of-scope selection for the current project.Suggested fix
useEffect(() => { let cancelled = false; + setWorkflows([]); setLoading(true); fetchWorkflows(projectId) .then((data) => { if (!cancelled) setWorkflows(data); }) - .catch((err) => addToast?.(getErrorMessage(err) || "Failed to load workflows", "error")) + .catch((err) => { + if (!cancelled) setWorkflows([]); + addToast?.(getErrorMessage(err) || "Failed to load workflows", "error"); + }) .finally(() => { if (!cancelled) setLoading(false); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/dashboard/app/components/WorkflowSelector.tsx` around lines 35 - 49, When starting a new fetch in the useEffect for projectId, clear any stale workflows immediately so an old project's IDs can't remain selectable; call setWorkflows([]) (or reset the workflows state) right before fetchWorkflows(projectId) is invoked (inside the effect where setLoading(true) is called) and keep the cancelled guard and error handling as-is so failures won't leave old workflow options in state; reference useEffect, setLoading, setWorkflows, fetchWorkflows, projectId, and addToast to locate the change.packages/dashboard/app/components/InlineCreateCard.tsx-394-394 (1)
394-394:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReset the workflow on the other clear paths too.
selectedWorkflowIdis only cleared after a successful save.handlePlanClickandhandleSubtaskClickclear the rest of the composer but leave the workflow behind, so the next task can inherit a hidden stale workflow.Suggested fix
const handlePlanClick = useCallback(() => { const trimmed = description.trim(); if (!trimmed) { addToast("Enter a description first", "error"); return; } onPlanningMode?.(trimmed); // Clear the input after triggering planning mode setDescription(""); + setSelectedWorkflowId(null); setDependencies([]); ... }, [description, onPlanningMode, addToast]); const handleSubtaskClick = useCallback(() => { const trimmed = description.trim(); if (!trimmed) { addToast("Enter a description first", "error"); return; } onSubtaskBreakdown?.(trimmed); // Clear the input after triggering subtask breakdown setDescription(""); + setSelectedWorkflowId(null); setDependencies([]); ... }, [description, onSubtaskBreakdown, addToast]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/dashboard/app/components/InlineCreateCard.tsx` at line 394, selectedWorkflowId is only cleared after a successful save, so other clear paths (handlePlanClick and handleSubtaskClick) leave a stale workflow selected; update those handlers to call setSelectedWorkflowId(null) (and any other "clear composer" helper functions used by them) so the workflow is reset whenever the composer is cleared or a new plan/subtask flow is started, ensuring no hidden inherited workflow remains.packages/dashboard/app/components/NewTaskModal.tsx-57-57 (1)
57-57:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDiscard/reset flow still carries the last workflow selection.
selectedWorkflowIdis not included in the dirty-state check andhandleClosenever clears it. If the user only changes the workflow, close skips the discard prompt; after reopening, the old workflow is still selected and can be applied to the next task accidentally.Suggested fix
useEffect(() => { const isDirty = description.trim() !== "" || dependencies.length > 0 || pendingImages.length > 0 || + selectedWorkflowId !== null || executorModel !== "" || validatorModel !== "" || planningModel !== "" || thinkingLevel !== "" || selectedWorkflowSteps.length > 0 || selectedAgentId !== null || reviewLevel !== undefined || autoMerge !== undefined || priority !== DEFAULT_TASK_PRIORITY || nodeId !== undefined || branchMode !== "project-default" || branch !== "" || baseBranch !== "" || githubTrackingEnabled || githubRepoOverrideTrimmed !== ""; setHasDirtyState(isDirty); - }, [description, dependencies, pendingImages, executorModel, validatorModel, planningModel, thinkingLevel, selectedWorkflowSteps, selectedAgentId, reviewLevel, autoMerge, priority, nodeId, branchMode, branch, baseBranch, githubTrackingEnabled, githubRepoOverrideTrimmed]); + }, [description, dependencies, pendingImages, selectedWorkflowId, executorModel, validatorModel, planningModel, thinkingLevel, selectedWorkflowSteps, selectedAgentId, reviewLevel, autoMerge, priority, nodeId, branchMode, branch, baseBranch, githubTrackingEnabled, githubRepoOverrideTrimmed]); const handleClose = useCallback(async () => { ... setSelectedPresetId(""); setPresetMode("default"); + setSelectedWorkflowId(null); setSelectedWorkflowSteps([]); ... }, [hasDirtyState, onClose, pendingImages, confirm]);Also applies to: 305-305
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/dashboard/app/components/NewTaskModal.tsx` at line 57, The modal's workflow selection isn't part of the dirty-state and isn't cleared on close; update the dirty check to include selectedWorkflowId and ensure handleClose (and any successful submit/close paths) calls setSelectedWorkflowId(null) to reset it; specifically modify the dirty-state logic where other fields are compared to include selectedWorkflowId and add setSelectedWorkflowId(null) to the handleClose function (and form submission/cleanup paths) so reopening the modal doesn't retain the previous workflow selection.packages/dashboard/app/components/InlineCreateCard.tsx-343-354 (1)
343-354:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftPersist the workflow selection in the same create-task transaction.
This follow-up request can land after the new task is already visible to the executor. Because workflow selection materializes the task’s compiled steps, there is a race where the task starts with the inherited/default workflow before
selectTaskWorkflow(...)runs. Thread the workflow id throughonSubmitor add a single create-and-select endpoint instead of patching it afterward.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/dashboard/app/components/InlineCreateCard.tsx` around lines 343 - 354, The current flow applies selectTaskWorkflow(task.id, selectedWorkflowId, projectId) after creating the task, causing a race; instead thread the selectedWorkflowId through the create path so the task is created with the workflow in one atomic operation: update the UI call site (InlineCreateCard's onSubmit) to pass selectedWorkflowId into the createTask request, and either extend the existing createTask API to accept a workflowId and set it server-side (in the same DB transaction) or add a new create-and-select endpoint that calls selectTaskWorkflow inside the creation transaction; remove the post-create selectTaskWorkflow call to avoid the race.packages/dashboard/src/routes/register-workflow-routes.ts-185-191 (1)
185-191:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
/workflow/inputcurrently clears any pause reason.This handler resumes the task unconditionally after adding a steering comment. That lets callers clear unrelated pause gates, including CLI approval pauses, without satisfying the guard the engine is waiting on. Only unpause when the task is in the await-input state; otherwise keep the task paused and just record the comment.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/dashboard/src/routes/register-workflow-routes.ts` around lines 185 - 191, The handler currently unconditionally clears pause info after addSteeringComment; instead, fetch the task before updating (e.g., call store.getTask or store.getTaskById with req.params.taskId), check its status, and only call store.updateTask(..., { status: null, paused: false, pausedReason: null }) when the task.status === "await-input"; otherwise just record the comment via store.addSteeringComment and leave the pause fields untouched. Ensure you use the existing getProjectContext, store.addSteeringComment, and store.updateTask calls and the same req.params.taskId to locate the task.packages/dashboard/src/routes/register-workflow-routes.ts-136-140 (1)
136-140:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't treat an omitted
workflowIdas an explicit clear.
undefinedcurrently follows the same branch asnull, so a malformed body like{}silently deletes the task's workflow selection instead of failing validation. Keepnullas the only clear signal and return400when the field is missing.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/dashboard/src/routes/register-workflow-routes.ts` around lines 136 - 140, The handler in register-workflow-routes.ts currently treats both null and undefined the same for workflowId; change the logic so only explicit null clears the selection: check if workflowId === null to call store.clearTaskWorkflowSelection(req.params.taskId) and return the null response, but if workflowId === undefined (field omitted) respond with a 400 validation error (res.status(400).json({...}) or call next with a BadRequest) instead of clearing; update any references to workflowId in this route handler to use the strict null check and a separate branch for undefined.packages/dashboard/src/__tests__/workflow-routes.test.ts-90-149 (1)
90-149: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winAdd negative-path coverage for the new mutation endpoints.
This suite only pins the happy paths. A couple of focused regressions for
{}vs{ workflowId: null }, mismatched CLI approval payloads, and/workflow/inputon a non-await-input pause would lock down the safety contract these routes are supposed to enforce.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/dashboard/src/__tests__/workflow-routes.test.ts` around lines 90 - 149, Add focused negative-path tests to packages/dashboard/src/__tests__/workflow-routes.test.ts: add cases for PUT /api/tasks/:taskId/workflow sending an empty body ({}) and a body with { workflowId: null } and assert 400/422/expected error; add tests that send a CLI-style approval payload that doesn't match the task's expected approval shape and assert rejection (use the same put helper and task created via store.createTask), and add a test that posts to POST /api/tasks/:taskId/workflow/input for a task paused not on an await-input step and assert 409/422 (non-await-input pause) to ensure the routes enforce the safety contract. Reference the existing test helpers post/put/get, store.createTask, and the routes /api/tasks/:taskId/workflow and /api/tasks/:taskId/workflow/input when adding these assertions.packages/dashboard/src/routes/register-workflow-routes.ts-169-176 (1)
169-176:⚠️ Potential issue | 🟠 Major | ⚡ Quick winOnly approve the command the task is actually paused on.
req.body.commandoverrides the command parsed fromtask.pausedReason, so this endpoint can whitelist an arbitrary CLI command and clear the pause without ever matching the pending approval prompt. The route should require a CLI-approval pause state and approve only that exact command.🔒 Proposed fix
const { store } = await getProjectContext(req); const task = await store.getTask(req.params.taskId); const reason = task.pausedReason ?? ""; const match = /^workflow-cli-approval:[^:]+:\s*(.*)$/s.exec(reason); - const command = (req.body?.command as string | undefined) ?? (match ? match[1].trim() : ""); - if (!command) throw badRequest("No pending CLI command to approve for this task"); + if (!task.paused || !match) { + throw badRequest("Task is not waiting for CLI approval"); + } + const command = match[1].trim(); + const requestedCommand = + typeof req.body?.command === "string" ? req.body.command.trim() : undefined; + if (requestedCommand && requestedCommand !== command) { + throw badRequest("Approved command does not match the pending CLI command"); + } await store.approveWorkflowCliCommand(command); await store.updateTask(req.params.taskId, { status: null, paused: false, pausedReason: null }); res.json({ approved: command });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/dashboard/src/routes/register-workflow-routes.ts` around lines 169 - 176, The route currently lets req.body.command override the command parsed from task.pausedReason, letting callers approve arbitrary commands; change the logic in the handler (use store.getTask, task.pausedReason, approveWorkflowCliCommand, updateTask) to first verify the task is paused and that task.pausedReason matches the /^workflow-cli-approval:[^:]+:\s*(.*)$/s pattern, extract the expectedCommand from that match, then require that either no req.body.command is provided or that req.body.command === expectedCommand (throw badRequest otherwise), and finally call store.approveWorkflowCliCommand(expectedCommand) and clear the pause via store.updateTask only for that exact extracted command.packages/dashboard/app/components/AppModals.tsx-32-32 (1)
32-32:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSync the dashboard lazy-load inventory for
WorkflowNodeEditor.This adds a new App-level
React.lazy()import, but the dashboard rule requires the AGENTS lazy-load inventory andpackages/dashboard/app/__tests__/lazy-loaded-views-docs.test.tsto stay in sync with App lazy imports. Please update that contract alongside this change, or keep this modal statically imported instead.As per coding guidelines,
packages/dashboard/app/**/*.{ts,tsx}must keep the AGENTS inventory of 19 lazy-loaded views in sync across App lazy imports andpackages/dashboard/app/__tests__/lazy-loaded-views-docs.test.ts.Also applies to: 389-399
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/dashboard/app/components/AppModals.tsx` at line 32, You added a new React.lazy import for WorkflowNodeEditor which breaks the dashboard's lazy-load inventory contract; either add WorkflowNodeEditor to the AGENTS lazy-load inventory used by the dashboard (the list asserted in packages/dashboard/app/__tests__/lazy-loaded-views-docs.test.ts) so the test remains in sync with App lazy imports, or revert to a static import for WorkflowNodeEditor so no inventory change is required. Locate the lazy import line (WorkflowNodeEditor) and update the test's array/count of lazy-loaded views to include this entry (keeping total at 19 if intended) or remove lazy-loading for WorkflowNodeEditor to preserve the existing inventory.packages/dashboard/app/components/WorkflowNodeEditor.tsx-152-168 (1)
152-168:⚠️ Potential issue | 🟠 Major | ⚡ Quick winConfig keys can never be removed from a selected node.
updateSelectedData()always mergespatch.configinto the previous config, so deletion flows never stick. The clear-maxRetriespath on Lines 494-496 already hits this, and the same problem leaves stale executor- or CLI-specific fields behind after switching modes. Saved workflows can therefore keepmaxRetries,cliCommand,modelId, etc. even after the UI appears to clear them.Suggested direction
- const updateSelectedData = useCallback( - (patch: Partial<WorkflowFlowNodeData> | { config: Record<string, unknown> }) => { + const updateSelectedData = useCallback( + ( + patch: + | Partial<WorkflowFlowNodeData> + | { + config: + | Record<string, unknown> + | ((prev: Record<string, unknown>) => Record<string, unknown>); + }, + ) => { if (!selectedNodeId) return; setNodes((ns) => ns.map((n) => n.id === selectedNodeId ? { ...n, data: { ...n.data, ...("config" in patch - ? { config: { ...n.data.config, ...patch.config } } + ? { + config: + typeof patch.config === "function" + ? patch.config((n.data.config ?? {}) as Record<string, unknown>) + : { ...(n.data.config ?? {}), ...patch.config }, + } : patch), }, } : n, ), ); },Then the clear paths can pass an updater that returns the exact next config object.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/dashboard/app/components/WorkflowNodeEditor.tsx` around lines 152 - 168, updateSelectedData currently always merges patch.config into the existing node.data.config which prevents removals; change it so when patch is of form { config: ... } it replaces node.data.config with the provided object (or accepts an updater function that returns the exact next config) instead of shallow-merging. Update the logic inside updateSelectedData (and the setNodes mapping) to detect "config" in patch and set data.config = typeof patch.config === "function" ? patch.config(n.data.config) : patch.config so clear paths can pass a full config object/updater and deletions will persist; keep existing merge behavior only for non-config patches.packages/dashboard/app/components/WorkflowNodeEditor.tsx-234-250 (1)
234-250:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRemove the invalid
react-hooks/exhaustive-depseslint suppression
// eslint-disable-next-line react-hooks/exhaustive-depsis itself a hard ESLint error here because thereact-hooks/exhaustive-depsrule isn’t defined in this repo’s ESLint config. Remove that suppression and update theuseEffectdependency array to cover the values used inside the effect (e.g.,projectId,addToast,selectedNode?.data.kind, and the relevant resource/loading state; includefetchModels/fetchAgents/fetchDiscoveredSkillstoo if they aren’t memoized).File:
packages/dashboard/app/components/WorkflowNodeEditor.tsx(around theuseEffectat lines 234-250; disable at ~249)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/dashboard/app/components/WorkflowNodeEditor.tsx` around lines 234 - 250, Remove the invalid eslint suppression line and expand the useEffect dependency array in WorkflowNodeEditor so it includes all values referenced inside: currentExecutor, selectedNode?.id, selectedNode?.data?.kind, projectId, addToast, models.length, agents.length, skills.length, and the fetch functions (fetchModels, fetchAgents, fetchDiscoveredSkills) unless those fetch functions are memoized; ensure the effect body still conditionally calls fetchModels, fetchAgents, or fetchDiscoveredSkills and updates state (setModels, setAgents, setSkills) as before, and if the fetch functions are not stable consider wrapping them in useCallback or otherwise stabilizing them to avoid unnecessary re-runs.packages/dashboard/app/components/TaskCard.tsx-1709-1713 (1)
1709-1713:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPrevent duplicate badge rendering for awaiting-input state.
When
isAwaitingInputis true, this dedicated "Needs input" badge renders. However, the regular status badge at line 1714 also renders because its condition!isPaused && visualStatus && visualStatus !== "queued"evaluates to true (isPausedchecks thepaused/userPausedflags, not the status string). This results in two badges displaying simultaneously: "Needs input" and "awaiting-user-input".Either:
- Add
!isAwaitingInputto the condition on line 1714, OR- Integrate awaiting-input handling into the regular status badge (lines 1714-1720) similar to how awaiting-approval is handled (line 1718).
Option 2 is more consistent with the existing pattern.
Recommended fix: integrate into regular status badge
- {isAwaitingInput && ( - <span className="card-status-badge awaiting-input"> - Needs input - </span> - )} - {!isPaused && visualStatus && visualStatus !== "queued" && ( + {!isPaused && !isAwaitingInput && visualStatus && visualStatus !== "queued" && ( <span className={`card-status-badge card-status-badge--${task.column}${isAwaitingApproval ? " awaiting-approval" : ""}${ACTIVE_STATUSES.has(visualStatus) ? " pulsing" : ""}${isFailed ? " failed" : ""}${isStuck ? " stuck" : ""}`} > {isStuck ? "Stuck" : isAwaitingApproval ? "Awaiting Approval" : getTaskStatusLabel(visualStatus)} </span> )} + {isAwaitingInput && ( + <span className="card-status-badge awaiting-input"> + Needs input + </span> + )}Or, integrate like awaiting-approval:
- {isAwaitingInput && ( - <span className="card-status-badge awaiting-input"> - Needs input - </span> - )} - {!isPaused && visualStatus && visualStatus !== "queued" && ( + {!isPaused && visualStatus && visualStatus !== "queued" && ( <span - className={`card-status-badge card-status-badge--${task.column}${isAwaitingApproval ? " awaiting-approval" : ""}${ACTIVE_STATUSES.has(visualStatus) ? " pulsing" : ""}${isFailed ? " failed" : ""}${isStuck ? " stuck" : ""}`} + className={`card-status-badge card-status-badge--${task.column}${isAwaitingApproval ? " awaiting-approval" : ""}${isAwaitingInput ? " awaiting-input" : ""}${ACTIVE_STATUSES.has(visualStatus) ? " pulsing" : ""}${isFailed ? " failed" : ""}${isStuck ? " stuck" : ""}`} > - {isStuck ? "Stuck" : isAwaitingApproval ? "Awaiting Approval" : getTaskStatusLabel(visualStatus)} + {isStuck ? "Stuck" : isAwaitingApproval ? "Awaiting Approval" : isAwaitingInput ? "Needs input" : getTaskStatusLabel(visualStatus)} </span> )}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/dashboard/app/components/TaskCard.tsx` around lines 1709 - 1713, The "Needs input" badge is duplicating the regular status badge; update the regular status-badge render logic in TaskCard.tsx so it handles the awaiting-input case like awaiting-approval instead of rendering both: modify the condition that currently uses isPaused, visualStatus and visualStatus !== "queued" to also account for isAwaitingInput (or better, remove the separate span and integrate awaiting-input by mapping isAwaitingInput to the same badge rendering path), and when isAwaitingInput is true render the status badge with the awaiting-user-input class/text; touch the symbols isAwaitingInput, visualStatus and the status-badge rendering block (the existing awaiting-approval handling) to mirror that pattern.packages/dashboard/app/components/WorkflowResultsTab.tsx-229-235 (1)
229-235:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReset the paused-action UI state when the blocked node/task changes.
submittedandinputTextnever get cleared after a successful resume. If the same modal later hits anotherawaiting-user-input/awaiting-cli-approvalpause, the banner stays stuck onResuming…and the user loses the controls until reload.Suggested fix
const [inputText, setInputText] = useState(""); const [submitting, setSubmitting] = useState(false); const [submitted, setSubmitted] = useState(false); const [expandedViewStepId, setExpandedViewStepId] = useState<string | null>(null); const [allWorkflowSteps, setAllWorkflowSteps] = useState<WorkflowStep[]>([]); const [isEditing, setIsEditing] = useState(false); const [selectedWorkflowId, setSelectedWorkflowId] = useState<string | null>(null); + useEffect(() => { + setInputText(""); + setSubmitting(false); + setSubmitted(false); + }, [taskId, taskStatus, taskPausedReason]); +Also applies to: 687-766
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/dashboard/app/components/WorkflowResultsTab.tsx` around lines 229 - 235, The UI stays stuck because inputText and submitted are not reset when the paused node/task changes; update the logic in WorkflowResultsTab.tsx to clear these states whenever the pause target changes or when a resume completes—e.g., in the handler that sets expandedViewStepId or selectedWorkflowId (and in any function that updates allWorkflowSteps or handles resume completion), call setInputText("") and setSubmitted(false) so new awaiting-user-input / awaiting-cli-approval pauses start with a fresh UI; ensure the same resets are applied in the code paths around resume action completion and where pause-related data is swapped (also apply similar resets in the other block referenced lines 687-766).packages/engine/src/workflow-graph-executor.ts-173-180 (1)
173-180:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
maxRetriesis being applied as total attempts.Both
maxRetriesPerNodeandnode.config.maxRetriesread as retry counts, but this loop executes onlymaxAttemptstimes total. So a value of2gives one retry instead of two. Either rename the public contract tomaxAttempts, or keep the current naming and add the initial attempt on top of the retry count.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/engine/src/workflow-graph-executor.ts` around lines 173 - 180, The code treats maxRetries as total attempts; change it to treat maxRetries as retry counts by adding the initial attempt. Compute attempts as retries+1: when node.config?.maxRetries is provided use attempts = Math.min(10, Math.floor(configured)) + 1 (allow configured >= 0), otherwise use attempts = this.maxRetriesPerNode + 1; keep the existing cap semantics on the retry count and then run the for loop for attempt = 0; attempt < attempts; attempt++ (symbols: node.config?.maxRetries, configured, maxAttempts, maxRetriesPerNode, the for loop with attempt).packages/engine/src/workflow-node-handlers.ts-78-84 (1)
78-84:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFail closed when an executable gate has no runner.
A gate with
prompt/scriptNamecurrently returns success whenrunCustomNodeis missing, which silently bypasses the gate and lets the workflow continue. Custom prompt/script nodes already throw in this situation; gate nodes should do the same instead of auto-passing.Suggested fix
const hasExecutableConfig = typeof node.config?.prompt === "string" || typeof node.config?.scriptName === "string"; - if (hasExecutableConfig && runCustomNode) { - return runCustomNode(node, context.task, context.context); + if (hasExecutableConfig) { + if (!runCustomNode) { + throw new WorkflowIrError(`No custom-node runner registered for node: ${node.id}`); + } + return runCustomNode(node, context.task, context.context); } return { outcome: "success" };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/engine/src/workflow-node-handlers.ts` around lines 78 - 84, The gate handler currently treats nodes with executable config (hasExecutableConfig) as success when runCustomNode is missing, letting gates silently pass; change the logic in the block that checks hasExecutableConfig and runCustomNode so that if hasExecutableConfig is true but runCustomNode is undefined, the handler throws an Error (or returns a failing outcome consistent with other custom node behavior) instead of returning { outcome: "success" }; update the branch around runCustomNode(node, context.task, context.context) to throw a clear error referencing the missing runner for the node (include node.id or node.type in the message) so gate nodes fail closed.packages/engine/src/executor.ts-3238-3253 (1)
3238-3253:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCatch thrown interpreter errors and continue with legacy execution.
Only the explicit
"fell-back"disposition reaches the legacy path here. IfWorkflowGraphTaskRunnerthrows instead,execute()exits before taking the normal executor lock, so the caller just logs the error and the task stays stranded inin-progressuntil some later recovery loop notices it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/engine/src/executor.ts` around lines 3238 - 3253, The code currently assumes WorkflowGraphTaskRunner.run won't throw, so thrown interpreter errors bypass the legacy path and leave tasks stuck; wrap the graph execution block (the new WorkflowGraphTaskRunner instantiation and the await this.store.getTask / await runner.run(detail, settings) sequence inside a try/catch around execute()'s graph branch, catch any error thrown by runner.run (or getTask), log it via executorLog.log including the error message/context, then return false so the caller proceeds with the legacy pipeline; keep existing handling for result.disposition ("fell-back" and "failed") intact (referencing WorkflowGraphTaskRunner, runner.run, executorLog.log, handleGraphFailure).packages/engine/src/executor.ts-3358-3367 (1)
3358-3367:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftCorrelate await-input replies to the pause that asked for them.
steering.length > 0is enough to markaskedBefore, so an await-input node can immediately succeed on the first visit by consuming an unrelated earlier steering comment. On resume it still cannot tell whether the latest comment predates the pause. This needs a per-node comment/timestamp watermark recorded when the node pauses, then only comments newer than that watermark should be treated as the reply.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/engine/src/executor.ts` around lines 3358 - 3367, The code currently treats any existing steeringComments as a valid reply; to fix, record a per-node watermark timestamp when the node actually pauses (e.g., store a numeric pauseTimestamp or steeringWatermark on the live/task record via this.store.updateTask at the moment you set live.paused) and on resume (in the logic around live.steeringComments / askedBefore and the block that consumes latest comment) filter steeringComments to only those with timestamps greater than that watermark (use the comment timestamp if present or the comment creation time), then consume the newest comment after the watermark and clear the watermark via this.store.updateTask when you mark the node resumed; update references to live.paused, askedBefore, and the consume block for node.id to use this watermark-based filtering.packages/engine/src/project-engine.ts-356-358 (2)
356-358:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSupport multiple waiters per task before exposing
onMerge()to another caller path.This wiring makes
onMerge(taskId)reachable from both the existing manual surface and the workflow interpreter, butmanualMergeResolversonly keeps a single resolver per task. A second request for the same task replaces the first entry, and only the last promise is ever resolved/rejected indrainMergeQueue(), leaving the earlier caller hung indefinitely. Store a shared promise or a resolver list per task before adding another producer here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/engine/src/project-engine.ts` around lines 356 - 358, The current wiring exposes onMerge via runtime.setMergeRequester but manualMergeResolvers stores only a single resolver per task, so subsequent callers overwrite earlier ones and get hung; change manualMergeResolvers (and related code) to hold either a shared Promise or a list/array of resolvers per task (e.g., map taskId -> Array<resolve/reject> or map taskId -> sharedPromise) and push new waiters when onMerge(taskId) is called, and update drainMergeQueue() to resolve or reject all stored resolvers (or settle the shared promise) for that task instead of only the last one; ensure code paths that remove entries from manualMergeResolvers clear the entire list and handle errors for every waiter.
356-358:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon’t route interpreter-driven merges through the manual-merge bypass.
onMerge()skips the normal auto-merge eligibility checks whenever amanualResolverexists, so this new workflow seam can still merge anin-reviewtask whensettings.autoMergeis false. That breaks the existing engine contract that auto-merge-off leavesin-reviewterminal until a human merges it. Please gate this seam separately (or have it return a non-merged/manual-required disposition) unless the caller is an actual human-triggered merge.Based on learnings, "When
settings.autoMerge: false,in-reviewis terminal-until-merged by a human."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/engine/src/project-engine.ts` around lines 356 - 358, The new interpreter merge seam calls this.runtime.setMergeRequester with a callback to onMerge(taskId), but onMerge bypasses auto-merge gating when manualResolver exists, allowing interpreter-driven merges to merge tasks even when settings.autoMerge is false; change the seam to either (a) call a new distinct method (e.g., requestInterpreterMerge or onInterpreterMerge) that enforces auto-merge rules and returns a non-merged/manual-required disposition when settings.autoMerge is false, or (b) modify onMerge to accept a caller-type flag (e.g., caller = 'human'|'interpreter') and early-check settings.autoMerge and manualResolver so interpreter calls do not bypass the normal auto-merge-off behavior; update the runtime.setMergeRequester registration to pass the appropriate caller-type when invoking the merge callback.
🟡 Minor comments (6)
docs/plans/2026-06-03-001-feat-executable-custom-workflows-node-editor-plan.md-120-138 (1)
120-138:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd fence languages to satisfy markdown lint.
Line 120 and Line 159 use unlabeled fenced blocks (
MD040); if markdownlint runs in CI, this can fail the docs check.Suggested patch
-``` +```text WorkflowIr graph compiled WorkflowStep set ───────────────── ───────────────────────── start ─▶ [gate: lint] ┐ @@ -> else: WorkflowCompileError("graph requires interpreter (deferred)") -``` +``` @@ -``` +```text packages/core/src/ workflow-definition-types.ts # NEW: WorkflowDefinition, layout, input types workflow-compiler.ts # NEW: compileWorkflowToSteps + validateLinearity @@ __tests__/workflow-node-editor.test.tsx # NEW -``` +```Also applies to: 159-185
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/plans/2026-06-03-001-feat-executable-custom-workflows-node-editor-plan.md` around lines 120 - 138, The issue is unlabeled fenced code blocks in the docs that trigger markdownlint MD040; update the two unlabeled fences around the WorkflowIr / compiled WorkflowStep diagram and the file list so they use a language label (e.g., "text") to satisfy linting. Locate the blocks adjacent to the validateLinearity(ir) description and the file listing that references workflow-definition-types.ts and workflow-compiler.ts and change the opening backticks to include `text` (so the fences read ```text) while keeping the content unchanged; no code logic in validateLinearity or WorkflowCompileError needs modification.packages/core/src/__tests__/workflow-definition-store.test.ts-67-93 (1)
67-93:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMake the
updatedAtcheck strict.This test says the update advances
updatedAt, buttoBeGreaterThanOrEqual(...)still passes when the timestamp is unchanged. That leaves the regression untested.Suggested assertion fix
- expect(new Date(updated.updatedAt).getTime()).toBeGreaterThanOrEqual( + expect(new Date(updated.updatedAt).getTime()).toBeGreaterThan( new Date(created.updatedAt).getTime(), );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/__tests__/workflow-definition-store.test.ts` around lines 67 - 93, The test currently uses a non-strict assertion for updatedAt which allows unchanged timestamps to pass; change the assertion in the test "updates name, description, IR, and layout and advances updatedAt" to assert that updated.updatedAt is strictly greater than created.updatedAt (use a strict comparison on the numeric timestamps from new Date(...).getTime()) after calling store.updateWorkflowDefinition so the test fails if the timestamp did not advance.packages/dashboard/app/components/workflow-flow-mapping.ts-56-65 (1)
56-65:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPreserve absent node names on save.
irToFlow()gives unnamed nodes a fallback label, butflowToIr()writes every truthy label back intoconfig.name. Saving an untouched workflow therefore injects synthetic names like"start","end", or generated node IDs, so the IR is no longer round-trippable.Suggested fix
const irNodes: WorkflowIr["nodes"] = nodes.map((node) => { const data = node.data; const config: Record<string, unknown> = { ...(data.config ?? {}) }; - if (data.label) config.name = data.label; + const fallbackLabel = data.kind === "merge" ? "Merge boundary" : node.id; + if ( + data.kind !== "start" && + data.kind !== "end" && + data.label && + data.label !== fallbackLabel + ) { + config.name = data.label; + } else { + delete config.name; + } if (data.kind === "merge") { config.seam = "merge"; return { id: node.id, kind: "prompt", config }; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/dashboard/app/components/workflow-flow-mapping.ts` around lines 56 - 65, The mapping currently copies any truthy node.data.label into config.name which writes synthetic fallback labels back into the IR; instead only set config.name when the original node config actually contained a name (i.e. use data.config?.name) so that labels generated by irToFlow (fallbacks like "start", "end" or auto IDs) are not persisted. Modify the irNodes construction in the nodes.map callback (referencing irNodes, nodes.map, and the local variable config) to prefer data.config.name and only assign config.name when that explicit config field exists.packages/dashboard/app/components/WorkflowResultsTab.tsx-690-710 (1)
690-710:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHandle resume-action failures explicitly.
If
submitTaskWorkflowInput(...)orapproveTaskWorkflowCli(...)rejects, the click handler surfaces an unhandled rejected promise and the user gets no feedback about why resume failed. Catch the error and render an inline error or toast so retrying is actionable.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/dashboard/app/components/WorkflowResultsTab.tsx` around lines 690 - 710, Both handlers (handleSubmitInput and handleApproveCli) currently use try/finally and swallow rejections, causing unhandled promise rejections and no user feedback; update each to use try/catch/finally, catch the error from submitTaskWorkflowInput and approveTaskWorkflowCli, set a local error state (e.g., resumeError via setResumeError) or call the app's toast utility to surface the error message, and keep the existing setSubmitting(true)/finally setSubmitting(false) behavior; ensure handleSubmitInput still clears inputText and sets submitted only on success, and reference the existing functions/vars (handleSubmitInput, handleApproveCli, submitTaskWorkflowInput, approveTaskWorkflowCli, setInputText, setSubmitted, setSubmitting) so the UI can render the inline error or toast and allow retry.packages/dashboard/app/components/TaskDetailModal.tsx-2745-2748 (1)
2745-2748:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTreat the new workflow wait states as paused here too.
isTaskInProgressstill only excludes the literal"paused"status, soawaiting-user-inputandawaiting-cli-approvalare now reported toWorkflowResultsTabas active runs. That keeps the live-log subscription running while the task is actually blocked on user action.Suggested fix
- isTaskInProgress={task.column === "in-progress" && task.status !== "paused"} + isTaskInProgress={ + task.column === "in-progress" + && !task.paused + && !task.userPaused + && task.status !== "paused" + && task.status !== "awaiting-user-input" + && task.status !== "awaiting-cli-approval" + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/dashboard/app/components/TaskDetailModal.tsx` around lines 2745 - 2748, Update the isTaskInProgress prop expression so it treats the new workflow wait states as paused too: change the check on task.status in the JSX where isTaskInProgress is set (the expression using task.column === "in-progress" && task.status !== "paused") to exclude "awaiting-user-input" and "awaiting-cli-approval" as well (i.e., only mark in-progress when column === "in-progress" and task.status is not one of "paused", "awaiting-user-input", "awaiting-cli-approval"); reference the isTaskInProgress prop assignment and the task.status values to locate and adjust the condition.packages/engine/src/executor.ts-3486-3496 (1)
3486-3496:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winClear the approval status once the CLI command is approved.
The await-input path resets
statusbefore resuming, but the approved-CLI path goes straight to command execution. That means a task can keep theawaiting-cli-approvalstatus through later graph nodes even though approval already happened.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/engine/src/executor.ts` around lines 3486 - 3496, The approved-CLI path never clears the stored approval, leaving tasks stuck with status awaiting-cli-approval; after the approval check (the branch where skipApproval is true or this.store.isWorkflowCliCommandApproved(rawCommand) returns true) call the store method that clears the approval for that command (e.g., this.store.clearWorkflowCliCommandApproval(rawCommand) or the equivalent clear method) before proceeding to runRawCliCommand; place the clear call just before creating env and invoking runRawCliCommand so the approval state is reset for subsequent nodes (references: skipApproval, isWorkflowCliCommandApproved, pauseForCliApproval, runRawCliCommand).
…ut, and node isolation Address PR #1363 review findings: - core: pausedReason was written in-memory and read by SELECT but never persisted by the task upsert (missing column/value) nor mapped back in rowToTask — so it was lost on every reload. Add it to both. This is the root cause behind the workflow CLI-approval / await-input pause cycle and also fixes token-budget / worktrunk pause reasons silently vanishing. - dashboard: approve-cli now derives the approved command exclusively from the task's pausedReason; a caller-supplied body.command is ignored, closing a trust-on-first-use bypass. - engine: await-input nodes resume only when THIS node paused the task (its marker on pausedReason), not on any pre-existing steering comment. - engine: write-capable custom nodes (coding/script/CLI) are refused until a task worktree exists, so they never mutate the shared repo root before the execute seam. - engine: document cliSkipApproval as an intentional workflow-author-only escape hatch; scriptName is now const (ESLint). - tests: pausedReason round-trip coverage in store-persistence; approve-cli body-command-ignored + no-pending-command coverage; built-in-aware list assertion in workflow-routes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Built-in (builtin:*) workflows are read-only at the store layer; reflect that in the editor so users aren't misled into editing changes that can't save. For a built-in: the node palette is disabled, the inspector fields are wrapped in a disabled fieldset, Save/Delete are replaced by a "Read-only built-in" label, and a "Duplicate to edit" action clones it into an editable user workflow. handleSave/handleDeleteWorkflow also early-return for built-ins as defense-in-depth. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…al pause The node "Auto-approve requests" toggle was captured but unused. The only human-approval pause reachable from a custom node is the CLI first-run trust-on-first-use gate (review-style nodes run as ephemeral readonly agents with no permission gate), so autoApprove now bypasses that pause — a superset of the CLI-specific cliSkipApproval flag. The inspector explains the effect when the toggle is on. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…gine-slow Shard 4 still wedged after the first quarantine — the hang consistently follows the branch-group fn-001 worktree tests (merge-routing, automerge-precedence, promotion-gate, pr-sync, single-pr-e2e), with the engine vitest process dying before printing a summary. These are the suites with known pre-existing failures (per-task-derived derivation). Move the family to *.slow.test.ts — the non-required engine-slow lane — alongside the worktree-invariants and shared-branch-group files. Live-git coverage preserved via test:slow/test:all. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…outrun it Migration 102 defers the destructive agentLogEntries drop until TaskStore copies legacy rows to JSONL and writes the __meta guard, then relies on a second init() pass gated on schemaVersion < SCHEMA_VERSION. Migrations 103-105 bump the version to 105 on the first pass, so the second pass never fired and the legacy table survived forever. Make the drop version-independent in migrate() and trigger the re-init whenever the legacy table remains. Also pin secrets-schema.test.ts to String(SCHEMA_VERSION) instead of the hardcoded "102" string the schema bump invalidated. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- db.ts: restrict migration-105 orphan-step cleanup to JSON arrays (json_type guard so json_each can't expand objects/strings) - project-engine.ts: requestInterpreterMerge throws on null task lookup instead of casting null into MergeResult (seam converts to clean failure) - executor.ts: truncate dual-observe shadow stage walk at the live terminal stage so healthy in-review tasks don't record a phantom merge transition Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Autofix skipped. No unresolved CodeRabbit review comments with fix instructions found. |
Summary
Adds executable custom workflows with a visual graph editor, and a flag-gated interpreter that can drive a task's whole lifecycle.
Author a workflow as a graph (start → prompt/script/gate/await-input → seams → end) in a new React Flow editor, then select it per task or as a project default — including from the new-task modal and quick-add card. Selected workflows compile to the existing WorkflowStep engine and run at the pre/post-merge boundaries with no change to the scheduler/executor/merger (legacy path is byte-for-byte unchanged with the flag off). Non-linear graphs are rejected with a clear message.
Behind
experimentalFeatures.workflowGraphExecutor, theWorkflowGraphTaskRunnerpromotes the graph to authoritative: planning → execute → review → merge seams delegate to the real engine implementations, custom nodes run on the WorkflowStep machinery, and any interpreter error falls back to legacy (a task is never stranded).What's included
workflows+task_workflow_selectiontables (migrations 103/104), the IR→steps compiler with linearity validation, per-task selection + project-default inheritance, and built-in workflows (Coding, Quick fix, Review-heavy, Compound engineering — invokingce-plan/ce-code-review/ce-compound).ProjectEngine.onMerge, and a flag-gated entry point with process-wide routing claims, pause-aware failure handling, and a merge-seam timeout.Testing
ce-code-review; the P0/P1 engine-safety cluster (recovery/liveness races, merge-seam timeout, cascade cleanup) was fixed in-branch.Known follow-ups
autoApprovetoggle is captured but not yet enforced in the permission policy.docs/plans/2026-06-03-002-...-cutover-plan.mdand deferred.fn_workflow_*agent tools yet (agents can't author/select workflows that users can); built-in editor edit/delete buttons aren't disabled yet (they error on attempt).Post-Deploy Monitoring & Validation
The interpreter is off by default (
experimentalFeatures.workflowGraphExecutorunset), so production behavior is unchanged on merge. When enabling for a project: watch[workflow-graph]log lines forfallback/faileddispositions andmerge-timeout; confirm graph-selected tasks reachdoneand thatawaiting-user-input/awaiting-cli-approvaltasks surface the badge/banner. Rollback = unset the flag (no schema rollback needed; tables are additive).🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests