diff --git a/.claude/agents/orchestrator.md b/.claude/agents/orchestrator.md index b1e9549..dba63ac 100644 --- a/.claude/agents/orchestrator.md +++ b/.claude/agents/orchestrator.md @@ -19,7 +19,7 @@ EVERY `gh` invocation — by you and by every agent you spawn — MUST go throug ## Your loop 1. **Scope.** Decompose the task into sub-tasks that are *independent* and *non-overlapping at the file level*. Use the `modules` map in `gates.json` to assign each sub-task to exactly one module/path. If two sub-tasks would touch the same files, either merge them into one sub-task or sequence them (declare the dependency). Scale effort to complexity: a trivial task gets ONE worker and no parallelism — do not fan out for its own sake. 2. **Present the plan and WAIT.** Output the plan: each sub-task's title, target module/path, owner boundary, dependencies, and which reviewers will gate it. Enter plan mode and wait for human approval before any code is written. This is the planning checkpoint. -3. **Delegate.** For each approved sub-task, spawn an `implementer` (it runs in its own git worktree/branch, so workers never clash). Respect `budget.max_parallel_workers` from `gates.json` — queue the rest. Give each implementer: the objective, its module boundary ("never edit outside ``"), the definition of done, and the required gates. +3. **Delegate.** For each approved sub-task, spawn an `implementer` (it runs in its own git worktree/branch, so workers never clash). Respect `budget.max_parallel_workers` from `gates.json` — queue the rest. Give each implementer: the objective, its module boundary ("never edit outside ``"), the definition of done, and the required gates. If an approved plan (issue #100's plan gate) was provided for this issue, forward it VERBATIM to the implementer and to every reviewer in step 4 as authoritative scope. 4. **Review gate.** When an implementer reports done, route its change through `reviewer` agents (one per lens in `gates.json.review.lenses`). Spawn each reviewer with the model from `budget.reviewer_models[]`, falling back to `budget.reviewer_model`. Require the configured majority/consensus to approve. On reject, feed the reasons back to the same implementer; on the re-review, re-run ONLY the lenses that rejected — an approval stands unless the fix touched files outside what that lens already approved. Do not advance a sub-task until its gates pass. 5. **Integrate.** Use the merge discipline from `CLAUDE.md` (default: PR-per-agent). Surface conflicts to the user; do not force-merge. 6. **Report.** End with a structured status block (see below). diff --git a/.claude/agents/reviewer.md b/.claude/agents/reviewer.md index 5bb09b5..b2b5b8f 100644 --- a/.claude/agents/reviewer.md +++ b/.claude/agents/reviewer.md @@ -26,6 +26,9 @@ If you touch GitHub at all (e.g. `gh pr diff`, `gh pr view`, `gh api`), route it 1. Read the diff and the surrounding code it affects. Stay scoped: the diff plus what it touches — don't crawl the repo. For long test/build logs, filter to the relevant lines (`grep`/`tail`) instead of reading whole outputs into context. 2. Apply ONLY your assigned lens — go deep, not broad: - **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"). - **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. diff --git a/.claude/gates.json b/.claude/gates.json index 22fabc2..121a101 100644 --- a/.claude/gates.json +++ b/.claude/gates.json @@ -71,5 +71,8 @@ }, "_notify_note": "issue #99 — needs-human push-notification seam, read by .claude/scripts/notify.sh (GATES_FILE-aware; same empty-means-skip convention as `gates`). Empty string = disabled (offline/CI-safe no-op) — the default; fill in a command to enable push notifications when the autonomous loop blocks on YOU (PR ready for review, CHANGES_REQUESTED addressed and awaiting re-review, attempt-budget/stall escalation, ...). Contract: severity/title/body-line reach the command BOTH as positional args ($1/$2/$3) and as NOTIFY_SEVERITY/NOTIFY_TITLE/NOTIFY_BODY env vars; throttled to one notification per (kind,target) per window (default 1800s — see notify.sh). Example commands (uncomment/adapt ONE):\n ntfy: curl -s -d \"$NOTIFY_BODY\" -H \"Title: $NOTIFY_TITLE\" -H \"Priority: $NOTIFY_SEVERITY\" ntfy.sh/\n notify-send: notify-send \"$NOTIFY_TITLE\" \"$NOTIFY_BODY\"\n webhook: curl -s -X POST -H 'Content-Type: application/json' -d \"{\\\"severity\\\":\\\"$NOTIFY_SEVERITY\\\",\\\"title\\\":\\\"$NOTIFY_TITLE\\\",\\\"body\\\":\\\"$NOTIFY_BODY\\\"}\" https://example.invalid/hook", - "notify": "" + "notify": "", + + "_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 ``) 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" } } diff --git a/.claude/scripts/loop-census.sh b/.claude/scripts/loop-census.sh index 238f878..7e769e6 100644 --- a/.claude/scripts/loop-census.sh +++ b/.claude/scripts/loop-census.sh @@ -29,8 +29,31 @@ # advance_ready= lowest-numbered planned issue with no branch, # only when open_prs=0 (the ADVANCE precondition) # AND not blocked by an open "Blocked by #N" edge +# plan_wait= one line per candidate that would otherwise be +# advance_ready but is awaiting owner review of a +# posted plan (labelled `plan-review`, no +# `plan-approved` yet) — issue #100's plan gate, +# see PLAN GATE below. Only emitted when +# plan.gate != "off". +# advance_mode=plan|implement-gated|implement only emitted alongside a +# non-"none" advance_ready when plan.gate != +# "off" (issue #100) — tells the tick which +# driver prompt variant to build. # cadence=FAST|WATCH|IDLE cron= desired cadence per the loop policy # +# --- PLAN GATE (issue #100) ------------------------------------------------- +# Optional, adapter-configured via plan.gate ("off" default | "label" | +# "always"; see gates.json). When enabled, each candidate's labels classify it +# as needs-plan (no plan posted yet — advance_mode=plan gates it into a +# PLAN-ONLY driver turn), awaiting-owner (`plan-review` label present, no +# `plan-approved` yet — treated like an open "Blocked by" edge: skipped for +# BOTH advance_ready and fallback_ready, reported via plan_wait=), +# gated-approved (`plan-approved` present — advance_mode=implement-gated, the +# approved plan comment is injected into the implementer/reviewers as +# authoritative scope), or ungated (advance_mode=implement, today's behavior). +# plan.gate="off" makes every line above a no-op — census output stays +# byte-identical to pre-#100 behavior. +# # The module label set is derived from $GATES_FILE (default .claude/gates.json) # → modules[].name, so the same script serves the self-hosted loop # (GATES_FILE=.claude/self/gates.json) and downstream adopters. @@ -114,6 +137,19 @@ stall_minutes=$(node -e ' ' "$gates" 2>/dev/null) case "$stall_minutes" in ''|*[!0-9]*) stall_minutes=30 ;; esac +# Plan gate mode (issue #100), adapter-configurable via plan.gate: "off" +# (default — this whole feature is a no-op, census output stays byte-identical +# to pre-#100 behavior) | "label" (gate only candidates that ALSO carry the +# plan-first label) | "always" (gate every planned+module candidate). Unknown/ +# missing value falls back to "off". Read ONCE, same node -e / require(gates) +# pattern as base/module_labels/stall_minutes above. +plan_mode=$(node -e ' + const g = require(process.argv[1]); + const v = (g.plan && g.plan.gate) || "off"; + console.log(["off", "label", "always"].includes(v) ? v : "off"); +' "$gates" 2>/dev/null) +case "$plan_mode" in off|label|always) ;; *) plan_mode=off ;; esac + events_file="${CLAUDE_EVENTS_FILE:-$root/.claude/state/events.jsonl}" # --- stall detection helper (issue #98) -------------------------------------- @@ -213,6 +249,9 @@ detail="" in_flight="" stalled_lines="" blocked_lines="" +plan_wait_lines="" +advance_plan_state="" +fallback_plan_state="" while IFS=$'\t' read -r num labels title; do [ -z "${num:-}" ] && continue hit=0 @@ -234,10 +273,41 @@ while IFS=$'\t' read -r num labels title; do if [ "$branch" = "none" ] && [ "$open_prs" -eq 0 ] && ! driver_unit_active "$num"; then eligible=1 fi + + # --- plan gate (issue #100): derive this candidate's plan state from its + # labels. When gated (mode==always, or mode==label+plan-first) and not yet + # plan-reviewed/approved, an "awaiting-owner" candidate (plan posted, owner + # hasn't approved/rejected it yet) is NOT eligible for advance — the same + # treatment as a "Blocked by" dependency: it's blocked on a human, not ready + # to advance. Entirely a no-op (plan_state stays "ungated", eligible + # untouched) when plan_mode=off, so this feature costs nothing on the + # default path and census output stays byte-identical. + plan_state="ungated" + if [ "$plan_mode" != "off" ]; then + case ",$labels," in + *",plan-approved,"*) plan_state="gated-approved" ;; + *",plan-review,"*) plan_state="awaiting-owner" ;; + *) + gated=0 + if [ "$plan_mode" = "always" ]; then + gated=1 + else + case ",$labels," in *",plan-first,"*) gated=1 ;; esac + fi + [ "$gated" -eq 1 ] && plan_state="needs-plan" + ;; + esac + if [ "$plan_state" = "awaiting-owner" ]; then + eligible=0 + plan_wait_lines+="plan_wait=$num"$'\n' + fi + fi + # fallback_ready: lowest-numbered otherwise-eligible candidate, IGNORING the # blocking-graph gate — used only if the gate leaves advance_ready="none". if [ "$eligible" -eq 1 ] && [ "$fallback_ready" = "none" ]; then fallback_ready="$num" + fallback_plan_state="$plan_state" fi if [ "$eligible" -eq 1 ] && [ "$advance_ready" = "none" ]; then # Fetch this candidate's body only now — we're actually considering it. @@ -260,6 +330,7 @@ while IFS=$'\t' read -r num labels title; do blocked_lines+="blocked=$num by=$first_open_blocker"$'\n' else advance_ready="$num" + advance_plan_state="$plan_state" fi fi @@ -295,6 +366,7 @@ done <<< "$planned" if [ "$advance_ready" = "none" ] && [ "$fallback_ready" != "none" ]; then echo "census: all planned candidates blocked (possible cycle); falling back to lowest-number #$fallback_ready" >&2 advance_ready="$fallback_ready" + advance_plan_state="$fallback_plan_state" fi echo "planned_issues=$planned_count" @@ -302,6 +374,18 @@ echo "planned_issues=$planned_count" [ -n "$in_flight" ] && printf '%s' "$in_flight" [ -n "$stalled_lines" ] && printf '%s' "$stalled_lines" [ -n "$blocked_lines" ] && printf '%s' "$blocked_lines" +[ -n "$plan_wait_lines" ] && printf '%s' "$plan_wait_lines" +# advance_mode (issue #100): only emitted when the plan gate is on AND a +# candidate was actually chosen — a tick reading this defaults to "implement" +# when the line is absent (plan_mode=off, or advance_ready=none), which is +# exactly today's ungated single-pass behavior. +if [ "$plan_mode" != "off" ] && [ "$advance_ready" != "none" ]; then + case "$advance_plan_state" in + needs-plan) echo "advance_mode=plan" ;; + gated-approved) echo "advance_mode=implement-gated" ;; + *) echo "advance_mode=implement" ;; + esac +fi echo "advance_ready=$advance_ready" # Desired cadence per the loop policy: FAST only when the loop can ACT now. diff --git a/.claude/scripts/loop-event.sh b/.claude/scripts/loop-event.sh index b0a3489..0a3c7ce 100644 --- a/.claude/scripts/loop-event.sh +++ b/.claude/scripts/loop-event.sh @@ -42,6 +42,16 @@ # Honors $GATES_FILE: not read directly here beyond quoting it into the # self-hosting adapter clause baked into the prompt below (loop-tick.sh and # loop-census.sh are what actually act on it). +# +# Plan gate (issue #100): for action=advance, loop-tick.sh's stdout carries an +# advance_mode=plan|implement-gated|implement line (present only when +# plan.gate != "off" — see gates.json) that this script greps out (never +# tail -1 — that's the verdict) to pick one of three ADVANCE prompt variants: +# a PLAN-ONLY turn that posts a structured plan comment + plan-review/ +# needs-human labels and writes no code, a normal implement turn with the +# owner-approved plan comment injected into the implementer/every reviewer as +# authoritative scope, or (mode absent/"implement") today's unchanged +# single-pass prompt. set -uo pipefail # Two-root derivation (issue #63): script_dir = sibling scripts, root = consumer project. @@ -88,6 +98,15 @@ case "$n" in ;; esac +# Plan-gate mode (issue #100): loop-tick.sh echoes advance_mode= into +# its own stdout ONLY alongside a genuine action=advance verdict -- grep the +# FULL tick output for it (never tail -1; that line is not the verdict). +# Defaults to "implement" (today's ungated single-pass prompt) whenever the +# line is absent -- action=feedback verdicts, and every advance verdict when +# plan.gate=off (the vast majority of ticks). +mode="$(printf '%s\n' "$tick_out" | sed -n 's/^advance_mode=//p' | tail -1)" +mode="${mode:-implement}" + # Adapter clause: only when this loop runs against a non-default adapter # (self-hosting). Mirrors the wording in .claude/self/pr-loop-self.md. adapter="" @@ -98,9 +117,37 @@ common="The tick (loop-tick.sh) already ran census/poll/merge/feedback-detection case "$verdict" in action=advance*) - prompt="Run the ADVANCE step of the autonomous PR loop for issue #$n. $common -Drive issue #$n through the orchestrator: scope → worktree implementer → gate.sh gates → reviewer lenses → bot PR. One issue in flight at a time — work ONLY issue #$n. \`backlog\` issues are owner-unapproved: if you file an issue yourself, label it backlog — NEVER planned (that label is the owner's formal approval, assigned by the owner alone)." action_line="action=advance issue=$n" + case "$mode" in + # --- plan.gate: needs-plan -> PLAN-ONLY turn (issue #100) ------------- + # Scope the issue and post ONE structured plan comment; apply the + # plan-review + needs-human labels for owner review; then STOP. No + # code, no branch, no PR -- the driver's job this turn is the plan + # artifact and the labels, nothing else. + plan) + prompt="Run the PLAN step of the autonomous PR loop for issue #$n (plan.gate). $common +This is a PLAN ONLY phase (plan.gate). Do NOT implement — write no code, create no feat/issue-$n-* branch, and open no PR. STOP once the plan comment and labels below are posted; do not spawn an implementer or any reviewer this turn. +1. Read issue #$n (\`bash $script_dir/bot-gh.sh issue view $n\`) and scope it: which module (per gates.json's \`modules[]\`) it belongs to, the files you expect the eventual implementation to touch, the implementation approach, and how each of the issue's acceptance criteria maps to that approach. +2. Post exactly ONE structured plan comment on issue #$n via \`bash $script_dir/bot-gh.sh issue comment $n --body \"...\"\`. The comment body MUST begin with the literal marker \`\` on its own first line, followed by the module, expected files, approach, and acceptance-criteria mapping from step 1 — this is the durable, reviewable plan artifact the owner and the later implement turn both read. +3. Create the plan-gate labels if they don't already exist (idempotent, mirrors needs-human.sh's own pattern): \`bash $script_dir/bot-gh.sh label create plan-review --color fbca04 --description \"Plan posted, awaiting owner review (plan.gate)\" --force\` and \`bash $script_dir/bot-gh.sh label create needs-human --color b60205 --description \"Loop is blocked on owner judgment\" --force\`. +4. Apply both labels to issue #$n: \`bash $script_dir/bot-gh.sh issue edit $n --add-label plan-review --add-label needs-human\`. +5. Report done and STOP. The owner reviews the plan comment on GitHub and either replaces \`plan-review\` with \`plan-approved\` (approve — the next tick implements it, with your plan injected as authoritative scope) or removes \`plan-review\` (request changes — the loop re-plans on a later tick)." + ;; + # --- plan.gate: gated-approved -> normal implement turn, PLUS the + # approved plan is authoritative scope for the implementer AND every + # reviewer (issue #100). ---------------------------------------------- + implement-gated) + prompt="Run the ADVANCE step of the autonomous PR loop for issue #$n. $common +Drive issue #$n through the orchestrator: scope → worktree implementer → gate.sh gates → reviewer lenses → bot PR. One issue in flight at a time — work ONLY issue #$n. \`backlog\` issues are owner-unapproved: if you file an issue yourself, label it backlog — NEVER planned (that label is the owner's formal approval, assigned by the owner alone). +This issue is plan-gated and APPROVED (plan.gate). Before implementing, fetch the approved plan: \`bash $script_dir/bot-gh.sh issue view $n --json comments\` and locate the comment whose body begins with the marker \`\`. Treat that plan as the AUTHORITATIVE scope for this issue. Inject the plan text VERBATIM into the implementer's spawn prompt as its authoritative scope, AND into every reviewer's spawn prompt. Instruct the correctness reviewer explicitly: a diff that exceeds the approved plan's declared files or approach is a valid reject reason under the correctness lens (\"exceeds approved scope\")." + ;; + # --- ungated (plan.gate=off, or label mode without plan-first) ------- + # today's single-pass prompt, byte-identical to pre-#100 behavior. + *) + prompt="Run the ADVANCE step of the autonomous PR loop for issue #$n. $common +Drive issue #$n through the orchestrator: scope → worktree implementer → gate.sh gates → reviewer lenses → bot PR. One issue in flight at a time — work ONLY issue #$n. \`backlog\` issues are owner-unapproved: if you file an issue yourself, label it backlog — NEVER planned (that label is the owner's formal approval, assigned by the owner alone)." + ;; + esac ;; *) prompt="Run the ADDRESS FEEDBACK step of the autonomous PR loop for PR #$n. $common diff --git a/.claude/scripts/loop-tick.sh b/.claude/scripts/loop-tick.sh index 4de1283..7d257fe 100644 --- a/.claude/scripts/loop-tick.sh +++ b/.claude/scripts/loop-tick.sh @@ -210,6 +210,13 @@ echo "=== verdict ===" # --- Parse census telemetry needed for the verdict ------------------------- advance_ready="$(printf '%s\n' "$census_out" | sed -n 's/^advance_ready=//p' | tail -1)" advance_ready="${advance_ready:-none}" +# Plan-gate mode (issue #100): census emits advance_mode=plan|implement-gated| +# implement alongside a non-"none" advance_ready ONLY when plan.gate != "off" +# — absent (default "implement") reproduces today's ungated single-pass +# behavior. Only meaningful for an actual action=advance verdict below; see +# where it's echoed into this tick's own stdout, scoped to that branch only. +advance_mode="$(printf '%s\n' "$census_out" | sed -n 's/^advance_mode=//p' | tail -1)" +advance_mode="${advance_mode:-implement}" in_flight_issues="$(printf '%s\n' "$census_out" | sed -n 's/^in_flight=//p')" # Cadence (FAST/WATCH/IDLE), for the tick record (issue #85) -- census emits # e.g. "cadence=FAST cron=* * * * *"; keep only the leading token. @@ -715,6 +722,13 @@ elif [ "$advance_ready" != "none" ] && [ -n "$advance_ready" ]; then printf 'issue=%s ts=%s\n' "$advance_ready" "$(date -u +%FT%TZ)" > "$tmp" mv -f "$tmp" "$lock_file" verdict="action=advance issue=$advance_ready" + # advance_mode telemetry (issue #100): only for a GENUINE advance dispatch + # (never on a refused/downgraded verdict above) -- loop-event.sh greps + # this out of the tick's full stdout (not the tail-1 verdict) to pick the + # right driver prompt variant. Printed BEFORE the final verdict line + # below, so it never disturbs the "verdict is the last stdout line" + # invariant. + echo "advance_mode=$advance_mode" fi else # --- stall/resume path (issue #98, reworked) ----------------------------- diff --git a/.claude/scripts/plan-gate.test.sh b/.claude/scripts/plan-gate.test.sh new file mode 100755 index 0000000..2e3dd03 --- /dev/null +++ b/.claude/scripts/plan-gate.test.sh @@ -0,0 +1,286 @@ +#!/usr/bin/env bash +# plan-gate.test.sh — offline smoke test for the optional spec/plan gate +# (issue #100): plan.gate = off|label|always, read once by loop-census.sh and +# threaded through as advance_mode= telemetry by loop-tick.sh, ending in one +# of three ADVANCE driver-prompt variants built by loop-event.sh. +# +# Two halves: +# 1. loop-census.sh (REAL script, mocked `gh` via a fake bot-gh.sh) — proves +# advance_mode is derived correctly from each candidate's labels per +# plan.gate mode, that gate=off never emits it (byte-identical to +# pre-#100 output), and that a `plan-review`-without-`plan-approved` +# candidate is skipped for advance_ready (awaiting the owner), mirroring +# loop-census.test.sh's fixture/mock pattern. +# 2. loop-event.sh (REAL script, fed a FAKE loop-tick.sh that emits a +# scripted advance_mode= line alongside its verdict) — proves the three +# prompt variants: PLAN-ONLY (no code/branch/PR), implement-gated (the +# approved-plan injection + "exceeds approved scope" clause), and +# ungated/off (today's unchanged single-pass prompt) — mirroring +# loop-event.test.sh's fixture pattern. +# +# Exit 0 on success, non-zero if any assertion fails. Runnable bare: +# bash .claude/scripts/plan-gate.test.sh +set -uo pipefail + +script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +census_src="$script_dir/loop-census.sh" +event_src="$script_dir/loop-event.sh" +resolve_roots_src="$script_dir/resolve-roots.sh" + +work="$(mktemp -d "${TMPDIR:-/tmp}/plan-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 +} + +# ============================================================================= +# PART 1: loop-census.sh — advance_mode derivation + awaiting-owner skip. +# ============================================================================= + +# build_census_fixture NAME GATE LABELS -- one planned+module:test candidate +# (issue 50) carrying LABELS, adapter's plan.gate=GATE. Mirrors +# loop-census.test.sh's fixture-scaffolding pattern (fake bot-gh.sh + real git +# repo so branch=none is genuine, not fabricated). +build_census_fixture() { + local name="$1" gate="$2" labels="$3" + local dir="$work/$name" + local scripts="$dir/.claude/scripts" + mkdir -p "$scripts" + cp "$census_src" "$scripts/loop-census.sh" + cp "$resolve_roots_src" "$scripts/resolve-roots.sh" + cat > "$dir/.claude/gates.json" < "$scripts/pr-feedback.sh" <<'EOF' +#!/usr/bin/env bash +exit 0 +EOF + cat > "$scripts/bot-gh.sh" <&2; exit 1 ;; + esac + ;; + *) echo "fake-bot-gh.sh: unhandled args: \$*" >&2; exit 1 ;; +esac +EOF + chmod +x "$scripts"/*.sh + git -C "$dir" init -q -b main + git -C "$dir" -c user.email=t@e.st -c user.name=t commit -q --allow-empty -m init + printf '%s\n' "$dir" +} + +run_census() { + # $1 = fixture dir. GATES_FILE unset explicitly (a test invoked from inside + # a GATES_FILE=.claude/self/gates.json gate run must not leak that into the + # fixture's own default-relative gates.json — same guard as + # loop-census.test.sh). + env -u GATES_FILE bash "$1/.claude/scripts/loop-census.sh" "acme/repo" +} + +# --- (1) gate=off: no advance_mode line at all, advance_ready picked normally +# (byte-identical to pre-#100 behavior — plan-related labels present but +# irrelevant, since the whole feature is a no-op on this path). ------------- +dirOff="$(build_census_fixture off-mode off "planned,module:test,plan-first")" +outOff="$(run_census "$dirOff")" +check "(1) gate=off: no advance_mode= line emitted at all" bash -c \ + '! printf "%s\n" "$1" | grep -q "^advance_mode="' _ "$outOff" +check "(1) gate=off: advance_ready=50 picked normally" bash -c \ + 'printf "%s\n" "$1" | grep -qx "advance_ready=50"' _ "$outOff" + +# Same fixture shape but the adapter has NO "plan" key at all (the realistic +# pre-#100 gates.json shape) -- proves the "missing -> off" fallback produces +# the exact same output as an explicit "off". +dirNoPlanKey="$work/no-plan-key" +scriptsNoPlanKey="$dirNoPlanKey/.claude/scripts" +mkdir -p "$scriptsNoPlanKey" +cp "$census_src" "$scriptsNoPlanKey/loop-census.sh" +cp "$resolve_roots_src" "$scriptsNoPlanKey/resolve-roots.sh" +cat > "$dirNoPlanKey/.claude/gates.json" <<'EOF' +{ + "modules": [{ "name": "test", "path": ".", "description": "", "owner": "" }], + "merge": { "baseBranch": "main" } +} +EOF +cp "$dirOff/.claude/scripts/pr-feedback.sh" "$scriptsNoPlanKey/pr-feedback.sh" +cp "$dirOff/.claude/scripts/bot-gh.sh" "$scriptsNoPlanKey/bot-gh.sh" +chmod +x "$scriptsNoPlanKey"/*.sh +git -C "$dirNoPlanKey" init -q -b main +git -C "$dirNoPlanKey" -c user.email=t@e.st -c user.name=t commit -q --allow-empty -m init +outNoPlanKey="$(run_census "$dirNoPlanKey")" +check "(1b) missing plan.gate key falls back to off — output matches the explicit-off run" \ + bash -c '[ "$1" = "$2" ]' _ "$outOff" "$outNoPlanKey" + +# --- (2) gate=label + plan-first (no plan-review/approved): needs-plan ------ +dirLabelFirst="$(build_census_fixture label-first label "planned,module:test,plan-first")" +outLabelFirst="$(run_census "$dirLabelFirst")" +check "(2) gate=label+plan-first: advance_mode=plan" bash -c \ + 'printf "%s\n" "$1" | grep -qx "advance_mode=plan"' _ "$outLabelFirst" +check "(2) gate=label+plan-first: advance_ready=50" bash -c \ + 'printf "%s\n" "$1" | grep -qx "advance_ready=50"' _ "$outLabelFirst" + +# --- (3) gate=label + plan-review (no plan-approved): awaiting-owner, NOT +# chosen as advance_ready -- the only candidate, so it stays "none". -------- +dirLabelReview="$(build_census_fixture label-review label "planned,module:test,plan-first,plan-review")" +outLabelReview="$(run_census "$dirLabelReview")" +check "(3) gate=label+plan-review: advance_ready stays none (awaiting owner)" bash -c \ + 'printf "%s\n" "$1" | grep -qx "advance_ready=none"' _ "$outLabelReview" +check "(3) gate=label+plan-review: plan_wait=50 telemetry emitted" bash -c \ + 'printf "%s\n" "$1" | grep -qx "plan_wait=50"' _ "$outLabelReview" +check "(3) gate=label+plan-review: no advance_mode= line (nothing was chosen)" bash -c \ + '! printf "%s\n" "$1" | grep -q "^advance_mode="' _ "$outLabelReview" + +# --- (4) gate=label + plan-approved: gated-approved -> implement-gated ----- +dirLabelApproved="$(build_census_fixture label-approved label "planned,module:test,plan-first,plan-approved")" +outLabelApproved="$(run_census "$dirLabelApproved")" +check "(4) gate=label+plan-approved: advance_mode=implement-gated" bash -c \ + 'printf "%s\n" "$1" | grep -qx "advance_mode=implement-gated"' _ "$outLabelApproved" +check "(4) gate=label+plan-approved: advance_ready=50" bash -c \ + 'printf "%s\n" "$1" | grep -qx "advance_ready=50"' _ "$outLabelApproved" + +# --- (5) gate=label WITHOUT plan-first: ungated -> implement, normal ------- +dirLabelUngated="$(build_census_fixture label-ungated label "planned,module:test")" +outLabelUngated="$(run_census "$dirLabelUngated")" +check "(5) gate=label, no plan-first: advance_mode=implement" bash -c \ + 'printf "%s\n" "$1" | grep -qx "advance_mode=implement"' _ "$outLabelUngated" +check "(5) gate=label, no plan-first: advance_ready=50" bash -c \ + 'printf "%s\n" "$1" | grep -qx "advance_ready=50"' _ "$outLabelUngated" + +# --- (6) gate=always WITHOUT plan-first: still gated -> needs-plan --------- +dirAlways="$(build_census_fixture always-mode always "planned,module:test")" +outAlways="$(run_census "$dirAlways")" +check "(6) gate=always, no plan-first: advance_mode=plan (gated regardless)" bash -c \ + 'printf "%s\n" "$1" | grep -qx "advance_mode=plan"' _ "$outAlways" +check "(6) gate=always, no plan-first: advance_ready=50" bash -c \ + 'printf "%s\n" "$1" | grep -qx "advance_ready=50"' _ "$outAlways" + +# ============================================================================= +# PART 2: loop-event.sh — the three ADVANCE prompt variants, driven off a +# scripted fake loop-tick.sh (mirrors loop-event.test.sh's own new_fixture +# pattern — loop-event.sh's own logic is "parse verdict + advance_mode from +# tick stdout, build a prompt", so faking tick's output is the correct unit +# boundary here; census's role in producing that same advance_mode is already +# covered end-to-end by PART 1 above). +# ============================================================================= +new_event_fixture() { + local name="$1" tick_out="$2" + local dir="$work/$name/.claude/scripts" + mkdir -p "$dir" "$work/$name/.claude/state" + cp "$event_src" "$dir/loop-event.sh" + cp "$resolve_roots_src" "$dir/resolve-roots.sh" + cat > "$dir/loop-tick.sh" <" "$1"' _ "$pfPlan" +check "PLAN prompt: instructs applying plan-review + needs-human labels" bash -c \ + 'grep -q "add-label plan-review" "$1" && grep -q "add-label needs-human" "$1"' _ "$pfPlan" +check "PLAN prompt: does NOT drive the full implement flow (no orchestrator/worktree/bot-PR sentence)" bash -c \ + '! grep -qF "Drive issue #61 through the orchestrator" "$1"' _ "$pfPlan" +check "PLAN prompt: does NOT instruct opening a PR (no 'scope -> ... -> bot PR' pipeline text)" bash -c \ + '! grep -qF "reviewer lenses → bot PR" "$1"' _ "$pfPlan" + +# --- implement-gated prompt (advance_mode=implement-gated) ------------------ +dirGated="$(new_event_fixture evt-gated 'cadence=FAST cron=* * * * * +advance_mode=implement-gated +action=advance issue=62')" +outGated="$(run_event "$dirGated")" +pfGated="$(printf '%s\n' "$outGated" | sed -n 's/^loop-event: prompt-file=//p')" +check "implement-gated prompt: verdict line unchanged (action=advance issue=62)" bash -c \ + 'printf "%s\n" "$1" | grep -qxF "loop-event: action=advance issue=62"' _ "$outGated" +check "implement-gated prompt: drives the normal implement flow" bash -c \ + 'grep -qF "Drive issue #62 through the orchestrator" "$1"' _ "$pfGated" +check "implement-gated prompt: fetches the approved plan via the plan-gate:plan marker" bash -c \ + 'grep -qF "" "$1"' _ "$pfGated" +check "implement-gated prompt: injects the plan into implementer AND every reviewer" bash -c \ + 'grep -qi "implementer" "$1" && grep -qi "reviewer" "$1" && grep -qi "authoritative scope" "$1"' _ "$pfGated" +check "implement-gated prompt: exceeding scope is a valid reject reason" bash -c \ + 'grep -qF "exceeds approved scope" "$1"' _ "$pfGated" + +# --- ungated / off prompt (advance_mode=implement, or the line absent +# entirely -- both must produce the SAME, today's-behavior prompt). --------- +dirUngated="$(new_event_fixture evt-ungated 'cadence=FAST cron=* * * * * +advance_mode=implement +action=advance issue=63')" +outUngated="$(run_event "$dirUngated")" +pfUngated="$(printf '%s\n' "$outUngated" | sed -n 's/^loop-event: prompt-file=//p')" +check "ungated prompt: drives the normal implement flow" bash -c \ + 'grep -qF "Drive issue #63 through the orchestrator" "$1"' _ "$pfUngated" +check "ungated prompt: no PLAN ONLY wording" bash -c '! grep -q "PLAN ONLY" "$1"' _ "$pfUngated" +check "ungated prompt: no approved-scope injection clause" bash -c '! grep -qF "exceeds approved scope" "$1"' _ "$pfUngated" + +# advance_mode= line entirely ABSENT (plan.gate=off never emits it) must +# produce the SAME today's-behavior prompt as the explicit "implement" case +# above (structural equivalence, not byte-for-byte: each fixture's prompt +# embeds its own $script_dir path, which necessarily differs per mktemp'd +# fixture directory). +dirAbsent="$(new_event_fixture evt-absent 'cadence=FAST cron=* * * * * +action=advance issue=63')" +outAbsent="$(run_event "$dirAbsent")" +pfAbsent="$(printf '%s\n' "$outAbsent" | sed -n 's/^loop-event: prompt-file=//p')" +check "absent advance_mode= (gate=off): still drives the normal implement flow" bash -c \ + 'grep -qF "Drive issue #63 through the orchestrator" "$1"' _ "$pfAbsent" +check "absent advance_mode= (gate=off): no PLAN ONLY wording" bash -c '! grep -q "PLAN ONLY" "$1"' _ "$pfAbsent" +check "absent advance_mode= (gate=off): no approved-scope injection clause" bash -c '! grep -qF "exceeds approved scope" "$1"' _ "$pfAbsent" + +echo "" +if [ "$fail" -eq 0 ]; then + echo "plan-gate.test.sh: PASS ($ok checks)" + exit 0 +else + echo "plan-gate.test.sh: FAIL (see FAIL lines above)" + exit 1 +fi diff --git a/.claude/self/gates.json b/.claude/self/gates.json index 2e4a5bb..8693bc3 100644 --- a/.claude/self/gates.json +++ b/.claude/self/gates.json @@ -50,5 +50,8 @@ "merge": { "policy": "pr-per-agent", "baseBranch": "main" }, "_notify_note": "issue #99 — needs-human push-notification seam, read by .claude/scripts/notify.sh (GATES_FILE-aware, same empty-means-skip convention as `gates`/`budget`). Empty string here = disabled (offline/CI-safe no-op); left EMPTY deliberately for this repo's own dogfooding — the owner picks a default later. Contract: severity/title/body-line reach the configured command BOTH as positional args ($1/$2/$3) and as NOTIFY_SEVERITY/NOTIFY_TITLE/NOTIFY_BODY env vars. Called by .claude/scripts/needs-human.sh's needs_human_flag() at every loop block-on-owner point (attempt-budget/stall escalation in loop-tick.sh, PR-ready-for-review/re-approval-needed in merge-ready.sh, CHANGES_REQUESTED-addressed-awaiting-re-review in pr-feedback.sh), throttled to one notification per (kind,target) per window (default 1800s, see notify.sh). Example commands (uncomment/adapt ONE):\n ntfy: curl -s -d \"$NOTIFY_BODY\" -H \"Title: $NOTIFY_TITLE\" -H \"Priority: $NOTIFY_SEVERITY\" ntfy.sh/\n notify-send: notify-send \"$NOTIFY_TITLE\" \"$NOTIFY_BODY\"\n webhook: curl -s -X POST -H 'Content-Type: application/json' -d \"{\\\"severity\\\":\\\"$NOTIFY_SEVERITY\\\",\\\"title\\\":\\\"$NOTIFY_TITLE\\\",\\\"body\\\":\\\"$NOTIFY_BODY\\\"}\" https://example.invalid/hook", - "notify": "" + "notify": "", + + "_plan_note": "issue #100 — optional spec/plan gate. plan.gate: off (default; unchanged single-pass advance behavior, byte-identical census output) | label (gate only planned issues that ALSO carry the plan-first label) | always (gate every planned+module candidate). Unknown/missing value falls back to off. Read by loop-census.sh (derives each candidate's plan state from its labels: plan-approved -> gated-approved, else plan-review -> awaiting-owner (blocked on the owner, skipped for advance_ready/fallback_ready like a 'Blocked by' edge), else gated-by-mode -> needs-plan, else ungated), loop-tick.sh (passes advance_mode through as telemetry), and loop-event.sh (builds a PLAN-ONLY driver prompt for needs-plan, injects the approved plan as authoritative scope for gated-approved, and today's single-pass prompt for ungated/off). Left off here so this repo's own self-hosted loop keeps its existing single-pass behavior — see docs/USAGE.md.", + "plan": { "gate": "off" } } diff --git a/docs/USAGE.md b/docs/USAGE.md index debcbaf..98b50ae 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -372,6 +372,39 @@ decide what the loop actually touches: labelled it `planned`. Commenting "approved" on an issue does nothing — nothing watches issue text; the `planned` label is the only approval signal. +**Optional spec/plan gate (`plan.gate`, issue #100).** By default the ADVANCE step goes straight from a +`planned` issue to implementation (one driver turn: scope → worktree implementer → gates → reviewers → +bot PR). `gates.json`'s `plan` block can insert a reviewable **plan artifact** — a structured issue +comment — before any code is written: +- **`plan.gate: "off"`** (default) — today's single-pass behavior, unchanged. +- **`plan.gate: "label"`** — gate only the `planned` candidates that ALSO carry a `plan-first` label. +- **`plan.gate: "always"`** — gate every `planned` + `module:*` candidate, no extra label needed. + +Label lifecycle for a gated issue: +1. The owner adds `plan-first` (label mode only; `always` mode needs nothing extra). +2. The loop's next ADVANCE turn is **PLAN-ONLY**: it reads the issue, scopes it (module, expected files, + approach, acceptance-criteria mapping), and posts **one** structured comment on the issue beginning with + the marker `` — implementing nothing, opening no branch, no PR. It then labels the + issue `plan-review` (awaiting the owner) and `needs-human` (issue #99's signal — see below). +3. The owner reviews the plan comment on GitHub. **To approve:** replace `plan-review` with `plan-approved` + — the next tick implements the issue, with the plan comment's contents injected verbatim into the + implementer's prompt and every reviewer's prompt as the **authoritative scope** (a diff that exceeds the + plan's declared files/approach is a valid reviewer reject: "exceeds approved scope"). **To request + changes:** remove `plan-review` — the issue drops back to needing a fresh plan, and a later tick re-plans + it from scratch. + +While an issue sits in `plan-review` (posted, not yet approved), `loop-census.sh` treats it like a `Blocked +by` dependency: it is skipped for `advance_ready` entirely — the loop will not implement a plan the owner +hasn't signed off on. + +**Owner-only approval, same caveat as `planned`:** the loop only checks whether the `plan-approved` label is +present, not *who* added it — GitHub identity is not enforced here any more than it is for `planned` itself +(see the two-label workflow above). Treat `plan-approved` as an owner-only convention, not a +technically-enforced gate. + +This reuses issue #99's `needs-human` signal (label + one-time comment + throttled notification) as the +"plan is ready for your review" ping, rather than inventing a second escalation channel. + **Model selection.** Drive the loop's tick sessions with **Sonnet**. Ticks are cheap but highly repetitive, and repetition is where smaller models degrade: a Haiku-driven tick session has been observed to stop invoking the step scripts entirely — fabricating census/merge output from the pattern of earlier