From afcb2110841515d175c86d677a90ca4e87e6ce62 Mon Sep 17 00:00:00 2001
From: Tym Rabchuk <tymrabchuk@gmail.com>
Date: Sat, 4 Apr 2026 17:52:04 -0400
Subject: [PATCH 01/11] Add generic YAML workflow engine with triage,
 scratchpads, and dirty-bit hook
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Replaces the need for hardcoded Go workflow implementations with a single
engine that reads and executes workflow YAML files deterministically —
zero tokens spent on orchestration, exact loop counting, and reliable
branch evaluation.

Engine (src/engine/):
- YAML parser with validation (types, duplicate IDs, branch targets)
- Sequential step execution with {{variable}} interpolation
- Branch evaluation (case-insensitive substring match → goto)
- Loop management (hard counter + until-string match)
- Parallel step dispatch via goroutines
- Budget enforcement, scratchpad setup/cleanup, session tracking

CLI: `devkit workflow <name> <description>` and `devkit workflow list`

Triage-based phase skipping:
- feature.yml: TINY/SMALL/MEDIUM/LARGE classification with fast paths
- bugfix.yml: TRIVIAL/NORMAL/COMPLEX classification with fast path

Scratchpads (skills/scratchpad/):
- Iteration memory protocol at .devkit/scratchpads/current.md
- Prevents Groundhog Day loops by recording what was tried
- Integrated into stuck skill recovery protocol

Dirty-bit feedback loop (hooks/dirty-bit.sh):
- Stop hook that detects cross-domain changes (backend/frontend/config/sql)
- Blocks completion if any touched domain lacks test evidence

18 engine tests covering parsing, interpolation, branching, loops,
budget, parallel dispatch, context cancellation, and all 12 real workflows.
---
 hooks/dirty-bit.sh         |  95 ++++++
 hooks/hooks.json           |  14 +-
 skills/scratchpad/SKILL.md |  56 ++++
 skills/stuck/SKILL.md      |   4 +
 src/cmd/workflow.go        | 132 +++++++++
 src/engine/engine.go       | 340 ++++++++++++++++++++++
 src/engine/engine_test.go  | 571 +++++++++++++++++++++++++++++++++++++
 src/engine/workflow.go     | 128 +++++++++
 src/go.mod                 |   1 +
 src/go.sum                 |   2 +
 workflows/bugfix.yml       |  42 ++-
 workflows/feature.yml      |  52 +++-
 12 files changed, 1432 insertions(+), 5 deletions(-)
 create mode 100755 hooks/dirty-bit.sh
 create mode 100644 skills/scratchpad/SKILL.md
 create mode 100644 src/cmd/workflow.go
 create mode 100644 src/engine/engine.go
 create mode 100644 src/engine/engine_test.go
 create mode 100644 src/engine/workflow.go

diff --git a/hooks/dirty-bit.sh b/hooks/dirty-bit.sh
new file mode 100755
index 0000000..47f51a2
--- /dev/null
+++ b/hooks/dirty-bit.sh
@@ -0,0 +1,95 @@
+#!/bin/bash
+# devkit Stop hook — dirty-bit feedback loop
+#
+# Tracks which domains were modified and warns if the appropriate
+# verification (tests/lint) hasn't been run for each touched domain.
+#
+# Domains: backend (Go/Python), frontend (TS/JS/JSX/TSX), config (YAML/JSON/TOML),
+#          test files, SQL/migrations
+#
+# Stop hook schema:
+#   { "decision": "approve" | "block", "reason": "string" }
+
+set -euo pipefail
+
+INPUT=$(cat)
+TRANSCRIPT=$(echo "$INPUT" | jq -r '.transcript // empty')
+
+# Get modified files from git
+REPO_ROOT=$(git rev-parse --show-toplevel 2>/dev/null || pwd)
+CHANGED_FILES=$(cd "$REPO_ROOT" && git diff --name-only HEAD 2>/dev/null; git diff --name-only --cached 2>/dev/null; git diff --name-only 2>/dev/null)
+
+if [ -z "$CHANGED_FILES" ]; then
+  jq -n '{ decision: "approve" }'
+  exit 0
+fi
+
+# Classify changed files into domains
+BACKEND=false
+FRONTEND=false
+CONFIG=false
+SQL=false
+DOMAINS_TOUCHED=""
+
+while IFS= read -r file; do
+  case "$file" in
+    *.go|*.py|*.rb|*.java|*.rs)
+      BACKEND=true ;;
+    *.ts|*.tsx|*.js|*.jsx|*.vue|*.svelte)
+      FRONTEND=true ;;
+    *.yml|*.yaml|*.json|*.toml|*.ini|*.env*)
+      CONFIG=true ;;
+    *.sql|**/migrations/*|**/migrate/*)
+      SQL=true ;;
+  esac
+done <<< "$CHANGED_FILES"
+
+# Build list of touched domains
+if [ "$BACKEND" = "true" ]; then DOMAINS_TOUCHED="$DOMAINS_TOUCHED backend"; fi
+if [ "$FRONTEND" = "true" ]; then DOMAINS_TOUCHED="$DOMAINS_TOUCHED frontend"; fi
+if [ "$CONFIG" = "true" ]; then DOMAINS_TOUCHED="$DOMAINS_TOUCHED config"; fi
+if [ "$SQL" = "true" ]; then DOMAINS_TOUCHED="$DOMAINS_TOUCHED sql"; fi
+
+# If only one domain or no code domains, approve
+DOMAIN_COUNT=$(echo "$DOMAINS_TOUCHED" | wc -w | tr -d ' ')
+if [ "$DOMAIN_COUNT" -le 1 ]; then
+  jq -n '{ decision: "approve" }'
+  exit 0
+fi
+
+# Multiple domains touched — check for test evidence per domain
+MISSING_VERIFICATION=""
+
+if [ "$BACKEND" = "true" ]; then
+  if ! echo "$TRANSCRIPT" | grep -qiE '(go test|pytest|python.*test|cargo test|bundle exec.*test|ALL_PASSING|ALL_TESTS_PASSING)'; then
+    MISSING_VERIFICATION="$MISSING_VERIFICATION backend"
+  fi
+fi
+
+if [ "$FRONTEND" = "true" ]; then
+  if ! echo "$TRANSCRIPT" | grep -qiE '(npm test|npx jest|npx vitest|yarn test|pnpm test|ALL_PASSING|ALL_TESTS_PASSING)'; then
+    MISSING_VERIFICATION="$MISSING_VERIFICATION frontend"
+  fi
+fi
+
+if [ "$SQL" = "true" ]; then
+  if ! echo "$TRANSCRIPT" | grep -qiE '(migrate|migration.*up|schema.*applied|ALL_PASSING)'; then
+    MISSING_VERIFICATION="$MISSING_VERIFICATION sql/migrations"
+  fi
+fi
+
+# If everything verified, approve
+if [ -z "$MISSING_VERIFICATION" ]; then
+  jq -n '{ decision: "approve" }'
+  exit 0
+fi
+
+# Multiple domains touched, some unverified — block
+DOMAINS_MSG=$(echo "$DOMAINS_TOUCHED" | xargs)
+MISSING_MSG=$(echo "$MISSING_VERIFICATION" | xargs)
+
+jq -n --arg domains "$DOMAINS_MSG" --arg missing "$MISSING_MSG" '{
+  decision: "block",
+  reason: ("Cross-domain changes detected (touched: " + $domains + "). Missing test/verification evidence for: " + $missing + ". Run the relevant test suite for each domain before completing.")
+}'
+exit 0
diff --git a/hooks/hooks.json b/hooks/hooks.json
index 7586c91..bacd65d 100644
--- a/hooks/hooks.json
+++ b/hooks/hooks.json
@@ -117,6 +117,18 @@
         ]
       }
     ],
-    "Stop": []
+    "Stop": [
+      {
+        "matcher": "Stop",
+        "hooks": [
+          {
+            "type": "command",
+            "command": "${CLAUDE_PLUGIN_ROOT}/hooks/dirty-bit.sh",
+            "statusMessage": "Checking cross-domain coverage...",
+            "timeout": 10
+          }
+        ]
+      }
+    ]
   }
 }
diff --git a/skills/scratchpad/SKILL.md b/skills/scratchpad/SKILL.md
new file mode 100644
index 0000000..aaf0ee2
--- /dev/null
+++ b/skills/scratchpad/SKILL.md
@@ -0,0 +1,56 @@
+---
+name: scratchpad
+description: Persistent iteration memory — prevents Groundhog Day loops by recording what was tried, what failed, and what to try next.
+---
+
+# Scratchpad Protocol
+
+Use scratchpads in any iterative loop to prevent repeating failed approaches.
+
+## Location
+
+`.devkit/scratchpads/current.md` — active scratchpad for the current task.
+
+Create `.devkit/scratchpads/` if it doesn't exist. Only one active scratchpad at a time.
+
+## Before Each Iteration
+
+Read `.devkit/scratchpads/current.md` if it exists. Check:
+- What approaches were already tried?
+- What failed and why?
+- What was suggested to try next?
+
+**Do not repeat a failed approach.** If you're about to try something already listed as failed, stop and pick a different strategy.
+
+## After Each Iteration
+
+Append to `.devkit/scratchpads/current.md`:
+
+```markdown
+## Iteration {N} — {timestamp}
+
+**Approach:** What you tried (one sentence)
+**Result:** pass | fail
+**Details:** What happened — error message, unexpected behavior, or success details
+**Next:** What to try next if this failed, or "N/A" if it passed
+```
+
+## On Completion
+
+When the task succeeds or the workflow ends, delete `.devkit/scratchpads/current.md`.
+Don't leave stale scratchpads — they'll confuse the next task.
+
+## Integration with stuck detection
+
+If `.devkit/scratchpads/current.md` shows 3+ failed iterations:
+1. Stop iterating
+2. Review all failed approaches in the scratchpad
+3. The pattern of failures often reveals the real problem
+4. Escalate to the user with the scratchpad content as evidence
+
+## Rules
+
+- One scratchpad per active task — don't create per-agent scratchpads
+- Keep entries concise — the scratchpad is read every iteration
+- Record failures honestly — "it didn't work" is useless; "returned 404 because endpoint expects POST not GET" is useful
+- Clean up when done — stale scratchpads are worse than no scratchpads
diff --git a/skills/stuck/SKILL.md b/skills/stuck/SKILL.md
index cbfc0c9..4f24d28 100644
--- a/skills/stuck/SKILL.md
+++ b/skills/stuck/SKILL.md
@@ -16,6 +16,10 @@ You are stuck if any of these are true:
 
 ## Recovery Protocol
 
+### 0. Check the Scratchpad
+
+Read `.devkit/scratchpads/current.md` first. It records what was already tried and why it failed. If the scratchpad shows 3+ failed iterations, skip straight to **Step 5: Escalate** — the pattern of failures is the diagnosis.
+
 ### 1. Stop and Diagnose
 
 Don't retry the same approach. Read the error carefully:
diff --git a/src/cmd/workflow.go b/src/cmd/workflow.go
new file mode 100644
index 0000000..53d159b
--- /dev/null
+++ b/src/cmd/workflow.go
@@ -0,0 +1,132 @@
+package cmd
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+	"strings"
+
+	"github.com/5uck1ess/devkit/engine"
+	"github.com/5uck1ess/devkit/lib"
+	"github.com/spf13/cobra"
+)
+
+var workflowCmd = &cobra.Command{
+	Use:   "workflow [name] [description...]",
+	Short: "Run a YAML workflow by name",
+	Long:  "Execute a workflow from the workflows/ directory. The engine handles step sequencing, branching, loops, and parallel dispatch deterministically.",
+	Example: `  devkit workflow feature "add JWT authentication"
+  devkit workflow bugfix "fix null pointer in handler"
+  devkit workflow list`,
+	Args: cobra.MinimumNArgs(1),
+	RunE: func(cmd *cobra.Command, args []string) error {
+		name := args[0]
+
+		// Handle "list" subcommand
+		if name == "list" {
+			return listWorkflows()
+		}
+
+		if len(args) < 2 {
+			return fmt.Errorf("usage: devkit workflow <name> <description>")
+		}
+
+		dirty, err := (&lib.Git{Dir: repoRoot}).HasUncommittedChanges()
+		if err != nil {
+			return fmt.Errorf("check git status: %w", err)
+		}
+		if dirty {
+			return fmt.Errorf("working tree has uncommitted changes — commit or stash first")
+		}
+
+		// Find workflow file
+		wfPath := findWorkflowFile(name)
+		if wfPath == "" {
+			return fmt.Errorf("workflow %q not found — run `devkit workflow list`", name)
+		}
+
+		wf, err := engine.ParseFile(wfPath)
+		if err != nil {
+			return fmt.Errorf("parse workflow: %w", err)
+		}
+
+		agentName, _ := cmd.Flags().GetString("agent")
+		runner, err := resolveRunner(agentName)
+		if err != nil {
+			return err
+		}
+
+		budget, _ := cmd.Flags().GetFloat64("budget")
+
+		eng := &engine.Engine{
+			DB:       db,
+			Git:      &lib.Git{Dir: repoRoot},
+			Runner:   runner,
+			RepoRoot: repoRoot,
+		}
+
+		description := strings.Join(args[1:], " ")
+		result, err := eng.RunWorkflow(cmd.Context(), wf, engine.RunConfig{
+			Input:     description,
+			BudgetUSD: budget,
+		})
+		if err != nil {
+			return err
+		}
+
+		printWorkflowResult(wf.Name, result)
+		return nil
+	},
+}
+
+func init() {
+	rootCmd.AddCommand(workflowCmd)
+	workflowCmd.Flags().Float64("budget", 0, "Maximum spend in USD (0 = unlimited)")
+}
+
+func findWorkflowFile(name string) string {
+	// Search in repo workflows/ directory, then plugin workflows/
+	candidates := []string{
+		filepath.Join(repoRoot, "workflows", name+".yml"),
+		filepath.Join(repoRoot, "workflows", name+".yaml"),
+	}
+	for _, c := range candidates {
+		if _, err := os.Stat(c); err == nil {
+			return c
+		}
+	}
+	return ""
+}
+
+func listWorkflows() error {
+	dir := filepath.Join(repoRoot, "workflows")
+	entries, err := os.ReadDir(dir)
+	if err != nil {
+		return fmt.Errorf("no workflows/ directory found in %s", repoRoot)
+	}
+
+	fmt.Println("Available workflows:")
+	for _, entry := range entries {
+		name := entry.Name()
+		if !strings.HasSuffix(name, ".yml") && !strings.HasSuffix(name, ".yaml") {
+			continue
+		}
+		wfName := strings.TrimSuffix(strings.TrimSuffix(name, ".yml"), ".yaml")
+		path := filepath.Join(dir, name)
+		wf, err := engine.ParseFile(path)
+		if err != nil {
+			fmt.Printf("  %-20s  (parse error: %v)\n", wfName, err)
+			continue
+		}
+		fmt.Printf("  %-20s  %s\n", wfName, wf.Description)
+	}
+	return nil
+}
+
+func printWorkflowResult(name string, r *engine.Result) {
+	fmt.Printf("\n=== %s Complete ===\n", name)
+	fmt.Printf("Session: %s\n", r.Session.ID)
+	fmt.Printf("Steps:   %d\n", len(r.Steps))
+	fmt.Printf("Cost:    $%.4f\n", r.TotalUSD)
+	fmt.Printf("\nRun `devkit status %s` for details.\n", r.Session.ID)
+}
diff --git a/src/engine/engine.go b/src/engine/engine.go
new file mode 100644
index 0000000..d9a144c
--- /dev/null
+++ b/src/engine/engine.go
@@ -0,0 +1,340 @@
+package engine
+
+import (
+	"context"
+	"fmt"
+	"os"
+	"path/filepath"
+	"strings"
+	"sync"
+
+	"github.com/5uck1ess/devkit/lib"
+	"github.com/5uck1ess/devkit/runners"
+)
+
+// Engine executes parsed workflows using a runner and database.
+type Engine struct {
+	DB       *lib.DB
+	Git      *lib.Git
+	Runner   runners.Runner
+	RepoRoot string
+}
+
+// RunConfig holds per-invocation settings.
+type RunConfig struct {
+	Input     string
+	BudgetUSD float64
+}
+
+// Result contains workflow execution results.
+type Result struct {
+	Session  *lib.Session
+	Steps    []lib.Step
+	Outputs  map[string]string
+	TotalUSD float64
+}
+
+// RunWorkflow executes a parsed workflow end-to-end.
+func (e *Engine) RunWorkflow(ctx context.Context, wf *Workflow, cfg RunConfig) (*Result, error) {
+	session := &lib.Session{
+		ID:        lib.NewSessionID(),
+		Workflow:  strings.ToLower(wf.Name),
+		Prompt:    cfg.Input,
+		Status:    "running",
+		BudgetUSD: cfg.BudgetUSD,
+	}
+	if err := e.DB.CreateSession(session); err != nil {
+		return nil, fmt.Errorf("create session: %w", err)
+	}
+	if err := lib.EnsureSessionDir(e.RepoRoot, session.ID); err != nil {
+		return nil, fmt.Errorf("create session dir: %w", err)
+	}
+
+	branchName := fmt.Sprintf("%s/%s", session.Workflow, session.ID)
+	if err := e.Git.CreateBranch(branchName); err != nil {
+		return nil, fmt.Errorf("create branch: %w", err)
+	}
+	fmt.Printf("%s session %s on branch %s\n\n", wf.Name, session.ID, branchName)
+
+	// Ensure scratchpad directory exists
+	scratchDir := filepath.Join(e.RepoRoot, ".devkit", "scratchpads")
+	os.MkdirAll(scratchDir, 0o755)
+
+	outputs := make(map[string]string)
+	stepIndex := buildStepIndex(wf.Steps)
+	var totalUSD float64
+	var iterNum int
+
+	opts := runners.RunOpts{
+		WorkDir:      e.RepoRoot,
+		AllowedTools: "Bash,Read,Edit,Write,Grep,Glob",
+		MaxTurns:     30,
+	}
+
+	overBudget := func() bool {
+		return cfg.BudgetUSD > 0 && totalUSD >= cfg.BudgetUSD
+	}
+
+	// Walk steps sequentially, with branch jumps
+	i := 0
+	for i < len(wf.Steps) {
+		if ctx.Err() != nil {
+			break
+		}
+		if overBudget() {
+			fmt.Printf("  Budget exhausted ($%.2f of $%.2f)\n", totalUSD, cfg.BudgetUSD)
+			break
+		}
+
+		step := &wf.Steps[i]
+
+		// Skip steps that are only referenced by parallel dispatchers
+		// (they're executed inline when the parallel step runs)
+		if step.Prompt == "" && len(step.Parallel) > 0 {
+			cost, err := e.runParallel(ctx, step, wf.Steps, stepIndex, session, cfg.Input, outputs, opts, &iterNum)
+			if err != nil {
+				e.DB.UpdateSessionStatus(session.ID, "failed")
+				break
+			}
+			totalUSD += cost
+			i++
+			continue
+		}
+
+		// Regular step
+		if step.Prompt == "" {
+			i++
+			continue
+		}
+
+		if step.Loop != nil {
+			cost, err := e.runLoop(ctx, step, session, cfg.Input, outputs, opts, &iterNum)
+			if err != nil {
+				e.DB.UpdateSessionStatus(session.ID, "failed")
+				break
+			}
+			totalUSD += cost
+		} else {
+			cost, output, err := e.runStep(ctx, step, session, cfg.Input, outputs, opts, &iterNum)
+			if err != nil {
+				e.DB.UpdateSessionStatus(session.ID, "failed")
+				break
+			}
+			totalUSD += cost
+			outputs[step.ID] = output
+
+			// Evaluate branch
+			if len(step.Branch) > 0 {
+				if target := EvalBranch(output, step.Branch); target != "" {
+					if idx, ok := stepIndex[target]; ok {
+						fmt.Printf("  → branching to %s\n\n", target)
+						i = idx
+						continue
+					}
+				}
+			}
+		}
+
+		i++
+	}
+
+	// Clean up scratchpad
+	os.Remove(filepath.Join(scratchDir, "current.md"))
+
+	// Commit any remaining changes
+	e.Git.CommitAll(fmt.Sprintf("%s(%s): complete", session.Workflow, session.ID))
+
+	e.DB.UpdateSessionStatus(session.ID, "done")
+	allSteps, _ := e.DB.GetSteps(session.ID)
+	lib.WriteReport(e.RepoRoot, session, allSteps, "completed")
+
+	return &Result{
+		Session:  session,
+		Steps:    allSteps,
+		Outputs:  outputs,
+		TotalUSD: totalUSD,
+	}, nil
+}
+
+// runStep executes a single step and records it in the database.
+func (e *Engine) runStep(ctx context.Context, step *WfStep, session *lib.Session, input string, outputs map[string]string, opts runners.RunOpts, iterNum *int) (float64, string, error) {
+	*iterNum++
+	fmt.Printf("--- %s (step %d) ---\n", step.ID, *iterNum)
+
+	prompt := Interpolate(step.Prompt, input, outputs)
+	dbStep := &lib.Step{
+		SessionID: session.ID,
+		Iteration: *iterNum,
+		Status:    "running",
+		AgentName: e.Runner.Name(),
+	}
+	e.DB.CreateStep(dbStep)
+
+	result, err := e.Runner.Run(ctx, prompt, opts)
+	if err != nil {
+		dbStep.Status = "failed"
+		dbStep.ChangeSummary = err.Error()
+		e.DB.UpdateStep(dbStep)
+		return 0, "", fmt.Errorf("step %s failed: %w", step.ID, err)
+	}
+
+	dbStep.Status = "kept"
+	dbStep.Kept = true
+	dbStep.CostUSD = result.CostUSD
+	dbStep.ChangeSummary = runners.TruncStr(result.Output, 200)
+	e.DB.UpdateStep(dbStep)
+	fmt.Printf("  done ($%.4f)\n\n", result.CostUSD)
+
+	return result.CostUSD, result.Output, nil
+}
+
+// runLoop executes a step repeatedly until the until-string is found or max iterations reached.
+func (e *Engine) runLoop(ctx context.Context, step *WfStep, session *lib.Session, input string, outputs map[string]string, opts runners.RunOpts, iterNum *int) (float64, error) {
+	var totalCost float64
+	maxIter := step.Loop.Max
+	if maxIter <= 0 {
+		maxIter = 1
+	}
+
+	for attempt := 1; attempt <= maxIter; attempt++ {
+		if ctx.Err() != nil {
+			return totalCost, ctx.Err()
+		}
+
+		*iterNum++
+		fmt.Printf("--- %s [%d/%d] (step %d) ---\n", step.ID, attempt, maxIter, *iterNum)
+
+		prompt := Interpolate(step.Prompt, input, outputs)
+		dbStep := &lib.Step{
+			SessionID: session.ID,
+			Iteration: *iterNum,
+			Status:    "running",
+			AgentName: e.Runner.Name(),
+		}
+		e.DB.CreateStep(dbStep)
+
+		result, err := e.Runner.Run(ctx, prompt, opts)
+		if err != nil {
+			dbStep.Status = "failed"
+			dbStep.ChangeSummary = err.Error()
+			e.DB.UpdateStep(dbStep)
+			// Loop continues on failure — try again
+			totalCost += result.CostUSD
+			fmt.Printf("  failed, retrying\n\n")
+			continue
+		}
+
+		totalCost += result.CostUSD
+		dbStep.Status = "kept"
+		dbStep.Kept = true
+		dbStep.CostUSD = result.CostUSD
+		dbStep.ChangeSummary = runners.TruncStr(result.Output, 200)
+		e.DB.UpdateStep(dbStep)
+
+		outputs[step.ID] = result.Output
+		fmt.Printf("  done ($%.4f)\n\n", result.CostUSD)
+
+		// Commit after each loop iteration
+		e.Git.CommitAll(fmt.Sprintf("%s: %s iteration %d", session.Workflow, step.ID, attempt))
+
+		// Check until condition
+		if step.Loop.Until != "" && strings.Contains(strings.ToUpper(result.Output), strings.ToUpper(step.Loop.Until)) {
+			fmt.Printf("  → loop complete (%s found)\n\n", step.Loop.Until)
+			break
+		}
+	}
+
+	return totalCost, nil
+}
+
+// runParallel dispatches multiple steps concurrently.
+func (e *Engine) runParallel(ctx context.Context, dispatcher *WfStep, allSteps []WfStep, stepIndex map[string]int, session *lib.Session, input string, outputs map[string]string, opts runners.RunOpts, iterNum *int) (float64, error) {
+	fmt.Printf("--- %s (parallel: %s) ---\n\n", dispatcher.ID, strings.Join(dispatcher.Parallel, ", "))
+
+	type parallelResult struct {
+		id     string
+		output string
+		cost   float64
+		err    error
+	}
+
+	var mu sync.Mutex
+	var wg sync.WaitGroup
+	results := make([]parallelResult, len(dispatcher.Parallel))
+
+	for j, pid := range dispatcher.Parallel {
+		idx, ok := stepIndex[pid]
+		if !ok {
+			return 0, fmt.Errorf("parallel step %q not found", pid)
+		}
+		step := &allSteps[idx]
+
+		wg.Add(1)
+		go func(j int, step *WfStep, pid string) {
+			defer wg.Done()
+
+			mu.Lock()
+			*iterNum++
+			myIter := *iterNum
+			mu.Unlock()
+
+			prompt := Interpolate(step.Prompt, input, outputs)
+			dbStep := &lib.Step{
+				SessionID: session.ID,
+				Iteration: myIter,
+				Status:    "running",
+				AgentName: e.Runner.Name(),
+			}
+
+			mu.Lock()
+			e.DB.CreateStep(dbStep)
+			mu.Unlock()
+
+			result, err := e.Runner.Run(ctx, prompt, opts)
+
+			mu.Lock()
+			defer mu.Unlock()
+
+			if err != nil {
+				dbStep.Status = "failed"
+				dbStep.ChangeSummary = err.Error()
+				e.DB.UpdateStep(dbStep)
+				results[j] = parallelResult{id: pid, err: err}
+				return
+			}
+
+			dbStep.Status = "kept"
+			dbStep.Kept = true
+			dbStep.CostUSD = result.CostUSD
+			dbStep.ChangeSummary = runners.TruncStr(result.Output, 200)
+			e.DB.UpdateStep(dbStep)
+
+			results[j] = parallelResult{id: pid, output: result.Output, cost: result.CostUSD}
+		}(j, step, pid)
+	}
+
+	wg.Wait()
+
+	var totalCost float64
+	for _, r := range results {
+		if r.err != nil {
+			fmt.Printf("  %s: failed (%v)\n", r.id, r.err)
+			continue
+		}
+		outputs[r.id] = r.output
+		totalCost += r.cost
+		fmt.Printf("  %s: done ($%.4f)\n", r.id, r.cost)
+	}
+	fmt.Println()
+
+	return totalCost, nil
+}
+
+// buildStepIndex maps step IDs to their index in the steps slice.
+func buildStepIndex(steps []WfStep) map[string]int {
+	idx := make(map[string]int, len(steps))
+	for i, s := range steps {
+		idx[s.ID] = i
+	}
+	return idx
+}
diff --git a/src/engine/engine_test.go b/src/engine/engine_test.go
new file mode 100644
index 0000000..ba7006e
--- /dev/null
+++ b/src/engine/engine_test.go
@@ -0,0 +1,571 @@
+package engine
+
+import (
+	"context"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/5uck1ess/devkit/lib"
+	"github.com/5uck1ess/devkit/runners"
+)
+
+// ---------------------------------------------------------------------------
+// test helpers
+// ---------------------------------------------------------------------------
+
+func tempDB(t *testing.T) *lib.DB {
+	t.Helper()
+	dir := t.TempDir()
+	db, err := lib.OpenDB(filepath.Join(dir, ".devkit", "devkit.db"))
+	if err != nil {
+		t.Fatalf("open db: %v", err)
+	}
+	t.Cleanup(func() { db.Close() })
+	return db
+}
+
+func initGitRepo(t *testing.T) (string, *lib.Git) {
+	t.Helper()
+	dir := t.TempDir()
+	cmds := [][]string{
+		{"git", "init", "-b", "main"},
+		{"git", "config", "user.email", "test@test.com"},
+		{"git", "config", "user.name", "Test"},
+	}
+	for _, args := range cmds {
+		cmd := exec.Command(args[0], args[1:]...)
+		cmd.Dir = dir
+		if out, err := cmd.CombinedOutput(); err != nil {
+			t.Fatalf("git setup %v: %s", args, out)
+		}
+	}
+	if err := os.WriteFile(filepath.Join(dir, "README.md"), []byte("# test\n"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	for _, args := range [][]string{
+		{"git", "add", "-A"},
+		{"git", "commit", "-m", "initial"},
+	} {
+		cmd := exec.Command(args[0], args[1:]...)
+		cmd.Dir = dir
+		if out, err := cmd.CombinedOutput(); err != nil {
+			t.Fatalf("git setup %v: %s", args, out)
+		}
+	}
+	return dir, &lib.Git{Dir: dir}
+}
+
+type mockRunner struct {
+	name      string
+	responses []runners.RunResult
+	errors    []error
+	callIdx   int
+	prompts   []string
+}
+
+func newMockRunner(responses []runners.RunResult, errs []error) *mockRunner {
+	return &mockRunner{name: "mock", responses: responses, errors: errs}
+}
+
+func (m *mockRunner) Name() string    { return m.name }
+func (m *mockRunner) Available() bool { return true }
+
+func (m *mockRunner) Run(ctx context.Context, prompt string, opts runners.RunOpts) (runners.RunResult, error) {
+	m.prompts = append(m.prompts, prompt)
+	idx := m.callIdx
+	m.callIdx++
+	if idx >= len(m.responses) {
+		return runners.RunResult{Output: "mock exhausted"}, nil
+	}
+	var err error
+	if idx < len(m.errors) {
+		err = m.errors[idx]
+	}
+	return m.responses[idx], err
+}
+
+func result(output string) runners.RunResult {
+	return runners.RunResult{Output: output, CostUSD: 0.01}
+}
+
+// ---------------------------------------------------------------------------
+// Parse tests
+// ---------------------------------------------------------------------------
+
+func TestParseMinimal(t *testing.T) {
+	yaml := `
+name: Test
+description: A test workflow
+steps:
+  - id: step1
+    model: fast
+    prompt: "Do something"
+`
+	wf, err := Parse([]byte(yaml))
+	if err != nil {
+		t.Fatalf("parse: %v", err)
+	}
+	if wf.Name != "Test" {
+		t.Errorf("name = %q, want Test", wf.Name)
+	}
+	if len(wf.Steps) != 1 {
+		t.Fatalf("steps = %d, want 1", len(wf.Steps))
+	}
+	if wf.Steps[0].ID != "step1" {
+		t.Errorf("step id = %q, want step1", wf.Steps[0].ID)
+	}
+}
+
+func TestParseWithLoop(t *testing.T) {
+	yaml := `
+name: Looper
+description: test
+steps:
+  - id: fix
+    model: smart
+    prompt: "Fix it"
+    loop:
+      max: 5
+      until: ALL_DONE
+`
+	wf, err := Parse([]byte(yaml))
+	if err != nil {
+		t.Fatalf("parse: %v", err)
+	}
+	if wf.Steps[0].Loop == nil {
+		t.Fatal("expected loop to be set")
+	}
+	if wf.Steps[0].Loop.Max != 5 {
+		t.Errorf("loop max = %d, want 5", wf.Steps[0].Loop.Max)
+	}
+	if wf.Steps[0].Loop.Until != "ALL_DONE" {
+		t.Errorf("loop until = %q, want ALL_DONE", wf.Steps[0].Loop.Until)
+	}
+}
+
+func TestParseWithBranch(t *testing.T) {
+	yaml := `
+name: Brancher
+description: test
+steps:
+  - id: classify
+    model: fast
+    prompt: "Classify"
+    branch:
+      - when: "TINY"
+        goto: quick
+      - when: "LARGE"
+        goto: full
+  - id: full
+    model: smart
+    prompt: "Full pipeline"
+  - id: quick
+    model: fast
+    prompt: "Quick fix"
+`
+	wf, err := Parse([]byte(yaml))
+	if err != nil {
+		t.Fatalf("parse: %v", err)
+	}
+	if len(wf.Steps[0].Branch) != 2 {
+		t.Fatalf("branches = %d, want 2", len(wf.Steps[0].Branch))
+	}
+	if wf.Steps[0].Branch[0].Goto != "quick" {
+		t.Errorf("branch[0].goto = %q, want quick", wf.Steps[0].Branch[0].Goto)
+	}
+}
+
+func TestParseValidation(t *testing.T) {
+	tests := []struct {
+		name string
+		yaml string
+		want string
+	}{
+		{"missing name", `steps: [{id: s, prompt: x}]`, "missing name"},
+		{"no steps", `name: T`, "no steps"},
+		{"duplicate id", `name: T
+steps:
+  - {id: a, prompt: x}
+  - {id: a, prompt: y}`, "duplicate step id"},
+		{"bad branch target", `name: T
+steps:
+  - id: a
+    prompt: x
+    branch: [{when: "x", goto: missing}]`, "branch target"},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			_, err := Parse([]byte(tt.yaml))
+			if err == nil {
+				t.Fatal("expected error")
+			}
+			if !strings.Contains(err.Error(), tt.want) {
+				t.Errorf("error %q doesn't contain %q", err.Error(), tt.want)
+			}
+		})
+	}
+}
+
+func TestParseBudget(t *testing.T) {
+	yaml := `
+name: Budgeted
+description: test
+budget:
+  limit: 300000
+  downgrade: fast
+steps:
+  - id: s1
+    model: smart
+    prompt: "Do"
+`
+	wf, err := Parse([]byte(yaml))
+	if err != nil {
+		t.Fatalf("parse: %v", err)
+	}
+	if wf.Budget.Limit != 300000 {
+		t.Errorf("budget limit = %d, want 300000", wf.Budget.Limit)
+	}
+	if wf.Budget.Downgrade != "fast" {
+		t.Errorf("budget downgrade = %q, want fast", wf.Budget.Downgrade)
+	}
+}
+
+// ---------------------------------------------------------------------------
+// Interpolation tests
+// ---------------------------------------------------------------------------
+
+func TestInterpolate(t *testing.T) {
+	outputs := map[string]string{
+		"plan":  "1. Do X\n2. Do Y",
+		"build": "compiled OK",
+	}
+	prompt := "Input: {{input}}\nPlan: {{plan}}\nBuild: {{build}}"
+	got := Interpolate(prompt, "add auth", outputs)
+
+	if !strings.Contains(got, "Input: add auth") {
+		t.Error("input not interpolated")
+	}
+	if !strings.Contains(got, "Plan: 1. Do X") {
+		t.Error("plan not interpolated")
+	}
+	if !strings.Contains(got, "Build: compiled OK") {
+		t.Error("build not interpolated")
+	}
+}
+
+func TestInterpolateMissing(t *testing.T) {
+	got := Interpolate("ref: {{missing}}", "input", map[string]string{})
+	if !strings.Contains(got, "{{missing}}") {
+		t.Error("missing variable should be left as-is")
+	}
+}
+
+// ---------------------------------------------------------------------------
+// Branch evaluation tests
+// ---------------------------------------------------------------------------
+
+func TestEvalBranch(t *testing.T) {
+	branches := []Branch{
+		{When: "TINY", Goto: "quick"},
+		{When: "SMALL", Goto: "plan"},
+	}
+
+	tests := []struct {
+		output string
+		want   string
+	}{
+		{"TINY: just a typo fix", "quick"},
+		{"tiny change", "quick"},      // case insensitive
+		{"SMALL: one function", "plan"},
+		{"MEDIUM: multiple files", ""}, // no match
+		{"LARGE: new subsystem", ""},
+	}
+
+	for _, tt := range tests {
+		got := EvalBranch(tt.output, branches)
+		if got != tt.want {
+			t.Errorf("EvalBranch(%q) = %q, want %q", tt.output, got, tt.want)
+		}
+	}
+}
+
+func TestEvalBranchFirstMatchWins(t *testing.T) {
+	branches := []Branch{
+		{When: "error", Goto: "retry"},
+		{When: "error", Goto: "fail"},
+	}
+	got := EvalBranch("got an error", branches)
+	if got != "retry" {
+		t.Errorf("first match should win, got %q", got)
+	}
+}
+
+// ---------------------------------------------------------------------------
+// Engine execution tests
+// ---------------------------------------------------------------------------
+
+func TestRunWorkflowSimple(t *testing.T) {
+	db := tempDB(t)
+	dir, git := initGitRepo(t)
+
+	runner := newMockRunner([]runners.RunResult{
+		result("planned: do A then B"),
+		result("implemented A and B"),
+	}, nil)
+
+	eng := &Engine{DB: db, Git: git, Runner: runner, RepoRoot: dir}
+	wf := &Workflow{
+		Name: "test",
+		Steps: []WfStep{
+			{ID: "plan", Model: "smart", Prompt: "Plan: {{input}}"},
+			{ID: "impl", Model: "smart", Prompt: "Implement: {{plan}}"},
+		},
+	}
+
+	res, err := eng.RunWorkflow(context.Background(), wf, RunConfig{Input: "add auth"})
+	if err != nil {
+		t.Fatalf("RunWorkflow: %v", err)
+	}
+	if res.TotalUSD != 0.02 {
+		t.Errorf("total cost = %f, want 0.02", res.TotalUSD)
+	}
+	if len(res.Steps) != 2 {
+		t.Errorf("steps = %d, want 2", len(res.Steps))
+	}
+
+	// Verify interpolation happened
+	if !strings.Contains(runner.prompts[0], "add auth") {
+		t.Error("input not interpolated in plan prompt")
+	}
+	if !strings.Contains(runner.prompts[1], "planned: do A then B") {
+		t.Error("plan output not interpolated in impl prompt")
+	}
+}
+
+func TestRunWorkflowBranch(t *testing.T) {
+	db := tempDB(t)
+	dir, git := initGitRepo(t)
+
+	runner := newMockRunner([]runners.RunResult{
+		result("TINY: just a typo"),  // triage output
+		result("fixed the typo"),     // quick-fix output
+	}, nil)
+
+	eng := &Engine{DB: db, Git: git, Runner: runner, RepoRoot: dir}
+	wf := &Workflow{
+		Name: "test",
+		Steps: []WfStep{
+			{ID: "triage", Model: "fast", Prompt: "Classify: {{input}}", Branch: []Branch{
+				{When: "TINY", Goto: "quick"},
+				{When: "SMALL", Goto: "plan"},
+			}},
+			{ID: "brainstorm", Model: "smart", Prompt: "Think about {{input}}"},
+			{ID: "plan", Model: "smart", Prompt: "Plan {{input}}"},
+			{ID: "quick", Model: "fast", Prompt: "Quick fix: {{input}}"},
+		},
+	}
+
+	res, err := eng.RunWorkflow(context.Background(), wf, RunConfig{Input: "fix typo"})
+	if err != nil {
+		t.Fatalf("RunWorkflow: %v", err)
+	}
+
+	// Should have skipped brainstorm and plan, jumped to quick
+	if runner.callIdx != 2 {
+		t.Errorf("runner called %d times, want 2 (triage + quick)", runner.callIdx)
+	}
+	if _, ok := res.Outputs["brainstorm"]; ok {
+		t.Error("brainstorm should have been skipped")
+	}
+	if _, ok := res.Outputs["quick"]; !ok {
+		t.Error("quick-fix should have been executed")
+	}
+}
+
+func TestRunWorkflowLoop(t *testing.T) {
+	db := tempDB(t)
+	dir, git := initGitRepo(t)
+
+	runner := newMockRunner([]runners.RunResult{
+		result("attempt 1: still failing"),
+		result("attempt 2: ALL_PASSING"),
+	}, nil)
+
+	eng := &Engine{DB: db, Git: git, Runner: runner, RepoRoot: dir}
+	wf := &Workflow{
+		Name: "test",
+		Steps: []WfStep{
+			{ID: "fix", Model: "smart", Prompt: "Fix tests", Loop: &Loop{Max: 5, Until: "ALL_PASSING"}},
+		},
+	}
+
+	res, err := eng.RunWorkflow(context.Background(), wf, RunConfig{Input: "fix"})
+	if err != nil {
+		t.Fatalf("RunWorkflow: %v", err)
+	}
+
+	// Should have run 2 iterations (found ALL_PASSING on second)
+	if runner.callIdx != 2 {
+		t.Errorf("runner called %d times, want 2", runner.callIdx)
+	}
+	if res.TotalUSD != 0.02 {
+		t.Errorf("total cost = %f, want 0.02", res.TotalUSD)
+	}
+}
+
+func TestRunWorkflowLoopMaxIterations(t *testing.T) {
+	db := tempDB(t)
+	dir, git := initGitRepo(t)
+
+	runner := newMockRunner([]runners.RunResult{
+		result("still broken"),
+		result("still broken"),
+		result("still broken"),
+	}, nil)
+
+	eng := &Engine{DB: db, Git: git, Runner: runner, RepoRoot: dir}
+	wf := &Workflow{
+		Name: "test",
+		Steps: []WfStep{
+			{ID: "fix", Model: "smart", Prompt: "Fix", Loop: &Loop{Max: 3, Until: "DONE"}},
+		},
+	}
+
+	_, err := eng.RunWorkflow(context.Background(), wf, RunConfig{Input: "fix"})
+	if err != nil {
+		t.Fatalf("RunWorkflow: %v", err)
+	}
+
+	if runner.callIdx != 3 {
+		t.Errorf("runner called %d times, want 3 (max)", runner.callIdx)
+	}
+}
+
+func TestRunWorkflowBudget(t *testing.T) {
+	db := tempDB(t)
+	dir, git := initGitRepo(t)
+
+	runner := newMockRunner([]runners.RunResult{
+		{Output: "step 1", CostUSD: 0.50},
+		{Output: "step 2", CostUSD: 0.50},
+		{Output: "step 3", CostUSD: 0.50}, // should not be reached
+	}, nil)
+
+	eng := &Engine{DB: db, Git: git, Runner: runner, RepoRoot: dir}
+	wf := &Workflow{
+		Name: "test",
+		Steps: []WfStep{
+			{ID: "s1", Model: "smart", Prompt: "Step 1"},
+			{ID: "s2", Model: "smart", Prompt: "Step 2"},
+			{ID: "s3", Model: "smart", Prompt: "Step 3"},
+		},
+	}
+
+	res, err := eng.RunWorkflow(context.Background(), wf, RunConfig{Input: "test", BudgetUSD: 1.00})
+	if err != nil {
+		t.Fatalf("RunWorkflow: %v", err)
+	}
+
+	// s1 costs 0.50, s2 costs 0.50, total = 1.00 >= budget, s3 skipped
+	if runner.callIdx != 2 {
+		t.Errorf("runner called %d times, want 2 (budget hit)", runner.callIdx)
+	}
+	if res.TotalUSD != 1.00 {
+		t.Errorf("total cost = %f, want 1.00", res.TotalUSD)
+	}
+}
+
+func TestRunWorkflowParallel(t *testing.T) {
+	db := tempDB(t)
+	dir, git := initGitRepo(t)
+
+	runner := newMockRunner([]runners.RunResult{
+		result("review A findings"),
+		result("review B findings"),
+	}, nil)
+
+	eng := &Engine{DB: db, Git: git, Runner: runner, RepoRoot: dir}
+	wf := &Workflow{
+		Name: "test",
+		Steps: []WfStep{
+			{ID: "review-a", Model: "smart", Prompt: "Review A"},
+			{ID: "review-b", Model: "fast", Prompt: "Review B"},
+			{ID: "dispatch", Parallel: []string{"review-a", "review-b"}},
+		},
+	}
+
+	res, err := eng.RunWorkflow(context.Background(), wf, RunConfig{Input: "review"})
+	if err != nil {
+		t.Fatalf("RunWorkflow: %v", err)
+	}
+
+	if _, ok := res.Outputs["review-a"]; !ok {
+		t.Error("review-a output missing")
+	}
+	if _, ok := res.Outputs["review-b"]; !ok {
+		t.Error("review-b output missing")
+	}
+}
+
+func TestRunWorkflowContextCancelled(t *testing.T) {
+	db := tempDB(t)
+	dir, git := initGitRepo(t)
+
+	runner := newMockRunner([]runners.RunResult{
+		result("step 1 done"),
+	}, nil)
+
+	ctx, cancel := context.WithCancel(context.Background())
+	cancel() // cancel immediately
+
+	eng := &Engine{DB: db, Git: git, Runner: runner, RepoRoot: dir}
+	wf := &Workflow{
+		Name: "test",
+		Steps: []WfStep{
+			{ID: "s1", Model: "smart", Prompt: "Step 1"},
+		},
+	}
+
+	_, err := eng.RunWorkflow(ctx, wf, RunConfig{Input: "test"})
+	if err != nil {
+		t.Fatalf("RunWorkflow should not error on cancel: %v", err)
+	}
+	if runner.callIdx != 0 {
+		t.Errorf("runner called %d times, want 0 (cancelled)", runner.callIdx)
+	}
+}
+
+// ---------------------------------------------------------------------------
+// Parse real workflow files
+// ---------------------------------------------------------------------------
+
+func TestParseRealWorkflows(t *testing.T) {
+	workflowDir := filepath.Join("..", "..", "workflows")
+	entries, err := os.ReadDir(workflowDir)
+	if err != nil {
+		t.Skip("workflows directory not found:", err)
+	}
+
+	for _, entry := range entries {
+		if !strings.HasSuffix(entry.Name(), ".yml") {
+			continue
+		}
+		t.Run(entry.Name(), func(t *testing.T) {
+			path := filepath.Join(workflowDir, entry.Name())
+			wf, err := ParseFile(path)
+			if err != nil {
+				t.Fatalf("parse %s: %v", entry.Name(), err)
+			}
+			if wf.Name == "" {
+				t.Error("workflow name is empty")
+			}
+			if len(wf.Steps) == 0 {
+				t.Error("workflow has no steps")
+			}
+		})
+	}
+}
diff --git a/src/engine/workflow.go b/src/engine/workflow.go
new file mode 100644
index 0000000..065dbc7
--- /dev/null
+++ b/src/engine/workflow.go
@@ -0,0 +1,128 @@
+// Package engine provides a generic YAML workflow execution engine.
+// It replaces hardcoded Go workflow implementations with a single engine
+// that reads and executes workflow YAML files deterministically.
+package engine
+
+import (
+	"fmt"
+	"os"
+	"strings"
+
+	"gopkg.in/yaml.v3"
+)
+
+// Workflow is the top-level YAML structure.
+type Workflow struct {
+	Name        string   `yaml:"name"`
+	Description string   `yaml:"description"`
+	Budget      Budget   `yaml:"budget"`
+	Steps       []WfStep `yaml:"steps"`
+}
+
+// Budget controls token spending limits.
+type Budget struct {
+	Limit     int    `yaml:"limit"`
+	Downgrade string `yaml:"downgrade"`
+}
+
+// WfStep is a single step in a workflow.
+type WfStep struct {
+	ID       string   `yaml:"id"`
+	Model    string   `yaml:"model"`
+	Prompt   string   `yaml:"prompt"`
+	Parallel []string `yaml:"parallel"`
+	Loop     *Loop    `yaml:"loop"`
+	Branch   []Branch `yaml:"branch"`
+}
+
+// Loop controls step repetition.
+type Loop struct {
+	Max   int    `yaml:"max"`
+	Until string `yaml:"until"`
+}
+
+// Branch routes execution based on step output.
+type Branch struct {
+	When string `yaml:"when"`
+	Goto string `yaml:"goto"`
+}
+
+// ParseFile reads and parses a workflow YAML file.
+func ParseFile(path string) (*Workflow, error) {
+	data, err := os.ReadFile(path)
+	if err != nil {
+		return nil, fmt.Errorf("read workflow: %w", err)
+	}
+	return Parse(data)
+}
+
+// Parse parses workflow YAML bytes.
+func Parse(data []byte) (*Workflow, error) {
+	var wf Workflow
+	if err := yaml.Unmarshal(data, &wf); err != nil {
+		return nil, fmt.Errorf("parse yaml: %w", err)
+	}
+	if err := validate(&wf); err != nil {
+		return nil, err
+	}
+	return &wf, nil
+}
+
+// validate checks the workflow for structural errors.
+func validate(wf *Workflow) error {
+	if wf.Name == "" {
+		return fmt.Errorf("workflow missing name")
+	}
+	if len(wf.Steps) == 0 {
+		return fmt.Errorf("workflow %q has no steps", wf.Name)
+	}
+
+	ids := make(map[string]bool)
+	for _, s := range wf.Steps {
+		if s.ID == "" {
+			return fmt.Errorf("step missing id in workflow %q", wf.Name)
+		}
+		if ids[s.ID] {
+			return fmt.Errorf("duplicate step id %q in workflow %q", s.ID, wf.Name)
+		}
+		ids[s.ID] = true
+	}
+
+	// Validate branch targets exist
+	for _, s := range wf.Steps {
+		for _, b := range s.Branch {
+			if !ids[b.Goto] {
+				return fmt.Errorf("branch target %q not found (step %q)", b.Goto, s.ID)
+			}
+		}
+		// Validate parallel references exist
+		for _, pid := range s.Parallel {
+			if !ids[pid] {
+				return fmt.Errorf("parallel step %q not found (step %q)", pid, s.ID)
+			}
+		}
+	}
+
+	return nil
+}
+
+// Interpolate replaces {{step-id}} and {{input}} placeholders in a prompt.
+func Interpolate(prompt string, input string, outputs map[string]string) string {
+	result := strings.ReplaceAll(prompt, "{{input}}", input)
+	for id, output := range outputs {
+		result = strings.ReplaceAll(result, "{{"+id+"}}", output)
+	}
+	return result
+}
+
+// EvalBranch checks step output against branch conditions.
+// Returns the goto target step ID, or "" if no match.
+func EvalBranch(output string, branches []Branch) string {
+	lower := strings.ToLower(output)
+	for _, b := range branches {
+		if strings.Contains(lower, strings.ToLower(b.When)) {
+			return b.Goto
+		}
+	}
+	return ""
+}
diff --git a/src/go.mod b/src/go.mod
index ee33b12..0b01954 100644
--- a/src/go.mod
+++ b/src/go.mod
@@ -16,6 +16,7 @@ require (
 	github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec // indirect
 	github.com/spf13/pflag v1.0.9 // indirect
 	golang.org/x/sys v0.42.0 // indirect
+	gopkg.in/yaml.v3 v3.0.1 // indirect
 	modernc.org/libc v1.70.0 // indirect
 	modernc.org/mathutil v1.7.1 // indirect
 	modernc.org/memory v1.11.0 // indirect
diff --git a/src/go.sum b/src/go.sum
index 3fbd220..1483f14 100644
--- a/src/go.sum
+++ b/src/go.sum
@@ -31,6 +31,8 @@ golang.org/x/sys v0.42.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw=
 golang.org/x/tools v0.42.0 h1:uNgphsn75Tdz5Ji2q36v/nsFSfR/9BRFvqhGBaJGd5k=
 golang.org/x/tools v0.42.0/go.mod h1:Ma6lCIwGZvHK6XtgbswSoWroEkhugApmsXyrUmBhfr0=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
+gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
+gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
 modernc.org/cc/v4 v4.27.1 h1:9W30zRlYrefrDV2JE2O8VDtJ1yPGownxciz5rrbQZis=
 modernc.org/cc/v4 v4.27.1/go.mod h1:uVtb5OGqUKpoLWhqwNQo/8LwvoiEBLvZXIQ/SmO6mL0=
 modernc.org/ccgo/v4 v4.32.0 h1:hjG66bI/kqIPX1b2yT6fr/jt+QedtP2fqojG2VrFuVw=
diff --git a/workflows/bugfix.yml b/workflows/bugfix.yml
index 597642d..c07c354 100644
--- a/workflows/bugfix.yml
+++ b/workflows/bugfix.yml
@@ -1,11 +1,29 @@
 name: Bug Fix
-description: Full lifecycle bug fix — reproduce, diagnose, fix, test, review
+description: Full lifecycle bug fix — triage, reproduce, diagnose, fix, test, review (with fast path for trivial fixes)
 
 budget:
   limit: 300000
   downgrade: fast
 
 steps:
+  - id: triage
+    model: fast
+    prompt: |
+      Classify this bug report by complexity:
+
+      {{input}}
+
+      Categories:
+      - TRIVIAL: Typo, off-by-one, missing import, wrong variable name, obvious one-liner.
+      - NORMAL: Requires investigation but likely a single root cause in one area.
+      - COMPLEX: Multiple possible causes, cross-cutting, or involves concurrency/state.
+
+      Output the category name (TRIVIAL, NORMAL, or COMPLEX) on the first line,
+      followed by a one-sentence justification.
+    branch:
+      - when: "TRIVIAL"
+        goto: quick-fix
+
   - id: reproduce
     model: smart
     prompt: |
@@ -24,12 +42,16 @@ steps:
 
       {{reproduce}}
 
+      Check .devkit/scratchpads/current.md for notes from previous attempts (if it exists).
+
       Trace the root cause. Read the code path, check assumptions,
       examine edge cases. Determine exactly WHY this happens,
       not just WHERE.
 
       Propose a specific fix with reasoning.
 
+      Append your diagnosis to .devkit/scratchpads/current.md.
+
   - id: fix
     model: smart
     prompt: |
@@ -68,9 +90,13 @@ steps:
 
       {{run-tests}}
 
+      Check .devkit/scratchpads/current.md for what was already tried.
+
       Fix any failures. The fix may have caused side effects —
       determine if the test or the code is wrong.
 
+      Append your fix attempt and result to .devkit/scratchpads/current.md.
+
       Run tests again. If all pass, say "ALL_PASSING".
     loop:
       max: 5
@@ -95,3 +121,17 @@ steps:
 
       ## Status
       Test suite status. Ready to commit or remaining concerns.
+
+      Clean up .devkit/scratchpads/current.md if it exists.
+
+  - id: quick-fix
+    model: smart
+    prompt: |
+      This is a trivial bug — no deep investigation needed.
+
+      Bug report: {{input}}
+
+      1. Find the bug and fix it directly.
+      2. Write a regression test that would have caught it.
+      3. Run the full test suite.
+      4. Produce a brief summary: what was wrong, what you changed, what test you added.
diff --git a/workflows/feature.yml b/workflows/feature.yml
index 0ba68c5..d47d131 100644
--- a/workflows/feature.yml
+++ b/workflows/feature.yml
@@ -1,11 +1,32 @@
 name: Feature
-description: Full lifecycle — brainstorm, plan, implement, test, lint, review
+description: Full lifecycle — triage, brainstorm, plan, implement, test, lint, review (with fast path for small changes)
 
 budget:
   limit: 500000
   downgrade: fast
 
 steps:
+  - id: triage
+    model: fast
+    prompt: |
+      Classify this feature request by scope. Be honest — most changes are smaller than they seem.
+
+      {{input}}
+
+      Categories:
+      - TINY: Typo fix, comment change, single-line config tweak, rename. No new logic.
+      - SMALL: Single function or file change. Clear, contained, no design decisions needed.
+      - MEDIUM: Multiple files, new component or endpoint, moderate complexity.
+      - LARGE: New subsystem, cross-cutting change, architectural work.
+
+      Output the category name (TINY, SMALL, MEDIUM, or LARGE) on the first line,
+      followed by a one-sentence justification.
+    branch:
+      - when: "TINY"
+        goto: quick-fix
+      - when: "SMALL"
+        goto: plan
+
   - id: brainstorm
     model: smart
     prompt: |
@@ -22,9 +43,10 @@ steps:
   - id: plan
     model: smart
     prompt: |
-      Based on this design:
+      Based on this context:
 
-      {{brainstorm}}
+      Feature request: {{input}}
+      Design (if available): {{brainstorm}}
 
       Create an implementation plan as a numbered todo list.
       Each item should be a single, testable change.
@@ -38,9 +60,15 @@ steps:
 
       {{plan}}
 
+      Check .devkit/scratchpads/current.md for notes from previous iterations (if it exists).
+
       Execute the next incomplete todo. Write the code, verify it works,
       then mark it done. Keep changes small and focused.
 
+      After each attempt, append to .devkit/scratchpads/current.md:
+      - What you tried
+      - Whether it worked or failed (and why)
+
       If all todos are complete, say "ALL_DONE".
     loop:
       max: 20
@@ -76,9 +104,13 @@ steps:
 
       {{run-tests}}
 
+      Check .devkit/scratchpads/current.md for what was already tried.
+
       Fix any failing tests. Determine if the bug is in the test
       or the implementation and fix accordingly.
 
+      Append your fix attempt and result to .devkit/scratchpads/current.md.
+
       Run tests again. If all pass, say "ALL_PASSING".
     loop:
       max: 8
@@ -151,3 +183,17 @@ steps:
 
       ## Status
       Ready to commit, or list remaining issues.
+
+      Clean up .devkit/scratchpads/current.md if it exists.
+
+  - id: quick-fix
+    model: smart
+    prompt: |
+      This is a tiny change — no design or planning needed.
+
+      Task: {{input}}
+
+      1. Make the change directly. Keep it minimal.
+      2. Run the linter on changed files.
+      3. Run the test suite to verify nothing broke.
+      4. Produce a one-paragraph summary of what you changed and why.

From 4d1f19d88bf484c48a78219b6c077835c359b275 Mon Sep 17 00:00:00 2001
From: Tym Rabchuk <tymrabchuk@gmail.com>
Date: Sat, 4 Apr 2026 18:02:33 -0400
Subject: [PATCH 02/11] Fix 6 review findings: race condition, error
 propagation, cycle detection
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Fixes from tri-review (Claude + Codex + Gemini consensus):

1. Fix nil deref on runner error in runLoop — no longer accesses
   result.CostUSD when err != nil
2. Fix data race on outputs map in runParallel — snapshot outputs
   before launching goroutines
3. Fix session status overwrite — track failed state, only mark
   "done" on clean exit
4. Fix silent loop failure — runLoop returns error when all
   iterations fail
5. Fix parallel error swallowing — runParallel returns error when
   ALL parallel steps fail
6. Add branch cycle detection — max 100 jumps before stopping

Additional fixes:
- Skip parallel-dispatched steps during sequential walk (no double execution)
- Evaluate branch conditions after loop steps (not just regular steps)
- Wire YAML budget config to RunConfig when CLI flag not set
- Propagate step errors from RunWorkflow return value

2 new tests: TestRunWorkflowLoopAllFail, TestRunWorkflowBranchCycleLimit
---
 src/cmd/workflow.go       |   5 ++
 src/engine/engine.go      | 119 +++++++++++++++++++++++++++++++++-----
 src/engine/engine_test.go |  69 ++++++++++++++++++++--
 3 files changed, 171 insertions(+), 22 deletions(-)

diff --git a/src/cmd/workflow.go b/src/cmd/workflow.go
index 53d159b..5fbdf9a 100644
--- a/src/cmd/workflow.go
+++ b/src/cmd/workflow.go
@@ -57,6 +57,11 @@ var workflowCmd = &cobra.Command{
 		}
 
 		budget, _ := cmd.Flags().GetFloat64("budget")
+		// CLI flag overrides YAML budget; fall back to YAML if flag not set
+		if budget == 0 && wf.Budget.Limit > 0 {
+			// Convert token budget to rough USD estimate ($0.01 per 1K tokens)
+			budget = float64(wf.Budget.Limit) / 1000.0 * 0.01
+		}
 
 		eng := &engine.Engine{
 			DB:       db,
diff --git a/src/engine/engine.go b/src/engine/engine.go
index d9a144c..ea104ca 100644
--- a/src/engine/engine.go
+++ b/src/engine/engine.go
@@ -75,7 +75,21 @@ func (e *Engine) RunWorkflow(ctx context.Context, wf *Workflow, cfg RunConfig) (
 		return cfg.BudgetUSD > 0 && totalUSD >= cfg.BudgetUSD
 	}
 
+	// Build set of step IDs that are dispatched by parallel steps,
+	// so we skip them during sequential walk (they run inside runParallel).
+	parallelChildren := make(map[string]bool)
+	for _, s := range wf.Steps {
+		for _, pid := range s.Parallel {
+			parallelChildren[pid] = true
+		}
+	}
+
 	// Walk steps sequentially, with branch jumps
+	failed := false
+	var stepErr error
+	branchCount := 0
+	const maxBranches = 100
+
 	i := 0
 	for i < len(wf.Steps) {
 		if ctx.Err() != nil {
@@ -88,12 +102,19 @@ func (e *Engine) RunWorkflow(ctx context.Context, wf *Workflow, cfg RunConfig) (
 
 		step := &wf.Steps[i]
 
-		// Skip steps that are only referenced by parallel dispatchers
-		// (they're executed inline when the parallel step runs)
+		// Skip steps that are dispatched by a parallel step
+		if parallelChildren[step.ID] {
+			i++
+			continue
+		}
+
+		// Parallel dispatcher step
 		if step.Prompt == "" && len(step.Parallel) > 0 {
 			cost, err := e.runParallel(ctx, step, wf.Steps, stepIndex, session, cfg.Input, outputs, opts, &iterNum)
 			if err != nil {
 				e.DB.UpdateSessionStatus(session.ID, "failed")
+				failed = true
+				stepErr = err
 				break
 			}
 			totalUSD += cost
@@ -101,7 +122,7 @@ func (e *Engine) RunWorkflow(ctx context.Context, wf *Workflow, cfg RunConfig) (
 			continue
 		}
 
-		// Regular step
+		// Skip empty steps
 		if step.Prompt == "" {
 			i++
 			continue
@@ -111,13 +132,35 @@ func (e *Engine) RunWorkflow(ctx context.Context, wf *Workflow, cfg RunConfig) (
 			cost, err := e.runLoop(ctx, step, session, cfg.Input, outputs, opts, &iterNum)
 			if err != nil {
 				e.DB.UpdateSessionStatus(session.ID, "failed")
+				failed = true
+				stepErr = err
 				break
 			}
 			totalUSD += cost
+
+			// Evaluate branch after loop (fix #10: branches on loop steps)
+			if len(step.Branch) > 0 {
+				if output, ok := outputs[step.ID]; ok {
+					if target := EvalBranch(output, step.Branch); target != "" {
+						branchCount++
+						if branchCount > maxBranches {
+							fmt.Println("  → branch limit reached, stopping")
+							failed = true
+							stepErr = fmt.Errorf("branch limit exceeded (%d jumps)", maxBranches)
+							break
+						}
+						fmt.Printf("  → branching to %s\n\n", target)
+						i = stepIndex[target]
+						continue
+					}
+				}
+			}
 		} else {
 			cost, output, err := e.runStep(ctx, step, session, cfg.Input, outputs, opts, &iterNum)
 			if err != nil {
 				e.DB.UpdateSessionStatus(session.ID, "failed")
+				failed = true
+				stepErr = err
 				break
 			}
 			totalUSD += cost
@@ -126,11 +169,15 @@ func (e *Engine) RunWorkflow(ctx context.Context, wf *Workflow, cfg RunConfig) (
 			// Evaluate branch
 			if len(step.Branch) > 0 {
 				if target := EvalBranch(output, step.Branch); target != "" {
-					if idx, ok := stepIndex[target]; ok {
-						fmt.Printf("  → branching to %s\n\n", target)
-						i = idx
-						continue
+					branchCount++
+					if branchCount > maxBranches {
+						fmt.Println("  → branch limit reached, stopping")
+						failed = true
+						break
 					}
+					fmt.Printf("  → branching to %s\n\n", target)
+					i = stepIndex[target]
+					continue
 				}
 			}
 		}
@@ -141,19 +188,31 @@ func (e *Engine) RunWorkflow(ctx context.Context, wf *Workflow, cfg RunConfig) (
 	// Clean up scratchpad
 	os.Remove(filepath.Join(scratchDir, "current.md"))
 
-	// Commit any remaining changes
-	e.Git.CommitAll(fmt.Sprintf("%s(%s): complete", session.Workflow, session.ID))
+	// Only mark done on clean exit (fix #3: don't overwrite "failed")
+	if !failed && ctx.Err() == nil {
+		e.Git.CommitAll(fmt.Sprintf("%s(%s): complete", session.Workflow, session.ID))
+		e.DB.UpdateSessionStatus(session.ID, "done")
+	} else if !failed {
+		e.DB.UpdateSessionStatus(session.ID, "cancelled")
+	}
 
-	e.DB.UpdateSessionStatus(session.ID, "done")
 	allSteps, _ := e.DB.GetSteps(session.ID)
-	lib.WriteReport(e.RepoRoot, session, allSteps, "completed")
+	stopReason := "completed"
+	if failed {
+		stopReason = "failed"
+	} else if ctx.Err() != nil {
+		stopReason = "cancelled"
+	} else if overBudget() {
+		stopReason = "budget_exhausted"
+	}
+	lib.WriteReport(e.RepoRoot, session, allSteps, stopReason)
 
 	return &Result{
 		Session:  session,
 		Steps:    allSteps,
 		Outputs:  outputs,
 		TotalUSD: totalUSD,
-	}, nil
+	}, stepErr
 }
 
 // runStep executes a single step and records it in the database.
@@ -189,6 +248,7 @@ func (e *Engine) runStep(ctx context.Context, step *WfStep, session *lib.Session
 }
 
 // runLoop executes a step repeatedly until the until-string is found or max iterations reached.
+// Returns an error if all iterations fail or the until condition is never met.
 func (e *Engine) runLoop(ctx context.Context, step *WfStep, session *lib.Session, input string, outputs map[string]string, opts runners.RunOpts, iterNum *int) (float64, error) {
 	var totalCost float64
 	maxIter := step.Loop.Max
@@ -196,6 +256,9 @@ func (e *Engine) runLoop(ctx context.Context, step *WfStep, session *lib.Session
 		maxIter = 1
 	}
 
+	succeeded := false
+	consecutiveFailures := 0
+
 	for attempt := 1; attempt <= maxIter; attempt++ {
 		if ctx.Err() != nil {
 			return totalCost, ctx.Err()
@@ -218,12 +281,13 @@ func (e *Engine) runLoop(ctx context.Context, step *WfStep, session *lib.Session
 			dbStep.Status = "failed"
 			dbStep.ChangeSummary = err.Error()
 			e.DB.UpdateStep(dbStep)
-			// Loop continues on failure — try again
-			totalCost += result.CostUSD
+			consecutiveFailures++
 			fmt.Printf("  failed, retrying\n\n")
 			continue
 		}
 
+		consecutiveFailures = 0
+		succeeded = true
 		totalCost += result.CostUSD
 		dbStep.Status = "kept"
 		dbStep.Kept = true
@@ -240,14 +304,19 @@ func (e *Engine) runLoop(ctx context.Context, step *WfStep, session *lib.Session
 		// Check until condition
 		if step.Loop.Until != "" && strings.Contains(strings.ToUpper(result.Output), strings.ToUpper(step.Loop.Until)) {
 			fmt.Printf("  → loop complete (%s found)\n\n", step.Loop.Until)
-			break
+			return totalCost, nil
 		}
 	}
 
+	if !succeeded {
+		return totalCost, fmt.Errorf("loop %s: all %d iterations failed", step.ID, maxIter)
+	}
+
 	return totalCost, nil
 }
 
 // runParallel dispatches multiple steps concurrently.
+// Returns an error if ALL parallel steps fail. Partial failures are logged but not fatal.
 func (e *Engine) runParallel(ctx context.Context, dispatcher *WfStep, allSteps []WfStep, stepIndex map[string]int, session *lib.Session, input string, outputs map[string]string, opts runners.RunOpts, iterNum *int) (float64, error) {
 	fmt.Printf("--- %s (parallel: %s) ---\n\n", dispatcher.ID, strings.Join(dispatcher.Parallel, ", "))
 
@@ -258,6 +327,12 @@ func (e *Engine) runParallel(ctx context.Context, dispatcher *WfStep, allSteps [
 		err    error
 	}
 
+	// Snapshot outputs before launching goroutines to avoid data race
+	outputSnap := make(map[string]string, len(outputs))
+	for k, v := range outputs {
+		outputSnap[k] = v
+	}
+
 	var mu sync.Mutex
 	var wg sync.WaitGroup
 	results := make([]parallelResult, len(dispatcher.Parallel))
@@ -278,7 +353,8 @@ func (e *Engine) runParallel(ctx context.Context, dispatcher *WfStep, allSteps [
 			myIter := *iterNum
 			mu.Unlock()
 
-			prompt := Interpolate(step.Prompt, input, outputs)
+			// Use snapshot for interpolation — safe for concurrent reads
+			prompt := Interpolate(step.Prompt, input, outputSnap)
 			dbStep := &lib.Step{
 				SessionID: session.ID,
 				Iteration: myIter,
@@ -316,9 +392,15 @@ func (e *Engine) runParallel(ctx context.Context, dispatcher *WfStep, allSteps [
 	wg.Wait()
 
 	var totalCost float64
+	var failCount int
+	var firstErr error
 	for _, r := range results {
 		if r.err != nil {
 			fmt.Printf("  %s: failed (%v)\n", r.id, r.err)
+			failCount++
+			if firstErr == nil {
+				firstErr = r.err
+			}
 			continue
 		}
 		outputs[r.id] = r.output
@@ -327,6 +409,11 @@ func (e *Engine) runParallel(ctx context.Context, dispatcher *WfStep, allSteps [
 	}
 	fmt.Println()
 
+	// Fail only if ALL parallel steps failed
+	if failCount == len(results) {
+		return totalCost, fmt.Errorf("all parallel steps failed, first: %w", firstErr)
+	}
+
 	return totalCost, nil
 }
 
diff --git a/src/engine/engine_test.go b/src/engine/engine_test.go
index ba7006e..6fb0a90 100644
--- a/src/engine/engine_test.go
+++ b/src/engine/engine_test.go
@@ -2,6 +2,7 @@ package engine
 
 import (
 	"context"
+	"fmt"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -77,14 +78,14 @@ func (m *mockRunner) Run(ctx context.Context, prompt string, opts runners.RunOpt
 	m.prompts = append(m.prompts, prompt)
 	idx := m.callIdx
 	m.callIdx++
-	if idx >= len(m.responses) {
-		return runners.RunResult{Output: "mock exhausted"}, nil
+	// Check errors first — if error is set, return zero result + error
+	if idx < len(m.errors) && m.errors[idx] != nil {
+		return runners.RunResult{}, m.errors[idx]
 	}
-	var err error
-	if idx < len(m.errors) {
-		err = m.errors[idx]
+	if idx < len(m.responses) {
+		return m.responses[idx], nil
 	}
-	return m.responses[idx], err
+	return runners.RunResult{Output: "mock exhausted"}, nil
 }
 
 func result(output string) runners.RunResult {
@@ -539,6 +540,62 @@ func TestRunWorkflowContextCancelled(t *testing.T) {
 	}
 }
 
+func TestRunWorkflowLoopAllFail(t *testing.T) {
+	db := tempDB(t)
+	dir, git := initGitRepo(t)
+
+	runner := newMockRunner(nil, []error{
+		fmt.Errorf("runner error 1"),
+		fmt.Errorf("runner error 2"),
+	})
+
+	eng := &Engine{DB: db, Git: git, Runner: runner, RepoRoot: dir}
+	wf := &Workflow{
+		Name: "test",
+		Steps: []WfStep{
+			{ID: "fix", Model: "smart", Prompt: "Fix", Loop: &Loop{Max: 2, Until: "DONE"}},
+		},
+	}
+
+	_, err := eng.RunWorkflow(context.Background(), wf, RunConfig{Input: "fix"})
+	if err == nil {
+		t.Fatal("expected error when all loop iterations fail")
+	}
+	if !strings.Contains(err.Error(), "all 2 iterations failed") {
+		t.Errorf("unexpected error: %v", err)
+	}
+}
+
+func TestRunWorkflowBranchCycleLimit(t *testing.T) {
+	db := tempDB(t)
+	dir, git := initGitRepo(t)
+
+	// Every step output contains "LOOP" which branches back to itself
+	responses := make([]runners.RunResult, 150)
+	for i := range responses {
+		responses[i] = result("LOOP back")
+	}
+	runner := newMockRunner(responses, nil)
+
+	eng := &Engine{DB: db, Git: git, Runner: runner, RepoRoot: dir}
+	wf := &Workflow{
+		Name: "test",
+		Steps: []WfStep{
+			{ID: "start", Model: "fast", Prompt: "Do", Branch: []Branch{
+				{When: "LOOP", Goto: "start"},
+			}},
+		},
+	}
+
+	_, err := eng.RunWorkflow(context.Background(), wf, RunConfig{Input: "test"})
+	// Engine should complete (possibly with failed status) but not hang
+	_ = err
+	// Should have stopped at maxBranches (100), not run forever
+	if runner.callIdx > 101 {
+		t.Errorf("runner called %d times, expected <= 101 (branch limit)", runner.callIdx)
+	}
+}
+
 // ---------------------------------------------------------------------------
 // Parse real workflow files
 // ---------------------------------------------------------------------------

From 57c332d1643448fd049f72eb90ad931ee4bc86f2 Mon Sep 17 00:00:00 2001
From: Tym Rabchuk <tymrabchuk@gmail.com>
Date: Sat, 4 Apr 2026 18:03:25 -0400
Subject: [PATCH 03/11] Sanitize workflow name to prevent path traversal

Validates workflow name against ^[a-zA-Z0-9_-]+$ before constructing
file paths, preventing directory traversal via crafted names like
"../../etc/passwd". Found by Gemini in tri-review.
---
 src/cmd/workflow.go | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/cmd/workflow.go b/src/cmd/workflow.go
index 5fbdf9a..37cd407 100644
--- a/src/cmd/workflow.go
+++ b/src/cmd/workflow.go
@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"regexp"
 	"strings"
 
 	"github.com/5uck1ess/devkit/engine"
@@ -31,6 +32,11 @@ var workflowCmd = &cobra.Command{
 			return fmt.Errorf("usage: devkit workflow <name> <description>")
 		}
 
+		// Validate workflow name to prevent path traversal
+		if !regexp.MustCompile(`^[a-zA-Z0-9_-]+$`).MatchString(name) {
+			return fmt.Errorf("invalid workflow name %q — use only letters, numbers, hyphens, underscores", name)
+		}
+
 		dirty, err := (&lib.Git{Dir: repoRoot}).HasUncommittedChanges()
 		if err != nil {
 			return fmt.Errorf("check git status: %w", err)

From 34e3ccfc445b448a818928c21022a26a65bb0aa1 Mon Sep 17 00:00:00 2001
From: Tym Rabchuk <tymrabchuk@gmail.com>
Date: Sat, 4 Apr 2026 18:06:10 -0400
Subject: [PATCH 04/11] Add Go code quality hook and harden existing hooks

New hook (go-review.sh):
- Detects error-path result access (accessing fields when err != nil)
- Flags goroutines with shared map access missing mutex/snapshot
- Warns on filepath.Join with unsanitized input variables
Registered as PostToolUse for *.go Edit/Write operations.

security-patterns.sh:
- Add filepath traversal pattern for Go (filepath.Join with user input)

subagent-stop.sh:
- Recognize go vet / go test -race as valid test evidence

These patterns are the top recurring LLM-generated bug categories
identified from the tri-review of the workflow engine PR.
---
 hooks/go-review.sh         | 72 ++++++++++++++++++++++++++++++++++++++
 hooks/hooks.json           | 12 +++++++
 hooks/security-patterns.sh |  1 +
 hooks/subagent-stop.sh     |  5 +++
 4 files changed, 90 insertions(+)
 create mode 100755 hooks/go-review.sh

diff --git a/hooks/go-review.sh b/hooks/go-review.sh
new file mode 100755
index 0000000..82cd7ce
--- /dev/null
+++ b/hooks/go-review.sh
@@ -0,0 +1,72 @@
+#!/bin/bash
+# devkit PostToolUse hook — Go code quality patterns
+#
+# Checks written Go code for common LLM-generated bug patterns:
+# 1. Accessing result fields in error paths
+# 2. Goroutines reading shared maps without protection
+# 3. Functions that always return nil error
+# 4. Unsanitized user input in filepath operations
+#
+# PostToolUse hook schema:
+#   { "hookSpecificOutput": { "hookEventName": "PostToolUse", "additionalContext": "string" } }
+
+set -euo pipefail
+
+INPUT=$(cat)
+TOOL_NAME=$(echo "$INPUT" | jq -r '.tool_name // empty')
+FILE_PATH=$(echo "$INPUT" | jq -r '.tool_input.file_path // empty')
+CONTENT=$(echo "$INPUT" | jq -r '.tool_input.content // .tool_input.new_string // empty')
+
+# Only check Go files
+case "$FILE_PATH" in
+  *.go) ;;
+  *) exit 0 ;;
+esac
+
+# Only check Edit/Write
+if [ "$TOOL_NAME" != "Edit" ] && [ "$TOOL_NAME" != "Write" ]; then
+  exit 0
+fi
+
+if [ -z "$CONTENT" ]; then
+  exit 0
+fi
+
+WARNINGS=""
+
+# Pattern 1: Accessing result after error check
+# Detects: if err != nil { ... } followed by result.Something on the same or next few lines
+if echo "$CONTENT" | grep -qE 'if err != nil' && echo "$CONTENT" | grep -qP 'err != nil[\s\S]{0,200}(result\.|res\.)'; then
+  # More specific: check if result is accessed INSIDE the error block
+  if echo "$CONTENT" | grep -qP 'if err != nil \{[^}]*(result\.|res\.)[^}]*\}'; then
+    WARNINGS="$WARNINGS\n- Possible result field access inside error path (result may be zero-value when err != nil)"
+  fi
+fi
+
+# Pattern 2: Goroutines with shared map access
+if echo "$CONTENT" | grep -qE 'go func' && echo "$CONTENT" | grep -qE 'map\[string\]'; then
+  if ! echo "$CONTENT" | grep -qE '(sync\.Mutex|sync\.RWMutex|sync\.Map|snapshot|Snap)'; then
+    WARNINGS="$WARNINGS\n- Goroutines detected with map usage but no visible mutex/snapshot — verify concurrent map access is safe"
+  fi
+fi
+
+# Pattern 3: filepath.Join with unsanitized variable
+if echo "$CONTENT" | grep -qE 'filepath\.Join.*\b(name|input|arg|param|user)'; then
+  if ! echo "$CONTENT" | grep -qE '(regexp|Regexp|MustCompile|MatchString|ValidateName|sanitize)'; then
+    WARNINGS="$WARNINGS\n- filepath.Join with potentially unsanitized input — validate before constructing paths"
+  fi
+fi
+
+if [ -n "$WARNINGS" ]; then
+  MSG=$(printf "Go code quality check:%b" "$WARNINGS")
+  jq -n --arg msg "$MSG" '{
+    hookSpecificOutput: {
+      hookEventName: "PostToolUse",
+      additionalContext: $msg
+    }
+  }'
+  exit 0
+fi
+
+# All clear
+exit 0
diff --git a/hooks/hooks.json b/hooks/hooks.json
index bacd65d..30128d5 100644
--- a/hooks/hooks.json
+++ b/hooks/hooks.json
@@ -102,6 +102,18 @@
             "timeout": 5
           }
         ]
+      },
+      {
+        "matcher": "Edit|Write",
+        "hooks": [
+          {
+            "type": "command",
+            "command": "${CLAUDE_PLUGIN_ROOT}/hooks/go-review.sh",
+            "if": "Edit(*.go) || Write(*.go)",
+            "statusMessage": "Go quality check...",
+            "timeout": 5
+          }
+        ]
       }
     ],
     "SubagentStop": [
diff --git a/hooks/security-patterns.sh b/hooks/security-patterns.sh
index ef92d94..b781cf0 100755
--- a/hooks/security-patterns.sh
+++ b/hooks/security-patterns.sh
@@ -71,6 +71,7 @@ if echo "$FILE_PATH" | grep -qE '\.go$'; then
   check_pattern 'exec\.Command\s*\(\s*"(sh|bash)"' "Security: shell execution via exec.Command — pass arguments directly, avoid sh -c"
   check_pattern 'md5\.New\s*\(' "Security: MD5 is cryptographically broken — use SHA-256 or better"
   check_pattern 'sha1\.New\s*\(' "Security: SHA-1 is deprecated — use SHA-256 or better"
+  check_pattern 'filepath\.(Join|Clean)\s*\([^)]*\b(name|input|arg|param)\b' "Security: filepath with user input — validate against path traversal (e.g., ^[a-zA-Z0-9_-]+$)"
 fi
 
 # --- SQL patterns (any file) ---
diff --git a/hooks/subagent-stop.sh b/hooks/subagent-stop.sh
index 2ba6bd0..18f89fe 100755
--- a/hooks/subagent-stop.sh
+++ b/hooks/subagent-stop.sh
@@ -39,6 +39,11 @@ if echo "$AGENT_OUTPUT" | grep -qE '(passed|failed|error).*(pytest|test)|pytest\
   TEST_EVIDENCE=true
 fi
 
+# Go vet (race detector evidence)
+if echo "$AGENT_OUTPUT" | grep -qE '(go vet|go test.*-race|-vet=|vet: )'; then
+  TEST_EVIDENCE=true
+fi
+
 # Generic pass/fail signals
 if echo "$AGENT_OUTPUT" | grep -qE '(ALL_PASSING|ALL_DONE|ALL_TESTS_PASSING|BUILD_SUCCESS|LINT_CLEAN|RESEARCH_COMPLETE)'; then
   TEST_EVIDENCE=true

From d02126b63242dd672b99a63fbe68b3392b83a7c0 Mon Sep 17 00:00:00 2001
From: Tym Rabchuk <tymrabchuk@gmail.com>
Date: Sat, 4 Apr 2026 18:13:08 -0400
Subject: [PATCH 05/11] Fix PR review findings: budget-in-loop, branch-limit
 error, test gaps

From PR review toolkit analysis:

silent-failure-hunter:
- Fix missing stepErr on non-loop branch limit path
- Check os.MkdirAll error for scratchpad dir
- Explicit _ = os.Remove for intentional discard

pr-test-analyzer:
- Add budget enforcement inside runLoop (overBudget + addCost callbacks)
- Add TestRunWorkflowStepFailure (non-loop step error propagation)
- Add TestRunWorkflowParallelPartialFailure (some fail, some succeed)
- Add TestRunWorkflowParallelAllFail (all parallel steps fail)
- Add TestRunWorkflowBudgetInLoop (budget respected mid-loop)
- Make mock runner thread-safe with sync.Mutex

24 engine tests now pass (was 20).
---
 src/engine/engine.go      |  27 ++++++---
 src/engine/engine_test.go | 120 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 140 insertions(+), 7 deletions(-)

diff --git a/src/engine/engine.go b/src/engine/engine.go
index ea104ca..0ddb1e3 100644
--- a/src/engine/engine.go
+++ b/src/engine/engine.go
@@ -58,7 +58,9 @@ func (e *Engine) RunWorkflow(ctx context.Context, wf *Workflow, cfg RunConfig) (
 
 	// Ensure scratchpad directory exists
 	scratchDir := filepath.Join(e.RepoRoot, ".devkit", "scratchpads")
-	os.MkdirAll(scratchDir, 0o755)
+	if err := os.MkdirAll(scratchDir, 0o755); err != nil {
+		return nil, fmt.Errorf("create scratchpad dir: %w", err)
+	}
 
 	outputs := make(map[string]string)
 	stepIndex := buildStepIndex(wf.Steps)
@@ -74,6 +76,8 @@ func (e *Engine) RunWorkflow(ctx context.Context, wf *Workflow, cfg RunConfig) (
 	overBudget := func() bool {
 		return cfg.BudgetUSD > 0 && totalUSD >= cfg.BudgetUSD
 	}
+	// addCost updates the running total (used by loops to keep overBudget accurate)
+	addCost := func(c float64) { totalUSD += c }
 
 	// Build set of step IDs that are dispatched by parallel steps,
 	// so we skip them during sequential walk (they run inside runParallel).
@@ -129,14 +133,15 @@ func (e *Engine) RunWorkflow(ctx context.Context, wf *Workflow, cfg RunConfig) (
 		}
 
 		if step.Loop != nil {
-			cost, err := e.runLoop(ctx, step, session, cfg.Input, outputs, opts, &iterNum)
+			// Note: addCost updates totalUSD live for budget checks inside the loop,
+			// so we don't add the returned cost again here.
+			_, err := e.runLoop(ctx, step, session, cfg.Input, outputs, opts, &iterNum, overBudget, addCost)
 			if err != nil {
 				e.DB.UpdateSessionStatus(session.ID, "failed")
 				failed = true
 				stepErr = err
 				break
 			}
-			totalUSD += cost
 
 			// Evaluate branch after loop (fix #10: branches on loop steps)
 			if len(step.Branch) > 0 {
@@ -173,6 +178,7 @@ func (e *Engine) RunWorkflow(ctx context.Context, wf *Workflow, cfg RunConfig) (
 					if branchCount > maxBranches {
 						fmt.Println("  → branch limit reached, stopping")
 						failed = true
+						stepErr = fmt.Errorf("branch limit exceeded (%d jumps)", maxBranches)
 						break
 					}
 					fmt.Printf("  → branching to %s\n\n", target)
@@ -185,8 +191,8 @@ func (e *Engine) RunWorkflow(ctx context.Context, wf *Workflow, cfg RunConfig) (
 		i++
 	}
 
-	// Clean up scratchpad
-	os.Remove(filepath.Join(scratchDir, "current.md"))
+	// Clean up scratchpad (best-effort)
+	_ = os.Remove(filepath.Join(scratchDir, "current.md"))
 
 	// Only mark done on clean exit (fix #3: don't overwrite "failed")
 	if !failed && ctx.Err() == nil {
@@ -248,8 +254,8 @@ func (e *Engine) runStep(ctx context.Context, step *WfStep, session *lib.Session
 }
 
 // runLoop executes a step repeatedly until the until-string is found or max iterations reached.
-// Returns an error if all iterations fail or the until condition is never met.
-func (e *Engine) runLoop(ctx context.Context, step *WfStep, session *lib.Session, input string, outputs map[string]string, opts runners.RunOpts, iterNum *int) (float64, error) {
+// Returns an error if all iterations fail. Respects budget via overBudget, reports cost via addCost.
+func (e *Engine) runLoop(ctx context.Context, step *WfStep, session *lib.Session, input string, outputs map[string]string, opts runners.RunOpts, iterNum *int, overBudget func() bool, addCost func(float64)) (float64, error) {
 	var totalCost float64
 	maxIter := step.Loop.Max
 	if maxIter <= 0 {
@@ -263,6 +269,10 @@ func (e *Engine) runLoop(ctx context.Context, step *WfStep, session *lib.Session
 		if ctx.Err() != nil {
 			return totalCost, ctx.Err()
 		}
+		if overBudget != nil && overBudget() {
+			fmt.Printf("  → budget exhausted, stopping loop\n")
+			break
+		}
 
 		*iterNum++
 		fmt.Printf("--- %s [%d/%d] (step %d) ---\n", step.ID, attempt, maxIter, *iterNum)
@@ -289,6 +299,9 @@ func (e *Engine) runLoop(ctx context.Context, step *WfStep, session *lib.Session
 		consecutiveFailures = 0
 		succeeded = true
 		totalCost += result.CostUSD
+		if addCost != nil {
+			addCost(result.CostUSD)
+		}
 		dbStep.Status = "kept"
 		dbStep.Kept = true
 		dbStep.CostUSD = result.CostUSD
diff --git a/src/engine/engine_test.go b/src/engine/engine_test.go
index 6fb0a90..be2c16d 100644
--- a/src/engine/engine_test.go
+++ b/src/engine/engine_test.go
@@ -7,6 +7,7 @@ import (
 	"os/exec"
 	"path/filepath"
 	"strings"
+	"sync"
 	"testing"
 
 	"github.com/5uck1ess/devkit/lib"
@@ -65,6 +66,7 @@ type mockRunner struct {
 	errors    []error
 	callIdx   int
 	prompts   []string
+	mu        sync.Mutex
 }
 
 func newMockRunner(responses []runners.RunResult, errs []error) *mockRunner {
@@ -75,6 +77,8 @@ func (m *mockRunner) Name() string    { return m.name }
 func (m *mockRunner) Available() bool { return true }
 
 func (m *mockRunner) Run(ctx context.Context, prompt string, opts runners.RunOpts) (runners.RunResult, error) {
+	m.mu.Lock()
+	defer m.mu.Unlock()
 	m.prompts = append(m.prompts, prompt)
 	idx := m.callIdx
 	m.callIdx++
@@ -596,6 +600,122 @@ func TestRunWorkflowBranchCycleLimit(t *testing.T) {
 	}
 }
 
+func TestRunWorkflowStepFailure(t *testing.T) {
+	db := tempDB(t)
+	dir, git := initGitRepo(t)
+
+	runner := newMockRunner(
+		[]runners.RunResult{result("plan done")},
+		[]error{nil, fmt.Errorf("implement failed")},
+	)
+
+	eng := &Engine{DB: db, Git: git, Runner: runner, RepoRoot: dir}
+	wf := &Workflow{
+		Name: "test",
+		Steps: []WfStep{
+			{ID: "plan", Model: "smart", Prompt: "Plan"},
+			{ID: "impl", Model: "smart", Prompt: "Implement"},
+		},
+	}
+
+	_, err := eng.RunWorkflow(context.Background(), wf, RunConfig{Input: "test"})
+	if err == nil {
+		t.Fatal("expected error when step fails")
+	}
+	if !strings.Contains(err.Error(), "impl failed") {
+		t.Errorf("error should reference step: %v", err)
+	}
+}
+
+func TestRunWorkflowParallelPartialFailure(t *testing.T) {
+	db := tempDB(t)
+	dir, git := initGitRepo(t)
+
+	// One succeeds, one fails — order depends on goroutine scheduling
+	runner := newMockRunner(
+		[]runners.RunResult{result("review ok")},
+		[]error{nil, fmt.Errorf("review crashed")},
+	)
+
+	eng := &Engine{DB: db, Git: git, Runner: runner, RepoRoot: dir}
+	wf := &Workflow{
+		Name: "test",
+		Steps: []WfStep{
+			{ID: "ra", Model: "smart", Prompt: "Review A"},
+			{ID: "rb", Model: "fast", Prompt: "Review B"},
+			{ID: "dispatch", Parallel: []string{"ra", "rb"}},
+		},
+	}
+
+	res, err := eng.RunWorkflow(context.Background(), wf, RunConfig{Input: "review"})
+	if err != nil {
+		t.Fatalf("partial failure should not error: %v", err)
+	}
+	// At least one step should have output (we don't know which got the success)
+	hasOutput := len(res.Outputs["ra"]) > 0 || len(res.Outputs["rb"]) > 0
+	if !hasOutput {
+		t.Error("expected at least one parallel step to have output")
+	}
+}
+
+func TestRunWorkflowParallelAllFail(t *testing.T) {
+	db := tempDB(t)
+	dir, git := initGitRepo(t)
+
+	runner := newMockRunner(nil, []error{
+		fmt.Errorf("review A failed"),
+		fmt.Errorf("review B failed"),
+	})
+
+	eng := &Engine{DB: db, Git: git, Runner: runner, RepoRoot: dir}
+	wf := &Workflow{
+		Name: "test",
+		Steps: []WfStep{
+			{ID: "ra", Model: "smart", Prompt: "Review A"},
+			{ID: "rb", Model: "fast", Prompt: "Review B"},
+			{ID: "dispatch", Parallel: []string{"ra", "rb"}},
+		},
+	}
+
+	_, err := eng.RunWorkflow(context.Background(), wf, RunConfig{Input: "review"})
+	if err == nil {
+		t.Fatal("expected error when all parallel steps fail")
+	}
+	if !strings.Contains(err.Error(), "all parallel steps failed") {
+		t.Errorf("unexpected error: %v", err)
+	}
+}
+
+func TestRunWorkflowBudgetInLoop(t *testing.T) {
+	db := tempDB(t)
+	dir, git := initGitRepo(t)
+
+	// Each iteration costs 0.50, budget is 1.00 — should stop after 2
+	responses := make([]runners.RunResult, 10)
+	for i := range responses {
+		responses[i] = runners.RunResult{Output: "still broken", CostUSD: 0.50}
+	}
+	runner := newMockRunner(responses, nil)
+
+	eng := &Engine{DB: db, Git: git, Runner: runner, RepoRoot: dir}
+	wf := &Workflow{
+		Name: "test",
+		Steps: []WfStep{
+			{ID: "fix", Model: "smart", Prompt: "Fix", Loop: &Loop{Max: 10, Until: "DONE"}},
+		},
+	}
+
+	res, err := eng.RunWorkflow(context.Background(), wf, RunConfig{Input: "fix", BudgetUSD: 1.00})
+	_ = err
+	// At $0.50/iter with $1.00 budget: 2 iterations run ($1.00), iteration 3 blocked
+	if runner.callIdx > 3 {
+		t.Errorf("runner called %d times, expected <= 3 (budget should stop loop)", runner.callIdx)
+	}
+	if res.TotalUSD > 1.50 {
+		t.Errorf("total cost $%.2f, expected <= $1.50", res.TotalUSD)
+	}
+}
+
 // ---------------------------------------------------------------------------
 // Parse real workflow files
 // ---------------------------------------------------------------------------

From eb308208aba687cc1ea967b4839dd43dbd3d074e Mon Sep 17 00:00:00 2001
From: Tym Rabchuk <tymrabchuk@gmail.com>
Date: Sat, 4 Apr 2026 18:14:25 -0400
Subject: [PATCH 06/11] Fix code-reviewer findings: grep -P on macOS, go.mod,
 hooks.json

- Replace grep -P (Perl regex, unavailable on macOS) with awk in
  go-review.sh for error-path detection pattern
- Remove unsupported "if" field from hooks.json go-review entry
  (hook self-filters via case statement)
- Run go mod tidy to promote yaml.v3 from indirect to direct
---
 hooks/go-review.sh | 8 ++++----
 hooks/hooks.json   | 1 -
 src/go.mod         | 2 +-
 src/go.sum         | 1 +
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/hooks/go-review.sh b/hooks/go-review.sh
index 82cd7ce..bf0884e 100755
--- a/hooks/go-review.sh
+++ b/hooks/go-review.sh
@@ -35,10 +35,10 @@ fi
 WARNINGS=""
 
 # Pattern 1: Accessing result after error check
-# Detects: if err != nil { ... } followed by result.Something on the same or next few lines
-if echo "$CONTENT" | grep -qE 'if err != nil' && echo "$CONTENT" | grep -qP 'err != nil[\s\S]{0,200}(result\.|res\.)'; then
-  # More specific: check if result is accessed INSIDE the error block
-  if echo "$CONTENT" | grep -qP 'if err != nil \{[^}]*(result\.|res\.)[^}]*\}'; then
+# Detects: if err != nil { ... result. or res. ... } on nearby lines
+# Uses awk instead of grep -P to stay macOS-compatible
+if echo "$CONTENT" | grep -qE 'if err != nil'; then
+  if echo "$CONTENT" | awk '/if err != nil \{/{found=1; buf=""} found{buf=buf $0 "\n"; if(/\}/){if(buf ~ /result\.|res\./){exit 0} found=0}} END{exit 1}'; then
     WARNINGS="$WARNINGS\n- Possible result field access inside error path (result may be zero-value when err != nil)"
   fi
 fi
diff --git a/hooks/hooks.json b/hooks/hooks.json
index 30128d5..6ca3460 100644
--- a/hooks/hooks.json
+++ b/hooks/hooks.json
@@ -109,7 +109,6 @@
           {
             "type": "command",
             "command": "${CLAUDE_PLUGIN_ROOT}/hooks/go-review.sh",
-            "if": "Edit(*.go) || Write(*.go)",
             "statusMessage": "Go quality check...",
             "timeout": 5
           }
diff --git a/src/go.mod b/src/go.mod
index 0b01954..aa00bf9 100644
--- a/src/go.mod
+++ b/src/go.mod
@@ -4,6 +4,7 @@ go 1.26.1
 
 require (
 	github.com/spf13/cobra v1.10.2
+	gopkg.in/yaml.v3 v3.0.1
 	modernc.org/sqlite v1.48.0
 )
 
@@ -16,7 +17,6 @@ require (
 	github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec // indirect
 	github.com/spf13/pflag v1.0.9 // indirect
 	golang.org/x/sys v0.42.0 // indirect
-	gopkg.in/yaml.v3 v3.0.1 // indirect
 	modernc.org/libc v1.70.0 // indirect
 	modernc.org/mathutil v1.7.1 // indirect
 	modernc.org/memory v1.11.0 // indirect
diff --git a/src/go.sum b/src/go.sum
index 1483f14..14b1067 100644
--- a/src/go.sum
+++ b/src/go.sum
@@ -30,6 +30,7 @@ golang.org/x/sys v0.42.0 h1:omrd2nAlyT5ESRdCLYdm3+fMfNFE/+Rf4bDIQImRJeo=
 golang.org/x/sys v0.42.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw=
 golang.org/x/tools v0.42.0 h1:uNgphsn75Tdz5Ji2q36v/nsFSfR/9BRFvqhGBaJGd5k=
 golang.org/x/tools v0.42.0/go.mod h1:Ma6lCIwGZvHK6XtgbswSoWroEkhugApmsXyrUmBhfr0=
+gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
 gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=

From 11456f74aa720099131b2a0e019b7a432ee2fb03 Mon Sep 17 00:00:00 2001
From: Tym Rabchuk <tymrabchuk@gmail.com>
Date: Sat, 4 Apr 2026 18:16:49 -0400
Subject: [PATCH 07/11] Add three self-learning hooks: go vet, shell
 portability, nil-error detection

go-vet-stop.sh (Stop hook):
- Runs go vet on modified Go packages before session completes
- Runs go test -race to catch data races (would have caught the
  outputs map race in runParallel)
- 90s timeout, only triggers when Go files changed

shell-compat.sh (PreToolUse hook):
- Flags macOS-incompatible constructs in shell scripts at write-time
- Catches: grep -P, sed -i without '', readlink -f, stat --format,
  xargs -d, date -d, mktemp --suffix
- Would have caught the grep -P issue in go-review.sh

go-nil-return.sh (PostToolUse hook):
- Detects Go functions with error return type that only ever return nil
- Uses awk to parse function boundaries and return statements
- Would have caught runLoop/runParallel always returning nil error

All three hooks learned from bugs found during this PR's review cycle.
---
 hooks/go-nil-return.sh | 96 ++++++++++++++++++++++++++++++++++++++++++
 hooks/go-vet-stop.sh   | 81 +++++++++++++++++++++++++++++++++++
 hooks/hooks.json       | 33 +++++++++++++++
 hooks/shell-compat.sh  | 82 ++++++++++++++++++++++++++++++++++++
 4 files changed, 292 insertions(+)
 create mode 100755 hooks/go-nil-return.sh
 create mode 100755 hooks/go-vet-stop.sh
 create mode 100755 hooks/shell-compat.sh

diff --git a/hooks/go-nil-return.sh b/hooks/go-nil-return.sh
new file mode 100755
index 0000000..1376382
--- /dev/null
+++ b/hooks/go-nil-return.sh
@@ -0,0 +1,96 @@
+#!/bin/bash
+# devkit PostToolUse hook — detects Go functions that always return nil error
+#
+# Scans written Go code for functions with error return types where
+# every return statement returns nil for the error. This pattern
+# silently swallows failures and is a top LLM-generated bug category.
+#
+# PostToolUse hook schema:
+#   { "hookSpecificOutput": { "hookEventName": "PostToolUse", "additionalContext": "string" } }
+
+set -euo pipefail
+
+INPUT=$(cat)
+TOOL_NAME=$(echo "$INPUT" | jq -r '.tool_name // empty')
+FILE_PATH=$(echo "$INPUT" | jq -r '.tool_input.file_path // empty')
+CONTENT=$(echo "$INPUT" | jq -r '.tool_input.content // .tool_input.new_string // empty')
+
+# Only check Go files on Edit/Write
+case "$FILE_PATH" in
+  *.go) ;;
+  *) exit 0 ;;
+esac
+if [ "$TOOL_NAME" != "Edit" ] && [ "$TOOL_NAME" != "Write" ]; then
+  exit 0
+fi
+[ -z "$CONTENT" ] && exit 0
+
+# Use awk to find functions that return error but only ever return nil
+# Strategy: track function signatures with error returns, then check
+# if ALL return statements in that function use nil for the error position
+WARNINGS=$(echo "$CONTENT" | awk '
+  # Match function declarations that return error (last return type)
+  /^func .*\)\s*(\(.*error\)|error)\s*\{/ {
+    fname = $0
+    sub(/\{.*/, "", fname)
+    in_func = 1
+    brace_depth = 1
+    has_return = 0
+    has_non_nil_err = 0
+    # Count opening brace
+    gsub(/[^{]/, "", $0); gsub(/[^}]/, "", tmp=$0)
+    next
+  }
+
+  in_func {
+    # Track brace depth
+    line = $0
+    for (i = 1; i <= length(line); i++) {
+      c = substr(line, i, 1)
+      if (c == "{") brace_depth++
+      if (c == "}") brace_depth--
+    }
+
+    # Check return statements
+    if ($0 ~ /return /) {
+      has_return = 1
+      # Check if error position is non-nil (not "nil" or "nil)")
+      if ($0 !~ /,\s*nil\s*$/ && $0 !~ /return nil\s*$/ && $0 !~ /,\s*nil\s*\)/) {
+        has_non_nil_err = 1
+      }
+    }
+
+    # End of function
+    if (brace_depth <= 0) {
+      if (has_return && !has_non_nil_err) {
+        # Strip leading whitespace from function name
+        gsub(/^[[:space:]]+/, "", fname)
+        print fname
+      }
+      in_func = 0
+    }
+  }
+')
+
+if [ -n "$WARNINGS" ]; then
+  # Limit to first 3 functions to avoid noise
+  FUNCS=$(echo "$WARNINGS" | head -3)
+  COUNT=$(echo "$WARNINGS" | wc -l | tr -d ' ')
+  MSG="Go nil-error pattern: ${COUNT} function(s) return error but only ever return nil. This silently swallows failures:"
+  while IFS= read -r fn; do
+    MSG="$MSG\n  - $fn"
+  done <<< "$FUNCS"
+  if [ "$COUNT" -gt 3 ]; then
+    MSG="$MSG\n  ... and $((COUNT - 3)) more"
+  fi
+
+  jq -n --arg msg "$MSG" '{
+    hookSpecificOutput: {
+      hookEventName: "PostToolUse",
+      additionalContext: $msg
+    }
+  }'
+  exit 0
+fi
+
+exit 0
diff --git a/hooks/go-vet-stop.sh b/hooks/go-vet-stop.sh
new file mode 100755
index 0000000..cddfea4
--- /dev/null
+++ b/hooks/go-vet-stop.sh
@@ -0,0 +1,81 @@
+#!/bin/bash
+# devkit Stop hook — enforces go vet + race detector on Go changes
+#
+# When Go files were modified in the session, runs go vet and
+# go test -race to catch concurrency bugs before session completes.
+#
+# Stop hook schema:
+#   { "decision": "approve" | "block", "reason": "string" }
+
+set -euo pipefail
+
+# Check if any Go files were modified
+REPO_ROOT=$(git rev-parse --show-toplevel 2>/dev/null || pwd)
+GO_CHANGES=$(cd "$REPO_ROOT" && {
+  git diff --name-only HEAD 2>/dev/null
+  git diff --name-only --cached 2>/dev/null
+  git diff --name-only 2>/dev/null
+} | grep '\.go$' | sort -u)
+
+if [ -z "$GO_CHANGES" ]; then
+  jq -n '{ decision: "approve" }'
+  exit 0
+fi
+
+# Find the Go module root (directory containing go.mod)
+GO_MOD_DIR=""
+for candidate in "$REPO_ROOT" "$REPO_ROOT/src" "$REPO_ROOT/cmd"; do
+  if [ -f "$candidate/go.mod" ]; then
+    GO_MOD_DIR="$candidate"
+    break
+  fi
+done
+
+if [ -z "$GO_MOD_DIR" ]; then
+  # No go.mod found — can't run vet, approve and move on
+  jq -n '{ decision: "approve" }'
+  exit 0
+fi
+
+# Run go vet
+VET_OUTPUT=$(cd "$GO_MOD_DIR" && go vet ./... 2>&1) || true
+if [ -n "$VET_OUTPUT" ]; then
+  jq -n --arg msg "go vet found issues in modified Go files. Fix before completing:\n$VET_OUTPUT" '{
+    decision: "block",
+    reason: $msg
+  }'
+  exit 0
+fi
+
+# Run go test -race on packages with changes (limited to 60s)
+# Extract unique package directories from changed files
+PACKAGES=""
+while IFS= read -r file; do
+  dir=$(dirname "$file")
+  # Convert filesystem path to Go package path relative to module
+  rel=$(echo "$dir" | sed "s|^${GO_MOD_DIR#$REPO_ROOT/}/||; s|^${GO_MOD_DIR#$REPO_ROOT/}$|.|")
+  if [ "$rel" = "$dir" ]; then
+    rel="./$(echo "$dir" | sed "s|^src/||")"
+  fi
+  PACKAGES="$PACKAGES ./$rel"
+done <<< "$GO_CHANGES"
+PACKAGES=$(echo "$PACKAGES" | tr ' ' '\n' | sort -u | tr '\n' ' ')
+
+if [ -n "$PACKAGES" ]; then
+  RACE_OUTPUT=$(cd "$GO_MOD_DIR" && timeout 60 go test -race -count=1 $PACKAGES 2>&1) || RACE_EXIT=$?
+  if [ "${RACE_EXIT:-0}" -ne 0 ]; then
+    # Check if it's specifically a race condition
+    if echo "$RACE_OUTPUT" | grep -qE 'DATA RACE|race detected'; then
+      RACE_LINES=$(echo "$RACE_OUTPUT" | grep -A5 'DATA RACE' | head -20)
+      jq -n --arg msg "Race condition detected in modified Go packages:\n$RACE_LINES" '{
+        decision: "block",
+        reason: $msg
+      }'
+      exit 0
+    fi
+    # Test failure but not a race — don't block on this hook (dirty-bit handles test coverage)
+  fi
+fi
+
+jq -n '{ decision: "approve" }'
+exit 0
diff --git a/hooks/hooks.json b/hooks/hooks.json
index 6ca3460..1bccd63 100644
--- a/hooks/hooks.json
+++ b/hooks/hooks.json
@@ -67,6 +67,17 @@
             "timeout": 5
           }
         ]
+      },
+      {
+        "matcher": "Edit|Write",
+        "hooks": [
+          {
+            "type": "command",
+            "command": "${CLAUDE_PLUGIN_ROOT}/hooks/shell-compat.sh",
+            "statusMessage": "Shell portability check...",
+            "timeout": 5
+          }
+        ]
       }
     ],
     "PostToolUse": [
@@ -113,6 +124,17 @@
             "timeout": 5
           }
         ]
+      },
+      {
+        "matcher": "Edit|Write",
+        "hooks": [
+          {
+            "type": "command",
+            "command": "${CLAUDE_PLUGIN_ROOT}/hooks/go-nil-return.sh",
+            "statusMessage": "Nil-error check...",
+            "timeout": 5
+          }
+        ]
       }
     ],
     "SubagentStop": [
@@ -139,6 +161,17 @@
             "timeout": 10
           }
         ]
+      },
+      {
+        "matcher": "Stop",
+        "hooks": [
+          {
+            "type": "command",
+            "command": "${CLAUDE_PLUGIN_ROOT}/hooks/go-vet-stop.sh",
+            "statusMessage": "Running go vet + race check...",
+            "timeout": 90
+          }
+        ]
       }
     ]
   }
diff --git a/hooks/shell-compat.sh b/hooks/shell-compat.sh
new file mode 100755
index 0000000..831a0b5
--- /dev/null
+++ b/hooks/shell-compat.sh
@@ -0,0 +1,82 @@
+#!/bin/bash
+# devkit PreToolUse hook — shell script portability check
+#
+# Flags non-portable constructs in shell scripts that break on macOS:
+# - grep -P (Perl regex, BSD grep doesn't support it)
+# - sed -i without '' (GNU vs BSD sed)
+# - readlink -f (use realpath or manual resolution)
+# - stat --format (GNU stat, not BSD)
+# - xargs -d (GNU xargs, not BSD)
+# - date -d (GNU date, not BSD)
+#
+# PreToolUse hook schema:
+#   { "hookSpecificOutput": { "hookEventName": "PreToolUse", "permissionDecision": "ask", ... } }
+
+set -euo pipefail
+
+INPUT=$(cat)
+TOOL_NAME=$(echo "$INPUT" | jq -r '.tool_name // empty')
+FILE_PATH=$(echo "$INPUT" | jq -r '.tool_input.file_path // empty')
+CONTENT=$(echo "$INPUT" | jq -r '.tool_input.content // .tool_input.new_string // empty')
+
+# Only check shell scripts
+case "$FILE_PATH" in
+  *.sh) ;;
+  *) exit 0 ;;
+esac
+
+# Only check Edit/Write
+if [ "$TOOL_NAME" != "Edit" ] && [ "$TOOL_NAME" != "Write" ]; then
+  exit 0
+fi
+
+[ -z "$CONTENT" ] && exit 0
+
+# Session dedup
+SEEN_FILE="/tmp/devkit-shellcompat-seen-$$"
+
+check_compat() {
+  local pattern="$1"
+  local message="$2"
+  local key="${FILE_PATH}:${pattern}"
+
+  if echo "$CONTENT" | grep -qE "$pattern"; then
+    if [ -f "$SEEN_FILE" ] && grep -qF "$key" "$SEEN_FILE" 2>/dev/null; then
+      return
+    fi
+    echo "$key" >> "$SEEN_FILE" 2>/dev/null
+
+    jq -n --arg reason "$message" '{
+      hookSpecificOutput: {
+        hookEventName: "PreToolUse",
+        permissionDecision: "ask",
+        permissionDecisionReason: $reason
+      }
+    }'
+    exit 0
+  fi
+}
+
+check_compat 'grep\s+(-[a-zA-Z]*P|--perl-regexp)' \
+  "Portability: grep -P (Perl regex) is unavailable on macOS — use grep -E, awk, or perl instead"
+
+check_compat 'sed\s+-i\s+[^'"'"'"]' \
+  "Portability: sed -i without '' breaks on macOS BSD sed — use sed -i '' for in-place edits"
+
+check_compat 'readlink\s+-f\b' \
+  "Portability: readlink -f is GNU-only — use realpath or manual loop on macOS"
+
+check_compat 'stat\s+--format' \
+  "Portability: stat --format is GNU-only — use stat -f on macOS"
+
+check_compat 'xargs\s+-d\b' \
+  "Portability: xargs -d is GNU-only — use tr + xargs or while-read on macOS"
+
+check_compat 'date\s+-d\b' \
+  "Portability: date -d is GNU-only — use date -j -f on macOS"
+
+check_compat 'mktemp\s+--suffix' \
+  "Portability: mktemp --suffix is GNU-only — use mktemp with template on macOS"
+
+# All clear
+exit 0

From 07df9fe0ef51c53be87be4fd5db4f9575452cb66 Mon Sep 17 00:00:00 2001
From: Tym Rabchuk <tymrabchuk@gmail.com>
Date: Sat, 4 Apr 2026 18:30:21 -0400
Subject: [PATCH 08/11] Harden type design: NewEngine constructor, validation,
 mutual exclusion
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Type-design-analyzer recommendations implemented:

1. NewEngine() constructor — validates all 4 fields are non-nil/non-empty,
   fields unexported to prevent post-construction mutation

2. RunConfig validation — rejects negative BudgetUSD at RunWorkflow entry

3. WfStep mutual exclusion — parallel steps cannot have prompt or loop,
   enforced at parse time with clear error messages

4. Budget validation — negative budget.limit rejected at parse time

5. Workflow.Validate() method — allows engine-boundary validation for
   workflows constructed directly (not via Parse)

29 engine tests (was 24): added NewEngine validation, negative budget,
parallel+prompt mutual exclusion, parallel+loop mutual exclusion.
---
 src/cmd/workflow.go       |  8 ++--
 src/engine/engine.go      | 92 +++++++++++++++++++++++++--------------
 src/engine/engine_test.go | 90 ++++++++++++++++++++++++++++++++------
 src/engine/workflow.go    | 19 ++++++++
 4 files changed, 158 insertions(+), 51 deletions(-)

diff --git a/src/cmd/workflow.go b/src/cmd/workflow.go
index 37cd407..7fd1d9c 100644
--- a/src/cmd/workflow.go
+++ b/src/cmd/workflow.go
@@ -69,11 +69,9 @@ var workflowCmd = &cobra.Command{
 			budget = float64(wf.Budget.Limit) / 1000.0 * 0.01
 		}
 
-		eng := &engine.Engine{
-			DB:       db,
-			Git:      &lib.Git{Dir: repoRoot},
-			Runner:   runner,
-			RepoRoot: repoRoot,
+		eng, err := engine.NewEngine(db, &lib.Git{Dir: repoRoot}, runner, repoRoot)
+		if err != nil {
+			return err
 		}
 
 		description := strings.Join(args[1:], " ")
diff --git a/src/engine/engine.go b/src/engine/engine.go
index 0ddb1e3..4ce5ee3 100644
--- a/src/engine/engine.go
+++ b/src/engine/engine.go
@@ -14,13 +14,31 @@ import (
 
 // Engine executes parsed workflows using a runner and database.
 type Engine struct {
-	DB       *lib.DB
-	Git      *lib.Git
-	Runner   runners.Runner
-	RepoRoot string
+	db       *lib.DB
+	git      *lib.Git
+	runner   runners.Runner
+	repoRoot string
+}
+
+// NewEngine creates a validated Engine. All fields are required.
+func NewEngine(db *lib.DB, git *lib.Git, runner runners.Runner, repoRoot string) (*Engine, error) {
+	if db == nil {
+		return nil, fmt.Errorf("engine: db is required")
+	}
+	if git == nil {
+		return nil, fmt.Errorf("engine: git is required")
+	}
+	if runner == nil {
+		return nil, fmt.Errorf("engine: runner is required")
+	}
+	if repoRoot == "" {
+		return nil, fmt.Errorf("engine: repoRoot is required")
+	}
+	return &Engine{db: db, git: git, runner: runner, repoRoot: repoRoot}, nil
 }
 
 // RunConfig holds per-invocation settings.
+// BudgetUSD of 0 means unlimited. Negative values are rejected.
 type RunConfig struct {
 	Input     string
 	BudgetUSD float64
@@ -36,6 +54,14 @@ type Result struct {
 
 // RunWorkflow executes a parsed workflow end-to-end.
 func (e *Engine) RunWorkflow(ctx context.Context, wf *Workflow, cfg RunConfig) (*Result, error) {
+	// Validate inputs at the engine boundary
+	if err := wf.Validate(); err != nil {
+		return nil, fmt.Errorf("invalid workflow: %w", err)
+	}
+	if cfg.BudgetUSD < 0 {
+		return nil, fmt.Errorf("invalid budget: %.2f (must be >= 0)", cfg.BudgetUSD)
+	}
+
 	session := &lib.Session{
 		ID:        lib.NewSessionID(),
 		Workflow:  strings.ToLower(wf.Name),
@@ -43,21 +69,21 @@ func (e *Engine) RunWorkflow(ctx context.Context, wf *Workflow, cfg RunConfig) (
 		Status:    "running",
 		BudgetUSD: cfg.BudgetUSD,
 	}
-	if err := e.DB.CreateSession(session); err != nil {
+	if err := e.db.CreateSession(session); err != nil {
 		return nil, fmt.Errorf("create session: %w", err)
 	}
-	if err := lib.EnsureSessionDir(e.RepoRoot, session.ID); err != nil {
+	if err := lib.EnsureSessionDir(e.repoRoot, session.ID); err != nil {
 		return nil, fmt.Errorf("create session dir: %w", err)
 	}
 
 	branchName := fmt.Sprintf("%s/%s", session.Workflow, session.ID)
-	if err := e.Git.CreateBranch(branchName); err != nil {
+	if err := e.git.CreateBranch(branchName); err != nil {
 		return nil, fmt.Errorf("create branch: %w", err)
 	}
 	fmt.Printf("%s session %s on branch %s\n\n", wf.Name, session.ID, branchName)
 
 	// Ensure scratchpad directory exists
-	scratchDir := filepath.Join(e.RepoRoot, ".devkit", "scratchpads")
+	scratchDir := filepath.Join(e.repoRoot, ".devkit", "scratchpads")
 	if err := os.MkdirAll(scratchDir, 0o755); err != nil {
 		return nil, fmt.Errorf("create scratchpad dir: %w", err)
 	}
@@ -68,7 +94,7 @@ func (e *Engine) RunWorkflow(ctx context.Context, wf *Workflow, cfg RunConfig) (
 	var iterNum int
 
 	opts := runners.RunOpts{
-		WorkDir:      e.RepoRoot,
+		WorkDir:      e.repoRoot,
 		AllowedTools: "Bash,Read,Edit,Write,Grep,Glob",
 		MaxTurns:     30,
 	}
@@ -116,7 +142,7 @@ func (e *Engine) RunWorkflow(ctx context.Context, wf *Workflow, cfg RunConfig) (
 		if step.Prompt == "" && len(step.Parallel) > 0 {
 			cost, err := e.runParallel(ctx, step, wf.Steps, stepIndex, session, cfg.Input, outputs, opts, &iterNum)
 			if err != nil {
-				e.DB.UpdateSessionStatus(session.ID, "failed")
+				e.db.UpdateSessionStatus(session.ID, "failed")
 				failed = true
 				stepErr = err
 				break
@@ -137,7 +163,7 @@ func (e *Engine) RunWorkflow(ctx context.Context, wf *Workflow, cfg RunConfig) (
 			// so we don't add the returned cost again here.
 			_, err := e.runLoop(ctx, step, session, cfg.Input, outputs, opts, &iterNum, overBudget, addCost)
 			if err != nil {
-				e.DB.UpdateSessionStatus(session.ID, "failed")
+				e.db.UpdateSessionStatus(session.ID, "failed")
 				failed = true
 				stepErr = err
 				break
@@ -163,7 +189,7 @@ func (e *Engine) RunWorkflow(ctx context.Context, wf *Workflow, cfg RunConfig) (
 		} else {
 			cost, output, err := e.runStep(ctx, step, session, cfg.Input, outputs, opts, &iterNum)
 			if err != nil {
-				e.DB.UpdateSessionStatus(session.ID, "failed")
+				e.db.UpdateSessionStatus(session.ID, "failed")
 				failed = true
 				stepErr = err
 				break
@@ -196,13 +222,13 @@ func (e *Engine) RunWorkflow(ctx context.Context, wf *Workflow, cfg RunConfig) (
 
 	// Only mark done on clean exit (fix #3: don't overwrite "failed")
 	if !failed && ctx.Err() == nil {
-		e.Git.CommitAll(fmt.Sprintf("%s(%s): complete", session.Workflow, session.ID))
-		e.DB.UpdateSessionStatus(session.ID, "done")
+		e.git.CommitAll(fmt.Sprintf("%s(%s): complete", session.Workflow, session.ID))
+		e.db.UpdateSessionStatus(session.ID, "done")
 	} else if !failed {
-		e.DB.UpdateSessionStatus(session.ID, "cancelled")
+		e.db.UpdateSessionStatus(session.ID, "cancelled")
 	}
 
-	allSteps, _ := e.DB.GetSteps(session.ID)
+	allSteps, _ := e.db.GetSteps(session.ID)
 	stopReason := "completed"
 	if failed {
 		stopReason = "failed"
@@ -211,7 +237,7 @@ func (e *Engine) RunWorkflow(ctx context.Context, wf *Workflow, cfg RunConfig) (
 	} else if overBudget() {
 		stopReason = "budget_exhausted"
 	}
-	lib.WriteReport(e.RepoRoot, session, allSteps, stopReason)
+	lib.WriteReport(e.repoRoot, session, allSteps, stopReason)
 
 	return &Result{
 		Session:  session,
@@ -231,15 +257,15 @@ func (e *Engine) runStep(ctx context.Context, step *WfStep, session *lib.Session
 		SessionID: session.ID,
 		Iteration: *iterNum,
 		Status:    "running",
-		AgentName: e.Runner.Name(),
+		AgentName: e.runner.Name(),
 	}
-	e.DB.CreateStep(dbStep)
+	e.db.CreateStep(dbStep)
 
-	result, err := e.Runner.Run(ctx, prompt, opts)
+	result, err := e.runner.Run(ctx, prompt, opts)
 	if err != nil {
 		dbStep.Status = "failed"
 		dbStep.ChangeSummary = err.Error()
-		e.DB.UpdateStep(dbStep)
+		e.db.UpdateStep(dbStep)
 		return 0, "", fmt.Errorf("step %s failed: %w", step.ID, err)
 	}
 
@@ -247,7 +273,7 @@ func (e *Engine) runStep(ctx context.Context, step *WfStep, session *lib.Session
 	dbStep.Kept = true
 	dbStep.CostUSD = result.CostUSD
 	dbStep.ChangeSummary = runners.TruncStr(result.Output, 200)
-	e.DB.UpdateStep(dbStep)
+	e.db.UpdateStep(dbStep)
 	fmt.Printf("  done ($%.4f)\n\n", result.CostUSD)
 
 	return result.CostUSD, result.Output, nil
@@ -282,15 +308,15 @@ func (e *Engine) runLoop(ctx context.Context, step *WfStep, session *lib.Session
 			SessionID: session.ID,
 			Iteration: *iterNum,
 			Status:    "running",
-			AgentName: e.Runner.Name(),
+			AgentName: e.runner.Name(),
 		}
-		e.DB.CreateStep(dbStep)
+		e.db.CreateStep(dbStep)
 
-		result, err := e.Runner.Run(ctx, prompt, opts)
+		result, err := e.runner.Run(ctx, prompt, opts)
 		if err != nil {
 			dbStep.Status = "failed"
 			dbStep.ChangeSummary = err.Error()
-			e.DB.UpdateStep(dbStep)
+			e.db.UpdateStep(dbStep)
 			consecutiveFailures++
 			fmt.Printf("  failed, retrying\n\n")
 			continue
@@ -306,13 +332,13 @@ func (e *Engine) runLoop(ctx context.Context, step *WfStep, session *lib.Session
 		dbStep.Kept = true
 		dbStep.CostUSD = result.CostUSD
 		dbStep.ChangeSummary = runners.TruncStr(result.Output, 200)
-		e.DB.UpdateStep(dbStep)
+		e.db.UpdateStep(dbStep)
 
 		outputs[step.ID] = result.Output
 		fmt.Printf("  done ($%.4f)\n\n", result.CostUSD)
 
 		// Commit after each loop iteration
-		e.Git.CommitAll(fmt.Sprintf("%s: %s iteration %d", session.Workflow, step.ID, attempt))
+		e.git.CommitAll(fmt.Sprintf("%s: %s iteration %d", session.Workflow, step.ID, attempt))
 
 		// Check until condition
 		if step.Loop.Until != "" && strings.Contains(strings.ToUpper(result.Output), strings.ToUpper(step.Loop.Until)) {
@@ -372,14 +398,14 @@ func (e *Engine) runParallel(ctx context.Context, dispatcher *WfStep, allSteps [
 				SessionID: session.ID,
 				Iteration: myIter,
 				Status:    "running",
-				AgentName: e.Runner.Name(),
+				AgentName: e.runner.Name(),
 			}
 
 			mu.Lock()
-			e.DB.CreateStep(dbStep)
+			e.db.CreateStep(dbStep)
 			mu.Unlock()
 
-			result, err := e.Runner.Run(ctx, prompt, opts)
+			result, err := e.runner.Run(ctx, prompt, opts)
 
 			mu.Lock()
 			defer mu.Unlock()
@@ -387,7 +413,7 @@ func (e *Engine) runParallel(ctx context.Context, dispatcher *WfStep, allSteps [
 			if err != nil {
 				dbStep.Status = "failed"
 				dbStep.ChangeSummary = err.Error()
-				e.DB.UpdateStep(dbStep)
+				e.db.UpdateStep(dbStep)
 				results[j] = parallelResult{id: pid, err: err}
 				return
 			}
@@ -396,7 +422,7 @@ func (e *Engine) runParallel(ctx context.Context, dispatcher *WfStep, allSteps [
 			dbStep.Kept = true
 			dbStep.CostUSD = result.CostUSD
 			dbStep.ChangeSummary = runners.TruncStr(result.Output, 200)
-			e.DB.UpdateStep(dbStep)
+			e.db.UpdateStep(dbStep)
 
 			results[j] = parallelResult{id: pid, output: result.Output, cost: result.CostUSD}
 		}(j, step, pid)
diff --git a/src/engine/engine_test.go b/src/engine/engine_test.go
index be2c16d..42f0b7e 100644
--- a/src/engine/engine_test.go
+++ b/src/engine/engine_test.go
@@ -96,6 +96,15 @@ func result(output string) runners.RunResult {
 	return runners.RunResult{Output: output, CostUSD: 0.01}
 }
 
+func mustEngine(t *testing.T, db *lib.DB, git *lib.Git, runner runners.Runner, repoRoot string) *Engine {
+	t.Helper()
+	eng, err := NewEngine(db, git, runner, repoRoot)
+	if err != nil {
+		t.Fatalf("NewEngine: %v", err)
+	}
+	return eng
+}
+
 // ---------------------------------------------------------------------------
 // Parse tests
 // ---------------------------------------------------------------------------
@@ -200,6 +209,23 @@ steps:
   - id: a
     prompt: x
     branch: [{when: "x", goto: missing}]`, "branch target"},
+		{"negative budget", `name: T
+budget: {limit: -100}
+steps: [{id: a, prompt: x}]`, "negative budget"},
+		{"parallel with prompt", `name: T
+steps:
+  - id: a
+    prompt: "do something"
+    parallel: [b]
+  - id: b
+    prompt: "other"`, "mutually exclusive"},
+		{"parallel with loop", `name: T
+steps:
+  - id: a
+    parallel: [b]
+    loop: {max: 3, until: DONE}
+  - id: b
+    prompt: "other"`, "mutually exclusive"},
 	}
 
 	for _, tt := range tests {
@@ -313,6 +339,44 @@ func TestEvalBranchFirstMatchWins(t *testing.T) {
 // Engine execution tests
 // ---------------------------------------------------------------------------
 
+func TestNewEngineValidation(t *testing.T) {
+	db := tempDB(t)
+	dir, git := initGitRepo(t)
+	runner := newMockRunner(nil, nil)
+
+	if _, err := NewEngine(nil, git, runner, dir); err == nil {
+		t.Error("expected error for nil db")
+	}
+	if _, err := NewEngine(db, nil, runner, dir); err == nil {
+		t.Error("expected error for nil git")
+	}
+	if _, err := NewEngine(db, git, nil, dir); err == nil {
+		t.Error("expected error for nil runner")
+	}
+	if _, err := NewEngine(db, git, runner, ""); err == nil {
+		t.Error("expected error for empty repoRoot")
+	}
+	if _, err := NewEngine(db, git, runner, dir); err != nil {
+		t.Errorf("valid args should succeed: %v", err)
+	}
+}
+
+func TestRunWorkflowNegativeBudget(t *testing.T) {
+	db := tempDB(t)
+	dir, git := initGitRepo(t)
+	runner := newMockRunner([]runners.RunResult{result("ok")}, nil)
+	eng := mustEngine(t, db, git, runner, dir)
+
+	wf := &Workflow{Name: "test", Steps: []WfStep{{ID: "s1", Prompt: "Do"}}}
+	_, err := eng.RunWorkflow(context.Background(), wf, RunConfig{Input: "test", BudgetUSD: -1.0})
+	if err == nil {
+		t.Fatal("expected error for negative budget")
+	}
+	if !strings.Contains(err.Error(), "invalid budget") {
+		t.Errorf("unexpected error: %v", err)
+	}
+}
+
 func TestRunWorkflowSimple(t *testing.T) {
 	db := tempDB(t)
 	dir, git := initGitRepo(t)
@@ -322,7 +386,7 @@ func TestRunWorkflowSimple(t *testing.T) {
 		result("implemented A and B"),
 	}, nil)
 
-	eng := &Engine{DB: db, Git: git, Runner: runner, RepoRoot: dir}
+	eng := mustEngine(t, db, git, runner, dir)
 	wf := &Workflow{
 		Name: "test",
 		Steps: []WfStep{
@@ -360,7 +424,7 @@ func TestRunWorkflowBranch(t *testing.T) {
 		result("fixed the typo"),     // quick-fix output
 	}, nil)
 
-	eng := &Engine{DB: db, Git: git, Runner: runner, RepoRoot: dir}
+	eng := mustEngine(t, db, git, runner, dir)
 	wf := &Workflow{
 		Name: "test",
 		Steps: []WfStep{
@@ -400,7 +464,7 @@ func TestRunWorkflowLoop(t *testing.T) {
 		result("attempt 2: ALL_PASSING"),
 	}, nil)
 
-	eng := &Engine{DB: db, Git: git, Runner: runner, RepoRoot: dir}
+	eng := mustEngine(t, db, git, runner, dir)
 	wf := &Workflow{
 		Name: "test",
 		Steps: []WfStep{
@@ -432,7 +496,7 @@ func TestRunWorkflowLoopMaxIterations(t *testing.T) {
 		result("still broken"),
 	}, nil)
 
-	eng := &Engine{DB: db, Git: git, Runner: runner, RepoRoot: dir}
+	eng := mustEngine(t, db, git, runner, dir)
 	wf := &Workflow{
 		Name: "test",
 		Steps: []WfStep{
@@ -460,7 +524,7 @@ func TestRunWorkflowBudget(t *testing.T) {
 		{Output: "step 3", CostUSD: 0.50}, // should not be reached
 	}, nil)
 
-	eng := &Engine{DB: db, Git: git, Runner: runner, RepoRoot: dir}
+	eng := mustEngine(t, db, git, runner, dir)
 	wf := &Workflow{
 		Name: "test",
 		Steps: []WfStep{
@@ -493,7 +557,7 @@ func TestRunWorkflowParallel(t *testing.T) {
 		result("review B findings"),
 	}, nil)
 
-	eng := &Engine{DB: db, Git: git, Runner: runner, RepoRoot: dir}
+	eng := mustEngine(t, db, git, runner, dir)
 	wf := &Workflow{
 		Name: "test",
 		Steps: []WfStep{
@@ -527,7 +591,7 @@ func TestRunWorkflowContextCancelled(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	cancel() // cancel immediately
 
-	eng := &Engine{DB: db, Git: git, Runner: runner, RepoRoot: dir}
+	eng := mustEngine(t, db, git, runner, dir)
 	wf := &Workflow{
 		Name: "test",
 		Steps: []WfStep{
@@ -553,7 +617,7 @@ func TestRunWorkflowLoopAllFail(t *testing.T) {
 		fmt.Errorf("runner error 2"),
 	})
 
-	eng := &Engine{DB: db, Git: git, Runner: runner, RepoRoot: dir}
+	eng := mustEngine(t, db, git, runner, dir)
 	wf := &Workflow{
 		Name: "test",
 		Steps: []WfStep{
@@ -581,7 +645,7 @@ func TestRunWorkflowBranchCycleLimit(t *testing.T) {
 	}
 	runner := newMockRunner(responses, nil)
 
-	eng := &Engine{DB: db, Git: git, Runner: runner, RepoRoot: dir}
+	eng := mustEngine(t, db, git, runner, dir)
 	wf := &Workflow{
 		Name: "test",
 		Steps: []WfStep{
@@ -609,7 +673,7 @@ func TestRunWorkflowStepFailure(t *testing.T) {
 		[]error{nil, fmt.Errorf("implement failed")},
 	)
 
-	eng := &Engine{DB: db, Git: git, Runner: runner, RepoRoot: dir}
+	eng := mustEngine(t, db, git, runner, dir)
 	wf := &Workflow{
 		Name: "test",
 		Steps: []WfStep{
@@ -637,7 +701,7 @@ func TestRunWorkflowParallelPartialFailure(t *testing.T) {
 		[]error{nil, fmt.Errorf("review crashed")},
 	)
 
-	eng := &Engine{DB: db, Git: git, Runner: runner, RepoRoot: dir}
+	eng := mustEngine(t, db, git, runner, dir)
 	wf := &Workflow{
 		Name: "test",
 		Steps: []WfStep{
@@ -667,7 +731,7 @@ func TestRunWorkflowParallelAllFail(t *testing.T) {
 		fmt.Errorf("review B failed"),
 	})
 
-	eng := &Engine{DB: db, Git: git, Runner: runner, RepoRoot: dir}
+	eng := mustEngine(t, db, git, runner, dir)
 	wf := &Workflow{
 		Name: "test",
 		Steps: []WfStep{
@@ -697,7 +761,7 @@ func TestRunWorkflowBudgetInLoop(t *testing.T) {
 	}
 	runner := newMockRunner(responses, nil)
 
-	eng := &Engine{DB: db, Git: git, Runner: runner, RepoRoot: dir}
+	eng := mustEngine(t, db, git, runner, dir)
 	wf := &Workflow{
 		Name: "test",
 		Steps: []WfStep{
diff --git a/src/engine/workflow.go b/src/engine/workflow.go
index 065dbc7..32bc90c 100644
--- a/src/engine/workflow.go
+++ b/src/engine/workflow.go
@@ -77,6 +77,11 @@ func validate(wf *Workflow) error {
 		return fmt.Errorf("workflow %q has no steps", wf.Name)
 	}
 
+	// Validate budget
+	if wf.Budget.Limit < 0 {
+		return fmt.Errorf("workflow %q has negative budget limit", wf.Name)
+	}
+
 	ids := make(map[string]bool)
 	for _, s := range wf.Steps {
 		if s.ID == "" {
@@ -86,6 +91,14 @@ func validate(wf *Workflow) error {
 			return fmt.Errorf("duplicate step id %q in workflow %q", s.ID, wf.Name)
 		}
 		ids[s.ID] = true
+
+		// Validate step mode mutual exclusion
+		if len(s.Parallel) > 0 && s.Prompt != "" {
+			return fmt.Errorf("step %q has both parallel and prompt — these are mutually exclusive", s.ID)
+		}
+		if len(s.Parallel) > 0 && s.Loop != nil {
+			return fmt.Errorf("step %q has both parallel and loop — these are mutually exclusive", s.ID)
+		}
 	}
 
 	// Validate branch targets exist
@@ -106,6 +119,12 @@ func validate(wf *Workflow) error {
 	return nil
 }
 
+// Validate re-validates a workflow that may have been constructed directly
+// (not via Parse). Call this at the engine boundary for safety.
+func (wf *Workflow) Validate() error {
+	return validate(wf)
+}
+
 // Interpolate replaces {{step-id}} and {{input}} placeholders in a prompt.
 func Interpolate(prompt string, input string, outputs map[string]string) string {
 	result := strings.ReplaceAll(prompt, "{{input}}", input)

From 12e00a1fce063f15432c29f355f632bc69771537 Mon Sep 17 00:00:00 2001
From: Tym Rabchuk <tymrabchuk@gmail.com>
Date: Sat, 4 Apr 2026 18:32:49 -0400
Subject: [PATCH 09/11] Fix gofmt formatting in engine_test.go

---
 src/engine/engine_test.go | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/engine/engine_test.go b/src/engine/engine_test.go
index 42f0b7e..3c92d24 100644
--- a/src/engine/engine_test.go
+++ b/src/engine/engine_test.go
@@ -310,7 +310,7 @@ func TestEvalBranch(t *testing.T) {
 		want   string
 	}{
 		{"TINY: just a typo fix", "quick"},
-		{"tiny change", "quick"},      // case insensitive
+		{"tiny change", "quick"}, // case insensitive
 		{"SMALL: one function", "plan"},
 		{"MEDIUM: multiple files", ""}, // no match
 		{"LARGE: new subsystem", ""},
@@ -420,8 +420,8 @@ func TestRunWorkflowBranch(t *testing.T) {
 	dir, git := initGitRepo(t)
 
 	runner := newMockRunner([]runners.RunResult{
-		result("TINY: just a typo"),  // triage output
-		result("fixed the typo"),     // quick-fix output
+		result("TINY: just a typo"), // triage output
+		result("fixed the typo"),    // quick-fix output
 	}, nil)
 
 	eng := mustEngine(t, db, git, runner, dir)

From bbc4e84cdc4ea6f90db1d61cc9a9c38daeeef30e Mon Sep 17 00:00:00 2001
From: Tym Rabchuk <tymrabchuk@gmail.com>
Date: Sat, 4 Apr 2026 18:42:02 -0400
Subject: [PATCH 10/11] Extract evalBranch closure to eliminate branch
 evaluation duplication

The branch evaluation logic was duplicated verbatim between the loop
and regular step paths. Extracted into a local evalBranch closure that
returns (stepIndex, error), called once after both paths. Removes ~20
lines of duplication while keeping identical behavior.

Found by code-simplifier agent.
---
 src/engine/engine.go | 66 ++++++++++++++++++++++----------------------
 1 file changed, 33 insertions(+), 33 deletions(-)

diff --git a/src/engine/engine.go b/src/engine/engine.go
index 4ce5ee3..a52302c 100644
--- a/src/engine/engine.go
+++ b/src/engine/engine.go
@@ -120,6 +120,28 @@ func (e *Engine) RunWorkflow(ctx context.Context, wf *Workflow, cfg RunConfig) (
 	branchCount := 0
 	const maxBranches = 100
 
+	// evalBranch checks branch conditions and returns the next step index, or -1 for fall-through.
+	evalBranch := func(step *WfStep) (int, error) {
+		if len(step.Branch) == 0 {
+			return -1, nil
+		}
+		output, ok := outputs[step.ID]
+		if !ok {
+			return -1, nil
+		}
+		target := EvalBranch(output, step.Branch)
+		if target == "" {
+			return -1, nil
+		}
+		branchCount++
+		if branchCount > maxBranches {
+			fmt.Println("  → branch limit reached, stopping")
+			return -1, fmt.Errorf("branch limit exceeded (%d jumps)", maxBranches)
+		}
+		fmt.Printf("  → branching to %s\n\n", target)
+		return stepIndex[target], nil
+	}
+
 	i := 0
 	for i < len(wf.Steps) {
 		if ctx.Err() != nil {
@@ -168,24 +190,6 @@ func (e *Engine) RunWorkflow(ctx context.Context, wf *Workflow, cfg RunConfig) (
 				stepErr = err
 				break
 			}
-
-			// Evaluate branch after loop (fix #10: branches on loop steps)
-			if len(step.Branch) > 0 {
-				if output, ok := outputs[step.ID]; ok {
-					if target := EvalBranch(output, step.Branch); target != "" {
-						branchCount++
-						if branchCount > maxBranches {
-							fmt.Println("  → branch limit reached, stopping")
-							failed = true
-							stepErr = fmt.Errorf("branch limit exceeded (%d jumps)", maxBranches)
-							break
-						}
-						fmt.Printf("  → branching to %s\n\n", target)
-						i = stepIndex[target]
-						continue
-					}
-				}
-			}
 		} else {
 			cost, output, err := e.runStep(ctx, step, session, cfg.Input, outputs, opts, &iterNum)
 			if err != nil {
@@ -196,22 +200,18 @@ func (e *Engine) RunWorkflow(ctx context.Context, wf *Workflow, cfg RunConfig) (
 			}
 			totalUSD += cost
 			outputs[step.ID] = output
+		}
 
-			// Evaluate branch
-			if len(step.Branch) > 0 {
-				if target := EvalBranch(output, step.Branch); target != "" {
-					branchCount++
-					if branchCount > maxBranches {
-						fmt.Println("  → branch limit reached, stopping")
-						failed = true
-						stepErr = fmt.Errorf("branch limit exceeded (%d jumps)", maxBranches)
-						break
-					}
-					fmt.Printf("  → branching to %s\n\n", target)
-					i = stepIndex[target]
-					continue
-				}
-			}
+		// Evaluate branch (applies to both loop and regular steps)
+		jump, err := evalBranch(step)
+		if err != nil {
+			failed = true
+			stepErr = err
+			break
+		}
+		if jump >= 0 {
+			i = jump
+			continue
 		}
 
 		i++

From a55310a3afd8018bdac3e7146f11828b5d98e207 Mon Sep 17 00:00:00 2001
From: Tym Rabchuk <tymrabchuk@gmail.com>
Date: Sat, 4 Apr 2026 18:43:01 -0400
Subject: [PATCH 11/11] Fix final review findings in hook scripts

go-vet-stop.sh:
- Replace `timeout` (GNU-only) with `perl -e 'alarm 60; exec @ARGV'`
  for macOS compatibility. Ironic that the shell-compat hook exists
  to catch exactly this.

shell-compat.sh:
- Fix session dedup: use PPID instead of $$ (each hook invocation
  is a new process, so $$ gives a unique PID every time, defeating
  the dedup mechanism).

go-nil-return.sh:
- Remove dead gsub code on function declaration line.
- Properly count all braces on the declaration line (handles inline
  struct literals) instead of hardcoding brace_depth=1.
---
 hooks/go-nil-return.sh | 11 ++++++++---
 hooks/go-vet-stop.sh   |  3 ++-
 hooks/shell-compat.sh  |  4 ++--
 3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/hooks/go-nil-return.sh b/hooks/go-nil-return.sh
index 1376382..f46a197 100755
--- a/hooks/go-nil-return.sh
+++ b/hooks/go-nil-return.sh
@@ -34,11 +34,16 @@ WARNINGS=$(echo "$CONTENT" | awk '
     fname = $0
     sub(/\{.*/, "", fname)
     in_func = 1
-    brace_depth = 1
+    brace_depth = 0
     has_return = 0
     has_non_nil_err = 0
-    # Count opening brace
-    gsub(/[^{]/, "", $0); gsub(/[^}]/, "", tmp=$0)
+    # Count all braces on the declaration line itself
+    line = $0
+    for (j = 1; j <= length(line); j++) {
+      c = substr(line, j, 1)
+      if (c == "{") brace_depth++
+      if (c == "}") brace_depth--
+    }
     next
   }
 
diff --git a/hooks/go-vet-stop.sh b/hooks/go-vet-stop.sh
index cddfea4..189a442 100755
--- a/hooks/go-vet-stop.sh
+++ b/hooks/go-vet-stop.sh
@@ -62,7 +62,8 @@ done <<< "$GO_CHANGES"
 PACKAGES=$(echo "$PACKAGES" | tr ' ' '\n' | sort -u | tr '\n' ' ')
 
 if [ -n "$PACKAGES" ]; then
-  RACE_OUTPUT=$(cd "$GO_MOD_DIR" && timeout 60 go test -race -count=1 $PACKAGES 2>&1) || RACE_EXIT=$?
+  # Use perl alarm for POSIX-compatible timeout (macOS has no `timeout` command)
+  RACE_OUTPUT=$(cd "$GO_MOD_DIR" && perl -e 'alarm 60; exec @ARGV' -- go test -race -count=1 $PACKAGES 2>&1) || RACE_EXIT=$?
   if [ "${RACE_EXIT:-0}" -ne 0 ]; then
     # Check if it's specifically a race condition
     if echo "$RACE_OUTPUT" | grep -qE 'DATA RACE|race detected'; then
diff --git a/hooks/shell-compat.sh b/hooks/shell-compat.sh
index 831a0b5..efebcb0 100755
--- a/hooks/shell-compat.sh
+++ b/hooks/shell-compat.sh
@@ -32,8 +32,8 @@ fi
 
 [ -z "$CONTENT" ] && exit 0
 
-# Session dedup
-SEEN_FILE="/tmp/devkit-shellcompat-seen-$$"
+# Session dedup — use PPID (stable across hook invocations within one Claude session)
+SEEN_FILE="/tmp/devkit-shellcompat-seen-${PPID:-0}"
 
 check_compat() {
   local pattern="$1"