fix(ci): authorize Repo Owners for visual acceptance

.github/scripts/visual-gate/authorization.mjs

@@ -0,0 +1,29 @@
+// Copyright (c) Meta Platforms, Inc. and affiliates.
+
+const MAINTAINER_PERMISSIONS = new Set(['maintain', 'admin']);
+const REPO_OWNER_ROLE = 'Repo Owner';
+
+export function visualAcceptanceIdentity(permissionLevel = {}) {
+  const permission = permissionLevel.permission ?? 'none';
+  const effectivePermission = permissionLevel.user?.permissions?.admin
+    ? 'admin'
+    : permissionLevel.user?.permissions?.maintain
+      ? 'maintain'
+      : permission;
+  return {
+    permission,
+    roleName: permissionLevel.role_name ?? null,
+    effectivePermission,
+  };
+}
+
+export function isVisualAcceptanceMaintainer({
+  permission,
+  roleName,
+  effectivePermission = permission,
+} = {}) {
+  return (
+    MAINTAINER_PERMISSIONS.has(effectivePermission) ||
+    roleName === REPO_OWNER_ROLE
+  );
+}

.github/scripts/visual-gate/authorization.test.mjs

@@ -0,0 +1,62 @@
+// Copyright (c) Meta Platforms, Inc. and affiliates.
+
+import {describe, expect, it} from 'vitest';
+
+import {
+  isVisualAcceptanceMaintainer,
+  visualAcceptanceIdentity,
+} from './authorization.mjs';
+
+const endpointIdentity = overrides =>
+  visualAcceptanceIdentity({
+    permission: 'write',
+    role_name: 'Custom role',
+    user: {
+      permissions: {
+        admin: false,
+        maintain: false,
+        pull: true,
+        push: true,
+        triage: true,
+      },
+    },
+    ...overrides,
+  });
+
+describe('visual acceptance authorization', () => {
+  it.each([
+    ['repository owner', endpointIdentity({role_name: 'Repo Owner'}), true],
+    [
+      'effective maintainer',
+      endpointIdentity({user: {permissions: {maintain: true}}}),
+      true,
+    ],
+    [
+      'effective administrator',
+      endpointIdentity({user: {permissions: {admin: true}}}),
+      true,
+    ],
+    ['ordinary writer', endpointIdentity(), false],
+    ['missing collaborator response', visualAcceptanceIdentity(), false],
+    ['unknown permission', {permission: 'unknown'}, false],
+    ['unknown role', {permission: 'write', roleName: 'Owner'}, false],
+    ['legacy maintainer record', {permission: 'maintain'}, true],
+    ['legacy administrator record', {permission: 'admin'}, true],
+  ])('%s', (_label, identity, expected) => {
+    expect(isVisualAcceptanceMaintainer(identity)).toBe(expected);
+  });
+
+  it('keeps the endpoint permission and role for the acceptance record', () => {
+    expect(
+      visualAcceptanceIdentity({
+        permission: 'write',
+        role_name: 'Repo Owner',
+        user: {permissions: {maintain: true}},
+      }),
+    ).toMatchObject({
+      permission: 'write',
+      roleName: 'Repo Owner',
+      effectivePermission: 'maintain',
+    });
+  });
+});

.github/scripts/visual-gate/visual-acceptance.mjs

@@ -11,9 +11,10 @@
 import {execFileSync} from 'node:child_process';
 import {createHash} from 'node:crypto';
 import fs from 'node:fs';
+import {createRequire} from 'node:module';
 import path from 'node:path';
-import {PNG} from 'pngjs';
 
+import {isVisualAcceptanceMaintainer} from './authorization.mjs';
 import {readStoryIndex, shotKey, storiesInPackages} from './lib/plan.mjs';
 
 const args = process.argv.slice(2);
@@ -276,14 +277,23 @@ function accept() {
   const approver = flag('approver') ?? '';
   const approverId = Number(flag('approver-id'));
   const permission = flag('permission') ?? '';
+  const effectivePermission = flag('effective-permission') ?? permission;
+  const roleName = flag('role-name');
   const commentId = Number(flag('comment-id'));
   const reason = validateReason(flag('reason'));
 
   validateIdentity(pr, head);
   if (!approver || !Number.isSafeInteger(approverId) || approverId <= 0)
     fail('invalid approver');
-  if (!['maintain', 'admin'].includes(permission))
-    fail('approver must have maintain/admin permission');
+  if (
+    !isVisualAcceptanceMaintainer({
+      permission,
+      effectivePermission,
+      roleName,
+    })
+  ) {
+    fail('approver must have maintain/admin permission or the Repo Owner role');
+  }
   if (!Number.isSafeInteger(commentId) || commentId <= 0)
     fail('invalid comment id');
 
@@ -351,6 +361,8 @@ function accept() {
       approver,
       approverId,
       permission,
+      effectivePermission,
+      roleName,
       reason,
       commentId,
       at: new Date().toISOString(),
@@ -467,7 +479,11 @@ function validateAcceptance(value, expected = {}) {
     !value.decision?.approver ||
     !Number.isSafeInteger(value.decision?.approverId) ||
     value.decision.approverId <= 0 ||
-    !['maintain', 'admin'].includes(value.decision?.permission) ||
+    !isVisualAcceptanceMaintainer({
+      permission: value.decision?.permission,
+      effectivePermission: value.decision?.effectivePermission,
+      roleName: value.decision?.roleName,
+    }) ||
     !Number.isSafeInteger(value.decision?.commentId) ||
     value.decision.commentId <= 0 ||
     !value.decision?.reason
@@ -584,7 +600,10 @@ function plan() {
   );
 }
 
+const require = createRequire(import.meta.url);
+
 function samePixels(first, second) {
+  const {PNG} = require('pngjs');
   const a = PNG.sync.read(fs.readFileSync(first));
   const b = PNG.sync.read(fs.readFileSync(second));
   return (

.github/scripts/visual-gate/visual-acceptance.test.mjs

@@ -234,6 +234,73 @@ describe('visual acceptance', () => {
     ).toBe(true);
   });
 
+  it('accepts without installed packages in the archive job', () => {
+    const isolated = path.join(root, 'isolated-visual-gate');
+    fs.cpSync(path.dirname(SCRIPT), isolated, {recursive: true});
+    const args = [path.join(isolated, 'visual-acceptance.mjs'), 'accept'];
+    for (const [name, value] of Object.entries(acceptanceFlags()))
+      args.push(`--${name}`, String(value));
+    expect(execFileSync(process.execPath, args, {encoding: 'utf8'})).toContain(
+      'Accepted 1 visual delta',
+    );
+  });
+
+  it('accepts a repository owner and records the reported permission and role', () => {
+    run(
+      'accept',
+      acceptanceFlags({
+        approver: 'cixzhang',
+        permission: 'write',
+        'effective-permission': 'maintain',
+        'role-name': 'Repo Owner',
+      }),
+    );
+    expect(
+      JSON.parse(fs.readFileSync(acceptanceFile(), 'utf8')).decision,
+    ).toMatchObject({
+      approver: 'cixzhang',
+      permission: 'write',
+      effectivePermission: 'maintain',
+      roleName: 'Repo Owner',
+    });
+  });
+
+  it.each(['maintain', 'admin'])(
+    'accepts effective %s capability while preserving the reported role',
+    effectivePermission => {
+      run(
+        'accept',
+        acceptanceFlags({
+          permission: 'write',
+          'effective-permission': effectivePermission,
+          'role-name': 'Custom role',
+        }),
+      );
+      expect(
+        JSON.parse(fs.readFileSync(acceptanceFile(), 'utf8')).decision,
+      ).toMatchObject({
+        permission: 'write',
+        effectivePermission,
+        roleName: 'Custom role',
+      });
+    },
+  );
+
+  it.each(['maintain', 'admin'])(
+    'validates a legacy %s record without role metadata',
+    permission => {
+      run('accept', acceptanceFlags({permission}));
+      const record = JSON.parse(fs.readFileSync(acceptanceFile(), 'utf8'));
+      delete record.decision.roleName;
+      delete record.decision.effectivePermission;
+      writeJSON(acceptanceFile(), record);
+      expect(JSON.parse(run('state', {pages, pr: 42, head: HEAD}))).toMatchObject({
+        state: 'success',
+        reason: 'accepted',
+      });
+    },
+  );
+
   it('requires an explanatory reason and maintainer permission', () => {
     expect(fail('accept', acceptanceFlags({reason: 'intentional...'}))).toMatch(
       /reason must explain/,

.github/scripts/visual-gate/workflow-concurrency.test.mjs

@@ -56,10 +56,27 @@ describe('visual acceptance workflow concurrency', () => {
     );
     expect(initialize).toContain("'No stable visual scope.'");
     expect(authorize).not.toContain(': write');
-    expect(authorize).not.toContain('actions/checkout');
+    expect(authorize).toContain('actions/checkout@v7');
+    expect(authorize).toContain('visualAcceptanceIdentity(response.data)');
+    expect(authorize).toContain('isVisualAcceptanceMaintainer(identity)');
+    expect(authorize).toContain(
+      "core.setOutput('effective_permission', identity.effectivePermission)",
+    );
+    expect(authorize).toContain(
+      "core.setOutput('role_name', identity.roleName ?? '')",
+    );
+    expect(authorize).not.toContain('author_association');
     expect(authorize).not.toContain('issues.createComment');
     expect(authorize).not.toContain('createCommitStatus');
     expect(accept).toContain('needs: authorize');
+    expect(accept).toContain(
+      'EFFECTIVE_PERMISSION: ${{ needs.authorize.outputs.effective_permission }}',
+    );
+    expect(accept).toContain(
+      'ROLE_NAME: ${{ needs.authorize.outputs.role_name }}',
+    );
+    expect(accept).toContain('--effective-permission "$EFFECTIVE_PERMISSION"');
+    expect(accept).toContain('--role-name "$ROLE_NAME"');
     expect(accept).toContain(
       'group: visual-acceptance-head-${{ needs.authorize.outputs.head_repo_id }}-${{ needs.authorize.outputs.head_ref }}',
     );
@@ -109,16 +126,15 @@ describe('visual acceptance workflow concurrency', () => {
     expect(capture).not.toContain('GITHUB_RUN_ATTEMPT:');
   });
 
-  it('normalizes GitHub role flags before authorizing a maintainer', () => {
+  it('uses the documented collaborator permission response shape', () => {
     const value = workflow('visual-acceptance.yml');
     const authorize = value.slice(
       value.indexOf('  authorize:'),
       value.indexOf('  accept:'),
     );
 
-    expect(authorize).toContain('actor.permissions?.admin');
-    expect(authorize).toContain('actor.permissions?.maintain');
-    expect(authorize).toContain("? 'maintain'");
+    expect(authorize).toContain('visualAcceptanceIdentity(response.data)');
+    expect(authorize).not.toContain('response.data.user.permission');
   });
 
   it('uses the same head identity for post-merge promotion', () => {

.github/workflows/visual-acceptance.yml

@@ -109,17 +109,29 @@ jobs:
       approver: ${{ steps.authorize.outputs.approver }}
       approver_id: ${{ steps.authorize.outputs.approver_id }}
       permission: ${{ steps.authorize.outputs.permission }}
+      effective_permission: ${{ steps.authorize.outputs.effective_permission }}
+      role_name: ${{ steps.authorize.outputs.role_name }}
       comment_id: ${{ steps.authorize.outputs.comment_id }}
       reason: ${{ steps.authorize.outputs.reason }}
     steps:
       # This job is deliberately read-only. The mutation job acquires the shared
       # head-branch lock using these trusted API-derived outputs.
+      - name: Checkout trusted default-branch code
+        uses: actions/checkout@v7
+
       - name: Authorize and resolve the decision
         id: authorize
         uses: actions/github-script@v9
         with:
           retries: 3
           script: |
+            const {pathToFileURL} = require('node:url');
+            const {
+              isVisualAcceptanceMaintainer,
+              visualAcceptanceIdentity,
+            } = await import(
+              pathToFileURL(`${process.env.GITHUB_WORKSPACE}/.github/scripts/visual-gate/authorization.mjs`).href,
+            );
             const {owner, repo} = context.repo;
             const number = context.issue.number;
             const {data: pr} = await github.rest.pulls.get({owner, repo, pull_number: number});
@@ -165,21 +177,16 @@ jobs:
               refuse('that evidence is not from the latest completed CI attempt for this head.');
               return;
             }
-            let permission = 'none';
+            let identity = visualAcceptanceIdentity();
             try {
               const response = await github.rest.repos.getCollaboratorPermissionLevel({
                 owner, repo, username: context.actor,
               });
-              const actor = response.data.user;
-              permission = actor.permissions?.admin
-                ? 'admin'
-                : actor.permissions?.maintain
-                  ? 'maintain'
-                  : actor.permission;
+              identity = visualAcceptanceIdentity(response.data);
             } catch (error) {
               if (error.status !== 404) throw error;
             }
-            if (!['maintain', 'admin'].includes(permission)) {
+            if (!isVisualAcceptanceMaintainer(identity)) {
               refuse('only a repository maintainer may accept stable visual changes.');
               return;
             }
@@ -188,7 +195,9 @@ jobs:
             core.setOutput('run_attempt', String(runAttempt));
             core.setOutput('approver', context.actor);
             core.setOutput('approver_id', String(context.payload.comment.user.id));
-            core.setOutput('permission', permission);
+            core.setOutput('permission', identity.permission);
+            core.setOutput('effective_permission', identity.effectivePermission);
+            core.setOutput('role_name', identity.roleName ?? '');
             core.setOutput('comment_id', String(context.payload.comment.id));
             core.setOutput('reason', reason);
 
@@ -238,6 +247,8 @@ jobs:
           APPROVER: ${{ needs.authorize.outputs.approver }}
           APPROVER_ID: ${{ needs.authorize.outputs.approver_id }}
           PERMISSION: ${{ needs.authorize.outputs.permission }}
+          EFFECTIVE_PERMISSION: ${{ needs.authorize.outputs.effective_permission }}
+          ROLE_NAME: ${{ needs.authorize.outputs.role_name }}
           COMMENT_ID: ${{ needs.authorize.outputs.comment_id }}
           REASON: ${{ needs.authorize.outputs.reason }}
         run: |
@@ -257,6 +268,8 @@ jobs:
               --approver "$APPROVER" \
               --approver-id "$APPROVER_ID" \
               --permission "$PERMISSION" \
+              --effective-permission "$EFFECTIVE_PERMISSION" \
+              --role-name "$ROLE_NAME" \
               --comment-id "$COMMENT_ID" \
               --reason "$REASON"
             if [ -z "$(git -C "$PAGES" status --porcelain -- "$DEST")" ]; then