Skip to content

ci: tolerate perf comment write failures #1013

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ryoppippi merged 1 commit into from
May 17, 2026
+47 −9
Merged

ci: tolerate perf comment write failures #1013

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 44 additions & 9 deletions .github/scripts/upsert-pr-comment.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,16 @@ type GitHubComment = {
};
};

class GitHubRequestError extends Error {
constructor(
message: string,
readonly status: number,
) {
super(message);
this.name = 'GitHubRequestError';
}
}

function requiredEnv(name: string): string {
const value = Bun.env[name];
if (value == null || value.length === 0) {
Expand All @@ -29,8 +39,9 @@ async function githubRequest<T>(path: string, options: RequestInit = {}): Promis
});

if (!response.ok) {
throw new Error(
throw new GitHubRequestError(
`${options.method ?? 'GET'} ${path} failed: ${response.status} ${await response.text()}`,
response.status,
);
}

Expand All @@ -46,6 +57,27 @@ const prNumber = requiredEnv('PR_NUMBER');
const marker = requiredEnv('COMMENT_MARKER');
const body = await Bun.file(requiredEnv('COMMENT_FILE')).text();

async function createComment(): Promise<void> {
await githubRequest(`/repos/${repository}/issues/${prNumber}/comments`, {
method: 'POST',
body: JSON.stringify({ body }),
});
}

async function tryCreateComment(): Promise<void> {
try {
await createComment();
} catch (error) {
if (error instanceof GitHubRequestError && error.status === 403) {
console.warn(
`Skipping PR comment because GitHub token cannot write comments: ${error.message}`,
);
return;
}
throw error;
}
}

const comments = await githubRequest<GitHubComment[]>(
`/repos/${repository}/issues/${prNumber}/comments?per_page=100`,
);
Expand All @@ -54,13 +86,16 @@ const existing = comments.find(
);

if (existing == null) {
await githubRequest(`/repos/${repository}/issues/${prNumber}/comments`, {
method: 'POST',
body: JSON.stringify({ body }),
});
await tryCreateComment();
} else {
await githubRequest(`/repos/${repository}/issues/comments/${existing.id}`, {
method: 'PATCH',
body: JSON.stringify({ body }),
});
try {
await githubRequest(`/repos/${repository}/issues/comments/${existing.id}`, {
method: 'PATCH',
body: JSON.stringify({ body }),
});
} catch (error) {
console.warn(`Failed to update existing PR comment; creating a new comment instead.`);
console.warn(error);
await tryCreateComment();
Comment on lines +91 to +99
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Avoid posting a new marker comment after ambiguous update failures.

This catch-all will create a fresh comment even when the PATCH may already have succeeded server-side and only the response handling failed locally. That can leave duplicate marker comments on the PR, and later runs may keep updating the oldest one while stale copies remain visible.

Safer fallback 
 	} catch (error) {
-		console.warn(`Failed to update existing PR comment; creating a new comment instead.`);
-		console.warn(error);
-		await tryCreateComment();
+		if (error instanceof GitHubRequestError && error.status === 404) {
+			console.warn(
+				'Existing PR comment no longer exists; creating a new comment instead.',
+			);
+			await tryCreateComment();
+			return;
+		}
+		throw error;
 	}
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/scripts/upsert-pr-comment.ts around lines 91 - 99, The catch-all in
the PATCH block (where githubRequest is called to update
`/issues/comments/${existing.id}`) should not unconditionally call
tryCreateComment on any local error; instead detect whether the update actually
failed server-side before creating a new marker: on error, re-fetch the comment
via githubRequest GET `/issues/comments/${existing.id}` (or inspect the PATCH
response status if available) and only call tryCreateComment when the GET
returns 404/410 or the PATCH returned a clear non-success HTTP status; if the
re-fetch shows the comment exists or the body matches the desired content, treat
the update as successful and do not create a new comment—apply this logic around
the existing githubRequest PATCH and tryCreateComment calls to avoid duplicate
marker comments.

}
}
3 changes: 3 additions & 0 deletions .github/workflows/ccusage-perf.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@ jobs:
--large-runs 1 \
--output "$RUNNER_TEMP/ccusage-perf-comment.md"
- name: Write job summary
run: cat "$RUNNER_TEMP/ccusage-perf-comment.md" >> "$GITHUB_STEP_SUMMARY"

- name: Upsert PR comment
env:
COMMENT_FILE: ${{ runner.temp }}/ccusage-perf-comment.md
Expand Down
Loading