fix(needs-human): REST label ops + confirmed-episode guard — stop per-tick comment spam (issue #169)#171
Conversation
…-tick comment spam (issue #169) needs-human.sh's once-per-episode comment guard derived "fresh episode" from the needs-human label's presence on the target, but every label write silently no-op'd on this environment: `gh label create` (gh 2.4.0 has no `gh label` subcommand) and `gh pr|issue edit --add-label`/`--remove-label` (GraphQL scope error with the bot token), each swallowed by `|| true`. The label never actually stuck, so every tick read "no label" as a fresh episode and re-posted the comment (99 times on PR #168, ~128 on PR #161). Replace every label read/write with `gh api` REST calls (proven to work with this token/gh version): POST .../labels to idempotently ensure the label exists, POST .../issues/N/labels to add it, DELETE .../issues/N/labels/needs-human to remove it, and a plain GET .../issues/N to read the current set — PRs are issues in the REST API, so this also unifies what used to be separate pr/issue code paths. Critical invariant: the episode comment now posts ONLY once the label add is CONFIRMED by re-reading the target's labels after the add, never from the add's exit code alone (old gh can exit non-zero on a call that actually succeeded). An unconfirmed add fails loudly via log-event.sh instead of posting a comment for an unverifiable episode — that silent-failure-plus- post-anyway combination is exactly the mechanism that produced the spam. Updates the gh stubs in needs-human.test.sh (rewritten with 10 scenarios covering the new REST shapes, the confirmed/unconfirmed comment gate, and 404-as-success on remove) and every caller test that exercises the seam: pr-feedback.test.sh, pr-comment-fix.test.sh, merge-ready.test.sh, loop-ceilings.test.sh, loop-tick.test.sh, pr-rebase.test.sh. Existing gh-call-count assertions are preserved (shapes adjusted, not the counting discipline). The #168/#161 spam comments are left in place, per scope. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
fix(needs-human): REST label ops + confirmed-episode guard — stop per-tick comment spam (issue #169) (not yet reviewed) |
9 similar comments
|
fix(needs-human): REST label ops + confirmed-episode guard — stop per-tick comment spam (issue #169) (not yet reviewed) |
|
fix(needs-human): REST label ops + confirmed-episode guard — stop per-tick comment spam (issue #169) (not yet reviewed) |
|
fix(needs-human): REST label ops + confirmed-episode guard — stop per-tick comment spam (issue #169) (not yet reviewed) |
|
fix(needs-human): REST label ops + confirmed-episode guard — stop per-tick comment spam (issue #169) (not yet reviewed) |
|
fix(needs-human): REST label ops + confirmed-episode guard — stop per-tick comment spam (issue #169) (not yet reviewed) |
|
fix(needs-human): REST label ops + confirmed-episode guard — stop per-tick comment spam (issue #169) (not yet reviewed) |
|
fix(needs-human): REST label ops + confirmed-episode guard — stop per-tick comment spam (issue #169) (not yet reviewed) |
|
fix(needs-human): REST label ops + confirmed-episode guard — stop per-tick comment spam (issue #169) (not yet reviewed) |
|
fix(needs-human): REST label ops + confirmed-episode guard — stop per-tick comment spam (issue #169) (not yet reviewed) |
Root cause
needs-human.sh's once-per-episode comment guard derives "fresh episode" from theneeds-humanlabel's presence on the target. On this environment, every label write silently no-ops:gh label create(needs-human.sh:99) — gh 2.4.0 has nogh labelsubcommand at all.gh pr edit N --add-label/gh issue edit N --add-labeland the--remove-labelpair — a GraphQL scope error with the bot token on gh 2.4.0.Each one is wrapped in
|| true, so the label never actually stuck. Every tick therefore read "no label" as a fresh episode and re-posted the escalation comment — PR #168 accumulated 99 identical comments overnight, PR #161 accumulated ~128.Fix
Every label read/write in
needs-human.shnow goes throughgh api(REST), the one call shape proven to work with this token/gh version:POST repos/{owner}/{repo}/labels(name=needs-human, color=b60205) — a 422 "already exists" is the expected steady state, swallowed like every other best-effort step in this file.GET repos/{owner}/{repo}/issues/{n}(-q .labels[].name) — PRs are issues in the REST API, so this single call unifies what used to be separategh pr view/gh issue viewbranches.POST repos/{owner}/{repo}/issues/{n}/labelswith{"labels":["needs-human"]}via--input -.DELETE repos/{owner}/{repo}/issues/{n}/labels/needs-human— a 404 (never labeled / already cleared) is treated identically to success.Old-gh quirk handled per the issue:
gh api -X DELETE/some POSTs can exit non-zero while actually succeeding (an empty-body parse quirk), so exit codes are never trusted alone — every consequential check re-reads the target's labels to verify by effect.The confirmed-before-comment invariant
The episode comment now posts only once the label add is CONFIRMED by re-reading the target's labels immediately after the add — never from the add's exit code alone. If the add can't be confirmed, the comment is suppressed (an unverifiable episode must never comment — that silent-failure-plus-post-anyway combination is exactly the mechanism that produced the #168/#161 spam), and the failure is logged loudly to
events.jsonlvialog-event.sh(role=orchestrator,phase=error) so it's visible in the cockpit instead of silently swallowed.This deliberately inverts the previous "fail open toward the comment" behavior for a failed label read — that old fail-open path is precisely how an unconfirmable episode used to spam a fresh comment every tick.
Tests
Rewrote
needs-human.test.sh(10 scenarios) covering the new REST call shapes, PR vs issue target unification, the confirmed/unconfirmed comment-gating invariant (including an events.jsonl assertion), and 404-as-success on remove. Updated the gh stubs (to REST shapes, using marker files to simulate GitHub's own label-persistence semantics) and adjusted assertions — without loosening the counting discipline — in every caller test that exercises this seam:pr-feedback.test.sh,pr-comment-fix.test.sh,merge-ready.test.sh,loop-ceilings.test.sh,loop-tick.test.sh,pr-rebase.test.sh.Caller function signatures (
needs_human_flag/needs_human_clear) are unchanged —merge-ready.sh,pr-feedback.sh,pr-ci-fix.sh,pr-comment-fix.sh,loop-tick.shneeded no code changes.Note
The existing spam on PRs #168/#161 is deliberately left in place — this PR only stops new spam from accumulating; comment cleanup on those PRs is a separate cleanup task per the issue.
Closes #169
🤖 Generated with Claude Code