Fix race condition when re-triggering GitHub Actions

When a GitHub Action is re-triggered, GitHub temporarily removes the old
check status before the new run starts. This causes a race where Tide may
merge the PR during the brief window when the check is missing.

This fix tracks previously seen contexts per PR/commit. When a required
context disappears, it's treated as PENDING to prevent premature merging.

Signed-off-by: Sascha Grunert <[email protected]>

Author: saschagrunert

pkg/tide/status.go

@@ -62,6 +62,40 @@ type storedState struct {
 	PreviousQuery string
 }
 
+// contextHistory tracks previously seen contexts for a PR/commit combination
+// to detect when required contexts disappear (e.g., when GitHub Actions are re-triggered)
+type contextHistory struct {
+	prCommitContexts map[string]sets.Set[string]
+	sync.Mutex
+}
+
+func newContextHistory() *contextHistory {
+	return &contextHistory{
+		prCommitContexts: make(map[string]sets.Set[string]),
+	}
+}
+
+// checkAndUpdate returns contexts that disappeared since last check, then updates history.
+// This detects the race condition when GitHub Actions are re-triggered and the check
+// status temporarily disappears before the new run starts.
+func (ch *contextHistory) checkAndUpdate(pr *CodeReviewCommon, currentContexts []string) []string {
+	key := fmt.Sprintf("%s/%s#%d:%s", pr.Org, pr.Repo, pr.Number, pr.HeadRefOID)
+
+	ch.Lock()
+	defer ch.Unlock()
+
+	var disappeared []string
+	if previous, exists := ch.prCommitContexts[key]; exists {
+		currentSet := sets.New[string](currentContexts...)
+		for ctx := range previous.Difference(currentSet) {
+			disappeared = append(disappeared, ctx)
+		}
+	}
+
+	ch.prCommitContexts[key] = sets.New[string](currentContexts...)
+	return disappeared
+}
+
 // statusController is a goroutine runs in the background
 type statusController struct {
 	pjClient           ctrlruntimeclient.Client
@@ -106,6 +140,9 @@ type statusUpdate struct {
 	// newPoolPending is a size 1 chan that signals that the main Tide loop has
 	// updated the 'poolPRs' field with a freshly updated pool.
 	newPoolPending chan bool
+	// contextHistory tracks previously seen contexts to detect disappearing contexts
+	// when GitHub Actions are re-triggered (addresses issue #337)
+	contextHistory *contextHistory
 }
 
 func (sc *statusController) shutdown() {
@@ -238,7 +275,8 @@ func requirementDiff(pr *PullRequest, q *config.TideQuery, cc contextChecker) (s
 	log := logrus.WithFields(pr.logFields())
 	for _, commit := range pr.Commits.Nodes {
 		if commit.Commit.OID == pr.HeadRefOID {
-			for _, ctx := range unsuccessfulContexts(append(commit.Commit.Status.Contexts, checkRunNodesToContexts(log, commit.Commit.StatusCheckRollup.Contexts.Nodes)...), cc, log) {
+			crc := CodeReviewCommonFromPullRequest(pr)
+			for _, ctx := range unsuccessfulContexts(append(commit.Commit.Status.Contexts, checkRunNodesToContexts(log, commit.Commit.StatusCheckRollup.Contexts.Nodes)...), cc, crc, nil, log) {
 				contexts = append(contexts, string(ctx.Context))
 			}
 		}

pkg/tide/tide.go

@@ -406,6 +406,7 @@ func NewController(
 	statusUpdate := &statusUpdate{
 		dontUpdateStatus: &threadSafePRSet{},
 		newPoolPending:   make(chan bool),
+		contextHistory:   newContextHistory(),
 	}
 
 	sc, err := newStatusController(ctx, logger, ghcStatus, mgr, gc, cfg, opener, statusURI, mergeChecker, usesGitHubAppsAuth, statusUpdate)
@@ -675,7 +676,11 @@ func (c *syncController) filterSubpools(mergeAllowed func(*CodeReviewCommon) (st
 				return
 			}
 			key := poolKey(sp.org, sp.repo, sp.branch)
-			if spFiltered := filterSubpool(c.provider, mergeAllowed, sp); spFiltered != nil {
+			var ch *contextHistory
+			if c.statusUpdate != nil {
+				ch = c.statusUpdate.contextHistory
+			}
+			if spFiltered := filterSubpool(c.provider, mergeAllowed, sp, ch); spFiltered != nil {
 				sp.log.WithField("key", key).WithField("pool", spFiltered).Debug("filtered sub-pool")
 
 				lock.Lock()
@@ -727,10 +732,10 @@ func (c *syncController) initSubpoolData(sp *subpool) error {
 // should be deleted.
 //
 // This function works for any source code provider.
-func filterSubpool(provider provider, mergeAllowed func(*CodeReviewCommon) (string, error), sp *subpool) *subpool {
+func filterSubpool(provider provider, mergeAllowed func(*CodeReviewCommon) (string, error), sp *subpool, ch *contextHistory) *subpool {
 	var toKeep []CodeReviewCommon
 	for _, pr := range sp.prs {
-		if !filterPR(provider, mergeAllowed, sp, &pr) {
+		if !filterPR(provider, mergeAllowed, sp, &pr, ch) {
 			toKeep = append(toKeep, pr)
 		}
 	}
@@ -752,7 +757,7 @@ func filterSubpool(provider provider, mergeAllowed func(*CodeReviewCommon) (stri
 //     retesting them.)
 //
 // This function works for any source code provider.
-func filterPR(provider provider, mergeAllowed func(*CodeReviewCommon) (string, error), sp *subpool, pr *CodeReviewCommon) bool {
+func filterPR(provider provider, mergeAllowed func(*CodeReviewCommon) (string, error), sp *subpool, pr *CodeReviewCommon, ch *contextHistory) bool {
 	log := sp.log.WithFields(pr.logFields())
 	// Skip PRs that are known to be unmergeable.
 	if reason, err := mergeAllowed(pr); err != nil {
@@ -778,7 +783,7 @@ func filterPR(provider provider, mergeAllowed func(*CodeReviewCommon) (string, e
 		}
 		return false
 	}
-	for _, ctx := range unsuccessfulContexts(contexts, sp.cc[pr.Number], log) {
+	for _, ctx := range unsuccessfulContexts(contexts, sp.cc[pr.Number], pr, ch, log) {
 		if ctx.State != githubql.StatusStatePending {
 			log.WithField("context", ctx.Context).Debug("filtering out PR as unsuccessful context is not pending")
 			return true
@@ -853,7 +858,11 @@ func (c *syncController) isPassingTests(log *logrus.Entry, pr *CodeReviewCommon,
 		// If we can't get the status of the commit, assume that it is failing.
 		return false
 	}
-	unsuccessful := unsuccessfulContexts(contexts, cc, log)
+	var ch *contextHistory
+	if c.statusUpdate != nil {
+		ch = c.statusUpdate.contextHistory
+	}
+	unsuccessful := unsuccessfulContexts(contexts, cc, pr, ch, log)
 	return len(unsuccessful) == 0
 }
 
@@ -862,8 +871,12 @@ func (c *syncController) isPassingTests(log *logrus.Entry, pr *CodeReviewCommon,
 // If the branchProtection is set to only check for required checks, we will skip
 // all non-required tests. If required tests are missing from the list, they will be
 // added to the list of failed contexts.
-func unsuccessfulContexts(contexts []Context, cc contextChecker, log *logrus.Entry) []Context {
+// It also detects when required contexts disappear (e.g., when GitHub Actions are re-triggered)
+// and treats them as PENDING to prevent premature merging (addresses issue #337).
+func unsuccessfulContexts(contexts []Context, cc contextChecker, pr *CodeReviewCommon, ch *contextHistory, log *logrus.Entry) []Context {
 	var failed []Context
+	contextNames := contextsToStrings(contexts)
+
 	for _, ctx := range contexts {
 		if string(ctx.Context) == statusContext {
 			continue
@@ -875,13 +888,31 @@ func unsuccessfulContexts(contexts []Context, cc contextChecker, log *logrus.Ent
 			failed = append(failed, ctx)
 		}
 	}
-	for _, c := range cc.MissingRequiredContexts(contextsToStrings(contexts)) {
+
+	// Add missing required contexts
+	for _, c := range cc.MissingRequiredContexts(contextNames) {
 		failed = append(failed, newExpectedContext(c))
 	}
 
+	// Check for disappeared contexts (race condition fix for issue #337)
+	// When a GitHub Action is re-triggered, the check may temporarily disappear
+	// from the status before the new run starts. Detect this and treat disappeared
+	// required contexts as non-passing to prevent premature merging.
+	if ch != nil && pr != nil {
+		for _, c := range ch.checkAndUpdate(pr, contextNames) {
+			if !cc.IsOptional(c) {
+				failed = append(failed, Context{
+					Context:     githubql.String(c),
+					State:       githubql.StatusStatePending,
+					Description: githubql.String("Context disappeared - likely re-triggered"),
+				})
+			}
+		}
+	}
+
 	log.WithFields(logrus.Fields{
 		"total_context_count":  len(contexts),
-		"context_names":        contextsToStrings(contexts),
+		"context_names":        contextNames,
 		"failed_context_count": len(failed),
 		"failed_context_names": contextsToStrings(failed),
 	}).Debug("Filtered out failed contexts")

pkg/tide/tide_test.go

@@ -2908,7 +2908,7 @@ func TestFilterSubpool(t *testing.T) {
 				mergeChecker: mmc,
 				logger:       logrus.WithContext(context.Background()),
 			}
-			filtered := filterSubpool(provider, mmc.isAllowedToMerge, sp)
+			filtered := filterSubpool(provider, mmc.isAllowedToMerge, sp, newContextHistory())
 			if len(tc.expectedPRs) == 0 {
 				if filtered != nil {
 					t.Fatalf("Expected subpool to be pruned, but got: %v", filtered)
@@ -5482,3 +5482,92 @@ func TestSerialRetestingConsidersPRThatIsCurrentlyBeingSRetested(t *testing.T) {
 	}
 
 }
+
+// TestUnsuccessfulContextsWithDisappearedContexts tests the fix for issue #337
+// where re-triggering GitHub Actions causes contexts to temporarily disappear
+func TestUnsuccessfulContextsWithDisappearedContexts(t *testing.T) {
+	log := logrus.NewEntry(logrus.StandardLogger())
+
+	testCases := []struct {
+		name             string
+		contexts         []Context
+		previousContexts []string
+		requiredContexts []string
+		expectedFailed   int
+	}{
+		{
+			name: "no disappeared contexts",
+			contexts: []Context{
+				{Context: githubql.String("test-1"), State: githubql.StatusStateSuccess},
+				{Context: githubql.String("test-2"), State: githubql.StatusStateSuccess},
+			},
+			previousContexts: []string{"test-1", "test-2"},
+			requiredContexts: []string{"test-1", "test-2"},
+			expectedFailed:   0,
+		},
+		{
+			name: "context disappeared - race condition",
+			contexts: []Context{
+				{Context: githubql.String("test-1"), State: githubql.StatusStateSuccess},
+			},
+			previousContexts: []string{"test-1", "test-2"},
+			requiredContexts: []string{"test-1", "test-2"},
+			expectedFailed:   2, // one missing required + one disappeared
+		},
+		{
+			name: "optional context disappeared - should not fail",
+			contexts: []Context{
+				{Context: githubql.String("test-1"), State: githubql.StatusStateSuccess},
+			},
+			previousContexts: []string{"test-1", "test-2"},
+			requiredContexts: []string{"test-1"},
+			expectedFailed:   1, // test-2 is unknown, treated as required by default
+		},
+		{
+			name: "first time seeing PR - no history",
+			contexts: []Context{
+				{Context: githubql.String("test-1"), State: githubql.StatusStateSuccess},
+			},
+			previousContexts: nil,
+			requiredContexts: []string{"test-1", "test-2"},
+			expectedFailed:   1, // only missing required, no disappeared
+		},
+		{
+			name: "context reappeared after disappearing",
+			contexts: []Context{
+				{Context: githubql.String("test-1"), State: githubql.StatusStateSuccess},
+				{Context: githubql.String("test-2"), State: githubql.StatusStatePending},
+			},
+			previousContexts: []string{"test-1"},
+			requiredContexts: []string{"test-1", "test-2"},
+			expectedFailed:   1, // test-2 is pending (not disappeared, it's present)
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			ch := newContextHistory()
+			pr := &CodeReviewCommon{
+				Org:        "test-org",
+				Repo:       "test-repo",
+				Number:     123,
+				HeadRefOID: "abc123",
+			}
+
+			// Set up previous context history if provided
+			if tc.previousContexts != nil {
+				ch.checkAndUpdate(pr, tc.previousContexts)
+			}
+
+			cc := &config.TideContextPolicy{
+				RequiredContexts: tc.requiredContexts,
+			}
+
+			failed := unsuccessfulContexts(tc.contexts, cc, pr, ch, log)
+
+			if len(failed) != tc.expectedFailed {
+				t.Errorf("expected %d failed contexts, got %d: %+v", tc.expectedFailed, len(failed), failed)
+			}
+		})
+	}
+}