diff --git a/.github/workflows/auto-update-pr-branches.yml b/.github/workflows/auto-update-pr-branches.yml index 6327e35d1f..5cfd50baff 100644 --- a/.github/workflows/auto-update-pr-branches.yml +++ b/.github/workflows/auto-update-pr-branches.yml @@ -2,12 +2,17 @@ name: "Auto-update PR branches" run-name: "Auto-update PR branches after push to ${{ github.ref_name }}" # When `main` advances, update every open PR that has auto-merge enabled -# and is BEHIND. Without this, the strict required-status-checks policy -# (`strict_required_status_checks_policy: true` on the `main` ruleset) -# blocks the auto-merge button forever — auto-merge fires only when checks -# are green AND the head is up-to-date with the base, but it does not -# update the branch on its own. This workflow closes that gap so polish / -# impl PRs squash-merge themselves end-to-end. +# and is BEHIND, so polish / impl PRs squash-merge themselves end-to-end +# without manual "Update branch" clicks. +# +# NOTE (verified 2026-07-24): the `main` ruleset currently sets +# `strict_required_status_checks_policy: false`, so an up-to-date head is +# NOT actually required to merge. This file used to claim the opposite. +# That means every update-branch call is optional work which also +# invalidates the green checks on the previous head and forces a full CI +# re-run. Retiring this workflow is worth evaluating — see #9772; it is +# kept for now because the ruleset could be tightened again, and because +# behaviour changes here affect every auto-merge PR in the repo. # # Why we don't use GitHub's native merge queue: this repository is # user-owned and merge_queue rules are not available on the account's @@ -62,9 +67,42 @@ jobs: --limit 200 \ --json number,title,headRefName,autoMergeRequest) + # Skip Dependabot branches. Updating them here is actively + # harmful: the update-branch merge commit is authored by + # github-actions[bot], and GitHub gates every workflow run + # triggered by that push behind manual approval + # ("action_required"). Because `main` advances every few minutes + # while the impl pipeline merges, the branch is re-updated long + # before anyone can approve — so CI never completes and + # auto-merge can never fire. PR #9674 accumulated 174 runs in + # 22 h that way: 162 action_required (github-actions[bot]) vs. + # only 4 green (dependabot[bot], from the original push). + # + # Leaving them alone is safe because the `main` ruleset is NOT + # strict (see the header) — a Dependabot PR merges while behind, + # so it never needs this update at all. Do NOT justify this by + # assuming Dependabot re-syncs them for us: it rebases on + # conflict / schedule / reopen, not on "merely behind", and it + # stops rebasing a branch entirely once a foreign commit lands on + # it. The point is that the head now stays put, so a branch this + # workflow already touched needs its gated run approved exactly + # once instead of being re-gated every few minutes. CANDIDATES=$(echo "$PRS" | jq -c ' - [ .[] | select(.autoMergeRequest != null) ] + [ .[] + | select(.autoMergeRequest != null) + | select(.headRefName | startswith("dependabot/") | not) + ] + ') + + SKIPPED=$(echo "$PRS" | jq ' + [ .[] + | select(.autoMergeRequest != null) + | select(.headRefName | startswith("dependabot/")) + ] | length ') + if [[ "$SKIPPED" -gt 0 ]]; then + echo "::notice::Skipping $SKIPPED Dependabot PR(s) — updating them gates their CI behind manual approval, and the non-strict ruleset means being behind does not block the merge (see comment above)" + fi COUNT=$(echo "$CANDIDATES" | jq 'length') echo "::notice::Found $COUNT open PR(s) with auto-merge enabled" diff --git a/.github/workflows/impl-merge.yml b/.github/workflows/impl-merge.yml index 5f450646b8..0ff40d12c0 100644 --- a/.github/workflows/impl-merge.yml +++ b/.github/workflows/impl-merge.yml @@ -47,7 +47,29 @@ jobs: run: | if [[ "$EVENT_NAME" == "workflow_dispatch" ]]; then PR_NUM="$INPUT_PR_NUMBER" - PR_DATA=$(gh pr view "$PR_NUM" --repo "$GH_REPO" --json headRefName,labels) + # Retry gh pr view — an unretried call here stranded an already + # ai-approved PR (#9742) when the GitHub GraphQL API returned + # HTTP 504 during the 2026-07-24 API brown-out. `bash -e` turned + # the blip into a permanently unmerged PR. Same 3x/linear-backoff + # shape as impl-review.yml's "Extract PR info". + PR_DATA="" + for attempt in 1 2 3; do + if gh pr view "$PR_NUM" --repo "$GH_REPO" --json headRefName,labels > /tmp/pr.json 2> /tmp/pr.err; then + PR_DATA=$(cat /tmp/pr.json) + break + fi + # Flattened: `::warning::` is line-oriented, and gh's HTTP errors + # are routinely multi-line — the tail would spill into the raw log. + PERR=$(head -c 500 /tmp/pr.err | tr '\n' ' ') + echo "::warning::gh pr view failed (attempt ${attempt}/3): ${PERR}" + if [ "$attempt" -lt 3 ]; then + sleep $((attempt * 5)) + fi + done + if [ -z "$PR_DATA" ]; then + echo "::error::gh pr view failed after 3 attempts for PR #${PR_NUM}" + exit 1 + fi BRANCH=$(echo "$PR_DATA" | jq -r '.headRefName') HAS_APPROVED=$(echo "$PR_DATA" | jq -r '[.labels[].name] | any(. == "ai-approved")') else @@ -224,6 +246,39 @@ jobs: for attempt in $(seq 1 $MAX_ATTEMPTS); do echo "::notice::Merge attempt $attempt/$MAX_ATTEMPTS" + # Re-read state first so a lost response is a no-op rather than a + # half-finished merge: `gh pr merge` can succeed server-side and + # still return 5xx, after which every later attempt fails with + # "already merged" and the step exits 1. Because all post-merge + # steps are gated on `should_run == 'true'` (implicit success()), + # that skipped GCS promotion, the impl:{lib}:done label, closing + # the issue and the Postgres sync — a merged PR with none of the + # bookkeeping, exactly the silent partial completion CLAUDE.md + # warns about for manually merged PRs. + # The state read is itself retried: a blip here would fall through + # to another `gh pr merge` on an already-merged PR and re-create the + # very failure mode this check exists to prevent — under exactly the + # brown-out conditions that motivate it. + STATE="" + for probe in 1 2 3; do + if STATE=$(gh pr view "$PR_NUM" --repo "$REPOSITORY" --json state -q .state 2> /tmp/state.err); then + break + fi + SERR=$(head -c 500 /tmp/state.err | tr '\n' ' ') + echo "::warning::gh pr view (state) failed (probe ${probe}/3): ${SERR}" + STATE="" + if [ "$probe" -lt 3 ]; then + sleep $((probe * 5)) + fi + done + if [ -z "$STATE" ]; then + echo "::warning::Could not read state for PR #${PR_NUM} — proceeding to merge attempt $attempt" + fi + if [ "$STATE" = "MERGED" ]; then + echo "::notice::PR #${PR_NUM} is already merged — continuing to post-merge steps" + exit 0 + fi + # Update branch before merge attempt gh pr update-branch "$PR_NUM" --repo "$REPOSITORY" 2>/dev/null || true sleep 2 @@ -241,14 +296,25 @@ jobs: --repo "$REPOSITORY" \ --squash \ --admin \ - --delete-branch; then + --delete-branch 2> /tmp/merge.err; then echo "::notice::Merge successful on attempt $attempt" exit 0 fi + # Second line of defence for the same lost-response case: if the + # state probe above is ALSO failing (same brown-out), the merge can + # already be done while every retry here reports "already merged" + # until the loop exits 1 and the post-merge bookkeeping is skipped. + # gh saying the PR is already merged IS the success signal. + MERR=$(head -c 500 /tmp/merge.err | tr '\n' ' ') + if grep -qiE 'already merged|pull request is (already )?(merged|closed)' /tmp/merge.err; then + echo "::notice::PR #${PR_NUM} reported as already merged — continuing to post-merge steps" + exit 0 + fi + if [ $attempt -lt $MAX_ATTEMPTS ]; then DELAY=$((attempt * 10)) - echo "::warning::Merge failed, retrying in ${DELAY}s..." + echo "::warning::Merge failed (${MERR}), retrying in ${DELAY}s..." sleep $DELAY fi done diff --git a/.github/workflows/impl-review.yml b/.github/workflows/impl-review.yml index 27b9175894..18f2c83d63 100644 --- a/.github/workflows/impl-review.yml +++ b/.github/workflows/impl-review.yml @@ -62,7 +62,8 @@ jobs: PR_DATA=$(cat /tmp/pr.json) break fi - echo "::warning::gh pr view failed (attempt ${attempt}/3): $(cat /tmp/pr.err)" + PERR=$(head -c 500 /tmp/pr.err | tr '\n' ' ') + echo "::warning::gh pr view failed (attempt ${attempt}/3): ${PERR}" if [ "$attempt" -lt 3 ]; then sleep $((attempt * 5)) fi @@ -850,12 +851,58 @@ jobs: THRESHOLD=$((90 - ATTEMPT_COUNT * 10)) if [ "$THRESHOLD" -lt 50 ]; then THRESHOLD=50; fi - # Check if verdict label was already added by earlier step - CURRENT_LABELS=$(gh pr view "$PR_NUM" --json labels -q '.labels[].name' 2>/dev/null || echo "") + # This step is the pipeline's only hand-off point: every downstream + # workflow (merge, repair) starts from a call made right here. An + # unretried blip therefore does not just fail a step — it strands the + # PR forever with a verdict label and nothing listening. That is + # exactly what the 2026-07-24 GitHub API brown-out did to four + # radar-basic PRs (HTTP 502 on the dispatch endpoint, HTTP 504 on + # GraphQL). Every API call below is retried 3x with linear backoff. + # `::warning::` is line-oriented, so multi-line gh stderr would leak + # everything after the first newline into the raw log — flatten it. + gh_retry() { + local what="$1"; shift + local attempt err + for attempt in 1 2 3; do + if "$@" 2> /tmp/gh_retry.err; then + return 0 + fi + err=$(head -c 500 /tmp/gh_retry.err | tr '\n' ' ') + echo "::warning::${what} failed (attempt ${attempt}/3): ${err}" + if [ "$attempt" -lt 3 ]; then + sleep $((attempt * 5)) + fi + done + echo "::error::${what} failed after 3 attempts for PR #${PR_NUM}" + return 1 + } + + # Check if verdict label was already added by earlier step. + # Track read success explicitly rather than inferring it from an + # empty result: a failed read and a genuinely label-less PR used to + # be indistinguishable, and both silently skipped the two branches + # below, dropping the PR out of the pipeline with no error at all. + CURRENT_LABELS="" + READ_OK=0 + for attempt in 1 2 3; do + if CURRENT_LABELS=$(gh pr view "$PR_NUM" --json labels -q '.labels[].name' 2> /tmp/labels.err); then + READ_OK=1 + break + fi + LERR=$(head -c 500 /tmp/labels.err | tr '\n' ' ') + echo "::warning::gh pr view (labels) failed (attempt ${attempt}/3): ${LERR}" + if [ "$attempt" -lt 3 ]; then + sleep $((attempt * 5)) + fi + done + if [ "$READ_OK" -ne 1 ]; then + echo "::error::Could not read labels for PR #${PR_NUM} after 3 attempts — refusing to decide blind" + exit 1 + fi if echo "$CURRENT_LABELS" | grep -q "ai-approved"; then echo "::notice::ai-approved label already set (Score $SCORE >= $THRESHOLD), triggering merge" - gh workflow run impl-merge.yml -f pr_number="$PR_NUM" + gh_retry "impl-merge dispatch" gh workflow run impl-merge.yml -f pr_number="$PR_NUM" exit 0 fi @@ -863,19 +910,62 @@ jobs: # Still have repair attempts left? (Max 4 repairs = 5 reviews total) if [ "$ATTEMPT_COUNT" -lt 4 ]; then echo "::notice::ai-rejected label set (Score $SCORE < $THRESHOLD), triggering repair attempt $((ATTEMPT_COUNT + 1))" - gh pr edit "$PR_NUM" --add-label "ai-attempt-${ATTEMPT}" 2>/dev/null || true - gh workflow run impl-repair.yml \ + + # Dispatch BEFORE labelling, and never let the label gate the + # dispatch: impl-repair.yml is workflow_dispatch-only, so this + # call is the single thing that keeps the PR moving. Losing the + # attempt label costs one over-strict review; losing the dispatch + # costs the whole PR. + DISPATCHED=0 + if gh_retry "impl-repair dispatch" gh workflow run impl-repair.yml \ -f pr_number="$PR_NUM" \ -f specification_id="$SPEC_ID" \ -f library="$LIBRARY" \ -f attempt="$ATTEMPT" \ - -f model="$MODEL" + -f model="$MODEL"; then + DISPATCHED=1 + fi + + # `ai-attempt-4` was never created in this repo, so on the 4th + # repair the add fails deterministically no matter how often it + # is retried (PRs #7268/#7039 reached "Repair Attempt 4/4" + # carrying only ai-attempt-1..3 — the old `|| true` hid it). + gh label create "ai-attempt-${ATTEMPT}" --color "fbca04" \ + --description "Repair attempt ${ATTEMPT}" 2>/dev/null || true + # Non-fatal, but loud: without it the next review resets to the + # attempt-0 threshold (>= 90) and the PR re-fails a bar it has + # already cleared. PR #9744 lost it exactly this way on 2026-07-24. + gh_retry "ai-attempt-${ATTEMPT} label" gh pr edit "$PR_NUM" --add-label "ai-attempt-${ATTEMPT}" \ + || echo "::error::attempt label lost — the next review will apply the stricter attempt-0 threshold" + + if [ "$DISPATCHED" -ne 1 ]; then + # Hand the PR to the watchdog rather than leaving it dead. + # `ai-rejected` + `ai-attempt-N` matches NO watchdog case + # (Case 2 excludes any verdict label, Case 4 excludes PRs that + # already have an attempt label), so dropping the verdict puts + # it squarely in Case 2, which re-dispatches impl-repair at the + # attempt number the label still encodes. + # Retried too: this removal IS the hand-off to the watchdog, so + # letting the same brown-out swallow it would leave the PR in + # exactly the stranded state it is meant to escape. + gh_retry "ai-rejected removal" gh pr edit "$PR_NUM" --remove-label "ai-rejected" \ + || echo "::error::could not drop ai-rejected — PR #${PR_NUM} is stranded (ai-rejected + ai-attempt-N matches no watchdog case) and needs manual attention" + echo "::error::impl-repair dispatch failed after 3 attempts — verdict label dropped so watchdog Case 2 picks PR #${PR_NUM} up" + exit 1 + fi else # All 4 repair attempts exhausted echo "::notice::All 4 repair attempts exhausted (Score $SCORE < 50)" gh pr edit "$PR_NUM" --add-label "quality-poor" - gh pr comment "$PR_NUM" --body "$(cat <<'EOF' + # Unquoted delimiter so $SCORE/$LIBRARY/$REPOSITORY/$RUN_ID expand + # — with <<'EOF' this comment rendered a literal "$SCORE/100". + # The markdown code span's backticks MUST stay escaped: in an + # unquoted heredoc they would otherwise run as command + # substitution. Expanding the variables is safe even though + # $LIBRARY derives from a branch name, because bash does not + # re-evaluate substituted values. + gh pr comment "$PR_NUM" --body "$(cat < --auto --squash`. - Checks pending on an up-to-date branch → enable auto-merge anyway; it fires when green. - - **BEHIND branch**: update it via `gh api -X PUT repos/{owner}/{repo}/pulls//update-branch` - (the installed `gh` has no `pr update-branch` subcommand). **Gotcha:** branch updates pushed - with `GITHUB_TOKEN` (incl. `auto-update-pr-branches.yml`) do NOT trigger required checks — - if checks stay `expected`/missing after the update, disable auto-merge, then - `gh pr close && gh pr reopen ` to re-trigger them. + - **BEHIND branch**: leave it alone. BEHIND does not block the merge — the `main` ruleset is + not strict (`strict_required_status_checks_policy: false`), so auto-merge fires on a behind + branch. **Never** run `gh api -X PUT .../pulls//update-branch` on a Dependabot PR: the + resulting merge commit is authored by `github-actions[bot]`, GitHub then gates that head's + workflow runs behind manual approval (`action_required`, so the checks never report), and + Dependabot permanently stops rebasing a branch once a foreign commit lands on it. PR #9674 + burned 174 runs in 22 h this way. `auto-update-pr-branches.yml` skips Dependabot branches + for the same reason (#9772). + - **Already poisoned** (head commit authored by `github-actions[bot]`, checks stuck at + `action_required`): approve the gated runs once — + `gh api -X POST repos/{owner}/{repo}/actions/runs//approve`. Approved runs do report + the required contexts on a bot-authored head (verified on #9674), and since + `auto-update-pr-branches.yml` no longer touches these branches the head now stays put, so + one approval is enough. `@dependabot recreate` also works and additionally restores a + clean `dependabot[bot]` head, but it re-resolves the versions. - `mergeStateStatus` is computed async — after any update, poll it a few seconds until it stabilizes before deciding (UNKNOWN → BEHIND/CLEAN/BLOCKED). 3. A dep bump breaking tests/config (e.g. a major with changed exports): consult **Context7**