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

Mark Swift incompatible OS as configuration error by henrymercer · Pull Request #3801 · github/codeql-action · GitHub

/ codeql-action Public

Mark Swift incompatible OS as configuration error - #3801

Merged
henrymercer merged 5 commits into
mainfrom
henrymercer/swift-incompatible-os
Apr 9, 2026
ConversationCommits (5)ChecksFiles changed
Merged

Mark Swift incompatible OS as configuration error#3801
henrymercer merged 5 commits into
mainfrom
henrymercer/swift-incompatible-os

Conversation

henrymercer commented Apr 2, 2026

Copy link
Copy Markdown
Contributor

Reduce monitor noise by treating the incompatible OS error on Swift as a configuration error rather than an issue with the CodeQL Action.

henrymercer requested a review from a team as a code owner April 2, 2026 17:48
Copilot AI review requested due to automatic review settings April 2, 2026 17:48
github-actions Bot added the size/XS Should be very easy to review label Apr 2, 2026
Copilot AI reviewed Apr 2, 2026
View reviewed changes

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 reduces monitoring noise by classifying CodeQL CLI’s “Swift analysis is only supported on macOS” / [incompatible-os] failure as a configuration error (so it’s treated as a user/workflow issue rather than an action failure).

Changes:

  • Added a new CliConfigErrorCategory.SwiftIncompatibleOs and matching patterns to recognize the Swift incompatible-OS error.
  • Added an AVA test to ensure the incompatible-OS Swift error is wrapped into a ConfigurationError.
  • Updated generated lib/ artifacts to reflect the TypeScript changes.

Reviewed changes

Copilot reviewed 2 out of 13 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/cli-errors.ts Adds SwiftIncompatibleOs category and regex candidates so the error is treated as a configuration error.
src/cli-errors.test.ts Adds coverage asserting Swift incompatible-OS stderr is wrapped as ConfigurationError.
lib/upload-sarif-action.js Generated bundle update reflecting new CLI error category.
lib/upload-sarif-action-post.js Generated bundle update reflecting new CLI error category.
lib/upload-lib.js Generated bundle update reflecting new CLI error category.
lib/start-proxy-action-post.js Generated bundle update reflecting new CLI error category.
lib/setup-codeql-action.js Generated bundle update reflecting new CLI error category.
lib/resolve-environment-action.js Generated bundle update reflecting new CLI error category.
lib/init-action.js Generated bundle update reflecting new CLI error category.
lib/init-action-post.js Generated bundle update reflecting new CLI error category.
lib/autobuild-action.js Generated bundle update reflecting new CLI error category.
lib/analyze-action.js Generated bundle update reflecting new CLI error category.
lib/analyze-action-post.js Generated bundle update reflecting new CLI error category.

mbg previously approved these changes Apr 2, 2026
View reviewed changes

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

This looks good to me, but couldn't we catch this earlier on / before we ever invoke the CLI? Or is this specifically to catch the case where there's no languages input but we are running on macOS and there's Swift code in the repo?

henrymercer commented Apr 8, 2026

Copy link
Copy Markdown
Contributor Author

This looks good to me, but couldn't we catch this earlier on / before we ever invoke the CLI? Or is this specifically to catch the case where there's no languages input but we are running on macOS and there's Swift code in the repo?

We could, but AFAICT the information about compatible OSes is not machine readable, so this would mean duplicating the knowledge about incompatible OSes between the CLI and the Action.

mbg commented Apr 8, 2026

Copy link
Copy Markdown
Member

We could, but AFAICT the information about compatible OSes is not machine readable, so this would mean duplicating the knowledge about incompatible OSes between the CLI and the Action.

Isn't Swift just a special case in that regard or do we have other language/OS pairs that are not compatible? If it's just a special case for Swift, then I wouldn't be overly opposed to adding a check for that in the Action so that we can fail earlier.

henrymercer commented Apr 8, 2026

Copy link
Copy Markdown
Contributor Author

Looks like we already taught the Action about it, but it was a warning rather than a configuration error. Let's update that.

github-actions Bot added size/S Should be easy to review and removed size/XS Should be very easy to review labels Apr 8, 2026
mbg previously approved these changes Apr 9, 2026
View reviewed changes

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

Thanks for finding that! One comment about that change.

Comment thread src/init-action.ts
Comment on lines +392 to +399
if (
config.languages.includes(KnownLanguage.swift) &&
process.platform === "linux"
) {
throw new ConfigurationError(
`Swift analysis on Ubuntu runner images is no longer supported. Please migrate to a macOS runner.`,
);
}

mbg Apr 9, 2026

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

What happens if someone tries to analyse Swift on Windows? Should that also result in an error here? I imagine that the warning was specifically because we used to support Swift analysis on Linux. Now that we are turning this into an error to fail an analysis early, should we include Windows as well?

henrymercer Apr 9, 2026

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 suppose we might as well :)

henrymercer added this pull request to the merge queue Apr 9, 2026
Merged via the queue into main with commit 597e12a Apr 9, 2026
225 checks passed
henrymercer deleted the henrymercer/swift-incompatible-os branch April 9, 2026 17:48
This was referenced Apr 15, 2026
Merged
Merged
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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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
mbg mbg approved these changes

Assignees

No one assigned

Labels

size/S Should be easy to review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL