fix(opscomments): skip key=value args as PR names#2712
Conversation
There was a problem hiding this comment.
Code Review
This pull request ensures that key-value arguments in comments (e.g., /test key=value) are not incorrectly parsed as PipelineRun names, allowing them to be correctly handled by on-comment annotations. The changes include logic updates in pkg/opscomments and pkg/provider, supported by new unit and E2E tests. A critical feedback point identifies a regression in pkg/provider/provider.go where comments containing both a specific PipelineRun name and parameters would fail to extract the name correctly; a suggestion was provided to split the input string by spaces before validating the name.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2712 +/- ##
==========================================
+ Coverage 59.25% 59.29% +0.03%
==========================================
Files 208 208
Lines 20573 20590 +17
==========================================
+ Hits 12191 12208 +17
Misses 7610 7610
Partials 772 772 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
cd02b60 to
35bd553
Compare
|
Will look at the deduplication of the opscomment logic in |
|
/retest |
35bd553 to
abfaa83
Compare
|
/retest |
but |
abfaa83 to
0e146f9
Compare
This fix is only for the case when |
I think then that's a collision |
|
this looks good! but we should clarify in docs that do not use PaC's standard GitOps comments in on-comment annotation. |
When a user posts "/test custom1=value", the key=value argument was being treated as a PipelineRun name, which bypassed on-comment annotation matching. Return empty string when the first argument contains "=" so the comment falls through to on-comment handling. Signed-off-by: Akshay Pant <akpant@redhat.com> Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Add a note advising users not to use PAC's standard GitOps commands (/test, /retest, /cancel, /ok-to-test) as on-comment annotation patterns, since they are processed before on-comment matching. Signed-off-by: Akshay Pant <akpant@redhat.com>
0e146f9 to
6c11540
Compare
Added a callout in 6c11540 |
📝 Description of the Change
When a user posts "/test custom1=value", the key=value argument was being treated as a PipelineRun name, which bypassed on-comment annotation matching. Return empty string when the first argument contains "=" so the comment falls through to on-comment handling.
🔗 Linked GitHub Issue
Fixes #1952
🧪 Testing Strategy
e2e test on Forgejo with

/test custom1=valuerun successfully.🤖 AI Assistance
AI assistance can be used for various tasks, such as code generation,
documentation, or testing.
Please indicate whether you have used AI assistance
for this PR and provide details if applicable.
Important
Slop will be simply rejected, if you are using AI assistance you need to make sure you
understand the code generated and that it meets the project's standards. you
need at least know how to run the code and deploy it (if needed). See
startpaac to make it easy
to deploy and test your code changes.
If the majority of the code in this PR was generated by an AI, please add a
Co-authored-bytrailer to your commit message.For example:
Co-authored-by: Claude noreply@anthropic.com
✅ Submitter Checklist
fix:,feat:) matches the "Type of Change" I selected above.make testandmake lintlocally to check for and fix anyissues. For an efficient workflow, I have considered installing
pre-commit and running
pre-commit installtoautomate these checks.