fix(triage): compute the flake-gate diff before the env -i re-exec - #9468
Merged
Conversation
Update the record-step pins to the RUNNER_TEMP-staged file and add a pin asserting the scrubbed child copies it rather than re-running git.
The scrubbed (env -i) child cannot read the shallow merge-ref objects: git global safe.directory lives under HOME, which env -i strips, so git refuses to read the base commit and the diff fails with "Could not access <base-oid>". Compute the NUL-delimited diff in the parent (normal environment, no PR code executes) and stage it under RUNNER_TEMP; the clean child copies it into the root-only gate home.
Collaborator
qwen-code-ci-bot
requested changes
Aug 19, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Thanks @yiliang114 — the underlying problem is real and verified. Run 32227155960 failed in its verify job at the Record changed test files step, and the job log shows exactly the renamed failure this PR describes: after #9464's base-OID change, the scrubbed child dies with error: Could not access '577f719130f103d67ba879bd6e969240f1f28e65' right after the env -i re-exec. Computing the diff in the parent's normal environment (where the same OID is provably readable) and having the scrubbed child only copy the staged file is a sensible direction for it. Before this moves to code review, though, the same housekeeping item that blocked #9464: the PR body doesn't use the repository's pull request template.
## What this PR does is there, but the rest of the required headings are missing:
## Why it's needed— the root-cause analysis currently lives inside## What this PR does/## Fix## Reviewer Test Plan(with### How to verify,### Evidence (Before & After), and the### Tested onOS table) — the body carries## Verificationinstead## Risk & Scope## Linked Issues— the body carries## Referenceinstead- the Chinese translation inside
<details>— the template asks for the full body to be mirrored in Chinese
Nearly everything is already written in the current body — the root-cause analysis, the failing-run link, the node --test verification — so this is a reformat rather than a rewrite. Please restructure it into the template sections: https://github.com/QwenLM/qwen-code/blob/main/.github/pull_request_template.md
One note while you're at it: the Tested on table wants ✅ /
Once the body follows the template, a re-run (@qwen-code /triage) will pick it up and continue the review. 🙏
中文说明
感谢 @yiliang114 —— 底层问题真实存在且已核实。运行 32227155960 的 verify job 在 Record changed test files 步骤失败，job 日志正是本 PR 描述的那个"改了名字但没修好"的错误：#9464 改为 base OID 之后，scrubbed 子进程在 env -i 重新 exec 之后立刻以 error: Could not access '577f719130f103d67ba879bd6e969240f1f28e65' 挂掉。在父进程的正常环境里（同一个 OID 在那里已被证明可读）计算 diff、让 scrubbed 子进程只复制暂存文件，方向是合理的。但在进入代码审查之前，有一个和 #9464 相同的流程问题：这个 PR 的正文没有使用本仓库的 pull request 模板。
## What this PR does 已有，但其余必需章节缺失：
## Why it's needed—— 根因分析目前写在## What this PR does/## Fix里## Reviewer Test Plan（含### How to verify、### Evidence (Before & After)以及### Tested on操作系统表格）—— 正文目前用的是## Verification## Risk & Scope## Linked Issues—— 正文目前用的是## Reference<details>中的中文翻译 —— 模板要求将正文完整翻译为中文
其实所需内容在现有正文里都已经写到了——根因分析、失败运行链接、node --test 验证——所以只需重新排版，不必重写。请按模板章节重新组织：https://github.com/QwenLM/qwen-code/blob/main/.github/pull_request_template.md
另外提醒一点：Tested on 表格需要按操作系统标注 ✅ /
正文符合模板后，重新触发（@qwen-code /triage）即可继续审查。🙏
— Qwen Code · qwen3.8-max
Collaborator
Author
|
@qwen-code /triage |
Collaborator
|
Sandboxed verification: The verification job did not complete (checkout, runner, or setup error) and produced no report. See the workflow run for details. 中文 — 判定：
|
Contributor
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. |
Collaborator
|
Triage re-run completed without a new review. The bot already has a review of its own on 机器人在 The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
Collaborator
Author
|
@qwen-code /triage |
Collaborator
|
Sandboxed verification: The verification job did not complete (checkout, runner, or setup error) and produced no report. See the workflow run for details. 中文 — 判定：
|
qwen-code-ci-bot
requested changes
Aug 19, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
— qwen3.8-max via Qwen Code /review (v0.21.14)
Collaborator
Author
|
@qwen-code /triage |
Collaborator
|
Sandboxed verification: The verification job did not complete (checkout, runner, or setup error) and produced no report. See the workflow run for details. 中文 — 判定：
|
Collaborator
|
Thanks for the PR! Template looks good ✓ — the body now follows the repo template, which was the Stage 1a blocker on the first pass. Problem: observed bug with solid evidence. Run 32227155960's Direction: aligned. Computing the diff in the parent's normal environment — the same environment where Size: not applicable — Approach: scope is right and minimal — the diff moves exactly the failing read out of the scrubbed child and pins the new shape in the structural suite. The two follow-up commits address the earlier Risk: no elevated risk signals (no high-risk-path matches). One observation carried into the code review: the staged list sits briefly in uid-1000-writable Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ —— 正文现已符合仓库模板，这是第一轮审查的 Stage 1a 阻塞项。 问题：已观测到的 bug，证据充分。运行 32227155960 的 方向：对齐。在父进程的正常环境里计算 diff —— 同一环境里 规模：不适用 —— 仅 方案：范围恰当且最小化 —— diff 恰好把失败的那次读取移出清空后的子进程，并在结构化测试套件里钉住新形态。两个后续 commit 处理了上一轮 风险：无升级风险信号（未命中高风险路径）。一个带入代码审查的观察点：暂存列表会短暂落在 uid-1000 可写的 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
Code reviewIndependent proposal first, from the title and root cause alone: don't try to make git work inside the The staging window is the thing that had to be right, and it holds. The new staging path sits in uid-1000-writable The earlier One non-blocking nit: the new Lint note: the previous round recorded the workflow as "not linted (tool limitation)"; on this head actionlint and yamllint both ran inside the green Test evidenceThis is an unattended run — no PR code was built or executed locally. Evidence below is the PR's own CI on the reviewed commit, read via the API plus the failing-run log for the original bug. The structural suite this PR updates is collected by CI as part of Not verified pre-merge, and by construction unverifiable: whether the end-to-end
Still in flight: the overall 中文说明代码审查先说独立方案（只看标题和根因）：不要试图让 git 在 暂存窗口是必须做对的部分，而它成立。 新暂存路径位于 uid-1000 可写的 上一轮 一个非阻塞的小问题：新增的 Lint 说明：上一轮记录该 workflow "未 lint（工具限制）"；在本 head 上，actionlint 与 yamllint 均已在绿色的 测试证据本次为无人值守运行——未在本地构建或执行任何 PR 代码。以下证据来自该 PR 自身在被审 commit 上的 CI（经 API 读取）及原始 bug 的失败运行日志。 本 PR 更新的结构化测试套件作为 合并前未验证、且构造上无法验证的一点：端到端的 （CI 表格见上方标记区域； — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
|
Confidence: 4/5 — clean, minimal fix for a verified failure; one non-blocking nit (the bare Stepping back: this is the third attempt at a failure that kept changing its name — The two follow-up commits show the review feedback actually landed — The earlier Stage 1a template block is resolved (body reformatted), and the prior Verdict: approve — deferred, because CI has not fully landed on this head: the 中文说明置信度：4/5 —— 对一个已证实失败的干净、最小化修复；一个非阻塞小问题（其余命令全部绝对路径的父进程分支里有一处裸 退一步看：这是对同一个失败的第三次尝试——失败不断换名字：先是 两个后续 commit 证明评审意见真正落地了—— 此前的 Stage 1a 模板阻塞已解决（正文已重排），上一轮 结论：批准 —— 但延迟执行，因为该 head 的 CI 尚未全部落地： — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
previously requested changes
Aug 19, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-triage.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/workflows/qwen-triage.yml:3001 — [review] gate intake transits uid-1000-writable RUNNER_TEMP on a dereferencing cp; hoisting GATE_HOME into the parent arm dissolves the hop (deferred — code unchanged since the previous round).github/workflows/qwen-triage.yml:2914 — [review] header comment's RUNNER_TEMP tamper-proofness claim contradicts the GATE_HOME comments and the raw staging copy lingers unfiltered (deferred — code unchanged since the previous round)
中文说明
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后（第 2 轮，非阻断）——已记录，本轮不要求修改：共 2 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.21.14)
…nd symlink plants
Collaborator
Author
|
Addressed the round-2 findings in
The two already-closed suggestions were addressed earlier: slash-pathed
|
yiliang114
enabled auto-merge
August 19, 2026 13:15
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
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-triage.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/workflows/qwen-triage.yml:3001 — [review] staged diff file lingers job-long in uid-1000-writable RUNNER_TEMP after the copy
中文说明
已审查——无阻断问题。 建议见行内评论。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后（第 3 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| // name `git diff` when describing what NOT to do) before asserting. | ||
| assert.doesNotMatch( | ||
| recordStep.run.slice(recordReExecAt).replace(/^\s*#.*$/gm, '').replace(/\\\n/g, ' '), | ||
| /\bgit\b[^\n]*\b(diff|log|show|whatchanged)\b/, |
Collaborator
There was a problem hiding this comment.
[Suggestion] The new no-git-in-child assertion hand-enumerates four git subcommands (diff|log|show|whatchanged), leaving the rest of git's read surface unguarded when the class-closing check — banning \bgit\b outright — is available and false-positive-free. A sweep over the transformed child slice (comment-strip + continuation-fold at the actual re-exec offset) finds zero \bgit\b occurrences today, so the blanket ban admits no false positive; mutant bodies with git cat-file -p, git rev-list HEAD, git archive HEAD | tar -x, or git --no-pager grep all slip through the current regex green, while the blanket ban catches every one. Any of those reintroduced in the scrubbed child resurrects exactly the env -i failure this PR fixes: git under env -i cannot read the shallow merge-ref objects, the record step errors, and the flakiness gate degrades.
Suggested change
| /\bgit\b[^\n]*\b(diff|log|show|whatchanged)\b/, | |
| /\bgit\b/, |
中文说明
新的“子进程禁止运行 git”断言只手工枚举了 4 个 git 子命令（diff|log|show|whatchanged），而类闭合的检查——直接禁止 \bgit\b——是可行的且不会误报，这让 git 其余的读取面处于无防护状态。对变换后的子进程切片（按真实 re-exec 位置截取、去注释、折叠续行）的扫描显示其中今天没有任何 \bgit\b 出现，因此全面禁止不会误报；带有 git cat-file -p、git rev-list HEAD、git archive HEAD | tar -x、git --no-pager grep 的变异体都能绿着通过当前正则，而全面禁止能全部捕获。这些命令中任何一个被重新引入清空后的子进程，都会复活本 PR 所修复的那个 env -i 失败：env -i 下 git 读不到浅克隆 merge-ref 对象，record 步骤报错，flake gate 降级。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| assert.match( | ||
| recordStep.run, | ||
| /^\s*\[0-9a-f\]\[0-9a-f\]\[0-9a-f\]\[0-9a-f\]\[0-9a-f\]\[0-9a-f\]\[0-9a-f\]\[0-9a-f\]\*\) ;;/m, | ||
| 'the base OID shape must be an 8+-hex prefix — a planted valid OID would yield an empty diff and starve the gate into n/a', |
Collaborator
There was a problem hiding this comment.
[Suggestion] The pins added for R2-3 cover the case header and the 8-hex arm, but the *) refusal arm of the moved case "$BASE_OID" validation (qwen-triage.yml:2920) has no assertion anywhere in this suite, even though neighbouring refusal branches (node-survivor refusal, grep-status refusal) are pinned. Mutation-verified: deleting the *) arm leaves the suite green (118 pass / 0 fail before and after). With that mutant live, a ${RUNNER_TEMP}/verify-base-oid holding a valid git revision that is not 8-hex — e.g. HEAD, plantable per the threat model this step's own comments document — falls through the case silently; git diff HEAD HEAD exits 0 with empty output; the child copies the empty staging file, grep no-matches (status 1, permitted), and the step logs "Recorded 0 changed test file(s)" — silently starving the flakiness gate into n/a instead of refusing loudly, which is exactly the outcome this line's own assertion message warns about. Add a presence pin for the refusal arm beside these assertions, e.g.:
assert.match(
recordStep.run,
/^\s*\*\) \/usr\/bin\/printf '::error::No trusted base OID recorded; refusing to record the flakiness-gate file list\.\\n'; exit 1 ;;$/m,
'a malformed or planted base OID must refuse the record, never fall through into the diff',
);中文说明
为 R2-3 新增的 pin 覆盖了 case 头部和 8-hex 分支，但移入父进程臂的 case "$BASE_OID" 校验（qwen-triage.yml:2920）的 *) 拒绝分支在整个套件里没有任何断言，尽管相邻的拒绝分支（node 进程幸存拒绝、grep 状态拒绝）都有 pin。变异验证：删除 *) 分支后套件仍然全绿（前后均为 118 通过 / 0 失败）。在该变异体存活时，${RUNNER_TEMP}/verify-base-oid 里若是一个合法但非 8-hex 的 git 修订——例如 HEAD，按本步骤自己注释写明的威胁模型这是可被种植的——会静默落空 case；git diff HEAD HEAD 以空输出退出码 0；子进程拷走空的暂存文件，grep 无匹配（状态 1，被允许），步骤记录 "Recorded 0 changed test file(s)"——flake gate 被静默饿成 n/a 而不是大声拒绝，这正是本行断言消息自己警告的结果。请在这些断言旁为拒绝分支补一条存在性 pin，例如上方代码块。
— qwen3.8-max via Qwen Code /review (v0.21.14)
doudouOUC
approved these changes
Aug 19, 2026
doudouOUC
left a comment
doudouOUC
left a comment
Collaborator
There was a problem hiding this comment.
Independent review — verified against HEAD b46aba58
Verdict: APPROVE. The fix is correct, the trust boundary is preserved, and the two open threads are test-pin breadth gaps rather than defects.
The core change is sound
The diagnosis is right: env -i strips HOME, so git loses the global safe.directory config and refuses to read the shallow merge-ref base object — hence Could not access <base-oid> inside the scrubbed child. Computing the diff in the parent and copying the result in is the correct shape.
I verified the ordering at HEAD rather than trusting the test names, and it matches what the new pins assert:
- step-script integrity check — refusing to run on snapshot mismatch (2903)
BASE_OIDread from theverify-base-oidfile captured earlier while.gitwas root-owned (2917), then shape-validated (2918)rm -rf -- "${RUNNER_TEMP:?}/flake-record-files-all"(2922)- the
git diffinto the staging path (2924) exec /usr/bin/env -i …re-exec, forwardingRUNNER_TEMP(2925)install -d -m 0700 -o root -g root "$GATE_HOME"in the child (2972)cpstaging →$GATE_HOME/files-all(3001)
Integrity check before the diff, and the gate home recreated root-only after the re-exec but before the copy — so the child never has to trust a pre-existing gate home.
Why running the diff in the un-scrubbed parent doesn't weaken the boundary
This is the part worth being explicit about, since the env -i scrub exists precisely to contain untrusted PR content:
git diff --name-onlygenerates no content diff, so.gitattributes-declared diff drivers andtextconvare never invoked — and in any case those needdiff.<driver>.command/textconvset in config, which a PR tree cannot supply.git diffalso runs no hooks. So the command reads metadata and executes nothing from the PR./usr/bin/gitand/usr/bin/catare absolute, so a plantedgitearlier onPATHcan't hijack the parent invocation.rm -rf --immediately before the>redirect closes the pre-planted-symlink/directory hole — without it, a root redirect could write through an attacker-planted link.${RUNNER_TEMP:?}prevents the empty-expansion footgun. Nothing untrusted executes between that write and the child'scp, so the window stays closed.BASE_OIDis quoted and can only reach git as a single argument whose first eight characters must be hex, so it can neither become a flag nor inject a command.
The two open threads — both accurate, both non-blocking
I confirmed each against the file rather than taking the finding at face value:
- Unpinned
*)refusal arm. Correct — no assertion anywhere references the refusal arm or itsNo trusted base OID recordedmessage, so deleting that arm would silently turn the shape validation into a no-op with the suite still green. The validation itself is present and correct in this diff; the gap is future-proofing, which puts it at Suggestion. Cheap to close with one moreassert.matchon the*)arm. - Hand-enumerated git subcommands. Correct — the child guard matches
\bgit\b[^\n]*\b(diff|log|show|whatchanged)\b, so a latergit rev-list/git ls-tree/git cat-filein the child would slip past the pin that exists to prevent exactly that class of failure. Since comments are already stripped before the assertion, broadening toward a bare\bgit\b(or adding the object-reading verbs) is low-risk.
Neither blocks merge: the production workflow logic in this PR is correct, and these only affect how well the tests would catch a future regression. Happy to see them folded in if you're touching the branch anyway, otherwise a follow-up is fine — worth recording the deferral so they aren't dropped.
CI is green (13/13 pass, Test (ubuntu-latest, Node 22.x) included), and the standing CHANGES_REQUESTED reflects earlier rounds whose threads are now resolved.
wenshao
approved these changes
Aug 19, 2026
Collaborator
|
Released in v0.21.15. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Fix the flake-gate "Record changed test files" step so it no longer runs
git diffinside theenv -iscrubbed child. The step now computes the NUL-delimited changed-test list in the parent (before the re-exec, where git can read the shallow merge-ref objects) and stages it underRUNNER_TEMP; the scrubbed child only copies it into the root-only gate home.Why it's needed
PR #9464 changed
git diff 'HEAD^1' HEADtogit diff "$BASE_OID" HEAD, but that only renamed the failure: the verify lane still fails witherror: Could not access '577f719…'(run 32227155960). The root cause is theenv -ire-exec — git's globalsafe.directory '*'lives underHOME, whichenv -istrips, so git's ownership check refuses to read the base commit object. The same object is readable one step earlier in the normal environment.Reviewer Test Plan
How to verify
Run the workflow structural suite and confirm the record-step pins pass:
The suite
qwen-triage: flakiness gate (#9125)passes, includingrecords the changed-test list…andthe gate home sits at the container root…. Confirm the record step'sgit diffnow redirects to${RUNNER_TEMP}/flake-record-files-alland the scrubbed child doescprather than re-running git.Evidence (Before & After)
N/A — non-UI CI workflow change.
Tested on
Environment (optional)
N/A — unit tests only.
Risk & Scope
RUNNER_TEMP(uid-1000-writable) before the re-exec. It is written by the parent and copied by the child immediately after, before any PR lifecycle code runs, so there is no window for a swap.Could not accessfailure is gone can only be observed after merge, when the next/triageruns the updated main-branch workflow.Linked Issues
Fixes the flake-gate verify-lane failure observed in #9432 triage (run 32227155960).
中文说明
本 PR 做了什么
修复 flake-gate「Record changed test files」步骤，使其不再在
env -i清空后的子进程里跑git diff。该步骤现在在 re-exec 之前（正常环境，能读到浅克隆 merge-ref 对象）计算出 NUL 分隔的改动测试文件列表，暂存到RUNNER_TEMP；清空后的子进程只把它拷贝进根目录的 gate home。为什么需要
#9464 把
git diff 'HEAD^1' HEAD改成git diff "$BASE_OID" HEAD，但只是把报错换了个名字：verify lane 仍报error: Could not access '577f719…'（run 32227155960）。根因是env -i清空环境 —— git 的全局safe.directory '*'存在HOME下，env -i把HOME也清了，git 的 ownership 检查就拒绝读 base commit 对象。同一个对象在前一步正常环境里是可读的。审阅测试计划
如何验证
运行 workflow 结构测试并确认 record 步骤的 pin 通过：
套件
qwen-triage: flakiness gate (#9125)通过，包括records the changed-test list…和the gate home sits at the container root…。确认 record 步骤的git diff现在重定向到${RUNNER_TEMP}/flake-record-files-all，且清空后的子进程用cp而不是重跑 git。证据（前后对比）
N/A —— 非 UI 的 CI workflow 改动。
测试环境
环境（可选）
N/A —— 仅单元测试。
风险与范围
RUNNER_TEMP（uid-1000 可写）。它由父进程写入、子进程随后立即拷贝，此时任何 PR 生命周期代码都还没跑，所以没有可被替换的窗口。Could not access是否消失，只能在合并后、下次/triage跑更新后的 main 分支 workflow 时观察到。关联 Issue
修复 #9432 triage 中观察到的 flake-gate verify-lane 失败（run 32227155960）。