ci(autofix): move agent prompts into a project skill#6306
Conversation
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. |
|
Thanks for the PR! (Updated re-run after 3 new commits addressing prior review feedback.) Template looks good ✓ Problem: CI infrastructure refactoring — agent prompts embedded in workflow YAML are hard to review and evolve. Directly observable by inspecting the existing workflow. No reproduction needed. Direction: Moving model-facing instructions into a repo-local skill aligns with how the project already structures agent behavior (other skills in Approach: Core extraction is well-scoped: three inline prompts → three skill modes. The PR also carries routing hardening (input sanitization via CI tests pass. Moving on to code review. 🔍 中文说明感谢贡献!(在 3 个新 commit 解决之前 review 反馈后的重新审查。) 模板完整 ✓ 问题: CI 基础设施重构——嵌在 workflow YAML 里的 agent prompt 难以 review 和演进。通过检查现有 workflow 文件即可确认。 方向: 把模型指令移到 repo-local skill 和项目已有的 方案: 核心提取范围合理。PR 同时包含路由加固( CI 测试通过。进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Code ReviewIndependent proposal: To extract agent prompts from workflow YAML, I'd create a SKILL.md with the three modes (assess/develop/address), write a small Node runner that reads the skill + composes the prompt + spawns qwen, and update the workflow to invoke the runner. Tests would verify prompts are gone from YAML and present in the skill. Comparison: The PR's approach matches this exactly. The runner script ( Previous CHANGES_REQUESTED items — all resolved:
No new critical issues found. The workflow changes are sound and well-tested. TestingThis is a CI workflow refactor with no user-visible TUI changes. Tmux real-scenario testing is N/A — the meaningful verification is the workflow structure test suite, which validates YAML content, skill content, runner behavior, and routing logic. CI result (ubuntu-latest, Node 22.x): ✅ pass (9m14s) The 30 workflow structure tests cover: prompt extraction fidelity, routing logic, input sanitization, duplicate-PR annotation, runner failure semantics, output validation, push behavior, step-summary reporting, and verification gate conditions. 中文说明代码审查独立方案: 提取 agent prompt 到 SKILL.md,写一个 Node runner 读取 skill 并调用 qwen,更新 workflow。测试验证 prompt 从 YAML 移除并存在于 skill 中。 对比: PR 的方案完全匹配。runner 脚本干净——正确的输入验证、超时处理、failure.md 语义。SKILL.md 忠实于原始 prompt。 之前的 CHANGES_REQUESTED 项目——全部已解决:
未发现新的关键问题。 测试这是 CI workflow 重构,无用户可见的 TUI 变更。Tmux 真实场景测试不适用。CI 结果:✅ 通过(30/30 测试)。 — Qwen Code · qwen3.7-max |
|
This PR has come a long way. The initial prompt extraction was clean, and the three follow-up commits systematically addressed every concern from the previous review: the missing Looking at the whole picture: the skill is well-structured (three modes with clear I/O contracts), the runner script has solid failure semantics, and the 30-test suite locks in the contract comprehensively. The routing hardening (sanitize_number, duplicate-PR detection, additive-only push) are all consistent with the skill's design principles and don't feel like scope creep on re-examination — they're necessary companion changes for a workflow that now delegates decision-making to a skill. My independent proposal matched the PR's approach. No critical issues found. CI passes. All previous CHANGES_REQUESTED items resolved. Approving. ✅ 中文说明这个 PR 经过了多轮迭代。初始 prompt 提取很干净,后续三个 commit 系统性地解决了之前 review 的所有问题:缺失的 整体来看:skill 结构良好(三个 mode,清晰的 I/O 契约),runner 脚本有可靠的失败语义,30 个测试全面锁定契约。路由加固与 skill 的设计原则一致。 独立方案匹配 PR 方案。未发现关键问题。CI 通过。所有之前的 CHANGES_REQUESTED 项目已解决。 批准。✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
1d663aa to
e1f8955
Compare
|
Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. 中文请勿对活跃的 PR 执行 rebase 或 force-push,因为这会使已有的评审评论失效。另外,供日后参考:作为集成流程的一部分,机器人始终会自动将所有改动压缩(squash)为单个提交。 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
[Suggestion] The authorize job in qwen-triage.yml (around line 64) still runs unconditionally for issue_comment events on closed issues. Since the downstream triage job now skips closed issues, the authorize job wastes CI resources (API calls, permission checks) on comments that will never produce a triage run. Consider adding github.event.issue.state == 'open' to the authorize job's if condition for the issue_comment path.
— qwen3.7-max via Qwen Code /review
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
— qwen3.7-max via Qwen Code /review
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No issues found. Downgraded from Approve to Comment: CI still running. The prompt extraction is clean and faithful to the originals. ESLint, tests, yamllint, actionlint, prettier, and shellcheck all pass. LGTM! ✅
— qwen3.7-max via Qwen Code /review
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
— qwen3.7-max via Qwen Code /review
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
— qwen3.7-max via Qwen Code /review
Addressed in 70c5202: restored the missing dual-label revalidation test coverage and resolved the review thread.
wenshao
left a comment
There was a problem hiding this comment.
No critical findings. Suggestion-level recommendations are in the Suggestion summary comment below.
— qwen3.7-max via Qwen Code /review
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Qwen Code Review — PR #6306
Scope: Workflow refactoring — inline YAML prompts extracted to repo-local skill (SKILL.md), Node.js runner script added, routing logic tightened.
Deterministic analysis: npm run build ✅ | 30/30 tests ✅ | ESLint clean (.mjs excluded by config) | tsc OOM (exit 134, non-blocking)
Summary of findings
| # | Severity | Finding | File |
|---|---|---|---|
| 1 | 🔴 Critical | review-address report step missing failure() || cancelled() — silently drops crash reports |
qwen-autofix.yml:1354 |
| 2 | 🟡 Suggestion | GITHUB_OUTPUT injection risk via workflow_dispatch inputs |
qwen-autofix.yml:176-177 |
| 3 | 🟡 Suggestion | Stale failure.md from assess-candidates may poison develop-issue |
qwen-autofix.yml:625-637 |
| 4 | 🟡 Suggestion | Test coverage gaps for new routing logic | qwen-autofix-workflow.test.js |
Test coverage gaps (detail)
The routing refactor introduces several untested code paths:
autophase gating:[[ "${EVENT_NAME}" == 'auto' && ... ]]is not exercisedreview-addressjob dependency onroute(vs oldissue-autofix) not verifiedneeds.route.outputs.issue_number/pr_numberpropagation to downstream jobs not testedanyOutputhelper code path (!missing(workdir, ['decision.json', 'failure.md'])) untested
— qwen3.7-max via Qwen Code /review
Suggestions — commit
|
| File | Issue | Suggested fix |
|---|---|---|
run-agent.mjs:99 |
Dead kebab-case keys from parseArgs spread. The options object spreads all parseArgs values (including print-prompt, qwen-bin) then manually adds camelCase aliases. The kebab-case keys are dead data no code path consumes. |
Extract only the needed values explicitly: const options = { mode: values.mode, issue: values.issue, ..., printPrompt: values['print-prompt'], qwenBin: values['qwen-bin'] }; |
run-agent.mjs:145-149 |
anyOutput: true branching logic (address-review partial-output success) is never tested. No test creates only address-summary.md or only no-action.md and verifies exit 0. |
Add a test that stubs a qwen subprocess writing only no-action.md in address-review mode and asserts exit 0; and a test that writes neither and asserts exit 1. |
scripts/tests/qwen-autofix-workflow.test.js:639 |
No --print-prompt test for assess-candidates mode. The existing print-prompt test only tests address-review. |
Add a --print-prompt test for assess-candidates that asserts the invocation includes --workdir and does not include --issue or --pr. |
scripts/tests/qwen-autofix-workflow.test.js:639 |
No develop-issue success path test. Every develop-issue test ends with .toThrow(); no test stubs a successful qwen run producing all expected outputs and asserts exit 0. |
Add a test that creates a stub writing all three required output files, runs develop-issue mode, and asserts result.status === 0 and stdout contains "completed develop-issue successfully". |
qwen-autofix.yml:374-377 |
When gh pr list fails (transient rate limit, network blip), all candidates are silently dropped with only a ::warning:: annotation. A scheduled run produces zero fixes with no clear signal. |
Use ::error:: instead of ::warning::, or annotate candidates with existingAutofixPr: null when the scan fails so they pass through. |
— qwen3.7-max via Qwen Code /review
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
wenshao
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestion-level recommendations are in the Suggestion summary comment below.
— qwen3.7-max via Qwen Code /review
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
[Suggestion] Issue-autofix verification gate (line ~658) is missing always(), unlike the review-address verification gate (line ~1207) which has it. When the develop-issue agent step crashes, the verification gate is skipped by GitHub Actions' default step-skipping behavior, so steps.verify.outputs.outcome is never set and the "Report dry-run / failure" step shows outcome=unknown instead of outcome=failed. Consider adding always() to the issue-autofix verification gate if condition to match the review-address pattern.
— qwen3.7-max via Qwen Code /review
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
What this PR does
This PR moves the model-facing AutoFix instructions into a repo-local AutoFix skill and changes the GitHub Actions workflow to invoke that skill for candidate assessment, issue implementation, and review-feedback handling. The workflow continues to own routing, credentials, checkout, sandbox setup, verification, push, and comments; the skill owns the agent decision rules that are easier to inspect and debug in the repository.
It also tightens comment-triggered triage so
/triagecomments on closed issues or PRs do not start the triage job or share the same concurrency group as runnable events.Why it's needed
The previous AutoFix flow embedded long prompts directly in workflow YAML, which made the behavior hard to review, evolve, and test as a coherent agent workflow. Moving that behavior into a skill gives maintainers a single place to reason about the bot's decision policy without changing the operational workflow around credentials and CI gates.
The triage guard avoids wasting runner capacity and credentials setup on comments that cannot result in useful triage because the target issue or PR is already closed.
Reviewer Test Plan
How to verify
Review that the AutoFix workflow still performs the same three model-driven stages but now invokes the repo-local skill modes instead of embedding long role/task prompts in the YAML. Confirm that the workflow still owns trusted operations such as verification and GitHub writes, while the skill explicitly tells the agent not to run project code or use credentials.
For triage, confirm that comment-triggered runs require an open issue or PR before evaluating the
/triagecommand, while issue events, manual dispatch, and pull request target events keep their existing routing behavior.Evidence (Before & After)
N/A. This is a CI workflow and bot-behavior refactor with workflow structure tests.
Tested on
Environment (optional)
Verified locally with targeted workflow tests, skill validation, workflow linting, YAML linting, and Prettier checks.
Risk & Scope
Linked Issues
N/A.
中文说明
What this PR does
这个 PR 把 AutoFix 面向模型的指令迁移到仓库内的 AutoFix skill,并让 GitHub Actions workflow 在候选 issue 评估、issue 实现、PR review 处理这三个阶段调用对应的 skill 模式。workflow 仍然负责触发路由、凭证、checkout、sandbox、验证、push 和评论;skill 负责更容易在仓库里查看和调试的 agent 决策规则。
同时,这个 PR 收紧了评论触发的 triage:已关闭 issue 或 PR 上的
/triage评论不会再启动 triage job,也不会和可运行事件共用同一个 concurrency group。Why it's needed
之前 AutoFix 流程把长 prompt 直接写在 workflow YAML 里,导致行为难以作为一个完整 agent workflow 被 review、演进和测试。迁移到 skill 后,维护者可以在一个明确的位置理解 bot 的决策策略,同时不改变凭证和 CI gate 这些运行编排逻辑。
triage guard 可以避免已经关闭的 issue 或 PR 评论消耗 runner 和凭证初始化,因为这类目标不会产生有意义的 triage 结果。
Reviewer Test Plan
How to verify
检查 AutoFix workflow 仍然执行相同的三个模型驱动阶段,但现在调用仓库内的 skill 模式,而不是在 YAML 中嵌入很长的 role/task prompt。确认 workflow 仍然负责验证和 GitHub 写入等可信操作,而 skill 明确要求 agent 不运行项目代码、不使用凭证。
对于 triage,确认评论触发的运行在判断
/triage命令前要求 issue 或 PR 处于 open 状态,同时 issue 事件、手动触发、pull_request_target 事件保持原有路由行为。Evidence (Before & After)
N/A。这是 CI workflow 和 bot 行为重构,已有 workflow 结构测试覆盖。
Tested on
Environment (optional)
本地已通过定向 workflow 测试、skill 校验、workflow lint、YAML lint 和 Prettier 检查。
Risk & Scope
Linked Issues
N/A。