Skip to content

Fix parsing of GHES pre-release versions #2969

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 14, 2025

Conversation

koesie10
Copy link
Member

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

  • Confirm this change is backwards compatible with existing workflows.
  • Confirm the readme has been updated if necessary.
  • Confirm the changelog has been updated if necessary.

@koesie10 koesie10 marked this pull request as ready for review July 14, 2025 09:38
@Copilot Copilot AI review requested due to automatic review settings July 14, 2025 09:38
@koesie10 koesie10 requested a review from a team as a code owner July 14, 2025 09:38
Copy link
Contributor

@Copilot 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 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

@@ -1132,6 +1132,14 @@ export function checkActionVersion(
}
}

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")) {
Copy link
Preview

Copilot AI Jul 14, 2025

Choose a reason for hiding this comment

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

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.

Suggested change
if (version.includes(".pre")) {
if (/\.pre\d+$/.test(version)) {

Copilot uses AI. Check for mistakes.

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");
Copy link
Preview

Copilot AI Jul 14, 2025

Choose a reason for hiding this comment

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

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.

Suggested change
version = version.replace(".pre", "-pre");
version = version.replaceAll(".pre", "-pre");

Copilot uses AI. Check for mistakes.

src/util.ts Outdated
Comment on lines 1135 to 1140
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);
Copy link
Preview

Copilot AI Jul 14, 2025

Choose a reason for hiding this comment

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

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.

Suggested change
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;
}

Copilot uses AI. Check for mistakes.

mbg
mbg previously approved these changes Jul 14, 2025
Copy link
Member

@mbg mbg left a comment

Choose a reason for hiding this comment

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

Looks like a reasonable fix for this scenario! Thanks for implementing and thanks to @cklin for spotting it.

Just one minor suggestion, otherwise LGTM.

Copy link
Member

@mbg mbg left a comment

Choose a reason for hiding this comment

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

Thanks for adding in the thorough documentation! ✨

@koesie10 koesie10 merged commit 6f936b5 into main Jul 14, 2025
282 checks passed
@koesie10 koesie10 deleted the koesie10/fix-ghes-version-parsing branch July 14, 2025 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants