Skip to content

Fix artifact comment posting for fork PRs#103

Merged
LibretroAdmin merged 1 commit intolibretro:masterfrom
Provenance-Emu:libretro/fix/artifact-comments
Apr 3, 2026
Merged

Fix artifact comment posting for fork PRs#103
LibretroAdmin merged 1 commit intolibretro:masterfrom
Provenance-Emu:libretro/fix/artifact-comments

Conversation

@JoeMatt
Copy link
Copy Markdown
Collaborator

@JoeMatt JoeMatt commented Apr 3, 2026

Summary

  • The post-artifacts job in c-cpp.yml silently failed with a 403 error on all fork PRs because GITHUB_TOKEN is read-only for fork PR workflows (GitHub security policy)
  • Moved artifact comment logic to artifacts.yml using workflow_run event, which runs in the base repo context with write permissions
  • Fixed workflow name trigger ("Build""C/C++ CI") so the workflow actually fires
  • Replaced stale third-party tonyhallett/artifacts-url-comments@v1.1.0 with actions/github-script@v7
  • Added fallback PR lookup by head branch when pull_requests array is empty

Test plan

  • Merge this PR, then push a new commit to any open fork PR
  • Verify the artifact comment appears on the PR after CI completes
  • Verify the comment updates (not duplicates) on subsequent pushes

🤖 Generated with Claude Code

The post-artifacts job in c-cpp.yml ran in the PR workflow context,
where GITHUB_TOKEN is read-only for fork PRs (GitHub security policy).
This caused a silent 403 error, so artifact links were never posted.

Fix: move the comment logic to artifacts.yml which uses workflow_run
event — this runs in the base repo context with write permissions.
Also fix the workflow name trigger ("Build" -> "C/C++ CI") and replace
the stale third-party action with actions/github-script.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 3, 2026 07:28
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes artifact link comment posting for fork-based pull requests by moving the PR-commenting logic out of the PR-triggered workflow (read-only token on forks) into a workflow_run-triggered workflow that runs in the base repo context with write permissions.

Changes:

  • Removed the PR artifact-comment job (and its permissions block) from .github/workflows/c-cpp.yml.
  • Added .github/workflows/artifacts.yml to post/update a single “Build Artifacts” comment after successful C/C++ CI runs, including a fallback PR lookup when the run has no associated PRs.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
.github/workflows/c-cpp.yml Removes in-workflow PR commenting so fork PR runs no longer hit 403s.
.github/workflows/artifacts.yml Adds a base-repo workflow_run workflow to upsert artifact links onto the PR.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +30 to +31
const prs = run.pull_requests;
if (!prs || prs.length === 0) {
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

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

const prs = run.pull_requests; is followed by a guard that allows prs to be falsy (if (!prs || prs.length === 0)), but the fallback later calls prs.push(...). If run.pull_requests is ever undefined/null, this will throw and prevent artifact comments from being posted. Initialize a local array (e.g., let prs = run.pull_requests ?? []) and push into that (or assign prs = pulls).

Suggested change
const prs = run.pull_requests;
if (!prs || prs.length === 0) {
let prs = run.pull_requests ?? [];
if (prs.length === 0) {

Copilot uses AI. Check for mistakes.
@LibretroAdmin LibretroAdmin merged commit 68265f9 into libretro:master Apr 3, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants