fix(gitlab): pin commit statuses to the same pipeline via pipeline_id caching by ab-ghosh · Pull Request #2671 · tektoncd/pipelines-as-code

ab-ghosh · 2026-04-09T17:34:10Z

📝 Description of the Change

When PAC posts commit statuses to GitLab for a merge request, the status updates can get split across two different GitLab pipelines. GitLab's auto-assignment logic can abruptly change routing mid-stream, leaving the original MR pipeline permanently stuck with stale intermediate statuses.
This fix caches the pipeline_id returned by the first SetCommitStatus response for a given (project, SHA) pair and passes it on all subsequent calls, ensuring all commit statuses land in the same GitLab pipeline.

🔗 Linked GitHub Issue

Fixes #
https://redhat.atlassian.net/browse/SRVKP-11437

🧪 Testing Strategy

🤖 AI Assistance

AI assistance can be used for various tasks, such as code generation,
documentation, or testing.

Please indicate whether you have used AI assistance
for this PR and provide details if applicable.

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

Important

Slop will be simply rejected, if you are using AI assistance you need to make sure you
understand the code generated and that it meets the project's standards. you
need at least know how to run the code and deploy it (if needed). See
startpaac to make it easy
to deploy and test your code changes.

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: Claude noreply@anthropic.com

✅ Submitter Checklist

codecov-commenter · 2026-04-09T17:37:32Z

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 87.75510% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.18%. Comparing base (88e8d39) to head (f00a0de).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/provider/gitlab/gitlab.go	87.75%	4 Missing and 2 partials ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2671      +/-   ##
==========================================
+ Coverage   59.12%   59.18%   +0.06%     
==========================================
  Files         208      208              
  Lines       20512    20552      +40     
==========================================
+ Hits        12127    12164      +37     
- Misses       7612     7614       +2     
- Partials      773      774       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

gemini-code-assist

Code Review

This pull request introduces a caching mechanism for GitLab pipeline IDs to ensure that multiple commit statuses for the same project and SHA are grouped within the same pipeline. It includes logic to retrieve, store, and apply these IDs during the status creation process, along with comprehensive unit tests. A critical issue was identified in the setOptPipelineID function where a stale pipeline ID from a source project could persist when falling back to a target project, potentially causing API failures.

pipelines-as-code · 2026-04-10T12:35:58Z

🤖 AI Analysis - pr-complexity-rating

Based on the provided information, this PR appears to be a synchronization/maintenance operation rather than a functional feature or bug fix implementation.

📊 PR Review Complexity

Dimension	Score	Rationale
Size	1	This is a merge commit; no functional diffs are introduced.
Logic complexity	1	Standard git merge; no new logic introduced.
Risk	1	Very low; purely synchronization of branches.
Cross-cutting	1	Limited to repository branch synchronization.
Test coverage	5	CI pipeline passed successfully.

Overall difficulty: Easy

Summary

This PR is a merge commit synchronizing main into the fix/gitlab-commit-status-pipeline-id-caching feature branch. The reviewer should primarily ensure that the merge did not introduce any regressions or conflicts that weren't resolved during the merge process.

Suggested reviewers focus

Conflict resolution: Verify that the merge didn't accidentally overwrite or revert recent changes in the feature branch.
CI Status: Confirm that the linters and tests passed (which they currently report as successful).

Generated by Pipelines-as-Code LLM Analysis

pipelines-as-code · 2026-04-10T12:46:31Z

🤖 AI Analysis - pr-complexity-rating

Based on the provided context, this PR appears to be a maintenance merge (synchronizing the feature branch with main) rather than a functional feature implementation or bug fix.

📊 PR Review Complexity

Dimension	Score	Rationale
Size	1	This is a merge commit; no direct logic changes are being introduced.
Logic complexity	1	Standard git merge reconciliation.
Risk	1	Low risk; standard synchronization.
Cross-cutting	1	Confined to branch synchronization.
Test coverage	5	CI pipeline passed successfully on the merged state.

Overall difficulty: Easy

Summary

This PR consists of a merge commit synchronizing a feature branch (fix/gitlab-commit-status-pipeline-id-caching) with the main branch. Reviewers should verify that no unexpected merge conflicts were introduced and that the CI pipeline remains green.

Suggested reviewers focus

Check Merge Integrity: Ensure that the automated merge did not accidentally revert or overwrite logic in the fix/gitlab-commit-status-pipeline-id-caching branch.
CI Status: Confirm that the existing test suite passes on the new merge commit.

Generated by Pipelines-as-Code LLM Analysis

chmouel · 2026-04-15T15:46:26Z

I am not sure I understand this PR, can you explain it to me? 😅

Your change get pipeline_id only remembered inside the current in-memory GitLab provider object. That helps only while that exact object is still being used. In PAC, the later/final status update is sent from a different phase of the system that creates a non shared new GitLab Provider{} instance, so that cached pipeline_id is lost.

In practice, this means the change may help for a few status calls in the same immediate flow, but it likely won’t keep all statuses for the PipelineRun attached to the same GitLab pipeline, which I think your goal.

Maybe we need to carry the pipeline_id via an annotation and getting read by the reconciler?

ab-ghosh · 2026-04-16T09:19:19Z

I am not sure I understand this PR, can you explain it to me? 😅

The issue which this PR addresses that when PAC posts commit statuses to GitLab for a merge request, the status updates can get split across two different GitLab pipelines. At some point during the continuous stream of status updates, GitLab stops associating them with the original MR pipeline and starts routing them to a new external pipeline. The original MR pipeline gets permanently stuck with stale intermediate statuses, while subsequent results sit in an orphaned pipeline invisible to the MR.
This issue can be because the pipeline association entirely up to GitLab's auto assignment logic, which can abruptly change routing mid-stream. To overcome this, my thought was to fetch the pipeline_id after the first status update returned by the first SetCommitStatus response pair and passes it on all subsequent calls

ab-ghosh · 2026-04-16T09:36:38Z

Your change get pipeline_id only remembered inside the current in-memory GitLab provider object. That helps only while that exact object is still being used. In PAC, the later/final status update is sent from a different phase of the system that creates a non shared new GitLab Provider{} instance, so that cached pipeline_id is lost.

yeah that's right, i missed this point, i think, as you suggested, after the first successful status update, we can add the pipeline_id via an annotation and then any subsequent CreateStatus call can reads it.

chmouel · 2026-04-17T08:44:08Z

can you make sure this is tested manually properly? without having to rely on the AI words?

zakisk · 2026-04-17T10:15:02Z

@ab-ghosh is your goal to have pipeline_id only for an event or for whole CI lifecycle of the merge request?

ab-ghosh · 2026-04-18T14:40:26Z

@zakisk it should be for whole CI lifecycle of the MR, so that for all subsequent status updates for the same SHA, we can pass the pipeline ID

zakisk · 2026-04-19T15:00:27Z

@zakisk it should be for whole CI lifecycle of the MR, so that for all subsequent status updates for the same SHA, we can pass the pipeline ID

Then you're only persistenting pipeline id for an event not whole lifecycle of the MR.

zakisk · 2026-04-21T05:21:20Z

/retest

theakshaypant

Could you share a screenshot of how this would be different from gitlab auto assigning the pipeline id.

zakisk · 2026-04-22T11:24:32Z

@ab-ghosh can you please add e2e test confirming that pipeline_id annotation is added on PipelineRuns

ab-ghosh · 2026-04-23T08:26:53Z

@zakisk Added the required E2E test for this feature

zakisk · 2026-04-24T06:16:16Z

+	if v.pipelineIDMu == nil {
+		v.pipelineIDMu = &sync.Mutex{}
+	}


why is it initialized here?

Removed. Moved the initialization to SetClient as suggested.

zakisk · 2026-04-24T06:19:40Z

+	if v.pipelineIDMu == nil {
+		v.pipelineIDMu = &sync.Mutex{}
+	}


you can just initialize it in SetClient provider func

zakisk · 2026-04-24T06:23:23Z

+	// Clear pipeline ID when falling back to the target project — the cached
+	// ID belongs to the source project's pipeline namespace and is invalid on
+	// a different project (fork MR scenario).


did you check this? if we set the PIpelineID to 0 then it will create new pipeline on subsequent events and issue is still there. and pipelines from commit status api are bound to SHA not project

commit status api are bound to SHA not project

Yeah, the main confusion was exactly this, I assumed pipelines from the commit status API are bound to the project, so I used a map by project ID and added a mutex. now i have used plain int64 and removed the map and mutex.

zakisk · 2026-04-24T06:25:10Z

+	// pipelineIDMu protects pipelineIDCache from concurrent access when
+	// multiple PipelineRun goroutines call CreateStatus simultaneously.
+	pipelineIDMu    *sync.Mutex
+	pipelineIDCache map[string]int64


its only per event so it should not be map[string]int64 you can just store it as int

Suggested change

pipelineIDCache map[string]int64

pipelineID int64

Ack, changed to pipelineID int64, removed the mutex as well

you shouldn't remove the mutex, its needed as this func is called in goroutine and multiple threads are accessing the pipelineID, my suggestion was to not use map for storing pipelineID

added the mutex

zakisk · 2026-04-24T06:29:41Z

+	if pid := v.getPipelineID(event.SHA); pid != 0 {
+		opt.PipelineID = gitlab.Ptr(pid)
+	} else if statusOpts.PipelineRun != nil {
+		if id, ok := statusOpts.PipelineRun.GetAnnotations()[keys.GitLabPipelineID]; ok {
+			pid, err := strconv.ParseInt(id, 10, 64)
+			if err == nil {
+				opt.PipelineID = gitlab.Ptr(pid)
+			}
+		}
+	}


Suggested change

if pid := v.getPipelineID(event.SHA); pid != 0 {

opt.PipelineID = gitlab.Ptr(pid)

} else if statusOpts.PipelineRun != nil {

if id, ok := statusOpts.PipelineRun.GetAnnotations()[keys.GitLabPipelineID]; ok {

pid, err := strconv.ParseInt(id, 10, 64)

if err == nil {

opt.PipelineID = gitlab.Ptr(pid)

}

}

}

if statusOpts.PipelineRun != nil {

if id, ok := statusOpts.PipelineRun.GetAnnotations()[keys.GitLabPipelineID]; ok {

pid, err := strconv.ParseInt(id, 10, 64)

if err == nil {

opt.PipelineID = gitlab.Ptr(pid)

v.pipelineIDMu.Lock()

v.pipelineID = gitlab.Ptr(pid)

v.pipelineIDMu.Unlock()

}

}

} else if v.pipelineID != 0 {

v.pipelineIDMu.Lock()

opt.PipelineID = gitlab.Ptr(v.pipelineID)

v.pipelineIDMu.Unlock()

}

zakisk · 2026-04-24T06:32:11Z

+
+// patchPipelineIDAnnotation stores the GitLab pipeline ID as a PipelineRun
+// annotation so the reconciler can read it back across Provider instances.
+func (v *Provider) patchPipelineIDAnnotation(ctx context.Context, statusOpts providerstatus.StatusOpts, cs *gitlab.CommitStatus) {


Suggested change

func (v *Provider) patchPipelineIDAnnotation(ctx context.Context, statusOpts providerstatus.StatusOpts, cs *gitlab.CommitStatus) {

func (v *Provider) patchPipelineIDAnnotation(ctx context.Context, statusOpts providerstatus.StatusOpts, pipelineID int64) {

if its only pipelineID then it doesn't make sense to pass whole CommitStatusOption struct

Done, added pipelineID int64.

zakisk · 2026-04-24T06:33:03Z

+	mergePatch := map[string]any{
+		"metadata": map[string]any{
+			"annotations": map[string]string{
+				keys.GitLabPipelineID: strconv.FormatInt(cs.PipelineID, 10),


Suggested change

keys.GitLabPipelineID: strconv.FormatInt(cs.PipelineID, 10),

keys.GitLabPipelineID: strconv.FormatInt(pipelineID, 10),

zakisk · 2026-04-24T06:35:23Z

 	}
 }

+func TestCreateStatusPipelineIDAnnotation(t *testing.T) {


there is already TestCreateStatus func can you add cases from this func and from below other two func to that one test func.

Done, merged into TestCreateStatus table. Only the test which address the CreateStatus call twice to verify in-memory caching across PipelineRuns kept separate

zakisk · 2026-04-27T11:34:36Z

+	if pr == nil || (pr.GetName() == "" && pr.GetGenerateName() == "") {
+		return
+	}
+	if _, ok := pr.GetAnnotations()[keys.GitLabPipelineID]; ok {


if the pipelineID is different than what's in PipelineRun's annotation here and you're just checking that keys.GitLabPipelineID does exist then it would be conflict. I know its very edge case...

I think despite an existing pipeline in pipeline run annotation, if you find different pipelineID then log a message saying this IMO but still using the old one...

Since this is a very rare scenario, a log message should be sufficient here

zakisk · 2026-04-28T06:40:49Z

/lgtm

pipelines-as-code

Congrats @ab-ghosh your PR Has been approved 🎉

✅ Pull Request Approved

Approval Status:

Required Approvals: 1
Current Approvals: 1

👥 Reviewers Who Approved:

Reviewer	Permission Level	Approval Status
@zakisk	`write`	✅

📝 Next Steps

Ensure all required checks pass
Comply with branch protection rules
Request a maintainer to merge using the /merge command (or merge it
directly if you have repository permission).

Automated by the PAC Boussole 🧭

zakisk · 2026-04-28T07:21:46Z

@ab-ghosh fix the go-testing...

When PAC posts multiple commit statuses for the same SHA, GitLab's auto-assignment logic can route them to different pipelines, leaving the MR pipeline permanently stuck with stale intermediate statuses. Cache the pipeline_id returned by the first SetCommitStatus response and pass it on subsequent calls for the same (project, SHA) pair so all statuses land in the same GitLab pipeline. Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Abhishek Ghosh <abghosh@redhat.com>

gemini-code-assist Bot reviewed Apr 9, 2026

View reviewed changes

Comment thread pkg/provider/gitlab/gitlab.go Outdated

ab-ghosh force-pushed the fix/gitlab-commit-status-pipeline-id-caching branch from 071640b to f57af59 Compare April 9, 2026 17:43

ab-ghosh force-pushed the fix/gitlab-commit-status-pipeline-id-caching branch from faba1a9 to a36c209 Compare April 16, 2026 14:38

ab-ghosh force-pushed the fix/gitlab-commit-status-pipeline-id-caching branch from a36c209 to e2b2fe2 Compare April 20, 2026 12:08

zakisk force-pushed the fix/gitlab-commit-status-pipeline-id-caching branch from e2b2fe2 to 3915309 Compare April 21, 2026 09:15

ab-ghosh force-pushed the fix/gitlab-commit-status-pipeline-id-caching branch 2 times, most recently from b408279 to ddbe9f2 Compare April 21, 2026 11:36

theakshaypant force-pushed the fix/gitlab-commit-status-pipeline-id-caching branch from ddbe9f2 to 8c64009 Compare April 22, 2026 03:33

theakshaypant reviewed Apr 22, 2026

View reviewed changes

ab-ghosh force-pushed the fix/gitlab-commit-status-pipeline-id-caching branch from 8c64009 to f921938 Compare April 22, 2026 12:14

zakisk reviewed Apr 24, 2026

View reviewed changes

ab-ghosh force-pushed the fix/gitlab-commit-status-pipeline-id-caching branch 3 times, most recently from 1d40666 to 5cd2740 Compare April 26, 2026 18:52

ab-ghosh requested a review from zakisk April 27, 2026 05:26

zakisk reviewed Apr 27, 2026

View reviewed changes

ab-ghosh force-pushed the fix/gitlab-commit-status-pipeline-id-caching branch from 5cd2740 to 35ad374 Compare April 27, 2026 13:17

zakisk force-pushed the fix/gitlab-commit-status-pipeline-id-caching branch from 35ad374 to 9df8451 Compare April 28, 2026 06:40

pipelines-as-code Bot approved these changes Apr 28, 2026

View reviewed changes

ab-ghosh force-pushed the fix/gitlab-commit-status-pipeline-id-caching branch from 9df8451 to ff4db37 Compare April 28, 2026 16:27

zakisk force-pushed the fix/gitlab-commit-status-pipeline-id-caching branch from ff4db37 to e71d69c Compare April 28, 2026 17:20

ab-ghosh force-pushed the fix/gitlab-commit-status-pipeline-id-caching branch 2 times, most recently from 4b4fbaf to f00a0de Compare April 28, 2026 19:42

zakisk force-pushed the fix/gitlab-commit-status-pipeline-id-caching branch from f00a0de to 5421660 Compare April 29, 2026 05:39

ab-ghosh force-pushed the fix/gitlab-commit-status-pipeline-id-caching branch from 5421660 to a302822 Compare April 30, 2026 04:32

zakisk approved these changes May 4, 2026

View reviewed changes

zakisk merged commit b0bcbac into tektoncd:main May 4, 2026
20 of 21 checks passed

	func (v Provider) patchPipelineIDAnnotation(ctx context.Context, statusOpts providerstatus.StatusOpts, cs gitlab.CommitStatus) {
	func (v *Provider) patchPipelineIDAnnotation(ctx context.Context, statusOpts providerstatus.StatusOpts, pipelineID int64) {

	keys.GitLabPipelineID: strconv.FormatInt(cs.PipelineID, 10),
	keys.GitLabPipelineID: strconv.FormatInt(pipelineID, 10),

Conversation

ab-ghosh commented Apr 9, 2026

📝 Description of the Change

🔗 Linked GitHub Issue

🧪 Testing Strategy

🤖 AI Assistance

✅ Submitter Checklist

Uh oh!

codecov-commenter commented Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

pipelines-as-code Bot commented Apr 10, 2026

🤖 AI Analysis - pr-complexity-rating

📊 PR Review Complexity

Summary

Suggested reviewers focus

Uh oh!

pipelines-as-code Bot commented Apr 10, 2026

🤖 AI Analysis - pr-complexity-rating

📊 PR Review Complexity

Summary

Suggested reviewers focus

Uh oh!

chmouel commented Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ab-ghosh commented Apr 16, 2026

Uh oh!

ab-ghosh commented Apr 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chmouel commented Apr 17, 2026

Uh oh!

zakisk commented Apr 17, 2026

Uh oh!

ab-ghosh commented Apr 18, 2026

Uh oh!

zakisk commented Apr 19, 2026

Uh oh!

zakisk commented Apr 21, 2026

Uh oh!

theakshaypant left a comment

Choose a reason for hiding this comment

Uh oh!

zakisk commented Apr 22, 2026

Uh oh!

ab-ghosh commented Apr 23, 2026

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!

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!

codecov-commenter commented Apr 9, 2026 •

edited

Loading

chmouel commented Apr 15, 2026 •

edited

Loading

ab-ghosh commented Apr 16, 2026 •

edited

Loading

ab-ghosh Apr 25, 2026 •

edited

Loading