mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-03-08 06:30:04 +00:00
fix: split perf report workflow for fork PR support (#9382)
## Summary Perf report workflow fails on fork PRs because `GITHUB_TOKEN` is read-only for forks, causing "Resource not accessible by integration" on the PR comment step. ## Changes - **What**: Split `ci-perf-report.yaml` into a data-collection workflow + a `workflow_run`-triggered reporter (`pr-perf-report.yaml`), matching the existing `ci-size-data`/`pr-size-report` pattern. Added fork PR permissions guidance to `.github/AGENTS.md`. - **ci-perf-report.yaml**: Removed the `report` job and `pull-requests: write` permission. Added PR metadata (number + base branch) artifact upload. - **pr-perf-report.yaml** (new): Triggered by `workflow_run` on the perf workflow. Downloads metrics + metadata artifacts, generates report, posts PR comment with write permissions from the default-branch context. ## Review Focus - The two-workflow split follows the same pattern as `ci-size-data.yaml` → `pr-size-report.yaml`, which already works for fork PRs. - The `workflow_run` trigger runs in the base repo context per [GitHub Security Lab guidance](https://securitylab.github.com/resources/github-actions-preventing-pwn-requests/), so it safely has write permissions even for fork PRs. - AGENTS.md guidance documents this pattern to prevent recurrence. Fixes the failure seen in https://github.com/Comfy-Org/ComfyUI_frontend/actions/runs/22684230751/job/65763595989?pr=9380 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9382-fix-split-perf-report-workflow-for-fork-PR-support-3196d73d365081b29b35ed354e7789e2) by [Unito](https://www.unito.io)
This commit is contained in:
8
.github/AGENTS.md
vendored
8
.github/AGENTS.md
vendored
@@ -13,3 +13,11 @@ This automated review performs comprehensive analysis:
|
||||
- Integration concerns
|
||||
|
||||
For implementation details, see `.claude/commands/comprehensive-pr-review.md`.
|
||||
|
||||
## GitHub Actions: Fork PR Permissions
|
||||
|
||||
Fork PRs get a **read-only `GITHUB_TOKEN`** — no PR comments, no secret access, no pushing.
|
||||
|
||||
Any workflow that needs write access must use the **two-workflow split**: a `pull_request`-triggered `ci-*.yaml` uploads artifacts (including PR metadata), then a `workflow_run`-triggered `pr-*.yaml` downloads them and posts comments with write permissions. See `ci-size-data` → `pr-size-report` or `ci-perf-report` → `pr-perf-report`. Use `.github/actions/post-pr-report-comment` for the comment step.
|
||||
|
||||
Never write PR comments directly from `pull_request` workflows or use `pull_request_target` to run untrusted code.
|
||||
|
||||
35
.github/actions/post-pr-report-comment/action.yaml
vendored
Normal file
35
.github/actions/post-pr-report-comment/action.yaml
vendored
Normal file
@@ -0,0 +1,35 @@
|
||||
name: Post PR Report Comment
|
||||
description: Reads a markdown report file and posts/updates a single idempotent comment on a PR.
|
||||
|
||||
inputs:
|
||||
pr-number:
|
||||
description: PR number to comment on
|
||||
required: true
|
||||
report-file:
|
||||
description: Path to the markdown report file
|
||||
required: true
|
||||
comment-marker:
|
||||
description: HTML comment marker for idempotent matching
|
||||
required: true
|
||||
token:
|
||||
description: GitHub token with pull-requests write permission
|
||||
required: true
|
||||
|
||||
runs:
|
||||
using: composite
|
||||
steps:
|
||||
- name: Read report
|
||||
id: report
|
||||
uses: juliangruber/read-file-action@b549046febe0fe86f8cb4f93c24e284433f9ab58 # v1.1.7
|
||||
with:
|
||||
path: ${{ inputs.report-file }}
|
||||
|
||||
- name: Create or update PR comment
|
||||
uses: actions-cool/maintain-one-comment@4b2dbf086015f892dcb5e8c1106f5fccd6c1476b # v3.2.0
|
||||
with:
|
||||
token: ${{ inputs.token }}
|
||||
number: ${{ inputs.pr-number }}
|
||||
body: |
|
||||
${{ steps.report.outputs.content }}
|
||||
${{ inputs.comment-marker }}
|
||||
body-include: ${{ inputs.comment-marker }}
|
||||
86
.github/workflows/ci-perf-report.yaml
vendored
86
.github/workflows/ci-perf-report.yaml
vendored
@@ -14,7 +14,6 @@ concurrency:
|
||||
|
||||
permissions:
|
||||
contents: read
|
||||
pull-requests: write
|
||||
|
||||
jobs:
|
||||
perf-tests:
|
||||
@@ -56,81 +55,16 @@ jobs:
|
||||
retention-days: 30
|
||||
if-no-files-found: warn
|
||||
|
||||
report:
|
||||
needs: perf-tests
|
||||
if: github.event_name == 'pull_request'
|
||||
runs-on: ubuntu-latest
|
||||
permissions:
|
||||
actions: read
|
||||
contents: read
|
||||
pull-requests: write
|
||||
|
||||
steps:
|
||||
- name: Checkout repository
|
||||
uses: actions/checkout@v6
|
||||
|
||||
- name: Setup Node
|
||||
uses: actions/setup-node@v6
|
||||
with:
|
||||
node-version: 22
|
||||
|
||||
- name: Download PR perf metrics
|
||||
continue-on-error: true
|
||||
uses: actions/download-artifact@v7
|
||||
with:
|
||||
name: perf-metrics
|
||||
path: test-results/
|
||||
|
||||
- name: Download baseline perf metrics
|
||||
uses: dawidd6/action-download-artifact@0bd50d53a6d7fb5cb921e607957e9cc12b4ce392 # v12
|
||||
with:
|
||||
branch: ${{ github.event.pull_request.base.ref }}
|
||||
workflow: ci-perf-report.yaml
|
||||
event: push
|
||||
name: perf-metrics
|
||||
path: temp/perf-baseline/
|
||||
if_no_artifact_found: warn
|
||||
|
||||
- name: Download historical perf baselines
|
||||
continue-on-error: true
|
||||
- name: Save PR metadata
|
||||
if: github.event_name == 'pull_request'
|
||||
run: |
|
||||
RUNS=$(gh api \
|
||||
"/repos/${{ github.repository }}/actions/workflows/ci-perf-report.yaml/runs?branch=${{ github.event.pull_request.base.ref }}&event=push&status=success&per_page=5" \
|
||||
--jq '.workflow_runs[].id' || true)
|
||||
mkdir -p temp/perf-meta
|
||||
echo "${{ github.event.number }}" > temp/perf-meta/number.txt
|
||||
echo "${{ github.event.pull_request.base.ref }}" > temp/perf-meta/base.txt
|
||||
|
||||
if [ -z "$RUNS" ]; then
|
||||
echo "No historical runs available"
|
||||
exit 0
|
||||
fi
|
||||
|
||||
mkdir -p temp/perf-history
|
||||
INDEX=0
|
||||
for RUN_ID in $RUNS; do
|
||||
DIR="temp/perf-history/$INDEX"
|
||||
mkdir -p "$DIR"
|
||||
gh run download "$RUN_ID" -n perf-metrics -D "$DIR/" 2>/dev/null || true
|
||||
INDEX=$((INDEX + 1))
|
||||
done
|
||||
|
||||
echo "Downloaded $(ls temp/perf-history/*/perf-metrics.json 2>/dev/null | wc -l) historical baselines"
|
||||
env:
|
||||
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
|
||||
|
||||
- name: Generate perf report
|
||||
run: npx --yes tsx scripts/perf-report.ts > perf-report.md
|
||||
|
||||
- name: Read perf report
|
||||
id: perf-report
|
||||
uses: juliangruber/read-file-action@b549046febe0fe86f8cb4f93c24e284433f9ab58 # v1.1.7
|
||||
- name: Upload PR metadata
|
||||
if: github.event_name == 'pull_request'
|
||||
uses: actions/upload-artifact@v6
|
||||
with:
|
||||
path: ./perf-report.md
|
||||
|
||||
- name: Create or update PR comment
|
||||
uses: actions-cool/maintain-one-comment@4b2dbf086015f892dcb5e8c1106f5fccd6c1476b # v3.2.0
|
||||
with:
|
||||
token: ${{ secrets.GITHUB_TOKEN }}
|
||||
number: ${{ github.event.pull_request.number }}
|
||||
body: |
|
||||
${{ steps.perf-report.outputs.content }}
|
||||
<!-- COMFYUI_FRONTEND_PERF -->
|
||||
body-include: '<!-- COMFYUI_FRONTEND_PERF -->'
|
||||
name: perf-meta
|
||||
path: temp/perf-meta/
|
||||
|
||||
102
.github/workflows/pr-perf-report.yaml
vendored
Normal file
102
.github/workflows/pr-perf-report.yaml
vendored
Normal file
@@ -0,0 +1,102 @@
|
||||
name: 'PR: Performance Report'
|
||||
|
||||
on:
|
||||
workflow_run:
|
||||
workflows: ['CI: Performance Report']
|
||||
types:
|
||||
- completed
|
||||
|
||||
permissions:
|
||||
contents: read
|
||||
pull-requests: write
|
||||
issues: write
|
||||
|
||||
jobs:
|
||||
comment:
|
||||
runs-on: ubuntu-latest
|
||||
if: >
|
||||
github.repository == 'Comfy-Org/ComfyUI_frontend' &&
|
||||
github.event.workflow_run.event == 'pull_request' &&
|
||||
github.event.workflow_run.conclusion == 'success'
|
||||
steps:
|
||||
- name: Checkout repository
|
||||
uses: actions/checkout@v6
|
||||
|
||||
- name: Setup Node
|
||||
uses: actions/setup-node@v6
|
||||
with:
|
||||
node-version: 22
|
||||
|
||||
- name: Download PR metadata
|
||||
uses: dawidd6/action-download-artifact@0bd50d53a6d7fb5cb921e607957e9cc12b4ce392 # v12
|
||||
with:
|
||||
name: perf-meta
|
||||
run_id: ${{ github.event.workflow_run.id }}
|
||||
path: temp/perf-meta/
|
||||
|
||||
- name: Resolve and validate PR metadata
|
||||
id: pr-meta
|
||||
uses: actions/github-script@v8
|
||||
with:
|
||||
script: |
|
||||
const fs = require('fs');
|
||||
const artifactPr = Number(fs.readFileSync('temp/perf-meta/number.txt', 'utf8').trim());
|
||||
const artifactBase = fs.readFileSync('temp/perf-meta/base.txt', 'utf8').trim();
|
||||
|
||||
// Resolve PR from trusted workflow context
|
||||
let pr = context.payload.workflow_run.pull_requests?.[0];
|
||||
if (!pr) {
|
||||
const { data: prs } = await github.rest.repos.listPullRequestsAssociatedWithCommit({
|
||||
owner: context.repo.owner,
|
||||
repo: context.repo.repo,
|
||||
commit_sha: context.payload.workflow_run.head_sha,
|
||||
});
|
||||
pr = prs.find(p => p.state === 'open');
|
||||
}
|
||||
|
||||
if (!pr) {
|
||||
core.setFailed('Unable to resolve PR from workflow_run context.');
|
||||
return;
|
||||
}
|
||||
|
||||
if (Number(pr.number) !== artifactPr) {
|
||||
core.setFailed(`Artifact PR number (${artifactPr}) does not match trusted context (${pr.number}).`);
|
||||
return;
|
||||
}
|
||||
|
||||
const trustedBase = pr.base?.ref;
|
||||
if (!trustedBase || artifactBase !== trustedBase) {
|
||||
core.setFailed(`Artifact base (${artifactBase}) does not match trusted context (${trustedBase}).`);
|
||||
return;
|
||||
}
|
||||
|
||||
core.setOutput('number', String(pr.number));
|
||||
core.setOutput('base', trustedBase);
|
||||
|
||||
- name: Download PR perf metrics
|
||||
uses: dawidd6/action-download-artifact@0bd50d53a6d7fb5cb921e607957e9cc12b4ce392 # v12
|
||||
with:
|
||||
name: perf-metrics
|
||||
run_id: ${{ github.event.workflow_run.id }}
|
||||
path: test-results/
|
||||
|
||||
- name: Download baseline perf metrics
|
||||
uses: dawidd6/action-download-artifact@0bd50d53a6d7fb5cb921e607957e9cc12b4ce392 # v12
|
||||
with:
|
||||
branch: ${{ steps.pr-meta.outputs.base }}
|
||||
workflow: ci-perf-report.yaml
|
||||
event: push
|
||||
name: perf-metrics
|
||||
path: temp/perf-baseline/
|
||||
if_no_artifact_found: warn
|
||||
|
||||
- name: Generate perf report
|
||||
run: npx --yes tsx scripts/perf-report.ts > perf-report.md
|
||||
|
||||
- name: Post PR comment
|
||||
uses: ./.github/actions/post-pr-report-comment
|
||||
with:
|
||||
pr-number: ${{ steps.pr-meta.outputs.number }}
|
||||
report-file: ./perf-report.md
|
||||
comment-marker: '<!-- COMFYUI_FRONTEND_PERF -->'
|
||||
token: ${{ secrets.GITHUB_TOKEN }}
|
||||
100
.github/workflows/pr-size-report.yaml
vendored
100
.github/workflows/pr-size-report.yaml
vendored
@@ -45,28 +45,76 @@ jobs:
|
||||
run_id: ${{ github.event_name == 'workflow_dispatch' && inputs.run_id || github.event.workflow_run.id }}
|
||||
path: temp/size
|
||||
|
||||
- name: Set PR number
|
||||
id: pr-number
|
||||
run: |
|
||||
if [ "${{ github.event_name }}" == "workflow_dispatch" ]; then
|
||||
echo "content=${{ inputs.pr_number }}" >> $GITHUB_OUTPUT
|
||||
else
|
||||
echo "content=$(cat temp/size/number.txt)" >> $GITHUB_OUTPUT
|
||||
fi
|
||||
- name: Resolve and validate PR metadata
|
||||
id: pr-meta
|
||||
uses: actions/github-script@v8
|
||||
with:
|
||||
script: |
|
||||
const fs = require('fs');
|
||||
|
||||
- name: Set base branch
|
||||
id: pr-base
|
||||
run: |
|
||||
if [ "${{ github.event_name }}" == "workflow_dispatch" ]; then
|
||||
echo "content=main" >> $GITHUB_OUTPUT
|
||||
else
|
||||
echo "content=$(cat temp/size/base.txt)" >> $GITHUB_OUTPUT
|
||||
fi
|
||||
// workflow_dispatch: validate artifact metadata against API-resolved PR
|
||||
if (context.eventName === 'workflow_dispatch') {
|
||||
const pullNumber = Number('${{ inputs.pr_number }}');
|
||||
const { data: dispatchPr } = await github.rest.pulls.get({
|
||||
owner: context.repo.owner,
|
||||
repo: context.repo.repo,
|
||||
pull_number: pullNumber,
|
||||
});
|
||||
|
||||
const artifactPr = Number(fs.readFileSync('temp/size/number.txt', 'utf8').trim());
|
||||
const artifactBase = fs.readFileSync('temp/size/base.txt', 'utf8').trim();
|
||||
|
||||
if (artifactPr !== dispatchPr.number) {
|
||||
core.setFailed(`Artifact PR number (${artifactPr}) does not match dispatch PR (${dispatchPr.number}).`);
|
||||
return;
|
||||
}
|
||||
if (artifactBase !== dispatchPr.base.ref) {
|
||||
core.setFailed(`Artifact base (${artifactBase}) does not match dispatch PR base (${dispatchPr.base.ref}).`);
|
||||
return;
|
||||
}
|
||||
|
||||
core.setOutput('number', String(dispatchPr.number));
|
||||
core.setOutput('base', dispatchPr.base.ref);
|
||||
return;
|
||||
}
|
||||
|
||||
// workflow_run: validate artifact metadata against trusted context
|
||||
const artifactPr = Number(fs.readFileSync('temp/size/number.txt', 'utf8').trim());
|
||||
const artifactBase = fs.readFileSync('temp/size/base.txt', 'utf8').trim();
|
||||
|
||||
let pr = context.payload.workflow_run.pull_requests?.[0];
|
||||
if (!pr) {
|
||||
const { data: prs } = await github.rest.repos.listPullRequestsAssociatedWithCommit({
|
||||
owner: context.repo.owner,
|
||||
repo: context.repo.repo,
|
||||
commit_sha: context.payload.workflow_run.head_sha,
|
||||
});
|
||||
pr = prs.find(p => p.state === 'open');
|
||||
}
|
||||
|
||||
if (!pr) {
|
||||
core.setFailed('Unable to resolve PR from workflow_run context.');
|
||||
return;
|
||||
}
|
||||
|
||||
if (Number(pr.number) !== artifactPr) {
|
||||
core.setFailed(`Artifact PR number (${artifactPr}) does not match trusted context (${pr.number}).`);
|
||||
return;
|
||||
}
|
||||
|
||||
const trustedBase = pr.base?.ref;
|
||||
if (!trustedBase || artifactBase !== trustedBase) {
|
||||
core.setFailed(`Artifact base (${artifactBase}) does not match trusted context (${trustedBase}).`);
|
||||
return;
|
||||
}
|
||||
|
||||
core.setOutput('number', String(pr.number));
|
||||
core.setOutput('base', trustedBase);
|
||||
|
||||
- name: Download previous size data
|
||||
uses: dawidd6/action-download-artifact@0bd50d53a6d7fb5cb921e607957e9cc12b4ce392 # v12
|
||||
with:
|
||||
branch: ${{ steps.pr-base.outputs.content }}
|
||||
branch: ${{ steps.pr-meta.outputs.base }}
|
||||
workflow: ci-size-data.yaml
|
||||
event: push
|
||||
name: size-data
|
||||
@@ -76,18 +124,10 @@ jobs:
|
||||
- name: Generate size report
|
||||
run: node scripts/size-report.js > size-report.md
|
||||
|
||||
- name: Read size report
|
||||
id: size-report
|
||||
uses: juliangruber/read-file-action@b549046febe0fe86f8cb4f93c24e284433f9ab58 # v1.1.7
|
||||
with:
|
||||
path: ./size-report.md
|
||||
|
||||
- name: Create or update PR comment
|
||||
uses: actions-cool/maintain-one-comment@4b2dbf086015f892dcb5e8c1106f5fccd6c1476b # v3.2.0
|
||||
- name: Post PR comment
|
||||
uses: ./.github/actions/post-pr-report-comment
|
||||
with:
|
||||
pr-number: ${{ steps.pr-meta.outputs.number }}
|
||||
report-file: ./size-report.md
|
||||
comment-marker: '<!-- COMFYUI_FRONTEND_SIZE -->'
|
||||
token: ${{ secrets.GITHUB_TOKEN }}
|
||||
number: ${{ steps.pr-number.outputs.content }}
|
||||
body: |
|
||||
${{ steps.size-report.outputs.content }}
|
||||
<!-- COMFYUI_FRONTEND_SIZE -->
|
||||
body-include: '<!-- COMFYUI_FRONTEND_SIZE -->'
|
||||
|
||||
Reference in New Issue
Block a user