From 6fe98e09d9b91611da99b6879f1e82a1fcf30655 Mon Sep 17 00:00:00 2001
From: Roberto Cano <3525807+robercano@users.noreply.github.com>
Date: Fri, 17 Jul 2026 09:32:35 +0200
Subject: [PATCH 1/2] =?UTF-8?q?fix(harness):=20gate=20hygiene=20=E2=80=94?=
 =?UTF-8?q?=20hook=20double-registration,=20preflight=20order,=20test=5Faf?=
 =?UTF-8?q?fected=20default=20(issue=20#129)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Three related gate-hygiene defects:
- SKILL.md/templates/CLAUDE.md now spell out the single-owner rule for the
  Stop/PostToolUse/PreToolUse hooks: once /orchestrator:setup wires them into a
  consumer's own .claude/settings.json (post-#128 vendoring), the orchestrator
  plugin must be disabled for everyday sessions or every gate double-fires each
  turn; the hand-off checklist now makes dropping enabledPlugins/
  extraKnownMarketplaces an explicit step.
- gate.sh's pnpm-lock freshness preflight now runs after the empty-gate skip,
  so a deliberately blanked gate ("" = not configured) stays a true no-op
  instead of failing the Stop hook when node_modules is momentarily stale.
- SKILL.md's test_affected interview guidance and the gates.json template now
  steer towards a cheap changed-scope filter instead of defaulting straight to
  the full test suite, which is unaffordable as a per-turn Stop hook.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
---
 .claude/scripts/gate.sh                   | 36 +++++++++++++----------
 .claude/skills/setup/SKILL.md             | 29 ++++++++++++++----
 .claude/skills/setup/templates/CLAUDE.md  |  5 +++-
 .claude/skills/setup/templates/gates.json |  1 +
 4 files changed, 48 insertions(+), 23 deletions(-)

diff --git a/.claude/scripts/gate.sh b/.claude/scripts/gate.sh
index 8c6e87f..3fbd0ff 100755
--- a/.claude/scripts/gate.sh
+++ b/.claude/scripts/gate.sh
@@ -12,22 +12,6 @@ key="${1:?usage: gate.sh <gate-name>}"
 # shellcheck source=resolve-roots.sh
 . "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/resolve-roots.sh"
 
-# Dependency-freshness preflight (pnpm-gated: no-ops unless the repo uses pnpm).
-# pnpm copies the resolved lockfile to node_modules/.pnpm/lock.yaml on every
-# install, so a byte-diff against the working pnpm-lock.yaml is a fast, offline
-# staleness check. When they differ, node_modules is behind the lockfile (e.g. a
-# merged PR added a dependency) and TS builds fail with opaque "Cannot find
-# module" errors printed to STDOUT — leaving Stop hooks to report an unhelpful
-# "No stderr output". Surface the real cause on STDERR and abort early so the fix
-# ('pnpm install') is obvious. Projects without a root pnpm-lock.yaml skip this.
-if [ -f "$root/pnpm-lock.yaml" ]; then
-  installed_lock="$root/node_modules/.pnpm/lock.yaml"
-  if [ ! -e "$installed_lock" ] || ! cmp -s "$root/pnpm-lock.yaml" "$installed_lock"; then
-    echo "gate.sh: node_modules is out of sync with pnpm-lock.yaml — run 'pnpm install' (gate '$key' aborted)." >&2
-    exit 1
-  fi
-fi
-
 # Which adapter to read. Defaults to the project adapter; set GATES_FILE to run a
 # different one (e.g. GATES_FILE=.claude/self/gates.json for the self-host loop —
 # see .claude/self/README.md). Relative paths resolve from the repo root.
@@ -47,6 +31,26 @@ if [ -z "$cmd" ]; then
   echo "gate.sh: gate '$key' not configured in gates.json — skipping"; exit 0
 fi
 
+# Dependency-freshness preflight (pnpm-gated: no-ops unless the repo uses pnpm).
+# pnpm copies the resolved lockfile to node_modules/.pnpm/lock.yaml on every
+# install, so a byte-diff against the working pnpm-lock.yaml is a fast, offline
+# staleness check. When they differ, node_modules is behind the lockfile (e.g. a
+# merged PR added a dependency) and TS builds fail with opaque "Cannot find
+# module" errors printed to STDOUT — leaving Stop hooks to report an unhelpful
+# "No stderr output". Surface the real cause on STDERR and abort early so the fix
+# ('pnpm install') is obvious. Projects without a root pnpm-lock.yaml skip this.
+# Runs AFTER the empty-gate skip above (issue #129) — a deliberately blanked gate
+# (the documented "not configured" marker) is a no-op and must stay one even when
+# node_modules is momentarily stale; only a gate that's actually about to execute
+# needs a fresh node_modules.
+if [ -f "$root/pnpm-lock.yaml" ]; then
+  installed_lock="$root/node_modules/.pnpm/lock.yaml"
+  if [ ! -e "$installed_lock" ] || ! cmp -s "$root/pnpm-lock.yaml" "$installed_lock"; then
+    echo "gate.sh: node_modules is out of sync with pnpm-lock.yaml — run 'pnpm install' (gate '$key' aborted)." >&2
+    exit 1
+  fi
+fi
+
 echo "▶ gate '$key': $cmd"
 
 # Token hygiene: gate output lands in an agent's context every time a hook fires, so a
diff --git a/.claude/skills/setup/SKILL.md b/.claude/skills/setup/SKILL.md
index 20933fc..ffb7561 100644
--- a/.claude/skills/setup/SKILL.md
+++ b/.claude/skills/setup/SKILL.md
@@ -59,7 +59,14 @@ Collect, confirming back as you go:
 3. **Gates** — exact shell commands (run from repo root) for `install`, `build`, `lint`, `typecheck`, `test`,
    `test_affected`, `coverage`, `e2e`, `security`. Empty = "skip" (fine, and the right default when a gate
    doesn't exist yet). Warn that a gate pointed at a command that can't pass will block the Stop hook. Ask
-   `coverage_threshold` (default 80). If unsure on `test_affected`, default it to the full `test` command.
+   `coverage_threshold` (default 80). **`test_affected` runs on the `Stop` hook after every single turn, so it
+   must be cheap** — steer towards a changed-scope filter (`turbo run test --filter='...[origin/main]'`, `nx
+   affected -t test`, `pnpm --filter '...[origin/main]' test`, etc. — see `docs/GETTING_STARTED.md` →
+   "Choosing `test_affected` per stack" for the full per-stack table) rather than defaulting straight to the
+   full `test` command. If the stack has no cheap affected-scoped option (e.g. Foundry, a slow monolithic
+   suite), the full command is an acceptable fallback ONLY if it's already fast enough to run every turn —
+   confirm that with the user rather than assuming it, and otherwise leave `test_affected` empty (skipped)
+   until a cheap filter exists.
 4. **Review** — `review.lenses` (default `["correctness","tests","security","performance"]`) and
    `review.consensus` (`all`, or an integer).
 5. **Budget/routing** — `orchestrator_model`/`worker_model`/`explorer_model`/`reviewer_model`
@@ -106,11 +113,16 @@ idempotently:
   `$CLAUDE_PROJECT_DIR/.claude/scripts/...`, plus baseline `permissions`/`sandbox`. Deliberately carries no
   `enabledPlugins`/`extraKnownMarketplaces` — keep those only in a settings.json you maintain yourself while
   installing/updating the plugin (e.g. the block from Step 1 of `docs/GETTING_STARTED.md`), not in the
-  runtime file, which must keep working with the plugin disabled. **If you already have a `settings.json`**
-  (likely, since you needed `enabledPlugins` to install the plugin in the first place), scaffold.sh reports it
-  "kept" and leaves it completely untouched — merge the four hooks above and the `permissions`/`sandbox`
-  blocks from `.claude/skills/setup/templates/settings.json` into your existing file by hand, then it's safe
-  to drop `enabledPlugins`/`extraKnownMarketplaces` from it once you don't need the plugin loaded anymore.
+  runtime file, which must keep working with the plugin disabled. **Single-owner rule (issue #129):** the
+  plugin's `hooks/hooks.json` and this file's `hooks` block fire the SAME hooks — while both are active
+  (plugin enabled AND this file wired), every gate runs TWICE per turn (the expensive `Stop` `test_affected`
+  is the costly one). Only one may own the hooks at a time; this file is the intended steady-state owner,
+  the plugin's copy is only needed transiently to run `/orchestrator:setup`/`/orchestrator:sync`. **If you
+  already have a `settings.json`** (likely, since you needed `enabledPlugins` to install the plugin in the
+  first place), scaffold.sh reports it "kept" and leaves it completely untouched — merge the four hooks above
+  and the `permissions`/`sandbox` blocks from `.claude/skills/setup/templates/settings.json` into your
+  existing file by hand. Either way, **drop `enabledPlugins`/`extraKnownMarketplaces` for `orchestrator@recode`
+  as part of finishing this setup run** (see step 11) — don't leave both registrations active "for later."
 - `.github/workflows/gates.yml` + `.github/actions/setup/action.yml` — the CI gate. Created if absent, left
   untouched if present.
 - `.gitignore` entries (append-if-missing, never duplicated): `.env`, `.env.*`, `!.env.example`,
@@ -242,6 +254,11 @@ gitignore entries, labels created, bot status, CI status, loop armed?, hardened?
 points in one line each: **label an issue `module:*` to queue it; approve the bot's PR to ship it.** Finish
 with an ordered checklist of everything only the human can complete, e.g.:
 - add `GH_BOT_TOKEN` to `.env` / add the bot as a write collaborator (if step 7 flagged it),
+- **disable the `orchestrator` plugin for everyday sessions now that `.claude/settings.json` owns the hooks
+  locally** — drop (or comment out) `enabledPlugins`/`extraKnownMarketplaces` for `orchestrator@recode`
+  wherever you set them (step 1's block). Until you do, the plugin's `hooks/hooks.json` and your local
+  `.claude/settings.json` both fire on every turn — double gate execution (issue #129). Re-enable only when
+  you next need to run `/orchestrator:setup` or `/orchestrator:sync`.
 - set branch protection / required status checks (if wanted),
 - OS-level isolation from `docs/HARDENING.md` Step 2 (sudo / VM / WSL interop) if hardening,
 - if the daemon path was chosen: run `bash .claude/scripts/arm-loop.sh` in a real terminal outside Claude
diff --git a/.claude/skills/setup/templates/CLAUDE.md b/.claude/skills/setup/templates/CLAUDE.md
index bdf225c..5c4a854 100644
--- a/.claude/skills/setup/templates/CLAUDE.md
+++ b/.claude/skills/setup/templates/CLAUDE.md
@@ -21,7 +21,10 @@
 This repo is set up for orchestrated multi-agent development. See `docs/USAGE.md`.
 - **Agents:** `.claude/agents/` — orchestrator, implementer (worktree-isolated), reviewer, test-runner.
 - **Adapter:** `.claude/gates.json` — module map, gate commands, model routing. **This is the file to keep current.**
-- **Gates run via** `.claude/scripts/gate.sh <name>` and the hooks in `.claude/settings.json`.
+- **Gates run via** `.claude/scripts/gate.sh <name>` and the hooks in `.claude/settings.json`. Keep the
+  `orchestrator` plugin **disabled** outside of running `/orchestrator:setup`/`/orchestrator:sync` — its own
+  `hooks/hooks.json` registers the same hooks, and if both the plugin and this file are active at once every
+  gate (notably the `Stop` `test_affected` check) runs twice per turn.
 - **Workflow:** `.claude/workflows/feature-fanout.js` for deterministic fan-out.
 
 ### Module boundaries (hard rule)
diff --git a/.claude/skills/setup/templates/gates.json b/.claude/skills/setup/templates/gates.json
index e9a0e83..7f31373 100644
--- a/.claude/skills/setup/templates/gates.json
+++ b/.claude/skills/setup/templates/gates.json
@@ -18,6 +18,7 @@
 
   "gates": {
     "_note": "Exact shell commands for this repo. Run from repo root. Empty string = skip.",
+    "_test_affected_note": "test_affected runs on the Stop hook after EVERY turn — keep it cheap (a changed-scope filter, not the full suite) or leave it empty until one exists. See docs/GETTING_STARTED.md -> 'Choosing test_affected per stack'.",
     "install": "",
     "build": "",
     "lint": "",

From c3f42b24a674c0080841148f767d12024cf3cc7c Mon Sep 17 00:00:00 2001
From: Roberto Cano <3525807+robercano@users.noreply.github.com>
Date: Fri, 17 Jul 2026 09:46:12 +0200
Subject: [PATCH 2/2] test(gate): add regression fixture for empty-gate skip vs
 pnpm-lock preflight (#129)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

gate.sh was the most-invoked harness script with no dedicated *.test.sh
fixture. Add gate.test.sh (mirrors the sibling offline/mock-root pattern)
covering: (1) a blanked/unconfigured gate key skips cleanly even with a
stale/absent node_modules/.pnpm/lock.yaml — the #129 regression, verified
to fail against the pre-fix preflight-before-skip ordering; (2) a
configured gate + stale lock aborts with the exact stderr message and
never runs the command; (3) a configured gate + fresh lock (or no
pnpm-lock.yaml) actually runs the command.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
---
 .claude/scripts/gate.test.sh | 156 +++++++++++++++++++++++++++++++++++
 1 file changed, 156 insertions(+)
 create mode 100755 .claude/scripts/gate.test.sh

diff --git a/.claude/scripts/gate.test.sh b/.claude/scripts/gate.test.sh
new file mode 100755
index 0000000..b00416f
--- /dev/null
+++ b/.claude/scripts/gate.test.sh
@@ -0,0 +1,156 @@
+#!/usr/bin/env bash
+# gate.test.sh — offline smoke test for gate.sh's dependency-freshness
+# preflight vs. the empty/unconfigured-gate skip (issue #129).
+#
+# gate.sh runs two checks in this order:
+#   1. pnpm-lock.yaml vs node_modules/.pnpm/lock.yaml staleness preflight
+#      (aborts with exit 1 + a specific stderr message when they differ).
+#   2. the requested gate key's command lookup in gates.json (skips with
+#      exit 0 when the key is blank/unconfigured).
+#
+# Before #129, the preflight ran unconditionally BEFORE the empty-gate skip,
+# so an unconfigured gate (e.g. "test_affected": "" on a repo that doesn't
+# wire it up yet) would still abort on a stale node_modules — even though
+# no command was ever going to run. The fix keeps the preflight (still
+# needed to guard configured gates) but this fixture pins the interaction:
+# a blank gate key must skip cleanly regardless of lockfile staleness,
+# while a configured gate key still gets the preflight's protection, and a
+# fresh/absent lockfile still lets a configured gate's command actually run.
+#
+# Uses REAL gate.sh + resolve-roots.sh copied into a mocked fixture root
+# (own pnpm-lock.yaml / node_modules / GATES_FILE) — mirrors
+# plan-gate.test.sh's fixture-scaffolding pattern. Offline, no network, no
+# real pnpm/node_modules touched.
+#
+# Exit 0 on success, non-zero if any assertion fails. Runnable bare:
+#   bash .claude/scripts/gate.test.sh
+set -uo pipefail
+
+script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+gate_src="$script_dir/gate.sh"
+resolve_roots_src="$script_dir/resolve-roots.sh"
+
+work="$(mktemp -d "${TMPDIR:-/tmp}/gate-test.XXXXXX")"
+trap 'rm -rf "$work"' EXIT
+
+fail=0
+ok=0
+check() {
+  local desc="$1"; shift
+  if "$@"; then
+    ok=$((ok + 1))
+    echo "ok - $desc"
+  else
+    fail=1
+    echo "FAIL - $desc"
+  fi
+}
+
+# new_fixture NAME -- a root dir carrying its own .claude/scripts/{gate.sh,
+# resolve-roots.sh} copy, so resolve-roots.sh's "*/.claude/scripts ->
+# strip suffix" rule makes root=$work/NAME (real script, mocked root).
+new_fixture() {
+  local name="$1"
+  local dir="$work/$name/.claude/scripts"
+  mkdir -p "$dir"
+  cp "$gate_src" "$dir/gate.sh"
+  cp "$resolve_roots_src" "$dir/resolve-roots.sh"
+  chmod +x "$dir"/*.sh
+  printf '%s\n' "$work/$name"
+}
+
+# write_gates_file NAME JSON_BODY -- an absolute-path GATES_FILE (gate.sh
+# takes absolute GATES_FILE values as-is, no root-relative resolution).
+write_gates_file() {
+  local name="$1" body="$2"
+  local f="$work/$name.gates.json"
+  printf '%s' "$body" > "$f"
+  printf '%s\n' "$f"
+}
+
+run_gate() {
+  # $1 = fixture root, $2 = GATES_FILE (absolute), $3 = gate key
+  GATES_FILE="$2" bash "$1/.claude/scripts/gate.sh" "$3"
+}
+
+# =============================================================================
+# (1) Blanked/unconfigured gate key => exit 0 (skip), EVEN with a stale/absent
+# node_modules/.pnpm/lock.yaml. This is the #129 regression: under the old
+# ordering the preflight ran first and would abort (exit 1) here even though
+# the gate is a no-op. Proves the fix reordered the checks correctly.
+# =============================================================================
+dir1="$(new_fixture blank-gate-stale-lock)"
+printf 'lockfileVersion: 6\n' > "$dir1/pnpm-lock.yaml"
+mkdir -p "$dir1/node_modules/.pnpm"
+printf 'lockfileVersion: 5\n' > "$dir1/node_modules/.pnpm/lock.yaml"  # deliberately mismatched
+gates1="$(write_gates_file blank-gate '{"gates":{"test_affected":""}}')"
+out1="$(run_gate "$dir1" "$gates1" test_affected 2>"$work/err1")"
+rc1=$?
+check "(1) blank gate key + stale lock: exit 0" bash -c '[ "$1" -eq 0 ]' _ "$rc1"
+check "(1) blank gate key + stale lock: skip message on stdout" bash -c \
+  'printf "%s\n" "$1" | grep -q "not configured in gates.json — skipping"' _ "$out1"
+check "(1) blank gate key + stale lock: no preflight abort on stderr" bash -c \
+  '! grep -q "node_modules is out of sync" "$1"' _ "$work/err1"
+
+# Same, but node_modules/.pnpm/lock.yaml absent entirely (not just stale).
+dir1b="$(new_fixture blank-gate-absent-lock)"
+printf 'lockfileVersion: 6\n' > "$dir1b/pnpm-lock.yaml"
+gates1b="$(write_gates_file blank-gate-absent '{"gates":{"test_affected":""}}')"
+out1b="$(run_gate "$dir1b" "$gates1b" test_affected 2>"$work/err1b")"
+rc1b=$?
+check "(1b) blank gate key + absent installed lock: exit 0" bash -c '[ "$1" -eq 0 ]' _ "$rc1b"
+check "(1b) blank gate key + absent installed lock: no preflight abort on stderr" bash -c \
+  '! grep -q "node_modules is out of sync" "$1"' _ "$work/err1b"
+
+# =============================================================================
+# (2) Configured gate + stale/mismatched lock => exit 1 with the exact
+# stderr message, naming the requested gate key. The command must NOT run.
+# =============================================================================
+dir2="$(new_fixture configured-gate-stale-lock)"
+printf 'lockfileVersion: 6\n' > "$dir2/pnpm-lock.yaml"
+mkdir -p "$dir2/node_modules/.pnpm"
+printf 'lockfileVersion: 5\n' > "$dir2/node_modules/.pnpm/lock.yaml"
+sideEffect2="$dir2/ran.txt"
+gates2="$(write_gates_file configured-gate "{\"gates\":{\"my_gate\":\"touch $sideEffect2\"}}")"
+out2="$(run_gate "$dir2" "$gates2" my_gate 2>"$work/err2")"
+rc2=$?
+check "(2) configured gate + stale lock: exit 1" bash -c '[ "$1" -eq 1 ]' _ "$rc2"
+check "(2) configured gate + stale lock: exact stderr message" bash -c \
+  'grep -qxF "gate.sh: node_modules is out of sync with pnpm-lock.yaml — run '"'"'pnpm install'"'"' (gate '"'"'my_gate'"'"' aborted)." "$1"' \
+  _ "$work/err2"
+check "(2) configured gate + stale lock: command never ran (no side effect)" bash -c '[ ! -e "$1" ]' _ "$sideEffect2"
+
+# =============================================================================
+# (3) Configured gate + fresh/matching lock (or no pnpm-lock.yaml at all) =>
+# the gate command actually runs (side effect + exit 0 propagated).
+# =============================================================================
+dir3="$(new_fixture configured-gate-fresh-lock)"
+printf 'lockfileVersion: 6\n' > "$dir3/pnpm-lock.yaml"
+mkdir -p "$dir3/node_modules/.pnpm"
+cp "$dir3/pnpm-lock.yaml" "$dir3/node_modules/.pnpm/lock.yaml"  # byte-identical => fresh
+sideEffect3="$dir3/ran.txt"
+gates3="$(write_gates_file configured-gate-fresh "{\"gates\":{\"my_gate\":\"touch $sideEffect3\"}}")"
+out3="$(run_gate "$dir3" "$gates3" my_gate 2>"$work/err3")"
+rc3=$?
+check "(3) configured gate + fresh lock: exit 0" bash -c '[ "$1" -eq 0 ]' _ "$rc3"
+check "(3) configured gate + fresh lock: command actually ran (side effect exists)" bash -c '[ -e "$1" ]' _ "$sideEffect3"
+check "(3) configured gate + fresh lock: no preflight abort on stderr" bash -c \
+  '! grep -q "node_modules is out of sync" "$1"' _ "$work/err3"
+
+# Same, but no pnpm-lock.yaml at all (non-pnpm repo) -- preflight is a no-op.
+dir3b="$(new_fixture configured-gate-no-lockfile)"
+sideEffect3b="$dir3b/ran.txt"
+gates3b="$(write_gates_file configured-gate-no-lockfile "{\"gates\":{\"my_gate\":\"touch $sideEffect3b\"}}")"
+out3b="$(run_gate "$dir3b" "$gates3b" my_gate 2>"$work/err3b")"
+rc3b=$?
+check "(3b) configured gate + no pnpm-lock.yaml: exit 0" bash -c '[ "$1" -eq 0 ]' _ "$rc3b"
+check "(3b) configured gate + no pnpm-lock.yaml: command actually ran" bash -c '[ -e "$1" ]' _ "$sideEffect3b"
+
+echo ""
+if [ "$fail" -eq 0 ]; then
+  echo "gate.test.sh: PASS ($ok checks)"
+  exit 0
+else
+  echo "gate.test.sh: FAIL (see FAIL lines above)"
+  exit 1
+fi