Skip to content

Emit warning for unrecognised repo properties with our common prefix#3570

Open
mbg wants to merge 1 commit intomainfrom
mbg/repo-props/warn-on-unexpected-props
Open

Emit warning for unrecognised repo properties with our common prefix#3570
mbg wants to merge 1 commit intomainfrom
mbg/repo-props/warn-on-unexpected-props

Conversation

@mbg
Copy link
Member

@mbg mbg commented Mar 12, 2026

Adds a warning to loadPropertiesFromApi when it encounters a repository property whose name matches our common prefix, but isn't one we recognise.

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.

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?

  • Unit tests - I am depending on unit test coverage (i.e. tests in .test.ts files).
  • End-to-end tests - I am depending on PR checks (i.e. tests in pr-checks).

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.
    • Dashboards - I will watch relevant dashboards for issues after the release. Consider whether this requires this change to be released at a particular time rather than as part of a regular release.
    • 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?

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

@mbg mbg self-assigned this Mar 12, 2026
@mbg mbg requested a review from a team as a code owner March 12, 2026 11:51
Copilot AI review requested due to automatic review settings March 12, 2026 11:51
@github-actions github-actions bot added the size/S Should be easy to review label Mar 12, 2026
Copy link
Contributor

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

Adds a warning when the action sees a repository property that looks like it belongs to the CodeQL Action (matches the shared prefix) but isn’t recognized by the current version, to help users diagnose version/property mismatches.

Changes:

  • Introduce a shared GITHUB_CODEQL_PROPERTY_PREFIX constant and warn on unknown prefixed properties in loadPropertiesFromApi.
  • Add a unit test asserting a warning is emitted for an unrecognized prefixed property.
  • Regenerate the compiled lib/ output.

Reviewed changes

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

File Description
src/feature-flags/properties.ts Adds the common prefix constant and warning behavior for unknown prefixed properties (with a dynamic-workflow suppression).
src/feature-flags/properties.test.ts Adds unit coverage for the new warning behavior.
lib/init-action.js Generated output reflecting the TypeScript changes (not reviewed).

Copy link
Contributor

@esbena esbena left a comment

Choose a reason for hiding this comment

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

LGTM, with a minor concern on how spammy this could become in practice.

Comment on lines +134 to +139
logger.warning(
`Found a repository property named '${property.property_name}', ` +
"which looks like a CodeQL Action repository property, " +
"but which is not understood by this version of the CodeQL Action. " +
"Do you need to update to a newer version?",
);
Copy link
Contributor

Choose a reason for hiding this comment

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

Users running old versions for whatever reasons will start seeing more and more log lines over time. Should we collapse to a single warning line and then have ...${unrecognized.join(", ")}... mentioning them all in a list?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/S Should be easy to review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants