Skip to content

Repository navigation

Commit d4311b9

Browse files
ryoppippiryoppippi
committed
ci(github): tolerate unauthenticated comment writes
The rust perf comment job can produce a skipped benchmark summary when the base pkg.pr.new package is unavailable, but creating the new PR comment failed with a 401 from the workflow token. That made the job fail even though the benchmark script completed and wrote the job summary. Treat 401 the same as the existing 403 no-comment-permission path, keep other GitHub failures fatal, and add a small in-source Vitest project for this script so the auth classification is covered.
1 parent c5cf51e commit d4311b9

2 files changed

Lines changed: 59 additions & 27 deletions

File tree

‎.github/scripts/upsert-pr-comment.ts‎

Lines changed: 50 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -52,23 +52,18 @@ async function githubRequest<T>(path: string, options: RequestInit = {}): Promis
5252
return response.json() as Promise<T>;
5353
}
5454

55-
const repository = requiredEnv('GITHUB_REPOSITORY');
56-
const prNumber = requiredEnv('PR_NUMBER');
57-
const marker = requiredEnv('COMMENT_MARKER');
58-
const body = await Bun.file(requiredEnv('COMMENT_FILE')).text();
59-
60-
async function createComment(): Promise<void> {
55+
async function createComment(repository: string, prNumber: string, body: string): Promise<void> {
6156
await githubRequest(`/repos/${repository}/issues/${prNumber}/comments`, {
6257
method: 'POST',
6358
body: JSON.stringify({ body }),
6459
});
6560
}
6661

67-
async function tryCreateComment(): Promise<void> {
62+
async function tryCreateComment(repository: string, prNumber: string, body: string): Promise<void> {
6863
try {
69-
await createComment();
64+
await createComment(repository, prNumber, body);
7065
} catch (error) {
71-
if (error instanceof GitHubRequestError && error.status === 403) {
66+
if (isCommentWriteAuthFailure(error)) {
7267
console.warn(
7368
`Skipping PR comment because GitHub token cannot write comments: ${error.message}`,
7469
);
@@ -78,24 +73,52 @@ async function tryCreateComment(): Promise<void> {
7873
}
7974
}
8075

81-
const comments = await githubRequest<GitHubComment[]>(
82-
`/repos/${repository}/issues/${prNumber}/comments?per_page=100`,
83-
);
84-
const existing = comments.find(
85-
(comment) => comment.user?.login === 'github-actions[bot]' && comment.body?.includes(marker),
86-
);
76+
function isCommentWriteAuthFailure(error: unknown): error is GitHubRequestError {
77+
return error instanceof GitHubRequestError && (error.status === 401 || error.status === 403);
78+
}
8779

88-
if (existing == null) {
89-
await tryCreateComment();
90-
} else {
91-
try {
92-
await githubRequest(`/repos/${repository}/issues/comments/${existing.id}`, {
93-
method: 'PATCH',
94-
body: JSON.stringify({ body }),
95-
});
96-
} catch (error) {
97-
console.warn(`Failed to update existing PR comment; creating a new comment instead.`);
98-
console.warn(error);
99-
await tryCreateComment();
80+
async function main(): Promise<void> {
81+
const repository = requiredEnv('GITHUB_REPOSITORY');
82+
const prNumber = requiredEnv('PR_NUMBER');
83+
const marker = requiredEnv('COMMENT_MARKER');
84+
const body = await Bun.file(requiredEnv('COMMENT_FILE')).text();
85+
const comments = await githubRequest<GitHubComment[]>(
86+
`/repos/${repository}/issues/${prNumber}/comments?per_page=100`,
87+
);
88+
const existing = comments.find(
89+
(comment) => comment.user?.login === 'github-actions[bot]' && comment.body?.includes(marker),
90+
);
91+
92+
if (existing == null) {
93+
await tryCreateComment(repository, prNumber, body);
94+
} else {
95+
try {
96+
await githubRequest(`/repos/${repository}/issues/comments/${existing.id}`, {
97+
method: 'PATCH',
98+
body: JSON.stringify({ body }),
99+
});
100+
} catch (error) {
101+
console.warn(`Failed to update existing PR comment; creating a new comment instead.`);
102+
console.warn(error);
103+
await tryCreateComment(repository, prNumber, body);
104+
}
100105
}
101106
}
107+
108+
if (import.meta.vitest != null) {
109+
describe(isCommentWriteAuthFailure, () => {
110+
it('treats unauthenticated comment writes as skippable', () => {
111+
expect(isCommentWriteAuthFailure(new GitHubRequestError('unauthenticated', 401))).toBe(true);
112+
});
113+
114+
it('treats forbidden comment writes as skippable', () => {
115+
expect(isCommentWriteAuthFailure(new GitHubRequestError('forbidden', 403))).toBe(true);
116+
});
117+
118+
it('keeps other GitHub errors fatal', () => {
119+
expect(isCommentWriteAuthFailure(new GitHubRequestError('server error', 500))).toBe(false);
120+
});
121+
});
122+
} else {
123+
await main();
124+
}

‎vitest.config.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,15 @@ export default defineConfig({
1818
globals: true,
1919
},
2020
},
21+
{
22+
test: {
23+
name: 'github-scripts',
24+
include: [],
25+
includeSource: ['.github/scripts/upsert-pr-comment.ts'],
26+
exclude: ['**/node_modules/**', '**/.git/**', '.direnv/**'],
27+
globals: true,
28+
},
29+
},
2130
],
2231
},
2332
});

0 commit comments

Comments
 (0)