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.

With 13 fields, the one-liner is hard to scan for an optional field vs a required one:

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

This makes the default value for thresholdStr visible without having to scroll and clearly separates optional from required fields.

The runSkipQueryGate describe group opens at line 72 inside the describe('check_skip_if_helpers.cjs - buildSearchQuery', ...) block (line 41) and closes at line 132, but the scope: default (repo-scoped) group that follows at line 134 is a child of buildSearchQuery, not a sibling of runSkipQueryGate. This makes the nesting hierarchy misleading and may cause test reporters to nest runSkipQueryGate under buildSearchQuery in their output.

Move runSkipQueryGate to the top-level, e.g.:

describe("check_skip_if_helpers.cjs - buildSearchQuery", () => { ... });

describe("check_skip_if_helpers.cjs - runSkipQueryGate", () => { ... });

The new runSkipQueryGate tests cover:

1. Invalid threshold — setFailed called, search never reached
2. Skip condition met — setOutput with "false", warning emitted

Missing: skip condition not met (totalCount below threshold). This is the most common production path and should assert setOutput(outputName, "true") and successMessage invoked:

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({
    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: (count, t) => count >= t,
    warningMessage: () => '',
    successMessage,
    denialSummaryMessage: () => '',
    denialSummaryNextStep: '',
  });

  expect(mockCore.setOutput).toHaveBeenCalledWith('skip_check_ok', 'true');
  expect(successMessage).toHaveBeenCalledWith(0, 1);
});

The function has three early-exit guards (missing skipQuery, missing workflowName, bad threshold). Only the third is tested. Specs for the first two document the contract and prevent regressions if the guards are ever reordered:

it('fails with config error when skipQuery is undefined', async () => {
  const { runSkipQueryGate } = await loadModule();
  await runSkipQueryGate({ skipQuery: undefined, workflowName: 'wf', ... });
  expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining('GH_AW_SKIP_QUERY not specified'));
});

it('fails with config error when workflowName is undefined', async () => {
  const { runSkipQueryGate } = await loadModule();
  await runSkipQueryGate({ skipQuery: 'is:issue', workflowName: undefined, ... });
  expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining('GH_AW_WORKFLOW_NAME not specified'));
});

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({
    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: () => false,
    warningMessage: () => '',
    successMessage: () => '',
    denialSummaryMessage: () => '',
    denialSummaryNextStep: '',
  });

  expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining('ERR_API'));
  expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining('rate limited'));
});

Without this, a future refactor of the catch block could silently lose the error code.

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.

The describe('check_skip_if_helpers.cjs - runSkipQueryGate', ...) block opens at line 72 inside the describe('check_skip_if_helpers.cjs - buildSearchQuery', ...) block that opened at line 41. As a result:

• All runSkipQueryGate tests are reported in CI under the buildSearchQuery test suite label, making failures misleading.
• The inner const loadModule = () => import('./check_skip_if_helpers.cjs') at line 74 is an exact duplicate of the one at line 43, shadowing it pointlessly — a clear artifact of copy-paste inside the wrong nesting level.
• The describe('scope: default (repo-scoped)', ...) block at line 134 is also still inside the outer buildSearchQuery describe (correct by coincidence, but confusing).

Fix: close the outer buildSearchQuery describe before opening runSkipQueryGate, and remove the duplicate loadModule declaration:

The test at line 109 ('applies policy decision and output name when skip condition matches') checks core.warning and core.setOutput, but never asserts that the denial summary was written. The writeDenialSummary call at helper line 80 is a significant side effect: it writes to the GitHub Actions job summary and is the operator-visible explanation for why a workflow was blocked.

The mocks for core.summary.addRaw and core.summary.write are already in place via mockCore.summary — but they are never asserted on.

Add assertions such as:

expect(mockCore.summary.addRaw).toHaveBeenCalled();
expect(mockCore.summary.write).toHaveBeenCalled();

Without this, a future regression that silently drops the writeDenialSummary call will not be caught by this test suite.

The helper destructures thresholdStr = '1' as a default at line 44, and the callers also apply their own defaults at the env-var layer (e.g., GH_AW_SKIP_MAX_MATCHES: maxMatchesStr = '1'). This double-default means:

• If a future caller forgets to set a default (or passes undefined explicitly), the threshold silently becomes 1 instead of triggering the config-validation error at line 57-59.
• The validation gate that says "must be a positive integer" becomes unreachable for the undefined case — the most likely misconfiguration scenario.

The default policy belongs in one place. Since the threshold default is env-var-specific ("what does a missing GH_AW_SKIP_MAX_MATCHES mean?"), it should live at the caller's env-destructuring layer. Remove the default from the helper's options destructuring and let an undefined thresholdStr fall through to the isNaN check so it fails loudly: