From fc5b276f7dca329d09dc76f9453bbd7285c11e39 Mon Sep 17 00:00:00 2001 From: lefarcen <935902669@qq.com> Date: Mon, 13 Jul 2026 12:56:12 +0800 Subject: [PATCH] feat(ci): surface merge-queue needs-validation ejection as a PR comment (#5519) A PR ejected from the merge queue by the needs-validation gate leaves no trace on the PR itself: the gate fails on the queue's transient ref, the PR's own checks stay green, and mergeQueueEntry silently goes null. The only way to learn why the PR fell out of the queue is to dig through merge_group workflow runs, which nobody watches (observed on #5329). Producer: the merge_group gate in ci.yml now emits a handoff/comment artifact targeting the labeled PR right before it blocks, and uploads it on the failure path. Consumer: comment.atom.yml accepts merge_group workflow_run events; since a merge_group run's head_sha is the queue's synthetic merge commit, artifacts from those runs are bound to their producing run by run_id, and the base-freshness skip is waived (PRs ahead in the queue move the base while the run completes). The live-PR head check still guards staleness. Topology coverage pins the new boundary: comment.atom consumes pull_request + merge_group; autofix/report stay pull_request-only. --- .github/workflows/ci.yml | 65 +++++++++++++++++++++++ .github/workflows/comment.atom.yml | 22 ++++++-- e2e/tests/packaged-smoke-workflow.test.ts | 30 ++++++++++- 3 files changed, 113 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d03a07a6b3..e88dfdcf9e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -919,6 +919,17 @@ jobs: timeout-minutes: 5 steps: + # The needs-validation gate below produces a `handoff/comment` artifact when it ejects a + # queued PR, and that production goes through `.github/scripts/handoff.py` (the only + # sanctioned source of handoff names and layout). Only the merge_group context can eject, + # so pull_request runs skip the checkout entirely. + - name: Checkout handoff helper + if: ${{ github.event_name == 'merge_group' }} + uses: actions/checkout@v6.0.2 + with: + sparse-checkout: .github + sparse-checkout-cone-mode: true + - name: Check workspace validation jobs env: NEEDS_JSON: ${{ toJSON(needs) }} @@ -967,6 +978,7 @@ jobs: fi - name: Block merge while the needs-validation label is present + id: needs_validation_gate # Hard gate: a PR that still carries `needs-validation` must not merge. Enforced ONLY in # the merge_group (merge-queue) context — never on pull_request. main requires the merge # queue, so every merge passes through merge_group; a needs-validation entry fails this @@ -992,11 +1004,52 @@ jobs: MERGE_GROUP_REF: ${{ github.event.merge_group.head_ref }} BASE_SHA: ${{ github.event.merge_group.base_sha }} HEAD_SHA: ${{ github.event.merge_group.head_sha }} + RUN_ID: ${{ github.run_id }} run: | set -euo pipefail if [ "$EVENT_NAME" != "merge_group" ]; then echo "needs-validation is gated at merge time in the merge_group context; nothing to enforce on $EVENT_NAME (PR check stays green)." else + # An ejection is invisible from the PR: this failure runs on the queue's transient + # ref, the PR's own checks stay green, and `mergeQueueEntry` just goes null. Leave a + # visible trace by producing a `handoff/comment` artifact for the labeled PR; the + # trusted `comment.atom.yml` consumer upserts it as a PR comment after this run + # completes. Best-effort: a handoff failure must never soften the block itself. + emit_ejection_notice() { + local pr="$1" + local pr_json pr_head pr_base handoff_id handoff_root handoff_dir marker + pr_json="$(gh api "repos/$REPO/pulls/$pr")" + pr_head="$(jq -r '.head.sha' <<< "$pr_json")" + pr_base="$(jq -r '.base.sha' <<< "$pr_json")" + handoff_id="needs-validation-pr-$pr" + handoff_root="$RUNNER_TEMP/handoff-comment-$handoff_id" + handoff_dir="$(python3 .github/scripts/handoff.py dir comment "$handoff_id" --root "$handoff_root")" + mkdir -p "$handoff_dir" + marker="" + { + printf '%s\n' "$marker" + printf 'Ejected from the merge queue: this PR still carries the `needs-validation` label.\n\n' + printf 'The merge queue gate ([run %s](%s/%s/actions/runs/%s)) blocked the queued group because of the label. That failure runs on the queue'"'"'s transient ref, so it never appears in this PR'"'"'s own checks — they stay green, and this notice is the only visible trace on the PR.\n\n' "$RUN_ID" "$GITHUB_SERVER_URL" "$REPO" "$RUN_ID" + printf 'To land this PR: complete the QA pass the label is tracking, remove the `needs-validation` label, then add the PR back to the merge queue.\n' + } > "$handoff_dir/body.md" + jq -n \ + --arg kind "comment" \ + --arg id "$handoff_id" \ + --arg head "$pr_head" \ + --arg base "$pr_base" \ + --arg marker "$marker" \ + --argjson schema_version 1 \ + --argjson pr_number "$pr" \ + --argjson run_id "$RUN_ID" \ + '{schema_version: $schema_version, kind: $kind, id: $id, pr_number: $pr_number, head_sha: $head, base_sha: $base, run_id: $run_id, marker: $marker}' \ + > "$handoff_dir/metadata.json" + python3 .github/scripts/handoff.py validate comment "$handoff_dir" >/dev/null + { + echo "comment_created=true" + echo "comment_name=$(python3 .github/scripts/handoff.py artifact-name comment "$handoff_id")" + echo "comment_path=$handoff_root" + } >> "$GITHUB_OUTPUT" + } # Resolve EVERY merge-group commit to at least one real PR and check its labels. # Each queued entry can be recovered from up to three signals: # 1. ref `.../pr--` — names the (tip) entry only; squash-safe. @@ -1043,6 +1096,9 @@ jobs: case " $checked " in *" $pr "*) ;; *) checked="$checked $pr";; esac if printf '%s\n' "$labels" | grep -qx 'needs-validation'; then echo "::error::PR #$pr still has 'needs-validation' — blocking merge." + # Fail-fast keeps this per-run notice on the FIRST labeled PR found; every + # labeled entry also fails its own queue run, so each still gets its own notice. + emit_ejection_notice "$pr" || echo "::warning::could not produce the ejection-notice handoff for PR #$pr; the block itself still stands." exit 1 fi echo "PR #$pr: no needs-validation label." @@ -1055,6 +1111,15 @@ jobs: echo "No 'needs-validation' label in the queued group ($checked ) — clear to merge." fi + # `failure()` is required: the gate exits 1 on the very path that produces the handoff, + # and the default `success()` condition would skip this upload exactly when it matters. + - name: Upload merge-queue ejection notice handoff + if: ${{ failure() && steps.needs_validation_gate.outputs.comment_created == 'true' }} + uses: actions/upload-artifact@v4 + with: + name: ${{ steps.needs_validation_gate.outputs.comment_name }} + path: ${{ steps.needs_validation_gate.outputs.comment_path }} + runtime_summary: name: Runtime summary needs: diff --git a/.github/workflows/comment.atom.yml b/.github/workflows/comment.atom.yml index 6c3564df83..d9d1a90776 100644 --- a/.github/workflows/comment.atom.yml +++ b/.github/workflows/comment.atom.yml @@ -18,7 +18,9 @@ concurrency: jobs: comment: name: Publish handoff comments - if: ${{ github.repository == 'nexu-io/open-design' && github.event.workflow_run.event == 'pull_request' }} + # merge_group is consumed alongside pull_request so the needs-validation gate can leave an + # ejection notice on the queued PR (the gate failure itself is invisible from the PR head). + if: ${{ github.repository == 'nexu-io/open-design' && (github.event.workflow_run.event == 'pull_request' || github.event.workflow_run.event == 'merge_group') }} runs-on: ubuntu-24.04 timeout-minutes: 10 @@ -74,6 +76,8 @@ jobs: GH_TOKEN: ${{ github.token }} REPO: ${{ github.repository }} RUN_HEAD_SHA: ${{ github.event.workflow_run.head_sha }} + RUN_EVENT: ${{ github.event.workflow_run.event }} + RUN_ID: ${{ github.event.workflow_run.id }} shell: bash run: | set -euo pipefail @@ -88,7 +92,16 @@ jobs: marker="$(jq -r '.marker' <<< "$entry_json")" body_path="$(jq -r '.body_path' <<< "$entry_json")" - if [ "$head_sha" != "$RUN_HEAD_SHA" ]; then + if [ "$RUN_EVENT" = "merge_group" ]; then + # A merge_group run's head_sha is the queue's synthetic merge commit, never the PR + # head, so bind the artifact to its producing run by run_id instead; the live-PR + # head check below still carries the staleness guarantee. + artifact_run_id="$(jq -r '.run_id' <<< "$entry_json")" + if [ "$artifact_run_id" != "$RUN_ID" ]; then + echo "Skipping comment handoff $id because artifact run $artifact_run_id does not match workflow_run $RUN_ID." + continue + fi + elif [ "$head_sha" != "$RUN_HEAD_SHA" ]; then echo "Skipping comment handoff $id because artifact head $head_sha does not match workflow_run head $RUN_HEAD_SHA." continue fi @@ -110,7 +123,10 @@ jobs: echo "Skipping stale comment handoff $id for $head_sha; current PR head is $current_head." continue fi - if [ "$current_base" != "$base_sha" ]; then + # Base freshness is meaningless for a merge-queue ejection notice: PRs ahead in the + # queue merge into the base branch while this run completes, so the base is EXPECTED + # to move; the notice stays valid as long as the PR head is unchanged (checked above). + if [ "$RUN_EVENT" != "merge_group" ] && [ "$current_base" != "$base_sha" ]; then echo "Skipping stale comment handoff $id for base $base_sha; current PR base is $current_base." continue fi diff --git a/e2e/tests/packaged-smoke-workflow.test.ts b/e2e/tests/packaged-smoke-workflow.test.ts index e4d6237eba..a8e89a295c 100644 --- a/e2e/tests/packaged-smoke-workflow.test.ts +++ b/e2e/tests/packaged-smoke-workflow.test.ts @@ -343,12 +343,40 @@ describe("packaged smoke workflow", () => { expect(dockerTrigger).not.toContain("branches: [main]"); expect(dockerTrigger).not.toContain("- main"); expect(commentWorkflow).toContain("workflows: [ci]"); - expect(commentWorkflow).toContain("github.event.workflow_run.event == 'pull_request'"); + // comment.atom consumes merge_group runs too, so the needs-validation gate can surface a + // queue-ejection notice on the PR; autofix/report stay pull_request-only trusted consumers. + expect(commentWorkflow).toContain( + "(github.event.workflow_run.event == 'pull_request' || github.event.workflow_run.event == 'merge_group')", + ); expect(autofixWorkflow).toContain("workflows: [ci]"); expect(autofixWorkflow).toContain("github.event.workflow_run.event == 'pull_request'"); + expect(autofixWorkflow).not.toContain("merge_group"); expect(autofixWorkflow).not.toContain("ci-nix"); expect(reportWorkflow).toContain("workflows: [ci]"); expect(reportWorkflow).toContain("github.event.workflow_run.event == 'pull_request'"); + expect(reportWorkflow).not.toContain("merge_group"); + }); + + it("[P2] surfaces a merge-queue needs-validation ejection as a PR comment handoff", async () => { + const [ciWorkflow, commentWorkflow] = await Promise.all([ + readFile(ciWorkflowPath, "utf8"), + readFile(commentWorkflowPath, "utf8"), + ]); + + // Producer: the merge_group gate emits a handoff/comment artifact for the labeled PR when + // it blocks, and uploads it on the failure path (the gate exits 1 exactly when it produces). + expect(ciWorkflow).toContain(""); + expect(ciWorkflow).toContain("emit_ejection_notice"); + expect(ciWorkflow).toContain( + "if: ${{ failure() && steps.needs_validation_gate.outputs.comment_created == 'true' }}", + ); + + // Consumer: a merge_group run's head_sha is the queue's synthetic merge commit, so the atom + // binds merge_group artifacts to their producing run by run_id and skips the base-freshness + // check (PRs ahead in the queue move the base while the run completes). + expect(commentWorkflow).toContain('"$RUN_EVENT" = "merge_group"'); + expect(commentWorkflow).toContain('"$artifact_run_id" != "$RUN_ID"'); + expect(commentWorkflow).toContain('[ "$RUN_EVENT" != "merge_group" ] && [ "$current_base" != "$base_sha" ]'); }); it("[P2] gates the backport auto-merge follow-up as a trusted workflow_run consumer", async () => {