[codex] test(ci): cover post-merge review follow-ups by yiliang114 · Pull Request #5899 · QwenLM/qwen-code

yiliang114 · 2026-06-26T09:43:59Z

What this PR does

Adds regression coverage for the Qwen triage and PR review workflow agent-state isolation paths, including the per-run QWEN_HOME reset, /tmp/stage-*.md cleanup, and agent-step QWEN_HOME environment wiring. It also fixes the stale PR review workflow cleanup log message so both workflows report stale agent state cleaned consistently.

Narrows the cold-import timeout override in the skill activation suite to only the integration tests that import the scheduler graph, and adds a matching hook timeout to the config session-env suite.

Why it's needed

Post-merge review on #5885 identified that the CI isolation logic was behaviorally important but not pinned by tests, so a future workflow edit could remove the cleanup or environment wiring without failing locally. Post-merge review on #5880 also pointed out that one timeout override was broader than necessary and another suite was missing the hook-timeout convention used by similar core tests.

Reviewer Test Plan

How to verify

Review the workflow test additions and confirm they fail if the QWEN_HOME cleanup, stage draft cleanup, agent-step QWEN_HOME, or cleanup echo is removed or changed. Confirm the skill activation suite applies the cold-import timeout only to the two integration tests, while the config session-env suite keeps file-level headroom and adds hookTimeout.

Commands run locally: npm run test:scripts -- scripts/tests/qwen-triage-workflow.test.js scripts/tests/qwen-resolve-workflow.test.js; cd packages/core && npx vitest run src/skills/skill-activation.test.ts src/config/config-session-env.test.ts; npm run build; npm run typecheck.

Evidence (Before & After)

Before: the newly added PR review workflow test failed because the cleanup step still emitted echo "stale review worktrees cleaned".

After: workflow tests passed with 21 tests; targeted core tests passed with 23 tests; full build and typecheck completed successfully. No tmux run was used because this change has no user-visible TUI behavior.

Tested on

OS	Status
🍏 macOS	✅ tested
🪟 Windows	⚠️ not tested
🐧 Linux	⚠️ not tested

Environment (optional)

Node v22.22.0, npm 10.9.4, local clean worktree.

Risk & Scope

Main risk or tradeoff: the workflow tests intentionally pin string-level YAML behavior, which is a narrow but appropriate guard for this CI-only shell wiring.
Not validated / out of scope: no live ECS runner execution and no tmux E2E, because this follow-up only changes workflow assertions, one cleanup log line, and Vitest timeout configuration.
Breaking changes / migration notes: none.

Linked Issues

References #5885 and #5880.

中文说明

What this PR does

为 Qwen triage 和 PR review workflow 的 agent 状态隔离路径补回归测试，覆盖每次运行的 QWEN_HOME reset、/tmp/stage-*.md 清理，以及 agent step 上的 QWEN_HOME 环境变量传递。同时修正 PR review workflow 清理步骤的旧日志文案，让两个 workflow 都一致输出 stale agent state cleaned。

把 skill activation 测试里的冷导入超时设置收窄到真正导入 scheduler graph 的两个 integration 用例，并为 config session-env 测试补上与类似 core 测试一致的 hook timeout。

Why it's needed

#5885 的合并后 review 指出 CI 隔离逻辑很关键但没有被测试 pin 住，后续有人改 workflow 时可能删掉 cleanup 或环境变量传递而本地不失败。#5880 的合并后 review 也指出一个 timeout override 范围过宽，另一个测试文件缺少同类 core 测试使用的 hook-timeout 配置。

Reviewer Test Plan

How to verify

检查新增 workflow 测试，确认如果移除或修改 QWEN_HOME cleanup、stage draft cleanup、agent step 的 QWEN_HOME 或 cleanup echo，测试会失败。确认 skill activation 测试的冷导入 timeout 只作用于两个 integration 用例，config session-env 测试仍保留文件级 headroom 并增加 hookTimeout。

本地运行过的命令：npm run test:scripts -- scripts/tests/qwen-triage-workflow.test.js scripts/tests/qwen-resolve-workflow.test.js；cd packages/core && npx vitest run src/skills/skill-activation.test.ts src/config/config-session-env.test.ts；npm run build；npm run typecheck。

Evidence (Before & After)

Before：新增的 PR review workflow 测试失败，因为 cleanup step 仍输出 echo "stale review worktrees cleaned"。

After：workflow tests 21 个通过；targeted core tests 23 个通过；完整 build 和 typecheck 成功。没有跑 tmux，因为这个 follow-up 没有用户可见的 TUI 行为。

Tested on

OS	Status
🍏 macOS	✅ tested
🪟 Windows	⚠️ not tested
🐧 Linux	⚠️ not tested

Environment (optional)

Node v22.22.0，npm 10.9.4，本地 clean worktree。

Risk & Scope

Main risk or tradeoff：workflow tests 有意 pin 住 YAML 字符串级行为；对这类 CI-only shell wiring 来说范围窄但合适。
Not validated / out of scope：没有做 live ECS runner 执行，也没有跑 tmux E2E，因为本 follow-up 只改 workflow 断言、一个 cleanup 日志，以及 Vitest timeout 配置。
Breaking changes / migration notes：无。

Linked Issues

References #5885 and #5880.

qwen-code-ci-bot · 2026-06-26T16:27:38Z

Thanks for the follow-up PR!

Template is complete ✓

On direction: this is a direct response to post-merge review feedback on #5885 and #5880 — CI isolation logic was identified as behaviorally important but unpinned, and the timeout overrides were flagged as either too broad or missing. Pure test + CI hygiene follow-up, squarely within scope.

On approach: the scope is tight and every change maps to a specific review comment. Five files, ~70 lines changed, zero feature creep. Nothing to cut.

Moving on to code review. 🔍

中文说明

感谢跟进 PR！

模板完整 ✓

方向：这是对 #5885 和 #5880 合并后 review 的直接回应——CI 隔离逻辑被认为行为上很重要但缺少测试 pin 住，timeout override 也被指出要么范围过宽、要么缺失。纯测试 + CI 维护性跟进，完全在范围内。

方案：范围紧凑，每个改动都对应一条具体的 review 意见。5 个文件、~70 行改动，没有功能蔓延。无需裁剪。

进入代码审查 🔍

— Qwen Code · qwen3.7-max

qwen-code-ci-bot · 2026-06-26T16:27:38Z

Code review: Read the diff against my own mental proposal for pinning CI isolation paths. The approach is sound — the new tests assert specific shell strings from the workflow YAML, which is the right granularity for CI-only shell wiring. One minor observation: the escapeRegExp, job(), and step() helpers in qwen-resolve-workflow.test.js are near-duplicates of the same helpers in qwen-triage-workflow.test.js, but they operate at different scope (job-section vs whole-file). Acceptable — extracting a shared test utility for two files would be over-abstracting per project conventions.

No correctness bugs, no security issues, no AGENTS.md violations.

Testing: This PR changes only CI workflow YAML, test files, and Vitest timeout config — no user-visible TUI behavior, so tmux testing is N/A. Ran the affected test suites instead:

$ npx vitest run --config ./scripts/tests/vitest.config.ts scripts/tests/qwen-triage-workflow.test.js scripts/tests/qwen-resolve-workflow.test.js

 ✓ scripts/tests/qwen-triage-workflow.test.js (7 tests) 4ms
 ✓ scripts/tests/qwen-resolve-workflow.test.js (14 tests) 5ms

 Test Files  2 passed (2)
      Tests  21 passed (21)
   Duration  240ms

$ cd packages/core && npx vitest run src/skills/skill-activation.test.ts src/config/config-session-env.test.ts

 ✓ src/skills/skill-activation.test.ts (20 tests) 2145ms
 ✓ src/config/config-session-env.test.ts (3 tests) 2621ms

 Test Files  2 passed (2)
      Tests  23 passed (23)
   Duration  4.75s

$ npm run typecheck
(all 6 packages: tsc --noEmit clean)

All 44 tests pass, typecheck clean. The PR author's evidence matches.

中文说明

代码审查： 对照我自己的方案来评审 CI 隔离路径的 pin 住方式。方案合理——新测试断言 workflow YAML 中的具体 shell 字符串，对 CI-only shell wiring 来说粒度恰当。一个小观察：qwen-resolve-workflow.test.js 中的 escapeRegExp、job() 和 step() helper 与 qwen-triage-workflow.test.js 中的几乎相同，但作用域不同（job 段落 vs 全文件）。可以接受——为两个文件抽取共享测试工具属于过度抽象。

没有正确性 bug、没有安全问题、没有违反 AGENTS.md。

测试： 本 PR 仅修改 CI workflow YAML、测试文件和 Vitest timeout 配置——无用户可见 TUI 行为，tmux 测试 N/A。改为运行受影响的测试套件：21 个 workflow 测试 + 23 个 core 测试全部通过，typecheck 通过。

— Qwen Code · qwen3.7-max

qwen-code-ci-bot · 2026-06-26T16:27:57Z

This is a clean follow-up that does exactly what it says — pins CI isolation behavior with regression tests and tightens timeout scoping. Every change traces back to a specific post-merge review comment on #5885 or #5880. Tests pass, typecheck passes, no surprises.

The helper duplication between the two workflow test files is minor and fits the project's preference for self-contained test files over shared utilities. Not worth extracting.

Approving. ✅

中文说明

这是一个干净的跟进 PR，做了它声称要做的事——用回归测试 pin 住 CI 隔离行为，并收紧 timeout 范围。每个改动都能追溯到 #5885 或 #5880 的合并后 review 意见。测试通过，typecheck 通过，没有意外。

两个 workflow 测试文件之间的 helper 重复很小，符合项目偏好（测试文件自包含优于共享工具）。不需要抽取。

批准 ✅

— Qwen Code · qwen3.7-max

qwen-code-ci-bot

LGTM, looks ready to ship. ✅

qwen-code-ci-bot

No review findings. Downgraded from Approve to Comment: CI still running.

— qwen3.7-max via Qwen Code /review

wenshao · 2026-06-26T21:30:09Z

    expect(workflow).not.toContain('qwen-fix-conflicts');
  });

+  it('isolates review agent state per run', () => {


[Suggestion] These regression tests verify the cleanup step contents and the agent step QWEN_HOME, but they do not assert that cleanup runs before the agent. If Clean stale agent state is later moved below Run review or Run Qwen Triage, stale state would still be used while both tests keep passing. Please add a relative-order assertion in both workflow tests, for example by comparing the scoped indices of the cleanup and agent steps.

expect(reviewJob.indexOf("- name: 'Clean stale agent state'")) .toBeLessThan(reviewJob.indexOf("- name: 'Run review'"));

— GPT-5 via Qwen Code /review

yiliang114 added 2 commits June 26, 2026 17:42

test(ci): cover post-merge review follow-ups

9d63884

test(core): scope skill activation timeout override

89845b2

yiliang114 marked this pull request as ready for review June 26, 2026 16:21

qwen-code-ci-bot approved these changes Jun 26, 2026

View reviewed changes

qwen-code-ci-bot reviewed Jun 26, 2026

View reviewed changes

wenshao reviewed Jun 26, 2026

View reviewed changes

wenshao approved these changes Jun 26, 2026

View reviewed changes

wenshao added this pull request to the merge queue Jun 26, 2026

Merged via the queue into QwenLM:main with commit 8189141 Jun 26, 2026
52 checks passed

Navigation Menu

Uh oh!

[codex] test(ci): cover post-merge review follow-ups - #5899

[codex] test(ci): cover post-merge review follow-ups#5899
wenshao merged 2 commits into
QwenLM:mainfrom
yiliang114:codex/post-merge-review-followups

yiliang114 commented Jun 26, 2026 •

edited

Loading

Uh oh!

qwen-code-ci-bot commented Jun 26, 2026

Uh oh!

qwen-code-ci-bot commented Jun 26, 2026

Uh oh!

qwen-code-ci-bot commented Jun 26, 2026

Uh oh!

qwen-code-ci-bot left a comment

Uh oh!

qwen-code-ci-bot left a comment

Uh oh!

wenshao Jun 26, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

yiliang114 commented Jun 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this PR does

Why it's needed

Reviewer Test Plan

How to verify

Evidence (Before & After)

Tested on

Environment (optional)

Risk & Scope

Linked Issues

What this PR does

Why it's needed

Reviewer Test Plan

How to verify

Evidence (Before & After)

Tested on

Environment (optional)

Risk & Scope

Linked Issues

Uh oh!

qwen-code-ci-bot commented Jun 26, 2026

Uh oh!

qwen-code-ci-bot commented Jun 26, 2026

Uh oh!

qwen-code-ci-bot commented Jun 26, 2026

Uh oh!

qwen-code-ci-bot left a comment

Choose a reason for hiding this comment

Uh oh!

qwen-code-ci-bot left a comment

Choose a reason for hiding this comment

Uh oh!

wenshao Jun 26, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

yiliang114 commented Jun 26, 2026 •

edited

Loading