ci: make the PR comment sticky and cancel superseded runs - #30
Merged
Conversation
- 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.
Contributor
|
| Filename | Overview |
|---|---|
| .github/workflows/ci.yml | Adds pull-request-scoped concurrency while assigning unique groups to push runs. |
| .github/workflows/issue-triage.yml | Tracks label creation and reapplies needs triage to the triggering issue when GitHub previously dropped that template label. |
| .github/workflows/pr-comment.yml | Serializes same-branch report runs, upserts a bot-owned marker comment, and emits an explicit unavailable-chart note for malformed valgrind logs. |
Reviews (4): Last reviewed commit: "ci: serialize report runs so the sticky ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Valgrind Memory Checks - PASSEDpie showData
title Test binaries under Valgrind (28 total)
"Clean" : 28
"Leaking" : 0
Full valgrind output (last 300 lines)Benchmarks (report-only) - report-onlyInformational only - shared runners are too noisy to gate on; full data is in the run artifacts. xychart-beta
title "Hot paths under 1 us (median)"
x-axis ["refcount", "primitive", "map", "string", "wrappers"]
y-axis "nanoseconds"
bar [4.64, 3.58, 78.13, 48.31, 34.89]
xychart-beta
title "Hot paths 1 us and up (median)"
x-axis ["list", "set", "json"]
y-axis "microseconds"
bar [7.832, 19.4, 3.669]
Full benchmark output (last 300 lines)Commit |
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.
Valgrind Memory Checks - PASSEDpie showData
title Test binaries under Valgrind (28 total)
"Clean" : 28
"Leaking" : 0
Full valgrind output (last 300 lines)Benchmarks (report-only) - report-onlyInformational only - shared runners are too noisy to gate on; full data is in the run artifacts. xychart-beta
title "Hot paths under 1 us (median)"
x-axis ["refcount", "primitive", "map", "string", "wrappers"]
y-axis "nanoseconds"
bar [5.203, 3.78, 75.87, 50.55, 35.52]
xychart-beta
title "Hot paths 1 us and up (median)"
x-axis ["list", "set", "json"]
y-axis "microseconds"
bar [4.945, 19.09, 4.003]
Full benchmark output (last 300 lines)Commit |
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.
Valgrind Memory Checks - PASSEDpie showData
title Test binaries under Valgrind (28 total)
"Clean" : 28
"Leaking" : 0
Full valgrind output (last 300 lines)Benchmarks (report-only) - report-onlyInformational only - shared runners are too noisy to gate on; full data is in the run artifacts. xychart-beta
title "Hot paths under 1 us (median)"
x-axis ["refcount", "primitive", "map", "string", "wrappers"]
y-axis "nanoseconds"
bar [4.625, 3.891, 78.55, 48.41, 34.03]
xychart-beta
title "Hot paths 1 us and up (median)"
x-axis ["list", "set", "json"]
y-axis "microseconds"
bar [8.106, 19.2, 3.745]
Full benchmark output (last 300 lines)Commit |
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.
|
Valgrind Memory Checks - PASSEDpie showData
title Test binaries under Valgrind (28 total)
"Clean" : 28
"Leaking" : 0
Full valgrind output (last 300 lines)Benchmarks (report-only) - report-onlyInformational only - shared runners are too noisy to gate on; full data is in the run artifacts. xychart-beta
title "Hot paths under 1 us (median)"
x-axis ["refcount", "primitive", "map", "string", "wrappers"]
y-axis "nanoseconds"
bar [4.644, 3.582, 78.58, 48.24, 34.77]
xychart-beta
title "Hot paths 1 us and up (median)"
x-axis ["list", "set", "json"]
y-axis "microseconds"
bar [8.008, 19.23, 3.682]
Full benchmark output (last 300 lines)Commit |
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
1 participant
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.
Part of the CI/CD rework (step 2 of the plan). Mechanical and independent of the
other six repo PRs in this batch - mergeable in any order.
What changes
Stop double-labelling new issues (muxlang/mux-context#19).
issue-triage.ymlapplied
needs triageunconditionally on everyissues: opened, but all 15 issuetemplates org-wide already set the label in their frontmatter, so the workflow
call was pure duplication. The file is byte-identical across 7 repos, so this same
diff lands in each.
ensureLabelis deliberately kept, and there is a comment saying why: GitHubsilently drops a label a template references if it does not exist in the repo, so
that call is what creates it. Removing both would quietly stop templates labelling
anything. Auto-assign and the
documentationheuristic are untouched.Cancel superseded PR runs. No concurrency group existed, so pushing three
commits in a row ran three full pipelines at once.
cancel-in-progressis scoped to pull requests rather than set to a blunttrue:cancelling a push to
mainwould discard the authoritative run for that commit.Verification
All workflow files in the repo still parse as valid YAML and remain ASCII-only.
Make the PR report comment sticky.
pr-comment.ymlended ingh pr comment,which creates a new comment on every push - muxlang/mux-compiler#317 has two
report comments from two pushes, and a 20-push PR would have twenty. It now
embeds a hidden
<!-- mux-ci-report -->marker and updates its own comment inplace.
The lookup filters on the marker and
github-actions[bot]authorship. Thatsecond condition is load-bearing: anyone can post a comment containing the
marker, and only a bot-authored comment is ours to overwrite. Verified the
selector against real API data on muxlang/mux-compiler#317 and #318.
Colour the valgrind pie chart. It emitted bare
pie showData, so mermaidused its default blue/purple palette (muxlang/mux-context#27). Now green for
clean, red for leaking, via a
themeVariablesinit directive.Stop the chart vanishing silently.
emit_valgrind_piereturned earlywhenever the log did not match its expected shape, so a format change would drop
the chart with no indication. It now says so in the comment. Both notes are
trusted literals, so the injection model in the file header is preserved.
Note: this workflow is
workflow_run-triggered, so it only takes effect from thecopy on
main- this PR cannot demonstrate its own change. Worth a look on thenext PR after merge. Existing comments have no marker, so the first run after
merge creates one fresh comment and updates that one thereafter.
🤖 Generated with Claude Code