fix: bootstrap engine binary on first run via committed wrapper

.claude-plugin/plugin.json

@@ -1,6 +1,6 @@
 {
   "name": "devkit",
-  "version": "2.1.1",
+  "version": "2.1.2",
   "description": "A deterministic development harness for AI agents — YAML workflow engine, self-learning hooks, and multi-agent consensus",
   "author": {
     "name": "5uck1ess"

.github/workflows/ci.yml

@@ -47,6 +47,61 @@ jobs:
       - name: Run hook smoke tests
         run: bash hooks/hooks_test.sh
 
+  # Catches the class of bug where the shipped marketplace tree is
+  # missing something the plugin needs at runtime. PR #52 shipped a
+  # plugin.json pointing at bin/devkit, but bin/ was gitignored — the
+  # plugin installed but the MCP server silently failed to start.
+  # This job runs in a clean dir with no local build artifacts.
+  fresh-install-smoke:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Bootstrap wrapper must exist and be executable
+        run: |
+          test -f bin/devkit || { echo "FATAL: bin/devkit wrapper missing from repo"; exit 1; }
+          test -x bin/devkit || { echo "FATAL: bin/devkit not executable"; exit 1; }
+
+      - name: Wrapper must not contain devkit-engine binary (that is gitignored)
+        run: |
+          if [ -e bin/devkit-engine ]; then
+            echo "FATAL: bin/devkit-engine should not be tracked in git (it is the cached binary)"
+            exit 1
+          fi
+
+      - name: plugin.json MCP command must match wrapper path
+        run: |
+          cmd=$(jq -r '.mcpServers["devkit-engine"].command' .claude-plugin/plugin.json)
+          expected='${CLAUDE_PLUGIN_ROOT}/bin/devkit'
+          if [ "$cmd" != "$expected" ]; then
+            echo "FATAL: plugin.json mcpServers.devkit-engine.command is '$cmd', expected '$expected'"
+            exit 1
+          fi
+
+      - name: shellcheck wrapper
+        run: |
+          sudo apt-get update -qq && sudo apt-get install -y -qq shellcheck
+          shellcheck bin/devkit
+
+      - name: Wrapper --help must exit with a download attempt (no local engine)
+        run: |
+          # With no local bin/devkit-engine and no network cache, the wrapper
+          # should try to download and either succeed or fail loudly. It must
+          # NOT silently pass with exit 0 and no output.
+          set +e
+          output=$(./bin/devkit --version 2>&1)
+          exit_code=$?
+          set -e
+          echo "wrapper exit=$exit_code"
+          echo "wrapper output: $output"
+          # The wrapper must either succeed (downloaded + ran --version) or
+          # emit a clear error. An empty output with exit 0 would be the silent
+          # failure class we're guarding against.
+          if [ $exit_code -eq 0 ] && [ -z "$output" ]; then
+            echo "FATAL: wrapper exited 0 with no output — silent failure class bug"
+            exit 1
+          fi
+
   validate-counts:
     runs-on: ubuntu-latest
     steps:

.gitignore

@@ -1,5 +1,8 @@
-# Compiled binaries
-bin/
+# Compiled engine binaries (wrapper bin/devkit is committed)
+bin/devkit-engine
+bin/devkit-engine-*
+bin/.checksums.*
+src/bin/
 
 # Credentials — never commit
 *.token

CHANGELOG.md

@@ -1,5 +1,36 @@
 # Changelog
 
+## 2.1.2
+
+### Make the MCP engine actually work + tighten enforcement (PR #54)
+
+Ships the critical fix that 2.1.0 missed, plus the determinism gaps that a full mega-pr review on #52 uncovered. Restores the engine to a state where step enforcement is not bypassable and command steps are not shell-injectable.
+
+#### Fixed (CRITICAL)
+- **Engine binary now actually ships with the plugin.** 2.1.0/2.1.1 set `mcpServers.devkit-engine.command` to `${CLAUDE_PLUGIN_ROOT}/bin/devkit` but `bin/` was gitignored, so the marketplace install had no binary. MCP server silently failed to start and every workflow silently fell back to direct agent dispatch. Fixed by committing a POSIX shell wrapper at `bin/devkit` that downloads the matching release asset on first run, verifies SHA256, caches it next to itself, and execs it. Local dev builds (`make install-plugin`) are used directly via a fast path.
+- **Shell injection in command steps.** `runCommand` interpolated `{{input}}` and `{{step-id}}` into `sh -c <command>`. LLM-chosen input or contaminated prior-step output could execute arbitrary shell. Fixed: command and gate strings are now literal YAML; values are passed via `$DEVKIT_INPUT` and `$DEVKIT_OUT_<step_id>` env vars. Workflow validator rejects `{{...}}` in command/gate strings at parse time with an explicit "shell injection mitigation" error. The 5 self-* workflows were updated to use `$DEVKIT_INPUT`.
+- **PreToolUse guard was trivially bypassable.** The blocklist only matched 11 hardcoded tool names (Bash, Edit, Write, ...) and missed Task, SlashCommand, TodoWrite, mcp__*, and any new tools Claude Code adds. Fixed: guard now uses an allowlist — during command steps, only `mcp__devkit*` tools and `TodoWrite` are permitted. Everything else returns exit 2.
+- **Stop guard failed open on corrupt session.json, guard failed closed.** Opposite policies on the same condition meant workflow completion could be silently approved on a corrupt state file. Fixed: both hooks now fail closed with a clear diagnostic when JSON parsing fails.
+- **Loop `until` matched substrings.** `strings.Contains("no failures found", "fail")` returned true, silently terminating loops. Same bug in branch `when:` matching. Fixed: both now use word-boundary matching (grep -w semantics) — `fail` won't match `failures`.
+
+#### Fixed (HIGH)
+- `completeWorkflow` ignored `git.CommitAll` errors — users saw "WORKFLOW COMPLETE" while their branch work was unpersisted. Now surfaces warnings in the result text and on stderr.
+- `runCommand` threw away command output on non-`*exec.ExitError` failures. Now returns the combined stream alongside the error, and distinguishes `context.DeadlineExceeded` (5-minute timeout) from "command crashed" with exit 124.
+- Hook fail-silent when `CLAUDE_PLUGIN_DATA` is unset. Now emits a stderr warning so the degraded state is observable.
+- TOCTOU race between hook `[[ -f session.json ]]` check and python `open()`: if the engine cleared the file between those two, hooks spuriously failed closed. Python code now handles `FileNotFoundError` explicitly as "no session".
+
+#### Fixed (MEDIUM)
+- YAML workflow parsing now uses `KnownFields(true)` — typos like `commnd:` fail loudly instead of silently dropping the field.
+- `make sync-version` no longer downgrades `plugin.json`. It only writes when the git tag is strictly higher than the current version, which prevents local dev builds on feature branches from clobbering manual version bumps.
+- `bin/devkit` wrapper emits `devkit:` diagnostics to stderr on download, checksum, install, and failure paths. Stdout stays clean so it doesn't corrupt the MCP stdio protocol.
+
+#### Added
+- **CI `fresh-install-smoke` job.** Runs on every PR in a clean checkout. Asserts `bin/devkit` exists and is executable, that `bin/devkit-engine` is not tracked, that `plugin.json`'s MCP command matches the wrapper path, runs `shellcheck bin/devkit`, and runs `./bin/devkit --version` to confirm it either succeeds or fails loudly — never exits 0 with empty output. This is the check that would have caught the v2.1.0 binary-shipping bug before merge.
+- Regression tests: `TestRunWorkflowCommandRejectsInterpolation`, `TestRunWorkflowCommandEnvInput`, `TestRunWorkflowCommandEnvPriorOutput`, `TestLoopUntilRejectsSubstring`.
+
+#### Removed
+- `presets/` — empty v1 directory with only `.gitkeep`, no references anywhere.
+
 ## 2.1.0
 
 ### MCP Engine — Deterministic Workflow Enforcement (PR #52)
@@ -21,10 +52,10 @@ Replaces the broken subprocess-spawning engine with an MCP server that runs insi
 - **Enforcement** — None (markdown honor system) → MCP tool scoping + PreToolUse exit 2
 - **Principle skills** — Loaded if Claude decided to → injected by engine per step
 - **Token usage** — ~50k+ for 8-step workflow → ~17k (~65% reduction)
-- **Skills and commands** — All 8 entry points (research, deep-research, autoloop, tri-review, tri-debug, tri-security, pr-ready, status) now use MCP tools instead of `ensure-engine.sh` + `devkit workflow run`
+- **Skills and commands** — All 8 entry points (research, deep-research, autoloop, tri-review, tri-debug, tri-security, pr-ready, status) now use MCP tools instead of `ensure-engine.sh` + `devkit workflow`
 
 #### Removed
-- `scripts/ensure-engine.sh` — no longer needed (binary ships in `bin/`, auto-PATH)
+- `scripts/ensure-engine.sh` — no longer needed; replaced by the committed `bin/devkit` wrapper in 2.1.2 (see below), which downloads the engine binary on first run and caches it next to itself. In 2.1.0 the wrapper was missing entirely, which shipped a broken plugin — 2.1.2 fixes that and adds CI smoke tests to prevent regression.
 - `scripts/install-engine.sh` — installed by plugin manifest
 
 #### Fixed
@@ -37,7 +68,7 @@ Replaces the broken subprocess-spawning engine with an MCP server that runs insi
 Major architectural shift: all command logic moved from LLM-interpreted markdown to Go-engine-driven YAML workflows.
 
 #### Changed
-- **24 → 8 slash commands** — 16 commands deleted, logic now in YAML workflows invoked via `devkit workflow run <name>` or context-activated skills
+- **24 → 8 slash commands** — 16 commands deleted, logic now in YAML workflows invoked via `devkit workflow <name> "<description>"` or context-activated skills
 - **4 ultra-thin wrappers** — tri-review, tri-debug, tri-security, pr-ready (one-liner pointing to workflow)
 - **4 kept as-is** — pr-monitor, status, setup-rules, workflow
 - **~3,600 lines removed** across PRs 1–5

CONTRIBUTING.md

@@ -5,7 +5,7 @@
 Most command logic lives in YAML workflows executed by the Go engine. Only 6 slash commands remain as tab-completable entry points — everything else is context-activated via skills.
 
 1. Create `workflows/my-workflow.yml` with steps, model assignments, and loop/gate definitions
-2. Test with `devkit_start` MCP tool or `devkit workflow run my-workflow "input"` from terminal
+2. Test with `devkit_start` MCP tool or `devkit workflow my-workflow "input"` from terminal
 3. Optionally add a context-activated skill in `skills/` to auto-trigger it
 
 See `skills/creating-workflows/SKILL.md` for YAML schema reference.

README.md

@@ -244,14 +244,21 @@ Language-specific rules that auto-activate when Claude reads matching files. Ins
 
 ```
 MCP Server (bin/devkit mcp — auto-started by plugin)
+  ├── bin/devkit = POSIX shell wrapper (committed to git)
+  │   └── On first run, downloads matching release asset from GitHub,
+  │       verifies SHA256, caches as bin/devkit-engine-v<ver>-<os>-<arch>,
+  │       then execs it. Local dev builds (make install-plugin) are used
+  │       directly via the fast path.
   ├── Tools: devkit_start, devkit_advance, devkit_status, devkit_list
   ├── State: session.json (hot, <50ms reads) + SQLite (cold history)
   ├── Parse YAML → validate steps, branches, budget
   ├── Walk steps:
   │   ├── Command steps → engine executes shell directly ($0 cost)
+  │   │   Values passed via $DEVKIT_INPUT / $DEVKIT_OUT_<step_id>
+  │   │   env vars — never interpolated into the command string.
   │   ├── Prompt steps → Claude works, calls devkit_advance when done
   │   ├── Loop with gate → run, verify, keep or revert
-  │   ├── Branch → case-insensitive substring match → goto
+  │   ├── Branch → case-insensitive word-boundary match → goto
   │   └── Parallel → Agent tool dispatch (Claude/Codex/Gemini)
   └── Principles injected per step (~120 tokens, not full skill files)
 
@@ -260,7 +267,7 @@ Enforcement:
   ├── PreToolUse hook — exit 2 blocks tools during command steps
   └── Stop hook — blocks session end during active workflows
 
-Terminal fallback (devkit workflow run <name>):
+Terminal usage (devkit workflow <name> "<description>"):
   └── Subprocess runners for Codex/Gemini CLI usage
 ```
 
@@ -282,6 +289,6 @@ devkit/
 │   ├── runners/       # Codex, Gemini interfaces (terminal fallback)
 │   ├── lib/           # DB, git, metrics, session state, reporting
 │   └── cmd/           # CLI entry points (including `devkit mcp`)
-├── bin/               # Auto-PATH binary (built by make install-plugin)
+├── bin/               # devkit wrapper (committed) + downloaded engine binaries (gitignored)
 └── .github/workflows/ # CI (build+test+vet) + auto-release (6 platforms)
 ```

ROADMAP.md

@@ -36,6 +36,6 @@ Items below were on the roadmap but determined to be unnecessary — either alre
 | Stop hook redesign | Still fires every turn, but exits early with `approve` when no files are changed — near-instant on clean trees, so the performance concern is moot. Revisit only if it causes measurable latency. |
 | Cost event hooks | Budget enforcement already exists in the Go engine via `overBudget()` + `addCost()` callbacks with hard limits |
 | Execution registry | Step tracking already handled by SQLite via `lib.DB` with status, cost, and timing per step |
-| Preset library | The 18 YAML workflows and 19 skills already serve this purpose |
+| Preset library | The 18 YAML workflows and 22 skills already serve this purpose |
 | Framework-specific review checklists | `lang-review.sh` covers language-level patterns; framework-specific rules are better added per-project via hookify |
 | Conditional hook firing | Hooks already self-filter internally (extension checks, changed-file checks); a generic condition system adds complexity for no current need |

bin/devkit

@@ -0,0 +1,137 @@
+#!/bin/sh
+# devkit wrapper — resolves and execs the platform engine binary.
+#
+# The real Go binary is not committed to git (platform-specific, ~10MB).
+# This wrapper downloads the matching release asset from GitHub on first
+# run, verifies its SHA256, caches it alongside this script, and execs it.
+#
+# All log output goes to stderr. Stdout stays clean because the MCP stdio
+# protocol runs over it when Claude Code invokes `devkit mcp`.
+
+set -eu
+
+SCRIPT_DIR=$(CDPATH='' cd -- "$(dirname -- "$0")" && pwd)
+PLUGIN_ROOT=$(CDPATH='' cd -- "$SCRIPT_DIR/.." && pwd)
+PLUGIN_JSON="$PLUGIN_ROOT/.claude-plugin/plugin.json"
+RELEASE_OWNER="5uck1ess"
+RELEASE_REPO="devkit"
+
+log() { printf 'devkit: %s\n' "$*" >&2; }
+die() { log "$*"; exit 1; }
+
+# Fast path: developer-built binary next to this script (from `make install-plugin`).
+LOCAL_DEV="$SCRIPT_DIR/devkit-engine"
+if [ -x "$LOCAL_DEV" ]; then
+    exec "$LOCAL_DEV" "$@"
+fi
+
+# Sets VERSION (global) so die() kills the parent shell on failure.
+read_version() {
+    [ -f "$PLUGIN_JSON" ] || die "plugin.json not found at $PLUGIN_JSON"
+    VERSION=$(sed -n 's/.*"version"[[:space:]]*:[[:space:]]*"\([^"]*\)".*/\1/p' "$PLUGIN_JSON" | head -1)
+    [ -n "$VERSION" ] || die "could not read version from $PLUGIN_JSON"
+}
+
+# Sets PLATFORM (global) so die() kills the parent shell on failure.
+detect_platform() {
+    uname_s=$(uname -s 2>/dev/null || echo unknown)
+    uname_m=$(uname -m 2>/dev/null || echo unknown)
+    case "$uname_s" in
+        Linux)  os=linux ;;
+        Darwin) os=darwin ;;
+        MINGW*|MSYS*|CYGWIN*) os=windows ;;
+        *) die "unsupported OS: $uname_s" ;;
+    esac
+    case "$uname_m" in
+        x86_64|amd64)  arch=amd64 ;;
+        arm64|aarch64) arch=arm64 ;;
+        *) die "unsupported arch: $uname_m" ;;
+    esac
+    PLATFORM="${os}-${arch}"
+}
+
+sha256_of() {
+    f=$1
+    if command -v sha256sum >/dev/null 2>&1; then
+        sha256sum "$f" | awk '{print $1}'
+    elif command -v shasum >/dev/null 2>&1; then
+        shasum -a 256 "$f" | awk '{print $1}'
+    else
+        die "need sha256sum or shasum to verify downloads"
+    fi
+}
+
+download() {
+    url=$1
+    out=$2
+    if command -v curl >/dev/null 2>&1; then
+        curl -fsSL --retry 2 --retry-delay 2 -o "$out" "$url"
+    elif command -v wget >/dev/null 2>&1; then
+        wget -q -O "$out" "$url"
+    else
+        die "need curl or wget to download engine binary"
+    fi
+}
+
+# ensure_engine sets ENGINE_PATH as a side effect rather than echoing it,
+# so `die` inside this function can `exit 1` on the parent shell. Command
+# substitution would run it in a subshell, hiding the failure exit code.
+ensure_engine() {
+    read_version
+    detect_platform
+    ext=""
+    case "$PLATFORM" in windows-*) ext=".exe" ;; esac
+
+    engine_name="devkit-engine-v${VERSION}-${PLATFORM}${ext}"
+    ENGINE_PATH="$SCRIPT_DIR/$engine_name"
+
+    if [ -x "$ENGINE_PATH" ]; then
+        return 0
+    fi
+
+    # Remove stale cached engines from old versions in the same directory.
+    find "$SCRIPT_DIR" -maxdepth 1 -name 'devkit-engine-v*' ! -name "$engine_name" \
+        -type f -exec rm -f {} + 2>/dev/null || true
+
+    tag="v${VERSION}"
+    asset="devkit-${PLATFORM}${ext}"
+    base_url="https://github.com/${RELEASE_OWNER}/${RELEASE_REPO}/releases/download/${tag}"
+
+    log "first-run: downloading engine ${tag} (${PLATFORM})…"
+
+    # Per-invocation temp dir so two simultaneous first runs never
+    # share tmp paths. Each process cleans up its own dir on exit;
+    # the final mv(2) is atomic within a filesystem, so whichever
+    # writer lands last produces the correct (bit-identical) binary.
+    tmp_dir=$(mktemp -d "${TMPDIR:-/tmp}/devkit-engine-XXXXXX") \
+        || die "mktemp -d failed"
+    tmp_bin="$tmp_dir/${engine_name}"
+    tmp_sums="$tmp_dir/checksums.txt"
+    trap 'rm -rf "$tmp_dir"' EXIT INT TERM
+
+    download "${base_url}/${asset}" "$tmp_bin" \
+        || die "download failed: ${base_url}/${asset}"
+    download "${base_url}/checksums.txt" "$tmp_sums" \
+        || die "checksum file download failed from ${base_url}/checksums.txt"
+
+    expected=$(awk -v name="$asset" '$2 == name || $2 == "*"name { print $1; exit }' "$tmp_sums")
+    [ -n "$expected" ] || die "no checksum entry for $asset in release $tag"
+
+    actual=$(sha256_of "$tmp_bin")
+    [ "$actual" = "$expected" ] \
+        || die "checksum mismatch for $asset (expected $expected, got $actual)"
+
+    chmod +x "$tmp_bin"
+    mv -f "$tmp_bin" "$ENGINE_PATH"
+    rm -rf "$tmp_dir"
+    trap - EXIT INT TERM
+
+    log "installed engine at $ENGINE_PATH"
+}
+
+ENGINE_PATH=""
+ensure_engine
+if [ -z "$ENGINE_PATH" ] || [ ! -x "$ENGINE_PATH" ]; then
+    die "engine path not resolved"
+fi
+exec "$ENGINE_PATH" "$@"