You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
- No title change needed (already clear and specific)
- Added missing context: root cause explanation and technical mechanism
- Explains GitHub's CheckRun removal behavior during re-trigger
- References PR kubernetes-sigs#563 which implements the fix
- Labels: /priority important-soon (can cause incorrect merges)
- No difficulty label (Level 3 issue, requires expertise)
- Recommendation: Post this comment (adds valuable context)
Copy file name to clipboardExpand all lines: ISSUE-TRIAGE.md
+57-3Lines changed: 57 additions & 3 deletions
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -447,11 +447,65 @@ While the solution is well-defined by PR #563, several factors elevate this to L
447
447
**Alternative Consideration**:
448
448
If PR #563 already implements a complete solution and only needs review, the remaining work (code review, testing verification, deployment) might be Level 2. However, implementing this from scratch is definitively Level 3.
449
449
450
+
## Proposed Issue Augmentation
451
+
452
+
### Title Change
453
+
-**No change needed**: Current title "Tide merges PR when retesting GitHub action" is clear and specific
454
+
455
+
### Proposed GitHub Comment
456
+
457
+
```
458
+
## Root Cause
459
+
460
+
This is a race condition in Tide's context checking logic. When a GitHub Action is re-triggered, GitHub temporarily **removes** the old CheckRun from its API before creating the new one. During this brief window (typically a few seconds), the required check is completely missing from GitHub's status API. If Tide's sync loop runs during this window, it sees "no unsuccessful contexts" and incorrectly proceeds with the merge.
461
+
462
+
## Technical Details
463
+
464
+
The issue occurs in the context evaluation flow (`pkg/tide/tide.go:865-889`). Tide doesn't currently track which contexts were previously seen for a commit. When a required context disappears, there's no way to distinguish between "context never existed" (might be legitimate) versus "context disappeared" (suspicious, likely a re-trigger). This allows the race window where GitHub has removed the old CheckRun but hasn't created the new one yet.
465
+
466
+
PR #563 addresses this by maintaining state of previously-seen contexts per PR/commit. When a context that was previously observed disappears, it's treated as PENDING rather than missing, preventing premature merges during the re-trigger window.
467
+
468
+
/priority important-soon
469
+
```
470
+
471
+
### Rationale
472
+
473
+
**What's being added**:
474
+
-**Root cause explanation**: Original issue described the symptoms (Tide merges during re-trigger) but not why it happens (GitHub's API behavior of removing old CheckRun before creating new one)
475
+
-**Technical mechanism**: Explains the specific code flow and why the race occurs (missing context detection can't distinguish between never-existed and disappeared)
476
+
-**Solution reference**: Points to PR #563 which implements the fix via context state tracking
477
+
-**Priority label**: This can cause incorrect merges in production deployments
478
+
479
+
**Why these labels**:
480
+
-`/area tide`: Already applied - correct, this is Tide's merge logic
481
+
-`/kind bug`: Already applied - correct, this is a race condition bug
482
+
-`/priority important-soon`: **NEW** - Warrants priority because it can cause incorrect merges, affecting production deployments. Not critical-urgent (doesn't break the system) but important enough to prioritize
483
+
-**No difficulty label**: Level 3 issue requiring expertise in concurrency and Tide architecture - not appropriate for good-first-issue or help-wanted
484
+
485
+
**What's NOT included**:
486
+
- Full code analysis details: Too technical for issue comment, available in PR #563
487
+
- Effort assessment details: Internal triage information, not needed on public issue
488
+
- Multiple solution approaches: PR #563 already has the recommended approach
489
+
- Detailed reproduction steps: Original issue already has example PR demonstrating the problem
490
+
491
+
### Should This Comment Be Posted?
492
+
493
+
**Recommendation**: Yes, post this comment.
494
+
495
+
**Reasons**:
496
+
- Adds valuable context not in original issue (root cause)
497
+
- Explains technical mechanism concisely
498
+
- Points to the fix (PR #563) for anyone wanting to help
499
+
- Priority label is warranted and should be explained
500
+
- Respectful to original reporter (doesn't duplicate their good work)
501
+
- Helps future readers understand the issue quickly
502
+
450
503
## Next Steps
451
504
452
505
1. ✅ **Review PR #563**: Solution tracks previously seen contexts to detect disappearing checks
453
506
2. ✅ **Root cause identified**: GitHub removes old CheckRun before new one starts during re-trigger
454
507
3. ✅ **Effort assessed**: Level 3 - Requires expertise due to concurrency complexity
0 commit comments