GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Select CodeQL bundles from a release by henrymercer · Pull Request #4172 · github/codeql-action

Conversation

henrymercer commented Sep 24, 2026

Copy link

Copy Markdown

Contributor

Add codeql-release.ts, which selects a CodeQL bundle from a GitHub release, and use it for the default bundle and the latest nightly. This is preparation for accepting GitHub release URLs in the tools input in a follow-up PR.

getRelease looks up a release with one API request and uses its API asset URLs, so it works for private repositories. getPublicRelease constructs /releases/download/ URLs without an API request, so a missing asset shows up as a 404 when downloading. selectBundle tries the compression methods we can extract in order of preference and requires a combined bundle.

A job can use a per-language bundle if the per_language_bundles feature flag is enabled, the languages input lists exactly one language, the job runs on github.com using a GitHub-hosted runner, and tar supports zstd. A per-language bundle must also be published for that language and platform, and the CLI must be version 2.27.1 or newer, unless we're using the latest nightly. These are the existing rules in getPerLanguageBundleLanguage. When a job can use a per-language bundle, selectBundle only selects it if the release also has a combined bundle with the same compression, which it keeps for the existing download-time fallback. The restriction to the same compression method exists to avoid the need to track varying compression methods between the initial attempt and the fallback, and for the standard releases we publish, it should be dead code.

getCodeQLBundleDownloadURL becomes selectDefaultBundle, which tries the same sources in the same order: this Action's repository on this instance, github/codeql-action on this instance, then the public release on github.com. It moves on to the next source if a release can't be found or has no compatible bundle. The nightly path now uses the tarSupportsZstd value from getCodeQLSource rather than checking tar again.

This changes the default path when we look up a release, which we do unless we're running the canonical Action on github.com:

When the job can use a per-language bundle, we find it and the combined bundle we'd fall back to with one release lookup, rather than one lookup each.
If the job can use a per-language bundle but the release doesn't have it, we use the release's combined bundle, with the same warning and fallback telemetry as when a per-language download returns 404. Previously we fetched the per-language bundle from the next repository.
If the release lacks the preferred compression method, we use the other compression method from the same release. Previously we moved on to the next repository.

Otherwise, tools inputs behave as before, and on github.com with the canonical Action the default path still makes no API requests.

This also renames logPerLanguageBundleFallback to logMissingPerLanguageBundle and rewords its warning to say that we expected a per-language bundle, following up on this review comment on #4167.

Commit-by-commit review recommended as the behaviour changes to bundle selection are all in the last commit.

Risk assessment

Low risk: On github.com with the canonical Action, we select the same bundles as before, and PR checks cover this. Elsewhere, the selected bundle only changes when a release we look up lacks the preferred bundle. Per-language bundles are still behind the per_language_bundles feature flag.

Which use cases does this change impact?

Workflow types:

Advanced setup
Managed

Products:

Code Scanning
Code Quality
Other first-party - Analyses using the shared CodeQL setup path.

Environments:

Dotcom
GHES

How did/will you validate this change?

Unit tests - New tests for codeql-release.ts. The setup-codeql.ts tests now expect one request per release, and gain cases for a fork release that lacks the per-language bundle, and for moving on to the public release when the fork's release has no compatible bundle or can't be found. The unit tests and lint pass locally at each commit.
End-to-end tests - PR checks exercise the default path on github.com and the nightly path, including per-language bundle selection from the latest nightly. They don't cover looking up a release, so that path relies on the unit tests. These still need to run for this PR.

If something goes wrong after this change is released, what are the mitigation and rollback strategies?

Rollback - Revert or release a fix. Bundle selection is not behind a feature flag.

How will you know if something goes wrong after this change is released?

Telemetry - Existing telemetry for tools downloads and initialization failures, including tools_per_language_bundle_fallback, which now also covers releases that lack the per-language bundle.

Are there any special considerations for merging or releasing this change?

No special considerations - This change can be merged at any time.

Merge / deployment checklist

Confirm this change is backwards compatible with existing workflows.
Consider adding a changelog entry for this change.
Confirm the readme and docs have been updated if necessary.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          Rename logPerLanguageBundleFallback to logMissingPerLanguageBundle

6ec2b18

Co-authored-by: Copilot App <[email protected]>

henrymercer requested a balanced review from Copilot

September 24, 2026 16:05

github-actions Bot added the size/XL May be very hard to review label

Sep 24, 2026

Copilot started reviewing on behalf of henrymercer

September 24, 2026 16:05

View session

Copilot AI reviewed

Sep 24, 2026

View reviewed changes

Copilot AI left a comment

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Warning

Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.

Copilot review overview

🟡 Changes recommended

Release tags containing slashes are encoded incorrectly, producing potentially invalid release and download URLs.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1

Open (1)

Encode complete tag names to preserve slashes in GitHub URLs · New

What changed in this PR

Adds release-aware CodeQL bundle selection, including private releases, compression fallback, and per-language bundles.

Changes:

Adds release lookup and bundle selection utilities.
Integrates selection and fallback telemetry into setup.
Expands tests and updates fallback messaging.

File	Description
`src/codeql-release.ts`	Implements release and bundle selection.
`src/codeql-release.test.ts`	Tests release selection behavior.
`src/setup-codeql.ts`	Integrates the new selector.
`src/setup-codeql.test.ts`	Updates integration and fallback tests.
`src/per-language-bundles.ts`	Renames and rewords fallback logging.
`src/codeql-bundle.ts`	Adds fallback state to download sources.
`lib/entry-points.js`	Generated output; excluded from review.

Files excluded by content exclusion policy (1)

lib/entry-points.js

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

src/codeql-release.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.

henrymercer marked this pull request as ready for review

September 24, 2026 19:39

henrymercer requested a review from a team as a code owner

September 24, 2026 19:39

mbg self-requested a review

September 25, 2026 14:16

henrymercer and others added 7 commits

September 25, 2026 17:18


          Document how getCodeQLSource handles the tools input

6393cd4

Co-authored-by: Copilot App <[email protected]>


          Add helpers for selecting a CodeQL bundle from a release

f03d2df

Co-authored-by: Copilot App <[email protected]>


          Pass tarSupportsZstd to getLatestNightlyBundle

303b251

Co-authored-by: Copilot App <[email protected]>


          Select the latest nightly bundle with selectBundle

ffc4532

Co-authored-by: Copilot App <[email protected]>


          Record per-language bundle fallbacks on the download source

caa2cae

Co-authored-by: Copilot App <[email protected]>


          Test falling back to the public release for the default bundle

027ab31

Co-authored-by: Copilot App <[email protected]>


          Select the default bundle from the first compatible release

7f964a9

Co-authored-by: Copilot App <[email protected]>

henrymercer force-pushed the henrymercer/select-bundles-from-releases branch from 065bc86 to 7f964a9 Compare

September 25, 2026 16:25

henrymercer mentioned this pull request

Sep 25, 2026

Accept GitHub release URLs in the tools input #4179

Closed

henrymercer added this pull request to stack #4180

September 25, 2026 18:26

mbg reviewed

Sep 28, 2026

View reviewed changes

mbg left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Sorry for not getting this reviewed more quickly. Thanks for addressing my previous, general feedback and breaking this work down across several PRs/commits.

Broadly, I think these changes are fine. I don't think I spotted any major issues with the overall behaviour/functionality. That said, and as you know, the overall logic of setup-codeql.ts is horribly complicated so I am not 100% confident that there isn't some corner case I have missed.

Ideally, I'd like us to break setup-codeql.ts, especially getCodeQLSource, down into separate modules with stronger individual guarantees about their assumptions and effects. The new parts in codeql-release.ts here might be a start of that, but I am a bit worried that there's not enough of a methodological improvement beyond splitting it out from setup-codeql.ts. For example, and I commented on this in depth, the getPublicRelease function expects reference to refer to a GitHub.com release, but accepts arbitrary references to releases. Down the line, that would allow it to be used incorrectly. Similarly, a lot of the fields in the new interfaces are just stringly typed. It would be nice to make these more restrictive where possible to encode assumptions that we make, by using existing structures we have elsewhere, even if we don't need all of the properties (like GitHubApiDetails), or possibly using template literal types.

Otherwise, most of my comments are about the docs and other minor points.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

src/per-language-bundles.ts

		`No per-language CodeQL bundle for '${language}' was found at ${location}, so ` +
		"falling back to the bundle that contains all languages. This analysis will still " +
		"produce correct results, but will take longer to set up.",
		`Expected a per-language CodeQL bundle for '${language}' at ${location}, but it wasn't ` +

mbg Sep 28, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Sanity-check: I believe location refers to a URL. (The two call sites pass it .url properties.) Is there any chance that it might contain sensitive information?

Minor: Should location be renamed to url in this function?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

src/setup-codeql.ts

Comment on lines +377 to +389

		* We handle the `tools` input in this order:
		*
		* - A local path is extracted without using the toolcache.
		* - `nightly` or `nightly-latest`, or the `force_nightly` feature flag in a dynamic workflow,
		* selects a bundle from the latest nightly release. We then continue with that bundle's URL.
		* - `linked`, or its old name `latest`, selects the version shipped with the Action.
		* - `toolcache` selects the latest version in the toolcache, falling back to the default version
		* outside dynamic workflows or if there isn't one.
		* - Any other value is the URL of a bundle.
		* - Without a `tools` input, we use the default version.
		*
		* Apart from a local path, we look for the resolved version in the toolcache before downloading. A
		* cached version takes precedence even if the job could use a per-language bundle.

mbg Sep 28, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Thank you for adding this!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

src/codeql-release.ts

Comment on lines +16 to +19

		serverURL: string;
		owner: string;
		repo: string;
		tagName: string;

mbg Sep 28, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Minor: Add per-field docs, particularly for serverURL. It would be good to explain why serverURL can be an arbitrary string, rather than e.g. one of the types from api-client.ts or util.ts.

As far as I can see, for getPublicRelease, it's always GITHUB_DOTCOM_URL. For getRelease, it could additionally be the .url value from a GitHubApiDetails object. Although maybe not technically required, do you think it would be nicer to type this as GitHubApiDetails?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

src/codeql-release.ts

Comment on lines +38 to +41

		function getReleasePageURL(reference: CodeQLReleaseReference): string {
		const { serverURL, owner, repo, tagName } = reference;
		return `${serverURL}/${owner}/${repo}/releases/tag/${encodeTag(tagName)}`;
		}

mbg Sep 28, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

IIRC the GitHub API returns this as html_url in responses. You already get hold of release objects in getRelease below and so could use it there (i.e. url: release.html_url). I suspect you probably added this so that getPublicRelease doesn't need to make an API call; if that assumption is correct, then it would be good to document that getReleasePageURL is used to avoid API calls.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

src/codeql-release.ts


		/** A GitHub release that contains CodeQL bundles. */
		export interface CodeQLRelease {
		/** The release's web page, for messages. */

mbg Sep 28, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Minor: "for messages" is a bit vague (here and elsewhere). Change this to e.g. "for log messages" and make it clearer why having this URL is more helpful to have than a release asset URL etc.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

src/setup-codeql.ts

		);
		}

		const { bundle } = await selectBundle(

mbg Sep 28, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I note that selectBundle replaces some of the code that was previously in the try/catch block above, but the selectBundle call here is outside of that.

A few thoughts:

Should the selectBundle call be inside the try/catch block above so that exceptions thrown by selectBundle get the "Failed to retrieve the latest nightly release" context? That would match the previous behaviour more closely, but we already have the release at this point so the error message isn't quite right.
A separate handler that adds a more accurate context?
In either case, I note that selectBundle can throw ConfigurationError and so we probably don't want to wrap that in an Error. Perhaps we ought to have a better utility for this kind of catch/re-throw construct that preserves ConfigurationErrors:

export const rethrowWith = (messagePrefix: string, err: unknown) => {
  if (err instanceof ConfigurationError) {
    throw new ConfigurationError(`${messagePrefix}${err.message}`);
  }
  throw new Error(`${messagePrefix}${getErrorMessage(err)}`);
};

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

src/setup-codeql.test.ts


		for (const status of [200, 404]) {
		test.serial(
		`getCodeQLSource falls back to the public release when the Action's release ${status === 200 ? "lacks a compatible bundle" : "can't be found"}`,

mbg Sep 28, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

"the Action's release" is ambiguous / unclear. I think this should be something like "the release in the running action's repository"

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

src/setup-codeql.test.ts

Comment on lines +1231 to +1236

		assets: [
		{
		name: "codeql-bundle-java-linux64.tar.zst",
		url: "https://api.github.com/repos/codeql-testing/action-fork/releases/assets/1",
		},
		],

mbg Sep 28, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Minor: Add a comment to note that, in the 200 case, the asset list doesn't contain a per-language bundle, which is what triggers the fallback in that case.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

src/setup-codeql.ts

Comment on lines +131 to +135

		const release = await getRelease(
		{ apiClient: api.getApiClient() },
		{ serverURL, owner, repo, tagName },
		);
		return await selectBundle(action, release, options);

mbg Sep 28, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I don't think getRelease nor selectBundle have an equivalent of the "Found CodeQL bundle" log message that was removed. Should selectBundle have it or should there be a replacement here after the selectBundle call?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

src/setup-codeql.ts

		}

		let compressionMethod: tar.CompressionMethod;
		let perLanguageBundleFallback: true \| undefined;

mbg Sep 28, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Minor: Add a comment explaining why you prefer undefined over false for this. (Presumably to exclude it from telemetry if it would otherwise be false).

Maybe also consider whether it would be worth having boolean | undefined to distinguish between (undefined = not relevant, false = not necessary).

Both also apply to the JSDoc in BundleSelection.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

henrymercer marked this pull request as draft

September 29, 2026 17:03

henrymercer closed this

Oct 6, 2026

Reviewers

mbg

mbg left review comments

Copilot code review Copilot

Copilot review effort

Applies to this pull request for everyone.Learn more about Copilot code review.

Copilot left review comments

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

size/XL

May be very hard to review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.