fix(ci): route E2E temp files off /tmp - #10376
Merged
Merged
Conversation
Collaborator
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
Collaborator
|
Thanks for the PR! Template looks good ✓ Problem: Observed, with evidence — run 33150824980 on Direction: Aligned — this is CI reliability on the exact workflow that failed, and it reuses the Size: Not applicable — no core paths touched (+17/-0 across a workflow YAML and its structural test). Approach: The scope feels right. The inserted block is byte-identical to the merged pattern in Risk: No elevated risk signals — neither changed file matches the high-risk path patterns. Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：已观测到，且有证据—— 方向：对齐——这是针对出错 workflow 本身的 CI 可靠性修复，并且复用了 规模：不适用——未触及核心路径（+17/-0，涉及一个 workflow YAML 及其结构测试）。 方案：范围合理。插入的代码块与 风险：无升级风险信号——两个变更文件均未命中高风险路径模式。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
Code reviewNo blockers. The diff is exactly the minimal change the problem needs. The added routing block is byte-identical to the pattern already merged in three places in The structural test follows the existing conventions of One non-blocking note: the routing snippet now exists four times across Testing evidenceUnattended CI run — per triage rules no PR code was built or executed here; the evidence below is the PR's own CI on the reviewed commit, fetched via the API. No failures so far. The Linux unit check — the one that runs the new structural test — is still in progress; not verified until it lands: the pending Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 中文说明代码审查无阻塞问题。diff 正是解决问题所需的最小改动。 新增的路由代码块与 结构测试遵循 一条非阻塞备注：该路由片段现在在 测试证据无人值守 CI 运行——按 triage 规则，此处未构建或执行任何 PR 代码；以下证据是所审提交上 PR 自身的 CI，通过 API 获取。目前无失败。Linux 单元检查（运行新结构测试的那个）仍在进行中；在落地前不算验证完成：待定的 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
|
Confidence: 5/5 — observed failure, minimal fix, proven pattern reused verbatim; nothing left to second-guess. My independent take on "E2E shards die when a reused runner's The one thing this PR's CI cannot show is the fix actually firing in a real E2E run — Approving, with approval deferred until CI lands green on 中文说明置信度：5/5 —— 已观测到的故障、最小化修复、逐字节复用已验证的模式；没有可再质疑的地方。 面对"复用 runner 的 本 PR 的 CI 唯一无法展示的是修复在真实 E2E 运行中生效—— 同意合入；因 Linux 单元检查（运行新的 workflow 测试）仍在进行中，批准将延迟到 — Qwen Code · qwen3.8-max Reviewed at |
yiliang114
commented
Aug 28, 2026
yiliang114
left a comment
yiliang114
left a comment
Collaborator
Author
There was a problem hiding this comment.
Reviewed at b957655 — no findings. Verified against the head source: the block sits at the top of the Run E2E tests step of e2e-test-linux (e2e.yml:212+); the mktemp fallback keeps the RUNNER_TEMP-based TMPDIR with no trap attached, so the cleanup can never rm a fallback directory; the trap body is single-quoted, $TMPDIR expands at EXIT time, and nothing reassigns TMPDIR after the trap is set, so the removal target stays the private /var/tmp/qwen-ci-XXXXXX. The new workflow-test pins match the actual step content, and the job key / step name it queries exist as written. CI so far is green on this head; the ubuntu Test leg (which executes the pinned helper test) is still running.
(Self-PR: GitHub blocks self-approval, so this is a comment review; merge approvals need to come from other maintainers.)
chiga0
approved these changes
Aug 28, 2026
chiga0
left a comment
chiga0
left a comment
Collaborator
There was a problem hiding this comment.
Scope: .github/workflows/e2e.yml (9 added lines) and scripts/tests/e2e-workflow.test.js (8 added lines). CI-only change; no source, no public API, no persisted format.
Tier: Scan → elevated to Standard on the stated-intent check (PR claims it reuses the main CI routing pattern).
Claim verified — ci.yml parity confirmed.
The PR states "reuses the temporary-directory routing already used by the main CI test jobs." Checked ci.yml at the base ref: the exact same block (export TMPDIR, Linux-gated mktemp -d /var/tmp/qwen-ci-XXXXXX, conditional trap … EXIT) appears at lines 568–579, 960–971, and 1132–1143. The claim holds.
Shell correctness. trap 'rm -rf "$TMPDIR" 2>/dev/null || true' EXIT — the single-quoted string defers $TMPDIR expansion to exit time. Since nothing in the run: block reassigns TMPDIR after this point, the correct directory is cleaned. The fallback chain RUNNER_TEMP:-${TMPDIR:-/tmp} is present on non-Linux paths too; safe.
Test validity. e2e-workflow.test.js checks string containment of mktemp -d /var/tmp/qwen-ci-XXXXXX and the trap. Reverting the fix would fail both assertions. Not vacuous.
Minor — observability parity gap (inline, .github/workflows/e2e.yml line 229)
The ci.yml version of this same TMPDIR block is followed by a background DFSAMPLE monitoring loop:
( while true; do echo "DFSAMPLE $(date ...) tmpdir[${TMPDIR}] space[...] inodes[...] memavail[...]" 2>/dev/null; sleep 10; done ) &ci.yml comment: "Sample the routed temp filesystem every 10s so the failing run captures the spike."
e2e.yml omits it. Not a correctness defect — the fix routes correctly. But if /var/tmp fills again in an E2E shard, there is no per-10s disk sample to diagnose the failure, unlike the main CI jobs. Consider adding the same loop for diagnostic parity.
CI at review time:
Test (ubuntu-latest, Node 22.x)— PENDING (the job most likely to rune2e-workflow.test.js)Integration Tests (CLI, No Sandbox)— SKIPPED (path filter; CI-only change, expected)Test (macos-latest, Node 22.x)/Test (windows-latest, Node 22.x)— SKIPPED (expected for a Linux-only CI change)Desktop Shell (ubuntu-22.04),Desktop Shell (windows-2022),Secret scan,Dependency CVE audit— all PASS
The pending ubuntu test is not a blocker here — the change is CI config only and the test is a YAML structure check that is observable locally. Approving; the pending result does not affect the correctness judgment.
Unreviewed dimensions: macOS and Windows test jobs SKIPPED (no evidence, not expected to cover this change). Working tree unavailable; execution rungs 1–3 not run (CI-only change, not needed).
No blockers found.
Reviewed with AI assistance.
| if [ -n "$QWEN_CI_TMPDIR" ]; then | ||
| TMPDIR="$QWEN_CI_TMPDIR" | ||
| export TMPDIR | ||
| trap 'rm -rf "$TMPDIR" 2>/dev/null || true' EXIT |
Collaborator
There was a problem hiding this comment.
Minor — observability parity gap
ci.yml follows the identical TMPDIR block with a background disk/memory sampling loop:
( while true; do echo "DFSAMPLE $(date -u +%H:%M:%S 2>/dev/null) tmpdir[${TMPDIR}] space[$(df -h "${TMPDIR}" 2>/dev/null | tail -1)] inodes[$(df -i "${TMPDIR}" 2>/dev/null | tail -1)] memavail[$(awk '/MemAvailable/ {print $2, $3}' /proc/meminfo 2>/dev/null)]" 2>/dev/null; sleep 10; done ) &The ci.yml comment explains the intent: "Sample the routed temp filesystem every 10s so the failing run captures the spike."
This is not a correctness issue — the routing and cleanup are correct. But if /var/tmp becomes a bottleneck again in an E2E shard, there will be no per-10s disk sample in the job log to diagnose it, unlike the main CI jobs. Consider adding the same monitoring loop here for diagnostic parity.
yiliang114
enabled auto-merge
August 28, 2026 08:16
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
Aug 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. ✅
Collaborator
|
Released in v0.22.3. |
This was referenced Aug 29, 2026
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
Gives every Linux E2E shard its own disk-backed temporary directory under
/var/tmp, cleans it when the test step exits, and falls back to the runner-provided temporary directory if allocation is unavailable. This reuses the temporary-directory routing already used by the main CI test jobs.Why it's needed
Main E2E run 33150824980 exhausted the reused runner's
/tmpwhile creating a test workspace. The same allocation failed on the initial attempt and both Vitest retries, while the other 119 tests in the shard passed. A prior scheduled run failed with the same/tmpENOSPC signature, so rerunning alone does not prevent recurrence.Regression origin
This failure was not introduced by #10274; that merge only triggered the main run where the runner exhausted
/tmp. The regression boundary is #10085, which moved the Linux E2E shards from ephemeral hosted runners to the persistent ECS pool without carrying over the per-run/var/tmprouting already used by the main CI jobs. That made accumulated or capacity-limited shared/tmpstate visible to later E2E runs.Reviewer Test Plan
How to verify
Confirm each Linux E2E shard creates a private temporary directory on
/var/tmp, exports it before Vitest starts, and removes only that private directory when the step exits. The E2E workflow structure test should continue to pass.Evidence (Before & After)
Before: Docker sandbox shard 1/3 failed three times with
ENOSPC: no space left on device, mkdtemp '/tmp/qwen-test-live-journal-recovery-XXXXXX'. After: the workflow regression test passed 9/9 locally and confirms the/var/tmproute plus cleanup trap.Tested on
Environment (optional)
Node.js 22; focused workflow test.
Risk & Scope
/tmpto the disk-backed/var/tmpfilesystem.Linked Issues
Closes #10375
中文说明
本 PR 的修改
为每个 Linux E2E shard 在
/var/tmp下创建独立的磁盘临时目录，在测试步骤退出时清理；如果目录分配失败，则回退到 runner 提供的临时目录。该方案复用了主 CI 测试任务已有的临时目录路由方式。修改原因
main E2E run 33150824980 在复用 runner 上创建测试 workspace 时耗尽了
/tmp。首次执行以及两次 Vitest 重试都在同一处分配失败，而该 shard 的其余 119 个测试均通过。此前一次定时运行也出现了相同的/tmpENOSPC，因此仅重跑不能避免复发。回归来源
该失败不是由 #10274 引入；该合并只触发了 runner 耗尽
/tmp的 main run。回归边界是 #10085：它将 Linux E2E shard 从临时 hosted runner 迁移到复用 ECS runner，但没有带上主 CI 已使用的每次运行独立/var/tmp路由，因此共享/tmp中累积或受容量限制的状态会影响后续 E2E run。Reviewer 测试计划
验证方式
确认每个 Linux E2E shard 都会在
/var/tmp创建私有临时目录，在 Vitest 启动前导出该目录，并在步骤退出时只删除自己的私有目录。E2E workflow 结构测试应继续通过。修改前后证据
修改前：Docker sandbox shard 1/3 连续三次失败，错误为
ENOSPC: no space left on device, mkdtemp '/tmp/qwen-test-live-journal-recovery-XXXXXX'。修改后：本地 workflow 回归测试 9/9 通过，并验证了/var/tmp路由和清理 trap。测试环境
环境（可选）
Node.js 22；聚焦 workflow 测试。
风险与范围
/tmp移到磁盘型/var/tmp文件系统。关联 Issue
Closes #10375