From 2395de8c07ca0796633756bbf75872fd2657f8f8 Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Tue, 18 Aug 2026 14:33:32 -0400 Subject: [PATCH 1/2] Reject the Percy build when a host shard failed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A dead shard contributes no snapshots, but finalize still seals the build. On main the project auto-approves without review, so that partial set becomes the baseline every later branch is compared against, and the snapshots the dead shard would have produced resurface as diffs on unrelated PRs. Capture the build id at finalize and reject the build when host-test did not succeed, so an incomplete set cannot become a baseline. percy build:reject authenticates with BrowserStack account credentials rather than the project token — @percy/cli-build's fetchCredentials reads BROWSERSTACK_USERNAME and BROWSERSTACK_ACCESS_KEY. Until those secrets are configured the reject step skips itself and a warning step records that an incomplete build was left to auto-approve, rather than failing the job. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/ci-host.yaml | 52 +++++++++++++++++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci-host.yaml b/.github/workflows/ci-host.yaml index 452d3a5ca21..bcbdd771ebd 100644 --- a/.github/workflows/ci-host.yaml +++ b/.github/workflows/ci-host.yaml @@ -909,18 +909,68 @@ jobs: cancel-in-progress: true needs: [host-test, check-percy] runs-on: ubuntu-latest + # Job-level so the reject step can test for their presence in its `if:` — + # a step's own `env:` block is not in scope for that step's condition. + env: + BROWSERSTACK_USERNAME: ${{ secrets.BROWSERSTACK_USERNAME }} + BROWSERSTACK_ACCESS_KEY: ${{ secrets.BROWSERSTACK_ACCESS_KEY }} steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - uses: ./.github/actions/init - name: Finalise Percy - run: npx percy build:finalize + id: finalize + run: | + set -eo pipefail + npx percy build:finalize 2>&1 | tee /tmp/percy-finalize.log + # Percy build URLs are https://percy.io////builds/. + # Allow 1-3 segments before /builds so a change to that structure + # doesn't break the parse, and tolerate no match — the reject step + # is gated on a non-empty build id anyway. + BUILD_URL=$(grep -oE 'https://percy\.io/[A-Za-z0-9_-]+(/[A-Za-z0-9_-]+){1,3}/builds/[0-9]+' /tmp/percy-finalize.log | tail -1) || true + echo "build_id=${BUILD_URL##*/}" >> "$GITHUB_OUTPUT" + echo "build_url=$BUILD_URL" >> "$GITHUB_OUTPUT" + if [ -z "$BUILD_URL" ]; then + echo "::warning::Could not parse the Percy build id from finalize output; the reject step will be skipped." + fi working-directory: packages/host env: PERCY_TOKEN: ${{ secrets.PERCY_TOKEN_HOST }} PERCY_PARALLEL_NONCE: ${{ github.run_id }}-${{ github.run_attempt }} + # A shard that dies contributes no snapshots, but finalize still seals + # the build. On `main`, where the project's auto-approve-branch-filter + # promotes builds without review, that partial set becomes the baseline + # every later branch is compared against — and the snapshots the dead + # shard would have produced resurface as diffs on unrelated PRs. + # Rejecting the build keeps it from becoming a baseline. + # + # `percy build:reject` authenticates with BrowserStack account + # credentials rather than the project token: @percy/cli-build reads + # BROWSERSTACK_USERNAME and BROWSERSTACK_ACCESS_KEY (see + # fetchCredentials in @percy/cli-build). Until those secrets exist the + # step skips itself rather than failing the job. + - name: Reject the Percy build when a host shard failed + if: >- + needs.host-test.result != 'success' + && steps.finalize.outputs.build_id != '' + && env.BROWSERSTACK_USERNAME != '' + run: | + set -eo pipefail + npx percy build:reject "${{ steps.finalize.outputs.build_id }}" + echo "::warning::Rejected Percy build ${{ steps.finalize.outputs.build_url }} — host-test finished as '${{ needs.host-test.result }}', so its snapshot set is incomplete and must not become a baseline." + working-directory: packages/host + env: + PERCY_TOKEN: ${{ secrets.PERCY_TOKEN_HOST }} + + - name: Note that an incomplete build was left unrejected + if: >- + needs.host-test.result != 'success' + && env.BROWSERSTACK_USERNAME == '' + run: | + echo "::warning::host-test finished as '${{ needs.host-test.result }}', so this Percy build's snapshot set is incomplete, but BROWSERSTACK_USERNAME / BROWSERSTACK_ACCESS_KEY are not configured — the build could not be rejected and may be auto-approved into the baseline." + host-merge-reports-and-publish: name: Merge Host reports and publish if: ${{ !cancelled() && (needs.host-test.result == 'success' || needs.host-test.result == 'failure') }} From 295553461b6081c1801d8177c22796545d55dc89 Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Wed, 19 Aug 2026 11:52:25 -0400 Subject: [PATCH 2/2] Require both BrowserStack credentials before rejecting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit percy build:reject exits non-zero when either the username or the access key is missing, so gating only on the username let a half-configured secret pair run the command and fail the finalize job — on a run that was already red from the shard failure that triggered it. Gate on both, and make the warning step the exact complement of the reject condition so an incomplete build cannot pass through both silently. The warning now reports which of the three preconditions was missing, since a misnamed secret and an unparsable build id produce the same outcome from the outside. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/ci-host.yaml | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci-host.yaml b/.github/workflows/ci-host.yaml index bcbdd771ebd..32eb5d5dabd 100644 --- a/.github/workflows/ci-host.yaml +++ b/.github/workflows/ci-host.yaml @@ -949,13 +949,16 @@ jobs: # `percy build:reject` authenticates with BrowserStack account # credentials rather than the project token: @percy/cli-build reads # BROWSERSTACK_USERNAME and BROWSERSTACK_ACCESS_KEY (see - # fetchCredentials in @percy/cli-build). Until those secrets exist the - # step skips itself rather than failing the job. + # fetchCredentials in @percy/cli-build). Both are required — the CLI + # exits non-zero when either is missing — so the step runs only when + # both are present and otherwise skips itself rather than failing the + # job. - name: Reject the Percy build when a host shard failed if: >- needs.host-test.result != 'success' && steps.finalize.outputs.build_id != '' && env.BROWSERSTACK_USERNAME != '' + && env.BROWSERSTACK_ACCESS_KEY != '' run: | set -eo pipefail npx percy build:reject "${{ steps.finalize.outputs.build_id }}" @@ -964,12 +967,18 @@ jobs: env: PERCY_TOKEN: ${{ secrets.PERCY_TOKEN_HOST }} + # The exact complement of the reject step's condition, so an incomplete + # build never passes through both silently. - name: Note that an incomplete build was left unrejected if: >- needs.host-test.result != 'success' - && env.BROWSERSTACK_USERNAME == '' + && ( + steps.finalize.outputs.build_id == '' + || env.BROWSERSTACK_USERNAME == '' + || env.BROWSERSTACK_ACCESS_KEY == '' + ) run: | - echo "::warning::host-test finished as '${{ needs.host-test.result }}', so this Percy build's snapshot set is incomplete, but BROWSERSTACK_USERNAME / BROWSERSTACK_ACCESS_KEY are not configured — the build could not be rejected and may be auto-approved into the baseline." + echo "::warning::host-test finished as '${{ needs.host-test.result }}', so this Percy build's snapshot set is incomplete, but it could not be rejected and may be auto-approved into the baseline. Percy build id: '${{ steps.finalize.outputs.build_id }}' (empty means the finalize output could not be parsed); BROWSERSTACK_USERNAME set: ${{ env.BROWSERSTACK_USERNAME != '' }}; BROWSERSTACK_ACCESS_KEY set: ${{ env.BROWSERSTACK_ACCESS_KEY != '' }}." host-merge-reports-and-publish: name: Merge Host reports and publish