From 1b1e3d89808dc08d6d4c47fc18815ea67a0ed1fa Mon Sep 17 00:00:00 2001 From: Cindy Zhang Date: Wed, 26 Aug 2026 22:27:57 -0700 Subject: [PATCH] fix(ci): preserve trusted visual run identity --- .../scripts/visual-gate/gate-context.test.mjs | 95 +++++++++++++++++++ .github/scripts/visual-gate/gate.mjs | 11 ++- .../visual-gate/workflow-concurrency.test.mjs | 29 ++++++ .github/workflows/pr-comment.yml | 4 +- .github/workflows/visual-acceptance.yml | 4 +- 5 files changed, 135 insertions(+), 8 deletions(-) create mode 100644 .github/scripts/visual-gate/gate-context.test.mjs diff --git a/.github/scripts/visual-gate/gate-context.test.mjs b/.github/scripts/visual-gate/gate-context.test.mjs new file mode 100644 index 0000000000000..177770e759782 --- /dev/null +++ b/.github/scripts/visual-gate/gate-context.test.mjs @@ -0,0 +1,95 @@ +// Copyright (c) Meta Platforms, Inc. and affiliates. + +import {execFileSync} from 'node:child_process'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import {fileURLToPath} from 'node:url'; + +import {afterEach, beforeEach, describe, expect, it} from 'vitest'; + +const SCRIPT = path.join( + path.dirname(fileURLToPath(import.meta.url)), + 'gate.mjs', +); +const IDENTITY_ENV = [ + 'ASTRYX_VISUAL_RUN_ID', + 'ASTRYX_VISUAL_RUN_ATTEMPT', + 'GITHUB_RUN_ID', + 'GITHUB_RUN_ATTEMPT', +]; + +let root; +let plan; +let output; + +function captureContext(overrides = {}) { + const env = {...process.env}; + for (const name of IDENTITY_ENV) delete env[name]; + Object.assign(env, overrides); + + execFileSync( + process.execPath, + [SCRIPT, 'check', '--plan-file', plan, '--max-shots', '0', '--out', output], + {env, encoding: 'utf8'}, + ); + return JSON.parse(fs.readFileSync(path.join(output, 'verdict.json'), 'utf8')) + .context; +} + +beforeEach(() => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'visual-gate-context-')); + plan = path.join(root, 'plan.json'); + output = path.join(root, 'output'); + fs.writeFileSync( + plan, + JSON.stringify([ + { + key: 'core-button--default__neutral-light', + storyId: 'core-button--default', + theme: 'neutral', + mode: 'light', + reasons: ['trusted:pr-scope'], + }, + ]), + ); +}); + +afterEach(() => fs.rmSync(root, {recursive: true, force: true})); + +describe('visual gate capture identity', () => { + it('records the GitHub run identity by default', () => { + expect( + captureContext({GITHUB_RUN_ID: '123', GITHUB_RUN_ATTEMPT: '2'}), + ).toMatchObject({ + runId: '123', + runAttempt: '2', + }); + }); + + it('records an explicit trusted recapture identity ahead of workflow defaults', () => { + expect( + captureContext({ + GITHUB_RUN_ID: '999', + GITHUB_RUN_ATTEMPT: '4', + ASTRYX_VISUAL_RUN_ID: '123', + ASTRYX_VISUAL_RUN_ATTEMPT: '2', + }), + ).toMatchObject({runId: '123', runAttempt: '2'}); + }); + + it('records missing identity as null', () => { + expect(captureContext()).toMatchObject({runId: null, runAttempt: null}); + }); + + it('preserves invalid explicit identity for downstream rejection', () => { + expect( + captureContext({ + GITHUB_RUN_ID: '999', + GITHUB_RUN_ATTEMPT: '4', + ASTRYX_VISUAL_RUN_ID: '', + ASTRYX_VISUAL_RUN_ATTEMPT: 'invalid', + }), + ).toMatchObject({runId: '', runAttempt: 'invalid'}); + }); +}); diff --git a/.github/scripts/visual-gate/gate.mjs b/.github/scripts/visual-gate/gate.mjs index c28d637b275f8..1c29d52ff8a76 100644 --- a/.github/scripts/visual-gate/gate.mjs +++ b/.github/scripts/visual-gate/gate.mjs @@ -47,6 +47,11 @@ const flag = name => { return index === -1 ? null : argv[index + 1]; }; const has = name => argv.includes(`--${name}`); +const captureRunIdentity = () => ({ + runId: process.env.ASTRYX_VISUAL_RUN_ID ?? process.env.GITHUB_RUN_ID ?? null, + runAttempt: + process.env.ASTRYX_VISUAL_RUN_ATTEMPT ?? process.env.GITHUB_RUN_ATTEMPT ?? null, +}); const config = loadConfig(REPO_ROOT); const storybookDir = path.resolve(flag('storybook-dir') ?? 'apps/storybook/dist'); @@ -201,8 +206,7 @@ async function runCapture(shots) { headSha: process.env.ASTRYX_PR_HEAD_SHA ?? null, baseSha: process.env.ASTRYX_PR_BASE_SHA ?? null, ref: process.env.GITHUB_REF ?? null, - runId: process.env.GITHUB_RUN_ID ?? null, - runAttempt: process.env.GITHUB_RUN_ATTEMPT ?? null, + ...captureRunIdentity(), }; fs.writeFileSync( path.join(outDir, 'manifest.json'), @@ -242,8 +246,7 @@ async function check() { headSha: process.env.ASTRYX_PR_HEAD_SHA ?? null, baseSha: process.env.ASTRYX_PR_BASE_SHA ?? null, ref: process.env.GITHUB_REF ?? null, - runId: process.env.GITHUB_RUN_ID ?? null, - runAttempt: process.env.GITHUB_RUN_ATTEMPT ?? null, + ...captureRunIdentity(), tiers, scoped: components.length > 0, components, diff --git a/.github/scripts/visual-gate/workflow-concurrency.test.mjs b/.github/scripts/visual-gate/workflow-concurrency.test.mjs index 5438e7588aee8..49ade917b241c 100644 --- a/.github/scripts/visual-gate/workflow-concurrency.test.mjs +++ b/.github/scripts/visual-gate/workflow-concurrency.test.mjs @@ -58,6 +58,18 @@ describe('visual acceptance workflow concurrency', () => { expect(accept).toContain('cancel-in-progress: false'); }); + it('grants PR mutation permission wherever labels and comments are projected', () => { + const value = workflow('visual-acceptance.yml'); + const initialize = value.slice( + value.indexOf(' initialize:'), + value.indexOf(' authorize:'), + ); + const accept = value.slice(value.indexOf(' accept:')); + + expect(initialize).toContain('pull-requests: write'); + expect(accept).toContain('pull-requests: write'); + }); + it('uses the same head identity for post-merge promotion', () => { const value = workflow('visual-acceptance-promote.yml'); @@ -66,4 +78,21 @@ describe('visual acceptance workflow concurrency', () => { ); expect(value).not.toContain('visual-acceptance-pr-'); }); + + it('passes triggering CI identity through dedicated capture variables', () => { + const value = workflow('pr-comment.yml'); + const capture = value.slice( + value.indexOf(' - name: Capture the trusted stable visual scope'), + value.indexOf(' # The Storybook bundle is untrusted.'), + ); + + expect(capture).toContain( + 'ASTRYX_VISUAL_RUN_ID: ${{ steps.identity.outputs.run_id }}', + ); + expect(capture).toContain( + 'ASTRYX_VISUAL_RUN_ATTEMPT: ${{ steps.identity.outputs.run_attempt }}', + ); + expect(capture).not.toContain('GITHUB_RUN_ID:'); + expect(capture).not.toContain('GITHUB_RUN_ATTEMPT:'); + }); }); diff --git a/.github/workflows/pr-comment.yml b/.github/workflows/pr-comment.yml index 6c7b43960f9cf..8094ee13ccb50 100644 --- a/.github/workflows/pr-comment.yml +++ b/.github/workflows/pr-comment.yml @@ -270,8 +270,8 @@ jobs: if: steps.identity.outputs.valid == 'true' && steps.scope.outputs.stable == 'true' env: GITHUB_SHA: ${{ steps.identity.outputs.head_sha }} - GITHUB_RUN_ID: ${{ steps.identity.outputs.run_id }} - GITHUB_RUN_ATTEMPT: ${{ steps.identity.outputs.run_attempt }} + ASTRYX_VISUAL_RUN_ID: ${{ steps.identity.outputs.run_id }} + ASTRYX_VISUAL_RUN_ATTEMPT: ${{ steps.identity.outputs.run_attempt }} ASTRYX_PR_HEAD_SHA: ${{ steps.identity.outputs.head_sha }} ASTRYX_PR_BASE_SHA: ${{ steps.identity.outputs.base_sha }} run: | diff --git a/.github/workflows/visual-acceptance.yml b/.github/workflows/visual-acceptance.yml index d9eab24bc1adf..1b41169925fd0 100644 --- a/.github/workflows/visual-acceptance.yml +++ b/.github/workflows/visual-acceptance.yml @@ -25,7 +25,7 @@ jobs: runs-on: ubuntu-slim permissions: contents: read - pull-requests: read + pull-requests: write issues: write statuses: write steps: @@ -207,7 +207,7 @@ jobs: runs-on: ubuntu-slim permissions: contents: write - pull-requests: read + pull-requests: write issues: write statuses: write steps: