Add generic YAML workflow engine - #14
Merged
Merged
Conversation
…bit hook
Replaces the need for hardcoded Go workflow implementations with a single
engine that reads and executes workflow YAML files deterministically —
zero tokens spent on orchestration, exact loop counting, and reliable
branch evaluation.
Engine (src/engine/):
- YAML parser with validation (types, duplicate IDs, branch targets)
- Sequential step execution with {{variable}} interpolation
- Branch evaluation (case-insensitive substring match → goto)
- Loop management (hard counter + until-string match)
- Parallel step dispatch via goroutines
- Budget enforcement, scratchpad setup/cleanup, session tracking
CLI: `devkit workflow <name> <description>` and `devkit workflow list`
Triage-based phase skipping:
- feature.yml: TINY/SMALL/MEDIUM/LARGE classification with fast paths
- bugfix.yml: TRIVIAL/NORMAL/COMPLEX classification with fast path
Scratchpads (skills/scratchpad/):
- Iteration memory protocol at .devkit/scratchpads/current.md
- Prevents Groundhog Day loops by recording what was tried
- Integrated into stuck skill recovery protocol
Dirty-bit feedback loop (hooks/dirty-bit.sh):
- Stop hook that detects cross-domain changes (backend/frontend/config/sql)
- Blocks completion if any touched domain lacks test evidence
18 engine tests covering parsing, interpolation, branching, loops,
budget, parallel dispatch, context cancellation, and all 12 real workflows.
…tion Fixes from tri-review (Claude + Codex + Gemini consensus): 1. Fix nil deref on runner error in runLoop — no longer accesses result.CostUSD when err != nil 2. Fix data race on outputs map in runParallel — snapshot outputs before launching goroutines 3. Fix session status overwrite — track failed state, only mark "done" on clean exit 4. Fix silent loop failure — runLoop returns error when all iterations fail 5. Fix parallel error swallowing — runParallel returns error when ALL parallel steps fail 6. Add branch cycle detection — max 100 jumps before stopping Additional fixes: - Skip parallel-dispatched steps during sequential walk (no double execution) - Evaluate branch conditions after loop steps (not just regular steps) - Wire YAML budget config to RunConfig when CLI flag not set - Propagate step errors from RunWorkflow return value 2 new tests: TestRunWorkflowLoopAllFail, TestRunWorkflowBranchCycleLimit
Validates workflow name against ^[a-zA-Z0-9_-]+$ before constructing file paths, preventing directory traversal via crafted names like "../../etc/passwd". Found by Gemini in tri-review.
New hook (go-review.sh): - Detects error-path result access (accessing fields when err != nil) - Flags goroutines with shared map access missing mutex/snapshot - Warns on filepath.Join with unsanitized input variables Registered as PostToolUse for *.go Edit/Write operations. security-patterns.sh: - Add filepath traversal pattern for Go (filepath.Join with user input) subagent-stop.sh: - Recognize go vet / go test -race as valid test evidence These patterns are the top recurring LLM-generated bug categories identified from the tri-review of the workflow engine PR.
From PR review toolkit analysis: silent-failure-hunter: - Fix missing stepErr on non-loop branch limit path - Check os.MkdirAll error for scratchpad dir - Explicit _ = os.Remove for intentional discard pr-test-analyzer: - Add budget enforcement inside runLoop (overBudget + addCost callbacks) - Add TestRunWorkflowStepFailure (non-loop step error propagation) - Add TestRunWorkflowParallelPartialFailure (some fail, some succeed) - Add TestRunWorkflowParallelAllFail (all parallel steps fail) - Add TestRunWorkflowBudgetInLoop (budget respected mid-loop) - Make mock runner thread-safe with sync.Mutex 24 engine tests now pass (was 20).
- Replace grep -P (Perl regex, unavailable on macOS) with awk in go-review.sh for error-path detection pattern - Remove unsupported "if" field from hooks.json go-review entry (hook self-filters via case statement) - Run go mod tidy to promote yaml.v3 from indirect to direct
…etection go-vet-stop.sh (Stop hook): - Runs go vet on modified Go packages before session completes - Runs go test -race to catch data races (would have caught the outputs map race in runParallel) - 90s timeout, only triggers when Go files changed shell-compat.sh (PreToolUse hook): - Flags macOS-incompatible constructs in shell scripts at write-time - Catches: grep -P, sed -i without '', readlink -f, stat --format, xargs -d, date -d, mktemp --suffix - Would have caught the grep -P issue in go-review.sh go-nil-return.sh (PostToolUse hook): - Detects Go functions with error return type that only ever return nil - Uses awk to parse function boundaries and return statements - Would have caught runLoop/runParallel always returning nil error All three hooks learned from bugs found during this PR's review cycle.
Type-design-analyzer recommendations implemented: 1. NewEngine() constructor — validates all 4 fields are non-nil/non-empty, fields unexported to prevent post-construction mutation 2. RunConfig validation — rejects negative BudgetUSD at RunWorkflow entry 3. WfStep mutual exclusion — parallel steps cannot have prompt or loop, enforced at parse time with clear error messages 4. Budget validation — negative budget.limit rejected at parse time 5. Workflow.Validate() method — allows engine-boundary validation for workflows constructed directly (not via Parse) 29 engine tests (was 24): added NewEngine validation, negative budget, parallel+prompt mutual exclusion, parallel+loop mutual exclusion.
The branch evaluation logic was duplicated verbatim between the loop and regular step paths. Extracted into a local evalBranch closure that returns (stepIndex, error), called once after both paths. Removes ~20 lines of duplication while keeping identical behavior. Found by code-simplifier agent.
go-vet-stop.sh: - Replace `timeout` (GNU-only) with `perl -e 'alarm 60; exec @argv'` for macOS compatibility. Ironic that the shell-compat hook exists to catch exactly this. shell-compat.sh: - Fix session dedup: use PPID instead of $$ (each hook invocation is a new process, so $$ gives a unique PID every time, defeating the dedup mechanism). go-nil-return.sh: - Remove dead gsub code on function declaration line. - Properly count all braces on the declaration line (handles inline struct literals) instead of hardcoding brace_depth=1.
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
src/engine/) — deterministic execution of workflow YAML files, replacing the need for hardcoded Go implementations. Zero tokens spent on orchestration: branch evaluation, loop counting, variable interpolation, and budget tracking all happen in Go.feature.ymlandbugfix.ymlnow classify task scope (TINY/SMALL/MEDIUM/LARGE) and skip unnecessary steps. A typo fix no longer runs a 14-step pipeline..devkit/scratchpads/current.md) that prevents Groundhog Day loops by recording what was tried and what failed.New files
src/engine/workflow.go— YAML types, parser, validator, interpolation, branch evalsrc/engine/engine.go—Engine.RunWorkflow()with loops, branches, parallel, budgetsrc/engine/engine_test.go— 18 testssrc/cmd/workflow.go—devkit workflow <name> <description>CLI commandskills/scratchpad/SKILL.md— scratchpad protocolhooks/dirty-bit.sh— cross-domain verification hookTest plan
go test ./...)devkit workflow feature "add X"runs end-to-enddevkit workflow listshows all workflows