From a40e42544eed1656df39e52c4984db6fcc18c574 Mon Sep 17 00:00:00 2001 From: claude-bot-andresmgsl Date: Thu, 23 Jul 2026 11:52:57 +0000 Subject: [PATCH] feat(labels): wire the ruling pass into both reconcilers + lib test suite Issue side: the claim-reclaim clock stops under a pending ruling (the decision still sees an unassigned claim), an already-applied stale heals off, and reconcile_ruling runs for any flagged issue on any queue state. PR side: reconcile_ruling rides the (#51) stale section's real-activity computation. test/ruling.test.sh pins the window boundaries, newest-event anchoring, per-event marker scoping, the markerless nudge reset, the unreadable-timeline rule, and that no scenario writes a label. Co-Authored-By: Claude Fable 5 --- .../issueflow-reconcile.sh | 32 ++- actions/labels-reconcile/labels-reconcile.sh | 18 +- test/ruling.test.sh | 196 ++++++++++++++++++ 3 files changed, 243 insertions(+), 3 deletions(-) create mode 100644 test/ruling.test.sh diff --git a/actions/issueflow-reconcile/issueflow-reconcile.sh b/actions/issueflow-reconcile/issueflow-reconcile.sh index 6956a55..d51d898 100644 --- a/actions/issueflow-reconcile/issueflow-reconcile.sh +++ b/actions/issueflow-reconcile/issueflow-reconcile.sh @@ -24,6 +24,10 @@ STALE_AFTER=$((ISSUEFLOW_STALE_HOURS * 3600)) QUEUE_LABELS=(ready claimed blocked) TRIAGE_ACTORS=() +# The needs-ruling invariants (#52) — one implementation for both surfaces. +# shellcheck source=lib/ruling.sh +. "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/../../lib/ruling.sh" + log() { printf 'issueflow: %s\n' "$*"; } run() { if [ -n "${DRY_RUN:-}" ]; then log "DRY_RUN: $*"; else "$@"; fi; } @@ -225,7 +229,16 @@ reconcile_issue() { assignees="$(jq '.assignees | length' <<<"$ISSUE_JSON")" grep -qxF "$n" <<<"${OPEN_PR_ISSUES:-}" && open_pr=true age="$(last_issue_activity "$n" "$(jq -r '.created_at' <<<"$ISSUE_JSON")")" - decision="$(claim_decision_at "$assignees" "$open_pr" "$age")" + if [ "$(ruling_stale_exempt <<<"$ISSUE_LABELS")" = EXEMPT ]; then + # Waiting on a human is legitimately quiet (#50 D10): the reclaim + # clock does not run under a pending ruling — the same treatment + # `blocked` gets by never reaching this branch at all. Only the clock + # stops: an unassigned claim is still a repair the decision must see, + # so it runs on a zero age rather than being skipped. + decision="$(claim_decision "$assignees" "$open_pr" 0)" + else + decision="$(claim_decision_at "$assignees" "$open_pr" "$age")" + fi case "$decision" in FLAG_UNASSIGNED) ensure_comment "$n" claimed-unassigned \ @@ -272,6 +285,23 @@ reconcile_issue() { log "#$n: completed epic nudged" fi fi + + # ---- the ruling invariants (#52), on any queue state ---- + # The flag composes with the queue labels (#50 D8), so this runs after the + # queue branches rather than inside one of them. The FLAG_CONFLICT return + # above still short-circuits it on purpose: a board lying about its queue + # state is repaired by triage before anything else is derived from it. + if has_issue_label needs-ruling; then + # An already-applied stale comes off: waiting on a human is legitimately + # quiet (#50 D10), and nothing on the issue side ever puts stale back. + if has_issue_label stale; then + run gh issue edit "$n" -R "$REPO" --remove-label stale >/dev/null + log "#$n: unstale (a ruling is pending)" + fi + [ -n "${age:-}" ] \ + || age="$(last_issue_activity "$n" "$(jq -r '.created_at' <<<"$ISSUE_JSON")")" + reconcile_ruling "$n" "$age" "$NOW" + fi } reconcile_opened_issue() { diff --git a/actions/labels-reconcile/labels-reconcile.sh b/actions/labels-reconcile/labels-reconcile.sh index 53f6d01..67e8a0e 100755 --- a/actions/labels-reconcile/labels-reconcile.sh +++ b/actions/labels-reconcile/labels-reconcile.sh @@ -50,6 +50,10 @@ LABELS="" RETIRED=(state:needs-rebase) STALE_AFTER=$((48 * 3600)) +# The needs-ruling invariants (#52) — one implementation for both surfaces. +# shellcheck source=lib/ruling.sh +. "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/../../lib/ruling.sh" + log() { printf 'labels: %s\n' "$*"; } run() { # every mutation goes through here — DRY_RUN=1 logs instead of doing @@ -412,7 +416,7 @@ $(configured_label_rows "$LABELS_CONF")" has_label() { grep -qxF "$1" <<<"$LABELS"; } reconcile_pr() { # $1 = PR number; relies on the globals set from its fetch - local n="$1" desired remove s args last_activity age + local n="$1" desired remove s args last_activity last_activity_epoch age desired="$(decide_state)" @@ -517,7 +521,8 @@ reconcile_pr() { # $1 = PR number; relies on the globals set from its fetch gh api --paginate "repos/$REPO/pulls/$n/commits" --jq '.[].commit.committer.date' } | sort | tail -n1 )" - age=$((NOW - $(date -d "$last_activity" +%s))) + last_activity_epoch="$(date -d "$last_activity" +%s)" + age=$((NOW - last_activity_epoch)) # needs-ruling joins blocked here: waiting on a human is legitimately quiet # (#50 D10). The 7-day nudge is #52's, once for both surfaces. if has_label blocked || has_label needs-ruling || [ "$age" -le "$STALE_AFTER" ]; then @@ -529,6 +534,15 @@ reconcile_pr() { # $1 = PR number; relies on the globals set from its fetch run gh issue edit "$n" -R "$REPO" --add-label stale >/dev/null log "#$n: stale ($((age / 3600))h quiet)" fi + + # ---- the ruling invariants (#52): the bare-flag check + the 7-day nudge -- + # The stale EXEMPTION above is #51's; these are the sweep halves that ride + # the same real-activity computation (lib/ruling.sh, shared with the issue + # side). Behind the flag check so flag-free PRs — all of them, almost + # always — cost no extra API reads. + if has_label needs-ruling; then + reconcile_ruling "$n" "$last_activity_epoch" "$NOW" + fi } main() { diff --git a/test/ruling.test.sh b/test/ruling.test.sh new file mode 100644 index 0000000..fa3a3ed --- /dev/null +++ b/test/ruling.test.sh @@ -0,0 +1,196 @@ +#!/usr/bin/env bash +set -u + +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +# shellcheck source=test/harness.sh +source "$ROOT/test/harness.sh" +# shellcheck source=lib/ruling.sh +source "$ROOT/lib/ruling.sh" + +TMP="$(mktemp -d)" +trap 'rm -rf "$TMP"' EXIT + +# --------------------------------------------------------------------------- +# Pure decisions. Epochs are arbitrary fixed numbers — no wall clock anywhere. +# --------------------------------------------------------------------------- + +check "needs-ruling is stale-exempt" 0 "EXEMPT" ruling_stale_exempt <<< $'claimed\nneeds-ruling' +check "a flag-free issue sweeps normally" 0 "SWEEP" ruling_stale_exempt <<< $'claimed' + +L=100000 # the labeled event, in every window case below + +check "escalation 14 minutes before the flag accompanies it" 0 "ACCOMPANIED" \ + ruling_bare_decision setter "$L" <<<"setter $((L - 840))" +check "escalation exactly 15 minutes before still accompanies (no earlier than)" 0 "ACCOMPANIED" \ + ruling_bare_decision setter "$L" <<<"setter $((L - 900))" +check "escalation after the flag accompanies it" 0 "ACCOMPANIED" \ + ruling_bare_decision setter "$L" <<<"setter $((L + 60))" +check "escalation 16 minutes before is bare" 0 "BARE" \ + ruling_bare_decision setter "$L" <<<"setter $((L - 960))" +check "somebody else's comment does not satisfy the contract" 0 "BARE" \ + ruling_bare_decision setter "$L" <<<"bystander $((L - 60))" +check "no comment at all is bare" 0 "BARE" \ + ruling_bare_decision setter "$L" >"$TMP/posted-$n" + file="$TMP/repos_${REPO%%/*}_${REPO#*/}_issues_${n}_comments.json" + [ -f "$file" ] || printf '[]\n' >"$file" + jq --arg b "$body" --arg at "$(iso "$NOW")" \ + '. + [{"user":{"login":"sweep-bot"},"created_at":$at,"html_url":"https://x/posted","body":$b}]' \ + "$file" >"$file.tmp" && mv "$file.tmp" "$file" + elif [ "$1" = issue ] && [ "$2" = edit ]; then + printf '%s\n' "$*" >>"$TMP/edits" + fi +} + +posts() { [ -f "$TMP/posted-$1" ] && grep -c '^----$' "$TMP/posted-$1" || echo 0; } +timeline_file() { printf '%s/repos_owner_repo_issues_%s_timeline.json' "$TMP" "$1"; } +comments_file() { printf '%s/repos_owner_repo_issues_%s_comments.json' "$TMP" "$1"; } + +# -- bare flag, swept twice: exactly one comment ----------------------------- +T=$((NOW - 3600)) +jq -n --arg at "$(iso "$T")" \ + '[{"event":"labeled","label":{"name":"needs-ruling"},"actor":{"login":"setter"},"created_at":$at}]' \ + >"$(timeline_file 9)" +printf '[]\n' >"$(comments_file 9)" +check "a bare flag is commented on" 0 "ruling flag is bare" reconcile_ruling 9 "$T" "$NOW" +check "...and the sweep never posted twice" 0 "" reconcile_ruling 9 "$T" "$NOW" +check "one bare comment across two sweeps" 0 "1" posts 9 +check "the bare comment carries its marker" 0 "" \ + grep -qF '' "$TMP/posted-9" +check "the bare comment names the missing contract" 0 "" \ + grep -q 'question' "$TMP/posted-9" + +# -- accompanied flag: silence ---------------------------------------------- +jq -n --arg at "$(iso "$T")" \ + '[{"event":"labeled","label":{"name":"needs-ruling"},"actor":{"login":"setter"},"created_at":$at}]' \ + >"$(timeline_file 10)" +jq -n --arg at "$(iso "$((T - 840))")" \ + '[{"user":{"login":"setter"},"created_at":$at,"html_url":"https://x/esc","body":"question, options, recommendation"}]' \ + >"$(comments_file 10)" +reconcile_ruling 10 "$T" "$NOW" >/dev/null +check "an accompanied flag posts nothing" 0 "0" posts 10 + +# -- re-flag: judged on its own escalation, marker scoped per event ---------- +T1=$((NOW - 86400)) T2=$((NOW - 7200)) +jq -n --arg t1 "$(iso "$T1")" --arg t2 "$(iso "$T2")" \ + '[{"event":"labeled","label":{"name":"needs-ruling"},"actor":{"login":"setter"},"created_at":$t1}, + {"event":"unlabeled","label":{"name":"needs-ruling"},"actor":{"login":"setter"},"created_at":$t2}, + {"event":"labeled","label":{"name":"needs-ruling"},"actor":{"login":"setter"},"created_at":$t2}]' \ + >"$(timeline_file 11)" +jq -n --arg esc "$(iso "$((T1 - 60))")" --arg marked "$(iso "$((T1 + 300))")" \ + '[{"user":{"login":"setter"},"created_at":$esc,"html_url":"https://x/esc1","body":"the first escalation"}, + {"user":{"login":"sweep-bot"},"created_at":$marked,"html_url":"https://x/bare1","body":"\nolder episode"}]' \ + >"$(comments_file 11)" +reconcile_ruling 11 "$T2" "$NOW" >/dev/null +check "a re-flag is re-checked against its own escalation" 0 "1" posts 11 + +# -- nudge: fires past 7 quiet days, links the escalation, resets itself ----- +T0=$((NOW - 8 * 86400)) +jq -n --arg at "$(iso "$T0")" \ + '[{"event":"labeled","label":{"name":"needs-ruling"},"actor":{"login":"setter"},"created_at":$at}]' \ + >"$(timeline_file 12)" +jq -n --arg at "$(iso "$((T0 - 60))")" \ + '[{"user":{"login":"setter"},"created_at":$at,"html_url":"https://x/esc12","body":"question, options, recommendation"}]' \ + >"$(comments_file 12)" +check "8 quiet days nudge" 0 "ruling nudge" reconcile_ruling 12 "$T0" "$NOW" +check "one nudge posted" 0 "1" posts 12 +check "the nudge links the escalation comment" 0 "" grep -qF 'https://x/esc12' "$TMP/posted-12" +check "the nudge addresses the decider" 0 "" grep -qF '@danmt' "$TMP/posted-12" +check "the nudge does not tag the flag-setter" 1 "" grep -qF '@setter' "$TMP/posted-12" +check "the nudge carries no marker — the comment itself resets the window" 1 "" \ + grep -qF "$RULING_BARE_MARKER" "$TMP/posted-12" +# The reset, through the surfaces' own activity computation: the nudge the +# stub appended is the newest comment, so the recomputed last-activity is NOW. +newest_at="$(jq -r 'map(.created_at) | max' "$(comments_file 12)")" +check "the posted nudge is now the newest activity" 0 "" \ + test "$(date -d "$newest_at" +%s)" = "$NOW" +reconcile_ruling 12 "$(date -d "$newest_at" +%s)" "$NOW" >/dev/null +check "a sweep right after the nudge holds its silence" 0 "1" posts 12 + +# -- 6 quiet days: silence --------------------------------------------------- +jq -n --arg at "$(iso "$((NOW - 6 * 86400))")" \ + '[{"event":"labeled","label":{"name":"needs-ruling"},"actor":{"login":"setter"},"created_at":$at}]' \ + >"$(timeline_file 13)" +jq -n --arg at "$(iso "$((NOW - 6 * 86400))")" \ + '[{"user":{"login":"setter"},"created_at":$at,"html_url":"https://x/esc13","body":"question, options, recommendation"}]' \ + >"$(comments_file 13)" +reconcile_ruling 13 $((NOW - 6 * 86400)) "$NOW" >/dev/null +check "6 quiet days do not nudge" 0 "0" posts 13 + +# -- unreadable timeline: nothing happens ------------------------------------ +check "an unreadable timeline invents no verdict" 0 "timeline unreadable" \ + reconcile_ruling 14 "$T" "$NOW" +check "...and posts nothing" 0 "0" posts 14 + +# -- across every scenario above: not one label write ------------------------ +check "the ruling sweep never wrote a label" 1 "" test -f "$TMP/edits" + +# Grep-level pin for #50 D9: no mutation call in the sweep code names the +# flag. The only writes reconcile_ruling makes are comments. +check "no add/remove-label mutation names the ruling flag" 1 "" \ + grep -rEn -- '(add|remove)-label[^"]*needs-ruling' "$ROOT/actions" "$ROOT/lib" + +summary