Hacktoberfest 2026: the issues maintainers tagged for October, open and beginner-friendly. Browse Hacktoberfest issues

[quality] automation workflows force-push a fixed PR branch with no concurrency group

Closed
#1,222 2 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

A pull request for this has already been merged.

  • #1310 by @mrbobbytables — merged

Assessment

Difficulty
4/5
Estimated time
3-5 days
Newbie friendliness
50/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
github-actions, javascript
Domain
ci-cd, devops

Research direction

Start with the four workflows named in the issue (refresh-radar-reports.yml, refresh-community-people.yml, import-architectures.yml, architecture-submission.yml) and the header of tests/ci-concurrency.test.mjs, which explains its PR-only scope. Adding a concurrency block with cancel-in-progress: false to each workflow is the small half. The contract half is harder: it means extending unexplainedConstantGroups and writing new scanners plus fixtures. Done when the four workflows declare groups and the test suite fails on a fixture that pushes a branch without one.

Written by the indexing model from the issue text.

Description

agent/quality hive/hosted-available-lke648397-260827-5n31 quality testing

Finding

Four workflows open a pull request with peter-evans/create-pull-request, naming the
branch themselves, and none of them declares a concurrency group:

workflow branch: triggers
refresh-radar-reports.yml automation/refresh-radar-reports schedule (57 2 * * *) + workflow_dispatch
refresh-community-people.yml automation/refresh-community-people schedule (37 2 * * 1) + workflow_dispatch
import-architectures.yml automation/import-architectures workflow_dispatch
architecture-submission.yml automation/architecture-issue-${{ github.event.issue.number }} issues: [labeled]

The branch name is a shared resource. From the action's own source (not assumed —
src/create-pull-request.ts), it pushes

git push --force-with-lease <remote> <branch>:refs/heads/<branch>

and, because all four pass delete-branch: true, once the branch stops differing from
the base it runs

git push --delete --force <remote> refs/heads/<branch>

So two overlapping runs of the same workflow write one ref. Two concrete outcomes:

  1. The loser's --force-with-lease fails on stale info — the run reports a hard
    failure
    instead of a refresh. (Worth being precise: --force-with-lease means this
    is a noisy failure, not a silent lost update.)
  2. One run finds no diff with base and --delete --forces the ref that the other run
    has just opened a pull request from.

Overlap is not hypothetical. import-architectures.yml is workflow_dispatch-only, so
pressing Run workflow twice is enough; the two refresh workflows are
schedule + workflow_dispatch with 15-minute job timeouts, so a maintainer dispatching
during the nightly cron run overlaps it. architecture-submission.yml races itself when
the architecture-ready label is removed and re-applied, or applied by two maintainers
at once.

Why no existing contract catches this

tests/ci-concurrency.test.mjs is deliberately scoped to PR-triggered workflows and says
so in its own header:

A new PR-triggered workflow can be added today with no concurrency block at all and the
suite stays green.

tests/ci-supply-chain.test.mjs covers pinning, permissions, runners and timeouts;
tests/automation-workflow-ordering.test.mjs covers step ordering in the generator
workflows. None of them asks whether a workflow that pushes a branch serialises itself
on it. The action's own README and docs/concepts-guidelines.md do not mention
concurrency either, so this is argued from the push mechanics above rather than from
vendor guidance.

Recommendation

One deliverable, two halves that must land in the same commit (see Who can land
this
). The group is keyed exactly as coarsely as the branch: constant branch → constant
group; per-issue branch → per-issue group, so two different submissions still proceed in
parallel. cancel-in-progress: false throughout — cancelling a run that has already
pushed its branch is the failure mode, not the cure.

Half 1 — the four workflows
diff --git a/.github/workflows/architecture-submission.yml b/.github/workflows/architecture-submission.yml
index 8f2ef17..bce93da 100644
--- a/.github/workflows/architecture-submission.yml
+++ b/.github/workflows/architecture-submission.yml
@@ -14,6 +14,15 @@ permissions:
   pull-requests: write
   issues: write
 
+# Serialised per issue, matching the per-issue branch this workflow pushes:
+# removing and re-applying the label, or two maintainers applying it at once,
+# would otherwise have two runs writing the same
+# refs/heads/automation/architecture-issue-<n>. Keyed per issue rather than
+# repository-wide so two different submissions still proceed in parallel.
+concurrency:
+  group: automation/architecture-issue-${{ github.event.issue.number }}
+  cancel-in-progress: false
+
 env:
   NODE_VERSION: '22'
   SUBMISSION_LABEL: architecture-ready
diff --git a/.github/workflows/import-architectures.yml b/.github/workflows/import-architectures.yml
index ae90771..41859ed 100644
--- a/.github/workflows/import-architectures.yml
+++ b/.github/workflows/import-architectures.yml
@@ -10,6 +10,13 @@ permissions:
   contents: write
   pull-requests: write
 
+# Serialised on the branch this workflow force-pushes; see the note in
+# refresh-radar-reports.yml. workflow_dispatch only, so two runs overlap as
+# soon as the button is pressed twice. Not cancelled, for the same reason.
+concurrency:
+  group: automation/import-architectures
+  cancel-in-progress: false
+
 env:
   NODE_VERSION: '22'
 
diff --git a/.github/workflows/refresh-community-people.yml b/.github/workflows/refresh-community-people.yml
index cbdb3fb..52e0fa4 100644
--- a/.github/workflows/refresh-community-people.yml
+++ b/.github/workflows/refresh-community-people.yml
@@ -9,6 +9,12 @@ permissions:
   contents: write
   pull-requests: write
 
+# Serialised on the branch this workflow force-pushes; see the note in
+# refresh-radar-reports.yml. Not cancelled, for the same reason.
+concurrency:
+  group: automation/refresh-community-people
+  cancel-in-progress: false
+
 env:
   NODE_VERSION: '22'
 
diff --git a/.github/workflows/refresh-radar-reports.yml b/.github/workflows/refresh-radar-reports.yml
index ea8e177..3c301ec 100644
--- a/.github/workflows/refresh-radar-reports.yml
+++ b/.github/workflows/refresh-radar-reports.yml
@@ -9,6 +9,17 @@ permissions:
   contents: write
   pull-requests: write
 
+# Serialised on the branch this workflow force-pushes. Two overlapping runs
+# (a dispatch during the nightly cron, or a cron that laps a slow run) both
+# write refs/heads/automation/refresh-radar-reports, and the loser's
+# --force-with-lease fails on stale info. Not cancelled: the run in flight
+# may already have pushed the branch, and `delete-branch: true` means a
+# half-finished run can delete the ref the other one's pull request is open
+# from.
+concurrency:
+  group: automation/refresh-radar-reports
+  cancel-in-progress: false
+
 env:
   NODE_VERSION: '22'
 

Three of these then need a SERIALISED_WORKFLOWS entry in
tests/ci-concurrency.test.mjs, because unexplainedConstantGroups already requires a
constant group to be argued for. architecture-submission.yml does not: its group varies
with the issue.

Half 2 — the contract, so the gap cannot reopen

Adds pullRequestBranches (find the action's branch: input), expressionsIn (compare
keys on what they vary with, not their text), isPerEvent (a per-issue key is not a
repository-wide lane) and racingBranchWorkflows, plus the three
SERIALISED_WORKFLOWS entries and 6 tests including fixtures for every failure arm.

The rule it enforces: a workflow's concurrency group must be no finer than the branch
it pushes.
A coarser group is accepted — it queues more than it has to, but the ref is
still held by one run at a time. A group varying with something the branch does not puts
two runs that write the same ref into different lanes, which is the bug.

diff --git a/tests/ci-concurrency.test.mjs b/tests/ci-concurrency.test.mjs
index 40c917b..7014f48 100644
--- a/tests/ci-concurrency.test.mjs
+++ b/tests/ci-concurrency.test.mjs
@@ -70,6 +70,24 @@ function isPullRequestTriggered(doc) {
 // Workflows that declare a concurrency group but must NOT cancel the run in
 // flight, with the reason. Keyed by file name.
 const SERIALISED_WORKFLOWS = new Map([
+  [
+    'import-architectures.yml',
+    'Force-pushes the constant branch automation/import-architectures and ' +
+      'deletes that ref when it stops differing from the base, so the branch ' +
+      'name is the resource two runs contend for and the group has to be as ' +
+      'constant as the branch is. Not PR-triggered, so it is outside the ' +
+      'cancel-in-progress contract below.',
+  ],
+  [
+    'refresh-community-people.yml',
+    'Force-pushes the constant branch automation/refresh-community-people; ' +
+      'same reasoning as import-architectures.yml.',
+  ],
+  [
+    'refresh-radar-reports.yml',
+    'Force-pushes the constant branch automation/refresh-radar-reports; ' +
+      'same reasoning as import-architectures.yml.',
+  ],
   [
     'deploy-gh-pages.yml',
     'Publishes the live site. Cancelling a deploy halfway can leave GitHub ' +
@@ -112,6 +130,45 @@ function isPerPullRequest(expression) {
   return PER_PR_CONTEXTS.some((context) => expression.includes(context));
 }
 
+// `unexplainedConstantGroups` below asks a different question from the per-PR
+// tests: not "does this key vary per pull request" but "does this key vary
+// with the event at all". A workflow triggered by `issues` that keys its group
+// on the issue number is not a repository-wide lane, so it is not what that
+// scan exists to catch, even though it is not per-PR either.
+const EVENT_KEYED_CONTEXTS = [
+  ...PER_PR_CONTEXTS,
+  'github.event.issue.number',
+  'github.event.inputs',
+  'inputs.',
+];
+
+function isPerEvent(expression) {
+  if (typeof expression !== 'string') return false;
+  if (!expression.includes('${{')) return false;
+  return EVENT_KEYED_CONTEXTS.some((context) => expression.includes(context));
+}
+
+// Steps that open or update a pull request by pushing a branch the workflow
+// names itself. The action force-pushes `branch:` to `refs/heads/<branch>` and,
+// with `delete-branch: true`, deletes that same ref once it stops differing
+// from the base, so the branch name is the shared resource two runs contend
+// for.
+const PR_BRANCH_ACTION = 'peter-evans/create-pull-request';
+
+function pullRequestBranches(doc) {
+  const jobs = doc?.jobs;
+  if (typeof jobs !== 'object' || jobs === null) return [];
+  return Object.values(jobs)
+    .flatMap((job) => (Array.isArray(job?.steps) ? job.steps : []))
+    .filter(
+      (step) =>
+        typeof step?.uses === 'string' && step.uses.includes(PR_BRANCH_ACTION),
+    )
+    .map((step) => step?.with?.branch)
+    .filter((branch) => typeof branch === 'string')
+    .map((branch) => branch.replace(/\s+/g, ' ').trim());
+}
+
 // The scanners below take the parsed workflows as an argument rather than
 // closing over the repository's own, so each failure path can be exercised
 // against a fixture. A guard whose failure branch never runs is a guard nobody
@@ -151,11 +208,64 @@ function unexplainedConstantGroups(parsedWorkflows, allowlist) {
   return parsedWorkflows
     .filter(({ name }) => !allowlist.has(name))
     .filter(({ doc }) => groupExpression(doc) !== undefined)
-    .filter(({ doc }) => !isPerPullRequest(groupExpression(doc)))
+    .filter(({ doc }) => !isPerEvent(groupExpression(doc)))
     .map(({ name }) => name)
     .sort();
 }
 
+// A workflow that pushes a pull request branch must serialise itself on the
+// same key that branch is named with, because that ref is what two overlapping
+// runs would both write.
+//
+// A constant `branch:` with no concurrency group is the sharp case: both runs
+// push `refs/heads/<branch>`, and the loser's `--force-with-lease` fails on
+// stale info, so the run reports a hard failure rather than a refresh. Worse,
+// `delete-branch: true` makes one run's "no diff with base" cleanup a
+// `push --delete --force` of the ref the other run just opened a pull request
+// from. A group keyed more finely than the branch has the same effect: two
+// runs that write one ref land in different lanes. A coarser group is
+// accepted -- it queues more than it has to, but the ref is still held by one
+// run at a time.
+function racingBranchWorkflows(parsedWorkflows) {
+  return parsedWorkflows
+    .flatMap(({ name, doc }) =>
+      pullRequestBranches(doc).map((branch) => ({
+        name,
+        branch,
+        group: groupExpression(doc),
+      })),
+    )
+    .filter(({ branch, group }) => {
+      if (group === undefined) return true;
+      // The group must be no finer than the branch. A coarser group is merely
+      // heavier than it needs to be -- it queues runs that could have
+      // proceeded in parallel, but the ref is still held by one run at a time.
+      // A group that varies with something the branch does not puts two runs
+      // that write the same ref into different lanes, which is the failure
+      // this contract exists to catch.
+      const branchContexts = expressionsIn(branch);
+      return expressionsIn(group).some(
+        (context) => !branchContexts.includes(context),
+      );
+    })
+    .map(
+      ({ name, branch, group }) => `${name}: ${branch} -> ${group ?? 'none'}`,
+    )
+    .sort();
+}
+
+// The `${{ ... }}` bodies of an expression, whitespace-normalised, so a branch
+// and a group can be compared on what they actually vary with rather than on
+// their surrounding text.
+function expressionsIn(text) {
+  if (typeof text !== 'string') return [];
+  // `s` so the match survives the newline Prettier's fold leaves inside a long
+  // expression; the body is whitespace-normalised below either way.
+  return [...text.matchAll(/\$\{\{(.+?)\}\}/gs)]
+    .map((match) => match[1].replace(/\s+/g, ' ').trim())
+    .sort();
+}
+
 // An entry in SERIALISED_WORKFLOWS is still earning its place while the
 // workflow either refuses to cancel or keys its group on something constant.
 // Once it does both of the things the contract asks for, the exemption is
@@ -214,6 +324,25 @@ test('every workflow with a constant concurrency group is recorded as deliberate
   );
 });
 
+test('the pull-request-branch detection is not vacuous', () => {
+  const pushers = workflows.filter(
+    ({ doc }) => pullRequestBranches(doc).length,
+  );
+  assert.ok(
+    pushers.length > 0,
+    `no workflow was found using ${PR_BRANCH_ACTION}, so the branch-race contract below would pass over an empty set`,
+  );
+});
+
+test('every workflow that pushes a PR branch serialises itself on that branch', () => {
+  const offenders = racingBranchWorkflows(workflows);
+  assert.deepEqual(
+    offenders,
+    [],
+    `two overlapping runs would both write the same refs/heads/<branch>: the loser's --force-with-lease fails on stale info, and 'delete-branch: true' can delete the ref the other run just opened a pull request from. Declare a concurrency group keyed exactly as the branch is:\n${offenders.join('\n')}`,
+  );
+});
+
 test('the serialised-workflow baseline retires itself', () => {
   for (const [name, reason] of SERIALISED_WORKFLOWS) {
     const entry = workflows.find((workflow) => workflow.name === name);
@@ -394,3 +523,172 @@ test('an exemption retires once the workflow satisfies both halves of the contra
     }),
   );
 });
+
+test('the branch-race scanner reports each way a group can fail to serialise', () => {
+  const cpr = (branch) => ({
+    jobs: {
+      refresh: {
+        steps: [
+          { uses: 'actions/checkout@v7' },
+          {
+            uses: `${PR_BRANCH_ACTION}@5f6978f`,
+            with: { branch, 'delete-branch': true },
+          },
+        ],
+      },
+    },
+  });
+
+  // Constant branch, no group at all: the sharp case.
+  assert.deepEqual(
+    racingBranchWorkflows(fixture('refresh.yml', cpr('automation/refresh'))),
+    ['refresh.yml: automation/refresh -> none'],
+  );
+
+  // Keyed branch, constant group: one lane for every issue is heavier than it
+  // needs to be, but the ref is still held by one run at a time, so the
+  // contract accepts it.
+  assert.deepEqual(
+    racingBranchWorkflows(
+      fixture('submit.yml', {
+        ...cpr('automation/issue-${{ github.event.issue.number }}'),
+        concurrency: { group: 'submit' },
+      }),
+    ),
+    [],
+  );
+
+  // Keyed branch, group keyed on something the branch does not vary with: two
+  // runs for the same issue land in different lanes and race the one ref.
+  assert.deepEqual(
+    racingBranchWorkflows(
+      fixture('submit.yml', {
+        ...cpr('automation/issue-${{ github.event.issue.number }}'),
+        concurrency: { group: 'submit-${{ github.ref }}' },
+      }),
+    ),
+    [
+      'submit.yml: automation/issue-${{ github.event.issue.number }} -> submit-${{ github.ref }}',
+    ],
+  );
+
+  // Keyed branch, group carrying the same context: compliant.
+  assert.deepEqual(
+    racingBranchWorkflows(
+      fixture('submit.yml', {
+        ...cpr('automation/issue-${{ github.event.issue.number }}'),
+        concurrency: {
+          group: 'submit-${{ github.event.issue.number }}',
+          'cancel-in-progress': false,
+        },
+      }),
+    ),
+    [],
+  );
+
+  // Constant branch with a constant group: compliant.
+  assert.deepEqual(
+    racingBranchWorkflows(
+      fixture('refresh.yml', {
+        ...cpr('automation/refresh'),
+        concurrency: { group: 'automation/refresh' },
+      }),
+    ),
+    [],
+  );
+
+  // A workflow that opens no pull request branch is not this contract's
+  // problem.
+  assert.deepEqual(
+    racingBranchWorkflows(
+      fixture('nightly.yml', { on: { schedule: [] }, jobs: {} }),
+    ),
+    [],
+  );
+});
+
+test('the branch reader finds the action wherever it sits', () => {
+  assert.deepEqual(pullRequestBranches({}), []);
+  assert.deepEqual(pullRequestBranches({ jobs: null }), []);
+  assert.deepEqual(pullRequestBranches({ jobs: { a: {} } }), []);
+  assert.deepEqual(pullRequestBranches({ jobs: { a: { steps: null } } }), []);
+  // A step with no `uses`, and one using a different action.
+  assert.deepEqual(
+    pullRequestBranches({
+      jobs: {
+        a: { steps: [{ run: 'npm ci' }, { uses: 'actions/checkout@v7' }] },
+      },
+    }),
+    [],
+  );
+  // The action pinned by SHA, with the branch folded across lines by Prettier.
+  assert.deepEqual(
+    pullRequestBranches({
+      jobs: {
+        a: {
+          steps: [
+            {
+              uses: `${PR_BRANCH_ACTION}@5f6978f`,
+              with: {
+                branch: 'automation/x-${{ github.event.issue.number\n }}',
+              },
+            },
+          ],
+        },
+      },
+    }),
+    ['automation/x-${{ github.event.issue.number }}'],
+  );
+  // The action present but naming no branch: nothing to serialise on.
+  assert.deepEqual(
+    pullRequestBranches({
+      jobs: { a: { steps: [{ uses: `${PR_BRANCH_ACTION}@v8`, with: {} }] } },
+    }),
+    [],
+  );
+  // Two jobs each pushing a branch.
+  assert.deepEqual(
+    pullRequestBranches({
+      jobs: {
+        a: {
+          steps: [{ uses: `${PR_BRANCH_ACTION}@v8`, with: { branch: 'x' } }],
+        },
+        b: {
+          steps: [{ uses: `${PR_BRANCH_ACTION}@v8`, with: { branch: 'y' } }],
+        },
+      },
+    }),
+    ['x', 'y'],
+  );
+});
+
+test('the expression reader compares what a key varies with, not its text', () => {
+  assert.deepEqual(expressionsIn('automation/refresh'), []);
+  assert.deepEqual(expressionsIn(undefined), []);
+  assert.deepEqual(expressionsIn('x-${{ github.ref }}'), ['github.ref']);
+  // Whitespace and fold differences must not make two equal keys compare
+  // unequal.
+  assert.deepEqual(
+    expressionsIn('x-${{   github.ref\n }}'),
+    expressionsIn('y-${{ github.ref }}'),
+  );
+  // Multiple expressions are compared as a set, so order in the text is
+  // irrelevant.
+  assert.deepEqual(
+    expressionsIn('${{ b }}-${{ a }}'),
+    expressionsIn('${{ a }}/${{ b }}'),
+  );
+});
+
+test('per-event keying is broader than per-PR keying but still demands an expression', () => {
+  assert.ok(isPerEvent('x-${{ github.event.issue.number }}'));
+  assert.ok(isPerEvent('x-${{ github.ref }}'));
+  assert.ok(isPerEvent('x-${{ inputs.target }}'));
+  // Per-PR keys are a subset of per-event keys.
+  assert.ok(isPerEvent('x-${{ github.head_ref }}'));
+  assert.ok(!isPerPullRequest('x-${{ github.event.issue.number }}'));
+  // Text mentioning a context is still one constant key.
+  assert.ok(!isPerEvent('github.event.issue.number'));
+  assert.ok(!isPerEvent('automation/refresh'));
+  assert.ok(!isPerEvent(undefined));
+});

Measured

At 593b9fe, node v26.10.0, TZ=UTC, locally:

  • Both halves applied: npm run test:unit → 2092 tests, 0 failures; npx prettier --check clean on all five files.
  • Workflow half reverted, contract half kept: the new test fails naming exactly the four files, which is the evidence the contract is not vacuous:
every workflow that pushes a PR branch serialises itself on that branch
  architecture-submission.yml: automation/architecture-issue-${{ github.event.issue.number }} -> none
  import-architectures.yml: automation/import-architectures -> none
  refresh-community-people.yml: automation/refresh-community-people -> none
  refresh-radar-reports.yml: automation/refresh-radar-reports -> none

Who can land this — no PR from me

This needs a human or an agent with workflow-write permission. Half 1 is under
.github/workflows/, which this agent's contributor-tier token cannot push: GitHub
rejects the diff server-side regardless of what the App installation grants. That is a
permission ceiling, not a judgement call.

Half 2 is pushable on its own, but PRing it alone would land a red test, since it
fails until Half 1 is applied. Splitting them would knowingly break main, so I have
deliberately not opened a partial PR. Both diffs above are complete and apply cleanly at
593b9fe; applying them is mechanical.

Priority

  • Impact: medium (failed automation runs and a deletable PR branch; no data corruption, because --force-with-lease fails loudly)
  • Effort: low (one concurrency: block per workflow, plus the contract that keeps it)

Filed by quality agent (hold-gated mode)


🐝 Hive Agent: quality | Instance: hosted-available-lke648397-260827-5n31 | SHA: 593b9fe

— hive: agent=quality backend=copilot model=claude-opus-5 copilot=1.0.88

Dominant language
JavaScript
Stars
0
Forks
2
Avg merge
8h 22m
Merged PRs (30d)
600

Getting set up

Open in Codespaces

Starts the project's dev container in your browser, under your own GitHub account.

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from cncf/endusers

All issues in cncf/endusers

Similar issues

More JavaScript issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.