Add a repository property for disabling overlay by henrymercer · Pull Request #3507 · github/codeql-action

henrymercer · 2026-02-24T19:32:38Z

Add a repository property that can be used to disable overlay for repositories that are owned by an organisation.

This means repositories that are owned by an organisation can disable overlay without needing to disable default setup too.

Risk assessment

For internal use only. Please select the risk level of this change:

Low risk: Changes are fully under feature flags, or have been fully tested and validated in pre-production environments and are highly observable, or are documentation or test only.

Which use cases does this change impact?

Workflow types:

Advanced setup - Impacts users who have custom CodeQL workflows.
Managed - Impacts users with dynamic workflows (Default Setup, Code Quality, ...).

Products:

Code Scanning - The changes impact analyses when analysis-kinds: code-scanning.

Environments:

Dotcom - Impacts CodeQL workflows on github.com and/or GitHub Enterprise Cloud with Data Residency.
GHES - Impacts CodeQL workflows on GitHub Enterprise Server.

How did/will you validate this change?

Test repository - This change will be tested on a test repository before merging. (backlinked)
Unit tests - I am depending on unit test coverage (i.e. tests in .test.ts files).

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

Rollback - Change can only be disabled by rolling back the release or releasing a new version with a fix.

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

Telemetry - I rely on existing telemetry or have made changes to the telemetry.
- Alerts - New or existing monitors will trip if something goes wrong with this change.

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

No special considerations - This change can be merged at any time.
Special considerations - This change should only be merged once certain preconditions are met. Please provide details of those or link to this PR from an internal issue.

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.

Copilot

Pull request overview

Adds a new repository property that allows org-owned repositories to disable overlay analysis without disabling default setup, integrating this property into overlay mode selection and strengthening typing for repository properties.

Changes:

Introduce github-codeql-disable-overlay repository property and parse repository properties into typed values (boolean/string).
Update overlay database mode selection to honor the new property (with env var taking precedence).
Add/adjust unit tests to cover new overlay selection behavior and updated API response typing.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/feature-flags/properties.ts	Adds new property name, typed property mapping/parsing, and updates API response typing.
src/feature-flags/properties.test.ts	Updates tests to align with the new exported API response type.
src/config-utils.ts	Threads repository properties into overlay mode computation and disables overlay when the property is set.
src/config-utils.test.ts	Adds tests verifying overlay is disabled/enabled based on the repository property and that env var precedence remains.
lib/init-action.js	Generated output updated to match TypeScript changes (not reviewed).

Copilot · 2026-02-24T19:35:39Z

src/feature-flags/properties.ts

 }

 /**
 * The API returns a list of `RepositoryProperty` objects.


The JSDoc still says the API returns a list of RepositoryProperty objects, but that type no longer exists and the response type is now GitHubRepositoryProperty[]/GitHubPropertiesResponse. Update the comment to match the actual types to avoid confusion for future maintainers.

Suggested change

* The API returns a list of `RepositoryProperty` objects.

* The API returns `GitHubPropertiesResponse`, a list of `GitHubRepositoryProperty` objects.

Copilot · 2026-02-24T19:35:39Z

src/feature-flags/properties.ts

+function isKnownPropertyName(value: string): value is RepositoryPropertyName {
+  return Object.values(RepositoryPropertyName).includes(
+    value as RepositoryPropertyName,
+  );


isKnownPropertyName recomputes Object.values(RepositoryPropertyName) on every call, and it's called once per remote property in loadPropertiesFromApi. Consider using a module-level Set<string> of known names (or caching the array) so the per-property lookup is O(1) without repeated allocations.

Suggested change

function isKnownPropertyName(value: string): value is RepositoryPropertyName {

return Object.values(RepositoryPropertyName).includes(

value as RepositoryPropertyName,

);

const KNOWN_REPOSITORY_PROPERTY_NAMES = new Set<string>(

Object.values(RepositoryPropertyName),

);

function isKnownPropertyName(value: string): value is RepositoryPropertyName {

return KNOWN_REPOSITORY_PROPERTY_NAMES.has(value);

Copilot · 2026-02-24T19:35:40Z

src/feature-flags/properties.ts

+const mapRepositoryProperties: {
+  [K in RepositoryPropertyName]: (value: string) => AllRepositoryProperties[K];
+} = {
+  [RepositoryPropertyName.DISABLE_OVERLAY]: (value) => value === "true",
+  [RepositoryPropertyName.EXTRA_QUERIES]: (value) => value,


New behavior maps DISABLE_OVERLAY from the API string value to a boolean (value === "true"), but there is no unit test exercising this parsing/mapping in loadPropertiesFromApi. Add a test case that returns github-codeql-disable-overlay from the mocked API and asserts the resulting RepositoryProperties contains the correct boolean.

Copilot · 2026-02-24T19:35:40Z

src/feature-flags/properties.ts

@@ -71,10 +92,8 @@ export async function loadPropertiesFromApi(
        );
      }



loadPropertiesFromApi validates that property_name exists, but it doesn't validate that value is present and a string before passing it into setProperty. If the API returns malformed objects (or unexpected types), this will silently coerce to incorrect values (e.g., undefined === "true" → false). Consider throwing when typeof property.value !== "string".

Suggested change

if (typeof property.value !== "string") {

throw new Error(

`Expected property '${property.property_name}' to have a string 'value', but got: ${JSON.stringify(property)}`,

);

}

mbg

Looks good, glad to see this coming together! A bunch of minor points / questions, but nothing critical.

mbg · 2026-02-24T19:41:25Z

src/feature-flags/properties.ts

 * A repository property has a name and a value.
 */
-export interface RepositoryProperty {
+interface GitHubRepositoryProperty {


Minor: In general, I think it's a good idea to export types that are (transitively) visible through other, exported types. I'd only leave them as not exported if they are really only used internally in one file.

mbg · 2026-02-24T19:43:21Z

src/feature-flags/properties.ts

+export type RepositoryProperties = Partial<AllRepositoryProperties>;
+
+const mapRepositoryProperties: {
+  [K in RepositoryPropertyName]: (value: string) => AllRepositoryProperties[K];


Minor: Name the function type to make it clearer what it is for? Possibly also a comment on mapRepositoryProperties itself?

mbg · 2026-02-24T19:44:03Z

src/feature-flags/properties.ts

+const mapRepositoryProperties: {
+  [K in RepositoryPropertyName]: (value: string) => AllRepositoryProperties[K];
+} = {
+  [RepositoryPropertyName.DISABLE_OVERLAY]: (value) => value === "true",


Minor: Since this is new, consider making this check case-insensitive?

This is consistent with other boolean checks, e.g. in environment variables. Also the repository properties UI has support for boolean properties, which will be set to either "true" or "false".

Agreed that it is consistent, but is it intuitive? Do we gain anything from being this strict here?

For the repo properties UI, doesn't that require each org to individually configure the property as a boolean? If so, it's easy to imagine someone accidentally configuring it as a string and then using True etc.

It's a valid point, but on the flip side if we allow True (or 1, etc) here, it's surprising when that isn't accepted elsewhere. We can make it clear in our instructions that this should be configured as a boolean property.

mbg · 2026-02-24T19:45:39Z

src/feature-flags/properties.ts

+function setProperty<K extends RepositoryPropertyName>(
+  properties: RepositoryProperties,
+  name: K,
+  value: string,
+): void {
+  properties[name] = mapRepositoryProperties[name](value);
+}
+


Minor/Question: Any particular reason why this is a separate function and isn't inlined below?

I couldn't prove that this was type safe when inlining it. If you can get this to work in a type safe way I'd be happy to change it.

mbg · 2026-02-24T19:47:07Z

src/feature-flags/properties.ts

 * Enumerates repository property names that have some meaning to us.
 */
 export enum RepositoryPropertyName {
+  DISABLE_OVERLAY = "github-codeql-disable-overlay",


Design question: Do you think it would make sense to have control over the base overlay / overlay usage enablement separately?

Hmm, that kind of distinction seems useful to us when we're rolling stuff out, but can you think of a good reason why customers would want to build overlay-base databases without using them in PR analysis?

henrymercer added 2 commits February 24, 2026 18:48

Add support for boolean repository properties

7ea93ee

Add repository property for disabling overlay

ed39a1e

Copilot AI review requested due to automatic review settings February 24, 2026 19:32

henrymercer requested a review from a team as a code owner February 24, 2026 19:32

github-actions bot added the size/M Should be of average difficulty to review label Feb 24, 2026

Copilot started reviewing on behalf of henrymercer February 24, 2026 19:33 View session

Copilot AI reviewed Feb 24, 2026

View reviewed changes

mbg previously approved these changes Feb 24, 2026

View reviewed changes

Improve validation and address review comments

2808ca7

henrymercer dismissed mbg’s stale review via 2808ca7 February 24, 2026 19:59

	* The API returns a list of `RepositoryProperty` objects.
	* The API returns `GitHubPropertiesResponse`, a list of `GitHubRepositoryProperty` objects.

		@@ -71,10 +92,8 @@ export async function loadPropertiesFromApi(
		);
		}

+      if (typeof property.value !== "string") {
+        throw new Error(
+          `Expected property '${property.property_name}' to have a string 'value', but got: ${JSON.stringify(property)}`,
+        );
+      }

Comments

Conversation

henrymercer commented Feb 24, 2026

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

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants