Skip to content

Deduplicate skip-query gate logic for setup action guards #43480

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pelikhan merged 4 commits into from
Jul 5, 2026
Merged

Deduplicate skip-query gate logic for setup action guards #43480

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8d70242
Initial plan
Copilot Jul 5, 2026
aa78625
refactor: deduplicate skip query gate logic
Copilot Jul 5, 2026
08ad2b9
refactor: address review feedback on runSkipQueryGate helper
Copilot Jul 5, 2026
21dac8f
fix: resolve TS2345 type error and address remaining review threads
Copilot Jul 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
fix: resolve TS2345 type error and address remaining review threads 
- Fix TS2345: parseInt(thresholdStr ?? "", 10) so undefined thresholdStr
  falls through to the isNaN guard instead of failing type-check
- Add prettier-ignore + multi-line destructure for readability
- Add ambient globals comment (core, github, context) for
  discoverability by static analysis and IDE tooling

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
  • Loading branch information
@gh-aw-bot
Copilot and gh-aw-bot authored Jul 5, 2026
commit 21dac8f23ddcec482e0d60ecabda5e77c77c3f13
11 changes: 9 additions & 2 deletions actions/setup/js/check_skip_if_helpers.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,15 @@ function buildSearchQuery(skipQuery, skipScope) {
* denialSummaryNextStep: string;
* }} options
*/
// Ambient globals provided by @actions/github-script: core, github, context
async function runSkipQueryGate(options) {
const { skipQuery, workflowName, thresholdStr, thresholdEnvVar, thresholdLabel, checkLabel, outputName, skipScope, shouldSkip, warningMessage, successMessage, denialSummaryMessage, denialSummaryNextStep } = options;
// prettier-ignore
const {
skipQuery, workflowName, thresholdStr,
thresholdEnvVar, thresholdLabel, checkLabel, outputName, skipScope,
shouldSkip, warningMessage, successMessage,
denialSummaryMessage, denialSummaryNextStep,
} = options;

if (!skipQuery) {
core.setFailed(`${ERR_CONFIG}: Configuration error: GH_AW_SKIP_QUERY not specified.`);

@github-actions github-actions Bot Jul 5, 2026

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.

The runSkipQueryGate function uses core, github, and context as globals — consistent with how the callers work under @actions/github-script. However, those globals are not declared or imported in this helper module, so static analysis (and IDE tooling) cannot verify them. Consider adding a brief comment referencing the expected ambient globals, or — if the module is ever tested in isolation without a global-script harness — accept them as implicit. The test file's beforeEach does set them up, so tests are fine; this is a documentation/discoverability concern only.

@copilot please address this.

@github-actions github-actions Bot Jul 5, 2026

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.

The destructuring line is very long (170+ chars) — the entire options bag unpacked on one line. Breaking it into a multi-line destructure would aid readability and make diffs easier to review if individual parameters change later.

const {
  skipQuery, workflowName, thresholdStr = "1",
  thresholdEnvVar, thresholdLabel, checkLabel, outputName, skipScope,
  shouldSkip, warningMessage, successMessage,
  denialSummaryMessage, denialSummaryNextStep,
} = options;

@copilot please address this.

Expand All @@ -55,7 +62,7 @@ async function runSkipQueryGate(options) {

core.info(`Running ${checkLabel} gate for workflow: ${workflowName}`);

const threshold = parseInt(thresholdStr, 10);
const threshold = parseInt(thresholdStr ?? "", 10);
if (isNaN(threshold) || threshold < 1) {
core.setFailed(`${ERR_CONFIG}: Configuration error: ${thresholdEnvVar} must be a positive integer, got "${thresholdStr}".`);
return;
Expand Down
Loading