feat(status): report skipped status for unmatched PipelineRuns - #2918
feat(status): report skipped status for unmatched PipelineRuns#2918zakisk wants to merge 1 commit into
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2918 +/- ##
==========================================
+ Coverage 80.66% 80.95% +0.28%
==========================================
Files 164 164
Lines 13911 13970 +59
==========================================
+ Hits 11222 11309 +87
+ Misses 1967 1942 -25
+ Partials 722 719 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Paco Review
|
![pipelines-as-code[bot]](https://avatars.githubusercontent.com/in/112592?s=60&v=4){kind=small}
08fde7b to
e2008a9
Compare
|
/paco review |
|
I can't request review from copilot. cc: @chmouel |
There was a problem hiding this comment.
Pull request overview
Adds an opt-in “status_check” feature to report provider statuses for PipelineRuns that do not match the incoming event, improving visibility in Git provider UIs when multiple .tekton/ PipelineRuns target different triggers.
Changes:
- Extend PipelineRun matching to return both matched and unmatched PipelineRuns, and (when enabled) report a “skipped/success/neutral” status for each unmatched PipelineRun.
- Update provider-specific status mapping/formatting to support a “skipped” conclusion (or best-effort equivalents where unsupported).
- Add unit/E2E test coverage and documentation/CRD updates for the new Repository
spec.settings.status_checkAPI.
Reviewed changes
Copilot reviewed 35 out of 36 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test/pkg/bitbucketdatacenter/crd.go | Pass Settings into Bitbucket DC E2E Repository CR creation. |
| test/pkg/bitbucketcloud/crd.go | Pass Settings into Bitbucket Cloud E2E Repository CR creation. |
| test/gitlab_merge_request_test.go | Add E2E coverage for skipped status reporting on GitLab MR. |
| test/github_pullrequest_test.go | Add E2E coverage for skipped status reporting (GitHub App + webhook). |
| test/gitea_pull_request_test.go | Add E2E coverage for skipped status reporting on Gitea/Forgejo. |
| test/bitbucket_datacenter_push_test.go | Update CreateCRD call signature to include opts/settings. |
| test/bitbucket_datacenter_pull_request_test.go | Add Bitbucket DC PR E2E coverage for skipped statuses + CreateCRD signature update. |
| test/bitbucket_datacenter_dynamic_variables_test.go | Update CreateCRD call signature to include opts/settings. |
| test/bitbucket_cloud_pullrequest_test.go | Add Bitbucket Cloud PR E2E coverage for skipped statuses. |
| pkg/provider/github/status.go | Map skipped conclusion appropriately for check runs and commit statuses; avoid empty DetailsURL in check run creation. |
| pkg/provider/github/status_test.go | Add unit coverage for skipped conclusion behavior (apps + webhook). |
| pkg/provider/gitea/status_test.go | Add unit coverage for skipped conclusion mapping (success + “Skipped” description). |
| pkg/provider/gitea/gitea.go | Add skipped conclusion formatting + map skipped to success for commit statuses. |
| pkg/provider/bitbucketdatacenter/bitbucketdatacenter.go | Map skipped conclusion to Bitbucket DC “UNKNOWN” state and update title text. |
| pkg/provider/bitbucketdatacenter/bitbucketdatacenter_test.go | Add unit coverage for skipped conclusion. |
| pkg/provider/bitbucketcloud/bitbucket.go | Update skipped title text for Bitbucket Cloud status reporting. |
| pkg/provider/bitbucketcloud/bitbucket_test.go | Add unit coverage for skipped conclusion. |
| pkg/pipelineascode/testdata/no-match/.tekton/nomatch.yaml | Add annotations to ensure deterministic “no-match” behavior for tests. |
| pkg/pipelineascode/pipelineascode.go | Report per-unmatched statuses (when enabled) after starting matched PipelineRuns; add helper reporter. |
| pkg/pipelineascode/pipelineascode_test.go | Add Run() test covering per-unmatched status reporting. |
| pkg/pipelineascode/pipelineascode_statuscheck_test.go | New unit tests for status reporting helper behavior and error emission. |
| pkg/pipelineascode/match.go | Plumb unmatched PipelineRuns through the matching flow. |
| pkg/pipelineascode/match_test.go | Update tests to validate unmatched PipelineRuns counts. |
| pkg/matcher/annotation_matcher.go | Return unmatched PipelineRuns alongside matches and populate unmatched list on non-match branches. |
| pkg/matcher/annotation_matcher_test.go | Add tests verifying unmatched PipelineRun tracking and related logs. |
| pkg/apis/pipelinesascode/v1alpha1/zz_generated.deepcopy.go | Regenerate deep-copies to include StatusCheck. |
| pkg/apis/pipelinesascode/v1alpha1/types.go | Add StatusCheck API types/constants and inheritance via Settings.Merge. |
| pkg/apis/pipelinesascode/v1alpha1/types_test.go | Add tests verifying StatusCheck inheritance/precedence. |
| docs/content/docs/operations/global-repository-settings.md | Link global settings doc to the new Status Check guide. |
| docs/content/docs/guides/statuses.md | Document the unmatched-PipelineRun status-check feature at a high level. |
| docs/content/docs/guides/repository-crd/status-check.md | New tech preview guide documenting configuration and provider behavior. |
| docs/content/docs/guides/_index.md | Update guide index card subtitle to mention status checks. |
| docs/content/docs/api/settings.md | Add API reference documentation for settings.status_check. |
| docs/content/docs/api/repository.md | Include status_check snippet in Repository API docs. |
| docs/content/docs/api/repository-spec.md | Include status_check snippet in RepositorySpec API docs. |
| config/300-repositories.yaml | CRD schema updates for the new spec.settings.status_check fields. |
Files not reviewed (1)
- pkg/apis/pipelinesascode/v1alpha1/zz_generated.deepcopy.go: Generated file
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
e2008a9 to
1d55e09
Compare
1d55e09 to
79b4ba9
Compare
|
/retest |
bd2762c to
d7a4b71
Compare
|
/retest |
d7a4b71 to
8805b0f
Compare
|
/retest |
d01f965 to
043ba0e
Compare
|
/retest |
|
/test go-testing |
043ba0e to
04f6150
Compare
|
/test go-testing |
0f09113 to
3756d41
Compare
| if event.EventType == triggertype.PullRequestLabeled.String() { | ||
| unmatchedPRs = append(unmatchedPRs, prun) |
There was a problem hiding this comment.
Consider a pipelinerun with annotation on-label: "ready-to-test". Would it not be shown as skipped on a push event?
There was a problem hiding this comment.
yes, that's the reason eventType is checked
There was a problem hiding this comment.
This contradicts with the description ... produces no status at all — it is impossible to tell from the PR whether it was skipped intentionally or never picked up. The on-label pipelinerun shows no status on pull request and it seems similar to the scenario described in the description.
There was a problem hiding this comment.
on-label pipelinerun shows
what do you mean by this?
There was a problem hiding this comment.
Consider a on-label.yaml which only runs on on-label: "ready-to-test" annotation. Based on the changes, this prun would only be shown as skipped for pull_request_labeled event, not push or other events.
This feels contradictory to the example in the description where
.tekton/build.yaml — on-event: pull_request, on-target-branch: main
.tekton/deploy.yaml — on-event: push, on-target-branch: main
where deploy.yaml shows as skipped on pull_request open.
By this logic, shouldn't we also show "skipped" status on push for on-label.yaml
|
If i understand okay every provider except the GitHub App path posts a PR comment for each unmatched PipelineRun (Status: completed + non-empty Text). N unmatched runs = N comments per push. This is going to annoy people quickly isnt it ? And you forgot to update the schemas: (the linter probably need to be fixed) 
it would be easier if we drop the aggregate stuff, ie: I have some issue with the naming, but maybe it's just me and happy to take vote (cc @theakshaypant ) what does it look more saner and explicit Before (this PR) settings:
status_check:
enabled: true
mode: per_unmatched_pipelinerun
no_match_conclusion: skippedSuggested change: settings:
status_checks:
report_unmatched: true
mode: per_pipelinerun
unmatched_conclusion: skippedthis feels more english to me, My reasoning for this:
|
@chmouel actually |
+1 on this regarding the suggested names
|
3756d41 to
875837e
Compare
yeah, it was valid point for gitlab, gitea and github webhook. fixed it.
it's updated now
yes, removed
changed |
|
/paco review |
|
/test linters |
| if event.EventType == triggertype.PullRequestLabeled.String() { | ||
| unmatchedPRs = append(unmatchedPRs, prun) |
There was a problem hiding this comment.
This contradicts with the description ... produces no status at all — it is impossible to tell from the PR whether it was skipped intentionally or never picked up. The on-label pipelinerun shows no status on pull request and it seems similar to the scenario described in the description.
875837e to
6844f34
Compare
When a repository has multiple PipelineRuns in .tekton/ targeting
different events, only the ones matching the incoming event run. The
rest are silently ignored, leaving gaps in the Git provider's status
checks UI.
For example, a repository with two PipelineRuns:
.tekton/build.yaml — on-event: pull_request, on-target-branch: main
.tekton/deploy.yaml — on-event: push, on-target-branch: main
When a pull request is opened, build.yaml matches and runs. Without
this feature, deploy.yaml produces no status at all — it is impossible
to tell from the PR whether it was skipped intentionally or never
picked up. With status_check enabled:
spec:
settings:
status_check:
enabled: true
mode: "per_unmatched_pipelinerun"
Pipelines-as-Code now reports a "skipped" status for deploy.yaml on
the pull request, making the full picture visible in the provider UI.
The matcher now returns both matched and unmatched PipelineRuns. After
the matched runs complete, the controller iterates the unmatched list
and calls CreateStatus on each with the configured conclusion. The
conclusion defaults to `skipped` but can be set to `success` or
`neutral` via the `no_match_conclusion` field.
Every provider maps that conclusion to its native state:
- GitHub App: check run conclusion "skipped"
- GitHub Webhook: commit status "success" (API has no skipped)
- GitLab: pipeline status "skipped"
- Bitbucket Cloud: build status "STOPPED"
- Bitbucket Data Center: build status "UNKNOWN"
- Gitea/Forgejo: commit status "success" (no skipped state)
The setting is inheritable from the global Repository CR via the
existing Settings.Merge path. A second mode (`aggregate`) is defined
in the CRD but not yet implemented.
The whole feature is behind an opt-in flag (enabled: false by default)
and marked as tech preview in the documentation.
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Zaki Shaikh <zashaikh@redhat.com>
6844f34 to
d2b2ce2
Compare
| } | ||
|
|
||
| matchedPRs, err = matcher.MatchPipelinerunByAnnotation(ctx, p.logger, pipelineRuns, p.run, p.event, p.vcx, p.eventEmitter, repo, false) | ||
| // unmatchedPRs are filtered out above if RemoteTasks is enabled so we should also check if RemoteTasks is enabled then |
There was a problem hiding this comment.
[question] What happens when there is an error in resolving a pipelinerun? Is it reported as skipped or is there an error reported for the prun?
There was a problem hiding this comment.
No, there will be no status check reporting if any error
📝 Description of the Change
When a repository has multiple PipelineRuns in .tekton/ targeting different events, only the ones matching the incoming event run. The rest are silently ignored, leaving gaps in the Git provider's status checks UI.
For example, a repository with two PipelineRuns:
.tekton/build.yaml — on-event: pull_request, on-target-branch: main
.tekton/deploy.yaml — on-event: push, on-target-branch: main
When a pull request is opened, build.yaml matches and runs. Without this feature, deploy.yaml produces no status at all — it is impossible to tell from the PR whether it was skipped intentionally or never picked up. With status_check enabled:
spec:
settings:
status_check:
enabled: true
mode: "per_unmatched_pipelinerun"
Pipelines-as-Code now reports a "skipped" status for deploy.yaml on the pull request, making the full picture visible in the provider UI.
The matcher now returns both matched and unmatched PipelineRuns. After the matched runs complete, the controller iterates the unmatched list and calls CreateStatus on each with the configured conclusion. The conclusion defaults to
skippedbut can be set tosuccessorneutralvia theno_match_conclusionfield.Every provider maps that conclusion to its native state:
The setting is inheritable from the global Repository CR via the existing Settings.Merge path. A second mode (
aggregate) is defined in the CRD but not yet implemented.The whole feature is behind an opt-in flag (enabled: false by default) and marked as tech preview in the documentation.
🔗 Linked GitHub Issue
Fixes #
🧪 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.