fix(ci): run the autofix scan lane on the persistent pool - #10055
Conversation
route and review-scan ran on GitHub-hosted runners. On 2026-08-25 a hosted-runner backlog queued route for over 20 minutes — longer than the cron period — so each newer scheduled tick superseded a still-queued one under the af-005 rule, and nine consecutive schedule runs died without scanning while the ecs-qwen pool stood mostly idle. No review-feedback work was dispatched for hours. Move both jobs onto the persistent pool with the exact fork-trust and kill-switch expression the heavy jobs already use; the command jobs stay hosted. Adds the af-148 rationale and routing regression pins.
|
|
|
Thanks for the PR! Re-run at Template looks good ✓ Problem: observed, not theoretical — corroborated against the Actions API on the first pass: eight consecutive scheduled Direction: aligned. This is the repo's own automation reliability, and the change stays deliberately narrow: only the two fan-out gate jobs ( Size: not applicable — no core Approach: matches what I'd have done independently, and the intervening rounds strengthened it rather than grew it: the pool-hygiene additions (gh reroute hardening, fixed per-run WORKDIR named for the age sweep) are consequences of moving onto a shared pool, not scope creep — the first review round proved both were required. The tempting alternatives — loosening the supersede rule, or moving the whole workflow — remain worse: the supersede rule is correct once routing isn't stuck in a hosted queue, and keeping the three small command jobs hosted minimizes the blast radius. Reusing the heavy workers' routing expression instead of writing a new one is the right call. Risk: no elevated risk signals — none of the Stage 1e high-risk paths are touched. The trust angle flagged on the first pass ( Moving on to code review. 🔍 中文说明感谢贡献！ 这是针对 模板完整 ✓ 问题： 已观测到，不是理论问题——首轮已通过 Actions API 核实：2026-08-25 有八轮连续定时 方向： 对齐。这是仓库自身自动化的可靠性修复，改动保持刻意收窄：只把两个扇出门控 job（ 规模： 不适用——未触及核心 方案： 与我独立想到的方案一致，且后续轮次是加固而非膨胀：池卫生改动（gh 重定向加固、按 age sweep 可回收命名的固定运行目录）是迁入共享池的必然后果，不是范围蔓延——第一轮评审已证明两者都必要。诱人替代方案——放宽取代规则或迁移整个工作流——仍然更差：路由不卡在托管队列时取代规则本身正确，三个小型命令 job 保留托管才能最小化爆炸半径。复用重型工作 job 的路由表达式而不是新写一个，是正确选择。 风险： 无升级风险信号——未触及任何 Stage 1e 高风险路径。首轮标记的信任面（ 进入代码审查。🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewRe-run at The byte-for-byte claim still checks out. I extracted both new Trust model holds under the move. WORKDIR lifecycle is coherent end to end. Job-level gh hardening precedes every gh call. Both jobs carry the heavy jobs' preamble (host pin, token unset, throwaway config dir) ahead of their first gh call, and both are wired into the shared pin loop — Size baseline is now exact. One non-blocking nit, recorded not requested: the routing matrix now covers five of the workflow's six trigger events — schedule, dispatch, issue_comment, pull_request, pull_request_review — but the TestingUnattended CI run — no PR code executed here; the evidence below is the PR's own CI on the reviewed commit, fetched via the API. The main suite Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 中文说明代码审查针对 逐字节一致的声明仍然成立。 提取两条新的 信任模型在迁移后成立。 WORKDIR 生命周期端到端自洽。 job 级 gh 加固先于每一次 gh 调用。 两个 job 都在首次 gh 调用前带上重型 job 的前置处理（钉住 host、清除令牌、一次性配置目录），并接入共享钉值循环—— 尺寸基线现在精确。 一条不阻塞的小问题，仅记录不要求修改： 路由矩阵现覆盖六个触发事件中的五个——schedule、dispatch、issue_comment、pull_request、pull_request_review——但 测试说明无人值守 CI 运行——此处未执行任何 PR 代码；以上证据来自 API 拉取的该提交自身 CI。主测试 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — an observed, corroborated incident fixed with the smallest change that can work; every claim verified at the current head, one non-blocking test-coverage nit left open. Stepping back: this is still what I would have done. The incident was real — eight consecutive scheduled rounds cancelled while the hosted queue sat backlogged, pulled from the run history on the first pass — and the response keeps resisting the two tempting overreaches: the supersede rule stays (correct once routing gets a runner in seconds), and the small command jobs stay hosted. Reusing the heavy workers' routing expression verbatim — md5-verified, not taken on faith — means the fork-trust and kill-switch semantics are inherited rather than re-implemented, and the regression pins plus the fail-loud harness make any future drift loud instead of silent. What the intervening rounds added is exactly the hardening the first review round correctly forced: the gh reroute preamble and the per-run WORKDIR lifecycle, both now pinned in tests. The two round-1 Criticals are fixed and verified by three subsequent review rounds and two human passes, and the only delta since the last human review is the test-only The one open reservation is the Stage 2 nit — no routing-matrix cell for the One note on merge state: the standing Approval is deferred until CI lands green on the reviewed commit — 中文说明信心：4/5 —— 一起已观测并核实过的事故，用最小可行改动修复；当前提交上所有声明均已核实，仅剩一条不阻塞的测试覆盖小问题。 退一步看：这仍然是我会做的方案。事故真实——首轮从运行历史中拉到托管队列拥堵时连续八轮定时扫描被取消——而改动持续克制住两个诱人的过度：取代规则保留（路由秒级拿到 runner 后它本身正确），小型命令 job 保留托管。逐字复用重型工作 job 的路由表达式——经 md5 核验，不是照单全收——fork 信任与总开关语义是继承来的而非重新实现的；回归钉加上"大声失败"守卫让未来的漂移无法静默。后续轮次补充的恰恰是第一轮评审正确要求的加固：gh 重定向前置处理与每运行一次的 WORKDIR 生命周期，两者现在都被测试钉住。第一轮的两个 Critical 已修复并经后续三轮评审与两次人工评审确认；自上次人工评审以来唯一的增量是该轮要求的、仅测试性质的 唯一未决的保留意见是 Stage 2 的小问题——路由矩阵没有 关于合并状态的一点说明：当前挂着的 批准推迟到 CI 在受审提交上变绿—— — Qwen Code · qwen3.8-max Reviewed at |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.22.0)
PR review: moving route and review-scan onto the pool exposed their gh calls to the shared, attacker-writable $HOME and left the scan's per-run WORKDIR uncleaned. Mirror the heavy jobs' gh reroute preamble in both steps, give the scan a reaper-visible WORKDIR created with umask 077 plus an always() cleanup step, extend the kill-switch test to all four events, correct the substitution-order comment, and pin the scan lane's no-checkout carve-out premise.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
.github/scripts/ci-runner-routing.test.mjs:466 — [probe] the fork-trust clause's write-access-author disjunct has no matrix cell (anchored on code unchanged since round 1)
中文说明
已审查。 建议见行内评论。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后（第 2 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| FLEET_FILE="$(mktemp)" | ||
| trap 'rm -f "${FLEET_FILE}"' EXIT | ||
| trap 'rm -f "${FLEET_FILE}"; rm -rf "${WORKDIR}"' EXIT |
There was a problem hiding this comment.
[Suggestion] The fleet file escapes every cleanup channel this diff establishes. FLEET_FILE="$(mktemp)" gives it a /tmp/tmp.* name: the extended EXIT trap is its only removal site, and the diff's own cleanup-step comment states the trap does not fire on a killed run; the new always() step removes only ${WORKDIR}; and the pool's age sweep (find /tmp -maxdepth 1 -name 'autofix*' -mmin +1440, lines 752/3656) never matches tmp.*. When af-005's supersede rule cancels an in-flight scheduled scan — the routine event class this PR responds to (nine cancelled rounds on 2026-08-25) — or a runner is hard-killed, one fleet dump is left on the shared pool runner forever. Hosted-VM teardown hid this; the pool move makes it permanent. Move the fleet file into the already-managed per-run dir so the existing trap, always() step, and age sweep all reclaim it (the later (umask 077; mkdir -p "${WORKDIR}") line then becomes redundant and can be dropped):
| FLEET_FILE="$(mktemp)" | |
| trap 'rm -f "${FLEET_FILE}"' EXIT | |
| trap 'rm -f "${FLEET_FILE}"; rm -rf "${WORKDIR}"' EXIT | |
| (umask 077; mkdir -p "${WORKDIR}") | |
| FLEET_FILE="${WORKDIR}/fleet.tsv" | |
| trap 'rm -f "${FLEET_FILE}"; rm -rf "${WORKDIR}"' EXIT |
witness: not run — needs a live runner kill/cancel; every quoted line verified in-tree (trap at 2129 is the sole removal site; the always() step at 3301 removes only ${WORKDIR}; the only sweeps, lines 752/3656, match autofix* only).
中文说明
fleet 文件逃出了本 diff 建立的每一条清理通道。FLEET_FILE="$(mktemp)" 让它得到 /tmp/tmp.* 命名:扩展后的 EXIT trap 是它唯一的删除点,而 diff 自己的清理步骤注释明确写了被杀死的运行中 EXIT trap 不会触发;新的 always() 步骤只删除 ${WORKDIR};池上的寿命清扫(find /tmp -maxdepth 1 -name 'autofix*' -mmin +1440,第 752/3656 行)永远匹配不到 tmp.*。当 af-005 的取代规则取消一次进行中的定时扫描——正是本 PR 应对的常规事件类型(2026-08-25 连续九轮被取消)——或 runner 被硬杀时,就会在共享池 runner 上永久留下一个 fleet 转储。托管 VM 的销毁掩盖了这一点;迁入池后它变成永久性的。把 fleet 文件挪进已被管理的每运行目录,现有的 trap、always() 步骤和寿命清扫就都能回收它(后面的 (umask 077; mkdir -p "${WORKDIR}") 一行随即冗余,可删)。
证据:未运行——需要真实的 runner 杀进程/取消;所有引用行已在树内核实(2129 行的 trap 是唯一删除点;3301 行的 always() 步骤只删 ${WORKDIR};唯一的清扫在 752/3656 行,只匹配 autofix*)。
— qwen3.8-max via Qwen Code /review (v0.22.0)
There was a problem hiding this comment.
Valid — the EXIT trap was the fleet file's only removal site, and neither the always() step nor the autofix* age sweep can reach a mktemp tmp.* name. Fixed in f9e56f4: FLEET_FILE now lives at ${WORKDIR}/fleet.tsv so the trap, the always() step, and the age sweep all reclaim it (the mkdir moved up accordingly and the later duplicate was dropped). The suite pins the new home and rejects the mktemp form, and the fleet summary-replay test anchors on the new line.
| printf '%s\t%s\t%s\n' "$1" "$2" "$3" >> "${FLEET_FILE}" | ||
| } | ||
| WORKDIR="$(mktemp -d)" | ||
| (umask 077; mkdir -p "${WORKDIR}") |
There was a problem hiding this comment.
[Suggestion] The fixed per-run path is created without the pre-clean its own siblings use. mkdir -p succeeds over a pre-existing path and follows symlinks, and github.run_id is public and sequential — af-148 itself states shared pool paths are attacker-writable. A co-tenant process on the pool can pre-plant /tmp/autofix-scan-<run_id> as a symlink before the scan step, and the scan's > redirects then write its CI_DEV_BOT_PAT-fetched API dumps into the attacker-chosen directory (probe-verified: mkdir -p exits 0 over a symlink-to-dir and the write lands in the link target). A pre-created plain directory survives too, and its planted bot-prs.json is consumed by the forced-PR -f guard at line 2491 — with a planted updatedAt past the 24h cutoff the forced candidate enters IDLE_PRS and, in 3 of 4 rotation slots, the forced scan emits targets=[] silently. The deleted mktemp -d was structurally immune; issue-autofix (line 743) and review-address (line 3647) both run rm -rf "${WORKDIR}" before the identical umask/mkdir for exactly this reason. Mirror the siblings:
| (umask 077; mkdir -p "${WORKDIR}") | |
| rm -rf "${WORKDIR}" | |
| (umask 077; mkdir -p "${WORKDIR}") |
中文说明
这个固定的每运行路径在创建时缺少其兄弟作业自带的预清理。mkdir -p 对已存在的路径会成功且会跟随符号链接,而 github.run_id 公开且顺序递增——af-148 自己也写明池上共享路径可被攻击者写入。池上的同租户进程可以在扫描步骤之前把 /tmp/autofix-scan-<run_id> 预植为符号链接,扫描的 > 重定向随后把用 CI_DEV_BOT_PAT 拉取的 API 转储写进攻击者选定的目录(探针验证:mkdir -p 对指向目录的符号链接返回 0,写入落在链接目标里)。预建的普通目录同样存活,其中植入的 bot-prs.json 会被第 2491 行强制 PR 路径的 -f 守卫读走——植入超过 24 小时阈值的 updatedAt 会把强制候选送进 IDLE_PRS,在四个轮换槽中的三个里,强制扫描会静默地输出 targets=[]。被删除的 mktemp -d 在结构上免疫;issue-autofix(743 行)与 review-address(3646 行)正是因为这个原因,在相同的 umask/mkdir 之前先跑 rm -rf "${WORKDIR}"。对齐兄弟作业:
(修复代码见上方 suggestion 块。)
— qwen3.8-max via Qwen Code /review (v0.22.0)
There was a problem hiding this comment.
Valid — probe-confirmed: mkdir -p exits 0 over a pre-planted symlink-to-dir and over a pre-created plain dir, and the run_id path is public. Fixed in f9e56f4 mirroring issue-autofix/review-address: rm -rf "${WORKDIR}" immediately before the umask/mkdir (rm -rf removes a planted symlink itself without following it). The adjacent pre-clean+create pair is pinned so it cannot be split or dropped.
| for (const [step, firstGh] of [ | ||
| [routeStep, 'gh api "repos/${REPO}/collaborators'], | ||
| [reviewScanJob, 'gh pr view'], | ||
| ]) { |
There was a problem hiding this comment.
[Suggestion] This loop is a near-verbatim copy of the gh-hardening pin loop directly above it (~line 10739): both assert the same four pins (GH_HOST present, unset GH_ENTERPRISE_TOKEN GH_TOKEN, the exact GH_CONFIG_DIR mktemp spelling, hardening-before-first-gh ordering), so the two must be edited in lockstep. They have already drifted once — the older loop asserts expect(step.indexOf(firstGh)).toBeGreaterThan(-1) separately while this one folds existence into expect(step.indexOf(firstGh)).toBeGreaterThan(ghPin) — equivalent today, but exactly the drift this file's pinning discipline exists to prevent. The next preamble-shape change that updates only the older loop leaves the scan-lane pin stale. Append the two scan-lane entries to the existing loop's array instead and drop this second loop:
// in the existing loop's entry array (~line 10739):
// af-148: the scan lane shares the pool with those PAT steps
[routeStep, 'gh api "repos/${REPO}/collaborators'],
[reviewScanJob, 'gh pr view'],中文说明
这个循环是紧挨在它上方(~10739 行)的 gh 加固钉扎循环的近乎逐字复制:两者断言同样的四个钉(GH_HOST 存在、unset GH_ENTERPRISE_TOKEN GH_TOKEN、GH_CONFIG_DIR mktemp 的精确拼写、加固先于首个 gh 调用的顺序),因此必须同步修改。它们已经漂移过一次——旧循环单独断言 expect(step.indexOf(firstGh)).toBeGreaterThan(-1),本循环把存在性折叠进 expect(step.indexOf(firstGh)).toBeGreaterThan(ghPin)——今天等价,但这正是本文件的钉扎纪律要防止的漂移形态。下一次加固形状变更若只更新旧循环,扫描车道的钉就会过期。应把两个扫描车道条目并入既有循环的数组并删除这第二个循环:
(修复代码见上方代码块。)
— qwen3.8-max via Qwen Code /review (v0.22.0)
There was a problem hiding this comment.
Applied in f9e56f4 — the scan-lane entries are folded into the PAT-step loop and the second loop is gone. The merged body keeps the older loop's stricter shape (separate existence assertion plus the ordering check), so the scan entries got strictly stronger, and the af-148 rationale moved into the shared comment. Mutation probes stripping GH_HOST from route and from the scan step both go red.
| expect(reviewScanJob).toContain('(umask 077; mkdir -p "${WORKDIR}")'); | ||
| expect(reviewScanJob).toContain("- name: 'Clean up scan workdir'"); | ||
| expect(reviewScanJob).toContain("if: 'always()'"); |
There was a problem hiding this comment.
[Suggestion] These pins assert the cleanup step's NAME and that SOME if: 'always()' string exists in the review-scan slice, but never the step's run: command — a no-op cleanup ships green. Probe-verified: mutating the workflow's cleanup step from run: 'rm -rf "${WORKDIR}"' to run: 'true' leaves all 219 tests passing, while every scan on the pool leaves its API dumps behind forever — precisely the leak af-148's cleanup step exists to close. The heavy-job sibling test in this same file already does it right: it slices each - name: 'Clean up autofix workdir' step (lines 10250–10258) and pins BOTH if: 'always()' and rm -rf "${WORKDIR}" inside the slice. Flip-check passed: the pin below is red against the mutant, green against the original.
| expect(reviewScanJob).toContain('(umask 077; mkdir -p "${WORKDIR}")'); | |
| expect(reviewScanJob).toContain("- name: 'Clean up scan workdir'"); | |
| expect(reviewScanJob).toContain("if: 'always()'"); | |
| expect(reviewScanJob).toContain('(umask 077; mkdir -p "${WORKDIR}")'); | |
| const scanCleanupStep = | |
| reviewScanJob.match( | |
| /- name: 'Clean up scan workdir'[\s\S]*?(?=\n[ ]{2}# ==========|$)/, | |
| )?.[0] ?? ''; | |
| expect(scanCleanupStep).toContain("if: 'always()'"); | |
| expect(scanCleanupStep).toContain('rm -rf "${WORKDIR}"'); |
中文说明
这些钉断言的是清理步骤的名字、以及 review-scan 切片里存在某个 if: 'always()' 字符串,但从不断言该步骤的 run: 命令——空操作清理也能全绿通过。探针验证:把工作流清理步骤从 run: 'rm -rf "${WORKDIR}"' 变异成 run: 'true',219 个测试仍然全部通过,而池上每次扫描都会永久留下 API 转储——恰是 af-148 清理步骤要堵的泄漏。同文件中重型作业的兄弟测试已经做对了:它切出每个 - name: 'Clean up autofix workdir' 步骤(10250–10258 行),在切片内同时钉住 if: 'always()' 与 rm -rf "${WORKDIR}"。翻转检查通过:下方钉扎对变异体变红、对原始工作流保持绿。
— qwen3.8-max via Qwen Code /review (v0.22.0)
There was a problem hiding this comment.
Valid — reproduced the probe: a run: 'true' cleanup shipped the old pins green. Fixed in f9e56f4: the test slices the 'Clean up scan workdir' step (same lookahead shape as the heavy-job cleanup pins) and asserts both if: 'always()' and rm -rf "${WORKDIR}" inside the slice. Flip-checked: the no-op mutant is red, the real step green.
| # 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 }}' |
There was a problem hiding this comment.
[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)
There was a problem hiding this comment.
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.
|
@qwen-code /takeover |
qwen-code-dev-bot
added
the
autofix/takeover
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。移除 |
Address round 2: pre-clean WORKDIR before creating it (run_id is public; mkdir -p alone accepts a pre-planted dir or symlink on the shared /tmp), move the fleet file inside WORKDIR so the always() step and the autofix* age sweep reclaim it when the EXIT trap cannot, fold the scan lane's gh hardening pins into the PAT-step loop so the preamble has one edit site, and pin what a mutant could previously drop silently: the WORKDIR value's autofix* prefix, the pre-clean ordering, the fleet-file home, and the cleanup step's actual rm -rf command inside its own slice.
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Review Summary
Verdict: COMMENT — 0 Criticals, 4 Suggestions
Findings
No Criticals found in this round. All previously identified Criticals from round 1 (gh hardening preamble, WORKDIR cleanup) have been verified as addressed.
Suggestions
1. runs-on expression duplication across jobs (Agent 3a)
- The ~250-character
runs-onexpression is copied verbatim betweenrouteandreview-scan(and the heavy jobs). Consider extracting to a YAML anchor:x-ecs-runs-on: &ecs-runs-on '${{ ... }}'and referencing asruns-on: *ecs-runs-on.
2. gh hardening preamble duplication across steps (Agent 3a)
- The three-line preamble (
export GH_HOST=github.com,unset GH_ENTERPRISE_TOKEN GH_TOKEN,export GH_CONFIG_DIR=...) is duplicated betweenroute's Decide phases step andreview-scan's Scan step. Consider extracting to a shared YAML anchor.
3. EXIT trap WORKDIR cleanup not pinned in tests (Agent 5)
- The test verifies that
FLEET_FILEmoved toWORKDIRand that the old pattern is gone, but does not explicitly assert the trap line:trap 'rm -f "${FLEET_FILE}"; rm -rf "${WORKDIR}"' EXIT. Thealways()cleanup step (the primary mechanism) IS tested, so the gap is narrow.
4. pull_request_target not excluded from runs-on expression (Agent 6b, low confidence)
- The
Cterm (github.event_name != 'pull_request' && github.event_name != 'pull_request_review') does not excludepull_request_target. If that trigger is added to the workflow, a fork PR would route to the ECS pool without the fork-trust clause. The trigger is not currently in theon:section, making this a forward-looking edge case.
Not reviewed
- Build & test: skipped (lightweight mode — no local worktree available on this Windows machine)
- Cross-file tracing: skipped (lightweight mode)
- Script lint: skipped (tool limitation — actionlint embedded-shell source mapping not supported for this workflow)
Re-checks from previous rounds
- Critical (gh hardening preamble): Fixed in 3bc6ade — both
routeandreview-scancarry the heavy jobs' preamble verbatim, ahead of any gh call. - Critical (WORKDIR cleanup): Fixed in f9e56f4 — job-level
WORKDIR: /tmp/autofix-scan-${run_id}, pre-clean, extended EXIT trap, andalways()cleanup step. - All 6 Suggestions from round 1: Verified as addressed in 3bc6ade and f9e56f4.
— reviewed via Qwen Code /review (v0.21.10)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
.github/scripts/ci-runner-routing.test.mjs:466 — [probe] the fork-trust clause's positive half (trusted association) has no matrix cell — a String(false) mutation of the contains() substitution survives all 15 tests (rediscovery of the roun…
Convergence: round 3 posted 2 inline comment(s), 2 of them reported for the first time; the previous round posted 5 (5 new). Findings keep coming back to the same files: scripts/tests/qwen-autofix-workflow.test.js (findings in round 2; 1 more now). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. No Critical finding is open on this round, so merging and moving the remaining Suggestion threads to a follow-up issue is available as an ending — a merged pull request cannot diverge further. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
已审查——无阻断问题。 建议见行内评论。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后（第 3 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 3 轮发布了 2 条行内评论，其中 2 条是首次提出；上一轮发布了 5 条（其中 5 条首次提出）。发现反复回到同一批文件：scripts/tests/qwen-autofix-workflow.test.js（第 2 轮已出过发现，本轮又有 1 条）。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。本轮没有未决的 Critical，因此"合入后把剩余 Suggestion 线程转到后续 issue"是一个可选的结束方式——已合入的 PR 不会继续发散。（仅为观察——本轮评审未因此扣留任何内容。）
— qwen3.8-max via Qwen Code /review (v0.22.0)
| 9646 qwen-autofix-fork-bridge.yml | ||
| 5942 qwen-autofix-fork-signal.yml | ||
| 397656 qwen-autofix.yml | ||
| 403605 qwen-autofix.yml |
There was a problem hiding this comment.
[Suggestion] The size baseline records 403605 bytes for qwen-autofix.yml, but the file at this commit is 404284 bytes — under-recorded by 679 bytes. The number was measured at 3bc6ade, and the final commit f9e56f4 grew the workflow by 15 lines without re-bumping the baseline. Both size enforcers only fail past recorded + 4096, so this passes green — but the reviewed line then understates this PR's growth (+5,949 shown vs +6,628 actual over the prior 397656), and 679 bytes of the next PR's growth allowance are silently consumed: a follow-up growing the file by ~3.5 KB trips the ratchet's hard-fail arm 679 bytes early, with a confusing overage attributed to it. Measured at the reviewed commit: git show HEAD:.github/workflows/qwen-autofix.yml | wc -c → 404284 vs recorded 403605 (at 3bc6ade the file is exactly 403605).
| 403605 qwen-autofix.yml | |
| 404284 qwen-autofix.yml |
中文说明
尺寸基线为 qwen-autofix.yml 记录的是 403605 字节，但本提交的该文件实际为 404284 字节——少记了 679 字节。该数字是在 3bc6ade 时测量的，最终提交 f9e56f4 让工作流增长了 15 行却没有重新更新基线。两个尺寸检查器只在超过 记录值 + 4096 时才失败，所以本 PR 依然通过——但这一行因此低估了本 PR 的实际增长（相对上一基线 397656，显示 +5,949，实际 +6,628），并悄悄吃掉下一个 PR 的 679 字节增长额度：后续让该文件增长约 3.5 KB 的 PR 会提前 679 字节触发硬失败，且超额部分被错误归因于它。已在被审提交上实测：git show HEAD:.github/workflows/qwen-autofix.yml | wc -c → 404284，记录值 403605（3bc6adeb 时文件恰为 403605）。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| expect(reviewScanJob).toContain( | ||
| "WORKDIR: '/tmp/autofix-scan-${{ github.run_id }}'", | ||
| ); |
There was a problem hiding this comment.
[Suggestion] This pin covers only the producer side of the reclaim contract its own comment declares. The consumer — find /tmp -maxdepth 1 -name 'autofix*' -mmin +1440 in the heavy jobs' Reset autofix workspace steps (qwen-autofix.yml ~752 and ~3667) — is pinned by no test anywhere in the repo (verified by sweep: the only -mmin pin in any test guards a different workflow's qwen-review-tools.* prefix). So a future edit to the sweep glob (narrowing it to autofix-review*, moving it to ${RUNNER_TEMP}) ships with every test green; the two sides then disagree on identity, and after a hard runner kill — the exact path af-148 designs for, where neither the EXIT trap nor the always() step fires — /tmp/autofix-scan-<run_id> with its API dumps is never reclaimed from persistent-pool runners, re-opening the leak this PR closes. Pin the consumer side next to this pin — assert the sweep line (or at minimum that its -name pattern still matches the pinned autofix-scan-* basename) in both heavy-job slices.
中文说明
这个钉只覆盖了它自身注释所声明的回收契约的生产者一侧。消费者——重型作业 Reset autofix workspace 步骤中的 find /tmp -maxdepth 1 -name 'autofix*' -mmin +1440（qwen-autofix.yml 约 752 与 3667 行）——全仓库没有任何测试钉住（已扫描核实：所有测试中唯一的 -mmin 钉属于另一个工作流的 qwen-review-tools.* 前缀）。因此未来修改清扫 glob（收窄为 autofix-review-*、迁到 ${RUNNER_TEMP} 等）可以在所有测试绿灯的情况下合入；两侧随即失去命名一致性，而在硬杀 runner 之后——正是 af-148 设计的场景，EXIT trap 与 always() 步骤都不会触发——/tmp/autofix-scan-<run_id> 及其 API 转储将永远无法从持久池 runner 上回收，重新打开本 PR 刚堵住的泄漏。请在这个钉旁边把消费者一侧也钉上：在两个重型作业切片中断言清扫行（至少断言其 -name 模式仍匹配被钉住的 autofix-scan-* 基名）。
— qwen3.8-max via Qwen Code /review (v0.22.0)
…line Round 3 review: the reclaim-contract pin covered only the producer side (the scan WORKDIR basename). Pin the consumer — the heavy jobs' age-sweep line — inside both Reset autofix workspace step slices, so narrowing the glob or moving the sweep ships red. Also pin the scan lane's EXIT trap line (the in-step cleanup twin of the always() step), and re-bump .size-baseline to the workflow's actual 404284 bytes, which f9e56f4's 15 lines left under-recorded by 679.
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Round summary — PR #10055 (address-review)Commit: Feedback dispositionsImplemented
Declined (with reasons)
Re-verified as already resolved (prior commits)
Files changed
Verification
Note: the new assertions pin behavior already present pre-round, so they pass against the pre-round branch as well; this is a coverage-addition round (no Critical/Request-changes defect resolved in code), and the mutation probes above are the witness that the pins have teeth. 中文说明本轮小结 — PR #10055（address-review）提交： 反馈处置已实施
已拒绝（附理由）
已复核确认此前提交已解决
变更文件
验证
说明：新断言钉住的是本轮之前就已存在的行为，因此在轮前分支上同样会通过；本轮属于补充覆盖（未在代码中解决 Critical/Request-changes 级缺陷），上述变异探针即为钉扎真实有效的证据。 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not explored to full depth (tool budget reached): "agent 3a": none — all checks above ran to completion..
Not reviewed: reverse audit — stopped before round 4 by the review time budget.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Deferred under the convergence posture (round 4, not a blocker) — recorded, not requested in this round:
.github/scripts/ci-runner-routing.test.mjs:466 — [probe] fork-trust association disjunct has no decisive routing-matrix cell.github/scripts/ci-runner-routing.test.mjs:492 — [probe] routing matrix covers only 4 of the 6 workflow events (issues, issue_comment unasserted).github/workflows/qwen-autofix.yml:2138 — [review] pooled jobs' mktemp scratch files leak on cancelled/timed-out runsscripts/tests/qwen-autofix-workflow.test.js:9986 — [probe] no negative pin guards the deleted step-local WORKDIR mktemp shape
中文说明
仅完成部分审查，审查缺口已披露。
未探索到全部深度（达到工具调用预算）："agent 3a"：none — all checks above ran to completion.。
未审查：反向审计——评审时间预算不足，未能开始第 4 轮。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后（第 4 轮，非阻断）——已记录，本轮不要求修改：共 4 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： AutoFix round result: no action neededThe only feedback newer than the last evaluation is the automated reviewer's round-4 review (
No commit was made; the branch head is unchanged. 中文说明AutoFix 本轮结果：无需操作上次评估之后唯一的新反馈是自动审查者的第 4 轮审查（
未创建任何提交；分支头保持不变。 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 542 passed · 0 failed · 542 total Flakiness gate: ✅ 2 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：542 通过 · 0 失败 · 542 总计 抖动门：✅ 2 changed test file(s) x 5 identical rounds, no divergence Verification reportPR 10055 Deep Verification — fix(ci): run the autofix scan lane on the persistent poolVerdict: 中文摘要结论:
Central claim + A/B tableCentral claim: the scan lane ( A/B design: an independent GitHub-expression evaluator (
Base side: both jobs' Supporting identity claims, all scripted:
Reviewer Test Plan walkthrough
Secondary claim: pool-hygiene hardening — verbatim slice replay (18/18)The embedded bash was extracted verbatim via YAML parse from head and executed under the step's own shell contract (
Lifecycle parity with the reference pattern ( Targeted gates (all proven live before citing)Witness:
Vacuity check / mutation matrix (10 mutants, no survivors)Each mutant was applied in a scratch worktree at head, the relevant suite re-run, and the file restored. Witness:
FindingsNo blocking findings. Two informational notes, neither requiring action:
Boundary notes (not findings): (a) the Not covered
MethodologyOne CI verify container (a live sample of the lane's Flakiness gate logEvidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
Ran a correctness + security pass at ad784af5 (diff 0756be0...ad784af5). No blocking findings — one non-blocking inline note.
What I verified:
- Runner routing — the two new
runs-onexpressions are byte-identical to the heavy jobs' expression (5 identical occurrences file-wide), including theMAINTAINER_ECS_RUNNER_DISABLEDkill-switch and the fork-trust clause; the new routing tests parse the real workflow rather than a hardcoded copy. - WORKDIR lifecycle — the job-level
/tmp/autofix-scan-<run_id>env is coherent end to end: the oldmktemp -dis deleted with no shadowing re-declaration, every API dump lands under${WORKDIR}, the EXIT trap and thealways()cleanup both resolve the job env, and the heavy jobs'autofix*age-sweep glob matches the new basename for hard-kill reclaim. - gh hardening — both preambles precede the first
ghcall in their steps; afterunset GH_TOKEN GH_ENTERPRISE_TOKEN, auth stays intact via the step-levelGITHUB_TOKENenv fallback (github.tokenin route,CI_DEV_BOT_PATin scan, behind the empty-PAT guard). No expression injection: interpolations are numeric/trusted only and all quote escaping balances. - Pool hygiene parity — the pre-clean (
rm -rf+umask 077 mkdir -p) and the throwawayGH_CONFIG_DIRleak are both byte-parity with the heavy jobs' established pattern (same residual race, same bounded leak), so parity rather than regression.
CI is green on this head. The earlier ci-bot CHANGES_REQUESTED round predates the fix commits; its round-3/4 re-reviews report no blockers, consistent with what I see.
| const runsOn = String(autofixDoc.jobs[jobName]['runs-on']); | ||
|
|
||
| it(`${jobName} reaches the persistent pool on schedule and dispatch`, () => { | ||
| for (const eventName of ['schedule', 'workflow_dispatch']) { |
There was a problem hiding this comment.
[P3, non-blocking] One thing I noticed here: the loops cover schedule / workflow_dispatch / pull_request / pull_request_review, but route's gate also admits issue_comment (/takeover and /retry) — exactly the lane this PR reroutes onto the pool. The expression itself handles it correctly (the first clause is trivially true for issue_comment — verified), so this is a coverage gap only. If someone later narrows the pool clause with an event allowlist, the takeover lane could silently demote back to hosted with no routing test catching it. Might be worth adding 'issue_comment' to this loop and to the kill-switch loop below.
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
LGTM — approving on the strength of my review pass at this exact head (ad784af5, details in my COMMENT review): the two new runs-on expressions are byte-identical to the heavy jobs' expression with the kill-switch and fork-trust clause intact, the WORKDIR lifecycle is coherent end to end with age-sweep reclaim coverage, and the gh reroute hardening precedes every gh call with auth intact after the token unset. No blocking findings; the one P3 (issue_comment lane missing from the routing tests) is non-blocking and can land via the takeover loop or a follow-up.
Note for merging: the standing CHANGES_REQUESTED is the ci-bot's round-1 state, which predates the fix commits — its round-3/4 re-reviews report no blockers.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 2/100 轮）。改动内容与我反驳保留之处如下： Round summaryAddressed the single non-blocking inline finding from the review at Feedback points and dispositions
Mutation probeThe round adds test iterations, not production guards, so the witness check is that the new iterations actually bind to the real expression. Probe: temporarily flipped Conflict notes
Verification
Commit: 中文说明本轮摘要处理了 反馈点及处理
变异探针（mutation probe）本轮只新增测试迭代、未新增生产代码守卫，因此见证检查的重点是新迭代确实绑定到真实表达式。探针：仅将 冲突说明
验证
提交： Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
|
@qwen-code /triage |
|
有一个需要阻塞合并修复的问题（复核基于当前 head
建议两处都对齐本文件 6042 行附近已有的安全写法：先检查赋值结果，失败时输出 除此之外，我跑过 merge-tree、diff-check、actionlint、runner routing 15/15 以及改动相关 workflow 测试，均通过；完整 suite 唯一的 timeout 在 |
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
|
🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Deferred under the convergence posture (round 5, not a blocker) — recorded, not requested in this round:
.github/workflows/qwen-autofix.yml:2112 — [review] GH_CONFIG_DIR mktemp dirs under RUNNER_TEMP are never removed
中文说明
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后（第 5 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| # Mirrors the heavy jobs' preamble. Full rationale → qwen-autofix.md#af-148 | ||
| export GH_HOST=github.com | ||
| unset GH_ENTERPRISE_TOKEN GH_TOKEN | ||
| export GH_CONFIG_DIR="$(mktemp -d "${RUNNER_TEMP}/autofix-gh-config.XXXXXX")" |
There was a problem hiding this comment.
[Critical] R5-1: The gh reroute hardening this step just gained can be silently disabled by a failing mktemp. export GH_CONFIG_DIR="$(mktemp -d ...)" reports export's exit status (0), not the command substitution's — verified under bash -e (the default shell here; the step sets no shell: key): a failing mktemp leaves GH_CONFIG_DIR empty and the script continues. gh treats an empty GH_CONFIG_DIR as unset and falls back to $HOME/.config/gh — on the persistent pool, the shared, attacker-writable config surface this preamble exists to close. So whenever the temp dir cannot be created (disk pressure, RUNNER_TEMP unavailable), the reroute hole re-opens silently: the collaborator-permission lookups below gate /takeover and /retry, so a forged answer admits an unauthorized commenter; the scan twin of this preamble carries CI_DEV_BOT_PAT in its Authorization header. Witness (probed on gh 2.45.0): bash -ec 'export GH_CONFIG_DIR="$(mktemp -d /nonexistent/XXXXXX)"; echo exit=$? value=[${GH_CONFIG_DIR}]' printed exit=0 value=[] (the step would continue), and with a $HOME config planted with prompt: disabled, GH_CONFIG_DIR= gh config get prompt printed disabled (empty value fell back to the HOME config) while a fresh empty GH_CONFIG_DIR printed enabled (planted config ignored). This file already holds the safe shape at lines ~6042-6046 — check the assignment, fail loudly, then export — apply it here and in review-scan:
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_DIRFix witness: extend the suite so the step must abort before any gh call when the mktemp assignment fails — removing the guard must turn that pin red. This re-checks, and confirms still open at this head, the blocker @qqqys raised in the PR thread.
中文说明
本步骤新增的 gh 重路由加固可能被一次失败的 mktemp 悄悄废除。export GH_CONFIG_DIR="$(mktemp -d ...)" 返回的是 export 的退出状态（0）而非命令替换的状态——已在 bash -e（此处的默认 shell，该步骤未设置 shell:）下验证：mktemp 失败时 GH_CONFIG_DIR 为空且脚本继续执行。gh 把空的 GH_CONFIG_DIR 按未设置处理，回退到 $HOME/.config/gh——在持久池上，这正是该前置要关闭的、共享且可被攻击者写入的配置面。因此只要临时目录创建失败（磁盘压力、RUNNER_TEMP 不可用），重路由漏洞就会悄悄重新打开：下文的协作者权限查询是 /takeover 与 /retry 的闸门，伪造的应答会放行未授权评论者；该前置在扫描步骤中的孪生版本还在 Authorization 头中携带 CI_DEV_BOT_PAT。见证（在 gh 2.45.0 上探针验证）：失败的 mktemp 以 exit=0、空值继续执行；植入 prompt: disabled 的 HOME 配置后，GH_CONFIG_DIR= gh config get prompt 读到植入值（disabled），而全新的空 GH_CONFIG_DIR 忽略它（enabled）。本文件约 6042-6046 行已有安全写法——先检查赋值、失败即大声报错退出、成功后再 export——请在本处与 review-scan 都改用（代码块见英文部分）。修复见证：扩展测试套件，要求 mktemp 赋值失败时步骤必须在任何 gh 调用之前中止——删除该守卫必须使该钉变红。本条复核并确认 @qqqys 在 PR 讨论中提出的阻塞问题在当前 head 仍然存在。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| # 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 GH_CONFIG_DIR="$(mktemp -d "${RUNNER_TEMP}/autofix-gh-config.XXXXXX")" |
There was a problem hiding this comment.
[Critical] R5-1 (second location): the same defect as the route copy of this preamble — export GH_CONFIG_DIR="$(mktemp -d ...)" masks a failing mktemp (bash reports export's status, 0), leaving GH_CONFIG_DIR empty; gh then falls back to the shared, attacker-writable $HOME/.config/gh. This step is the higher-value target: its gh calls carry Authorization: token <CI_DEV_BOT_PAT> (step env GITHUB_TOKEN), and the autofix loop trusts that bot identity unconditionally — a planted http_unix_socket reroute exfiltrates it whenever the temp dir cannot be created. Probed on gh 2.45.0: GH_CONFIG_DIR= gh config get prompt read the planted HOME config (disabled), while a fresh empty GH_CONFIG_DIR ignored it (enabled); bash -ec showed the failed mktemp continuing with exit 0 and an empty value. Fix both steps with this file's existing safe shape (~6042-6046): check the assignment, ::error + exit 1 on failure, then export GH_CONFIG_DIR, and pin that no gh call runs when mktemp fails — removing the guard must turn the pin red.
中文说明
R5-1（第二处）：与该前置在 route 中的副本同一缺陷——export GH_CONFIG_DIR="$(mktemp -d ...)" 会掩盖 mktemp 的失败（bash 返回 export 的状态 0），GH_CONFIG_DIR 留空后 gh 回退到共享且可被攻击者写入的 $HOME/.config/gh。本步骤是价值更高的目标：其 gh 调用携带 Authorization: token <CI_DEV_BOT_PAT>（步骤 env GITHUB_TOKEN），而自动修复循环对该机器人身份的信任是无条件的——只要临时目录创建失败，植入的 http_unix_socket 改道即可将其泄露。已在 gh 2.45.0 上探针验证：GH_CONFIG_DIR= gh config get prompt 读到植入的 HOME 配置（disabled），全新的空 GH_CONFIG_DIR 则忽略它（enabled）；bash -ec 显示失败的 mktemp 以 exit 0 继续、变量为空。请用本文件已有的安全写法（约 6042-6046 行）修复两个步骤：先检查赋值、失败时 ::error + exit 1、再 export GH_CONFIG_DIR，并钉住 mktemp 失败时不会执行任何 gh 调用——删除该守卫必须使钉变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| for (const jobName of ['route', 'review-scan']) { | ||
| const runsOn = String(autofixDoc.jobs[jobName]['runs-on']); | ||
|
|
||
| it(`${jobName} reaches the persistent pool on schedule, dispatch, and issue_comment`, () => { |
There was a problem hiding this comment.
[Suggestion] R5-3: This routing matrix covers five of the workflow's six triggers — the issues lane, which route's if: admits (its concurrency group even carries an issues clause) and which this PR reroutes onto the persistent pool, has no pin in either test file. A plain allowlist rewrite of the pool clause stays loud (the fail-loud guard fires), but the forced companion edit — modeling the new == terms in the substitution table — passes all 15 tests while issues is silently demoted back to hosted, re-entering exactly the hosted backlog this PR exists to escape. Probed by mutant: with the three new terms modeled and issues unpinned the suite is green and issues resolves to hosted by construction; adding 'issues' to the pool-reach loop turns that mutant red (must scan from the pool on issues).
for (const eventName of [
'schedule',
'workflow_dispatch',
'issue_comment',
'issues',
]) {Fix witness: this same test — with 'issues' in the loop, an allowlist-narrowing of the pool clause fails with must scan from the pool on issues.
中文说明
R5-3：该路由矩阵覆盖了工作流六个触发器中的五个——route 的 if: 接受 issues 通道（其并发组甚至有 issues 子句），本 PR 又把它重路由到持久池，但两个测试文件都没有为它设钉。把池条款直接改写成允许列表会被大声捕获（fail-loud 守卫触发），但伴随的必然修改——在替换表中为新的 == 词条建模——能让全部 15 个测试通过，同时 issues 被悄悄降级回托管池，重新落入本 PR 要逃离的托管排队。变异探针验证：为三个新词条建模且不为 issues 设钉时，套件全绿而 issues 按构造解析为托管；把 'issues' 加入"到达持久池"循环后，该变异变红（must scan from the pool on issues）。修复见证：正是本测试——循环加入 'issues' 后，池条款被收窄为允许列表时会以 must scan from the pool on issues 失败。
— qwen3.8-max via Qwen Code /review (v0.22.2)
export VAR="$(mktemp -d ...)" reports export's status, not the substitution's: under bash -e a failing mktemp continued with an empty GH_CONFIG_DIR, and gh treats that as unset — falling back to the shared, attacker-writable ~/.config/gh the route and review-scan preambles exist to close. Check the assignment, ::error + exit 1, then export, and pin the abort behaviourally: both steps run under a mktemp that refuses only the gh-config template must fail before any gh call — removing the guard turns the witness red (probed). Also pin the issues trigger in the scan-lane routing matrix: without it an allowlist narrowing of the pool clause demotes issues back to hosted while the suite stays green (probed by mutant).
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 3/100 轮）。改动内容与我反驳保留之处如下： AutoFix review round — PR #10055Addressed all three inline findings (two Critical, one Suggestion) and the Feedback dispositions[rc:3863969031] Critical R5-1 — Reproduced on this head before changing anything: under [rc:3863969041] Critical R5-1 (second location) — Same defect, same fix, same witnesses. This step's gh calls carry [ic:5424844439] @qqqys — blocking comment — FIXED by the same change The maintainer comment describes exactly the two sites above and asks for [rc:3863969056] Suggestion R5-3 — routing matrix misses the Added [rv:5031893899] review body — no action requested this round The body records only a tool limitation (actionlint embedded-shell source Changes
Mutation probes (witness verification)
Verification
Build/typecheck/lint and the focused Vitest/node-test runs above are the 中文说明AutoFix 评审轮次 — PR #10055在一批修改中处理了全部三条行内发现（两条 Critical、一条 Suggestion）以及维护者的阻塞性评论。无冲突（ 反馈处理[rc:3863969031] Critical R5-1 — 修改前已在当前 head 上复现：在 [rc:3863969041] Critical R5-1（第二处）— 同一缺陷、同一修复、同一见证。该步骤的 gh 调用在 [ic:5424844439] @qqqys — 阻塞性评论 — 已由同一修改解决 该维护者评论描述的正是上述两处位置，并要求采用安全赋值写法、补一个 [rc:3863969056] Suggestion R5-3 — 路由矩阵遗漏 在 [rv:5031893899] 评审正文 — 本轮无行动要求 正文仅记录了一项工具限制（actionlint 尚不支持 embedded-shell 源映射）和一条在收敛姿态下明确延后的条目（ 变更内容
变异探针（见证验证）
验证
以上 build/typecheck/lint 与聚焦的 Vitest/node-test 运行即本轮所需闸门；未改动任何包源码，故不适用集成或 bundle 运行；未改动任何 settings 源，故不适用 schema 重新生成。 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No blocking issues. LGTM! ✅
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Deferred under the convergence posture (round 6, not a blocker) — recorded, not requested in this round:
scripts/tests/qwen-autofix-workflow.test.js:10800 — [probe] mktemp-guard fix stops at two call sites; three PAT-bearing preambles keep the unguarded shape, pinned by the guardedMint else-branch.github/scripts/ci-runner-routing.test.mjs:501 — [probe] kill-switch loop omits the issues trigger its sibling pool-reach loop pinsscripts/tests/qwen-autofix-workflow.test.js:9987 — [probe] no negative pin against step-local WORKDIR mktemp re-introduction in review-scan
中文说明
无阻断问题。LGTM！✅
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后（第 6 轮，非阻断）——已记录，本轮不要求修改：共 3 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.2)
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Verified independently at 36f880b. Approving — one non-blocking note inline.
What I checked myself, beyond the bot rounds:
- Expression identity: extracted all five pool
runs-onlines inqwen-autofix.ymlat this head (route,review-scan, plus the three heavy jobs) — exactly one unique string, and it is byte-identical to the heavy jobs' expression onmain. Fork-trust clause, repository guard, and kill-switch are inherited, not re-implemented, so they cannot drift. - Trust model holds on the pool: neither
routenorreview-scanhas a checkout oruses:step (now pinned by the workflow tests), so no attacker-controlled code executes in these jobs. The gh reroute hardening —GH_HOSTpin, token unset, guarded throwawayGH_CONFIG_DIR— precedes the first gh call in both jobs, and the guarded mint correctly fails closed: a lone assignment insideif !preserves the substitution's exit status and is exempt frombash -e. The empty-PAT fail-fast stays as the backstop, and the pool already runs PAT-bearing jobs under the same expression, so this introduces no new attack surface. - WORKDIR lifecycle is coherent end to end: fixed
/tmp/autofix-scan-<run_id>name matches the heavy jobs'autofix*age sweep (now pinned on the consumer side too), pre-clean +umask 077before create, fleet file moved inside WORKDIR, in-step EXIT trap plus thealways()cleanup step. - Tests run locally at this head:
node --test .github/scripts/ci-runner-routing.test.mjs→ 15/15 pass;npx vitest run --config ./scripts/tests/vitest.config.ts qwen-autofix-workflow workflow-size→ 415/415 pass. PR CI is green, includingTest (ubuntu-latest, Node 22.x). - Routing helper semantics: the new
pull_request_reviewsubstitution is quote-anchored, so it cannot match inside (or be shadowed by) thepull_requestpattern; unrecognized expression terms fail loud instead of silently passing. The deferred coverage gaps from round 6 (kill-switch loop omitsissues, no negative pin against step-local WORKDIR shadowing) are consistent-by-construction today and fine as follow-ups.
中文说明
在 36f880b 上独立核实，批准 —— 另有一条非阻断的 inline 评论。
- 表达式一致性：提取 head 上全部 5 处池
runs-on（route、review-scan及三个重型 job），去重后只剩一个字符串，且与main上重型 job 的表达式逐字节一致。fork 信任条款、仓库守卫与总开关均为继承，不可能漂移。 - 池上信任模型成立：两个 job 都没有 checkout /
uses:步骤（测试已钉住），攻击者代码不会在这些 job 里执行。gh 重定向加固（钉GH_HOST、清 token、受保护的一次性GH_CONFIG_DIR）先于两个 job 的首次 gh 调用；受保护的 mint 正确做到失败即关：if !内的单独赋值保留命令替换的退出码，且不受bash -e影响。空 PAT 快速失败仍是兜底；持久池本就在同一表达式下运行携带同一 PAT 的 job，没有引入新攻击面。 - WORKDIR 生命周期端到端自洽：固定名
/tmp/autofix-scan-<run_id>匹配重型 job 的autofix*age sweep（消费端也已钉住），创建前预清理 +umask 077，fleet 文件移入 WORKDIR，步骤内 EXIT trap 加always()清理步骤。 - 本地实测：路由测试 15/15 通过；
qwen-autofix-workflow+workflow-size两个 vitest 文件 415/415 通过。PR CI 全绿，含Test (ubuntu-latest, Node 22.x)。 - 路由辅助函数语义：新增的
pull_request_review替换项带引号锚定，不会与pull_request模式互相误匹配；未识别的表达式项会大声失败。第 6 轮记录的两个延后项（kill-switch 循环未覆盖issues事件、缺少针对步骤级 WORKDIR 遮蔽的反向钉）当前按构造一致，留作后续即可。
| 9646 qwen-autofix-fork-bridge.yml | ||
| 5942 qwen-autofix-fork-signal.yml | ||
| 397656 qwen-autofix.yml | ||
| 404284 qwen-autofix.yml |
There was a problem hiding this comment.
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 留足容差。
Drop the duplicate repository-clause substitution entry the main merge left byte-identical twins of (keep the trailing one #10055 added; one entry suffices — every routed expression carries the clause at most once, probed by deleting the survivor). Record web-shell-visuals.yml's actual 24640 bytes in .size-baseline instead of the 23375 undercount. Restore serve-ab's caveat that the association clause is routing convenience, not a security boundary, in the security-model header. Pin what was deletable with the suite green: the chmod half of the workspace heal, the pool-only non-interactive best-effort ffmpeg install ahead of the GIF conversion, and the persistent npm cache export ahead of npm ci — each probe-red on removal.
|
Released in v0.22.3. |
6 participants
What this PR does
Runs the autonomous-fix scan lane — the routing phase that decides what work exists and the scan that enumerates every PR with new feedback — on the repository's persistent self-hosted runner pool instead of GitHub-hosted runners, using the exact fork-trust and kill-switch routing expression the heavy worker jobs already use. The small command jobs (label toggle, re-arm marker, acknowledgement) stay hosted by design. The long-form design record gains a new entry documenting the incident and the routing decision, and new regression guards pin the routing matrix.
Why it's needed
On 2026-08-25 a GitHub-hosted runner backlog queued the routing phase of every scheduled scan for over twenty minutes — longer than the scan cadence itself. The workflow's existing rule lets a newer scheduled tick supersede an older one that has not started yet; that is correct when the older tick already did its work, but during the backlog it meant each tick cancelled its still-queued predecessor. Nine consecutive scheduled rounds died without ever scanning and no review-feedback work was dispatched for hours, while the self-hosted pool stood mostly idle (~70 idle machines at the time). The scan lane is the critical path for the whole fan-out — while it sits queued, no worker can start — so it must not depend on a pool that can backlog for hours. Moving it to the persistent pool removes the hosted queue from the critical path; the supersede rule itself stays exactly as designed and becomes harmless again once routing gets a runner in seconds.
Reviewer Test Plan
How to verify
Confirm the scan lane's routing expression matches the heavy workers' byte-for-byte (fork-trust clause and maintainer kill-switch included), then run the pinned regression guards:
node --test .github/scripts/ci-runner-routing.test.mjs— 15/15 pass, including six new tests asserting the scan lane reaches the persistent pool on schedule and dispatch, keeps untrusted fork-PR lanes hosted, and obeys the kill-switch on every event.npx vitest run --config ./scripts/tests/vitest.config.ts qwen-autofix-workflow workflow-size— the updated routing pin passes (the old pin asserted the scan lane stayed hosted; the carve-out is documented in-test) and the workflow stays under the size gate with its baseline updated in this PR.ecs-qwenrunner instead of queueing onubuntu-latest.Evidence (Before & After)
N/A (CI routing change; no user-visible surface).
Before: 2026-08-25 ~15:00–17:40 UTC — nine consecutive scheduled rounds cancelled before scanning; routing jobs waited 16–24 minutes in the hosted queue (verified via run/job timelines and cancellation annotations, including "Canceling since a higher priority waiting request for qwen-autofix-route-cron exists").
After: routing and scan phases schedule onto the persistent pool; the regression pins guard the routing matrix against silent drift.
Tested on
Environment (optional)
Unit and workflow regression tests only; no sandbox involved.
Risk & Scope
Linked Issues
None — observed live on 2026-08-25; the design record entry af-148 is added in this PR.
中文说明
这个 PR 做了什么
把自动修复的扫描通道——决定"有什么活要干"的路由阶段,以及枚举所有有新反馈 PR 的扫描阶段——从 GitHub 托管 runner 迁到本仓库的持久自托管 runner 池,使用的路由表达式与重型工作 job 完全一致(含 fork 信任条款和维护者总开关)。三个小型命令 job(标签切换、重新武装标记、确认回执)按设计保留在托管池。长篇设计记录新增一条条目,记录本次事故与路由决策,并新增回归守卫钉住路由矩阵。
为什么需要
2026-08-25,一次 GitHub 托管 runner 拥堵让每轮定时扫描的路由阶段排队超过 20 分钟——比扫描周期本身还长。工作流现有规则允许较新的定时轮次取代尚未开始的旧轮次;当旧轮次已经干过活时这是对的,但拥堵期间它意味着每一轮都取消还在排队的上一轮。连续九轮定时扫描没扫成任何东西,数小时内没有派发任何评审反馈处理工作,而自托管池基本空转(当时约 70 台空闲)。扫描通道是整个扇出的关键路径——它排队时,任何工作 job 都无法启动——所以它不能依赖一个可能拥堵数小时的池。迁到持久池把托管队列移出关键路径;取代规则本身保持原设计不动,路由秒级拿到 runner 后它自然恢复无害。
评审测试计划
如何验证
确认扫描通道的路由表达式与重型工作 job 逐字节一致(含 fork 信任条款与维护者总开关),然后运行回归守卫:
node --test .github/scripts/ci-runner-routing.test.mjs—— 15/15 通过,其中 6 个新测试断言:扫描通道在 schedule 与 dispatch 事件下进入持久池、不可信 fork PR 车道保留托管、总开关在所有事件下必胜。npx vitest run --config ./scripts/tests/vitest.config.ts qwen-autofix-workflow workflow-size—— 更新后的路由钉通过(旧钉断言扫描通道留托管;例外已在测试注释中说明),工作流仍在尺寸门槛内,基线已在本 PR 同步更新。ecs-qwenrunner 上启动,而不再在ubuntu-latest上排队。证据(前后对比)
N/A(CI 路由改动,无用户可见界面)。
改动前:2026-08-25 约 15:00–17:40 UTC——连续九轮定时扫描在扫描前被取消;路由 job 在托管队列等待 16–24 分钟(通过 run/job 时间线与取消注解核实,包括 "Canceling since a higher priority waiting request for qwen-autofix-route-cron exists")。
改动后:路由与扫描阶段调度到持久池;回归守卫防止路由矩阵被悄悄改动。
测试平台
环境(可选)
仅单元测试与工作流回归测试;不涉及沙箱。
风险与范围
关联 Issue
无——2026-08-25 线上直接观测到;设计记录条目 af-148 随本 PR 新增。