fix(ci): retain disk samples when npm install fails - #10517
Merged
Conversation
The in-repo slice of #10035: fail fast on a saturated self-hosted host before npm ci instead of dying on ENOSPC mid-run, and keep the disk-pressure timeline from a failed run as an artifact so the peak can be correlated with the job and runner after cleanup reclaims the host.
…trings Co-authored-by: Qwen-Coder <qwen-coder @alibabacloud.com>
The sample-file setup landed inside the routing block that no-ak-integration-ci pins identical across test/test_macos/test_windows, breaking the identity assertion. Move the DISK_SAMPLES definition and DISKCONTEXT header ahead of the routing block; the DFSAMPLE lines still carry the routed tmpdir.
Keep the disk-floor shell comparisons inside bash's signed integer range by rejecting numeric overrides that cannot be compared safely. Add the existing helper test coverage for the overflow boundary.\n\nCo-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Pin the three behaviours of validate_floor_override that the current suite leaves unpinned, each verified to catch its mutant: - INT64_MAX is a legal floor and must reach the disk comparison (tightening the length check to -gt 18 now goes red). - Zero-padded overrides padded past 19 raw characters are normalized and accepted (removing the leading-zero strip now goes red). - 20-digit values are rejected by the length branch, which the existing 19-digit case never exercises (widening to -gt 20 now goes red). Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Resolve the ci.yml helper-test list while preserving both main and PR entries. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Collaborator
Collaborator
|
Thanks for the follow-up! Template looks good ✓ Problem: observed, not theoretical — this continues #10035 (OPEN, P2), the real ENOSPC incident on high-concurrency self-hosted runners, whose original failure happened inside Direction: aligned — repository-side CI observability for an open P2 incident, extending the machinery #10394 introduces (same samples file, same upload step) instead of adding a parallel mechanism. Nothing product-facing to question. Size: no core paths touched — 17 production lines in Approach: scope feels right. Start the sampler before Risk: no elevated risk signals — the changed paths ( Moving on to code review. 🔍 中文说明感谢这个后续补丁！ 模板完整 ✓ 问题：已观测到，不是理论问题——这是 #10035（OPEN，P2）的延续，即高并发自托管 runner 上真实发生的 ENOSPC 事故，原始失败就发生在 方向：对齐——针对一个 open 的 P2 事故的仓库内 CI 可观测性改进，扩展而非重复 #10394 引入的机制（同一采样文件、同一上传步骤）。没有产品方向的疑问。 规模：未触及核心路径—— 方案：范围合理。在 风险：无升级风险信号——变更路径不匹配任何与 revert 相关的高风险路径。Stage 1-pre（重复检查）已跳过，因为 base 不是默认分支，stacked PR 属预期情况。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
Code reviewIndependent baseline first: to keep install-failure evidence I would initialize the same No blockers, no convention violations:
No sequence diagram or files table — two files, one mechanism. Testing evidenceUnattended CI run — I did not execute any PR code. Evidence for the reviewed commit, via the API:
Sandboxed verification could settle the mechanics before retargeting:
Every check above is bot orchestration, not CI validation. The missing 中文说明代码审查先给独立基线：要保留安装失败的证据，我会在 Install 步骤里、 无阻塞问题，无规范违反：
测试证据本次为无人值守 CI 运行，未执行任何 PR 代码。针对被审提交的证据（来自 API）：
沙箱验证可以在切换 base 之前先钉住机制： 上表全部为 bot 编排类检查，不是 CI 验证。缺失的 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
|
Confidence: 3/5 — clean, minimal, well-tested change; approval withheld only because the stacked base means this PR's own CI can never run until it is retargeted to Stepping back: this is exactly the follow-up the #10035 incident work needed — the base PR adds the gate and the unit-test timeline, and this one closes the hole the incident actually came through ( What holds it at 3/5 is procedural, not code quality:
⏸️ Deferring to @yiliang114 — nothing to fix in the code; the call is simply to land #10394 first and retarget this PR, at which point CI covers it and approval can follow. 中文说明Confidence: 3/5 —— 干净、最小、测试充分的改动；暂不 approve 仅因为 stacked base 导致本 PR 自己的 CI 在切回 退一步看：这正是 #10035 事故处理需要的后续——base PR 加了门槛和单测 timeline，这个补上了事故实际发生的入口（ 停在 3/5 的是流程原因，不是代码质量：
⏸️ 转交 @yiliang114 —— 代码无需修改；只需先合并 #10394 再把本 PR 切回 — Qwen Code · qwen3.8-max Reviewed at |
wenshao
previously approved these changes
Aug 29, 2026
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/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
已审查——无阻断问题。 建议见行内评论。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.22.3)
CanReader
reviewed
Aug 29, 2026
CanReader
left a comment
CanReader
left a comment
There was a problem hiding this comment.
Ran the new helper test locally and it passes:
$ node --test .github/scripts/ci-disk-pressure.test.mjs
✔ starts sampling before npm ci and preserves those samples for upload
✔ keeps install failure status while writing the pre-install sample
tests 2 | pass 2 | fail 0
The shell fidelity is right, which is the part I most expected to be wrong. defaults.run.shell: 'bash' at the top of ci.yml means Actions runs these blocks as bash --noprofile --norc -eo pipefail, and the test drives the step with bash -e -o pipefail -c, so it is exercising the same failure semantics production gets. Asserting the exit status is preserved as 42 through the trap is a good call — that is exactly the regression a naive trap would introduce.
One real issue, latent rather than present. Under -e -o pipefail, whether the whole sample= assignment survives a failing df depends entirely on which substitution runs last, because the assignment takes the exit status of the last command substitution. Right now the last one is the awk … || true, so it is always 0 and the step is safe. Add one field after memavail[...], or reorder them, and the Install step starts aborting whenever that command fails. Demonstrated:
# current field order — survives
$ bash -e -o pipefail -c 'x="space[$(df -h /nonexistent 2>/dev/null|tail -1)] mem[$(awk "…" /proc/meminfo 2>/dev/null || true)]"; echo ok'
ok
# one unguarded field appended — step dies
$ bash -e -o pipefail -c 'x="mem[$(awk "…" 2>/dev/null || true)] load[$(cat /proc/loadavgXX 2>/dev/null)]"; echo ok'
(exit 1, "ok" never printed)
Since the whole point of this step is to collect evidence when the runner is already unhealthy, having the sampler able to kill the install on a future edit seems worth closing off. Guarding the assignment itself rather than the last field makes it order-independent:
sample="DFSAMPLE ... memavail[...]" || trueI confirmed that form survives an unguarded failing substitution in any position. The || true currently reads as if it only guards the awk, which is what makes this easy to trip over later.
chiga0
previously approved these changes
Aug 30, 2026
chiga0
left a comment
chiga0
left a comment
Collaborator
There was a problem hiding this comment.
No blocking findings. Standard tier — CI observability stacked on #10394.
What I checked:
ci.yml (+15/-2):
- Pre-install sampling:
DISKCONTEXTheader written sync →sample_diskcalled sync (firstDFSAMPLE) → background sampler loop started (while sleep 10; do sample_disk; done &) →npm ciruns. Order matches the assertions in the new test. ✓ - Exit status:
npm ciis the last command in the step; the EXIT trap fires afternpm cireturns and does not alter the step exit code. Behavioral test confirmed (status 42 with a stub npm). ✓ - Trap cleanup:
pkill -TERM -P $SAMPLER_PID 2>/dev/null || true; kill $SAMPLER_PID 2>/dev/null || true— kills children then parent, both guarded with|| true. ✓ - Unit-test step: now guards
DISKCONTEXTwrite withif [ ! -s "$DISK_SAMPLES" ]so the install-phase samples aren't overwritten when install ran successfully; appends (>>) to same file otherwise. ✓ /proc/meminfousage has2>/dev/null || truefallback — produces an empty field on non-Linux, which is acceptable. ✓HELPER_TESTSupdated to includeci-disk-pressure.test.mjs. ✓
ci-disk-pressure.test.mjs (new, 83 lines):
- Structural test: parses
ci.ymldirectly and assertsDISK_SAMPLES=andDFSAMPLEappear beforenpm ci, trap is present, unit-test step uses>>, upload step usesif-no-files-found: ignore. - Behavioral test: runs actual step shell under
bash -e -o pipefailwith a fakenpmthat exits 42 — verifies status=42 preserved and log containsDISKCONTEXT+DFSAMPLE. CanReader ran this locally: 2/2 pass. ✓
CI: PR targets feature branch (fix/ci-disk-floor-gate-10035), not main — full matrix doesn't run here by design (Linux CI deferred to the stacked stack's CI once retargeted to main per PR description). route and remove-suspicious-attachments pass. ✓
Cross-check: wenshao approved; qwen-code-ci-bot: no blockers, suggestions only (inline style notes); CanReader: independently verified exit-status preservation and test pass.
Reviewed with AI assistance.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
yiliang114
requested review from
LaZzyMan,
doudouOUC,
qqqys and
tanzhenxin
as code owners
August 30, 2026 08:16
yiliang114
dismissed stale reviews from chiga0 and wenshao
August 30, 2026 08:17
The base branch was changed.
yiliang114
enabled auto-merge
August 30, 2026 08:17
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Keep the install sampler non-fatal regardless of command-substitution order and satisfy the regex lint rule in its contract test. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Collaborator
Author
|
Closed the remaining closeout items in |
The disk-floor and disk-sampling legs added ~1 KB of real workflow content; combined with prior main growth this pushed ci.yml 4848 bytes past its recorded 98043, over the 4096 allowance. Bump the ratchet to the current 102891 so the check reflects the reviewed growth. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
chiga0
approved these changes
Aug 30, 2026
chiga0
left a comment
chiga0
left a comment
Collaborator
There was a problem hiding this comment.
Re-review — incremental range 26654c53→c0da45dd (round 2)
No blocking findings.
Round 1 ledger: no tracked findings from round 1; nothing to re-verify.
Incremental changes reviewed:
.github/workflows/ci.yml—sample_diskmoves|| truefrom inside$()to after the closing quote; semantically equivalent underset -e(both protect againstawkfailure). Timeout-minutes expression widens to 90 min on ECS runners and 60 min on hosted, scoped by runner-label substring match — correct..github/workflows/qwen-autofix.yml/qwen-code-pr-review.yml— runner pool migrated fromecs-qwentoecs-agent; pool assertion updated to accept bothecs-qwen-*andecs-agent-*during transition — correct..github/workflows/sdk-java.yml—flockmutex on self-hosted path (fd 9, wait 1200 s, released on process exit); OS reclaims on hard kill — correct..github/scripts/ci-disk-pressure.test.mjs— tightened to assert exactDISK_SAMPLESpath, sampler-loop presence beforenpm ci, format-consistency between install and test phases,result.error === undefinedsentinel, and upload artifact path. These are all new assertions that would have failed with the pre-PR code — non-vacuous..github/actionlint.yaml+.github/workflows/update-ecs-runner-qwen.yml— addsecs-update-hk-3runner label consistently in both places..github/scripts/web-shell-visuals-publish.test.mjs— fixture scoped to a parent directory withpackage.jsonto separate ESM/CJS contexts; cleanup now removes the scope root — correct.
Cross-check: qwen-code-ci-bot (COMMENT, no blockers) and CanReader (tests pass locally) — consistent with my findings, no misses identified.
CI at review time: Dependency CVE audit ✓ · Secret scan ✓ · assign ✓. Build/test jobs still pending; no red jobs visible. Pending is not green — disclosed here.
Not reviewed: docs-only files (new design docs under docs/) and 0-diff files carried by merge commits — no patch content present.
Reviewed with AI assistance.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not reviewed: reverse audit — stopped before round 6 by the review time budget.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未审查：反向审计——评审时间预算不足，未能开始第 6 轮。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.22.3)
Comment on lines
+56
to
+61
| return match[0] | ||
| .replaceAll('${RUNNER_TEMP:-/tmp}', '${TMPDIR}') | ||
| .replace( | ||
| ' /proc/meminfo 2>/dev/null || true)]', | ||
| ' /proc/meminfo 2>/dev/null)]', | ||
| ); |
Collaborator
There was a problem hiding this comment.
[Suggestion] R2-1: The sampleFormat normalizer carries a branch that can never fire: the .replace(...) call below targets the spelling /proc/meminfo 2>/dev/null || true)] (guard inside the command substitution), but neither sampler copy at HEAD has it — the install copy carries its || true outside the assignment's closing quote (ci.yml:485) and the test-step copy has no guard there at all (ci.yml:685). The branch is not merely dead: if a future edit adds || true inside one copy's awk substitution, this normalizer silently absorbs the divergence and the parity assertion stays green — the two samplers drift, and a mixed-format timeline reaches the humans correlating ENOSPC failures, the artifact's only consumer. Drop the (?: \|\| true)? optional group from the regex on line 53 as well; the regex already ends its match at )]", which excludes both copies' trailing guards by construction.
Witness:
intact test + diverged ci.yml (" || true" added inside the install awk substitution):
# pass 2 / # fail 0 <- divergence absorbed
fixed test (.replace call and optional group removed) + diverged ci.yml:
not ok 1 ... AssertionError <- divergence flagged (# fail 1)
fixed test on HEAD ci.yml:
# pass 2 <- fix safe at HEAD
Suggested change
| return match[0] | |
| .replaceAll('${RUNNER_TEMP:-/tmp}', '${TMPDIR}') | |
| .replace( | |
| ' /proc/meminfo 2>/dev/null || true)]', | |
| ' /proc/meminfo 2>/dev/null)]', | |
| ); | |
| return match[0].replaceAll('${RUNNER_TEMP:-/tmp}', '${TMPDIR}'); |
中文说明
sampleFormat 归一化器里有一个永远不会触发的分支：下面的 .replace(...) 针对的是 /proc/meminfo 2>/dev/null || true)]（guard 位于命令替换内部）这种写法，但 HEAD 上的两份采样器副本都不是这种写法——install 副本的 || true 位于赋值引号之外（ci.yml:485），test 步骤副本在该位置完全没有 guard（ci.yml:685）。这个分支不只是死代码：如果未来某次编辑把 || true 加进某个副本的 awk 替换内部，该归一化会悄悄吸收掉这次格式分歧，奇偶断言仍然为绿——两个采样器从此漂移，混杂格式的 timeline 会送到关联 ENOSPC 故障的人工排查者手中，而这个 artifact 只有这一个消费者。请同时去掉第 53 行正则中的 (?: \|\| true)? 可选组；正则本身以 )]" 结尾，构造上已经排除了两份副本尾部的 guard。
— qwen3.8-max via Qwen Code /review (v0.22.3)
Comment on lines
+68
to
+69
| assert.equal(headerLine(install), headerLine(tests)); | ||
| assert.equal(sampleFormat(install), sampleFormat(tests)); |
Collaborator
There was a problem hiding this comment.
[Suggestion] R2-2: The contract suite pins the sampler wiring selectively, and this round demonstrated three behaviours that can silently regress with the whole suite green. (1) The EXIT trap is pinned only by presence, not by ordering against npm ci — the trap can move below npm ci while every assertion stays green, and then bash -e aborts at a failing npm ci before the trap ever registers, so the orphaned sampler holds the step's stdout and the runner waits for output EOF until the job timeout (60 hosted / 90 ECS) instead of failing red promptly. (2) sampleFormat's replaceAll('${RUNNER_TEMP:-/tmp}', '${TMPDIR}') is symmetric while the intended asymmetry is directional — regressing either copy's tmpdir expression is absorbed, so the test step can end up sampling a filesystem other than the routed one, or the install step can sample empty space/inode fields when TMPDIR is unset. (3) The test-step sampler's append to $DISK_SAMPLES (ci.yml:685) is pinned by nothing — deleting that segment leaves the suite green, and a job that fails mid-suite then uploads an artifact containing only install-phase samples, silently losing the ENOSPC-during-test spike the artifact exists to capture. The block below adds the three demonstrated pins; beyond them, please stop extending the suite pin by pin — pin each sampler block as one contiguous literal (the way the guard block is already pinned) or extend the behavioural execution the second test uses to the test-step sampler, so any future edit to the sampler wiring turns the suite red instead of one unpinned behaviour surfacing per review round.
Witness:
trap moved below npm ci: test 1 still "ok 1"; EOF model intact stdoutEofAtMs=120 vs mutant null (15s window), DFSAMPLE written 10090ms after the step's bash exited
tmpdir mutants (either copy regressed): # pass 2 / # fail 0 (absorbed); direction pins vs each mutant: not ok 1 AssertionError
append segment deleted at ci.yml:685: # pass 2 / # fail 0; with the added assertion: not ok 1 (# fail 1); intact + assertion: # pass 2
Suggested change
| assert.equal(headerLine(install), headerLine(tests)); | |
| assert.equal(sampleFormat(install), sampleFormat(tests)); | |
| assert.equal(headerLine(install), headerLine(tests)); | |
| assert.equal(sampleFormat(install), sampleFormat(tests)); | |
| assert.ok(install.indexOf('trap ') > install.indexOf('SAMPLER_PID=$!')); | |
| assert.ok(npmCi > install.indexOf('trap ')); | |
| assert.match(install, /tmpdir\[\$\{RUNNER_TEMP:-\/tmp\}\]/); | |
| assert.match(tests, /tmpdir\[\$\{TMPDIR\}\]/); | |
| assert.match( | |
| tests, | |
| /echo "\$sample" >> "\$DISK_SAMPLES" 2>\/dev\/null \|\| true/, | |
| ); |
The trap pin must keep the trap after SAMPLER_PID=$! (ci.yml:491-492), the tmpdir pins must keep the deliberate asymmetry — the install step samples ${RUNNER_TEMP:-/tmp} before any routing exists and the test step samples the routed ${TMPDIR} (ci.yml:669-670, routing block ci.yml:676-684) — and the append pin must keep the 2>/dev/null || true guard (ci.yml:685, install copy at ci.yml:487), because under the workflow's bash -e an unguarded failing append would kill the sampler loop. Each added assertion must go red against its demonstrated mutant: remove any one of the three pins, re-apply that mutant to ci.yml, and confirm the suite fails — every pin flipped red on its mutant and stayed green on the intact tree during this review.
中文说明
合约测试套件对采样器接线只做选择性固定（pin），本轮演示了三种可以在整套测试保持绿色的情况下悄悄回归的行为。(1) EXIT trap 只被固定了"存在"，没有固定相对 npm ci 的顺序——trap 可以移到 npm ci 之下而所有断言仍为绿；随后 npm ci 失败时 bash -e 会在 trap 注册之前中止，孤儿采样器继续持有该步骤的 stdout，runner 会一直等待输出 EOF 直到 job 超时（托管 60 分钟 / ECS 90 分钟），而不是立刻红掉。(2) sampleFormat 的 replaceAll('${RUNNER_TEMP:-/tmp}', '${TMPDIR}') 是对称的，但预期的不对称是有方向的——任一副本的 tmpdir 表达式回归都会被吸收，test 步骤可能采样到非路由文件系统的磁盘压力，或 install 步骤在 TMPDIR 未设置时采样到空的 space/inode 字段。(3) test 步骤采样器向 $DISK_SAMPLES 的追加（ci.yml:685）没有任何断言固定——删除该段后整套测试仍为绿，job 在测试中途失败时上传的 artifact 将只包含安装阶段的样本，悄悄丢失这个 artifact 本要捕获的测试期间 ENOSPC 峰值。下方代码块补上三个已演示的固定；在此之外，请停止逐个补 pin——把每个采样器块作为一整段连续字面量固定（就像守卫块已有的固定方式），或把第二个测试使用的行为执行扩展到 test 步骤的采样器，让未来任何对采样器接线的修改都能把测试变红，而不是每轮评审都浮现一个未固定行为。
约束：trap 的固定必须保持 trap 位于 SAMPLER_PID=$! 之后（ci.yml:491-492）；tmpdir 固定必须保留刻意的不对称——install 步骤在路由存在之前采样 ${RUNNER_TEMP:-/tmp}，test 步骤采样路由后的 ${TMPDIR}（ci.yml:669-670，路由块 ci.yml:676-684）；追加固定必须保留 2>/dev/null || true 守卫（ci.yml:685，install 副本见 ci.yml:487），因为在 workflow 的 bash -e 下，未守卫的失败追加会杀死采样循环。修复见证：每个新增断言都必须在其演示的突变体下变红——删除三个固定中的任意一个，把对应的突变体重新应用到 ci.yml，确认套件失败；本轮评审中每个固定都已在突变体下变红、在完好代码下保持绿色。
— qwen3.8-max via Qwen Code /review (v0.22.3)
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. |
yiliang114
commented
Aug 31, 2026
yiliang114
left a comment
yiliang114
left a comment
Collaborator
Author
There was a problem hiding this comment.
Reviewed: no findings. Sampling starts before npm ci (header + 10s sampler + EXIT trap that kills the sampler tree without touching the step exit status), the test step only writes the DISKCONTEXT header when the sample file is empty so install-time evidence survives, and the behavioral test proves a failing npm (exit 42) keeps its status while the samples land on disk. Upload stays failure()-only. CI green. (Self-PR: GitHub blocks self-approval, needs another maintainer.)
qwen-code-dev-bot
approved these changes
Aug 31, 2026
qwen-code-dev-bot
left a comment
qwen-code-dev-bot
left a comment
Collaborator
There was a problem hiding this comment.
Gate APPROVE at head 99640471 (merge-only since c0da45dd — per-blob comparison of all PR-owned files shows zero change; my review at c0da45d carries verbatim). The install-step sampler is correctly bounded (EXIT-trap kills the loop, all writes || true, header preserved via [ ! -s ]), the contract suite is wired into both HELPER_TESTS paths, and the pin suite proves it collects. On the 2 open threads: both are test-quality Suggestions (dead normalizer branch in the contract test, selective pinning) — recorded, not blockers. Re the earlier attempt-5 Test(ubuntu) red that held this review: today's #10624 run reproduced the identical signature (five-second-timeout failures only in web-shell suites and one latency-budget test — all in code this PR never touches) on this week's loaded ECS runners; the red was suite-wide load flakiness, not this change. At this head Test (ubuntu) has already gone green; web-shell E2E smoke was still finishing at submission. Maintainer instruction: decided on the review conclusion.
中文：在 head 99640471（相对 c0da45d 纯 merge，自有文件逐 blob 零变化，审查结论原样有效）按 Review 结论通过。install 采样器生命周期正确、契约测试双路径接线完整；2 条遗留为测试质量类建议、不阻塞。此前 attempt-5 的 Test 红今日在 #10624 上复现同签名（全部为 web-shell 套件 5 秒超时 + 一个时延预算用例，均在本 PR 未触碰的代码），确认是本周负载抖动而非本变更；本 head 上 Test(ubuntu) 已绿，提交时 web-shell E2E 尚在跑。
Collaborator
|
Released in v0.23.0. |
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
6 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
Starts the Test job's disk-pressure sampler before
npm ci, writes the first sample synchronously, and keeps sampling every 10 seconds during dependency installation. The sampler is stopped by anEXITtrap without changing the install command's exit status. The later unit-test sampler preserves and appends to the same file, so the existing failure artifact now includes the install timeline instead of starting after installation.This was originally stacked on #10394. After #10394 merged, the branch was refreshed with the latest
mainand this PR was retargeted tomain.Why it's needed
#10394 adds the pre-install disk floor and persists the existing unit-test timeline, but the original #10035 incident failed inside
npm ci. If installation fails after the point-in-time gate passes, the current workflow retains only the gate sample in the job log because the timeline file is not created until the unit-test step. This follow-up closes that remaining repository-side observability gap without adding cross-step background-process state.Reviewer Test Plan
How to verify
node --test --test-concurrency=1 .github/scripts/ci-disk-pressure.test.mjs .github/scripts/check-disk-floor.test.mjs— 8/8 pass. The behavioral case executes the real install-step shell with a fakenpmthat exits 42, verifies the status remains 42, and verifies bothDISKCONTEXTand the synchronous pre-installDFSAMPLEwere written.npm run test:scripts -- scripts/tests/no-ak-integration-ci.test.js— 11 passed, 2 platform-specific cases skipped on macOS.actionlint .github/workflows/ci.yml,yamllint .github/workflows/ci.yml,shellcheck .github/scripts/check-disk-floor.sh, andgit diff --checkpass.Upload disk-pressure samplesand the artifact should contain samples timestamped before and duringnpm ci.Evidence (Before & After)
N/A — CI infrastructure change with no UI surface.
Tested on
Environment (optional)
Local macOS Bash 3.2 and Node.js 22; Linux workflow execution is delegated to this PR CI after retargeting to
main.Risk & Scope
df/inode/memory sample runs every 10 seconds duringnpm ci; the first sample is synchronous so an immediate install failure still leaves evidence.Linked Issues
Part of #10035. #10394 is merged; this follow-up is now based on
mainand does not close the host-level infrastructure work.中文说明
这个 PR 做了什么
在 Test job 执行
npm ci前启动磁盘压力采样器，同步写入第一个样本，并在依赖安装期间每 10 秒继续采样。采样器通过EXITtrap 停止，不改变安装命令本身的退出码。后续单测阶段的采样器会保留并追加同一个文件，因此既有的失败 artifact 现在能够包含安装阶段 timeline，而不是从安装结束后才开始记录。这个 PR 原先以 #10394 为 base；#10394 合并后，分支已合入最新
main，PR 也已切回main。为什么需要
#10394 增加了安装前磁盘门槛，并持久化既有的单测 timeline，但 #10035 的原始事故发生在
npm ci内。如果瞬时门槛通过后安装仍然失败，当前 workflow 只会在 job 日志中保留门槛单点采样，因为 timeline 文件到单测步骤才创建。这个 follow-up 在不引入跨 step 后台进程状态的前提下，补齐剩余的仓库内可观测性缺口。Reviewer Test Plan
如何验证
node --test --test-concurrency=1 .github/scripts/ci-disk-pressure.test.mjs .github/scripts/check-disk-floor.test.mjs：8/8 通过。行为测试用退出码 42 的假npm执行真实安装步骤 shell，确认退出码仍为 42，并确认DISKCONTEXT与同步的安装前DFSAMPLE均已写入。npm run test:scripts -- scripts/tests/no-ak-integration-ci.test.js：11 个通过，macOS 上跳过 2 个特定平台用例。actionlint、yamllint、shellcheck与git diff --check均通过。Upload disk-pressure samples，artifact 应包含npm ci之前及执行期间的时间戳采样。Evidence（Before & After）
N/A——CI 基础设施变更，没有 UI 表面。
测试平台
环境（可选）
本地 macOS Bash 3.2 与 Node.js 22；切回
main后，Linux workflow 执行交由本 PR 的 CI 验证。风险与范围
npm ci期间每 10 秒执行一次轻量的df/inode/内存采样；第一个样本同步写入，因此安装立即失败时仍有证据。关联 Issue
Part of #10035。#10394 已合并；本 follow-up 现以
main为 base，不会关闭主机级基础设施工作。