feat: add skip CI command support #2309

theakshaypant · 2025-11-03T10:40:09Z

📝 Description of the Change

Implement support for skip CI commands in commit messages to allow users to skip PipelineRun execution. Supports [skip ci], [ci skip], [skip tkn], and [tkn skip] commands.

When a skip command is detected in the commit message, PipelineRun execution is skipped. However, GitOps commands (/test, /retest, etc.) will still trigger PipelineRuns regardless of the skip command, allowing users to manually trigger CI when needed.

👨🏻‍ Linked Jira

https://issues.redhat.com/browse/SRVKP-8933

🔗 Linked GitHub Issue

N/A

🚀 Type of Change

🧪 Testing Strategy

🤖 AI Assistance

I have not used any AI assistance for this PR.
I have used AI assistance for this PR.

If you have used AI assistance, please provide the following details:

Which LLM was used?

Extent of AI Assistance:

Documentation and research only
Unit tests or E2E tests only
Code generation (parts of the code)
Full code generation (most of the PR)
PR description and comments
Commit message(s)

Important

If the majority of the code in this PR was generated by an AI, please add a Co-authored-by trailer to your commit message.
For example:

Co-authored-by: Gemini gemini@google.com
Co-authored-by: ChatGPT noreply@chatgpt.com
Co-authored-by: Claude noreply@anthropic.com
Co-authored-by: Cursor noreply@cursor.com
Co-authored-by: Copilot Copilot@users.noreply.github.com

**💡You can use the script ./hack/add-llm-coauthor.sh to automatically add
these co-author trailers to your commits.

✅ Submitter Checklist

pipelines-as-code · 2025-11-03T10:40:33Z

🔍 PR Lint Feedback

Note: This automated check helps ensure your PR follows our contribution guidelines.

⚠️ Items that need attention:

🤖 AI attribution

The following commits lack an explicit AI attribution footer:

5a43e25 feat: add skip CI command support

If no AI assistance was used for a commit, you can ignore this warning.
Otherwise add an Assisted-by: or Co-authored-by: footer referencing the AI used.

ℹ️ Next Steps

Review and address the items above
Push new commits to update this PR
This comment will be automatically updated when issues are resolved

🔧 Admin Tools (click to expand)

Automated Issue/Ticket Creation:

/issue-create - Generate a GitHub issue from this PR content using AI
/jira-create - Create a SRVKP Jira ticket from this PR content using AI

⚠️ Important: Always review and edit generated content before finalizing tickets/issues.
The AI-generated content should be used as a starting point and may need adjustments.

These commands are available to maintainers and will post the generated content as PR comments for review.

_{🤖 This feedback was generated automatically by the PR CI system}

zakisk · 2025-11-03T14:13:59Z

pkg/pipelineascode/pipelineascode.go

+	// Check skip logic after annotation matching, so that the EventType has been properly updated.
+	// In order for the PR to be skipped, the commit message should include the skip command.
+	// We don't want to skip the PR when the event is a GitOps command. For example, if the
+	// user has added `[skip ci]` to the commit, they don't need to push again to trigger the
+	// PipelineRun, they can simply add a comment to trigger the CI via `/test pr-name` or on-comment.
+	if p.event.HasSkipCommand && !opscomments.IsAnyOpsEventType(p.event.EventType) {
+		return nil
+	}


@theakshaypant you don't need to let the flow come here, you can skip the event right there in ParsePayload func of every provider with an appropriate message

for example, we do for skip-push-events-for-pr-commits

pipelines-as-code/pkg/provider/github/parse_payload.go

Line 350 in f3db57f

if v.pacInfo.SkipPushEventForPRCommits && len(prs) > 0 {

Thanks! This is helpful.
I was trying to keep this check in providers initially but was facing an issue where the on-comment event type (hence the first line of the comment) was a no-ops event before annotation matching.
I will try the flow you suggested.

@zakisk ! I was trying out your suggestion and IIUC we are doing an early exit by having the check here, right? Would it be correct to say that we can only early exit for push event since the payload contains the commit message? We will still have to wait for GetCommitInfo in case of pull_request event type and of course we handle the comments at a later stage as well.

as discussed we're going to ignore comment events under this feature, we only do it in push and pull_request and if you don't have commit message in pull_request payload then you have github client in parse_payload you can get the commit message there. and also you need to assign appropriate values to the fields in the event struct which are being assigned on call of GetCommitInfo so that you can avoid double API call to GetCommit endpoint.

theakshaypant · 2025-11-05T09:27:59Z

pkg/pipelineascode/match_test.go

 			wantErr:       true,
 			wantErrMsg:    "failed to run create status, user is not allowed to run the CI",
 		},
-		{


"commit not found" test removed - GetCommitInfo is now called earlier in sinker.go for PR events, not in verifyRepoAndUser. The error "could not find commit info" would be caught in processEvent before reaching verifyRepoAndUser.

theakshaypant · 2025-11-05T09:28:48Z

pkg/pipelineascode/pipelineascode.go

+	// Defensive skip-CI check: this is a safety net in case events bypass the early check in sinker.
+	// Primary skip detection happens in sinker.processEvent() for performance, but this ensures
+	// nothing slips through (e.g., tests that call Run() directly, or edge cases).
+	// Skip only for non-GitOps events (GitOps commands can override skip-CI).
+	if p.event.HasSkipCommand && !opscomments.IsAnyOpsEventType(p.event.EventType) {
+		p.logger.Infof("CI skipped: commit contains skip command in message (secondary check)")
+		return nil
+	}
+


As mentioned in the comment, have kept this as a safety net, ideally this would not be required.

theakshaypant · 2025-11-05T09:30:08Z

pkg/pipelineascode/match.go

+	// Set up authenticated client with proper token scoping
+	// NOTE: This is typically already done in sinker.processEvent() for all event types,
+	// but we call it here as a safety net for edge cases (e.g., tests calling Run() directly,
+	// or if the early setup in sinker failed/was skipped). The call is idempotent.
+	err = SetupAuthenticatedClient(ctx, p.vcx, p.k8int, p.run, p.event, repo, p.globalRepo, p.pacInfo, p.logger)


Open to hearing if we need this. I saw an error during early setup of git client so added this backup.
I was not able to reproduce that issue. Again, ideally this would not be required.

theakshaypant · 2025-11-05T09:32:43Z

pkg/provider/github/parse_payload.go

-	// regenerate token scoped to the repo IDs
-	if v.pacInfo.SecretGHAppRepoScoped && installationIDFrompayload != -1 && len(v.RepositoryIDs) > 0 {
-		repoLists := []string{}
-		if v.pacInfo.SecretGhAppTokenScopedExtraRepos != "" {
-			// this is going to show up a lot in the logs but i guess that
-			// would make people fix the value instead of being lost into
-			// the top of the logs at controller start.
-			for _, configValue := range strings.Split(v.pacInfo.SecretGhAppTokenScopedExtraRepos, ",") {
-				configValueS := strings.TrimSpace(configValue)
-				if configValueS == "" {
-					continue
-				}
-				repoLists = append(repoLists, configValueS)
-			}
-			v.Logger.Infof("Github token scope extended to %v keeping SecretGHAppRepoScoped to true", repoLists)
-		}
-		token, err := v.CreateToken(ctx, repoLists, processedEvent)
-		if err != nil {
-			return nil, err
-		}
-		processedEvent.Provider.Token = token
-	}
-


Have moved the token scoping to client_setup to avoid calling it twice.
Did not consider a fail safe here, I guess. 😅

theakshaypant · 2025-11-05T09:33:33Z

pkg/provider/github/parse_payload_test.go

 			seedData:  vaildSecret,
 			nilClient: false,
 		},
-		{


Tests for repository ID setting and token scoping with extra repos have been moved to pkg/adapter/sinker_test.go:TestSetupClient_GitHubAppVsOther since this logic is handled in sinker.go via the centralized SetupAuthenticatedClient function.

test/github_skip_ci_test.go

theakshaypant · 2025-11-05T09:53:22Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces a well-implemented feature for skipping CI builds via commit messages. The changes are robust, featuring an efficient early check in the sinker and a reliable fallback mechanism. The refactoring to centralize client authentication is a significant improvement to the codebase's maintainability. The feature is consistently applied across all supported providers, and the accompanying documentation and extensive test coverage are excellent. I have a couple of suggestions to further enhance maintainability.

pkg/adapter/sinker_test.go

pkg/provider/provider.go

theakshaypant · 2025-11-05T10:14:11Z

/retest

theakshaypant · 2025-11-07T03:54:14Z

@chmouel @zakisk Should we also have a metric for skipped pipelines?

zakisk · 2025-11-09T18:34:27Z

pkg/adapter/sinker.go

+
+		// For ALL events: Setup authenticated client early (including token scoping)
+		// This centralizes client setup and token scoping in one place for all event types
+		repo, err := s.findMatchingRepository(ctx)
+		if err != nil {
+			// Continue with normal flow - repository matching will be handled in matchRepoPR
+			s.logger.Debugf("Could not find matching repository for early client setup: %v", err)
+		} else {
+			// We found the repository, now setup client with token scoping
+			// If setup fails here, it's a configuration error and we should fail fast
+			if err := s.setupClient(ctx, repo); err != nil {
+				return fmt.Errorf("client setup failed: %w", err)
+			}
+			s.logger.Debugf("Client setup completed early in sinker for event type: %s", s.event.EventType)
+		}
+
+		// For PUSH events: commit message is already in event.SHATitle from the webhook payload
+		// We can check immediately without any API calls or repository lookups
+		if s.event.EventType == "push" && provider.SkipCI(s.event.SHATitle) {
+			s.logger.Infof("CI skipped for push event: commit %s contains skip command in message", s.event.SHA)
+			return nil
+		}
+
+		// For PULL REQUEST events: commit message needs to be fetched via API
+		// Get commit info for skip-CI detection (only if we successfully set up client above)
+		if s.event.EventType == "pull_request" && repo != nil {
+			// Get commit info (including commit message) via API
+			if err := s.vcx.GetCommitInfo(ctx, s.event); err != nil {
+				return fmt.Errorf("could not get commit info: %w", err)
+			}
+			// Check for skip-ci commands in pull request events
+			if s.event.HasSkipCommand {
+				s.logger.Infof("CI skipped for pull request event: commit %s contains skip command in message", s.event.SHA)
+				return nil
+			}
+		}


as I've said before, you can move this to ParsePayload, as name suggests ParsePayload func is to verify the payload

Since this logic was common for all providers, I kept it here to avoid duplication.

yeah, but there is one more call to GetCommitInfo then that's not needed to avoid more API calls

There are a couple of calls which are duplicated in this code at the moment. I have highlighted some of them in my comments, looking for inputs for how useful (or expensive) they can be.

As for having GetCommitInfo only once, I would personally prefer having it here (moving it outside the pull_request conditional of course) rather than duplicating this logic for each provider. (And removing from match.go if we want to avoid multiple API calls.)

Open to hearing contrary perspective to this.

And removing from match.go if we want to avoid multiple API calls.

yeah, removing would work. and also call get_commit endpoint only if one of the field SHA, SHATitle, and ShaURL is empty so that we can avoid API call for GitLab etc.

if event.SHA == "" || event.SHATitle == "" || event.SHAURL == ""

docs/content/docs/guide/gitops_commands.md

zakisk · 2025-11-12T07:42:01Z

@theakshaypant IIRC, on the design doc we have agreed upon having a neutral check for skipped event letting user know that event is being skipped intentionally.
cc: @chmouel

theakshaypant · 2025-11-12T08:01:18Z

@theakshaypant IIRC, on the design doc we have agreed upon having a neutral check for skipped event letting user know that event is being skipped intentionally. cc: @chmouel

From my understanding, we only had started a discussion about adding a neutral check, but no final agreement was reached. The current implementation follows GitHub Actions’ behavior for skipped events, which keeps things consistent IMO.

For instance, if a CI has GitHub workflow and PipelineRun configured and a user adds [skip ci] to the commit message:

In current implementation, none of the pipelines would be listed.
For skipped pipeline behaviour, the status would should a neutral check for PAC CI and not list the GH workflows.

Happy to continue the discussion here.

Implement support for skip CI commands in commit messages to allow users to skip PipelineRun execution. Supports [skip ci], [ci skip], [skip tkn], and [tkn skip] commands. When a skip command is detected in the commit message, PipelineRun execution is skipped. However, GitOps commands (/test, /retest, etc.) will still trigger PipelineRuns regardless of the skip command, allowing users to manually trigger CI when needed. Jira: https://issues.redhat.com/browse/SRVKP-8933 Signed-off-by: Akshay Pant <akshay.akshaypant@gmail.com>

pipelines-as-code bot added feature New feature or request gitops providers labels Nov 3, 2025

theakshaypant force-pushed the SRVKP-8933-skip-ci branch 2 times, most recently from 86a3521 to 84ea887 Compare November 3, 2025 12:04

zakisk reviewed Nov 3, 2025

View reviewed changes

theakshaypant force-pushed the SRVKP-8933-skip-ci branch 2 times, most recently from 3947c53 to 752398f Compare November 5, 2025 09:40

theakshaypant commented Nov 5, 2025

View reviewed changes

gemini-code-assist bot reviewed Nov 5, 2025

View reviewed changes

pkg/adapter/sinker_test.go Outdated Show resolved Hide resolved

pkg/provider/provider.go Outdated Show resolved Hide resolved

theakshaypant force-pushed the SRVKP-8933-skip-ci branch 2 times, most recently from 5df28c0 to 963047e Compare November 5, 2025 10:50

theakshaypant marked this pull request as ready for review November 5, 2025 11:32

theakshaypant requested a review from zakisk November 6, 2025 04:41

zakisk reviewed Nov 9, 2025

View reviewed changes

docs/content/docs/guide/gitops_commands.md Outdated Show resolved Hide resolved

zakisk reviewed Nov 9, 2025

View reviewed changes

docs/content/docs/guide/gitops_commands.md Outdated Show resolved Hide resolved

zakisk reviewed Nov 9, 2025

View reviewed changes

docs/content/docs/guide/gitops_commands.md Outdated Show resolved Hide resolved

zakisk reviewed Nov 9, 2025

View reviewed changes

docs/content/docs/guide/gitops_commands.md Outdated Show resolved Hide resolved

theakshaypant force-pushed the SRVKP-8933-skip-ci branch from 963047e to 3ea4e39 Compare November 10, 2025 08:15

theakshaypant requested review from chmouel and zakisk November 10, 2025 11:01

theakshaypant force-pushed the SRVKP-8933-skip-ci branch from 3ea4e39 to 5a43e25 Compare November 12, 2025 12:09

feat: add skip CI command support #2309

Are you sure you want to change the base?

feat: add skip CI command support #2309

Uh oh!

Conversation

theakshaypant commented Nov 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📝 Description of the Change

👨🏻‍ Linked Jira

🔗 Linked GitHub Issue

🚀 Type of Change

🧪 Testing Strategy

🤖 AI Assistance

✅ Submitter Checklist

Uh oh!

pipelines-as-code bot commented Nov 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔍 PR Lint Feedback

⚠️ Items that need attention:

🤖 AI attribution

ℹ️ Next Steps

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

theakshaypant commented Nov 5, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

theakshaypant commented Nov 5, 2025

Uh oh!

theakshaypant commented Nov 7, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zakisk Nov 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zakisk commented Nov 12, 2025

Uh oh!

theakshaypant commented Nov 12, 2025

Uh oh!

Reviewers

Assignees

Labels

theakshaypant commented Nov 3, 2025 •

edited

Loading

pipelines-as-code bot commented Nov 3, 2025 •

edited

Loading

zakisk Nov 12, 2025 •

edited

Loading