ci: isolate per-run agent state for triage and PR review - #5885
Merged
Conversation
Both the triage job and the PR-review job run the qwen agent directly on the persistent self-hosted ECS pool with no per-run isolation. $HOME, /tmp and the workspace are reused between runs, so a prior run's agent session/memory (default ~/.qwen) or leftover draft comments (/tmp/stage-*.md, which survive git clean) can bleed into the next run. This surfaced on #5874: its triage posted #5872's review verbatim (wrong author, wrong approver, wrong diff), while the same run's internal stage actually exercised #5874 — the PR id was correct, the agent state was stale. Point QWEN_HOME at a per-run $RUNNER_TEMP/qwen-home on the Qwen step and reset it (plus /tmp/stage-*.md) in the pre-run cleanup, for both jobs. QWEN_HOME relocates the entire global qwen dir (storage.ts), so this isolates sessions/memory/temp without touching $HOME and disturbing git/npm. The tmux-testing job is already container-isolated and unchanged. Refs #5882
Collaborator
|
Thanks for the PR! Template looks good ✓ On direction: this directly fixes #5882 — a critical CI reliability issue where the triage bot posted another PR's analysis due to shared agent state on the self-hosted runner. Clearly within scope; CI reliability is foundational. No CHANGELOG reference needed for internal CI changes. On approach: the scope is minimal and focused — two workflow files, +34/-10, each change directly serves the stated goal. Using Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 方向：直接修复 #5882 —— 自建 runner 上 agent 状态共享导致 triage bot 把另一个 PR 的分析贴到了错误的 PR 上。CI 可靠性是基础设施，方向毫无疑问。内部 CI 改动不需要 CHANGELOG 引用。 方案：范围最小且聚焦 —— 两个 workflow 文件，+34/-10，每处改动都直接服务于目标。在 step 级使用 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Collaborator
Code ReviewIndependent proposal: To fix cross-PR state contamination on self-hosted runners, I would set Diff analysis: Both workflow files receive identical, focused changes. No correctness bugs, no security issues, no scope creep. Both files' changes are consistent. Reuse check: YAML validation: Both files parse cleanly. TestingThis is a CI-only workflow change — no user-visible CLI behavior to exercise in tmux. The PR author correctly marks macOS/Windows as N/A and Linux as Verification performed:
中文说明代码审查独立方案： 为修复自建 runner 上的跨 PR 状态污染，我会在启动 qwen agent 的 step 上将 Diff 分析： 两个 workflow 文件收到相同的、聚焦的改动。 无正确性 bug、无安全问题、无范围蔓延。两个文件的改动一致。 复用检查： YAML 校验： 两个文件解析正常。 测试这是纯 CI workflow 改动 —— 无用户可见的 CLI 行为可在 tmux 中测试。PR 作者正确地将 macOS/Windows 标记为 N/A，Linux 标记为 已执行的验证：
— Qwen Code · qwen3.7-max |
Collaborator
ReflectionThis PR fixes a real, embarrassing production bug — the triage bot posted another PR's analysis because agent state leaked between runs on the shared self-hosted runner. The motivation is clear, the root cause analysis in #5882 is thorough, and the fix is minimal. Looking at the whole picture: the approach matches what I'd independently propose. The PR description is unusually thorough for a CI change — the incident analysis, the "why not just containerize" reasoning, and the explicit scope boundaries (acknowledging the If I had to maintain this in six months: the comments in the cleanup step explain what's being reset and why, the Approving. ✅ 中文说明反思本 PR 修复了一个真实且令人尴尬的生产 bug —— triage bot 发布了另一个 PR 的分析，因为 agent 状态在共享的自建 runner 上泄漏到了下一次运行。动机明确，#5882 中的根因分析详尽，修复最小化。 整体来看：方案与我独立提出的方案一致。 PR 描述对于一个 CI 改动来说异常详尽 —— 事件分析、"为什么不直接容器化" 的推理、以及明确的范围边界（承认 如果六个月后我需要维护这段代码：清理步骤的注释解释了正在重置什么以及为什么， 批准 ✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
approved these changes
Jun 26, 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. |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
[Suggestion] .github/workflows/qwen-code-pr-review.yml:358 — The trailing echo still reads "stale review worktrees cleaned" (the old step name), while the triage workflow correctly updated its equivalent to "stale agent state cleaned". The step was renamed in both files but only the triage echo was updated.
echo "stale agent state cleaned"
— qwen3.7-max via Qwen Code /review
qwen-code-ci-bot
approved these changes
Jun 26, 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. ✅
Collaborator
Author
Validation
Run 28220474741 (triage on
Supporting checks:
The "not auto-migrated" warning is expected and harmless in CI — the triage agent gets skills from the checked-out |
Collaborator
Author
|
Non-blocking follow-up — one narrower path this PR doesn't fully close. The skill writes its draft comments to a literal Low severity: same-PR runs are cancelled by the concurrency group, and sessions/memory (the proven carrier) are now isolated. The clean fix is to have the skill write drafts under |
yiliang114
enabled auto-merge
June 26, 2026 06:28
DennisYu07
approved these changes
Jun 26, 2026
chiga0
approved these changes
Jun 26, 2026
chiga0
left a comment
chiga0
left a comment
Collaborator
There was a problem hiding this comment.
Overview
Final Verdict: Approve — Clean fix for a real and embarrassing cross-run state contamination bug. The approach is correct: QWEN_HOME relocates the entire agent state directory (sessions, memory, settings, temp) per storage.ts, so pointing it at ${RUNNER_TEMP}/qwen-home gives full isolation without touching $HOME. The ${RUNNER_TEMP:?} guard prevents catastrophic empty-path rm -rf.
Findings Summary
- Critical/Major: 0 items
- Minor: 0 items
- Nit: 1 item (echo text inconsistency, noted by CI bot)
Cross-Validation
| Finding | Other Reviewer | My Assessment |
|---|---|---|
Echo text "stale review worktrees cleaned" not updated in qwen-code-pr-review.yml |
qwen-code-ci-bot | Confirmed — triage workflow correctly updated to "stale agent state cleaned" but PR-review workflow still has the old text. Cosmetic only. |
Additional Audit Coverage
${RUNNER_TEMP:?}guard: Correct bash idiom — fails loudly ifRUNNER_TEMPis unset/empty, preventingrm -rfon an empty path. Both cleanup steps use it consistently.QWEN_HOMEconsistency: Both workflows setQWEN_HOMEidentically — cleanup step uses${RUNNER_TEMP:?}/qwen-home, Qwen step uses${{ runner.temp }}/qwen-home. These resolve to the same path (GitHub Actionsrunner.temp= shellRUNNER_TEMP).rm -rfsafety: All destructive operations have2>/dev/null || true— cleanup never fails the job, matching the "never fail the job" intent in comments.- Existing worktree cleanup preserved: Both workflows retain the
.qwen/tmp/*andgit worktree prunecleanup for interrupted runs. The newQWEN_HOMEreset is additive. - tmux-testing job untouched: Correct — it's already container-isolated, so per-run state isolation is handled by Docker.
This review was generated by QoderWork AI
doudouOUC
reviewed
Jun 26, 2026
doudouOUC
left a comment
doudouOUC
left a comment
Collaborator
There was a problem hiding this comment.
[Critical] Missing test coverage for agent isolation logic
The new QWEN_HOME reset, /tmp/stage-*.md cleanup, and QWEN_HOME env var on the agent step — introduced in both review-pr and the main triage job — have no test assertions. scripts/tests/qwen-resolve-workflow.test.js only covers resolve-pr, and scripts/tests/qwen-triage-workflow.test.js only covers the tmux-testing sub-workflow.
A future edit could silently drop the QWEN_HOME env var or the cleanup block, re-introducing the exact cross-PR state bleed this PR fixes. Suggest adding toContain assertions following the existing pattern:
it('isolates agent state per run', () => {
expect(cleanStep).toContain('QWEN_HOME="${RUNNER_TEMP:?}/qwen-home"');
expect(cleanStep).toContain('rm -rf "$QWEN_HOME"');
expect(cleanStep).toContain('rm -f /tmp/stage-*.md');
});
it('passes QWEN_HOME to the agent step', () => {
expect(agentStep).toContain("QWEN_HOME: '${{ runner.temp }}/qwen-home'");
});[Suggestion] Stale echo in review workflow: line 358 still reads echo "stale review worktrees cleaned" while the step was renamed to "Clean stale agent state". The triage workflow correctly updated its echo to "stale agent state cleaned" — this one was missed.
[Suggestion] Concurrent jobs share $RUNNER_TEMP/qwen-home — if a triage and review run execute concurrently on the same self-hosted runner, Job B's cleanup rm -rf destroys Job A's in-flight agent state. Consider adding ${{ github.run_id }} to the path for uniqueness:
QWEN_HOME="${RUNNER_TEMP:?}/qwen-home-${{ github.run_id }}"
# Also sweep orphaned dirs from crashed prior runs:
rm -rf "${RUNNER_TEMP:?}"/qwen-home-* 2>/dev/null || true— qwen3.7-max via Qwen Code /review
| set -uo pipefail | ||
| # Fresh per-run agent home (must match QWEN_HOME on the Qwen step | ||
| # below) + drop any leftover stage drafts. | ||
| QWEN_HOME="${RUNNER_TEMP:?}/qwen-home" |
Collaborator
There was a problem hiding this comment.
[Suggestion] QWEN_HOME path coupling relies only on comments. The cleanup step computes QWEN_HOME="${RUNNER_TEMP:?}/qwen-home" as a shell variable that evaporates when the run: block exits, while the qwen step uses ${{ runner.temp }}/qwen-home as an independent GitHub Actions expression. The string literal qwen-home is duplicated in 4 locations across 2 files with only "must match" comments as coupling.
Consider using step outputs to make the coupling mechanical:
- id: 'agent-state'
name: 'Clean stale agent state'
run: |-
set -uo pipefail
QWEN_HOME="${RUNNER_TEMP:?}/qwen-home"
echo "qwen_home=${QWEN_HOME}" >> "$GITHUB_OUTPUT"
rm -rf "$QWEN_HOME" 2>/dev/null || true
mkdir -p "$QWEN_HOME"
# ... rest of cleanupThen in the Qwen step env:
QWEN_HOME: '${{ steps.agent-state.outputs.qwen_home }}'— qwen3.7-max via Qwen Code /review
| # Fresh per-run agent home (must match QWEN_HOME on the Qwen step | ||
| # below) + drop any leftover stage drafts. | ||
| QWEN_HOME="${RUNNER_TEMP:?}/qwen-home" | ||
| rm -rf "$QWEN_HOME" 2>/dev/null || true |
Collaborator
There was a problem hiding this comment.
[Suggestion] rm -rf "$QWEN_HOME" destroys the entire QWEN_HOME including potential bin/, skills/, extensions/ caches on every run. This is broader than the stated intent ("session/memory cannot leak").
If isolation of session and memory is the goal, consider targeted cleanup:
rm -rf "$QWEN_HOME/projects" "$QWEN_HOME/sessions" 2>/dev/null || trueThis preserves bin/, skills/, extensions/ caches across runs while still preventing session/memory bleed. Alternatively, add a comment explaining cache destruction is intentional.
— qwen3.7-max via Qwen Code /review
doudouOUC
approved these changes
Jun 26, 2026
doudouOUC
left a comment
doudouOUC
left a comment
Collaborator
There was a problem hiding this comment.
No new issues found. The per-run QWEN_HOME isolation is correctly scoped to the two self-hosted agent-running jobs, and the implementation is sound. All concerns raised during review were already covered in existing PR comments.
— qwen3.7-max via Qwen Code /review
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
Gives the triage job (
qwen-triage.yml) and the PR-review job (qwen-code-pr-review.yml) a fresh, per-run home for the qwen agent, and scrubs leftover draft files before each run. Both jobs run the agent directly on the persistent self-hosted ECS pool, where$HOME,/tmpand the workspace are reused between runs — so without isolation one run's state can bleed into the next.Each job now points
QWEN_HOMEat${RUNNER_TEMP}/qwen-homeon the step that runs qwen, and resets that directory (plus/tmp/stage-*.md) in the existing pre-run cleanup step.QWEN_HOMErelocates the entire global qwen dir (sessions, memory, settings, temp — seepackages/core/src/config/storage.ts), so this isolates agent state without touching$HOMEand disturbing git/npm/gh. The already container-isolatedtmux-testingjob is unchanged.Why it's needed
On #5874 (
perf(cli): skip spawnSync wrapper, author @doudouOUC) the triage bot posted three comments whose content belonged entirely to a different PR, #5872 (fix(cli): make alt+t expand thinking on macOS…, author @chiga0) — wrong author, wrong approver, wrong diff.It was not a wrong-PR-id bug. The run resolved the number correctly (
number=5874,prompt: /triage 5874,PR_AUTHOR: doudouOUC), and the same run's internal Stage 2 actually exercised #5874 (tmux-testedqwen serve,/healthok). But all three posted stages carried #5872's analysis — including the tell "matches my independent proposal exactly" — because the agent's session/memory from a prior run persisted on the reused self-hosted home. The same exposure exists in the PR-review job, which is worse since it casts an actual review verdict.Reviewer Test Plan
How to verify
This is a CI-only change to two workflow files; the effect is observable on the self-hosted runner, not locally.
actionlintreports no new findings (the two pre-existing warnings —environment.deploymentand an SC2016 in an unrelated script — are untouched).QWEN_HOME=$RUNNER_TEMP/qwen-home— confirm via the run log that~/.qwenis no longer used.Evidence (Before & After)
N/A — CI workflow change, no user-visible behavior.
Tested on
Linux is where these jobs run (self-hosted ECS); verification happens on the next live triage/review run.
Environment (optional)
N/A — no local runtime; validated with
actionlint+ YAML parse.Risk & Scope
QwenLM/qwen-code-actionever sets its ownQWEN_HOME, the step-level value here takes precedence — intended. The${RUNNER_TEMP:?}guard fails the cleanup loudly rather than runningrm -rfon an empty path./tmp/stage-*.md; this PR scrubs them per run rather than relocating them. Moving the skill's draft path under$QWEN_HOMEis a sensible follow-up but out of scope here.Linked Issues
Fixes #5882
中文说明
做了什么
给 triage job(
qwen-triage.yml)和 PR-review job(qwen-code-pr-review.yml)的 qwen agent 每次运行一个全新的 home,并在每次运行前清掉残留草稿文件。这两个 job 都直接跑在常驻的自建 ECS 机器上,$HOME、/tmp和 workspace 在多次运行间复用 —— 不隔离的话,一次运行的状态会漏到下一次。现在每个 job 在跑 qwen 的那一步把
QWEN_HOME指向${RUNNER_TEMP}/qwen-home,并在已有的运行前清理步骤里重置该目录(外加/tmp/stage-*.md)。QWEN_HOME会重定位整个全局 qwen 目录(session、memory、settings、temp —— 见packages/core/src/config/storage.ts),因此隔离了 agent 状态,又不改$HOME、不影响 git/npm/gh。已经用容器隔离的tmux-testingjob 不动。为什么需要
在 #5874(
perf(cli): skip spawnSync wrapper,作者 @doudouOUC)上,triage bot 发的三条评论内容完全属于另一个 PR #5872(fix(cli): make alt+t…,作者 @chiga0)—— 作者错、批准人错、diff 错。这不是传错 PR id。运行时号是对的(
number=5874、prompt: /triage 5874、PR_AUTHOR: doudouOUC),而且同一次运行的内部 Stage 2 确实测了 #5874(tmux 跑了qwen serve,/health正常)。但三条评论都带的是 #5872 的分析(包括 "matches my independent proposal exactly" 这句铁证),因为上一轮的 agent session/memory 残留在复用的自建 home 里。PR-review job 有同样的暴露,而且它会真的下评审结论,更糟。风险
QwenLM/qwen-code-action自己设了QWEN_HOME,这里的 step 级取值会覆盖它 —— 符合预期。${RUNNER_TEMP:?}守卫会在路径为空时直接报错,而不是对空路径rm -rf。/tmp/stage-*.md;本 PR 是每次运行清掉它们,而非改写路径。把草稿路径挪到$QWEN_HOME下是合理的后续,不在本 PR 范围。