From acc46e58cb596d74124bb4832f2ebb49794bbb54 Mon Sep 17 00:00:00 2001 From: qqqys Date: Sun, 23 Aug 2026 00:32:36 +0000 Subject: [PATCH] fix(autofix): give the repair pass a budget it can finish in (#9691) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(autofix): give the repair pass a budget it can finish in The repair attempt ran on a hardcoded 18-minute agent budget while the primary attempt gets 120 minutes from a configurable default. Raise the repair budget to 45 minutes and carry the step and job caps that bound it. The repair attempt is handed strictly less to work with than the primary one: a deterministic rejection is an opaque check failure, not the structured review feedback the primary attempt receives, so it must first re-derive which change caused the rejection before it can amend anything. Giving that 15% of the primary budget inverted the difficulty and the allowance. Measured on four takeover PRs over nine rounds on 2026-08-21: the primary attempt reported `Autofix agent completed address-review successfully.` in 9 of 9 rounds, and the repair attempt hit `timeout (1080000ms)` in 9 of 9. Every one of those rounds discarded work the primary attempt had already finished — on #9340 a completed `origin/main` conflict resolution across three files with two mutation probes and `vitest run src/commands/review/` green at 97 files / 4335 tests. Three such rounds tripped TIMEOUT_WINDOW_CAP and parked the PR at its round cap with `autofix/needs-human`. The rejections themselves were a mix — a flaky unrelated test (#9648), a genuine defect in the PR, and a scope violation — so this is not a substitute for fixing any one of them. It is the step they all funnel through: whatever the gate rejects on, the repair attempt has to be able to finish before the round can push. Carried bounds, each preserving its documented margin: - repair step cap 20m → 55m (budget + the same 10-minute margin the primary attempt keeps, so the internal kill path still writes `agent-timeout` before the step cap fires) - review-address job cap 300m → 330m (the four long steps now sum to 305m plus the 25m setup/report reserve) - PENDING_STALE_MIN 330 → 360 (its 30-minute margin over the job cap, so a live review-address run is never aged out mid-flight) 45 minutes is deliberately a fraction of the primary budget: a repair that cannot land in 45m is a handoff, not a longer retry. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VgTjRF91xANQh6SY9YGyCf * fix(autofix): carry the raised repair bounds through sibling prose * fix(autofix): revert design-record edits outside this PR's footprint (#9691) Co-authored-by: Qwen-Coder --------- Co-authored-by: Claude Opus 5 (1M context) Co-authored-by: qwen-code-dev-bot Co-authored-by: Qwen-Coder --- .../run-autofix-review-verification.sh | 4 +-- .github/workflows/qwen-autofix.yml | 27 +++++++++++++------ scripts/tests/qwen-autofix-workflow.test.js | 12 ++++----- 3 files changed, 27 insertions(+), 16 deletions(-) diff --git a/.github/scripts/run-autofix-review-verification.sh b/.github/scripts/run-autofix-review-verification.sh index 8f3c3c5bef..4ca5790d58 100755 --- a/.github/scripts/run-autofix-review-verification.sh +++ b/.github/scripts/run-autofix-review-verification.sh @@ -119,7 +119,7 @@ reject_fix() { if [[ "${preexisting}" == 'true' ]]; then # NOT retryable: the repair agent is only allowed to amend this round's # fix, and a failure that exists without the fix is outside that boundary - # by definition — the 18-minute repair budget cannot reach it. The remedy + # by definition — the 45-minute repair budget cannot reach it. The remedy # is a base update (merge main into the branch), not a repair. echo "preexisting=true" >> "${GITHUB_OUTPUT}" elif [[ "${retryable}" == 'true' ]]; then @@ -1047,7 +1047,7 @@ fi # a DEFECT-CLAIM round only when resolved-comments.txt marks a finding # resolved-in-code whose thread is Critical-tagged or belongs to a # CHANGES_REQUESTED review (matched in rc.json/rv.json). Those rounds get a -# non-retryable rejection on all-green — the 18-minute repair pass cannot +# non-retryable rejection on all-green — the 45-minute repair pass cannot # make a nonexistent defect reproduce; the next full round re-reads the # feedback with the evidence in LAST_REJECTION and can decline or escalate # instead. Every OTHER src+test round (a refactor pinning existing diff --git a/.github/workflows/qwen-autofix.yml b/.github/workflows/qwen-autofix.yml index 784400e61b..9073e3697f 100644 --- a/.github/workflows/qwen-autofix.yml +++ b/.github/workflows/qwen-autofix.yml @@ -2057,7 +2057,7 @@ jobs: # Keep the predicate as narrow as the job's own `if:` — concurrency is # evaluated BEFORE it, so without the do_review conjunct a dispatch with # `phase: issue` + `pr_number: N` (route emits pr_number unconditionally) - # would park this skipped job in that PR's shared slot behind a 300-minute + # would park this skipped job in that PR's shared slot behind a 330-minute # address round, stalling the issue phase that `needs` it. concurrency: group: "qwen-pr-head-write-${{ needs.route.outputs.do_review == 'true' && needs.route.outputs.pr_number || github.run_id }}" @@ -2362,11 +2362,11 @@ jobs: # Pending-check staleness bound (invariant across candidate PRs, computed # once): ignore a check stuck far past any legitimate runtime. The bound # must sit ABOVE real check durations here — review-pr can take ~50m and - # a review-address JOB runs up to its 300-minute cap — so an active run + # a review-address JOB runs up to its 330-minute cap — so an active run # keeps blocking and is never aged out mid-flight (which would enqueue - # the PR against a live check and double-process the feedback). 330 holds + # the PR against a live check and double-process the feedback). 360 holds # a 30-minute margin over that cap. - PENDING_STALE_MIN=330 + PENDING_STALE_MIN=360 PENDING_CUTOFF="$(date -u -d "${PENDING_STALE_MIN} minutes ago" +%Y-%m-%dT%H:%M:%SZ)" # Repetition-guard cutoff for the stale-base update marker (invariant @@ -3445,7 +3445,7 @@ jobs: # write+ (internal) authors at scan AND address time. That is an # Full rationale → qwen-autofix.md#af-038 runs-on: '${{ (github.repository == ''QwenLM/qwen-code'' && vars.MAINTAINER_ECS_RUNNER_DISABLED != ''true'' && (github.event_name != ''pull_request'' && github.event_name != ''pull_request_review'' || github.event.pull_request.head.repo.full_name == github.repository || contains(fromJSON(''["OWNER","MEMBER","COLLABORATOR"]''), github.event.pull_request.author_association))) && fromJSON(''["self-hosted", "linux", "x64", "ecs-qwen"]'') || fromJSON(''["ubuntu-latest"]'') }}' - timeout-minutes: 300 + timeout-minutes: 330 permissions: contents: 'read' strategy: @@ -3702,7 +3702,7 @@ jobs: ;; esac # The label routing pins ecs-qwen, but a mis-labelled registration - # must not silently claim a PAT-bearing 300-minute job — assert the + # must not silently claim a PAT-bearing 330-minute job — assert the # pool by name on the self-hosted branch too. if [[ "${RUNNER_ENVIRONMENT}" == 'self-hosted' ]]; then case "${RUNNER_NAME}" in @@ -4946,7 +4946,10 @@ jobs: # the CLI without any sandbox (#9527 review). if: |- ${{ always() && steps.verify.outputs.retryable == 'true' && steps.sandbox_image.outcome == 'success' }} - timeout-minutes: 20 + # 55m: the 45-minute budget below plus the same 10-minute margin the + # primary attempt keeps under its own backstop, so the internal kill + # path still writes `agent-timeout` before the step cap fires. + timeout-minutes: 55 env: QWEN_SANDBOX_IMAGE: '${{ steps.sandbox_image.outputs.image }}' DOCKER_HOST: '' @@ -4958,7 +4961,15 @@ jobs: OPENAI_MODEL: '${{ vars.QWEN_AUTOFIX_MODEL || vars.QWEN_PR_REVIEW_MODEL }}' NO_PROXY: '127.0.0.1,localhost,::1' QWEN_HOME: '${{ runner.temp }}/qwen-autofix-review-home' - QWEN_TIMEOUT_MS: '1080000' + # 45m. The repair attempt starts from a deterministic rejection — + # an opaque check failure, not the structured review feedback the + # primary attempt is handed — so it must re-derive which change + # caused it before it can amend anything. At the previous 18m it + # ran out mid-diagnosis on every observed rejection while the + # primary attempt (120m) had already succeeded, and the round's + # work was discarded. Still a fraction of the primary budget: a + # repair that cannot land in 45m is a handoff, not a longer retry. + QWEN_TIMEOUT_MS: '2700000' CONFLICT: '${{ steps.prepare.outputs.conflict }}' BASE: 'main' SETTINGS_JSON: |- diff --git a/scripts/tests/qwen-autofix-workflow.test.js b/scripts/tests/qwen-autofix-workflow.test.js index 64bc9cf90d..15ceb42e05 100644 --- a/scripts/tests/qwen-autofix-workflow.test.js +++ b/scripts/tests/qwen-autofix-workflow.test.js @@ -597,9 +597,9 @@ describe('qwen-autofix workflow', () => { expect(reviewScanJob).toContain('echo "targets=[]" >> "${GITHUB_OUTPUT}"'); expect(reviewScanJob).toContain('active checks in flight; skipping until'); // Staleness bound must sit above legitimate check runtimes (a review-address - // job runs up to its 300-minute cap) so an active run is never aged out + // job runs up to its 330-minute cap) so an active run is never aged out // mid-flight. - expect(reviewScanJob).toContain('PENDING_STALE_MIN=330'); + expect(reviewScanJob).toContain('PENDING_STALE_MIN=360'); // The staleness filter itself, including the comparison operator: a check only // blocks if its start is newer than the cutoff. Asserting `> $cut` too means a // flipped comparison (which would age out live checks → double-processing) is @@ -4178,7 +4178,7 @@ describe('qwen-autofix workflow', () => { // No rollup entries → dispatchable. expect(runMarkerCheck([])).toBe('pass'); - // A stranded marker must NOT keep blocking through the 330-minute + // A stranded marker must NOT keep blocking through the 360-minute // HAS_PENDING_CHECKS gate after its TTL expired: replay the gate's jq // over fixture rollups. const pendingGate = reviewScanJob.match( @@ -4228,7 +4228,7 @@ describe('qwen-autofix workflow', () => { checkRun('build', 'IN_PROGRESS', '2026-08-17T07:50:00Z'), ]), ).toBe('true'); - // ...a check stuck past the 330-minute horizon is aged out... + // ...a check stuck past the 360-minute horizon is aged out... expect( runPendingGate([ checkRun('build', 'IN_PROGRESS', '2026-08-17T01:00:00Z'), @@ -14672,9 +14672,9 @@ exit 1 expect(repairDeterministicRejectionStep).toContain( "steps.verify.outputs.retryable == 'true'", ); - expect(repairDeterministicRejectionStep).toContain('timeout-minutes: 20'); + expect(repairDeterministicRejectionStep).toContain('timeout-minutes: 55'); expect(repairDeterministicRejectionStep).toContain( - "QWEN_TIMEOUT_MS: '1080000'", + "QWEN_TIMEOUT_MS: '2700000'", ); const settingsJson = (step) => step.match(/SETTINGS_JSON: \|-\n([\s\S]*?)\n {8}run: \|-/)?.[1] ?? '';