diff --git a/.github/scripts/dor_reconcile.sh b/.github/scripts/dor_reconcile.sh index 8f91fe88a..83113fa58 100644 --- a/.github/scripts/dor_reconcile.sh +++ b/.github/scripts/dor_reconcile.sh @@ -309,8 +309,19 @@ done < <(gh issue list --repo "$OWNER/$REPO" --state closed --label "$LABEL" --l # 3c. A health issue nobody opens is still silence, and a stalled build has no other signal at all β€” # the flow died before it could say anything. Comment ONCE on the issue itself and mark it, so it # reaches notifications without re-nagging every hour; clear the mark as soon as it recovers. -marked="$(gh issue list --repo "$OWNER/$REPO" --state open --label dor-stuck --label "$LABEL" --limit 50 \ - --json number --jq '.[].number' 2>/dev/null || true)" +# +# Scope the existing marks by the SAME ownership rule the walk uses, NOT by the gate label. Since the +# walk began following board membership, `stalled` can hold an issue that carries no gate label β€” and +# a `--label "$LABEL"` query can never return that issue, so its mark would be invisible, the dedupe +# below would never fire, and the πŸ’€ comment would repeat every hour on something already broken. +# 38 open items on the Feature board have no gate label, so this was one dead build away from firing. +marked="" +while IFS="$US" read -r m_num m_is_bug; do + [ -n "$m_num" ] || continue + if belongs_here "$m_is_bug"; then marked="${marked}${m_num} "; fi +done < <(gh issue list --repo "$OWNER/$REPO" --state open --label dor-stuck --limit 50 \ + --json number,labels \ + --jq '.[] | [(.number|tostring), (([.labels[].name] | index("bug")) != null | tostring)] | join("'"$US"'")' 2>/dev/null || true) for num in $stalled; do case " $(printf '%s ' $marked)" in *" $num "*) continue ;; esac gh issue edit "$num" --repo "$OWNER/$REPO" --add-label dor-stuck >/dev/null 2>&1 || true diff --git a/changes/reconcile-dorstuck-scope.md b/changes/reconcile-dorstuck-scope.md new file mode 100644 index 000000000..6e4345eac --- /dev/null +++ b/changes/reconcile-dorstuck-scope.md @@ -0,0 +1 @@ +- Fixed the "this build has died" notice being able to repeat every hour. It comments directly on the affected issue and is meant to do so only once, remembering with a marker label β€” but it looked that marker up in a way that could never find it on an issue without an `enhancement` / `bug` label, so those issues would have been re-notified on every sweep. 38 open items on the Feature board are in that position. diff --git a/test/ci-scripts/test-dor-reconcile-liveness.sh b/test/ci-scripts/test-dor-reconcile-liveness.sh index 584b2fe50..5cc1281bf 100644 --- a/test/ci-scripts/test-dor-reconcile-liveness.sh +++ b/test/ci-scripts/test-dor-reconcile-liveness.sh @@ -85,7 +85,16 @@ case "$args" in "issue view"*"--json title"*) sed -n "s/^$3\t//p" "$FIX/titles.tsv" ;; # The previous sweep's health issue: its state and the fingerprint buried in its body. "issue view"*"--json state,body"*) serve health.json "$@" ;; - *"--state open --label dor-stuck"*) cat "$FIX/marked.txt" 2>/dev/null || true ;; + # Issues already marked `dor-stuck`. GitHub AND-s repeated --label filters, and the stub has to + # model that: a query gated on the pipeline label sees a different set from one scoped by + # ownership, and that difference IS the behaviour under test. + *"--state open --label dor-stuck"*) + want=""; prev="" + for a in "$@"; do [ "$prev" = "--label" ] && want="$want $a"; prev="$a"; done + jq -c --arg want "$want" \ + '[ .[] | select( (($want | split(" ") | map(select(. != ""))) - [.labels[].name]) == [] ) ]' \ + "$FIX/marked.json" | jq -r "$(jq_arg "$@")" | tr -d '\r' + ;; # The gate-label query β€” only issues MISSING from the board still matter to the walk. *"--state open --label"*"--limit 201"*) serve issues.json "$@" ;; *"--state closed --label"*) : ;; # no closed issues claiming a sidekick @@ -137,6 +146,15 @@ issue_node() { # $1 number $2 created $3 updated $4 labels csv labels: [ $l | split(",")[] | select(. != "") | {name: .} ] }' } +# Say that a previous sweep already marked #370 `dor-stuck`, so the "comment once" dedupe has +# something to find. $2 = the issue's other labels, which is what decides whether a gate-labelled +# query can see the mark at all. +mark_stuck() { # $1 dir $2 extra labels csv + jq -cn --arg l "$2" \ + '[{ number: 370, labels: ([{name:"dor-stuck"}] + [ $l | split(",")[] | select(. != "") | {name:.} ]) }]' \ + > "$1/marked.json" +} + # Give the fixture a health issue as a previous sweep left it, so Β§4 has something to compare against. seed_health() { # $1 dir $2 OPEN|CLOSED $3 body jq -cn '[{number: 886, title: "DoR pipeline health"}]' > "$1/health_list.json" @@ -166,6 +184,7 @@ scenario() { : > "$dir/writes.log" printf '370\tCollapse managed resources\n' > "$dir/titles.tsv" printf '[]\n' > "$dir/health_list.json" + printf '[]\n' > "$dir/marked.json" if [ "$place" = offboard ]; then board_doc > "$dir/board.json" @@ -213,7 +232,7 @@ assert_contains "…and is told to re-dispatch, not that the agent never ran" "r assert_lacks "…and is NOT mis-reported as un-routed" "πŸ•³οΈ #370" "$out" assert_contains "the issue itself gets marked dor-stuck" "add-label dor-stuck" \ "$(cat "$TMP/dead/writes.log")" -assert_contains "…and commented on directly" "died mid-flight" \ +assert_contains "…and commented on directly" "COMMENT: issue comment 370" \ "$(cat "$TMP/dead/writes.log")" # ── 2. A build that IS alive must stay quiet ──────────────────────────────── @@ -274,7 +293,27 @@ out="$(run_sweep "$(scenario "$TMP/offboard" '' 600 '' '' 'state:decompose' '' ' assert_contains "an issue missing from the board still reaches the heal path" \ "#370 is missing from the board" "$out" -# ── 10. An item filed on the wrong board ──────────────────────────────────── +# ── 10. The "comment once" mark must work without the gate label too ──────── +# πŸ’€ comments directly on the issue, and only once β€” the mark is how it remembers. Once the walk +# started following board membership, `stalled` could hold an issue with no gate label, while the +# mark was still looked up with `--label dor-stuck --label "$LABEL"`. That query can never return +# such an issue, so its mark was invisible and the comment repeated every hour on something already +# broken. 38 open Feature-board items have no gate label; this was one dead build away from firing. +dir="$(scenario "$TMP/stuckmark" 'Building' 40 '' '' '' nogate)" +mark_stuck "$dir" "" # marked dor-stuck, and carrying no gate label +run_sweep "$dir" >/dev/null +assert_lacks "an already-marked stalled issue is not commented on again" "COMMENT: issue comment 370" \ + "$(cat "$dir/writes.log")" +assert_contains "…though it is still reported in the health digest" "πŸ’€ #370" \ + "$(cat "$dir/writes.log")" + +# ── 11. …and an unmarked one still gets its one comment ───────────────────── +dir="$(scenario "$TMP/stuckfirst" 'Building' 40 '' '' '' nogate)" +run_sweep "$dir" >/dev/null +assert_contains "a newly stalled issue is commented on once" "COMMENT: issue comment 370" \ + "$(cat "$dir/writes.log")" + +# ── 12. An item filed on the wrong board ──────────────────────────────────── # #819 sat on the Feature board at "Blocked (external)" while the Bug board had it at "Awaiting # functional acceptance". Membership-driven, it must be named as misfiled rather than un-routed. out="$(run_sweep "$(scenario "$TMP/wrongboard" 'Blocked (external)' 600 '' '' '' nogate bug)")"