diff --git a/.github/scripts/maintenance-reconcile-lost-runs/reconcile.test.mjs b/.github/scripts/maintenance-reconcile-lost-runs/reconcile.test.mjs index 2290772aa2..93b29686b1 100644 --- a/.github/scripts/maintenance-reconcile-lost-runs/reconcile.test.mjs +++ b/.github/scripts/maintenance-reconcile-lost-runs/reconcile.test.mjs @@ -20,6 +20,7 @@ import { test } from "node:test"; import { createGitHub, GitHubError } from "../../actions/check-trusted-trigger/check-trusted-trigger.mjs"; import { createRateLimitLatch } from "../../actions/log-api-budget/rate-guard.mjs"; import { GITHUB_STATUS_URL, JUDGE_PREDECESSOR, JUDGE_STEP, KILL_SWITCH_NOTE, main, NPM_STATUS_URL, PR_UNBLOCK_STOP_NOTE, prefilterSpec, WRITE_STEP } from "./reconcile.mjs"; +import { switchHold } from "../pr-merge/merge-holds.mjs"; import { tempDir } from "../../../lib/test-tmp.mjs"; const REPO = "melodic-software/claude-code-plugins"; @@ -1518,9 +1519,13 @@ test("ci-status's lanes-hold step still fails a lanes App PR with the kill-switc const workflow = readFileSync(new URL("../../workflows/pr-require-checks.yml", import.meta.url), "utf8"); const step = /- name: Fail a lanes App pull request its body or a member holds\n(?:(?:[ ]{8,}.*)?\n)+/.exec(workflow)?.[0] ?? ""; assert.match(step, /LANES_DISABLED: \$\{\{ vars\.CLAUDE_LANES_DISABLED \}\}/); - const fail = step.split("\n").findIndex((line) => line.includes(`echo "::error::#$pr: ${KILL_SWITCH_NOTE} (`)); + // The step prints merge-holds.mjs switchHold's reason as its error. + assert.match(step, /const \{ switchHold \} = await import\(/); + const fail = step.split("\n").findIndex((line) => line.includes('echo "::error::${reason//%/%25}"')); assert.ok(fail > 0, "the kill-switch error line is in the step"); assert.match(step.split("\n")[fail + 1], /^[ ]+exit 1$/); + const lanes = { number: 1, user: { type: "Bot", id: 7 } }; + assert.ok(switchHold(lanes, { appId: "7", disabled: "true", repoDisabled: "false" }).startsWith(`#1: ${KILL_SWITCH_NOTE} (`)); }); // Any PR whose newest pr-require-checks run failed only in ci-status, on its diff --git a/.github/workflows/pr-require-checks.yml b/.github/workflows/pr-require-checks.yml index af1cc7d2e6..6757cfb749 100644 --- a/.github/workflows/pr-require-checks.yml +++ b/.github/workflows/pr-require-checks.yml @@ -4502,7 +4502,8 @@ jobs: # an escalation puts the label on so a person decides; removing it is the # approval, and the `unlabeled` event re-runs this job. The label's name # is the base commit's .work-item-tracker.json role_labels.human-gated - # (default `needs-human`), resolved as pr-merge.yml resolves it, and read + # (default `needs-human`), resolved by the base checkout's merge-holds.mjs + # humanGatedLabel as pr-merge.yml resolves it, and read # at the base so the pull request cannot rename its own hold. Labels are # read live, not from the event payload, which a later label event # outdates. The reason is the newest hold comment's text: a review @@ -4534,15 +4535,20 @@ jobs: echo "::error::could not read .work-item-tracker.json at $BASE_SHA; re-run this run." exit 1 fi - human_gated=needs-human - if [ -n "$tracker" ]; then - human_gated=$(jq -er '[objects | .config | objects | .role_labels | objects - | .["human-gated"] | strings | gsub("^\\s+|\\s+$"; "")][0] // "" - | if . == "" then "needs-human" else . end' <<<"$tracker") || { - echo "::error::cannot read the human-gated role label from .work-item-tracker.json at $BASE_SHA" - exit 1 + # The base module's humanGatedLabel; an empty tracker config names none. + # The module is not the first argument, which would run its main. + # shellcheck disable=SC2016 # JavaScript, not shell + human_gated=$(TRACKER=$tracker node --input-type=module -e 'import { pathToFileURL } from "node:url"; + const { humanGatedLabel } = await import(pathToFileURL(process.argv[2]).href); + const text = process.env.TRACKER ?? ""; + const label = humanGatedLabel(text === "" ? null : JSON.parse(text)); + if (typeof label !== "string" || !label) { + throw new Error("no verdict"); } - fi + console.log(label);' label "$BASE_CONFIG/.github/scripts/pr-merge/merge-holds.mjs") && [ -n "$human_gated" ] || { + echo "::error::cannot read the human-gated role label from .work-item-tracker.json at $BASE_SHA with its merge-holds.mjs" + exit 1 + } echo "label=$human_gated" >>"$GITHUB_OUTPUT" for attempt in 1 2 3; do if raw=$(pr_read "repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/labels?per_page=100" --paginate) && labels=$(jq -r '.[].name' <<<"$raw"); then @@ -4946,8 +4952,10 @@ jobs: # pull request a person opened when pr-verify fails its head, and removes # it on a pass. It is read here only: a red ci-status on a pull_request # event stops pr-verify from judging the next head (pr-verify.yml), so - # the pass that removes the label would never come. The human-gated label is resolved at the group's base commit, - # as the step above resolves it. It reads only that one pull request: under the ruleset's ALLGREEN grouping a + # the pass that removes the label would never come. The rule is the base + # checkout's merge-holds.mjs labelHolds (the human-gated label by its + # humanGatedLabel, as the step above resolves it), which select-tests' + # queueHolds runs too; a base module that cannot judge fails closed. It reads only that one pull request: under the ruleset's ALLGREEN grouping a # pull request merges only when its own group passes, and a failed group # removes the pull request it was built for, never one ahead of it # (HEADGREEN would need every entry read), so failing on an earlier @@ -4984,15 +4992,33 @@ jobs: echo "::error::could not read .work-item-tracker.json at $BASE_SHA; re-run this run." exit 1 fi - human_gated=needs-human - if [ -n "$tracker" ]; then - human_gated=$(jq -er '[objects | .config | objects | .role_labels | objects - | .["human-gated"] | strings | gsub("^\\s+|\\s+$"; "")][0] // "" - | if . == "" then "needs-human" else . end' <<<"$tracker") || { - echo "::error::cannot read the human-gated role label from .work-item-tracker.json at $BASE_SHA" - exit 1 - } + module=$BASE_CONFIG/.github/scripts/pr-merge/merge-holds.mjs + if [ ! -s "$module" ]; then + echo "::error::cannot read merge-holds.mjs at $BASE_SHA; re-run this run." + exit 1 fi + # The base module's labelHolds over the tracker config (an empty one + # names no label) and the live labels: `label` prints the human-gated + # label, `holds` the reasons as a JSON list of strings. The module is + # never the first argument, which would run its main. + # shellcheck disable=SC2016 # JavaScript, not shell + label_holds() { + TRACKER=$tracker node --input-type=module -e 'import { readFileSync } from "node:fs"; + import { pathToFileURL } from "node:url"; + const [mode, module, labels, number] = process.argv.slice(1); + const { humanGatedLabel, labelHolds } = await import(pathToFileURL(module).href); + const text = process.env.TRACKER ?? ""; + const tracker = text === "" ? null : JSON.parse(text); + const out = mode === "label" ? humanGatedLabel(tracker) : labelHolds(JSON.parse(readFileSync(labels, "utf8")), tracker, Number(number)); + if (mode === "label" ? typeof out !== "string" || !out : !Array.isArray(out) || !out.every((reason) => typeof reason === "string" && reason)) { + throw new Error("no verdict"); + } + console.log(mode === "label" ? out : JSON.stringify(out));' "$@" + } + human_gated=$(label_holds label "$module") && [ -n "$human_gated" ] || { + echo "::error::cannot read the human-gated role label from .work-item-tracker.json at $BASE_SHA with its merge-holds.mjs" + exit 1 + } for attempt in 1 2 3; do if labels=$(gh_read "repos/$GITHUB_REPOSITORY/issues/$pr/labels?per_page=100" --paginate --jq '.[].name'); then break @@ -5000,21 +5026,20 @@ jobs: [ "$attempt" -lt 3 ] || { echo "::error::could not read the labels of #$pr; re-run this run."; exit 1; } sleep $((attempt * 5)) done - held=0 - if grep -qxF do-not-merge <<<"$labels"; then - held=1 - echo "::error::#$pr carries the 'do-not-merge' label; the merge queue removes it. Remove the label and re-queue to merge." - fi - if grep -qxF -- "$human_gated" <<<"$labels"; then - held=1 - echo "::error::#$pr carries '$human_gated'; the merge queue removes it. Removing the label approves this pull request; re-queue it to merge." + jq -Rsc 'split("\n") | map(select(length > 0))' <<<"$labels" >"$RUNNER_TEMP/queued-labels.json" + if ! reasons=$(label_holds holds "$module" "$RUNNER_TEMP/queued-labels.json" "$pr") || [ -z "$reasons" ] || + ! failing=$(jq -r 'if type == "array" then .[] else error("not a list") end' <<<"$reasons"); then + echo "::error::merge-holds.mjs at $BASE_SHA could not judge the labels of #$pr; re-run this run." + exit 1 fi - if grep -qxF pr-verify-failed <<<"$labels"; then - held=1 - echo "::error::#$pr carries 'pr-verify-failed': pr-verify failed its head, and a pass at a new commit removes the label. The merge queue removes it; re-queue it once the label is off." + if [ -n "$failing" ]; then + while IFS= read -r reason; do + echo "::error::${reason//%/%25}" + done <<<"$failing" + exit 1 fi - [ "$held" -eq 0 ] || exit 1 echo "#$pr carries neither 'do-not-merge' nor '$human_gated'." + held=0 # The trust hold, for a pull request an automation opened: its linked # issue (babysit_merge_policy._linked_issue: the closing issues here, # or with none, the one issue its plain `Refs:` lines name, by the @@ -5213,7 +5238,9 @@ jobs: # one armed before it was turned on stops in the merge queue, as the # gate stops: CLAUDE_LANES_DISABLED and CLAUDE_LANES_REPO_DISABLED from # the vars context, no API read, and only the exact value `false` on - # both proceeds. Every other author's pull request is not affected. + # both proceeds, by the base checkout's merge-holds.mjs switchHold, which + # select-tests' queueHolds runs too; a base module that cannot judge + # fails closed. Every other author's pull request is not affected. # Once both read `false`, maintenance-reconcile-lost-runs.yml re-runs # ci-status on each open lanes App pull request this failed, at most 3 # per half-hourly run, when ci-status was the run's only failed job on @@ -5422,8 +5449,27 @@ jobs: echo "#$pr is not the lanes App's; pr-merge's gate or a person reads its holds." exit 0 fi - if [ "${LANES_DISABLED:-}" != false ] || [ "${LANES_REPO_DISABLED:-}" != false ]; then - echo "::error::#$pr: the lanes kill switch is on (CLAUDE_LANES_DISABLED or CLAUDE_LANES_REPO_DISABLED is not 'false'), so no lanes App pull request merges; it merges once both read 'false' and ci-status is re-run, which the lost-runs reconciler does when this is the run's only failure." + # The kill switch, by the base checkout's merge-holds.mjs switchHold: + # it answers JSON null or the hold's reason, and anything else fails + # closed. + printf '%s\n' "$pull" >"$RUNNER_TEMP/lanes-pull.json" + # shellcheck disable=SC2016 # JavaScript, not shell + if [ ! -s "$BASE_CONFIG/.github/scripts/pr-merge/merge-holds.mjs" ] || + ! switched=$(node --input-type=module -e 'import { readFileSync } from "node:fs"; + import { pathToFileURL } from "node:url"; + const { switchHold } = await import(pathToFileURL(process.argv[2]).href); + const { LANES_APP_ID: appId, LANES_DISABLED: disabled, LANES_REPO_DISABLED: repoDisabled } = process.env; + const reason = switchHold(JSON.parse(readFileSync(process.argv[1], "utf8")), { appId, disabled, repoDisabled }); + if (reason !== null && (typeof reason !== "string" || !reason)) { + throw new Error("no verdict"); + } + console.log(JSON.stringify(reason));' "$RUNNER_TEMP/lanes-pull.json" "$BASE_CONFIG/.github/scripts/pr-merge/merge-holds.mjs") || + [ -z "$switched" ] || ! reason=$(jq -er 'if . == null then "" else strings end' <<<"$switched"); then + echo "::error::merge-holds.mjs at the base commit could not judge the lanes kill switch for #$pr; re-run this run." + exit 1 + fi + if [ -n "$reason" ]; then + echo "::error::${reason//%/%25}" exit 1 fi BASE_SHA=$GROUP_BASE_SHA diff --git a/scripts/ci-hold-human-gated.test.sh b/scripts/ci-hold-human-gated.test.sh index f0c901b9a1..b80d2f0e90 100755 --- a/scripts/ci-hold-human-gated.test.sh +++ b/scripts/ci-hold-human-gated.test.sh @@ -151,6 +151,16 @@ expect() { rm -rf "$TMP_ROOT/base-config" mkdir -p "$TMP_ROOT/base-config" [[ -z "${STUB_TRACKER+set}" ]] || printf '%s\n' "$STUB_TRACKER" >"$TMP_ROOT/base-config/.work-item-tracker.json" + # The base's merge-holds.mjs: this tree's, STUB_MODULE_TEXT in its place, or + # none with STUB_NO_MODULE. + if [[ -z "${STUB_NO_MODULE:-}" ]]; then + mkdir -p "$TMP_ROOT/base-config/.github/scripts/pr-merge" + if [[ -n "${STUB_MODULE_TEXT:-}" ]]; then + printf '%s\n' "$STUB_MODULE_TEXT" >"$TMP_ROOT/base-config/.github/scripts/pr-merge/merge-holds.mjs" + else + cp "$ROOT/.github/scripts/pr-merge/merge-holds.mjs" "$TMP_ROOT/base-config/.github/scripts/pr-merge/" + fi + fi if [[ -n "${STUB_PIPELINE:-}" ]]; then mkdir -p "$TMP_ROOT/base-config/docs/conventions" printf '%s\n' "$STUB_PIPELINE" >"$TMP_ROOT/base-config/docs/conventions/pr-pipeline.yaml" @@ -283,7 +293,24 @@ expect "a base checkout that failed fails closed" 1 0 "" 0 "::error::could not r unset STUB_TRACKER STUB_CHECKOUT_FAILED export STUB_TRACKER='' expect "an empty tracker config falls back to needs-human" 1 1 "needs-human" 0 "carries 'needs-human'" -unset STUB_TRACKER +export STUB_TRACKER=' ' +expect "a whitespace-only tracker config fails closed" 1 0 "" 0 "::error::cannot read the human-gated role label" +export STUB_TRACKER='{"config":{"role_labels":{"human-gated":" owner-call "}}}' +expect "the tracker's label is trimmed" 1 1 "owner-call" 0 "::error::#6419 carries 'owner-call'. $APPROVE" +export STUB_TRACKER='{"config":{"role_labels":{"human-gated":" "}}}' +expect "a blank tracker label falls back to needs-human" 1 1 "needs-human" 0 "carries 'needs-human'" +# The rule is the base's merge-holds.mjs humanGatedLabel: a base that cannot +# judge fails closed before any read. +export STUB_TRACKER="$CUSTOM" STUB_NO_MODULE=1 +expect "a base with no merge-holds.mjs fails closed" 1 0 "" 0 "::error::cannot read the human-gated role label from .work-item-tracker.json at $BASE with its merge-holds.mjs" +unset STUB_NO_MODULE +export STUB_MODULE_TEXT='export const other = 1;' +expect "a base module without humanGatedLabel fails closed" 1 0 "" 0 "::error::cannot read the human-gated role label" +export STUB_MODULE_TEXT='export const humanGatedLabel = () => 7;' +expect "a base module answering no label fails closed" 1 0 "" 0 "::error::cannot read the human-gated role label" +export STUB_MODULE_TEXT='export const humanGatedLabel = () => "";' +expect "a base module answering an empty label fails closed" 1 0 "" 0 "::error::cannot read the human-gated role label" +unset STUB_TRACKER STUB_MODULE_TEXT export BASE_SHA=main expect "a base that is not a commit SHA fails closed without a call" 1 0 "" 0 "::error::the base commit 'main' is not a commit SHA" unset BASE_SHA diff --git a/scripts/ci-hold-merge-group.test.sh b/scripts/ci-hold-merge-group.test.sh index b1ce9f2ffa..16dbbe055f 100755 --- a/scripts/ci-hold-merge-group.test.sh +++ b/scripts/ci-hold-merge-group.test.sh @@ -141,7 +141,13 @@ expect() { [[ -z "${STUB_TRACKER+set}" ]] || printf '%s\n' "$STUB_TRACKER" >"$config/.work-item-tracker.json" mkdir -p "$config/.github/standards/trusted-actors" "$config/.github/scripts/pr-merge" printf '%s\n' "${STUB_ACTORS:-$ACTORS}" >"$config/.github/standards/trusted-actors/trusted-actors.json" - cp "$ROOT/.github/scripts/pr-merge/merge-holds.mjs" "$config/.github/scripts/pr-merge/" + # The base's merge-holds.mjs: this tree's, STUB_MODULE_TEXT in its place, or + # none with STUB_NO_MODULE. + if [[ -n "${STUB_MODULE_TEXT:-}" ]]; then + printf '%s\n' "$STUB_MODULE_TEXT" >"$config/.github/scripts/pr-merge/merge-holds.mjs" + elif [[ -z "${STUB_NO_MODULE:-}" ]]; then + cp "$ROOT/.github/scripts/pr-merge/merge-holds.mjs" "$config/.github/scripts/pr-merge/" + fi [[ -z "${STUB_CHECKOUT_FAILED:-}" ]] || config= out="$(PATH="$TMP_ROOT/bin:$PATH" STUB_LOG="$TMP_ROOT/log" GITHUB_REF="$5" STUB_LABELS="$6" STUB_FAILS="$7" \ GITHUB_REPOSITORY=o/r RUNNER_TEMP="$TMP_ROOT" LANES_APP_ID="$LANES" BASE_SHA="${BASE_SHA-$BASE}" BASE_CONFIG="$config" \ @@ -188,6 +194,31 @@ expect "an invalid tracker config fails closed" 1 0 - "$QUEUE" "" 0 "::error::ca export STUB_TRACKER="$CUSTOM" STUB_CHECKOUT_FAILED=1 expect "a base checkout that failed fails closed" 1 0 - "$QUEUE" "" 0 "::error::could not read .work-item-tracker.json at $BASE; re-run this run." unset STUB_TRACKER STUB_CHECKOUT_FAILED +export STUB_TRACKER='' +expect "an empty tracker config falls back to needs-human" 1 1 6419 "$QUEUE" "needs-human" 0 "::error::#6419 carries 'needs-human'" +export STUB_TRACKER=' ' +expect "a whitespace-only tracker config fails closed" 1 0 - "$QUEUE" "" 0 "::error::cannot read the human-gated role label" +unset STUB_TRACKER +# Several hold labels each name their hold. +expect "every hold label is named" 1 1 6419 "$QUEUE" "pr-verify-failed,needs-human,do-not-merge" 0 \ + "::error::#6419 carries the 'do-not-merge' label; the merge queue removes it. Remove the label and re-queue to merge. +::error::#6419 carries 'needs-human'; the merge queue removes it." +# The rule is the base's merge-holds.mjs labelHolds: a base that cannot judge +# fails closed, before the labels read when it cannot name the label. +export STUB_NO_MODULE=1 +expect "a base with no merge-holds.mjs fails closed without a call" 1 0 - "$QUEUE" "do-not-merge" 0 "::error::cannot read merge-holds.mjs at $BASE; re-run this run." +unset STUB_NO_MODULE +export STUB_MODULE_TEXT='export const refsIssue = () => null;' +expect "a base module without labelHolds fails closed without a call" 1 0 - "$QUEUE" "" 0 "::error::cannot read the human-gated role label from .work-item-tracker.json at $BASE with its merge-holds.mjs" +export STUB_MODULE_TEXT='export const humanGatedLabel = () => "needs-human";' +expect "a base module with humanGatedLabel alone fails closed" 1 1 6419 "$QUEUE" "" 0 "::error::merge-holds.mjs at $BASE could not judge the labels of #6419; re-run this run." +export STUB_MODULE_TEXT='export const humanGatedLabel = () => "needs-human"; export const labelHolds = () => null;' +expect "a base module answering no list fails closed" 1 1 6419 "$QUEUE" "" 0 "could not judge the labels of #6419" +export STUB_MODULE_TEXT='export const humanGatedLabel = () => "needs-human"; export const labelHolds = () => [""];' +expect "a base module answering an empty reason fails closed" 1 1 6419 "$QUEUE" "" 0 "could not judge the labels of #6419" +export STUB_MODULE_TEXT='export const humanGatedLabel = () => "needs-human"; export const labelHolds = () => { process.exit(0); };' +expect "a base module exiting silently fails closed" 1 1 6419 "$QUEUE" "" 0 "could not judge the labels of #6419" +unset STUB_MODULE_TEXT export BASE_SHA=main expect "a base that is not a commit SHA fails closed without a call" 1 0 - "$QUEUE" "" 0 "::error::the base commit 'main' is not a commit SHA" unset BASE_SHA diff --git a/scripts/ci-lanes-pr-hold.test.sh b/scripts/ci-lanes-pr-hold.test.sh index 1278286337..f629e240da 100755 --- a/scripts/ci-lanes-pr-hold.test.sh +++ b/scripts/ci-lanes-pr-hold.test.sh @@ -250,10 +250,12 @@ review() { jq -nc --arg login "$1" --arg assoc "$2" --arg at "$3" --arg body "$4 '{user: {login: $login, type: "User"}, author_association: $assoc, submitted_at: $at, body: $body}'; } # The base commit's config, as the base_config checkout holds it: this tree's -# pr-pipeline.yaml and trusted-actor list. STUB_BASE_CONFIG replaces its path. -mkdir -p "$TMP_ROOT/base-config/docs/conventions" "$TMP_ROOT/base-config/.github/standards/trusted-actors" +# pr-pipeline.yaml, trusted-actor list and merge-holds.mjs, whose switchHold +# judges the kill switch. STUB_BASE_CONFIG replaces its path. +mkdir -p "$TMP_ROOT/base-config/docs/conventions" "$TMP_ROOT/base-config/.github/standards/trusted-actors" "$TMP_ROOT/base-config/.github/scripts/pr-merge" cp "$ROOT/docs/conventions/pr-pipeline.yaml" "$TMP_ROOT/base-config/docs/conventions/" cp "$ROOT/.github/standards/trusted-actors/trusted-actors.json" "$TMP_ROOT/base-config/.github/standards/trusted-actors/" +cp "$MODULE" "$TMP_ROOT/base-config/.github/scripts/pr-merge/" # expect