Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .github/workflows/qwen-triage.yml
Original file line number Diff line number Diff line change
Expand Up @@ -574,6 +574,17 @@ jobs:
ESCAPED=$(printf '%s' "$OPENAI_MODEL" | sed -e 's/[&/\]/\\&/g')
sed -i "s/qwen3\.7-max/${ESCAPED}/g" "$TARGET"

- name: 'Ensure qwen CLI'
run: |-
set -euo pipefail
Comment on lines +577 to +579

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The new "Ensure qwen CLI" step — the core fix of this PR — has no test in the workflow suite (scripts/tests/qwen-triage-workflow.test.js), which already asserts step content AND ordering for the analogous "Install tmux runner tools" step. — Concrete cost: a future edit could remove or rename this step, drop --loglevel=error (re-exposing the exit-code-243 noise this PR fixes), change the package specifier, or reorder it after "Run Qwen Triage" (making the pre-install a no-op for the action that needs it), and nothing in the suite would catch it.

Suggested test (follows the existing tmux-step pattern):

it('ensures qwen CLI is installed before the triage action', () => {
  const ensureStep = stepIn('triage', 'Ensure qwen CLI');
  expect(ensureStep).toContain('command -v qwen');
  expect(ensureStep).toContain('npm install -g');
  expect(ensureStep).toContain('@qwen-code/qwen-code@latest');
  // Must precede the action that depends on it.
  const triageJob = job('triage');
  expect(
    triageJob.indexOf("- name: 'Ensure qwen CLI'"),
  ).toBeLessThan(triageJob.indexOf("- name: 'Run Qwen Triage'"));
});
中文说明

新增的 "Ensure qwen CLI" 步骤是本 PR 的核心修复,但工作流测试套件(scripts/tests/qwen-triage-workflow.test.js)中没有对应测试,而该套件已对类似的 "Install tmux runner tools" 步骤断言了步骤内容与顺序。具体代价:未来的修改可能删除或重命名该步骤、去掉 --loglevel=error(重新暴露本 PR 修复的 exit-code-243 噪声)、更改包名,或将其排到 "Run Qwen Triage" 之后(使预装对依赖它的 action 失效),而套件中没有任何测试能发现。

— qwen3.8-max-preview via Qwen Code /review

Comment on lines +577 to +579

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] This load-bearing step has no inline comment explaining why it exists, while every other non-trivial step in this job ("Restore workspace ownership", "Clean stale agent state", "Inject model name", and the qwen-code-action settings block) carries one. — Concrete cost: a future maintainer seeing "Ensure qwen CLI" immediately before "Run Qwen Triage" (an action known to install qwen internally) may conclude the step is redundant and delete it; on non-root runners whose npm global prefix is not writable, the action's internal npm install --silent --global then fails with exit code 243 again, re-breaking fleet-wide triage.

Suggested comment above the step:

      # Pre-install the qwen CLI so qwen-code-action's internal
      # `npm install --silent --global` finds it on PATH and becomes a
      # no-op. Without this, that install fails with exit code 243 on
      # non-root runners whose npm global prefix is not writable.
      - name: 'Ensure qwen CLI'
中文说明

这个关键步骤没有内联注释说明其存在原因,而本 job 中其他所有非平凡步骤("Restore workspace ownership"、"Clean stale agent state"、"Inject model name" 以及 qwen-code-action 配置块)都有注释。具体代价:未来维护者看到 "Ensure qwen CLI" 紧挨着 "Run Qwen Triage"(一个已知会内部安装 qwen 的 action),可能认为该步骤冗余而删除;在 npm 全局目录不可写的非 root runner 上,action 内部的 npm install --silent --global 会再次以 exit code 243 失败,使全 fleet 的 triage 重新中断。

— qwen3.8-max-preview via Qwen Code /review

if command -v qwen >/dev/null 2>&1; then
echo "qwen already installed:"
qwen --version
else
npm install -g --loglevel=error --no-audit @qwen-code/qwen-code@latest
qwen --version
fi

- name: 'Run Qwen Triage'
id: 'triage'
uses: 'QwenLM/qwen-code-action@6d08e91aa807257b9c8af60edb5bb6bb2d7d951f'
Expand Down
Loading