From 9cb4baf81d5b23bdbf38088d9d368cd0d3284d34 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Tue, 11 Aug 2026 09:11:08 +0200 Subject: [PATCH 1/2] fix(dor): stop the reconcile sweep mis-reading every routed issue as un-routed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The per-issue record was `@tsv` read back with `IFS=$'\t'`. Tab is an IFS *whitespace* character, so bash collapses a run of tabs into one delimiter: an empty field in the middle of the record shifts every field after it left. `sk_label` is empty on almost every issue (only a live build holds a sidekick), so the common record `...state:decompose` parsed as sk_label='state:decompose', state_label=''. Two consequences, both live: * every correctly-routed issue took the "no state:* label" branch and was reported "the agent likely never ran" — 30 of them on the Feature board and #997 on the Bug board, all false; * that branch `continue`s, so the drift check, the approval-backlog count and the stale-waiting warnings below it never ran for any routed issue at all. Join on US (\x1f) instead, which is not IFS whitespace, so empty fields survive. Verified against live repo data: #997 now yields sk_label='', state_label='state:out-of-pipeline'; under the old scheme it yields the reverse. A read-only replay over both boards shows all 31 current exceptions go silent and none turns into a drift flag. Also treat "Out of pipeline" as terminal in the closed-issue check. The column means "handled by the normal dev flow", so closing from it is the expected end, not drift — #995 and #874 were being nagged about forever for being resolved exactly as intended. The harness could not have caught this: its fixture hand-encoded the record, so the jq -> `read` contract was duplicated rather than tested. The stub now serves the JSON `gh` would return and applies the script's OWN --jq program to it. 7 new assertions; the 4 that target these defects fail against main, the 9 existing #995 assertions still pass. Co-Authored-By: Claude Opus 5 (1M context) --- .github/scripts/dor_reconcile.sh | 28 ++++-- changes/reconcile-field-shift.md | 4 + .../ci-scripts/test-dor-reconcile-liveness.sh | 85 +++++++++++++++++-- 3 files changed, 100 insertions(+), 17 deletions(-) create mode 100644 changes/reconcile-field-shift.md diff --git a/.github/scripts/dor_reconcile.sh b/.github/scripts/dor_reconcile.sh index 2cf797d46..479ec1387 100644 --- a/.github/scripts/dor_reconcile.sh +++ b/.github/scripts/dor_reconcile.sh @@ -111,22 +111,29 @@ build_phase() { } # 2. Walk every OPEN enhancement issue. Capture the list first so a transient failure aborts under -# set -e rather than silently reporting "healthy"; state_label is LAST so an empty label (the -# common case) is a trailing field that `read` strips cleanly instead of shifting the columns. -issues_tsv="$(gh issue list --repo "$OWNER/$REPO" --state open --label "$LABEL" --limit 201 \ +# set -e rather than silently reporting "healthy". +# +# Records are joined on US (\x1f), NOT tabs. Tab is an *IFS whitespace* character, so under +# `IFS=$'\t'` bash collapses a run of tabs into ONE delimiter and an empty field in the middle of +# the record silently shifts every field after it left. `sk_label` is empty on almost every issue +# (only a live build holds a sidekick), so the overwhelmingly common record — +# `…state:decompose` — parsed as sk_label='state:decompose', state_label='' and made +# every correctly-routed issue look un-routed. US is not IFS whitespace, so empty fields survive. +# Neither an issue number nor a GitHub label name can contain a control character. +issues_rows="$(gh issue list --repo "$OWNER/$REPO" --state open --label "$LABEL" --limit 201 \ --json number,labels,createdAt,updatedAt \ --jq '.[] | [(.number|tostring), (.createdAt | fromdateiso8601 | tostring), (.updatedAt | fromdateiso8601 | tostring), ((([.labels[].name] | index("needs-vouch")) != null) | tostring), ([.labels[].name | select(startswith("sk:"))][0] // ""), - ([.labels[].name | select(startswith("state:"))][0] // "")] | @tsv')" -if [ "$(printf '%s' "$issues_tsv" | grep -c .)" -ge 201 ]; then + ([.labels[].name | select(startswith("state:"))][0] // "")] | join("\u001f")')" +if [ "$(printf '%s' "$issues_rows" | grep -c .)" -ge 201 ]; then add_ex "⚠️ Over 200 open ${LABEL} issues — reconcile inspected only the first 200; add pagination." - issues_tsv="$(printf '%s\n' "$issues_tsv" | head -n 200)" + issues_rows="$(printf '%s\n' "$issues_rows" | head -n 200)" fi approval_backlog=0 -while IFS=$'\t' read -r num created_epoch updated_epoch needs_vouch sk_label state_label; do +while IFS=$'\x1f' read -r num created_epoch updated_epoch needs_vouch sk_label state_label; do [ -n "$num" ] || continue status="$(board_status_of "$num")" @@ -213,14 +220,17 @@ while IFS=$'\t' read -r num created_epoch updated_epoch needs_vouch sk_label sta [ -z "$open_pr" ] && add_ex "🧟 #${num} still claims \`${sk_label}\` with no open PR — that box is probably holding a stale env. Release it, or drop the label if it already was." fi fi -done < <(printf '%s\n' "$issues_tsv") +done < <(printf '%s\n' "$issues_rows") [ "$approval_backlog" -gt 0 ] && add_ex "🚦 ${approval_backlog} issue(s) waiting in **Awaiting approval** — the Product board's value gate." # 3. Closed issues still parked in a non-terminal board Status. +# "Out of pipeline" is terminal too: it means "this is not a feature for this process — handled by +# the normal dev flow", so being closed from that column is the expected end, not drift. Flagging it +# nagged forever on issues that were already resolved exactly as intended (#995, #874). while IFS=$'\t' read -r num istate status; do [ "$istate" = "CLOSED" ] || continue - case "$status" in ""|"Done") : ;; *) add_ex "🔚 #${num} is CLOSED but still on the board as **${status}** — move it to Done or off the board." ;; esac + case "$status" in ""|"Done"|"Out of pipeline") : ;; *) add_ex "🔚 #${num} is CLOSED but still on the board as **${status}** — move it to Done or off the board." ;; esac done < <(printf '%s\n' "$board") # 3b. Closed issues that still claim a sidekick: the release never ran, or ran against the wrong box diff --git a/changes/reconcile-field-shift.md b/changes/reconcile-field-shift.md new file mode 100644 index 000000000..9f333c4f0 --- /dev/null +++ b/changes/reconcile-field-shift.md @@ -0,0 +1,4 @@ +- Fixed the DoR pipeline health report claiming that correctly-routed issues were "on the board with no `state:*` label — the agent likely never ran". Any issue that had a routing label but no reserved sidekick was misread as un-routed, which was every routed issue on both boards: all 31 items the two health issues were reporting were false alarms. +- Restored the health report's drift detection (routing label disagrees with the board's Status column), which had never been able to run for the same reason. +- The waiting-on-a-human signals — the approval-gate backlog count and the long-stale warnings — now reach the health report; they were being skipped alongside the drift check. +- The health report no longer nags about closed issues sitting in **Out of pipeline**. That column means "handled by the normal dev flow", so being closed from it is the expected ending, not drift. diff --git a/test/ci-scripts/test-dor-reconcile-liveness.sh b/test/ci-scripts/test-dor-reconcile-liveness.sh index 0cc463f40..66695198f 100644 --- a/test/ci-scripts/test-dor-reconcile-liveness.sh +++ b/test/ci-scripts/test-dor-reconcile-liveness.sh @@ -7,6 +7,14 @@ # before the liveness check, and the build side deliberately runs without a state label. Nothing # tested this file, which is why it shipped shadowed and stayed that way. # +# The complementary failure was the record encoding itself. `@tsv` + `IFS=$'\t'` collapses a run of +# tabs (tab is IFS *whitespace*), so an empty `sk_label` in the middle of the record shifted every +# field after it: `state_label` came out empty on every issue that had a state label but no sidekick +# — i.e. nearly all of them. Every correctly-routed issue was reported "un-routed", and the drift +# check, which needs the label, never ran at all. The first version of this harness could not see it +# because the fixture hand-encoded the record; the stub now serves JSON and applies the script's OWN +# --jq program to it, so the jq → `read` contract is under test rather than duplicated here. +# # Approach: put a stub `gh` on PATH that serves fixtures and records writes, then run the REAL # script end to end and assert on the health-report body it produces. No network, no tokens. # @@ -17,6 +25,11 @@ set -euo pipefail REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" SCRIPT="$REPO_ROOT/.github/scripts/dor_reconcile.sh" +command -v jq >/dev/null 2>&1 || { + echo "jq is required: the stub applies the script's real --jq program to the JSON fixtures." >&2 + exit 1 +} + PASS=0 FAIL=0 @@ -63,9 +76,17 @@ case "$args" in *"actions/runs?status="*) cat "$FIX/live_runs.txt" ;; # has_live_run's title lookup — `gh issue view --repo … --json title`, so the number is $3. "issue view"*"--json title"*) sed -n "s/^$3\t//p" "$FIX/titles.tsv" ;; - # The open-issue walk. The script's own --jq is bypassed: we serve the TSV it expects. + # The open-issue walk. Serve the JSON real `gh` would return and run the script's OWN --jq program + # over it, so the record encoding is exercised instead of being hand-copied into the fixture. *"--state open --label dor-stuck"*) cat "$FIX/marked.txt" 2>/dev/null || true ;; - *"--state open --label"*"--limit 201"*) cat "$FIX/issues.tsv" ;; + *"--state open --label"*"--limit 201"*) + jq_prog=""; prev="" + for a in "$@"; do + if [ "$prev" = "--jq" ]; then jq_prog="$a"; break; fi + prev="$a" + done + jq -r "$jq_prog" "$FIX/issues.json" + ;; *"--state closed --label"*) : ;; # no closed issues claiming a sidekick *"--state all --limit 100"*) : ;; # no health issue yet -> the script creates one "pr list"*) : ;; # no open PR (zombie check) @@ -90,10 +111,11 @@ run_sweep() { sed -n '/^CREATE_BODY: /,$p' "$fix/writes.log" 2>/dev/null || true } -# Build a fixture dir. $2 = board Status, $3 = minutes since the issue was updated, -# $4 = "live" to make a run look alive for it. +# Build a fixture dir holding ONE open issue (#370). +# $2 board Status $3 minutes since the issue was last updated $4 "live" to give it a live run +# $5 its `sk:*` label ('' for none) $6 its `state:*` label ('' for none, as the build side leaves it) scenario() { - local dir="$1" status="$2" upd_min="$3" live="${4:-}" + local dir="$1" status="$2" upd_min="$3" live="${4:-}" sk="${5-sk:sk3}" state="${6-}" local now created updated now="$(date -u +%s)" created=$(( now - 3600 * 1000 )) # ancient: opened ~42 days ago, like #370 @@ -103,9 +125,11 @@ scenario() { : > "$dir/writes.log" printf '370\tOPEN\t%s\n' "$status" > "$dir/board.tsv" printf '370\tCollapse managed resources\n' > "$dir/titles.tsv" - # number, created, updated, needs_vouch, sk_label, state_label — state_label EMPTY, as the - # build side always leaves it. - printf '370\t%s\t%s\tfalse\tsk:sk3\t\n' "$created" "$updated" > "$dir/issues.tsv" + jq -n --argjson c "$created" --argjson u "$updated" --arg sk "$sk" --arg st "$state" \ + '[{ number: 370, createdAt: ($c|todate), updatedAt: ($u|todate), + labels: ([{name:"enhancement"}] + + (if $sk == "" then [] else [{name:$sk}] end) + + (if $st == "" then [] else [{name:$st}] end)) }]' > "$dir/issues.json" if [ "$live" = "live" ]; then printf 'Collapse managed resources\n' > "$dir/live_runs.txt" else @@ -114,6 +138,18 @@ scenario() { printf '%s' "$dir" } +# A board that still lists CLOSED issues and no open ones, so only the closed-issue check speaks. +# Two rows, so the assertions are two-sided: #370 carries the status under test, #371 is the control +# that must always be flagged. +closed_scenario() { + local dir="$1" status="$2" + rm -rf "$dir"; mkdir -p "$dir"; make_stub "$dir" + : > "$dir/writes.log"; : > "$dir/live_runs.txt"; : > "$dir/titles.tsv" + printf '370\tCLOSED\t%s\n371\tCLOSED\tBuilding\n' "$status" > "$dir/board.tsv" + printf '[]\n' > "$dir/issues.json" + printf '%s' "$dir" +} + TMP="$(mktemp -d)" trap 'rm -rf "$TMP"' EXIT @@ -148,6 +184,39 @@ assert_contains "an issue nothing owns is still flagged un-routed" "🕳️ #370 out="$(run_sweep "$(scenario "$TMP/fresh" '' 60)")" assert_lacks "a recently-updated issue is not flagged on its creation age" "🕳️ #370" "$out" +echo +echo "DoR reconcile — the record must not shift when a field is empty" +echo + +# ── 5. An issue WITH a state label and NO sidekick — the common shape ─────── +# The record is `…state:decompose`: `sk_label` empty in the MIDDLE. Under @tsv/IFS-tab the +# two delimiters collapsed, `state_label` came out empty, and the sweep reported the issue as +# un-routed while the drift check silently never ran. Status here disagrees with the label, so a +# working parse MUST produce 🔀 — which also makes the 🕳️ assertion non-vacuous. +out="$(run_sweep "$(scenario "$TMP/drift" 'Awaiting design' 600 '' '' 'state:decompose')")" +assert_lacks "a labelled issue is not mis-reported as un-routed" "🕳️ #370" "$out" +assert_contains "the drift check can see the state label again" "🔀 #370" "$out" +assert_contains "…and names both sides of the disagreement" \ + 'label `state:decompose` (→ Decompose) but board Status is **Awaiting design**' "$out" + +# ── 6. …and when the label and the board agree, the sweep says nothing ────── +run_sweep "$(scenario "$TMP/routed" 'Decompose' 600 '' '' 'state:decompose')" >/dev/null +assert_lacks "a correctly-routed issue publishes no health report at all" "CREATE_BODY" \ + "$(cat "$TMP/routed/writes.log")" + +echo +echo "DoR reconcile — closed issues on the board" +echo + +# ── 7. "Out of pipeline" is terminal; closing from it is the expected end ─── +out="$(run_sweep "$(closed_scenario "$TMP/closed-oop" 'Out of pipeline')")" +assert_contains "a closed issue parked mid-pipeline is still flagged" "🔚 #371" "$out" +assert_lacks "a closed 'Out of pipeline' issue is not nagged about forever" "🔚 #370" "$out" + +# ── 8. …and "Done" stays terminal too ─────────────────────────────────────── +out="$(run_sweep "$(closed_scenario "$TMP/closed-done" 'Done')")" +assert_lacks "a closed Done issue is not flagged" "🔚 #370" "$out" + echo echo " $PASS passed, $FAIL failed" [ "$FAIL" -eq 0 ] From a07e2b4f0bb88d89c3f3872bcfc216bf741a93f3 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Tue, 11 Aug 2026 10:06:49 +0200 Subject: [PATCH 2/2] fix(dor): keep flagging closed issues parked in "Out of pipeline" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reverts the second half of the previous commit. Leaving the pipeline is not the same as being filed away: a closed issue sitting in "Out of pipeline" still has to be walked over to Done, and the 🔚 line is the only reminder that it is there. Done stays the single resting state on the board. The field-shift fix is untouched — that is what silences the 31 false 🕳️ lines. Test 7 now pins the behaviour rather than removing it: a closed "Out of pipeline" issue MUST be flagged, with a second closed row as the control so both arms of the assertion are two-sided. 17 assertions; the same 4 field-shift ones fail against main. Co-Authored-By: Claude Opus 5 (1M context) --- .github/scripts/dor_reconcile.sh | 8 ++++---- changes/reconcile-field-shift.md | 1 - .../ci-scripts/test-dor-reconcile-liveness.sh | 19 ++++++++++++------- 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/.github/scripts/dor_reconcile.sh b/.github/scripts/dor_reconcile.sh index 479ec1387..6c6405187 100644 --- a/.github/scripts/dor_reconcile.sh +++ b/.github/scripts/dor_reconcile.sh @@ -225,12 +225,12 @@ done < <(printf '%s\n' "$issues_rows") [ "$approval_backlog" -gt 0 ] && add_ex "🚦 ${approval_backlog} issue(s) waiting in **Awaiting approval** — the Product board's value gate." # 3. Closed issues still parked in a non-terminal board Status. -# "Out of pipeline" is terminal too: it means "this is not a feature for this process — handled by -# the normal dev flow", so being closed from that column is the expected end, not drift. Flagging it -# nagged forever on issues that were already resolved exactly as intended (#995, #874). +# "Out of pipeline" is NOT terminal here, deliberately: an issue that left the pipeline and has since +# been closed still has to be walked over to Done, and this line is the only reminder that it is +# sitting there. Done is the single resting state on the board. while IFS=$'\t' read -r num istate status; do [ "$istate" = "CLOSED" ] || continue - case "$status" in ""|"Done"|"Out of pipeline") : ;; *) add_ex "🔚 #${num} is CLOSED but still on the board as **${status}** — move it to Done or off the board." ;; esac + case "$status" in ""|"Done") : ;; *) add_ex "🔚 #${num} is CLOSED but still on the board as **${status}** — move it to Done or off the board." ;; esac done < <(printf '%s\n' "$board") # 3b. Closed issues that still claim a sidekick: the release never ran, or ran against the wrong box diff --git a/changes/reconcile-field-shift.md b/changes/reconcile-field-shift.md index 9f333c4f0..c90c225f0 100644 --- a/changes/reconcile-field-shift.md +++ b/changes/reconcile-field-shift.md @@ -1,4 +1,3 @@ - Fixed the DoR pipeline health report claiming that correctly-routed issues were "on the board with no `state:*` label — the agent likely never ran". Any issue that had a routing label but no reserved sidekick was misread as un-routed, which was every routed issue on both boards: all 31 items the two health issues were reporting were false alarms. - Restored the health report's drift detection (routing label disagrees with the board's Status column), which had never been able to run for the same reason. - The waiting-on-a-human signals — the approval-gate backlog count and the long-stale warnings — now reach the health report; they were being skipped alongside the drift check. -- The health report no longer nags about closed issues sitting in **Out of pipeline**. That column means "handled by the normal dev flow", so being closed from it is the expected ending, not drift. diff --git a/test/ci-scripts/test-dor-reconcile-liveness.sh b/test/ci-scripts/test-dor-reconcile-liveness.sh index 66695198f..53e6d3c19 100644 --- a/test/ci-scripts/test-dor-reconcile-liveness.sh +++ b/test/ci-scripts/test-dor-reconcile-liveness.sh @@ -139,8 +139,8 @@ scenario() { } # A board that still lists CLOSED issues and no open ones, so only the closed-issue check speaks. -# Two rows, so the assertions are two-sided: #370 carries the status under test, #371 is the control -# that must always be flagged. +# Two rows, so every assertion is two-sided: #370 carries the status under test, #371 is the control +# that must always be flagged (which also proves a report was published at all). closed_scenario() { local dir="$1" status="$2" rm -rf "$dir"; mkdir -p "$dir"; make_stub "$dir" @@ -208,14 +208,19 @@ echo echo "DoR reconcile — closed issues on the board" echo -# ── 7. "Out of pipeline" is terminal; closing from it is the expected end ─── +# ── 7. Done is the ONLY resting state — "Out of pipeline" must still nag ──── +# Leaving the pipeline is not the same as being filed away: a closed issue parked in "Out of +# pipeline" still has to be walked over to Done, and this report line is the only reminder that it +# is sitting there. Pinned because it is tempting to read that column as terminal and silence it. out="$(run_sweep "$(closed_scenario "$TMP/closed-oop" 'Out of pipeline')")" -assert_contains "a closed issue parked mid-pipeline is still flagged" "🔚 #371" "$out" -assert_lacks "a closed 'Out of pipeline' issue is not nagged about forever" "🔚 #370" "$out" +assert_contains "a closed issue parked mid-pipeline is flagged" "🔚 #371" "$out" +assert_contains "…and a closed 'Out of pipeline' issue is flagged too" "🔚 #370" "$out" -# ── 8. …and "Done" stays terminal too ─────────────────────────────────────── +# ── 8. …but a closed issue that reached Done is left alone ────────────────── +# #371 still flags, so the report exists and the assertion below is not vacuous. out="$(run_sweep "$(closed_scenario "$TMP/closed-done" 'Done')")" -assert_lacks "a closed Done issue is not flagged" "🔚 #370" "$out" +assert_contains "the control row still flags" "🔚 #371" "$out" +assert_lacks "a closed Done issue is not flagged" "🔚 #370" "$out" echo echo " $PASS passed, $FAIL failed"