Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion .claude/agents/reviewer.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
5 changes: 4 additions & 1 deletion .claude/gates.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
}
56 changes: 54 additions & 2 deletions .claude/scripts/merge-ready.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down Expand Up @@ -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
Expand All @@ -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"
Expand Down
Loading
Loading