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
20 changes: 19 additions & 1 deletion .github/workflows/qwen-autofix.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}'
Expand Down
9 changes: 8 additions & 1 deletion .github/workflows/qwen-code-pr-review.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
41 changes: 41 additions & 0 deletions scripts/tests/qwen-resolve-workflow.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Loading