diff --git a/.changes/source-control/fix-lanes-hold-index-mode-pass-read-3f9c2a71.md b/.changes/source-control/fix-lanes-hold-index-mode-pass-read-3f9c2a71.md new file mode 100644 index 00000000000..b36914045da --- /dev/null +++ b/.changes/source-control/fix-lanes-hold-index-mode-pass-read-3f9c2a71.md @@ -0,0 +1,7 @@ +--- +bump: none +--- + +### Changed + +- The babysit-prs verify-rules sync test lifts the lanes App hold step's pass-name list along with its lookup loop, since the loop now reads the names from that list. Test-only: nothing that ships changes. diff --git a/.github/workflows/pr-require-checks.yml b/.github/workflows/pr-require-checks.yml index 0d6b9b46626..9c995edd683 100644 --- a/.github/workflows/pr-require-checks.yml +++ b/.github/workflows/pr-require-checks.yml @@ -6436,17 +6436,31 @@ jobs: # holds on it and on the missing pass, and a trusted approval of the # head lifts both. An allow with no listing has no pass to bind. listed=true + pass_names=("pr-verify-$pr-$head-pass" "pr-verify-$pr-$head-pass-$digest" + "pr-verify-$pr-$head-pass-notes" "pr-verify-$pr-$head-pass-notes-$digest") if [ ! -s "$BABYSIT_VERIFY_LISTING" ]; then if jq -e --arg head "$head" '.head == $head and (.refusal | type) == "string"' "$RUNNER_TEMP/evidence.json" >/dev/null 2>&1; then listed=false echo "::notice::the verdict reader refused #$pr before listing artifacts; merge-holds.mjs judges its refusal with no pass." + elif [ -f "$(dirname "$VERIFY_EVIDENCE_SCRIPT")/verify-legacy-fails.json" ]; then + # FAIL index mode: the reader reads by exact name and saves no + # listing, so the head's pass names are read the same way. + found="" + for name in "${pass_names[@]}"; do + page=$(read_api "the $name artifacts" "repos/$GITHUB_REPOSITORY/actions/artifacts?name=$name&per_page=100" --jq '.artifacts[]') || exit 1 + found+="$page"$'\n' + done + jq -sc '{artifacts: .}' <<<"$found" >"$BABYSIT_VERIFY_LISTING" || { + echo "::error::the pass artifacts of #$pr do not parse." + exit 1 + } + echo "the verdict reader ran in FAIL index mode for #$pr; its pass artifacts were read by exact name." else echo "::error::the verdict reader read no artifact listing for #$pr: $(tr -d '\r\n%' <"$RUNNER_TEMP/evidence.json")" exit 1 fi fi - [ "$listed" = false ] || for name in "pr-verify-$pr-$head-pass" "pr-verify-$pr-$head-pass-$digest" \ - "pr-verify-$pr-$head-pass-notes" "pr-verify-$pr-$head-pass-notes-$digest"; do + [ "$listed" = false ] || for name in "${pass_names[@]}"; do verdict=${name#"pr-verify-$pr-$head-"} verdict=${verdict%"-$digest"} runs=$(jq -r --arg name "$name" '.artifacts[] | select(.name == $name and .expired == false) | "\(.workflow_run.id) \(.id)"' "$BABYSIT_VERIFY_LISTING") || { diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_verify_rules_sync.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_verify_rules_sync.py index 3170c8c8222..ee94ec346f2 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_verify_rules_sync.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_verify_rules_sync.py @@ -775,6 +775,8 @@ def at(prefix: str, after: int = 0) -> int: raise AssertionError(f"no line starting {prefix!r} in the queue lookup") digest = at("digest=$(") + names = at("pass_names=(") + names_end = next(n for n in range(names, len(body)) if body[n].rstrip().endswith(")")) loop = at('[ "$listed" = false ] || for name in') loop_end = body.index(" done", loop) artifact = at("artifact=$(jq -sr") @@ -783,6 +785,7 @@ def at(prefix: str, after: int = 0) -> int: "\n".join( [ body[digest], + *body[names : names_end + 1], *body[loop : loop_end + 1], *body[artifact : artifact_end + 1], ] diff --git a/scripts/ci-lanes-pr-hold.test.sh b/scripts/ci-lanes-pr-hold.test.sh index 99188a7b9e9..c22ba0eeb93 100755 --- a/scripts/ci-lanes-pr-hold.test.sh +++ b/scripts/ci-lanes-pr-hold.test.sh @@ -889,6 +889,17 @@ for refusal in verify:unreadable verify:too-old; do done STUB_NO_LISTING=1 STUB_EVIDENCE="$(jq -nc --arg head "$SHA" '{refusal: null, head: $head}')" \ expect "in the merge queue, a reader that allows a head with no listing fails closed" 1 3 "::error::the verdict reader read no artifact listing for #7" +# FAIL index mode (the legacy FAILs file beside the reader): the reader saves +# no listing, so the head's pass names are read by exact name instead. +: >"$TMP_ROOT/verify-legacy-fails.json" +STUB_NO_LISTING=1 expect "in the merge queue in FAIL index mode, a pass at the head passes" 0 3 "#7: no verifier hold: pr-verify passed this head." +if [[ "$(lookups)" -ge 1 && "$(lookups)" -le 4 ]]; then + ok "in FAIL index mode the pass names are read by exact name ($(lookups) reads)" +else + fail "in FAIL index mode the pass names took $(lookups) reads" +fi +STUB_NO_LISTING=1 STUB_ARTIFACTS='{}' expect "in the merge queue in FAIL index mode, no pass at the head holds" 1 3 "pr-verify" +rm -f "$TMP_ROOT/verify-legacy-fails.json" # A cold reader (no carried run cache) runs only when GITHUB_TOKEN's core # pool has the cold read's cost for every entry the queue builds at once # (the merge_queue rule's max_entries_to_build, STUB_RULES, 1 by default)