From 28c3e23e15959592cd831e06f822f063aa50816a Mon Sep 17 00:00:00 2001 From: wenshao Date: Sat, 18 Jul 2026 00:41:01 +0800 Subject: [PATCH 1/7] ci(autofix): fan out review targets and stop route-scan starvation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two throughput fixes for the review loop, both observed live: - review-scan emitted ONE newest-first target per scan ("single-target worker"). With sparse cron ticks this starves older armed PRs for hours — an armed PR sat unprocessed for 16h while newer PRs took every tick. Emit EVERY eligible target instead: the address matrix's max-parallel (3) bounds simultaneity and the per-PR concurrency groups already prevent duplicate same-PR runs, so one surviving scan drains the whole backlog. - route used a single shared concurrency group with cancel-in-progress. Under runner backlog a route job sits QUEUED for minutes, and any newer event (review submissions arrive constantly) cancelled it — five consecutive dispatched scans died this way; during event storms no full scan survived at all. Cron ticks keep deduping through a shared 'route-cron' group, but dispatches and review/issue events now get unique per-run groups: route is a seconds-long job, so never cancelling it costs nothing and every trigger is guaranteed to route. Contract test updated: fan-out asserted (no single-target break, matrix max-parallel), new route concurrency expression pinned. 50/50. --- .github/workflows/qwen-autofix.yml | 17 ++++++++++++--- scripts/tests/qwen-autofix-workflow.test.js | 23 +++++++++++++++++---- 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/.github/workflows/qwen-autofix.yml b/.github/workflows/qwen-autofix.yml index 61c91fb3478..f85b03ed3b9 100644 --- a/.github/workflows/qwen-autofix.yml +++ b/.github/workflows/qwen-autofix.yml @@ -90,8 +90,15 @@ jobs: runs-on: 'ubuntu-latest' timeout-minutes: 5 concurrency: - group: 'qwen-autofix-route' - cancel-in-progress: true + # Cron ticks dedupe among themselves (a newer tick supersedes a queued + # one). Dispatches and review/issue events each route independently: + # under runner backlog a route job can sit QUEUED for minutes, and the + # old shared cancel-in-progress group let any newer event kill pending + # full scans — observed as hours of scan starvation during review-event + # storms. Route is a seconds-long job, so unique groups cost nothing. + group: "${{ github.event_name == 'schedule' && 'qwen-autofix-route-cron' || format('qwen-autofix-route-{0}', github.run_id) }}" + cancel-in-progress: |- + ${{ github.event_name == 'schedule' }} permissions: contents: 'read' outputs: @@ -1152,7 +1159,11 @@ jobs: --arg round "${ROUND}" --arg wm "${EFF_WM}" \ '. + [{pr: $pr, branch: $branch, issue: $issue, round: $round, watermark: $wm}]' \ <<< "${TARGETS}")" - break # one PR per scheduled scan + # Fan out: emit EVERY eligible PR. The address matrix bounds + # simultaneity (max-parallel) and the per-PR concurrency groups + # prevent duplicate same-PR runs, so one scan drains the whole + # backlog instead of serving a single newest-first target per tick + # (which starved older PRs for hours when cron ticks were sparse). done COUNT="$(jq 'length' <<< "${TARGETS}")" diff --git a/scripts/tests/qwen-autofix-workflow.test.js b/scripts/tests/qwen-autofix-workflow.test.js index f0592557de7..6705fe35f44 100644 --- a/scripts/tests/qwen-autofix-workflow.test.js +++ b/scripts/tests/qwen-autofix-workflow.test.js @@ -200,7 +200,7 @@ describe('qwen-autofix workflow', () => { expect(workflow).toContain('.[0:10] | map(. + {autofixTier: 1})'); }); - it('runs scheduled autofix as a 10-minute single-target worker', () => { + it('runs scheduled autofix as a 10-minute multi-target fan-out worker', () => { expect(workflow).toContain("cron: '*/10 * * * *'"); expect(workflow).not.toContain("cron: '0 0,12 * * *'"); expect(workflow).not.toContain("cron: '0 4,8,16,20 * * *'"); @@ -215,7 +215,13 @@ describe('qwen-autofix workflow', () => { expect(reviewScanJob).toContain('isCrossRepository'); expect(reviewScanJob).toContain('not an open in-repo main-targeting PR'); expect(reviewScanJob).toContain('.isCrossRepository != true'); - expect(reviewScanJob).toContain('break # one PR per scheduled scan'); + // Fan-out: one scan emits EVERY eligible PR (no single-target break). The + // address matrix's max-parallel bounds simultaneity and per-PR concurrency + // groups prevent duplicate same-PR runs; a single-target break starved + // older PRs for hours whenever cron ticks were sparse. + expect(reviewScanJob).not.toContain('break # one PR per scheduled scan'); + expect(reviewScanJob).toContain('Fan out: emit EVERY eligible PR'); + expect(workflow).toContain("max-parallel: 3"); expect(reviewScanJob).toContain('statusCheckRollup'); expect(reviewScanJob).toContain('HAS_PENDING_CHECKS'); expect(reviewScanJob).toContain('N_FAILED_CHECKS'); @@ -303,8 +309,17 @@ describe('qwen-autofix workflow', () => { "ASSIGNEE_LOGIN: '${{ github.event.assignee.login }}'", ); expect(workflow).toContain("permissions:\n contents: 'read'"); - expect(routeJob).toContain("group: 'qwen-autofix-route'"); - expect(routeJob).toContain('cancel-in-progress: true'); + // Route concurrency: cron ticks share one group and supersede each other, + // but dispatches and review/issue events get unique per-run groups — a + // shared cancel-in-progress group let any newer event kill pending full + // scans while route jobs sat queued behind runner backlog. + expect(routeJob).toContain( + "group: \"${{ github.event_name == 'schedule' && 'qwen-autofix-route-cron' || format('qwen-autofix-route-{0}', github.run_id) }}\"", + ); + expect(routeJob).toContain( + "cancel-in-progress: |-\n ${{ github.event_name == 'schedule' }}", + ); + expect(routeJob).not.toContain("group: 'qwen-autofix-route'"); expect(workflow).toContain( 'gh api "repos/${REPO}/collaborators/${SENDER_LOGIN}/permission"', ); From 66075483d19df3d65e17ee0d24d257278f5d1e5f Mon Sep 17 00:00:00 2001 From: wenshao Date: Sat, 18 Jul 2026 09:26:25 +0800 Subject: [PATCH 2/7] ci(autofix): cap targets emitted per scan (review defense-in-depth) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review note on the fan-out: bound the scan's output for a pathological backlog. Clarifications recorded in-thread — the loop lives in review-scan (timeout 15m), not route (5m), and the pre-change worst case already walked the full candidate list (break fired on the first ELIGIBLE PR, not the first candidate) — but an explicit bound is good hygiene: emit at most MAX_TARGETS_PER_SCAN (10) targets, LOG the deferral (never a silent cap), and let the next scan pick up the remainder since their signals persist. Contract test pins the cap, the deferral log, and the slice. --- .github/workflows/qwen-autofix.yml | 12 ++++++++++++ scripts/tests/qwen-autofix-workflow.test.js | 5 +++++ 2 files changed, 17 insertions(+) diff --git a/.github/workflows/qwen-autofix.yml b/.github/workflows/qwen-autofix.yml index f85b03ed3b9..fccc7423f10 100644 --- a/.github/workflows/qwen-autofix.yml +++ b/.github/workflows/qwen-autofix.yml @@ -77,6 +77,9 @@ env: # Hard cap on automated review-address rounds per PR. After this the bot stops # and leaves the PR for a human. MAX_ROUNDS: '5' + # Upper bound on review targets emitted per scan (fan-out defense-in-depth; + # excess is logged and deferred to the next scan). + MAX_TARGETS_PER_SCAN: '10' # Do not claim more issues when too many existing autofix PRs are still open. MAX_OPEN_AUTOFIX_PRS: '5' @@ -1166,6 +1169,15 @@ jobs: # (which starved older PRs for hours when cron ticks were sparse). done + # Defense-in-depth bound on a pathological backlog: cap the targets + # emitted per scan so both the address matrix size and this job's + # runtime stay bounded. Never a silent cap — log what was deferred; + # the next scan picks up the remainder (their signals persist). + TOTAL="$(jq 'length' <<< "${TARGETS}")" + if [[ "${TOTAL}" -gt "${MAX_TARGETS_PER_SCAN}" ]]; then + echo "⚠️ ${TOTAL} eligible PRs; emitting the first ${MAX_TARGETS_PER_SCAN}, deferring the rest to the next scan" + TARGETS="$(jq -c --argjson n "${MAX_TARGETS_PER_SCAN}" '.[0:$n]' <<< "${TARGETS}")" + fi COUNT="$(jq 'length' <<< "${TARGETS}")" echo "📋 ${COUNT} PR(s) to process" echo "targets=${TARGETS}" >> "${GITHUB_OUTPUT}" diff --git a/scripts/tests/qwen-autofix-workflow.test.js b/scripts/tests/qwen-autofix-workflow.test.js index 6705fe35f44..b453f234eed 100644 --- a/scripts/tests/qwen-autofix-workflow.test.js +++ b/scripts/tests/qwen-autofix-workflow.test.js @@ -222,6 +222,11 @@ describe('qwen-autofix workflow', () => { expect(reviewScanJob).not.toContain('break # one PR per scheduled scan'); expect(reviewScanJob).toContain('Fan out: emit EVERY eligible PR'); expect(workflow).toContain("max-parallel: 3"); + // Pathological-backlog bound: targets per scan are capped, the excess is + // LOGGED and deferred to the next scan (never a silent cap). + expect(workflow).toContain("MAX_TARGETS_PER_SCAN: '10'"); + expect(reviewScanJob).toContain('deferring the rest to the next scan'); + expect(reviewScanJob).toContain(".[0:$n]"); expect(reviewScanJob).toContain('statusCheckRollup'); expect(reviewScanJob).toContain('HAS_PENDING_CHECKS'); expect(reviewScanJob).toContain('N_FAILED_CHECKS'); From 8d391cace71bb4e3f01bff21613d1dd5372aa70f Mon Sep 17 00:00:00 2001 From: wenshao Date: Sat, 18 Jul 2026 11:23:08 +0800 Subject: [PATCH 3/7] =?UTF-8?q?ci(autofix):=20review=20round=202=20?= =?UTF-8?q?=E2=80=94=20per-target=20route=20coalescing,=20busy-PR=20skip,?= =?UTF-8?q?=20in-loop=20budget?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both criticals and the suggestion from review, each verified against live campaign observations: - Route concurrency is now keyed by TARGET: cron ticks still coalesce with each other; review events coalesce PER PR (near-simultaneous reviews on one PR route once — the one useful side effect of the old shared group, restored — without events on other PRs cancelling this one); issue events coalesce per issue; dispatches stay unique and are never cancelled. This keeps the starvation fix while closing the duplicate-forced-scan window the per-run_id grouping had opened. - The scan now skips any PR whose review-address job is RUNNING OR QUEUED in a live autofix run (one runs-list plus a jobs-view per live run). A fanned-out matrix holds queued jobs past a 10-minute tick and schedule/dispatch runs never surface in the PR's checks, so without this the next scan re-emitted the same PRs and per-PR groups accumulated duplicates that later replayed stale watermarks — the exact duplicate-round behavior observed live on the fleet. - The per-scan target budget now BREAKS the candidate loop instead of slicing after it, so it genuinely bounds scan runtime and API usage (each candidate costs several serial reads); the deferral is logged and the remainder keeps its signals for the next scan. Contract test updated for all three (route expression per target, busy-skip message + capture regex, in-loop budget break). 50/50. --- .github/workflows/qwen-autofix.yml | 74 +++++++++++++++------ scripts/tests/qwen-autofix-workflow.test.js | 35 ++++++++-- 2 files changed, 81 insertions(+), 28 deletions(-) diff --git a/.github/workflows/qwen-autofix.yml b/.github/workflows/qwen-autofix.yml index fccc7423f10..657f08f0586 100644 --- a/.github/workflows/qwen-autofix.yml +++ b/.github/workflows/qwen-autofix.yml @@ -93,15 +93,19 @@ jobs: runs-on: 'ubuntu-latest' timeout-minutes: 5 concurrency: - # Cron ticks dedupe among themselves (a newer tick supersedes a queued - # one). Dispatches and review/issue events each route independently: - # under runner backlog a route job can sit QUEUED for minutes, and the - # old shared cancel-in-progress group let any newer event kill pending - # full scans — observed as hours of scan starvation during review-event - # storms. Route is a seconds-long job, so unique groups cost nothing. - group: "${{ github.event_name == 'schedule' && 'qwen-autofix-route-cron' || format('qwen-autofix-route-{0}', github.run_id) }}" + # Concurrency is keyed by TARGET, not shared and not fully unique: + # • cron ticks share one group (a newer tick supersedes a queued one) + # • review events coalesce PER PR (two reviews on the same PR seconds + # apart route once — the old shared group's one useful side effect, + # kept, without letting events on OTHER PRs cancel this one) + # • issue events coalesce PER issue + # • dispatches are unique per run and are never cancelled + # The old single shared cancel-in-progress group let ANY newer event kill + # pending full scans while route jobs sat queued behind runner backlog — + # observed as hours of scan starvation during review-event storms. + group: "${{ github.event_name == 'schedule' && 'qwen-autofix-route-cron' || (github.event_name == 'pull_request_review' && format('qwen-autofix-route-pr-{0}', github.event.pull_request.number)) || (github.event_name == 'issues' && format('qwen-autofix-route-issue-{0}', github.event.issue.number)) || format('qwen-autofix-route-{0}', github.run_id) }}" cancel-in-progress: |- - ${{ github.event_name == 'schedule' }} + ${{ github.event_name != 'workflow_dispatch' }} permissions: contents: 'read' outputs: @@ -1014,8 +1018,33 @@ jobs: # check and double-process the feedback). PENDING_STALE_MIN=240 PENDING_CUTOFF="$(date -u -d "${PENDING_STALE_MIN} minutes ago" +%Y-%m-%dT%H:%M:%SZ)" + + # PRs whose review-address is already RUNNING OR QUEUED in any live + # autofix run must not be re-targeted. Schedule/dispatch runs execute + # against main's SHA, so their matrix jobs never appear in the PR's + # statusCheckRollup — and a fanned-out matrix holds queued jobs well + # past a 10-minute tick, so without this the next scan re-emits the + # same PRs and the per-PR address groups accumulate duplicates that + # later replay stale watermarks. One runs-list plus one jobs-view per + # live run is cheap (there are at most a few). + BUSY_PRS=' ' + while IFS= read -r LIVE_RUN; do + [[ -z "${LIVE_RUN}" ]] && continue + while IFS= read -r BUSY; do + [[ -n "${BUSY}" ]] && BUSY_PRS="${BUSY_PRS}${BUSY} " + done < <(gh run view "${LIVE_RUN}" --repo "${REPO}" --json jobs \ + --jq '.jobs[] | select(.status != "completed") | .name | capture("^review-address \\((?[0-9]+),") | .pr' 2> /dev/null) + done < <(gh run list --repo "${REPO}" --workflow qwen-autofix.yml \ + --limit 15 --json databaseId,status \ + --jq '.[] | select(.status != "completed") | .databaseId' 2> /dev/null) + [[ "${BUSY_PRS}" != ' ' ]] && echo "🚧 address in flight/queued for PR(s):${BUSY_PRS}" + TARGETS='[]' for PR in ${CANDIDATES}; do + if [[ "${BUSY_PRS}" == *" ${PR} "* ]]; then + echo "⏳ #${PR}: review-address already in flight or queued — skipping" + continue + fi # One PR fetch for the branch name, check rollup, and creation time (the # watermark floor below) — avoids extra round-trips per candidate PR. PR_META="$(gh pr view "${PR}" --repo "${REPO}" \ @@ -1162,22 +1191,23 @@ jobs: --arg round "${ROUND}" --arg wm "${EFF_WM}" \ '. + [{pr: $pr, branch: $branch, issue: $issue, round: $round, watermark: $wm}]' \ <<< "${TARGETS}")" - # Fan out: emit EVERY eligible PR. The address matrix bounds - # simultaneity (max-parallel) and the per-PR concurrency groups - # prevent duplicate same-PR runs, so one scan drains the whole - # backlog instead of serving a single newest-first target per tick - # (which starved older PRs for hours when cron ticks were sparse). + # Fan out: emit EVERY eligible PR up to the per-scan budget. The + # address matrix bounds simultaneity (max-parallel) and the per-PR + # concurrency groups plus the busy-PR skip above prevent duplicate + # same-PR runs, so one scan drains the whole backlog instead of + # serving a single newest-first target per tick (which starved + # older PRs for hours when cron ticks were sparse). The budget + # break bounds this loop's RUNTIME and API usage too — each + # candidate costs several serial API reads, so scanning past a + # full budget would spend hundreds of calls for nothing. Never a + # silent cap: the deferral is logged and the next scan picks up + # the remainder (their signals persist). + if [[ "$(jq 'length' <<< "${TARGETS}")" -ge "${MAX_TARGETS_PER_SCAN}" ]]; then + echo "⚠️ target budget (${MAX_TARGETS_PER_SCAN}) reached; deferring the remaining candidates to the next scan" + break + fi done - # Defense-in-depth bound on a pathological backlog: cap the targets - # emitted per scan so both the address matrix size and this job's - # runtime stay bounded. Never a silent cap — log what was deferred; - # the next scan picks up the remainder (their signals persist). - TOTAL="$(jq 'length' <<< "${TARGETS}")" - if [[ "${TOTAL}" -gt "${MAX_TARGETS_PER_SCAN}" ]]; then - echo "⚠️ ${TOTAL} eligible PRs; emitting the first ${MAX_TARGETS_PER_SCAN}, deferring the rest to the next scan" - TARGETS="$(jq -c --argjson n "${MAX_TARGETS_PER_SCAN}" '.[0:$n]' <<< "${TARGETS}")" - fi COUNT="$(jq 'length' <<< "${TARGETS}")" echo "📋 ${COUNT} PR(s) to process" echo "targets=${TARGETS}" >> "${GITHUB_OUTPUT}" diff --git a/scripts/tests/qwen-autofix-workflow.test.js b/scripts/tests/qwen-autofix-workflow.test.js index b453f234eed..85330013acb 100644 --- a/scripts/tests/qwen-autofix-workflow.test.js +++ b/scripts/tests/qwen-autofix-workflow.test.js @@ -222,11 +222,23 @@ describe('qwen-autofix workflow', () => { expect(reviewScanJob).not.toContain('break # one PR per scheduled scan'); expect(reviewScanJob).toContain('Fan out: emit EVERY eligible PR'); expect(workflow).toContain("max-parallel: 3"); - // Pathological-backlog bound: targets per scan are capped, the excess is - // LOGGED and deferred to the next scan (never a silent cap). + // Pathological-backlog bound: the budget BREAKS the candidate loop (so it + // bounds runtime and API usage, not just matrix size), the deferral is + // LOGGED, and the next scan picks up the remainder. expect(workflow).toContain("MAX_TARGETS_PER_SCAN: '10'"); - expect(reviewScanJob).toContain('deferring the rest to the next scan'); - expect(reviewScanJob).toContain(".[0:$n]"); + expect(reviewScanJob).toContain( + 'deferring the remaining candidates to the next scan', + ); + expect(reviewScanJob).toMatch(/target budget \(\$\{MAX_TARGETS_PER_SCAN\}\) reached[\s\S]{0,120}break/); + // Fanned-out matrices hold QUEUED jobs past a tick and schedule/dispatch + // runs never appear in the PR's checks — the scan must skip PRs whose + // review-address is already running or queued in any live autofix run. + expect(reviewScanJob).toContain( + 'review-address already in flight or queued — skipping', + ); + expect(reviewScanJob).toContain( + 'capture("^review-address \\\\((?[0-9]+),")', + ); expect(reviewScanJob).toContain('statusCheckRollup'); expect(reviewScanJob).toContain('HAS_PENDING_CHECKS'); expect(reviewScanJob).toContain('N_FAILED_CHECKS'); @@ -318,11 +330,22 @@ describe('qwen-autofix workflow', () => { // but dispatches and review/issue events get unique per-run groups — a // shared cancel-in-progress group let any newer event kill pending full // scans while route jobs sat queued behind runner backlog. + // Per-TARGET keys: cron ticks coalesce with each other; review events + // coalesce per PR (near-simultaneous reviews on one PR route once, without + // events on OTHER PRs cancelling this one); issue events per issue; + // dispatches unique and never cancelled. + expect(routeJob).toContain("'qwen-autofix-route-cron'"); + expect(routeJob).toContain( + "format('qwen-autofix-route-pr-{0}', github.event.pull_request.number)", + ); + expect(routeJob).toContain( + "format('qwen-autofix-route-issue-{0}', github.event.issue.number)", + ); expect(routeJob).toContain( - "group: \"${{ github.event_name == 'schedule' && 'qwen-autofix-route-cron' || format('qwen-autofix-route-{0}', github.run_id) }}\"", + "format('qwen-autofix-route-{0}', github.run_id)", ); expect(routeJob).toContain( - "cancel-in-progress: |-\n ${{ github.event_name == 'schedule' }}", + "cancel-in-progress: |-\n ${{ github.event_name != 'workflow_dispatch' }}", ); expect(routeJob).not.toContain("group: 'qwen-autofix-route'"); expect(workflow).toContain( From 13676f6f0d715b48fb3c15840c7076d4b49f66e6 Mon Sep 17 00:00:00 2001 From: wenshao Date: Sat, 18 Jul 2026 16:35:20 +0800 Subject: [PATCH 4/7] ci(autofix): discard stale duplicate targets via live-watermark revalidation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review: the busy-set closes the queued-matrix window but not the pre-matrix one — two near-simultaneous same-PR triggers can both scan before either has emitted a matrix job, so both emit the PR with the same stale watermark, and the per-PR address group QUEUES (not discards) the duplicate. That queueing is exactly what makes revalidation sound: address jobs for one PR run strictly one at a time, so when the duplicate reaches prepare, the first job's eval marker is already posted. Prepare now recomputes the watermark from LIVE markers; if it advanced past the matrix watermark and nothing (reviews / inline / issue comments / failed checks) is newer — and there is no conflict — the run marks itself stale and the address + verify steps are skipped entirely: no agent run, no marker, no comment, no push. Contract test pins the revalidation, both step gates, and the now-three shared address-carve-out sites. 50/50. --- .github/workflows/qwen-autofix.yml | 47 ++++++++++++++++++++- scripts/tests/qwen-autofix-workflow.test.js | 19 ++++++++- 2 files changed, 63 insertions(+), 3 deletions(-) diff --git a/.github/workflows/qwen-autofix.yml b/.github/workflows/qwen-autofix.yml index 657f08f0586..4c4f1114010 100644 --- a/.github/workflows/qwen-autofix.yml +++ b/.github/workflows/qwen-autofix.yml @@ -1395,6 +1395,47 @@ jobs: [[ -z "${NEWEST}" ]] && NEWEST="${WATERMARK}" echo "newest=${NEWEST}" >> "${GITHUB_OUTPUT}" + # Live-watermark revalidation: two near-simultaneous triggers for the + # SAME PR can both pass their (per-target, route-level) gates and both + # scan before either has emitted a matrix job, so both emit this PR + # with the same stale watermark. The per-PR address concurrency group + # QUEUES the duplicate rather than discarding it — but that queueing + # is exactly what makes this check sound: address jobs for one PR run + # strictly one at a time, so by the time the duplicate runs here, the + # first job's eval marker is posted and visible. Recompute the + # watermark from LIVE markers; if nothing is newer and there is no + # conflict, this run is a stale duplicate and discards itself. + STALE='false' + LIVE_EVAL_WM="$(jq -r --arg ab "${AUTOFIX_BOT}" ' + [ .[] | select((.user.login // "") == $ab) | (.body // "") + | [ scan("") ] | .[] + | .[0] ] | max // ""' "${WORKDIR}/ic.json")" + if [[ -n "${LIVE_EVAL_WM}" && "${LIVE_EVAL_WM}" > "${WATERMARK}" && "${CONFLICT}" != "true" ]]; then + LIVE_NEW="$(jq -rs \ + --arg wm "${LIVE_EVAL_WM}" --arg rb "${REVIEW_BOT}" --arg ab "${AUTOFIX_BOT}" \ + --argjson trust "${TRUSTED_ASSOC}" ' + ((.[0] | map(select((.submitted_at // "") > $wm) + | select((.user.login // "") != $ab) + | select(((.author_association // "") | IN($trust[])) or (.user.login // "") == $rb) + | select((.state // "") | IN("CHANGES_REQUESTED", "COMMENTED"))) | length) + + (.[1] | map(select((.created_at // "") > $wm) + | select((.user.login // "") != $ab) + | select(((.author_association // "") | IN($trust[])) or (.user.login // "") == $rb)) | length) + + (.[2] | map(select((.created_at // "") > $wm) + | select((.user.login // "") != $ab) + | select(((.author_association // "") | IN($trust[])) or (.user.login // "") == $rb) + | select((.body // "") | test("") ] | .[] - | .[0] ] | max // ""' "${WORKDIR}/ic.json")" - if [[ -n "${LIVE_EVAL_WM}" && "${LIVE_EVAL_WM}" > "${WATERMARK}" && "${CONFLICT}" != "true" ]]; then + | [ scan("") ] | .[] ]' "${WORKDIR}/ic.json")" + LIVE_EVAL_WM="$(jq -r 'map(.[0]) | max // ""' <<< "${LIVE_MARKS}")" + LIVE_MAX_ROUND="$(jq -r 'map(.[2] | tonumber) | max // 0' <<< "${LIVE_MARKS}")" + if [[ -n "${LIVE_EVAL_WM}" && "${CONFLICT}" != "true" ]] \ + && { [[ "${LIVE_EVAL_WM}" > "${WATERMARK}" ]] || [[ "${LIVE_MAX_ROUND}" -gt "${ROUND}" ]]; }; then LIVE_NEW="$(jq -rs \ --arg wm "${LIVE_EVAL_WM}" --arg rb "${REVIEW_BOT}" --arg ab "${AUTOFIX_BOT}" \ --argjson trust "${TRUSTED_ASSOC}" ' - ((.[0] | map(select((.submitted_at // "") > $wm) + (.[0] | map(select((.submitted_at // "") > $wm) | select((.user.login // "") != $ab) | select(((.author_association // "") | IN($trust[])) or (.user.login // "") == $rb) | select((.state // "") | IN("CHANGES_REQUESTED", "COMMENTED"))) | length) @@ -1449,7 +1456,7 @@ jobs: "${WORKDIR}/rv.json" "${WORKDIR}/rc.json" "${WORKDIR}/ic.json" "${WORKDIR}/checks.json")" if [[ "${LIVE_NEW}" == "0" ]]; then STALE='true' - echo "🫥 stale duplicate target: a sibling run already evaluated through ${LIVE_EVAL_WM} and nothing is newer — discarding without action or marker" + echo "🫥 stale duplicate target: a sibling run already evaluated through ${LIVE_EVAL_WM} (round ${LIVE_MAX_ROUND}) and nothing is newer — discarding without action or marker" fi fi echo "stale=${STALE}" >> "${GITHUB_OUTPUT}" diff --git a/scripts/tests/qwen-autofix-workflow.test.js b/scripts/tests/qwen-autofix-workflow.test.js index 158fabdfb99..2b539f50ef5 100644 --- a/scripts/tests/qwen-autofix-workflow.test.js +++ b/scripts/tests/qwen-autofix-workflow.test.js @@ -300,6 +300,118 @@ describe('qwen-autofix workflow', () => { expect(reviewScanJob).toContain('could not fetch PR metadata'); }); + it('behaviorally replays the stale-duplicate revalidation, including the conflict-only transition', () => { + // Extract the stale-gate VERBATIM from 'Prepare branch and feedback' + // (drift fails the test) and replay it over fixture feedback files. The + // subtle case: a conflict-only duplicate. Both scans emit the PR with + // watermark W; the first serialized job resolves the conflict, and with + // no newer feedback its marker keeps ts=W while its ROUND advances — so + // a ts-only comparison misses it. The gate must also treat + // same-ts-but-newer-round (with the conflict now cleared) as stale. + const staleGate = prepareBranchAndFeedbackStep.match( + /(STALE='false'\n[\s\S]*?echo "stale=\$\{STALE\}" >> "\$\{GITHUB_OUTPUT\}")/, + )?.[1]; + expect(staleGate).toBeTruthy(); + const W = '2026-07-18T08:00:00Z'; + const runStaleGate = ({ marks, conflict, round, reviews = [] }) => { + const dir = mkdtempSync(join(tmpdir(), 'autofix-stale-')); + try { + writeFileSync( + join(dir, 'ic.json'), + JSON.stringify( + marks.map((m) => ({ + user: { login: 'qwen-code-dev-bot' }, + created_at: '2026-07-18T09:00:00Z', + body: `eval `, + })), + ), + ); + writeFileSync(join(dir, 'rv.json'), JSON.stringify(reviews)); + writeFileSync(join(dir, 'rc.json'), '[]'); + writeFileSync(join(dir, 'checks.json'), '[]'); + const out = join(dir, 'out.txt'); + writeFileSync(out, ''); + execFileSync('bash', ['-c', staleGate.replace(/\n {10}/g, '\n')], { + env: { + ...process.env, + WORKDIR: dir, + GITHUB_OUTPUT: out, + WATERMARK: W, + ROUND: String(round), + CONFLICT: conflict, + AUTOFIX_BOT: 'qwen-code-dev-bot', + REVIEW_BOT: 'qwen-code-ci-bot', + TRUSTED_ASSOC: '["OWNER","MEMBER","COLLABORATOR"]', + }, + encoding: 'utf8', + }); + return readFileSync(out, 'utf8').includes('stale=true'); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }; + // Conflict-only duplicate: sibling resolved and marked round 3 at ts=W; + // our matrix says round 2, the conflict is now cleared → stale. + expect( + runStaleGate({ + marks: [ + { ts: W, round: 2 }, + { ts: W, round: 3 }, + ], + conflict: 'false', + round: 2, + }), + ).toBe(true); + // First job of a conflict round: round has not advanced → proceeds. + expect( + runStaleGate({ + marks: [{ ts: W, round: 2 }], + conflict: 'false', + round: 2, + }), + ).toBe(false); + // A live conflict is always actionable, even past a sibling's marker. + expect( + runStaleGate({ + marks: [ + { ts: W, round: 2 }, + { ts: W, round: 3 }, + ], + conflict: 'true', + round: 2, + }), + ).toBe(false); + // ts-advanced duplicate (the original case): sibling evaluated through a + // newer live watermark and nothing newer exists → stale. + expect( + runStaleGate({ + marks: [{ ts: '2026-07-18T08:30:00Z', round: 3 }], + conflict: 'false', + round: 2, + }), + ).toBe(true); + // Round advanced BUT trusted feedback arrived after the live watermark — + // the queued job has real work and must NOT discard itself. + expect( + runStaleGate({ + marks: [ + { ts: W, round: 2 }, + { ts: W, round: 3 }, + ], + conflict: 'false', + round: 2, + reviews: [ + { + submitted_at: '2026-07-18T08:45:00Z', + user: { login: 'doudouOUC' }, + author_association: 'MEMBER', + state: 'CHANGES_REQUESTED', + }, + ], + }), + ).toBe(false); + }); + it('falls back to existing issue backlog only when review has no target', () => { expect(issueAutofixJob).toContain("needs: ['route', 'review-scan']"); expect(issueAutofixJob).toContain('always()');