fix(bitbucketcloud): truncate commit status key to 40 char limit#2702
Conversation
|
|
||
| // GetCheckName returns the key for build commit status and the reason to have it | ||
| // unique for bitbucket cloud is that the key is limited to 40 characters. | ||
| // If the key is longer than 40 characters, it will be truncated to the first 40 characters. |
There was a problem hiding this comment.
this is going to make a collision risk, since the random name of a PR which ensure uniqueness is a suffix, what about a numbered suffix in either when they have the same name or when it's truncated
There was a problem hiding this comment.
by number you mean pipelinerun sequence number?
PIpelines as Code / pipelinerun-1
PIpelines as Code / pipelinerun-2
PIpelines as Code / pipelinerun-3
like this?
There was a problem hiding this comment.
but this doesn't look good because there could be other kind of pipeline run e.g. on-comment that could lead to mist numbering 😕 thinking about something new
There was a problem hiding this comment.
Code Review
This pull request implements a new GetCheckName function to handle Bitbucket Cloud's 40-character limit for commit status keys, along with supporting unit and E2E tests and a new Status struct. Review feedback highlights the accidental removal of the e2e build tag in the test suite and suggests improving the truncation logic to preserve unique suffixes in PipelineRun names to prevent status key collisions.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2702 +/- ##
==========================================
+ Coverage 59.19% 59.24% +0.04%
==========================================
Files 208 208
Lines 20543 20566 +23
==========================================
+ Hits 12161 12184 +23
Misses 7610 7610
Partials 772 772 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
b1d3544 to
d00e795
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request implements a custom GetCheckName function for Bitbucket Cloud to ensure commit status keys adhere to the 40-character limit. It includes comprehensive unit tests for the truncation logic, a new Status struct for API interactions, and an E2E test to verify status reporting. Feedback was provided regarding a bug where ApplicationName was not truncated when the pipeline run name was empty, as well as a recommendation to use rune-based slicing to safely handle multi-byte characters in status keys.
d00e795 to
1453475
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a mechanism to persist and report build statuses for Bitbucket Cloud by storing a unique status key in PipelineRun annotations. It includes the implementation of GetBBCloudStatusKey to handle Bitbucket's 40-character key limit and adds corresponding unit and E2E tests. A significant concern was raised regarding the use of a loop index for generating the status key, as this index is unstable across re-runs and could lead to incorrect status updates or collisions in Bitbucket.
d9210ba to
f695fc1
Compare
f695fc1 to
a820717
Compare
a820717 to
70c6732
Compare
6c97e41 to
5184ad5
Compare
theakshaypant
left a comment
There was a problem hiding this comment.
Overall looks fine, just a comment on the added e2e test.
b5c6f97 to
062a80d
Compare
Bitbucket Cloud limits commit status keys to 40 characters and uses them to identify build statuses. Generated a new key on each CreateStatus call causing duplicate statuses instead of updating existing ones. Fix the 40-char truncation to safely handle short names, use 1-based indexing for status keys, and persist the generated key as a PipelineRun annotation (pipeline-run-status-key) so subsequent calls reuse the same key. Move GetCheckName to provider.GetBBCloudStatusKey, add Index field to matcher.Match for stable instance counts, and add e2e test to verify build status reporting. https://redhat.atlassian.net/browse/SRVKP-11710 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Zaki Shaikh <zashaikh@redhat.com>
062a80d to
a715ee5
Compare
|
/retest |
1 similar comment
|
/retest |
theakshaypant
left a comment
There was a problem hiding this comment.
LGTM.
The hashing mechanism to add as a suffix when commit status key is truncated would prevent clashing if multiple pipelinerun have similar starting strings.
Handles cases when application name is to be included/excluded. Sufficient unit tests added. E2e test looks good after the update using a LONG pipelinerun name.


📝 Description of the Change
Bitbucket Cloud limits commit status keys to 40 characters. Move GetCheckName to the bitbucketcloud package to handle truncation locally, add unit tests, fix the e2e test to correctly decode paginated commit status responses, and add the Status type for commit status parsing.
🔗 JIRA
https://redhat.atlassian.net/browse/SRVKP-11710
🧪 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.
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-bytrailer to your commit message.For example:
Co-authored-by: Claude noreply@anthropic.com
✅ Submitter Checklist
fix:,feat:) matches the "Type of Change" I selected above.make testandmake lintlocally to check for and fix anyissues. For an efficient workflow, I have considered installing
pre-commit and running
pre-commit installtoautomate these checks.