Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 99 additions & 1 deletion .github/scripts/ci-runner-routing.test.mjs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// Runner-routing regression guards for ci.yml and serve-ab.yml.
// Runner-routing regression guards for ci.yml, serve-ab.yml, and the
// qwen-autofix.yml scan lane.
//
// classify_pr carries the routing logic TWICE — the `runs-on` expression
// (which selects the classify job's own runner) and the `pick_runner` shell
Expand Down Expand Up @@ -68,6 +69,13 @@ function evalRunsOn(expression, { ecsDisabled, eventName, sameRepo, assoc }) {
/github\.event_name == 'merge_group'/,
String(eventName === 'merge_group'),
],
// Longest term first as a convention; both patterns are quote-anchored
// (the closing quote is part of each regex), so neither can match
// inside the other and the substitution order is behaviorally inert.
[
/github\.event_name != 'pull_request_review'/,
String(eventName !== 'pull_request_review'),
],
[
/github\.event_name != 'pull_request'/,
String(eventName !== 'pull_request'),
Expand All @@ -80,6 +88,7 @@ function evalRunsOn(expression, { ecsDisabled, eventName, sameRepo, assoc }) {
/contains\(fromJSON\('\["OWNER","MEMBER","COLLABORATOR"\]'\), github\.event\.pull_request\.author_association\)/,
String(TRUSTED.includes(assoc)),
],
[/github\.repository == 'QwenLM\/qwen-code'/, 'true'],
];
let expr = expression.replace(/^\$\{\{\s*/, '').replace(/\s*\}\}$/, '');
for (const [term, value] of substitutions) {
Expand Down Expand Up @@ -421,3 +430,92 @@ describe('serve-ab.yml runner routing', () => {
);
});
});

describe('qwen-autofix.yml scan-lane runner routing', () => {
// route and review-scan gate the WHOLE fan-out: while they sit queued no
// review-address leg starts. A hosted-runner backlog queued them past the
// cron period, and the cron supersede rule then starved every scan round
// (2026-08-25) — so pin the lane on the persistent pool, with the
// fork-trust clause and the kill-switch intact.
const autofixDoc = parse(
readFileSync(join(workflowsDir, 'qwen-autofix.yml'), 'utf8'),
);
// evalRunsOn unwraps the winning fromJSON label to the array it names, so
// compare against arrays, not the ECS/HOSTED string constants above.
const ECS_LABELS = ['self-hosted', 'linux', 'x64', 'ecs-qwen'];
const HOSTED_LABELS = ['ubuntu-latest'];

for (const jobName of ['route', 'review-scan']) {
const runsOn = String(autofixDoc.jobs[jobName]['runs-on']);

it(`${jobName} reaches the persistent pool on schedule, dispatch, issue_comment, and issues`, () => {
// issue_comment is route's /takeover and /retry lane, issues its
// label/assign trigger lane for issue-autofix — pin both beside the
// cron and dispatch triggers so a later event-allowlist narrowing of
// the pool clause cannot silently demote either back to hosted.
for (const eventName of [
'schedule',
'workflow_dispatch',
'issue_comment',
'issues',
]) {
assert.deepEqual(
evalRunsOn(runsOn, {
ecsDisabled: false,
eventName,
sameRepo: false,
assoc: '',
}),
ECS_LABELS,
`${jobName} must scan from the pool on ${eventName}`,
);
}
});

it(`${jobName} keeps untrusted fork PR lanes hosted`, () => {
for (const eventName of ['pull_request', 'pull_request_review']) {
assert.deepEqual(
evalRunsOn(runsOn, {
ecsDisabled: false,
eventName,
sameRepo: false,
assoc: 'NONE',
}),
HOSTED_LABELS,
`${jobName} fork lane (${eventName}) must stay hosted`,
);
assert.deepEqual(
evalRunsOn(runsOn, {
ecsDisabled: false,
eventName,
sameRepo: true,
assoc: 'NONE',
}),
ECS_LABELS,
`${jobName} same-repo lane (${eventName}) must reach the pool`,
);
}
});

it(`${jobName} obeys the kill-switch on every event`, () => {
for (const eventName of [
'schedule',
'workflow_dispatch',
'issue_comment',
'pull_request',
'pull_request_review',
]) {
assert.deepEqual(
evalRunsOn(runsOn, {
ecsDisabled: true,
eventName,
sameRepo: true,
assoc: 'OWNER',
}),
HOSTED_LABELS,
`kill-switch must win on ${eventName}`,
);
}
});
}
});
2 changes: 1 addition & 1 deletion .github/workflows/.size-baseline
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
6495 pr-self-report-label.yml
9646 qwen-autofix-fork-bridge.yml
5942 qwen-autofix-fork-signal.yml
397656 qwen-autofix.yml
404284 qwen-autofix.yml

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.

Non-blocking nit: this records 404284, but qwen-autofix.yml at the head commit is 405,204 bytes — the final "fail loud when the scan lane's gh config dir cannot be minted" commit grew the file by 920 bytes without re-bumping (verified via the contents API: 404284 at both 590c32a and the merge 4bcd1ca, 405204 at 36f880b). Well within the 4,096-byte growth allowance, so the ratchet stays green — but since the previous commit made this baseline exact again, a re-bump to 405204 would keep it tight and preserve the full allowance headroom for the next PR that touches this file.

非阻断小问题:这里记录的是 404284,但 head 提交上 qwen-autofix.yml 实际为 405,204 字节——最后一个"fail loud"提交让文件涨了 920 字节却没有同步 bump(经 contents API 核实:590c32a 与合并提交 4bcd1ca 均为 404284,36f880b 为 405204)。远在 4,096 字节增长容差之内,ratchet 保持绿色——但既然前一个提交刚把基线调回精确值,顺手 re-bump 到 405204 能保持基线收紧,也给下一个改这个文件的 PR 留足容差。

7061 qwen-ci-flaky-rerun.yml
158010 qwen-code-pr-review.yml
79041 qwen-fleet-shepherd.yml
Expand Down
41 changes: 41 additions & 0 deletions .github/workflows/qwen-autofix.md
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,7 @@ task-oriented guides — what a maintainer types and what happens next — see:
- [145. review-address · Report dry-run / failure — CUMULATIVE timeout breaker — the sibling of the consecutive one above, for the…](#af-145)
- [146. review-address · Report dry-run / failure — The agent committed (verify recorded committed=true before any gate could fail),…](#af-146)
- [147. review-address · Report dry-run / failure — Same byte-budget hygiene as the English excerpt above. 3000 bytes ≈ 1000 CJK…](#af-147)
- [148. route — Persistent pool, not hosted: a hosted backlog queued route past the cron period, and af-005's…](#af-148)

---

Expand Down Expand Up @@ -3727,3 +3728,43 @@ forbids HTML in failure.zh.md), but must not be able to open
or close a <details>/<summary> that swallows the closing tag
the workflow emits below.
```

<a id="af-148"></a>

### 148. route — Persistent pool, not hosted: a hosted backlog queued route past the cron period, and af-005's supersede then starved every scan round.

In `route` and `review-scan`.

```text
route and review-scan run on the persistent pool, not the
hosted one. They are short trusted base-repo jobs, but they
gate the WHOLE fan-out: while they sit queued, no
review-address leg starts. On 2026-08-25 a hosted-runner
backlog queued route for over 20 minutes — longer than the
cron period — and af-005's newer-tick-supersedes-older rule
then cancelled every still-queued round: nine consecutive
schedule runs died without scanning while the ecs-qwen pool
stood mostly idle. The supersede rule stays — it is right
once route gets a runner in seconds; the fix is taking the
hosted queue out of the critical path. review-scan moves
with it because it shares the gate, and its own hosted waits
delayed every fan-out by the same backlog. Both keep the
fork-trust clause of the sibling lanes: pull_request and
pull_request_review resolve this file from the PR's own
merge commit, so only same-repo heads and write-access
authors may reach the persistent pool; everything else stays
hosted, and the kill-switch wins everywhere. Neither job
checks code out — route only decides phases, and review-scan
only calls the API — so the shared workspace needs no
restore or wipe step here. The pool still leaves two marks
on the lane, both closed in the same change. One: gh reads
its config from the shared, attacker-writable $HOME, so both
steps carry the heavy jobs' gh hardening preamble — a planted
~/.config/gh/config.yml could reroute their gh calls into a
local socket, taking CI_DEV_BOT_PAT with the scan and forged
collaborator-permission answers with route. Two: review-scan
fills a per-run WORKDIR with API dumps that no VM teardown
removes on the pool, so it gets a fixed autofix* per-run path
(the age sweep can reclaim it after a hard kill), an EXIT
trap, and an always() cleanup step mirroring issue-autofix.
```
75 changes: 70 additions & 5 deletions .github/workflows/qwen-autofix.yml
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,10 @@ jobs:
# Full rationale → qwen-autofix.md#af-004
if: |-
${{ github.repository == 'QwenLM/qwen-code' && (github.event_name != 'issue_comment' || (github.event.issue.pull_request && (startsWith(github.event.comment.body, '@qwen-code /takeover') || startsWith(github.event.comment.body, '@qwen-code /retry')))) && (github.event_name != 'pull_request' || github.event.label.name == 'autofix/takeover') && (github.event_name != 'pull_request_review' || github.event.pull_request.state == 'open') }}
runs-on: 'ubuntu-latest'
# Persistent pool, not hosted: a hosted backlog queued route past the cron
# period, and af-005's supersede then starved every scan round.
# Full rationale → qwen-autofix.md#af-148
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: 5
concurrency:
# Concurrency is keyed by TARGET, not shared and not fully unique:
Expand Down Expand Up @@ -256,6 +259,23 @@ jobs:
COMMENT_PR_AUTHOR: '${{ github.event.issue.user.login }}'
HAS_PR_URL: '${{ github.event.issue.pull_request.url }}'
run: |-
# gh has its own reroute channels: pin the host, drop planted tokens,
# and point gh at a fresh empty config dir — on the persistent pool
# the shared ~/.config/gh is attacker-writable (config.yml can carry
# http_unix_socket transport reroutes), and a forged collaborator-
# permission response here would open the /takeover and /retry gates.
# Mirrors the heavy jobs' preamble. Full rationale → qwen-autofix.md#af-148
export GH_HOST=github.com
unset GH_ENTERPRISE_TOKEN GH_TOKEN
# `export VAR="$(...)"` reports export's status, not the
# substitution's: a failing mktemp must abort the step, not continue
# with an empty GH_CONFIG_DIR that gh treats as unset — falling back
# to the shared ~/.config/gh this preamble closes.
if ! GH_CONFIG_DIR="$(mktemp -d "${RUNNER_TEMP}/autofix-gh-config.XXXXXX")"; then
echo "::error::could not create gh config dir; refusing to run gh without reroute hardening"
exit 1
fi
export GH_CONFIG_DIR
DO_ISSUE=false
DO_REVIEW=false
TAKEOVER_ACK=''
Expand Down Expand Up @@ -2054,7 +2074,8 @@ jobs:
needs: 'route'
if: |-
${{ needs.route.outputs.do_review == 'true' }}
runs-on: 'ubuntu-latest'
# Same pool decision as route. Full rationale → qwen-autofix.md#af-148
Comment thread
wenshao marked this conversation as resolved.
Comment thread
wenshao marked this conversation as resolved.
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: 15
# A forced scan can write the same status comment as review-address.
# Share its per-PR lock so neither writer can erase the other's state.
Expand All @@ -2072,6 +2093,11 @@ jobs:
enum_failed: '${{ steps.scan.outputs.enum_failed }}'
env:
REPO: '${{ github.repository }}'
# Per-run home for the scan's API dumps. A fixed autofix* name (not
# mktemp's tmp.*) so the age sweep in the heavy jobs can reclaim it
# after a hard runner kill; cleaned normally by the always() step.
# Full rationale → qwen-autofix.md#af-148
WORKDIR: '/tmp/autofix-scan-${{ github.run_id }}'

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] Nothing pins this value — specifically the autofix* prefix that is the load-bearing contract with the pool's age sweep (find /tmp -maxdepth 1 -name 'autofix*' -mmin +1440, lines 752/3656). autofix-scan appears nowhere in scripts/tests/qwen-autofix-workflow.test.js, while the sibling pool jobs' WORKDIR values are pinned (lines 10232/10237). Mutate the prefix to /tmp/scan- and every existing assertion stays green (the EXIT trap, the umask/mkdir line and the cleanup step all reference ${WORKDIR} symbolically, so normal-exit cleanup still works) — but after a hard runner kill, the one case the in-step trap and the always() step cannot cover per this diff's own comment, the sweep never matches the abandoned dir and scan dumps accumulate on the pool indefinitely. Probe-verified: the prefix mutant ships the full suite green; the pin below flips it red. Add beside the other scan-lane pins in scripts/tests/qwen-autofix-workflow.test.js:

expect(reviewScanJob).toContain("WORKDIR: '/tmp/autofix-scan-${{ github.run_id }}'");
中文说明

没有任何钉扎固定这个值——尤其是 autofix* 前缀,它是与池上寿命清扫(find /tmp -maxdepth 1 -name 'autofix*' -mmin +1440,第 752/3656 行)之间的承重契约。scripts/tests/qwen-autofix-workflow.test.js 中没有出现任何 autofix-scan,而兄弟池作业的 WORKDIR 值都有钉扎(10232/10237 行)。把前缀变异成 /tmp/scan-,所有既有断言仍然全绿(EXIT trap、umask/mkdir 行与清理步骤都以符号方式引用 ${WORKDIR},正常退出时的清理依旧有效)——但在 runner 被硬杀后(按本 diff 自己的注释,这是步骤内 trap 与 always() 步骤唯一覆盖不到的情形),清扫永远匹配不到被遗弃的目录,扫描转储会在池上无限累积。探针验证:前缀变异体下整套测试仍绿;加上下方钉扎后变红。加在 scripts/tests/qwen-autofix-workflow.test.js 其他扫描车道钉扎旁:

(修复代码见上方代码块。)

— qwen3.8-max via Qwen Code /review (v0.22.0)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Applied in f9e56f4 — the exact value WORKDIR: '/tmp/autofix-scan-${{ github.run_id }}' is pinned beside the other scan-lane pins, with a comment naming the autofix* prefix as the age-sweep contract. Flip-checked: the /tmp/scan- prefix mutant now fails the suite.

steps:
- name: 'Scan for PRs with new feedback'
id: 'scan'
Expand All @@ -2083,6 +2109,24 @@ jobs:
REVIEW_SENDER: '${{ needs.route.outputs.review_sender }}'
DISPATCH_SOURCE: "${{ github.event_name == 'workflow_dispatch' && inputs.source || '' }}"
run: |-
# gh reroute hardening, mirroring the heavy jobs: pin the host, drop
# planted tokens, and use a fresh empty config dir — the pool's
# shared ~/.config/gh is attacker-writable, and a config.yml with an
# http_unix_socket reroute would swallow this step's gh calls,
# CI_DEV_BOT_PAT in the Authorization header included.
# Full rationale → qwen-autofix.md#af-148
export GH_HOST=github.com
unset GH_ENTERPRISE_TOKEN GH_TOKEN
# `export VAR="$(...)"` reports export's status, not the
# substitution's: a failing mktemp must abort the step, not continue
# with an empty GH_CONFIG_DIR that gh treats as unset — falling back
# to the shared ~/.config/gh this preamble closes.
if ! GH_CONFIG_DIR="$(mktemp -d "${RUNNER_TEMP}/autofix-gh-config.XXXXXX")"; then
echo "::error::could not create gh config dir; refusing to run gh without reroute hardening"
exit 1
fi
export GH_CONFIG_DIR

# Every lane that reaches this scan is supposed to hold the PAT:
# route now declines the one event GitHub is known to run without
# secrets (a fork PR's own review) before it can set do_review.
Expand All @@ -2092,17 +2136,28 @@ jobs:
exit 1
fi

# Pre-clean before create, mirroring the heavy jobs: run_id is
# public and sequential, and mkdir -p alone would succeed over a
# dir or symlink pre-planted on the shared pool /tmp — the scan's
# redirects would then land in an attacker-chosen place, and a
# planted bot-prs.json would feed the forced-PR guard below.
rm -rf "${WORKDIR}"
(umask 077; mkdir -p "${WORKDIR}")

# Fleet visibility: every per-PR decision below also records a row so
# the run summary shows the WHOLE managed fleet in one table.
# Reconstructing this by hand (list bot PRs, regex each one's eval
# markers, cross-check checks and fork state) was the only way to see
# a stall, so stalls stayed invisible until someone went looking.
FLEET_FILE="$(mktemp)"
trap 'rm -f "${FLEET_FILE}"' EXIT
# Inside WORKDIR, not mktemp's /tmp/tmp.*: on a cancelled run the
# EXIT trap never fires, and only the always() step and the heavy
# jobs' autofix* age sweep reclaim what this step leaves — neither
# can reach a tmp.* name.
FLEET_FILE="${WORKDIR}/fleet.tsv"
trap 'rm -f "${FLEET_FILE}"; rm -rf "${WORKDIR}"' EXIT
fleet_row() {
printf '%s\t%s\t%s\n' "$1" "$2" "$3" >> "${FLEET_FILE}"
}
WORKDIR="$(mktemp -d)"

read_forced_pr_meta() {
local attempt meta
Expand Down Expand Up @@ -3264,6 +3319,16 @@ jobs:
echo "targets=${TARGETS}" >> "${GITHUB_OUTPUT}"
echo "has_targets=$([[ "${COUNT}" -gt 0 ]] && echo true || echo false)" >> "${GITHUB_OUTPUT}"

# Nothing else removes the per-run WORKDIR; on the persistent pool
# every scan would leave its API dumps behind forever. always() covers
# cancellation (the in-step EXIT trap does not fire on a killed run);
# only a hard runner kill abandons the dir, and the autofix* age sweep
# in the heavy jobs reclaims it. Mirrors issue-autofix's cleanup step.
# Full rationale → qwen-autofix.md#af-148
- name: 'Clean up scan workdir'
if: 'always()'
run: 'rm -rf "${WORKDIR}"'

# ===========================================================================
# REVIEW PHASE (build) — compile the trusted-base CLI bundle ONCE per scan
# and fan it out to the address legs as an artifact. Each leg otherwise
Expand Down
Loading
Loading