From 6647fe26b877d07e5d64548e9127c36c9041b969 Mon Sep 17 00:00:00 2001 From: Derek Corniello Date: Fri, 31 Jul 2026 12:24:23 -0700 Subject: [PATCH 1/4] ci: make the PR comment sticky and cancel superseded runs - issue-triage: drop the unconditional `needs triage` application. Every issue template already sets the label in its frontmatter, so this was pure duplication. ensureLabel stays deliberately: GitHub silently drops a template label that does not exist in the repo, so that call is what creates it. - add a concurrency group so a newer push supersedes an in-flight PR run. A push to main is never cancelled - that run is the authoritative record for the commit. - pr-comment: update our own report comment in place instead of posting a new one on every push, matching on a hidden marker AND github-actions[bot] authorship so a comment anyone can forge is never overwritten. Colour the valgrind pie green/red rather than mermaid's default blue/purple, and say so in the comment when the log cannot be parsed instead of dropping the chart. --- .github/workflows/ci.yml | 6 ++++++ .github/workflows/issue-triage.yml | 5 ++++- .github/workflows/pr-comment.yml | 32 ++++++++++++++++++++++++++++-- 3 files changed, 40 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0b65d9a..17a6cee 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -6,6 +6,12 @@ on: pull_request: types: [opened, synchronize, reopened] +concurrency: + # A newer push supersedes an in-flight PR run. A push to main is never + # cancelled: its run is the authoritative record for that commit. + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + jobs: detect-changes: name: Detect Changed Paths diff --git a/.github/workflows/issue-triage.yml b/.github/workflows/issue-triage.yml index 24ebbca..edfdf20 100644 --- a/.github/workflows/issue-triage.yml +++ b/.github/workflows/issue-triage.yml @@ -49,8 +49,11 @@ jobs: }); } }; + // Every issue template already sets `needs triage` in its frontmatter, + // so applying it here was pure duplication. ensureLabel must stay: + // GitHub silently drops a template label that does not exist in the + // repo, so this call is what creates it. await ensureLabel(labelName, 'ededed', 'Not yet reviewed or categorized'); - await applyLabelIfMissing(labelName); if (!context.payload.issue.assignee) { await github.rest.issues.addAssignees({ owner: context.repo.owner, diff --git a/.github/workflows/pr-comment.yml b/.github/workflows/pr-comment.yml index 64a237d..29958eb 100644 --- a/.github/workflows/pr-comment.yml +++ b/.github/workflows/pr-comment.yml @@ -185,13 +185,24 @@ jobs: # The valgrind job prints ">>> valgrind " per test binary and # "::error::Valgrind reported ..." on a failing one. Count those markers # (integers only) - no fork string reaches the rendered body. + # A log with no ">>> valgrind" markers means the job changed its output + # format: say so in the comment. Dropping the chart silently is how a + # broken report goes unnoticed. The note is a trusted literal - no + # fork-controlled text reaches the body through this path. emit_valgrind_pie() { local logf="$1" total fails clean total="$(grep -Ec '^>>> valgrind ' "$logf" || true)" fails="$(grep -Ec '^::error::Valgrind reported' "$logf" || true)" - { [[ "$total" =~ ^[0-9]+$ ]] && [[ "$fails" =~ ^[0-9]+$ ]] && [ "$total" -gt 0 ] && [ "$fails" -le "$total" ]; } || return 0 + if ! { [[ "$total" =~ ^[0-9]+$ ]] && [[ "$fails" =~ ^[0-9]+$ ]] && [ "$total" -gt 0 ] && [ "$fails" -le "$total" ]; }; then + printf '_Chart unavailable: no ">>> valgrind" markers found in the log._\n\n' + return 0 + fi clean=$(( total - fails )) printf '```mermaid\n' + # Mermaid's default pie palette is dark blue/purple, which reads as + # arbitrary. Force clean=green, leaking=red: pie1 colors the first + # slice below, pie2 the second. + printf '%s\n' "%%{init: {'themeVariables': {'pie1': '#2da44e', 'pie2': '#cf222e'}}}%%" printf 'pie showData\n' printf ' title Test binaries under Valgrind (%s total)\n' "$total" printf ' "Clean" : %s\n' "$clean" @@ -199,6 +210,10 @@ jobs: printf '```\n\n' } + # Identifies our own report comment so each push updates it in place + # instead of appending one per push. Invisible in rendered markdown. + MARKER="" + : > body.md wrote=0 for spec in "${specs[@]}"; do @@ -248,4 +263,17 @@ jobs: exit 0 fi printf '_Commit `%s` - [full run](%s)_\n' "$HEAD_SHA" "$RUN_URL" >> body.md - gh pr comment "$PR" --repo "$REPO" --body-file body.md + printf '\n%s\n' "$MARKER" >> body.md + + # Update our own previous comment instead of stacking one per push. The + # author filter is load-bearing: anyone can post a comment containing + # the marker, and only a github-actions[bot] comment is ours to edit. + existing="$(gh api --paginate --slurp "repos/$REPO/issues/$PR/comments?per_page=100" \ + | jq -r --arg m "$MARKER" \ + '[.[][] | select(.user.login == "github-actions[bot]" and (.body | contains($m)))] | .[0].id // empty')" + if [ -n "$existing" ]; then + gh api -X PATCH "repos/$REPO/issues/comments/$existing" -F body=@body.md --silent + echo "Updated report comment $existing on PR #$PR." + else + gh pr comment "$PR" --repo "$REPO" --body-file body.md + fi From a117e4188bd33cf20ee5875cef7ef4a185e8a4bb Mon Sep 17 00:00:00 2001 From: Derek Corniello Date: Fri, 31 Jul 2026 12:41:22 -0700 Subject: [PATCH 2/4] ci: label the issue whose template label was dropped Review catch. Dropping the unconditional apply left one real gap: when `needs triage` does not exist yet, GitHub silently drops it from the opening issue's template frontmatter, and `ensureLabel` creating the label afterwards does not retroactively label the issue that triggered the run. That issue would sit outside triage until labelled by hand. Restoring the unconditional apply would undo the point of the change, so apply it only on the run that actually created the label - the exact case where the template's request was already dropped. `ensureLabel` now reports whether it created the label; every later issue gets the label from its own template. --- .github/workflows/issue-triage.yml | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/.github/workflows/issue-triage.yml b/.github/workflows/issue-triage.yml index edfdf20..d6f55be 100644 --- a/.github/workflows/issue-triage.yml +++ b/.github/workflows/issue-triage.yml @@ -15,6 +15,9 @@ jobs: with: script: | const labelName = 'needs triage'; + // Returns true only when this run actually created the label, which + // is the case where GitHub already dropped it from the opening + // issue's template frontmatter. const ensureLabel = async (name, color, description) => { try { await github.rest.issues.getLabel({ @@ -22,6 +25,7 @@ jobs: repo: context.repo.repo, name, }); + return false; } catch { try { await github.rest.issues.createLabel({ @@ -31,10 +35,12 @@ jobs: color, description, }); + return true; } catch (createError) { if (createError.status !== 422) { throw createError; } + return false; } } }; @@ -50,10 +56,18 @@ jobs: } }; // Every issue template already sets `needs triage` in its frontmatter, - // so applying it here was pure duplication. ensureLabel must stay: - // GitHub silently drops a template label that does not exist in the - // repo, so this call is what creates it. - await ensureLabel(labelName, 'ededed', 'Not yet reviewed or categorized'); + // so applying it here unconditionally was pure duplication. ensureLabel + // must stay: GitHub silently drops a template label that does not exist + // in the repo, so this call is what creates it. + // + // The one case that still needs an explicit apply is the run that + // creates the label: the template's request was already dropped, and + // creating the label now does not retroactively label the issue that + // triggered this run. Every later issue gets it from its template. + const createdTriageLabel = await ensureLabel(labelName, 'ededed', 'Not yet reviewed or categorized'); + if (createdTriageLabel) { + await applyLabelIfMissing(labelName); + } if (!context.payload.issue.assignee) { await github.rest.issues.addAssignees({ owner: context.repo.owner, From 3bdee4c9f45289a98b76c740a51d8a0fea55649b Mon Sep 17 00:00:00 2001 From: Derek Corniello Date: Fri, 31 Jul 2026 12:53:52 -0700 Subject: [PATCH 3/4] ci: close the label creation race and stop evicting pending main runs Two review catches, both real. Label creation race: when two issues open concurrently before `needs triage` exists, both runs see it missing, one creates it and the other gets 422. The losing run returned false and skipped the recovery apply - but its issue also had the template label dropped, so it stayed outside triage. The meaningful signal is "the label was absent when this run looked", not "this run won the create race", so ensureLabel now reports that instead. Pending-run eviction: a shared concurrency group keeps only ONE pending run and cancels any earlier one, and that happens regardless of cancel-in-progress - it applies to queued runs, not in-flight ones. Scoping cancel-in-progress to pull requests therefore did not protect main: three rapid pushes would evict the middle commit's queued run before it ever executed. Push runs now get a unique group keyed on run_id, so they never share one and never displace each other; PR runs still supersede by PR number. --- .github/workflows/ci.yml | 10 ++++++---- .github/workflows/issue-triage.yml | 13 ++++++++----- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 17a6cee..6f480c1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -7,10 +7,12 @@ on: types: [opened, synchronize, reopened] concurrency: - # A newer push supersedes an in-flight PR run. A push to main is never - # cancelled: its run is the authoritative record for that commit. - group: ${{ github.workflow }}-${{ github.ref }} - cancel-in-progress: ${{ github.event_name == 'pull_request' }} + # Supersede an in-flight run for the SAME PR. Push runs deliberately get a + # unique group (run_id): a shared group keeps only one pending run, so rapid + # pushes to main would evict an intermediate commit's queued run before it + # ever ran, leaving that commit unverified. + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.run_id }} + cancel-in-progress: true jobs: detect-changes: diff --git a/.github/workflows/issue-triage.yml b/.github/workflows/issue-triage.yml index d6f55be..55f3dbf 100644 --- a/.github/workflows/issue-triage.yml +++ b/.github/workflows/issue-triage.yml @@ -15,9 +15,10 @@ jobs: with: script: | const labelName = 'needs triage'; - // Returns true only when this run actually created the label, which - // is the case where GitHub already dropped it from the opening - // issue's template frontmatter. + // Returns true when the label was ABSENT at the moment this run + // looked - whether this run created it or lost the race to a + // concurrent one. That is exactly the condition under which GitHub + // already dropped it from the opening issue's template frontmatter. const ensureLabel = async (name, color, description) => { try { await github.rest.issues.getLabel({ @@ -35,13 +36,15 @@ jobs: color, description, }); - return true; } catch (createError) { if (createError.status !== 422) { throw createError; } - return false; + // 422 means a concurrent run created it first. The label was + // still missing when THIS run looked, so this run's issue had + // its template label dropped too and needs the same recovery. } + return true; } }; const applyLabelIfMissing = async (name) => { From 187a46f7b71c44edcfd5c6f3bf5d6e8a128498eb Mon Sep 17 00:00:00 2001 From: Derek Corniello Date: Fri, 31 Jul 2026 13:05:51 -0700 Subject: [PATCH 4/4] ci: serialize report runs so the sticky comment cannot double-post Review catch. The sticky-comment update is a lookup-then-create, and this workflow had no concurrency control: two Build runs completing close together for the same PR could both find no marker and both post, producing exactly the duplicate comments the marker exists to prevent. GitHub has no atomic comment upsert, so serializing per PR is what makes the pair safe. The group is keyed on the trusted workflow_run head repository and branch, since the PR number is not resolved until a step inside the job. cancel-in-progress stays false so an in-flight post is never killed midway. A queued run can still be superseded, which is the behaviour we want here: the newest report is the one worth posting. --- .github/workflows/pr-comment.yml | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/.github/workflows/pr-comment.yml b/.github/workflows/pr-comment.yml index 29958eb..b8a8362 100644 --- a/.github/workflows/pr-comment.yml +++ b/.github/workflows/pr-comment.yml @@ -22,6 +22,20 @@ on: workflows: ["Build"] types: [completed] +# Serialize report runs for the same PR branch. The sticky-comment update is a +# lookup-then-create: two Build runs completing close together would both find +# no marker and both post, producing exactly the duplicate comments the marker +# exists to prevent. GitHub offers no atomic comment upsert, so serializing per +# PR is what makes the pair safe. Keyed on the trusted workflow_run head fields +# because the PR number is not resolved until a step below. +# +# cancel-in-progress is false so an in-flight post is never killed midway. A +# queued run can still be superseded by a newer one, which is correct here: the +# newest report is the one worth posting. +concurrency: + group: ${{ github.workflow }}-${{ github.event.workflow_run.head_repository.full_name }}-${{ github.event.workflow_run.head_branch }} + cancel-in-progress: false + permissions: contents: read actions: read # download artifacts + read job conclusions from the run