From 3ea2c90d2324daa4e66a9a7e647a439daa147a69 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Fri, 17 Jul 2026 13:56:18 -0700 Subject: [PATCH] refactor(ci): DRY the retried label-fetch into a composite action Address @ethernet8023's review nits (both flagged non-blocking): - Extract the copy-pasted retried `gh pr view` label-fetch loop (lint.yml ci-review gate + supply-chain-audit.yml mcp-catalog gate) into a single .github/actions/gh-pr-labels composite action. It preserves the exact semantics the reviewer verified: retry on transient API failure, exit 1 after N attempts (re-runnable), and a clean fetch that simply lacks the label reports has-label=false rather than hard-failing. Both gates now consume the has-label output. - Rewrite the Dockerfile playwright retry from the compact `&& break || { ... }` one-liner into the readable multi-line if/sleep form used everywhere else. E2E-verified the composite action against a stubbed gh across four cases: label present -> has-label=true; absent -> has-label=false; API always fails -> exit 1 + annotation, no output written; transient (fail-once) -> recovers and reports true. All workflow/action YAML parses; bash -n clean. --- .github/actions/gh-pr-labels/action.yml | 80 ++++++++++++++++++++++++ .github/workflows/lint.yml | 28 +++------ .github/workflows/supply-chain-audit.yml | 26 +++----- Dockerfile | 9 ++- 4 files changed, 106 insertions(+), 37 deletions(-) create mode 100644 .github/actions/gh-pr-labels/action.yml diff --git a/.github/actions/gh-pr-labels/action.yml b/.github/actions/gh-pr-labels/action.yml new file mode 100644 index 00000000000..ba2aa95cb20 --- /dev/null +++ b/.github/actions/gh-pr-labels/action.yml @@ -0,0 +1,80 @@ +name: Fetch PR labels (retried) +description: >- + Fetch a PR's labels via `gh pr view`, retrying on transient GitHub API + failures so a network blip is never misread as "label absent". Distinguishes + an API failure (hard error after N attempts) from a genuinely-missing label + (clean output that simply doesn't contain the required label), and exposes a + ready-made `has-label` output for the required-label gate. + +inputs: + pr: + description: PR number to read labels from. + required: true + required-label: + description: >- + Label to check for. When set, the `has-label` output is 'true'/'false'. + When empty, only `labels` is populated. + required: false + default: "" + attempts: + description: Max attempts before treating it as a hard API failure. + required: false + default: "3" + delay: + description: Seconds to wait between attempts. + required: false + default: "10" + +outputs: + labels: + description: Newline-separated list of label names on the PR. + value: ${{ steps.fetch.outputs.labels }} + has-label: + description: >- + 'true' if `required-label` is present, 'false' otherwise. Empty when + no `required-label` was supplied. + value: ${{ steps.fetch.outputs.has-label }} + +runs: + using: composite + steps: + - id: fetch + shell: bash + env: + _PR: ${{ inputs.pr }} + _REQUIRED: ${{ inputs.required-label }} + _ATTEMPTS: ${{ inputs.attempts }} + _DELAY: ${{ inputs.delay }} + run: | + set -euo pipefail + # Retry the label fetch: a transient API blip must not read as + # "label absent" (which would hard-fail the required-label gate on a + # PR that actually carries the label). A hard API failure after all + # attempts is a distinct exit-1 (re-runnable); a clean fetch that + # simply lacks the label is a normal 'false'. + labels="" + for i in $(seq 1 "$_ATTEMPTS"); do + if labels=$(gh pr view "$_PR" --json labels --jq '.labels[].name'); then + break + fi + if [ "$i" = "$_ATTEMPTS" ]; then + echo "::error::Could not fetch PR labels after $_ATTEMPTS attempts (GitHub API failure — re-run this job)." + exit 1 + fi + echo "::warning::gh pr view failed (attempt $i); retrying in ${_DELAY}s" + sleep "$_DELAY" + done + + { + echo "labels<<__HERMES_LABELS__" + echo "$labels" + echo "__HERMES_LABELS__" + } >> "$GITHUB_OUTPUT" + + if [ -n "$_REQUIRED" ]; then + if echo "$labels" | grep -Fxq "$_REQUIRED"; then + echo "has-label=true" >> "$GITHUB_OUTPUT" + else + echo "has-label=false" >> "$GITHUB_OUTPUT" + fi + fi diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index e1c42a50e63..86545e03eab 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -177,29 +177,19 @@ jobs: steps: - name: Checkout code uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - - name: Require ci-reviewed label - id: label-check + - name: Fetch ci-reviewed label + id: labels env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + uses: ./.github/actions/gh-pr-labels + with: + pr: ${{ github.event.pull_request.number }} + required-label: ci-reviewed + - name: Require ci-reviewed label + id: label-check run: | set -euo pipefail - PR="${{ github.event.pull_request.number }}" - # Retry the label fetch: a transient API blip must not read as - # "label absent" (which hard-fails the job below on a PR that - # actually carries the label). - LABELS="" - for i in 1 2 3; do - if LABELS=$(gh pr view "$PR" --json labels --jq '.labels[].name'); then - break - fi - if [ "$i" = 3 ]; then - echo "::error::Could not fetch PR labels after 3 attempts (GitHub API failure — re-run this job)." - exit 1 - fi - echo "::warning::gh pr view failed (attempt $i); retrying in 10s" - sleep 10 - done - if echo "$LABELS" | grep -Fxq 'ci-reviewed'; then + if [ "${{ steps.labels.outputs.has-label }}" = "true" ]; then echo "reviewed=true" >> "$GITHUB_OUTPUT" echo "ci-reviewed label present." exit 0 diff --git a/.github/workflows/supply-chain-audit.yml b/.github/workflows/supply-chain-audit.yml index 3dd7888546a..a79d8dea4d5 100644 --- a/.github/workflows/supply-chain-audit.yml +++ b/.github/workflows/supply-chain-audit.yml @@ -238,27 +238,21 @@ jobs: with: fetch-depth: 0 + - name: Fetch mcp-catalog-reviewed label + id: labels + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + uses: ./.github/actions/gh-pr-labels + with: + pr: ${{ github.event.pull_request.number }} + required-label: mcp-catalog-reviewed + - name: Require explicit MCP catalog review label env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | set -euo pipefail - PR="${{ github.event.pull_request.number }}" - # Retry the label fetch so an API blip doesn't read as "label - # absent" and falsely block the PR. - LABELS="" - for i in 1 2 3; do - if LABELS=$(gh pr view "$PR" --json labels --jq '.labels[].name'); then - break - fi - if [ "$i" = 3 ]; then - echo "::error::Could not fetch PR labels after 3 attempts (GitHub API failure — re-run this job)." - exit 1 - fi - echo "::warning::gh pr view failed (attempt $i); retrying in 10s" - sleep 10 - done - if echo "$LABELS" | grep -Fxq 'mcp-catalog-reviewed'; then + if [ "${{ steps.labels.outputs.has-label }}" = "true" ]; then echo "MCP catalog review label present." exit 0 fi diff --git a/Dockerfile b/Dockerfile index 6803adc2e1d..7639db0c4e5 100644 --- a/Dockerfile +++ b/Dockerfile @@ -134,8 +134,13 @@ ENV npm_config_install_links=false RUN npm install --prefer-offline --no-audit --fetch-retries=5 && \ for i in 1 2 3; do \ - npx playwright install --with-deps chromium --only-shell && break || \ - { [ "$i" = 3 ] && exit 1; echo "playwright install failed (attempt $i); retrying in 10s"; sleep 10; }; \ + npx playwright install --with-deps chromium --only-shell && break; \ + if [ "$i" = 3 ]; then \ + echo "playwright install failed after 3 attempts" >&2; \ + exit 1; \ + fi; \ + echo "playwright install failed (attempt $i); retrying in 10s"; \ + sleep 10; \ done && \ npm cache clean --force