ci: add workflow to trigger specific builds on PRs by comments#152
ci: add workflow to trigger specific builds on PRs by comments#152patrick-stephens merged 1 commit intomainfrom
Conversation
Signed-off-by: Patrick Stephens <pat@fluent.do>
patrick-stephens
force-pushed
the
add_per_build_workflow
branch
from
469f6d8 to
4e7cfa5
Compare
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
5 issues found across 1 file
Prompt for AI agents (all 5 issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name=".github/workflows/pr-comment-build.yaml">
<violation number="1" location=".github/workflows/pr-comment-build.yaml:101">
P1: Missing `always()` in condition. This job depends on conditional build jobs that will be skipped when not requested. Without `always()`, this job won't run when dependencies are skipped, so the results comment will never be posted.</violation>
<violation number="2" location=".github/workflows/pr-comment-build.yaml:117">
P1: Rule violated: **Require Safe Shell Scripts (set -euo pipefail or explicit env checks)**
Shell script lacks `set -euo pipefail` and contains unguarded variable usage. If `grep` fails to find the pattern, `VERSION` will be empty and the script continues silently, propagating an empty version to downstream builds. Add strict shell options or use guarded parameter expansion.</violation>
<violation number="3" location=".github/workflows/pr-comment-build.yaml:149">
P1: Inconsistent condition for Linux build check. The output is set to `'true'` or `'false'` (strings), but this condition checks `!= ''`. Since `'false' != ''` evaluates to true, Linux builds will be triggered even when not requested. Use `== 'true'` for consistency with Windows and macOS checks.</violation>
<violation number="4" location=".github/workflows/pr-comment-build.yaml:187">
P2: Incorrect iteration over paginated results. `github.paginate()` returns a flattened array of jobs directly, not response objects with `jobs` arrays. The inner loop `for (const j of job.jobs || [])` will never execute since `job.jobs` will be undefined.</violation>
<violation number="5" location=".github/workflows/pr-comment-build.yaml:249">
P1: `ISSUE_NUMBER` is used but not defined in the `env` block. This will be `undefined`, causing the comment body to show 'undefined' and `Number(undefined)` to return `NaN`, breaking the API call.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
| }); | ||
| const byName = {}; | ||
| for (const job of pages) { | ||
| for (const j of job.jobs || []) { |
There was a problem hiding this comment.
P2: Incorrect iteration over paginated results. github.paginate() returns a flattened array of jobs directly, not response objects with jobs arrays. The inner loop for (const j of job.jobs || []) will never execute since job.jobs will be undefined.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/pr-comment-build.yaml, line 187:
<comment>Incorrect iteration over paginated results. `github.paginate()` returns a flattened array of jobs directly, not response objects with `jobs` arrays. The inner loop `for (const j of job.jobs || [])` will never execute since `job.jobs` will be undefined.</comment>
<file context>
@@ -0,0 +1,251 @@
+ });
+ const byName = {};
+ for (const job of pages) {
+ for (const j of job.jobs || []) {
+ // Keep the first occurrence
+ if (!byName[j.name]) {
</file context>
| needs: | ||
| - pr-build-comment | ||
| - pr-build-get-metadata | ||
| if: needs.pr-build-comment.outputs.should-run == 'true' && needs.pr-build-comment.outputs.build-linux != '' |
There was a problem hiding this comment.
P1: Inconsistent condition for Linux build check. The output is set to 'true' or 'false' (strings), but this condition checks != ''. Since 'false' != '' evaluates to true, Linux builds will be triggered even when not requested. Use == 'true' for consistency with Windows and macOS checks.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/pr-comment-build.yaml, line 149:
<comment>Inconsistent condition for Linux build check. The output is set to `'true'` or `'false'` (strings), but this condition checks `!= ''`. Since `'false' != ''` evaluates to true, Linux builds will be triggered even when not requested. Use `== 'true'` for consistency with Windows and macOS checks.</comment>
<file context>
@@ -0,0 +1,251 @@
+ needs:
+ - pr-build-comment
+ - pr-build-get-metadata
+ if: needs.pr-build-comment.outputs.should-run == 'true' && needs.pr-build-comment.outputs.build-linux != ''
+ uses: ./.github/workflows/call-build-linux-packages.yaml
+ with:
</file context>
| await github.rest.issues.createComment({ | ||
| owner, | ||
| repo, | ||
| issue_number: Number(process.env.ISSUE_NUMBER), |
There was a problem hiding this comment.
P1: ISSUE_NUMBER is used but not defined in the env block. This will be undefined, causing the comment body to show 'undefined' and Number(undefined) to return NaN, breaking the API call.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/pr-comment-build.yaml, line 249:
<comment>`ISSUE_NUMBER` is used but not defined in the `env` block. This will be `undefined`, causing the comment body to show 'undefined' and `Number(undefined)` to return `NaN`, breaking the API call.</comment>
<file context>
@@ -0,0 +1,251 @@
+ await github.rest.issues.createComment({
+ owner,
+ repo,
+ issue_number: Number(process.env.ISSUE_NUMBER),
+ body
+ });
</file context>
| pr-build-get-metadata: | ||
| name: Get build metadata | ||
| needs: pr-build-comment | ||
| if: needs.pr-build-comment.outputs.should-run == 'true' |
There was a problem hiding this comment.
P1: Missing always() in condition. This job depends on conditional build jobs that will be skipped when not requested. Without always(), this job won't run when dependencies are skipped, so the results comment will never be posted.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/pr-comment-build.yaml, line 101:
<comment>Missing `always()` in condition. This job depends on conditional build jobs that will be skipped when not requested. Without `always()`, this job won't run when dependencies are skipped, so the results comment will never be posted.</comment>
<file context>
@@ -0,0 +1,251 @@
+ pr-build-get-metadata:
+ name: Get build metadata
+ needs: pr-build-comment
+ if: needs.pr-build-comment.outputs.should-run == 'true'
+ runs-on: ubuntu-latest
+ permissions:
</file context>
There was a problem hiding this comment.
@cubic-dev-ai this should only run if we decide to run the builds, the condition it has should detect that?
There was a problem hiding this comment.
@patrick-stephens You're right that the if: needs.pr-build-comment.outputs.should-run == 'true' condition correctly gates whether builds should trigger overall—that part works fine to detect if a valid /build command was parsed.
The issue I flagged is separate: even when builds are triggered (or partially skipped), the response job (which posts the results comment) depends on the build jobs via needs. When a build job like pr-build-windows isn't requested, it gets skipped entirely. GitHub Actions then skips the response job too, because skipped dependencies block the whole chain. That's why you need always() added to the response job's if condition, like this:
pr-build-comment-response:
name: Respond to PR comment
if: always() && needs.pr-build-comment.outputs.should-run == 'true'
# ... rest unchangedThis ensures the response always runs to report what happened (success, skip, etc.), regardless of which builds fired. Without it, you'd never get that summary comment back on the PR.
| # Look for the default line in the Dockerfile | ||
| id: set-version | ||
| run: | | ||
| VERSION=$(grep "ARG FLUENTDO_AGENT_VERSION=" Dockerfile.ubi | cut -d '=' -s -f 2 -) |
There was a problem hiding this comment.
P1: Rule violated: Require Safe Shell Scripts (set -euo pipefail or explicit env checks)
Shell script lacks set -euo pipefail and contains unguarded variable usage. If grep fails to find the pattern, VERSION will be empty and the script continues silently, propagating an empty version to downstream builds. Add strict shell options or use guarded parameter expansion.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/pr-comment-build.yaml, line 117:
<comment>Shell script lacks `set -euo pipefail` and contains unguarded variable usage. If `grep` fails to find the pattern, `VERSION` will be empty and the script continues silently, propagating an empty version to downstream builds. Add strict shell options or use guarded parameter expansion.</comment>
<file context>
@@ -0,0 +1,251 @@
+ # Look for the default line in the Dockerfile
+ id: set-version
+ run: |
+ VERSION=$(grep "ARG FLUENTDO_AGENT_VERSION=" Dockerfile.ubi | cut -d '=' -s -f 2 -)
+ echo "Using version: $VERSION"
+ echo "version=$VERSION" >> $GITHUB_OUTPUT
</file context>
There was a problem hiding this comment.
@cubic-dev-ai is this just meaning for pipefail? Github Actions should automatically handle set -eu
There was a problem hiding this comment.
@patrick-stephens No, it's not just about pipefail—the full set -euo pipefail combo is important for different reasons, and GitHub Actions doesn't fully handle this automatically.
GitHub does set set -e by default in shell steps (exits on command failure), but it skips -u (treat unset vars as errors) and -o pipefail (fail if any part of a pipeline fails, like your grep | cut chain). Without -u, if $GITHUB_OUTPUT were somehow unset (unlikely but possible), it'd silently fail. More critically, without -o pipefail, if grep finds nothing but cut runs anyway, the pipeline succeeds and VERSION stays empty—your script keeps going, echoing an empty version and passing it downstream, which could trigger broken builds.
Adding set -euo pipefail at the top of that step ensures the whole thing fails fast if the grep/cut doesn't produce a valid version. You could also guard it like this:
- name: Get the versions to use
id: set-version
run: |
set -euo pipefail
VERSION=$(grep "ARG FLUENTDO_AGENT_VERSION=" Dockerfile.ubi | cut -d '=' -s -f 2 -)
if [[ -z "$VERSION" ]]; then
echo "Error: Version not found in Dockerfile.ubi"
exit 1
fi
echo "Using version: $VERSION"
echo "version=$VERSION" >> "$GITHUB_OUTPUT"
shell: bashThat way, if the file's missing or the ARG isn't there, the job fails early with a clear message instead of propagating junk.
1 participant
Workflow that lets us add a comment to a PR to build a specific target rather than having to label to build all. Means we can say to just build a specific Linux target rather than all of them for example.
These will not be status checks so we separately add a response comment with the results. It does mean you can trigger a single build after review or other builds are complete as well.
Summary by cubic
Add a PR comment-triggered CI workflow to run specific platform builds by writing “/build” on a PR. Builds don’t create status checks; the workflow replies with a results comment and job links.
Written for commit 4e7cfa5. Summary will update automatically on new commits.