fix: skip MR comment for non-permission errors

pkg/provider/gitlab/gitlab.go

@@ -319,12 +319,26 @@ func (v *Provider) CreateStatus(_ context.Context, event *info.Event, statusOpts
 		v.Logger.Debugf("created commit status on source project ID %d", event.TargetProjectID)
 		return nil
 	}
-	if _, _, err2 := v.Client().Commits.SetCommitStatus(event.TargetProjectID, event.SHA, opt); err2 == nil {
+	_, _, err2 := v.Client().Commits.SetCommitStatus(event.TargetProjectID, event.SHA, opt)
+	if err2 == nil {
 		v.Logger.Debugf("created commit status on target project ID %d", event.TargetProjectID)
 		// we managed to set the status on the target repo, all good we are done
 		return nil
 	}
-	v.Logger.Debugf("cannot set status with the GitLab token on the target project: %v", err)
+	v.Logger.Debugf("cannot set status with the GitLab token on the target project: %v", err2)
+
+	// Skip creating MR comments if the error is a state transition error
+	// (e.g., "Cannot transition status via :run from :running").
+	// This means the status is already set, so we should not create a comment.
+	// For other errors (permission issues, network errors, etc.), we fall back to comments.
+	isTransitionErr := func(e error) bool {
+		return e != nil && strings.Contains(e.Error(), "Cannot transition status")
+	}
+	if isTransitionErr(err) || isTransitionErr(err2) {
+		v.Logger.Debugf("skipping MR comment as error is a state transition issue (status already set)")
+		return nil
+	}
+
 	// we only show the first error as it's likely something the user has more control to fix
 	// the second err is cryptic as it needs a dummy gitlab pipeline to start
 	// with and will only give more confusion in the event namespace

pkg/provider/gitlab/gitlab_test.go

@@ -261,7 +261,7 @@ func TestCreateStatus(t *testing.T) {
 			},
 		},
 		{
-			name:       "commit status fails on both projects but continues",
+			name:       "commit status fails with state transition error skips MR comment",
 			wantClient: true,
 			wantErr:    false,
 			fields: fields{
@@ -273,10 +273,70 @@ func TestCreateStatus(t *testing.T) {
 				},
 				event: &info.Event{
 					TriggerTarget:   "pull_request",
-					SourceProjectID: 404, // Will fail
-					TargetProjectID: 405, // Will fail
+					SourceProjectID: 422, // Will fail with "Cannot transition status" error
+					TargetProjectID: 423, // Will fail with "Cannot transition status" error
 					SHA:             "abc123",
 				},
+				postStr: "", // No MR comment expected for state transition errors
+			},
+		},
+		{
+			name:       "generic error on both projects creates MR comment",
+			wantClient: true,
+			wantErr:    false,
+			fields: fields{
+				targetProjectID: 100,
+			},
+			args: args{
+				statusOpts: provider.StatusOpts{
+					Conclusion: "success",
+				},
+				event: &info.Event{
+					TriggerTarget:   "pull_request",
+					SourceProjectID: 400, // Will fail with generic 400 error
+					TargetProjectID: 405, // Will fail with generic 400 error
+					SHA:             "abc123def",
+				},
+				postStr: "has successfully", // MR comment expected for non-transition errors
+			},
+		},
+		{
+			name:       "permission error 403 on source creates MR comment",
+			wantClient: true,
+			wantErr:    false,
+			fields: fields{
+				targetProjectID: 100,
+			},
+			args: args{
+				statusOpts: provider.StatusOpts{
+					Conclusion: "success",
+				},
+				event: &info.Event{
+					TriggerTarget:   "pull_request",
+					SourceProjectID: 403, // Will fail with 403 Forbidden
+					TargetProjectID: 400, // Will fail with generic 400 error
+					SHA:             "abc123ghi",
+				},
+				postStr: "has successfully",
+			},
+		},
+		{
+			name:       "permission error 401 on source creates MR comment",
+			wantClient: true,
+			wantErr:    false,
+			fields: fields{
+				targetProjectID: 100,
+			},
+			args: args{
+				statusOpts: provider.StatusOpts{
+					Conclusion: "success",
+				},
+				event: &info.Event{
+					TriggerTarget:   "pull_request",
+					SourceProjectID: 401, // Will fail with 401 Unauthorized
+					TargetProjectID: 400, // Will fail with generic 400 error
+					SHA:             "abc123jkl",
+				},
 				postStr: "has successfully",
 			},
 		},
@@ -318,31 +378,53 @@ func TestCreateStatus(t *testing.T) {
 					// Mock source project commit status endpoint
 					sourceStatusPath := fmt.Sprintf("/projects/%d/statuses/%s", tt.args.event.SourceProjectID, tt.args.event.SHA)
 					mux.HandleFunc(sourceStatusPath, func(rw http.ResponseWriter, _ *http.Request) {
-						if tt.args.event.SourceProjectID == 404 {
-							// Simulate failure on source project
+						switch tt.args.event.SourceProjectID {
+						case 400:
+							rw.WriteHeader(http.StatusBadRequest)
+							fmt.Fprint(rw, `{"message": "400 Bad Request"}`)
+						case 401:
+							rw.WriteHeader(http.StatusUnauthorized)
+							fmt.Fprint(rw, `{"message": "401 Unauthorized"}`)
+						case 403:
+							rw.WriteHeader(http.StatusForbidden)
+							fmt.Fprint(rw, `{"message": "403 Forbidden"}`)
+						case 404:
 							rw.WriteHeader(http.StatusNotFound)
 							fmt.Fprint(rw, `{"message": "404 Project Not Found"}`)
-							return
+						case 422:
+							rw.WriteHeader(http.StatusBadRequest)
+							fmt.Fprint(rw, `{"message": "Cannot transition status via :run from :running"}`)
+						default:
+							rw.WriteHeader(http.StatusCreated)
+							fmt.Fprint(rw, `{}`)
 						}
-						// Success on source project
-						rw.WriteHeader(http.StatusCreated)
-						fmt.Fprint(rw, `{}`)
 					})
 				}
 
 				if tt.args.event.TargetProjectID != 0 {
 					// Mock target project commit status endpoint
 					targetStatusPath := fmt.Sprintf("/projects/%d/statuses/%s", tt.args.event.TargetProjectID, tt.args.event.SHA)
 					mux.HandleFunc(targetStatusPath, func(rw http.ResponseWriter, _ *http.Request) {
-						if tt.args.event.TargetProjectID == 404 {
-							// Simulate failure on target project
+						switch tt.args.event.TargetProjectID {
+						case 400, 405:
+							rw.WriteHeader(http.StatusBadRequest)
+							fmt.Fprint(rw, `{"message": "400 Bad Request"}`)
+						case 401:
+							rw.WriteHeader(http.StatusUnauthorized)
+							fmt.Fprint(rw, `{"message": "401 Unauthorized"}`)
+						case 403:
+							rw.WriteHeader(http.StatusForbidden)
+							fmt.Fprint(rw, `{"message": "403 Forbidden"}`)
+						case 404:
 							rw.WriteHeader(http.StatusNotFound)
 							fmt.Fprint(rw, `{"message": "404 Project Not Found"}`)
-							return
+						case 423:
+							rw.WriteHeader(http.StatusBadRequest)
+							fmt.Fprint(rw, `{"message": "Cannot transition status via :run from :running"}`)
+						default:
+							rw.WriteHeader(http.StatusCreated)
+							fmt.Fprint(rw, `{}`)
 						}
-						// Success on target project
-						rw.WriteHeader(http.StatusCreated)
-						fmt.Fprint(rw, `{}`)
 					})
 				}