From 576577b8648c509edab8f31d2f698b39abf9fb02 Mon Sep 17 00:00:00 2001 From: Roberto Cano <3525807+robercano@users.noreply.github.com> Date: Thu, 16 Jul 2026 16:00:01 +0200 Subject: [PATCH] feat(loop): gate advance_ready on open "Blocked by #N" edges (issue #97) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit loop-census.sh's ADVANCE selection now skips a planned candidate while any issue it declares "Blocked by" (via cockpit.sh's existing --parse-blocking parser, reused not reimplemented) is still open, emitting a `blocked= by=` census line and picking the next unblocked lowest-numbered candidate instead. Since census re-runs every tick, a blocker closing makes the skipped issue eligible again for free. A "Blocked by" cycle falls back to the lowest-numbered candidate (logged to stderr) instead of wedging the loop. Task-list/parent-child refs (`- [ ] #N`) intentionally do not gate — only the explicit phrase does. Co-Authored-By: Claude Sonnet 5 --- .claude/scripts/loop-census.sh | 103 +++++++++++- .claude/scripts/loop-census.test.sh | 240 ++++++++++++++++++++++++++++ docs/USAGE.md | 8 +- 3 files changed, 347 insertions(+), 4 deletions(-) diff --git a/.claude/scripts/loop-census.sh b/.claude/scripts/loop-census.sh index ece9219..d6d584a 100644 --- a/.claude/scripts/loop-census.sh +++ b/.claude/scripts/loop-census.sh @@ -15,8 +15,15 @@ # hasn't reached PR stage. A tick uses this to # avoid double-spawning an orchestrator for an # issue that already has a worktree in progress. +# blocked= by= one line per candidate that would otherwise be +# advance_ready but is skipped because its body +# says "Blocked by #N" and issue N is still OPEN +# (issue #97). Only the first open blocker per +# candidate is reported — one is enough to +# explain the skip; a candidate may have more. # 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 # cadence=FAST|WATCH|IDLE cron= desired cadence per the loop policy # # The module label set is derived from $GATES_FILE (default .claude/gates.json) @@ -28,6 +35,28 @@ # `tail -1` — reported "No actionable activity" while ADVANCE work sat ready. # A tick may claim "No actionable activity" ONLY when this census prints zeros. # +# --- BLOCKING-GRAPH GATE (issue #97) ---------------------------------------- +# advance_ready additionally skips any otherwise-eligible candidate (branch= +# none, open_prs=0, no active driver) whose body contains an explicit +# "Blocked by #N" edge to an issue that is STILL OPEN. We reuse cockpit.sh's +# ONE `--parse-blocking` parser (shelled out to, never reimplemented here) to +# extract that edge — we deliberately do NOT gate on task-list/parent-child +# refs (`- [ ] #N`): those are cosmetic tracker structure, and tracking issues +# stay in `backlog` forever precisely so the loop works the chain and never +# the tracker (see docs/USAGE.md). Because this census re-runs every tick, a +# blocker closing makes the previously-blocked issue eligible again for free +# — no extra bookkeeping needed. +# +# CYCLE SAFETY: bash has no cheap topological-sort/cycle-detection story, and +# this script doesn't attempt one. If, after applying the blocker filter, NO +# planned candidate qualifies as advance_ready — which is exactly what a +# "Blocked by" cycle among planned issues produces, as well as the +# genuinely-all-blocked case — we fall back to the lowest-numbered otherwise- +# eligible candidate (branch=none, open_prs=0, no active driver — ignoring +# the block) and log that fallback to stderr. This only fires when at least +# one candidate was otherwise eligible; with zero eligible candidates, +# advance_ready stays "none" exactly as before this feature. +# # Repo derived from the git remote; override with $1. Bot login via $BOT_LOGIN. # Invoke as `bash .claude/scripts/loop-census.sh` (pre-approve that exact # command). Read-only: advances no cursor, mutates nothing — safe to re-run. @@ -59,6 +88,27 @@ driver_unit_active() { case "$st" in active|activating) return 0 ;; *) return 1 ;; esac } +# --- blocking-graph helpers (issue #97) ------------------------------------- +# is_open_issue: is issue number $1 present in the pre-fetched open-issue set? +is_open_issue() { + [ -n "$open_issue_set" ] || return 1 + printf '%s\n' "$open_issue_set" | grep -qx "$1" +} + +# get_blocked_by: given the JSON emitted by `cockpit.sh --parse-blocking` +# (or an empty string), print each blockedBy issue number on its own line. +# Guarded against empty/malformed input — never aborts the caller. +get_blocked_by() { + local json="$1" + [ -n "$json" ] || return 0 + node -e ' + try { + const o = JSON.parse(process.argv[1] || "{}"); + (o.blockedBy || []).forEach((n) => console.log(n)); + } catch (e) { /* malformed/empty — print nothing */ } + ' "$json" 2>/dev/null || true +} + open_prs=$(gh pr list -R "$repo" --state open --base "$base" --json number --jq 'length') echo "open_prs=$open_prs" @@ -66,6 +116,13 @@ echo "open_prs=$open_prs" # in_flight (branch exists, no PR yet) apart from already-at-PR-stage. open_pr_branches=$(gh pr list -R "$repo" --state open --base "$base" --json headRefName --jq '.[].headRefName') +# All open issue numbers (bounded --limit, matching cockpit.sh's own --state +# open fetch) — used to decide whether a candidate's "Blocked by #N" target +# is still open. `|| true` guards a transient gh failure from wedging the +# whole census; an empty set just makes is_open_issue always report false, +# i.e. the blocking gate degrades to a no-op (same as before this feature). +open_issue_set=$(gh issue list -R "$repo" --state open --json number --jq '.[].number' --limit 200 2>/dev/null) || true + feedback_prs=$(bash "$script_dir/pr-feedback.sh" "$repo" | grep -c . || true) echo "feedback_prs=$feedback_prs" @@ -75,8 +132,10 @@ planned=$(gh issue list -R "$repo" --state open --label planned --json number,ti planned_count=0 advance_ready="none" +fallback_ready="none" detail="" in_flight="" +blocked_lines="" while IFS=$'\t' read -r num labels title; do [ -z "${num:-}" ] && continue hit=0 @@ -93,10 +152,40 @@ while IFS=$'\t' read -r num labels title; do branch=$(git -C "$root" branch -a --list "*feat/issue-$num-*" | head -1 | sed 's/^[* ]*//;s|^remotes/||') || true [ -n "$branch" ] || branch="none" detail+="issue=$num branch=$branch title=$title"$'\n' - if [ "$advance_ready" = "none" ] && [ "$branch" = "none" ] && [ "$open_prs" -eq 0 ] \ - && ! driver_unit_active "$num"; then - advance_ready="$num" + + eligible=0 + if [ "$branch" = "none" ] && [ "$open_prs" -eq 0 ] && ! driver_unit_active "$num"; then + eligible=1 + 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" + fi + if [ "$eligible" -eq 1 ] && [ "$advance_ready" = "none" ]; then + # Fetch this candidate's body only now — we're actually considering it. + body=$(gh issue view "$num" -R "$repo" --json body --jq '.body // ""' 2>/dev/null) || true + parse_json="" + if [ -n "$body" ]; then + parse_json=$(printf '%s' "$body" | bash "$script_dir/cockpit.sh" --parse-blocking 2>/dev/null) || true + fi + first_open_blocker="" + if [ -n "$parse_json" ]; then + while IFS= read -r bnum; do + [ -n "$bnum" ] || continue + if is_open_issue "$bnum"; then + first_open_blocker="$bnum" + break + fi + done <<< "$(get_blocked_by "$parse_json")" + fi + if [ -n "$first_open_blocker" ]; then + blocked_lines+="blocked=$num by=$first_open_blocker"$'\n' + else + advance_ready="$num" + fi fi + # in_flight: a branch exists for this issue but no open PR carries it yet # (branch may be printed with a "origin/" remote prefix above; strip it — # or match it as a "/"-suffix — before comparing against headRefName, which @@ -113,9 +202,17 @@ while IFS=$'\t' read -r num labels title; do fi done <<< "$planned" +# Cycle / all-blocked fallback (issue #97): only trips when at least one +# candidate was otherwise eligible but every one of them got blocked. +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" +fi + echo "planned_issues=$planned_count" [ -n "$detail" ] && printf '%s' "$detail" [ -n "$in_flight" ] && printf '%s' "$in_flight" +[ -n "$blocked_lines" ] && printf '%s' "$blocked_lines" 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-census.test.sh b/.claude/scripts/loop-census.test.sh index fe0f47b..0e11b41 100644 --- a/.claude/scripts/loop-census.test.sh +++ b/.claude/scripts/loop-census.test.sh @@ -38,6 +38,7 @@ set -uo pipefail script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" census_src="$script_dir/loop-census.sh" resolve_roots_src="$script_dir/resolve-roots.sh" +cockpit_src="$script_dir/cockpit.sh" work="$(mktemp -d "${TMPDIR:-/tmp}/loop-census-test.XXXXXX")" trap 'rm -rf "$work"' EXIT @@ -262,6 +263,245 @@ outB="$(env -u GATES_FILE PATH="$curated_bin" bash "$dirB/.claude/scripts/loop-c check "driver_unit_active guard (b): systemctl unavailable — guard no-ops, advance_ready falls back to issue 5" bash -c ' printf "%s\n" "$1" | grep -qx "advance_ready=5"' _ "$outB" +# --------------------------------------------------------------------------- +# Blocking-graph gate (issue #97): advance_ready must skip a candidate whose +# body says "Blocked by #N" while N is still OPEN, emit a `blocked= by=` +# census line for it, and pick the next unblocked lowest-numbered candidate +# instead. A blocker closing (dropping out of the open-issue set) must make +# the previously-blocked candidate eligible again on the very next run — no +# extra state, since census re-derives everything from the current gh state +# every time it's invoked. A "Blocked by" cycle between two planned issues +# must not wedge the loop: fall back to the lowest-numbered of the cycle and +# log the fallback to stderr. Task-list refs (`- [ ] #N`) must NOT gate. +# +# Each fixture below reuses the REAL cockpit.sh (`--parse-blocking` seam), +# copied in verbatim — never reimplemented — plus a stub bot-gh.sh that +# dispatches on `issue list` (with/without `--label`, to tell the planned- +# issue TSV fetch apart from the all-open-issue-numbers fetch) and +# `issue view --json body` (per-candidate body fetch). +# --------------------------------------------------------------------------- +scaffold_blocking_fixture() { + local dir="$1" + local scripts="$dir/.claude/scripts" + mkdir -p "$scripts" + cp "$census_src" "$scripts/loop-census.sh" + cp "$resolve_roots_src" "$scripts/resolve-roots.sh" + cp "$cockpit_src" "$scripts/cockpit.sh" + cat > "$dir/.claude/gates.json" <<'EOF' +{ + "modules": [{ "name": "test", "path": ".", "description": "", "owner": "" }], + "merge": { "baseBranch": "main" } +} +EOF + cat > "$scripts/pr-feedback.sh" <<'EOF' +#!/usr/bin/env bash +exit 0 +EOF + chmod +x "$scripts/pr-feedback.sh" "$scripts/cockpit.sh" "$scripts/loop-census.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 +} + +run_blocking_fixture() { + # $1 = fixture dir, $2 = stderr capture file. Stdout returned on stdout. + env -u GATES_FILE bash "$1/.claude/scripts/loop-census.sh" "acme/repo" 2>"$2" +} + +# --- (a)+(b): issue 20 "Blocked by #99", issue 21 no blockers. Two states of +# the SAME fixture shape, differing only in whether 99 is in the open set. --- +dirBlockOpen="$work/blockOpen" +scaffold_blocking_fixture "$dirBlockOpen" +cat > "$dirBlockOpen/.claude/scripts/bot-gh.sh" <<'EOF' +#!/usr/bin/env bash +case "$1" in + repo) echo "acme/repo" ;; + pr) + if printf '%s\n' "$*" | grep -q 'headRefName'; then + : # no open PRs + else + echo 0 + fi + ;; + issue) + case "$2" in + list) + if printf '%s\n' "$*" | grep -q -- '--label'; then + printf '20\tplanned,module:test\tCandidate twenty\n' + printf '21\tplanned,module:test\tCandidate twenty one\n' + else + # all-open-issue-numbers fetch: 99 (the blocker) is still OPEN. + printf '20\n21\n99\n' + fi + ;; + view) + case "$3" in + 20) echo '{"body":"Blocked by #99"}' ;; + 21) echo '{"body":"no blockers here"}' ;; + *) echo '{"body":""}' ;; + esac + ;; + *) echo "unhandled issue subcmd: $*" >&2; exit 1 ;; + esac + ;; + *) echo "fake-bot-gh.sh: unhandled args: $*" >&2; exit 1 ;; +esac +EOF +chmod +x "$dirBlockOpen/.claude/scripts/bot-gh.sh" +errBlockOpen="$work/blockOpen.stderr" +outBlockOpen="$(run_blocking_fixture "$dirBlockOpen" "$errBlockOpen")" + +check "(a) issue 20 blocked by OPEN #99 is not advance_ready" bash -c \ + '! printf "%s\n" "$1" | grep -qx "advance_ready=20"' _ "$outBlockOpen" +check "(a) blocked=20 by=99 census line emitted" bash -c \ + 'printf "%s\n" "$1" | grep -qx "blocked=20 by=99"' _ "$outBlockOpen" +check "(a) advance_ready instead picks unblocked candidate 21" bash -c \ + 'printf "%s\n" "$1" | grep -qx "advance_ready=21"' _ "$outBlockOpen" + +# Same fixture, but 99 has since been closed (dropped from the open set) — +# copy the fixture and swap only the bot-gh.sh's open-issue-numbers branch. +dirBlockClosed="$work/blockClosed" +scaffold_blocking_fixture "$dirBlockClosed" +cat > "$dirBlockClosed/.claude/scripts/bot-gh.sh" <<'EOF' +#!/usr/bin/env bash +case "$1" in + repo) echo "acme/repo" ;; + pr) + if printf '%s\n' "$*" | grep -q 'headRefName'; then + : # no open PRs + else + echo 0 + fi + ;; + issue) + case "$2" in + list) + if printf '%s\n' "$*" | grep -q -- '--label'; then + printf '20\tplanned,module:test\tCandidate twenty\n' + printf '21\tplanned,module:test\tCandidate twenty one\n' + else + # all-open-issue-numbers fetch: 99 (the blocker) is now CLOSED — + # absent from this list entirely. + printf '20\n21\n' + fi + ;; + view) + case "$3" in + 20) echo '{"body":"Blocked by #99"}' ;; + 21) echo '{"body":"no blockers here"}' ;; + *) echo '{"body":""}' ;; + esac + ;; + *) echo "unhandled issue subcmd: $*" >&2; exit 1 ;; + esac + ;; + *) echo "fake-bot-gh.sh: unhandled args: $*" >&2; exit 1 ;; +esac +EOF +chmod +x "$dirBlockClosed/.claude/scripts/bot-gh.sh" +errBlockClosed="$work/blockClosed.stderr" +outBlockClosed="$(run_blocking_fixture "$dirBlockClosed" "$errBlockClosed")" + +check "(b) blocker #99 closed -> no blocked=20 line" bash -c \ + '! printf "%s\n" "$1" | grep -qx "blocked=20 by=99"' _ "$outBlockClosed" +check "(b) issue 20 becomes advance_ready again once its blocker closes" bash -c \ + 'printf "%s\n" "$1" | grep -qx "advance_ready=20"' _ "$outBlockClosed" + +# --- (c): cycle — issue 30 "Blocked by #31", issue 31 "Blocked by #30", both +# open+planned. Must not wedge: falls back to the lowest-numbered (#30) and +# logs the fallback to stderr. --- +dirCycle="$work/cycle" +scaffold_blocking_fixture "$dirCycle" +cat > "$dirCycle/.claude/scripts/bot-gh.sh" <<'EOF' +#!/usr/bin/env bash +case "$1" in + repo) echo "acme/repo" ;; + pr) + if printf '%s\n' "$*" | grep -q 'headRefName'; then + : # no open PRs + else + echo 0 + fi + ;; + issue) + case "$2" in + list) + if printf '%s\n' "$*" | grep -q -- '--label'; then + printf '30\tplanned,module:test\tCandidate thirty\n' + printf '31\tplanned,module:test\tCandidate thirty one\n' + else + printf '30\n31\n' + fi + ;; + view) + case "$3" in + 30) echo '{"body":"Blocked by #31"}' ;; + 31) echo '{"body":"Blocked by #30"}' ;; + *) echo '{"body":""}' ;; + esac + ;; + *) echo "unhandled issue subcmd: $*" >&2; exit 1 ;; + esac + ;; + *) echo "fake-bot-gh.sh: unhandled args: $*" >&2; exit 1 ;; +esac +EOF +chmod +x "$dirCycle/.claude/scripts/bot-gh.sh" +errCycle="$work/cycle.stderr" +outCycle="$(run_blocking_fixture "$dirCycle" "$errCycle")" + +check "(c) cycle: advance_ready is non-none (loop does not wedge)" bash -c \ + '! printf "%s\n" "$1" | grep -qx "advance_ready=none"' _ "$outCycle" +check "(c) cycle: advance_ready falls back to the lowest-numbered issue (30)" bash -c \ + 'printf "%s\n" "$1" | grep -qx "advance_ready=30"' _ "$outCycle" +check "(c) cycle: both directions reported as blocked=" bash -c \ + 'printf "%s\n" "$1" | grep -qx "blocked=30 by=31" && printf "%s\n" "$1" | grep -qx "blocked=31 by=30"' _ "$outCycle" +check "(c) cycle: fallback logged to stderr" bash -c \ + 'grep -q "all planned candidates blocked" "$1" && grep -q "falling back to lowest-number #30" "$1"' _ "$errCycle" + +# --- (d): task-list edge (`- [ ] #N`) but NO "Blocked by" — must NOT gate. --- +dirTasklist="$work/tasklist" +scaffold_blocking_fixture "$dirTasklist" +cat > "$dirTasklist/.claude/scripts/bot-gh.sh" <<'EOF' +#!/usr/bin/env bash +case "$1" in + repo) echo "acme/repo" ;; + pr) + if printf '%s\n' "$*" | grep -q 'headRefName'; then + : # no open PRs + else + echo 0 + fi + ;; + issue) + case "$2" in + list) + if printf '%s\n' "$*" | grep -q -- '--label'; then + printf '40\tplanned,module:test\tTracker forty\n' + else + printf '40\n41\n' + fi + ;; + view) + case "$3" in + 40) echo '{"body":"- [ ] #41 sub-task, not a blocker"}' ;; + *) echo '{"body":""}' ;; + esac + ;; + *) echo "unhandled issue subcmd: $*" >&2; exit 1 ;; + esac + ;; + *) echo "fake-bot-gh.sh: unhandled args: $*" >&2; exit 1 ;; +esac +EOF +chmod +x "$dirTasklist/.claude/scripts/bot-gh.sh" +errTasklist="$work/tasklist.stderr" +outTasklist="$(run_blocking_fixture "$dirTasklist" "$errTasklist")" + +check "(d) task-list ref alone does not gate — advance_ready=40" bash -c \ + 'printf "%s\n" "$1" | grep -qx "advance_ready=40"' _ "$outTasklist" +check "(d) no blocked= line emitted for a task-list-only reference" bash -c \ + '! printf "%s\n" "$1" | grep -q "^blocked="' _ "$outTasklist" + echo "" if [ "$fail" -eq 0 ]; then echo "loop-census.test.sh: PASS ($ok checks)" diff --git a/docs/USAGE.md b/docs/USAGE.md index bab2c97..debcbaf 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -356,7 +356,13 @@ decide what the loop actually touches: The ADVANCE step picks the **lowest-numbered open issue labelled `planned` + `module:*`** with no existing `feat/issue--*` branch — one at a time, and only when there are zero open PRs. Tracking issues (plans split into `Blocked by` sub-issue chains) stay `backlog` forever so the loop works the - chain, never the tracker. + chain, never the tracker. **"Blocked by #N" in an issue body is load-bearing for this selection, not + merely cosmetic for the cockpit graph** (`loop-census.sh`, issue #97): the census skips a `planned` + candidate while any issue it declares "Blocked by" is still open, emits a `blocked= by=` line + explaining the skip, and picks the next unblocked lowest-numbered candidate instead — a blocker + closing makes the skipped issue eligible again on the very next tick, no extra bookkeeping required. + A "Blocked by" cycle falls back to the lowest-numbered candidate rather than wedging the loop. Only + the explicit "Blocked by" phrase gates; task-list/parent-child refs (`- [ ] #N`) do not. - **Owner-approval merge gate.** Workers author PRs as the **bot** (`bot-gh.sh`); the MERGE step (above) only merges PRs the repo **owner** has Approved on GitHub that are CI-green and mergeable. It never approves on the owner's behalf.