Remove support for combining SARIF runs with non-unique categories #2959
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a feature-flagged error when attempting to combine multiple SARIF runs that share the same category, and updates related merge logic and tests.
- Add
throwIfCombineSarifFilesDisabled&shouldDisableCombineSarifFilesto enforce non-unique-category checks behind theDisableCombineSarifFilesflag. - Extend
combineSarifFilesUsingCLIto call the new check and update deprecation messaging. - Register and configure
DisableCombineSarifFilesin the feature flags and expand test coverage accordingly.
Reviewed Changes
Copilot reviewed 6 out of 9 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/upload-lib.ts | Added new helper functions for feature-flagged enforcement and integrated them into merge flow |
| src/upload-lib.test.ts | Added comprehensive AVA tests for the new enforcement logic across GitHub variants |
| src/feature-flags.ts | Defined the DisableCombineSarifFiles feature flag and its environment default |
| lib/* | Generated JavaScript reflecting the above TypeScript changes (mirrors TS sources) |
Comments suppressed due to low confidence (1)
src/upload-lib.ts:182
- You're using
semver.lthere but there's no import forsemver. Please addimport * as semver from "semver";at the top of the file.
) {
| await throwIfCombineSarifFilesDisabled( | ||
| sarifObjects, | ||
| features, | ||
| gitHubVersion, | ||
| ); |
There was a problem hiding this comment.
I assume the deprecation warning a few lines further down is not necessary if the conditions for this are true. Should it be removed now or later?
There was a problem hiding this comment.
That's true, but we can only remove the deprecation warning when the action drops support for GHES 3.17. In all GHES versions between 3.14 (including) and 3.18 (excluding), the action should still show the deprecation warning and should never give this error. Until support for GHES 3.17 is dropped, customers can still sync their action from dotcom to their GHES instance and expect it to work without breaking changes.
There was a problem hiding this comment.
Makes sense -- probably also worth tracking then
This removes support for combining SARIF runs with non-unique categories, and will instead throw a configuration error. This behavior is only enabled behind a feature flag.
This is intentionally not backwards compatible when the feature flag is enabled, as this was announced over a year ago and a deprecation warning is shown for every upload that would run into this error.
Merge / deployment checklist