Skip to content
118 changes: 100 additions & 18 deletions .github/workflows/qwen-autofix.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1829,10 +1829,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
# review-address is capped at 120m — 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).
PENDING_STALE_MIN=240
# a review-address JOB runs up to its 300-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
# a 30-minute margin over that cap.
PENDING_STALE_MIN=330
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
Expand Down Expand Up @@ -2573,7 +2574,7 @@ jobs:
if: |-
${{ needs.review-scan.outputs.has_targets == 'true' }}
runs-on: 'ubuntu-latest'
timeout-minutes: 150
timeout-minutes: 300
permissions:
contents: 'read'
strategy:
Expand All @@ -2586,7 +2587,11 @@ jobs:
# 7th PR waited 81 minutes for a slot it could have had immediately.
# 5 halves that tail while staying a real bound — the point of the cap
# is that a backlog cannot open an unbounded number of agent runs at
# once, not the specific number.
# once, not the specific number. The 300-minute job cap raises the
# worst-case hold to 5 runner-hours per slot (25 across the fleet) and
# holds the per-PR head-write concurrency group for the same window.
# The cap itself does not lengthen the queueing tail above; the raised
# 120-minute budget does, for the PRs that exhaust it.
max-parallel: 5
matrix:
target: '${{ fromJSON(needs.review-scan.outputs.targets) }}'
Expand Down Expand Up @@ -3317,7 +3322,7 @@ jobs:
echo '--- feedback.md ---'
cat "${WORKDIR}/feedback.md"

# The agent below runs for up to 80 minutes and the verification gate adds
# The agent below runs for up to 130 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
Expand Down Expand Up @@ -3374,13 +3379,38 @@ jobs:
# revalidation in prepare) — no agent run, no marker, no comment.
if: |-
${{ steps.prepare.outputs.stale != 'true' }}
# Bound the agent well below the 150-minute job timeout so a runaway agent
# fails THIS step (not the whole job), leaving the always() verify and
# report steps time to run and post a handoff. A job-level timeout would
# cancel those steps too and leave the loop silent. The 80-minute primary
# attempt plus one 20-minute repair leaves ~50 minutes for setup, two
# verification passes, and reporting.
timeout-minutes: 80
# Bound the agent below the job timeout so a runaway agent fails THIS
# step (not the whole job), leaving the always() verify and report
# steps time to run and post a handoff. A job-level timeout would
# cancel those steps too and leave the loop silent.
#
# This step timeout is the BACKSTOP for a runaway that ignores the
# agent's own timer; QWEN_TIMEOUT_MS below is the real budget.
# Invariant: budget <= backstop - margin, where the margin covers
# the internal kill path (SIGTERM, 10s grace, SIGKILL, marker write).
#
# Measured on run 30646547838:
#
# setup (12 steps, ends at 'Post autofix status comment') 5-7m
# Triage and address #8005 round 9 50m03s (its own timer)
# #8211 12m45s
# Verification gate #8211 22m48s
# push + report + finalize 3-4s
#
# Setup runs in EARLIER steps, so it never competes with the agent
# for this cap. Worst-case budget:
#
# setup 7
# Triage and address 130 (120 budget + 10 margin)
# Verification gate 60 (2.6x the measured 22m48s)
# Repair 20
# Repair verification 60
# report 3
# -------------------------------
# worst case 280 => job timeout 300, and the job runs
# on ubuntu-latest, whose own ceiling
# is 360.
timeout-minutes: 130
env:
PR: '${{ env.PR }}'
ISSUE: '${{ env.ISSUE }}'
Expand All @@ -3389,6 +3419,17 @@ 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'
# The primary attempt's real budget: 120m, with a 10-minute margin
# under the 130-minute step backstop above. The margin covers the
# internal kill path (SIGTERM, 10s grace, SIGKILL, marker write);
# if the step cap fires first, `agent-timeout` is never written and
# the report step misclassifies the round as a crash.
# QWEN_AUTOFIX_TIMEOUT_MS can only LOWER the fallback without a code
# change: the run block clamps it to the 7,200,000 ms ceiling
# (BUDGET_CAP_MS, the fallback itself), so raising the budget still
# requires editing this default, BUDGET_CAP_MS, and the step backstop,
# while a misconfigured variable degrades to a warning, not a misreport.
QWEN_TIMEOUT_MS: '${{ vars.QWEN_AUTOFIX_TIMEOUT_MS || 7200000 }}'

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The vars.QWEN_AUTOFIX_TIMEOUT_MS override has no upper clamp, so the budget↔cap invariant this PR adds a test to protect only holds for the hardcoded fallback 7200000. — Failure scenario: a maintainer sets the repo variable to a round "give it more time" value such as 9000000 (150 min); a non-empty string is truthy in GitHub expressions, so the expression evaluates to 9000000, not the fallback. run-agent.mjs:21 then arms its internal timer at 150 min, but the step's timeout-minutes: 130 kills the process first — timedOut stays false, the agent-timeout marker (run-agent.mjs:329-330) is never written, and the report step misclassifies the round as a crash, which is exactly the misreport the new comment warns about. The headroom is only 10 minutes, so any override ≥ 7,800,000 ms breaks the invariant, and the new test (which only sees the literal fallback) cannot catch it.

Recommended fix — state the ceiling next to the override (and optionally clamp in run-agent.mjs with Math.min(parsed, cap) so the marker is still written):

# The override MUST stay below 7,800,000 ms (the 130-minute step cap above);
# at or above it the step cap wins the race, agent-timeout is never written,
# and the round is reported as a crash.
QWEN_TIMEOUT_MS: '${{ vars.QWEN_AUTOFIX_TIMEOUT_MS || 7200000 }}'
中文说明

[Suggestion] vars.QWEN_AUTOFIX_TIMEOUT_MS 覆盖值没有上限钳制,因此本 PR 新增测试所保护的「预算↔上限」不变量只对硬编码回退值 7200000 成立。—— 失败场景:维护者把仓库变量设成一个取整的「多给点时间」的值,例如 9000000(150 分钟);非空字符串在 GitHub 表达式中为真,因此表达式求值为 9000000 而非回退值。run-agent.mjs:21 随即把内部计时器定为 150 分钟,但 step 的 timeout-minutes: 130 会先杀掉进程——timedOut 保持为 false,agent-timeout 标记(run-agent.mjs:329-330)永远不会写出,上报步骤便把该轮误判为崩溃,而这正是新增注释所警告的误报。余量只有 10 分钟,因此任何 ≥ 7,800,000 ms 的覆盖值都会破坏该不变量,而新测试(只能看到字面回退值)无法捕获它。

建议修复——在覆盖值旁注明上限(并可选地在 run-agent.mjs 中用 Math.min(parsed, cap) 钳制,使标记仍会写出):见上方代码块。

— qwen3.8-max-preview via Qwen Code /review

CONFLICT: '${{ steps.prepare.outputs.conflict }}'
BASE: 'main'
SETTINGS_JSON: |-
Expand Down Expand Up @@ -3441,6 +3482,35 @@ jobs:
# write-capable collaborators); keep AUTOFIX_OPENAI_API_KEY a
# low-privilege, quota-bounded, rotatable key.
git config core.hooksPath .husky
# Clamp the override to the budget ceiling: a repo variable past
# 7,200,000 ms (120m) would arm the timer past the 130-minute step
# backstop, the cap would fire first, and the round would be
# misreported as a crash. Malformed values fall back to the same
# ceiling (run-agent.mjs's own || handles the empty/NaN case).
# The {1,8} width bound keeps 10# inside int64: a 19+ digit value
# wraps negative in (( )) and slips past the comparison unclamped.
# 10# forces base-10: a zero-padded value is octal in (( )) and would
# error past the guard the same way.
# A FLOOR, not just a ceiling — and the floor guards the likelier
# mistake. Every comment here, the PR body and the operator message
# all speak in MINUTES; this one variable wants MILLISECONDS. A
# maintainer told to "raise the agent time budget" who sets
# QWEN_AUTOFIX_TIMEOUT_MS=120 arms a 120 ms timer: every round
# SIGTERMs instantly, writes agent-timeout, and reports "ran out of
# time (timeout (120ms))" until TIMEOUT_WINDOW_CAP trips and AutoFix
# stops on the PR — advising the human to raise the budget they just
# raised, with no ::warning:: anywhere in that loop. 60000 rejects
# every minutes-shaped value (1..999) and every 0/000, which the
# bare regex admitted while the message claimed positivity.
BUDGET_CAP_MS=7200000
BUDGET_FLOOR_MS=60000
if [[ ! "${QWEN_TIMEOUT_MS}" =~ ^[0-9]{1,8}$ ]] ||
(( 10#${QWEN_TIMEOUT_MS} < BUDGET_FLOOR_MS )) ||
(( 10#${QWEN_TIMEOUT_MS} > BUDGET_CAP_MS )); then
echo "::warning::QWEN_TIMEOUT_MS=${QWEN_TIMEOUT_MS} is not an integer of MILLISECONDS in [${BUDGET_FLOOR_MS}, ${BUDGET_CAP_MS}] (120 means 120ms, not 120 minutes); clamping to ${BUDGET_CAP_MS}"
QWEN_TIMEOUT_MS="${BUDGET_CAP_MS}"
fi
export QWEN_TIMEOUT_MS
# Trusted staged copy in the mirrored layout — resolves
# ../SKILL.md to the trusted staged SKILL, never the PR branch's.
node "${RUNNER_TEMP}/autofix-skill/scripts/run-agent.mjs" \
Expand All @@ -3456,6 +3526,14 @@ jobs:
if: |-
${{ always() && steps.prepare.outputs.stale != 'true' }}
continue-on-error: true
# Unbounded until now, and the largest consumer in the job (22m48s
# measured on #8211). Left unbounded it eats the job timeout, and a
# JOB timeout cancels the always() reporters — the silent round this
# design exists to prevent. Bounded here it degrades to the ordinary
# verification-failure path instead: continue-on-error keeps the job
# alive, 'Finalize verification' sees an empty outcome, falls through
# its case to exit 1, and the always() report step posts.
timeout-minutes: 60
run: |-
bash "${RUNNER_TEMP}/run-autofix-review-verification.sh"

Expand Down Expand Up @@ -3560,6 +3638,8 @@ jobs:
if: |-
${{ always() && steps.repair.outputs.attempted == 'true' }}
continue-on-error: true
# Same bound as the first pass, for the same reason.
timeout-minutes: 60
run: |-
bash "${RUNNER_TEMP}/run-autofix-review-verification.sh"

Expand Down Expand Up @@ -3686,8 +3766,10 @@ jobs:
# Salvage a race-lost push instead of discarding the run. The
# per-PR head-write concurrency group serialises THIS repo's
# workflows, but it cannot stop the PR author (or anything on the
# fork side) pushing during the agent's ~50-minute window —
# observed twice in one day (#7983, #7985): a one-shot push died
# fork side) pushing during the agent's ~120-minute window. The
# stated budget widened it from ~50m, so a race-lost push is that
# much likelier and the retry loop below stays bounded at 3 merges.
# Observed twice in one day (#7983, #7985): a one-shot push died
# `fetch first` and a full verified agent run was thrown away.
# On rejection, fetch the moved head and MERGE it into the local
# line (merge, not rebase: the agent's own conflict-resolution
Expand Down Expand Up @@ -4215,7 +4297,7 @@ jobs:
# this is usually a one-off; the last automatic attempt names
# the real fix (split the PR or raise the budget).
CAUSE="ran out of time before finishing (${AGENT_TIMEOUT})"
LAST_FIX="a human should split the PR or raise the agent time budget, then re-arm"
LAST_FIX="a human should split the PR or raise the agent time budget and its step backstop, then re-arm"
elif [[ -z "${DETAIL_FILE}" ]]; then
CAUSE="crashed before it could evaluate the feedback"
LAST_FIX="a human should take over this PR"
Expand Down Expand Up @@ -4429,7 +4511,7 @@ jobs:
# failed differently (a gate rejection landing on a window
# that already carries the cap — the exact rollout state
# of #7929/#7846).
HEADLINE="🤖 AutoFix stopped: this counting window now contains ${TIMEOUT_N} time-budget exhaustions (pushed rounds in between included; this round itself may have failed differently). That is ${TIMEOUT_N} full agent runs that pushed nothing. A human should split or reduce the PR (or raise the agent time budget), then comment \`${RETRY_COMMAND}\` to re-arm. Until then future scans will skip this PR."
HEADLINE="🤖 AutoFix stopped: this counting window now contains ${TIMEOUT_N} time-budget exhaustions (pushed rounds in between included; this round itself may have failed differently). That is ${TIMEOUT_N} full agent runs that pushed nothing. A human should split or reduce the PR (or raise the agent time budget AND its step backstop together), then comment \`${RETRY_COMMAND}\` to re-arm. Until then future scans will skip this PR."
fi
fi
fi
Expand Down
Loading
Loading