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

Show file tree
Hide file tree
Changes from all commits
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
81 changes: 80 additions & 1 deletion actions/setup/js/check_skip_if_helpers.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
// @ts-check
/// <reference types="@actions/github-script" />

const { getErrorMessage } = require("./error_helpers.cjs");
const { ERR_API, ERR_CONFIG } = require("./error_codes.cjs");
const { writeDenialSummary } = require("./pre_activation_summary.cjs");

/**
* Builds the GitHub search query, optionally scoping it to the current repository.
* @param {string} skipQuery - The base query string
Expand All @@ -18,4 +22,79 @@ function buildSearchQuery(skipQuery, skipScope) {
return searchQuery;
}

module.exports = { buildSearchQuery };
/**
* Shared runner for skip-if query gates.
* @param {{
* skipQuery: string | undefined;
* workflowName: string | undefined;
* thresholdStr: string | undefined;
* thresholdEnvVar: string;
* thresholdLabel: string;
* checkLabel: string;
* outputName: string;
* skipScope: string | undefined;
* shouldSkip: (totalCount: number, threshold: number) => boolean;
* warningMessage: (totalCount: number, threshold: number) => string;
* successMessage: (totalCount: number, threshold: number) => string;
* denialSummaryMessage: (totalCount: number, threshold: number) => string;
* denialSummaryNextStep: string;
* }} options
*/
// Ambient globals provided by @actions/github-script: core, github, context
async function runSkipQueryGate(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.

return;
}

if (!workflowName) {

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

[/codebase-design] workflowName is validated here but never referenced inside the function — dead validation.

💡 Fix or remove

The extracted helper checks workflowName is present but then never uses it — not in logs, not in the denial summary, nowhere. Either incorporate it (e.g., log the active workflow) or remove the field from the interface and all callers.

// Option A: use it in a log
core.info(`Running ${checkLabel} gate for workflow: ${workflowName}`);

// Option B: remove from interface and callers
// — eliminates the misleading contract

A required option that is validated but silently discarded makes the interface harder to reason about.

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

workflowName is validated but never used — dead validation or a latent bug from the pre-refactor code.

💡 Details

workflowName is destructured, checked with if (!workflowName), and then never referenced again anywhere in the function body. It doesn't appear in the search query, the log output, the denial summary message, or any other call.

Two possible interpretations:

  1. The validation is intentional but wrong: callers must supply GH_AW_WORKFLOW_NAME for no functional reason — the helper ignores it entirely.
  2. The field was meant to be used: it was likely supposed to appear in the denial summary or scoped query and was silently dropped during the refactor.

The pre-refactor code had the same pattern, but this PR cements it into a shared helper used by both call sites. Either use workflowName (e.g., in the denial summary or log line) or remove it from the options schema and eliminate the GH_AW_WORKFLOW_NAME requirement entirely.

core.setFailed(`${ERR_CONFIG}: Configuration error: GH_AW_WORKFLOW_NAME not specified.`);
return;
}

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

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;
}

core.info(`Checking ${checkLabel} query: ${skipQuery}`);
core.info(`${thresholdLabel}: ${threshold}`);

const searchQuery = buildSearchQuery(skipQuery, skipScope);

try {
const {
data: { total_count: totalCount },
} = await github.rest.search.issuesAndPullRequests({
q: searchQuery,
per_page: 1,
});

core.info(`Search found ${totalCount} matching items`);

if (shouldSkip(totalCount, threshold)) {
core.warning(warningMessage(totalCount, threshold));
core.setOutput(outputName, "false");
await writeDenialSummary(denialSummaryMessage(totalCount, threshold), denialSummaryNextStep);
return;
}

core.info(successMessage(totalCount, threshold));
core.setOutput(outputName, "true");
} catch (error) {
core.setFailed(`${ERR_API}: Failed to execute search query: ${getErrorMessage(error)}`);
}
}

module.exports = { buildSearchQuery, runSkipQueryGate };
87 changes: 87 additions & 0 deletions actions/setup/js/check_skip_if_helpers.test.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,3 +109,90 @@ describe("check_skip_if_helpers.cjs - buildSearchQuery", () => {
});
});
});

describe("check_skip_if_helpers.cjs - runSkipQueryGate", () => {
/** @returns {Promise<{runSkipQueryGate: (options: any) => Promise<void>}>} */
const loadModule = () => import("./check_skip_if_helpers.cjs");

/** @type {Record<string, any>} */
const baseOptions = {
skipQuery: "is:issue is:open",
workflowName: "test-workflow",
thresholdStr: "1",
thresholdEnvVar: "GH_AW_SKIP_MAX_MATCHES",
thresholdLabel: "Maximum matches threshold",
checkLabel: "skip-if-match",
outputName: "skip_check_ok",
skipScope: undefined,
shouldSkip: (/** @type {number} */ totalCount, /** @type {number} */ threshold) => totalCount >= threshold,
warningMessage: () => "skip warning",
successMessage: () => "success message",
denialSummaryMessage: () => "summary message",
denialSummaryNextStep: "summary next step",
};

beforeEach(() => {
global.github = {
rest: {
search: {
issuesAndPullRequests: vi.fn(),
},
},
};
});

it("fails with config error when skipQuery is undefined", async () => {
const { runSkipQueryGate } = await loadModule();
await runSkipQueryGate({ ...baseOptions, skipQuery: undefined });
expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("GH_AW_SKIP_QUERY not specified"));
expect(global.github.rest.search.issuesAndPullRequests).not.toHaveBeenCalled();
});

it("fails with config error when workflowName is undefined", async () => {
const { runSkipQueryGate } = await loadModule();
await runSkipQueryGate({ ...baseOptions, workflowName: undefined });
expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("GH_AW_WORKFLOW_NAME not specified"));
expect(global.github.rest.search.issuesAndPullRequests).not.toHaveBeenCalled();
});

it("uses thresholdEnvVar in validation errors", async () => {
const { runSkipQueryGate } = await loadModule();
await runSkipQueryGate({ ...baseOptions, thresholdStr: "invalid" });
expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("GH_AW_SKIP_MAX_MATCHES must be a positive integer"));
expect(global.github.rest.search.issuesAndPullRequests).not.toHaveBeenCalled();
});

it("applies policy decision and output name when skip condition matches", async () => {
global.github.rest.search.issuesAndPullRequests.mockResolvedValue({ data: { total_count: 2 } });
const { runSkipQueryGate } = await loadModule();

await runSkipQueryGate({ ...baseOptions });

expect(mockCore.warning).toHaveBeenCalledWith("skip warning");
expect(mockCore.setOutput).toHaveBeenCalledWith("skip_check_ok", "false");
expect(mockCore.summary.addRaw).toHaveBeenCalled();
expect(mockCore.summary.write).toHaveBeenCalled();
});

it("sets output to true and logs success when skip condition is not met", async () => {
global.github.rest.search.issuesAndPullRequests.mockResolvedValue({ data: { total_count: 0 } });
const { runSkipQueryGate } = await loadModule();
const successMessage = vi.fn(() => "all clear");

await runSkipQueryGate({ ...baseOptions, successMessage });

expect(mockCore.setOutput).toHaveBeenCalledWith("skip_check_ok", "true");
expect(successMessage).toHaveBeenCalledWith(0, 1);
expect(mockCore.warning).not.toHaveBeenCalled();
});

it("calls setFailed with ERR_API prefix when search throws", async () => {
global.github.rest.search.issuesAndPullRequests.mockRejectedValue(new Error("rate limited"));
const { runSkipQueryGate } = await loadModule();

await runSkipQueryGate({ ...baseOptions });

expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("ERR_API"));
expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("rate limited"));
});
});
63 changes: 16 additions & 47 deletions actions/setup/js/check_skip_if_match.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,57 +1,26 @@
// @ts-check
/// <reference types="@actions/github-script" />

const { getErrorMessage } = require("./error_helpers.cjs");
const { ERR_API, ERR_CONFIG } = require("./error_codes.cjs");
const { buildSearchQuery } = require("./check_skip_if_helpers.cjs");
const { writeDenialSummary } = require("./pre_activation_summary.cjs");
const { runSkipQueryGate } = require("./check_skip_if_helpers.cjs");

async function main() {
const { GH_AW_SKIP_QUERY: skipQuery, GH_AW_WORKFLOW_NAME: workflowName, GH_AW_SKIP_MAX_MATCHES: maxMatchesStr = "1", GH_AW_SKIP_SCOPE: skipScope } = process.env;

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

if (!workflowName) {
core.setFailed(`${ERR_CONFIG}: Configuration error: GH_AW_WORKFLOW_NAME not specified.`);
return;
}

const maxMatches = parseInt(maxMatchesStr, 10);
if (isNaN(maxMatches) || maxMatches < 1) {
core.setFailed(`${ERR_CONFIG}: Configuration error: GH_AW_SKIP_MAX_MATCHES must be a positive integer, got "${maxMatchesStr}".`);
return;
}

core.info(`Checking skip-if-match query: ${skipQuery}`);
core.info(`Maximum matches threshold: ${maxMatches}`);

const searchQuery = buildSearchQuery(skipQuery, skipScope);

try {
const {
data: { total_count: totalCount },
} = await github.rest.search.issuesAndPullRequests({
q: searchQuery,
per_page: 1,
});

core.info(`Search found ${totalCount} matching items`);

if (totalCount >= maxMatches) {
core.warning(`🔍 Skip condition matched (${totalCount} items found, threshold: ${maxMatches}). Workflow execution will be prevented by activation job.`);
core.setOutput("skip_check_ok", "false");
await writeDenialSummary(`Skip-if-match query matched: ${totalCount} item(s) found (threshold: ${maxMatches}).`, "Update `on.skip-if-match:` in the workflow frontmatter if this skip was unexpected.");
return;
}

core.info(`✓ Found ${totalCount} matches (below threshold of ${maxMatches}), workflow can proceed`);
core.setOutput("skip_check_ok", "true");
} catch (error) {
core.setFailed(`${ERR_API}: Failed to execute search query: ${getErrorMessage(error)}`);
}
await runSkipQueryGate({
skipQuery,
workflowName,
thresholdStr: maxMatchesStr,
thresholdEnvVar: "GH_AW_SKIP_MAX_MATCHES",
thresholdLabel: "Maximum matches threshold",
checkLabel: "skip-if-match",
outputName: "skip_check_ok",
skipScope,
shouldSkip: (totalCount, threshold) => totalCount >= threshold,
warningMessage: (totalCount, threshold) => `🔍 Skip condition matched (${totalCount} items found, threshold: ${threshold}). Workflow execution will be prevented by activation job.`,
successMessage: (totalCount, threshold) => `✓ Found ${totalCount} matches (below threshold of ${threshold}), workflow can proceed`,
denialSummaryMessage: (totalCount, threshold) => `Skip-if-match query matched: ${totalCount} item(s) found (threshold: ${threshold}).`,
denialSummaryNextStep: "Update `on.skip-if-match:` in the workflow frontmatter if this skip was unexpected.",
});
}

module.exports = { main };
63 changes: 16 additions & 47 deletions actions/setup/js/check_skip_if_no_match.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,57 +1,26 @@
// @ts-check
/// <reference types="@actions/github-script" />

const { getErrorMessage } = require("./error_helpers.cjs");
const { ERR_API, ERR_CONFIG } = require("./error_codes.cjs");
const { buildSearchQuery } = require("./check_skip_if_helpers.cjs");
const { writeDenialSummary } = require("./pre_activation_summary.cjs");
const { runSkipQueryGate } = require("./check_skip_if_helpers.cjs");

async function main() {
const { GH_AW_SKIP_QUERY: skipQuery, GH_AW_WORKFLOW_NAME: workflowName, GH_AW_SKIP_MIN_MATCHES: minMatchesStr = "1", GH_AW_SKIP_SCOPE: skipScope } = process.env;

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

if (!workflowName) {
core.setFailed(`${ERR_CONFIG}: Configuration error: GH_AW_WORKFLOW_NAME not specified.`);
return;
}

const minMatches = parseInt(minMatchesStr, 10);
if (isNaN(minMatches) || minMatches < 1) {
core.setFailed(`${ERR_CONFIG}: Configuration error: GH_AW_SKIP_MIN_MATCHES must be a positive integer, got "${minMatchesStr}".`);
return;
}

core.info(`Checking skip-if-no-match query: ${skipQuery}`);
core.info(`Minimum matches threshold: ${minMatches}`);

const searchQuery = buildSearchQuery(skipQuery, skipScope);

try {
const {
data: { total_count: totalCount },
} = await github.rest.search.issuesAndPullRequests({
q: searchQuery,
per_page: 1,
});

core.info(`Search found ${totalCount} matching items`);

if (totalCount < minMatches) {
core.warning(`🔍 Skip condition matched (${totalCount} items found, minimum required: ${minMatches}). Workflow execution will be prevented by activation job.`);
core.setOutput("skip_no_match_check_ok", "false");
await writeDenialSummary(`Skip-if-no-match query returned too few results: ${totalCount} item(s) found (minimum required: ${minMatches}).`, "Update `on.skip-if-no-match:` in the workflow frontmatter if this skip was unexpected.");
return;
}

core.info(`✓ Found ${totalCount} matches (meets or exceeds minimum of ${minMatches}), workflow can proceed`);
core.setOutput("skip_no_match_check_ok", "true");
} catch (error) {
core.setFailed(`${ERR_API}: Failed to execute search query: ${getErrorMessage(error)}`);
}
await runSkipQueryGate({
skipQuery,
workflowName,
thresholdStr: minMatchesStr,
thresholdEnvVar: "GH_AW_SKIP_MIN_MATCHES",
thresholdLabel: "Minimum matches threshold",
checkLabel: "skip-if-no-match",
outputName: "skip_no_match_check_ok",
skipScope,
shouldSkip: (totalCount, threshold) => totalCount < threshold,
warningMessage: (totalCount, threshold) => `🔍 Skip condition matched (${totalCount} items found, minimum required: ${threshold}). Workflow execution will be prevented by activation job.`,
successMessage: (totalCount, threshold) => `✓ Found ${totalCount} matches (meets or exceeds minimum of ${threshold}), workflow can proceed`,
denialSummaryMessage: (totalCount, threshold) => `Skip-if-no-match query returned too few results: ${totalCount} item(s) found (minimum required: ${threshold}).`,
denialSummaryNextStep: "Update `on.skip-if-no-match:` in the workflow frontmatter if this skip was unexpected.",
});
}

module.exports = { main };
Loading