From 374005ef7740f8f60afe7e047c806cb2990c47a5 Mon Sep 17 00:00:00 2001 From: cndgrr <59120057+cndgrr@users.noreply.github.com> Date: Mon, 3 Aug 2026 19:24:59 +0000 Subject: [PATCH 1/9] feat(issueflow): echo the parsed blocker set when it changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The clause parse is exact and unforgiving, and its output was invisible: every incident in this class was found by a human running the parser by hand, hours or days late. The sweep now states what it read — one marker comment per distinct parsed set, comment-only, no label writes. Refs #252 --- .../issueflow-reconcile.sh | 55 ++++++++++++++++++- 1 file changed, 54 insertions(+), 1 deletion(-) diff --git a/actions/issueflow-reconcile/issueflow-reconcile.sh b/actions/issueflow-reconcile/issueflow-reconcile.sh index 4e29f84..ccc63bd 100644 --- a/actions/issueflow-reconcile/issueflow-reconcile.sh +++ b/actions/issueflow-reconcile/issueflow-reconcile.sh @@ -314,6 +314,33 @@ blocked_cross_references() { # body on stdin -> qualified refs, one per line blocked_reference_records | awk -F '\t' '$1 == "CROSS" { print $2 }' | sort -u } +blocked_parse_set() { # $1 local refs, $2 cross refs -> "{#7, #12}" | "{}" + # The parse, rendered once. The comment, the marker and the log line all + # read this one string, so the three can never disagree about what the + # machine read. Both classes are shown because both are parsed: the locals + # in the numeric order blocked_references answers, then the qualified + # references blocked_cross_references answers — a cross-repo clause is as + # capable of being readable-but-wrong as a local one. + local rendered + rendered="$( + { [ -z "$1" ] || sed 's/^/#/' <<<"$1" + [ -z "${2:-}" ] || printf '%s\n' "$2" + } | awk '{ printf "%s%s", (NR > 1 ? ", " : ""), $0 } END { printf "\n" }' + )" + printf '{%s}\n' "$rendered" +} + +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. + local slug + slug="$(printf '%s' "$1" | tr -c '[:alnum:]' '-' | sed 's/--*/-/g; s/^-//; s/-$//')" + printf 'blockers-parsed-%s\n' "${slug:-none}" +} + 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 @@ -450,7 +477,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 + local merged_ref_pr="" transition_marker="" transition_handled=false parsed_set="" local unchecked="" remove_claimed=claimed decision="$(queue_decision <<<"$ISSUE_LABELS")" case "$decision" in @@ -549,6 +576,32 @@ The merge releases the claim; no builder owes a draft. Triage owes completion in elif has_issue_label blocked; then refs="$(blocked_references <<<"$(jq -r '.body // ""' <<<"$ISSUE_JSON")")" cross_refs="$(blocked_cross_references <<<"$(jq -r '.body // ""' <<<"$ISSUE_JSON")")" + # The parse is echoed before any verdict is derived from it (#252). The + # clause parse is exact and unforgiving, and its output was invisible: + # crew#308 silently parsed a negated "no longer blocked by #221" as a + # blocker, crew#71 spent five days as an unresolvable queue conflict, and + # crew#284's declaration had to be re-derived by hand-running the parser. + # Every one of those was found by a human running the parser, hours or + # days late. `blocked-unparseable` already catches the UNREADABLE + # declaration; this catches the readable-but-wrong one, which no flag can + # 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 + +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 +finds, so a sentence like \"no longer blocked by #9\" contributes #9 like any +other; over-retaining is the deliberate direction of error, because a stale +\`blocked\` is a triage comment away and a false \`ready\` sends a builder into +work that cannot merge. If this set names something you did not declare, or +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.*" + log "#$n: blocked declarations parse to $parsed_set" states="$(reference_states <<<"$refs")" decision="$(blocked_decision "$refs" "$states" "$cross_refs")" case "$decision" in From 5cfb69e10471c697b89b57b46600ae9d191779bc Mon Sep 17 00:00:00 2001 From: cndgrr <59120057+cndgrr@users.noreply.github.com> Date: Mon, 3 Aug 2026 19:28:31 +0000 Subject: [PATCH 2/9] test(issueflow): the parse echo, mutation-proven in both directions The idempotency contract is the marker's scope, so both directions are pinned: an unchanged set must reuse its marker (or a 15-minute cron repeats itself forever) and a changed one must not (or a misparse hides under a marker the thread already carries). crew#308's negated clause is replayed through the sweep, and the empty parse is echoed beside the untouched `blocked-unparseable` flag. Refs #252 --- .../issueflow-reconcile.sh | 2 +- changelog.d/252.md | 9 ++ test/issueflow-reconcile.test.sh | 88 +++++++++++++++++++ 3 files changed, 98 insertions(+), 1 deletion(-) create mode 100644 changelog.d/252.md diff --git a/actions/issueflow-reconcile/issueflow-reconcile.sh b/actions/issueflow-reconcile/issueflow-reconcile.sh index ccc63bd..4cd916d 100644 --- a/actions/issueflow-reconcile/issueflow-reconcile.sh +++ b/actions/issueflow-reconcile/issueflow-reconcile.sh @@ -323,7 +323,7 @@ blocked_parse_set() { # $1 local refs, $2 cross refs -> "{#7, #12}" | "{}" # capable of being readable-but-wrong as a local one. local rendered rendered="$( - { [ -z "$1" ] || sed 's/^/#/' <<<"$1" + { [ -z "$1" ] || awk '{ print "#" $0 }' <<<"$1" [ -z "${2:-}" ] || printf '%s\n' "$2" } | awk '{ printf "%s%s", (NR > 1 ? ", " : ""), $0 } END { printf "\n" }' )" diff --git a/changelog.d/252.md b/changelog.d/252.md new file mode 100644 index 0000000..3b47994 --- /dev/null +++ b/changelog.d/252.md @@ -0,0 +1,9 @@ +### Added + +- The issue sweep now echoes an issue's parsed `Blocked by` set as a comment + whenever that set changes, so a readable-but-wrong declaration is visible in + one sweep instead of days later, when a human happens to run the parser by + hand (#252). +- The echo's marker carries the parsed set itself: an unchanged parse never + re-posts on a 15-minute cron, and a changed one always speaks. Comment-only + — no path here writes a label (#252). diff --git a/test/issueflow-reconcile.test.sh b/test/issueflow-reconcile.test.sh index e36adda..3be7cc2 100644 --- a/test/issueflow-reconcile.test.sh +++ b/test/issueflow-reconcile.test.sh @@ -222,6 +222,39 @@ check "cross-repo-only blocker is flagged distinctly" 0 "FLAG_CROSS_REPO" \ blocked_decision "" "" "rig#112" check "cross-repo blocker prevents false promotion when locals close" 0 "FLAG_CROSS_REPO" \ blocked_decision "9" "CLOSED" "rig#9" + +# The parse echo (#252): the machine states what it read, so a +# readable-but-wrong declaration is visible in one sweep instead of five days. +check "the rendered set names the locals in parse order" 0 "{#7, #12}" \ + blocked_parse_set "$(printf '7\n12\n')" "" +check "a single blocker still renders as a set" 0 "{#12}" blocked_parse_set "12" "" +check "an empty parse renders as the empty set" 0 "{}" blocked_parse_set "" "" +check "cross-repo references are echoed beside the locals" 0 "{#12, rig#9}" \ + blocked_parse_set "12" "rig#9" +check "a cross-repo-only parse is echoed too" 0 "{heavy-duty/box#9}" \ + blocked_parse_set "" "heavy-duty/box#9" +# crew#308: a *negated* marker phrase unions as the thing it denies, and the +# silent result was a set nobody saw until a human ran the parser. Echoed, the +# union is visible in the thread that contains the declaration. +echo_308="$(blocked_parse_set \ + "$(blocked_references <<<'Blocked by #162, #265. It is no longer blocked by #221.')" \ + "$(blocked_cross_references <<<'Blocked by #162, #265. It is no longer blocked by #221.')")" +check "the #308 shape echoes the negation-unioned blocker verbatim" 0 "" test \ + "$echo_308" = "{#162, #221, #265}" +# The marker is scoped to the SET's value — the whole idempotency contract. +# Mutation proof, both directions: same set must reuse its marker (or a +# 15-minute cron repeats itself forever), different set must not (or a +# misparse is echoed under a marker the thread already carries, and stays +# invisible — exactly the failure this change exists to close). +check "an unchanged set reuses its marker" 0 "" test \ + "$(blocked_parse_marker '{#7, #12}')" = "$(blocked_parse_marker '{#7, #12}')" +check "a changed set takes a different marker" 1 "" test \ + "$(blocked_parse_marker '{#7, #12}')" = "$(blocked_parse_marker '{#7, #12, #19}')" +check "the empty set has a marker of its own" 0 "blockers-parsed-none" \ + blocked_parse_marker "{}" +check "the marker survives a cross-repo reference's punctuation" 0 \ + "blockers-parsed-12-heavy-duty-box-9" \ + blocked_parse_marker "{#12, heavy-duty/box#9}" # shellcheck disable=SC2016 # expansions belong to the generated fake gh printf '%s\n' \ '#!/usr/bin/env bash' \ @@ -640,6 +673,61 @@ check "no reconciler mutation names offsite (#68 D4)" 1 "" \ "$ROOT/actions/issueflow-reconcile/issueflow-reconcile.sh" \ "$ROOT/actions/labels-reconcile/labels-reconcile.sh" +# -- the parse echo: one comment per changed set, none per sweep (#252) ------ +# The whole point is a sweep-visible statement of what was read, so it is +# probed through the sweep and not only as a rendering: the marker has to +# survive the comment body, the second pass has to find it, and the third has +# to miss it because the declaration changed. +printf '{"state":"open"}\n' >"$TMP/repos_owner_repo_issues_90.json" +printf '{"state":"open"}\n' >"$TMP/repos_owner_repo_issues_91.json" +printf '{"state":"open"}\n' >"$TMP/repos_owner_repo_issues_92.json" +printf '[]\n' >"$(cfix 35)" +echo_edits_before="$(wc -l <"$TMP/issue-edits")" +first_echo="$(issue_probe 35 blocked 1 false "" "Part of #1. Blocked by #90, #91.")" +check "a first parse is echoed, naming the set" 0 "" \ + grep -qF 'parse to: {#90, #91}' "$TMP/posted-35" +check "...and the sweep log carries the same set" 0 \ + "issueflow: #35: blocked declarations parse to {#90, #91}" \ + printf '%s\n' "$first_echo" +issue_probe 35 blocked 1 false "" "Part of #1. Blocked by #90, #91." >/dev/null +check "an unchanged parse draws nothing on the next sweep" 0 "1" \ + grep -cF '' "$TMP/posted-35" +changed_echo="$(issue_probe 35 blocked 1 false "" "Part of #1. Blocked by #90, #91, #92.")" +check "a body edit that changes the set draws exactly one new echo" 0 "1" \ + grep -cF '' "$TMP/posted-35" +check "...naming the new set" 0 "" \ + grep -qF 'parse to: {#90, #91, #92}' "$TMP/posted-35" +check "...and saying so in the sweep log" 0 \ + "issueflow: #35: blocked declarations parse to {#90, #91, #92}" \ + printf '%s\n' "$changed_echo" +check "...and leaving the first echo alone" 0 "1" \ + grep -cF '' "$TMP/posted-35" +# shellcheck disable=SC2016 # positional parameters belong to bash -c +check "no label write comes from the echo path" 0 "" \ + bash -c 'test "$1" -eq "$(wc -l <"$2")"' _ "$echo_edits_before" "$TMP/issue-edits" + +# crew#308, replayed through the sweep: the declaration denies #221 and the +# parse unions it anyway. Nobody saw that set for as long as it stayed inside +# the machine; the echo puts it in the thread that contains the declaration. +printf '{"state":"open"}\n' >"$TMP/repos_owner_repo_issues_162.json" +printf '{"state":"open"}\n' >"$TMP/repos_owner_repo_issues_221.json" +printf '{"state":"open"}\n' >"$TMP/repos_owner_repo_issues_265.json" +printf '[]\n' >"$(cfix 36)" +issue_probe 36 blocked 1 false "" \ + 'Blocked by #162, #265. It is no longer blocked by #221.' >/dev/null +check "the #308 misparse is echoed verbatim, denial and all" 0 "" \ + grep -qF 'parse to: {#162, #221, #265}' "$TMP/posted-36" + +# The empty parse says so, and the flag that catches the UNREADABLE +# declaration is untouched beside it: one comment states what was read, the +# other states that nothing was. +printf '[]\n' >"$(cfix 37)" +issue_probe 37 blocked 1 false "" 'No declaration anywhere in this body.' >/dev/null +check "an empty parse is echoed as the empty set" 0 "" \ + grep -qF 'parse to: {}' "$TMP/posted-37" +check "...and blocked-unparseable still fires beside it" 0 "" \ + grep -qF '' "$TMP/posted-37" + # -- an already-applied stale heals off, and no edit names the flag ---------- jq -n --arg l "$(iso_at $((INOW - 3600)))" \ '[{"event":"labeled","label":{"name":"needs-ruling"},"actor":{"login":"setter"},"created_at":$l}]' >"$(tfix 23)" From 5910c361373212642ebb69b098bd2b15eda3c327 Mon Sep 17 00:00:00 2001 From: cndgrr <59120057+cndgrr@users.noreply.github.com> Date: Mon, 3 Aug 2026 20:05:04 +0000 Subject: [PATCH 3/9] fix(issueflow): key the parse echo to the set, not to a slug of it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The marker claimed to be scoped to the parsed set's value and was scoped to a lossy rendering of it: `tr -c '[:alnum:]' '-'` maps `acme/widgets#9` and `acme-widgets#9` — both parses this reconciler accepts — onto one marker, so a declaration edited between them found the old echo and said nothing. Silence in exactly the case the echo exists to speak about. The identity is now a digest of the exact rendered set. The readable slug stays in front of it and decides nothing. Distinguishing `/` would have closed the reported pair and left the class: `-`, `_` and `.` are all legal in a qualifier and all collapse the same way, so all four are pinned, and the sweep probe observes the second echo actually landing. Refs #252 --- .../issueflow-reconcile.sh | 14 ++++- test/issueflow-reconcile.test.sh | 53 +++++++++++++++++-- 2 files changed, 60 insertions(+), 7 deletions(-) diff --git a/actions/issueflow-reconcile/issueflow-reconcile.sh b/actions/issueflow-reconcile/issueflow-reconcile.sh index 4cd916d..accda04 100644 --- a/actions/issueflow-reconcile/issueflow-reconcile.sh +++ b/actions/issueflow-reconcile/issueflow-reconcile.sh @@ -336,9 +336,19 @@ blocked_parse_marker() { # $1 rendered set -> the echo's idempotency marker # 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. - local slug + # + # 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. + local slug digest slug="$(printf '%s' "$1" | tr -c '[:alnum:]' '-' | sed 's/--*/-/g; s/^-//; s/-$//')" - printf 'blockers-parsed-%s\n' "${slug:-none}" + digest="$(printf '%s' "$1" | sha256sum | cut -c1-12)" + printf 'blockers-parsed-%s-%s\n' "${slug:-none}" "$digest" } blocked_decision() { # $1 local refs, $2 OPEN/CLOSED states, $3 cross-repo refs diff --git a/test/issueflow-reconcile.test.sh b/test/issueflow-reconcile.test.sh index 3be7cc2..b0098cc 100644 --- a/test/issueflow-reconcile.test.sh +++ b/test/issueflow-reconcile.test.sh @@ -250,11 +250,28 @@ check "an unchanged set reuses its marker" 0 "" test \ "$(blocked_parse_marker '{#7, #12}')" = "$(blocked_parse_marker '{#7, #12}')" check "a changed set takes a different marker" 1 "" test \ "$(blocked_parse_marker '{#7, #12}')" = "$(blocked_parse_marker '{#7, #12, #19}')" -check "the empty set has a marker of its own" 0 "blockers-parsed-none" \ +check "the empty set has a marker of its own" 0 "blockers-parsed-none-44136fa355b3" \ blocked_parse_marker "{}" check "the marker survives a cross-repo reference's punctuation" 0 \ - "blockers-parsed-12-heavy-duty-box-9" \ + "blockers-parsed-12-heavy-duty-box-9-c8e36fe3b793" \ blocked_parse_marker "{#12, heavy-duty/box#9}" +# The readable half of the marker is many-to-one and must not be the identity. +# Every pair below is two declarations this reconciler genuinely accepts, that +# slug identically; a slug-keyed marker made the second one find the first +# one's marker and post nothing — silence in the one case the echo exists to +# speak about. Distinguishing `/` alone would close the first pair and leave +# the rest, so the digest of the whole rendered set is what decides. +check "the slug alone cannot separate a qualified ref from a hyphenated one" 0 "" \ + test "$(printf '%s' '{acme/widgets#9}' | tr -c '[:alnum:]' '-' | sed 's/--*/-/g; s/^-//; s/-$//')" \ + = "$(printf '%s' '{acme-widgets#9}' | tr -c '[:alnum:]' '-' | sed 's/--*/-/g; s/^-//; s/-$//')" +for collides_with_acme_widgets in '{acme-widgets#9}' '{acme_widgets#9}' '{acme.widgets#9}'; do + check "...but the marker does, against $collides_with_acme_widgets" 1 "" test \ + "$(blocked_parse_marker '{acme/widgets#9}')" \ + = "$(blocked_parse_marker "$collides_with_acme_widgets")" +done +check "the qualifier's punctuation reaches the marker's identity" 1 "" test \ + "$(blocked_parse_marker '{#12, heavy-duty/box#9}')" \ + = "$(blocked_parse_marker '{#12, heavy-duty-box#9}')" # shellcheck disable=SC2016 # expansions belong to the generated fake gh printf '%s\n' \ '#!/usr/bin/env bash' \ @@ -691,17 +708,17 @@ check "...and the sweep log carries the same set" 0 \ printf '%s\n' "$first_echo" issue_probe 35 blocked 1 false "" "Part of #1. Blocked by #90, #91." >/dev/null check "an unchanged parse draws nothing on the next sweep" 0 "1" \ - grep -cF '' "$TMP/posted-35" + grep -cF "" "$TMP/posted-35" changed_echo="$(issue_probe 35 blocked 1 false "" "Part of #1. Blocked by #90, #91, #92.")" check "a body edit that changes the set draws exactly one new echo" 0 "1" \ - grep -cF '' "$TMP/posted-35" + grep -cF "" "$TMP/posted-35" check "...naming the new set" 0 "" \ grep -qF 'parse to: {#90, #91, #92}' "$TMP/posted-35" check "...and saying so in the sweep log" 0 \ "issueflow: #35: blocked declarations parse to {#90, #91, #92}" \ printf '%s\n' "$changed_echo" check "...and leaving the first echo alone" 0 "1" \ - grep -cF '' "$TMP/posted-35" + grep -cF "" "$TMP/posted-35" # shellcheck disable=SC2016 # positional parameters belong to bash -c check "no label write comes from the echo path" 0 "" \ bash -c 'test "$1" -eq "$(wc -l <"$2")"' _ "$echo_edits_before" "$TMP/issue-edits" @@ -728,6 +745,32 @@ check "an empty parse is echoed as the empty set" 0 "" \ check "...and blocked-unparseable still fires beside it" 0 "" \ grep -qF '' "$TMP/posted-37" +# The collision the round found, replayed through the sweep: two declarations +# whose parsed sets differ but whose slugs do not. Keyed on the slug, the +# second edit found the first echo's marker and posted nothing — the machine +# silently gating on `acme-widgets#9` while the thread said `acme/widgets#9`, +# which is the readable-but-wrong shape this whole change exists to surface. +# Asserted end-to-end, so it is the second echo landing that is observed. +printf '[]\n' >"$(cfix 38)" +issue_probe 38 blocked 1 false "" 'Blocked by acme/widgets#9.' >/dev/null +check "a qualified cross-repo declaration is echoed" 0 "1" \ + grep -cF 'parse to: {acme/widgets#9}' "$TMP/posted-38" +collision_edits_before="$(wc -l <"$TMP/issue-edits")" +issue_probe 38 blocked 1 false "" 'Blocked by acme-widgets#9.' >/dev/null +check "a slug-colliding edit still draws its own echo" 0 "1" \ + grep -cF 'parse to: {acme-widgets#9}' "$TMP/posted-38" +check "...under a marker of its own" 0 "1" \ + grep -cF "" "$TMP/posted-38" +check "...leaving the colliding first echo alone" 0 "1" \ + grep -cF "" "$TMP/posted-38" +# The cross-repo flag is marker-constant across both parses, so it stays at one +# while the echo moves: what spoke on the second sweep was the changed set. +check "...and not re-flagging cross-repo, which did not change" 0 "1" \ + grep -cF '' "$TMP/posted-38" +# shellcheck disable=SC2016 # positional parameters belong to bash -c +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" + # -- an already-applied stale heals off, and no edit names the flag ---------- jq -n --arg l "$(iso_at $((INOW - 3600)))" \ '[{"event":"labeled","label":{"name":"needs-ruling"},"actor":{"login":"setter"},"created_at":$l}]' >"$(tfix 23)" From 195c49b8e100e20f768b6ce77dfca083eca21cc8 Mon Sep 17 00:00:00 2001 From: cndgrr <59120057+cndgrr@users.noreply.github.com> Date: Mon, 3 Aug 2026 20:07:23 +0000 Subject: [PATCH 4/9] test(issueflow): the marker's collision test is pairwise, not through one form MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Anchoring every pair on the `/` spelling passed under a fix that only taught the slug about `/` — and that fix still collapses `acme-widgets#9`, `acme_widgets#9` and `acme.widgets#9` onto one marker. Found by mutating the implementation to that cheap fix and watching the suite stay green on the cases that matter. The contract is that no two distinct parses collide, so the assertion is now every pair. Refs #252 --- test/issueflow-reconcile.test.sh | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/test/issueflow-reconcile.test.sh b/test/issueflow-reconcile.test.sh index b0098cc..14a38b3 100644 --- a/test/issueflow-reconcile.test.sh +++ b/test/issueflow-reconcile.test.sh @@ -264,10 +264,20 @@ check "the marker survives a cross-repo reference's punctuation" 0 \ check "the slug alone cannot separate a qualified ref from a hyphenated one" 0 "" \ test "$(printf '%s' '{acme/widgets#9}' | tr -c '[:alnum:]' '-' | sed 's/--*/-/g; s/^-//; s/-$//')" \ = "$(printf '%s' '{acme-widgets#9}' | tr -c '[:alnum:]' '-' | sed 's/--*/-/g; s/^-//; s/-$//')" -for collides_with_acme_widgets in '{acme-widgets#9}' '{acme_widgets#9}' '{acme.widgets#9}'; do - check "...but the marker does, against $collides_with_acme_widgets" 1 "" test \ - "$(blocked_parse_marker '{acme/widgets#9}')" \ - = "$(blocked_parse_marker "$collides_with_acme_widgets")" +# Pairwise, and deliberately so. Anchoring every pair on the `/` spelling +# would pass under a fix that only taught the slug about `/` — and that fix +# leaves `{acme-widgets#9}`, `{acme_widgets#9}` and `{acme.widgets#9}` sharing +# one marker. The contract is that no two distinct parses collide, so the test +# is every pair, not every pair through one representative. +marker_family=( + '{acme/widgets#9}' '{acme-widgets#9}' '{acme_widgets#9}' '{acme.widgets#9}' +) +for left in "${marker_family[@]}"; do + for right in "${marker_family[@]}"; do + [ "$left" != "$right" ] || continue + check "...but the marker does: $left vs $right" 1 "" test \ + "$(blocked_parse_marker "$left")" = "$(blocked_parse_marker "$right")" + done done check "the qualifier's punctuation reaches the marker's identity" 1 "" test \ "$(blocked_parse_marker '{#12, heavy-duty/box#9}')" \ From 04bdde7b1e4cdb85abe893cbd2ba89a68e00e9c2 Mon Sep 17 00:00:00 2001 From: cndgrr <59120057+cndgrr@users.noreply.github.com> Date: Mon, 3 Aug 2026 20:35:10 +0000 Subject: [PATCH 5/9] fix(issueflow): the parse echo is idempotent against the last echo, not the history ensure_comment's any-occurrence grep answers "have I ever said this", which is right for a flag like blocked-unparseable and wrong for a value that changes. A -> B -> A found A's own first echo and stayed silent, leaving the thread's newest echo asserting B while the sweep gated on A: a stale parse presented as the current one, and the third edit did change the parsed set, so the criterion says it speaks. blocked_parse_echo_needed compares this parse's marker against the LAST blockers-parsed-* marker on the thread. The read stays inside guarded_read / skip_issue, so an unreadable history still fails closed (#247 D1) rather than answering "nothing echoed yet" and re-posting. ensure_comment is untouched for every other caller. Refs #252 --- .../issueflow-reconcile.sh | 49 ++++++++++++++----- test/issueflow-reconcile.test.sh | 29 +++++++++++ 2 files changed, 66 insertions(+), 12 deletions(-) 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 '" "$TMP/posted-35" +# AC-1's other input, and it is not the sweep above. A re-sweep of a +# BYTE-IDENTICAL body is quiet under both spellings of the decision — the one +# that keys on the parse and the one that keys on the body — so it cannot tell +# them apart. Only an edit that changes the prose and preserves the parse can: +# the refs are reordered and sentences are added on either side, and the set is +# still {#90, #91}. What this pins is that the marker is a function of the +# PARSE and not of the prose around it, which is the property the echo's whole +# idempotency rests on and which nothing else in the suite states. +preserved_edits_before="$(wc -l <"$TMP/issue-edits")" +issue_probe 35 blocked 1 false "" \ + "Some new prose here. Blocked by #91, #90. And more text." >/dev/null +check "a body edit that preserves the parse draws nothing" 0 "1" \ + grep -cF "" "$TMP/posted-35" +check "...and adds no echo under any other marker either" 0 "1" \ + grep -cF '" "$TMP/posted-35"