Merge pull request #179 from robercano/feat/issue-94-protected-paths

feat(harness): protected-paths guard (issue #94 Layer 2)

Author: robercano-ghbot

.claude/agents/reviewer.md

@@ -28,7 +28,11 @@ If you touch GitHub at all (e.g. `gh pr diff`, `gh pr view`, `gh api`), route it
    - **correctness**: logic errors, edge cases, off-by-one, error handling, race conditions, broken invariants.
      If the task provides an APPROVED PLAN / authoritative scope (e.g. issue #100's plan gate), also verify
      the diff stays within it — a diff that exceeds the approved plan's declared files or approach is a
-     valid reject under this lens ("exceeds approved scope").
+     valid reject under this lens ("exceeds approved scope"). Protected-paths hard reject (issue #94 Layer
+     2): if the diff touches any path matching the adapter's `protectedPaths` globs (`.claude/gates.json`,
+     resolved via `GATES_FILE`), REJECT — an agent PR must not modify the loop's own control plane.
+     EXCEPTION: if the adapter's `protectedPaths` is empty or absent (the self-hosting override), skip this
+     check (the harness files are the product).
    - **tests**: do tests actually exercise the change? coverage of edge/failure paths? meaningful assertions, not just "it runs"? Run the test gate if needed.
    - **security**: injection, auth/access control, unsafe input, secrets, dependency risk, (for smart contracts) reentrancy/overflow/access — defer to the project security skill if configured.
    - **performance**: needless work, N+1, allocations, blocking calls, complexity regressions.

.claude/gates.json

@@ -77,5 +77,8 @@
   "commentFix": { "botAllowlist": [] },
 
   "_plan_note": "issue #100 — optional spec/plan gate before implementation starts. plan.gate: off (default; today's single-pass advance behavior, unchanged) | label (gate only planned issues that ALSO carry a plan-first label) | always (gate every planned+module:* issue). Unknown/missing value falls back to off. When gated, the loop posts ONE structured plan comment on the issue (marked `<!-- plan-gate:plan -->`) and labels it plan-review + needs-human instead of implementing; the owner reviews the comment and either replaces plan-review with plan-approved (approve — the next tick implements with the plan injected into the implementer and reviewers as authoritative scope) or removes plan-review (request changes — the loop re-plans). Owner-only approval is NOT identity-enforced — same limitation as the `planned` label itself (see docs/USAGE.md). Read by loop-census.sh/loop-tick.sh/loop-event.sh.",
-  "plan": { "gate": "off" }
+  "plan": { "gate": "off" },
+
+  "_protectedPaths_note": "issue #94 Layer 2 — deterministic protected-paths guard. Array of glob patterns (\"*\" = one path segment, \"**\" = any depth). An agent-authored PR whose diff touches ANY matching path is NEVER auto-merged: merge-ready.sh blocks the merge and labels the PR needs-human, and reviewers hard-reject it (see .claude/agents/reviewer.md). Protects the loop's own control plane from an injected instruction that tries to edit it. EMPTY ARRAY = disabled (same empty-means-skip convention as `gates`/`notify`) — self-hosted repos whose harness files ARE the product override this to [] in .claude/self/gates.json. Read from the adapter resolved via GATES_FILE (falls back to this root adapter). Documented in docs/HARDENING.md.",
+  "protectedPaths": [".claude/**", ".github/workflows/**", "gates.json", "**/gates.json"]
 }

.claude/scripts/merge-ready.sh

@@ -42,7 +42,13 @@ repo="${1:-$(gh repo view --json nameWithOwner -q .nameWithOwner)}"
 # shellcheck source=needs-human.sh
 if [ -f "$script_dir/needs-human.sh" ]; then . "$script_dir/needs-human.sh"; fi
 owner="${MERGE_APPROVER:-${repo%%/*}}"   # the approver whose APPROVED review authorizes a merge
-gates="$root/.claude/gates.json"
+
+# Adapter file: honor GATES_FILE (the self-host loop points at .claude/self/gates.json),
+# fall back to the shipped root adapter. Both merge.baseBranch and protectedPaths
+# (issue #94 Layer 2) are read from it, so the self-adapter's permissive protectedPaths
+# override applies when the loop runs self-hosted.
+gates_rel="${GATES_FILE:-.claude/gates.json}"
+case "$gates_rel" in /*) gates="$gates_rel";; *) gates="$root/$gates_rel";; esac
 base="$(node -e "try{const g=require('$gates');process.stdout.write((g.merge&&g.merge.baseBranch)||'main')}catch(e){process.stdout.write('main')}")"
 
 # Decide MERGE / SKIP:<reason> for one PR's JSON (read on stdin).
@@ -82,13 +88,52 @@ decide() {
   ' "$base" "$owner"
 }
 
+# protected_paths_check (issue #94 Layer 2): reads the adapter's protectedPaths
+# globs and the PR's changed-file list (`.files[].path`, on stdin) and prints the
+# protected path(s) the diff touches (comma-joined), or nothing. Empty/absent
+# protectedPaths = disabled (prints nothing) — same empty-means-skip convention as
+# `notify`. Deterministic (no LLM); "*" matches one path segment, "**" any depth.
+protected_paths_check() {
+  node -e '
+    const gates = process.argv[1];
+    let globs = [];
+    try { const g = require(gates); if (Array.isArray(g.protectedPaths)) globs = g.protectedPaths; } catch (e) {}
+    if (!globs.length) process.exit(0);
+    let p; try { p = JSON.parse(require("fs").readFileSync(0, "utf8")); } catch (e) { process.exit(0); }
+    const files = (p.files || []).map(f => f && f.path).filter(Boolean);
+    function toRe(glob) {
+      let re = "";
+      for (let i = 0; i < glob.length; i++) {
+        const c = glob[i];
+        if (c === "*") {
+          if (glob[i + 1] === "*") { re += ".*"; i++; if (glob[i + 1] === "/") i++; }
+          else re += "[^/]*";
+        } else if ("\\^$.|?+()[]{}".includes(c)) { re += "\\" + c; }
+        else re += c;
+      }
+      return new RegExp("^" + re + "$");
+    }
+    const res = globs.map(toRe);
+    const hits = files.filter(f => res.some(r => r.test(f)));
+    if (hits.length) process.stdout.write([...new Set(hits)].join(", "));
+  ' "$gates"
+}
+
 merged=0; skipped=0
 for n in $(gh pr list -R "$repo" --base "$base" --state open --json number -q '.[].number'); do
-  data="$(gh pr view "$n" -R "$repo" --json number,title,isDraft,baseRefName,headRefName,mergeable,reviews,statusCheckRollup,commits)"
+  data="$(gh pr view "$n" -R "$repo" --json number,title,isDraft,baseRefName,headRefName,mergeable,reviews,statusCheckRollup,commits,files)"
   verdict="$(printf '%s' "$data" | decide)"
   title="$(printf '%s' "$data" | node -e 'process.stdout.write((JSON.parse(require("fs").readFileSync(0,"utf8")).title)||"")')"
   head_branch="$(printf '%s' "$data" | node -e 'process.stdout.write((JSON.parse(require("fs").readFileSync(0,"utf8")).headRefName)||"")')"
 
+  # Protected-paths guard (issue #94 Layer 2): even an owner-approved, CI-green PR
+  # must not auto-merge if its diff touches a path the adapter marks protected.
+  protected_hit=""
+  if [ "$verdict" = "MERGE" ]; then
+    protected_hit="$(printf '%s' "$data" | protected_paths_check)"
+    [ -n "$protected_hit" ] && verdict="SKIP:protected-paths"
+  fi
+
   # needs-human (issue #99): a PR is genuinely blocked on the OWNER for
   # exactly two of decide()'s skip reasons -- no review submitted yet, or a
   # stale approval that no longer covers the current head (new commits
@@ -112,6 +157,13 @@ for n in $(gh pr list -R "$repo" --base "$base" --state open --json number -q '.
           "PR #$n ready for your review" "$title ($reason_text)"
       fi
       ;;
+    SKIP:protected-paths)
+      if command -v needs_human_flag >/dev/null 2>&1; then
+        needs_human_flag "pr:$n" "protected-paths" "high" \
+          "PR #$n touches protected paths -- human review required" \
+          "$title: this PR's diff touches protected path(s): $protected_hit. Auto-merge is blocked by the protected-paths guard (issue #94 Layer 2). A human must review and merge it manually."
+      fi
+      ;;
     *)
       if command -v needs_human_clear >/dev/null 2>&1; then
         needs_human_clear "pr:$n" "pr-review"