From cde158504f813f288ec223cd0786d2bb8fba7e07 Mon Sep 17 00:00:00 2001 From: Tym Rabchuk Date: Sat, 11 Apr 2026 10:54:34 -0400 Subject: [PATCH 1/4] fix(pr-ready): default enforce to soft so workflow can run shell/edit steps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Without an explicit `enforce:` field, the engine defaults to `hard` (src/engine/workflow.go:88-90), and every prompt-only step is classified as "prompt" (src/mcp/tools.go:260-268). Under prompt+hard the guard (src/cmd/guard.go:349-364) only permits Read/Grep/Glob/TodoWrite/ NotebookRead/Skill + devkit MCP tools, which blocks the 7 pr-ready steps whose bodies explicitly require Bash or Edit (validate, necessity, lint, test, doc-check, create-pr, monitor). Soft enforcement still routes through the guard — stderr nudges remain and the Stop-hook still blocks session end until advance is called — it just stops hard-blocking tool calls the step needs. Matches the pattern already used by tri-review.yml and tri-security.yml. Fixes #78 --- workflows/pr-ready.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/workflows/pr-ready.yml b/workflows/pr-ready.yml index ff8b9f1..71ba76d 100644 --- a/workflows/pr-ready.yml +++ b/workflows/pr-ready.yml @@ -1,5 +1,6 @@ name: PR Ready description: Full PR preparation pipeline — validate, drop unrelated changes, lint, test, security, changelog, create PR, monitor CI and reviews until resolved +enforce: soft steps: - id: validate From 643fea149d559992a02f8b0368440f935d0609c6 Mon Sep 17 00:00:00 2001 From: Tym Rabchuk Date: Sat, 11 Apr 2026 11:00:56 -0400 Subject: [PATCH 2/4] fix(workflows): default enforce to soft for 16 shell/edit-using workflows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extends PR #78's quick fix beyond pr-ready.yml after auditing every workflow step body against the prompt+hard allowlist (Read/Grep/Glob/TodoWrite/NotebookRead/Skill + devkit MCP). Flipped to `enforce: soft` — step bodies explicitly require Bash, Edit, Write, WebFetch, WebSearch, AskUserQuestion, or Task: feature implement/gen-tests/run-tests/fix-tests/lint/fix-lint/quick-fix bugfix diagnose/fix/regression-test/run-tests/fix-tests/quick-fix refactor refactor/run-tests/fix-tests self-lint fix (Edit) self-test fix (Edit) self-perf optimize (Edit) self-improve improve (Edit) self-migrate migrate (Edit) self-audit measure-quality/measure-security/measure-git (Bash) audit deps/lint (Bash) autoloop baseline/fix/measure/keep/revert (Bash + Edit) onboard architect (Task) + guide (Write) doc-gen generate (Task) + write (Write) test-gen generate (Task) + run-fix (Bash) research clarify (ask_user) + search-* (WebSearch) + summarize (WebFetch) deep-research same pattern — WebFetch/WebSearch/ask_user throughout Left at default (hard) after verification: tri-debug Read-only diagnosis, no edits/shells tri-dispatch pure reasoning, no tool use (tri-review, tri-security already soft) Verification path: MCP devkit_advance does NOT skip parallel children (src/mcp/tools.go advances CurrentIndex linearly), so each search-* and measure-* prompt body lands as a main-line step and hits the guard. This is unlike engine.go which builds parallelChildren and skips them. That confirms research/deep-research/self-audit need soft. Tests: `go test -count=1 ./...` green. TestParseAllShippedWorkflows already exercises every workflow YAML through the engine's parser and validate path. Refs #78 --- workflows/audit.yml | 1 + workflows/autoloop.yml | 1 + workflows/bugfix.yml | 1 + workflows/deep-research.yml | 1 + workflows/doc-gen.yml | 1 + workflows/feature.yml | 1 + workflows/onboard.yml | 1 + workflows/refactor.yml | 1 + workflows/research.yml | 1 + workflows/self-audit.yml | 1 + workflows/self-improve.yml | 1 + workflows/self-lint.yml | 1 + workflows/self-migrate.yml | 1 + workflows/self-perf.yml | 1 + workflows/self-test.yml | 1 + workflows/test-gen.yml | 1 + 16 files changed, 16 insertions(+) diff --git a/workflows/audit.yml b/workflows/audit.yml index e314dd6..74b7a54 100644 --- a/workflows/audit.yml +++ b/workflows/audit.yml @@ -1,5 +1,6 @@ name: Project Audit description: Unified project health audit — detect ecosystem, audit deps, lint, security, report +enforce: soft steps: - id: detect diff --git a/workflows/autoloop.yml b/workflows/autoloop.yml index ec47236..09b3f6d 100644 --- a/workflows/autoloop.yml +++ b/workflows/autoloop.yml @@ -1,5 +1,6 @@ name: Autoloop description: Autonomous improvement loop inspired by karpathy/autoresearch — audit, fix, measure, keep or revert, repeat +enforce: soft budget: limit: 500000 diff --git a/workflows/bugfix.yml b/workflows/bugfix.yml index c07c354..3069c52 100644 --- a/workflows/bugfix.yml +++ b/workflows/bugfix.yml @@ -1,5 +1,6 @@ name: Bug Fix description: Full lifecycle bug fix — triage, reproduce, diagnose, fix, test, review (with fast path for trivial fixes) +enforce: soft budget: limit: 300000 diff --git a/workflows/deep-research.yml b/workflows/deep-research.yml index 09d7224..821c25d 100644 --- a/workflows/deep-research.yml +++ b/workflows/deep-research.yml @@ -1,5 +1,6 @@ name: Deep Research description: ACH-enhanced deep research — perspective discovery, competing hypotheses, directed disconfirmation, evidence matrix +enforce: soft budget: limit: 400000 diff --git a/workflows/doc-gen.yml b/workflows/doc-gen.yml index 1fe3ea8..289a710 100644 --- a/workflows/doc-gen.yml +++ b/workflows/doc-gen.yml @@ -1,5 +1,6 @@ name: Documentation Generation description: Generate code documentation via the documenter agent — analyze code → spawn documenter → write output +enforce: soft steps: - id: analyze diff --git a/workflows/feature.yml b/workflows/feature.yml index d47d131..8b97cde 100644 --- a/workflows/feature.yml +++ b/workflows/feature.yml @@ -1,5 +1,6 @@ name: Feature description: Full lifecycle — triage, brainstorm, plan, implement, test, lint, review (with fast path for small changes) +enforce: soft budget: limit: 500000 diff --git a/workflows/onboard.yml b/workflows/onboard.yml index 1182eda..8a9d778 100644 --- a/workflows/onboard.yml +++ b/workflows/onboard.yml @@ -1,5 +1,6 @@ name: Codebase Onboarding description: Generate an onboarding guide for new contributors — analyze structure → spawn researcher → write guide +enforce: soft steps: - id: analyze diff --git a/workflows/refactor.yml b/workflows/refactor.yml index 65a1cd2..37920cb 100644 --- a/workflows/refactor.yml +++ b/workflows/refactor.yml @@ -1,5 +1,6 @@ name: Refactor description: Full lifecycle refactor — analyze, plan, restructure, verify nothing broke +enforce: soft budget: limit: 400000 diff --git a/workflows/research.yml b/workflows/research.yml index b21011f..27cf07e 100644 --- a/workflows/research.yml +++ b/workflows/research.yml @@ -1,5 +1,6 @@ name: Research description: Research workflow — clarify, decompose, parallel search, summarize, corroborate, synthesize +enforce: soft budget: limit: 200000 diff --git a/workflows/self-audit.yml b/workflows/self-audit.yml index ce0ef0f..25db591 100644 --- a/workflows/self-audit.yml +++ b/workflows/self-audit.yml @@ -1,5 +1,6 @@ name: Self-Audit description: Codebase audit inspired by karpathy/autoresearch — measure everything, rank hypotheses by evidence, present actionable plan +enforce: soft budget: limit: 200000 diff --git a/workflows/self-improve.yml b/workflows/self-improve.yml index 6e5817d..3d402d8 100644 --- a/workflows/self-improve.yml +++ b/workflows/self-improve.yml @@ -1,5 +1,6 @@ name: Self-Improve description: Metric-gated improvement loop — run command, fix issues, repeat until passing +enforce: soft steps: - id: baseline diff --git a/workflows/self-lint.yml b/workflows/self-lint.yml index d9ef97c..eb579d3 100644 --- a/workflows/self-lint.yml +++ b/workflows/self-lint.yml @@ -1,5 +1,6 @@ name: Self-Lint description: Run linter, fix violations deterministically, repeat until clean +enforce: soft steps: - id: baseline diff --git a/workflows/self-migrate.yml b/workflows/self-migrate.yml index 679fecc..d3a0957 100644 --- a/workflows/self-migrate.yml +++ b/workflows/self-migrate.yml @@ -1,5 +1,6 @@ name: Self-Migrate description: Incremental migration loop — migrate code one piece at a time with tests as safety gate +enforce: soft steps: - id: baseline diff --git a/workflows/self-perf.yml b/workflows/self-perf.yml index be3a765..4824219 100644 --- a/workflows/self-perf.yml +++ b/workflows/self-perf.yml @@ -1,5 +1,6 @@ name: Self-Perf description: Profile performance, optimize hot paths deterministically, verify improvement +enforce: soft steps: - id: baseline diff --git a/workflows/self-test.yml b/workflows/self-test.yml index 76f23af..40974e4 100644 --- a/workflows/self-test.yml +++ b/workflows/self-test.yml @@ -1,5 +1,6 @@ name: Self-Test description: Run tests, fix failures deterministically, repeat until all pass +enforce: soft steps: # $DEVKIT_INPUT is the shell command provided by the caller (e.g. diff --git a/workflows/test-gen.yml b/workflows/test-gen.yml index 9b74e44..3cf08dd 100644 --- a/workflows/test-gen.yml +++ b/workflows/test-gen.yml @@ -1,5 +1,6 @@ name: Test Generation description: Generate tests for code, run them, iterate until they pass — analyze → generate via test-writer agent → run-fix loop → report +enforce: soft steps: - id: analyze From af25f3aad8bfb4644367701648e83b285f2f3f0f Mon Sep 17 00:00:00 2001 From: Tym Rabchuk Date: Sat, 11 Apr 2026 11:33:14 -0400 Subject: [PATCH 3/4] feat(engine): per-step enforce override + surgical workflow soft-flips MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the "all-or-nothing" problem introduced by PR #64's prompt+hard block. Each workflow step can now declare `enforce: soft` individually so mid-step hard-block stays where it's doing useful work (pure reasoning steps) and only relaxes where the step body explicitly needs Bash/Edit/Write/WebFetch/Task. Engine: - WfStep.Enforce (`yaml:"enforce"`) — empty inherits workflow default - engine.EffectiveEnforce(wf, step) — fall-through helper, nil-safe - validate() rejects step-level enforce values other than hard|soft|"" with a clear error MCP state propagation: - SessionState.Enforce is now re-derived on every transition: 1. devkit_start uses EffectiveEnforce(wf, &firstStep) 2. advanceTool normal transition recomputes for nextStep 3. advancePastLoop recomputes after loop exit - Loop iterations stay on the same step, so intra-loop state is unchanged (already correct). Workflows — reverted blanket `enforce: soft` from PR #80's first round and applied per-step overrides only where the step body requires a blocked tool. Pure-reasoning steps now go back to hard: pr-ready: 8 soft, security stays hard (read-only review) feature: 8 soft, triage/brainstorm/plan/review-smart/review-fast hard bugfix: 8 soft, triage hard refactor: 3 soft, analyze/plan/comparison hard self-lint/test/perf/improve/migrate: 1 soft each (fix step), summary hard self-audit: 3 soft (measure-*), detect/analyze/synthesize hard audit: 2 soft (deps/lint), detect (command)/report hard autoloop: 6 soft (baseline/fix/measure/keep/revert/report), audit/compare hard onboard: 2 soft (architect/guide), analyze hard doc-gen: 2 soft (generate/write), analyze hard test-gen: 2 soft (generate/run-fix), analyze/report hard research: 6 soft (clarify/search-*/summarize/follow-up), decompose/synthesize hard deep-research: 8 soft (clarify/perspectives/search-*/extract-claims/disconfirm/self-critique), decompose/hypotheses/evidence-matrix/synthesize hard tri-review: 1 soft (gather), 3 reviews + consolidate hard tri-security: 1 soft (gather), 3 audits + consolidate hard tri-debug, tri-dispatch: fully hard (already correct) Net effect: PR #64's mid-step tool block retains coverage on 30+ pure-reasoning steps across the workflow surface; soft only applies to steps whose body explicitly instructs the agent to run a blocked tool. Tests: - TestParseStepLevelEnforceOverride — parser accepts step.enforce - TestParseValidation/invalid_step_enforce — rejects bad values - TestEffectiveEnforce — nil-safe fall-through matrix - TestAdvancePropagatesStepEnforce — MCP state transitions flip enforce as the workflow walks mixed steps - TestParseAllShippedWorkflows — all 21 YAMLs parse + validate - `go test -count=1 ./...` green (cmd/engine/lib/mcp/runners) Refs #78 --- src/engine/engine_test.go | 74 +++++++++++++++++++++++++++++ src/engine/workflow.go | 27 +++++++++++ src/mcp/tools.go | 17 +++++-- src/mcp/tools_test.go | 94 +++++++++++++++++++++++++++++++++++++ workflows/audit.yml | 3 +- workflows/autoloop.yml | 7 ++- workflows/bugfix.yml | 9 +++- workflows/deep-research.yml | 9 +++- workflows/doc-gen.yml | 4 +- workflows/feature.yml | 9 +++- workflows/onboard.yml | 4 +- workflows/pr-ready.yml | 10 +++- workflows/refactor.yml | 4 +- workflows/research.yml | 7 ++- workflows/self-audit.yml | 4 +- workflows/self-improve.yml | 2 +- workflows/self-lint.yml | 2 +- workflows/self-migrate.yml | 2 +- workflows/self-perf.yml | 2 +- workflows/self-test.yml | 2 +- workflows/test-gen.yml | 4 +- workflows/tri-review.yml | 2 +- workflows/tri-security.yml | 2 +- 23 files changed, 276 insertions(+), 24 deletions(-) diff --git a/src/engine/engine_test.go b/src/engine/engine_test.go index 87950a5..bc5c9cd 100644 --- a/src/engine/engine_test.go +++ b/src/engine/engine_test.go @@ -260,6 +260,11 @@ steps: prompt: x - id: fetch_data prompt: y`, "collide under env key"}, + {"invalid step enforce", `name: T +steps: + - id: a + prompt: x + enforce: maybe`, `invalid enforce "maybe"`}, } for _, tt := range tests { @@ -1415,6 +1420,75 @@ steps: } } +func TestParseStepLevelEnforceOverride(t *testing.T) { + yaml := []byte(` +name: mixed-enforce +steps: + - id: review + prompt: Pure reasoning, stays hard. + - id: fix + prompt: Writes files, needs soft. + enforce: soft + - id: summary + prompt: Pure reasoning again. + enforce: hard +`) + wf, err := Parse(yaml) + if err != nil { + t.Fatalf("parse failed: %v", err) + } + if wf.Enforce != "hard" { + t.Errorf("workflow enforce = %q, want default hard", wf.Enforce) + } + if wf.Steps[0].Enforce != "" { + t.Errorf("step 0 enforce = %q, want empty (inherit)", wf.Steps[0].Enforce) + } + if wf.Steps[1].Enforce != "soft" { + t.Errorf("step 1 enforce = %q, want soft", wf.Steps[1].Enforce) + } + if wf.Steps[2].Enforce != "hard" { + t.Errorf("step 2 enforce = %q, want hard", wf.Steps[2].Enforce) + } +} + +func TestEffectiveEnforce(t *testing.T) { + tests := []struct { + name string + wfField string + stepField string + want string + }{ + {"step soft overrides wf hard", "hard", "soft", "soft"}, + {"step hard overrides wf soft", "soft", "hard", "hard"}, + {"empty step inherits wf soft", "soft", "", "soft"}, + {"empty step inherits wf hard", "hard", "", "hard"}, + {"both empty → default hard", "", "", "hard"}, + {"nil workflow → default hard", "", "", "hard"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + wf := &Workflow{Enforce: tt.wfField} + step := &WfStep{Enforce: tt.stepField} + got := EffectiveEnforce(wf, step) + if got != tt.want { + t.Errorf("EffectiveEnforce = %q, want %q", got, tt.want) + } + }) + } + + // Nil-safety: caller might pass nil for either arg during early + // setup paths. Should not panic; should fall through to "hard". + if got := EffectiveEnforce(nil, nil); got != "hard" { + t.Errorf("EffectiveEnforce(nil, nil) = %q, want hard", got) + } + if got := EffectiveEnforce(nil, &WfStep{Enforce: "soft"}); got != "soft" { + t.Errorf("EffectiveEnforce(nil, soft) = %q, want soft", got) + } + if got := EffectiveEnforce(&Workflow{Enforce: "soft"}, nil); got != "soft" { + t.Errorf("EffectiveEnforce(soft, nil) = %q, want soft", got) + } +} + func TestInterpolateDeterministic(t *testing.T) { // Regression: map iteration order is randomized in Go; Interpolate // must sort keys so a step output containing {{another-id}} renders diff --git a/src/engine/workflow.go b/src/engine/workflow.go index 33c1bbe..d551a46 100644 --- a/src/engine/workflow.go +++ b/src/engine/workflow.go @@ -41,6 +41,26 @@ type WfStep struct { Loop *Loop `yaml:"loop"` Branch []Branch `yaml:"branch"` Principles []string `yaml:"principles"` // per-step override + // Enforce overrides the workflow-level enforce for this step only. + // Empty inherits from Workflow.Enforce. Lets a workflow keep most + // prompt steps under hard (mid-step tool block) while allowing + // specific steps whose body needs Bash/Edit/Write/WebFetch to run + // under soft. The Stop-hook still blocks session end on soft steps. + Enforce string `yaml:"enforce"` +} + +// EffectiveEnforce returns the enforcement mode for a step, falling back +// to the workflow-level setting when the step does not override it. +// Callers should use this instead of reading step.Enforce directly so +// that the fall-through is consistent everywhere state transitions. +func EffectiveEnforce(wf *Workflow, step *WfStep) string { + if step != nil && step.Enforce != "" { + return step.Enforce + } + if wf != nil && wf.Enforce != "" { + return wf.Enforce + } + return "hard" } // Loop controls step repetition. @@ -151,6 +171,13 @@ func validate(wf *Workflow) error { 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) } + // Step-level enforce override: empty inherits from workflow, + // otherwise must be hard|soft. Command steps get guarded + // differently (engine runs them), but allow the field anyway + // for symmetry — it's a no-op there rather than a parse error. + if s.Enforce != "" && s.Enforce != "hard" && s.Enforce != "soft" { + return fmt.Errorf("step %q has invalid enforce %q — must be \"hard\" or \"soft\"", s.ID, s.Enforce) + } if s.Command != "" && s.Loop != nil { return fmt.Errorf("step %q has both command and loop — these are mutually exclusive", s.ID) } diff --git a/src/mcp/tools.go b/src/mcp/tools.go index 487e9c3..6a965a3 100644 --- a/src/mcp/tools.go +++ b/src/mcp/tools.go @@ -160,11 +160,16 @@ func (s *Server) startTool() (mcpmcp.Tool, mcpgo.ToolHandlerFunc) { CurrentIndex: 0, TotalSteps: len(wf.Steps), StepType: stepType(firstStep), - Enforce: wf.Enforce, - Branch: wf.BranchMode, - Status: "starting", - StartedAt: time.Now(), - Outputs: map[string]string{}, + // Per-step enforce override falls back to workflow level; + // SessionState.Enforce is always the *current step's* + // effective enforce, re-derived on every transition so + // the hook sees the right value without needing to know + // the step index. + Enforce: engine.EffectiveEnforce(wf, &firstStep), + Branch: wf.BranchMode, + Status: "starting", + StartedAt: time.Now(), + Outputs: map[string]string{}, }, nil }) if err != nil { @@ -478,6 +483,7 @@ func (s *Server) advanceTool() (mcpmcp.Tool, mcpgo.ToolHandlerFunc) { state.CurrentStep = nextStep.ID state.CurrentIndex = nextIndex state.StepType = stepType(nextStep) + state.Enforce = engine.EffectiveEnforce(wf, &nextStep) state.Busy = false if err := lib.WriteSessionJSON(s.dataDir, state); err != nil { return mcpmcp.NewToolResultError(fmt.Sprintf("write state: %v", err)), nil @@ -665,6 +671,7 @@ func (s *Server) advancePastLoop(wf *engine.Workflow, state *lib.SessionState) ( state.CurrentStep = nextStep.ID state.CurrentIndex = nextIndex state.StepType = stepType(nextStep) + state.Enforce = engine.EffectiveEnforce(wf, &nextStep) state.Busy = false if err := lib.WriteSessionJSON(s.dataDir, state); err != nil { return mcpmcp.NewToolResultError(fmt.Sprintf("write state: %v", err)), nil diff --git a/src/mcp/tools_test.go b/src/mcp/tools_test.go index f30702a..631d7ca 100644 --- a/src/mcp/tools_test.go +++ b/src/mcp/tools_test.go @@ -563,6 +563,100 @@ steps: } } +// TestAdvancePropagatesStepEnforce verifies that SessionState.Enforce is +// re-derived from the *current* step on every transition so that a +// workflow with mixed per-step enforce correctly flips the hook's +// enforcement mode as the workflow walks from step to step. +// +// This is the structural guarantee that backs per-step `enforce:` as a +// meaningful override: the hot-state file the hook reads must reflect +// the step the agent is currently on, not the workflow default. +func TestAdvancePropagatesStepEnforce(t *testing.T) { + wfDir := t.TempDir() + dataDir := t.TempDir() + + // Workflow-level default is hard. Middle step overrides to soft. + // Third step inherits (stays hard). Verifies all three transition + // paths: start (seeds first step), advance (normal), advance + // (back to inherited hard). + writeFile(t, filepath.Join(wfDir, "mixed.yml"), `name: mixed +description: Mixed per-step enforce test +steps: + - id: review + prompt: Read-only review. + - id: apply + prompt: Apply the fix. + enforce: soft + - id: summarize + prompt: Summarize what happened. +`) + + srv := newTestServer(t, dataDir, wfDir) + _, startHandler := srv.startTool() + startReq := mcpmcp.CallToolRequest{} + startReq.Params.Arguments = map[string]interface{}{ + "workflow": "mixed", + "input": "demo", + } + if _, err := startHandler(context.Background(), startReq); err != nil { + t.Fatalf("start: %v", err) + } + + state, err := lib.ReadSessionJSON(dataDir) + if err != nil || state == nil { + t.Fatalf("read session after start: %v", err) + } + if state.CurrentStep != "review" { + t.Fatalf("expected starting step review, got %s", state.CurrentStep) + } + if state.Enforce != "hard" { + t.Errorf("step 1 (review) enforce = %q, want hard (inherited from workflow default)", state.Enforce) + } + sessionID := state.ID + + _, advHandler := srv.advanceTool() + advance := func(output string) { + t.Helper() + req := mcpmcp.CallToolRequest{} + req.Params.Arguments = map[string]interface{}{ + "session": sessionID, + "output": output, + } + res, err := advHandler(context.Background(), req) + if err != nil { + t.Fatalf("advance: %v", err) + } + if res.IsError { + tc, _ := res.Content[0].(mcpmcp.TextContent) + t.Fatalf("advance error: %s", tc.Text) + } + } + + advance("review output") + state, _ = lib.ReadSessionJSON(dataDir) + if state == nil { + t.Fatal("session gone after advance 1") + } + if state.CurrentStep != "apply" { + t.Fatalf("expected step apply, got %s", state.CurrentStep) + } + if state.Enforce != "soft" { + t.Errorf("step 2 (apply) enforce = %q, want soft (per-step override)", state.Enforce) + } + + advance("apply output") + state, _ = lib.ReadSessionJSON(dataDir) + if state == nil { + t.Fatal("session gone after advance 2") + } + if state.CurrentStep != "summarize" { + t.Fatalf("expected step summarize, got %s", state.CurrentStep) + } + if state.Enforce != "hard" { + t.Errorf("step 3 (summarize) enforce = %q, want hard (back to inherited)", state.Enforce) + } +} + func TestAdvanceCommandStep(t *testing.T) { wfDir := t.TempDir() dataDir := t.TempDir() diff --git a/workflows/audit.yml b/workflows/audit.yml index 74b7a54..01a8e78 100644 --- a/workflows/audit.yml +++ b/workflows/audit.yml @@ -1,6 +1,5 @@ name: Project Audit description: Unified project health audit — detect ecosystem, audit deps, lint, security, report -enforce: soft steps: - id: detect @@ -12,6 +11,7 @@ steps: - id: deps model: smart + enforce: soft # runs npm audit / govulncheck / pip-audit / cargo audit prompt: | Detected ecosystems: {{detect}} @@ -25,6 +25,7 @@ steps: - id: lint model: smart + enforce: soft # runs eslint / golangci-lint / ruff / clippy prompt: | Detected ecosystems: {{detect}} diff --git a/workflows/autoloop.yml b/workflows/autoloop.yml index 09b3f6d..e9ba2fa 100644 --- a/workflows/autoloop.yml +++ b/workflows/autoloop.yml @@ -1,6 +1,5 @@ name: Autoloop description: Autonomous improvement loop inspired by karpathy/autoresearch — audit, fix, measure, keep or revert, repeat -enforce: soft budget: limit: 500000 @@ -9,6 +8,7 @@ budget: steps: - id: baseline model: fast + enforce: soft # runs user-supplied metric command via Bash prompt: | Run the user's metric command to establish a baseline measurement. @@ -57,6 +57,7 @@ steps: - id: fix model: smart + enforce: soft # edits source to apply hypothesis prompt: | Make the change recommended by the audit. @@ -77,6 +78,7 @@ steps: - id: measure model: fast + enforce: soft # re-runs the metric command prompt: | Run the EXACT same metric command from the baseline to measure the result. @@ -118,6 +120,7 @@ steps: - id: keep model: fast + enforce: soft # git add + git commit + scratchpad edit prompt: | The change IMPROVED the metric. Keep it. @@ -146,6 +149,7 @@ steps: - id: revert model: fast + enforce: soft # git checkout + scratchpad edit prompt: | The change REGRESSED or had no effect. Revert it. @@ -174,6 +178,7 @@ steps: - id: report model: smart + enforce: soft # deletes scratchpad at the end prompt: | The autoloop session is complete. Write a final report. diff --git a/workflows/bugfix.yml b/workflows/bugfix.yml index 3069c52..11c420b 100644 --- a/workflows/bugfix.yml +++ b/workflows/bugfix.yml @@ -1,6 +1,5 @@ name: Bug Fix description: Full lifecycle bug fix — triage, reproduce, diagnose, fix, test, review (with fast path for trivial fixes) -enforce: soft budget: limit: 300000 @@ -27,6 +26,7 @@ steps: - id: reproduce model: smart + enforce: soft # "run the failing case" requires Bash prompt: | Bug report: {{input}} @@ -38,6 +38,7 @@ steps: - id: diagnose model: smart + enforce: soft # appends to scratchpad prompt: | Reproduction: @@ -55,6 +56,7 @@ steps: - id: fix model: smart + enforce: soft # edits source to implement the fix prompt: | Diagnosis: @@ -65,6 +67,7 @@ steps: - id: regression-test model: smart + enforce: soft # writes a new test file prompt: | A bug was fixed: @@ -78,6 +81,7 @@ steps: - id: run-tests model: fast + enforce: soft # runs the full test suite prompt: | Run the full test suite including the new regression test. Report results. @@ -86,6 +90,7 @@ steps: - id: fix-tests model: smart + enforce: soft # edits code/tests + re-runs prompt: | Test results: @@ -105,6 +110,7 @@ steps: - id: summary model: fast + enforce: soft # deletes scratchpad file prompt: | Bug fix complete. Summary: @@ -127,6 +133,7 @@ steps: - id: quick-fix model: smart + enforce: soft # edits + writes test + runs tests prompt: | This is a trivial bug — no deep investigation needed. diff --git a/workflows/deep-research.yml b/workflows/deep-research.yml index 821c25d..35f3e9c 100644 --- a/workflows/deep-research.yml +++ b/workflows/deep-research.yml @@ -1,6 +1,5 @@ name: Deep Research description: ACH-enhanced deep research — perspective discovery, competing hypotheses, directed disconfirmation, evidence matrix -enforce: soft budget: limit: 400000 @@ -9,6 +8,7 @@ budget: steps: - id: clarify model: smart + enforce: soft # AskUserQuestion prompt: | The user wants to deep-research: {{input}} @@ -22,6 +22,7 @@ steps: - id: perspectives model: smart + enforce: soft # WebSearch + WebFetch via Jina prompt: | Research question: {{clarify}} @@ -59,6 +60,7 @@ steps: - id: search-1 model: general + enforce: soft # WebSearch prompt: | Execute web search for sub-questions 1-2 from this decomposition: {{decompose}} @@ -67,6 +69,7 @@ steps: - id: search-2 model: general + enforce: soft # WebSearch prompt: | Execute web search for sub-questions 3-4 from this decomposition: {{decompose}} @@ -75,6 +78,7 @@ steps: - id: search-3 model: general + enforce: soft # WebSearch prompt: | Execute web search for sub-questions 5+ from this decomposition: {{decompose}} @@ -87,6 +91,7 @@ steps: - id: extract-claims model: smart + enforce: soft # WebFetch via Jina Reader prompt: | Research question: {{clarify}} @@ -128,6 +133,7 @@ steps: - id: disconfirm model: smart + enforce: soft # WebSearch + WebFetch for disconfirming evidence prompt: | Research question: {{clarify}} Hypotheses: {{hypotheses}} @@ -177,6 +183,7 @@ steps: - id: self-critique model: smart + enforce: soft # may do ONE targeted WebSearch to fill gaps prompt: | Research question: {{clarify}} Evidence matrix and sensitivity: {{evidence-matrix}} diff --git a/workflows/doc-gen.yml b/workflows/doc-gen.yml index 289a710..45d9e01 100644 --- a/workflows/doc-gen.yml +++ b/workflows/doc-gen.yml @@ -1,10 +1,10 @@ name: Documentation Generation description: Generate code documentation via the documenter agent — analyze code → spawn documenter → write output -enforce: soft steps: - id: analyze model: general + # analyze stays hard: Read-only surface extraction. prompt: | Target: {{input}} @@ -23,6 +23,7 @@ steps: - id: generate model: smart + enforce: soft # spawns documenter agent via Task tool prompt: | Target: {{input}} Analysis: {{analyze}} @@ -44,6 +45,7 @@ steps: - id: write model: fast + enforce: soft # writes doc files to disk prompt: | Generated docs: {{generate}} diff --git a/workflows/feature.yml b/workflows/feature.yml index 8b97cde..a79dc0d 100644 --- a/workflows/feature.yml +++ b/workflows/feature.yml @@ -1,6 +1,5 @@ name: Feature description: Full lifecycle — triage, brainstorm, plan, implement, test, lint, review (with fast path for small changes) -enforce: soft budget: limit: 500000 @@ -56,6 +55,7 @@ steps: - id: implement model: smart + enforce: soft # writes code + appends to scratchpad prompt: | Implementation plan: @@ -77,6 +77,7 @@ steps: - id: gen-tests model: smart + enforce: soft # creates test files prompt: | Implementation complete. The feature: @@ -92,6 +93,7 @@ steps: - id: run-tests model: fast + enforce: soft # runs project test suite prompt: | Run the full test suite (not just the new tests). Report which pass, which fail, and the error messages. @@ -100,6 +102,7 @@ steps: - id: fix-tests model: smart + enforce: soft # edits source + test files, re-runs tests prompt: | Test results: @@ -119,6 +122,7 @@ steps: - id: lint model: fast + enforce: soft # runs project linter prompt: | Run the project's linter on the changed files. Report any violations. @@ -127,6 +131,7 @@ steps: - id: fix-lint model: smart + enforce: soft # edits violations, re-runs linter prompt: | Lint violations: @@ -163,6 +168,7 @@ steps: - id: final-report model: fast + enforce: soft # deletes scratchpad file prompt: | Feature session complete. Produce a summary: @@ -189,6 +195,7 @@ steps: - id: quick-fix model: smart + enforce: soft # edits + runs linter + runs tests prompt: | This is a tiny change — no design or planning needed. diff --git a/workflows/onboard.yml b/workflows/onboard.yml index 8a9d778..fdd66e2 100644 --- a/workflows/onboard.yml +++ b/workflows/onboard.yml @@ -1,10 +1,10 @@ name: Codebase Onboarding description: Generate an onboarding guide for new contributors — analyze structure → spawn researcher → write guide -enforce: soft steps: - id: analyze model: general + # analyze stays hard: Read/Glob-only structural scan. prompt: | Target codebase: {{input}} @@ -24,6 +24,7 @@ steps: - id: architect model: smart + enforce: soft # spawns researcher agent via Task tool prompt: | Target codebase: {{input}} Initial analysis: {{analyze}} @@ -44,6 +45,7 @@ steps: - id: guide model: smart + enforce: soft # writes docs/ONBOARDING.md prompt: | Initial analysis: {{analyze}} Architecture: {{architect}} diff --git a/workflows/pr-ready.yml b/workflows/pr-ready.yml index 71ba76d..f409b0b 100644 --- a/workflows/pr-ready.yml +++ b/workflows/pr-ready.yml @@ -1,10 +1,10 @@ name: PR Ready description: Full PR preparation pipeline — validate, drop unrelated changes, lint, test, security, changelog, create PR, monitor CI and reviews until resolved -enforce: soft steps: - id: validate model: fast + enforce: soft # runs git status / git log prompt: | Validate the current branch is ready for PR preparation: - Not on main/master @@ -15,6 +15,7 @@ steps: - id: necessity model: smart + enforce: soft # may Edit files to remove debug artifacts prompt: | Review the diff (git diff main...HEAD) for unnecessary changes: - Debug artifacts (console.log, print statements) @@ -25,6 +26,7 @@ steps: - id: lint model: smart + enforce: soft # runs linter + edits violations prompt: | Run the project's linter on changed files. Fix any violations. @@ -36,6 +38,7 @@ steps: - id: test model: smart + enforce: soft # runs test suite + edits failing tests prompt: | Run the full test suite. Fix any failures. @@ -47,6 +50,7 @@ steps: - id: security model: smart + # security stays hard: read-only review, no file writes. prompt: | Review changed files for security issues: - Hardcoded secrets or API keys @@ -60,6 +64,7 @@ steps: - id: doc-check model: smart + enforce: soft # commits docs edits when the rule is mechanical prompt: | Classify the diff (git diff main...HEAD) and decide which project docs need updating so the user does not have to ask every PR. @@ -152,6 +157,7 @@ steps: - id: changelog model: fast + enforce: soft # runs git diff main...HEAD prompt: | Generate a changelog entry from git diff main...HEAD. Summarize what changed and why. Any `docs: update ...` commit @@ -160,6 +166,7 @@ steps: - id: create-pr model: smart + enforce: soft # git push + gh pr create prompt: | Create the PR: 1. Push the branch to remote @@ -175,6 +182,7 @@ steps: - id: monitor model: smart + enforce: soft # gh pr checks, gh api, git push for review fixes prompt: | The PR was just created. Watch it until merge-ready. diff --git a/workflows/refactor.yml b/workflows/refactor.yml index 37920cb..b897f32 100644 --- a/workflows/refactor.yml +++ b/workflows/refactor.yml @@ -1,6 +1,5 @@ name: Refactor description: Full lifecycle refactor — analyze, plan, restructure, verify nothing broke -enforce: soft budget: limit: 400000 @@ -35,6 +34,7 @@ steps: - id: refactor model: smart + enforce: soft # edits source + runs compiler to verify prompt: | Refactoring plan: @@ -50,6 +50,7 @@ steps: - id: run-tests model: fast + enforce: soft # runs test suite prompt: | Run the full test suite to verify the refactoring didn't break anything. Report results. @@ -58,6 +59,7 @@ steps: - id: fix-tests model: smart + enforce: soft # edits code/tests + re-runs prompt: | Test results after refactoring: diff --git a/workflows/research.yml b/workflows/research.yml index 27cf07e..fcf5489 100644 --- a/workflows/research.yml +++ b/workflows/research.yml @@ -1,6 +1,5 @@ name: Research description: Research workflow — clarify, decompose, parallel search, summarize, corroborate, synthesize -enforce: soft budget: limit: 200000 @@ -9,6 +8,7 @@ budget: steps: - id: clarify model: smart + enforce: soft # uses AskUserQuestion prompt: | The user wants to research: {{input}} @@ -37,6 +37,7 @@ steps: - id: search-1 model: general + enforce: soft # WebSearch prompt: | Execute web search for the FIRST sub-question from this decomposition: {{decompose}} @@ -45,6 +46,7 @@ steps: - id: search-2 model: general + enforce: soft # WebSearch prompt: | Execute web search for the SECOND sub-question from this decomposition: {{decompose}} @@ -53,6 +55,7 @@ steps: - id: search-3 model: fast + enforce: soft # WebSearch prompt: | Execute web search for the THIRD sub-question from this decomposition: {{decompose}} @@ -64,6 +67,7 @@ steps: - id: summarize model: smart + enforce: soft # WebFetch via Jina Reader prompt: | Research question: {{clarify}} @@ -86,6 +90,7 @@ steps: - id: follow-up model: general + enforce: soft # WebSearch + WebFetch prompt: | Research question: {{clarify}} diff --git a/workflows/self-audit.yml b/workflows/self-audit.yml index 25db591..1cb8edb 100644 --- a/workflows/self-audit.yml +++ b/workflows/self-audit.yml @@ -1,6 +1,5 @@ name: Self-Audit description: Codebase audit inspired by karpathy/autoresearch — measure everything, rank hypotheses by evidence, present actionable plan -enforce: soft budget: limit: 200000 @@ -21,6 +20,7 @@ steps: - id: measure-quality model: general + enforce: soft # runs go vet, tsc, eslint, ruff, clippy prompt: | Stack detected: {{detect}} @@ -34,6 +34,7 @@ steps: - id: measure-security model: general + enforce: soft # runs govulncheck, npm audit, grep for secrets prompt: | Stack detected: {{detect}} @@ -46,6 +47,7 @@ steps: - id: measure-git model: fast + enforce: soft # runs git log / git ls-tree prompt: | Measure git health: - 30-day commit count diff --git a/workflows/self-improve.yml b/workflows/self-improve.yml index 3d402d8..550958f 100644 --- a/workflows/self-improve.yml +++ b/workflows/self-improve.yml @@ -1,6 +1,5 @@ name: Self-Improve description: Metric-gated improvement loop — run command, fix issues, repeat until passing -enforce: soft steps: - id: baseline @@ -8,6 +7,7 @@ steps: - id: improve model: smart + enforce: soft # edits source to apply fixes prompt: | Current metric output: diff --git a/workflows/self-lint.yml b/workflows/self-lint.yml index eb579d3..6eda503 100644 --- a/workflows/self-lint.yml +++ b/workflows/self-lint.yml @@ -1,6 +1,5 @@ name: Self-Lint description: Run linter, fix violations deterministically, repeat until clean -enforce: soft steps: - id: baseline @@ -8,6 +7,7 @@ steps: - id: fix model: smart + enforce: soft # edits source to fix violations prompt: | Current lint output: diff --git a/workflows/self-migrate.yml b/workflows/self-migrate.yml index d3a0957..cdeaf47 100644 --- a/workflows/self-migrate.yml +++ b/workflows/self-migrate.yml @@ -1,6 +1,5 @@ name: Self-Migrate description: Incremental migration loop — migrate code one piece at a time with tests as safety gate -enforce: soft steps: - id: baseline @@ -8,6 +7,7 @@ steps: - id: migrate model: smart + enforce: soft # edits source + updates imports/references prompt: | Test baseline: diff --git a/workflows/self-perf.yml b/workflows/self-perf.yml index 4824219..1ce7a33 100644 --- a/workflows/self-perf.yml +++ b/workflows/self-perf.yml @@ -1,6 +1,5 @@ name: Self-Perf description: Profile performance, optimize hot paths deterministically, verify improvement -enforce: soft steps: - id: baseline @@ -8,6 +7,7 @@ steps: - id: optimize model: smart + enforce: soft # edits source to apply optimization prompt: | Performance baseline: diff --git a/workflows/self-test.yml b/workflows/self-test.yml index 40974e4..60c0ce8 100644 --- a/workflows/self-test.yml +++ b/workflows/self-test.yml @@ -1,6 +1,5 @@ name: Self-Test description: Run tests, fix failures deterministically, repeat until all pass -enforce: soft steps: # $DEVKIT_INPUT is the shell command provided by the caller (e.g. @@ -11,6 +10,7 @@ steps: - id: fix model: smart + enforce: soft # edits code/tests to fix failures prompt: | Test output: diff --git a/workflows/test-gen.yml b/workflows/test-gen.yml index 3cf08dd..47accac 100644 --- a/workflows/test-gen.yml +++ b/workflows/test-gen.yml @@ -1,10 +1,10 @@ name: Test Generation description: Generate tests for code, run them, iterate until they pass — analyze → generate via test-writer agent → run-fix loop → report -enforce: soft steps: - id: analyze model: general + # analyze stays hard: Read-only framework detection. prompt: | Target: {{input}} @@ -23,6 +23,7 @@ steps: - id: generate model: smart + enforce: soft # spawns test-writer agent via Task tool prompt: | Target: {{input}} Analysis: {{analyze}} @@ -40,6 +41,7 @@ steps: - id: run-fix model: general + enforce: soft # runs tests + dispatches fix agents prompt: | Test files just created: {{generate}} diff --git a/workflows/tri-review.yml b/workflows/tri-review.yml index 4f6de33..eb561fd 100644 --- a/workflows/tri-review.yml +++ b/workflows/tri-review.yml @@ -1,10 +1,10 @@ name: Tri-Review description: Three-tier model review — reviews run in parallel, then consolidated -enforce: soft steps: - id: gather model: fast + enforce: soft # runs git diff / file reads to collect the diff prompt: | Collect the code or diff to review: {{input}} diff --git a/workflows/tri-security.yml b/workflows/tri-security.yml index ce4e872..833e507 100644 --- a/workflows/tri-security.yml +++ b/workflows/tri-security.yml @@ -1,10 +1,10 @@ name: Tri-Security description: Three-tier security audit — parallel review focused on vulnerabilities -enforce: soft steps: - id: gather model: fast + enforce: soft # runs git diff / file reads to collect the code prompt: | Collect the code to audit: {{input}} From 56327d612c1c98d81b3307f4b937c91de7cd9d0f Mon Sep 17 00:00:00 2001 From: Tym Rabchuk Date: Sat, 11 Apr 2026 11:44:47 -0400 Subject: [PATCH 4/4] address pr review: drop aspirational nil-safety, reject enforce on command steps, close test gaps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #80 review round. Aggregated feedback from code-reviewer, pr-test-analyzer, silent-failure-hunter, comment-analyzer, and type-design-analyzer. Addressed the high-severity items in-scope for this PR; deferred the named-type / guard-helper-dedup / state-field- rename refactors to follow-ups since they touch on-disk format or cross PR #64's territory. Engine: - EffectiveEnforce now takes values (Workflow, WfStep) instead of pointers. All current callers own concrete structs by the time they reach a transition, so the nil-safety branches were aspirational — dropping them makes the compiler enforce presence and removes a silent "nil,nil → hard" fall-through that would hide programming errors. - validate() now rejects `enforce:` on command steps. The previous comment claimed it was a no-op on command steps, but guard.go's command branch uniformly consults SessionState.Enforce, so marking a command step `soft` would let agent tools slip through while the engine is executing it. Fail loudly at parse time. - WfStep.Enforce YAML tag now `omitempty` so round-trips don't serialize empty strings. - Comment on WfStep.Enforce no longer hardcodes the guard tool list (would rot if guard.go gains or drops a tool). MCP state propagation: - All three call sites (startTool, advanceTool, advancePastLoop) updated to pass *wf / step by value. No behavior change. - Added a comment at handleLoopAdvance's continue path explaining why state.Enforce is NOT re-derived mid-loop (iterations stay on the same step, so effective enforce is stable). Tests: - TestEffectiveEnforce: reworked for value-based signature; nil edge cases replaced with zero-value cases (which the compiler now constructs safely). - TestParseValidation: new case `enforce on command step` asserts parse-time rejection. - TestAdvancePropagatesStepEnforceReverseOverride: NEW. Soft-default workflow with a hard-override step; asserts the state flips soft→hard→soft as the workflow walks. This is the symmetric case to the original test and was the biggest missing end-to-end gap (previously only unit-tested via EffectiveEnforce). - TestAdvancePropagatesStepEnforceAfterLoop: NEW. A loop step with `enforce: soft` hits its max, and advancePastLoop must re-derive enforce for the post-loop step (which inherits the workflow default hard). Before this test, advancePastLoop's state.Enforce write had zero coverage — a regression on tools.go:674 would not have been caught by the suite. - TestAdvancePropagatesStepEnforceAcrossBranch: NEW. A step with a `branch:` clause jumps past a sequential step with a different enforce to the target. Guards against a future refactor computing enforce from CurrentIndex+1 instead of the branch target. YAML comment cleanup (comment-analyzer feedback): - pr-ready.yml validate: "runs git status / git log" → "inspects branch state via git" (was over-claiming specific subcommands). - pr-ready.yml doc-check: "commits docs edits when mechanical" → "edits doc files (and commits them)" (the blocker is Edit, not the commit). - research.yml / deep-research.yml search-*: removed redundant "# WebSearch" annotations (the step body "Execute web search for..." is self-evident; the comment was pure WHAT). - audit.yml deps/lint: removed tool-list enumerations (rot-prone when new ecosystems added; body already lists them). - autoloop.yml measure: removed "re-runs the metric command" (restated the first line of the prompt body verbatim). - bugfix.yml reproduce: fake-quoted "run the failing case" was not in the step body; rewrote as "may run the failing case to confirm the bug". Deferred to follow-up PRs (tracked in PR body): - Named EnforceMode type across Workflow / WfStep / SessionState (touches src/lib/state_json.go JSON on-disk format). - Collapse guard.go's parallel effectiveEnforce helper now that state.Enforce is always concrete post-transition (PR #64 territory — warrants its own review). - Rename SessionState.Enforce → StepEnforce to signal the semantic shift from workflow-scoped to step-scoped. Refs #78 --- src/engine/engine_test.go | 32 +++--- src/engine/workflow.go | 41 +++++--- src/mcp/tools.go | 18 ++-- src/mcp/tools_test.go | 201 ++++++++++++++++++++++++++++++++++++ workflows/audit.yml | 4 +- workflows/autoloop.yml | 2 +- workflows/bugfix.yml | 2 +- workflows/deep-research.yml | 6 +- workflows/pr-ready.yml | 4 +- workflows/research.yml | 6 +- 10 files changed, 263 insertions(+), 53 deletions(-) diff --git a/src/engine/engine_test.go b/src/engine/engine_test.go index bc5c9cd..0e5eafd 100644 --- a/src/engine/engine_test.go +++ b/src/engine/engine_test.go @@ -265,6 +265,11 @@ steps: - id: a prompt: x enforce: maybe`, `invalid enforce "maybe"`}, + {"enforce on command step", `name: T +steps: + - id: a + command: "echo hi" + enforce: soft`, "enforce on a command step"}, } for _, tt := range tests { @@ -1453,40 +1458,29 @@ steps: func TestEffectiveEnforce(t *testing.T) { tests := []struct { - name string - wfField string + name string + wfField string stepField string - want string + want string }{ {"step soft overrides wf hard", "hard", "soft", "soft"}, {"step hard overrides wf soft", "soft", "hard", "hard"}, {"empty step inherits wf soft", "soft", "", "soft"}, {"empty step inherits wf hard", "hard", "", "hard"}, - {"both empty → default hard", "", "", "hard"}, - {"nil workflow → default hard", "", "", "hard"}, + {"both zero → default hard", "", "", "hard"}, + {"zero wf + soft step → soft", "", "soft", "soft"}, + {"zero wf + hard step → hard", "", "hard", "hard"}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - wf := &Workflow{Enforce: tt.wfField} - step := &WfStep{Enforce: tt.stepField} + wf := Workflow{Enforce: tt.wfField} + step := WfStep{Enforce: tt.stepField} got := EffectiveEnforce(wf, step) if got != tt.want { t.Errorf("EffectiveEnforce = %q, want %q", got, tt.want) } }) } - - // Nil-safety: caller might pass nil for either arg during early - // setup paths. Should not panic; should fall through to "hard". - if got := EffectiveEnforce(nil, nil); got != "hard" { - t.Errorf("EffectiveEnforce(nil, nil) = %q, want hard", got) - } - if got := EffectiveEnforce(nil, &WfStep{Enforce: "soft"}); got != "soft" { - t.Errorf("EffectiveEnforce(nil, soft) = %q, want soft", got) - } - if got := EffectiveEnforce(&Workflow{Enforce: "soft"}, nil); got != "soft" { - t.Errorf("EffectiveEnforce(soft, nil) = %q, want soft", got) - } } func TestInterpolateDeterministic(t *testing.T) { diff --git a/src/engine/workflow.go b/src/engine/workflow.go index d551a46..5202660 100644 --- a/src/engine/workflow.go +++ b/src/engine/workflow.go @@ -44,20 +44,24 @@ type WfStep struct { // Enforce overrides the workflow-level enforce for this step only. // Empty inherits from Workflow.Enforce. Lets a workflow keep most // prompt steps under hard (mid-step tool block) while allowing - // specific steps whose body needs Bash/Edit/Write/WebFetch to run - // under soft. The Stop-hook still blocks session end on soft steps. - Enforce string `yaml:"enforce"` + // specific steps whose body needs tools the hard mode blocks to + // run under soft. The Stop-hook still blocks session end on soft + // steps, so end-of-turn drift is still caught. + Enforce string `yaml:"enforce,omitempty"` } // EffectiveEnforce returns the enforcement mode for a step, falling back -// to the workflow-level setting when the step does not override it. -// Callers should use this instead of reading step.Enforce directly so -// that the fall-through is consistent everywhere state transitions. -func EffectiveEnforce(wf *Workflow, step *WfStep) string { - if step != nil && step.Enforce != "" { +// to the workflow-level setting when the step does not override it, and +// to "hard" when neither is set. Callers must use this instead of +// reading step.Enforce directly so the fall-through is consistent at +// every state transition. Takes values (not pointers) so the compiler +// enforces that both fields exist at the call site — every current +// caller owns concrete structs by the time they reach a transition. +func EffectiveEnforce(wf Workflow, step WfStep) string { + if step.Enforce != "" { return step.Enforce } - if wf != nil && wf.Enforce != "" { + if wf.Enforce != "" { return wf.Enforce } return "hard" @@ -172,11 +176,20 @@ func validate(wf *Workflow) error { return fmt.Errorf("step %q has invalid expect %q — must be \"success\" or \"failure\"", s.ID, s.Expect) } // Step-level enforce override: empty inherits from workflow, - // otherwise must be hard|soft. Command steps get guarded - // differently (engine runs them), but allow the field anyway - // for symmetry — it's a no-op there rather than a parse error. - if s.Enforce != "" && s.Enforce != "hard" && s.Enforce != "soft" { - return fmt.Errorf("step %q has invalid enforce %q — must be \"hard\" or \"soft\"", s.ID, s.Enforce) + // otherwise must be hard|soft. Reject on command steps — the + // guard honors SessionState.Enforce uniformly (see guard.go's + // command branch), so marking a command step `soft` would let + // arbitrary agent tool calls slip through while the engine is + // executing that step. Since command steps are engine-owned + // and never need per-step overrides, fail loudly at parse time + // instead of producing a sharp edge at runtime. + if s.Enforce != "" { + if s.Enforce != "hard" && s.Enforce != "soft" { + return fmt.Errorf("step %q has invalid enforce %q — must be \"hard\" or \"soft\"", s.ID, s.Enforce) + } + if s.Command != "" { + return fmt.Errorf("step %q has enforce on a command step — enforce is only meaningful for prompt steps (the engine executes command steps directly)", s.ID) + } } if s.Command != "" && s.Loop != nil { return fmt.Errorf("step %q has both command and loop — these are mutually exclusive", s.ID) diff --git a/src/mcp/tools.go b/src/mcp/tools.go index 6a965a3..157b31b 100644 --- a/src/mcp/tools.go +++ b/src/mcp/tools.go @@ -160,12 +160,10 @@ func (s *Server) startTool() (mcpmcp.Tool, mcpgo.ToolHandlerFunc) { CurrentIndex: 0, TotalSteps: len(wf.Steps), StepType: stepType(firstStep), - // Per-step enforce override falls back to workflow level; - // SessionState.Enforce is always the *current step's* - // effective enforce, re-derived on every transition so - // the hook sees the right value without needing to know - // the step index. - Enforce: engine.EffectiveEnforce(wf, &firstStep), + // SessionState.Enforce is the current step's effective + // enforce, re-derived on every transition so the hook + // always reads the mode that matches state.CurrentStep. + Enforce: engine.EffectiveEnforce(*wf, firstStep), Branch: wf.BranchMode, Status: "starting", StartedAt: time.Now(), @@ -483,7 +481,7 @@ func (s *Server) advanceTool() (mcpmcp.Tool, mcpgo.ToolHandlerFunc) { state.CurrentStep = nextStep.ID state.CurrentIndex = nextIndex state.StepType = stepType(nextStep) - state.Enforce = engine.EffectiveEnforce(wf, &nextStep) + state.Enforce = engine.EffectiveEnforce(*wf, nextStep) state.Busy = false if err := lib.WriteSessionJSON(s.dataDir, state); err != nil { return mcpmcp.NewToolResultError(fmt.Sprintf("write state: %v", err)), nil @@ -648,6 +646,10 @@ func (s *Server) handleLoopAdvance(ctx context.Context, wf *engine.Workflow, sta // Continue loop — return same step for another iteration. // Clear the advance claim as part of this write (see advanceTool). + // state.Enforce is NOT re-derived: loop iterations stay on the same + // step, so the step's effective enforce does not change. If that + // invariant ever breaks (e.g. enforce becomes iteration-dependent), + // recompute here like the other transition sites do. state.Busy = false if err := lib.WriteSessionJSON(s.dataDir, state); err != nil { return mcpmcp.NewToolResultError(fmt.Sprintf("write loop state: %v", err)), nil @@ -671,7 +673,7 @@ func (s *Server) advancePastLoop(wf *engine.Workflow, state *lib.SessionState) ( state.CurrentStep = nextStep.ID state.CurrentIndex = nextIndex state.StepType = stepType(nextStep) - state.Enforce = engine.EffectiveEnforce(wf, &nextStep) + state.Enforce = engine.EffectiveEnforce(*wf, nextStep) state.Busy = false if err := lib.WriteSessionJSON(s.dataDir, state); err != nil { return mcpmcp.NewToolResultError(fmt.Sprintf("write state: %v", err)), nil diff --git a/src/mcp/tools_test.go b/src/mcp/tools_test.go index 631d7ca..f543e22 100644 --- a/src/mcp/tools_test.go +++ b/src/mcp/tools_test.go @@ -657,6 +657,207 @@ steps: } } +// TestAdvancePropagatesStepEnforceReverseOverride verifies the symmetric +// case: a workflow that defaults to soft but has an individual step +// marked hard. Ensures the per-step override works both directions +// through the advance path, and that a soft-default workflow's hard +// step still triggers the mid-step tool block from PR #64. +func TestAdvancePropagatesStepEnforceReverseOverride(t *testing.T) { + wfDir := t.TempDir() + dataDir := t.TempDir() + + writeFile(t, filepath.Join(wfDir, "reverse.yml"), `name: reverse +description: Soft default with hard override +enforce: soft +steps: + - id: collect + prompt: Collect evidence, can run shell. + - id: review + prompt: Pure reasoning, no shell. + enforce: hard + - id: write + prompt: Write the result. +`) + + srv := newTestServer(t, dataDir, wfDir) + _, startHandler := srv.startTool() + startReq := mcpmcp.CallToolRequest{} + startReq.Params.Arguments = map[string]interface{}{ + "workflow": "reverse", + "input": "demo", + } + if _, err := startHandler(context.Background(), startReq); err != nil { + t.Fatalf("start: %v", err) + } + + state, _ := lib.ReadSessionJSON(dataDir) + if state == nil { + t.Fatal("no session after start") + } + if state.Enforce != "soft" { + t.Errorf("step 1 (collect) enforce = %q, want soft (inherited from soft default)", state.Enforce) + } + sessionID := state.ID + + _, advHandler := srv.advanceTool() + advance := func(output string) { + t.Helper() + req := mcpmcp.CallToolRequest{} + req.Params.Arguments = map[string]interface{}{"session": sessionID, "output": output} + res, err := advHandler(context.Background(), req) + if err != nil || res.IsError { + t.Fatalf("advance failed: %v", err) + } + } + + advance("collected") + state, _ = lib.ReadSessionJSON(dataDir) + if state.CurrentStep != "review" { + t.Fatalf("expected review, got %s", state.CurrentStep) + } + if state.Enforce != "hard" { + t.Errorf("step 2 (review) enforce = %q, want hard (per-step override flips soft→hard)", state.Enforce) + } + + advance("reviewed") + state, _ = lib.ReadSessionJSON(dataDir) + if state.CurrentStep != "write" { + t.Fatalf("expected write, got %s", state.CurrentStep) + } + if state.Enforce != "soft" { + t.Errorf("step 3 (write) enforce = %q, want soft (back to inherited default)", state.Enforce) + } +} + +// TestAdvancePropagatesStepEnforceAfterLoop exercises the +// advancePastLoop path: a loop step exits (via max iterations), and the +// following step has an explicit per-step enforce override. Without +// state.Enforce re-derivation in advancePastLoop, the post-loop step +// would carry the loop step's enforce and the hook would apply the +// wrong mode. +func TestAdvancePropagatesStepEnforceAfterLoop(t *testing.T) { + wfDir := t.TempDir() + dataDir := t.TempDir() + + writeFile(t, filepath.Join(wfDir, "loop-enforce.yml"), `name: loop-enforce +description: Loop step followed by a step with explicit enforce override +steps: + - id: iterate + prompt: Run one iteration. + enforce: soft + loop: + max: 2 + - id: wrapup + prompt: Wrap up after the loop. +`) + + srv := newTestServer(t, dataDir, wfDir) + _, startHandler := srv.startTool() + startReq := mcpmcp.CallToolRequest{} + startReq.Params.Arguments = map[string]interface{}{ + "workflow": "loop-enforce", + "input": "demo", + } + if _, err := startHandler(context.Background(), startReq); err != nil { + t.Fatalf("start: %v", err) + } + + state, _ := lib.ReadSessionJSON(dataDir) + if state == nil { + t.Fatal("no session after start") + } + if state.CurrentStep != "iterate" || state.Enforce != "soft" { + t.Errorf("expected iterate/soft, got %s/%s", state.CurrentStep, state.Enforce) + } + sessionID := state.ID + + _, advHandler := srv.advanceTool() + advance := func() { + t.Helper() + req := mcpmcp.CallToolRequest{} + req.Params.Arguments = map[string]interface{}{"session": sessionID, "output": "one iteration done"} + if _, err := advHandler(context.Background(), req); err != nil { + t.Fatalf("advance: %v", err) + } + } + + advance() // iteration 1/2 + state, _ = lib.ReadSessionJSON(dataDir) + if state.CurrentStep != "iterate" || state.Enforce != "soft" { + t.Errorf("mid-loop iter1: got %s/%s, want iterate/soft", state.CurrentStep, state.Enforce) + } + + advance() // iteration 2/2 — loop hits max, advancePastLoop fires + state, _ = lib.ReadSessionJSON(dataDir) + if state == nil { + t.Fatal("session gone after loop exit") + } + if state.CurrentStep != "wrapup" { + t.Fatalf("expected wrapup after loop exit, got %s", state.CurrentStep) + } + if state.Enforce != "hard" { + t.Errorf("wrapup enforce = %q, want hard (wrapup has no override, workflow default is hard — advancePastLoop must re-derive)", state.Enforce) + } +} + +// TestAdvancePropagatesStepEnforceAcrossBranch exercises the branch +// jump path in advanceTool: a step with a `branch:` clause that routes +// past sequential steps to a target with a different per-step enforce. +// Guards against a future refactor that computes enforce from +// CurrentIndex+1 instead of the branch target. +func TestAdvancePropagatesStepEnforceAcrossBranch(t *testing.T) { + wfDir := t.TempDir() + dataDir := t.TempDir() + + writeFile(t, filepath.Join(wfDir, "branch-enforce.yml"), `name: branch-enforce +description: Branch skipping a sequential step with a different enforce +steps: + - id: classify + prompt: Output TARGET exactly. + branch: + - when: TARGET + goto: jump-target + - id: skipped + prompt: Should be skipped. + enforce: soft + - id: jump-target + prompt: Reached via branch. + enforce: soft +`) + + srv := newTestServer(t, dataDir, wfDir) + _, startHandler := srv.startTool() + startReq := mcpmcp.CallToolRequest{} + startReq.Params.Arguments = map[string]interface{}{ + "workflow": "branch-enforce", + "input": "demo", + } + if _, err := startHandler(context.Background(), startReq); err != nil { + t.Fatalf("start: %v", err) + } + + state, _ := lib.ReadSessionJSON(dataDir) + if state.Enforce != "hard" { + t.Errorf("classify enforce = %q, want hard (inherited)", state.Enforce) + } + sessionID := state.ID + + _, advHandler := srv.advanceTool() + req := mcpmcp.CallToolRequest{} + req.Params.Arguments = map[string]interface{}{"session": sessionID, "output": "TARGET"} + if _, err := advHandler(context.Background(), req); err != nil { + t.Fatalf("advance: %v", err) + } + + state, _ = lib.ReadSessionJSON(dataDir) + if state.CurrentStep != "jump-target" { + t.Fatalf("expected branch to jump-target, got %s", state.CurrentStep) + } + if state.Enforce != "soft" { + t.Errorf("jump-target enforce = %q, want soft (per-step override on branch target, not on skipped step)", state.Enforce) + } +} + func TestAdvanceCommandStep(t *testing.T) { wfDir := t.TempDir() dataDir := t.TempDir() diff --git a/workflows/audit.yml b/workflows/audit.yml index 01a8e78..6921d6d 100644 --- a/workflows/audit.yml +++ b/workflows/audit.yml @@ -11,7 +11,7 @@ steps: - id: deps model: smart - enforce: soft # runs npm audit / govulncheck / pip-audit / cargo audit + enforce: soft prompt: | Detected ecosystems: {{detect}} @@ -25,7 +25,7 @@ steps: - id: lint model: smart - enforce: soft # runs eslint / golangci-lint / ruff / clippy + enforce: soft prompt: | Detected ecosystems: {{detect}} diff --git a/workflows/autoloop.yml b/workflows/autoloop.yml index e9ba2fa..74f3b6e 100644 --- a/workflows/autoloop.yml +++ b/workflows/autoloop.yml @@ -78,7 +78,7 @@ steps: - id: measure model: fast - enforce: soft # re-runs the metric command + enforce: soft prompt: | Run the EXACT same metric command from the baseline to measure the result. diff --git a/workflows/bugfix.yml b/workflows/bugfix.yml index 11c420b..ce495a1 100644 --- a/workflows/bugfix.yml +++ b/workflows/bugfix.yml @@ -26,7 +26,7 @@ steps: - id: reproduce model: smart - enforce: soft # "run the failing case" requires Bash + enforce: soft # may run the failing case to confirm the bug prompt: | Bug report: {{input}} diff --git a/workflows/deep-research.yml b/workflows/deep-research.yml index 35f3e9c..38310d4 100644 --- a/workflows/deep-research.yml +++ b/workflows/deep-research.yml @@ -60,7 +60,7 @@ steps: - id: search-1 model: general - enforce: soft # WebSearch + enforce: soft prompt: | Execute web search for sub-questions 1-2 from this decomposition: {{decompose}} @@ -69,7 +69,7 @@ steps: - id: search-2 model: general - enforce: soft # WebSearch + enforce: soft prompt: | Execute web search for sub-questions 3-4 from this decomposition: {{decompose}} @@ -78,7 +78,7 @@ steps: - id: search-3 model: general - enforce: soft # WebSearch + enforce: soft prompt: | Execute web search for sub-questions 5+ from this decomposition: {{decompose}} diff --git a/workflows/pr-ready.yml b/workflows/pr-ready.yml index f409b0b..6019cab 100644 --- a/workflows/pr-ready.yml +++ b/workflows/pr-ready.yml @@ -4,7 +4,7 @@ description: Full PR preparation pipeline — validate, drop unrelated changes, steps: - id: validate model: fast - enforce: soft # runs git status / git log + enforce: soft # inspects branch state via git prompt: | Validate the current branch is ready for PR preparation: - Not on main/master @@ -64,7 +64,7 @@ steps: - id: doc-check model: smart - enforce: soft # commits docs edits when the rule is mechanical + enforce: soft # edits doc files (and commits them) prompt: | Classify the diff (git diff main...HEAD) and decide which project docs need updating so the user does not have to ask every PR. diff --git a/workflows/research.yml b/workflows/research.yml index fcf5489..4c00f80 100644 --- a/workflows/research.yml +++ b/workflows/research.yml @@ -37,7 +37,7 @@ steps: - id: search-1 model: general - enforce: soft # WebSearch + enforce: soft prompt: | Execute web search for the FIRST sub-question from this decomposition: {{decompose}} @@ -46,7 +46,7 @@ steps: - id: search-2 model: general - enforce: soft # WebSearch + enforce: soft prompt: | Execute web search for the SECOND sub-question from this decomposition: {{decompose}} @@ -55,7 +55,7 @@ steps: - id: search-3 model: fast - enforce: soft # WebSearch + enforce: soft prompt: | Execute web search for the THIRD sub-question from this decomposition: {{decompose}}