Fix parsing of GHES pre-release versions - #2969
Conversation
Sorry, something went wrong.
There was a problem hiding this comment.
Pull Request Overview
This PR fixes the parsing of GHES (GitHub Enterprise Server) pre-release versions to handle the non-standard semver format used by GHES. The main issue is that GHES pre-release versions like 3.18.0.pre1 are not valid semver format, so they need to be transformed to 3.18.0-pre1 before being parsed.
- Adds a new parseGhesVersion utility function to normalize GHES version strings
- Updates version comparison logic to use the new parser for GHES versions
- Adds comprehensive test coverage for the new functionality
Reviewed Changes
Copilot reviewed 6 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file| File | Description |
|---|---|
| src/util.ts | Adds parseGhesVersion function to normalize GHES pre-release version format |
| src/upload-lib.ts | Updates version comparisons to use the new parser for GHES versions |
| src/upload-lib.test.ts | Adds test for GHES 3.16 pre-release version handling and improves test assertions |
| lib/util.js | Generated JavaScript for the new utility function |
| lib/upload-lib.js | Generated JavaScript for the updated upload library |
| lib/upload-lib.test.js | Generated JavaScript for the updated tests |
Sorry, something went wrong.
|
|
||
| export function parseGhesVersion(version: string): semver.SemVer { | ||
| // GHES pre-release versions are in the format "3.18.0.pre1", which is not a valid semver version. | ||
| if (version.includes(".pre")) { |
Copilot
AI
Jul 14, 2025
There was a problem hiding this comment.
The condition version.includes(".pre") may be too broad and could match unintended patterns. Consider using a more specific pattern like /\.pre\d+$/ to ensure it only matches the expected GHES pre-release format at the end of the version string.
| if (version.includes(".pre")) { | |
| if (/\.pre\d+$/.test(version)) { |
Sorry, something went wrong.
| export function parseGhesVersion(version: string): semver.SemVer { | ||
| // GHES pre-release versions are in the format "3.18.0.pre1", which is not a valid semver version. | ||
| if (version.includes(".pre")) { | ||
| version = version.replace(".pre", "-pre"); |
Copilot
AI
Jul 14, 2025
There was a problem hiding this comment.
Using replace(".pre", "-pre") only replaces the first occurrence. If there could be multiple .pre patterns in a version string, consider using replaceAll() or a global regex. However, if only one occurrence is expected, this is acceptable.
| version = version.replace(".pre", "-pre"); | |
| version = version.replaceAll(".pre", "-pre"); |
Sorry, something went wrong.
| export function parseGhesVersion(version: string): semver.SemVer { | ||
| // GHES pre-release versions are in the format "3.18.0.pre1", which is not a valid semver version. | ||
| if (version.includes(".pre")) { | ||
| version = version.replace(".pre", "-pre"); | ||
| } | ||
| return new semver.SemVer(version); |
Copilot
AI
Jul 14, 2025
There was a problem hiding this comment.
The function doesn't handle potential errors from new semver.SemVer(version). If the version string is still invalid after transformation, this will throw an exception. Consider adding error handling or documenting this behavior.
| export function parseGhesVersion(version: string): semver.SemVer { | |
| // GHES pre-release versions are in the format "3.18.0.pre1", which is not a valid semver version. | |
| if (version.includes(".pre")) { | |
| version = version.replace(".pre", "-pre"); | |
| } | |
| return new semver.SemVer(version); | |
| export function parseGhesVersion(version: string): semver.SemVer | null { | |
| // GHES pre-release versions are in the format "3.18.0.pre1", which is not a valid semver version. | |
| if (version.includes(".pre")) { | |
| version = version.replace(".pre", "-pre"); | |
| } | |
| try { | |
| return new semver.SemVer(version); | |
| } catch (error) { | |
| core.warning(`Invalid GHES version string: ${version}. Error: ${error}`); | |
| return null; | |
| } |
Sorry, something went wrong.
mbg
left a comment
There was a problem hiding this comment.
Thanks for adding in the thorough documentation! ✨
Sorry, something went wrong.
This fixes parsing of GHES pre-release versions for the combining SARIF files check. The version 3.18.0.pre1 is not valid semver, so this just transforms it to 3.16.0-pre1.
Merge / deployment checklist