Skip to content

fix: Improve GitLab commit status handling#2253

Merged
chmouel merged 1 commit intotektoncd:mainfrom
chmouel:SRVKP-8908-update-gitlab-commit-status
Sep 25, 2025
Merged

fix: Improve GitLab commit status handling#2253
chmouel merged 1 commit intotektoncd:mainfrom
chmouel:SRVKP-8908-update-gitlab-commit-status

Conversation

@chmouel
Copy link
Copy Markdown
Member

@chmouel chmouel commented Sep 23, 2025

📝 Description of the Change

This commit addresses confusion around when GitLab comments are posted versus when commit status updates are used.

Changes:

  • Documentation: Added clear explanation that comments are only posted when commit status updates fail, with visual example showing GitLab Pipelines tab where status appears when successful
  • Code clarity: Refactored commit status logic with better error handling and detailed logging when both source/target project status updates fail
  • Test reliability: Enhanced GitLab MR test to properly verify pipeline status success and ensure no comments are posted when status updates work
  • Test accuracy: Removed redundant CreateStatus test cases and improved SHA tracking for more reliable test execution

The key fix is clarifying that Pipelines-as-Code tries to set commit status on both source (fork) and target (upstream) projects, and only falls back to posting comments when both attempts fail due to permission issues.

Fixes the misleading documentation that didn't properly explain the relationship between GitLab API permissions, commit status updates, and comment posting behavior.

🔗 Linked GitHub Issue

Fixes #

👨🏻‍ Linked Jira

Jira: https://issues.redhat.com/browse/SRVKP-8908

🚀 Type of Change

  • 🐛 Bug fix (fix:)
  • ✨ New feature (feat:)
  • 💥 Breaking change (feat!:, fix!:)
  • 📚 Documentation update (docs:)
  • ⚙️ Chore (chore:)
  • 💅 Refactor (refactor:)
  • 🔧 Enhancement (enhance:)
  • 📦 Dependency update (deps:)

🧪 Testing Strategy

  • Unit tests
  • Integration tests
  • End-to-end tests
  • Manual testing
  • Not Applicable

✅ Submitter Checklist

  • 📝 My commit messages are clear, informative, and follow the project's How to write a git commit message guide. The Gitlint linter ensures in CI it's properly validated
  • ✨ I have ensured my commit message prefix (e.g., fix:, feat:) matches the "Type of Change" I selected above.
  • ♽ I have run make test and make lint locally to check for and fix any
    issues. For an efficient workflow, I have considered installing
    pre-commit and running pre-commit install to
    automate these checks.
  • 📖 I have added or updated documentation for any user-facing changes.
  • 🧪 I have added sufficient unit tests for my code changes.
  • 🎁 I have added end-to-end tests where feasible. See README for more details.
  • 🔎 I have addressed any CI test flakiness or provided a clear reason to bypass it.
  • If adding a provider feature, I have filled in the following and updated the provider documentation:
    • GitHub App
    • GitHub Webhook
    • Gitea/Forgejo
    • GitLab
    • Bitbucket Cloud
    • Bitbucket Data Center

Copilot AI review requested due to automatic review settings September 23, 2025 11:23
@gemini-code-assist
Copy link
Copy Markdown

Summary of Changes

Hello @chmouel, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly improves the handling of GitLab commit statuses within Pipelines-as-Code, particularly in merge request scenarios involving forks. The primary goal is to ensure that users consistently see pipeline statuses in the GitLab UI, falling back to comments only when API permission issues prevent status updates on both the source and target repositories. This change clarifies the expected behavior, enhances system robustness, and provides better diagnostic information.

Highlights

  • GitLab Commit Status Logic Refinement: The core logic for updating GitLab commit statuses has been refactored to first attempt setting the status on the source project (fork) and then fall back to the target project (upstream repository) if the initial attempt fails. This ensures that a commit status is displayed whenever possible, even with limited token permissions on forks.
  • Improved Error Handling and Logging: Enhanced error handling and detailed logging have been implemented for GitLab commit status updates. If both source and target project status updates fail, a comprehensive debug message is now logged, guiding users to check GitLab token access for the source repository.
  • Documentation Clarity: The documentation for GitLab Merge Request comment behavior has been significantly updated. It now clearly explains that comments are only posted as a fallback mechanism when both source and target commit status updates fail, and includes a visual example of where successful status updates appear in the GitLab UI.
  • Enhanced GitLab E2E Test Reliability: The GitLab Merge Request end-to-end test has been improved to accurately verify successful pipeline status updates and confirm that no comments are posted when status updates function correctly. This is achieved by capturing the commit SHA and directly polling the GitLab API for pipeline status.
  • Streamlined Test Cases: Redundant CreateStatus test cases have been removed, and new, more comprehensive unit tests have been added to cover various scenarios for the GitLab commit status fallback logic, including success on source, fallback to target, and failure on both.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR improves GitLab commit status handling by clarifying the relationship between commit status updates and comment posting behavior. The key insight is that Pipelines-as-Code tries to set commit status on both source (fork) and target (upstream) projects, only falling back to posting comments when both attempts fail due to permission issues.

  • Enhanced GitLab commit status logic with proper fallback from source to target project and detailed logging
  • Updated documentation to clearly explain when comments are posted versus when pipeline status appears in GitLab UI
  • Improved test reliability by capturing commit SHA and verifying pipeline status success instead of just checking for absence of comments

Reviewed Changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
test/pkg/scm/scm.go Trims whitespace from SHA return value for more reliable commit tracking
test/gitlab_merge_request_test.go Refactored test to verify pipeline status success and proper SHA tracking instead of just comment absence
pkg/provider/gitlab/gitlab_test.go Added comprehensive test cases for commit status scenarios including fallback behavior
pkg/provider/gitlab/gitlab.go Improved commit status logic with proper source/target fallback and enhanced logging
docs/content/docs/guide/repositorycrd.md Updated documentation to clarify when GitLab comments vs pipeline status are used

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread test/gitlab_merge_request_test.go
Comment thread test/gitlab_merge_request_test.go
Comment thread test/gitlab_merge_request_test.go
@pipelines-as-code pipelines-as-code bot added documentation Improvements or additions to documentation fix gitlab labels Sep 23, 2025
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request significantly improves the GitLab commit status handling by attempting to post a status on both the source and target projects before falling back to creating a comment. This is a great enhancement for users in fork-based workflows. The associated documentation updates are clear, and the testing improvements, especially the polling for GitLab pipeline status, make the e2e tests much more robust and reliable. I have one suggestion to further improve the new debug logging for better diagnostics.

Comment thread pkg/provider/gitlab/gitlab.go
@chmouel chmouel force-pushed the SRVKP-8908-update-gitlab-commit-status branch from 40208e0 to 8b9dcb3 Compare September 23, 2025 11:33
Comment thread docs/content/docs/guide/repositorycrd.md Outdated
Comment thread pkg/provider/gitlab/gitlab.go
This commit addresses confusion around when GitLab comments are posted
versus when commit status updates are used.

Changes:
* Documentation: Added clear explanation that comments are only posted
  when commit status updates fail, with visual example showing GitLab
  Pipelines tab where status appears when successful
* Code clarity: Refactored commit status logic with better error
  handling and detailed logging when both source/target project status
  updates fail
* Test reliability: Enhanced GitLab MR test to properly verify pipeline
  status success and ensure no comments are posted when status updates work
* Test accuracy: Removed redundant CreateStatus test cases and improved
  SHA tracking for more reliable test execution

The key fix is clarifying that Pipelines-as-Code tries to set commit status
on both source (fork) and target (upstream) projects, and only falls back
to posting comments when both attempts fail due to permission issues.

Fixes the misleading documentation that didn't properly explain the
relationship between GitLab API permissions, commit status updates, and
comment posting behavior.

Jira: https://issues.redhat.com/browse/SRVKP-8908
Signed-off-by: Chmouel Boudjnah <chmouel@redhat.com>
@chmouel chmouel force-pushed the SRVKP-8908-update-gitlab-commit-status branch from 8b9dcb3 to 653f7ce Compare September 23, 2025 14:36
@zakisk
Copy link
Copy Markdown
Member

zakisk commented Sep 24, 2025

/lgtm

Copy link
Copy Markdown

@pipelines-as-code pipelines-as-code bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Congrats @chmouel 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 admin

📝 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 🧭

@chmouel chmouel merged commit 49df439 into tektoncd:main Sep 25, 2025
8 checks passed
@chmouel chmouel deleted the SRVKP-8908-update-gitlab-commit-status branch September 25, 2025 12:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation fix gitlab

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants