fix(ci): restore workspace ownership in cleanup to prevent EACCES - #7931
Merged
Conversation
The verify and tmux jobs chown the workspace to node:node for the build step, but the end-of-job cleanup never restores ownership. On self-hosted runners, the next job's checkout step (running as the runner user) fails with EACCES when trying to delete node-owned files in .qwen/ and node_modules/. Add a chown back to the runner user (detected via stat on $RUNNER_TEMP) at the end of both "Clean up runner workspace" steps. Only runs when the runner UID is non-root (i.e. on self-hosted runners where the runner user differs from root).
Collaborator
Collaborator
|
Thanks for the PR! Template looks good ✓ Problem: observed bug with strong evidence — the CI logs from this very PR's run show Direction: CI infrastructure fix that keeps the self-hosted runner pipeline functional. Clearly within scope. Size: not applicable — no core paths touched. Single file ( Approach: the scope feels right. Detect the runner UID via Risk: no elevated risk signals. Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：已观测到的 bug，证据充分——本 PR 的 CI 运行日志中 checkout 阶段就出现了 方向：CI 基础设施修复，保持 self-hosted runner 流水线正常运行。明确在范围内。 规模：不适用——未触及核心路径。单文件（ 方案：范围合理。通过 风险：无升级风险信号。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Collaborator
|
Code review — independent proposal first: given "cleanup steps don't restore ownership after chown -R node:node", I'd add a chown -R back to the runner user at the end of both cleanup steps, detecting the UID from a runner-owned path like RUNNER_TEMP, guarded to skip when running as root. The PR does exactly this. The implementation is clean. The stat -c / stat -f fallback covers Linux and macOS. The No findings. CI test evidence — the one red check tells its own story: Test (ubuntu-latest, Node 22.x) failed in 15s during actions/checkout — the runner couldn't delete node-owned files left by a previous job: This is the exact bug this PR fixes — pre-existing infrastructure failure, not caused by this change (a workflow YAML edit cannot cause unit test failures). All other checks are skipped or passing. Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 Real-scenario testing: N/A — CI-only change, nothing user-visible. 中文说明代码审查——先独立提出方案：鉴于"cleanup 步骤在 chown -R node:node 后没有恢复所有权"，我会在两个 cleanup 步骤末尾加 chown -R 回 runner 用户，从 RUNNER_TEMP 等 runner 所有的路径检测 UID，并在以 root 运行时跳过。PR 的实现与此完全一致。 实现干净。stat -c / stat -f 回退覆盖 Linux 和 macOS。 无发现。 CI 测试证据——唯一的红色检查自己说明了问题： Test (ubuntu-latest, Node 22.x) 在 15 秒内于 actions/checkout 阶段失败——runner 无法删除上一个 job 留下的 node 所有的文件： 这正是本 PR 修复的 bug——预先存在的基础设施故障，不是此更改导致的（workflow YAML 编辑不可能导致单元测试失败）。所有其他检查均为跳过或通过。 真实场景测试：N/A——纯 CI 更改，无用户可见变化。 — Qwen Code · qwen3.8-max-preview Reviewed at |
Collaborator
|
Confidence: 5/5 — clean across every stage; would merge without hesitation. This is a textbook infrastructure fix: the problem is real (the CI failure on this very run is the bug), the solution is the minimal correct change, and the implementation is careful — portable The unit test failure is the pre-existing EACCES issue this PR resolves — it fails at checkout before any test code runs. Merging this PR is the fix for that failure. 中文说明置信度：5/5——每个阶段都很干净；毫不犹豫地合并。 这是一个教科书式的基础设施修复：问题是真实的（本次运行的 CI 失败就是这个 bug），方案是最小且正确的更改，实现很仔细——可移植的 单元测试失败是本 PR 解决的预先存在的 EACCES 问题——它在 checkout 阶段就失败了，根本没有运行任何测试代码。合并这个 PR 就是修复该失败的方法。 — Qwen Code · qwen3.8-max-preview Reviewed at |
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. ✅
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
Local verification reportI rebuilt this failure locally and ran the real cleanup scripts against it. The mechanism is correct and the tmux lane is fully repaired — but the fix does not close the failure quoted in the PR body. The Recommendation: merge, with the one-line How this was verifiedNo GitHub involved — the
Both lanes were seeded with their real end-of-job state: Result
The residual: the
|
| probe | result |
|---|---|
P1 uid/gid detection via $RUNNER_TEMP |
✅ self-hosted → uid=3001; GitHub-hosted (root-owned) → uid=0, chown correctly skipped |
P2 $RUNNER_TEMP unset under the step's real set -uo pipefail |
✅ no hard failure — the subshell dies, echo 0 wins, restore is skipped. Fails safe |
P3 stat -c → stat -f fallback |
✅ works under a BSD/macOS stat. On GNU the fallback is unreachable (GNU -f is filesystem-mode), which is the correct outcome |
P4 chown -R vs. symlinks planted by PR code |
✅ important: link targets are untouched (chown -R defaults to -P). A PR that plants evil -> /etc does not get /etc chowned |
P5 $RUNNER_TEMP/*-results |
|
| P6 cost | ✅ chown -R over 51,001 paths: 0.10–0.16 s on ext4 |
Note on the PR body
Main risk:
chown -Ron ~50k node_modules files takes a few seconds.
Measured at 0.1 s (P6) — the cost is negligible even on the critical path, let alone in an if: always() cleanup step. The stated risk is not a real one.
Not covered
- No real self-hosted runner and no Docker on this box: the container is modelled by uid equivalence. That is exact for file-permission semantics (no userns remapping is in play — the container's root writes root-owned files that the host also sees as root), but it does not exercise anything Docker-specific.
actions/checkoutis modelled from its documented flow plus the real failing log, not by running the action itself.- The autofix "Push and report" failure mentioned in the PR body was out of scope, as stated.
中文完整版
本地验证报告
我在本地把这个故障完整复现了一遍，并用真实的 cleanup 脚本跑了 A/B。机制是对的，tmux lane 被完全修好了 —— 但这个 PR 并没有闭合它在描述里引用的那个报错。 verify lane 仍然会把下一个 job 卡死，因为 .qwen 不只是被 chown 成 node，它还被 去掉了写位。补一行即可，下面已验证。
结论：建议合入，并补上那行 chmod（不补也严格优于 main，所以放到 follow-up PR 也可以）。
怎么验证的
完全不经过 GitHub，在一台 Linux 机器上复刻了 ecs-qwen-runner-sg-15 的拓扑：
| 部件 | 线上 | Harness |
|---|---|---|
| 宿主机 runner 用户 | github-runner，/home/github-runner/actions-runner-15/_work/… |
github-runner（uid 3001），同样的路径形状 |
tmux-testing / verify |
node:22-bookworm 容器，以 root 运行，workspace 与 $RUNNER_TEMP 是 bind mount |
root + node 用户；bind mount 的容器与宿主机共享 uid 命名空间，DAC 语义完全一致 |
| 被测步骤 | Clean up runner workspace |
用 PyYAML 从 qwen-triage.yml 原样抽取该步骤的 run: 块 —— base = merge-base 0afb48b1e，PR = 56b7dd3e2 |
| 下一个 job | actions/checkout@v6.0.3 的 Checkout base branch（ref: main） |
prepareExistingDirectory（git clean -ffdx → 失败则对每个顶层条目 io.rmRF）加上 ref 切换，以 github-runner 身份运行 |
| workspace | 真实仓库树 | 真实仓库树，含真实的 .qwen/（skills/、e2e-tests/、agents/） |
两条 lane 都按各自真实的 job 结束态构造：tmux-testing → chown -R node:node $GITHUB_WORKSPACE；verify → 同上，再加 Re-pin the verifier 步骤的 chown -R root:root .qwen + chmod -R a-w,a+rX .qwen。
结果
| job 结束态 | base（无 restore） | PR #7931 |
|---|---|---|
tmux-testing —— node 所有 |
❌ 失败（EACCES） | ✅ 通过 |
verify —— node 所有 + root 所有、且去写位的 .qwen |
❌ 失败（EACCES） | ❌ 仍然失败（EACCES） |
残留问题：verify lane
verify lane 不只是改所有者，它还把写位摘掉了：
chown -R root:root .qwen
chmod -R a-w,a+rX .qwen # 目录 -> dr-xr-xr-x，文件 -> r--r--r--恢复所有者并不恢复写位，而 unlink/rmdir 要的是父目录的写权限 —— 只是"拥有"它没有用。打上本 PR 之后，github-runner 拥有 .qwen/e2e-tests 却依然动不了它，两条 checkout 路径都还是失败：
- ref 切换路径 ——
error: unable to unlink old '.qwen/e2e-tests/2026-06-13-file-history-snapshot-persistence.md': Permission denied - 删除目录内容路径（sg-15 当时实际所处的状态：之前一次删到一半，
.git已经没了）——EACCES: permission denied, rmdir '…/.qwen/agents'
第一条里的文件，和 run 30354856247 失败日志里的是同一个。
两个 cleanup 步骤各补一行即可 —— 两条 lane、两条 checkout 路径都已实测通过：
if [ "$RUNNER_UID" != "0" ]; then
chown -R "$RUNNER_UID:$RUNNER_GID" "$GITHUB_WORKSPACE" 2>/dev/null || true
+ chmod -R u+rwX "$GITHUB_WORKSPACE" 2>/dev/null || true
fiu+rwX 不会造成 mode 漂移：大写 X 只在 a+rX 之后仍留有 x 位的地方补 x，而 git 只跟踪 0755/0644 这一个区分。
两个小口子
(a) [ -e .git ] || exit 0 这道 guard 在 restore 之前。 新增的块是追加在步骤末尾的，所以如果 cleanup 时 workspace 里没有 .git（agent 以 yolo 模式跑 PR 代码，postinstall 或被诱导的 agent 都可能把它删掉），步骤会提前返回，所有权永远不会被恢复。探针 P7：workspace 保持 node 所有。把 restore 提到 EXIT trap 里可以解决（P7b），顺带也覆盖了 exit 1 的路径。
(b) $RUNNER_TEMP 没有被恢复。 prepare 步骤同时把 $RUNNER_TEMP/tmux-results、verify-results、verify-context chown 给了 node，而新增的块只覆盖 $GITHUB_WORKSPACE。它们会一直是 node 所有，runner 用户清不掉（P5）。影响比 workspace 小 —— runner 每个 job 会重建 _temp，而且 mkdir -p 加一次新的 chown 能自愈这条路径 —— 但它是同一个泄漏，顺手带上不花什么成本。
其余都是绿的
| 探针 | 结果 |
|---|---|
P1 通过 $RUNNER_TEMP 探测 uid/gid |
✅ self-hosted → uid=3001；GitHub-hosted（root 所有）→ uid=0，正确跳过 chown |
P2 在该步骤真实的 set -uo pipefail 下 $RUNNER_TEMP 未设置 |
✅ 不会硬失败 —— 子 shell 死掉、echo 0 生效、restore 被跳过，失败方向是安全的 |
P3 stat -c → stat -f 回退 |
✅ 在 BSD/macOS 版 stat 下工作正常；在 GNU 上这条回退不可达（GNU 的 -f 是文件系统模式），这正是期望行为 |
P4 chown -R 与 PR 代码埋的符号链接 |
✅ 重要：链接目标不受影响（chown -R 默认 -P）。埋一个 evil -> /etc 不会导致 /etc 被 chown |
P5 $RUNNER_TEMP/*-results |
|
| P6 开销 | ✅ 对 51,001 个路径做 chown -R：ext4 上 0.10–0.16 秒 |
关于 PR 描述
Main risk:
chown -Ron ~50k node_modules files takes a few seconds.
实测 0.1 秒（P6）—— 即便放在关键路径上开销也可以忽略，何况它在 if: always() 的 cleanup 步骤里。这条风险实际不成立。
未覆盖
- 没有真实 self-hosted runner，机器上也没有 Docker：容器是用 uid 等价性建模的。对文件权限语义而言这是精确的（这里不涉及 userns remapping —— 容器里 root 写出的文件在宿主机上同样是 root 所有），但它不覆盖任何 Docker 特有的行为。
actions/checkout是按其公开流程加上真实失败日志建模的，没有真正运行这个 action。- PR 描述里提到的 autofix "Push and report" 失败按其声明属于范围外，未涉及。
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
Collaborator
Author
yiliang114
approved these changes
Jul 28, 2026
Collaborator
Author
|
🤖 Addressed the latest review feedback (round 1/10). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/10 轮）。改动内容与我反驳保留之处如下： Review feedback addressed1. Verify lane still fails: write bits not restored (Required — fixed)Feedback: The verify lane strips write permissions from Decision: Implemented. Added 2. EXIT trap: restore skipped when
|
qwen-code-dev-bot
pushed a commit
that referenced
this pull request
Jul 28, 2026
…7931) The verify lane strips write permissions from .qwen (chmod -R a-w,a+rX), so restoring ownership alone does not fix EACCES — the runner user owns the directory but still cannot unlink or rmdir inside it. Add chmod -R u+rwX after chown to restore write bits. Hoist the restore into an EXIT trap so it fires even when the .git guard or an error exits the cleanup step early.
doudouOUC
requested changes
Jul 28, 2026
Comment on lines
+2844
to
+2845
| chown -R "$RUNNER_UID:$RUNNER_GID" "$GITHUB_WORKSPACE" 2>/dev/null || true | ||
| fi |
Collaborator
There was a problem hiding this comment.
[Critical] verify lane 仍然失败：chown -R 恢复了所有权但没有恢复写权限 — Failure scenario: verify job 的 "Re-pin the verifier" 步骤（第 2365 行）执行 chmod -R a-w,a+rX .qwen 将 .qwen 变为 dr-xr-xr-x。cleanup 的 chown -R 恢复了 runner user 所有权，但没有写权限的目录不允许 unlink/rmdir。下一个 job 的 actions/checkout（在宿主机上以 runner user 身份运行，不在容器内）失败：EACCES: permission denied, unlink '.../.qwen/e2e-tests/...'。审查者 @wenshao 通过本地 A/B 复现验证了此问题（comment 5104025458）：tmux-testing lane 通过，verify lane 仍然失败。
Suggested change
| chown -R "$RUNNER_UID:$RUNNER_GID" "$GITHUB_WORKSPACE" 2>/dev/null || true | |
| fi | |
| chown -R "$RUNNER_UID:$RUNNER_GID" "$GITHUB_WORKSPACE" 2>/dev/null || true | |
| chmod -R u+rwX "$GITHUB_WORKSPACE" 2>/dev/null || true | |
| fi |
中文说明
verify job 的 "Re-pin the verifier" 步骤执行 chmod -R a-w,a+rX .qwen 去掉了写权限。仅恢复所有者（chown）不够——unlink/rmdir 需要父目录的写权限，不仅仅是所有权。下一个 job 的 actions/checkout 在宿主机上以 runner user 运行（不在容器内），遇到 EACCES。建议在 chown -R 后添加 chmod -R u+rwX。大写 X 只在已有 x 位处补回执行位，不会造成 mode 漂移。
— qwen3.7-max via Qwen Code /review
Comment on lines
+1529
to
+1530
| RUNNER_UID="$(stat -c '%u' "$RUNNER_TEMP" 2>/dev/null || stat -f '%u' "$RUNNER_TEMP" 2>/dev/null || echo 0)" | ||
| RUNNER_GID="$(stat -c '%g' "$RUNNER_TEMP" 2>/dev/null || stat -f '%g' "$RUNNER_TEMP" 2>/dev/null || echo 0)" |
Collaborator
There was a problem hiding this comment.
[Suggestion] 静默失败路径缺少诊断日志 — Concrete cost: 如果两个 stat 命令都失败（$RUNNER_TEMP 未设置或不可读），UID/GID 静默回退到 "0"，chown 被跳过，没有任何日志输出说明发生了什么。下一个 job 出现 EACCES 时，日志无法指示根本原因。
Suggested change
| RUNNER_UID="$(stat -c '%u' "$RUNNER_TEMP" 2>/dev/null || stat -f '%u' "$RUNNER_TEMP" 2>/dev/null || echo 0)" | |
| RUNNER_GID="$(stat -c '%g' "$RUNNER_TEMP" 2>/dev/null || stat -f '%g' "$RUNNER_TEMP" 2>/dev/null || echo 0)" | |
| RUNNER_UID="$(stat -c '%u' "$RUNNER_TEMP" 2>/dev/null || stat -f '%u' "$RUNNER_TEMP" 2>/dev/null || echo 0)" | |
| RUNNER_GID="$(stat -c '%g' "$RUNNER_TEMP" 2>/dev/null || stat -f '%g' "$RUNNER_TEMP" 2>/dev/null || echo 0)" | |
| echo "Ownership restoration: RUNNER_UID=$RUNNER_UID, RUNNER_GID=$RUNNER_GID" |
中文说明
如果 stat 失败导致 UID 回退为 "0"，chown 被跳过但没有任何日志输出。当后续 job 出现 EACCES 时，运维人员无法从日志判断是否因为所有权恢复被跳过。建议添加 echo 输出检测到的 UID/GID。
— qwen3.7-max via Qwen Code /review
Comment on lines
+1525
to
+1528
| # Restore workspace ownership to the runner user so the next job's | ||
| # checkout step can clean the tree without EACCES. The prepare step | ||
| # chowned everything to node:node; without this, node_modules and | ||
| # .qwen/ remain node-owned and the next checkout's rm -rf fails. |
Collaborator
There was a problem hiding this comment.
[Suggestion] 重复代码无交叉引用 — Concrete cost: 这个 9 行块在 tmux-testing（此处）和 verify（约第 2837 行）的 cleanup 步骤中完全相同地出现了两次。修改其中一处（如添加 chmod）时，维护者可能不知道另一处也需要同步更新。
Suggested change
| # Restore workspace ownership to the runner user so the next job's | |
| # checkout step can clean the tree without EACCES. The prepare step | |
| # chowned everything to node:node; without this, node_modules and | |
| # .qwen/ remain node-owned and the next checkout's rm -rf fails. | |
| # Restore workspace ownership to the runner user so the next job's | |
| # checkout step can clean the tree without EACCES. The prepare step | |
| # chowned everything to node:node; without this, node_modules and | |
| # .qwen/ remain node-owned and the next checkout's rm -rf fails. | |
| # (duplicated near line ~2837 in verify job — keep in sync) |
中文说明
这 9 行代码在 tmux-testing 和 verify 两个 job 的 cleanup 步骤中完全一样。未来修改其中一处时（比如加 chmod），维护者可能不知道另一处也需要改。建议在注释中添加交叉引用，说明两处需保持同步。
— qwen3.7-max via Qwen Code /review
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
5 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
Adds a
chown -Rback to the runner user at the end of both "Clean up runner workspace" steps (tmux-testing and verify jobs) inqwen-triage.yml.Why it's needed
The verify and tmux jobs
chown -R node:node $GITHUB_WORKSPACEbefore runningnpm ciandnpm run buildas thenodeuser. The end-of-job cleanup step removes stale worktrees and tmp dirs but never restores ownership. On self-hosted runners, the next job'sactions/checkoutstep (running as the runner user, e.g.github-runner) fails with:This was the root cause of all PR Review and Triage failures observed today (10+ runs).
How it works
The runner user's UID/GID is detected via
staton$RUNNER_TEMP(which is always owned by the runner user). If the UID is non-root (i.e. we're on a self-hosted runner where the runner user differs from root), the workspace ischowned back. On GitHub-hosted runners (where everything runs as the same user), the UID is 0 and the chown is skipped.Reviewer Test Plan
How to verify
/verifyor/tmuxrun on any PR (uses self-hosted runner).Tested on
CI-only change on self-hosted runners.
Risk & Scope
chown -Ron ~50k node_modules files takes a few seconds. This runs in the cleanup step (if: always()) which is not on the critical path.Linked Issues
N/A
中文说明
verify/tmux job 的 build 步骤把 workspace chown 给 node 用户，但 cleanup 步骤没有 chown 回来。self-hosted runner 上下一个 job 的 checkout 删不掉 node 所有的文件，报 EACCES。在 cleanup 末尾加 chown 回 runner 用户，通过 stat $RUNNER_TEMP 检测 runner UID，非 root 时才执行。