From d3bed0936315628438b498c3c3f2ecb643063602 Mon Sep 17 00:00:00 2001 From: Deluan Date: Sat, 5 Sep 2026 14:25:03 -0400 Subject: [PATCH] ci: fix three change-detection gaps found in review `grep -q` exits on its first match, which closes the pipe under the still- writing `printf`. Under `pipefail` the killed writer, not the successful match, sets the exit status, so a large diff emitted `false` for an area that had changed. It is a race rather than a buffer threshold: it starts firing at around 1200 paths, well under the 64 KiB pipe buffer, and it kills the second `grep` in the build pipeline too. Both pipelines are now here-strings. The largest diff in this repo's history is 519 files, so this was latent, but a repo-wide sweep or a mass directory move reaches it. pipeline.yml and this script are now inputs to the go, js and i18n filters. A pipeline-only change previously skipped every suite that pipeline.yml defines, so a broken test command merged green and surfaced on master. Worse, a wrong gating expression is invisible in every run: `outputs.golang` instead of `outputs.go` reads as empty on master pushes too, and the suite disappears silently and permanently. This change adds 63 such expressions. Of 82 historical pipeline-only commits, 38 edited a job that is now gated, and the measured cost is about 12 extra full runs a year. The download-link workflow returned before looking for its own previous comment, so a PR that built binaries and then became docs-only kept advertising artifacts from a commit that is no longer the head. That branch was unreachable until this PR gated the build. It now deletes the stale comment. Deleting rather than rewording is deliberate: the comment is matched by its header, so a reworded body would either have to keep a header that contradicts it or start posting duplicates. The lookup is also paginated now; it saw only the first 30 comments. --- .github/workflows/detect-changes.sh | 13 ++++++++----- .github/workflows/download-link-on-pr.yml | 19 +++++++++++++------ 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/.github/workflows/detect-changes.sh b/.github/workflows/detect-changes.sh index 138db4871..30987013d 100755 --- a/.github/workflows/detect-changes.sh +++ b/.github/workflows/detect-changes.sh @@ -9,6 +9,9 @@ # build - anything that ends up in a binary, image or package (i.e. every # change except the doc-only paths in $DOC_ONLY_RE) # +# pipeline.yml and this script are inputs to every suite they gate: a wrong +# gating expression skips a suite green, in every run, with no other signal. +# # Only pull requests are narrowed. Master pushes, tags and manual runs always get # every flag, so a release can never be built from a partially validated tree. # @@ -20,9 +23,9 @@ set -uo pipefail export LC_ALL=C -GO_RE='(\.go$|(^|/)go\.(mod|sum)$|^Makefile$|^\.golangci\.yml$|^resources/|^db/migrations/|^tests/)' -JS_RE='^ui/' -I18N_RE='(^resources/i18n/|^ui/src/i18n/en\.json$|^\.github/workflows/validate-translations\.sh$)' +GO_RE='(\.go$|(^|/)go\.(mod|sum)$|^Makefile$|^\.golangci\.yml$|^resources/|^db/migrations/|^tests/|^\.github/workflows/(pipeline\.yml|detect-changes\.sh)$)' +JS_RE='(^ui/|^\.github/workflows/(pipeline\.yml|detect-changes\.sh)$)' +I18N_RE='(^resources/i18n/|^ui/src/i18n/en\.json$|^\.github/workflows/validate-translations\.sh$|^\.github/workflows/(pipeline\.yml|detect-changes\.sh)$)' DOC_ONLY_RE='(\.md$|^LICENSE$|^\.git-blame-ignore-revs$|^\.gitignore$|^\.devcontainer/)' emit() { printf '%s=%s\n' "$1" "$2" | tee -a "${GITHUB_OUTPUT:-/dev/null}"; } @@ -50,14 +53,14 @@ printf '%s\n' "$files" | sed 's/^/ /' echo flag() { # $1=name $2=regex - if printf '%s\n' "$files" | grep -qE "$2"; then emit "$1" true; else emit "$1" false; fi + if grep -qE "$2" <<< "$files"; then emit "$1" true; else emit "$1" false; fi } flag go "$GO_RE" flag js "$JS_RE" flag i18n "$I18N_RE" -if printf '%s\n' "$files" | grep -vE "$DOC_ONLY_RE" | grep -q '[^[:space:]]'; then +if [ -n "$(grep -vE "$DOC_ONLY_RE" <<< "$files")" ]; then emit build true else emit build false diff --git a/.github/workflows/download-link-on-pr.yml b/.github/workflows/download-link-on-pr.yml index fdfc82e08..115eb2e34 100644 --- a/.github/workflows/download-link-on-pr.yml +++ b/.github/workflows/download-link-on-pr.yml @@ -35,20 +35,27 @@ jobs: const {data: {artifacts}} = await github.rest.actions.listWorkflowRunArtifacts({owner, repo, run_id}); const downloadable = artifacts.filter((art) => !art.name.startsWith('octocov-')); - // A PR that changes nothing reaching a binary builds nothing, so an - // empty list is expected rather than a problem. + const header = `Download the artifacts for this pull request:`; + + const comments = await github.paginate(github.rest.issues.listComments, {repo, owner, issue_number}); + // Match on the body too: octocov also comments as github-actions[bot]. + const existing_comment = comments.find((c) => c.user.login === 'github-actions[bot]' && c.body.startsWith(header)); + + // A PR that changes nothing reaching a binary builds nothing. Delete + // rather than reword: the matcher above keys off the header. if (!downloadable.length) { + if (existing_comment) { + core.info(`Deleting stale comment ${existing_comment.id}`); + await github.rest.issues.deleteComment({repo, owner, comment_id: existing_comment.id}); + } return core.info(`No artifacts found`); } - const header = `Download the artifacts for this pull request:`; + let body = `${header}\n`; for (const art of downloadable) { body += `\n* [${art.name}.zip](https://nightly.link/${owner}/${repo}/actions/artifacts/${art.id}.zip)`; } - const {data: comments} = await github.rest.issues.listComments({repo, owner, issue_number}); - // Match on the body too: octocov also comments as github-actions[bot]. - const existing_comment = comments.find((c) => c.user.login === 'github-actions[bot]' && c.body.startsWith(header)); if (existing_comment) { core.info(`Updating comment ${existing_comment.id}`); await github.rest.issues.updateComment({repo, owner, comment_id: existing_comment.id, body});