GV

GeezerVison

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

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Use `setup-codeql` logic for finding release assets in `start-proxy` by mbg · Pull Request #4198 · github/codeql-action

Conversation

@mbg

@mbg mbg commented Oct 6, 2026 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

The start-proxy Action currently uses its own getReleaseByVersion function to get hold of the release that we expect to find the proxy assets at. That function has hard-coded owner and repo values for the default github/codeql-action repository. This repository may not be available in all environments. In such environments, getReleaseByVersion fails and we fall-back to the hard-coded release obtained from getFallbackUrl instead.

Broadly, this PR does the following:

  • getCodeQLActionRepository and getCodeQLBundleDownloadURL are refactored out of setup-codeql into a new setup/repository module.
  • We then use getCodeQLAssetDownloadURL (renamed from getCodeQLBundleDownloadURL) in the start-proxy action to apply the same logic for finding a suitable release as used by setup-codeql.
  • The hard-coded fallback has been removed. The default URL returned by getCodeQLBundleDownloadURL is essentially equivalent, except that it uses the linked version rather than a hard-coded one. We have been shipping proxy artifacts successfully with releases for a good while now, so the fallback shouldn't be needed anymore.

There are some drive-by improvements to logging and assertions for sinon stubs.

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:

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

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
@github-actions github-actions Bot added the size/L May be hard to review label Oct 6, 2026
@mbg
mbg force-pushed the mbg/start-proxy/deprecate-fallback branch from 362e7d4 to cd87ccd Compare October 6, 2026 15:56
mbg added 8 commits October 6, 2026 17:07
@mbg
Allow logging functions to accept string[] directly
909fda1
@mbg
Refactor getCodeQLActionRepository out of setup-codeql
cb7acf5
@mbg
Use ActionState for getCodeQLActionRepository
3f57a00
@mbg
Refactor getCodeQLBundleDownloadURL out of setup-codeql
da07d40
@mbg
Add utility for assertions on sinon stubs with better output by default
35f12de
@mbg
Type the assetKind parameter
a4f9f6f
@mbg
Add getDefaultDotComDownloadURL
de99f0e
@mbg
Use getCodeQLAssetDownloadURL for start-proxy
133ffeb
@mbg
mbg force-pushed the mbg/start-proxy/deprecate-fallback branch from cd87ccd to 133ffeb Compare October 6, 2026 16:10
@mbg
mbg marked this pull request as ready for review October 6, 2026 16:12
@mbg
mbg requested a review from a team as a code owner October 6, 2026 16:12
@mbg
mbg requested review from henrymercer and a balanced review from Copilot October 6, 2026 16:12
Copilot started reviewing on behalf of mbg October 6, 2026 16:13
View session

This comment was marked as outdated.

Sign in to view

Sorry, something went wrong.

Uh oh!

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

@mbg
Fix onceWith not asserting the once part
f9a9962
@github-actions github-actions Bot added size/XL May be very hard to review and removed size/L May be hard to review labels Oct 6, 2026
henrymercer
henrymercer reviewed Oct 7, 2026
View reviewed changes
Comment thread src/setup/repository.test.ts Resolved

Uh oh!

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

Comment thread src/setup/repository.ts Resolved

Uh oh!

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

Comment thread src/setup/repository.ts Resolved

Uh oh!

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

Comment thread src/setup/repository.ts Outdated
},
);

for (const [apiURL, repository] of uniqueDownloadSources) {

@henrymercer henrymercer Oct 7, 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 is just a URL, not an API URL, i.e. it doesn't start with api.

Sorry, something went wrong.

Uh oh!

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

All reactions

@mbg mbg Oct 12, 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.

The name was just copied from setup-codeql.ts, but I have renamed this to instanceURL to address the feedback. While doing that I also noticed that apiURL /instanceURL isn't actually used at all beyond the short-circuit check and in log messages. That seems like it may have been an oversight in your PR when you added this, because without this we just end up checking for the repos on whatever instance getApiClient decides to use instead of the intended apiURL /instanceURL?

Sorry, something went wrong.

Uh oh!

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

All reactions

@henrymercer henrymercer Oct 12, 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.

without this we just end up checking for the repos on whatever instance getApiClient decides to use instead of the intended apiURL /instanceURL?

Sorry, could you explain what you mean by this? The intent is to check the called Action repo on the same GH instance, the canonical github/codeql-action on the same GH instance, followed by github/codeql-action on Dotcom.

Sorry, something went wrong.

Uh oh!

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

All reactions

@mbg mbg Oct 13, 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.

Suppose that we are on GHE with GITHUB_SERVER_URL being something like https://my-org.ghe.com and that GITHUB_ACTION_REPOSITORY is test-org/my-codeql-action-fork, then we have the following (expected/intended) value for potentialDownloadSources:

[
  ["https://my-org.ghe.com", "test-org/my-codeql-action-fork"],
  ["https://my-org.ghe.com", "github/codeql-action"],
  ["https://github.com", "github/codeql-action"]
]

The for (const [instanceURL, repository] of uniqueDownloadSources) loop, however, never explicitly uses instanceURL for getReleaseByTag. The getApiClient call makes its own call to getApiDetails(env) which will always get it the GitHubApiDetails based on GITHUB_SERVER_URL and GITHUB_API_URL.

This happens to work as expected, because the only entry in potentialDownloadSources that would want different GitHubApiDetails is the last element of the array and we short-circuit the getReleaseByTag call there to exit the loop and return the result of getDefaultDotcomDownloadURL.

However, there are two problems with this:

  1. It is not immediately obvious that this happens to work out for any potentialDownloadSources that is currently possible.
  2. If we ever make a change that allows an additional entry in potentialDownloadSources where the instance URL doesn't match GITHUB_SERVER_URL, or the repository name is not GITHUB_ACTION_REPOSITORY if the instance URL is GITHUB_DOTCOM_URL, then this leads to a hard-to-troubleshoot issue where the short-circuit check doesn't apply and we instead call getReleaseByTag with the wrong GitHubApiDetails.

I'd suggest we either change this so that we configure the api client correctly in the loop for the instance given by instanceURL, to avoid shooting ourselves in the foot in the future. Alternatively, we should document this very explicitly and perhaps add a test to make sure we never end up with this scenario. I'll have a look.

Sorry, something went wrong.

Uh oh!

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

All reactions

@henrymercer henrymercer Oct 14, 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.

Ah I see, thanks for the explanation! I agree the code indeed currently does the right thing, but that hardening makes sense.

Sorry, something went wrong.

Uh oh!

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

All reactions
Comment thread src/testing-utils.ts Resolved

Uh oh!

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

Comment thread src/setup/repository.ts Resolved

Uh oh!

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

Comment thread src/start-proxy.ts Resolved

Uh oh!

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

henrymercer
henrymercer reviewed Oct 7, 2026
View reviewed changes

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

Couple of additional comments

Sorry, something went wrong.

Uh oh!

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

All reactions
Comment thread src/logging.ts Resolved

Uh oh!

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

Comment thread src/setup/repository.ts Resolved

Uh oh!

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

Comment thread src/start-proxy.ts Resolved

Uh oh!

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

Comment thread src/start-proxy.test.ts Resolved

Uh oh!

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

Comment thread src/start-proxy.test.ts Resolved

Uh oh!

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

Comment thread src/setup/repository.test.ts Resolved

Uh oh!

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

Comment thread src/setup/repository.ts Resolved

Uh oh!

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

@mbg mbg mentioned this pull request Oct 7, 2026
Do not use core.exportVariable in unit tests (2nd attempt) #4212
Merged
@debanjanbasu debanjanbasu mentioned this pull request Oct 10, 2026
start-proxy selects an AMD64 binary on Linux ARM64 #4173
Closed
mbg added 6 commits October 12, 2026 14:52
@mbg
Merge branch 'main' into mbg/start-proxy/deprecate-fallback
265d882
@mbg
Allow initializeEnvironment to accept Env
c1b620c
@mbg
Add and use RepositoryNwoStr type
ad8ebce
@mbg
Use GITHUB_DOTCOM_URL in getDefaultDotComDownloadURL
1a92e13
@mbg
Use "asset" in comment
e93db40
@mbg
Use DownloadSource for potentialDownloadSources
fdc069b
mbg added 8 commits October 12, 2026 15:31
@mbg
DotCom => Dotcom
06bd6e2
@mbg
Log more in getProxyBinaryPath
3d7016f
@mbg
Await toolcache.downloadTool
dd3898a
@mbg
Remove duplicated assertions
ac44fd0
@mbg
Fix mockGetReleaseByTag usage
a1794a0
@mbg
Rename apiURL to instanceURL
8fed29e
@mbg
Add InstanceURL and use in DownloadSource
5aa103d
@mbg
Use parseRepositoryNwo in getCodeQLAssetDownloadURL
b67b5ab
This was referenced Oct 13, 2026
Make auth in GitHubApiDetails optional #4230
Merged
Use correct GitHubApiDetails instance for getApiClient in getCodeQLAssetDownloadURL #4231
Open
henrymercer
henrymercer reviewed Oct 14, 2026
View reviewed changes

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

Couple of small things, and there are some conflicts to address, but otherwise this looks good to me!

Sorry, something went wrong.

Uh oh!

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

All reactions
Comment thread src/logging.ts Resolved

Uh oh!

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

Comment thread src/start-proxy.ts Resolved

Uh oh!

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

Comment thread src/start-proxy.ts Resolved

Uh oh!

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

mbg added 3 commits October 14, 2026 13:56
@mbg
Merge remote-tracking branch 'origin/main' into mbg/start-proxy/depre…
9da9625
…cate-fallback
@mbg
Fix typo in log message
71ad3a7
@mbg
Rename proxyBin to proxyDir
962aaad
@mbg mbg mentioned this pull request Oct 14, 2026
Remove getRunnerLogger #4235
Merged
@mbg
Revert "Allow logging functions to accept string[] directly"
28d5dd9
This reverts commit 909fda1.
@mbg
mbg requested review from henrymercer and a balanced review from Copilot October 14, 2026 14:08
Copilot started reviewing on behalf of mbg October 14, 2026 14:09
View session
Copilot AI reviewed Oct 14, 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.

Warning

  • Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.

🟡 Changes recommended

Repository NWO validation accepts malformed values that can fail later outside the intended validation handling.

1 open finding
  • Medium severity Require exactly two non-empty repository name segments · New
1 resolved since last review
  • Medium severity Ensure onceWith rejects multiple calls
Files excluded by content exclusion policy (1)
  • lib/entry-points.js

🧠 Review effort: Balanced

Sorry, something went wrong.

Uh oh!

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

All reactions
Comment thread src/repository.ts Resolved

Uh oh!

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

henrymercer
henrymercer approved these changes Oct 14, 2026
View reviewed changes
@mbg
mbg added this pull request to the merge queue Oct 14, 2026
Merged via the queue into main with commit f5b8e09 Oct 14, 2026
235 checks passed

Uh oh!

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

@mbg
mbg deleted the mbg/start-proxy/deprecate-fallback branch October 14, 2026 14:54
github-merge-queue Bot pushed a commit that referenced this pull request Oct 14, 2026
@mbg
Add change note for #4198
c8bd019
This was referenced Oct 15, 2026
Merge main into releases/v4 #4239
Merged
Merge releases/v4 into releases/v3 #4241
Merged
@mbg mbg mentioned this pull request Oct 16, 2026
Remove TRAP cache cleanup #4243
Merged
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
@henrymercer henrymercer henrymercer approved these changes

Uh oh!

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

Assignees

No one assigned

Labels

size/XL May be very 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

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