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) {
@@ -909,6 +921,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 +1126,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

@@ -770,17 +770,28 @@ func (s *Server) cleanupStaleNonPRE2EInstances() {
 }
 
 // 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.
+// If E2EServerVersion is "latest" (or empty/whitespace, which is treated as "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.
+//
+// Empty/whitespace handling: a missing or blank E2EServerVersion in the deployed config
+// previously flowed through as "" to CreateInstallation, which silently failed. Treating
+// it as "latest" makes the misconfiguration recoverable and is loudly logged so the
+// operator can fix the config without losing E2E coverage in the meantime.
 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

server/push_events.go

@@ -29,26 +29,53 @@ func (s *Server) handlePushEvent(event *github.PushEvent) {
 	// Detect if this is a release branch push
 	if s.Config.E2EAutoTriggerOnRelease && s.isReleaseBranch(branch) {
 		logger.WithField("type", "release_branch").Info("Release branch detected, triggering E2E tests")
-		version := extractVersionFromReleaseBranch(branch, s.Config.E2EReleasePatternPrefix)
-		go s.handlePushEventE2E(event, branch, version)
+		go s.handlePushEventE2E(event, branch)
 		return
 	}
 
 	// Detect if this is a master/main branch push
 	if s.Config.E2EAutoTriggerOnMaster && (branch == "master" || branch == "main") {
 		logger.WithField("type", "master_main").Info("Master/main branch detected, triggering E2E tests")
-		go s.handlePushEventE2E(event, branch, "")
+		go s.handlePushEventE2E(event, branch)
 		return
 	}
 
-	logger.Debug("Push event does not match E2E trigger conditions")
+	// Promoted from Debug to Info so operators can see why an event was dropped
+	// without having to enable debug logging. The field values reveal which gate
+	// closed (auto-trigger flag off, release prefix mismatch, or unrecognised branch).
+	logger.WithFields(logrus.Fields{
+		"auto_release":           s.Config.E2EAutoTriggerOnRelease,
+		"auto_master":            s.Config.E2EAutoTriggerOnMaster,
+		"release_pattern_prefix": s.Config.E2EReleasePatternPrefix,
+		"is_release_branch":      s.isReleaseBranch(branch),
+	}).Info("Push event does not match E2E trigger conditions")
 }
 
-// isReleaseBranch checks if a branch name is a release branch
+// isReleaseBranch checks if a branch name is a release branch.
+// An empty E2EReleasePatternPrefix is rejected — strings.HasPrefix(any, "") is
+// always true, which would silently mark every branch (including master/main and
+// feature branches) as a release branch.
 func (s *Server) isReleaseBranch(branch string) bool {
+	if s.Config.E2EReleasePatternPrefix == "" {
+		return false
+	}
 	return strings.HasPrefix(branch, s.Config.E2EReleasePatternPrefix)
 }
 
+// serverVersionForPushEvent returns the Mattermost server version to provision for
+// any push event (release branch, master, or main). Always uses the latest stable
+// release; the branch name is intentionally ignored.
+//
+// Why we don't derive a version from the branch: a "release-9.0" branch would yield
+// "9.0", but Docker Hub publishes full SemVer tags like "9.0.0". Provisioning with
+// "9.0" results in an installation create error and the E2E workflow is never
+// dispatched. The "Latest Version Servers" feature (PR #87) is unambiguous — push
+// events always test against the latest stable release.
+func (s *Server) serverVersionForPushEvent(branch string) string {
+	_ = branch // intentionally unused; documented above
+	return s.resolveE2EServerVersion()
+}
+
 // extractBranchName extracts the branch name from the ref
 // GitHub sends refs in the format "refs/heads/branch-name"
 func extractBranchName(ref string) string {
@@ -59,17 +86,11 @@ func extractBranchName(ref string) string {
 	return strings.Join(parts[2:], "/")
 }
 
-// extractVersionFromReleaseBranch extracts version from release branch name
-// Example: "release-8.0" -> "8.0"
-func extractVersionFromReleaseBranch(branch string, prefix string) string {
-	if !strings.HasPrefix(branch, prefix) {
-		return ""
-	}
-	return strings.TrimPrefix(branch, prefix)
-}
-
-// handlePushEventE2E orchestrates E2E testing for push events (release or master/main)
-func (s *Server) handlePushEventE2E(event *github.PushEvent, branch string, version string) {
+// handlePushEventE2E orchestrates E2E testing for push events (release or master/main).
+// The branch name is used only to classify the run (RELEASE vs MASTER) and to dispatch
+// the workflow against the correct ref. The Mattermost server version provisioned for
+// testing is always the latest stable release — see serverVersionForPushEvent.
+func (s *Server) handlePushEventE2E(event *github.PushEvent, branch string) {
 	repoName := event.GetRepo().GetName()
 	commit := event.GetHeadCommit()
 	sha := ""
@@ -78,10 +99,9 @@ func (s *Server) handlePushEventE2E(event *github.PushEvent, branch string, vers
 	}
 
 	logger := s.Logger.WithFields(logrus.Fields{
-		"repo":    repoName,
-		"branch":  branch,
-		"version": version,
-		"sha":     sha,
+		"repo":   repoName,
+		"branch": branch,
+		"sha":    sha,
 	})
 
 	// Determine if this is a desktop or mobile repository
@@ -109,7 +129,7 @@ func (s *Server) handlePushEventE2E(event *github.PushEvent, branch string, vers
 	logger.WithField("instanceType", instanceType).Info("Creating E2E instances for push event")
 
 	// Create instances based on repo type
-	instances, err := s.createMultipleE2EInstancesForPushEvent(repoName, instanceType, branch, version, sha)
+	instances, err := s.createMultipleE2EInstancesForPushEvent(repoName, instanceType, branch, sha)
 	if err != nil {
 		logger.WithError(err).Error("Failed to create E2E instances")
 		return
@@ -147,7 +167,7 @@ func (s *Server) handlePushEventE2E(event *github.PushEvent, branch string, vers
 
 // createMultipleE2EInstancesForPushEvent creates all platform instances in parallel.
 // Results are returned in platforms[] order so index-based assignment is stable.
-func (s *Server) createMultipleE2EInstancesForPushEvent(repoName, instanceType, branch, version, _ string) ([]*E2EInstance, error) {
+func (s *Server) createMultipleE2EInstancesForPushEvent(repoName, instanceType, branch, _ string) ([]*E2EInstance, error) {
 	var platforms []string
 	if instanceType == "desktop" {
 		platforms = []string{"linux", "macos", "windows"}
@@ -161,11 +181,10 @@ func (s *Server) createMultipleE2EInstancesForPushEvent(repoName, instanceType,
 		"platformCount": len(platforms),
 	})
 
-	// Name format: {type}-{version}-{platform}-{hex6}
-	serverVersion := s.resolveE2EServerVersion()
-	if version != "" {
-		serverVersion = version
-	}
+	// Name format: {type}-{version}-{platform}-{hex6}.
+	// Always provision the latest stable Mattermost release for push events —
+	// see serverVersionForPushEvent for why branch-derived versions are not used.
+	serverVersion := s.serverVersionForPushEvent(branch)
 	sanitizedVersion := sanitizeForDNS(serverVersion)
 	uid := e2eUniqueSuffix()
 

server/workflow_run.go

@@ -157,7 +157,14 @@ func (s *Server) handleWorkflowRunEventWithInputs(payload *WorkflowRunWebhookPay
 		return
 	}
 
-	logger.Debug("Ignoring workflow_run event (not relevant to E2E lifecycle)")
+	// Promoted from Debug to Info so operators can see why a workflow_run event was
+	// ignored without enabling debug logging. The most common cause of "nightly never
+	// fires" is workflowName not matching the configured E2ENightlyTriggerWorkflowName
+	// — this log line surfaces the comparison directly.
+	logger.WithFields(logrus.Fields{
+		"configured_nightly_name":   s.Config.E2ENightlyTriggerWorkflowName,
+		"configured_test_workflows": s.Config.E2ETestWorkflowNames,
+	}).Info("Ignoring workflow_run event (not relevant to E2E lifecycle)")
 }
 
 // handleNightlyE2ETrigger provisions instances and dispatches the test workflow.