FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Introduce a type-level check that CodeQL is passed during feature flag lookup if it is needed by henrymercer · Pull Request #3386 · github/codeql-action · GitHub

Introduce a type-level check that CodeQL is passed during feature flag lookup if it is needed - #3386

Merged
henrymercer merged 4 commits into
mainfrom
henrymercer/codeql-ff-improve-safety
Jan 5, 2026
Merged

Introduce a type-level check that CodeQL is passed during feature flag lookup if it is needed#3386
henrymercer merged 4 commits into
mainfrom
henrymercer/codeql-ff-improve-safety

Conversation

Copy link
Copy Markdown
Contributor

Introduce a type-level check that CodeQL is passed during feature flag lookup if it is needed.

  • Use satisfies rather than a type annotation to maintain a more precise type for featureConfig
  • Add FeatureWithoutCLI type to record features whose enablement don't depend on the version of CodeQL
  • Update FeatureEnablement interface to add a call for FeatureWithoutCLI that doesn't require CodeQL, and make the call for Feature require CodeQL
  • Update places that inspect featureConfig to do a safe cast to the former FeatureConfig type using satisfies FeatureConfig as FeatureConfig

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?

Dev change only.

How did/will you validate this change?

  • 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.

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.

henrymercer requested a review from a team as a code owner January 5, 2026 16:27
Copilot AI review requested due to automatic review settings January 5, 2026 16:27
github-actions Bot added the size/M Should be of average difficulty to review label Jan 5, 2026
henrymercer changed the title Henrymercer/codeql ff improve safety Introduce a type-level check that CodeQL is passed during feature flag lookup if it is needed Jan 5, 2026
henrymercer requested a review from mbg January 5, 2026 16:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Pull request overview

This PR introduces compile-time type safety for feature flag lookups by ensuring that CodeQL is passed when required. The key changes include:

  • Refactored featureConfig to use the satisfies operator for better type inference
  • Added FeatureWithoutCLI type to identify features that don't depend on CodeQL version
  • Updated FeatureEnablement interface with method overloads to enforce CodeQL requirement
  • Updated test helper functions to properly handle the new type distinctions

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

File Description
src/feature-flags.ts Refactored type definitions and interface to add compile-time safety for CodeQL requirements; added FeatureWithoutCLI type and method overloads
src/feature-flags.test.ts Updated test helper function getFeatureIncludingCodeQlIfRequired to use new type system; added imports for new types
src/testing-utils.ts Added type cast in mock getValue implementation to support both method overloads
lib/*.js Auto-generated JavaScript files reflecting the TypeScript changes (not reviewed per guidelines)

Comment thread src/feature-flags.test.ts Outdated

mbg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM, other than one minor point. Very similar to what I came up with :)

Comment thread src/testing-utils.ts
},
getValue: async (feature) => {
return enabledFeatures.includes(feature);
return enabledFeatures.includes(feature as Feature);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Minor: could this be a type annotation on feature?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I agree that's slightly better, but I don't think it's worth going through another round of approval :)

henrymercer merged commit db72691 into main Jan 5, 2026
241 checks passed
henrymercer deleted the henrymercer/codeql-ff-improve-safety branch January 5, 2026 17:13
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/M Should be of average difficulty to review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants


Back | FazBrowse Home | New Git URL