fix(opscomments): skip key=value args as PR names

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>

Author: theakshaypant

pkg/opscomments/comments.go

@@ -182,6 +182,9 @@ func getNameFromComment(typeOfComment, comment string) string {
 		return ""
 	}
 
-	// trim spaces
-	return strings.TrimSpace(firstArg[1])
+	name := strings.TrimSpace(firstArg[1])
+	if strings.Contains(name, "=") {
+		return ""
+	}
+	return name
 }

pkg/opscomments/comments_test.go

@@ -86,6 +86,18 @@ func TestGetNameFromFunction(t *testing.T) {
 			expected:    "prname",
 			commentType: testComment,
 		},
+		{
+			name:        "key value arg is not a pipeline name",
+			comment:     "/test custom1=value",
+			expected:    "",
+			commentType: testComment,
+		},
+		{
+			name:        "key value args without pipeline name",
+			comment:     "/test foo=bar hello=moto",
+			expected:    "",
+			commentType: testComment,
+		},
 		{
 			name:        "get name from cancel comment",
 			comment:     "/cancel prname",
@@ -295,6 +307,12 @@ func TestSetEventTypeTestPipelineRun(t *testing.T) {
 			wantType:   TestSingleCommentEventType.String(),
 			wantTestPr: "prname",
 		},
+		{
+			name:       "test with key value arg treated as test all",
+			comment:    "/test custom1=value",
+			wantType:   TestSingleCommentEventType.String(),
+			wantTestPr: "",
+		},
 		{
 			name:         "cancel single pr",
 			comment:      "/cancel prname",

pkg/provider/provider.go

@@ -106,8 +106,18 @@ func getNameFromComment(typeOfComment, comment string) string {
 	splitTest := strings.Split(comment, typeOfComment)
 	// now get the first line
 	getFirstLine := strings.Split(splitTest[1], "\n")
-	// trim spaces
-	return strings.TrimSpace(getFirstLine[0])
+
+	// and the first argument
+	firstArg := strings.Split(getFirstLine[0], " ")
+	if len(firstArg) < 2 {
+		return ""
+	}
+
+	name := strings.TrimSpace(firstArg[1])
+	if strings.Contains(name, "=") {
+		return ""
+	}
+	return name
 }
 
 func GetPipelineRunAndBranchOrTagNameFromTestComment(comment, prefix string) (string, string, string, error) {
@@ -157,6 +167,9 @@ func getPipelineRunAndBranchOrTagNameFromComment(typeOfComment, comment string)
 		prData := strings.Split(strings.TrimSpace(branchData[0]), " ")
 		if len(prData) > 1 {
 			prName = strings.TrimSpace(prData[0])
+			if strings.Contains(prName, "=") {
+				prName = ""
+			}
 		}
 	} else {
 		// get the second part of the typeOfComment (/test, /retest or /cancel)
@@ -166,6 +179,9 @@ func getPipelineRunAndBranchOrTagNameFromComment(typeOfComment, comment string)
 		// trim spaces
 		// adapt for the comment contains the key=value pair
 		prName = strings.Split(strings.TrimSpace(getFirstLine[0]), " ")[0]
+		if strings.Contains(prName, "=") {
+			prName = ""
+		}
 	}
 	return prName, branchName, tagName, nil
 }

pkg/provider/provider_test.go

@@ -214,6 +214,11 @@ func TestGetPipelineRunFromComment(t *testing.T) {
 			comment: "before \n /test abc-01-pr \n after",
 			want:    "abc-01-pr",
 		},
+		{
+			name:    "test with key value is not a pipeline name",
+			comment: "/test custom1=value",
+			want:    "",
+		},
 		{
 			name:    "retest a pipeline",
 			comment: "/retest abc-01-pr",
@@ -260,6 +265,11 @@ func TestGetPipelineRunFromCancelComment(t *testing.T) {
 			comment: "/cancel abc-01-pr",
 			want:    "abc-01-pr",
 		},
+		{
+			name:    "cancel with key value is not a pipeline name",
+			comment: "/cancel custom1=value",
+			want:    "",
+		},
 		{
 			name:    "string before cancel command",
 			comment: "abc \n /cancel abc-01-pr",
@@ -365,6 +375,19 @@ func TestGetPipelineRunAndBranchNameFromTestComment(t *testing.T) {
 			branchName: "",
 			wantError:  false,
 		},
+		{
+			name:      "test with only key value is not a pipeline name",
+			comment:   "/test custom1=value",
+			prName:    "",
+			wantError: false,
+		},
+		{
+			name:       "test with key value and branch is not a pipeline name",
+			comment:    "/test custom1=value branch:nightly",
+			prName:     "",
+			branchName: "nightly",
+			wantError:  false,
+		},
 		{
 			name:      "string before retest command",
 			comment:   "abc \n /retest abc-01-pr",

test/gitea_gitops_commands_test.go

@@ -130,6 +130,57 @@ func TestGiteaOnCommentAnnotation(t *testing.T) {
 	assert.NilError(t, err)
 }
 
+// TestGiteaOnCommentTestOverride tests that a PipelineRun with
+// on-comment: "^/test.*" is triggered when posting "/test custom1=value".
+// This verifies that key=value arguments are not mistaken for a PipelineRun
+// name, which would bypass the on-comment annotation matching entirely.
+func TestGiteaOnCommentTestOverride(t *testing.T) {
+	var err error
+	ctx := context.Background()
+	topts := &tgitea.TestOpts{
+		TargetRefName: names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("pac-e2e-test"),
+		RepoCRParams: &[]v1alpha1.Params{
+			{
+				Name:  "custom1",
+				Value: "default",
+			},
+		},
+	}
+	topts.TargetNS = topts.TargetRefName
+	topts.ParamsRun, topts.Opts, topts.GiteaCNX, err = tgitea.Setup(ctx)
+	assert.NilError(t, err, fmt.Errorf("cannot do gitea setup: %w", err))
+	ctx, err = cctx.GetControllerCtxInfo(ctx, topts.ParamsRun)
+	assert.NilError(t, err)
+	assert.NilError(t, pacrepo.CreateNS(ctx, topts.TargetNS, topts.ParamsRun))
+	assert.NilError(t, secret.Create(ctx, topts.ParamsRun, map[string]string{"secret": "SHHHHHHH"}, topts.TargetNS, "pac-secret"))
+	topts.TargetEvent = triggertype.PullRequest.String()
+	topts.YAMLFiles = map[string]string{
+		".tekton/on-comment-test-override.yaml": "testdata/pipelinerun-on-comment-test-override.yaml",
+	}
+	topts.ExpectEvents = false
+	_, f := tgitea.TestPR(t, topts)
+	defer f()
+
+	tgitea.PostCommentOnPullRequest(t, topts, "/test custom1=overridden")
+	waitOpts := twait.Opts{
+		RepoName:        topts.TargetNS,
+		Namespace:       topts.TargetNS,
+		MinNumberStatus: 1,
+		PollTimeout:     twait.DefaultTimeout,
+	}
+	repo, err := twait.UntilRepositoryUpdated(context.Background(), topts.ParamsRun.Clients, waitOpts)
+	assert.NilError(t, err)
+	assert.Equal(t, len(repo.Status), 1, "should have exactly 1 status")
+	assert.Equal(t, *repo.Status[0].EventType, opscomments.OnCommentEventType.String(),
+		"should have matched via on-comment annotation, not built-in /test handler")
+
+	last := repo.Status[0]
+	err = twait.RegexpMatchingInPodLog(context.Background(), topts.ParamsRun, topts.TargetNS,
+		fmt.Sprintf("tekton.dev/pipelineRun=%s", last.PipelineRunName), "step-task",
+		*regexp.MustCompile("custom1 is overridden"), "", 2, nil)
+	assert.NilError(t, err)
+}
+
 // TestGiteaTestPipelineRunExplicitlyWithTestComment will test a pipelinerun
 // even if it hasn't matched when we are doing a /test comment.
 func TestGiteaTestPipelineRunExplicitlyWithTestComment(t *testing.T) {

test/testdata/pipelinerun-on-comment-test-override.yaml

@@ -0,0 +1,19 @@
+---
+apiVersion: tekton.dev/v1beta1
+kind: PipelineRun
+metadata:
+  name: "on-comment-test-override"
+  annotations:
+    pipelinesascode.tekton.dev/target-namespace: "\\ .TargetNamespace //"
+    pipelinesascode.tekton.dev/on-comment: "^/test.*"
+spec:
+  pipelineSpec:
+    tasks:
+      - name: task
+        taskSpec:
+          steps:
+            - name: task
+              image: registry.access.redhat.com/ubi10/ubi-micro
+              script: |
+                echo "event_type is {{ event_type }}"
+                echo "custom1 is {{ custom1 }}"