fix(triage): retry a transient npm ci before blaming the PR for it - #7884
Merged
Conversation
Run 30319209722 posted `Sandboxed verification: fail` on PR #7856 with "The PR could not be built ... treated as a PR failure verdict rather than an infrastructure failure." The PR changed six source files. The install died here: Error: spawnSync .../web-templates/node_modules/esbuild/bin/esbuild ETXTBSY at validateBinaryVersion (esbuild/install.js:99:28) ETXTBSY is npm writing a dependency's binary and that package's own install script exec'ing it before the write is closed — a race, and one the PR had no part in. The comment accused its author anyway. Both sandbox lanes now retry `npm ci` once. `npm ci` removes node_modules before installing, so the second attempt cannot inherit the half-written file; the transient class is simply absorbed instead of being classified. Retrying rather than classifying is the point. The obvious fix — match ETXTBSY in the log and downgrade to infra-error — would read text the PR's own lifecycle scripts can print, which is exactly the forgeable signal the verdict logic was rewritten to stop consulting: it let a PR launder its own deterministic breakage into "infrastructure, please re-run". A retry consults nothing. A genuinely broken tree fails twice and still earns `fail`, and the second attempt is only ever paid on a path that is already failing. The build is deliberately not retried: a compile error is deterministic, so a second run would only double the cost of an honest failure. Because the install now gets two chances, the sentence that blames the PR says so — "failed twice in a row". PREPARE_ATTEMPTS is initialised rather than defaulted at the point of use, or an inherited value would claim a single-shot `npm run build` had failed twice, which is the same false accusation pointed the other way. Mutation-verified 5/5: an unbounded retry, no retry at all, a retry on the build, dropping the initialisation, and reverting the wording each turn at least one test red. The retry itself is covered behaviourally — the real step text runs against a stubbed `runuser`/`npm`, so the tests count actual install attempts instead of asserting that a loop exists, which would pass on both a loop that never retries and one that never stops.
Collaborator
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
Collaborator
|
Thanks for the PR! Template looks good ✓ Problem: observed, not theoretical. Run 30319209722 posted a Direction: aligned. This is the triage workflow's own reliability, squarely in scope. The deliberate choice to retry rather than match Size: not applicable — the change is in Approach: the scope feels right and genuinely minimal. Bounded to one retry, unconditional, build left single-shot (compile errors are deterministic), and the "failed twice in a row" wording is threaded through so the accusation matches what now earns it. The Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：已观测到，而非理论性。运行 30319209722 对 #7856（一个仅改动 6 个源文件的 PR）发布了 方向：对齐。这是 triage 工作流自身的可靠性，完全在范围内。刻意选择重试而非在日志里匹配 规模：不适用——改动位于 方案：范围合理且真正最小化。限定为一次重试、无条件触发、构建保持单次执行（编译错误是确定性的），并贯通了"连续失败两次"的措辞，使指控与如今支撑它的依据相符。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Collaborator
Code reviewMy independent take before reading the diff: a transient install race that has nothing to do with the PR should be absorbed by a bounded retry, and the retry must not gate on anything the PR can print — matching Reading the actual bash, the loop is correct: The duplication of the retry block across the two lanes is the one thing a reviewer might eyeball, but it matches how this workflow is written — the lanes are deliberately self-contained copies, and there is no clean way to share shell across separate YAML jobs. Not a blocker. The tests are the strong part: they execute the real step text with No correctness, security, or convention issues found. CI test evidenceSignal read: the unit suite that actually exercises Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 中文说明代码审查：我在看 diff 之前的独立判断是——与 PR 无关的瞬时安装竞态应当被一次有界重试吸收，且重试不能以 PR 能打印的任何东西为条件；在日志里匹配 实际读 bash：循环正确—— 重试块在两条车车间的重复是评审者唯一可能多看一眼的地方，但这正符合该工作流的写法——两条车道刻意是自包含的副本，且没有干净的办法在独立 YAML job 之间共享 shell。非阻塞。 测试是亮点：它们执行真实的步骤文本，stub 掉 未发现正确性、安全性或规范问题。 CI 测试证据：真正执行 — Qwen Code · qwen3.8-max-preview Reviewed at |
Collaborator
|
Confidence: 5/5 — clean across every stage; a real, reproduced bug fixed the minimal way, with the security property (no PR-controlled signal) explicitly preserved and behaviorally tested. Stepping back: this is exactly the kind of PR the gate should wave through. The problem was observed and evidenced (a real false If I had to maintain this in six months I'd thank the author. The only thing I cannot confirm from here is the unit suite's verdict — it is still running on the reviewed commit — so approval is deferred until that lands green rather than attested now. Approval deferred until CI lands green on 中文说明回顾全局：这正是门禁应当放行的那类 PR。问题已被观测且有证据（#7856 上一次真实的错误 若六个月后由我来维护，我会感谢作者。我在此处唯一无法确认的是单元测试的判决——它在被审 commit 上仍在运行——因此批准被推迟到它绿了之后，而非现在背书。 批准推迟至 CI 在 — Qwen Code · qwen3.8-max-preview Reviewed at |
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. |
qwen-code-ci-bot
approved these changes
Jul 28, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
qwen-code-ci-bot
approved these changes
Jul 28, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
No issues found. LGTM! ✅
中文说明
未发现问题。LGTM！✅
— qwen3.7-max via Qwen Code /review
doudouOUC
approved these changes
Jul 28, 2026
doudouOUC
left a comment
doudouOUC
left a comment
Collaborator
There was a problem hiding this comment.
No issues found. LGTM! ✅
中文说明
未发现问题。LGTM！✅
— qwen3.7-max via Qwen Code /review
This was referenced Jul 28, 2026
Collaborator
|
Released in v0.21.1. |
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
3 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
Retries
npm cionce in both sandbox lanes, so a transient install race stops being reported as the PR's fault.Run 30319209722 posted "Sandboxed verification: fail — The PR could not be built ... treated as a PR failure verdict rather than an infrastructure failure" on #7856, a six-file source change. The install died here:
ETXTBSYis npm writing a dependency's binary and that package's own install script exec'ing it before the write is closed. It is a race, and the PR had no part in it — but the comment accused its author, on a lane whose whole purpose is to produce evidence a maintainer can trust.Two changes, applied to the
verifyandtmux-testinglanes:npm ciremovesnode_modulesbefore installing, so the second attempt cannot inherit the half-written file.The build is deliberately not retried: a compile error is deterministic, so a second run would only double the cost of an honest failure.
Why it's needed
The obvious fix is to match
ETXTBSYin the log and downgrade toinfra-error. That would be a regression, and the code already says why:A retry consults nothing. It sidesteps the classification question entirely rather than reopening it:
failfail(twice)The extra attempt is only ever paid on a path that is already failing.
Reviewer Test Plan
How to verify
The retry is covered behaviourally, not structurally. Asserting "the step contains a loop" would pass on a loop that never retries and on one that never stops — the two ways this can actually be wrong. So the tests run the real step text with
runuserstubbed to drop its own arguments and exec the rest (which keeps the genuineenv -u …stripping in the path under test), and annpmstub that fails a set number of times, then count actual attempts:retrying oncefail/installafter 2 attemptsThe middle row is the bug: before this change it emitted
verdict=fail. The third row pins the bound — a retry that never stops would report 99.Mutation-verified 5/5, each turning at least one test red:
-ge 2→-ge 99)expected 99 to be 2PREPARE_ATTEMPTS=''Worth recording: mutation 1 appeared to survive on the first pass. It had not — the
perl -0pi -e 's/\Q"$install_attempt" -ge 2\E/…/'interpolated$install_attemptas an empty Perl variable, so the file was never modified.\Qsuppresses metacharacters, not interpolation. Re-run with the$escaped, it kills the test. A mutation that does not change the file is not evidence of anything.PREPARE_ATTEMPTSis initialised rather than defaulted at the point of use, and mutation 4 is what makes that load-bearing:${PREPARE_ATTEMPTS:-}does nothing against an inherited value, and the result would be a report claiming a single-shotnpm run buildhad failed twice — the same false accusation this PR removes, pointed the other way.77 tests, 75 pass;
actionlintfinding-set identical tomain(no new findings); both prepare steps and both publish steps passbash -nandshellcheck -S warning --enable=all; prettier and eslint clean.Evidence (Before & After)
Real comment posted on #7856 by run 30319209722 — the accusation this removes:
After, for the same log: the install is retried, succeeds, and no verdict is posted at all. When a tree really is broken, the copy states what it is now claiming:
Tested on
Linux:
ETXTBSYis the Linux-side race being absorbed; the retry logic itself is OS-independent and is exercised on macOS with stubs. Confirmation on a runner needs one/verifyrun that hits a flaky install, which is by nature not schedulable.Risk & Scope
npm cibefore reportingfail. Bounded and only on an already-failing path; theverifyjob's budget is 60 min andtmux-testing's 45 min, against an install of a few minutes.emit_blocktruncates from the head (buf.subarray(0, max)), so a long enough first attempt could push the decisive second failure out of the 20,000-char window. Measured against the real feat(web-shell): add composer footer renderer #7856 log — one failing attempt renders to 2,701 bytes, two to roughly 5.4 KB — that leaves ~3.7× headroom, so it is not reachable at observed sizes. It is a pre-existing weakness (a single >20 KB failure truncates its own error away onmaintoo) that this change makes about twice as easy to reach. Tail-truncating install logs is the real fix and is deliberately left out of scope.dubious ownershipinconsistency that stops most machines earlier atClean stale agent state; the two proxy-watchdog tests that time out onmain(20 chunks × 200 ms exceeds vitest's 5 s default) — those are fixed by fix(triage): make the build-process guard diagnosable and zombie-aware #7858 and not duplicated here to avoid a conflict.Linked Issues
Follow-up to #7710 and #7753. Independent of #7858 (different steps in the same file). No issues closed.
中文说明
本 PR 做了什么
在两条沙箱车道中为
npm ci增加一次重试，使瞬时安装竞态不再被报告为 PR 的问题。运行 30319209722 在 #7856（一个仅改动 6 个源文件的 PR）上发布了 "Sandboxed verification: fail —— 该 PR 无法构建……判定为 PR 失败而非基础设施故障"。安装失败于：
ETXTBSY是 npm 写入依赖的二进制文件后、该包自身的安装脚本在写入尚未关闭时就去 exec 它——这是一个竞态，与该 PR 毫无关系。但评论仍然指控了它的作者，而这条车道的全部意义正是产出维护者可以信赖的证据。两处改动，
verify与tmux-testing两条车道同时应用：安装重试一次（npm ci会在安装前删除node_modules，因此第二次尝试不可能继承半写入的文件）；并在措辞中说明重试过——归咎于 PR 的那句话现在写作"连续失败两次"，因为这才是支撑这项指控的依据。构建刻意不重试：编译错误是确定性的，重跑只会让一次诚实的失败成本翻倍。
为什么需要
直觉修法是在日志中匹配
ETXTBSY并降级为infra-error。那会是一次倒退，而代码里已经写明了原因：重试则不查询任何东西。它绕开了整个分类问题，而不是重新打开它：
failfail（两次）额外的那次尝试只在已经失败的路径上付出。
评审验证方案
重试是行为性覆盖而非结构性覆盖。断言"该步骤包含一个循环"会在"从不重试的循环"和"永不停止的循环"上同时通过——而这恰是它可能出错的两种方式。因此测试运行真实的步骤文本：
runuser被 stub 为丢弃自身参数并 exec 其余部分（从而保留被测路径中真正的env -u …剥离逻辑），npmstub 失败指定次数，然后统计实际尝试次数：retrying oncefail/installafter 2 attempts中间一行就是这个 bug：改动前它会发出
verdict=fail。第三行钉住了上界——永不停止的重试会报告 99 次。5/5 变异验证，每项至少使一条测试变红：无界重试（
-ge 2→-ge 99）→expected 99 to be 2；完全不重试 → 2 条失败；给 build 也加重试循环 → 1 条失败；去掉PREPARE_ATTEMPTS=''→ 构建报告谎称"连续失败两次"；还原为单次尝试的措辞 → 2 条失败。值得记录：变异 1 起初看似存活。其实并没有——
perl -0pi -e 's/\Q"$install_attempt" -ge 2\E/…/'把$install_attempt当作 Perl 变量插值成了空串，文件根本没被修改。\Q抑制的是元字符，不是插值。把$转义后重跑，它确实致死。一个没有改动文件的变异不能证明任何事情。PREPARE_ATTEMPTS是显式初始化而非在使用处默认，变异 4 正是让这一点成为承重结构的原因：${PREPARE_ATTEMPTS:-}对继承而来的值毫无作用，其后果会是一份声称单次执行的npm run build失败了两次的报告——正是本 PR 要消除的那种错误归因，只是方向相反。77 条测试、75 条通过；
actionlint的发现集与main完全一致（无新增）；两个 prepare 步骤与两个 publish 步骤均通过bash -n与shellcheck -S warning --enable=all；prettier 与 eslint 干净。测试平台
macOS ✅；Windows N/A；Linux⚠️ ——
ETXTBSY正是被吸收的 Linux 侧竞态，而重试逻辑本身与操作系统无关，已在 macOS 上用 stub 完整验证。在 runner 上的最终确认需要一次恰好遇到 flaky 安装的/verify运行，这本质上无法排期。风险与范围
fail前多花一次npm ci。这是有界的，且只发生在已经失败的路径上；verifyjob 的预算是 60 分钟、tmux-testing是 45 分钟，而安装耗时只有几分钟。emit_block从头部截断（buf.subarray(0, max)），因此足够长的第一次尝试可能把决定性的第二次失败挤出 20,000 字符窗口。以 feat(web-shell): add composer footer renderer #7856 的真实日志实测——单次失败渲染为 2,701 字节，两次约 5.4 KB——尚有约 3.7 倍余量，在已观测的规模下不可达。这是一个既有弱点（在main上，单次超过 20 KB 的失败同样会把自己的错误截掉），本改动使其大约容易触发一倍。真正的修法是对安装日志做尾部截断，刻意留作范围外。Clean stale agent state失败的 runner 侧dubious ownership不一致；以及在main上超时的两条 proxy-watchdog 测试（20 个数据块 × 200 ms 超过 vitest 默认的 5 秒）——那两条由 fix(triage): make the build-process guard diagnosable and zombie-aware #7858 修复，此处不重复以避免冲突。关联 Issue
#7710 与 #7753 的后续。与 #7858 相互独立（同一文件的不同步骤）。不关闭任何 issue。