fix(e2e): Fix push/release E2E trigger bugs and silent failures

config/config-matterwick.default.json

@@ -74,11 +74,14 @@
   "E2ELabel": "E2E/Run",
   "E2EMobileIOSLabel": "E2E/Run-iOS",
   "E2EMobileAndroidLabel": "E2E/Run-Android",
+  "E2EResetServersLabel": "E2E/Reset-Servers",
   "E2EUsername": "admin",
+  "E2EPassword": "",
   "E2EServerVersion": "latest",
   "E2EAutoTriggerOnRelease": true,
   "E2EAutoTriggerOnMaster": true,
   "E2EReleasePatternPrefix": "release-",
   "E2ENightlyTriggerWorkflowName": "E2E Nightly Trigger",
-  "E2ETestWorkflowNames": ["Electron Playwright Tests", "E2E", "Compatibility Matrix Testing"]
+  "E2ETestWorkflowNames": ["Electron Playwright Tests", "E2E", "Compatibility Matrix Testing"],
+  "E2EInstanceMaxAge": 6
 }

server/e2e_dryrun_test.go

@@ -421,10 +421,22 @@ func TestDryRun_DesktopPushEvent(t *testing.T) {
 		assert.False(t, s.isReleaseBranch("feature-branch"))
 	})
 
-	t.Run("version extracted from release branch", func(t *testing.T) {
-		assert.Equal(t, "8.0", extractVersionFromReleaseBranch("release-8.0", "release-"))
-		assert.Equal(t, "10.5", extractVersionFromReleaseBranch("release-10.5", "release-"))
-		assert.Equal(t, "", extractVersionFromReleaseBranch("master", "release-"))
+	t.Run("empty release pattern prefix never matches any branch", func(t *testing.T) {
+		// strings.HasPrefix(any, "") is always true. Without this guard, a missing
+		// or empty E2EReleasePatternPrefix in the deployed config would classify
+		// every push (master, feature branches, anything) as a release branch and
+		// trigger spurious E2E provisioning on every push.
+		empty := newDryRunServer(t, "", "mattermost")
+		empty.Config.E2EReleasePatternPrefix = ""
+
+		assert.False(t, empty.isReleaseBranch("master"),
+			"empty prefix must not match master")
+		assert.False(t, empty.isReleaseBranch("release-8.0"),
+			"empty prefix must not match release-8.0")
+		assert.False(t, empty.isReleaseBranch("anything"),
+			"empty prefix must not match arbitrary branches")
+		assert.False(t, empty.isReleaseBranch(""),
+			"empty prefix must not match empty branch")
 	})
 
 	t.Run("branch name extracted from git ref", func(t *testing.T) {
@@ -433,6 +445,18 @@ func TestDryRun_DesktopPushEvent(t *testing.T) {
 		assert.Equal(t, "feature/my-branch", extractBranchName("refs/heads/feature/my-branch"))
 	})
 
+	t.Run("tag refs are not treated as branch refs", func(t *testing.T) {
+		// extractBranchName("refs/tags/release-9.0") returns "release-9.0",
+		// which would match isReleaseBranch and trigger unintended E2E provisioning.
+		// handlePushEvent must guard against non-refs/heads/ refs before calling extractBranchName.
+		tagRef := "refs/tags/release-9.0"
+		assert.False(t, strings.HasPrefix(tagRef, "refs/heads/"),
+			"tag ref must be filtered before branch-trigger evaluation")
+		// The extracted value looks like a release branch — the guard is what prevents it.
+		assert.Equal(t, "release-9.0", extractBranchName(tagRef),
+			"extractBranchName is unaware of ref type; caller must pre-filter")
+	})
+
 	t.Run("desktop push always creates linux/macos/windows instances", func(t *testing.T) {
 		// createMultipleE2EInstancesForPushEvent uses desktop platforms for push events
 		expectedPlatforms := []string{"linux", "macos", "windows"}
@@ -909,6 +933,33 @@ func TestDryRun_ResolveE2EServerVersion(t *testing.T) {
 		assert.False(t, called, "GitHub API must not be called when E2EServerVersion is not 'latest'")
 	})
 
+	t.Run("empty config falls back to latest resolution, not empty version", func(t *testing.T) {
+		// A missing E2EServerVersion field in the deployed config decodes to "".
+		// Before the fix, "" was returned as-is and flowed to CreateInstallation,
+		// which silently failed to provision any server. Empty must be treated
+		// as "latest" so the GitHub-releases lookup runs.
+		body := `[{"tag_name":"v12.0.0","draft":false,"prerelease":false}]`
+		srv := mockReleasesServer(t, body, http.StatusOK)
+		s := newDryRunServer(t, "", "mattermost")
+		s.Config.E2EServerVersion = ""
+		s.githubAPIBase = srv.URL + "/"
+
+		assert.Equal(t, "12.0.0", s.resolveE2EServerVersion(),
+			"empty E2EServerVersion must fall back to latest resolution, not return empty")
+	})
+
+	t.Run("whitespace-only config falls back to latest resolution", func(t *testing.T) {
+		// Defensive: treat whitespace as empty for the same reason — a config-edit
+		// typo with stray whitespace should not silently break provisioning.
+		body := `[{"tag_name":"v12.0.0","draft":false,"prerelease":false}]`
+		srv := mockReleasesServer(t, body, http.StatusOK)
+		s := newDryRunServer(t, "", "mattermost")
+		s.Config.E2EServerVersion = "   "
+		s.githubAPIBase = srv.URL + "/"
+
+		assert.Equal(t, "12.0.0", s.resolveE2EServerVersion())
+	})
+
 	t.Run("RC tags skipped, first stable tag returned with v stripped", func(t *testing.T) {
 		body := `[
 			{"tag_name":"v11.7.0-rc2","draft":false},
@@ -1087,6 +1138,37 @@ func TestDryRun_ResolveE2EServerVersion(t *testing.T) {
 	})
 }
 
+// ------------------------------------------------------------
+// 12b. Push-event server version selection — always latest, ignore branch suffix
+// ------------------------------------------------------------
+
+func TestDryRun_PushEventServerVersion(t *testing.T) {
+	// Push events (release branch, master, main) must always provision the latest
+	// stable Mattermost release. Deriving a version from a release branch name
+	// (e.g. "release-9.0" → "9.0") would attempt to pull a Docker tag that
+	// typically doesn't exist (Docker Hub publishes full SemVer like "9.0.0"),
+	// causing silent installation failures and no E2E workflow dispatch.
+	t.Run("release branch push uses latest version, ignoring branch-derived version", func(t *testing.T) {
+		body := `[{"tag_name":"v12.0.0","draft":false,"prerelease":false}]`
+		srv := mockReleasesServer(t, body, http.StatusOK)
+		s := newDryRunServerLatest(t, srv)
+
+		assert.Equal(t, "12.0.0", s.serverVersionForPushEvent("release-9.0"),
+			"release-9.0 push must provision latest server (12.0.0), not branch-derived 9.0")
+		assert.Equal(t, "12.0.0", s.serverVersionForPushEvent("release-10.5"),
+			"release-10.5 push must provision latest server (12.0.0), not branch-derived 10.5")
+	})
+
+	t.Run("master push uses latest version", func(t *testing.T) {
+		body := `[{"tag_name":"v12.0.0","draft":false,"prerelease":false}]`
+		srv := mockReleasesServer(t, body, http.StatusOK)
+		s := newDryRunServerLatest(t, srv)
+
+		assert.Equal(t, "12.0.0", s.serverVersionForPushEvent("master"))
+		assert.Equal(t, "12.0.0", s.serverVersionForPushEvent("main"))
+	})
+}
+
 // ------------------------------------------------------------
 // 13. MM_SERVER_VERSION sourced from instance, not config
 // ------------------------------------------------------------

server/e2e_tests.go

@@ -769,18 +769,18 @@ func (s *Server) cleanupStaleNonPRE2EInstances() {
 	logger.Info("Non-PR E2E instance cleanup scan complete")
 }
 
-// resolveE2EServerVersion returns the Mattermost server version to use for E2E instances.
-// If E2EServerVersion is "latest", it fetches the mattermost/mattermost GitHub releases,
-// skips drafts, prerelease-flagged releases, and RC/beta/alpha tag-name patterns, then
-// returns the first (newest) fully stable tag stripped of its "v" prefix
-// (e.g. "v11.6.0" → "11.6.0") to match the Docker Hub tag format.
-//
-// The resolved version is cached in memory for 1 hour so that back-to-back provisioning
-// calls (e.g. three parallel platform instances) share a single GitHub API round-trip.
-// Falls back to "master" on any API error or when no stable release is found.
+// resolveE2EServerVersion returns the server version for E2E provisioning.
+// "latest" (or empty) fetches the newest stable mattermost/mattermost release — skips
+// drafts, prereleases, and RC/beta/alpha tags — strips the "v" prefix, and caches the
+// result for 1 hour. Falls back to "master" on API error or if no stable release is found.
 func (s *Server) resolveE2EServerVersion() string {
-	if s.Config.E2EServerVersion != "latest" {
-		return s.Config.E2EServerVersion
+	cfg := strings.TrimSpace(s.Config.E2EServerVersion)
+	if cfg == "" {
+		s.Logger.Warn("[resolveE2EServerVersion] E2EServerVersion is empty in config; defaulting to 'latest'")
+		cfg = "latest"
+	}
+	if cfg != "latest" {
+		return cfg
 	}
 
 	const cacheTTL = 1 * time.Hour
@@ -795,14 +795,12 @@ func (s *Server) resolveE2EServerVersion() string {
 	}
 	s.e2eVersionCacheLock.Unlock()
 
-	// 10-second timeout prevents blocking instance-creation goroutines indefinitely
-	// if the GitHub API is slow or unreachable.
 	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
 	defer cancel()
 
 	client := newGithubClient(s.Config.GithubAccessToken)
 
-	// Redirect to a mock server when running tests (githubAPIBase is empty in production).
+	// githubAPIBase is only set in tests to redirect to a mock server.
 	if s.githubAPIBase != "" {
 		if baseURL, parseErr := url.Parse(s.githubAPIBase); parseErr == nil {
 			client.BaseURL = baseURL
@@ -826,18 +824,15 @@ func (s *Server) resolveE2EServerVersion() string {
 	}
 
 	for _, r := range releases {
-		// Skip drafts and GitHub's explicit prerelease flag first.
 		if r.Draft || r.Prerelease {
 			continue
 		}
-		// Also skip by tag-name pattern as a secondary guard for releases whose
-		// prerelease flag may not be set correctly (e.g. some RC tags).
+		// Secondary guard: some RC tags don't have the prerelease flag set correctly.
 		lower := strings.ToLower(r.TagName)
 		if strings.Contains(lower, "-rc") || strings.Contains(lower, "-beta") || strings.Contains(lower, "-alpha") {
 			continue
 		}
-		// Strip "v" prefix to match Docker Hub tag format (e.g. "v11.6.0" → "11.6.0").
-		version := strings.TrimPrefix(r.TagName, "v")
+		version := strings.TrimPrefix(r.TagName, "v") // Docker Hub uses "11.6.0" not "v11.6.0"
 		s.Logger.WithField("version", version).Info("[resolveE2EServerVersion] Resolved latest Mattermost server version")
 
 		s.e2eVersionCacheLock.Lock()