vndangkhoa/open-design
1
0
Fork 0
mirror of https://github.com/nexu-io/open-design.git synced 2026-06-01 03:14:35 +07:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

fix(ci): run fork visual reports from trusted code (#2935)

Browse source 
* fix: run fork visual reports from trusted code

* fix: auto-approve strict web visual capture

* fix: address visual report review feedback

Generated-By: looper 0.9.1 (runner=fixer, agent=opencode)

* fix: propagate visual report storage failures

Generated-By: looper 0.9.1 (runner=fixer, agent=opencode)

* fix: validate PR screenshots before upload

Generated-By: looper 0.9.1 (runner=fixer, agent=opencode)

* fix: validate visual PR identity before comment

* fix: harden fork visual report validation

Generated-By: looper 0.9.1 (runner=fixer, agent=opencode)

* fix: address remaining fork visual report review feedback

Generated-By: looper 0.9.1 (runner=fixer, agent=opencode)

* fix: handle stale fork visual report lookup

Generated-By: looper 0.9.1 (runner=fixer, agent=opencode)

* fix: allow stale fork visual report fallback

Generated-By: looper 0.9.1 (runner=fixer, agent=opencode)
This commit is contained in:
Marc Chan 2026-05-26 14:17:04 +08:00 committed by GitHub
parent f4d53486ec
commit 125dcd0174
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 720 additions and 114 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
.github/workflows/fork-pr-workflow-approval.yml vendored
View file

@ -4,7 +4,8 @@ name: fork-pr-workflow-approval
# or executes the fork head; the TypeScript policy script only reads PR metadata
# through the GitHub API and approves pending pull_request runs when the touched
# paths are inside the low-risk source allowlist. It only approves low-privilege
# pull_request workflows (`ci`, visual capture, visual verify); privileged
# pull_request workflows (`ci`, visual verify, and strict web-source visual
# capture); privileged
# workflow_run / release / deploy workflows stay on manual gates.
on:
pull_request_target:

267
.github/workflows/visual-pr-comment.yml vendored
View file

@ -27,20 +27,21 @@ concurrency:
jobs:
comment:
name: Publish PR visual diff comment
# Fork PR capture artifacts are untrusted. Publish comments/R2 reports for
# same-repository PRs automatically; fork captures require maintainer
# workflow_dispatch with a specific capture run id and PR number.
if: ${{ github.event_name == 'workflow_dispatch' || (github.event.workflow_run.conclusion == 'success' && github.event.workflow_run.head_repository.full_name == github.repository) }}
# Fork PR capture artifacts are untrusted. Always validate the live PR state
# and only execute trusted base-repository code before publishing comments
# or reports.
if: ${{ github.event_name == 'workflow_dispatch' || github.event.workflow_run.conclusion == 'success' }}
runs-on: ubuntu-24.04
timeout-minutes: 15
steps:
- name: Checkout
- name: Checkout same-repository workflow head
if: ${{ github.event_name == 'workflow_run' && github.event.workflow_run.head_repository.full_name == github.repository }}
uses: actions/checkout@v6.0.2
with:
fetch-depth: 0
repository: ${{ github.event_name == 'workflow_run' && github.event.workflow_run.head_repository.full_name || github.repository }}
ref: ${{ github.event_name == 'workflow_run' && github.event.workflow_run.head_sha || github.sha }}
repository: ${{ github.event.workflow_run.head_repository.full_name }}
ref: ${{ github.event.workflow_run.head_sha }}
- name: Setup pnpm
uses: pnpm/action-setup@v6.0.8
@ -57,7 +58,7 @@ jobs:
uses: actions/download-artifact@v8
with:
run-id: ${{ github.event.workflow_run.id || inputs.capture_run_id }}
path: visual-artifact
path: ${{ runner.temp }}/visual-artifact
merge-multiple: true
github-token: ${{ github.token }}
@ -68,16 +69,19 @@ jobs:
GH_TOKEN: ${{ github.token }}
WORKFLOW_PR_NUMBER: ${{ github.event.workflow_run.pull_requests[0].number || inputs.pr_number }}
WORKFLOW_HEAD_SHA: ${{ github.event.workflow_run.head_sha || '' }}
WORKFLOW_HEAD_REPOSITORY: ${{ github.event.workflow_run.head_repository.full_name || '' }}
WORKFLOW_HEAD_BRANCH: ${{ github.event.workflow_run.head_branch || '' }}
WORKFLOW_RUN_ID: ${{ github.event.workflow_run.id || inputs.capture_run_id }}
run: |
set -euo pipefail
manifest="visual-artifact/visual-report/manifest.json"
artifact_dir="$RUNNER_TEMP/visual-artifact"
manifest="$artifact_dir/visual-report/manifest.json"
if [ ! -f "$manifest" ]; then
manifest="visual-artifact/manifest.json"
manifest="$artifact_dir/manifest.json"
fi
if [ ! -f "$manifest" ]; then
echo "Capture manifest not found" >&2
find visual-artifact -maxdepth 4 -type f >&2
find "$artifact_dir" -maxdepth 4 -type f >&2
exit 1
fi
manifest_pr_number="$(jq -r '.pr_number' "$manifest")"
@ -88,22 +92,6 @@ jobs:
echo "Invalid manifest pr_number: $manifest_pr_number" >&2
exit 1
fi
derived_pr_number="$WORKFLOW_PR_NUMBER"
if [ -z "$derived_pr_number" ] && [ -n "$WORKFLOW_HEAD_SHA" ]; then
derived_pr_number="$(gh api "repos/$GITHUB_REPOSITORY/commits/$WORKFLOW_HEAD_SHA/pulls" --jq '.[0].number // empty')"
fi
pr_number="$derived_pr_number"
if [ -z "$pr_number" ]; then
pr_number="$manifest_pr_number"
fi
if ! [[ "$pr_number" =~ ^[0-9]+$ ]]; then
echo "Unable to derive PR number from workflow event, capture manifest, or GitHub API for workflow head $WORKFLOW_HEAD_SHA." >&2
exit 1
fi
if [ -n "$derived_pr_number" ] && [ "$manifest_pr_number" != "$derived_pr_number" ]; then
echo "Manifest pr_number ($manifest_pr_number) does not match GitHub-derived PR number ($derived_pr_number)." >&2
exit 1
fi
if ! [[ "$base_sha" =~ ^[0-9a-f]{40}$ ]]; then
echo "Invalid manifest base_sha: $base_sha" >&2
exit 1
@ -116,13 +104,73 @@ jobs:
echo "Invalid manifest capture_outcome: $capture_outcome" >&2
exit 1
fi
source_head_sha="$WORKFLOW_HEAD_SHA"
source_head_repository="$WORKFLOW_HEAD_REPOSITORY"
source_head_branch="$WORKFLOW_HEAD_BRANCH"
if [ -z "$source_head_sha" ] || [ -z "$source_head_repository" ] || [ -z "$source_head_branch" ]; then
run_json="$(gh api "repos/$GITHUB_REPOSITORY/actions/runs/$WORKFLOW_RUN_ID")"
if [ -z "$source_head_sha" ]; then
source_head_sha="$(jq -r '.head_sha // empty' <<< "$run_json")"
fi
if [ -z "$source_head_repository" ]; then
source_head_repository="$(jq -r '.head_repository.full_name // empty' <<< "$run_json")"
fi
if [ -z "$source_head_branch" ]; then
source_head_branch="$(jq -r '.head_branch // empty' <<< "$run_json")"
fi
fi
manifest_head="$(jq -r '.head_sha' "$manifest")"
if ! [[ "$manifest_head" =~ ^[0-9a-f]{40}$ ]]; then
echo "Invalid manifest head_sha: $manifest_head" >&2
exit 1
fi
if [ -n "$WORKFLOW_HEAD_SHA" ] && [ "$manifest_head" != "$WORKFLOW_HEAD_SHA" ]; then
echo "Artifact manifest head_sha ($manifest_head) does not match workflow_run head_sha ($WORKFLOW_HEAD_SHA)." >&2
if ! [[ "$source_head_sha" =~ ^[0-9a-f]{40}$ ]]; then
echo "Unable to resolve trusted workflow_run head_sha for run $WORKFLOW_RUN_ID." >&2
exit 1
fi
if [ "$manifest_head" != "$source_head_sha" ]; then
echo "Artifact manifest head_sha ($manifest_head) does not match workflow_run head_sha ($source_head_sha)." >&2
exit 1
fi
if [ -z "$source_head_repository" ]; then
echo "Unable to resolve trusted workflow_run head repository for run $WORKFLOW_RUN_ID." >&2
exit 1
fi
if [ -z "$source_head_branch" ]; then
echo "Unable to resolve trusted workflow_run head branch for run $WORKFLOW_RUN_ID." >&2
exit 1
fi
pr_number="$WORKFLOW_PR_NUMBER"
if [ -z "$pr_number" ]; then
head_owner="${source_head_repository%%/*}"
if [ -z "$head_owner" ] || [ "$head_owner" = "$source_head_repository" ]; then
echo "Unable to resolve trusted workflow_run head owner from $source_head_repository." >&2
exit 1
fi
encoded_head="$(jq -rn --arg head "$head_owner:$source_head_branch" '$head|@uri')"
matching_pr_numbers="$(
gh api "repos/$GITHUB_REPOSITORY/pulls?state=open&head=$encoded_head&per_page=100" \
| jq -r --arg repo "$source_head_repository" \
'.[] | select((.head.repo.full_name // "") == $repo) | .number' \
| paste -sd ' ' -
)"
match_count="$(wc -w <<< "$matching_pr_numbers" | tr -d ' ')"
if [ "$match_count" -gt 1 ]; then
echo "Unable to resolve a unique open PR from trusted workflow metadata for $source_head_repository@$source_head_branch ($source_head_sha); found $match_count matches." >&2
exit 1
fi
if [ "$match_count" -eq 1 ]; then
pr_number="$matching_pr_numbers"
else
pr_number="$manifest_pr_number"
fi
fi
if ! [[ "$pr_number" =~ ^[0-9]+$ ]]; then
echo "Unable to derive PR number from trusted workflow metadata for workflow head $source_head_sha." >&2
exit 1
fi
if [ -n "$WORKFLOW_PR_NUMBER" ] && [ "$manifest_pr_number" != "$WORKFLOW_PR_NUMBER" ]; then
echo "Manifest pr_number ($manifest_pr_number) does not match workflow_run PR number ($WORKFLOW_PR_NUMBER)." >&2
exit 1
fi
{
@ -132,12 +180,96 @@ jobs:
echo "base_sha=$base_sha"
echo "run_id=$run_id"
echo "capture_outcome=$capture_outcome"
echo "source_head_repository=$source_head_repository"
echo "source_head_branch=$source_head_branch"
} >> "$GITHUB_OUTPUT"
- name: Validate live PR state for trusted checkout
if: ${{ github.event_name == 'workflow_dispatch' }}
id: trusted-pr
shell: bash
env:
GH_TOKEN: ${{ github.token }}
EVENT_NAME: ${{ github.event_name }}
PR_NUMBER: ${{ steps.manifest.outputs.pr_number }}
ARTIFACT_HEAD_SHA: ${{ steps.manifest.outputs.head_sha }}
ARTIFACT_BASE_SHA: ${{ steps.manifest.outputs.base_sha }}
SOURCE_HEAD_REPOSITORY: ${{ steps.manifest.outputs.source_head_repository }}
SOURCE_HEAD_BRANCH: ${{ steps.manifest.outputs.source_head_branch }}
run: |
set -euo pipefail
skip_or_fail_stale() {
local message="$1"
if [ "$EVENT_NAME" = "workflow_dispatch" ]; then
echo "$message" >&2
exit 1
fi
echo "$message"
echo "stale=true" >> "$GITHUB_OUTPUT"
exit 0
}
pr_json="$(gh api "repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER")"
pr_state="$(jq -r '.state' <<< "$pr_json")"
pr_is_draft="$(jq -r '.draft' <<< "$pr_json")"
pr_is_cross_repository="$(jq -r '(.head.repo.full_name // "") != (.base.repo.full_name // "")' <<< "$pr_json")"
current_head="$(jq -r '.head.sha' <<< "$pr_json")"
current_base="$(jq -r '.base.sha' <<< "$pr_json")"
current_head_repository="$(jq -r '.head.repo.full_name // empty' <<< "$pr_json")"
current_head_branch="$(jq -r '.head.ref // empty' <<< "$pr_json")"
if [ "$pr_state" != "open" ]; then
skip_or_fail_stale "Skipping trusted checkout for PR $PR_NUMBER because state is $pr_state."
fi
if [ "$pr_is_draft" != "false" ]; then
skip_or_fail_stale "Skipping trusted checkout for PR $PR_NUMBER because it is draft."
fi
if [ "$current_head" != "$ARTIFACT_HEAD_SHA" ]; then
skip_or_fail_stale "Skipping stale visual artifact for $ARTIFACT_HEAD_SHA; current PR head is $current_head."
fi
if [ "$current_base" != "$ARTIFACT_BASE_SHA" ]; then
skip_or_fail_stale "Skipping stale visual artifact for base $ARTIFACT_BASE_SHA; current PR base is $current_base."
fi
if [ "$current_head_repository" != "$SOURCE_HEAD_REPOSITORY" ]; then
echo "Artifact PR head repository ($current_head_repository) does not match trusted workflow_run head repository ($SOURCE_HEAD_REPOSITORY)." >&2
exit 1
fi
if [ "$current_head_branch" != "$SOURCE_HEAD_BRANCH" ]; then
echo "Artifact PR head branch ($current_head_branch) does not match trusted workflow_run head branch ($SOURCE_HEAD_BRANCH)." >&2
exit 1
fi
{
echo "stale=false"
echo "base_sha=$current_base"
echo "is_cross_repository=$pr_is_cross_repository"
} >> "$GITHUB_OUTPUT"
- name: Checkout trusted base revision
if: ${{ steps.trusted-pr.outputs.stale != 'true' && (github.event_name == 'workflow_dispatch' || github.event.workflow_run.head_repository.full_name != github.repository) }}
uses: actions/checkout@v6.0.2
with:
fetch-depth: 0
repository: ${{ github.repository }}
ref: ${{ steps.trusted-pr.outputs.base_sha }}
- name: Resolve pnpm store path
if: ${{ steps.trusted-pr.outputs.stale != 'true' }}
id: pnpm-store
shell: bash
run: echo "path=$(pnpm store path --silent)" >> "$GITHUB_OUTPUT"
- name: Restore pnpm store cache
if: ${{ steps.trusted-pr.outputs.stale != 'true' }}
uses: actions/cache/restore@v5.0.5
with:
path: ${{ steps.pnpm-store.outputs.path }}
key: pnpm-store-${{ runner.os }}-${{ hashFiles('pnpm-lock.yaml') }}
- name: Install dependencies
if: ${{ steps.trusted-pr.outputs.stale != 'true' }}
run: pnpm install --frozen-lockfile
- name: Stop stale visual runs
if: ${{ steps.trusted-pr.outputs.stale != 'true' }}
id: stale
env:
GH_TOKEN: ${{ github.token }}
PR_NUMBER: ${{ steps.manifest.outputs.pr_number }}
@ -151,58 +283,15 @@ jobs:
current_head="$(jq -r '.headRefOid' <<< "$pr_json")"
current_base="$(jq -r '.baseRefOid' <<< "$pr_json")"
if [ "$pr_state" != "OPEN" ]; then
echo "Refusing trusted checkout for PR $PR_NUMBER because state is $pr_state." >&2
exit 1
echo "Skipping visual report for PR $PR_NUMBER because state is $pr_state."
echo "stale=true" >> "$GITHUB_OUTPUT"
exit 0
fi
if [ "$pr_is_draft" != "false" ]; then
echo "Refusing trusted checkout for PR $PR_NUMBER because it is draft." >&2
exit 1
echo "Skipping visual report for PR $PR_NUMBER because it is draft."
echo "stale=true" >> "$GITHUB_OUTPUT"
exit 0
fi
if [ "$current_head" != "$ARTIFACT_HEAD_SHA" ]; then
echo "Artifact head_sha ($ARTIFACT_HEAD_SHA) does not match live PR head ($current_head)." >&2
exit 1
fi
if [ "$current_base" != "$ARTIFACT_BASE_SHA" ]; then
echo "Artifact base_sha ($ARTIFACT_BASE_SHA) does not match live PR base ($current_base)." >&2
exit 1
fi
echo "base_sha=$current_base" >> "$GITHUB_OUTPUT"
- name: Checkout trusted base revision
if: ${{ github.event_name == 'workflow_dispatch' }}
uses: actions/checkout@v6.0.2
with:
clean: false
fetch-depth: 0
repository: ${{ github.repository }}
ref: ${{ steps.trusted-pr.outputs.base_sha }}
- name: Resolve pnpm store path
id: pnpm-store
shell: bash
run: echo "path=$(pnpm store path --silent)" >> "$GITHUB_OUTPUT"
- name: Restore pnpm store cache
uses: actions/cache/restore@v5.0.5
with:
path: ${{ steps.pnpm-store.outputs.path }}
key: pnpm-store-${{ runner.os }}-${{ hashFiles('pnpm-lock.yaml') }}
- name: Install dependencies
run: pnpm install --frozen-lockfile
- name: Stop stale visual runs
id: stale
env:
GH_TOKEN: ${{ github.token }}
PR_NUMBER: ${{ steps.manifest.outputs.pr_number }}
ARTIFACT_HEAD_SHA: ${{ steps.manifest.outputs.head_sha }}
ARTIFACT_BASE_SHA: ${{ steps.manifest.outputs.base_sha }}
run: |
set -euo pipefail
pr_json="$(gh pr view "$PR_NUMBER" --repo "$GITHUB_REPOSITORY" --json headRefOid,baseRefOid)"
current_head="$(jq -r '.headRefOid' <<< "$pr_json")"
current_base="$(jq -r '.baseRefOid' <<< "$pr_json")"
if [ "$current_head" != "$ARTIFACT_HEAD_SHA" ]; then
echo "Skipping stale visual artifact for $ARTIFACT_HEAD_SHA; current PR head is $current_head."
echo "stale=true" >> "$GITHUB_OUTPUT"
@ -216,7 +305,7 @@ jobs:
echo "stale=false" >> "$GITHUB_OUTPUT"
- name: Build visual diff report
if: ${{ steps.stale.outputs.stale != 'true' }}
if: ${{ steps.trusted-pr.outputs.stale != 'true' && steps.stale.outputs.stale != 'true' }}
env:
R2_ACCOUNT_ID: ${{ secrets.R2_ACCOUNT_ID }}
R2_ACCESS_KEY_ID: ${{ secrets.R2_ACCESS_KEY_ID }}
@ -235,12 +324,12 @@ jobs:
--head-sha "${{ steps.manifest.outputs.head_sha }}" \
--base-sha "${{ steps.manifest.outputs.base_sha }}" \
--capture-outcome "${{ steps.manifest.outputs.capture_outcome }}" \
--screenshots ../visual-artifact/visual-screenshots \
--screenshots "$RUNNER_TEMP/visual-artifact/visual-screenshots" \
--comment-out ui/reports/visual-report/comment.md \
--manifest-out ui/reports/visual-report/report-manifest.json
- name: Upsert PR comment
if: ${{ steps.stale.outputs.stale != 'true' }}
if: ${{ steps.trusted-pr.outputs.stale != 'true' && steps.stale.outputs.stale != 'true' }}
env:
GH_TOKEN: ${{ github.token }}
PR_NUMBER: ${{ steps.manifest.outputs.pr_number }}
@ -248,9 +337,19 @@ jobs:
ARTIFACT_BASE_SHA: ${{ steps.manifest.outputs.base_sha }}
run: |
set -euo pipefail
pr_json="$(gh pr view "$PR_NUMBER" --repo "$GITHUB_REPOSITORY" --json headRefOid,baseRefOid)"
pr_json="$(gh pr view "$PR_NUMBER" --repo "$GITHUB_REPOSITORY" --json state,isDraft,headRefOid,baseRefOid)"
pr_state="$(jq -r '.state' <<< "$pr_json")"
pr_is_draft="$(jq -r '.isDraft' <<< "$pr_json")"
current_head="$(jq -r '.headRefOid' <<< "$pr_json")"
current_base="$(jq -r '.baseRefOid' <<< "$pr_json")"
if [ "$pr_state" != "OPEN" ]; then
echo "Skipping visual comment for PR $PR_NUMBER because state is $pr_state."
exit 0
fi
if [ "$pr_is_draft" != "false" ]; then
echo "Skipping visual comment for PR $PR_NUMBER because it is draft."
exit 0
fi
if [ "$current_head" != "$ARTIFACT_HEAD_SHA" ]; then
echo "Skipping stale visual comment for $ARTIFACT_HEAD_SHA; current PR head is $current_head."
exit 0
@ -270,7 +369,7 @@ jobs:
fi
- name: Upload visual report artifact
if: ${{ always() && steps.stale.outputs.stale != 'true' }}
if: ${{ always() && steps.trusted-pr.outputs.stale != 'true' && steps.stale.outputs.stale != 'true' }}
uses: actions/upload-artifact@v7
with:
name: visual-pr-report-${{ steps.manifest.outputs.pr_number }}-${{ steps.manifest.outputs.run_id }}

101
e2e/scripts/visual-report.ts
View file

@ -54,6 +54,13 @@ type ComparedCase = {
error?: string;
};
type CompareCaseOps = {
putFile?: (r2: R2Config, key: string, filePath: string) => Promise<void>;
findBaseline?: (r2: R2Config, caseName: string, candidateShas: string[]) => Promise<BaselineLookup | null>;
downloadObject?: (r2: R2Config, key: string, outputPath: string) => Promise<void>;
writeDiffPng?: (mainPath: string, prPath: string, diffPath: string) => Promise<number>;
};
const scriptDir = path.dirname(fileURLToPath(import.meta.url));
const e2eDir = path.resolve(scriptDir, '..');
@ -154,7 +161,7 @@ async function comparePr(options: ParsedArgs): Promise<void> {
console.log(`Wrote visual report for ${compared.length} cases to ${commentOut}.`);
}
async function compareCase(input: {
export async function compareCase(input: {
r2: R2Config;
prNumber: string;
runId: string;
@ -162,12 +169,29 @@ async function compareCase(input: {
visualCase: VisualCase;
candidateShas: string[];
outputDir: string;
}): Promise<ComparedCase> {
}, ops: CompareCaseOps = {}): Promise<ComparedCase> {
const { r2, prNumber, runId, headSha, visualCase, candidateShas, outputDir } = input;
const putFileOp = ops.putFile ?? putFile;
const findBaselineOp = ops.findBaseline ?? findBaseline;
const downloadObjectOp = ops.downloadObject ?? downloadObject;
const writeDiffPngOp = ops.writeDiffPng ?? writeDiffPng;
const prKey = prImageKey(prNumber, runId, 'pr', visualCase.name);
await putFile(r2, prKey, visualCase.path);
const baseline = await findBaseline(r2, visualCase.name, candidateShas);
const visualBuffer = await readFile(visualCase.path);
try {
validatePngBuffer(visualBuffer, visualCase.path);
} catch (error) {
return {
name: visualCase.name,
status: 'failed',
error: error instanceof Error ? error.message : String(error),
};
}
await putFileOp(r2, prKey, visualCase.path);
const baseline = await findBaselineOp(r2, visualCase.name, candidateShas);
if (baseline == null) {
return {
name: visualCase.name,
@ -178,36 +202,42 @@ async function compareCase(input: {
const mainPath = path.join(outputDir, 'main', `${visualCase.name}.png`);
const diffPath = path.join(outputDir, 'diff', `${visualCase.name}.png`);
await downloadObject(r2, baseline.key, mainPath);
await downloadObjectOp(r2, baseline.key, mainPath);
let diffPixels: number;
try {
const diffPixels = await writeDiffPng(mainPath, visualCase.path, diffPath);
const mainKey = prImageKey(prNumber, runId, 'main', visualCase.name);
const diffKey = prImageKey(prNumber, runId, 'diff', visualCase.name);
await putFile(r2, mainKey, mainPath);
await putFile(r2, diffKey, diffPath);
return {
name: visualCase.name,
status: diffPixels > 0 ? 'changed' : 'unchanged',
diffPixels,
baselineSha: baseline.sha,
baselineBehindBy: baseline.behindBy,
mainUrl: publicUrl(r2, mainKey),
prUrl: publicUrl(r2, prKey),
diffUrl: publicUrl(r2, diffKey),
};
diffPixels = await writeDiffPngOp(mainPath, visualCase.path, diffPath);
} catch (error) {
return {
name: visualCase.name,
status: 'failed',
baselineSha: baseline.sha,
baselineBehindBy: baseline.behindBy,
mainUrl: publicUrl(r2, baseline.key),
...(baseline == null
? {}
: {
baselineSha: baseline.sha,
baselineBehindBy: baseline.behindBy,
mainUrl: publicUrl(r2, baseline.key),
}),
prUrl: publicUrl(r2, prKey),
error: error instanceof Error ? error.message : String(error),
};
}
const mainKey = prImageKey(prNumber, runId, 'main', visualCase.name);
const diffKey = prImageKey(prNumber, runId, 'diff', visualCase.name);
await putFileOp(r2, mainKey, mainPath);
await putFileOp(r2, diffKey, diffPath);
return {
name: visualCase.name,
status: diffPixels > 0 ? 'changed' : 'unchanged',
diffPixels,
baselineSha: baseline.sha,
baselineBehindBy: baseline.behindBy,
mainUrl: publicUrl(r2, mainKey),
prUrl: publicUrl(r2, prKey),
diffUrl: publicUrl(r2, diffKey),
};
}
async function writeDiffPng(mainPath: string, prPath: string, diffPath: string): Promise<number> {
@ -425,6 +455,29 @@ function setPixel(png: PNG, x: number, y: number, color: readonly [number, numbe
png.data[index + 3] = color[3];
}
function validatePngBuffer(buffer: Buffer, filePath: string): void {
assertPngHeaderSize(buffer, filePath);
const png = PNG.sync.read(buffer);
assertPngSize(png, filePath);
}
function assertPngHeaderSize(buffer: Buffer, filePath: string): void {
if (buffer.length < 24) {
throw new Error(`Visual case ${filePath} is not a valid PNG file`);
}
const signature = buffer.subarray(0, 8);
const expectedSignature = Buffer.from([137, 80, 78, 71, 13, 10, 26, 10]);
if (!signature.equals(expectedSignature)) {
throw new Error(`Visual case ${filePath} is not a valid PNG file`);
}
const ihdrLength = buffer.readUInt32BE(8);
const chunkType = buffer.toString('ascii', 12, 16);
if (ihdrLength !== 13 || chunkType !== 'IHDR') {
throw new Error(`Visual case ${filePath} is not a valid PNG file`);
}
assertPngPixels(buffer.readUInt32BE(16), buffer.readUInt32BE(20), filePath);
}
function assertPngSize(png: PNG, filePath: string): void {
assertPngPixels(png.width, png.height, filePath);
}

273
e2e/tests/visual-report.test.ts
View file

@ -1,9 +1,14 @@
import { tmpdir } from 'node:os';
import path from 'node:path';
import { mkdtemp, mkdir, readFile, rm, writeFile } from 'node:fs/promises';
import { describe, expect, test } from 'vitest';
import { PNG } from 'pngjs';
import {
assertPngPixels,
compareCase,
diffBoxesFromMask,
drawBox,
mergeDiffBoxes,
@ -19,6 +24,245 @@ describe('visual report PNG sizing', () => {
/maximum allowed is 4000000 pixels/,
);
});
test('malformed screenshots fail one case without preventing later valid cases', async () => {
const workDir = await mkdtemp(path.join(tmpdir(), 'visual-report-'));
try {
const outputDir = path.join(workDir, 'output');
const goodPath = path.join(workDir, 'visual-good.png');
const badPath = path.join(workDir, 'visual-bad.png');
const pngBuffer = PNG.sync.write(createFilledPng(2, 2));
await writeFile(goodPath, pngBuffer);
await writeFile(badPath, Buffer.from('not-a-png'));
const r2 = {
bucket: 'visual-bucket',
publicOrigin: 'https://example.invalid',
client: {} as never,
};
const ops = {
putFile: async () => {},
findBaseline: async () => ({ sha: 'a'.repeat(40), key: 'baseline/visual-good.png', behindBy: 0 }),
downloadObject: async (_r2: unknown, _key: string, outputPath: string) => {
await mkdir(path.dirname(outputPath), { recursive: true });
await writeFile(outputPath, pngBuffer);
},
writeDiffPng: async (_mainPath: string, prPath: string, diffPath: string) => {
await mkdir(path.dirname(diffPath), { recursive: true });
await writeFile(diffPath, await readFile(prPath));
return 0;
},
};
const malformed = await compareCase(
{
r2,
prNumber: '12',
runId: '34',
headSha: 'b'.repeat(40),
visualCase: { name: 'visual-bad', path: badPath },
candidateShas: ['c'.repeat(40)],
outputDir,
},
ops,
);
const valid = await compareCase(
{
r2,
prNumber: '12',
runId: '34',
headSha: 'b'.repeat(40),
visualCase: { name: 'visual-good', path: goodPath },
candidateShas: ['c'.repeat(40)],
outputDir,
},
ops,
);
expect(malformed.status).toBe('failed');
expect(malformed.name).toBe('visual-bad');
expect(valid).toMatchObject({
name: 'visual-good',
status: 'unchanged',
prUrl: 'https://example.invalid/visual-regression/pr-12/34/pr/visual-good.png',
});
} finally {
await rm(workDir, { recursive: true, force: true });
}
});
test('required R2 failures propagate instead of returning a failed case', async () => {
const workDir = await mkdtemp(path.join(tmpdir(), 'visual-report-'));
try {
const outputDir = path.join(workDir, 'output');
const goodPath = path.join(workDir, 'visual-good.png');
const pngBuffer = PNG.sync.write(createFilledPng(2, 2));
await writeFile(goodPath, pngBuffer);
const r2 = {
bucket: 'visual-bucket',
publicOrigin: 'https://example.invalid',
client: {} as never,
};
await expect(compareCase(
{
r2,
prNumber: '12',
runId: '34',
headSha: 'b'.repeat(40),
visualCase: { name: 'visual-good', path: goodPath },
candidateShas: ['c'.repeat(40)],
outputDir,
},
{
putFile: async () => {},
findBaseline: async () => ({ sha: 'a'.repeat(40), key: 'baseline/visual-good.png', behindBy: 0 }),
downloadObject: async () => {
throw new Error('download failed');
},
writeDiffPng: async () => 0,
},
)).rejects.toThrow('download failed');
} finally {
await rm(workDir, { recursive: true, force: true });
}
});
test('missing screenshot files propagate instead of returning a failed case', async () => {
const workDir = await mkdtemp(path.join(tmpdir(), 'visual-report-'));
try {
const outputDir = path.join(workDir, 'output');
const missingPath = path.join(workDir, 'visual-missing.png');
const r2 = {
bucket: 'visual-bucket',
publicOrigin: 'https://example.invalid',
client: {} as never,
};
await expect(compareCase(
{
r2,
prNumber: '12',
runId: '34',
headSha: 'b'.repeat(40),
visualCase: { name: 'visual-missing', path: missingPath },
candidateShas: ['c'.repeat(40)],
outputDir,
},
{
putFile: async () => {},
findBaseline: async () => null,
downloadObject: async () => {
throw new Error('download should not run');
},
writeDiffPng: async () => {
throw new Error('diff should not run');
},
},
)).rejects.toThrow(/ENOENT|no such file/i);
} finally {
await rm(workDir, { recursive: true, force: true });
}
});
test('malformed screenshots without a baseline fail before any PR upload', async () => {
const workDir = await mkdtemp(path.join(tmpdir(), 'visual-report-'));
try {
const outputDir = path.join(workDir, 'output');
const badPath = path.join(workDir, 'visual-bad.png');
await writeFile(badPath, Buffer.from('not-a-png'));
const uploadedKeys: string[] = [];
const r2 = {
bucket: 'visual-bucket',
publicOrigin: 'https://example.invalid',
client: {} as never,
};
const result = await compareCase(
{
r2,
prNumber: '12',
runId: '34',
headSha: 'b'.repeat(40),
visualCase: { name: 'visual-bad', path: badPath },
candidateShas: ['c'.repeat(40)],
outputDir,
},
{
putFile: async (_r2: unknown, key: string) => {
uploadedKeys.push(key);
},
findBaseline: async () => null,
downloadObject: async () => {
throw new Error('download should not run');
},
writeDiffPng: async () => {
throw new Error('diff should not run');
},
},
);
expect(result).toMatchObject({
name: 'visual-bad',
status: 'failed',
});
expect(uploadedKeys).toEqual([]);
} finally {
await rm(workDir, { recursive: true, force: true });
}
});
test('huge-dimension PNG headers fail before decode and upload', async () => {
const workDir = await mkdtemp(path.join(tmpdir(), 'visual-report-'));
try {
const outputDir = path.join(workDir, 'output');
const badPath = path.join(workDir, 'visual-huge.png');
await writeFile(badPath, createHugeHeaderPng(100_000, 100_000));
const uploadedKeys: string[] = [];
const r2 = {
bucket: 'visual-bucket',
publicOrigin: 'https://example.invalid',
client: {} as never,
};
const result = await compareCase(
{
r2,
prNumber: '12',
runId: '34',
headSha: 'b'.repeat(40),
visualCase: { name: 'visual-huge', path: badPath },
candidateShas: ['c'.repeat(40)],
outputDir,
},
{
putFile: async (_r2: unknown, key: string) => {
uploadedKeys.push(key);
},
findBaseline: async () => null,
downloadObject: async () => {
throw new Error('download should not run');
},
writeDiffPng: async () => {
throw new Error('diff should not run');
},
},
);
expect(result).toMatchObject({
name: 'visual-huge',
status: 'failed',
error: expect.stringMatching(/maximum allowed is 4000000 pixels/),
});
expect(uploadedKeys).toEqual([]);
} finally {
await rm(workDir, { recursive: true, force: true });
}
});
});
describe('visual diff box extraction', () => {
@ -102,6 +346,35 @@ function createMask(width: number, height: number, pixels: ReadonlyArray<readonl
return png;
}
function createFilledPng(width: number, height: number): PNG {
const png = new PNG({ width, height });
for (let y = 0; y < height; y += 1) {
for (let x = 0; x < width; x += 1) {
const index = (png.width * y + x) << 2;
png.data[index] = 12;
png.data[index + 1] = 34;
png.data[index + 2] = 56;
png.data[index + 3] = 255;
}
}
return png;
}
function createHugeHeaderPng(width: number, height: number): Buffer {
const buffer = Buffer.alloc(33);
Buffer.from([137, 80, 78, 71, 13, 10, 26, 10]).copy(buffer, 0);
buffer.writeUInt32BE(13, 8);
buffer.write('IHDR', 12, 'ascii');
buffer.writeUInt32BE(width, 16);
buffer.writeUInt32BE(height, 20);
buffer[24] = 8;
buffer[25] = 6;
buffer[26] = 0;
buffer[27] = 0;
buffer[28] = 0;
return buffer;
}
function redPixels(png: PNG): string[] {
const pixels: string[] = [];
for (let y = 0; y < png.height; y += 1) {

160
scripts/approve-fork-pr-workflows.test.ts
View file

@ -1,9 +1,11 @@
import assert from "node:assert/strict";
import { readFile } from "node:fs/promises";
import test from "node:test";
import {
hasPullApprovalStateDrift,
isAllowedChangedPath,
isAllowedVisualCaptureChangedPath,
isDeniedChangedPath,
isPendingApprovalRun,
listPendingApprovalRuns,
@ -11,6 +13,24 @@ import {
waitForPendingApprovalRuns,
} from "./approve-fork-pr-workflows.ts";
test("visual-pr-comment resolves empty workflow_run.pull_requests from trusted repo/branch metadata and leaves stale SHA handling to trusted-pr", async () => {
const workflow = await readFile(new URL("../.github/workflows/visual-pr-comment.yml", import.meta.url), "utf8");
const manifestStep = workflow.match(/- name: Read capture manifest[\s\S]*?- name: Validate live PR state for trusted checkout/u)?.[0];
const trustedPrStep = workflow.match(/- name: Validate live PR state for trusted checkout[\s\S]*?- name: Checkout trusted base/u)?.[0];
assert.ok(manifestStep, "Expected Read capture manifest step block in workflow");
assert.ok(trustedPrStep, "Expected trusted-pr validation step block in workflow");
assert.match(manifestStep, /encoded_head=.*\$head\|@uri/u);
assert.match(manifestStep, /pulls\?state=open&head=\$encoded_head&per_page=100/);
assert.match(manifestStep, /jq -r --arg repo "\$source_head_repository"/u);
assert.doesNotMatch(manifestStep, /select\(\.head\.sha == \$sha/u);
assert.match(manifestStep, /match_count=.*wc -w/u);
assert.match(manifestStep, /if \[ "\$match_count" -gt 1 \]; then/u);
assert.match(manifestStep, /found \$match_count matches/);
assert.match(manifestStep, /pr_number="\$manifest_pr_number"/u);
assert.match(trustedPrStep, /Skipping stale visual artifact for \$ARTIFACT_HEAD_SHA; current PR head is \$current_head\./u);
});
test("isPendingApprovalRun matches approval-gated fork PR runs from GitHub's captured payload shape", () => {
const pull = {
number: 2683,
@ -107,6 +127,23 @@ test("isPendingApprovalRun rejects runs outside the allowlist or without action_
false,
);
assert.equal(
isPendingApprovalRun(
{
id: 26273463771,
name: "Visual PR Capture",
event: "pull_request",
status: "completed",
conclusion: "action_required",
head_sha: "734076155c44e569304856590019cea54506fdab",
path: ".github/workflows/visual-pr-capture.yml@main",
pull_requests: [],
},
pull,
),
false,
);
assert.equal(
isPendingApprovalRun(
{
@ -125,6 +162,56 @@ test("isPendingApprovalRun rejects runs outside the allowlist or without action_
);
});
test("isPendingApprovalRun approves visual capture only for strict web source changes", () => {
const pull = {
number: 2683,
state: "open",
changed_files: 1,
head: {
ref: "fix/button-copy",
sha: "734076155c44e569304856590019cea54506fdab",
repo: { full_name: "someone/open-design" },
},
base: {
ref: "main",
sha: "4cd93a5c7a7b0db1961c854e55f8e0e6b1b45542",
repo: { full_name: "nexu-io/open-design" },
},
};
const run = {
id: 26273463771,
name: "Visual PR Capture",
event: "pull_request",
status: "completed",
conclusion: "action_required",
head_sha: "734076155c44e569304856590019cea54506fdab",
path: ".github/workflows/visual-pr-capture.yml@main",
pull_requests: [],
};
assert.equal(
isPendingApprovalRun(run, pull, [
{ filename: "apps/web/src/components/Button.tsx", status: "modified" },
{ filename: "apps/web/src/styles/button.css", status: "modified" },
]),
true,
);
assert.equal(
isPendingApprovalRun(run, pull, [{ filename: "apps/web/public/logo.png", status: "modified" }]),
false,
);
assert.equal(
isPendingApprovalRun(run, pull, [
{
filename: "apps/web/src/components/Button.tsx",
previous_filename: "scripts/build.ts",
status: "renamed",
},
]),
false,
);
});
test("runTargetsPullRequest accepts empty run.pull_requests only when the head SHA maps to this one open PR", () => {
const pull = {
number: 2683,
@ -517,6 +604,68 @@ test("listPendingApprovalRuns paginates all pull_request runs for the head SHA a
);
});
test("listPendingApprovalRuns applies strict changed-path filtering only to visual capture", async () => {
const pull = {
number: 2683,
state: "open",
changed_files: 1,
head: {
ref: "fix/readme-copy",
sha: "734076155c44e569304856590019cea54506fdab",
repo: { full_name: "someone/open-design" },
},
base: {
ref: "main",
sha: "4cd93a5c7a7b0db1961c854e55f8e0e6b1b45542",
repo: { full_name: "nexu-io/open-design" },
},
};
const workflowRuns = [
{
id: 26273463769,
name: "CI",
event: "pull_request",
head_branch: pull.head.ref,
head_repository: pull.head.repo,
status: "completed",
conclusion: "action_required",
head_sha: pull.head.sha,
path: ".github/workflows/ci.yml@main",
pull_requests: [],
},
{
id: 26273463770,
name: "Visual PR Capture",
event: "pull_request",
head_branch: pull.head.ref,
head_repository: pull.head.repo,
status: "completed",
conclusion: "action_required",
head_sha: pull.head.sha,
path: ".github/workflows/visual-pr-capture.yml@main",
pull_requests: [],
},
];
const deps = {
loadWorkflowRunsResponsePage: async () => ({ workflow_runs: workflowRuns }),
loadPullRequestsForHeadSha: async () => [pull],
};
assert.deepEqual(
(await listPendingApprovalRuns("nexu-io/open-design", pull, [{ filename: "README.md", status: "modified" }], deps)).map((run) => run.id),
[26273463769],
);
assert.deepEqual(
(await listPendingApprovalRuns(
"nexu-io/open-design",
pull,
[{ filename: "apps/web/src/components/Button.tsx", status: "modified" }],
deps,
)).map((run) => run.id),
[26273463769, 26273463770],
);
});
test("hasPullApprovalStateDrift ignores base tip churn but still rejects base retargeting and head drift", () => {
const pull = {
number: 2683,
@ -582,6 +731,17 @@ test("isAllowedChangedPath allows ordinary app and package source/test paths whi
assert.equal(isAllowedChangedPath("apps/packaged/vitest.config.ts"), false);
});
test("isAllowedVisualCaptureChangedPath is limited to web ts tsx and css source", () => {
assert.equal(isAllowedVisualCaptureChangedPath("apps/web/src/app/page.tsx"), true);
assert.equal(isAllowedVisualCaptureChangedPath("apps/web/src/lib/theme.ts"), true);
assert.equal(isAllowedVisualCaptureChangedPath("apps/web/src/components/Button.css"), true);
assert.equal(isAllowedVisualCaptureChangedPath("apps/web/src/assets/icon.svg"), false);
assert.equal(isAllowedVisualCaptureChangedPath("apps/web/public/logo.png"), false);
assert.equal(isAllowedVisualCaptureChangedPath("apps/web/package.json"), false);
assert.equal(isAllowedVisualCaptureChangedPath("apps/web/tests/Button.test.tsx"), false);
assert.equal(isAllowedVisualCaptureChangedPath("packages/contracts/src/api.ts"), false);
});
test("waitForPendingApprovalRuns retries until action_required runs appear and keeps polling through the retry window", async () => {
const run = {
id: 26273463769,

30
scripts/approve-fork-pr-workflows.ts
View file

@ -78,6 +78,8 @@ const allowedWorkflowPaths = new Set([
".github/workflows/visual-pr-verify.yml",
]);
const visualPrCaptureWorkflowPath = ".github/workflows/visual-pr-capture.yml";
export function normalizeWorkflowPath(path: string): string {
const suffixIndex = path.indexOf("@");
return suffixIndex >= 0 ? path.slice(0, suffixIndex) : path;
@ -114,6 +116,10 @@ export function isAllowedChangedPath(path: string): boolean {
);
}
export function isAllowedVisualCaptureChangedPath(path: string): boolean {
return /^apps\/web\/src\/.+\.(?:css|ts|tsx)$/.test(path);
}
export function isDeniedChangedPath(path: string): boolean {
return (
path.startsWith(".github/") ||
@ -151,12 +157,23 @@ function changedPathSet(file: PullRequestFile): string[] {
return [file.filename, file.previous_filename].filter((path): path is string => Boolean(path));
}
export function isPendingApprovalRun(run: WorkflowRun, pull: PullRequest): boolean {
function allChangedPathsMatch(files: PullRequestFile[], predicate: (path: string) => boolean): boolean {
return files.every((file) => changedPathSet(file).every(predicate));
}
function workflowAllowsChangedFiles(workflowPath: string, files: PullRequestFile[] | undefined): boolean {
if (workflowPath !== visualPrCaptureWorkflowPath) return true;
return files != null && files.length > 0 && allChangedPathsMatch(files, isAllowedVisualCaptureChangedPath);
}
export function isPendingApprovalRun(run: WorkflowRun, pull: PullRequest, files?: PullRequestFile[]): boolean {
const workflowPath = normalizeWorkflowPath(run.path);
return (
run.head_sha === pull.head.sha &&
run.event === "pull_request" &&
(run.status === "action_required" || run.conclusion === "action_required") &&
allowedWorkflowPaths.has(normalizeWorkflowPath(run.path))
allowedWorkflowPaths.has(workflowPath) &&
workflowAllowsChangedFiles(workflowPath, files)
);
}
@ -330,8 +347,11 @@ async function listWorkflowRunsForHeadSha(
export async function listPendingApprovalRuns(
repo: string,
pull: PullRequest,
deps: ListPendingApprovalRunsDeps = {},
filesOrDeps: PullRequestFile[] | ListPendingApprovalRunsDeps = [],
maybeDeps: ListPendingApprovalRunsDeps = {},
): Promise<WorkflowRun[]> {
const files = Array.isArray(filesOrDeps) ? filesOrDeps : undefined;
const deps = Array.isArray(filesOrDeps) ? maybeDeps : filesOrDeps;
const loadWorkflowRunsResponsePage = deps.loadWorkflowRunsResponsePage ?? ((path: string) => github<WorkflowRunsResponse>(path));
const loadPullRequestsForHeadSha =
deps.loadPullRequestsForHeadSha ?? ((currentRepo: string, headSha: string) => listPullRequestsForHeadSha(currentRepo, headSha));
@ -360,7 +380,7 @@ export async function listPendingApprovalRuns(
return workflowRuns.filter(
(run) =>
isPendingApprovalRun(run, pull) &&
isPendingApprovalRun(run, pull, files) &&
runTargetsPullRequest(run, pull, associatedPullsForHeadSha, associatedPullsForHeadRef),
);
}
@ -413,7 +433,7 @@ async function main(): Promise<void> {
return;
}
const pendingRuns = await waitForPendingApprovalRuns(() => listPendingApprovalRuns(repo, pull));
const pendingRuns = await waitForPendingApprovalRuns(() => listPendingApprovalRuns(repo, pull, files));
if (pendingRuns.length === 0) {
console.log(`No action_required pull_request workflow runs found for PR #${prNumber} at ${pull.head.sha}.`);