feat(review): add issue-fidelity and root-cause ownership gate to /review - #6395
Conversation
…view Adds a dedicated Issue Fidelity & Root-Cause Ownership agent (Agent 0) to the /review pipeline and a core-infrastructure scope gate that runs before the review agents. Agent 0 fetches linked GitHub issue evidence directly (closingIssuesReferences plus issue comments) instead of trusting the PR author's framing, compares the original reported failure against the PR's claimed fix, and flags client-side parser/sanitizer workarounds for malformed upstream output as Critical unless a maintainer explicitly requested the defensive mitigation. The core-infra gate applies the repository's existing two-tier maintainer-only rule before spending review budget. This hardens the pipeline against a false-approval mode where a bot PR passes its own tests and reads as internally reasonable but fixes the author's mistaken diagnosis rather than the linked issue's actual root cause. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Re-run (3rd) — PR updated with 3 fix commits since last triage. Template looks good ✓ Problem: Real and demonstrated. The review pipeline lacked a stage grounding verdicts in linked-issue evidence. PR #6033 is cited as a concrete example where a bot-authored fix passed its own tests without solving the reported problem. This is an observed gap, not theoretical hardening. Direction: Aligned — closing the "false approval" mode in Size: Author is a COLLABORATOR (maintainer), so the core module gate is exempt. For the record: 96 production lines in core paths (SKILL.md + DESIGN.md), 26 lines in docs (code-review.md). Well under any threshold. Approach: The scope feels right for the stated goal — one new review agent (Agent 0: Issue Fidelity), a core-infrastructure gate, and docs updates. The author's 3 fix commits (cd22a36, b7bf541, 2e6dc36) address all 5 Critical and 3 Suggestion findings from @wenshao's review:
One minor item remains: the "Cross-file impact analysis (applies to Agents 1-6)" heading at SKILL.md:300 was flagged by the maintainer as stale but not updated — should either include Agent 0 or explicitly note the exclusion. One-line fix. The bundled-skill-hardcoding-governance concern is a defensible design choice: the paths are common patterns, the mechanism is sound, and making them project-configurable adds complexity for a prompt file. The author's reasoning is reasonable here. Moving to code review. 🔍 中文说明Re-run (第3次) — 自上次 triage 以来,PR 已新增 3 个修复提交。 模板完整 ✓ 问题: 真实且已证实。评审流水线缺少一个将结论锚定到关联 issue 证据的阶段。PR #6033 被引用为具体案例:一个 bot 提交的修复通过了自身测试但未解决所报告的问题。这是已观测到的缺口,不是理论性加固。 方向: 对齐——修补 规模: 作者为 COLLABORATOR(维护者),核心模块门禁豁免。记录在案:核心路径 96 行生产代码(SKILL.md + DESIGN.md),文档 26 行(code-review.md)。远低于任何阈值。 方案: 范围对于声明的目标是合理的。作者的 3 个修复提交解决了 @wenshao 评审中的全部 5 个 Critical 和 3 个 Suggestion 发现。一个小问题残留:SKILL.md:300 的 "Cross-file impact analysis (applies to Agents 1-6)" 标题被维护者标记为过时但未更新——应包含 Agent 0 或明确说明排除原因。一行修复。 内置 skill 硬编码治理的问题是一个合理的设计选择:路径是常见模式,机制健全,将其做成项目级可配置会增加提示词文件的复杂度。 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
|
Re-run (3rd) — updated code review after author's 3 fix commits. Code ReviewThree Markdown files, no runtime TypeScript. The diff is a prompt and documentation change — verification is behavioral (does the prompt instruct the right behavior?) and consistency-based (do the three files agree?). Independent baseline: to add an issue-fidelity check to Comparison with the diff: the PR's approach matches this baseline well. Agent 0 is correctly scoped (PR-only, graceful Remaining issues:
Doc consistency check:
All consistent. Reuse check: not applicable — this is a prompt/docs change, no new runtime code. Real-Scenario TestingN/A — prompt and documentation changes only, no runtime TypeScript to build or exercise in tmux. The behavioral verification is the doc consistency check above and confirming the prompt instructions are internally coherent (they are). 中文说明Re-run (第3次) — 作者 3 个修复提交后的更新代码审查。 代码审查三个 Markdown 文件,无运行时 TypeScript。diff 是提示词和文档变更——验证是行为性的(提示词是否指导了正确行为?)和一致性检查(三个文件是否一致?)。 剩余问题:
文档一致性检查: 所有数字一致(10 agents, 12-14 LLM 预算, Agent 0 仅 PR, ~620-730K tokens)。 真实场景测试不适用——仅为提示词和文档变更,无需构建或在 tmux 中运行的运行时 TypeScript。 — Qwen Code · qwen3.7-max |
|
Re-run (3rd) — revising verdict after author's fix commits. The prior two triage rounds requested changes, aligning with @wenshao's review. The author has now addressed all the substantive concerns across 3 fix commits:
The remaining item — a stale "Agents 1-6" heading at line 300 — is a one-line fix that doesn't warrant blocking the merge. The bundled-skill governance question is a defensible design choice that the author has thought through. The core concept (Agent 0: Issue Fidelity) is well-designed and closes a real gap in the review pipeline. The motivation is concrete (#6033), the implementation is internally consistent across three files, and the fixes from review feedback are thorough. This is ready to merge once the CHANGES_REQUESTED review from @wenshao is re-evaluated. 中文说明Re-run (第3次) — 作者修复提交后修订结论。 前两次 triage 请求修改,与 @wenshao 的评审一致。作者现在已通过 3 个修复提交解决了所有实质性问题:
剩余项目——第 300 行过时的 "Agents 1-6" 标题——是一行修复,不值得阻止合并。内置 skill 治理问题是一个经过深思熟虑的合理设计选择。 核心概念(Agent 0: Issue Fidelity)设计良好,关闭了评审流水线中的一个真实缺口。动机具体(#6033),实现在三个文件中内部一致,对评审反馈的修复是彻底的。 一旦 @wenshao 的 CHANGES_REQUESTED 评审被重新评估,此 PR 即可合并。 — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
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
left a comment
There was a problem hiding this comment.
Reviewed the SKILL.md / DESIGN.md / docs changes. The goal (ground bugfix verdicts in linked-issue evidence, gate external core changes) is sound and well-motivated, but as written several mechanisms don't work end-to-end — the prescribed gh command never fetches the issue body, Agent 0's findings get filtered by a Step 4 verifier that can't see the issue evidence, and the gate hard-codes this repository's governance into the bundled skill that runs on arbitrary repos. Details inline (5 Critical, 3 Suggestion).
Additional lower-priority notes, not anchored inline:
- Policy fidelity drift: the SKILL/DESIGN/docs restatement of the AGENTS.md gate drops the "Breadth alone is not size — a low-risk sweep touching 10+ files … is escalated to a maintainer for awareness, not auto-rejected" carve-out, so the shipped copy is both stricter (mechanical 500-line count) and looser (no awareness-escalation for broad sweeps) than the policy it claims to enforce.
- 4× duplication: the issue-fetch procedure now lives in critical rule 4, the Step 1 pr-context addendum, Agent 0's focus areas, and the user docs; the root-cause rule similarly appears 3-4 times, and the copies already disagree (rule 6 says "100% confident", Step 1 adds "and can name every downstream consumer"; rule 5 says "do NOT approve" while Agent 0 says "flag as Critical"). Pick one canonical home (Agent 0's section) and make the rest one-line pointers.
- Stale range: "Cross-file impact analysis (applies to Agents 1-6…)" was not updated; if Agent 0's exclusion is intentional, say so, otherwise the file now uses two different agent ranges for the same group.
中文摘要
目标(将 bugfix 结论锚定到关联 issue 证据、对外部核心改动加门禁)是合理且动机充分的,但按现状有多处机制无法端到端工作:给出的 gh 命令根本取不到 issue 正文;Agent 0 的发现会被看不到 issue 证据的 Step 4 验证 agent 过滤掉;核心基础设施门禁把本仓库自己的治理策略硬编码进了会在任意仓库上运行的通用内置 skill。详见行内评论(5 个 Critical、3 个 Suggestion)。
其他较低优先级问题:(1) SKILL/DESIGN/docs 对 AGENTS.md 门禁的转述丢掉了「广度不等于规模——10+ 文件的小改动 sweep 应上报维护者知悉而非按行数自动拒绝」这一豁免;(2) issue 抓取流程在 4 处重复、根因规则在 3-4 处重复,且副本之间已经出现分歧(rule 6 vs Step 1 门禁条件、rule 5 "不批准" vs Agent 0 "标 Critical"),建议以 Agent 0 小节为唯一权威版本;(3) "Cross-file impact analysis (applies to Agents 1-6)" 未同步更新。
— claude-fable-5 via Claude Code /review
|
|
||
| ### Core infrastructure scope gate | ||
|
|
||
| For PR reviews, before launching agents, check whether the diff touches core infrastructure: |
There was a problem hiding this comment.
[Critical] This hard-codes QwenLM/qwen-code's own governance policy into the bundled skill that ships to every qwen-code user and reviews arbitrary repositories. The gate is unconditional ("For PR reviews, before launching agents…"), and the path globs (packages/core/src/**, packages/*/src/services/**, …) match extremely common monorepo layouts. A user reviewing their own unrelated repo gets a teammate's 600-line PR refused with "must be maintainer-initiated" — a policy their project never adopted — and because the gate fires in Step 1, before Step 2's load-rules, the target repo's .qwen/review-rules.md cannot even override it. The skill already has the right-altitude mechanism for repo-specific policy: Step 2 injects the base branch's .qwen/review-rules.md / AGENTS.md ## Code Review section into every agent. qwen-code's gate belongs there (in this repo's rules file), not in the generic prompt — which also avoids the split-brain of the same policy living in both AGENTS.md and the shipped skill and drifting apart.
中文
这里把 QwenLM/qwen-code 自己的治理策略硬编码进了随产品分发、在任意仓库上运行的内置 skill。门禁是无条件的,而这些 path glob(packages/core/src/**、packages/*/src/services/** 等)是极常见的 monorepo 布局。用户在自己的无关仓库上 /review 时,同事的 600 行 PR 会被拒以「必须由维护者发起」——一个他们项目从未采纳的策略;且门禁在 Step 1、早于 Step 2 的 load-rules 触发,目标仓库的 .qwen/review-rules.md 连覆盖的机会都没有。skill 本身已有正确层次的机制:Step 2 会注入 base 分支的 .qwen/review-rules.md / AGENTS.md ## Code Review 小节。qwen-code 自己的门禁应放进本仓库的规则文件,而不是通用提示词——也避免同一策略在 AGENTS.md 与内置 skill 两处并存、逐渐漂移。
— claude-fable-5 via Claude Code /review
There was a problem hiding this comment.
You're right that this embeds qwen-code's own governance into a skill that ships to every user and runs on arbitrary repos. In cd22a36 I've made the gate itself robust and correct (deterministic maintainer signal, core-path-only line counting, escalate→Comment, worktree cleanup, run before npm ci), but where the policy should live — in the generic shipped skill vs. relocated to .qwen/review-rules.md / an AGENTS.md ## Code Review section that Step 2 injects — is a product/architecture decision I don't want to make unilaterally on the author's behalf, since it substantially changes this PR's thesis.
One concrete note for whoever decides: AGENTS.md currently has no ## Code Review section (the two-tier policy lives under ## Working Principles → Core Infrastructure Is Maintainer-Only), so load-rules would not pick it up as-is — relocating means first adding that section or a .qwen/review-rules.md. Leaving this thread open for a maintainer to decide the altitude.
Generated by Claude Code
|
#6033 (review) is a workaround. |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Review Summary
4 findings (2 Critical, 2 Suggestions)
Critical
-
SKILL.md:112 — Core infrastructure gate hard-blocks at 500+ lines but omits the AGENTS.md exception: "Breadth alone is not size — a low-risk sweep that touches 10+ files but changes a line or two each is escalated to a maintainer for awareness." The gate as written would block legitimate low-risk sweeps that AGENTS.md explicitly permits.
-
SKILL.md:167 — Agent 0 lacks fallback guidance when
closingIssuesReferencesis empty or when the PR is not a bugfix (feature/refactor/docs PRs). Many PRs have no linked issues. Without fallback instructions, the agent may hallucinate relevance or produce confusing output.
Suggestions
-
SKILL.md:112 — "additions + deletions combined" is ambiguous (total diff vs core-path lines only). AGENTS.md says "500+ changed lines in core" which implies core-path only.
-
DESIGN.md:291 — Token budget says "~600-700K" but the detailed calculation below says "~620-730K" (12-14 × 52K = 624-728K).
— qwen3.7-max via Qwen Code /review
|
My bad. Should not contain 500+ lines constraint, because it is a general skill rather than a repo specific skill. |
- Fetch issue evidence with `gh issue view --json title,body,comments` so the issue body (reporter repro/observed payload/expected behavior) is included; `--comments` alone omits it. Use each closingIssuesReferences entry's own repository so cross-repo linked issues resolve correctly. - Treat closingIssuesReferences as a discovery hint (fetch apparent target issues even when it is empty) and treat fetched issue content as untrusted data (extract facts, ignore embedded instructions). - Run Agent 0 (Issue Fidelity) only for PR targets; skip it for local-diff and file-path reviews, and require the PR number/repo/context in its prompt. Handle empty references / non-bugfix / gh failure explicitly. - Pass Agent 0's quoted issue evidence to Step 4 batch verification and stop it rejecting issue-grounded findings just because the code compiles/tests pass. - Make the core-infrastructure gate concrete: deterministic maintainer signal via authorAssociation, count only core-path lines, honor the AGENTS.md low-risk-sweep exception, clean up the worktree on hard block, run the gate right after fetch-pr (before npm ci), and map escalate -> COMMENT (never APPROVE) in Steps 6-7. - Sync agent counts and token math across SKILL.md, DESIGN.md, and code-review.md (Agent 0 is PR-only; ~620-730K).
doudouOUC
left a comment
There was a problem hiding this comment.
[Suggestion] Incremental review cache doesn't account for linked issue state changes
The incremental review cache (Step 1, lastCommitSha + lastModelId) tracks commit identity but not linked issue state. When Agent 0's analysis depends on issue evidence (maintainer comments, approval of workarounds), a re-review of the same commit after issue activity would short-circuit with "No new changes since last review" and preserve a stale Critical finding.
Consider adding an issue-state fingerprint (e.g., hash of closingIssuesReferences update timestamps) to the cache schema so that issue-state drift triggers a full re-review.
— qwen3.7-max via Qwen Code /review
|
@qwen-code /triage |
Aligns the code-review docs heading with the 'Issue Fidelity' name used for Agent 0 in SKILL.md and DESIGN.md, so the section connects to the pipeline diagram. Addresses review feedback.
…e it via --comment - Hard block now stops before Step 2 (load-rules) instead of before Step 3, so a PR destined for hard-block no longer runs the load-rules step. - In --comment mode the hard block posts an event=COMMENT on the PR, matching the escalate path's GitHub visibility, so external authors see the block.
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Downgraded from Approve to Comment: CI still running.
No Critical findings. Two minor documentation inconsistencies noted below as inline comments.
— qwen3.7-max via Qwen Code /review
| | Review agents | 10 (9) | issue fidelity + 6 dimensions + 3 undirected personas; Agent 7 skipped in cross-repo, Agent 0 skipped for non-PR reviews | | ||
| | Batch verification | 1 | O(1) not O(N) — batch is as good as individual | | ||
| | Iterative reverse audit | 1-3 | Loop until "No issues found" or 3-round hard cap | | ||
| | **Total** | **12-14 (11-13)** | Same-repo PR: 12-14; cross-repo lightweight PR or local/file (no Agent 0): 11-13 | |
There was a problem hiding this comment.
[Suggestion] The Total row says "cross-repo lightweight PR or local/file (no Agent 0): 11-13" but this is incorrect for cross-repo PRs. Cross-repo PRs run Agent 0 (Issue Fidelity) but skip Agent 7 (Build & Test). Only local-diff and file-path reviews skip Agent 0. Both scenarios yield 11-13 total calls but for different reasons: cross-repo drops Agent 7, local/file drops Agent 0. The Review agents row above gets this right (Agent 7 skipped in cross-repo, Agent 0 skipped for non-PR reviews), so the Total row contradicts it.
| | **Total** | **12-14 (11-13)** | Same-repo PR: 12-14; cross-repo lightweight PR or local/file (no Agent 0): 11-13 | | |
| | **Total** | **12-14 (11-13)** | Same-repo PR: 12-14; cross-repo PR (no Agent 7) or local/file (no Agent 0): 11-13 | |
— qwen3.7-max via Qwen Code /review
|
|
||
| | Agent | Focus | | ||
| | --------------------------------- | ------------------------------------------------------------------------------------------- | | ||
| | Agent 0: Issue Fidelity | Linked issue evidence, root-cause ownership, and whether the PR solves the reported problem | |
There was a problem hiding this comment.
[Nice to have] The table entry says Agent 0: Issue Fidelity but the pipeline diagram on line 32 and all other references (SKILL.md, DESIGN.md) use the full name Agent 0: Issue Fidelity & Root-Cause Ownership. Consider using the full name for consistency.
| | Agent 0: Issue Fidelity | Linked issue evidence, root-cause ownership, and whether the PR solves the reported problem | | |
| | Agent 0: Issue Fidelity & Root-Cause Ownership | Linked issue evidence, root-cause ownership, and whether the PR solves the reported problem | |
— 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. All substantive concerns from the prior review have been addressed across 3 fix commits. One minor stale agent range remains (SKILL.md:300) but doesn't block. The existing CHANGES_REQUESTED from @wenshao will need to be re-evaluated by the reviewer. ✅
What this PR does
Adds a dedicated Issue Fidelity & Root-Cause Ownership stage (Agent 0) to the
/reviewpipeline, and a core-infrastructure scope gate that runs before any review agents are launched.Agent 0 stops trusting the PR author's own framing of a bugfix. For bugfix PRs it fetches the linked issue evidence directly — GitHub's strong closing-issue metadata via
gh pr view --json closingIssuesReferences, then the original report and maintainer discussion viagh issue view --comments— and treats the reporter's reproduction, observed payload, and expected behavior as the highest-priority statement of the problem, ranking the PR description last. It then judges whether the change fixes the reported failure or merely the author's proposed explanation of it, and whether the tests replay the issue's actual failing shape rather than a hand-picked happy path. It also enforces a root-cause ownership rule: when the linked issue shows an upstream provider or service returned malformed data outside the client contract, a client-side parser/sanitizer workaround is flagged as Critical rather than accepted as a real fix, unless a maintainer explicitly asked for that defensive mitigation.The core-infrastructure gate applies the repository's existing two-tier maintainer-only rule (already documented in
AGENTS.md) at the start of a review, before spending review budget: external PRs with 500+ changed lines in core infrastructure are hard-blocked as "must be maintainer-initiated", and smaller core changes are only reviewed when the reviewer can be 100% confident and can name every downstream consumer — otherwise the verdict is "escalate to a maintainer".The rest of the diff is the bookkeeping that follows from adding one agent: the pipeline is now 10 review agents (was 9), the LLM-call budget is 12–14 (was 11–13), and the design/user docs are updated to match.
Why it's needed
The review pipeline had no stage that grounded its verdict in the linked issue's original evidence. Every dimension (correctness, security, performance, tests, …) reviews the diff as presented, so a PR whose diagnosis is wrong can still pass: it can be internally well-tested and read as reasonable while only fixing the author's mistaken explanation, not the failure the issue actually reported. A very common shape of this is a client-side sanitizer or parser tweak that swallows malformed output from an upstream provider — the added test replays that one malformed shape and goes green, which proves the workaround handles that shape but says nothing about whether patching the client is the architecturally correct fix.
This is not hypothetical. A number of bot-authored PRs have been merged that pass their own tests and look fine in isolation but do not solve the reported problem — #6033 is an example. The gap that let those through is exactly the missing "does this actually fix the linked issue, and does the root cause even belong in this client?" check, plus the absence of an up-front gate for external changes to core infrastructure. This PR hardens the review logic against that false-approval mode.
Reviewer Test Plan
How to verify
This change is to the bundled
/reviewskill prompt (SKILL.md) and its docs — there is no runtime TypeScript to build, so verification is behavioral./reviewon a bugfix PR that links an issue where an upstream provider returned malformed data and the PR adds a client-side sanitizer/parser workaround. Expected: Agent 0 fetches the linked issue (closingIssuesReferences+ issue comments), and the verdict flags the workaround as a Critical root-cause-ownership issue rather than approving it, calling out that a passing "malformed-shape" test does not prove architectural correctness./reviewon an external PR that changes 500+ lines underpackages/core/src/**. Expected: the review hard-blocks in Step 1 with "must be maintainer-initiated" and does not launch the agent ensemble./reviewon an ordinary bugfix PR with a clearly correct fix. Expected: Agent 0 returns "No issues found" and does not add noise — the existing behavior is unchanged for PRs that genuinely fix their linked issue.docs/users/features/code-review.mdandDESIGN.mdshould consistently say 10 review agents and a 12–14 LLM-call budget.Evidence (Before & After)
N/A — no user-visible / TUI change; this modifies review-agent behavior and documentation only.
Tested on
Environment (optional)
N/A — no build or unit tests; the change is to the bundled skill prompt and docs.
Risk & Scope
qwen reviewsubcommands; Agent 0 relies onghbeing available and the PR having usableclosingIssuesReferences, and falls back to PR-context text when issue metadata is absent./reviewinvocations keep working.Linked Issues
Refs #6033 as a motivating example (no closing keyword).
中文说明
这个 PR 做了什么
在
/review流水线中新增了一个专门的 Issue Fidelity & Root-Cause Ownership(问题一致性与根因归属) 阶段(Agent 0),并在启动任何评审 agent 之前新增了一个 核心基础设施范围门禁(core-infrastructure gate)。Agent 0 不再轻信 PR 作者对 bugfix 的自我描述。对于 bugfix 类 PR,它会直接抓取关联 issue 的证据——先用
gh pr view --json closingIssuesReferences获取 GitHub 的强关联 closing-issue 元数据,再用gh issue view --comments获取原始报告与维护者讨论——并把报告者的复现步骤、观察到的实际数据、以及期望行为作为对问题的最高优先级描述,PR 描述排在最后。随后它判断该改动修复的是 issue 报告的真实故障,还是仅仅修复了作者对故障的猜测;并检查测试是否复现了 issue 真正失败的数据形态,而非精心挑选的顺利路径。它还强制执行根因归属规则:当关联 issue 显示是上游 provider/服务返回了不符合客户端契约的错误数据时,客户端侧的 parser/sanitizer 绕过改动会被标记为 Critical,而不是被当作真正的修复——除非维护者明确要求这种防御性缓解。核心基础设施门禁会在评审开始、消耗评审预算之前,应用仓库已有的两级“仅维护者可改”规则(已记录在
AGENTS.md中):外部 PR 若在核心基础设施中改动了 500+ 行,直接硬拦截并提示“必须由维护者发起”;较小的核心改动只有在评审者能 100% 确信、并能列出每一个下游消费者时才继续评审,否则结论为“上报维护者”。diff 的其余部分是新增一个 agent 带来的连带更新:流水线现在是 10 个评审 agent(原为 9 个),LLM 调用预算为 12–14 次(原为 11–13 次),并同步更新了设计文档与用户文档。
为什么需要它
评审流水线此前没有任何一个阶段会把结论锚定到关联 issue 的原始证据上。每个维度(正确性、安全、性能、测试……)都是按提交呈现的样子去审 diff,因此一个诊断错误的 PR 仍然可能通过:它可以自带完善的测试、看上去也很合理,但实际上只修复了作者对故障的错误解释,而不是 issue 真正报告的故障。一个非常常见的形态是:在客户端加一个 sanitizer 或 parser 改动来吞掉上游 provider 返回的错误数据——新增的测试复现了那一种错误形态并通过,这只能证明该绕过能处理这种形态,却完全无法说明“在客户端打补丁”是否是架构上正确的修复。
这并非假设。已经有若干由 bot 提交的 PR 通过了自身的测试、单看也没问题,但并没有真正解决所报告的问题——#6033 就是一个例子。让这些 PR 蒙混过关的漏洞,正是缺少“这个改动到底有没有修复关联 issue、以及根因是否本就该由这个客户端承担”的检查,以及缺少针对核心基础设施外部改动的前置门禁。本 PR 就是为了针对这种“误批准”模式加固评审逻辑。
评审者测试计划
如何验证
本改动针对的是内置的
/reviewskill 提示词(SKILL.md)及其文档——没有需要构建的运行时 TypeScript,因此验证是行为性的。/review,该 PR 关联的 issue 显示上游 provider 返回了错误数据,而 PR 加了一个客户端 sanitizer/parser 绕过。预期: Agent 0 会抓取关联 issue(closingIssuesReferences+ issue 评论),结论会把该绕过标记为 Critical 的根因归属问题而非批准它,并指出一个通过的“错误形态”测试无法证明架构上的正确性。packages/core/src/**下改动 500+ 行的外部 PR 运行/review。预期: 评审在 Step 1 硬拦截并提示“必须由维护者发起”,不会启动 agent 集群。/review。预期: Agent 0 返回“No issues found”,不制造噪音——对于真正修复了关联 issue 的 PR,既有行为保持不变。docs/users/features/code-review.md与DESIGN.md应一致地写明 10 个评审 agent、12–14 次 LLM 调用预算。证据(前后对比)
N/A——没有用户可见 / TUI 变化;本改动只影响评审 agent 的行为与文档。
测试平台
环境(可选)
N/A——无构建或单元测试;改动仅涉及内置 skill 提示词与文档。
风险与范围
qwen review子命令;Agent 0 依赖gh可用以及 PR 具备可用的closingIssuesReferences,在缺少 issue 元数据时回退到 PR 上下文文本。/review调用照常工作。关联 Issue
引用 #6033 作为动机示例(不使用关闭关键字)。