-
Epic
-
Resolution: Won't Do
-
Major
-
None
-
None
-
improve-skip-push-event-for-pr-commits-to-only-skip-actual-duplicate-runs
-
False
-
-
False
-
To Do
-
-
Story (Required)
As a DevOps engineer trying to configure CI/CD pipelines that trigger only on push events I want the skip-push-event-for-pr-commits feature
to only skip push events when an actual duplicate PipelineRun would occur so that my pipelines are not incorrectly blocked when there are
open pull requests that would not create duplicate runs.
Background (Required)
The skip-push-event-for-pr-commits feature was introduced to prevent duplicate PipelineRuns when a commit appears in both a push event
and a pull request event. However, the current implementation has a logic flaw that causes false positives.
Current Behavior:
The feature checks if a commit SHA exists in ANY open pull request and skips ALL push events for that commit, regardless of whether:
- The pipeline is configured to trigger on PR events
- The push event's branch matches the PR's head or base branch
- An actual duplicate run would occur
Customer Impact:
Ford Motor Company (Case #04264459) is experiencing production issues where:
Developer 1 opens PR from qa → main (remains open for approval)
Developer 2 pushes to qa branch (different feature)
Developer 2's push event is SKIPPED because the commit is now "part of the open PR"
No pipeline runs, even though NO PR event trigger is configured
This affects multiple development teams working in parallel and has become a widespread issue across their production clusters.
Related Work:
SRVKP-9111: Fixed tag push events being skipped (resolved in 1.21)- SRVKP-9351: RFE to not auto-enable features on upgrade
Out of scope
- Changing the default value of skip-push-event-for-pr-commits (covered by SRVKP-9351)
- Supporting other Git providers beyond GitHub in this RFE (can be addressed separately)
- Advanced duplicate detection based on PipelineRun history (too complex due to retention policies)
Approach (Required)
Option 1: Branch-Aware Skip Logic (Recommended)
Enhance the duplicate detection logic in pkg/provider/github/parse_payload.go (lines 337-345) to be branch-aware:
// Current problematic logic: if v.pacInfo.SkipPushEventForPRCommits && len(prs) > 0 && !isGitTagEvent { isPartOfPR, prNumber := v.isCommitPartOfPullRequest(sha, org, repoName, prs) if isPartOfPR { // SKIPS ALL PUSH EVENTS - TOO AGGRESSIVE return nil, nil } } // Proposed improved logic: if v.pacInfo.SkipPushEventForPRCommits && len(prs) > 0 && !isGitTagEvent { // Extract the branch name from the push event pushBranch := extractBranchFromRef(gitEvent.GetRef()) // e.g., "refs/heads/qa" -> "qa" // Check if this push would duplicate a PR event isDuplicate, prNumber := v.wouldCreateDuplicateRun(sha, pushBranch, org, repoName, prs) if isDuplicate { v.Logger.Infof("Skipping push event for commit %s on branch %s as it would duplicate PR #%d", sha, pushBranch, prNumber) return nil, nil } }
The new wouldCreateDuplicateRun function should only return true when:
The commit is part of an open PR, AND
The push event's branch matches EITHER:
- The PR's head branch (the branch being merged FROM), OR
- The PR's base branch (the branch being merged TO)
Why this works:
In Ford's scenario:
- PR: qa → main (open)
- Push: to qa branch
- Push branch (qa) == PR head branch (qa) → Would skip (correct, as PR event would also trigger)
- Push: to dev branch (different developer)
- Push branch (dev) != PR head branch (qa) AND != PR base branch (main) → Would NOT skip (correct, no duplicate)
Alternative Option 2: CEL Expression Analysis
Query the repository's PipelineRun CEL expressions to determine if PR events are configured. Only skip push events if:
Commit is in open PR, AND
Repository has PipelineRuns with event == "pull_request" configured
This is more accurate but requires additional API calls and CEL parsing complexity.
Dependencies
- None - this is a standalone enhancement to existing functionality
Acceptance Criteria (Mandatory)
AC1: Branch Matching Logic
- GIVEN a commit that is part of an open PR from branch A to branch B
- WHEN a push event occurs on branch A (PR head branch)
- THEN the push event SHOULD be skipped
AC2: Non-Matching Branch Logic
- GIVEN a commit that is part of an open PR from branch A to branch B
- WHEN a push event occurs on branch C (unrelated branch)
- THEN the push event SHOULD NOT be skipped
AC3: Ford's Scenario
- GIVEN:
- PR open: qa → main
- Pipeline configured with: event == "push" && target_branch == "qa"
- NO PR event configuration
- WHEN a developer pushes to qa branch
- THEN pipeline run SHOULD trigger (currently broken)
AC4: Tag Events Remain Unaffected
- GIVEN the fix for
SRVKP-9111is in place - WHEN a tag is pushed for a commit in an open PR
- THEN the push event SHOULD NOT be skipped (already working in 1.21)
AC5: Backward Compatibility
- GIVEN repositories where push and PR events genuinely would duplicate
- WHEN a commit is pushed to a PR's head branch
- THEN the push event SHOULD be skipped (existing behavior preserved)
Edge Cases to Consider:
- Multiple open PRs containing the same commit
- PRs targeting different base branches
- Force pushes that change commit SHAs
- Merged PRs vs open PRs
INVEST Checklist
- Dependencies identified: None
- Blockers noted and expected delivery timelines set: No blockers
- Design is implementable: Yes, requires changes to pkg/provider/github/parse_payload.go
- Acceptance criteria agreed upon: Pending customer validation
- Story estimated: Pending engineering review
Done Checklist
- Code is completed, reviewed, documented and checked in
- Unit tests added to pkg/provider/github/parse_payload_test.go covering all acceptance criteria
- Integration tests added to test/github_pullrequest_test.go for Ford's specific scenario
- Documentation updated in docs/content/docs/install/settings.md to clarify the behavior
- Customer case #04264459 validated with fix
- Acceptance criteria are met
—
Additional Context
Customer Quote from Case:
According to the feature description, this option should prevent duplicate PipelineRuns. However, it should not incorrectly assume that a
run is a duplicate when it is not.
Log Evidence:
"msg":"Skipping push event for commit 925dcca61fcc6b33a325de16b9be54c7a5773d66 as it belongs to pull request #33"
File References:
- Implementation: pkg/provider/github/parse_payload.go:337-345
- Configuration: pkg/params/settings/config.go:76
- Tests: pkg/provider/github/parse_payload_test.go
- is blocked by
-
SRVKP-9978 Testing for the epic
-
- To Do
-