From 548674077694e92cf488fe6e8d6e8007eed72879 Mon Sep 17 00:00:00 2001 From: Tym Rabchuk Date: Thu, 9 Apr 2026 20:01:59 -0400 Subject: [PATCH 1/2] Add expect field to engine for command step exit code assertions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New `expect` field on command steps: - `expect: failure` — step fails if exit code is 0 (bug not reproducible) - `expect: success` (default) — non-zero exit is informational Enables bugfix reproduction gates: repro must fail before fix, pass after. Validated only on command steps. - src/engine/workflow.go: Add Expect field to WfStep, validation - src/engine/engine.go: Check expect condition after command execution - src/engine/engine_test.go: 6 new tests (parse, validate, runtime) - skills/creating-workflows: Document expect field --- skills/creating-workflows/SKILL.md | 1 + src/engine/engine.go | 10 +++ src/engine/engine_test.go | 137 +++++++++++++++++++++++++++++ src/engine/workflow.go | 7 ++ 4 files changed, 155 insertions(+) diff --git a/skills/creating-workflows/SKILL.md b/skills/creating-workflows/SKILL.md index 058474b..752ef80 100644 --- a/skills/creating-workflows/SKILL.md +++ b/skills/creating-workflows/SKILL.md @@ -37,6 +37,7 @@ steps: - **model** — Which model tier runs this step. Pick based on task complexity. - **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`. +- **expect** — Only for `command` steps. Values: `success` (default — non-zero exit is informational) or `failure` (step fails if exit code is 0). Enables bugfix reproduction gates: repro must fail before fix, pass after. - **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. 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. diff --git a/src/engine/engine.go b/src/engine/engine.go index b784ca5..6c2035c 100644 --- a/src/engine/engine.go +++ b/src/engine/engine.go @@ -297,6 +297,16 @@ func (e *Engine) runStep(ctx context.Context, step *WfStep, session *lib.Session return 0, "", fmt.Errorf("step %s command failed: %w", step.ID, err) } + // Check expect condition: "failure" means non-zero exit is expected (zero = step fails). + if step.Expect == "failure" && exitCode == 0 { + reason := fmt.Sprintf("expected failure but got exit code 0") + dbStep.Status = "failed" + dbStep.ChangeSummary = reason + e.db.UpdateStep(dbStep) + fmt.Printf(" %s\n\n", reason) + return 0, "", fmt.Errorf("step %s: %s", step.ID, reason) + } + // Include exit code in output so downstream steps can check it fullOutput := fmt.Sprintf("%s\nexit code: %d", strings.TrimRight(output, "\n"), exitCode) diff --git a/src/engine/engine_test.go b/src/engine/engine_test.go index fcdccc1..42c28a3 100644 --- a/src/engine/engine_test.go +++ b/src/engine/engine_test.go @@ -927,6 +927,143 @@ func TestRunWorkflowCommandNonZeroExit(t *testing.T) { } } +// --------------------------------------------------------------------------- +// Expect field tests +// --------------------------------------------------------------------------- + +func TestRunWorkflowExpectFailure(t *testing.T) { + db := tempDB(t) + dir, git := initGitRepo(t) + runner := newMockRunner(nil, nil) + eng := mustEngine(t, db, git, runner, dir) + + // expect: failure — non-zero exit should succeed (bug repro) + wf := &Workflow{ + Name: "test", + Steps: []WfStep{ + {ID: "repro", Command: "exit 1", Expect: "failure"}, + }, + } + + res, err := eng.RunWorkflow(context.Background(), wf, RunConfig{Input: "test"}) + if err != nil { + t.Fatalf("expected success for non-zero exit with expect:failure, got: %v", err) + } + if _, ok := res.Outputs["repro"]; !ok { + t.Fatal("repro output missing") + } +} + +func TestRunWorkflowExpectFailureButGotZero(t *testing.T) { + db := tempDB(t) + dir, git := initGitRepo(t) + runner := newMockRunner(nil, nil) + eng := mustEngine(t, db, git, runner, dir) + + // expect: failure — zero exit should FAIL (bug not reproducible) + wf := &Workflow{ + Name: "test", + Steps: []WfStep{ + {ID: "repro", Command: "exit 0", Expect: "failure"}, + }, + } + + _, err := eng.RunWorkflow(context.Background(), wf, RunConfig{Input: "test"}) + if err == nil { + t.Fatal("expected error for exit 0 with expect:failure, got nil") + } + if !strings.Contains(err.Error(), "expected failure") { + t.Errorf("error = %q, want it to contain 'expected failure'", err.Error()) + } +} + +func TestRunWorkflowExpectSuccessDefault(t *testing.T) { + db := tempDB(t) + dir, git := initGitRepo(t) + runner := newMockRunner(nil, nil) + eng := mustEngine(t, db, git, runner, dir) + + // No expect field (default) — non-zero exit is informational, not fatal + wf := &Workflow{ + Name: "test", + Steps: []WfStep{ + {ID: "check", Command: "exit 1"}, + }, + } + + res, err := eng.RunWorkflow(context.Background(), wf, RunConfig{Input: "test"}) + if err != nil { + t.Fatalf("default expect should not fail on non-zero exit: %v", err) + } + if !strings.Contains(res.Outputs["check"], "exit code: 1") { + t.Error("exit code not in output") + } +} + +func TestParseExpectField(t *testing.T) { + yaml := ` +name: Expect Test +description: test + +steps: + - id: repro + command: "npm test" + expect: failure + - id: verify + command: "npm test" + expect: success +` + wf, err := Parse([]byte(yaml)) + if err != nil { + t.Fatalf("Parse: %v", err) + } + if wf.Steps[0].Expect != "failure" { + t.Errorf("step 0 expect = %q, want failure", wf.Steps[0].Expect) + } + if wf.Steps[1].Expect != "success" { + t.Errorf("step 1 expect = %q, want success", wf.Steps[1].Expect) + } +} + +func TestValidateExpectOnPromptStep(t *testing.T) { + yaml := ` +name: Bad +description: test + +steps: + - id: broken + model: smart + prompt: "do something" + expect: failure +` + _, err := Parse([]byte(yaml)) + if err == nil { + t.Fatal("expected validation error for expect on prompt step") + } + if !strings.Contains(err.Error(), "expect without command") { + t.Errorf("error = %q, want 'expect without command'", err.Error()) + } +} + +func TestValidateExpectInvalidValue(t *testing.T) { + yaml := ` +name: Bad +description: test + +steps: + - id: broken + command: "echo hi" + expect: maybe +` + _, err := Parse([]byte(yaml)) + if err == nil { + t.Fatal("expected validation error for invalid expect value") + } + if !strings.Contains(err.Error(), "invalid expect") { + t.Errorf("error = %q, want 'invalid expect'", err.Error()) + } +} + // --------------------------------------------------------------------------- // Gate tests // --------------------------------------------------------------------------- diff --git a/src/engine/workflow.go b/src/engine/workflow.go index af32d6e..0eff7e0 100644 --- a/src/engine/workflow.go +++ b/src/engine/workflow.go @@ -31,6 +31,7 @@ type WfStep struct { Model string `yaml:"model"` Prompt string `yaml:"prompt"` Command string `yaml:"command"` + Expect string `yaml:"expect"` Parallel []string `yaml:"parallel"` Loop *Loop `yaml:"loop"` Branch []Branch `yaml:"branch"` @@ -101,6 +102,12 @@ func validate(wf *Workflow) error { 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.Expect != "" && s.Command == "" { + return fmt.Errorf("step %q has expect without command — expect only applies to command steps", s.ID) + } + if s.Expect != "" && s.Expect != "success" && s.Expect != "failure" { + return fmt.Errorf("step %q has invalid expect %q — must be \"success\" or \"failure\"", s.ID, s.Expect) + } if s.Command != "" && s.Loop != nil { return fmt.Errorf("step %q has both command and loop — these are mutually exclusive", s.ID) } From 152e7d4b22d15b54496f9b6a389c6f5bdeb82f41 Mon Sep 17 00:00:00 2001 From: Tym Rabchuk Date: Thu, 9 Apr 2026 20:05:01 -0400 Subject: [PATCH 2/2] Make expect: success enforce exit code 0, add symmetric tests Review consensus: expect: success must fail on non-zero exit (symmetric with expect: failure failing on zero exit). Default (no expect) remains informational. - engine.go: Add expect:success check (fail on non-zero) - engine.go: Fix fmt.Sprintf with no format args - engine_test.go: Add expect:success pass/fail tests, strengthen expect:failure test with output assertion - creating-workflows SKILL.md: Update docs for symmetric semantics --- skills/creating-workflows/SKILL.md | 2 +- src/engine/engine.go | 12 +++++-- src/engine/engine_test.go | 56 ++++++++++++++++++++++++++++-- 3 files changed, 64 insertions(+), 6 deletions(-) diff --git a/skills/creating-workflows/SKILL.md b/skills/creating-workflows/SKILL.md index 752ef80..824a4d9 100644 --- a/skills/creating-workflows/SKILL.md +++ b/skills/creating-workflows/SKILL.md @@ -37,7 +37,7 @@ steps: - **model** — Which model tier runs this step. Pick based on task complexity. - **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`. -- **expect** — Only for `command` steps. Values: `success` (default — non-zero exit is informational) or `failure` (step fails if exit code is 0). Enables bugfix reproduction gates: repro must fail before fix, pass after. +- **expect** — Only for `command` steps. Values: `success` (step fails if exit code is non-zero) or `failure` (step fails if exit code is 0). Omit for the default behavior where all exit codes are informational. Enables bugfix reproduction gates: repro with `expect: failure` must fail before fix, verify with `expect: success` must pass after. - **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. 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. diff --git a/src/engine/engine.go b/src/engine/engine.go index 6c2035c..56e6231 100644 --- a/src/engine/engine.go +++ b/src/engine/engine.go @@ -297,9 +297,17 @@ func (e *Engine) runStep(ctx context.Context, step *WfStep, session *lib.Session return 0, "", fmt.Errorf("step %s command failed: %w", step.ID, err) } - // Check expect condition: "failure" means non-zero exit is expected (zero = step fails). + // Check expect condition on exit code. if step.Expect == "failure" && exitCode == 0 { - reason := fmt.Sprintf("expected failure but got exit code 0") + reason := "expected failure but got exit code 0" + dbStep.Status = "failed" + dbStep.ChangeSummary = reason + e.db.UpdateStep(dbStep) + fmt.Printf(" %s\n\n", reason) + return 0, "", fmt.Errorf("step %s: %s", step.ID, reason) + } + if step.Expect == "success" && exitCode != 0 { + reason := fmt.Sprintf("expected success but got exit code %d", exitCode) dbStep.Status = "failed" dbStep.ChangeSummary = reason e.db.UpdateStep(dbStep) diff --git a/src/engine/engine_test.go b/src/engine/engine_test.go index 42c28a3..e89446f 100644 --- a/src/engine/engine_test.go +++ b/src/engine/engine_test.go @@ -949,9 +949,13 @@ func TestRunWorkflowExpectFailure(t *testing.T) { if err != nil { t.Fatalf("expected success for non-zero exit with expect:failure, got: %v", err) } - if _, ok := res.Outputs["repro"]; !ok { + output, ok := res.Outputs["repro"] + if !ok { t.Fatal("repro output missing") } + if !strings.Contains(output, "exit code: 1") { + t.Errorf("output should contain exit code, got %q", output) + } } func TestRunWorkflowExpectFailureButGotZero(t *testing.T) { @@ -977,7 +981,53 @@ func TestRunWorkflowExpectFailureButGotZero(t *testing.T) { } } -func TestRunWorkflowExpectSuccessDefault(t *testing.T) { +func TestRunWorkflowExpectSuccessPass(t *testing.T) { + db := tempDB(t) + dir, git := initGitRepo(t) + runner := newMockRunner(nil, nil) + eng := mustEngine(t, db, git, runner, dir) + + // expect: success — exit 0 should succeed + wf := &Workflow{ + Name: "test", + Steps: []WfStep{ + {ID: "check", Command: "exit 0", Expect: "success"}, + }, + } + + res, err := eng.RunWorkflow(context.Background(), wf, RunConfig{Input: "test"}) + if err != nil { + t.Fatalf("expected success for exit 0 with expect:success, got: %v", err) + } + if !strings.Contains(res.Outputs["check"], "exit code: 0") { + t.Error("exit code not in output") + } +} + +func TestRunWorkflowExpectSuccessButGotNonZero(t *testing.T) { + db := tempDB(t) + dir, git := initGitRepo(t) + runner := newMockRunner(nil, nil) + eng := mustEngine(t, db, git, runner, dir) + + // expect: success — non-zero exit should FAIL + wf := &Workflow{ + Name: "test", + Steps: []WfStep{ + {ID: "check", Command: "exit 1", Expect: "success"}, + }, + } + + _, err := eng.RunWorkflow(context.Background(), wf, RunConfig{Input: "test"}) + if err == nil { + t.Fatal("expected error for exit 1 with expect:success, got nil") + } + if !strings.Contains(err.Error(), "expected success") { + t.Errorf("error = %q, want it to contain 'expected success'", err.Error()) + } +} + +func TestRunWorkflowNoExpectDefault(t *testing.T) { db := tempDB(t) dir, git := initGitRepo(t) runner := newMockRunner(nil, nil) @@ -993,7 +1043,7 @@ func TestRunWorkflowExpectSuccessDefault(t *testing.T) { res, err := eng.RunWorkflow(context.Background(), wf, RunConfig{Input: "test"}) if err != nil { - t.Fatalf("default expect should not fail on non-zero exit: %v", err) + t.Fatalf("default (no expect) should not fail on non-zero exit: %v", err) } if !strings.Contains(res.Outputs["check"], "exit code: 1") { t.Error("exit code not in output")