diff --git a/skills/creating-workflows/SKILL.md b/skills/creating-workflows/SKILL.md index 615a73d..058474b 100644 --- a/skills/creating-workflows/SKILL.md +++ b/skills/creating-workflows/SKILL.md @@ -18,11 +18,13 @@ budget: # Optional: token budget steps: - id: string # Unique step identifier model: string # Model tier: "smart", "general", "fast" - prompt: string # Instruction (supports ${{variable}} interpolation) + prompt: string # Instruction (supports {{variable}} interpolation) + command: string # Shell command — runs directly, no LLM (mutually exclusive with prompt) parallel: [ids] # Optional: run these step IDs concurrently loop: # Optional: repeat this step max: number # Maximum iterations until: string # Stop condition (string match in output) + gate: string # Shell command run after each iteration — exit 0 keeps, non-zero reverts branch: # Optional: conditional execution if: string # Condition expression then: string # Step id to jump to if true @@ -33,9 +35,11 @@ steps: - **id** — Must be unique. Used for branch targets and output references. - **model** — Which model tier runs this step. Pick based on task complexity. -- **prompt** — The instruction. Use `${{steps.previous_id.output}}` to reference earlier outputs. Use `${{input.field}}` for workflow inputs. +- **prompt** — The instruction. Use `{{step-id}}` to reference earlier outputs. Use `{{input}}` for workflow input. Mutually exclusive with `command`. +- **command** — Shell command run directly (no LLM). Output is captured and available via `{{step-id}}`. Costs $0. Mutually exclusive with `prompt`. - **parallel** — Lists step IDs to run concurrently. Results collected before next sequential step. -- **loop** — Repeats with `max` iterations. Exits early if output contains `until` string. +- **loop** — Repeats with `max` iterations. Exits early if output contains `until` string. Optional `gate` command enforces quality after each iteration. +- **loop.gate** — Shell command run after each loop iteration. Exit 0 = keep changes and commit. Non-zero = revert changes via `git checkout`. 3 consecutive gate failures trigger stuck detection and stop the loop. - **branch** — Routes execution conditionally. Both `then` and `else` reference step `id`s. ## Minimal Example @@ -58,6 +62,35 @@ steps: Original: ${{input.document}} ``` +## Command + Gate Example + +```yaml +name: lint-and-fix +description: Deterministic lint loop with gate enforcement +steps: + - id: baseline + command: "eslint src/ 2>&1 || true" + + - id: fix + model: smart + prompt: | + Lint output: {{baseline}} + Fix ONE group of related issues. + loop: + max: 10 + until: "exit code: 0" + gate: "eslint src/" + + - id: report + command: "echo 'Lint session complete'" +``` + +Key behaviors: +- `command` steps run shell commands directly — no LLM tokens spent. +- `gate` runs after each loop iteration. Exit 0 keeps changes, non-zero reverts via git. +- 3 consecutive gate failures stop the loop (stuck detection). +- Command output includes `exit code: N` for downstream branching. + ## Tips - Keep prompts focused. One task per step. diff --git a/src/engine/engine.go b/src/engine/engine.go index a52302c..b784ca5 100644 --- a/src/engine/engine.go +++ b/src/engine/engine.go @@ -1,9 +1,12 @@ package engine import ( + "bytes" "context" + "errors" "fmt" "os" + "os/exec" "path/filepath" "strings" "sync" @@ -174,8 +177,8 @@ func (e *Engine) RunWorkflow(ctx context.Context, wf *Workflow, cfg RunConfig) ( continue } - // Skip empty steps - if step.Prompt == "" { + // Skip empty steps (no prompt, no command) + if step.Prompt == "" && step.Command == "" { i++ continue } @@ -247,9 +250,66 @@ func (e *Engine) RunWorkflow(ctx context.Context, wf *Workflow, cfg RunConfig) ( }, stepErr } +// runCommand executes a shell command and returns its combined output. +func (e *Engine) runCommand(ctx context.Context, command string) (string, int, error) { + cmd := exec.CommandContext(ctx, "sh", "-c", command) + cmd.Dir = e.repoRoot + + var out bytes.Buffer + cmd.Stdout = &out + cmd.Stderr = &out + + err := cmd.Run() + exitCode := 0 + if err != nil { + var exitErr *exec.ExitError + if errors.As(err, &exitErr) { + exitCode = exitErr.ExitCode() + } else { + return "", 1, fmt.Errorf("command execution failed: %w", err) + } + } + return out.String(), exitCode, 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++ + + // Command step: run shell command directly, no LLM cost. + if step.Command != "" { + command := Interpolate(step.Command, input, outputs) + fmt.Printf("--- %s (step %d, command) ---\n", step.ID, *iterNum) + + dbStep := &lib.Step{ + SessionID: session.ID, + Iteration: *iterNum, + Status: "running", + AgentName: "shell", + } + e.db.CreateStep(dbStep) + + output, exitCode, err := e.runCommand(ctx, command) + if err != nil { + dbStep.Status = "failed" + dbStep.ChangeSummary = err.Error() + e.db.UpdateStep(dbStep) + return 0, "", fmt.Errorf("step %s command failed: %w", step.ID, err) + } + + // Include exit code in output so downstream steps can check it + fullOutput := fmt.Sprintf("%s\nexit code: %d", strings.TrimRight(output, "\n"), exitCode) + + dbStep.Status = "kept" + dbStep.Kept = true + dbStep.ChangeSummary = runners.TruncStr(fullOutput, 200) + e.db.UpdateStep(dbStep) + fmt.Printf(" done (exit %d)\n\n", exitCode) + + return 0, fullOutput, nil + } + + // Prompt step: run through LLM runner. fmt.Printf("--- %s (step %d) ---\n", step.ID, *iterNum) prompt := Interpolate(step.Prompt, input, outputs) @@ -281,6 +341,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. Respects budget via overBudget, reports cost via addCost. +// If a gate command is set, it runs after each iteration — non-zero exit reverts the iteration. 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 @@ -290,6 +351,7 @@ func (e *Engine) runLoop(ctx context.Context, step *WfStep, session *lib.Session succeeded := false consecutiveFailures := 0 + stuckDetected := false for attempt := 1; attempt <= maxIter; attempt++ { if ctx.Err() != nil { @@ -322,23 +384,77 @@ func (e *Engine) runLoop(ctx context.Context, step *WfStep, session *lib.Session continue } + iterCost := result.CostUSD + + // Gate check: run shell command, revert if non-zero exit. + if step.Loop.Gate != "" { + gateCmd := Interpolate(step.Loop.Gate, input, outputs) + fmt.Printf(" gate: %s\n", runners.TruncStr(gateCmd, 80)) + _, exitCode, gateErr := e.runCommand(ctx, gateCmd) + + // Distinguish "gate couldn't execute" from "gate ran and returned non-zero". + // A startup/context error is fatal — the gate never validated anything. + if gateErr != nil { + dbStep.Status = "failed" + dbStep.CostUSD = iterCost + dbStep.ChangeSummary = fmt.Sprintf("gate error: %s", gateErr) + e.db.UpdateStep(dbStep) + totalCost += iterCost + if addCost != nil { + addCost(iterCost) + } + return totalCost, fmt.Errorf("loop %s: gate command failed: %w", step.ID, gateErr) + } + + if exitCode != 0 { + reason := fmt.Sprintf("gate failed (exit %d)", exitCode) + fmt.Printf(" → %s, reverting iteration\n\n", reason) + if revertErr := e.git.RevertAll(); revertErr != nil { + fmt.Printf(" → revert failed: %s\n", revertErr) + dbStep.Status = "failed" + dbStep.ChangeSummary = fmt.Sprintf("%s; revert failed: %s", reason, revertErr) + e.db.UpdateStep(dbStep) + return totalCost, fmt.Errorf("loop %s: revert failed after gate failure: %w", step.ID, revertErr) + } + dbStep.Status = "reverted" + dbStep.Kept = false + dbStep.CostUSD = iterCost + dbStep.ChangeSummary = reason + e.db.UpdateStep(dbStep) + consecutiveFailures++ + totalCost += iterCost + if addCost != nil { + addCost(iterCost) + } + if consecutiveFailures >= 3 { + fmt.Printf(" → 3 consecutive gate failures, stopping loop\n\n") + stuckDetected = true + break + } + continue + } + fmt.Printf(" → gate passed\n") + } + consecutiveFailures = 0 succeeded = true - totalCost += result.CostUSD + totalCost += iterCost if addCost != nil { - addCost(result.CostUSD) + addCost(iterCost) } dbStep.Status = "kept" dbStep.Kept = true - dbStep.CostUSD = result.CostUSD + dbStep.CostUSD = iterCost dbStep.ChangeSummary = runners.TruncStr(result.Output, 200) e.db.UpdateStep(dbStep) outputs[step.ID] = result.Output - fmt.Printf(" done ($%.4f)\n\n", result.CostUSD) + fmt.Printf(" done ($%.4f)\n\n", iterCost) - // Commit after each loop iteration - e.git.CommitAll(fmt.Sprintf("%s: %s iteration %d", session.Workflow, step.ID, attempt)) + // Commit after each successful loop iteration + if commitErr := e.git.CommitAll(fmt.Sprintf("%s: %s iteration %d", session.Workflow, step.ID, attempt)); commitErr != nil { + fmt.Printf(" → commit failed: %s\n", commitErr) + } // Check until condition if step.Loop.Until != "" && strings.Contains(strings.ToUpper(result.Output), strings.ToUpper(step.Loop.Until)) { @@ -348,6 +464,9 @@ func (e *Engine) runLoop(ctx context.Context, step *WfStep, session *lib.Session } if !succeeded { + if stuckDetected { + return totalCost, fmt.Errorf("loop %s: stuck after %d consecutive gate failures (ran %d of %d iterations)", step.ID, consecutiveFailures, consecutiveFailures, maxIter) + } return totalCost, fmt.Errorf("loop %s: all %d iterations failed", step.ID, maxIter) } diff --git a/src/engine/engine_test.go b/src/engine/engine_test.go index 3c92d24..fcdccc1 100644 --- a/src/engine/engine_test.go +++ b/src/engine/engine_test.go @@ -226,6 +226,23 @@ steps: loop: {max: 3, until: DONE} - id: b prompt: "other"`, "mutually exclusive"}, + {"command with prompt", `name: T +steps: + - id: a + command: "echo hi" + prompt: "do thing"`, "mutually exclusive"}, + {"parallel with command", `name: T +steps: + - id: a + command: "echo hi" + parallel: [b] + - id: b + prompt: "other"`, "mutually exclusive"}, + {"command with loop", `name: T +steps: + - id: a + command: "echo hi" + loop: {max: 3, until: DONE}`, "mutually exclusive"}, } for _, tt := range tests { @@ -784,6 +801,306 @@ func TestRunWorkflowBudgetInLoop(t *testing.T) { // Parse real workflow files // --------------------------------------------------------------------------- +// --------------------------------------------------------------------------- +// Command step tests +// --------------------------------------------------------------------------- + +func TestRunWorkflowCommandStep(t *testing.T) { + db := tempDB(t) + dir, git := initGitRepo(t) + + // Runner should NOT be called for command steps + runner := newMockRunner(nil, nil) + eng := mustEngine(t, db, git, runner, dir) + + wf := &Workflow{ + Name: "test", + Steps: []WfStep{ + {ID: "check", Command: "echo hello world"}, + }, + } + + res, err := eng.RunWorkflow(context.Background(), wf, RunConfig{Input: "test"}) + if err != nil { + t.Fatalf("RunWorkflow: %v", err) + } + + output, ok := res.Outputs["check"] + if !ok { + t.Fatal("command step output missing") + } + if !strings.Contains(output, "hello world") { + t.Errorf("output = %q, want it to contain 'hello world'", output) + } + if !strings.Contains(output, "exit code: 0") { + t.Errorf("output should contain exit code, got %q", output) + } + // No LLM cost for command steps + if res.TotalUSD != 0 { + t.Errorf("total cost = %f, want 0 for command-only workflow", res.TotalUSD) + } + // Runner should not have been called + if runner.callIdx != 0 { + t.Errorf("runner called %d times, want 0 for command step", runner.callIdx) + } +} + +func TestRunWorkflowCommandInterpolation(t *testing.T) { + db := tempDB(t) + dir, git := initGitRepo(t) + runner := newMockRunner(nil, nil) + eng := mustEngine(t, db, git, runner, dir) + + wf := &Workflow{ + Name: "test", + Steps: []WfStep{ + {ID: "greet", Command: "echo {{input}}"}, + }, + } + + res, err := eng.RunWorkflow(context.Background(), wf, RunConfig{Input: "howdy"}) + if err != nil { + t.Fatalf("RunWorkflow: %v", err) + } + if !strings.Contains(res.Outputs["greet"], "howdy") { + t.Errorf("input not interpolated in command output: %q", res.Outputs["greet"]) + } +} + +func TestRunWorkflowCommandChainedWithPrompt(t *testing.T) { + db := tempDB(t) + dir, git := initGitRepo(t) + + runner := newMockRunner([]runners.RunResult{ + result("analyzed: found 3 issues"), + }, nil) + eng := mustEngine(t, db, git, runner, dir) + + wf := &Workflow{ + Name: "test", + Steps: []WfStep{ + {ID: "lint", Command: "echo 'error: unused var x'"}, + {ID: "fix", Model: "smart", Prompt: "Fix these issues: {{lint}}"}, + }, + } + + res, err := eng.RunWorkflow(context.Background(), wf, RunConfig{Input: "test"}) + if err != nil { + t.Fatalf("RunWorkflow: %v", err) + } + + // Command output should be interpolated into the prompt + if !strings.Contains(runner.prompts[0], "unused var x") { + t.Errorf("command output not interpolated into prompt: %q", runner.prompts[0]) + } + // Only the prompt step should cost money + if res.TotalUSD != 0.01 { + t.Errorf("total cost = %f, want 0.01", res.TotalUSD) + } +} + +func TestRunWorkflowCommandNonZeroExit(t *testing.T) { + db := tempDB(t) + dir, git := initGitRepo(t) + runner := newMockRunner(nil, nil) + eng := mustEngine(t, db, git, runner, dir) + + wf := &Workflow{ + Name: "test", + Steps: []WfStep{ + {ID: "fail", Command: "echo 'errors found'; exit 1"}, + }, + } + + res, err := eng.RunWorkflow(context.Background(), wf, RunConfig{Input: "test"}) + if err != nil { + t.Fatalf("RunWorkflow: %v", err) + } + + // Non-zero exit should NOT be a fatal error — output is still captured + output := res.Outputs["fail"] + if !strings.Contains(output, "errors found") { + t.Errorf("output missing, got %q", output) + } + if !strings.Contains(output, "exit code: 1") { + t.Errorf("exit code not captured, got %q", output) + } +} + +// --------------------------------------------------------------------------- +// Gate tests +// --------------------------------------------------------------------------- + +func TestRunWorkflowLoopGatePass(t *testing.T) { + db := tempDB(t) + dir, git := initGitRepo(t) + + runner := newMockRunner([]runners.RunResult{ + result("fixed something ALL_DONE"), + }, nil) + eng := mustEngine(t, db, git, runner, dir) + + wf := &Workflow{ + Name: "test", + Steps: []WfStep{ + {ID: "fix", Model: "smart", Prompt: "Fix", Loop: &Loop{ + Max: 5, + Until: "ALL_DONE", + Gate: "true", // always passes + }}, + }, + } + + res, err := eng.RunWorkflow(context.Background(), wf, RunConfig{Input: "test"}) + if err != nil { + t.Fatalf("RunWorkflow: %v", err) + } + // Gate passes, until found on first iteration → 1 call + if runner.callIdx != 1 { + t.Errorf("runner called %d times, want 1", runner.callIdx) + } + if len(res.Steps) == 0 { + t.Fatal("expected at least one step") + } + if res.Steps[0].Status != "kept" { + t.Errorf("step status = %q, want kept", res.Steps[0].Status) + } +} + +func TestRunWorkflowLoopGateFail(t *testing.T) { + db := tempDB(t) + dir, git := initGitRepo(t) + + // 3 attempts, all gate-fail, then stuck detection kicks in + runner := newMockRunner([]runners.RunResult{ + result("attempt 1"), + result("attempt 2"), + result("attempt 3"), + }, nil) + eng := mustEngine(t, db, git, runner, dir) + + wf := &Workflow{ + Name: "test", + Steps: []WfStep{ + {ID: "fix", Model: "smart", Prompt: "Fix", Loop: &Loop{ + Max: 10, + Until: "DONE", + Gate: "false", // always fails + }}, + }, + } + + _, err := eng.RunWorkflow(context.Background(), wf, RunConfig{Input: "test"}) + // All iterations reverted → all failed + if err == nil { + t.Fatal("expected error when all iterations are gate-reverted") + } + // Should have stopped after 3 consecutive failures + if runner.callIdx != 3 { + t.Errorf("runner called %d times, want 3 (stuck detection)", runner.callIdx) + } +} + +func TestRunWorkflowLoopGateRecovery(t *testing.T) { + db := tempDB(t) + dir, git := initGitRepo(t) + + // First attempt fails gate, second passes and hits until + runner := newMockRunner([]runners.RunResult{ + result("attempt 1"), + result("attempt 2 ALL_DONE"), + }, nil) + eng := mustEngine(t, db, git, runner, dir) + + // Gate: exit 1 on first call, exit 0 on second. + // Counter file must be OUTSIDE the repo so git revert doesn't reset it. + counterDir := t.TempDir() + counterFile := filepath.Join(counterDir, "gate-counter") + if err := os.WriteFile(counterFile, []byte("0"), 0o644); err != nil { + t.Fatal(err) + } + gateScript := fmt.Sprintf( + `count=$(cat %q); count=$((count + 1)); printf '%%s' "$count" > %q; test "$count" -ge 2`, + counterFile, counterFile, + ) + + wf := &Workflow{ + Name: "test", + Steps: []WfStep{ + {ID: "fix", Model: "smart", Prompt: "Fix", Loop: &Loop{ + Max: 5, + Until: "ALL_DONE", + Gate: gateScript, + }}, + }, + } + + res, err := eng.RunWorkflow(context.Background(), wf, RunConfig{Input: "test"}) + if err != nil { + t.Fatalf("RunWorkflow: %v", err) + } + // 2 runner calls: first reverted, second kept + if runner.callIdx != 2 { + t.Errorf("runner called %d times, want 2", runner.callIdx) + } + + // Check that we have both reverted and kept steps + var reverted, kept int + for _, s := range res.Steps { + switch s.Status { + case "reverted": + reverted++ + case "kept": + kept++ + } + } + if reverted != 1 { + t.Errorf("reverted steps = %d, want 1", reverted) + } + if kept != 1 { + t.Errorf("kept steps = %d, want 1", kept) + } +} + +func TestParseCommandStep(t *testing.T) { + yaml := ` +name: CmdTest +description: test +steps: + - id: run + command: "echo hello" +` + wf, err := Parse([]byte(yaml)) + if err != nil { + t.Fatalf("parse: %v", err) + } + if wf.Steps[0].Command != "echo hello" { + t.Errorf("command = %q, want 'echo hello'", wf.Steps[0].Command) + } +} + +func TestParseLoopGate(t *testing.T) { + yaml := ` +name: GateTest +description: test +steps: + - id: fix + model: smart + prompt: "Fix" + loop: + max: 5 + until: DONE + gate: "npm test" +` + wf, err := Parse([]byte(yaml)) + if err != nil { + t.Fatalf("parse: %v", err) + } + if wf.Steps[0].Loop.Gate != "npm test" { + t.Errorf("gate = %q, want 'npm test'", wf.Steps[0].Loop.Gate) + } +} + func TestParseRealWorkflows(t *testing.T) { workflowDir := filepath.Join("..", "..", "workflows") entries, err := os.ReadDir(workflowDir) diff --git a/src/engine/workflow.go b/src/engine/workflow.go index 32bc90c..af32d6e 100644 --- a/src/engine/workflow.go +++ b/src/engine/workflow.go @@ -30,6 +30,7 @@ type WfStep struct { ID string `yaml:"id"` Model string `yaml:"model"` Prompt string `yaml:"prompt"` + Command string `yaml:"command"` Parallel []string `yaml:"parallel"` Loop *Loop `yaml:"loop"` Branch []Branch `yaml:"branch"` @@ -39,6 +40,7 @@ type WfStep struct { type Loop struct { Max int `yaml:"max"` Until string `yaml:"until"` + Gate string `yaml:"gate"` } // Branch routes execution based on step output. @@ -93,8 +95,14 @@ func validate(wf *Workflow) error { 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 s.Command != "" && s.Prompt != "" { + return fmt.Errorf("step %q has both command and prompt — these are mutually exclusive", s.ID) + } + if len(s.Parallel) > 0 && (s.Prompt != "" || s.Command != "") { + return fmt.Errorf("step %q has both parallel and prompt/command — these are mutually exclusive", s.ID) + } + if s.Command != "" && s.Loop != nil { + return fmt.Errorf("step %q has both command and loop — 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) diff --git a/workflows/self-lint.yml b/workflows/self-lint.yml index 4e41282..428649e 100644 --- a/workflows/self-lint.yml +++ b/workflows/self-lint.yml @@ -1,29 +1,30 @@ name: Self-Lint -description: Run linter, fix violations, repeat until clean +description: Run linter, fix violations deterministically, repeat until clean steps: - id: baseline - model: fast - prompt: | - Run the project's linter and report all violations. - {{input}} - - Group by severity. Show file, line, and rule name. + command: "{{input}} 2>&1 || true" - id: fix model: smart prompt: | - Lint violations: + Current lint output: {{baseline}} - Fix the violations. Prioritize errors over warnings. + Fix lint/type violations. Prioritize errors over warnings. + Fix ONE group of related issues at a time. Don't change code behavior — only fix lint issues. + Don't disable lint rules — fix the underlying issue. - Run the linter again. If clean, say "ALL_PASSING". + After fixing, say DONE. loop: - max: 6 - until: ALL_PASSING + max: 20 + until: "exit code: 0" + gate: "{{input}}" + + - id: verify + command: "{{input}} 2>&1 || true" - id: summary model: fast @@ -34,6 +35,6 @@ steps: {{baseline}} Final state: - {{fix}} + {{verify}} - Summarize what was fixed. + Summarize what was fixed and what remains (if anything).