Use the combined bundle when queries may need other languages' library packs by henrymercer · Pull Request #4184 · github/codeql-action

henrymercer · 2026-09-30T18:35:34Z

We support custom configuration files that include packs for other languages, for example:

name: Use custom queries
disable-default-queries: true
queries:
  ...
  - name: Go queries
    uses: codeql-testing/go-querypack@master
  - name: Cpp queries
    uses: codeql-testing/cpp-querypack@second-branch
  - name: JavaScript queries
    uses: codeql-testing/javascript-querypack/show_ifs2.ql@master
  - name: Python queries
    uses: codeql-testing/python-querypack/show_ifs2.ql@second-branch

as taken from our PR checks.

If these custom queries live in compiled packs, then the packs ship their own dependencies. However if the custom query is just a path to a QL file, then the dependencies are resolved from a bundle.

This creates an issue with per-language bundles: CodeQL resolves every configured query before selecting the ones for the analyzed language, so a single-language analysis with a configuration like the above will fail. This is evidenced by failures in the "Go: Custom queries" and "Start proxy" PR checks when using per-language bundles.

This PR only selects a per-language bundle when the query configuration known before CodeQL is set up can't reference such queries: there's no configuration file, the config input is unset or only sets threat models and model packs like default setup's does, and the queries input and github-codeql-extra-queries repository property only name built-in query suites. This avoids loading configuration files before setting up CodeQL, at the cost of using the combined bundle for configuration files that only use built-in queries. The config input is parsed once, before setting up CodeQL, and used both for this check and to configure the analysis. As a result, an invalid config input now fails before CodeQL is downloaded, with an error that names the config input rather than a temporary file.

This PR also modifies setup-codeql to always use the combined bundle, since it can't tell whether the queries that a workflow runs with the CLI will need library packs for other languages. A separate commit corrects the docs for its languages and analysis-kinds inputs, which said to also pass them to init, even though init fails if setup-codeql has run in the same job.

It also disables per-language bundles in the "Export file baseline information" PR check, since a per-language bundle only reports file baseline information for its own language. The second commit moves defaultSuites so that per-language-bundles.ts can use it without an import cycle.

Finally, it moves per-language bundles to a new per_language_bundles_v2 feature flag, so that we can roll them out to Action versions that include this fix without also enabling them for earlier versions.

Risk assessment

Low risk: The change only affects bundle selection when the per_language_bundles or per_language_bundles_v2 feature flag is enabled.

Which use cases does this change impact?

Workflow types:

Advanced setup - Single-language analyses with a configuration file, a config input, or queries that aren't built-in query suites, and workflows that pass a single language to setup-codeql.
Managed - Default setup analyses that get a configuration file, or queries that aren't built-in query suites, from repository properties.

Products:

Code Scanning
Code Quality

Environments:

Dotcom - Per-language bundles are only selected on GitHub.com.

How did/will you validate this change?

Unit tests - Each source of query configuration, and bundle selection for releases and nightlies. The setup-codeql Action's entry point has no unit tests, so its reason is only covered indirectly.
End-to-end tests - "Go: Custom queries" and "Start proxy" use configuration files that reference other languages' queries, so they cover this change whenever they download a bundle with per-language bundles enabled.

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

Feature flags - Disable per-language bundle selection with per_language_bundles_v2.

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

Telemetry - Monitor init failures in analyses that use a 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.

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

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

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

…p-codeql` Co-authored-by: Copilot App <[email protected]>

Copilot

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

🟢 Approval recommended

Bundle eligibility is consistently propagated and covered by focused tests without unresolved correctness issues.

Review effort: Balanced
Findings: None

What changed in this PR

Updates bundle selection to use combined bundles whenever configured queries may depend on other languages’ library packs.

Changes:

Detects query configurations requiring combined bundles.
Propagates bundle-selection reasoning through CodeQL setup.
Updates tests, documentation, and PR-check configuration.

File	Description
`src/per-language-bundles.ts`	Adds query eligibility logic.
`src/per-language-bundles.test.ts`	Tests eligibility and explanations.
`src/setup-codeql.ts`	Applies eligibility to release and nightly bundles.
`src/setup-codeql.test.ts`	Tests combined-bundle selection.
`src/setup-codeql-action.ts`	Forces combined bundles for standalone setup.
`src/init-action.ts`	Collects query configuration inputs.
`src/init.ts`	Propagates bundle-selection reasoning.
`src/codeql.ts`	Propagates setup options.
`src/codeql.test.ts`	Updates setup calls.
`src/upload-lib.ts`	Updates initialization call.
`src/config/db-config.ts`	Centralizes built-in suite names.
`src/analyze.ts`	Uses centralized suite names.
`src/analyze.test.ts`	Updates suite import.
`setup-codeql/action.yml`	Corrects input documentation.
`pr-checks/checks/export-file-baseline-information.yml`	Disables per-language bundles for the baseline check.
`lib/entry-points.js`	Generated output; excluded from review.
`.github/workflows/__export-file-baseline-information.yml`	Generated workflow; excluded from review.

Files excluded by content exclusion policy (2)

.github/workflows/__export-file-baseline-information.yml
lib/entry-points.js

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

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

mbg

Thank you for taking care of this! I think this approach makes sense to work around the issue. Query customisation is reasonably rare so that this still allows most users to benefit from per-language bundles, while avoiding any issues like we saw in CI for those that do customise them.

I left a few comments, with one or two points about long-term maintainability and otherwise minor comments. So, generally this looks pretty good already and it shouldn't be far off from being ready to merge.

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

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

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

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

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

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

mbg

Thank you for addressing the comments from my previous review! I've had some more comments on the changes here. The main ones are again related to the maintainability of duplicated logic and whether we can reduce that or move some of the work to a common place before any of the consumers.

mbg · 2026-10-02T12:15:06Z

+ * Parses the `queries` input, a comma-separated list of queries that's combined with the queries
+ * from the configuration if it starts with '+'. The `input` of the result is `undefined` if the
+ * value is unset or empty. Entries aren't validated, so an empty entry becomes `{ uses: "" }`.


Minor: This comment is largely about how this function is used, not what it does. I.e. this function is completely disconnected from the queries input and doesn't control how the returned combines value is interpreted by the caller.

Addressed in 1c0814d

mbg · 2026-10-02T12:15:47Z

+ * Parses the `github-codeql-extra-queries` repository property, which has the same format as the
+ * `queries` input. The `input` of the result is `undefined` if the value is unset or empty. Entries
+ * aren't validated, so an empty entry becomes `{ uses: "" }`.


Minor: Same issue with this comment as with the one for parseQueriesInput.

Addressed in 1c0814d

mbg · 2026-10-02T12:18:46Z

+export function parseExtraQueriesProperty(
+  value: string | undefined,
+): Augmentation<QuerySpec[]> {


This function is essentially the same as parseQueriesInput, except that it provides the additional argument to parseQueriesFromInput. When I added the repo property, I added the extra, optional parameter to parseQueriesFromInput so that the function could be shared between the two uses.

Consider whether parseQueriesInput could have a second, optional parameter for the error like parseQueriesFromInput does to avoid having a mostly duplicate parseExtraQueriesProperty or whether we could move the shouldCombine call into parseQueriesFromInput.

Addressed in 1c0814d. Since we're calling parseQueriesFromInput in two places, I removed the error parameter from parseQueriesFromInput as part of these changes so that we don't need to create the error message in two places.

mbg · 2026-10-02T12:26:35Z

+  const query = findNonBuiltInQuery(
+    parseQueriesInput(inputs.queriesInput).input,
+  );


Thanks for changing this to better reuse the existing implementation!

Another concern here, which I don't think is blocking for this PR, is that getOtherLanguagePacksReason is more conservative than it needs to be. Specifically, we ignore the combines property and so are ignoring the precedence / combination rules that are implemented in combineQueries. Since that may discard some of the configured queries if others take precedence / don't allow combining, not all of the configured queries may actually end up getting passed to the CLI. If that is the case, then we might disable per-language bundles here even though we may not need to. Since this is more conservative than necessary, it is fine as-is, but we could look into reusing more of the combineQueries logic here to ensure consistency and to allow per-language bundles in more cases.

I agree that we are more conservative than necessary, which is fine, and that it would make sense to look into refactoring this as future work.

mbg · 2026-10-02T12:31:15Z

+ * @throws A `ConfigurationError` if the `queries` input or the `github-codeql-extra-queries`
+ *   repository property is a '+' with no queries after it, unless an input that's checked earlier
+ *   already gives a reason.


Observation: getOtherLanguagePacksReason will now throw if parseQueriesInput or parseExtraQueriesProperty throws. Previously, getOtherLanguagePacksReason would never have thrown an error.

It's probably fine because the circumstances under which an error is thrown here would lead to an error later on in any case, but it moves that error to an earlier point in the execution.

I don't think that's an issue because I don't think there's anything happening in-between the two throwing sites that would particularly matter, but if we wanted to be more conservative then we could catch the exceptions in getOtherLanguagePacksReason and treat them as equivalent to no custom queries being configured in the respective case.

It's probably fine because the circumstances under which an error is thrown here would lead to an error later on in any case, but it moves that error to an earlier point in the execution.

Exactly this — we're going to fail later anyway after downloading the CodeQL CLI, so if anything it is better to fail earlier on. Note that with the follow up changes I've made, this reasoning now applies to invalid YAML in the config input too.

mbg · 2026-10-02T12:33:41Z

-  // queries. For example, default setup only uses it for threat models and model packs.
+  // The `config` input can configure queries in the same way as a configuration file. We assume
+  // that dynamic workflows, which GitHub manages, don't use it to add queries. For example, default
+  // setup only uses it for threat models and model packs.


Super minor: instead of the example at the end, we could mention the DefaultSetupConfig type here (from db-config.ts) to look at for what assumptions we do make about the configuration provided by Default Setup. Since that affects the behaviour of the Action, we are more likely to update it if things change than the example in this comment.

I think DEFAULT_SETUP_CONFIG_SCHEMA is what we want, since we're interested in the UserConfig properties set by default setup, rather than the eventual augmented user configuration. I've added a comment in f6a7f00.

mbg · 2026-10-02T12:50:45Z

+ * uses, which are threat models and model packs, to valid values. Returns `false` otherwise,
+ * including if `contents` isn't valid YAML.
+ */
+export function matchesDefaultSetupConfigSchema(contents: string): boolean {


matchesDefaultSetupConfigSchema implements similar logic to mergeDefaultSetupAndUserConfigs with some minor differences (mergeDefaultSetupAndUserConfigs expects to already be given a UserConfig object and doesn't parse the YAML itself; both handle the outcomes of checkSchema differently).

Two concerns with this:

We may end up parsing and validating the config twice. That seems a bit unnecessary.

If we change one for any reason, we should also change the other.

Alongside the similar points for getOtherLanguagePacksReason, I am starting to think that maybe we should refactor this more so that we perform the input processing once before getOtherLanguagePacksReason and initConfig are called, and then pass the results of the input processing to both. I appreciate that it's more work to do that than we'd like for this though, and I haven't checked if any of the relevant input processing in initConfig has a dependency on the CLI.

Could we at least modify the flow so that the config input is parsed as YAML earlier and the resulting object can be used here as well as in mergeDefaultSetupAndUserConfigs?

Could we at least modify the flow so that the config input is parsed as YAML earlier and the resulting object can be used here as well as in mergeDefaultSetupAndUserConfigs?

I've done this, so we only parse config as YAML once now. I'd prefer to leave the rest to future work, since this PR is already changing enough functionality.

mbg · 2026-10-02T12:52:39Z

+  } catch (error) {
+    if (error instanceof yaml.YAMLException) {
+      return false;
+    }
+    throw error;
+  }


If we are keeping the YAML processing here, I think it would be good to add an inline comment here before the if (error instanceof ...) check to explain why we are happy to treat a YAMLException as indicative that the configuration wasn't provided by Default Setup. (Maybe also add a logger.debug call to log that case.)

This is no longer relevant given the refactoring to only parse YAML once. If we have invalid YAML we'll now fail init before we download the CLI.

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

…fig` input Co-authored-by: Copilot App <[email protected]>

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

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

henrymercer · 2026-10-02T15:30:42Z

@copilot resolve the merge conflicts in this pull request

…age-pr-check-failures # Conflicts: # lib/entry-points.js Co-authored-by: henrymercer <[email protected]>

Copilot · 2026-10-02T15:41:15Z

@copilot resolve the merge conflicts in this pull request

Merged main in a47cc9f. The only conflict was in the generated lib/entry-points.js, because main bumped @actions/cache to 5.3.0. I resolved it by installing the merged dependencies and regenerating lib with rm -rf lib && npm run-script build.

The PR's diff against main is unchanged: 21 files, +732/−191. The only lines in the rebuilt lib/entry-points.js that differ from main are this PR's changes. Some identifiers come out renamed (logger → logger2, isObject → isObject2), which matches how main's build already names them.
check-js.sh reports the JS is up to date, all 1162 unit tests pass, ESLint passes on the changed files, and pr-checks/sync.ts doesn't change any generated workflows.

npm ci can't run in my sandbox: 31 resolved URLs in package-lock.json point to ms-feed-*.pkgs.visualstudio.com, which I can't reach, and those entries are on main too. To install, I pointed those entries at registry.npmjs.org in a temporary copy of the lockfile and kept the integrity hashes. I put the original lockfile back before committing, so the packages are identical and the committed lockfile is unchanged.

henrymercer · 2026-10-02T15:43:44Z

npm ci can't run in my sandbox: 31 resolved URLs in package-lock.json point to ms-feed-*.pkgs.visualstudio.com, which I can't reach, and those entries are on main too. To install, I pointed those entries at registry.npmjs.org in a temporary copy of the lockfile and kept the integrity hashes. I put the original lockfile back before committing, so the packages are identical and the committed lockfile is unchanged.

@mbg is preparing a fix for this.

henrymercer and others added 5 commits September 29, 2026 19:31

Disable per-language bundles in the file baseline PR check

b5a476b

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

Move defaultSuites to config/db-config.ts

5e3132d

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

Avoid per-language bundles when queries may need other languages' lib…

1609a51

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

Always use the combined bundle in setup-codeql

d8b6f2a

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

Fix the docs for the languages and analysis-kinds inputs of `setu…

7816c33

…p-codeql` Co-authored-by: Copilot App <[email protected]>

github-actions Bot added the size/L May be hard to review label Sep 30, 2026

henrymercer requested a balanced review from Copilot October 1, 2026 09:17

Copilot started reviewing on behalf of henrymercer October 1, 2026 09:18 View session

Copilot AI reviewed Oct 1, 2026

View reviewed changes

henrymercer and others added 2 commits October 1, 2026 11:48

Use a new feature flag for per-language bundles

87a1923

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

Omit the feature flag name from the per-language bundle debug log

2da0d29

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

henrymercer marked this pull request as ready for review October 1, 2026 11:32

henrymercer requested a review from a team as a code owner October 1, 2026 11:32

mbg reviewed Oct 1, 2026

View reviewed changes

henrymercer and others added 7 commits October 1, 2026 17:44

Read the config and queries inputs once in init

a98f604

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

Extract parsing of the queries input and the extra queries reposito…

a4fbe39

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

Reuse the parsing of query inputs when choosing a bundle

70897a7

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

Explain that the config input can configure queries

725421c

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

Explain what a reason to use the combined bundle means where it's che…

1bb99cb

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

Check what the config input sets instead of exempting dynamic workf…

a6cd2a5

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

Explain why the file baseline PR check uses the combined bundle

ce28f3e

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

mbg reviewed Oct 2, 2026

View reviewed changes

henrymercer and others added 4 commits October 2, 2026 14:22

Parse lists of queries with a single function

1c0814d

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

Share the check of the properties that Default Setup sets in the `con…

e869836

…fig` input Co-authored-by: Copilot App <[email protected]>

Parse the config input once

113b18e

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

Point to the Default Setup config schema from the per-language bundle…

f6a7f00

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

henrymercer added Rebuild Re-transpile JS & re-generate workflows and removed Rebuild Re-transpile JS & re-generate workflows labels Oct 2, 2026

Copilot started work on behalf of henrymercer October 2, 2026 15:31 View session

Merge remote-tracking branch 'origin/main' into henrymercer/per-langu…

a47cc9f

…age-pr-check-failures # Conflicts: # lib/entry-points.js Co-authored-by: henrymercer <[email protected]>

Copilot finished work on behalf of henrymercer October 2, 2026 15:42

github-actions Bot added size/XL May be very hard to review and removed size/L May be hard to review labels Oct 2, 2026

Merge branch 'main' into henrymercer/per-language-pr-check-failures

c275b4a

Conversation

henrymercer commented Sep 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Risk assessment

Which use cases does this change impact?

How did/will you validate this change?

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

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

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

Merge / deployment checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟢 Approval recommended

Uh oh!

mbg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mbg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

henrymercer commented Oct 2, 2026

Uh oh!

Copilot AI commented Oct 2, 2026

Uh oh!

henrymercer commented Oct 2, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

henrymercer commented Sep 30, 2026 •

edited

Loading