feat(cli): Add review settings for attribution, default effort, and default comment - #8994
Conversation
|
Re-run at Template looks good ✓ Problem: unchanged from the first pass — this is a feature, not a fix, so no reproduction is required, and the friction it addresses is concrete: Direction: aligned. Per-invocation defaults for a first-party skill belong in user settings, every default preserves current behavior, and the operator-scopes-only rule keeps repository-controlled content out of review policy. Claude Code's CHANGELOG confirms the pattern ( Size: core paths touched ( Approach: scope still feels right — three small settings under one Risk: no elevated risk signals — none of the changed files match the revert-correlated high-risk paths. Moving on to code review. 🔍 中文说明在 模板完整 ✓ 问题：与首轮结论一致——这是 feature 而非 fix，无需复现，痛点具体：频繁用 方向：对齐。一方 skill 的调用级默认值放在用户设置里合理，所有默认值保持现状行为，"仅操作者作用域"规则把仓库可控内容挡在 review 策略之外。Claude Code 的 CHANGELOG 印证了这一方向（ 规模：触及核心路径（ 方案：范围依然合理—— 风险：无升级风险信号——改动文件均未命中与 revert 相关的高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewMy independent proposal — written from the title and motivation before reading the diff: a What I verified by reading the code at this head:
No new critical blockers. The four round-7 Suggestions are still open at this head — I verified each against the code: the two refusal-advice misroutes (the missing-args-file refusal carries the One scope note repeated from Stage 1: the repository-context array-bound raise (128 → 256) is a deliberate commit the description should name. Files changed (24 of 24 shown)
Test evidence — the PR's own CIAll checks that run for this fork PR completed green on the reviewed commit; the macOS/Windows unit legs and the merge-queue-only integration suite are skipped for fork PRs by design. The suite genuinely pins this change: the new tests exercise the changed API surface directly (they would not compile without the diff), assert the gate's acceptance and cross-PR refusal, the effort precedence chain, the scope flag at every resolution site, and both quadratic-strip shapes with wall-time ceilings.
The one reservation my first pass carried — that live posting was proven only by mocked- 中文说明代码审查读 diff 之前我先按标题与动机独立写了方案：一个仅从操作者作用域解析的 在这个 head 上逐项核实：
没有新的关键阻塞项。第 7 轮的四个 Suggestion 在这个 head 上仍未处理——我逐一对照代码确认过：两处拒绝建议误路由（缺少参数文件的拒绝文案恰好包含 一点与 Stage 1 重复的范围提示：repository-context 数组上限提升（128 → 256）是有意提交，描述里应当点名。 测试证据 —— PR 自身的 CI该 fork PR 实际运行的所有检查在被审 commit 上全绿；macOS/Windows 单测与仅 merge-queue 的集成套件按设计对 fork PR 跳过。套件确实钉住了本次改动：新测试直接调用被改的 API 面（没有 diff 连编译都过不了），断言了授权门的接受与跨 PR 拒绝、effort 优先级链、每个解析点的作用域 flag、以及两种二次方剥离形状的墙钟上限。 首轮遗留的唯一保留意见——实际发布只由 mock 了 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — a correct, carefully gated feature whose write path is preserved, tested, and now live-verified; the remaining items are four non-blocking Suggestions the round-7 review already surfaced, not doubts about the change itself. Stepping back: the implementation matches the proposal I wrote before reading the diff, and the eight review rounds have done what they are supposed to — every Critical from rounds 1, 4, and 6 is closed in code I re-verified at this head, and the test growth (158 → 969 lines) is almost entirely mutation-probed pins rather than coverage padding. The design decision that matters — a standing setting may replace the What keeps this at 4 rather than 5, all non-blocking and already in the autofix loop's scope: the two refusal-advice classes that still misroute (guidance prose only — the gate fails closed and nothing posts wrongly), the Approving, pinned to the reviewed commit. 中文说明回顾整体：实现与我读 diff 之前独立写出的方案一致，八轮 review 也起到了应有作用——第 1、4、6 轮的每个 Critical 都已在这个 head 的代码里关闭并被我重新核实，测试的增长（158 → 969 行）几乎全部是经过突变探针验证的钉子，而非覆盖率填充。最关键的设计决策——常驻配置可以在授权门里替代 给 4 分而不是 5 分的原因，均不阻塞且已在 autofix 循环范围内：仍有两类拒绝建议被误路由（只是引导文案——门失败关闭，不会有错误发布）； 批准，钉在所审 commit 上。 — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Not reviewed: build-and-test — packages/cli and packages/core test commands did not complete successfully in this environment; Node 22 macOS/Windows checks are skipped in CI.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
[Critical] R1-3 Attribution-off reviews still fail footer-only model ID validation (packages/cli/src/commands/review/compose-review.ts:1036). Failure scenario: when review.attribution is false and a custom model ID contains a newline or the literal footer marker, footer-only validation still throws and refuses submission even though no footer would be emitted. Fix: condition footer-safety and footer-specific nonempty validation on attribution being enabled.
中文说明
未审查：build-and-test — packages/cli and packages/core test commands did not complete successfully in this environment; Node 22 macOS/Windows checks are skipped in CI。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
[Critical] R1-3 Attribution-off reviews still fail footer-only model ID validation (packages/cli/src/commands/review/compose-review.ts:1036). Failure scenario: when review.attribution is false and a custom model ID contains a newline or the literal footer marker, footer-only validation still throws and refuses submission even though no footer would be emitted. Fix: condition footer-safety and footer-specific nonempty validation on attribution being enabled.
— gpt-5.6-sol via Qwen Code /review (v0.21.10)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not explored to full depth (tool budget reached): You are review agent reverse-audit — Reverse audit agen...: none — all checks above completed within budget.; PR #8994 adds three user settings to the /review skill — ...: none — all checks I named were completed within budget..
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
中文说明
未探索到全部深度（达到工具调用预算）：You are review agent reverse-audit — Reverse audit agen...：none — all checks above completed within budget.；PR #8994 adds three user settings to the /review skill — ...：none — all checks I named were completed within budget.。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
— qwen3.8-max via Qwen Code /review (v0.21.10)
|
@qwen-code /takeover |
qwen-code-dev-bot
added
the
autofix/takeover
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. This is a fork PR, so the first round comes from the next scheduled scan (usually within minutes). Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。本 PR 来自 fork，首轮处理将由下一次定时扫描执行（通常几分钟内）。移除 |
![qwen-code-autofix[bot]](https://github.githubassets.com/images/gravatars/gravatar-user-420.png){kind=avatar}
… gaps (QwenLM#8994) Address review feedback on the review settings: - Resolve review.attribution/effort/comment from operator-controlled settings scopes only (system defaults, user, system); a repository's .qwen/settings.json is content under review and must not control whether findings publish, whether the review names its model, or how deeply the pipeline verifies. - Normalize the configured review.effort through the same case- insensitive validation as the --effort flag, so "Low" cannot miss the exact comparisons the forcings run and invalid values cannot leak into the verdict. - Gate the modelId requirement and footer-safety validation on attribution: with the footer gated off, the field has no consumer and must not refuse the run. - Pass the standing review.comment setting into publish-assets' call of the shared authorisation gate, so both callers agree on what authorises a run. - Make presubmit's self-comment detection footer-independent by also matching the reviewing account's own top-level comments, so attribution-off posts still dedup. - Align SKILL.md's Step 7 gate and every --comment branch on comment.effective, and add handler-level wiring tests for all configured defaults.
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Review feedback round — all 10 findings addressedCommit: Critical findings
Suggestions
Declined / escalatedNone — every finding was verified against the code and addressed. Conflict notesNone ( VerificationCommands actually run (all from the repository root unless noted):
中文说明审查反馈轮次——全部 10 条 finding 已处理提交： Critical 类
Suggestion 类
拒绝 / 升级无——所有 finding 均对照代码核实并处理。 冲突说明无（ 验证实际执行的命令（除特别注明外均在仓库根目录）：
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not reviewed: verification and reverse audit — neither the verifier nor the reverse auditor was launched with a prompt this skill builds — the posted findings were ruled on, and the misses the rest of the review left were hunted, if at all, without the briefs this skill certifies against.
中文说明
已审查。 建议见行内评论。
未审查：验证与反向审计——验证 agent 与反向审计 agent 都没有用本 skill 构建的 prompt 启动——发布的发现即便被裁定过、评审其余部分遗漏的问题即便被搜寻过，也都缺失了本 skill 用以认证的 brief。
— deepseek-v4-flash via Qwen Code /review (v0.21.10)
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 2/100 轮）。改动内容与我反驳保留之处如下： Review round summary — PR #8994Feedback dispositions[Suggestion] Before acting, the failure scenario ("deleting it leaves every affected test green") was checked with a mutation test: changing The suggestion is still implemented because the module is new in this PR and is the only one under
Both default mutations ( ConflictsNone ( Verification
Commit: 中文说明评审轮次总结 — PR #8994反馈处理[Suggestion] 在动手之前，先用变异测试核查了其失败场景（"删掉它也不会让任何受影响的测试变红"）：把 建议仍然被采纳实现，因为该模块是本 PR 新增的，且是
两个默认值变异（ 冲突无（ 验证
提交： Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix round: no new changes — all feedback re-verified as already landedThis round's feedback contains no new findings. All eleven inline comments are confirmations of fixes that already landed in commits cf7fd06 and 58455a7, both of which are already on this PR's head ( Per-item verification (all confirmed present at HEAD):
No code was modified this round; the working tree is clean and HEAD is unchanged. All eleven threads are marked resolved in the code per the re-verification below. Verification
中文说明Autofix 本轮：无新变更——所有反馈均已复核为已落地本轮反馈不包含任何新发现。全部 11 条行内评论都是对已落地修复的确认，这些修复位于提交 cf7fd06 与 58455a7 中，两者均已在本 PR 的 head（ 逐条核验（均确认存在于 HEAD）：
本轮未修改任何代码；工作区干净，HEAD 未变。依据下方复核结果，全部 11 个线程均被标记为已在代码中解决。 验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): You are review agent reverse-audit — Reverse audit agen...: none — I finished within budget. The one adjacent area I did not deep-walk is the SKILL.md consumption of presubmit's inflated buckets (chunk 4 territory); the …; You are review agent reverse-audit — Reverse audit agen...: none — all planned checks completed within budget..
中文说明
已审查。 建议见行内评论。
未探索到全部深度（达到工具调用预算）：You are review agent reverse-audit — Reverse audit agen...：none — I finished within budget. The one adjacent area I did not deep-walk is the SKILL.md consumption of presubmit's inflated buckets (chunk 4 territory); the …；You are review agent reverse-audit — Reverse audit agen...：none — all planned checks completed within budget.。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| why: verdict.comment.requested | ||
| ? `\`--comment\` was in the review arguments for #${authorisedPr}` | ||
| : `\`review.comment\` is enabled in settings, and the review arguments name #${authorisedPr}`, |
There was a problem hiding this comment.
[Suggestion] The new setting-path why text is asserted by no test — the mutation that swaps the ternary's two branches survives the entire suite (mutation-verified: the whole review directory passes 2417 tests with the swap in place; a probe test flips red/green). — Failure scenario: why is user-visible audit text printed on success (Posted COMMENT to repo#PR — <why>), on refusal, and persisted in the JSON result (reason). With the branches swapped, a post authorised solely by the standing review.comment setting reports "--comment was in the review arguments" — attributing an irreversible public write to a flag the user never typed.
Suggested fix: in the existing setting-authorisation tests, also assert the why/reason text names review.comment, plus one assertion that the flag path still names --comment.
中文说明
问题： 新增的设置路径 why 文案没有任何测试断言——交换三元表达式两个分支的变异可以通过整个测试套件（已用变异验证：交换后整个 review 目录 2417 个测试全部通过；探针测试可使其红绿翻转）。
失败场景： why 是用户可见的审计文案：成功时打印（Posted COMMENT to repo#PR — <why>）、拒绝时打印，并持久化到 JSON 结果（reason）。若两个分支被交换，仅由常驻 review.comment 设置授权的发布会报告 "--comment was in the review arguments"——把一次不可逆的公开发布归因于用户从未输入过的 flag。
建议修复： 在现有的设置授权测试中，同时断言 why/reason 文案点名 review.comment，并保留一条断言 flag 路径仍点名 --comment。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| body: | ||
| footer === undefined | ||
| ? comment.body.replace(REVIEW_FOOTER_RE, '') | ||
| : `${comment.body.replace(REVIEW_FOOTER_RE, '')}\n\n${footer}`, |
There was a problem hiding this comment.
[Suggestion] The new footer-less strip branch applies REVIEW_FOOTER_RE unguarded, while compose-review.ts's stripReviewFooter guards on FOOTER_MARKER because the regex's unanchored leading \s* scans quadratically on long whitespace runs in marker-less bodies. Probe-measured: 477 ms @ 30k chars, 1862 ms @ 60k, 7184 ms @ 120k (~4x per doubling); the marker-guarded replace ran in 0.01–0.05 ms. — Failure scenario: this diff newly routes modelId-absent bodies through the unguarded regex (the old code early-returned before any regex when modelId was absent). A submission whose inline comment bodies contain long whitespace runs and no footer marker makes qwen review submit stall seconds-to-minutes per comment before posting.
Suggested fix: guard on FOOTER_MARKER as compose-review.ts does, or export stripReviewFooter from review-footer.ts and share it:
const stripped = comment.body.includes(FOOTER_MARKER)
? comment.body.replace(REVIEW_FOOTER_RE, '')
: comment.body;中文说明
问题： 新增的无 footer 剥离分支无条件套用 REVIEW_FOOTER_RE，而 compose-review.ts 的 stripReviewFooter 会先用 FOOTER_MARKER 守卫——因为该正则未锚定的前导 \s* 在无标记、含长空白串的正文上呈二次方扫描。探针实测：30k 字符 477 ms、60k 1862 ms、120k 7184 ms（每翻倍约 4 倍）；加了 marker 守卫的替换仅 0.01–0.05 ms。
失败场景： 本 diff 新把 modelId 缺失的正文引入这个无守卫正则（旧代码在 modelId 缺失时会在任何正则之前提前返回）。一次行内评论正文含长空白串且无 footer 标记的提交，会让 qwen review submit 在发布前每条评论卡住数秒到数分钟。
建议修复： 按 compose-review.ts 的方式用 FOOTER_MARKER 守卫，或从 review-footer.ts 导出 stripReviewFooter 供两处共用（代码块见英文部分）。
— qwen3.8-max via Qwen Code /review (v0.21.10)
|
|
||
| - **No designation, no publish** — unset or malformed `QWEN_REVIEW_ASSETS_REPO` is exit 3 and `{"published": false}`, not a fallback to some repo it picked. A refusal is a complete outcome: the findings keep their local `assetFiles` paths, which the terminal report and the saved report can still name. | ||
| - **Unauthorised run, no publish** — it reads the same verbatim args record `submit` reads, through the same shared gate (`lib/authorization.ts`), and refuses unless this run was authorised to post the review itself (an effective `--comment` naming this PR, or `--user-authorized` under Step 7's rules). A terminal-only review must not push the PR's behaviour to a public branch. Since an effective `--comment` forces high effort, low and medium runs can never publish — no separate rule needed. | ||
| - **Unauthorised run, no publish** — it reads the same verbatim args record `submit` reads, through the same shared gate (`lib/authorization.ts`), and refuses unless this run was authorised to post the review itself (an effective `--comment` naming this PR — typed as the flag or standing via the `review.comment` setting — or `--user-authorized` under Step 7's rules). A terminal-only review must not push the PR's behaviour to a public branch. Since an effective `--comment` forces high effort, low and medium runs can never publish — no separate rule needed. |
There was a problem hiding this comment.
[Suggestion] The stated invariant "low and medium runs can never publish — no separate rule needed" rests on the setting's state at Step 1 equaling its state at write time, but the new authorisation source is re-resolved live at the write (submit.ts ~639, publish-assets.ts ~550), and neither write path checks effort. Probe-verified: with frozen args 42 --effort medium, defaultComment: true yields ok: true ("review.comment is enabled in settings…") while the pre-diff call shape refuses; the gate never reads the resolved effort. — Failure scenario: an operator runs /review 42 --effort medium with review.comment off (a medium run, which Step 7 says never posts); mid-session the operator enables review.comment: true in ~/.qwen/settings.json; on "post comments" the CLI gate re-resolves live → comment.effective=true, target binds PR #42 → authorised, and the medium review publishes, contradicting the invariant this same bullet states. (Consent is present — the operator flipped the setting; the harm is the quality-tier rule being bypassed.)
Suggested fix: snapshot the Step-1 review.comment value into the session-private record the gate trusts and read the snapshot at write time, or soften this bullet to state the stability assumption.
中文说明
问题： 此处声明的不变量"low 与 medium 运行永远无法发布——无需额外规则"依赖于 Step 1 时的设置状态等于写入时的状态，但新增的授权来源在写入时是实时重新解析的（submit.ts ~639、publish-assets.ts ~550），且两条写入路径都不检查 effort。已用探针验证：冻结参数为 42 --effort medium 时，defaultComment: true 返回 ok: true（"review.comment is enabled in settings…"），而 diff 前的调用形态会拒绝；授权门从不读取已解析的 effort。
失败场景： 操作者在 review.comment 关闭时运行 /review 42 --effort medium（一次 medium 运行，Step 7 规定其永不发布）；会话中途操作者在 ~/.qwen/settings.json 里开启 review.comment: true；随后 "post comments" 时 CLI 授权门实时重解析 → comment.effective=true、目标绑定 PR #42 → 授权通过，medium review 被发布，与同一条目自己声明的不变量矛盾。（同意是存在的——操作者自己打开了设置；危害是质量分层规则被绕过。）
建议修复： 在 Step 1 把 review.comment 的值快照进授权门信任的会话私有记录，写入时读快照；或把这一条目软化为明确说明该稳定性假设。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| const review = operatorReviewSettings(); | ||
| runSubmit(argv as unknown as SubmitArgs, cliVersion, { | ||
| attribution: review.attribution, | ||
| defaultComment: review.comment, |
There was a problem hiding this comment.
[Suggestion] This diff adds a second authorisation source to submit (the standing review.comment setting, wired in the anchored lines), but the same file's --skill-args help text (line 617) still states exclusively "Its --comment is what authorises a post" — now a false description of the gate. Probe-verified against the built CLI's qwen review submit --help: the setting is never named as an authorising source, while lib/authorization.ts's JSDoc was updated from "two" to "three things". — Failure scenario: an operator who enabled review.comment: true reads --help to understand why a run posted without --comment; the help names only the recorded --comment, so they either misdiagnose a sanctioned post as a gate bypass, or wrongly conclude posting without the flag is impossible.
Suggested fix: reword the describe at line 617 to "Its --comment — or the standing review.comment setting — is what authorises a post."
中文说明
问题： 本 diff 为 submit 新增了第二个授权来源（常驻 review.comment 设置，即锚定行处的接线），但同文件的 --skill-args 帮助文案（第 617 行）仍声称"Its --comment is what authorises a post"——现在这是对授权门的错误描述。已对构建后的 CLI 运行 qwen review submit --help 验证：帮助输出从未提及该设置是授权来源，而 lib/authorization.ts 的 JSDoc 已从"两件事"更新为"三件事"。
失败场景： 开启了 review.comment: true 的操作者通过 --help 排查为什么某次运行没有 --comment 也发布了；帮助只提到记录在案的 --comment，于是要么把一次合法授权的发布误诊为绕过授权门，要么错误地认为不带 flag 不可能发布。
建议修复： 把第 617 行的 describe 改为 "Its --comment — or the standing review.comment setting — is what authorises a post."
— qwen3.8-max via Qwen Code /review (v0.21.10)
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 3/100 轮）。改动内容与我反驳保留之处如下： Autofix address-review summary — PR #8994 (round: feedback after 2026-08-12T14:37:18Z)Commit: All four inline findings were resolved in the code. Feedback points and decisions
Conflict notes
VerificationCommands actually run, in order (Node v22.23.2):
中文说明Autofix address-review 摘要 — PR #8994（本轮反馈：2026-08-12T14:37:18Z 之后）提交： 四条行内发现全部已在代码中解决。 反馈点与处理决定
冲突说明
验证实际执行的命令及结果（Node v22.23.2，按顺序）：
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
|
🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
|
🤖 Addressed the latest review feedback (round 4/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 4/100 轮）。改动内容与我反驳保留之处如下： Autofix review round — failed-check repairFeedback point
No review comments, inline comments, or issue-level comments were in this round's feedback; the failed check was the only actionable item. No base conflict ( Root causeThe failing test is Evidence chain:
Decision and changeRe-calibrated the shared bound instead of shrinking the manifest globs: narrowing Changes (one commit,
Also investigated and ruled out as local-environment artifacts (not CI failures): VerificationCommands actually run (trusted project commands), on the final tree:
Environment-specific note: the exact GitHub-hosted runner cannot be reproduced here; verification above re-runs the Test job's runnable steps locally, including the full unit suite under the CI job's environment variables. 中文说明Autofix 审查轮次 — 失败检查修复反馈点
本轮反馈中没有审查评论、行内评论或议题级评论；失败检查是唯一可操作的项目。无基线冲突（ 根因失败的测试是 证据链：
决策与改动选择重新校准共享上限，而不是收缩 manifest 的 glob：收缩 改动（单个提交
另经排查并排除为本地环境因素（非 CI 失败）： 验证实际运行的命令（项目可信命令），针对最终代码树：
环境特异性说明：此处无法完全复刻 GitHub 托管运行器；以上验证在本地重新运行了 Test 作业中可运行的各步骤，包括在 CI 作业的环境变量下运行完整单元测试套件。 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
Maintainer verification report — real bundles, real GitHub writesVerdict: every testable claim in the description holds end-to-end — with one inaccuracy: the three settings are NOT in the Environment
Unit tests
|
Scenario (review parse-args "8994") |
Head result | Base result |
|---|---|---|
{"review":{"effort":"Low"}} (case test) |
low / effortSource: "configured" |
high / "default" (setting ignored) |
same + explicit --effort medium |
medium / "explicit" (flag wins) |
— |
effort: "low" + comment: true, PR target |
high / "forced-by-comment", warning names review.comment the setting, not a flag |
— |
effort: "low" + comment: true, local target |
low / "configured" (forcing only when comment is effective) |
— |
{"review":{"effort":"turbo"}} |
warns Invalid review.effort value "turbo" in settings; using the default effort. and falls back |
— |
review.comment — the authorization gate, live
Refusals (real submit, exit 3, nothing posted):
- No flag, no setting →
REFUSED … `--comment` was not in the review arguments. - Setting on but recorded args name PR 9999, submission targets OpenAI API Error: 401 Incorecct API Key provided #6 →
the review arguments authorise pull request #9999, but this submission targets #6— the setting replaces the flag, not the target binding. - Setting written only in the workspace
.qwen/settings.json→ refused exactly like no setting (operator scopes only); the identical content in the user scope authorizes (control).
Authorized post with no --comment typed: S3 below landed, and the gate's message names the setting — Authorised (`review.comment` is enabled in settings, and the review arguments name #6). On the base bundle the identical invocation is refused.
publish-assets agrees with submit on the same gate: without the setting it refused to publish evidence images for this PR; with it, it published the 5 screenshots this very comment embeds (pr-assets/8994-review @ 38a9d5bc) — so a posting run cannot be refused its evidence.
review.attribution — live posts, byte-checked via the API
- S1, default (on): review body and inline comment both carry
_— qwen3-coder-verify via Qwen Code /review (v0.21.10)_. - S2, off: neither carries a footer, and a forged footer planted in the drafted comment was stripped from the posted result.
- modelId consequence: with attribution off, a payload without
state.modelIdpasses consistency (--dry-run: would post); with attribution on the same payload is refused withcompose-review: modelId is required.
S1 (footer on):
S2 (footer off, forged footer stripped — body and inline both clean):
S3 (posted with no --comment flag, standing setting authorised it):
Presubmit stays sighted without the footer — A/B
Live review presubmit against the target PR carrying S2's footer-less comment:
- Head:
existingComments.total: 3— the footer-less post is recognized (author + finding-shape viaseverityOf). - Base:
existingComments.total: 2— the footer-regex-only filter is blind to it. This is precisely the dedup blindness the PR closes.
Schema and docs
packages/vscode-ide-companion/schemas/settings.schema.json: re-ranscripts/generate-settings-schema.tson head → zero diff (regeneration is faithful).- User docs and bundled
SKILL.mddescribe the three keys and the operator-scopes-only rule accurately.
Finding — the /settings dialog claim
The PR description says all three settings are toggleable in the /settings dialog, but all three schema entries set showInDialog: false, and the dialog lists only showInDialog === true keys (getDialogSettingKeys(), packages/cli/src/utils/settingsUtils.ts). Confirmed live on the head TUI — searching the dialog for "review" and "attribution" surfaces only pre-existing entries:
The user docs make no dialog claim, so this is description-level only — either flip the three to showInDialog: true (they seem like reasonable dialog citizens) or amend the description before merge.
中文版本（完整对照）
维护者验证报告 —— 真实构建、真实 GitHub 写入
结论：描述中所有可实测的声称端到端全部成立——只有一处不实：三个设置项均不在 /settings 对话框中（三项 schema 全部 showInDialog: false），与"三项均可在 /settings 对话框中切换"矛盾。其余全部通过。
环境
- Head
aa42fc7e独立 worktree 检出，npm run build && npm run bundle产出真实dist/cli.js（v0.21.10）。 - Base = merge-base
4a281f2e（即当前origin/main），同样方式构建，用于前后 A/B 对照。 - 每个场景使用独立
QWEN_HOME；写入路径用真实gh对一次性靶场 PR（wenshao/qwen-code#6，已关闭）实际发布，上文截图即其产物。
单元测试
npx vitest run src/commands/review src/config src/commands/review.test.ts（最后一个钉住 qwen review 子命令清单，目录 glob 容易漏）：107 个文件 3663 通过 / 1 失败。唯一失败 stale-bundle.test.ts（SKILL.md 引文与通知行数 5≠6）在同环境的 base 提交上逐字节复现——本机 macOS 既有问题，与本 PR 无关；PR 的 ubuntu CI 测试腿（唯一实跑测试的腿）是绿的。
review.effort
场景（review parse-args "8994"） |
Head | Base |
|---|---|---|
{"review":{"effort":"Low"}}（大小写） |
low / "configured" |
high / "default"（设置被无视） |
同上 + 显式 --effort medium |
medium / "explicit"（flag 优先） |
— |
effort: "low" + comment: true，PR 目标 |
high / "forced-by-comment"，警告点名配置项而非 flag |
— |
| 同上，本地目标 | low / "configured"（仅 comment 生效时才强制） |
— |
{"review":{"effort":"turbo"}} |
警告并回落内置默认 | — |
review.comment —— 授权门实测
拒绝路径（真实 submit，exit 3，未发布）：
- 无 flag 无设置 →
REFUSED … `--comment` was not in the review arguments。 - 设置开启但记录参数指向 PR 9999、提交目标 OpenAI API Error: 401 Incorecct API Key provided #6 → 拒绝并点名两个 PR 号——设置替代的是 flag，不是目标绑定。
- 设置只写在工作区
.qwen/settings.json→ 与无设置完全一样地拒绝（仅操作者作用域）；同样内容写用户作用域则授权（对照组）。
未输入 --comment 的授权发布：下方 S3 成功落地，授权信息点名配置项——Authorised (`review.comment` is enabled in settings, and the review arguments name #6)。同样的调用在 base bundle 上被拒绝。
publish-assets 与 submit 共享同一授权门：无设置时拒绝为本 PR 发布证据图；有设置时成功发布本评论内嵌的 5 张截图（pr-assets/8994-review @ 38a9d5bc）——发布 review 的运行不会在证据图上被拒。
review.attribution —— 真实发布、API 逐字节核对
- S1 默认（开）：review 正文与 inline 评论均带
_— qwen3-coder-verify via Qwen Code /review (v0.21.10)_。 - S2 关闭：两处均无 footer，且草稿中伪造的 footer 被剥离后才发布。
- modelId 联动：attribution 关闭时无
state.modelId的 payload 通过一致性检查（--dry-run判定可发布）；开启时同一 payload 被拒并报compose-review: modelId is required。
（截图见上文英文部分 S1/S2/S3。）
presubmit 无 footer 仍可见 —— A/B
对携带 S2 无 footer 评论的靶场 PR 实跑 review presubmit：
- Head：
existingComments.total: 3—— 无 footer 的帖子按作者 + finding 形态（severityOf）被识别。 - Base：
existingComments.total: 2—— 仅按 footer 正则过滤，对其失明。这正是本 PR 修掉的 dedup 盲区。
Schema 与文档
settings.schema.json：head 上重跑scripts/generate-settings-schema.ts→ 零 diff。- 用户文档与内置
SKILL.md对三个键及"仅操作者作用域"规则的描述准确。
Finding —— /settings 对话框声称
PR 描述称三项可在 /settings 对话框切换，但三项 schema 均为 showInDialog: false，而对话框只列出 showInDialog === true 的键（getDialogSettingKeys()，packages/cli/src/utils/settingsUtils.ts）。已在 head TUI 实测确认：对话框内搜索 "review" 与 "attribution" 只出现旧条目（截图见上）。用户文档没有该声称，属描述层问题——建议合并前要么把三项改为 showInDialog: true（放进对话框也合理），要么修订描述。
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. 1 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): PR #8994 adds three operator-scope-only review settings t...: none — all planned checks completed within budget.; PR #8994 adds three operator-scope-only review settings t...: none — the chunk and all implementation cross-checks above completed within budget.; PR #8994 adds three operator-scope-only review settings t...: none — all checks I started were completed within budget.; PR #8994 adds three operator-scope-only review settings t...: none — all checks above completed within budget.; PR #8994 adds three operator-scope-only review settings t...: none — all checks above were completed within budget..
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
中文说明
已审查。 建议见行内评论。 1 条建议级发现无法锚定到改动行，已丢弃；此处无需进一步处理。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）：PR #8994 adds three operator-scope-only review settings t...：none — all planned checks completed within budget.；PR #8994 adds three operator-scope-only review settings t...：none — the chunk and all implementation cross-checks above completed within budget.；PR #8994 adds three operator-scope-only review settings t...：none — all checks I started were completed within budget.；PR #8994 adds three operator-scope-only review settings t...：none — all checks above completed within budget.；PR #8994 adds three operator-scope-only review settings t...：none — all checks above were completed within budget.。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| export function stripReviewFooter(body: string): string { | ||
| return body.includes(FOOTER_MARKER) | ||
| ? body.replace(REVIEW_FOOTER_RE, '') | ||
| : body; | ||
| } |
There was a problem hiding this comment.
[Suggestion] The marker guard only skips the regex for marker-less bodies; a body that quotes the full footer shape (_— + via Qwen Code /review) mid-text with no trailing footer still runs the start-unanchored \s*-leading regex — probe-measured quadratic: 169 ms / 674 ms / 2781 ms at 20k / 40k / 80k chars when a long whitespace run follows the quoted footer (~4× per doubling). The added perf test pins only the marker-absent case. The shape pre-existed at both old call sites, so this PR narrows the surface — the finding stands on the new shared function and its linearity-contract docstring. — Failure scenario: a model-drafted comment or bodyCriticals entry quotes the literal footer (findings about this dogfooded subsystem do — the repo's own tests and PR comments carry _— … via Qwen Code /review strings) and contains a long contiguous whitespace run with no footer at the end → includes(FOOTER_MARKER) passes, REVIEW_FOOTER_RE scans unanchored, and the compose/submit step stalls (≈1.8 s per comment at GitHub's ~65k inline-comment cap; the doc comment's own measured shape was ~20 s at 80k).
Suggested fix: narrow the guard's remainder — only run the regex when the footer marker appears in the body's trimmed tail (a strippable trailing footer's marker sits near the end) — or strip via a linear backward walk over footer-shaped lines. A probe applying the tail-window fix took the pathological shape from ~2.8 s to <0.1 ms while every correctness control stayed green.
中文说明
问题： marker 守卫只对不含 marker 的 body 跳过正则；body 在中段引用完整 footer 形态（_— + via Qwen Code /review）且没有尾部 footer 时，仍会执行开头未锚定的 \s* 正则——probe 实测二次方增长：引用 footer 后跟长空白段时，20k / 40k / 80k 字符分别耗时 169 ms / 674 ms / 2781 ms（每翻倍约 4 倍）。新增的性能测试只钉住了无 marker 的场景。该形态在两个旧调用点本就存在，本 PR 收窄了暴露面——此发现针对新的共享函数及其线性契约 docstring。
失败场景： 模型起草的评论或 bodyCriticals 条目在中段引用字面 footer（对这个自举子系统的发现确实会这样做——仓库自己的测试与 PR 评论中都带 _— … via Qwen Code /review 字符串），且包含一段长连续空白、结尾没有 footer → includes(FOOTER_MARKER) 通过，REVIEW_FOOTER_RE 无锚点扫描，compose/submit 步骤因此停滞（按 GitHub 约 65k 的行内评论上限估算约每条评论 1.8 秒；doc 注释自己记录的形态是 80k 时约 20 秒）。
建议修复： 收窄守卫的判定范围——仅当 footer marker 出现在 body 的修剪尾部时才跑正则（可剥离的尾部 footer 的 marker 必然靠近结尾）——或改为对 footer 形态的行做线性向后遍历剥离。probe 验证尾部窗口修复可将病态形态从约 2.8 s 降至 0.1 ms 以下，且所有正确性对照仍为绿。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| } | ||
|
|
||
| const verdict = parseReviewArgs(raw); | ||
| const verdict = parseReviewArgs(raw, { comment: req.defaultComment }); |
There was a problem hiding this comment.
[Suggestion] The missing-args-file catch branch immediately above this line (unchanged by the diff) still says the run "cannot show that --comment was requested" and never names the standing review.comment setting, which this PR makes an equal authorising source. The adjacent branch was rewritten for exactly this misdirection (R3-2), but this sibling kept the old single-source wording; it is also unpinned by any test (the gate tests go through the authFor helper, which always writes the args file). The refusal behaviour itself is correct fail-closed — only the diagnostic is stale. — Failure scenario: operator enables review.comment: true and runs /review 123; the session's args record is absent (never written, or a session-id/path mismatch). The gate correctly refuses — but the persisted refusal claims the run failed to show "--comment was requested", when the setting requested posting and the real missing element is any recorded PR target to bind the write to; the operator hunts for a flag they never typed.
Suggested fix: in the catch branch above, when req.defaultComment === true, refuse with text like "no review arguments were recorded at , so there is no pull request for the standing review.comment setting to bind to" (keep the existing wording when the setting is off).
中文说明
问题： 此行上方紧邻的 missing-args-file catch 分支（未被本 diff 修改）仍然声称该运行"无法证明 --comment 被请求过"，从未提及本 PR 使其成为同等授权来源的常驻 review.comment 配置。相邻分支正是因为同样的误导被重写（R3-2），但这个兄弟分支保留了旧的单来源措辞；它也没有任何测试钉住（授权门测试都经由 authFor 辅助函数，后者总会写入 args 文件）。拒绝行为本身是正确的 fail-closed——只是诊断信息过时了。
失败场景： 运维者开启 review.comment: true 后运行 /review 123；会话 args 记录缺失（从未写入，或 session-id/路径不匹配）。授权门正确拒绝——但持久化的拒绝信息声称该运行无法证明"--comment 被请求过"，而实际上是配置请求了发布、真正缺失的是可供绑定写入的记录 PR 目标；运维者会去寻找一个从未输入过的 flag。
建议修复： 在 catch 分支中，当 req.defaultComment === true 时，以类似"未在 记录到任何 review 参数，因此常驻 review.comment 配置没有可绑定的 pull request"的措辞拒绝（配置关闭时保留现有措辞）。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| // so both read the same constants instead of keeping lockstep copies that can | ||
| // drift. | ||
| export const MAX_ARRAY_ITEMS = 128; | ||
| export const MAX_ARRAY_ITEMS = 256; |
There was a problem hiding this comment.
[Suggestion] The 128→256 bound change left two stale bounds citations where the array cap is MAX_ARRAY_ITEMS (now 256) × MAX_NOTE_LENGTH (512): (1) packages/cli/src/commands/review/compose-review.ts:1866-1869 — the MAX_DIMENSIONS rationale comment still says "at the schema bounds (128 x 512 chars)"; (2) packages/cli/src/commands/review/compose-review.test.ts:720 — "The schema admits 128 dimensions x 512 chars". Every other 128 citation was re-pinned in lockstep (design doc, both bound-test files, even the dedup-test prose); these two survivors were found independently by three auditors and the verifier. Behaviour is unaffected (slice(0, MAX_DIMENSIONS) caps output either way) — this is a lockstep-maintenance miss. — Failure scenario: a maintainer sizing the worst-case disclosure paragraph from either comment computes half the real worst case (128 × 512 ≈ 65 KB instead of 256 × 512 ≈ 131 KB); a boundary fixture built from it ("the schema admits 128") composes a schema-legal context — the wire validator accepts up to 256 — so the intended fail-closed pin silently tests nothing.
Suggested fix: update both comments to 256 × 512 (or reference MAX_ARRAY_ITEMS × MAX_NOTE_LENGTH by name so they cannot drift again), in the same edit.
中文说明
问题： 128→256 的边界修改遗留了两处过时的边界引用——这两处的数组上限是 MAX_ARRAY_ITEMS（现为 256）× MAX_NOTE_LENGTH（512）：(1) packages/cli/src/commands/review/compose-review.ts:1866-1869 —— MAX_DIMENSIONS 的理由注释仍写 "at the schema bounds (128 x 512 chars)"；(2) packages/cli/src/commands/review/compose-review.test.ts:720 —— "The schema admits 128 dimensions x 512 chars"。其余所有 128 引用都已同步更新（设计文档、两个边界测试文件，甚至 dedup 测试的示例文本）；这两处遗漏由三个审计 agent 与验证 agent 独立发现。行为不受影响（slice(0, MAX_DIMENSIONS) 两种情况下都限制输出）——这是同步维护的遗漏。
失败场景： 维护者按这两处注释估算最坏情况披露段大小时会算出真实最坏值的一半（128 × 512 ≈ 65 KB，而非 256 × 512 ≈ 131 KB）；据此构造的边界夹具（"schema 只接受 128 条"）实际上是 schema 合法的上下文——wire 校验器最多接受 256 条——意图中的 fail-closed 钉住就静默失效了。
建议修复： 同一次编辑中把两处注释更新为 256 × 512（或直接按名引用 MAX_ARRAY_ITEMS × MAX_NOTE_LENGTH），使其不再漂移。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| body: 'we fixed this, thanks', | ||
| path: 'a.ts', | ||
| line: 12, | ||
| commit_id: 'abc123', | ||
| in_reply_to_id: 1, |
There was a problem hiding this comment.
[Suggestion] The "does not author-match replies" fixture never exercises the !c.in_reply_to_id guard: its body ('we fixed this, thanks') fails the severityOf shape gate regardless of the reply flag, so deleting !c.in_reply_to_id && from the presubmit filter keeps the whole suite green (mutation-verified: 55/55 pass with the guard removed). Giving the fixture a finding-shaped body flips the probe — the same mutation then fails this test. — Failure scenario: the guard can be dropped in a later refactor with no red test; a finding-shaped reply by the reviewing account (a body opening **[Critical]**/**[Suggestion]** — the format this skill mandates on finding prose) at the current commit's path:line then lands in the overlap bucket, and blockOnExistingComments deterministically and silently withholds a genuinely new finding at that location — the exact failure the shape gate exists to prevent.
| body: 'we fixed this, thanks', | |
| path: 'a.ts', | |
| line: 12, | |
| commit_id: 'abc123', | |
| in_reply_to_id: 1, | |
| body: '**[Critical]** confirmed, thanks', | |
| path: 'a.ts', | |
| line: 12, | |
| commit_id: 'abc123', | |
| in_reply_to_id: 1, |
With a finding-shaped body the assertion still expects total 0 — now only the !c.in_reply_to_id term can produce that.
中文说明
问题： "does not author-match replies" 夹具从未真正检验 !c.in_reply_to_id 守卫：其 body（'we fixed this, thanks'）无论是否有回复标记都会先被 severityOf 形态门拒绝，因此从 presubmit 过滤器中删除 !c.in_reply_to_id && 后整个套件仍然全绿（突变验证：删除守卫后 55/55 通过）。把夹具改为 finding 形态的 body 即可翻转探针——同样的突变会让该测试失败。
失败场景： 该守卫可在后续重构中被删除而没有任何测试变红；审查账号以 finding 形态（以 **[Critical]**/**[Suggestion]** 开头——本 skill 对 finding 文本强制的格式）在当前 commit 的同一 path:line 回复时，该回复会落入 overlap 桶，blockOnExistingComments 将确定性地、静默地扣下该位置真正的新发现——正是形态门要防止的失败。
建议修复： 见 suggestion 块——给回复夹具一个 finding 形态的 body，使形态门不再掩盖守卫（仍期望 total 0——此时只有 !c.in_reply_to_id 项能产生该结果）。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| // Wiring leg: dropping `defaultComment` from the handler call leaves the | ||
| // direct runPublishAssets test green while production refuses. The | ||
| // workspace-polluted mock stands guard on the scope flag at the same | ||
| // time — it answers a flag-less call with comment:true. |
There was a problem hiding this comment.
[Suggestion] The workspace-polluted loadSettings mock this comment advertises as a scope-flag guard can never trip in this file: the only handler-driven test is this success case, which sets reviewSettingsMock to { comment: true } — both mock branches then answer comment:true, so dropping skipWorkspaceSettings on this path changes nothing; and every refusal test goes through run() → runPublishAssets directly, which never reads settings (the only operatorReviewSettings call is inside the handler). Mutation-verified: dropping the flag keeps this suite green, while the identical mutation reddens submit.test.ts's live handler-refusal guard. Sibling suites (submit, compose-review, parse-args) all carry a live tripwire; this is the only one whose polluted view coincides with its single handler test's expectation. The scope property itself is pinned at the source (review-settings.test.ts toHaveBeenCalledWith), so this is a vacuous-guard + overclaiming-comment quality issue, not an unguarded hole. — Failure scenario: a future change resolving review.comment without skipWorkspaceSettings on the publish-assets handler path ships with this suite green, while maintainers trust the comment that a second guard exists here.
Suggested fix: add a handler-path refusal test — args without --comment, reviewSettingsMock.mockReturnValue({}), invoke publishAssetsCommand.handler, expect exitCode 3 (with the scope flag dropped the polluted branch answers comment:true, the run publishes, and the test reddens); also correct the two overclaiming comments.
中文说明
问题： 这段注释宣称 workspace 污染的 loadSettings mock 是 scope flag 的守卫，但本文件中没有任何测试能触发它：唯一的 handler 驱动测试就是这个成功用例，它把 reviewSettingsMock 设为 { comment: true }——两个 mock 分支都返回 comment:true，因此在此路径上丢弃 skipWorkspaceSettings 不会有任何变化；而所有拒绝测试都走 run() → runPublishAssets 直连，从不读取配置（唯一的 operatorReviewSettings 调用在 handler 内部）。突变验证：丢弃该 flag 本套件仍为绿，而同样的突变会让 submit.test.ts 中有效的 handler 拒绝守卫变红。兄弟套件（submit、compose-review、parse-args）都有能触发的守卫；本文件是唯一一个污染视图与唯一 handler 测试期望重合的。scope 属性本身在源头有钉住（review-settings.test.ts 的 toHaveBeenCalledWith），因此这是空守卫 + 注释夸大，而不是未设防的漏洞。
失败场景： 未来若 publish-assets handler 路径改为不经 skipWorkspaceSettings 解析 review.comment，本套件仍为绿地通过，而维护者相信这里还有第二道守卫。
建议修复： 补一个 handler 路径的拒绝测试——args 不含 --comment、reviewSettingsMock.mockReturnValue({})、调用 publishAssetsCommand.handler、期望 exitCode 3（scope flag 被丢弃时污染分支返回 comment:true，运行会发布，测试变红）；同时修正两处夸大的注释。
— qwen3.8-max via Qwen Code /review (v0.21.10)
|
🤖 Addressed the latest review feedback (round 7/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 7/100 轮）。改动内容与我反驳保留之处如下： Autofix round summary — PR #8994One commit: Feedback points and dispositions1. [rc:3772321981] Automated reviewer [Suggestion] — presubmit reply fixture masks the reply guard → IMPLEMENTED The "does not author-match replies" fixture never exercised the Re-verified the mutation locally: with the guard temporarily deleted, exactly this one test fails (1/55); with the guard restored (byte-identical, zero diff), 55/55 pass. The probe now flips as the reviewer described. 2. @wenshao maintainer verification report (issue-level) — the The report verified every testable claim end-to-end except one: the PR description says all three review settings are toggleable in the Decision: flipped the three leaf definitions (
One consequence, checked and documented: Also noted from the report: the one unit failure seen on the maintainer's machine ( 3. Deferred non-Critical feedback (5 items) → NOT TOUCHED, per Critical-only mode The ci-bot review and inline rc:3772321967 / rc:3772321973 / rc:3772321978 / rc:3772321996 are in the 4. Failed checks / still-red checks → none reported this round. Conflict
VerificationCommands actually run and their results:
中文说明Autofix 本轮总结 — PR #8994一个提交： 反馈点与处置1. [rc:3772321981] 自动审查器 [Suggestion] —— presubmit 回复夹具掩盖了回复守卫 → 已实现 "does not author-match replies" 夹具从未真正检验 本地复验了突变：临时删除守卫后，恰好只有该测试失败（1/55）；恢复守卫后（逐字节一致，零 diff）55/55 通过。探针现已按审查器所述翻转。 2. @wenshao 维护者验证报告（issue 级）—— 报告端到端验证了描述中所有可实测的声称，仅有一处不实：PR 描述称三个 review 设置均可在 决定：把三个叶子定义（
一个已核查并记录的连带影响： 报告中的另一点说明：维护者本机看到的唯一单测失败（ 3. 已延后的非 Critical 反馈（5 项）→ 未触碰，遵循 Critical-only 模式 ci-bot 的 review 及 inline rc:3772321967 / rc:3772321973 / rc:3772321978 / rc:3772321996 位于 4. 失败检查 / 持续失败的检查 → 本轮无报告。 冲突
验证实际执行的命令及结果：
Deferred non-Critical feedbackCritical-only mode is active after 5 change-producing rounds. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. (
中文说明完成 5 个产生改动的轮次后进入仅处理 Critical 的模式。本轮可执行区域已排除下方非 Critical 反馈；这些条目保持开放，留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后；达到预算的作者（如有）在下方点名。（评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI (merge-queue-only by design) and its suite did not run locally this round.
Not explored to full depth (tool budget reached): "You are review agent reverse-audit — Reverse audit agent…": none — all checks above completed within budget.; "You are review agent reverse-audit — Reverse audit agent…": none — finished well under the tool ceiling.; chunk 2: none — all checks I intended completed within budget.; "You are review agent reverse-audit — Reverse audit agent…": none — finished within budget; no check left unfinished.; "PR #8994 adds three operator-scope-only user settings to…": none — I completed every check my method mandated within the tool budget., and 4 more.
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI (merge-queue-only by design) and its suite did not run locally this round。
未探索到全部深度（达到工具调用预算）："You are review agent reverse-audit — Reverse audit agent…"：none — all checks above completed within budget.；"You are review agent reverse-audit — Reverse audit agent…"：none — finished well under the tool ceiling.；chunk 2：none — all checks I intended completed within budget.；"You are review agent reverse-audit — Reverse audit agent…"：none — finished within budget; no check left unfinished.；"PR #8994 adds three operator-scope-only user settings to…"：none — I completed every check my method mandated within the tool budget.，另有 4 条。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| export function stripReviewFooter(body: string): string { | ||
| return body.includes(FOOTER_MARKER) |
There was a problem hiding this comment.
[Critical] R5-1: The marker guard only short-circuits marker-LESS bodies; a body that CONTAINS FOOTER_MARKER but carries no trailing footer still runs the unanchored, quadratic REVIEW_FOOTER_RE. — Failure scenario: a model-written body quoting or truncating a forged footer (_— … via Qwen Code /review present mid-text, no valid trailing footer) followed by a long single-line whitespace run → probe-measured at HEAD: 40k chars 738 ms, 80k ≈ 3.0 s, 160k ≈ 10.8 s (~4× per doubling — quadratic), while the marker-less control is ~0.1 ms. Reachable through every strip site (submit strips every non-empty comment body; compose-review strips drafted entries and ledger entries), and the new perf tests pin only the marker-less shape, so this class ships green. No adversary needed: the model-looping shapes this function exists to strip contain the marker by definition. Note: the fix must bound from the tail — bounding at the last marker occurrence was measured and does NOT work (the whitespace run sits after the last marker line and stays inside the bounded suffix).
export function stripReviewFooter(body: string): string {
if (!body.includes(FOOTER_MARKER)) return body;
// A match can only live at the tail ($-anchored regex): run the
// quadratic-prone replace over a bounded slice, not the whole body.
const tail = body.slice(-STRIP_TAIL_LIMIT); // e.g. 8192: covers any realistic accumulated footer run
const stripped = tail.replace(REVIEW_FOOTER_RE, '');
return stripped === tail ? body : body.slice(0, body.length - tail.length) + stripped;
}plus a timing pin for the marker-present/no-trailing-footer shape next to the existing marker-less one.
中文说明
问题： marker 守卫只对不含 marker 的 body 短路；含 FOOTER_MARKER 但没有尾部 footer 的 body 仍会运行未锚定、二次方复杂度的 REVIEW_FOOTER_RE。
失败场景： 模型生成的 body 在中部引用或截断了一个伪造 footer（_— … via Qwen Code /review 存在但没有合法的尾部 footer），其后跟随一长串单行空白 → 在 HEAD 上实测：4 万字符 738 ms、8 万 ≈ 3.0 s、16 万 ≈ 10.8 s（每次翻倍约 4 倍——二次方增长），而无 marker 对照仅 ~0.1 ms。所有 strip 调用点均可触发（submit 对每个非空评论 body 都 strip；compose-review 对草稿条目与 ledger 条目 strip），而新增的性能钉住测试只覆盖无 marker 形态，因此该形态会带病合入。无需攻击者：本函数要剥离的模型死循环形态本身就含 marker。注意：修复必须从尾部界定——实测"以最后一个 marker 出现位置界定"并不可行（空白串位于最后一个 marker 行之后，仍在被界定的后缀内）。
建议修复： 见代码块——把 replace 限制在有界的尾部切片上（regex 以 $ 锚定，匹配只能存在于尾部），并为"含 marker 但无尾部 footer"的形态补一个与现有无 marker 钉住并列的耗时上限测试。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| // Finding-shaped on purpose: a hand-written body fails the severityOf | ||
| // shape gate first, so the test would stay green with the reply guard | ||
| // deleted (mutation-verified) — with this body, only the | ||
| // !c.in_reply_to_id term keeps the reply out of the dedup set. |
There was a problem hiding this comment.
[Suggestion] R6-1: The comment states the opposite of the test's actual mutation property — it claims "the test would stay green with the reply guard deleted", but the fixture body ('**[Critical]** confirmed, thanks') PASSES severityOf, so deleting !c.in_reply_to_id makes the test RED. — Failure scenario: mutation-probed at HEAD: deleting the guard from the presubmit filter fails this test (AssertionError: expected 1 to be +0 at the existingComments.total assertion); restoring it returns to green. The comment even contradicts its own final sentence ("only the !c.in_reply_to_id term keeps the reply out"). A maintainer trusting the stated guarantee holds a wrong mental model of which guard this test protects — e.g. believes the reply guard is untested and safe to drop, or misreads a legitimate red on guard deletion as unrelated breakage.
| // Finding-shaped on purpose: a hand-written body fails the severityOf | |
| // shape gate first, so the test would stay green with the reply guard | |
| // deleted (mutation-verified) — with this body, only the | |
| // !c.in_reply_to_id term keeps the reply out of the dedup set. | |
| // Finding-shaped on purpose: the body PASSES the severityOf shape gate, | |
| // so deleting the `!c.in_reply_to_id` reply guard makes this test red | |
| // (mutation-verified) — only that term keeps the reply out of the dedup | |
| // set. |
中文说明
问题： 该注释与测试真实的突变性质相反——它声称"删除回复守卫后测试仍保持绿色"，但夹具 body（'**[Critical]** confirmed, thanks'）能通过 severityOf，因此删除 !c.in_reply_to_id 会让测试变红。
失败场景： 在 HEAD 上做突变探针验证：从 presubmit 过滤器中删除该守卫后此测试失败（existingComments.total 断言处 AssertionError: expected 1 to be +0）；恢复后重新变绿。注释甚至与其自身最后一句（"只有 !c.in_reply_to_id 项能把该回复挡在外面"）自相矛盾。信任该注释保证的维护者会对这个测试究竟保护哪个守卫形成错误心智模型——例如以为回复守卫未被测试、可以安全删除，或把删除守卫后合理的变红误读为无关的破坏。
建议修复： 见 suggestion 块——改写为与实测突变性质一致的表述。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| } | ||
|
|
||
| const auth = authorization(args); | ||
| const auth = authorization(args, defaultComment); |
There was a problem hiding this comment.
[Suggestion] R6-4 (with sibling claim R6-5, same advice block): runSubmit's refusal advice unconditionally appends "Re-run with --comment" to every refusal — wrong for the target-binding refusals this diff's review.comment setting path newly reaches. — Failure scenario (probe-verified end-to-end against the real runSubmit): operator sets review.comment: true, and (a) the recorded args name no PR (/review somefile.ts) — the gate fails closed with the diff's own corrected why ("…do not name a pull request…"), or (b) the submission targets a different PR/repo/host than the recorded args authorise ("…authorise pull request #123, but this submission targets #456" — R6-5). In both cases the advice then says "Re-run with --comment": a flag already stood in for by the setting and unable to fix a target binding. The reviewing model/user reads this stderr prose to choose its retry, so the advice buys a futile retry loop (or an unearned --user-authorized escalation) — precisely the misdirection the adjacent added comment in authorization.ts condemns ("blaming a missing --comment flag the operator never typed… misdirects"). The mismatch tests pin only exit 3, not the advice text. Reachability: the orchestrated flow is pre-empted by the parse-args verdict, so direct invocation and "post comments" follow-ups on terminal-only reviews are the window.
// in runSubmit's refusal block: branch the remedy on the refusal reason
const remedy = auth.why.includes('`--comment` was not in the review arguments')
? 'Re-run with `--comment`, or pass --user-authorized only after …'
: 'Re-invoke the review naming the pull request this submission targets, '
+ 'or pass --user-authorized only after …';(the advice text sits in unchanged lines below the anchored call site, hence a sketch rather than a one-click suggestion)
中文说明
问题： runSubmit 的拒绝建议无条件给每个拒绝追加"Re-run with --comment"——对本 diff 的 review.comment 配置路径新可达的 target-binding 类拒绝而言是错误建议。
失败场景：（已对真实 runSubmit 做端到端探针验证）运维者设置 review.comment: true，且 (a) 记录的参数未点名 PR（/review somefile.ts）——门以本 diff 自己修正过的 why 正确拒绝（"…do not name a pull request…"）；或 (b) 提交指向的 PR/repo/host 与记录参数授权的不同（"…authorise pull request #123, but this submission targets #456"——即 R6-5）。两种情况下建议都说"Re-run with --comment"：该 flag 已由配置替代，且无法修复 target 绑定。审查模型/用户读这段 stderr 来决定重试，于是建议买来无效的重试循环（或不该有的 --user-authorized 升级）——正是 authorization.ts 中相邻新增注释所谴责的误导（"归咎于操作者从未输入过的 --comment flag……是误导"）。mismatch 测试只钉住 exit 3，未钉住建议文本。可达性：编排流程会被 parse-args 裁决预先拦下，因此窗口是直接调用与终端-only review 之后的"post comments"跟进。
建议修复： 在 runSubmit 的拒绝块中按拒绝原因分支给出补救建议——仅"未请求 comment"类拒绝保留 --comment 建议；绑定类拒绝改为"重新发起点名目标 PR 的 review"。（建议文本位于锚定行下方的未改动行，故给出示意代码而非一键 suggestion。）
— qwen3.8-max via Qwen Code /review (v0.21.11)
| // The production call carries `{ skipWorkspaceSettings: true }` — the | ||
| // authorisation default resolves from operator scopes only. A caller | ||
| // that forgets the flag reads the workspace-polluted view instead, and | ||
| // the refusal assertions redden. |
There was a problem hiding this comment.
[Suggestion] R5-5: The tripwire comment overclaims — "the refusal assertions redden" is false for this file: no refusal assertion here runs through the handler where operatorReviewSettings() lives. — Failure scenario: mutation-probed: deleting skipWorkspaceSettings: true from review-settings.ts leaves this file 35/35 green — the only publishAssetsCommand.handler invocation (line 217) is a success test whose mock returns comment: true either way, and all 15 refusal assertions go through run() → runPublishAssets directly, which never touches settings. The real guards are submit.test.ts's handler-level refusal test and review-settings.test.ts's direct scope assertion (both verified to redden under the same mutation). A maintainer relying on this comment concludes the flag is locally pinned when it is not — same stale/overclaiming-comment class as the confirmed R5-3/R6-1/R6-3. The comment still overclaims at the reviewed commit (the deferred round-5 ledger item).
| // The production call carries `{ skipWorkspaceSettings: true }` — the | |
| // authorisation default resolves from operator scopes only. A caller | |
| // that forgets the flag reads the workspace-polluted view instead, and | |
| // the refusal assertions redden. | |
| // The production call carries `{ skipWorkspaceSettings: true }` — the | |
| // authorisation default resolves from operator scopes only. A caller that | |
| // forgets the flag reads the workspace-polluted view instead; the guards | |
| // that redden are submit.test.ts's handler-level refusal test and | |
| // review-settings.test.ts's direct assertion. This file's own refusals | |
| // bypass the handler, so only the wiring leg below exercises the mock. |
中文说明
问题： 绊线注释过度声称——"the refusal assertions redden"（拒绝断言会变红）在本文件中不成立：此文件没有任何拒绝断言经过 operatorReviewSettings() 所在的 handler。
失败场景： 突变探针验证：从 review-settings.ts 删除 skipWorkspaceSettings: true 后本文件仍 35/35 全绿——唯一的 publishAssetsCommand.handler 调用（第 217 行）是成功用例，其 mock 两种情况下都返回 comment: true；全部 15 个拒绝断言都经 run() → runPublishAssets 直达，从不触碰 settings。真正的守卫是 submit.test.ts 的 handler 级拒绝测试与 review-settings.test.ts 的直接作用域断言（同一突变下均已验证变红）。依赖该注释的维护者会误以为该 flag 在本文件内已被钉住——与已确认的 R5-3/R6-1/R6-3 同属"陈旧/过度声称的注释"类。在受审 commit 上该注释仍未修正（第 5 轮被延后的 ledger 条目）。
建议修复： 见 suggestion 块——改为点名真实守卫的表述。
— qwen3.8-max via Qwen Code /review (v0.21.11)
|
🤖 Addressed the latest review feedback (round 8/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 8/100 轮）。改动内容与我反驳保留之处如下： Autofix round summary — PR #8994Four inline findings were actionable this round (one Critical, three Suggestions; the The round-6 review verdict itself ( Findings and dispositions
Changes
Conflict notes: Commit: Verification
中文说明Autofix 本轮总结 — PR #8994本轮共有 4 条可执行的内联发现（1 条 Critical、3 条 Suggestion； 第 6 轮 review 结论本身（ 发现与处置
改动
冲突说明： 提交： 验证
Deferred non-Critical feedbackCritical-only mode is active after 5 change-producing rounds. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. ( 中文说明完成 5 个产生改动的轮次后进入仅处理 Critical 的模式。本轮可执行区域已排除下方非 Critical 反馈；这些条目保持开放，留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后；达到预算的作者（如有）在下方点名。（评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5 (rounds 4 and 5 reported findings; every report was verified, but the loop reached the cap without two consecutive dry rounds).
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI (merge-queue-only by design) and its suite did not run locally this round.
Not explored to full depth (tool budget reached): "You are review agent reverse-audit — Reverse audit agent…": none — all checks above completed within budget.; "PR #8994 adds three operator-scope-only user settings to…": none — all planned checks completed within budget.; "You are review agent reverse-audit — Reverse audit agent…": none — all checks completed within budget.; "You are review agent reverse-audit — Reverse audit agent…": (none — all checks I started were completed within budget.); "You are review agent reverse-audit — Reverse audit agent…": none — all planned checks completed within budget., and 8 more.
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未审查：reverse audit — did not converge within the reverse-audit round cap of 5 (rounds 4 and 5 reported findings; every report was verified, but the loop reached the cap without two consecutive dry rounds)。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI (merge-queue-only by design) and its suite did not run locally this round。
未探索到全部深度（达到工具调用预算）："You are review agent reverse-audit — Reverse audit agent…"：none — all checks above completed within budget.；"PR #8994 adds three operator-scope-only user settings to…"：none — all planned checks completed within budget.；"You are review agent reverse-audit — Reverse audit agent…"：none — all checks completed within budget.；"You are review agent reverse-audit — Reverse audit agent…"：(none — all checks I started were completed within budget.)；"You are review agent reverse-audit — Reverse audit agent…"：none — all planned checks completed within budget.，另有 8 条。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| // setting already stood in for the flag on exactly those refusals — so | ||
| // advising it there buys the futile retry loop authorization.ts's refusal | ||
| // wording exists to prevent. | ||
| const advice = auth.why.includes('`--comment` was') |
There was a problem hiding this comment.
[Suggestion] The two-way refusal-advice classifier misroutes the missing-args-file refusal when the standing review.comment setting is on: that refusal ("no review arguments were recorded at …, so this run cannot show that --comment was requested") also contains the substring `--comment` was, so branch 1 fires — "the user did not ask to publish … Re-run with --comment" — even though the operator DID ask via the setting, and the real blocker is the missing argument record, which no flag can recreate. — Failure scenario (probe-verified end-to-end against runSubmit): operator enables review.comment: true, then qwen review submit runs where no session args file exists (a manual retry outside a /review session, or a "post comments" follow-up after clearSkillArgs). The gate fails closed at the readFileSync step before consulting defaultComment; branch 1 fires with a false premise and an unfixable remedy. Nothing posts wrongly — only the guidance misdirects. The fix below flips the probe.
| const advice = auth.why.includes('`--comment` was') | |
| const advice = auth.why.includes('`--comment` was') && !defaultComment |
(or, more durably, have reviewWriteAuthorization return a refusal kind and branch on that instead of a substring; also add the missing-args-file + defaultComment=true class to the advice-class test.)
中文说明
问题： 双向拒绝建议分类器在常驻 review.comment 配置开启时，会把"缺少参数文件"这一拒绝误路由到 flag 分支：该拒绝文案（"no review arguments were recorded at …, so this run cannot show that --comment was requested"）同样包含子串 `--comment` was，于是分支 1 触发——"用户没有要求发布……请重跑并带 --comment"——但操作者其实已通过配置提出了请求，真正的阻塞是缺失的参数记录，任何 flag 都无法重建它。
失败场景（已对 runSubmit 端到端探针验证）： 运维者启用 review.comment: true 后，在没有会话参数文件的场景运行 qwen review submit（例如在 /review 会话之外手工重试，或 clearSkillArgs 之后的 "post comments" 跟进）。授权门在 readFileSync 步骤就失败关闭，尚未查询 defaultComment；分支 1 以错误前提和无法奏效的补救建议触发。不会发生错误发布——只是引导文案误导。下方修复已被探针验证可翻转行为。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| const dialogKeys = getDialogSettingKeys(); | ||
| expect(dialogKeys).toContain('review.attribution'); |
There was a problem hiding this comment.
[Suggestion] The /settings dialog exposes all three review.* keys regardless of the selected scope; with the Workspace scope selected, toggling persists them into the repository's .qwen/settings.json — exactly the file operatorReviewSettings() loads with skipWorkspaceSettings: true. — Failure scenario (probe-verified end-to-end through the real SettingsDialog save path): selecting Workspace scope and toggling the three keys writes {"review": {"comment": true, "effort": "low", "attribution": false}} into <repo>/.qwen/settings.json; operatorReviewSettings() sees none of it (control arm: the same save into User scope IS honored). The dialog then shows the settings as enabled while every later PR review ignores them, and an inert policy key lands in a committable, repo-controlled file. The two pre-existing operator-only keys both ship showInDialog: false plus a load-time workspace warning — the established convention for operator-only keys — so this diff introduces the divergence rather than inheriting it. The security direction is safe (a repo still cannot ENABLE posting); the cost is silent UX divergence plus repo-file pollution.
Fix sketch (no one-click suggestion — the change lives in the dialog, not this test):
// In SettingsDialog item generation / toggle path: skip (or mark read-only)
// keys honored only from operator scopes when the selected scope is
// Workspace — e.g. a schema flag like operatorScopesOnly checked next to
// the existing toggle-type gate; or route their writes to the User scope
// with a notice. Alternatively add a load-time warning for review.* found
// in a workspace file, matching the two existing operator-only keys.中文说明
问题： /settings 对话框无论当前选择的作用域都会展示这三个 review.* 键；在选中 Workspace 作用域时切换开关，会把值写入仓库的 .qwen/settings.json——而 operatorReviewSettings() 恰好用 skipWorkspaceSettings: true 忽略这个文件。
失败场景（已用真实 SettingsDialog 保存路径端到端探针验证）： 选中 Workspace 作用域并切换这三个键后，{"review": {"comment": true, "effort": "low", "attribution": false}} 被写入 <repo>/.qwen/settings.json；operatorReviewSettings() 对它们一律不可见（对照组：同样内容写入 User 作用域则生效）。对话框显示配置已启用，而之后每次 PR review 都无视它们；同时一个无效的策略键落入了可提交、仓库可控的文件。既有的两个仅操作者作用域键都采用 showInDialog: false + 加载期工作区告警的约定——本 diff 引入了这一偏离而非继承它。安全方向是安全的（仓库依然无法启用自动发布）；代价是静默的 UX 背离与仓库文件污染。修复示意：在对话框条目生成/切换路径中，当选中作用域为 Workspace 时跳过（或只读化）仅操作者作用域生效的键，或把这类写入路由到 User 作用域并给出提示；也可以像既有两个操作者专属键那样，为工作区文件中出现的 review.* 增加加载期告警。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| // A post that was never requested: the remedy names the flag. | ||
| runSubmit(args({ skillArgs: file('advice-flag.txt', '6771') })); | ||
| expect(advice()).toContain('Re-run with `--comment`'); |
There was a problem hiding this comment.
[Suggestion] A third refusal class misroutes in the two-way advice split (sibling of the R7-1 finding on submit.ts:441, distinct instance): when the recorded arguments name no PR and comment was never requested (no --comment, setting off), the gate emits the flag-worded refusal `--comment` was not in the review arguments (authorization.ts), whose substring routes to branch-1 advice "Re-run with --comment" — but the real blocker is that the arguments name no pull request, and the flag cannot bind one. — Failure scenario (probe-verified): a file-scoped review's args record ('src/foo.ts', no flag, setting off) → branch-1 advice fires; the prescribed retry ('src/foo.ts --comment') is refused AGAIN with "do not name a pull request" — one full wasted review re-run before branch-2 advice finally says stop. The control arm (PR target) posts correctly on retry, so only the PR-less class misroutes; the gate stays fail-closed throughout. Note the asymmetry visible in this very test: the same PR-less args with the setting ON correctly get class-2 advice below, while setting OFF gets the misdirecting flag advice. Any fix must also update the "Neither source requested it: the original wording stands" pin, which currently asserts the misrouting wording.
Fix sketch (no one-click suggestion — the fix spans authorization.ts and this test):
// authorization.ts: emit the target-binding wording when !commentRequested
// and the parsed target is not PR-shaped — the flag remedy only makes sense
// when the arguments name a PR. Or return a refusal kind from
// reviewWriteAuthorization and branch on it in submit.ts.
// Then pin the third class here:
// runSubmit(args({ skillArgs: file('advice-nothing.txt', 'src/foo.ts') }));
// expect(advice()).not.toContain('Re-run with `--comment`');中文说明
问题： 双向建议分支存在第三类误路由（与 submit.ts:441 的 R7-1 同源但属不同实例）：当记录的参数未点名 PR 且从未请求 comment（无 --comment、配置关闭）时，授权门发出 flag 措辞的拒绝 `--comment` was not in the review arguments，其子串被路由到分支 1 建议"重跑并带 --comment"——但真正的阻塞是参数没有点名 pull request，而 flag 无法绑定目标。
失败场景（已探针验证）： 文件级 review 的参数记录（'src/foo.ts'、无 flag、配置关闭）→ 分支 1 建议触发；按其建议重跑（'src/foo.ts --comment'）会再次被拒（"do not name a pull request"）——在分支 2 建议终于叫停之前，白白浪费一整轮 review 重跑。对照组（PR 目标）重跑即可正常发布，因此只有无 PR 这一类被误导；授权门全程失败关闭。注意本测试自身可见的不对称：同样的无 PR 参数，配置开启时在下方正确拿到第 2 类建议，配置关闭时却拿到误导性的 flag 建议。任何修复还必须更新 "Neither source requested it: the original wording stands" 这个钉住用例——它目前断言的正是被误路由的措辞。修复示意：在 authorization.ts 中，当 !commentRequested 且解析目标不是 PR 形态时改用目标绑定措辞（flag 补救只在参数点名 PR 时有意义），或让 reviewWriteAuthorization 返回拒绝类别、在 submit.ts 按类别分支；然后在此补钉第三类。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| // only the marker guard keeps this linear. The attribution-off path | ||
| // routes such bodies through the strip; an unguarded replace dies on the | ||
| // suite timeout long before the assertion runs. |
There was a problem hiding this comment.
[Suggestion] This rationale overclaims what the test pins: with STRIP_TAIL_LIMIT in place, deleting ONLY the marker guard (if (!body.includes(FOOTER_MARKER)) return body;) leaves the whole suite green — mutation-probed at the reviewed commit: 297/297 pass with the guard removed; the timeout fires only when the guard AND the tail limit are removed together. The body's _— opener sits ~500 KB before the tail, so slice(-8192) hands the regex 8189 spaces + end — marker-less and opener-less — and the literal prefilter rejects it in microseconds. Worse, the guard is pinned by NO test anywhere: review-footer.test.ts's "no regex, no rewrite" comment makes the same claim and is falsified by the same mutation run. Same stale/overclaiming-comment class as this PR's own prior-round entries R6-1 and R5-5. — Failure scenario: a refactor drops the fast path believing this test holds it; nothing reddens; marker-less bodies (the common case) then pay the slice+replace cost on every strip. Bounded by the tail limit, so no quadratic regression — but the claimed pin never existed.
| // only the marker guard keeps this linear. The attribution-off path | |
| // routes such bodies through the strip; an unguarded replace dies on the | |
| // suite timeout long before the assertion runs. | |
| // routes such bodies through the strip. Linearity is the guard plus | |
| // STRIP_TAIL_LIMIT jointly: the guard skips slice+replace entirely on | |
| // marker-less bodies; the tail limit bounds the marker-present worst case. |
(Also fix the sibling overclaim in review-footer.test.ts's marker-less test comment; and if the fast path alone is worth pinning, assert the regex never runs on a marker-less body — e.g. a spy — since the guard is output-neutral and cannot be pinned by output.)
中文说明
问题： 该注释夸大了测试实际钉住的内容：在 STRIP_TAIL_LIMIT 存在的前提下，只删除 marker 守卫（if (!body.includes(FOOTER_MARKER)) return body;）整个套件仍然全绿——已在受审 commit 上做突变探针验证：删除守卫后 297/297 通过；只有同时删除守卫与尾部上限才会超时。测试 body 的 _— 起始位于距尾部约 500 KB 处，slice(-8192) 交给正则的只是 8189 个空格 + end——既无 marker 也无起始符——字面量预过滤在微秒内就拒绝了匹配。更糟的是，该守卫在任何地方都没有测试钉住：review-footer.test.ts 中 "no regex, no rewrite" 测试的注释做了同样声称，并被同一次突变运行证伪。与本 PR 自己前几轮的 R6-1、R5-5 同属"陈旧/过度声称的注释"类。
失败场景： 重构者以为本测试钉住了该快速路径而将其删除；没有任何测试变红；无 marker 的 body（常见情形）从此每次 strip 都要付出 slice+replace 的代价。有尾部上限兜底，不会二次方退化——但声称中的钉住从来不存在。建议修复：把注释改写为"线性由守卫 + STRIP_TAIL_LIMIT 共同保证"（守卫让无 marker body 完全跳过 slice+replace；尾部上限界定含 marker 的最坏情形），并同步修正 review-footer.test.ts 中同类的过度声称；若快速路径本身值得钉住，可断言正则不会在无 marker body 上运行（例如用 spy）——守卫对输出无影响，无法用输出断言钉住。
— qwen3.8-max via Qwen Code /review (v0.21.11)
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 167 passed · 0 failed · 167 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：167 通过 · 0 失败 · 167 总计 Verification reportVerification report — PR #8994Verdict: 中文摘要
Central claim and A/B proofCentral claim: the three operator-scoped settings change Method: each cell runs the compiled dist of both builds through the real production seams — the Table 1 — settings through the real handler (head vs base). Witness:
|
| Cell | user settings.json |
workspace settings.json |
raw | head | base |
|---|---|---|---|---|---|
| S1 | {} |
{} |
6711 |
high/default, comment f/f | identical (defaults preserved) |
| S2 | effort medium |
{} |
6711 |
medium/configured | high/default (no consumer) |
| S3 | effort Low |
{} |
src/foo.ts --fix |
medium/forced-by-fix (case-normalized, floored) | medium/default |
| S4 | comment true |
{} |
6711 |
high/default, comment f/t (PR default already high — nothing to force) | high/default, f/f |
| S5 | comment true, effort low |
{} |
6711 |
high/forced-by-comment, warning names review.comment |
high/default |
| S6 | comment true |
{} |
src/foo.ts |
medium/default, f/f, no warnings (inert on local) | identical |
| S7 | {} |
comment/attribution/effort set | 6711 |
high/default, f/f — workspace ignored | identical |
| S8 | effort medium |
effort low |
6711 |
medium/configured (user scope wins; workspace skipped) | high/default |
| S9 | effort auto |
{} |
6711 |
high/default, no warning | identical |
| S10 | effort bogus |
{} |
6711 |
high/default + Invalid review.effort value "bogus" in settings |
high/default, no warning |
| S11 | {} |
{} |
6711 --effort low --comment |
high/forced-by-comment | identical |
| S12 | effort medium |
{} |
6711 --effort low |
low/explicit (flag beats configured) | identical |
| S13 | effort HIGH |
{} |
'' |
high/configured (case-insensitive) | medium/default |
81/81 assertions; 6 cells byte-identical head==base (the default-preservation oracle). Two mid-round harness-expectation corrections (S4's effortSource, and the ladder rung, below) were my misreadings of by-design behavior that the PR's own tests pin; the corrected expectations are the ones tabled.
Table 2 — the shared authorisation gate. Witness: 02-gate-ab-setting-authorises.png
| Cell | recorded args | request | head | base |
|---|---|---|---|---|
| G1 | 6771 |
pr 6771, setting on | ok, why names the setting + #6771 | refused (no consumer) |
| G2 | 6771 --comment |
pr 6771 | ok | identical (flag path preserved) |
| G3 | 9999 |
pr 6771, setting on | refused — binding holds (#9999 ≠ #6771) | refused |
| G4 | src/foo.ts |
pr 6771, setting on | refused — "do not name a pull request" (real blocker) | refused, blames a missing --comment (the misdirection head fixes) |
| G5 | 6771 |
pr 6771, no setting | refused | identical |
| G6 | PR URL | repo matches, setting on | ok | refused |
| G7 | PR URL | repo mismatch, setting on | refused — repo binding holds | refused |
| G8 | (missing file) | setting on | refused, fail-closed | identical |
| G9 | 6771 |
--user-authorized |
ok | identical |
28/28. The setting replaces the flag, never the binding (G3/G7), and the refusal wording names the real blocker (G4).
Table 3 — compose/attribution and the refusal advice
- C1 (18/18 with C2/C3): attribution off composes with an empty modelId (its only consumer is gated off) and no footer; attribution on still refuses an empty modelId exactly like base; default-on output is byte-identical to base across three input shapes.
- C3: forged footers in drafted
bodyCriticalsare stripped under both attribution arms; attribution on leaves exactly one (canonical) footer. - R1–R3 (12/12):
runSubmit's refusal advice matches the refusal class on head ("invoked naming it" for binding refusals, flag advice only for never-requested); base advisedRe-run with --commentfor binding refusals too — the futile-retry misdirection the head commit fixes, demonstrated on the control arm.
Table 4 — footer-strip scaling ladder (the head commit). Witness: 03-strip-ladder-base-quadratic-head-bounded.png
Marker-carrying body, long whitespace run, no trailing footer (a quoted/truncated forged footer — the natural output of the loop the strip exists for):
| N (whitespace run) | head ms | base ms (the unbounded replace) |
|---|---|---|
| 2 000 | 2 | 2 |
| 3 000 | 4 | 4 |
| 5 000 | 11 | 11 |
| 20 000 | 29 | 169 |
| 80 000 | 29 | 2 704 |
| 160 000 | 29 | 10 873 |
Base grows ~16× per 4× input (quadratic, matching the PR's "~4× per doubling" probe); head is flat at 29 ms — the 8192-char tail bound holds. Marker-less bodies: 0 ms on both arms at 200k (the pre-existing marker guard is intact on head). Trailing footer after a 20k/80k run: stripped, fast, on head.
Mutation matrix (vacuity)
Each mutant applied in the working tree, targeted suite run, tree restored byte-identical (git status clean after). Unmutated control green (69 review files, 2478 pass).
| Mutant | Suite | Result |
|---|---|---|
M0 attribution fallback true→false (positive control) |
review-settings.test | killed — 3 failed / 6 passed, exactly the default tests |
M1 delete skipWorkspaceSettings: true |
review-settings + parse-args + submit | killed — 10 failed / 141 passed (scope assertion + all wiring tests) |
M2 delete the tail bound (tail = body) |
review-footer.test | killed — 1 failed / 9 passed, the bounding test at 17 851 ms vs its 2 000 ms ceiling; the marker-guard test stayed green (correct fine attribution) |
M3 delete severityOf(c) !== null from presubmit's authorship fallback |
presubmit.test | killed — 1 failed / 54 passed, exactly "does not author-match hand-written top-level comments" |
M4 delete defaultComment: review.comment in the submit handler |
submit.test | killed — 1 failed / 63 passed, exactly the handler wiring test |
M5 delete the configuredEffort branch |
parse-args.test | killed — 9 failed / 69 passed, all in the configured-effort area |
No survivors. Every kill is attributable to the test the PR says pins that guard.
Targeted gates
packages/clisrc/commands/review: 69 files, 2478 passed, 4 skipped, 0 failed.src/commands/review src/config: 107 files, 3652 passed, 4 skipped, 12 failed — all insrc/config/settings.test.ts, a file this PR does not touch, and byte-identical on the base build (same 12 test names, A/A control). Pre-existing environment-dependent failures (home-dir/${VAR}-from-.envresolution in this container), not attributable to the PR; listed in Not covered.npx tsc --noEmit(packages/cli): exit 0.npm run generate:settings-schema: regeneratedsettings.schema.jsonis byte-identical to the committed file (0-line diff) — the "regenerated" claim verified.
Findings
F1 (informational, non-blocking) — the tail bound trades completeness for linearity past ~200 accumulated footers. A 300-footer chain (11 698 chars > the 8 192 tail) strips to 3 508 chars: the leading fragment of the chain survives, whereas the base's unbounded replace stripped all of it. The code comment names exactly this tradeoff ("far past any real re-compose loop"), and the unbounded alternative is the measured quadratic hazard (Table 4). A re-compose loop emitting 200+ footers would post a visible fragment of stale footers — cosmetic, and the canonical-footer safety properties still hold (the trailing footer is stripped/appended per the rules in every probe).
F2 (informational, non-blocking) — residual trailing whitespace when a >8k whitespace run precedes a trailing footer. a finding + 20 000 spaces + footer strips the footer but leaves 11 845 trailing spaces (a full-body $-anchored \s* would have left 0). GitHub renders trailing whitespace as nothing; the safety property (no footer survives) holds. Both F1/F2 require adversarial whitespace/loop shapes in model-drafted text and are bounded by construction at the 8192 window.
Not covered
- Per-commit attribution: the checkout is depth-2;
git rev-list HEAD^1..HEAD^2reaches 1 of the 13 commits in the metadata snapshot (shallow boundary). The aggregateHEAD^1..HEADdiff is what was verified. - Real GitHub posting: no credentials by design. The posting-side seams (gate verdicts, composed bodies, strip behavior, refusal advice) were driven from the compiled dist; the
ghwrite path itself is covered by the PR's own mocked unit tests, not by this round. /settingsdialog UI: not exercised interactively (no TUI change in the PR); dialog membership of the three keys is pinned by the suite (getDialogSettingKeystest, green).- 12 pre-existing
settings.test.tsfailures: environment-dependent (real-HOME/.envresolution in this container). Proven unrelated by the A/A control; the PR's CI on GitHub runners presumably exercises them in a compatible environment. - Reviewer Test Plan step 2 (manual post against a scratch PR): cannot be performed here (no token); its observable seams are the harness cells above.
Methodology
Environment: the CI verify container (node:22-bookworm), npm ci + npm run build pre-run at head; base control rebuilt only packages/cli in a scratch worktree at HEAD^1 (removed after the captures). Purity checks quoted: git diff HEAD^1..HEAD --name-only shows no package.json/lockfile change and only packages/core/src/skills/bundled/review/SKILL.md among internal deps; readlink -f of node_modules/@qwen-code/qwen-code-core from the base tree points at the head tree's core, whose runtime code is unchanged, and the vitest base control additionally received per-package node_modules symlinks (deps identical, lockfile untouched). Harnesses (harness/*.mjs, rerunnable) drive compiled dist through real seams — the yargs parse-args --stdin handler with temp HOME/workspace settings files and no mocks, reviewWriteAuthorization with session id cleared, composeReview, runSubmit's refusal path, and stripReviewFooter vs a faithful reconstruction of base's private strip from base's own exported constants; every timing rung ran in an isolated child under a 30 s cap. Raw logs: logs-0*.txt; captures: evidence/*.png. Assertion counts come only from the five harness runs (81+28+18+12+28 = 167); mutation and gate numbers are vitest's own, quoted per run above.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
Approved at f77b448. Verified independently, not just from prior rounds:
- Schema & validation:
settingsSchema.ts+ regeneratedsettings.schema.jsonagree (types, defaults, effort enum).operatorReviewSettings()re-validates hand-edited values (non-booleanattribution→ default true, non-booleancommentnever enables posting, non-string effort dropped) — invalid settings fail safe instead of misbehaving. All three carry the operator-scope-only note per repo convention. - Precedence: code reads explicit
--effort> configured > built-in, with--comment-forces-high and--fix-floors-medium overriding the configured value; tests pin every leg including handler wiring, case normalisation ("Low"), invalid-configured warning, and'auto'→ built-in without warning. - Back-compat: defaults (
attribution: true,comment: false, effortauto→built-in rule,defaults = {}) reproduce today's behaviour exactly when no settings are present; pinned by the resolver and parse-args suites. - Security: settings resolve with
skipWorkspaceSettings: true, so a repo cannot set review policy for its reviewers; thereview.commentsetting substitutes the flag in the gate only — target binding is untouched, and both gate callers (submit,publish-assets) resolve it identically. - Prior Criticals closed at head: R5-1's quadratic footer strip is now
stripReviewFooterwith marker guard +STRIP_TAIL_LIMITtail bound (the$-anchored regex can only match in the tail, covering the marker-present/no-trailing-footer class); presubmit dedup stays sighted attribution-off via the trimmedseverityOfauthorship fallback (medefined at presubmit.ts:466, ahead of the filter).
Non-blocking nits (all post-head, round-7 Suggestion-level, none gate this):
- The
/settingsdialog toggles persist at Workspace scope into the repo.qwen/settings.json, where they are silently ignored — consider theshowInDialog: falseconvention of the pre-existing operator-only keys (or a load-time warning). Security direction is safe as-is. - The two-way refusal-advice split in
runSubmitstill misroutes the missing-args-file refusal whenreview.commentis on, and the PR-less/setting-off refusal — advice-only, fail-closed behaviour is correct. - Two test-rationale comments overclaim what their tests pin (footer marker guard; publish-assets tripwire) — comment hygiene only.
CI on head sha: green — all checks success/skipped (Integration Tests (CLI, No Sandbox) skipped by design, merge-queue-only).
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
|
Released in v0.21.12. |
…ew.attribution (QwenLM#9027) * feat(cli): Add review settings for attribution, default effort, and default comment * fix(cli): resolve review settings from operator scopes and close gate gaps (QwenLM#8994) Address review feedback on the review settings: - Resolve review.attribution/effort/comment from operator-controlled settings scopes only (system defaults, user, system); a repository's .qwen/settings.json is content under review and must not control whether findings publish, whether the review names its model, or how deeply the pipeline verifies. - Normalize the configured review.effort through the same case- insensitive validation as the --effort flag, so "Low" cannot miss the exact comparisons the forcings run and invalid values cannot leak into the verdict. - Gate the modelId requirement and footer-safety validation on attribution: with the footer gated off, the field has no consumer and must not refuse the run. - Pass the standing review.comment setting into publish-assets' call of the shared authorisation gate, so both callers agree on what authorises a run. - Make presubmit's self-comment detection footer-independent by also matching the reviewing account's own top-level comments, so attribution-off posts still dedup. - Align SKILL.md's Step 7 gate and every --comment branch on comment.effective, and add handler-level wiring tests for all configured defaults. * test(cli): pin the review-settings operator defaults with unit tests (QwenLM#8994) * fix(cli): share the guarded footer strip and pin the gate audit text (QwenLM#8994) * fix(cli): raise the repository-context array bound to 256 (QwenLM#8994) * fix(cli): validate review setting values and tighten the review gates (QwenLM#8994) * feat(cli): drop the AI template tells from unattributed /review posts review.attribution: false already drops the footer; the posted text still read as machine output. With attribution off, inline comments now post without the **[Critical]**/**[Suggestion]** prefixes and are written as plain reviewer prose, the review body loses its fixed template markers (LGTM! ✅, the⚠️ glyph, the **[Critical]** bullets in body lists), and the Step 1 verdict carries the attribution flag so the orchestrator can pick its register. The severity strip happens in the final post object only — counting, the unmarked gate, and the ledger all still run on the marked payload, so verdict semantics are unchanged and the default mode is byte-identical to before. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): align presubmit dedup with severityOf and normalize auto effort (QwenLM#8994) * feat(cli): make plain prose the only register for posted /review text The template voice is worse prose for every audience, not just the unattributed one, so the phrasing now goes plain unconditionally: comment bodies drop the '— Failure scenario: trigger → outcome' label and arrow notation (the evidence rule is unchanged — trigger and wrong outcome must be in the sentences), and the fixed review-body copy loses LGTM! ✅ and the⚠️ glyph in both modes. What still follows review.attribution is the machine-readable layer — the severity prefixes and the footer — because qwen-autofix.yml's Critical-only mode greps posted bodies for the literal **[Critical]** marker. With prose unconditional there is no register to branch on, so the parse-args verdict's attribution field goes away again; submit keeps stripping prefix and footer at post time when the operator turned attribution off. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): keep the copy humans actually write — restore LGTM! and the warning glyph Readability, not concealment, is the criterion: reviewers type LGTM! and reach for the⚠️ glyph on a warning line every day, and both aid scanning. The earlier commits stripped them along with the real scaffolding, which overshot. What goes is only the labelled failure-scenario template; the fixed review-body copy is now byte-identical to before in both attribution modes, and the pr-context LGTM filter needs no change after all. * fix(cli): close the review-found gaps in the unattributed post path CI review on the PR found three real defects and four test/doc gaps in the first two commits; all addressed: - presubmit dedup went blind to attribution-off posts: the authorship fallback gated on severityOf, but submit strips exactly that prefix, so a later round re-posted its own findings as duplicates. Attribution-off comments now carry an invisible <!-- qwen-review --> marker and presubmit matches on it — from any account, which also closes 8994's documented other-accounts gap. - The attribution-off body-Critical branch quoted entries verbatim, leaking a model-written **[Critical]** marker into the posted body; it now strips like the inline path does. - The SKILL.md payload example still showed the labelled template the rewritten body-format paragraph forbids; both examples now show plain prose. - A comment that is nothing but its severity marker is refused at the consistency gate instead of posting the bare marker. - Forged footers followed by text survived the trailing-anchored strip and were the only attribution an unattributed post carried; the off leg now strips footer-shaped lines regardless of position. - The setting's description now names both stripping consequences (autofix Critical-only deferral; the invisible marker), and the loosened clean-approve test assertions are exact again. * test(cli): pin the reply guard with a finding-shaped reply fixture The unmarked reply body was excluded by the severityOf gate even with the reply guard deleted, so nothing pinned the guard itself (mutation-verified by CI review). * fix(cli): close the round-3 review findings on the unattributed post path Five Criticals and twelve Suggestions from the third CI review round, all addressed: - presubmit dedup: the invisible-marker branch was ungated — any account could plant the public marker string on a line expecting a blocker and have the next round silently withhold it. The branch now requires authorship by the reviewing account plus the exact trailing shape submit posts; adversarial and quote-reply fixtures pin both guards. The 'other accounts escape dedup' disclosure from 8994 stands again. - The marker-only gate was dead under attribution ON (the canonical footer was appended before the check) and stacked markers bypassed it: the strip is now iterative, delegates the classification to severityOf, and the gate refuses when the footer-and-marker-stripped remainder is empty or still marker-led. - bodyCriticals and cannot-tell entries now get the forged-footer strip on the unattributed leg (a surviving mid-entry footer was the post's only attribution), the cannot-tell parse trims before matching, and ledger titles strip the marker (the ledger rides the body as an HTML comment the autofix grep reads). - stripForgedFooterLines rewritten line-based: closing underscore optional (looping-model truncation), CRLF tolerated, 400-char line bound, fence- and indented-code aware, and byte-identical when nothing matches. - The comment marker now carries severity (<!-- qwen-review critical -->); pr-context's blocker promotion reads it, so an unresolved unattributed Critical re-enters the re-check section every round — including past the ledger's horizon. - Tests: stripForgedFooterLines unit coverage, grouped cannot-tell strip, ledger leg under a prNumber plan, and the adversarial presubmit shapes; loosened assertions re-tightened. Docs and the settings description now match the shipped behavior. * fix(cli): close the round-4 review findings — marker read/write hardening Seven Criticals and four Suggestions from the fourth CI review round: - commentMarkerSeverity now reads only the trailing posted shape, and submit strips pre-existing bare marker lines before appending the canonical marker — a marker string quoted or planted in a reviewed file can no longer choose the severity the classifiers see. - The marker disjunct in the blocker classification is gated on the reviewing account, via one shared predicate (isBlockerBody) now used by BOTH pr-context and comment-status — an empty planted 'critical' comment no longer becomes a permanent irrefutable blocker, and the two consumers can no longer diverge on the posted shape. - The ledger's drafted-comments leg strips like the bodyCriticals leg (iterative markers, forged footer lines first, footer spans off the title), and stripSeverityPrefix now strips to empty for marker-only bodies — the submit gate refuses exactly that shape, in both modes. - The fence scanner is a faithful model now: ~~~ fences count, a fence opener indented 4+ spaces does not open one, and lines inside a simple HTML block never toggle fence state. - Producer/consumer roundtrip tests pin the marker shape (the drift class the module header exists to prevent); the iterative strip, the attribution-on marker-only gate, and the strip order each carry the assertion the mutations showed missing. * fix(cli): make the unattributed strip a fixpoint, closing the round-5 escapes Seven Criticals from the fifth CI review round, all probe-verified escape hatches in the strip chain, closed by restructuring it: - One shared stripForUnattributedPost iterated to a fixpoint now serves every attribution-off leg (submit's post transform and gate, compose's body lists, both ledger legs), so the sites cannot drift on order: forged footer lines, severity prefixes (leading AND paragraph-initial, via a new fence-aware stripParagraphMarkers), bare marker lines, and footer spans interleave arbitrarily in a looping draft and only the fixpoint posts none of them. - The marker-only gate runs the full chain: a prefix over a bare marker line no longer posts an empty visible comment carrying a live marker. - Marker-only body Criticals and cannot-tell entries are refused at compose (both modes), mirroring submit's gate — an empty-stripped entry no longer counts toward REQUEST_CHANGES while rendering nothing. - The version-parens truncation (the natural mid-character cut) is admitted by all three footer regexes; blockquoted forged footer lines strip; HTML blocks stop shielding footer lines (their content renders visibly) while still not toggling fence state. - The design doc's definitional line now says what ships: no VISIBLE attribution — the machine contract moves to the invisible severity marker. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): drop the dead bare-marker arm and tolerate whitespace before the colon - submit's post transform no longer references COMMENT_MARKER: the unmarked gate runs first, so every body reaching the transform has a known severity and posts the severity-carrying marker unconditionally. - stripSeverityPrefix tolerates whitespace before the colon after a marker and treats a whitespace-only remainder as empty (a trailing newline no longer survives as a phantom non-empty body). * fix(cli): close the round-6 findings — faithful fences, render-nothing gates Seven Criticals from the sixth CI review round: - mapLinesAware now applies the map inside HTML blocks (the round-5 fix updated the comment but not the code — the shield stood), tracks the opening fence by delimiter character and run length with no info string on the closer (CommonMark), and the chain's final span pass is line-aware so fenced quotations survive the full strip. - The emptiness gates (submit + both compose lists) project through a new rendersAsNothing — Cf characters, HTML comments, hollowed fences, and forged-footer residue are not content — and run the full post-transform chain, so a scaffolded-but-invisible comment can no longer post, count toward REQUEST_CHANGES, and re-promote as an unanswerable blocker. - stripCommentMarkerLines admits blockquoted marker lines, matching its sibling regexes. - buildLedger keeps the carried id and title when the finding text starts on the line after the severity marker (trimStart before titleOf) — a regression from routing titles through the new chain. * fix(cli): close the round-7 findings — faithful quotes, code spans, render-nothing classes * fix(cli): close the round-8 findings — one displayed projection for every strip * fix(cli): close the round-9 findings — fail-closed identity, one shape per leg * fix(cli): close the round-10 findings — rendered-text signals, bounded spans, fence-safe entries - blocker prose scan reads only rendered text: an HTML comment renders as nothing, so a planted `<!-- [critical] -->` can no longer promote an invisible, irrefutable blocker through the ungated channel - the footer-span version group admits only the version shape footerVersion validates — a span truncated inside the parens can no longer swallow the prose after it - the marker strips admit the full-width colon, closing the marker-only refusal's ASCII-only hole in bilingual drafts - entries containing a code-fence delimiter line are refused for redraft: the one-line collapse turns them into an unclosed fence that swallows the rest of the posted body - the identity fail-closed trigger narrows to what identity actually gates — critical markers on root comments — so a planted reply cannot convert a transient identity blip into a repeating refusal - the ledger's carried-id anchor reads through render-nothing residue left between the marker and the id, ending the silent renumbering - the marker-only contract covers trailing Cf/comment residue * fix(cli): close the round-12 Criticals — bounded footer version, drop-scoped blank cleanup, quote-preserving markers, gate-matched post leg * fix(cli): close seven review sanitation entrances from round-20 review (QwenLM#9027) - CR-aware line model: scanLines and rendersAsNothing split on CR/CRLF; entry lists normalize line endings on ingest, so a bare CR can no longer hide a forged footer, a hollow fence, or a fence delimiter from the refusal and emptiness gates (R20-1) - empty-login identity lookups fail closed like thrown ones in both pr-context and comment-status while a critical marker is posted (R20-2) - whitespace-only body-list entries fail the renders-nothing gates instead of vanishing before them; the dead raw sha check drops (R20-4) - drop-collapse never touches blank runs around an HTML-block content drop — quotation blanks render and survive (R20-6) - attribution-off posts refuse drafts whose post-strip shape leaves a fence open at the appended invisible marker (R20-9) - carriedClaimLine slices on the classifier's projection and both colon widths; presubmit reads carried ids off the attribution-off posted shape (R18-1) - duplicates disclosure routes through the attribution-off fixpoint chain like every other body leg (R15-1) --------- Co-authored-by: qwen-code-autofix[bot] <qwen-code-autofix[bot]@users.noreply.github.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>
5 participants
What this PR does
Adds three user settings for the
/reviewskill, resolved from operator-controlled settings scopes only (system defaults → user → system); a repository's.qwen/settings.jsoncannot set them, because repository-controlled content must not control the policy a review of that repository runs under. All three are toggleable in the/settingsdialog:review.attribution(boolean, defaulttrue) — controls the attribution footer naming the reviewing model and CLI version (e.g. — qwen3-coder via Qwen Code /review (v0.21.2)) that is appended to posted review bodies and inline comments. With attribution off the footer is not emitted and modelId is neither required nor validated (it has no consumer), while forged footer copies in drafted comments are still stripped. Presubmit's duplicate detection stays sighted without the footer: it also recognises the reviewing account's own top-level comments by author.review.effort(enumauto/low/medium/high, default"auto") — the default effort when--effortis not given."auto"keeps the built-in rule (high for PRs, medium for local changes); the configured value is normalised through the same case-insensitive validation as the flag, and invalid values fall back to the built-in default. An explicit--effortalways wins, and the existing forcings are untouched: an effective--commentstill forces high (posting requires a verified review) and an effective--fixstill floors at medium. The verdict gains aneffortSource: "configured"value and the skill doc describes it.review.comment(boolean, defaultfalse) — treats every PR/reviewas if--commentwas passed, so findings are published without typing the flag each time. This replaces the flag in the authorization gate, not the target binding: the write is still authorised only for the pull request the recorded invocation names (repo and host included for URL targets), and the success message states that the standing setting — not a typed flag — authorised the post. Both callers of the shared gate (submitandpublish-assets) resolve it the same way, so a posting run never publishes the review while being refused its evidence images.Defaults preserve the current behavior exactly; the IDE
settings.schema.jsonis regenerated and the user docs and bundled skill doc are updated (Step 7's gate and every comment-specified branch now key offcomment.effective).Why it's needed
Users who review many PRs currently re-type
--commentand--efforton every invocation, and there is no way to make the AI-attribution footer match what a given account or repository wants posted. These are per-user decisions — the settings put them where user preferences belong (operator-level config), instead of requiring flags on every run.Reviewer Test Plan
How to verify
cd packages/cli && npx vitest run src/commands/review src/config— all pass; the suites pin the configured-effort precedence chain (explicit > configured > built-in, with the--comment/--fixforcings still overriding), effort normalisation ("Low"→low, invalid → built-in default), the standing comment setting producing an effective comment on PR targets only, the authorization gate accepting the setting while refusing a submission bound to a different PR,publish-assetsagreeing withsubmiton what authorises a run, presubmit recognising attribution-off posts, and handler-level wiring for all three settings (parse-args, submit, compose-review).{"review": {"attribution": false}}in~/.qwen/settings.json, run/review <pr>against a scratch PR, and confirm neither the review body nor the inline comments carry thevia Qwen Code /reviewfooter; with the key absent the footer appears as before. Same forreview.effort: "low"(verdict showseffortSource: "configured") andreview.comment: true(posting authorises without--comment, message names the setting). A workspace.qwen/settings.jsoncarrying any of the three keys has no effect (operator scopes only).Evidence (Before & After)
N/A — no TUI change; the observable effect is the posted GitHub review content, covered by the unit tests above.
Tested on
Environment (optional)
Unit tests plus
npm run devfor handler wiring checks.Risk & Scope
review.comment: truemakes publishing the default for every PR review. That is the explicit intent of the switch (a standing authorization the user opts into); posting still binds to the PR named in the invocation, so it cannot act as a bearer token for other PRs. Presubmit's attribution-off recognition keys on the authenticated account's own top-level comments — attribution-off posts from other accounts remain undetectable (no footer, no authorship signal), which the setting's documentation notes.Linked Issues
None.
中文说明
这个 PR 做了什么
为
/reviewskill 增加三个用户配置项，仅从操作者可控的配置作用域解析（系统默认 → 用户 → 系统）；仓库的.qwen/settings.json无法设置它们——仓库可控的内容不应控制对该仓库所做 review 的策略。三项均可在/settings对话框中切换：review.attribution（boolean，默认true）——控制发布的 review body 与 inline comment 上标注模型与 CLI 版本的署名 footer（例如 — qwen3-coder via Qwen Code /review (v0.21.2)）。关闭后不输出 footer，modelId 也不再被要求或校验（已无消费者），同时仍会剥离草稿评论中伪造的署名副本。presubmit 的重复检测不依赖 footer 也保持可见：它还会按作者识别审查账号自己的顶层评论。review.effort（枚举auto/low/medium/high，默认"auto"）——未显式指定--effort时的默认 effort。"auto"保持内置规则（PR 用 high，本地改动用 medium）；配置值经过与 flag 相同的大小写不敏感校验归一化，非法值回落到内置默认。显式--effort始终最高优先级，且原有强制规则不变：生效的--comment仍强制 high（发布需要已验证的 review），生效的--fix仍下限 medium。裁决结果新增effortSource: "configured"取值，skill 文档同步说明。review.comment（boolean，默认false）——每个 PR/review视为带了--comment，无需每次输入 flag 即发布 findings。它替代的是授权门里的 flag，而不是目标绑定：写入仍然只授权给本次调用点名的 pull request（URL 目标还包括 repo 与 host），且授权成功的提示信息会说明是常驻配置而非用户输入的 flag 授权的发布。共享授权门的两个调用方（submit与publish-assets）以相同方式解析该配置，因此发布 review 的运行不会在证据图片上被拒绝。所有默认值完全保持现状行为；IDE 的
settings.schema.json已重新生成，用户文档与 bundled skill 文档同步更新（Step 7 授权门与所有"是否指定 comment"的分支统一以comment.effective为准）。为什么需要
频繁 review PR 的用户目前每次都要重复输入
--comment和--effort，且无法让 AI 署名 footer 匹配账号或仓库的发布预期。这些都是用户个人的决定——配置项把它们放到用户偏好该在的位置（操作者级配置），而不是要求每次运行时都带 flag。评审者测试计划
如何验证
cd packages/cli && npx vitest run src/commands/review src/config—— 全部通过；套件钉住了配置 effort 的优先级链（显式 > 配置 > 内置，且--comment/--fix的强制规则仍然生效）、effort 归一化（"Low"→low、非法值 → 内置默认）、常驻 comment 配置只在 PR 目标上生效、授权门接受该配置但向另一个 PR 提交时仍然拒绝、publish-assets与submit对授权依据保持一致、presubmit 能识别无署名的历史评论，以及三个配置项的 handler 级接线（parse-args、submit、compose-review）。~/.qwen/settings.json写入{"review": {"attribution": false}}，对着试验 PR 运行/review <pr>，确认 review body 与 inline comment 均不带via Qwen Code /reviewfooter；不配置时 footer 照常出现。review.effort: "low"（裁决显示effortSource: "configured"）与review.comment: true（无需--comment即可授权发布，提示信息点名该配置）同理。工作区.qwen/settings.json写任何这三个键都不生效（仅操作者作用域）。证据（改动前后）
N/A —— 无 TUI 变化；可观察的效果是发布到 GitHub 的 review 内容，已由上述单元测试覆盖。
测试环境
环境（可选）
单元测试 +
npm run dev验证 handler 接线。风险与范围
review.comment: true会让每个 PR review 默认发布。这正是该开关的明确意图（用户主动开启的常驻授权）；发布仍绑定调用点名的 PR，因此不会成为对其他 PR 的通行证。presubmit 对无署名评论的识别基于已认证账号自己的顶层评论——其他账号的无署名评论仍不可识别（无 footer、无作者信号），配置文档已注明。关联 Issue
无。