fix: use dynamic prefix in retest error

Convert ErrNoFailedPipelineToRetest from a sentinel error variable
to a custom error type that accepts the gitops command prefix. This
ensures the error message displays the correct command syntax
(e.g. /pac retest) based on the repository's configured prefix.

Signed-off-by: Zaki Shaikh <zashaikh@redhat.com>
Assisted-by: Claude Opus 4.6 (via Claude Code)

Author: zakisk

docs/content/docs/guides/gitops-commands/advanced.md

@@ -87,6 +87,8 @@ Example:
 /pac test
 ```
 
+You can also configure GitOps command prefix in [Global Repository CR]({{< relref "/docs/operations/global-repository-settings" >}}) so that it will be applied to all Repository CRs those are not defining their own prefix.
+
 ## Cancelling a PipelineRun
 
 **What it does:** The `/cancel` command stops running PipelineRuns by commenting on the pull request.

pkg/apis/pipelinesascode/v1alpha1/types.go

@@ -228,6 +228,10 @@ func (s *Settings) Merge(newSettings *Settings) {
 	if newSettings.AIAnalysis != nil && s.AIAnalysis == nil {
 		s.AIAnalysis = newSettings.AIAnalysis
 	}
+
+	if newSettings.GitOpsCommandPrefix != "" && s.GitOpsCommandPrefix == "" {
+		s.GitOpsCommandPrefix = newSettings.GitOpsCommandPrefix
+	}
 }
 
 type Policy struct {

pkg/apis/pipelinesascode/v1alpha1/types_test.go

@@ -58,6 +58,7 @@ func TestMergeSpecs(t *testing.T) {
 					Policy: &Policy{
 						OkToTest: []string{"ok1", "ok2"},
 					},
+					GitOpsCommandPrefix: "pac",
 				}, // Initialize as needed
 				GitProvider:      gp, // Initialize as needed
 				Incomings:        incomings,
@@ -71,6 +72,7 @@ func TestMergeSpecs(t *testing.T) {
 					Policy: &Policy{
 						OkToTest: []string{"ok1", "ok2"},
 					},
+					GitOpsCommandPrefix: "pac",
 				},
 				Incomings:        incomings,
 				GitProvider:      gp,
@@ -87,6 +89,7 @@ func TestMergeSpecs(t *testing.T) {
 					Policy: &Policy{
 						OkToTest: []string{"ok1", "ok2"},
 					},
+					GitOpsCommandPrefix: "pac",
 				}, // Initialize as needed
 				GitProvider: &GitProvider{}, // Initialize as needed
 			},
@@ -110,6 +113,7 @@ func TestMergeSpecs(t *testing.T) {
 					Policy: &Policy{
 						OkToTest: []string{"ok1", "ok2"},
 					},
+					GitOpsCommandPrefix: "pac",
 				},
 				Incomings:        incomings,
 				GitProvider:      gp,

pkg/matcher/annotation_matcher.go

@@ -2,7 +2,6 @@ package matcher
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"regexp"
 	"strings"
@@ -37,9 +36,20 @@ const (
 	maxCommentLogLength = 160
 )
 
-// ErrNoFailedPipelineToRetest is returned when /retest or /ok-to-test is used
-// but all matching pipelines have already succeeded for this commit.
-var ErrNoFailedPipelineToRetest = errors.New("All PipelineRuns for this commit have already succeeded. Use `/retest <pipeline-name>` to re-run a specific pipeline or `/test` to re-run all pipelines.") // nolint:revive,staticcheck
+// NoFailedPipelineToRetestError is an error type returned when /retest or
+// /ok-to-test is used but all matching pipelines have already succeeded for
+// this commit. The underlying string value is the gitops comment prefix used
+// to construct the user-facing error message with the correct command syntax.
+type NoFailedPipelineToRetestError string
+
+func (e NoFailedPipelineToRetestError) Error() string {
+	return fmt.Sprintf("All PipelineRuns for this commit have already succeeded. Use `%sretest <pipeline-name>` to re-run a specific pipeline or `%stest` to re-run all pipelines.", string(e), string(e))
+}
+
+func (e NoFailedPipelineToRetestError) Is(target error) bool {
+	_, ok := target.(NoFailedPipelineToRetestError)
+	return ok
+}
 
 // prunBranch is value from annotations and baseBranch is event.Base value from event.
 func branchMatch(prunBranch, baseBranch string) bool {
@@ -426,7 +436,7 @@ func MatchPipelinerunByAnnotation(ctx context.Context, logger *zap.SugaredLogger
 			logger.Debugf("MatchPipelinerunByAnnotation: filtering successful templates for event_type=%s", event.EventType)
 			filtered := filterSuccessfulTemplates(ctx, logger, cs, event, repo, vcx, matchedPRs)
 			if len(filtered) == 0 {
-				return nil, ErrNoFailedPipelineToRetest
+				return nil, NoFailedPipelineToRetestError(provider.GetGitOpsCommentPrefix(repo))
 			}
 			return filtered, nil
 		}

pkg/matcher/annotation_matcher_test.go

@@ -2282,7 +2282,12 @@ func TestMatchPipelinerunByAnnotation(t *testing.T) {
 			wantErrNoFailedPipelineToRetest: true,
 			repo: &v1alpha1.Repository{
 				ObjectMeta: metav1.ObjectMeta{Name: "test-repo", Namespace: "test-ns"},
-				Spec:       v1alpha1.RepositorySpec{URL: "https://github.com/org/repo"},
+				Spec: v1alpha1.RepositorySpec{
+					URL: "https://github.com/org/repo",
+					Settings: &v1alpha1.Settings{
+						GitOpsCommandPrefix: "pac",
+					},
+				},
 			},
 			seedData: &testclient.Data{
 				PipelineRuns: []*tektonv1.PipelineRun{
@@ -2336,7 +2341,7 @@ func TestMatchPipelinerunByAnnotation(t *testing.T) {
 			matches, err := MatchPipelinerunByAnnotation(ctx, logger, tt.args.pruns, cs, &tt.args.runevent, &ghprovider.Provider{}, eventEmitter, repo, true)
 			if tt.wantErrNoFailedPipelineToRetest {
 				assert.Assert(t, err != nil, "expected ErrNoFailedPipelineToRetest")
-				assert.Assert(t, errors.Is(err, ErrNoFailedPipelineToRetest), "expected ErrNoFailedPipelineToRetest, got: %v", err)
+				assert.Assert(t, errors.Is(err, NoFailedPipelineToRetestError("/pac ")), "expected ErrNoFailedPipelineToRetest, got: %v", err)
 				assert.Equal(t, len(matches), 0, "expected no matches when all pipelines already succeeded")
 				return
 			}

pkg/pipelineascode/match.go

@@ -14,6 +14,7 @@ import (
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/matcher"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/opscomments"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/params/triggertype"
+	"github.com/openshift-pipelines/pipelines-as-code/pkg/provider"
 	providerstatus "github.com/openshift-pipelines/pipelines-as-code/pkg/provider/status"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/resolve"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/secrets"
@@ -268,8 +269,9 @@ func (p *PacRun) getPipelineRunsFromRepo(ctx context.Context, repo *v1alpha1.Rep
 	var matchedPRs []matcher.Match
 	if p.event.TargetTestPipelineRun == "" {
 		if matchedPRs, err = matcher.MatchPipelinerunByAnnotation(ctx, p.logger, pipelineRuns, p.run, p.event, p.vcx, p.eventEmitter, repo, true); err != nil {
+			prefix := provider.GetGitOpsCommentPrefix(repo)
 			// Check if all pipelines have already succeeded - post comment so user gets feedback
-			if errors.Is(err, matcher.ErrNoFailedPipelineToRetest) {
+			if errors.Is(err, matcher.NoFailedPipelineToRetestError(prefix)) {
 				p.logger.Infof("RepositoryAllPipelinesSucceeded: %s", err.Error())
 				p.eventEmitter.EmitMessage(nil, zap.InfoLevel, "RepositoryAllPipelinesSucceeded", err.Error())
 				if commentErr := p.vcx.CreateComment(ctx, p.event, err.Error(), ""); commentErr != nil {

pkg/provider/github/acl.go

@@ -81,8 +81,10 @@ func (v *Provider) IsAllowed(ctx context.Context, event *info.Event) (bool, erro
 		Logger:       v.Logger,
 	}
 
+	prefix := provider.GetGitOpsCommentPrefix(v.repo)
+
 	// Try to detect a policy rule allowing this
-	tType, _ := v.detectTriggerTypeFromPayload("", event.Event)
+	tType, _ := v.detectTriggerTypeFromPayload("", event.Event, prefix)
 	policyAllowed, policyReason := aclPolicy.IsAllowed(ctx, tType)
 
 	switch policyAllowed {

pkg/provider/github/detect.go

@@ -6,6 +6,7 @@ import (
 	"net/http"
 
 	"github.com/google/go-github/v81/github"
+	"github.com/openshift-pipelines/pipelines-as-code/pkg/opscomments"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/params/triggertype"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/provider"
 	"go.uber.org/zap"
@@ -45,7 +46,9 @@ func (v *Provider) Detect(req *http.Request, payload string, logger *zap.Sugared
 	}
 
 	_ = json.Unmarshal([]byte(payload), &eventInt)
-	eType, errReason := v.detectTriggerTypeFromPayload(eventType, eventInt)
+	// at this moment we don't any info about repository CR so passing "/" as gitOpsCommentPrefix
+	// if its valid event then that will be done in ParsePayload function ahead.
+	eType, errReason := v.detectTriggerTypeFromPayload(eventType, eventInt, "/")
 	if eType != "" {
 		return setLoggerAndProceed(true, "", nil)
 	}
@@ -56,7 +59,7 @@ func (v *Provider) Detect(req *http.Request, payload string, logger *zap.Sugared
 // detectTriggerTypeFromPayload will detect the event type from the payload,
 // filtering out the events that are not supported.
 // first arg will get the event type and the second one will get an error string explaining why it's not supported.
-func (v *Provider) detectTriggerTypeFromPayload(ghEventType string, eventInt any) (triggertype.Trigger, string) {
+func (v *Provider) detectTriggerTypeFromPayload(ghEventType string, eventInt any, gitOpsCommentPrefix string) (triggertype.Trigger, string) {
 	switch event := eventInt.(type) {
 	case *github.PushEvent:
 		if event.GetPusher() != nil {
@@ -76,13 +79,13 @@ func (v *Provider) detectTriggerTypeFromPayload(ghEventType string, eventInt any
 		if event.GetAction() == "created" &&
 			event.GetIssue().IsPullRequest() &&
 			event.GetIssue().GetState() == "open" {
-			if provider.IsTestRetestComment(event.GetComment().GetBody()) {
+			if opscomments.IsTestRetestComment(event.GetComment().GetBody(), gitOpsCommentPrefix) {
 				return triggertype.Retest, ""
 			}
-			if provider.IsOkToTestComment(event.GetComment().GetBody()) {
+			if opscomments.IsOkToTestComment(event.GetComment().GetBody(), gitOpsCommentPrefix) {
 				return triggertype.OkToTest, ""
 			}
-			if provider.IsCancelComment(event.GetComment().GetBody()) {
+			if opscomments.IsCancelComment(event.GetComment().GetBody(), gitOpsCommentPrefix) {
 				return triggertype.Cancel, ""
 			}
 		}
@@ -99,10 +102,10 @@ func (v *Provider) detectTriggerTypeFromPayload(ghEventType string, eventInt any
 		return "", fmt.Sprintf("check_run: unsupported action \"%s\"", event.GetAction())
 	case *github.CommitCommentEvent:
 		if event.GetAction() == "created" {
-			if provider.IsTestRetestComment(event.GetComment().GetBody()) {
+			if opscomments.IsTestRetestComment(event.GetComment().GetBody(), gitOpsCommentPrefix) {
 				return triggertype.Retest, ""
 			}
-			if provider.IsCancelComment(event.GetComment().GetBody()) {
+			if opscomments.IsCancelComment(event.GetComment().GetBody(), gitOpsCommentPrefix) {
 				return triggertype.Cancel, ""
 			}
 			// Here, the `/ok-to-test` command is ignored because it is intended for pull requests.

pkg/provider/provider.go

@@ -112,13 +112,14 @@ func getNameFromComment(typeOfComment, comment string) string {
 
 func GetPipelineRunAndBranchOrTagNameFromTestComment(comment, prefix string) (string, string, string, error) {
 	commentType := opscomments.CommentEventType(comment, prefix)
+	typeOfComment := ""
 	if commentType == opscomments.TestSingleCommentEventType || commentType == opscomments.TestAllCommentEventType {
-		typeOfComment := prefix + "test"
-		return getPipelineRunAndBranchOrTagNameFromComment(typeOfComment, comment)
+		typeOfComment = prefix + "test"
+	} else {
+		// if type of comment is not test single, it is retest single because we've checked the type before
+		// calling this function.
+		typeOfComment = prefix + "retest"
 	}
-	// if type of comment is not test single, it is retest single because we've checked the type before
-	// calling this function.
-	typeOfComment := prefix + "retest"
 	return getPipelineRunAndBranchOrTagNameFromComment(typeOfComment, comment)
 }
 

test/pkg/github/pr.go

@@ -133,11 +133,9 @@ func (g *PRTest) RunPullRequest(ctx context.Context, t *testing.T) {
 	assert.NilError(t, err)
 	g.Logger = runcnx.Clients.Log
 	g.Cnx = runcnx
-	preSettings := g.Options.Settings
+	settings := g.Options.Settings
 	g.Options = opts
-	if preSettings.Github != nil {
-		g.Options.Settings = preSettings
-	}
+	g.Options.Settings = settings
 	g.Provider = ghcnx
 	g.TargetNamespace = targetNS
 	g.CommitTitle = fmt.Sprintf("Testing %s with Github APPS integration on %s", g.Label, targetNS)
@@ -265,9 +263,7 @@ func (g *PRTest) RunPushRequest(ctx context.Context, t *testing.T) {
 	g.Cnx = runcnx
 	preSettings := g.Options.Settings
 	g.Options = opts
-	if preSettings.Github != nil {
-		g.Options.Settings = preSettings
-	}
+	g.Options.Settings = preSettings
 	g.Provider = ghcnx
 	g.TargetNamespace = targetNS
 	g.PRNumber = -1
@@ -306,7 +302,7 @@ func (g *PRTest) RunPushRequest(ctx context.Context, t *testing.T) {
 		}
 	}
 
-	if g.Options.Settings.Github != nil {
+	if g.Options.Settings != nil && g.Options.Settings.Github != nil {
 		opts.Settings = g.Options.Settings
 	}
 	err = CreateCRD(ctx, t, repoinfo, runcnx, opts, ghcnx, targetNS)