ci: support full validation on workflow_dispatch (no PR context)#181
Merged
Conversation
The Build CodeQL Packs workflow (ci.yml) previously assumed every run had
a PR context (github.event.number). For workflow_dispatch runs this value
is empty, which caused two problems:
1. Unquoted script arguments (pr-compile.sh, pr-suites-packs.sh) meant an
empty github.event.number was dropped entirely by bash word-splitting,
shifting the language argument into position 1 and leaving position 2
unset - crashing immediately under set -u ("unbound variable") before
any query compilation happened.
2. Every step was gated on dorny/paths-filter's src output, which is
PR-diff-based and not meaningful for a manual "verify everything" run.
Fixes:
- Quote the PR-number/language args passed to pr-compile.sh and
pr-suites-packs.sh so an empty value is passed as an explicit empty
string instead of vanishing.
- Add || github.event_name == 'workflow_dispatch' to every
steps.changes.outputs.src == 'true' step condition (12 occurrences
across compile-and-test, extensions, library-sources, and configs jobs)
so a manual dispatch always runs every step regardless of path-filter.
- Give pr-compile.sh, pr-suites-packs.sh, and pr-configs.sh a "full mode"
fallback that activates whenever PR_NUMBER is empty: instead of calling
gh pr view / gh pr comment (which require a PR), each script now
enumerates the relevant files directly from the filesystem (full
--warnings=error compile of the language, find over suites/*.qls,
glob over configs/*.yml) and skips any PR-comment logic. This keeps a
single dual-mode script per concern instead of forking separate
PR/non-PR script files, avoiding future logic drift between the two
paths.
This lets ci.yml be dispatched manually on any branch (including main)
to get a real, full compile-and-test validation without requiring an
open pull request - closing the validation gap that let PR #179 get
merged on the strength of the wrong CI signal.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the “Build CodeQL Packs” CI workflow and its helper scripts so the workflow can be run via workflow_dispatch on any branch and still perform full compile/test validation without requiring a pull request context.
Changes:
- Adjusts
ci.ymlstep gating so manual dispatch runs all relevant steps regardless ofpaths-filteroutput. - Quotes PR number/language args passed into CI scripts to avoid argument shifting when PR context is missing.
- Adds “no PR number” fallback logic to the PR-oriented scripts to enumerate files/configs from the filesystem instead of using
gh pr viewand PR comments.
Show a summary per file
| File | Description |
|---|---|
| .github/workflows/ci.yml | Allows workflow_dispatch to run the full validation path by bypassing PR-diff-based gating and quoting script arguments. |
| .github/scripts/pr-suites-packs.sh | Adds a non-PR mode that resolves all suites and rebuilds packs directly from the working tree. |
| .github/scripts/pr-configs.sh | Adds a non-PR mode that validates all configs/*.yml directly, and refactors compilation into a helper function. |
| .github/scripts/pr-compile.sh | Adds a non-PR mode for full strict compilation, but currently performs redundant compilation work (see PR comment). |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Low
…LANGUAGE codeql pack install/create requires a directory containing a qlpack.yml directly. No language has a top-level qlpack.yml (only nested src/, lib/, ext/, ext-library-sources/) so `codeql pack install "$LANGUAGE"` fails with "ERROR: The directory is not a qlpack." Loop over the actual pack subdirectories instead, matching what codeql-workspace.yml's `**/qlpack.yml` globs actually resolve to. Caught by dispatching this branch's workflow_dispatch run directly - cpp/go/ruby all failed identically before this fix.
…klog publish.yml never runs --warnings=error (only `codeql pack install`/`publish`, which don't treat warnings as fatal), so full-mode's strict recompile of the entire language directory was a brand-new, stricter check than what actually gates real publishing - and it tripped on 5 pre-existing deprecated-API warnings (getResult, getAUse, SsaVariable) that no PR-mode diff has ever caught because no PR has ever touched all of them at once. The plain compile check above already covers the whole language directory without treating warnings as fatal, so full-mode is done once that passes. Filed a follow-up issue to clean up the warning backlog and then make PR-mode itself trigger a full strict compile when a PR touches .codeqlversion or a codeql-pack.lock.yml, since a dependency/CLI bump can change behavior across every query, not just the files literally edited. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What
Fixes
ci.yml("Build CodeQL Packs") so it can be run viaworkflow_dispatchon any branch and get a real, full compile-and-test validation - without
requiring an open pull request.
Why
While validating the CLI 2.24.3 bump (#179 / #180), we discovered
ci.ymlsilently assumed every run has a PR context (
github.event.number). Forworkflow_dispatchruns this value is empty, which broke things two ways:pr-compile.sh/pr-suites-packs.shwere invoked with unquoted
${{ github.event.number }}. When empty, bashword-splitting drops the argument entirely, shifting
LANGUAGEintoposition
$1and leaving$2unset -set -uthen throwsunbound variableimmediately, beforecodeql query compileever runs.steps.changes.outputs.src == 'true'fromdorny/paths-filter, which isPR-diff-based and not meaningful for a manual "verify everything" run.
This is exactly the gap that let PR #179 get merged on the strength of the
wrong CI signal (the repo's own CodeQL self-scan
Analyzejobs looked green,but the real
compile-and-testjob never actually re-ran on the fix commit).Changes
pr-compile.shandpr-suites-packs.shso an empty value is passed as an explicit emptystring instead of vanishing.
|| github.event_name == 'workflow_dispatch'to everysteps.changes.outputs.src == 'true'condition (12 occurrences acrosscompile-and-test,extensions,library-sources, andconfigsjobs)so a manual dispatch always runs every step regardless of the path filter.
pr-compile.sh,pr-suites-packs.sh, andpr-configs.sha "fullmode" fallback that activates whenever
PR_NUMBERis empty: instead ofgh pr view/gh pr comment(which require a PR), each script enumeratesthe relevant files directly from the filesystem and skips PR-comment
logic. Kept as single dual-mode scripts (not separate PR/non-PR files) to
avoid future logic drift.
Validation plan
(real
pull_requestevent) - confirms no regression.workflow_dispatchthis branch directly to provethe new non-PR path actually compiles all 7 languages without
crashing.