Ignore non-created issue comment events in prow plugins#5170
Open
petr-muller wants to merge 4 commits intoopenshift:mainfrom
Open
Ignore non-created issue comment events in prow plugins#5170petr-muller wants to merge 4 commits intoopenshift:mainfrom
petr-muller wants to merge 4 commits intoopenshift:mainfrom
Conversation
Contributor
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughIssue-comment handlers in multiple commands now ignore all GitHub issue comment event actions except Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 13 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (13 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: petr-muller The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Pull request overview
This PR prevents several custom Prow plugins from re-processing slash commands when an existing GitHub issue/PR comment is edited or deleted, by early-returning unless the IssueCommentEvent action is created (matching the existing pj-rehearse behavior).
Changes:
- Add
IssueCommentActionCreatedguards to ignore non-createdIssueCommentEventactions in multiple plugins. - Ensure slash-command handlers only run on newly created comments (avoids accidental re-triggers on edits/deletes).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| cmd/pipeline-controller/main.go | Ignore non-created issue comment events before running pipeline slash-command handling. |
| cmd/payload-testing-prow-plugin/server.go | Ignore non-created issue comment events before processing payload-testing commands. |
| cmd/multi-pr-prow-plugin/server.go | Ignore non-created issue comment events before processing /testwith commands. |
| cmd/backport-verifier/server.go | Ignore non-created issue comment events before processing /validate-backports. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
petr-muller
force-pushed
the
pj-rehearse-edits
branch
from
f72453f to
1dab8b7
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
cmd/payload-testing-prow-plugin/server_test.go (1)
1400-1434: ⚡ Quick winVerify Kubernetes-side effects are also zero.
At Line 1428 you only assert no GitHub comments. Please also assert that no
PullRequestPayloadQualificationRunobjects are created for edited/deleted actions.Proposed test extension
+import ( + ... + ctrlruntimeclient "sigs.k8s.io/controller-runtime/pkg/client" +) ... for _, action := range []github.IssueCommentEventAction{github.IssueCommentActionEdited, github.IssueCommentActionDeleted} { t.Run(string(action), func(t *testing.T) { ghc.IssueCommentsAdded = nil s.handleIssueComment(logrus.WithField("test", t.Name()), github.IssueCommentEvent{ Action: action, Repo: github.Repo{Owner: github.User{Login: "openshift"}}, Issue: github.Issue{ Number: 123, PullRequest: &struct{}{}, }, Comment: github.IssueComment{ Body: "/payload 4.10 nightly informing", }, }) if len(ghc.IssueCommentsAdded) > 0 { t.Errorf("expected no comments for %s action, got: %v", action, ghc.IssueCommentsAdded) } + var runs prpqv1.PullRequestPayloadQualificationRunList + if err := s.kubeClient.List(context.TODO(), &runs, &ctrlruntimeclient.ListOptions{Namespace: "ci"}); err != nil { + t.Fatalf("failed to list payload runs: %v", err) + } + if len(runs.Items) > 0 { + t.Errorf("expected no payload runs for %s action, got: %d", action, len(runs.Items)) + } }) }🤖 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 `@cmd/payload-testing-prow-plugin/server_test.go` around lines 1400 - 1434, Extend TestHandleIssueCommentIgnoresNonCreatedActions to also assert no Kubernetes PullRequestPayloadQualificationRun objects are created when actions are edited/deleted: after calling s.handleIssueComment, use the server's kubeClient (s.kubeClient) to List PullRequestPayloadQualificationRun objects (type PullRequestPayloadQualificationRun) in namespace s.namespace and assert the returned list is empty (length==0); do this inside the t.Run subtest alongside the existing ghc.IssueCommentsAdded check so both GitHub and K8s side-effects are verified for the non-created actions.cmd/backport-verifier/server_test.go (1)
79-108: ⚡ Quick winAlso assert labels remain unchanged for non-created actions.
At Line 103 only comments are checked. Since this path should be a no-op, please also assert no labels were added/removed.
Proposed assertion
if len(client.comments[orp]) > 0 { t.Errorf("expected no comments for %s action, got: %v", action, client.comments[orp]) } + if len(client.labels[orp]) > 0 { + t.Errorf("expected no labels for %s action, got: %v", action, client.labels[orp]) + }🤖 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 `@cmd/backport-verifier/server_test.go` around lines 79 - 108, The test TestHandleIssueCommentIgnoresNonCreatedActions currently only asserts no comments were added for non-created actions; also assert labels remain unchanged by checking fakeClient.labels[orp] is empty (or equal to its initial value) after calling server.handleIssueComment. Locate the orprepopr variable (orp), the fakeClient with labels map, and add an assertion similar to the comments check that len(client.labels[orp]) == 0 (or compare slices) to ensure handleIssueComment is a no-op for edited/deleted actions.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@cmd/multi-pr-prow-plugin/server_test.go`:
- Around line 399-424: The test TestHandleIssueCommentIgnoresNonCreatedActions
currently calls s.handleIssueComment but asserts nothing; change the test to
verify no side effects occur by using a recording fakeReporter (or extend
fakeReporter) and asserting it recorded zero reports after each call, assert the
fakeGithubClient (fakeGithubClient) recorded no API interactions, and assert the
fake Kubernetes client (created by fakectrlruntimeclient.NewFakeClient()) has no
created ProwJob or related resources; perform these zero-call/empty-state
assertions inside the t.Run subtest after invoking server.handleIssueComment to
ensure non-created actions produce no work.
---
Nitpick comments:
In `@cmd/backport-verifier/server_test.go`:
- Around line 79-108: The test TestHandleIssueCommentIgnoresNonCreatedActions
currently only asserts no comments were added for non-created actions; also
assert labels remain unchanged by checking fakeClient.labels[orp] is empty (or
equal to its initial value) after calling server.handleIssueComment. Locate the
orprepopr variable (orp), the fakeClient with labels map, and add an assertion
similar to the comments check that len(client.labels[orp]) == 0 (or compare
slices) to ensure handleIssueComment is a no-op for edited/deleted actions.
In `@cmd/payload-testing-prow-plugin/server_test.go`:
- Around line 1400-1434: Extend TestHandleIssueCommentIgnoresNonCreatedActions
to also assert no Kubernetes PullRequestPayloadQualificationRun objects are
created when actions are edited/deleted: after calling s.handleIssueComment, use
the server's kubeClient (s.kubeClient) to List
PullRequestPayloadQualificationRun objects (type
PullRequestPayloadQualificationRun) in namespace s.namespace and assert the
returned list is empty (length==0); do this inside the t.Run subtest alongside
the existing ghc.IssueCommentsAdded check so both GitHub and K8s side-effects
are verified for the non-created actions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 8c1a8513-93da-4880-9136-c89265a2ac4b
📒 Files selected for processing (4)
cmd/backport-verifier/server_test.gocmd/multi-pr-prow-plugin/server_test.gocmd/payload-testing-prow-plugin/server_test.gocmd/pipeline-controller/main_test.go
✅ Files skipped from review due to trivial changes (1)
- cmd/pipeline-controller/main_test.go
The handler processed all IssueCommentEvent actions (created, edited, deleted), which meant editing a comment containing a /payload-job command would unintentionally re-trigger the payload job. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The handler processed all IssueCommentEvent actions (created, edited, deleted), which meant editing a comment containing a /testwith command would unintentionally re-trigger the job. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The handler processed all IssueCommentEvent actions (created, edited, deleted), which meant editing a comment containing a /validate-backports command would unintentionally re-trigger the verification. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
petr-muller
force-pushed
the
pj-rehearse-edits
branch
from
1dab8b7 to
9e7bd71
Compare
Member
Author
|
/retest-required |
Contributor
|
Pipeline controller notification No second-stage tests were triggered for this PR. This can happen when:
Use |
The handler processed all IssueCommentEvent actions (created, edited, deleted), which meant editing a comment containing a /pipeline command would unintentionally re-trigger the pipeline. Also avoids taking the mutex lock for events that will be ignored. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
petr-muller
force-pushed
the
pj-rehearse-edits
branch
from
9e7bd71 to
8038421
Compare
Contributor
|
@petr-muller: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
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.
Summary
Several custom prow plugins (
payload-testing,multi-pr-prow-plugin,backport-verifier,pipeline-controller) process allIssueCommentEventactions, includingeditedanddeleted. This means editing a comment containing a slash command (e.g./payload-job) unintentionally re-triggers the action.Add an early return for non-
createdactions in each plugin'shandleIssueComment, matching whatpj-rehearsealready does.🤖 Generated with Claude Code
This PR makes four custom Prow plugins ignore IssueCommentEvent actions other than "created", preventing edited or deleted comments from re-triggering slash-command processing.
What changed in practice
Components affected and behavior changes
Impact for CI users and operators