Skip to content

fix(autofix): improve review addressing and verification - #6382

Merged
wenshao merged 5 commits into
QwenLM:mainfrom
yiliang114:codex/autofix-pr-template
Jul 6, 2026
Merged

fix(autofix): improve review addressing and verification#6382
wenshao merged 5 commits into
QwenLM:mainfrom
yiliang114:codex/autofix-pr-template

Conversation

@yiliang114

@yiliang114 yiliang114 commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

Updates AutoFix so implementation agents verify changes before committing, and expands the workflow so review follow-up can keep moving as comments arrive. The issue-fix and review-addressing paths get npm shell access for build, typecheck, lint, and focused Vitest verification, while candidate assessment remains unable to run npm. Review follow-up now reacts to trusted review/comment feedback and the explicit @qwen-code /address-review command, rechecks the PR state and new feedback before expensive setup, and allows more review-addressing jobs to run in parallel. Scheduled issue autofix now pauses when the open bot autofix PR backlog is already high.

Why it's needed

AutoFix could previously write code and hand it to the later workflow gate without first catching basic build or test failures. Review feedback was also only handled on the next scheduled sweep, so a small queue of bot PRs could sit for hours while the issue phase continued opening more work. This keeps each agent responsible for validating its own change, responds to review feedback immediately when it comes from trusted maintainers or collaborators, and adds simple backpressure so issue autofix does not outrun review closeout.

Reviewer Test Plan

How to verify

Confirm that the AutoFix issue-fix and review-addressing paths include npm shell access, but candidate assessment does not. Confirm that review-addressing is triggered by trusted review comments, trusted submitted reviews, and the explicit @qwen-code /address-review command on an open PR. Confirm that scheduled issue autofix is skipped once there are already 10 open bot autofix PRs, and that review-addressing rechecks PR state, draft status, head SHA, mergeability, previous rounds, and new feedback before running Node setup or agent work. Confirm that review-addressing uses a higher parallelism limit while still relying on per-PR concurrency and the live recheck to avoid duplicate expensive work.

Local verification run for this update: actionlint .github/workflows/qwen-autofix.yml, npx prettier --check .github/workflows/qwen-autofix.yml scripts/tests/qwen-autofix-workflow.test.js, npx vitest run scripts/tests/qwen-autofix-workflow.test.js, and git diff --check.

Evidence (Before & After)

N/A; this is workflow and process behavior.

Tested on

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

Environment (optional)

Local macOS workspace with repository npm dependencies installed.

Risk & Scope

  • Main risk or tradeoff: AutoFix agents can now run npm commands during implementation, so the allowed command surface stays limited to verification-oriented npm commands and the later independent gate still runs as a backstop.
  • Not validated / out of scope: A live post-merge AutoFix run exercising the exact trusted review feedback path end to end.
  • Breaking changes / migration notes: None.

Linked Issues

None.

中文说明

这个 PR 做了什么

更新 AutoFix,让实现 agent 在创建提交前先验证改动,同时扩展 workflow,让 review 跟进可以在评论到达时继续推进。issue 修复和 review 处理路径会获得 npm shell 权限,用于 build、typecheck、lint 和 focused Vitest 验证;候选 issue 评估阶段仍然不能运行 npm。review 跟进现在会响应受信任的 review/comment 反馈和显式的 @qwen-code /address-review 命令,会在昂贵 setup 前重新检查 PR 状态和新增反馈,并允许更多 review-addressing job 并行运行。scheduled issue autofix 在打开的 bot autofix PR backlog 已经较高时会暂停。

为什么需要

此前 AutoFix 可能先写代码,然后不做基础 build 或测试就交给后面的 workflow gate。review 反馈也只能等下一次 scheduled sweep 处理,所以一小批 bot PR 可能会排队数小时,而 issue phase 还在继续打开更多工作。这个改动让每个 agent 先负责验证自己的改动,在受信任 maintainer 或 collaborator 给出 review 反馈时立即响应,并加入简单的 backpressure,避免 issue autofix 跑得比 review closeout 更快。

Reviewer Test Plan

如何验证

确认 AutoFix 的 issue 修复和 review 处理路径包含 npm shell 权限,但候选 issue 评估路径不包含。确认 review-addressing 会被受信任的 review comment、受信任的 submitted review,以及 open PR 上显式的 @qwen-code /address-review 命令触发。确认 scheduled issue autofix 会在已经有 10 个 open bot autofix PR 时跳过,并且 review-addressing 会在运行 Node setup 或 agent work 前重新检查 PR 状态、draft 状态、head SHA、mergeability、历史轮次和新增反馈。确认 review-addressing 使用更高的并行上限,同时仍依赖 per-PR concurrency 和 live recheck 避免重复的昂贵工作。

这次更新的本地验证:actionlint .github/workflows/qwen-autofix.ymlnpx prettier --check .github/workflows/qwen-autofix.yml scripts/tests/qwen-autofix-workflow.test.jsnpx vitest run scripts/tests/qwen-autofix-workflow.test.jsgit diff --check

证据(Before & After)

N/A;这是 workflow 和流程行为改动。

测试平台

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

环境(可选)

本地 macOS workspace,已安装仓库 npm 依赖。

风险与范围

  • 主要风险或权衡:AutoFix agent 现在可以在实现阶段运行 npm 命令,所以允许的命令范围仍限制在验证相关 npm 命令内,后续独立 gate 仍会作为兜底。
  • 未验证 / 不在范围内:合并后真实 AutoFix run 对受信任 review feedback 路径的完整端到端验证。
  • 破坏性变更 / 迁移说明:无。

关联 Issue

无。

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the PR! Re-run review after author's changes.

Template looks good ✓ — all required sections present, bilingual.

Problem: This addresses a real, observable gap — the existing SKILL.md explicitly told agents not to run verification (Do not run project code, tests, builds, package scripts, or the CLI yourself), and the workflow allowlist didn't include those commands. The gap is directly visible in the code being modified. Not theoretical.

Direction: Aligned. AutoFix agents should verify their own output before committing. The independent verification gate still runs as a backstop — this is defense-in-depth, not a replacement.

Approach: The diff is tightly scoped to its core claim: adding pre-commit verification (4 npm commands) to the develop-issue and address-review allowlists + updating skill instructions. Tests updated to match. However, the PR body describes a significantly broader set of changes (review follow-up triggers, backlog backpressure at 10 PRs, recheck logic before expensive setup, higher parallelism) that are not present in this diff. Those features may already exist or belong in a separate PR — either way, the PR body oversells what's actually here.

Moving on to code review. 🔍

中文说明

感谢贡献!Re-run 复查。

模板完整 ✓

问题:这是一个真实的、可观测的工作流缺陷——现有 SKILL.md 明确告诉 agent 不要运行验证命令,workflow 允许列表也没有包含这些命令。问题直接体现在被修改的代码中,不是理论性的。

方向:对齐。AutoFix agent 应该在提交前验证自己的输出。独立验证 gate 仍作为兜底——这是 defense-in-depth。

方案:diff 严格限定在核心主张范围内:给 develop-issue 和 address-review 添加 4 个 npm 命令到允许列表 + 更新 skill 说明。测试同步更新。但 PR 描述提到了明显更广的改动(review 跟进触发、10 个 PR backlog 上限、昂贵 setup 前重新检查、更高并行度)在本次 diff 中并未出现。这些功能可能已经存在,或者属于另一个 PR——无论如何,PR body 的描述超出了实际内容。

进入代码审查 🔍

Qwen Code · qwen3.7-max

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator

Code Review

The change is well-scoped and consistent with its stated goal. Three files, each doing exactly one job:

  • Workflow (.github/workflows/qwen-autofix.yml): adds 4 verification commands (npm run build, npm run typecheck, npm run lint, npx vitest) to the allowlists in both develop-issue and address-review sections. The commands are specific — bare run_shell_command(npm) is NOT allowlisted, and tests verify this. npm publish, npm exec, and npm run bundle are explicitly prohibited.

  • Skill (.qwen/skills/autofix/SKILL.md): replaces the blanket "do not run anything" rule with a targeted allowlist plus explicit prohibitions. Adds verification steps to both develop-issue (step 6) and address-review outcomes. Provides a clear fallback path (failure.md) when verification can't pass. The responsibility split is now clean: skill owns pre-commit verification, workflow owns final independent verification.

  • Tests (.scripts/tests/qwen-autofix-workflow.test.js): correctly verifies that developFixStep and triageAndAddressStep contain the 4 commands, that assessCandidatesStep does NOT, and that broader npm access is blocked. One test timeout bumped from default to 10s.

No correctness issues, no security concerns, no over-abstraction. The allowlist approach is the right security model — specific commands only, no wildcards.

One note: the PR body describes features (review follow-up triggers via @qwen-code /address-review, backlog backpressure, PR state rechecks) that are absent from the diff. The tests explicitly verify these features are NOT present (e.g., expect(workflow).not.toContain('@qwen-code /address-review')). The PR body should be updated to match what the diff actually ships.

Testing

Non-TUI change (CI workflow + skill docs). Verification via project tests:

✓  scripts/tests/qwen-autofix-workflow.test.js (37 tests) 483ms

 Test Files  1 passed (1)
      Tests  37 passed (37)
   Duration  737ms

Prettier: All matched files use Prettier code style!

All 37 workflow tests pass, confirming:

  • Verification commands are in the correct allowlists (develop-fix + address-review only)
  • Candidate assessment does NOT get npm access
  • Dangerous npm commands remain blocked globally
  • Skill text matches the new verification instructions
中文说明

代码审查

改动范围合理,与声明的目标一致。三个文件各司其职:

  • Workflow:给 develop-issue 和 address-review 的允许列表添加 4 个验证命令。命令是具体的——run_shell_command(npm) 裸命令未被允许,测试验证了这一点。npm publishnpm execnpm run bundle 被明确禁止。
  • Skill:将"什么都不能运行"的规则替换为定向允许列表 + 明确禁止项。给 develop-issue 和 address-review 都加了验证步骤。验证失败时有清晰的 fallback(failure.md)。
  • Tests:正确验证了 developFixStep 和 triageAndAddressStep 包含这 4 个命令,assessCandidatesStep 不包含,更广范围的 npm 访问被阻止。

无正确性问题,无安全隐患,无过度抽象。

一点说明:PR body 描述了一些 diff 中不存在的功能(@qwen-code /address-review 触发、backlog backpressure、PR 状态重新检查)。测试明确验证这些功能不存在。PR body 应该更新以匹配实际内容。

测试

非 TUI 改动(CI workflow + skill 文档)。通过项目测试验证:37 个测试全部通过。

Qwen Code · qwen3.7-max

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator

This is a clean, focused change that does one thing well: it gives AutoFix implementation agents the ability to verify their own work before committing. The problem is real (agents were told not to run any verification), the fix is minimal (4 specific commands added to 2 allowlists), and the security model is sound (allowlist, not open access; independent verification gate still runs).

The independent proposal I formed before reading the diff matches what the PR does: add npm run build, npm run typecheck, npm run lint, and npx vitest to the workflow allowlists for the develop and address-review jobs, and update the skill instructions accordingly. The PR's execution matches its design.

One reservation: the PR body significantly oversells the scope — it describes review follow-up triggers, backlog backpressure, PR state rechecks, and higher parallelism, none of which are in this diff. The tests explicitly verify these features are absent. The author should update the PR body to match what's actually shipped so reviewers aren't looking for changes that aren't there.

All 37 workflow tests pass. Prettier clean. No correctness or security concerns. The change is small enough to revert trivially if needed.

This is ready to merge once the PR body is corrected to match the actual diff scope. Approving.

中文说明

这是一个干净、专注的改动,做好了一件事:让 AutoFix 实现 agent 在提交前能验证自己的工作。问题是真实的(agent 被告知不能运行任何验证),修复是最小的(4 个具体命令加到 2 个允许列表),安全模型是合理的(允许列表而非开放访问;独立验证 gate 仍然运行)。

一个保留意见:PR body 显著夸大了范围——描述了 review 跟进触发、backlog backpressure、PR 状态重新检查和更高并行度,这些都不在 diff 中。作者应该更新 PR body 以匹配实际内容。

37 个 workflow 测试全部通过。Prettier 通过。无正确性或安全问题。改动足够小,需要时可以轻松回滚。

PR body 修正后即可合并。批准。

Qwen Code · qwen3.7-max

@qwen-code-ci-bot qwen-code-ci-bot left a comment

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.

LGTM, looks ready to ship. ✅

Comment thread .github/workflows/qwen-autofix.yml Outdated
Comment thread scripts/tests/qwen-autofix-workflow.test.js
@yiliang114 yiliang114 changed the title fix(autofix): run verification before committing fix(autofix): improve review addressing and verification Jul 6, 2026
Comment thread .github/workflows/qwen-autofix.yml Outdated
Comment thread .github/workflows/qwen-autofix.yml Outdated
Comment thread .github/workflows/qwen-autofix.yml Outdated
Comment thread .github/workflows/qwen-autofix.yml Outdated
Comment thread .github/workflows/qwen-autofix.yml Outdated
Comment thread .github/workflows/qwen-autofix.yml Outdated
Comment thread scripts/tests/qwen-autofix-workflow.test.js
@yiliang114

Copy link
Copy Markdown
Collaborator Author

Review follow-up summary:

  • Fixed: allowed focused npx vitest verification, added live recheck gh api failure handling, renamed the global autofix PR backlog limit, and strengthened the existing live recheck workflow test.
  • Not changed: I did not extract the autofix marker regex into a shared constant/script because that is a workflow refactor beyond the closeout scope.
  • Not changed: I did not remove the pull_request_review_comment trigger because that changes event semantics and should be decided separately.
  • Not changed: I did not add an early ownership API guard in the router because downstream review-scan already enforces ownership and the proposed guard adds another API call before the existing validation.

@qwen-code-ci-bot qwen-code-ci-bot left a comment

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.

No high-confidence issues found. The change is well-scoped: grants AutoFix agents npm verification access, updates SKILL.md for pre-commit checks, and adjusts tests accordingly.

⚠️ Downgraded from Approve to Comment: CI still running.

Three low-confidence observations for human review:

  1. Verification ordering — develop-issue runs verification (step 6) before the skeptical diff review (step 7), unlike address-review which has the correct order. If the review step triggers edits, verification becomes stale.
  2. Missing npm run lint:fix — not in the allowlist, so agents must manually fix lint violations instead of using the auto-fixer.
  3. Missing deny-list test assertions — tests assert absence of dangerous npm subcommands but not non-npm commands (curl, wget, etc.). Blocked by whitelist architecture but would harden tests against future expansion.

— qwen3.7-max via Qwen Code /review

@yiliang114

Copy link
Copy Markdown
Collaborator Author

@qwen-code /triage

@qwen-code-ci-bot qwen-code-ci-bot left a comment

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.

LGTM — clean, focused change. Tests pass, security model is sound. One note: the PR body describes features (review triggers, backlog backpressure, recheck logic) that aren't in this diff — worth updating the description to match what's actually shipped so reviewers aren't looking for missing changes. ✅

@wenshao
wenshao added this pull request to the merge queue Jul 6, 2026
Merged via the queue into QwenLM:main with commit 5f7b57f Jul 6, 2026
35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants