diff --git a/.github/workflows/qwen-autofix.yml b/.github/workflows/qwen-autofix.yml index dc853ab2069..ae1b9d5e425 100644 --- a/.github/workflows/qwen-autofix.yml +++ b/.github/workflows/qwen-autofix.yml @@ -2073,8 +2073,26 @@ jobs: max-parallel: 3 matrix: target: '${{ fromJSON(needs.review-scan.outputs.targets) }}' + # Serialises every writer of THIS PR's head branch, across workflows. + # GitHub concurrency groups are repository-scoped, so sharing one name with + # qwen-code-pr-review.yml's resolve-pr job is what makes the two mutually + # exclusive — a per-workflow name only guards against itself. + # Without this, a `@qwen-code /resolve` and this job's own conflict path + # both merge the base branch and both push. Observed on #7355: /resolve + # pushed at 03:51, this job pushed at 04:05 and was rejected `fetch first`, + # discarding a full agent run and leaving no marker to show for it. + # Serialising is strictly better than racing: this job fetches the head by + # NAME at job start, so the second run reads the winner's result instead of + # a stale base — its work is usable and its push lands, rather than being + # rejected and thrown away. It may still spend an agent run: the + # address-time recheck re-verifies lifecycle and consent (state, labels, + # author, base, head branch) but not whether the conflict is still there. + # The prefix is a LITERAL on both sides: job-level `concurrency` cannot read + # the `env` context, so the two files cannot share a constant. A test pins + # them equal instead, because a rename in one file alone silently unlocks + # the race again with nothing failing. concurrency: - group: 'qwen-autofix-review-${{ matrix.target.pr }}' + group: 'qwen-pr-head-write-${{ matrix.target.pr }}' cancel-in-progress: false env: REPO: '${{ github.repository }}' diff --git a/.github/workflows/qwen-code-pr-review.yml b/.github/workflows/qwen-code-pr-review.yml index 273d51b391d..1095e2ccef6 100644 --- a/.github/workflows/qwen-code-pr-review.yml +++ b/.github/workflows/qwen-code-pr-review.yml @@ -932,8 +932,15 @@ jobs: # ephemeral, which suits the sandboxed conflict-resolution job. runs-on: 'ubuntu-latest' timeout-minutes: 120 + # Shared with qwen-autofix.yml's review-address job — see the rationale + # there. GitHub concurrency groups are repository-scoped, so the identical + # prefix is what serialises the two workflows against each other; both + # merge the base branch and push to this PR's head, and racing them costs a + # whole agent run on the loser (observed on #7355). The prefix must stay + # byte-identical in both files; a test pins that, because renaming one side + # alone re-opens the race with nothing failing. concurrency: - group: 'qwen-resolve-${{ github.event.issue.number || github.event.inputs.pr_number }}' + group: 'qwen-pr-head-write-${{ github.event.issue.number || github.event.inputs.pr_number }}' cancel-in-progress: false # Least-privilege: every write in this job (push, PR comments, the # acknowledge reaction) uses an explicit PAT, so the implicit GITHUB_TOKEN diff --git a/scripts/tests/qwen-resolve-workflow.test.js b/scripts/tests/qwen-resolve-workflow.test.js index 430b390a92c..609b1924e3e 100644 --- a/scripts/tests/qwen-resolve-workflow.test.js +++ b/scripts/tests/qwen-resolve-workflow.test.js @@ -113,6 +113,47 @@ describe('qwen resolve workflow', () => { 'utf8', ); + it('serialises /resolve against the autofix conflict path on the same PR head', () => { + // Both jobs merge the base branch and push to the PR's head. They live in + // different workflows, so a per-workflow concurrency name guards each only + // against itself. Observed on #7355: /resolve pushed at 03:51, the autofix + // leg pushed at 04:05 and was rejected `fetch first`, throwing away a full + // agent run. GitHub concurrency groups are repository-scoped, so an + // IDENTICAL prefix in both files is what makes them mutually exclusive. + const autofix = readFileSync( + path.join(repoRoot, '.github/workflows/qwen-autofix.yml'), + 'utf8', + ); + const groupOf = (text, jobName) => + job(text, jobName).match( + /\n {4}concurrency:\n {6}group: '([a-z-]+?)-\$\{\{/, + )?.[1]; + + const resolveLock = groupOf(workflow, 'resolve-pr'); + const autofixLock = groupOf(autofix, 'review-address'); + expect(resolveLock).toBeTruthy(); + // The invariant: renaming one side alone silently re-opens the race, and + // nothing else in the suite would notice. + expect(autofixLock).toBe(resolveLock); + + // Each side must still key the group on the PR number — a shared prefix + // with a per-run suffix would serialise nothing. + expect(job(workflow, 'resolve-pr')).toContain( + `group: '${resolveLock}-\${{ github.event.issue.number || github.event.inputs.pr_number }}'`, + ); + expect(job(autofix, 'review-address')).toContain( + `group: '${autofixLock}-\${{ matrix.target.pr }}'`, + ); + // Queue, never cancel: the loser of the race must run after the winner and + // re-check, not be discarded (or discard the winner's in-flight work). + for (const [text, name] of [ + [workflow, 'resolve-pr'], + [autofix, 'review-address'], + ]) { + expect(job(text, name)).toContain('cancel-in-progress: false'); + } + }); + it('uses the existing PR command workflow', () => { expect( existsSync(