diff --git a/actions/issueflow-reconcile/issueflow-reconcile.sh b/actions/issueflow-reconcile/issueflow-reconcile.sh index accda04..329a3b6 100644 --- a/actions/issueflow-reconcile/issueflow-reconcile.sh +++ b/actions/issueflow-reconcile/issueflow-reconcile.sh @@ -331,26 +331,48 @@ blocked_parse_set() { # $1 local refs, $2 cross refs -> "{#7, #12}" | "{}" } blocked_parse_marker() { # $1 rendered set -> the echo's idempotency marker - # Scoped to the SET's value, not to the issue and not to the sweep: an - # unchanged parse finds its own marker and stays quiet on a 15-minute cron, - # and a changed one cannot find it, so the change is what speaks. One - # consequence is deliberate: a declaration edited back to a set already - # echoed stays quiet too, because the thread already carries that echo. + # Scoped to the SET's value, not to the issue and not to the sweep: the + # marker names WHAT was echoed, and blocked_parse_echo_needed decides whether + # it is still what the thread is saying. # # The identity is the DIGEST, not the slug beside it. Slugging is many-to-one # — `{acme/widgets#9}` and `{acme-widgets#9}` are both parses this reconciler # accepts, and both slug to `acme-widgets-9` — so a slug-keyed marker lets a # changed set find the old marker and say nothing, silence in precisely the # case the echo exists to speak about. Distinguishing `/` would close that - # pair and leave the class: `-`, `_` and `.` are all legal in a qualifier and - # all collapse the same way. The slug stays in front so a human reading the - # raw comment can still see which set it belongs to; it decides nothing. + # pair and leave the class: `-`, `_` and `.` are legal in a qualifier token + # and all collapse the same way. The slug stays in front so a human reading + # the raw comment can still see which set it belongs to; it decides nothing. local slug digest slug="$(printf '%s' "$1" | tr -c '[:alnum:]' '-' | sed 's/--*/-/g; s/^-//; s/-$//')" digest="$(printf '%s' "$1" | sha256sum | cut -c1-12)" printf 'blockers-parsed-%s-%s\n' "${slug:-none}" "$digest" } +blocked_parse_echo_needed() { # $1 issue, $2 this parse's marker → 0 echo, 1 quiet + # Idempotency for the parse echo is against the LAST parse echo on the + # thread, not against any historical one. ensure_comment's any-occurrence + # grep is right for a flag like `blocked-unparseable`, whose question is + # "have I ever said this"; it is wrong for a value that changes, whose + # question is "is this still what I am saying". The difference is A -> B -> A: + # under an any-occurrence search the return to A finds A's own first echo and + # stays silent, leaving the thread's most recent echo asserting B while the + # sweep gates on A. A stale parse presented as the current one is the exact + # failure #252 exists to kill, and the third edit changed the parsed set, so + # the criterion says it speaks. + # + # Comparing markers rather than re-rendering the last set keeps the digest as + # the only identity: two sets are the same here iff blocked_parse_marker says + # so, the same rule the marker itself is built on. + local bodies last + guarded_read bodies gh api --paginate "repos/$REPO/issues/$1/comments" --jq '.[].body' \ + || skip_issue "$1" "could not read its comments: $(read_failure_reason "$READ_FAILURE_STDERR")" + # The read fails closed above (#247 D1): an unreadable history skips the + # issue rather than answering "nothing echoed yet" and re-posting. + last="$(grep -o '' <<<"$bodies" | tail -n 1)" + [ "$last" != "" ] +} + blocked_decision() { # $1 local refs, $2 OPEN/CLOSED states, $3 cross-repo refs local refs="$1" states="$2" cross_refs="${3:-}" if [ -n "$cross_refs" ]; then echo FLAG_CROSS_REPO @@ -487,7 +509,7 @@ last_issue_activity() { # $1 issue, $2 created_at → epoch; non-zero if a read reconcile_issue() { local n="$1" decision refs cross_refs states age created assignees open_pr=false label owners - local merged_ref_pr="" transition_marker="" transition_handled=false parsed_set="" + local merged_ref_pr="" transition_marker="" transition_handled=false parsed_set="" parse_marker="" local unchecked="" remove_claimed=claimed decision="$(queue_decision <<<"$ISSUE_LABELS")" case "$decision" in @@ -597,8 +619,10 @@ The merge releases the claim; no builder owes a draft. Triage owes completion in # detect because the machine cannot judge what a human meant — only state # what it read, and let the human see the divergence in one sweep. parsed_set="$(blocked_parse_set "$refs" "$cross_refs")" - ensure_comment "$n" "$(blocked_parse_marker "$parsed_set")" \ - "This issue's \`Blocked by\` declarations parse to: $parsed_set + parse_marker="$(blocked_parse_marker "$parsed_set")" + if blocked_parse_echo_needed "$n" "$parse_marker"; then + run gh issue comment "$n" -R "$REPO" --body " +This issue's \`Blocked by\` declarations parse to: $parsed_set That is the exact set this sweep gates on — what the machine read, never a judgment about whether it is what you meant. The parse unions every clause it @@ -610,7 +634,8 @@ omits something you did, edit the declaration — the next sweep echoes the correction. *Comment only: nothing on this path writes a label. The marker carries the set -itself, so an unchanged parse never re-posts.*" +itself, so a parse unchanged since the last echo never re-posts.*" >/dev/null + fi log "#$n: blocked declarations parse to $parsed_set" states="$(reference_states <<<"$refs")" decision="$(blocked_decision "$refs" "$states" "$cross_refs")" diff --git a/test/issueflow-reconcile.test.sh b/test/issueflow-reconcile.test.sh index 14a38b3..748bf46 100644 --- a/test/issueflow-reconcile.test.sh +++ b/test/issueflow-reconcile.test.sh @@ -781,6 +781,35 @@ check "...and not re-flagging cross-repo, which did not change" 0 "1" \ check "no label write comes from the colliding-edit path either" 0 "" \ bash -c 'test "$1" -eq "$(wc -l <"$2")"' _ "$collision_edits_before" "$TMP/issue-edits" +# A -> B -> A. The marker names the SET, so the return to A is a marker this +# thread has carried before; the question the echo has to answer is not "have I +# ever said this" but "is this still what I am saying". Searching the whole +# history answers the first, and the return went silent while the thread's +# newest echo asserted B and the sweep gated on A — a stale parse presented as +# the current one, which is the readable-but-wrong shape #252 exists to kill. +# All four sweeps are driven, because the bug is only visible as a sequence. +printf '[]\n' >"$(cfix 52)" +issue_probe 52 blocked 1 false "" 'Blocked by #90, #91.' >/dev/null +issue_probe 52 blocked 1 false "" 'Blocked by #90, #91, #92.' >/dev/null +return_edits_before="$(wc -l <"$TMP/issue-edits")" +issue_probe 52 blocked 1 false "" 'Blocked by #90, #91.' >/dev/null +check "a set edited back to a previously echoed one speaks again" 0 "3" \ + grep -cF '" "$TMP/posted-52" +# The assertion the silence used to fail: it is the NEWEST echo that has to +# name what the sweep gates on, not merely some echo somewhere in the thread. +# shellcheck disable=SC2016 # positional parameters belong to bash -c +check "...leaving the newest echo naming the set the sweep now gates on" 0 \ + "parse to: {#90, #91}" \ + bash -c 'grep -o "parse to: {[^}]*}" "$1" | tail -n 1' _ "$TMP/posted-52" +issue_probe 52 blocked 1 false "" 'Blocked by #90, #91.' >/dev/null +check "an unchanged sweep after the return still draws nothing" 0 "3" \ + grep -cF '