Skip to content

fix(needs-human): REST label ops + confirmed-episode guard — stop per-tick comment spam (issue #169) #171

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
robercano-ghbot merged 1 commit into from
Jul 21, 2026
Merged

fix(needs-human): REST label ops + confirmed-episode guard — stop per-tick comment spam (issue #169) #171

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
39 changes: 33 additions & 6 deletions .claude/scripts/loop-ceilings.test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ new_fixture() {
# gh calls still landing only in this fixture's own logging bot-gh.sh.
cp "$script_dir/needs-human.sh" "$dir/needs-human.sh"
cp "$script_dir/notify.sh" "$dir/notify.sh"
cp "$script_dir/log-event.sh" "$dir/log-event.sh"

cat > "$dir/loop-census.sh" <<EOF
#!/usr/bin/env bash
Expand Down Expand Up @@ -105,15 +106,41 @@ EOF
# canned output for the two calls the ceiling code actually reads stdout
# from: `gh issue create` (needs a URL ending in a number) and
# `gh issue view --json state --jq .state` (needs OPEN/CLOSED).
#
# Issue #169: needs-human.sh's label reads/writes now go through `gh api`
# (REST) instead of `gh pr/issue edit --*-label`/`gh pr|issue view --json
# labels`. This stub simulates GitHub's own per-issue/PR label state via
# marker FILES the REST add/remove touch/remove (keyed by issue/PR
# number, under ../state/gh-labels/), so the post-add CONFIRM read
# (issue #169's comment-gating invariant) sees the label actually
# "stuck" for the attempt-budget escalation scenarios below (5, 6).
cat > "$dir/bot-gh.sh" <<'EOF'
#!/usr/bin/env bash
log="$(dirname "$0")/../state/gh-calls.log"
mkdir -p "$(dirname "$log")"
labels_dir="$(dirname "$0")/../state/gh-labels"
mkdir -p "$(dirname "$log")" "$labels_dir"
printf '%s\n' "$*" >> "$log"
case "$1 $2" in
"issue create") echo "https://github.com/acme/repo/issues/777" ;;
"issue view") echo "${FAKE_ISSUE_STATE:-OPEN}" ;;
esac
if [ "$1" = "api" ]; then
case "$*" in
*"-X POST"*"/labels --input -"*)
num="$(printf '%s\n' "$*" | sed -E 's#.*/issues/([0-9]+)/labels --input -.*#\1#')"
touch "$labels_dir/$num"
;;
*"-X DELETE"*"/labels/needs-human"*)
num="$(printf '%s\n' "$*" | sed -E 's#.*/issues/([0-9]+)/labels/needs-human.*#\1#')"
rm -f "$labels_dir/$num"
;;
*"-q .labels[].name"*)
num="$(printf '%s\n' "$*" | sed -E 's#.*/issues/([0-9]+) .*#\1#')"
[ -f "$labels_dir/$num" ] && echo "needs-human"
;;
*) : ;;
esac
fi
exit 0
EOF
chmod +x "$dir/bot-gh.sh"
Expand Down Expand Up @@ -250,16 +277,16 @@ node -e '
out5="$(run_tick "$dir5")"
check "scenario 5 (attempts 5 >= budget 5): verdict is action=none" bash -c '[ "$(verdict_of "$1")" = "action=none" ]' _ "$out5"
check "scenario 5: diagnostic cites the attempt budget" bash -c 'printf "%s\n" "$1" | grep -q "attempt budget exceeded for issue=42"' _ "$out5"
check "scenario 5: exactly 4 gh calls (label-presence read, label create, issue edit, issue comment)" bash -c '[ "$(gh_calls "$1" | wc -l | tr -d " ")" -eq 4 ]' _ "$dir5"
check "scenario 5: the issue itself (not a PR) was labeled needs-human" bash -c 'gh_calls "$1" | grep -q "^issue edit 42 --add-label needs-human"' _ "$dir5"
check "scenario 5: exactly 5 gh calls (ensure-label create, label-presence read, label add, confirm read, issue comment -- issue #169 REST)" bash -c '[ "$(gh_calls "$1" | wc -l | tr -d " ")" -eq 5 ]' _ "$dir5"
check "scenario 5: the issue itself (not a PR) was labeled needs-human via REST" bash -c 'gh_calls "$1" | grep -qF -- "-X POST repos/acme/repo/issues/42/labels --input -"' _ "$dir5"
check "scenario 5: escalated is now persisted true, attempts unchanged at 5" node -e '
const fs = require("fs");
const j = JSON.parse(fs.readFileSync(process.argv[1], "utf8"));
if (j["42"].escalated !== true || j["42"].attempts !== 5) throw new Error("got " + JSON.stringify(j));
' "$dir5/../state/loop-issue-attempts.json"
out5b="$(run_tick "$dir5")"
check "scenario 5b (still over budget, second tick): verdict is still action=none" bash -c '[ "$(verdict_of "$1")" = "action=none" ]' _ "$out5b"
check "scenario 5b: no additional gh calls (escalated guard held) -- still exactly 4" bash -c '[ "$(gh_calls "$1" | wc -l | tr -d " ")" -eq 4 ]' _ "$dir5"
check "scenario 5b: no additional gh calls (escalated guard held) -- still exactly 5" bash -c '[ "$(gh_calls "$1" | wc -l | tr -d " ")" -eq 5 ]' _ "$dir5"

# ---------------------------------------------------------------------------
# 6. Per-issue attempt budget applies across advance AND feedback phases of
Expand All @@ -275,7 +302,7 @@ node -e '
' "$dir6"
out6="$(run_tick "$dir6")"
check "scenario 6 (feedback for issue 42's PR, budget already exhausted): verdict is action=none" bash -c '[ "$(verdict_of "$1")" = "action=none" ]' _ "$out6"
check "scenario 6: the PR (17), not the issue, was labeled/commented needs-human" bash -c 'gh_calls "$1" | grep -q "^pr edit 17 --add-label needs-human" && gh_calls "$1" | grep -q "^pr comment 17"' _ "$dir6"
check "scenario 6: the PR (17), not the issue, was labeled/commented needs-human" bash -c 'gh_calls "$1" | grep -qF -- "-X POST repos/acme/repo/issues/17/labels --input -" && gh_calls "$1" | grep -q "^pr comment 17"' _ "$dir6"

# ---------------------------------------------------------------------------
# 7. Daily action ceiling: UNDER the ceiling (default 50) -> advance
Expand Down Expand Up @@ -437,7 +464,7 @@ node -e '
out11="$(run_tick "$dir11")"
check "scenario 11 (ci-fix for issue 42's PR, budget already exhausted): verdict is action=none" bash -c '[ "$(verdict_of "$1")" = "action=none" ]' _ "$out11"
check "scenario 11: diagnostic cites the attempt budget for issue=42, not PR 23" bash -c 'printf "%s\n" "$1" | grep -q "attempt budget exceeded for issue=42"' _ "$out11"
check "scenario 11: the PR (23), not the issue, was labeled/commented needs-human" bash -c 'gh_calls "$1" | grep -q "^pr edit 23 --add-label needs-human" && gh_calls "$1" | grep -q "^pr comment 23"' _ "$dir11"
check "scenario 11: the PR (23), not the issue, was labeled/commented needs-human" bash -c 'gh_calls "$1" | grep -qF -- "-X POST repos/acme/repo/issues/23/labels --input -" && gh_calls "$1" | grep -q "^pr comment 23"' _ "$dir11"

# ---------------------------------------------------------------------------
# 12. Precedence (issue #96): a ci-fix candidate wins over an ALSO-ready
Expand Down
29 changes: 25 additions & 4 deletions .claude/scripts/loop-tick.test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -647,13 +647,34 @@ check "scenario 16: resume-attempts file now records count=2" node -e '
' "$resume15"

# Scenario 17: 3rd stall (count already at 2) -> escalate to needs-human
# instead of resuming again: label create + issue edit + issue comment (all
# instead of resuming again: label create + label add + issue comment (all
# via bot-gh.sh, captured here into a plain log file), escalated:true
# persisted, and an escalated-to-needs-human event logged.
#
# Issue #169: needs-human.sh's label reads/writes now go through `gh api`
# (REST). This stub simulates GitHub's own label state via a marker FILE the
# REST add touches, so the post-add CONFIRM read (issue #169's
# comment-gating invariant) sees the label actually "stuck" and the episode
# comment posts.
gh_calls17="$work/scenario17-gh-calls.log"
label_marker17="$work/scenario17-label.marker"
cat > "$dir15/bot-gh.sh" <<EOF
#!/usr/bin/env bash
printf '%s\n' "\$*" >> "$gh_calls17"
case "\$1" in
api)
case "\$*" in
*"-X POST"*"/issues/42/labels --input -")
touch "$label_marker17"
;;
*"-q .labels[].name"*)
[ -f "$label_marker17" ] && printf 'needs-human\n'
;;
*) : ;;
esac
;;
*) : ;;
esac
exit 0
EOF
chmod +x "$dir15/bot-gh.sh"
Expand All @@ -664,9 +685,9 @@ check "scenario 17: resume-attempts file now escalated=true (count stays 2)" nod
const j = JSON.parse(fs.readFileSync(process.argv[1], "utf8"));
if (!j["42"] || j["42"].count !== 2 || j["42"].escalated !== true) throw new Error("got " + JSON.stringify(j));
' "$resume15"
check "scenario 17: needs-human label create + issue edit + issue comment all dispatched via bot-gh.sh" bash -c '
grep -q "^label create needs-human" "$1" &&
grep -q "^issue edit 42 --add-label needs-human" "$1" &&
check "scenario 17: needs-human label create + label add (REST) + issue comment all dispatched via bot-gh.sh" bash -c '
grep -qF -- "-X POST repos/acme/repo/labels " "$1" &&
grep -qF -- "-X POST repos/acme/repo/issues/42/labels --input -" "$1" &&
grep -q "^issue comment 42 " "$1"
' _ "$gh_calls17"
check "scenario 17: escalated-to-needs-human event logged" bash -c 'grep -q "\"phase\":\"escalated-to-needs-human\"" "$1"' _ "$ticks15_events"
Expand Down
78 changes: 42 additions & 36 deletions .claude/scripts/merge-ready.test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@
# comment, `pr merge`) that flows through the real needs-human.sh seam this
# script sources.
#
# Issue #169: needs-human.sh's label reads/writes now go through `gh api`
# (REST) instead of `gh pr edit --*-label`/`gh pr view --json labels`. The
# fake bot-gh.sh below simulates GitHub's own label state via a marker FILE
# the REST add touches, so the post-add CONFIRM read (issue #169's
# comment-gating invariant) sees the label actually "stuck".
#
# Exit 0 on success, non-zero if any assertion fails. Runnable bare:
# bash .claude/scripts/merge-ready.test.sh
set -uo pipefail
Expand Down Expand Up @@ -52,6 +58,7 @@ new_fixture() {
cp "$script_dir/resolve-roots.sh" "$scripts/resolve-roots.sh"
cp "$script_dir/needs-human.sh" "$scripts/needs-human.sh"
cp "$script_dir/notify.sh" "$scripts/notify.sh"
cp "$script_dir/log-event.sh" "$scripts/log-event.sh"
chmod +x "$scripts"/*.sh
cat > "$dir/.claude/gates.json" <<EOF
{ "merge": { "baseBranch": "main" }, "notify": "printf 'fired\\n' >> $work/$name-notify-fired.txt" }
Expand Down Expand Up @@ -90,26 +97,26 @@ case "\$1" in
fi
;;
view)
if printf '%s\n' "\$*" | grep -q -- '--json labels'; then
[ -f "$labeled_markerA" ] && printf 'needs-human\n'
else
cat <<'JSON'
cat <<'JSON'
{"number":10,"title":"Add widget","isDraft":false,"baseRefName":"main","headRefName":"feat/issue-10-widget","mergeable":"MERGEABLE","reviews":[],"statusCheckRollup":[],"commits":[{"committedDate":"2026-01-01T00:00:00Z"}]}
JSON
fi
;;
edit)
if printf '%s\n' "\$*" | grep -q -- '--add-label needs-human'; then
touch "$labeled_markerA"
fi
;;
comment) : ;;
merge) exit 1 ;;
*) : ;;
esac
;;
label) : ;;
issue) : ;;
api)
case "\$*" in
*"-X POST"*"/issues/10/labels --input -")
touch "$labeled_markerA"
;;
*"-q .labels[].name"*)
[ -f "$labeled_markerA" ] && printf 'needs-human\n'
;;
*) : ;;
esac
;;
*) echo "unhandled: \$*" >&2; exit 1 ;;
esac
EOF
Expand All @@ -120,7 +127,7 @@ outA2="$(env -u GATES_FILE bash "$dirA/.claude/scripts/merge-ready.sh" "acme/rep

check "A: first run's verdict is skip:no-owner-review" bash -c 'printf "%s\n" "$1" | grep -q "\"reason\":\"no-owner-review\""' _ "$outA1"
check "A: second run's verdict is ALSO skip:no-owner-review (still unreviewed)" bash -c 'printf "%s\n" "$1" | grep -q "\"reason\":\"no-owner-review\""' _ "$outA2"
check "A: needs-human label add attempted on BOTH runs" bash -c '[ "$(grep -c "pr edit 10 --add-label needs-human" "$1")" -eq 2 ]' _ "$gh_logA"
check "A: needs-human label add attempted (REST) on BOTH runs" bash -c '[ "$(grep -c -- "-X POST repos/acme/repo/issues/10/labels --input -" "$1")" -eq 2 ]' _ "$gh_logA"
check "A: exactly ONE comment across BOTH runs (episode-gated, finding #1)" bash -c '[ "$(grep -c "pr comment 10 --body" "$1")" -eq 1 ]' _ "$gh_logA"
check "A: no merge was attempted (skip path)" bash -c '! grep -q "^pr merge" "$1"' _ "$gh_logA"

Expand All @@ -132,6 +139,7 @@ check "A: no merge was attempted (skip path)" bash -c '! grep -q "^pr merge" "$1
# ---------------------------------------------------------------------------
dirB="$(new_fixture scenarioB)"
gh_logB="$work/scenarioB-gh.log"
labeled_markerB="$work/scenarioB-labeled.marker"
cat > "$dirB/.claude/scripts/bot-gh.sh" <<EOF
#!/usr/bin/env bash
printf '%s\n' "\$*" >> "$gh_logB"
Expand All @@ -144,31 +152,35 @@ case "\$1" in
fi
;;
view)
if printf '%s\n' "\$*" | grep -q -- '--json labels'; then
: # never labeled yet -- fresh episode
else
cat <<'JSON'
cat <<'JSON'
{"number":11,"title":"Fix bug","isDraft":false,"baseRefName":"main","headRefName":"fix/issue-11-bug","mergeable":"MERGEABLE","reviews":[{"author":{"login":"acme"},"state":"APPROVED","submittedAt":"2026-01-01T00:00:00Z"}],"statusCheckRollup":[],"commits":[{"committedDate":"2026-01-02T00:00:00Z"}]}
JSON
fi
;;
edit) : ;;
comment) : ;;
merge) exit 1 ;;
*) : ;;
esac
;;
label) : ;;
issue) : ;;
api)
case "\$*" in
*"-X POST"*"/issues/11/labels --input -")
touch "$labeled_markerB"
;;
*"-q .labels[].name"*)
[ -f "$labeled_markerB" ] && printf 'needs-human\n'
;;
*) : ;;
esac
;;
*) echo "unhandled: \$*" >&2; exit 1 ;;
esac
EOF
chmod +x "$dirB/.claude/scripts/bot-gh.sh"
outB="$(env -u GATES_FILE bash "$dirB/.claude/scripts/merge-ready.sh" "acme/repo" 2>&1)"

check "B: verdict is skip:approval-stale" bash -c 'printf "%s\n" "$1" | grep -q "approval-stale"' _ "$outB"
check "B: label add attempted (fresh episode)" grep -q "pr edit 11 --add-label needs-human" "$gh_logB"
check "B: comment posted (fresh episode, ready-for-review body)" bash -c 'grep -q "pr comment 11 --body" "$1"' _ "$gh_logB"
check "B: label add attempted via REST (fresh episode)" grep -qF -- "-X POST repos/acme/repo/issues/11/labels --input -" "$gh_logB"
check "B: comment posted (fresh, CONFIRMED episode, ready-for-review body)" bash -c 'grep -q "pr comment 11 --body" "$1"' _ "$gh_logB"

# ---------------------------------------------------------------------------
# C. Successful MERGE: owner-approved, CI-green, head_branch matches
Expand All @@ -190,22 +202,16 @@ case "\$1" in
fi
;;
view)
if printf '%s\n' "\$*" | grep -q -- '--json labels'; then
: # not relevant on the merge path
else
cat <<'JSON'
cat <<'JSON'
{"number":12,"title":"Ship feature","isDraft":false,"baseRefName":"main","headRefName":"feat/issue-77-thing","mergeable":"MERGEABLE","reviews":[{"author":{"login":"acme"},"state":"APPROVED","submittedAt":"2026-01-02T00:00:00Z"}],"statusCheckRollup":[],"commits":[{"committedDate":"2026-01-01T00:00:00Z"}]}
JSON
fi
;;
merge) exit 0 ;;
edit) : ;;
comment) : ;;
*) : ;;
esac
;;
label) : ;;
issue) : ;;
api) : ;; # every REST call here is a needs_human_clear DELETE no-op
*) echo "unhandled: \$*" >&2; exit 1 ;;
esac
EOF
Expand All @@ -215,11 +221,11 @@ outC="$(env -u GATES_FILE bash "$dirC/.claude/scripts/merge-ready.sh" "acme/repo
check "C: PR merged" bash -c 'printf "%s\n" "$1" | grep -q "\"action\":\"merged\""' _ "$outC"
check "C: gh pr merge invoked with --merge --delete-branch" grep -q "pr merge 12 -R acme/repo --merge --delete-branch" "$gh_logC"
check "C: no comment posted on the merge path (verdict never hits the flag case)" bash -c '! grep -q "^pr comment" "$1"' _ "$gh_logC"
check "C: pr:12 pr-review/changes-requested cleared -- 'pr edit 12 --remove-label needs-human' appears 3x (case-default once, success block twice)" \
bash -c '[ "$(grep -c "pr edit 12 --remove-label needs-human" "$1")" -eq 3 ]' _ "$gh_logC"
check "C: head_branch->issue-number regex clears issue 77's attempt-budget AND stall (2x 'issue edit 77 --remove-label needs-human')" \
bash -c '[ "$(grep -c "issue edit 77 --remove-label needs-human" "$1")" -eq 2 ]' _ "$gh_logC"
check "C: no needs-human label/comment ever added on the merge path" bash -c '! grep -q "add-label" "$1"' _ "$gh_logC"
check "C: pr:12 pr-review/changes-requested cleared via REST DELETE -- appears 3x (case-default once, success block twice)" \
bash -c '[ "$(grep -c -- "-X DELETE repos/acme/repo/issues/12/labels/needs-human" "$1")" -eq 3 ]' _ "$gh_logC"
check "C: head_branch->issue-number regex clears issue 77's attempt-budget AND stall (2x REST DELETE .../issues/77/labels/needs-human)" \
bash -c '[ "$(grep -c -- "-X DELETE repos/acme/repo/issues/77/labels/needs-human" "$1")" -eq 2 ]' _ "$gh_logC"
check "C: no needs-human label ever ADDED (REST POST) on the merge path" bash -c '! grep -q -- "-X POST" "$1"' _ "$gh_logC"

echo ""
if [ "$fail" -eq 0 ]; then
Expand Down
Loading
Loading