GeezerVison

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

GeezerVison is showing github.com — Minor improvements to prepare for #4221 by mbg · Pull Request #4262 · github/codeql-action

Conversation

mbg commented Oct 22, 2026

Copy link

Copy Markdown

Member

This branch contains a few cherry-picked commits from #4221 which can be reviewed and merged in isolation. Just some minor refactoring and enhancements.

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.
Code Quality - The changes impact analyses when analysis-kinds: code-quality.
Other first-party - The changes impact other first-party analyses.
Third-party analyses - The changes affect the upload-sarif action.

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?

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

mbg added 5 commits

October 22, 2026 14:30


          Make getCodeQLForCmd use Env

7e3e4f2


          Refactor swift check out of init-action.ts

1b037de


          Use ActionConfig in diagnostics.ts

bc1b5da


          Refactor top-level code into checkSarif

00d7ef8


          Allow check-diagnostics.ts to check multiple SARIF files

5a54684

Copilot AI max review requested due to automatic review settings

October 22, 2026 13:41

mbg requested a review from a team as a code owner

October 22, 2026 13:41

github-actions Bot added the size/M Should be of average difficulty to review label

Oct 22, 2026

Copilot started reviewing on behalf of mbg

October 22, 2026 13:45

View session

Base automatically changed from mbg/diagnostics/failed-sarif to main

October 22, 2026 13:53

Copilot AI reviewed

Oct 22, 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.

🟡 Changes recommended

Directory-based SARIF discovery misses existing files on Windows because glob patterns contain backslash separators.

1 open issue

Use cwd with fixed glob pattern for cross-platform SARIF discovery · New

1 file not reviewed

lib/entry-points.js: Excluded by your content exclusion settings

What changed in this PR

Prepares the CodeQL Action for the runner-compatibility checks in #4221 through focused refactoring and expanded diagnostic validation.

Changes:

Extracts Swift platform validation into a helper with unit tests.
Adds environment injection to CLI helpers and narrows configuration requirements for diagnostics.
Extends SARIF diagnostic checks to accept directories and adds the glob dependency.

File	Description
`src/testing-utils.ts`	Supports injected environments in the CLI test helper.
`src/languages/swift.ts`	Extracts Swift runner compatibility validation.
`src/languages/swift.test.ts`	Tests Swift and non-Swift platform handling.
`src/init-action.ts`	Calls the extracted Swift validation helper.
`src/diagnostics.ts`	Narrows diagnostic helpers’ configuration types.
`src/codeql.ts`	Routes selected environment accesses through an injected environment.
`src/cli/output-cache.ts`	Accepts a read-only environment for cache paths.
`src/actions-util.ts`	Accepts a read-only environment for temporary paths.
`pr-checks/package.json`	Declares the `glob` dependency.
`pr-checks/check-diagnostics.ts`	Adds recursive directory-based SARIF validation.
`package-lock.json`	Records the PR-checks dependency change.

_{Review effort: Max}

Sorry, something went wrong.

Uh oh!

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

All reactions

Comment thread

pr-checks/check-diagnostics.ts Resolved

Uh oh!

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


          Improve globSync call

7a0df8f

mbg requested a review from henrymercer

October 22, 2026 13:58


          Merge branch 'main' into mbg/cherry-picks/macos-deprecation

0c057be

henrymercer reviewed

Oct 22, 2026

View reviewed changes

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

Thanks for factoring these changes out into a separate PR!

Sorry, something went wrong.

Uh oh!

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

All reactions

Comment thread

src/languages/swift.ts Resolved

Uh oh!

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

Comment thread

src/languages/swift.test.ts Resolved

Uh oh!

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

Comment thread

src/languages/swift.test.ts Resolved

Uh oh!

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

Comment thread

src/codeql.ts Resolved

Uh oh!

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

Comment thread

src/codeql.ts

		logger: Logger,
		cmd: string,
		checkVersion: boolean,
		env: Env = getEnv(),

henrymercer Oct 22, 2026

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.

I think we probably want to have the CLI inherit env when we invoke it. Currently the CLI inherits process.env, which will match env in Actions, but in tests it might mismatch and result in confusing behaviour.

Sorry, something went wrong.

Uh oh!

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

All reactions

mbg Oct 23, 2026

Copy link

Copy Markdown

Member Author

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.

Good catch. It makes sense to propagate this down for consistency/sanity, even if there might not be a real scenario where this would bite us right now.

Sorry, something went wrong.

Uh oh!

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

All reactions

henrymercer Oct 27, 2026

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.

This looks good now. Should we add some unit tests to check the forwarding?

Sorry, something went wrong.

Uh oh!

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

All reactions

mbg added 6 commits

October 23, 2026 12:07


          Rename isSwiftCompatible

fbf3125


          Check that a ConfigurationError is thrown

2d3b6d4


          Merge remote-tracking branch 'origin/main' into mbg/cherry-picks/maco…

5a097f3

…s-deprecation


          Remove unused type parameters from Env classes

d7e4421


          Use env.export in getCodeQLForCmd

7cd2203


          Allow runTool to inherit env

6d448e0

github-actions Bot added size/L May be hard to review and removed size/M Should be of average difficulty to review labels

Oct 23, 2026

mbg requested review from henrymercer and a max review from Copilot

October 23, 2026 12:28

Copilot started reviewing on behalf of mbg

October 23, 2026 12:29

View session

Copilot AI reviewed

Oct 23, 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.

🔵 Needs a closer look

The cross-platform CLI environment refactor needs human validation, particularly where commands still use different environment sources.

1 open issue

Supplied environment is not propagated to all tool runners · New

1 resolved since last review

Use cwd with fixed glob pattern for cross-platform SARIF discovery

1 file not reviewed

lib/entry-points.js: Excluded by your content exclusion settings

_{Review effort: Max}

Sorry, something went wrong.

Uh oh!

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

All reactions

Comment thread

src/codeql.ts Resolved

Uh oh!

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

mbg added 3 commits

October 26, 2026 10:46


          Fix JSDoc

49de083


          Pass env to all ToolRunners in codeql.ts

516bd7c


          Merge branch 'main' into mbg/cherry-picks/macos-deprecation

18f7f5c


          Merge remote-tracking branch 'origin/main' into mbg/cherry-picks/maco…

2dc8ba8

…s-deprecation

henrymercer approved these changes

Oct 27, 2026

View reviewed changes

Comment thread

src/actions-util.ts Resolved

Uh oh!

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

Comment thread

src/codeql.ts Resolved

Uh oh!

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

Comment thread

src/codeql.ts Resolved

Uh oh!

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

Comment thread

src/codeql.ts

		logger: Logger,
		cmd: string,
		checkVersion: boolean,
		env: Env = getEnv(),

henrymercer Oct 27, 2026

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.

This looks good now. Should we add some unit tests to check the forwarding?

Sorry, something went wrong.

Uh oh!

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

All reactions

mbg added this pull request to the merge queue

Oct 28, 2026

Merged via the queue into main with commit 354d3a2

Oct 28, 2026

228 checks passed

Uh oh!

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

mbg deleted the mbg/cherry-picks/macos-deprecation branch

October 28, 2026 11:46

This was referenced Nov 3, 2026

Merge main into releases/v4 #4286

Merged

Merge releases/v4 into releases/v3 #4291

Merged

Reviewers

Copilot code review Copilot

Copilot review effort

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

Copilot left review comments

henrymercer

henrymercer approved these changes

Uh oh!

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

Assignees

No one assigned

Labels

size/L

May be 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 01402503 — 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.