From 2c514b50b966a3c573001f57cf593cdf8a74906f Mon Sep 17 00:00:00 2001 From: Shaojin Wen Date: Thu, 6 Aug 2026 13:27:39 +0800 Subject: [PATCH] fix(autofix): serialize scan-and-pick issue runs in one concurrency group (#8435) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(autofix): serialize scan-and-pick issue runs in one concurrency group The issue-phase concurrency group fell back to github.run_id for every run that route did not force a target — which is every scheduled run and every label-triggered run. Run-unique groups exclude nothing, so two overlapping scans (cron effectively fires every 40-70 minutes while the job may run for up to 180) could both pass the live label recheck during the minutes-long assess call, double-claim the same issue, and burn two multi-hour agent runs on it; the loser then fails its push and posts a withdraw comment. Key forced dispatches per issue, label events on the payload issue, and all scan-and-pick runs (cron or unforced dispatch) on one shared 'scheduled' group. cancel-in-progress stays false so a superseded tick still runs when targets remain. * fix(autofix): make concurrency group pin prettier-stable (#8435) * fix(tests): sync qwen-resolve-workflow timeout pins with repository variables (#8435) * fix(ci): pin issue-phase concurrency group equal to FORCED_ISSUE (#8435) * test(ci): anchor right edge of issue-phase concurrency group pin (#8435) * fix(autofix): keep never-runnable runs out of the issue-phase concurrency group (#8435) * test(autofix): pin issue-autofix concurrency gate to the job if predicate (#8435) * fix(autofix): exclude dry runs from the issue-phase concurrency groups (#8435) Co-authored-by: Qwen-Coder * test(autofix): pin the Claim/Publish dry-run step gates (#8435) Co-authored-by: Qwen-Coder Co-authored-by: Qwen-Coder --------- Co-authored-by: verify Co-authored-by: 易良 <1204183885@qq.com> Co-authored-by: qwen-code-dev-bot Co-authored-by: qwen-code-dev-bot Co-authored-by: Qwen-Coder --- .github/workflows/qwen-autofix.yml | 27 ++++++++- scripts/tests/qwen-autofix-workflow.test.js | 67 ++++++++++++++++++++- 2 files changed, 91 insertions(+), 3 deletions(-) diff --git a/.github/workflows/qwen-autofix.yml b/.github/workflows/qwen-autofix.yml index 49ffaa1dff..11ebfd91a3 100644 --- a/.github/workflows/qwen-autofix.yml +++ b/.github/workflows/qwen-autofix.yml @@ -660,8 +660,31 @@ jobs: }} runs-on: 'ubuntu-latest' timeout-minutes: 180 + # route.issue_number is only set for forced dispatches; label events carry + # the issue in the payload, and scan-and-pick runs (cron, unforced + # dispatch) share one 'scheduled' group. The old github.run_id fallback + # made every scan-and-pick run its own group, so two overlapping scans + # (cron fires every 40-70min, this job runs up to 180) could double-claim + # the same issue — the claim recheck runs after assess and only narrows + # the race to the short gap between the recheck and the claim's label + # write; it does not close it. Queued (never cancelled) so the newest + # pending tick still runs after a long scan if targets remain; + # intermediate ticks are superseded, which is fine because each run + # rescans from scratch. + # + # GitHub evaluates concurrency before the job `if`, but after `needs`, so + # the group is gated on the same runnability predicate as the `if` above, + # plus a dry-run exclusion: runs whose issue phase will not execute + # (do_issue=false takeover, review, and command events; label events + # failing the decide gates; scheduled ticks whose review-scan still has + # targets) and dry runs (if-runnable, but their Claim/Publish steps are + # gated off) get a run-unique group instead — a run that never claims + # entering a target-keyed group would replace the single pending run + # there and silently cancel it. Same precedent as qwen-triage.yml's + # triage/tmux jobs. concurrency: - group: 'qwen-autofix-issue-${{ needs.route.outputs.issue_number || github.run_id }}' + group: >- + ${{ needs.route.outputs.do_issue == 'true' && needs.route.outputs.dry_run != 'true' && (github.event_name != 'schedule' || (needs.review-scan.result == 'success' && needs.review-scan.outputs.has_targets != 'true')) && format('qwen-autofix-issue-{0}', needs.route.outputs.issue_number || github.event.issue.number || 'scheduled') || format('qwen-autofix-issue-run-{0}', github.run_id) }} cancel-in-progress: false permissions: contents: 'read' @@ -786,6 +809,8 @@ jobs: id: 'scan' env: GITHUB_TOKEN: '${{ secrets.CI_DEV_BOT_PAT }}' + # Must resolve to the same issue as this job's concurrency group + # expression; a test pins the two equal. FORCED_ISSUE: '${{ needs.route.outputs.issue_number || github.event.issue.number }}' run: |- mkdir -p "${WORKDIR}" diff --git a/scripts/tests/qwen-autofix-workflow.test.js b/scripts/tests/qwen-autofix-workflow.test.js index 05ea5dd534..8a827f294e 100644 --- a/scripts/tests/qwen-autofix-workflow.test.js +++ b/scripts/tests/qwen-autofix-workflow.test.js @@ -1758,7 +1758,11 @@ describe('qwen-autofix workflow', () => { it('falls back to existing issue backlog only when review has no target', () => { expect(issueAutofixJob).toContain("needs: ['route', 'review-scan']"); - expect(issueAutofixJob).toContain('always()'); + // Anchor the job `if` opening: a bare toContain('always()') is also + // satisfied by step-level always()s elsewhere in the job. + expect(issueAutofixJob).toContain( + "if: |-\n ${{\n always() &&\n needs.route.outputs.do_issue == 'true' &&", + ); expect(issueAutofixJob).toContain("needs.review-scan.result == 'success'"); expect(issueAutofixJob).toContain( "github.event_name != 'schedule' || (needs.review-scan.result == 'success' && needs.review-scan.outputs.has_targets != 'true')", @@ -1894,9 +1898,68 @@ describe('qwen-autofix workflow', () => { expect(workflow).toContain( 'if [[ "${ISSUE_STATE}" == \'open\' && "${_late_ready}" == \'true\' && "${_late_approved}" == \'true\' && "${sender_is_trusted}" == \'true\' ]]; then', ); + // Issue-phase mutual exclusion: forced dispatches key per issue, label + // events key on the payload issue, and every scan-and-pick run (cron or + // unforced dispatch) shares ONE group. A run-unique fallback here let two + // overlapping scans double-claim the same issue — the claim recheck runs + // after assess and only narrows the race to the short gap between the + // recheck and the claim's label write; it does not close it. GitHub + // evaluates concurrency before the job `if`, but after `needs`, so the + // group is gated on the job `if`'s runnability predicate plus a dry-run + // exclusion: keyed-group occupants must CLAIM, and dry runs are + // if-runnable yet skip Claim/Publish, so they get a run-unique group + // just like never-runnable runs and cannot supersede a pending + // target-keyed run. The right edge is anchored to + // cancel-in-progress because the value is a folded block scalar — a + // run-unique suffix appended as a continuation line would also become + // part of the group value while a trailing-newline anchor stayed green. expect(issueAutofixJob).toContain( - "group: 'qwen-autofix-issue-${{ needs.route.outputs.issue_number || github.run_id }}'", + "group: >-\n ${{ needs.route.outputs.do_issue == 'true' && needs.route.outputs.dry_run != 'true' && (github.event_name != 'schedule' || (needs.review-scan.result == 'success' && needs.review-scan.outputs.has_targets != 'true')) && format('qwen-autofix-issue-{0}', needs.route.outputs.issue_number || github.event.issue.number || 'scheduled') || format('qwen-autofix-issue-run-{0}', github.run_id) }}\n cancel-in-progress: false", ); + expect(issueAutofixJob).not.toContain('|| github.run_id }}'); + // The group identity and the scan step's FORCED_ISSUE env are + // load-bearingly coupled: both must resolve to the same issue on every + // trigger path, or runs aimed at one issue land in different groups and + // the double-claim race reopens while every literal pin above stays + // green. Assert the two expressions equal so neither side can drift + // alone. + const groupKeyedOn = issueAutofixJob.match( + /format\('qwen-autofix-issue-\{0\}', (.+?) \|\| 'scheduled'\)/, + )?.[1]; + const forcedIssueSource = issueAutofixJob.match( + /id: 'scan'[\s\S]*?FORCED_ISSUE: '\$\{\{ (.+?) \}\}'/, + )?.[1]; + expect(groupKeyedOn).toBeTruthy(); + expect(groupKeyedOn).toBe(forcedIssueSource); + // The gate duplicated into the group expression must stay equal to the + // job `if` predicate minus `always() &&` (anchored where the job `if` + // opens), plus the dry-run exclusion the `if` does not need — dry runs + // execute but never claim, so they must not enter a keyed group: the + // gate clause now occurs on both sides, so the literal pins of it are + // satisfied by the group copy even if the `if:`-side occurrence drifts. + const normalize = (text) => text.replace(/\s+/g, ' ').trim(); + const ifPredicate = normalize( + issueAutofixJob.match(/if: \|-\n\s*\$\{\{\n([\s\S]*?)\n\s*\}\}/)?.[1] ?? + '', + ).replace(/^always\(\) && /, ''); + const gatePredicate = normalize( + issueAutofixJob.match( + /group: >-\n\s*\$\{\{\s*(.+?)\s*&& format\('qwen-autofix-issue-\{0\}'/, + )?.[1] ?? '', + ); + expect(ifPredicate).toBeTruthy(); + expect(gatePredicate).toBe( + ifPredicate.replace( + "needs.route.outputs.do_issue == 'true' && ", + "needs.route.outputs.do_issue == 'true' && needs.route.outputs.dry_run != 'true' && ", + ), + ); + // Dry runs get run-unique groups above because they never claim — that + // invariant rests on these step `if:` gates, which nothing else asserts: + // dropping the clause from either gate lets a dry run and a scheduled + // real run claim the same issue while every group pin stays green. + expect(claimIssueStep).toContain("needs.route.outputs.dry_run != 'true'"); + expect(publishPrStep).toContain("needs.route.outputs.dry_run != 'true'"); expect(workflow).toContain( '(.labels // []) | map(.name) as $labels | ($labels | index($ready))', );