From 549ec1b230938c75a9a4326a2a0b3374bab6a5da Mon Sep 17 00:00:00 2001 From: Kyleasmth Date: Mon, 21 Sep 2026 16:33:21 -0700 Subject: [PATCH 1/4] fix(ci): never leave a stale success status when PR context fails When `context` failed the gate job was skipped entirely, so whatever status was last written to that head SHA stood. A signoff edited or deleted without a push therefore left an earlier `success` green on an unsigned major. The PR lookup now retries, and a `pull_request` run whose context still fails posts a failing status from the event payload. `issue_comment` payloads carry no head SHA, so that path relies on the retry and on the next push. Found by Greptile on the RN Expo port (#201); the condition is identical in both workflows, so this lands here first. --- .github/scripts/major-release-signoff.test.sh | 3 ++ .github/workflows/major-release-signoff.yml | 53 ++++++++++++++++++- 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/.github/scripts/major-release-signoff.test.sh b/.github/scripts/major-release-signoff.test.sh index b4afb442..414e4d4c 100644 --- a/.github/scripts/major-release-signoff.test.sh +++ b/.github/scripts/major-release-signoff.test.sh @@ -33,6 +33,9 @@ extract_step "Resolve PR context" > "$TMP/context.sh" extract_step "Decide whether a signoff is required" > "$TMP/decision.sh" extract_step "Regenerate and verify release contents" > "$TMP/verify.sh" +# The context step backs off between retries; real delays would add ~9s per error case. +export RETRY_BACKOFF_SECONDS=0 + mkdir "$TMP/bin" cat > "$TMP/bin/gh" <<'EOF' #!/usr/bin/env bash diff --git a/.github/workflows/major-release-signoff.yml b/.github/workflows/major-release-signoff.yml index 9a451ff9..87cdd87a 100644 --- a/.github/workflows/major-release-signoff.yml +++ b/.github/workflows/major-release-signoff.yml @@ -56,7 +56,23 @@ jobs: # Resolve every event through the API. In particular, issue_comment payloads # contain no PR head, and created/edited/deleted comments must evaluate the # current immutable head rather than the head from an earlier workflow run. - PR_JSON=$(gh api "repos/$REPOSITORY/pulls/$PR_NUMBER") + # Retry rather than die on the first blip. Everything downstream needs the head + # SHA this call returns, and on issue_comment there is no payload fallback, so a + # transient API failure here would otherwise leave the status already on the SHA + # untouched -- including a success whose signoff has since been revoked. + PR_JSON="" + for attempt in 1 2 3; do + if PR_JSON=$(gh api "repos/$REPOSITORY/pulls/$PR_NUMBER"); then + break + fi + PR_JSON="" + echo "::warning::Could not read PR $PR_NUMBER (attempt $attempt of 3)." + sleep $((attempt * ${RETRY_BACKOFF_SECONDS:-3})) + done + if [ -z "$PR_JSON" ]; then + echo "::error::Could not read PR $PR_NUMBER after 3 attempts." + exit 1 + fi HEAD_SHA=$(jq -r '.head.sha // empty' <<<"$PR_JSON") if ! [[ "$HEAD_SHA" =~ ^[0-9a-f]{40}$ ]]; then echo "::error::Could not resolve a full PR head SHA." @@ -125,6 +141,41 @@ jobs: echo "generated_release_pr=$GENERATED_RELEASE_PR" >> "$GITHUB_OUTPUT" + # `context` is what resolves the head SHA, so when it fails the gate below is skipped + # and whatever status was last written to that SHA stands -- including a `success` whose + # signoff comment has since been edited or deleted. Write a failing status from the event + # payload so a revocation can never leave a stale green on a major release. + # + # `pull_request` only: an `issue_comment` payload carries no head SHA, and the call that + # would resolve one is the call that just failed. That path is covered by the retry in + # `context` and by the next push re-evaluating. + # + # `failure`, not `!= 'success'`: a cancelled run has been superseded by a newer one, and + # painting the PR red for that is the noise #404 removed. + context_unresolved: + if: always() && needs.context.result == 'failure' && github.event_name == 'pull_request' + name: Signoff status when context is unresolved + needs: context + runs-on: ubuntu-latest + permissions: + statuses: write + steps: + - name: Post an unresolved status + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + REPOSITORY: ${{ github.repository }} + RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + run: | + set -euo pipefail + gh api "repos/$REPOSITORY/statuses/$HEAD_SHA" \ + -f state=failure \ + -f context="$STATUS_CONTEXT" \ + -f description="Could not resolve PR context; release impact is unknown." \ + -f target_url="$RUN_URL" > /dev/null + echo "Posted a failing status: PR context could not be resolved." + exit 1 + generated_release: # The metadata and comparison shape are a cheap fail-closed prefilter. Prove the # contents separately by reproducing Changesets' output from the immutable base. From bbf1b782c82eb23acb9943644d4d53d4f767f04d Mon Sep 17 00:00:00 2001 From: Kyleasmth <45497561+Kyleasmth@users.noreply.github.com> Date: Wed, 23 Sep 2026 11:29:41 -0700 Subject: [PATCH 2/4] fix(ci): recover the head SHA so a revoked signoff fails on comment events too Claude-Session: https://claude.ai/code/session_01Gnt93sVjs6ApXSzkDC2efU --- .github/scripts/major-release-signoff.test.sh | 102 +++++++++++++++++- .github/workflows/major-release-signoff.yml | 31 ++++-- 2 files changed, 120 insertions(+), 13 deletions(-) diff --git a/.github/scripts/major-release-signoff.test.sh b/.github/scripts/major-release-signoff.test.sh index 414e4d4c..4357f98d 100644 --- a/.github/scripts/major-release-signoff.test.sh +++ b/.github/scripts/major-release-signoff.test.sh @@ -32,17 +32,25 @@ extract_step() { extract_step "Resolve PR context" > "$TMP/context.sh" extract_step "Decide whether a signoff is required" > "$TMP/decision.sh" extract_step "Regenerate and verify release contents" > "$TMP/verify.sh" - -# The context step backs off between retries; real delays would add ~9s per error case. -export RETRY_BACKOFF_SECONDS=0 +extract_step "Post an unresolved status" > "$TMP/unresolved.sh" mkdir "$TMP/bin" +# The context step backs off between retries; real delays would add ~9s per error case. +cat > "$TMP/bin/sleep" <<'EOF' +#!/usr/bin/env bash +exit 0 +EOF +chmod +x "$TMP/bin/sleep" cat > "$TMP/bin/gh" <<'EOF' #!/usr/bin/env bash set -u if [[ "$2" == *"/pulls/"* ]]; then + echo called >> "$MOCK_PULL_CALLS" if [ "${MOCK_PULL_ERROR:-0}" = "1" ]; then exit 1; fi + if [ "$(wc -l < "$MOCK_PULL_CALLS")" -le "${MOCK_PULL_FAILURES:-0}" ]; then exit 1; fi cat "$MOCK_PR_FILE" +elif [[ "$2" == *"/statuses/"* ]]; then + echo "$2" >> "${MOCK_STATUS_CALLS:-/dev/null}" elif [[ "$2" == *"/compare/"* ]]; then echo called >> "$MOCK_COMPARE_CALLS" if [ "${MOCK_COMPARE_ERROR:-0}" = "1" ]; then exit 1; fi @@ -52,6 +60,17 @@ else fi EOF chmod +x "$TMP/bin/gh" +cat > "$TMP/bin/git" <> "\${MOCK_LSREMOTE_CALLS:-/dev/null}" + [ "\${MOCK_LSREMOTE_ERROR:-0}" = "1" ] && exit 1 + printf '%s\trefs/pull/400/head\n' "\$MOCK_LSREMOTE_SHA" + exit 0 +fi +exec $(command -v git) "\$@" +EOF +chmod +x "$TMP/bin/git" cat > "$TMP/bin/pnpm" <<'EOF' #!/usr/bin/env bash set -euo pipefail @@ -71,6 +90,7 @@ chmod +x "$TMP/bin/pnpm" HEAD_SHA=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa BASE_SHA=bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb +RECOVERED_SHA=cccccccccccccccccccccccccccccccccccccccc VALID_PR=$(jq -n \ --arg head "$HEAD_SHA" --arg base "$BASE_SHA" \ '{ @@ -103,13 +123,14 @@ run_context_case() { fi : > "$output" : > "$TMP/compare-calls" + : > "$TMP/pull-calls" printf '%s\n' "$pr_json" > "$pr_file" printf '%s\n' "$compare_json" > "$compare_file" if PATH="$TMP/bin:$PATH" \ EVENT_PR_NUMBER="$event_pr_number" EVENT_ISSUE_NUMBER="$event_issue_number" \ REPOSITORY=youversion/platform-sdk-react \ GITHUB_OUTPUT="$output" MOCK_PR_FILE="$pr_file" MOCK_COMPARE_FILE="$compare_file" \ - MOCK_COMPARE_CALLS="$TMP/compare-calls" bash "$TMP/context.sh" >/dev/null 2>&1 && + MOCK_COMPARE_CALLS="$TMP/compare-calls" MOCK_PULL_CALLS="$TMP/pull-calls" bash "$TMP/context.sh" >/dev/null 2>&1 && grep -Fxq "generated_release_pr=$expected_release_pr" "$output" && grep -Fxq "generated_candidate=$expected_candidate" "$output"; then pass "$name" @@ -123,12 +144,13 @@ run_context_error_case() { local output="$TMP/output" : > "$output" : > "$TMP/compare-calls" + : > "$TMP/pull-calls" printf '%s\n' "$VALID_PR" > "$TMP/pr.json" printf '%s\n' "$VALID_COMPARE" > "$TMP/compare.json" if env PATH="$TMP/bin:$PATH" \ EVENT_PR_NUMBER= EVENT_ISSUE_NUMBER=400 REPOSITORY=youversion/platform-sdk-react \ GITHUB_OUTPUT="$output" MOCK_PR_FILE="$TMP/pr.json" MOCK_COMPARE_FILE="$TMP/compare.json" \ - MOCK_COMPARE_CALLS="$TMP/compare-calls" "$error_var"=1 \ + MOCK_COMPARE_CALLS="$TMP/compare-calls" MOCK_PULL_CALLS="$TMP/pull-calls" "$error_var"=1 \ bash "$TMP/context.sh" >/dev/null 2>&1; then result=success else @@ -144,6 +166,55 @@ run_context_error_case() { fi } +run_context_retry_case() { + local name="$1" pull_failures="$2" expected_result="$3" expected_calls="$4" + local output="$TMP/output" result calls + : > "$output" + : > "$TMP/compare-calls" + : > "$TMP/pull-calls" + printf '%s\n' "$VALID_PR" > "$TMP/pr.json" + printf '%s\n' "$VALID_COMPARE" > "$TMP/compare.json" + if env PATH="$TMP/bin:$PATH" \ + EVENT_PR_NUMBER= EVENT_ISSUE_NUMBER=400 REPOSITORY=youversion/platform-sdk-react \ + GITHUB_OUTPUT="$output" MOCK_PR_FILE="$TMP/pr.json" MOCK_COMPARE_FILE="$TMP/compare.json" \ + MOCK_COMPARE_CALLS="$TMP/compare-calls" MOCK_PULL_CALLS="$TMP/pull-calls" \ + MOCK_PULL_FAILURES="$pull_failures" \ + bash "$TMP/context.sh" >/dev/null 2>&1; then + result=success + else + result=failure + fi + calls=$(wc -l < "$TMP/pull-calls" | tr -d ' ') + if [ "$result" = "$expected_result" ] && [ "$calls" = "$expected_calls" ]; then + pass "$name" + else + fail "$name" "expected $expected_result after $expected_calls PR API calls; got $result after $calls" + fi +} + +run_unresolved_case() { + local name="$1" payload_sha="$2" lsremote_error="$3" expected_sha="$4" + local statuses="$TMP/status-calls" posted + : > "$statuses" + : > "$TMP/lsremote-calls" + env PATH="$TMP/bin:$PATH" \ + EVENT_ISSUE_NUMBER=400 PAYLOAD_HEAD_SHA="$payload_sha" \ + REPOSITORY=youversion/platform-sdk-react STATUS_CONTEXT=major-release-signoff \ + RUN_URL=https://example.invalid/run GH_TOKEN=token \ + MOCK_STATUS_CALLS="$statuses" MOCK_LSREMOTE_SHA="$RECOVERED_SHA" \ + MOCK_LSREMOTE_ERROR="$lsremote_error" MOCK_LSREMOTE_CALLS="$TMP/lsremote-calls" \ + bash "$TMP/unresolved.sh" >/dev/null 2>&1 || true + posted=$(sed 's|.*/statuses/||' "$statuses" | tr -d '\n') + if [ -z "$expected_sha" ]; then + [ ! -s "$statuses" ] && posted="" || posted="$(cat "$statuses")" + fi + if [ "$posted" = "$expected_sha" ]; then + pass "$name" + else + fail "$name" "expected a failing status on '$expected_sha'; got '$posted'" + fi +} + run_context_case "accepts the exact generated release PR and consumed changeset shape" \ true true "$VALID_PR" "$VALID_COMPARE" run_context_case "human issue comments resolve the current PR identity and head" \ @@ -185,6 +256,19 @@ run_context_error_case "comparison API errors retain generated identity and fail success true false MOCK_COMPARE_ERROR run_context_error_case "PR API errors fail the resolver closed" failure false false MOCK_PULL_ERROR +# The retry exists so a blip cannot leave a revoked signoff's status untouched. Assert it +# both recovers and gives up, by call count -- a loop that never retries also "passes" a +# test that only checks the outcome. +run_context_retry_case "transient PR API errors recover on retry" 2 success 3 +run_context_retry_case "persistent PR API errors stop after three attempts" 3 failure 3 + +# The whole point of the job: an unresolved context must still land a failing status on the +# head, whichever event triggered it. issue_comment payloads carry no SHA, so it comes from +# the git ref instead of the REST call that just failed. +run_unresolved_case "a pull_request payload head takes the failing status" "$HEAD_SHA" 0 "$HEAD_SHA" +run_unresolved_case "an issue_comment recovers the head over git" "" 0 "$RECOVERED_SHA" +run_unresolved_case "an unrecoverable head posts no status at all" "" 1 "" + VERIFY_REPO="$TMP/verify-repo" git init --quiet "$VERIFY_REPO" git -C "$VERIFY_REPO" config commit.gpgsign false @@ -299,5 +383,13 @@ else fail "generated releases publish an explicit lifecycle-aware success" "status wording is missing" fi +if awk '/^ context_unresolved:/{f=1} f && /^ if:/{print; exit}' "$WORKFLOW" | + grep -Fq "event_name"; then + fail "an unresolved context fails the status on every event" \ + "context_unresolved is gated to a subset of events, so a revocation can leave a stale success" +else + pass "an unresolved context fails the status on every event" +fi + printf '\n%d passed, %d failed\n' "$passes" "$failures" [[ "$failures" -eq 0 ]] diff --git a/.github/workflows/major-release-signoff.yml b/.github/workflows/major-release-signoff.yml index 87cdd87a..553ab1f1 100644 --- a/.github/workflows/major-release-signoff.yml +++ b/.github/workflows/major-release-signoff.yml @@ -67,7 +67,9 @@ jobs: fi PR_JSON="" echo "::warning::Could not read PR $PR_NUMBER (attempt $attempt of 3)." - sleep $((attempt * ${RETRY_BACKOFF_SECONDS:-3})) + if [ "$attempt" -lt 3 ]; then + sleep $((attempt * 3)) + fi done if [ -z "$PR_JSON" ]; then echo "::error::Could not read PR $PR_NUMBER after 3 attempts." @@ -143,31 +145,44 @@ jobs: # `context` is what resolves the head SHA, so when it fails the gate below is skipped # and whatever status was last written to that SHA stands -- including a `success` whose - # signoff comment has since been edited or deleted. Write a failing status from the event - # payload so a revocation can never leave a stale green on a major release. + # signoff comment has since been edited or deleted. Write a failing status so a revocation + # can never leave a stale green on a major release. # - # `pull_request` only: an `issue_comment` payload carries no head SHA, and the call that - # would resolve one is the call that just failed. That path is covered by the retry in - # `context` and by the next push re-evaluating. + # The head SHA has to come from somewhere other than the API call that just failed. + # `pull_request` payloads carry it; `issue_comment` payloads do not, so read + # `refs/pull/N/head` over git, which is a different backend from the REST call in + # `context` and needs no scope beyond `contents: read`. # # `failure`, not `!= 'success'`: a cancelled run has been superseded by a newer one, and # painting the PR red for that is the noise #404 removed. context_unresolved: - if: always() && needs.context.result == 'failure' && github.event_name == 'pull_request' + if: always() && needs.context.result == 'failure' name: Signoff status when context is unresolved needs: context runs-on: ubuntu-latest permissions: + contents: read statuses: write steps: - name: Post an unresolved status env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - HEAD_SHA: ${{ github.event.pull_request.head.sha }} + EVENT_ISSUE_NUMBER: ${{ github.event.issue.number }} + PAYLOAD_HEAD_SHA: ${{ github.event.pull_request.head.sha }} REPOSITORY: ${{ github.repository }} RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} run: | set -euo pipefail + HEAD_SHA="$PAYLOAD_HEAD_SHA" + if [ -z "$HEAD_SHA" ]; then + HEAD_SHA=$(git ls-remote \ + "https://x-access-token:$GH_TOKEN@github.com/$REPOSITORY" \ + "refs/pull/$EVENT_ISSUE_NUMBER/head" | cut -f1) || HEAD_SHA="" + fi + if ! [[ "$HEAD_SHA" =~ ^[0-9a-f]{40}$ ]]; then + echo "::error::PR context is unresolved and no head SHA could be recovered." + exit 1 + fi gh api "repos/$REPOSITORY/statuses/$HEAD_SHA" \ -f state=failure \ -f context="$STATUS_CONTEXT" \ From 5199698ad2b08d951c2f9f5a37217b86bd634146 Mon Sep 17 00:00:00 2001 From: Kyleasmth <45497561+Kyleasmth@users.noreply.github.com> Date: Wed, 23 Sep 2026 11:39:43 -0700 Subject: [PATCH 3/4] chore: add an empty changeset for a CI-only change Claude-Session: https://claude.ai/code/session_01Gnt93sVjs6ApXSzkDC2efU --- .changeset/blue-spiders-rescue.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .changeset/blue-spiders-rescue.md diff --git a/.changeset/blue-spiders-rescue.md b/.changeset/blue-spiders-rescue.md new file mode 100644 index 00000000..a845151c --- /dev/null +++ b/.changeset/blue-spiders-rescue.md @@ -0,0 +1,2 @@ +--- +--- From 91a1c8371e443624ecd75703f698d52fd62eb10e Mon Sep 17 00:00:00 2001 From: Kyleasmth <45497561+Kyleasmth@users.noreply.github.com> Date: Thu, 24 Sep 2026 11:10:25 -0700 Subject: [PATCH 4/4] test(ci): assert the unresolved status payload, not just its target commit Claude-Session: https://claude.ai/code/session_01Gnt93sVjs6ApXSzkDC2efU --- .github/scripts/major-release-signoff.test.sh | 34 +++++++++++++++---- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/.github/scripts/major-release-signoff.test.sh b/.github/scripts/major-release-signoff.test.sh index 9cf94b28..90cf61f7 100644 --- a/.github/scripts/major-release-signoff.test.sh +++ b/.github/scripts/major-release-signoff.test.sh @@ -50,7 +50,7 @@ if [[ "$2" == *"/pulls/"* ]]; then if [ "$(wc -l < "$MOCK_PULL_CALLS")" -le "${MOCK_PULL_FAILURES:-0}" ]; then exit 1; fi cat "$MOCK_PR_FILE" elif [[ "$2" == *"/statuses/"* ]]; then - echo "$2" >> "${MOCK_STATUS_CALLS:-/dev/null}" + echo "$*" >> "${MOCK_STATUS_CALLS:-/dev/null}" elif [[ "$2" == *"/compare/"* ]]; then echo called >> "$MOCK_COMPARE_CALLS" if [ "${MOCK_COMPARE_ERROR:-0}" = "1" ]; then exit 1; fi @@ -192,9 +192,12 @@ run_context_retry_case() { fi } +# Asserts the whole request, not just its target. A test that only checks which +# SHA was addressed passes just as happily when the job posts `state=success`, +# which is the one outcome this job exists to prevent. run_unresolved_case() { local name="$1" payload_sha="$2" lsremote_error="$3" expected_sha="$4" - local statuses="$TMP/status-calls" posted + local statuses="$TMP/status-calls" call result=0 : > "$statuses" : > "$TMP/lsremote-calls" env PATH="$TMP/bin:$PATH" \ @@ -203,15 +206,32 @@ run_unresolved_case() { RUN_URL=https://example.invalid/run GH_TOKEN=token \ MOCK_STATUS_CALLS="$statuses" MOCK_LSREMOTE_SHA="$RECOVERED_SHA" \ MOCK_LSREMOTE_ERROR="$lsremote_error" MOCK_LSREMOTE_CALLS="$TMP/lsremote-calls" \ - bash "$TMP/unresolved.sh" >/dev/null 2>&1 || true - posted=$(sed 's|.*/statuses/||' "$statuses" | tr -d '\n') + bash "$TMP/unresolved.sh" >/dev/null 2>&1 || result=$? + call=$(cat "$statuses") + + # No recoverable head: the job must address no status at all rather than guess. if [ -z "$expected_sha" ]; then - [ ! -s "$statuses" ] && posted="" || posted="$(cat "$statuses")" + if [ ! -s "$statuses" ] && [ "$result" -ne 0 ]; then + pass "$name" + else + fail "$name" "expected no status request and a nonzero exit; got '$call' and exit $result" + fi + return fi - if [ "$posted" = "$expected_sha" ]; then + + local problem="" + [[ "$call" == *"/statuses/$expected_sha"* ]] || problem="wrong target SHA" + [[ "$call" == *"state=failure"* ]] || problem="${problem:-status was not failure}" + [[ "$call" == *"context=major-release-signoff"* ]] || problem="${problem:-wrong context}" + [[ "$call" == *"target_url=https://example.invalid/run"* ]] || problem="${problem:-no target_url}" + [[ "$call" == *"Could not resolve PR context"* ]] || problem="${problem:-no description}" + # The job exits nonzero so the check is red in the PR UI, not merely recorded. + [ "$result" -ne 0 ] || problem="${problem:-job exited 0}" + + if [ -z "$problem" ]; then pass "$name" else - fail "$name" "expected a failing status on '$expected_sha'; got '$posted'" + fail "$name" "$problem; call was '$call' (exit $result)" fi }