diff --git a/.github/workflows/qwen-autofix.yml b/.github/workflows/qwen-autofix.yml index b2fcf4a4083..e87c9738a5c 100644 --- a/.github/workflows/qwen-autofix.yml +++ b/.github/workflows/qwen-autofix.yml @@ -2944,6 +2944,51 @@ jobs: echo '--- feedback.md ---' cat "${WORKDIR}/feedback.md" + # The agent below runs for up to 80 minutes and the verification gate adds + # more, but nothing reaches the PR thread until "Push and report" at the + # very end: a maintainer who just engaged takeover sees silence and cannot + # tell a working round from a stuck one. The agent's output already + # streams live to the Actions log, so publish that link up front. + # Upserted by marker so one status comment per PR is EDITED each round + # (edits notify nobody) rather than stacking a new comment against a + # 100-round cap. Runs after prepare so a revalidated-away stale duplicate + # never announces a round it will not run. Best-effort: a status post that + # fails warns and continues ā€” it must never cost the round. + - name: 'Post autofix status comment' + id: 'post_status' + if: |- + ${{ steps.prepare.outputs.stale != 'true' && needs.route.outputs.dry_run != 'true' }} + env: + GITHUB_TOKEN: '${{ secrets.CI_DEV_BOT_PAT }}' + EFFECTIVE_ROUND: '${{ steps.prepare.outputs.effective_round }}' + RUN_URL: '${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}' + run: |- + set -uo pipefail + MARKER='' + ROUND_DISPLAY="${EFFECTIVE_ROUND:-${ROUND}}" + BODY="$(printf '%s\n\nšŸ”„ **AutoFix is working on this PR** ā€” round %s/%s. [Watch live progress](%s); this round posts its report here when it finishes.\n\n
\näø­ę–‡čÆ“ę˜Ž\n\nšŸ”„ **AutoFix ę­£åœØ处ē†ę­¤ PR** ā€”ā€” ē¬¬ %s/%s č½®ć€‚[ęŸ„ēœ‹å®žę—¶čæ›åŗ¦](%s)ļ¼›ęœ¬č½®ē»“ęŸåŽä¼šåœØę­¤å‘åøƒęŠ„å‘Šć€‚\n\n
' \ + "${MARKER}" "${ROUND_DISPLAY}" "${MAX_ROUNDS}" "${RUN_URL}" \ + "${ROUND_DISPLAY}" "${MAX_ROUNDS}" "${RUN_URL}")" + STATUS_ID="$(gh api "repos/${REPO}/issues/${PR}/comments" --paginate | + jq -rs --arg m "${MARKER}" --arg ab "${AUTOFIX_BOT}" \ + '[ .[][] | select((.user.login // "") == $ab) + | select((.body // "") | contains($m)) ] | last | .id // empty')" || + STATUS_ID='' + if [[ -n "${STATUS_ID}" ]]; then + gh api --method PATCH "repos/${REPO}/issues/comments/${STATUS_ID}" \ + -f body="${BODY}" > /dev/null || + echo "::warning::Failed to update the autofix status comment on PR #${PR}; continuing." + else + STATUS_ID="$(gh api "repos/${REPO}/issues/${PR}/comments" \ + -f body="${BODY}" --jq '.id')" || + { + STATUS_ID='' + echo "::warning::Failed to post the autofix status comment on PR #${PR}; continuing." + } + fi + # Hand the id to the finalize step so it does not repeat this scan. + echo "comment_id=${STATUS_ID}" >> "${GITHUB_OUTPUT}" + - name: 'Triage and address' id: 'address' # Skipped entirely for a stale duplicate target (see the live-watermark @@ -3667,3 +3712,52 @@ jobs: } > "${WORKDIR}/report.md" gh pr comment "${PR}" --repo "${REPO}" --body-file "${WORKDIR}/report.md" || echo "::warning::Failed to post handoff comment on PR #${PR}" fi + + # Flip the status comment out of "working" so a finished round never + # leaves a live-looking line behind. PATCH-only on purpose: a round that + # never posted a status (stale duplicate, dry run) must not gain one here. + # The verdict stays in the round report this job already posts; this only + # records that the round ended, and keeps the run link reachable. + # Gated on 'stale' for the same reason the announcement is: the per-PR + # concurrency group serialises duplicate address jobs, so the discarded + # one runs AFTER the real round already finalised. Ungated, it would + # overwrite that round's "finished" with its own "ended without + # publishing" and report a successful round as a failed one. An empty + # 'stale' (prepare itself crashed) still finalises ā€” that IS this job's + # round, and it is exactly the case that must not stay "working". + - name: 'Finalize autofix status comment' + if: |- + ${{ always() && steps.prepare.outputs.stale != 'true' && needs.route.outputs.dry_run != 'true' }} + env: + GITHUB_TOKEN: '${{ secrets.CI_DEV_BOT_PAT }}' + EFFECTIVE_ROUND: '${{ steps.prepare.outputs.effective_round }}' + OUTCOME: '${{ steps.verify.outputs.outcome }}' + RUN_URL: '${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}' + # The id the announcement wrote. Empty means this round never + # announced (its step was skipped, or the post itself failed) ā€” then + # no comment claims this round is working, so there is nothing to + # flip and no reason to scan for one. A previous round's comment is + # already terminal, and the next round's announcement re-PATCHes it. + STATUS_ID: '${{ steps.post_status.outputs.comment_id }}' + run: |- + set -uo pipefail + MARKER='' + if [[ -z "${STATUS_ID}" ]]; then + echo "This round posted no status comment on PR #${PR}; nothing to finalize." + exit 0 + fi + ROUND_DISPLAY="${EFFECTIVE_ROUND:-${ROUND}}" + # 'fixed'/'noop' are the two outcomes that published a round report; + # anything else means the round stopped before publishing one. + if [[ "${OUTCOME:-}" == 'fixed' || "${OUTCOME:-}" == 'noop' ]]; then + EN="$(printf 'āœ… **AutoFix round %s finished** ā€” [view run](%s). See this round'"'"'s report below.' "${ROUND_DISPLAY}" "${RUN_URL}")" + ZH="$(printf 'āœ… **AutoFix ē¬¬ %s č½®å·²å®Œęˆ** ā€”ā€” [ęŸ„ēœ‹čæč”Œ](%s)ć€‚ęœ¬č½®ęŠ„å‘Šč§äø‹ę–¹ć€‚' "${ROUND_DISPLAY}" "${RUN_URL}")" + else + EN="$(printf 'āš ļø **AutoFix round %s ended without publishing a report** ā€” [view run](%s).' "${ROUND_DISPLAY}" "${RUN_URL}")" + ZH="$(printf 'āš ļø **AutoFix ē¬¬ %s č½®ē»“ęŸä½†ęœŖ发åøƒęŠ„告** ā€”ā€” [ęŸ„ēœ‹čæč”Œ](%s)怂' "${ROUND_DISPLAY}" "${RUN_URL}")" + fi + BODY="$(printf '%s\n\n%s\n\n
\näø­ę–‡čÆ“ę˜Ž\n\n%s\n\n
' \ + "${MARKER}" "${EN}" "${ZH}")" + gh api --method PATCH "repos/${REPO}/issues/comments/${STATUS_ID}" \ + -f body="${BODY}" > /dev/null || + echo "::warning::Failed to finalize the autofix status comment on PR #${PR}; continuing." diff --git a/scripts/tests/qwen-autofix-workflow.test.js b/scripts/tests/qwen-autofix-workflow.test.js index cd020ea231e..730ce9864e5 100644 --- a/scripts/tests/qwen-autofix-workflow.test.js +++ b/scripts/tests/qwen-autofix-workflow.test.js @@ -102,7 +102,15 @@ const triageAndAddressStep = )?.[0] ?? ''; const prepareBranchAndFeedbackStep = workflow.match( - /- name: 'Prepare branch and feedback'[\s\S]*?(?=\n[ ]{6}- name: 'Triage and address')/, + /- name: 'Prepare branch and feedback'[\s\S]*?(?=\n[ ]{6}- name: 'Post autofix status comment')/, + )?.[0] ?? ''; +const postStatusCommentStep = + workflow.match( + /- name: 'Post autofix status comment'[\s\S]*?(?=\n[ ]{6}- name: 'Triage and address')/, + )?.[0] ?? ''; +const finalizeStatusCommentStep = + workflow.match( + /- name: 'Finalize autofix status comment'[\s\S]*?(?=\n[ ]{6}- name: '|$)/, )?.[0] ?? ''; const resetAutofixWorkspaceSteps = workflow.match( @@ -293,9 +301,12 @@ describe('qwen-autofix workflow', () => { // discards itself ā€” no agent run, no marker, no comment. expect(prepareBranchAndFeedbackStep).toContain('LIVE_EVAL_WM'); expect(prepareBranchAndFeedbackStep).toContain('stale duplicate target'); + // Four gates, and both status-comment steps are among them: a discarded + // duplicate must neither announce a round it will never run nor rewrite + // the status the real round already finalised. expect( workflow.split("steps.prepare.outputs.stale != 'true'").length - 1, - ).toBe(2); + ).toBe(4); expect(reviewScanJob).toContain( 'capture("^review-address \\\\((?[0-9]+),")', ); @@ -4809,6 +4820,78 @@ describe('qwen-autofix workflow', () => { ).toBe(0); }); + it('announces a working round up front and closes the same status comment', () => { + // The whole point: the live run link reaches the thread BEFORE the + // 80-minute agent step, not after it. Without this the PR is silent from + // takeover until "Push and report", so a working round and a stuck one + // look identical. + expect(postStatusCommentStep.length).toBeGreaterThan(0); + expect(postStatusCommentStep).toContain(''); + expect(postStatusCommentStep).toContain( + 'actions/runs/${{ github.run_id }}', + ); + expect(postStatusCommentStep).toContain('Watch live progress'); + // Announced only for a round that will really run, and never on a dry run. + expect(postStatusCommentStep).toContain( + "steps.prepare.outputs.stale != 'true'", + ); + expect(postStatusCommentStep).toContain( + "needs.route.outputs.dry_run != 'true'", + ); + // One comment per PR, EDITED each round: a new comment per round would + // stack up to MAX_ROUNDS of them on a managed PR. + expect(postStatusCommentStep).toContain('--method PATCH'); + expect(postStatusCommentStep).toContain('contains($m)'); + // Best-effort ā€” a failed status post warns and continues, never costs a round. + expect(postStatusCommentStep).toContain('set -uo pipefail'); + expect(postStatusCommentStep).toContain('continuing.'); + expect(finalizeStatusCommentStep).toContain('set -uo pipefail'); + expect(finalizeStatusCommentStep).toContain('continuing.'); + // Repository convention for anything posted verbatim as a PR comment. + expect(postStatusCommentStep).toContain('äø­ę–‡čÆ“ę˜Ž'); + + // Runs on every ending (including a crashed agent) so no finished round + // leaves a live-looking "working" line behind. + expect(finalizeStatusCommentStep.length).toBeGreaterThan(0); + expect(finalizeStatusCommentStep).toContain('always()'); + // ...but NOT for a discarded duplicate. The per-PR concurrency group runs + // it after the real round already finalised, so an ungated finalize would + // overwrite that round's "finished" with its own "ended without + // publishing" ā€” reporting a successful round as a failed one. + expect(finalizeStatusCommentStep).toContain( + "steps.prepare.outputs.stale != 'true'", + ); + expect(finalizeStatusCommentStep).toContain( + "needs.route.outputs.dry_run != 'true'", + ); + expect(finalizeStatusCommentStep).toContain(''); + expect(finalizeStatusCommentStep).toContain('--method PATCH'); + expect(finalizeStatusCommentStep).toContain('äø­ę–‡čÆ“ę˜Ž'); + // PATCH-ONLY: a round that never announced (stale duplicate, dry run) must + // not gain a status comment at the end. + expect(finalizeStatusCommentStep).toContain('nothing to finalize'); + expect(finalizeStatusCommentStep).not.toContain( + 'gh api "repos/${REPO}/issues/${PR}/comments" -f body=', + ); + // The announcement hands over the id it just wrote (both branches), so the + // finalize never repeats the paginated comment scan ā€” one scan per round, + // not two, on a PR that can accumulate hundreds of comments over 100 rounds. + expect(postStatusCommentStep).toContain("id: 'post_status'"); + expect(postStatusCommentStep).toContain( + 'echo "comment_id=${STATUS_ID}" >> "${GITHUB_OUTPUT}"', + ); + expect(postStatusCommentStep).toContain("--jq '.id'"); + expect(finalizeStatusCommentStep).toContain( + "STATUS_ID: '${{ steps.post_status.outputs.comment_id }}'", + ); + expect(finalizeStatusCommentStep).not.toContain('--paginate'); + // Tells a round that published a report from one that died before it. + expect(finalizeStatusCommentStep).toContain("== 'fixed'"); + expect(finalizeStatusCommentStep).toContain( + 'ended without publishing a report', + ); + }); + it('renders the whole managed fleet into the run summary', () => { // Diagnosing a stall used to mean listing bot PRs, regexing each one's eval // markers, and cross-checking checks and fork state by hand - so stalls