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
This commit is contained in:
parent
195c49b8e1
commit
04bdde7b1e
2 changed files with 66 additions and 12 deletions
|
|
@ -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
|
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
|
# Scoped to the SET's value, not to the issue and not to the sweep: the
|
||||||
# unchanged parse finds its own marker and stays quiet on a 15-minute cron,
|
# marker names WHAT was echoed, and blocked_parse_echo_needed decides whether
|
||||||
# and a changed one cannot find it, so the change is what speaks. One
|
# it is still what the thread is saying.
|
||||||
# consequence is deliberate: a declaration edited back to a set already
|
|
||||||
# echoed stays quiet too, because the thread already carries that echo.
|
|
||||||
#
|
#
|
||||||
# The identity is the DIGEST, not the slug beside it. Slugging is many-to-one
|
# 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
|
# — `{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
|
# 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
|
# changed set find the old marker and say nothing, silence in precisely the
|
||||||
# case the echo exists to speak about. Distinguishing `/` would close that
|
# case the echo exists to speak about. Distinguishing `/` would close that
|
||||||
# pair and leave the class: `-`, `_` and `.` are all legal in a qualifier and
|
# pair and leave the class: `-`, `_` and `.` are legal in a qualifier token
|
||||||
# all collapse the same way. The slug stays in front so a human reading the
|
# and all collapse the same way. The slug stays in front so a human reading
|
||||||
# raw comment can still see which set it belongs to; it decides nothing.
|
# the raw comment can still see which set it belongs to; it decides nothing.
|
||||||
local slug digest
|
local slug digest
|
||||||
slug="$(printf '%s' "$1" | tr -c '[:alnum:]' '-' | sed 's/--*/-/g; s/^-//; s/-$//')"
|
slug="$(printf '%s' "$1" | tr -c '[:alnum:]' '-' | sed 's/--*/-/g; s/^-//; s/-$//')"
|
||||||
digest="$(printf '%s' "$1" | sha256sum | cut -c1-12)"
|
digest="$(printf '%s' "$1" | sha256sum | cut -c1-12)"
|
||||||
printf 'blockers-parsed-%s-%s\n' "${slug:-none}" "$digest"
|
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 '<!-- issueflow:blockers-parsed-[[:alnum:]-]* -->' <<<"$bodies" | tail -n 1)"
|
||||||
|
[ "$last" != "<!-- issueflow:$2 -->" ]
|
||||||
|
}
|
||||||
|
|
||||||
blocked_decision() { # $1 local refs, $2 OPEN/CLOSED states, $3 cross-repo refs
|
blocked_decision() { # $1 local refs, $2 OPEN/CLOSED states, $3 cross-repo refs
|
||||||
local refs="$1" states="$2" cross_refs="${3:-}"
|
local refs="$1" states="$2" cross_refs="${3:-}"
|
||||||
if [ -n "$cross_refs" ]; then echo FLAG_CROSS_REPO
|
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() {
|
reconcile_issue() {
|
||||||
local n="$1" decision refs cross_refs states age created assignees open_pr=false label owners
|
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
|
local unchecked="" remove_claimed=claimed
|
||||||
decision="$(queue_decision <<<"$ISSUE_LABELS")"
|
decision="$(queue_decision <<<"$ISSUE_LABELS")"
|
||||||
case "$decision" in
|
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
|
# 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.
|
# what it read, and let the human see the divergence in one sweep.
|
||||||
parsed_set="$(blocked_parse_set "$refs" "$cross_refs")"
|
parsed_set="$(blocked_parse_set "$refs" "$cross_refs")"
|
||||||
ensure_comment "$n" "$(blocked_parse_marker "$parsed_set")" \
|
parse_marker="$(blocked_parse_marker "$parsed_set")"
|
||||||
"This issue's \`Blocked by\` declarations parse to: $parsed_set
|
if blocked_parse_echo_needed "$n" "$parse_marker"; then
|
||||||
|
run gh issue comment "$n" -R "$REPO" --body "<!-- issueflow:$parse_marker -->
|
||||||
|
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
|
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
|
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.
|
correction.
|
||||||
|
|
||||||
*Comment only: nothing on this path writes a label. The marker carries the set
|
*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"
|
log "#$n: blocked declarations parse to $parsed_set"
|
||||||
states="$(reference_states <<<"$refs")"
|
states="$(reference_states <<<"$refs")"
|
||||||
decision="$(blocked_decision "$refs" "$states" "$cross_refs")"
|
decision="$(blocked_decision "$refs" "$states" "$cross_refs")"
|
||||||
|
|
|
||||||
|
|
@ -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 "" \
|
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"
|
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 '<!-- issueflow:blockers-parsed-' "$TMP/posted-52"
|
||||||
|
check "...under the returning set's own marker, twice on the thread now" 0 "2" \
|
||||||
|
grep -cF "<!-- issueflow:$(blocked_parse_marker '{#90, #91}') -->" "$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 '<!-- issueflow:blockers-parsed-' "$TMP/posted-52"
|
||||||
|
# shellcheck disable=SC2016 # positional parameters belong to bash -c
|
||||||
|
check "no label write comes from the returning-set path either" 0 "" \
|
||||||
|
bash -c 'test "$1" -eq "$(wc -l <"$2")"' _ "$return_edits_before" "$TMP/issue-edits"
|
||||||
|
|
||||||
# -- an already-applied stale heals off, and no edit names the flag ----------
|
# -- an already-applied stale heals off, and no edit names the flag ----------
|
||||||
jq -n --arg l "$(iso_at $((INOW - 3600)))" \
|
jq -n --arg l "$(iso_at $((INOW - 3600)))" \
|
||||||
'[{"event":"labeled","label":{"name":"needs-ruling"},"actor":{"login":"setter"},"created_at":$l}]' >"$(tfix 23)"
|
'[{"event":"labeled","label":{"name":"needs-ruling"},"actor":{"login":"setter"},"created_at":$l}]' >"$(tfix 23)"
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue