fix: stop Heroku workflow from failing on every issue comment - #1008
Closed
rainhead wants to merge 1 commit into
Closed
fix: stop Heroku workflow from failing on every issue comment#1008rainhead wants to merge 1 commit into
rainhead wants to merge 1 commit into
Conversation
The `issue_comment` trigger existed so that commenting
`/refresh-heroku-status` on a PR would re-post the Heroku status checks.
It never worked, and it fired on every comment in the repo.
The `deployment-status` job's guard read:
github.event.deployment_status.state != 'pending' || (...)
On an `issue_comment` event that field is empty, so `'' != 'pending'` is
true and the job ran regardless of the comment body. It then built the
API route from `github.event.deployment.sha`, which is likewise empty on
a comment event, producing `POST /repos/orcasound/orcasite/statuses/`
and a 404. Across the last 200 runs, all 144 `issue_comment` runs failed
and all 56 `deployment_status` runs succeeded.
The feature is not salvageable as written: a comment payload carries no
deployment context to forward. Drop the trigger and both
`/refresh-heroku-status` clauses, leaving the working `deployment_status`
path alone.
Also bump the pinned actions, which were behind and triggering Node 20
deprecation warnings:
- step-security/harden-runner v2.12.2 -> v2.20.0
- octokit/request-action v2.4.0 -> v3.0.0
- jtalk/url-health-check-action v4 -> v5
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Aef29UGmzgktdmpr9tFiCz
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe Heroku workflow now processes only non-pending deployment statuses, removes manual refresh handling, and updates pinned runner, health-check, and deployment-status actions. ChangesHeroku deployment workflow
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Collaborator
Author
|
Folded into #999. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The
Herokuworkflow fails on every comment posted anywhere in the repo — e.g. this run.The
issue_commenttrigger was there so that commenting/refresh-heroku-statuson a PR would re-post the Heroku status checks. It never worked, and the guard didn't actually gate on the magic phrase:On an
issue_commenteventgithub.event.deployment_status.stateis empty, and'' != 'pending'is true — so the first branch short-circuits and the job runs on any comment. It then builds the API route fromgithub.event.deployment.sha, also empty on a comment event, yieldingPOST /repos/orcasound/orcasite/statuses/→404 Not Found.Across the last 200 runs:
deployment_statusissue_commentZero successes on the comment path, ever.
Fix
Removed the
issue_commenttrigger and both/refresh-heroku-statusclauses. The feature isn't salvageable as written — a comment payload carries no deployment context to forward, so making it work would mean resolving the PR head SHA and then querying the deployments API for its latest status. Given nobody has had a working version of this to miss, deleting it seemed better than building it.The
deployment_statuspath — which works, and is still in active use for PR review apps — is untouched.Drive-by
Bumped the pinned actions, which were behind and producing the Node 20 deprecation warnings on this workflow:
step-security/harden-runnerv2.12.2 → v2.20.0octokit/request-actionv2.4.0 → v3.0.0 (node24 runner)jtalk/url-health-check-actionv4 → v5 (node24 runner; the inputs used here are unchanged in v5)One cosmetic warning will remain:
Unexpected input(s) 'repository', 'state', ....request-action'saction.ymlonly declaresrouteandmediaType, but the action reads everyINPUT_*as a request parameter — that's by design, and it's why the 56 real runs succeeded.Testing
deployment_statuscan't be exercised from a PR branch, since the workflow that runs is the one on the target branch. Thedeployment_statusjob is byte-identical apart from the action SHAs, and the removed conditions only ever affectedissue_commentruns. The real check is that this PR's own comments no longer spawn failingHerokuruns once merged.🤖 Generated with Claude Code
https://claude.ai/code/session_01Aef29UGmzgktdmpr9tFiCz
Summary by CodeRabbit
Bug Fixes
Chores