Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions .github/workflows/pr-review-mention.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,6 @@ on:

permissions: {}

concurrency:
group: >-
pr-review-mention-${{ format('{0}-{1}', github.event_name,
github.event.issue.number || github.event.pull_request.number) }}
cancel-in-progress: false

jobs:
pr-review-mention:
permissions:
Expand Down
3 changes: 2 additions & 1 deletion scripts/tests/dev-lead-workflow.bats
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@ print('ok')
# follow-up traffic. A per-repo concurrency block would drift the stub and
# fight the centralised grouping — re-inflating the failure/cancellation rate.
# This is the one point where dev-lead deliberately diverges from the
# add-to-project / pr-review-mention stubs (which DO carry a concurrency block).
# add-to-project stub (which carries a concurrency block); pr-review-mention
# no longer carries one either after issue #404 re-synced it to the central standard.
run python3 -c "
import sys, yaml
wf = yaml.safe_load(open(sys.argv[1])) or {}
Expand Down
75 changes: 23 additions & 52 deletions scripts/tests/pr-review-mention-workflow.bats
Original file line number Diff line number Diff line change
@@ -1,14 +1,20 @@
#!/usr/bin/env bats
# Tests for .github/workflows/pr-review-mention.yml
# Guards the concurrency fix (issue #333). The #323 attempt used
# `cancel-in-progress: true`, but when it superseded a *pending* reusable-caller
# run (before the reusable resolved) GitHub recorded the cancellation as a
# startup failure — a burst of comment events on one conversation produced a
# wave of 0-job "failure" runs that inflated the Fleet Monitor failure rate.
# The fix mirrors add-to-project.yml (#331): `cancel-in-progress: false` with a
# group keyed on the event name AND the PR/issue number, so superseded runs are
# recorded as clean `cancelled` (not `failure`) and distinct event types on the
# same conversation don't cross-cancel.
#
# This file is a THIN CALLER STUB. Its `on:` triggers, `permissions:` grants and
# `concurrency:` surface are owned centrally by the canonical
# standards/workflows/pr-review-mention.yml and are not repo-adjustable
# (ci-standards.md → centralization tiers). The canonical stub carries NO
# top-level `concurrency:` block, so the stub must not declare one either —
# issue #404 re-synced the surface by removing a local block that had drifted in.
Comment thread
don-petry marked this conversation as resolved.
#
# History: #333 previously added a local `concurrency:` block here (group keyed
# on event name + PR/issue number, `cancel-in-progress: false`) to stop a comment
# burst from producing a wave of 0-job "failure" runs that inflated the Fleet
# Monitor failure rate. That protection is centrally owned now: if it is still
# wanted it belongs in the org reusable (pr-review-mention-reusable.yml), never
# re-added to this caller stub. These tests therefore assert the stub carries no
# local concurrency surface.

WORKFLOW=".github/workflows/pr-review-mention.yml"

Expand All @@ -28,52 +34,17 @@ setup() {
[ "$status" -eq 0 ]
}

@test "pr-review-mention workflow declares a concurrency block" {
@test "pr-review-mention stub declares no local concurrency block (centrally owned, #404)" {
# The canonical standards/workflows/pr-review-mention.yml carries no top-level
# concurrency surface, and the concurrency surface of a thin caller stub is not
# repo-adjustable (ci-standards.md → centralization tiers). A local block is
# drift; #404 removed one that had crept in. If comment-burst concurrency
# control is needed it must live in the org reusable, not this stub.
run python3 -c "
import sys, yaml
wf = yaml.safe_load(open(sys.argv[1])) or {}
assert 'concurrency' in wf, 'workflow has no top-level concurrency block'
c = wf.get('concurrency')
assert isinstance(c, dict), 'concurrency must be a mapping with group/cancel-in-progress'
assert c.get('group'), 'concurrency.group must be set'
print('ok')
" "$WORKFLOW"
[ "$status" -eq 0 ]
[[ "$output" == "ok" ]]
}

@test "pr-review-mention concurrency does not cancel in-progress runs" {
# cancel-in-progress must be false: superseding a pending reusable-caller run
# makes GitHub record it as a startup failure, so a comment burst inflates the
# failure rate (#333). With false, superseded runs are recorded as clean
# `cancelled`. Mirrors add-to-project.yml (#331).
run python3 -c "
import sys, yaml
wf = yaml.safe_load(open(sys.argv[1])) or {}
c = wf.get('concurrency') or {}
val = c.get('cancel-in-progress')
assert val is False, f'cancel-in-progress must be false, got: {val!r}'
print('ok')
" "$WORKFLOW"
[ "$status" -eq 0 ]
[[ "$output" == "ok" ]]
}

@test "pr-review-mention concurrency group is keyed per event and conversation" {
# The workflow fires on issue_comment, pull_request_review_comment and
# pull_request events, whose github.ref is the default branch for comment
# events. Keying only on github.ref would collapse runs across unrelated PRs,
# so the group keys on the PR/issue number. It also keys on github.event_name
# so distinct event types on the same conversation get distinct groups and do
# not cross-cancel (mirrors add-to-project.yml, #333/#331).
run python3 -c "
import sys, yaml
wf = yaml.safe_load(open(sys.argv[1])) or {}
c = wf.get('concurrency') or {}
group = c.get('group', '')
assert 'github.event_name' in group, f'group must key on the event name, got: {group!r}'
assert 'github.event.issue.number' in group, f'group must key on issue number, got: {group!r}'
assert 'github.event.pull_request.number' in group, f'group must key on PR number, got: {group!r}'
concurrency = wf.get('concurrency')
assert 'concurrency' not in wf, f'stub must not declare a local concurrency block (centrally owned), got: {concurrency!r}'
print('ok')
" "$WORKFLOW"
Comment thread
don-petry marked this conversation as resolved.
[ "$status" -eq 0 ]
Expand Down
Loading