Add feature for forcing the nightly bundle in dynamic workflows#3484
Add feature for forcing the nightly bundle in dynamic workflows#3484
nightly bundle in dynamic workflows#3484Conversation
c4ada50 to
a61e3cb
Compare
There was a problem hiding this comment.
Pull request overview
This pull request adds a new ForceNightly feature flag that allows forcing the use of the nightly CodeQL CLI bundle in dynamic workflows (Default Setup, CCR, etc.). The feature flag is restricted to dynamic workflow events or test mode, preventing it from affecting advanced workflows. The PR includes comprehensive unit tests and an end-to-end PR check to validate the functionality.
Changes:
- Added
ForceNightlyfeature flag to enable nightly CLI for dynamic workflows - Enhanced
getCodeQLSourcewith documentation and logic to support forced nightly builds - Added unit tests for both
tools: nightlyand ForceNightly feature flag scenarios - Created new PR check workflow to validate the feature in CI
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/feature-flags.ts | Adds the ForceNightly feature flag definition with environment variable and default value |
| src/setup-codeql.ts | Implements logic to force nightly CLI when feature flag is enabled in dynamic workflows; adds JSDoc documentation and exports for testing |
| src/setup-codeql.test.ts | Adds comprehensive unit tests for nightly CLI selection via both explicit input and feature flag |
| pr-checks/checks/bundle-from-nightly.yml | Defines PR check template to validate ForceNightly feature works as expected |
| .github/workflows/__bundle-from-nightly.yml | Auto-generated workflow file from the PR check template |
| lib/*.js | Auto-generated JavaScript transpilation of TypeScript source changes (not reviewed per guidelines) |
esbena
left a comment
There was a problem hiding this comment.
LGTM with some superficial logging observability/logging concerns.
Also set default `GITHUB_EVENT_NAME` in `setupActionsVars`
e7c08f9 to
e315c6f
Compare
henrymercer
left a comment
There was a problem hiding this comment.
I had a quick look over the last two commits.
- I like the solution with storing unwritten nolang diagnostics.
- I would suggest changing the severity of the diagnostic to info, since the UI will complain relatively loudly about warnings.
- The changes look correct but it is hard to say for sure that they will work without a PR check or example run — consider running this on a test repo before merging.
|
@henrymercer I have made those changes and ran a test in a test repository. See the references in the PR timeline. |
esbena
left a comment
There was a problem hiding this comment.
Latest (trivial) changes LGTM, and the sample run looks as advertised.
Adds a
Featurewhich, when enabled, and the workflow was triggered by adynamicevent forcesgetCodeQLSourceto pick the latest, nightly release.Some drive-by improvements and observations:
getCodeQLSource.!toolsInput.startsWith("http")check would probably cause problems if a file happened to have a path that starts withhttp. Probably not a very likely problem, but could be tricky to troubleshoot if it happens. We could improve this by performing more explicit checks forhttp://andhttps://or whether the suspected file exists locally or not. Outside of the scope of this PR though.tools: nightlywhich didn't seem to exist.getNightlyToolsUrlseems to assume that the nightly tag iscodeql-bundle-followed by a semver, but this is no longer the case for nightly releases. There's a fallback logic which handles this fine, but we should probably update this to expect the date-based tags.Risk assessment
For internal use only. Please select the risk level of this change:
Which use cases does this change impact?
Workflow types:
dynamicworkflows (Default Setup, CCR, ...).Products:
analysis-kinds: code-scanning.analysis-kinds: code-quality.Environments:
github.comand/or GitHub Enterprise Cloud with Data Residency.How did/will you validate this change?
.test.tsfiles).pr-checks).If something goes wrong after this change is released, what are the mitigation and rollback strategies?
How will you know if something goes wrong after this change is released?
Are there any special considerations for merging or releasing this change?
Merge / deployment checklist