diff --git a/.github/actions/resolve-config/callers.test.mjs b/.github/actions/resolve-config/callers.test.mjs index 5df39c1a649..9c95640474c 100644 --- a/.github/actions/resolve-config/callers.test.mjs +++ b/.github/actions/resolve-config/callers.test.mjs @@ -6480,10 +6480,15 @@ test("pr-unblock.yml: resolve-conflicts-select skips a PR whose base is not the const dir = tempDir("pr-unblock-base-"); // A stand-in gh answering the pulls read's --jq with the head SHA, base ref and head repository. writeFileSync(join(dir, "gh"), `#!/usr/bin/env bash\nprintf '%s\\n' "${"d".repeat(40)}" "$FAKE_BASE" o/r\n`, { mode: 0o755 }); + // The step sources its read helper from the default-branch checkout at .base. + const work = join(dir, "work"); + mkdirSync(work); + symlinkSync(fileURLToPath(new URL("../../..", import.meta.url)), join(work, ".base")); const head = (base) => { const out = join(dir, `out-${base.replaceAll("/", "-")}`); writeFileSync(out, ""); const run = spawnSync("bash", ["-c", step("head").run], { + cwd: work, encoding: "utf8", env: { ...process.env, PATH: `${dir}:${process.env.PATH}`, FAKE_BASE: base, PR_NUMBER: "10981", GITHUB_REPOSITORY: "o/r", DEFAULT_BRANCH: "main", GITHUB_OUTPUT: out }, }); diff --git a/.github/scripts/pr-unblock/read-pr-head.bash b/.github/scripts/pr-unblock/read-pr-head.bash new file mode 100644 index 00000000000..10d3ada473a --- /dev/null +++ b/.github/scripts/pr-unblock/read-pr-head.bash @@ -0,0 +1,37 @@ +# shellcheck shell=bash +# shellcheck disable=SC2034,SC2154 # pr_fields is read by the sourcing step; GITHUB_REPOSITORY is set by Actions +# read_pr_fields : reads the pull request through `gh api` +# (the api-budget wrapper's gh when the caller defined it) and leaves the +# fields, one per line, in pr_fields. A failed read prints +# `::error::the PR could not be read (): ` and returns 1; the +# caller ends the step. Classes: rate-limit (a 429, a rate-limit message or the +# wrapper's refusal) is never read again; transient (a 5xx, a network error or +# a body that is not JSON) is read once more after 5 to 10 s; other is final. +# Source it from the default branch checkout, never from the PR's head. +read_pr_fields() { + local err attempt class reason + err=$(mktemp) + for attempt in 1 2; do + if pr_fields=$(gh api "repos/$GITHUB_REPOSITORY/pulls/$1" --jq "$2" 2>"$err"); then + rm -f "$err" + return 0 + fi + class=other + if grep -qiE 'rate limit|RATE_LIMITED|abuse detection|HTTP 429|was not sent' "$err" || + { declare -F api_budget_stop_until >/dev/null && [[ -n $(api_budget_stop_until core) ]]; }; then + class=rate-limit + elif grep -qiE 'HTTP 5[0-9][0-9]|unexpected end of JSON input|invalid character|error connecting to|connection reset|connection refused|i/o timeout|tls handshake timeout|unexpected eof|timed out|timeout' "$err"; then + class=transient + fi + reason=$(tr '\r\n' ' ' <"$err" | cut -c1-300 | sed 's/ *$//') + reason=${reason//%/%25} + if [[ $class == transient && $attempt == 1 ]]; then + echo "::warning::reading #$1 failed (transient): ${reason:-no error text}; reading again" + sleep $((5 + RANDOM % 6)) + continue + fi + echo "::error::the PR could not be read ($class): ${reason:-no error text}" + rm -f "$err" + return 1 + done +} diff --git a/.github/scripts/pr-unblock/workflow-gates.test.mjs b/.github/scripts/pr-unblock/workflow-gates.test.mjs index f43b2a07749..c224960376c 100644 --- a/.github/scripts/pr-unblock/workflow-gates.test.mjs +++ b/.github/scripts/pr-unblock/workflow-gates.test.mjs @@ -5,7 +5,7 @@ // skipped subscription call or a forged model-call output must not turn. import assert from "node:assert/strict"; import { spawnSync } from "node:child_process"; -import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { copyFileSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { test } from "node:test"; @@ -1198,12 +1198,14 @@ for (const [job, written] of [ const dir = mkdtempSync(join(tmpdir(), "pr-head-")); try { writeFileSync(join(dir, "gh"), '#!/usr/bin/env bash\nexec jq -r "$4 | . // \\"\\"" "$PR_PAYLOAD"\n', { mode: 0o755 }); + placeReader(dir); const read = (name, headRepo) => { const payload = join(dir, `${name}.json`); const output = join(dir, `${name}.output`); writeFileSync(payload, JSON.stringify({ head: { sha, repo: headRepo }, base: { ref: "main" } })); writeFileSync(output, ""); const run = spawnSync("bash", ["-e", "-c", script], { + cwd: dir, env: { PATH: `${dir}:${process.env.PATH}`, PR_PAYLOAD: payload, PR_NUMBER: "7", DEFAULT_BRANCH: "main", GITHUB_REPOSITORY: repo, GITHUB_OUTPUT: output }, encoding: "utf8", }); @@ -1223,6 +1225,103 @@ for (const [job, written] of [ } }); +// The default branch checkout's copy of the shared PR read, as the steps source it. +function placeReader(dir) { + const target = join(dir, ".base/.github/scripts/pr-unblock"); + mkdirSync(target, { recursive: true }); + copyFileSync(new URL("./read-pr-head.bash", import.meta.url), join(target, "read-pr-head.bash")); +} + +for (const job of ["resolve-conflicts-select", "sync-shared-copies-select", "address-feedback-select", "review-dependency-select"]) { + test(`pr-unblock.yml: ${job} reads the PR head through the shared helper from the default branch checkout`, () => { + const script = stepScript(stepBlock(UNBLOCK, job, "Read the PR head")); + assert.match(script, /^\. \.base\/\.github\/scripts\/pr-unblock\/read-pr-head\.bash$/m); + assert.match(script, /^read_pr_fields "\$PR_NUMBER" '[^']+' \|\| exit 1$/m); + assert.doesNotMatch(script, /^ *\{ read .*< <\(gh api/m); + }); +} + +// A failed read of the PR reports its error text and class. The stub gh answers +// from a script of one spec per call (ok, empty, or fail:); the stub +// sleep records its argument. +test("pr-unblock.yml: resolve-conflicts-select classifies a failed PR read and retries only a transient one", () => { + const script = stepScript(stepBlock(UNBLOCK, "resolve-conflicts-select", "Read the PR head")); + const repo = "melodic-software/claude-code-plugins"; + const sha = "a".repeat(40); + const dir = mkdtempSync(join(tmpdir(), "pr-read-")); + try { + writeFileSync( + join(dir, "gh"), + `#!/usr/bin/env bash +n=$(( $(cat "$STUB_DIR/calls" 2>/dev/null || echo 0) + 1 )) +echo "$n" >"$STUB_DIR/calls" +spec=$(sed -n "\${n}p" "$STUB_DIR/script") +case $spec in + ok) printf '%s\\n' "${sha}" main "${repo}" ;; + empty) ;; + *) printf '%s\\n' "\${spec#fail:}" >&2; exit 1 ;; +esac +`, + { mode: 0o755 }, + ); + writeFileSync(join(dir, "sleep"), '#!/usr/bin/env bash\necho "$1" >>"$STUB_DIR/sleeps"\n', { mode: 0o755 }); + placeReader(dir); + const read = (specs) => { + for (const file of ["calls", "sleeps", "output"]) writeFileSync(join(dir, file), ""); + writeFileSync(join(dir, "script"), specs.join("\n") + "\n"); + const run = spawnSync("bash", ["-e", "-c", script], { + cwd: dir, + env: { PATH: `${dir}:${process.env.PATH}`, STUB_DIR: dir, PR_NUMBER: "7", DEFAULT_BRANCH: "main", GITHUB_REPOSITORY: repo, GITHUB_OUTPUT: join(dir, "output"), TMPDIR: dir }, + encoding: "utf8", + }); + const lines = (file) => readFileSync(join(dir, file), "utf8").split("\n").filter(Boolean); + return { status: run.status, stdout: run.stdout, calls: lines("calls").length ? Number(lines("calls")[0]) : 0, sleeps: lines("sleeps"), written: readFileSync(join(dir, "output"), "utf8") }; + }; + + for (const text of ["HTTP 502: Bad Gateway", "unexpected end of JSON input", "invalid character 'b' looking for beginning of value", "error connecting to api.github.com"]) { + const run = read([`fail:${text}`, "ok"]); + assert.equal(run.status, 0, `${text}: ${run.stdout}`); + assert.equal(run.calls, 2, text); + assert.equal(run.sleeps.length, 1, text); + assert.ok(run.sleeps[0] >= 5 && run.sleeps[0] <= 10, `${text}: waited ${run.sleeps[0]}`); + assert.match(run.stdout, /^::warning::reading #7 failed \(transient\): /m, text); + assert.equal(run.written, `sha=${sha}\ndefault-base=true\n`, text); + } + + const twice = read(["fail:HTTP 503: unavailable", "fail:HTTP 503: unavailable", "ok"]); + assert.equal(twice.status, 1); + assert.equal(twice.calls, 2, "a transient failure is read once more, not twice"); + assert.match(twice.stdout, /^::error::the PR could not be read \(transient\): HTTP 503: unavailable$/m); + assert.equal(twice.written, ""); + + const other = read(["fail:gh: Not Found (HTTP 404)\r::error::forged%", "ok"]); + assert.equal(other.status, 1); + assert.equal(other.calls, 1); + assert.deepEqual(other.sleeps, []); + assert.match(other.stdout, /^::error::the PR could not be read \(other\): gh: Not Found \(HTTP 404\) ::error::forged%25$/m); + assert.doesNotMatch(other.stdout, /names no head SHA/); + + for (const text of ["gh: API rate limit exceeded for installation (HTTP 403)", "gh: secondary rate limit (HTTP 403)", "gh: too many requests (HTTP 429)", "'gh api x' was not sent: GitHub's core API rate limit holds until 2026-11-04T05:00:00Z"]) { + const limited = read([`fail:${text}`, "ok"]); + assert.equal(limited.status, 1, text); + assert.equal(limited.calls, 1, `${text}: a rate limit is never read again`); + assert.deepEqual(limited.sleeps, [], text); + assert.match(limited.stdout, /^::error::the PR could not be read \(rate-limit\): /m, text); + } + + const bare = read(["fail:"]); + assert.equal(bare.status, 1); + assert.match(bare.stdout, /^::error::the PR could not be read \(other\): no error text$/m); + + const empty = read(["empty"]); + assert.equal(empty.status, 1); + assert.equal(empty.calls, 1); + assert.match(empty.stdout, /^::error::the PR names no head SHA$/m); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + // With no head-sha output, no later job posts a status on that head. for (const [activity, select] of [ ["resolve-conflicts", "resolve-conflicts-select"], diff --git a/.github/workflows/pr-unblock.yml b/.github/workflows/pr-unblock.yml index be247948d1a..79652e314cf 100644 --- a/.github/workflows/pr-unblock.yml +++ b/.github/workflows/pr-unblock.yml @@ -242,11 +242,14 @@ jobs: # api-budget notice (.github/actions/log-api-budget). # shellcheck source=/dev/null { . .base/.github/actions/log-api-budget/api-budget.bash 2>/dev/null && api_budget_wrap_gh github-token; } || true + # shellcheck source=/dev/null + . .base/.github/scripts/pr-unblock/read-pr-head.bash if ! [[ $PR_NUMBER =~ ^[1-9][0-9]{0,9}$ ]]; then echo "::error::the pr-number input is not a PR number" exit 1 fi - { read -r sha && read -r base && read -r head_repo; } < <(gh api "repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER" --jq '.head.sha, .base.ref, .head.repo.full_name') || true + read_pr_fields "$PR_NUMBER" '.head.sha, .base.ref, .head.repo.full_name' || exit 1 + { read -r sha && read -r base && read -r head_repo; } <<<"$pr_fields" || true if ! [[ ${sha:-} =~ ^[0-9a-f]{40}$ ]]; then echo "::error::the PR names no head SHA" exit 1 @@ -936,6 +939,8 @@ jobs: # api-budget notice (.github/actions/log-api-budget). # shellcheck source=/dev/null { . .base/.github/actions/log-api-budget/api-budget.bash 2>/dev/null && api_budget_wrap_gh github-token; } || true + # shellcheck source=/dev/null + . .base/.github/scripts/pr-unblock/read-pr-head.bash if ! [[ $PR_NUMBER =~ ^[1-9][0-9]{0,9}$ ]]; then echo "::error::the pr-number input is not a PR number" exit 1 @@ -944,7 +949,8 @@ jobs: echo "::error::the AUTOMATION_LANES_APP_SENDER_ID variable must hold the lanes App bot's numeric user id" exit 1 fi - { read -r sha && read -r base && read -r head_repo && read -r author; } < <(gh api "repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER" --jq '.head.sha, .base.ref, .head.repo.full_name, .user.id') || true + read_pr_fields "$PR_NUMBER" '.head.sha, .base.ref, .head.repo.full_name, .user.id' || exit 1 + { read -r sha && read -r base && read -r head_repo && read -r author; } <<<"$pr_fields" || true if ! [[ ${sha:-} =~ ^[0-9a-f]{40}$ ]]; then echo "::error::the PR names no head SHA" exit 1 @@ -1360,11 +1366,14 @@ jobs: # api-budget notice (.github/actions/log-api-budget). # shellcheck source=/dev/null { . .base/.github/actions/log-api-budget/api-budget.bash 2>/dev/null && api_budget_wrap_gh github-token; } || true + # shellcheck source=/dev/null + . .base/.github/scripts/pr-unblock/read-pr-head.bash if ! [[ $PR_NUMBER =~ ^[1-9][0-9]{0,9}$ ]]; then echo "::error::the pr-number input is not a PR number" exit 1 fi - { read -r sha && read -r head_repo; } < <(gh api "repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER" --jq '.head.sha, .head.repo.full_name') || true + read_pr_fields "$PR_NUMBER" '.head.sha, .head.repo.full_name' || exit 1 + { read -r sha && read -r head_repo; } <<<"$pr_fields" || true if ! [[ ${sha:-} =~ ^[0-9a-f]{40}$ ]]; then echo "::error::the PR names no head SHA" exit 1 @@ -2417,11 +2426,14 @@ jobs: # api-budget notice (.github/actions/log-api-budget). # shellcheck source=/dev/null { . .base/.github/actions/log-api-budget/api-budget.bash 2>/dev/null && api_budget_wrap_gh github-token; } || true + # shellcheck source=/dev/null + . .base/.github/scripts/pr-unblock/read-pr-head.bash if ! [[ $PR_NUMBER =~ ^[1-9][0-9]{0,9}$ ]]; then echo "::error::the pr-number input is not a PR number" exit 1 fi - { read -r sha && read -r head_repo; } < <(gh api "repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER" --jq '.head.sha, .head.repo.full_name') || true + read_pr_fields "$PR_NUMBER" '.head.sha, .head.repo.full_name' || exit 1 + { read -r sha && read -r head_repo; } <<<"$pr_fields" || true if ! [[ ${sha:-} =~ ^[0-9a-f]{40}$ ]]; then echo "::error::the PR names no head SHA" exit 1