fix(ci): harden tmux triage reporting - #5548
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Hardens the on-demand tmux real-user testing path in the qwen triage workflow by making report/transcript embedding safer, ensuring the selected OpenAI model is propagated into the tmux-launched app flow, improving verdict messaging for incomplete runs, and deferring heavier tmux harness setup until a PR is confirmed runnable.
Changes:
- Fix HTML escaping for embedded tmux artifacts by switching from bash pattern replacement to
sedescaping. - Propagate
OPENAI_MODELinto the isolated env passed to the tmux real-user test run. - Split runner setup so “resolver” tooling installs early, while tmux/qwen harness installs only when
decision == 'run', and clarify timeout/infra-error PR comments.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
scripts/tests/qwen-triage-workflow.test.js |
Adds a regression test that validates escaping, model propagation, verdict messaging, and deferred harness installation in the workflow. |
.github/workflows/qwen-triage.yml |
Updates tmux-testing and publish-tmux steps to harden escaping, pass OPENAI_MODEL, improve timeout/infra-error reporting, and defer heavy installs until runnable PRs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Collaborator
|
Thanks for the PR! Template looks good ✓ On direction: focused follow-up to #5203 hardening the tmux triage workflow in five concrete ways — HTML escaping security fix, OPENAI_MODEL propagation, accurate verdict reporting, install ordering for supply-chain safety, and GITHUB_* env stripping for PR-controlled scripts. All real problems, all squarely within CI infrastructure scope. No direction concerns. On approach: the scope feels right — each change is small, self-contained, and directly tied to a stated problem. The install step split (resolver tools vs tmux harness) is clean, and moving npm install before checkout with an explicit Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 方向：作为 #5203 的后续，针对 tmux triage workflow 做了五项具体加固——HTML 转义安全修复、OPENAI_MODEL 传递、verdict 准确报告、安装顺序调整防止供应链风险、PR 脚本中 GITHUB_* 环境变量剥离。都是真实问题，完全在 CI 基础设施范围内，没有方向性顾虑。 方案：范围合理——每项改动都很小、自包含，直接对应一个已声明的问题。安装步骤拆分（resolver 工具 vs tmux 工具链）干净利落，npm install 移到 checkout 前并显式指定 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Collaborator
|
Code review: Read the full diff and cross-referenced the current workflow YAML. The implementation is clean and correct. The The verdict case statement is well-structured: each case sets both The The Test file ( No blockers found. No AGENTS.md violations. Testing: N/A — this is a CI workflow hardening change with no user-visible surface. The PR's own test file validates all workflow changes statically. CI is green across all platforms (Test pass on macOS, Ubuntu, Windows). 中文说明代码审查： 阅读了完整 diff 并交叉对照了当前 workflow YAML。实现干净且正确。
verdict case 语句结构良好：每个 case 同时设置 对
测试文件很全面——每个测试用例同时验证新代码的存在和旧模式的缺失。 未发现阻塞性问题，无 AGENTS.md 违规。 测试： N/A——这是 CI workflow 加固，没有用户可见界面。PR 自带的测试文件静态验证了所有 workflow 变更。CI 在所有平台（macOS、Ubuntu、Windows）上均为绿色。 — Qwen Code · qwen3.7-max |
Collaborator
|
This PR does exactly what it says and does it well. Five focused hardening changes, each solving a real problem identified in the #5203 follow-up. The HTML escaping fix closes a real injection vector, the install reordering with explicit registry pinning is a clean supply-chain defense, and the verdict reporting changes will genuinely reduce confusion for PR authors who'd otherwise see "exercised the affected flow" on a timeout. My independent proposal would have covered the same ground — the problems here are obvious once you look at the workflow with fresh eyes, and the solutions are the minimal correct ones. No over-engineering, no speculative additions. The test file validates every change statically, and CI is green across all three platforms. The code is straightforward bash and JavaScript, easy to maintain. If I had to touch this workflow in six months, I'd have no trouble understanding what each piece does and why. LGTM. Approving. ✅ 中文说明这个 PR 做到了它所声明的一切，而且做得很好。五项聚焦的加固改动，每一项都解决了 #5203 后续中发现的真实问题。HTML 转义修复关闭了真实的注入向量，安装顺序调整加上显式 registry 固定是干净的供应链防御，verdict 报告改动将切实减少 PR 作者在 timeout 时看到"exercised the affected flow"的困惑。 我的独立方案会涵盖相同的内容——用新的眼光审视 workflow，这些问题都很明显，解决方案也是最小正确方案。没有过度工程，没有投机性添加。测试文件静态验证了每一项改动，CI 在三个平台上均为绿色。 代码是直白的 bash 和 JavaScript，易于维护。如果六个月后需要修改这个 workflow，理解每段代码的作用和原因不会有任何困难。 LGTM。批准。✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
previously approved these changes
Jun 21, 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. |
wenshao
reviewed
Jun 21, 2026
wenshao
reviewed
Jun 21, 2026
wenshao
reviewed
Jun 21, 2026
wenshao
left a comment
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
No new findings on the latest commit (e01a1ed0) — it cleanly addresses the prior review: the deferred-install ordering contract is now documented, emit_block captures head's exit so the render-failure fallback actually fires (and emits a ::warning::), and the verdict description is split so an unrecognized verdict no longer claims the flow was "exercised". Verified the delta locally (escaping + fallback behavior empirically, test 4/4).
— claude-opus-4-8[1m] via /qreview
qwen-code-ci-bot
requested changes
Jun 21, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
No review findings. Downgraded from Approve to Comment: CI still running. The four changes are clean, well-tested, and a clear hardening improvement. — qwen3.7-max via Qwen Code /review
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Downgraded from Approve to Comment: CI still running. No review findings — all four changes (HTML escaping, OPENAI_MODEL propagation, verdict-specific reporting, deferred install) are clean and correct. Deterministic analysis (tsc, eslint, YAML validation, vitest, bash -n) all pass. 9-agent parallel review found 0 high-confidence issues after deduplication against existing comments. — qwen3.7-max via Qwen Code /review
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM. The follow-up hardening is well-executed: sed-based HTML escaping with pipefail is more robust than bash pattern replacement, the OPENAI_MODEL propagation is properly guarded, verdict-specific labels and descriptions prevent misleading comments on non-pass paths, the deferred heavy install ordering is correct (resolver → decision → tmux/qwen harness → checkout), and the explicit --registry flag closes the .npmrc attack surface. Test coverage is thorough with positive anchors, negative assertions on the resolver step, and consumption checks for computed variables. Downgraded from Approve to Comment: CI still running. — qwen3.7-max via Qwen Code /review
qwen-code-ci-bot
requested changes
Jun 21, 2026
qwen-code-ci-bot
requested changes
Jun 21, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Test (ubuntu-latest, Node 22.x), Test (windows-latest, Node 22.x), Test (macos-latest, Node 22.x)). The critical finding below is the likely cause — the test asserts if/elif syntax but the workflow uses case/esac.
Collaborator
|
Heads-up: CI is red here — the Test job fails on all three platforms (macOS / Ubuntu / Windows), deterministically. (failing run) ( Root cause: the new test and the workflow YAML in this same PR disagree on the shape of the verdict dispatch.
case "${VERDICT:-}" in
infra-error) VERDICT_LABEL='infra-error (crash/OOM)' ;;
timeout) VERDICT_LABEL='timeout' ;;
pass) VERDICT_LABEL='pass' ;;
fail) VERDICT_LABEL='fail' ;;
*) VERDICT_LABEL='unknown' ;;
esac…but expect(postStep).toContain('if [ "${VERDICT:-}" = "infra-error" ]; then');
expect(postStep).toContain('elif [ "${VERDICT:-}" = "timeout" ]; then');
expect(postStep).toContain('elif [ "${VERDICT:-}" = "pass" ] || [ "${VERDICT:-}" = "fail" ]; then');
expect(postStep).toContain('VERDICT_LABEL="${VERDICT:-unknown}"');None of those four structural strings exist in the YAML (it uses Fix: align the two. Either update those four assertions to the 中文版提醒一下：这个 PR 的 CI 是红的 —— Test 任务在三个平台（macOS / Ubuntu / Windows）上稳定失败。（失败的 run） （ 根因：同一个 PR 里，新测试和工作流 YAML 对"verdict 分发"的写法不一致。
case "${VERDICT:-}" in
infra-error) VERDICT_LABEL='infra-error (crash/OOM)' ;;
timeout) VERDICT_LABEL='timeout' ;;
pass) VERDICT_LABEL='pass' ;;
fail) VERDICT_LABEL='fail' ;;
*) VERDICT_LABEL='unknown' ;;
esac但 expect(postStep).toContain('if [ "${VERDICT:-}" = "infra-error" ]; then');
expect(postStep).toContain('elif [ "${VERDICT:-}" = "timeout" ]; then');
expect(postStep).toContain('elif [ "${VERDICT:-}" = "pass" ] || [ "${VERDICT:-}" = "fail" ]; then');
expect(postStep).toContain('VERDICT_LABEL="${VERDICT:-unknown}"');这四个"结构性"字符串在 YAML 里都不存在（YAML 用的是 修法： 让两者对齐。要么把这四条断言改成 |
wenshao
reviewed
Jun 22, 2026
wenshao
reviewed
Jun 22, 2026
Collaborator
|
@qwen-code /triage |
wenshao
previously approved these changes
Jun 22, 2026
qwen-code-ci-bot
previously approved these changes
Jun 22, 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. ✅
yiliang114
dismissed stale reviews from qwen-code-ci-bot and wenshao
via
June 22, 2026 06:02
7190ad6
wenshao
reviewed
Jun 22, 2026
| ;; | ||
| fail) | ||
| VERDICT_LABEL='fail' | ||
| DESCRIPTION='Launched the changed app in a real tmux session and exercised the affected flow.' |
Collaborator
There was a problem hiding this comment.
[Suggestion] The pass) and fail) case arms produce byte-identical DESCRIPTION strings. A fail verdict renders as "tmux real-user testing: fail" followed by text that reads as if the test succeeded ("exercised the affected flow"), which can mislead readers skimming the PR comment.
Consolidating to pass|fail) also prevents a future edit from accidentally drifting one arm but not the other:
Suggested change
| DESCRIPTION='Launched the changed app in a real tmux session and exercised the affected flow.' | |
| pass|fail) | |
| VERDICT_LABEL="$VERDICT" | |
| DESCRIPTION='Launched the changed app in a real tmux session and exercised the affected flow.' | |
| ;; |
— qwen3.7-max via Qwen Code /review
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
No new review findings on this follow-up commit. Downgraded from Approve to Comment: CI still running.
All four hardening changes are clean and correctly address the prior review's Critical findings: npm install is now registry-pinned and pre-checkout, GITHUB_STEP_SUMMARY is stripped, tests use case matching the actual workflow, and HTML escaping is robust (sed with &-first ordering, null-byte stripping, pipefail-guarded pipeline). The new 6-test Vitest suite passes locally; ESLint and YAML parse are both clean.
— qwen3.7-max via Qwen Code /review
wenshao
approved these changes
Jun 22, 2026
Collaborator
|
@qwen-code /triage |
qwen-code-ci-bot
approved these changes
Jun 22, 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. ✅
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
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
This hardens the on-demand tmux real-user testing path added in #5203. It fixes HTML escaping for embedded tmux artifacts, passes the selected OpenAI model into the app launched under tmux, avoids claiming that timeout, infrastructure-error, or unrecognized-verdict runs exercised the affected flow, and keeps the heavier tmux/qwen harness setup gated on PRs that actually need tmux testing while installing it before PR checkout so PR-controlled npm config cannot affect the global qwen install.
Why it's needed
The merged workflow still had a few real follow-up issues: bash pattern replacement could leave unsafe artifact output under-escaped, the app-under-test could miss the expected OpenAI model/auth environment, timeout and infra-error comments could read like a completed real-user run, no-op PRs still paid unnecessary self-hosted runner setup cost, and moving npm installation after checkout would let a PR-controlled .npmrc influence the package registry used for the global qwen install.
Reviewer Test Plan
How to verify
Run the targeted workflow regression test and static workflow checks. The regression test covers artifact escaping and fallback behavior, conditional OPENAI_MODEL propagation, timeout/infra-error/unknown-verdict reporting, using the computed verdict label and description in the posted comment, and keeping the heavy tmux/qwen install after the run decision but before PR checkout.
Evidence (Before & After)
N/A. This is a CI workflow hardening change with no user-visible UI surface.
Tested on
Environment (optional)
Verified locally with targeted Vitest, Prettier,
git diff --check, YAML parsing, andbash -nover the workflowrun:blocks.Risk & Scope
Linked Issues
Follow-up to #5203.
中文说明
这个 PR 做了什么
这个 PR 加固 #5203 新增的按需 tmux real-user testing 路径。它修复嵌入 tmux artifact 时的 HTML 转义问题，把选中的 OpenAI model 传给 tmux 中启动的 app-under-test，避免 timeout、infra-error 或无法识别 verdict 的结果评论误称已经执行了 affected flow，并且仍然只在确实需要 tmux 测试的 PR 上安装较重的 tmux/qwen harness，同时把安装放在 PR checkout 之前，避免 PR 控制的 npm 配置影响全局 qwen 安装。
为什么需要
已合并的 workflow 仍有几个真实 follow-up 问题：bash pattern replacement 可能导致不可信 artifact 输出转义不足；app-under-test 可能拿不到预期的 OpenAI model/auth 环境；timeout 和 infra-error 评论可能看起来像一次已完成的真实用户测试；无操作路径仍会消耗不必要的 self-hosted runner setup 成本；如果 npm 安装被移到 checkout 之后，PR 提交的 .npmrc 还可能影响全局 qwen 安装使用的 registry。
Reviewer Test Plan
How to verify
运行 targeted workflow regression test 和静态 workflow 检查。回归测试覆盖 artifact escaping 和 fallback、条件式 OPENAI_MODEL 传递、timeout/infra-error/unknown-verdict reporting、评论中实际使用计算出的 verdict label 和 description，以及较重 tmux/qwen 安装保持在 run decision 之后但 PR checkout 之前。
Evidence (Before & After)
N/A。这是 CI workflow 加固，没有用户可见 UI 变化。
Tested on
Environment (optional)
本地已验证 targeted Vitest、Prettier、
git diff --check、YAML parse，以及 workflowrun:blocks 的bash -n。Risk & Scope
Linked Issues
#5203 的 follow-up。