Skip to content

fix: avoid reading comments from issue #0, if action is called w/o PR#508

Merged
rdhar merged 2 commits intoOP5dev:mainfrom
birjj:fix/avoid-gh-api-on-0
Sep 16, 2025
Merged

fix: avoid reading comments from issue #0, if action is called w/o PR#508
rdhar merged 2 commits intoOP5dev:mainfrom
birjj:fix/avoid-gh-api-on-0

Conversation

@birjj
Copy link
Copy Markdown
Contributor

@birjj birjj commented Sep 16, 2025

Previously the action would attempt to call gh api /repos/${{ github.repository }}/issues/${{ steps.identifier.outputs.pr }}/comments, even if steps.identifier.outputs.pr was 0. This would result in an HTTP 404 error if no relevant PR was detected by the action.

This PR fixes it by adding an early abort if no relevant PR is found. The old logic was:

  1. Fetch list of comments from PR, whatever it was (⚠️ this was the problem, attempting to fetch from PR 0)
  2. Set create_comment="true" if relevant, and if PR != 0.
  3. Delete the comment if relevant, and a comment existed (depends on step 0, which is why it's is placed where it is)
  4. Create/update comment

The fix is to simply skip all the steps if PR == 0 by inserting an early abort. I also removed the "if PR != 0" checks from the later steps, because those became redundant.

This was a regression from #503. This PR fixes #507

@birjj birjj marked this pull request as ready for review September 16, 2025 12:12
@birjj birjj requested a review from rdhar as a code owner September 16, 2025 12:12
@birjj
Copy link
Copy Markdown
Contributor Author

birjj commented Sep 16, 2025

I have validated that using the action from this PR fixes the gh: Not Found (HTTP 404) error I was getting. I have not yet validated that it still adds comments to PRs when it should.

@devantler
Copy link
Copy Markdown

I have validated that using the action from this PR fixes the gh: Not Found (HTTP 404) error I was getting. I have not yet validated that it still adds comments to PRs when it should.

And I have now validated that it still adds comments to PRs when it should.

@birjj
Copy link
Copy Markdown
Contributor Author

birjj commented Sep 16, 2025

To be more specific we have validated that the action still adds comments when changes are planned (when ran with comment-pr: on-diff, we haven't tested comment-pr: always), and that it still deletes them when no changes are planned (also in bug #506). It's not a super in-depth test, but I don't believe we've broken anything ;)

Copy link
Copy Markdown
Member

@rdhar rdhar left a comment

Choose a reason for hiding this comment

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

Thank you, this is a superb fix and more than happy to expedite given earlier context.

Totally a regression error on my part, and reinforces the need for more thorough CI tests.

@rdhar rdhar merged commit 8f1f86d into OP5dev:main Sep 16, 2025
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.

Regression: Action can no longer be used for non-PR triggers

3 participants