Skip to content

test(dev-lead): assert per-issue/per-PR concurrency lanes (align with #450, unblocked by #449) #451

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
don-petry merged 6 commits into from
Jun 7, 2026
Merged

test(dev-lead): assert per-issue/per-PR concurrency lanes (align with #450, unblocked by #449) #451

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
65c38fd
test(dev-lead): assert per-issue/per-PR concurrency lanes, not per-re…
claude Jun 7, 2026
b71b74d
fix(reviews): address review comments [skip ci-relay]
donpetry-bot Jun 7, 2026
45a128f
ci(dev-lead): run concurrency-config on dev-lead workflow changes pus…
claude Jun 7, 2026
cdbe75e
Merge branch 'main' into fix/concurrency-test-per-lane
don-petry Jun 7, 2026
d59f973
fix(reviews): address review comments [skip ci-relay]
donpetry-bot Jun 7, 2026
8712c4c
fix(reviews): address review comments [skip ci-relay]
donpetry-bot Jun 7, 2026
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
7 changes: 6 additions & 1 deletion .github/workflows/test-dev-lead.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ on:
- 'scripts/lib/**'
- 'tests/dev-lead/**'
- 'prompts/dev-lead/**'
# Guard the concurrency-config test on workflow changes too — otherwise a
# dev-lead.yml edit can land on main without ever running this suite
# (how the per-repo->per-lane mismatch stayed hidden after #450).
- '.github/workflows/dev-lead*.yml'
- '.github/workflows/test-dev-lead.yml'
workflow_dispatch: {}

permissions:
Expand Down Expand Up @@ -73,5 +78,5 @@ jobs:
contents: read
steps:
- uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
- name: Validate concurrency config (per-repo serialization)
- name: Validate concurrency config (per-issue/per-PR lanes)
run: bash tests/dev-lead/integration/test_concurrency_config.sh
100 changes: 88 additions & 12 deletions tests/dev-lead/integration/test_concurrency_config.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,18 @@
#!/usr/bin/env bash
# Validates that dev-lead workflow files use per-repo serialized concurrency
# (a single 'dev-lead' group) rather than per-PR or per-issue groups.
# Validates that dev-lead workflow files use per-issue / per-PR concurrency lanes
# (dev-lead-issue-<n>, dev-lead-pr-<n>) plus a per-SHA ci-relay lane, rather than a
# single repo-wide 'dev-lead' group.
#
# History: #278 collapsed everything into one per-repo 'dev-lead' group so at most
# one dispatch ran at a time — needed only because the agent checked out PR branches
# IN PLACE, so concurrent runs corrupted each other's working tree. That serialization
# then starved issue pickups: PR follow-up traffic (reviews/comments/ci-relay) held the
# single slot and queued issue-labeled runs were superseded and cancelled before they
# could open a PR (petry-projects/.github#402).
#
# #449 (#448) removed the root hazard by checking out each PR branch in an isolated
# git worktree, so different issues/PRs can now run in parallel safely. #450 restored
# per-issue / per-PR lanes. This test guards that design against regression.
set -euo pipefail

FAIL=0
Expand All @@ -25,20 +37,84 @@ check_present() {
fi
}

WORKFLOW=".github/workflows/dev-lead.yml"
check_order() {
local file="$1" first="$2" second="$3" label="$4"
local line_first line_second
line_first=$(grep -nF "$first" "$file" | head -1 | cut -d: -f1)
line_second=$(grep -nF "$second" "$file" | head -1 | cut -d: -f1)
if [ -z "$line_first" ] || [ -z "$line_second" ]; then
echo "FAIL [$label]: could not find both patterns in $file"
FAIL=$((FAIL + 1))
elif [ "$line_first" -lt "$line_second" ]; then
echo "PASS [$label]: correct ordering in $file"
else
echo "FAIL [$label]: '$first' must appear before '$second' in $file (first-match-wins routing)"
FAIL=$((FAIL + 1))
fi
}

# Both the inline workflow and the reusable (source of truth for all consumer
# stubs) must carry the per-lane routing.
for WORKFLOW in \
".github/workflows/dev-lead.yml" \
".github/workflows/dev-lead-reusable.yml"; do
echo "── $WORKFLOW ──"
Comment thread
don-petry marked this conversation as resolved.
if [ ! -f "$WORKFLOW" ]; then
echo "FAIL: $WORKFLOW does not exist. Please run this script from the repository root."
FAIL=$((FAIL + 1))
continue
fi

# Per-issue and per-PR lanes must be present so a labeled-issue pickup is never
# cancelled by unrelated PR follow-up traffic.
# Match format('dev-lead-…') to target the concurrency expression, not comments.
check_present "$WORKFLOW" "format('dev-lead-pr-" "per-PR lane"

@chatgpt-codex-connector chatgpt-codex-connector Bot Jun 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Assert direct pull_request routing

For pull_request, pull_request_review, and pull_request_review_comment events, the PR lane depends on the github.event.pull_request.number && format('dev-lead-pr-...') branch, but this check can be satisfied by the issue-comment or repository-dispatch PR branches alone. If the direct pull_request.number predicate is removed, those PR/review events fall through to the run-id fallback while this test still passes; please assert that predicate specifically, as the new issue-comment and dispatch assertions do.

Useful? React with 👍 / 👎.

check_present "$WORKFLOW" "format('dev-lead-issue-" "per-issue lane"
Comment thread
don-petry marked this conversation as resolved.

# Must not have per-PR or per-issue group keys (those allowed parallel runs)
check_absent "$WORKFLOW" "dev-lead-pr-" "no per-PR group"
check_absent "$WORKFLOW" "dev-lead-issue-" "no per-issue group"
# ci-relay keeps its own ephemeral per-SHA slot so it fires immediately.
check_present "$WORKFLOW" "format('dev-lead-ci-relay-" "ci-relay per-SHA lane"

@chatgpt-codex-connector chatgpt-codex-connector Bot Jun 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Assert check_run routing to the relay lane

For check_run events, the per-SHA relay lane depends on the github.event_name == 'check_run' && format('dev-lead-ci-relay-...') predicate, but this assertion only requires the lane token to appear somewhere. If the check_run predicate is removed or changed so completed check runs fall through to dev-lead-run-<run_id>, the test still passes while CI-failure relays no longer share the intended per-SHA slot; please assert the full predicate-to-lane binding as the later PR-specific checks do.

Useful? React with 👍 / 👎.


# ci-relay must still get its own ephemeral per-SHA slot
check_present "$WORKFLOW" "dev-lead-ci-relay-" "ci-relay per-SHA group preserved"
# Assert the actual predicate-to-lane bindings, not just token presence.
# issue_comment on a PR has issue.pull_request set; that predicate must route
# to the PR lane so PR follow-up traffic shares the PR slot with other PR events.
check_present "$WORKFLOW" \
"github.event.issue.pull_request && format('dev-lead-pr-" \
"issue_comment-on-PR predicate routes to PR lane"
# repository_dispatch (ci-failure relay) carries client_payload.pr_number.
check_present "$WORKFLOW" \
"github.event.client_payload.pr_number && format('dev-lead-pr-" \
"repository_dispatch routes to PR lane via client_payload.pr_number"
Comment thread
don-petry marked this conversation as resolved.
# client_payload.pr_number predicate must appear BEFORE the final run-id fallback.
# If reordered, the fallback is always truthy and every repository_dispatch
# (ci-failure relay) gets a unique run lane instead of sharing the PR lane.
check_order "$WORKFLOW" \
"github.event.client_payload.pr_number && format('dev-lead-pr-" \
"format('dev-lead-run-{0}', github.run_id)" \
"repository_dispatch predicate before run-id fallback"
# issue.pull_request must appear BEFORE issue.number in the || chain.
# If reordered, an issue_comment on a PR would fall through to the issue lane
# and share a slot with issue pickups, allowing cancellation.
check_order "$WORKFLOW" \
"github.event.issue.pull_request && format('dev-lead-pr-" \
"github.event.issue.number && format('dev-lead-issue-" \
"issue_comment-on-PR predicate before generic issue predicate"

# The fallback group must be the bare repo-scoped key
check_present "$WORKFLOW" "'dev-lead'" "per-repo 'dev-lead' fallback group"
# Must NOT collapse back into a single repo-wide group (the #278 regression).
# Guard all common bare-value spellings so a regression can't slip through
# on quote-style changes.
check_absent "$WORKFLOW" "group: dev-lead" "no bare unquoted group: dev-lead"
check_absent "$WORKFLOW" "group: 'dev-lead'" "no bare single-quoted group: dev-lead"
check_absent "$WORKFLOW" 'group: "dev-lead"' "no bare double-quoted group: dev-lead"
Comment thread
don-petry marked this conversation as resolved.
# Also reject a repo-wide fallback hidden inside the ${{ ... }} expression.
# The group: >- form puts the value on the next lines starting with ||, so
# "group: dev-lead" patterns above would miss a regression like || 'dev-lead'.
check_absent "$WORKFLOW" "|| 'dev-lead'" "no repo-wide || 'dev-lead' fallback in expression"
check_absent "$WORKFLOW" '|| "dev-lead"' 'no repo-wide || "dev-lead" fallback in expression'

# cancel-in-progress must be false so queued runs aren't dropped
check_present "$WORKFLOW" "cancel-in-progress: false" "cancel-in-progress is false"
# cancel-in-progress must be false so a queued same-lane run waits for the active
# run to finish instead of cancelling it (in-flight pickups always complete).
check_present "$WORKFLOW" "cancel-in-progress: false" "cancel-in-progress is false"
done

echo ""
if [ "$FAIL" -eq 0 ]; then
Expand Down
Loading