Skip to content

Commit 62b361a

Browse files
committed
fix: split benchmark PR comment into workflow_run for fork permissions
The benchmark workflow's Comment on PR step fails with 403 'Resource not accessible by integration' on fork PRs because the GITHUB_TOKEN from pull_request events has read-only access. Split the commenting logic into a separate workflow triggered by workflow_run, which runs in the base repo context and has write permissions to post comments on fork PRs.
1 parent b2f757e commit 62b361a

2 files changed

Lines changed: 86 additions & 52 deletions

File tree

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
name: Benchmark Comment
2+
3+
on:
4+
workflow_run:
5+
workflows: ["Benchmark"]
6+
types:
7+
- completed
8+
9+
permissions:
10+
pull-requests: write
11+
12+
jobs:
13+
comment:
14+
runs-on: ubuntu-24.04
15+
if: github.event.workflow_run.event == 'pull_request'
16+
steps:
17+
- name: Download benchmark results
18+
uses: actions/download-artifact@v4
19+
with:
20+
name: benchmark-results
21+
path: benchmark-results/
22+
run-id: ${{ github.event.workflow_run.id }}
23+
github-token: ${{ secrets.GITHUB_TOKEN }}
24+
25+
- name: Comment on PR
26+
uses: actions/github-script@v7
27+
with:
28+
script: |
29+
const fs = require('fs');
30+
31+
const prNumber = parseInt(fs.readFileSync('benchmark-results/pr_number.txt', 'utf8').trim());
32+
const summaryContent = fs.readFileSync('benchmark-results/summary.md', 'utf8');
33+
34+
const body = `### 📊 Benchmark Comparison
35+
36+
${summaryContent}
37+
38+
<details>
39+
<summary>Benchmark Details</summary>
40+
41+
- Baseline: \`main\` branch
42+
- Comparison: This PR
43+
- Threshold: 10% regression triggers warning
44+
45+
</details>`;
46+
47+
// Find existing comment
48+
const { data: comments } = await github.rest.issues.listComments({
49+
owner: context.repo.owner,
50+
repo: context.repo.repo,
51+
issue_number: prNumber,
52+
});
53+
54+
const botComment = comments.find(comment =>
55+
comment.user.type === 'Bot' &&
56+
comment.body.includes('📊 Benchmark Comparison')
57+
);
58+
59+
if (botComment) {
60+
await github.rest.issues.updateComment({
61+
owner: context.repo.owner,
62+
repo: context.repo.repo,
63+
comment_id: botComment.id,
64+
body: body
65+
});
66+
} else {
67+
await github.rest.issues.createComment({
68+
owner: context.repo.owner,
69+
repo: context.repo.repo,
70+
issue_number: prNumber,
71+
body: body
72+
});
73+
}

‎.github/workflows/benchmark.yaml‎

Lines changed: 13 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ on:
1212

1313
permissions:
1414
contents: read
15-
pull-requests: write
1615

1716
env:
1817
CARGO_TERM_COLOR: always
@@ -42,7 +41,7 @@ jobs:
4241
- name: Run benchmarks on PR branch
4342
run: |
4443
cargo bench --bench e2e_body_forwarding -- --noplot --save-baseline pr
45-
44+
4645
- name: Checkout main branch
4746
uses: actions/checkout@v4
4847
with:
@@ -69,7 +68,7 @@ jobs:
6968
echo "\`\`\`" >> $GITHUB_STEP_SUMMARY
7069
critcmp main pr >> $GITHUB_STEP_SUMMARY
7170
echo "\`\`\`" >> $GITHUB_STEP_SUMMARY
72-
71+
7372
# Check for regressions > 10% by parsing critcmp output
7473
# critcmp outputs factors like 1.15x for 15% slower, we check for factors > 1.10
7574
COMPARISON=$(critcmp main pr --threshold 10 2>&1 || true)
@@ -90,56 +89,18 @@ jobs:
9089
echo "regression=false" >> $GITHUB_OUTPUT
9190
fi
9291
93-
- name: Comment on PR
94-
uses: actions/github-script@v7
92+
- name: Save benchmark results
93+
run: |
94+
mkdir -p benchmark-results
95+
cp $GITHUB_STEP_SUMMARY benchmark-results/summary.md
96+
echo "${{ github.event.pull_request.number }}" > benchmark-results/pr_number.txt
97+
echo "${{ steps.compare.outputs.regression }}" > benchmark-results/regression.txt
98+
99+
- name: Upload benchmark results
100+
uses: actions/upload-artifact@v4
95101
with:
96-
script: |
97-
const fs = require('fs');
98-
const summary = process.env.GITHUB_STEP_SUMMARY;
99-
100-
// Read the step summary
101-
const summaryContent = fs.readFileSync(summary, 'utf8');
102-
103-
const body = `### 📊 Benchmark Comparison
104-
105-
${summaryContent}
106-
107-
<details>
108-
<summary>Benchmark Details</summary>
109-
110-
- Baseline: \`main\` branch
111-
- Comparison: This PR
112-
- Threshold: 10% regression triggers warning
113-
114-
</details>`;
115-
116-
// Find existing comment
117-
const { data: comments } = await github.rest.issues.listComments({
118-
owner: context.repo.owner,
119-
repo: context.repo.repo,
120-
issue_number: context.issue.number,
121-
});
122-
123-
const botComment = comments.find(comment =>
124-
comment.user.type === 'Bot' &&
125-
comment.body.includes('📊 Benchmark Comparison')
126-
);
127-
128-
if (botComment) {
129-
await github.rest.issues.updateComment({
130-
owner: context.repo.owner,
131-
repo: context.repo.repo,
132-
comment_id: botComment.id,
133-
body: body
134-
});
135-
} else {
136-
await github.rest.issues.createComment({
137-
owner: context.repo.owner,
138-
repo: context.repo.repo,
139-
issue_number: context.issue.number,
140-
body: body
141-
});
142-
}
102+
name: benchmark-results
103+
path: benchmark-results/
143104

144105
- name: Fail if regression detected
145106
if: steps.compare.outputs.regression == 'true'

0 commit comments

Comments
 (0)