fix(review): recover bilingual register from the live PR when the plan omits the Han flag

[wenshao]

What

The posted review body renders bilingually — English, with the complete Chinese version collapsed beneath it — only when the plan compose-review reads carries prDescriptionHasHan: true. That flag is written in exactly one place (fetch-pr) and read in exactly one place, from a plan path the orchestrator supplies. Two gaps follow from that single source:

• a plan-diff plan never records the flag, and
• a run that improvises the pipeline can hand compose-review a plan that is not fetch-pr's report at all.

Either way the switch fails safe to English, and a Chinese-authored PR gets an English-only review. Observed on #7686: the four bot reviews composed off a proper plan were bilingual; the one composed off an improvised plan was English-only, even though the PR description carried the full 中文说明 block from the start.

Fix

When the flag is absent but the plan still names the PR (ownerRepo + prNumber), recover the signal from the live description with a single gh pr view, and test that recovered text for Han.

• Absent-flag path only. A recorded true/false is authoritative and spends no network, so every healthy fetch-pr review is byte-for-byte unchanged.
• The signal stays the CLI's own — the real PR body, which the caller cannot forge — so the recovery tightens the "the caller cannot toggle the register" property rather than loosening it.
• Fail-safe. Any failure of the fetch falls back to English: the language must never take the review down.

Tests

Five cases in compose-review.test.ts: a recovered body with Han folds in the Chinese; a recovered body without Han stays English; a recorded false never fetches; a plan with no PR identity never fetches; a throwing fetch falls back to English without crashing. 101/101 pass; eslint --max-warnings 0 clean; the change adds no typecheck errors.

中文说明

做了什么

只有当 compose-review 读到的 plan 带 prDescriptionHasHan: true 时,发布的 review 正文才双语渲染(英文在上,完整中文折叠其下)。这个标记只有 fetch-pr 一处会写,也只有一处会读——从编排器传入的 plan 路径读。单一来源带来两个缺口:

• plan-diff 产出的 plan 从不记录这个标记;
• 没有忠实执行流水线的运行,可能把一个根本不是 fetch-pr 报告的 plan 喂给 compose-review。

两种情况下开关都 fail-safe 到纯英文,于是中文作者的 PR 拿到全英文 review。在 #7686 上观察到:基于正常 plan 组出的 4 条 bot review 都是双语,唯独基于一次跑歪的 plan 组出的那条是纯英文——尽管该 PR 描述从创建起就带完整 中文说明 折叠块。

修复

当标记缺失、但 plan 仍带 PR 身份(ownerRepo + prNumber)时,用一次 gh pr view 从真实 PR 描述重算 Han 信号。

• 只走"缺字段"这条路:记录为 true/false 的都是权威值,不联网——因此每一次健康的 fetch-pr review 字节级不变。
• 信号仍是 CLI 自己拿的——真实 PR 正文,caller 无法伪造——所以这一兜底是强化而非削弱"caller 不能切换 register"这一属性。
• Fail-safe:取数一旦失败即回退英文,语言绝不能拖垮 review。

测试

compose-review.test.ts 新增 5 例:重算出中文→折叠中文;重算无中文→保持英文;记录为 false→绝不联网;plan 无 PR 身份→绝不联网;fetch 抛错→回退英文且不崩溃。101/101 通过;eslint --max-warnings 0 干净;改动不新增 typecheck 报错。

[qwen-code-ci-bot]

✅ Qwen Triage finished — CI landed green on 9148756 and the deferred approval was posted. finalize run

✅ Qwen Triage 已完成 —— 9148756 的 CI 全绿，延迟审批已提交。查看 finalize 运行

[qwen-code-ci-bot]

Re-running triage at the current head (9148756e), which has moved on considerably since the last pass — three autofix rounds landed, addressing the review feedback (round 2's missing test on the submit strip, plus the wording nit).

Template: headings still don't match the template verbatim (## What / ## Fix / ## Tests rather than ## What this PR does / ## Why it's needed / ## Reviewer Test Plan), but everything the template asks for is present — what, why, a test plan, and the full bilingual 中文说明 block. Treating the heading names as a nit, not a gate, same as before.

Problem: real and observed, not theoretical. On #7686 the four bot reviews composed off a proper fetch-pr plan were bilingual; the one composed off an improvised plan came out English-only, even though the PR description carried the 中文说明 block from the start. Root cause is concrete: prDescriptionHasHan is written only by fetch-pr (verified at fetch-pr.ts:379), so a plan-diff plan — which names the PR but never records the flag — fails safe to English.

Direction: aligned. This is the review tooling rendering Chinese-authored PRs correctly — squarely in scope, touching no auth/sandbox/model/telemetry/release surface. The only API change is additive and optional (a PrBodyFetcher type and an optional input field that production never sets), so there's no public-contract break.

Size: production logic is ~126 lines (compose-review.ts 105+/11−, submit.ts 6+/2−, roster.ts 1+/1−); tests are ~252 lines (compose-review.test.ts 222, submit.test.ts 30); the one core-path file, packages/core/src/skills/bundled/review/SKILL.md (5+/3−), is bundled-skill documentation prose, not executable logic. Title is fix, not refactor, so no core-refactor block applies, and the count is well under any threshold. The core-path consumer is the review-skill orchestrator that reads this doc and invokes compose-review — named and accounted for.

Approach: minimal and focused. The three-state check (recorded boolean → authoritative; absent + PR identity → recover from the live description; otherwise English) is the right shape, and the GHE --host routing for the new gh pr view matches what the sibling subcommands already do. No drive-by changes.

Moving on to code review. 🔍

中文说明

在最新 head（9148756e）上重新运行 triage——自上次以来 head 已大幅推进，经历了三轮 autofix，处理了评审反馈（第 2 轮在 submit 剥离处缺失的测试，以及措辞小问题）。

模板： 标题仍与模板不完全一致（## What / ## Fix / ## Tests，而非 ## What this PR does / ## Why it's needed / ## Reviewer Test Plan），但模板要求的内容都在——做了什么、为什么、测试方案，以及完整的双语 中文说明 折叠块。与之前一致，标题命名只算小问题，不作为拦截。

问题： 真实且已观测，不是理论问题。#7686 上,基于正常 fetch-pr plan 组出的 4 条 bot review 都是双语；唯独基于一次跑歪的 plan 组出的那条是纯英文——尽管该 PR 描述从创建起就带 中文说明 块。根因具体：prDescriptionHasHan 只有 fetch-pr 会写（已在 fetch-pr.ts:379 核实），所以 plan-diff plan（带 PR 身份但从不记录这个标记）会 fail-safe 到纯英文。

方向： 对齐。这是让 review 工具正确渲染中文作者的 PR——完全在范围内，不触及 auth/sandbox/model/telemetry/release。唯一的 API 改动是可选、向后兼容的新增（一个 PrBodyFetcher 类型和一个生产中从不设置的可选输入字段），不破坏公共契约。

规模： 生产逻辑约 126 行（compose-review.ts 105+/11−，submit.ts 6+/2−，roster.ts 1+/1−）；测试约 252 行（compose-review.test.ts 222，submit.test.ts 30）；唯一的核心路径文件 packages/core/src/skills/bundled/review/SKILL.md（5+/3−）是 bundled skill 的文档 prose，不是可执行逻辑。标题是 fix 而非 refactor，不触发核心重构拦截，行数也远低于任何阈值。核心路径的消费方是读取此文档并调用 compose-review 的 review skill 编排器——已点名。

方案： 最小且聚焦。三态判断（记录为 boolean → 权威；缺失 + 带 PR 身份 → 从真实描述重算；否则英文）正是合适的形态，而为新增 gh pr view 加的 GHE --host 路由与兄弟子命令既有做法一致。无顺手改动。

进入代码审查 🔍

— Qwen Code · qwen3.8-max-preview

_{Reviewed at 9148756e1aec62f1da04f3d1befc1d60929f00bc · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

Code review — no blockers.

My independent proposal for this problem is exactly what the PR does: when prDescriptionHasHan is absent (distinct from a recorded false) and the plan still names the PR, recover the signal from the live description with one gh pr view, test it for Han, and fail safe to English on any error. The implementation matches it, and I didn't find a simpler path it missed.

What I checked specifically at this head:

• The three-state logic is correct. A recorded true/false is authoritative and spends no network (every healthy fetch-pr review and every English-authored PR takes this path, byte-for-byte unchanged). Only the absent flag with a PR identity reaches the network. I confirmed the recorded flag and the recovery cannot disagree about the same text: fetch-pr.ts:379 computes the flag with the identical \p{Script=Han} regex on the same body field the recovery reads, and fetch-pr writes no report when its metadata fetch throws — so an authoritative false always means a real read that found no Han.
• The recovery is fail-safe at every edge. JSON.parse of a plan that isn't an object (null, string, number) yields undefined on the optional-chained reads rather than throwing; a non-JSON gh response throws inside fetchPrBodyViaGh and is caught by bilingualFromPlan's outer try. Language never takes the review down.
• The security property holds, and is tightened. Both production entry points (compose-review's handler and submit.ts) build their input from JSON.parse, which can never yield a function — so prBodyFetcher is structurally always undefined in production and the CLI's own gh pr view is used. Both also delete/destructure-strip the seam, so even a non-function value can't reach bilingualFromPlan, throw, and drop the fold through the fail-safe. The recovery reads the real PR body, which the caller cannot forge.
• The GHE routing is correct. fetchPrBodyViaGh calls the shared gh(...args) helper, which honours GH_HOST; the handler calls setGhHost(host) before composing, exactly like the sibling subcommands. The --host option and its test mirror comment-status/presubmit.
• Conventions are clean. ESM, unknown + type guards (no any), kebab-case files, colocated tests, reuse of the existing gh helper and isPositivePrNumber (newly exported from roster.ts rather than duplicated). Comments explain the why (the perf(core): Lazy-load first-use dependencies #7686 incident, the fail-safe rationale), not the what.

Two carry-over items from the round-3 review, both explicitly non-blocking and which I agree don't gate this merge: (1) the --host test asserts the post-handler setter state rather than the host at the moment of the gh call — a strict-superset replacement that captures the host inside a mocked gh would also pin the placement against a reorder; worth a follow-up. (2) On the publish path submit still trusts the plan's PR identity rather than its own already-validated --repo/--pr, so the description's "the caller cannot toggle the register" is true for the seam but not yet literally true for an authoritative-false plan a caller wrote — a hardening follow-up, not a regression. A small nit also stands: SKILL.md's Step 6 sentence ("needs no authorisation") predates the new optional gh pr view. None of these block.

Test evidence — the PR's own CI on the reviewed commit, fetched via the API (unattended run; PR code is never executed here). precheck-pr / precheck (lint + typecheck + build) is green. The ubuntu unit suite — which carries the bilingual-recovery cases, the production-reader gh pr view case, the submit strip case, and the --host case — is still in progress at review time; no check has failed. macOS/Windows unit and the integration jobs are skipped on this head (conditional matrix), not failed. Separately, the author's round-3 comment reports a mutation pass at this head (143/143 baseline; removing the submit strip, gutting fetchPrBodyViaGh, or flipping the boolean check each reddens) — that's the author's claim, and it's consistent with the test shapes in the diff.

Final CI results for 9148756 (auto-updated by the triage finalize job after CI completed):

Check	Conclusion
Classify PR	✅ success
Test (ubuntu-latest, Node 22.x)	✅ success
web-shell E2E Smoke (ubuntu-latest, Node 22.x)	✅ success

_{One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。}

Real-scenario testing: N/A. The change is inside the review-bot pipeline (bilingual body composition and its GHE host routing), not an interactive TUI surface a user drives — its behavior is exercised by the unit suite above, not by the CLI product. A maintainer can still trigger the isolated @qwen-code /tmux job if they want live confirmation.

中文说明

代码审查——无阻断问题。

我对这个问题的独立方案与 PR 完全一致：当 prDescriptionHasHan 缺失（区别于记录为 false）且 plan 仍带 PR 身份时，用一次 gh pr view 从真实描述重算信号，测 Han，任何失败都 fail-safe 到英文。实现与此一致，我也没找到它遗漏的更简路径。

我在本 head 上专门核对了：

• 三态逻辑正确。 记录为 true/false 的都是权威值，不联网（每一次健康的 fetch-pr review 和每一个英文作者的 PR 都走这条路，字节级不变）。只有缺失标记且带 PR 身份才会联网。我核实了记录标记与重算不会对同一段文字产生分歧：fetch-pr.ts:379 用完全相同的 \p{Script=Han} 正则、在重算所读的同一个 body 字段上计算该标记，且 fetch-pr 在其元数据取数抛错时不写报告——所以权威 false 总意味着一次真实读取且未发现 Han。
• 重算在每个边界都 fail-safe。 对非对象的 plan（null、字符串、数字）做 JSON.parse 时，可选链读取得到的是 undefined 而非抛错；gh 返回非 JSON 时会在 fetchPrBodyViaGh 内抛错，被 bilingualFromPlan 外层 try 捕获。语言绝不会拖垮 review。
• 安全属性成立，且被强化。 两个生产入口（compose-review 的 handler 和 submit.ts）都用 JSON.parse 构造输入，而 JSON 永远 parse 不出函数——所以 prBodyFetcher 在生产中结构性地恒为 undefined，用的是 CLI 自己的 gh pr view。两处还都 delete/解构剥离了这个缝，因此即便是非函数值也到不了 bilingualFromPlan、不会抛错经由 fail-safe 丢掉折叠。重算读的是真实 PR 正文，caller 无法伪造。
• GHE 路由正确。 fetchPrBodyViaGh 调用共享的 gh(...args) helper，它遵循 GH_HOST；handler 在组稿前调用 setGhHost(host)，与兄弟子命令完全相同。--host 选项及其测试与 comment-status/presubmit 一致。
• 约定干净。 ESM、unknown + 类型守卫（无 any）、kebab-case 文件名、同目录测试，并复用既有 gh helper 和 isPositivePrNumber（从 roster.ts 新导出而非复制）。注释解释为什么（perf(core): Lazy-load first-use dependencies #7686 事件、fail-safe 理由），而非做了什么。

第 3 轮评审留下两项，均明确不阻塞、我也同意不作为合并门槛：(1) --host 测试断言的是 handler 返回后的 setter 末态，而非 gh 调用发生那一刻的 host——一个在 mock 的 gh 内部捕获 host 的严格超集替换，还能钉住位置防止重排；值得后续跟进。(2) 在发布路径上 submit 仍信任 plan 的 PR 身份，而非它自己已校验的 --repo/--pr，所以描述里"caller 不能切换 register"对缝成立，对一个 caller 自写的权威 false plan 尚不字面成立——属加固性后续，不是回归。还有个小问题：SKILL.md Step 6 那句（"不需要授权"）早于新增的可选 gh pr view。这些都不阻塞。

测试证据——审查 commit 上 PR 自己的 CI，经 API 获取（无人值守运行，此处绝不执行 PR 代码）。precheck-pr / precheck（lint + typecheck + build）为绿。ubuntu 单测套件——含双语重算用例、生产读取器 gh pr view 用例、submit 剥离用例和 --host 用例——在审查时仍在运行；无任何 check 失败。macOS/Windows 单测和集成作业在此 head 上为 skipped（条件矩阵），并非失败。另外，作者第 3 条评论报告了本 head 上的变异测试（143/143 基线；移除 submit 剥离、掏空 fetchPrBodyViaGh、或翻转 boolean 判断各自变红）——那是作者的声明，与 diff 中的测试形态一致。

真实场景测试： N/A。改动位于 review bot 流水线内部（双语正文组稿及其 GHE host 路由），不是用户驱动的交互式 TUI 界面——其行为由上面的单测套件覆盖，而非 CLI 产品。maintainer 如想要现场确认，仍可触发隔离的 @qwen-code /tmux 作业。

— Qwen Code · qwen3.8-max-preview

_{Reviewed at 9148756e1aec62f1da04f3d1befc1d60929f00bc · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

Confidence: 5/5 — clean across every stage; would merge without hesitation.

This is a tight, well-motivated fix, and it has only gotten sharper since the last pass. The problem is real and observed (#7686 — an English-only review over a Chinese-authored PR), the root cause is nailed down (the Han flag has a single writer, fetch-pr, so any plan that isn't fetch-pr's report fails safe to English), and the fix is the minimal thing that solves it: distinguish a recorded false from an absent flag, and recover the signal from the live PR only in the absent case. The three autofix rounds since the last triage closed exactly the gap I'd have raised — the submit strip now has a load-bearing test — plus a wording nit.

Going back to my independent proposal: the PR matches it, and I didn't find a simpler path it missed. The part that holds up best under scrutiny is the security reasoning — keeping the language signal the CLI's own (a function seam that can't survive JSON.parse, recovery from the un-forgeable live body, both entry points stripping the seam) actually tightens the property the original code was protecting. The recorded flag and the recovery use the same regex on the same field, so they can't disagree. If I had to maintain this in six months, the comments would thank me: they record the incident, the three states, and the fail-safe rationale, not a narration of the code. This also matches the maintainer's read at this exact head (recommends merge) and the round-3 review's conclusion (comment, no blockers).

The two carry-over suggestions from round 3 are genuine but non-blocking, and I've recorded them in Stage 2 as follow-ups rather than gates: pin the --host test at the moment of the call, and have submit prefer its own validated --repo/--pr over the plan's identity so the "caller cannot toggle the register" claim is literally true on the publish path. Neither is a regression.

Housekeeping: the bot's stale CHANGES_REQUESTED from two commits back (431cb75) — the concern it raised was fixed and verified in the rounds since — is dismissed alongside this pass so the real state is visible.

Approval is deferred until CI lands green. The ubuntu unit suite (which carries the recovery, production-reader, submit-strip, and --host cases) is still in progress on 9148756e; precheck is already green and nothing has failed. I'm leaving the marker below so the finalize step posts the commit-pinned approval once every check on this SHA completes green — and withholds it if anything lands red or the head moves again.

中文说明

置信度：5/5——每一阶段都干净；毫不犹豫地合并。

这是一个紧凑、动机充分的修复，且自上次以来更加精炼。问题真实且已观测（#7686——中文作者的 PR 拿到纯英文 review），根因定位精确（Han 标记只有 fetch-pr 一个写入方，所以任何不是 fetch-pr 报告的 plan 都会 fail-safe 到英文），修复也是解决问题所需的最小改动：把记录为 false 与缺失标记区分开，只在缺失时从真实 PR 重算信号。自上次 triage 以来的三轮 autofix 恰好补上了我会提的缺口——submit 剥离现在有了一个承重测试——外加一个措辞小问题。

回到我的独立方案：PR 与之吻合，我也没找到它遗漏的更简路径。最经得起推敲的是安全推理——把语言信号保留为 CLI 自己的（一个无法穿过 JSON.parse 的函数缝，从不可伪造的真实正文重算，两个入口都剥离该缝）实际上强化了原代码所要保护的属性。记录标记与重算在同一字段上用同一正则，因此不会分歧。如果六个月后由我维护，这些注释会让我感激：它们记录了事件、三种状态和 fail-safe 理由，而不是对代码的复述。这也与 maintainer 在本 head 上的判断（推荐合并）和第 3 轮评审的结论（comment，无阻塞）一致。

第 3 轮的两项遗留建议是真实的但不阻塞，我已在 Stage 2 记为后续而非门槛：在调用发生那一刻钉住 --host 测试；让 submit 优先采用自己已校验的 --repo/--pr 而非 plan 的身份，使"caller 不能切换 register"这一说法在发布路径上字面成立。两者都不是回归。

事务性处理： bot 两个 commit 前（431cb75）那条过时的 CHANGES_REQUESTED——其所提问题已在后续轮次修复并核实——随本次一并 dismiss，让真实状态可见。

批准推迟到 CI 转绿。 ubuntu 单测套件（含重算、生产读取器、submit 剥离和 --host 用例）在 9148756e 上仍在运行；precheck 已绿，且无失败。我在下方留下标记，待此 SHA 上所有 check 转绿后由 finalize 步骤发出按 commit 钉住的批准——若有任何 check 转红或 head 再次移动，则不发。

— Qwen Code · qwen3.8-max-preview

_{Reviewed at 9148756e1aec62f1da04f3d1befc1d60929f00bc · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

Reviewed. Suggestions are inline.

中文说明

已审查。 建议见行内评论。

— qwen3.7-max via Qwen Code /review

[qwen-code-ci-bot]

LGTM, looks ready to ship — CI landed green after the review. ✅

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[qwen-code-ci-bot]

Reviewed — no blockers. Suggestions are inline.

中文说明

已审查——无阻断问题。 建议见行内评论。

— qwen3.7-max via Qwen Code /review

[doudouOUC]

No issues found. LGTM! ✅

— qwen3.7-max via Qwen Code /review

[wenshao]

@qwen-code /triage

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[qwen-code-ci-bot]

Triage re-run completed without a new review.

⚠️ The bot has no review of its own on 35d61b32c7b433dbd122106f8f9c4aac302c2649. If this re-run was meant to approve, it did not — an approval left by another account is a separate vote and does not count as the bot's own.

The stage comments above were updated with the latest result. View workflow run.

[qwen-code-ci-bot]

LGTM, looks ready to ship — CI landed green after the review. ✅

[qwen-code-ci-bot]

Reviewed. Suggestions are inline.

— qwen3.7-max via Qwen Code /review

[qwen-code-dev-bot]

🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下：

Review feedback addressed

Critical — submit doesn't strip prBodyFetcher (CHANGES_REQUESTED)

Decision: Implemented.

submit.ts:292 destructured only env from the model-written state, letting prBodyFetcher ride the rest-spread into bilingualFromPlan. A non-function value (null, "suppress") would be called, throw, and the fail-safe catch would drop the Chinese fold — the exact regression this PR closes, re-openable through the only write path in the skill.

Fix: const { env: _dropped, prBodyFetcher: _droppedFetcher, ...rest } = state; — one line, mirroring the handler boundary that already strips it.

Suggestion rc:3652252711 — no test for delete parsed.prBodyFetcher

Decision: Implemented.

Added a handler-level test analogous to the existing env stripping test: writes a state JSON carrying "prBodyFetcher": "suppress" and a plan with ownerRepo/prNumber but no prDescriptionHasHan, invokes the handler, and asserts the composed body still carries the Chinese fold. If the delete line were removed, "suppress" would throw and the fold would be absent — the test reddens.

Suggestion — fetchPrBodyViaGh is untested (mutation-verified)

Decision: Implemented.

All five existing recovery tests inject a fetcher, leaving the production reader unpinned. Added a test that mocks gh (via vi.mock('./lib/gh.js', …) with importOriginal to preserve getGhHost/setGhHost), returns a Han body, and asserts both the Chinese fold and that gh was called with exactly ['pr','view','7686','--repo','QwenLM/qwen-code','--json','body']. A wrong --json field, a dropped JSON.parse, or a body→bodyText slip now reddens.

Suggestion — SKILL.md inline-comment language vs the recovery

Decision: Implemented.

Updated the two places that describe the bilingual switch:

• Rule 2 (line 22): the parenthetical now notes the recovery path when the flag is absent but the plan names the PR.
• Step 7 bilingual-comments paragraph (line 835): the trigger now reads "the Step 1 fetch report says prDescriptionHasHan: true — or, when no fetch report exists (a plan-diff or improvised pipeline), the PR description itself is written in Chinese".

Suggestion — --host fallback to plan's recorded host

Decision: Declined.

Falling back to a plan-recorded host (setGhHost(host ?? planHost)) requires reading host from the plan JSON and, for the lightweight path, recording it in plan-diff's report — a schema change to a different command's output. That is a design decision about plan-diff's contract, out of scope for this PR. The current behaviour (404 → English fallback) is no worse than today; the stranger's-PR-body scenario requires a same-named repo on github.com, which is rare. Recording this as a follow-up candidate.

Nit — typeof === 'boolean' vs === undefined

Decision: Declined.

The reviewer notes junk values ("true", 1, null) now trigger a fetch where they previously meant English at zero cost. This is harmless in practice (fetch-pr always writes a boolean) and adding a guard for impossible input violates Simplicity First. Not worth the diff growth.

Verification

• npm run build — passed
• npm run typecheck — passed
• npm run lint — passed
• vitest run src/commands/review/compose-review.test.ts — 104 passed (was 102; +2 new tests)
• vitest run src/commands/review/submit.test.ts — 38 passed
• vitest run src/commands/review/ — 901 passed (was 899; +2 new tests)

中文说明

审查反馈处理

Critical — submit 没有剥掉 prBodyFetcher（CHANGES_REQUESTED）

决定：已实现。

submit.ts:292 只从模型写入的 state 中解构了 env，让 prBodyFetcher 随 rest 展开进入 bilingualFromPlan。非函数值（null、"suppress"）会被调用、抛错，fail-safe catch 会丢掉中文折叠——正是本 PR 要关掉的那个 bug，通过 skill 唯一的写路径被重新打开。

修复：const { env: _dropped, prBodyFetcher: _droppedFetcher, ...rest } = state;——一行，与已经剥掉它的 handler 边界保持一致。

建议 rc:3652252711 — delete parsed.prBodyFetcher 没有测试

决定：已实现。

添加了一个类似于现有 env 剥离测试的处理层测试：写入一个携带 "prBodyFetcher": "suppress" 的 state JSON，其 plan 包含 ownerRepo/prNumber 但没有 prDescriptionHasHan，调用处理函数，断言组合后的正文仍包含中文折叠。如果 delete 那行被移除，"suppress" 会抛错、折叠消失——测试变红。

建议 — fetchPrBodyViaGh 没有测试（已用变异验证）

决定：已实现。

现有 5 个兜底测试全部注入 fetcher，生产读取器没有被钉住。添加了一个 mock gh 的测试（通过 vi.mock('./lib/gh.js', …) 配合 importOriginal 保留 getGhHost/setGhHost），返回中文 body，断言中文折叠出现并且 gh 收到的参数恰好是 ['pr','view','7686','--repo','QwenLM/qwen-code','--json','body']。--json 字段写错、漏掉 JSON.parse、或 body 打成 bodyText，现在都会变红。

建议 — SKILL.md 行内评论语言与兜底路径不一致

决定：已实现。

更新了描述双语开关的两处：

• 规则 2（第 22 行）：括号内现在注明了当标记缺失但 plan 仍带 PR 身份时的兜底路径。
• Step 7 双语评论段落（第 835 行）：触发条件现在读作「Step 1 的 fetch 报告说 prDescriptionHasHan: true——或者，当没有 fetch 报告时（plan-diff 或跑歪的流水线），PR 描述本身是中文」。

建议 — --host 回退到 plan 记录的 host

决定：拒绝。

回退到 plan 记录的 host（setGhHost(host ?? planHost)）需要从 plan JSON 读取 host，并且对 lightweight 路径需要在 plan-diff 的报告里记录它——这是对另一个命令输出的 schema 变更，属于 plan-diff 契约的设计决策，超出本 PR 范围。当前行为（404 → 英文回退）不比现状差；陌生人的 PR body 场景需要 github.com 上存在同名仓库，极为罕见。记录为后续候选。

Nit — typeof === 'boolean' vs === undefined

决定：拒绝。

审查者指出脏值（"true"、1、null）现在会触发一次 fetch，而原来是零成本英文。实践中无害（fetch-pr 一定写布尔），为不可能的输入添加守卫违反 Simplicity First。不值得增加 diff。

验证

• npm run build — 通过
• npm run typecheck — 通过
• npm run lint — 通过
• vitest run src/commands/review/compose-review.test.ts — 104 通过（原 102；新增 2 个测试）
• vitest run src/commands/review/submit.test.ts — 38 通过
• vitest run src/commands/review/ — 901 通过（原 899；新增 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/模型 qwen3.8-max-preview

[wenshao]

Review — round 2 @ 082b6f3a

Both findings from the previous round are genuinely fixed, and I re-verified each by mutation rather than by reading:

Mutation applied to the PR head	PR's 142 tests
fetchPrBodyViaGh body replaced with return ''	🔴 2 fail (was green last round)
setGhHost(host) removed from the compose-review handler	🔴 1 fail
delete parsed.prBodyFetcher removed from the handler	🔴 1 fail
prBodyFetcher removed from submit.ts's destructure	🟢 142 pass ⟵

The first three close last round's "seam-only tests survive gutting the real reader" gap. The fourth is the one open item.

I also drove the real posting path (runSubmit → compose → composeReview → bilingualFromPlan) with gh mocked to return a Chinese body, and confirmed the recovery works end-to-end: gh pr view <n> --repo <o/r> --json body is issued and the composed body carries <details><summary>中文说明</summary>. The feature does what the description says on the path that matters.

prettier, eslint --max-warnings 0, and tsc --noEmit are clean on all changed files. plan-diff does write ownerRepo/prNumber (plan-diff.ts:87), so the motivating gap named in the description is really reachable — the recovery is not aimed at a vacuous case. runSubmit calls setGhHost(args.host) as its first statement, so the recovery routes correctly on the posting path too; the new --host on compose-review correctly covers the standalone Step 6 invocation.

---

Blocker — the submit-boundary strip is unpinned

packages/cli/src/commands/review/submit.ts:295

submit is the only boundary that posts, and its strip is the one with no test. Deleting prBodyFetcher: _droppedFetcher from the destructure leaves all 142 tests green. The sibling strip in compose-review's own handler — same guard, non-posting boundary — did get a test in this PR, so the asymmetry looks unintentional rather than deliberate.

Failure scenario: someone refactors that destructure (a pick()/omit() helper, or a cleanup of the void _droppedFetcher lint dance) and drops the field. CI stays green. A state JSON carrying "prBodyFetcher": null then reaches bilingualFromPlan, is called, throws, and the fail-safe catch returns English — a Chinese-authored PR gets an English-only review, which is exactly the regression this PR exists to close, through the door that publishes.

A discriminating test (fails with the strip removed, passes with it):

it('strips a caller-supplied prBodyFetcher — a state JSON cannot suppress the Chinese fold', () => {
  // submit.test.ts's module mock currently stubs `gh` inline; hoist it like
  // `ghMock` so this test can point the recovery at a Chinese body.
  ghViewMock.mockReturnValue('{"body":"这个 PR 修复了双语渲染。"}');
  const planPath = file('plan.json', {
    chunks: [],
    ownerRepo: 'QwenLM/qwen-code',
    prNumber: '6771',
  });
  runSubmit(
    args({
      userAuthorized: true,
      review: file('r.json', {
        commit_id: 'abc123',
        comments: [],
        state: { modelId: 'm', planPath, prBodyFetcher: null },
      }),
    }),
  );
  const body = (JSON.parse(ghMock.mock.calls[0][0]) as { body: string }).body;
  // Without the strip, `null` is invoked, throws, and the fail-safe drops the fold.
  expect(body).toContain('中文说明');
});

I ran exactly this shape as a throwaway probe: it passes at 082b6f3a and fails with the strip removed, while the PR's own suites stay green in both states.

---

Suggestion — the register is still the caller's on the posting path

compose-review.ts:1121-1128, submit.ts:295

The description's central claim is that the recovered signal "stays the CLI's own — the real PR body, which the caller cannot forge". compose-review.ts:300 states the underlying assumption outright: "What it supplies is planPath — a path, whose contents the CLI wrote." That is an assumption, not an enforced property. submit never checks that the file at planPath is one the CLI wrote, nor that the plan's PR identity matches its own --repo/--pr, which it has already validated (isRepo, isDiffLine, plus the authorisation cross-check at submit.ts:246).

Two behaviours I confirmed by driving runSubmit against a caller-written plan file, on a PR whose live description is Chinese:

• A plan carrying {"prDescriptionHasHan": false} composes an English body and spends zero network — the recorded-value short-circuit hands the caller the register directly, which is a cheaper lever than prBodyFetcher ever was.
• A plan carrying {"ownerRepo": "attacker/elsewhere", "prNumber": "1"} sends the language probe to gh pr view 1 --repo attacker/elsewhere, while submit was authorised for QwenLM/qwen-code#6771.

Neither is a regression — the flag was already read from a caller-named path — but the recovery is presented as tightening this property, and on the posting path there is a clean way to actually make it true: thread submit's already-validated args.repo/args.pr into composeReview and prefer them over the plan's fields when the recovery fires. That also removes the second-order point that plan.ownerRepo reaches gh --repo with none of the isRepo() shape checking submit applies to its own --repo (benign today — execFileSync, no shell, and a failed fetch falls back to English — but it is the guard this file already established for the same value).

---

Nits

• SKILL.md:22 now reads "the bilingual rendering for Chinese PRs is deterministic, keyed on prDescriptionHasHan (or … recovered from the live description …)". Once the register can come from a live fetch it is no longer strictly deterministic — a description edited between fetch-pr and Step 6 changes it. Worth softening to "deterministic when the plan records the flag".
• Same sentence nests a parenthetical inside a parenthetical across ~60 words; splitting it into two sentences would read better in a rule block that opens with "most commonly violated — read these first".
• The recovery now fires twice per review in the absent-flag path (Step 6 compose-review, then Step 7 submit), each with gh's 2-retry/3s-6s synchronous backoff. Harmless, but a GitHub hiccup adds up to ~9s of blocking before a post. Not worth changing; noting it so it is not a surprise.

---

Verdict: the feature is correct and well-verified where it is tested. Add the submit-boundary regression test and this is ready; the register-forging point is a follow-up if you'd rather keep this PR tight.

中文说明

复审 — 第 2 轮 @ 082b6f3a

上一轮的两个问题都已真正修复，我用变异测试而非阅读逐条复验：

对 PR HEAD 施加的变异	PR 自带的 142 个测试
fetchPrBodyViaGh 函数体替换为 return ''	🔴 2 个失败（上一轮为全绿）
从 compose-review handler 移除 setGhHost(host)	🔴 1 个失败
从 handler 移除 delete parsed.prBodyFetcher	🔴 1 个失败
从 submit.ts 解构中移除 prBodyFetcher	🟢 142 全部通过 ⟵

前三项补上了上一轮"只测 seam、掏空真实读取器仍全绿"的缺口。第四项是唯一遗留项。

我还驱动了真实发布路径（runSubmit → compose → composeReview → bilingualFromPlan），把 gh mock 成返回中文正文，确认兜底端到端有效：确实发出了 gh pr view <n> --repo <o/r> --json body，组出的正文带 <details><summary>中文说明</summary>。在真正要紧的那条路径上，功能与描述一致。

所有改动文件的 prettier、eslint --max-warnings 0、tsc --noEmit 均干净。plan-diff 确实会写 ownerRepo/prNumber（plan-diff.ts:87），所以描述里说的缺口是真实可达的——兜底不是打在一个空案例上。runSubmit 的第一条语句就是 setGhHost(args.host)，因此发布路径上的路由也是对的；新加到 compose-review 的 --host 正确覆盖了 Step 6 的独立调用。

---

阻塞项 — submit 边界的剥离没有被测试钉住

packages/cli/src/commands/review/submit.ts:295

submit 是唯一会发布的边界，而它的剥离恰恰没有测试。把解构里的 prBodyFetcher: _droppedFetcher 删掉，142 个测试仍然全绿。本 PR 给同一守卫在 compose-review handler（不发布的那个边界）上补了测试，所以这个不对称看起来是疏漏而非有意为之。

失效场景：有人重构这行解构（换成 pick()/omit() 辅助函数，或"清理"掉 void _droppedFetcher 这个 lint 应付写法）而丢掉该字段，CI 依然全绿。此后带 "prBodyFetcher": null 的 state JSON 会抵达 bilingualFromPlan，被当函数调用、抛错，fail-safe 的 catch 回退英文——正是本 PR 要关闭的那个回归，而且是从会公开发布的那扇门进来的。

一个有判别力的测试（移除剥离则红，保留则绿）见上方英文部分的代码块。我已按这个形状跑过一次性探针：在 082b6f3a 通过，移除剥离后失败，而两种状态下 PR 自带的测试套件都保持全绿。

---

建议 — 发布路径上，语言 register 仍然由 caller 说了算

compose-review.ts:1121-1128、submit.ts:295

描述的核心论断是：重算出的信号"仍是 CLI 自己拿的——真实 PR 正文，caller 无法伪造"。而 compose-review.ts:300 把底层假设写得很直白："caller 提供的是 planPath——一个路径，其内容是 CLI 写的。" 这是假设，不是被强制执行的属性。submit 既不校验 planPath 指向的文件是否真由 CLI 写出，也不校验 plan 里的 PR 身份是否与它自己已经校验过的 --repo/--pr 一致（isRepo、isDiffLine，外加 submit.ts:246 的授权交叉校验）。

我用一个 caller 自己写的 plan 文件驱动 runSubmit（目标 PR 的线上描述为中文），确认了两点行为：

• plan 带 {"prDescriptionHasHan": false} → 组出英文正文，且零网络请求。已记录值的短路直接把 register 交到了 caller 手上，比 prBodyFetcher 这条路更省事。
• plan 带 {"ownerRepo": "attacker/elsewhere", "prNumber": "1"} → 语言探测发往 gh pr view 1 --repo attacker/elsewhere，而 submit 被授权的是 QwenLM/qwen-code#6771。

两者都不是本 PR 引入的回归——该标记本来就是从 caller 指定的路径读的。但既然兜底被表述为"强化"这一属性，发布路径上有一个干净的做法可以让它真正成立：把 submit 已校验的 args.repo/args.pr 传入 composeReview，在兜底触发时优先于 plan 里的字段。这同时也解决了次级问题：plan.ownerRepo 直接进入 gh --repo，却没有经过 submit 对自己 --repo 所用的 isRepo() 形状校验（当前无害——execFileSync 不过 shell，取数失败即回退英文——但这正是本文件已为同类值确立的守卫）。

---

小问题

• SKILL.md:22 现在写作"中文 PR 的双语渲染是确定性的，以 prDescriptionHasHan 为准（或……从线上描述重算……）"。一旦 register 可能来自线上取数，就不再严格确定——在 fetch-pr 与 Step 6 之间编辑描述会改变结果。建议弱化为"当 plan 记录了该标记时是确定性的"。
• 同一句在约 60 词内嵌套了括号中的括号；在一个以"最常被违反——请先读这些"开头的规则块里，拆成两句会更好读。
• 缺标记路径下，兜底现在每次 review 触发两次（Step 6 compose-review，Step 7 submit），每次都带 gh 的 2 次重试 / 3s-6s 同步退避。无害，但 GitHub 抖动时发布前最多会多阻塞约 9s。不建议改动，只是记录下来以免意外。

---

结论： 功能本身正确，且在有测试覆盖的地方验证充分。补上 submit 边界的回归测试即可合并；register 可被伪造那一项，如果想保持本 PR 聚焦，可作为后续处理。

[gwinthis]

Follow-up verification (new head 082b6f3)

Increment verified: submit.ts now strips prBodyFetcher at its own compose boundary too — the same defense my scenario F exercised on the compose-review CLI handler now also covers the sibling entry point, so a state JSON smuggling the seam can no longer reach bilingualFromPlan via submit either (a non-function value there would have thrown and dropped the fold through the fail-safe). Same stripping discipline as env, applied at both boundaries; fetchPrBodyViaGh and the handler stripping are now pinned by tests.

compose-review.test.ts at the new head: 104/104 passed locally (up from 102 — the two new boundary pins included). Previous LGTM stands, now with the injection surface closed at both entrances.

中文摘要

增量核实：submit.ts 在自己的 compose 边界同样剥离 prBodyFetcher——我此前场景 F 只验证了 compose-review CLI 入口，如今兄弟入口 submit 也套用同一剥离纪律（与 env 一致），非函数值经该路径压制中文折叠的间接注入被封死；fetchPrBodyViaGh 与 handler 剥离均有测试钉住。新 head 本地 104/104 通过。维持 LGTM。

— independent review loop, incremental re-verification

[qwen-code-ci-bot]

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

中文说明

⚠️ 已从批准降级为评论：CI still running。 已审查。

— qwen3.7-max via Qwen Code /review

[qwen-code-dev-bot]

🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 2/100 轮）。改动内容与我反驳保留之处如下：

Review feedback addressed — round 3

Blocker — submit-boundary strip is unpinned → Fixed

Added a discriminating test in submit.test.ts that fails when the prBodyFetcher strip is removed from submit.ts's destructure and passes with it in place. The test drives runSubmit with a state JSON carrying prBodyFetcher: null and a plan that names the PR but omits prDescriptionHasHan, so the bilingual recovery fires through the default fetchPrBodyViaGh. Without the strip, null is invoked as a function, throws, and the fail-safe catch drops the Chinese fold — the test asserts the fold is present (中文说明), proving the strip held.

To support this, the gh mock in submit.test.ts was hoisted (ghViewMock) so the test can point the recovery at a Chinese body, matching the pattern already used in compose-review.test.ts.

Mutation-verified: removing prBodyFetcher: _droppedFetcher from the destructure reddens exactly this test (1 failed | 38 passed); restoring it returns to 39 passed.

Suggestion — register is still the caller's on the posting path → Declined (follow-up)

The reviewer's analysis is correct: plan.ownerRepo/plan.prNumber reach the recovery gh call without being cross-checked against submit's already-validated args.repo/args.pr. This is not a regression (the flag was always read from a caller-named path), and the reviewer explicitly suggested deferring: "the register-forging point is a follow-up if you'd rather keep this PR tight." Threading validated identity into composeReview changes its signature and every call site — a larger change best handled in a focused follow-up PR.

Nit — SKILL.md "deterministic" wording → Fixed

Softened "is deterministic, keyed on prDescriptionHasHan" to "is deterministic when the plan records the flag (prDescriptionHasHan)" — accurate now that the register can also come from a live fetch.

Nit — SKILL.md nested parenthetical → Fixed

Split the parenthetical-inside-a-parenthetical into two sentences: the flag path and the recovery path are now separate clauses, readable in a rule block that opens with "most commonly violated — read these first".

Nit — double recovery fire (~9s blocking) → Declined (no change needed)

The reviewer noted this is "harmless" and "not worth changing." Agreed — the synchronous backoff is bounded and the double-fire only occurs on the absent-flag path, which is the uncommon case.

Verification

• npm run build — passed
• npm run typecheck — passed
• npm run lint — passed
• npx prettier --check (changed files) — passed
• vitest run src/commands/review/submit.test.ts — 39 passed (38 existing + 1 new)
• vitest run src/commands/review/compose-review.test.ts — 104 passed
• Mutation test: removing the strip from submit.ts → 1 failed | 38 passed (confirms the new test is discriminating)

中文说明

复审反馈处理 — 第 3 轮

阻塞项 — submit 边界的剥离没有被测试钉住 → 已修复

在 submit.test.ts 中新增了一个有判别力的测试：移除 submit.ts 解构中的 prBodyFetcher 剥离时测试失败，保留时通过。测试驱动 runSubmit，state JSON 携带 prBodyFetcher: null，plan 文件有 PR 身份但缺少 prDescriptionHasHan，因此双语兜底会通过默认的 fetchPrBodyViaGh 触发。若剥离被移除，null 会被当函数调用、抛错，fail-safe 的 catch 会丢掉中文折叠——测试断言折叠存在（中文说明），证明剥离生效。

为支持此测试，submit.test.ts 中的 gh mock 被提升为 ghViewMock（与 compose-review.test.ts 已有的模式一致），使测试可以将兜底指向中文正文。

变异验证：从解构中移除 prBodyFetcher: _droppedFetcher 后恰好只有此测试变红（1 失败 | 38 通过）；恢复后回到 39 全部通过。

建议 — 发布路径上语言 register 仍由 caller 说了算 → 拒绝（后续处理）

审阅者的分析正确：plan.ownerRepo/plan.prNumber 进入兜底 gh 调用时没有与 submit 已校验的 args.repo/args.pr 交叉校验。这不是回归（该标记本来就是从 caller 指定的路径读的），审阅者也明确建议延后："如果想保持本 PR 聚焦，register 可被伪造那一项可作为后续处理。"将已校验的身份传入 composeReview 会改变其签名及所有调用点——更大的改动最好在专门的后续 PR 中处理。

小问题 — SKILL.md "deterministic" 措辞 → 已修复

将 "is deterministic, keyed on prDescriptionHasHan" 弱化为 "is deterministic when the plan records the flag (prDescriptionHasHan)"——在 register 也可能来自线上取数的现在，这样更准确。

小问题 — SKILL.md 嵌套括号 → 已修复

将括号中的括号拆成两个句子：标记路径和兜底路径现在是独立的子句，在以"最常被违反——请先读这些"开头的规则块中更易阅读。

小问题 — 兜底双次触发（约 9 秒阻塞）→ 拒绝（无需改动）

审阅者指出这"无害"且"不建议改动"。同意——同步退避是有界的，双次触发只在缺标记路径上发生，而那是少见情况。

验证

• npm run build — 通过
• npm run typecheck — 通过
• npm run lint — 通过
• npx prettier --check（改动文件）— 通过
• vitest run src/commands/review/submit.test.ts — 39 通过（38 个已有 + 1 个新增）
• vitest run src/commands/review/compose-review.test.ts — 104 通过
• 变异测试：从 submit.ts 移除剥离 → 1 失败 | 38 通过（确认新测试有判别力）

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/模型 qwen3.8-max-preview

[wenshao]

Review — round 3 @ 9148756e

The head moved while I was verifying (082b6f3a → 9148756e), so this is a review of the new head. Round 2's blocker is fixed, and I confirmed the fix by mutation rather than by reading it. The wording nit was taken too, in the exact form suggested.

Baseline in an isolated worktree at 9148756e: 143/143 (104 compose-review + 39 submit), eslint --max-warnings 0 clean on all five changed TS files, prettier --check clean, Test (ubuntu 22.x) green on the PR.

Mutation applied to 9148756e	PR's 143 tests
prBodyFetcher removed from submit.ts:295 destructure	🔴 1 fail ⟵ was 🟢 142 pass last round
recovered body changed to English (is the new test load-bearing?)	🔴 1 fail
fetchPrBodyViaGh body replaced with return ''	🔴 3 fail
typeof plan?.prDescriptionHasHan === 'boolean' → === true	🔴 1 fail
try/catch around the recovery fetch removed	🔴 1 fail
--json body → --json bodyText	🔴 1 fail
setGhHost(host) moved below the composeReview(…) call in the handler	🟢 143 pass ⟵ open
isPositivePrNumber(plan?.prNumber) → plan?.prNumber !== undefined	🟢 143 pass

The new submit.test.ts:711 test is the real thing, not a test-shaped comment: removing the strip reddens it, and it also reddens when the recovered body is switched to English or the production reader is gutted — so it genuinely drives runSubmit → compose → bilingualFromPlan → gh pr view and reads the fold out of the posted payload. That was the one guard in this PR that CI could not see; it can now.

Four properties the design depends on, checked this round and clean:

• A recorded false really is a read that happened. The new doc comment claims "fetch-pr fetched the body and found no Han". If fetch-pr could record false for a body it failed to read, the authoritative-false short-circuit would make the recovery unreachable in exactly the case it exists for. It cannot: the metadata fetch throws and no report is written (fetch-pr.ts:206-215), and the flag is computed with the same \p{Script=Han} regex on the same body field the recovery reads (fetch-pr.ts:375), so the two paths cannot disagree about the same text.
• No new network on local reviews. plan-diff writes ownerRepo/prNumber only when the caller resolved a PR (plan-diff.ts:86-88), so a /code-review plan hits !ownerRepo || !prNumber and returns before any fetch.
• No new real gh spawns in CI. compose-review.test.ts and submit.test.ts are the only suites that build a plan carrying ownerRepo, and both mock ./lib/gh.js; nothing else can reach fetchPrBodyViaGh unmocked.
• ensureAuthenticated is correctly not called here. Adding it would turn a command documented as needing no authorisation into one that fails hard; catch → English is the right shape for a language signal.

---

Suggestion — the --host test asserts the setter, not the routing

packages/cli/src/commands/review/compose-review.test.ts:832, compose-review.ts:1232

Commit 4's message says the change is "covered by a test that drives the handler with --host and asserts the routing took". What the test asserts is getGhHost() === 'github.example.com' after the handler returns — the end state of module env. Two consequences:

1. Its input is { modelId } with no planPath, so bilingualFromPlan returns at if (!planPath) and no gh call happens in that test at all. The one code path the option exists for is never exercised by it.
2. Moving setGhHost(host) from the top of the handler to below the composeReview(…) call leaves 143/143 green — while on a GHE review whose plan lacks the flag, the recovery's gh pr view then runs with no GH_HOST, hits github.com, fails, and composes the English-only body that disagrees with what submit posts. That is precisely the bug commit 4 was written to prevent, shipping with CI green.

A reorder is not a contrived mutant here: cleanup.ts:304,369 already establishes a save-and-restore pattern around setGhHost in this same directory, and applying that hygiene to this handler is a natural follow-up in which the placement is easy to get wrong.

Capturing the host at the moment of the call closes both. I ran this at 9148756e: green at head, red under the drop-mutant and red under the reorder-mutant.

it('routes the recovery fetch at the PR host — GH_HOST is set when gh pr view runs', () => {
  const dir = mkdtempSync(join(tmpdir(), 'compose-host-'));
  const planPath = join(dir, 'plan.json');
  // PR identity, no `prDescriptionHasHan` — the recovery fires.
  writeFileSync(
    planPath,
    JSON.stringify({ chunks: [], ownerRepo: 'QwenLM/qwen-code', prNumber: '7686' }),
  );
  const inputPath = join(dir, 'in.json');
  writeFileSync(inputPath, JSON.stringify({ planPath, modelId: MODEL }), 'utf8');
  const commentsPath = join(dir, 'comments.json');
  writeFileSync(commentsPath, '[]', 'utf8');

  let hostAtCall: string | undefined = 'NEVER CALLED';
  ghMock.mockImplementation(() => {
    hostAtCall = getGhHost();
    return '{"body":"这个 PR 修复了双语渲染。"}';
  });

  setGhHost(undefined);
  try {
    (composeReviewCommand.handler as (argv: unknown) => void)({
      input: inputPath,
      comments: commentsPath,
      host: 'github.example.com',
    });
  } finally {
    setGhHost(undefined);
  }
  expect(ghMock).toHaveBeenCalled(); // the path under test was actually taken
  expect(hostAtCall).toBe('github.example.com');
});

It is a strict superset of the current test's coverage, so it can replace that test rather than sit beside it.

---

Suggestion (carried) — the register is still the caller's on the posting path

compose-review.ts:1118-1131, submit.ts:295 vs submit.ts:438

Restating compactly, since nothing in this head changes it. submit never checks that the file at planPath is one the CLI wrote, nor that the plan's PR identity matches the --repo/--pr it has already validated. Driving runSubmit against a caller-written plan: {"prDescriptionHasHan": false} composes English with zero network — the authoritative-false short-circuit is a cheaper lever than prBodyFetcher ever was — and {"ownerRepo": "attacker/elsewhere", "prNumber": "1"} sends the language probe to a repo submit was not authorised for. Neither is a regression, but the description bills the recovery as tightening "the caller cannot toggle the register", and there is a clean way to make that true on the posting path: thread submit's validated args.repo/args.pr into composeReview and prefer them when the recovery fires. That also closes the second-order point that plan.ownerRepo reaches gh --repo with none of the isRepo() shape checking submit applies to its own --repo at line 438 — benign today (execFileSync, no shell, failed fetch → English), but it is the guard this file already established for the same value.

---

Nits

• SKILL.md:654 still characterises Step 6 as "It writes nothing, posts nothing, and needs no authorisation, so run it on every high-effort review." On the absent-flag path it now issues a gh pr view. This PR updated rule 2 and the Step 7 paragraph and added a --host comment to the Step 6 command block, but left the one sentence that tells the model what the command does — the same class of staleness as the rule-2 wording just fixed.
• Commit 2 exists so one PR-number rule has one definition; the call site's use of it is unpinned — swapping isPositivePrNumber(plan?.prNumber) for plan?.prNumber !== undefined keeps all 143 green. The consequence is only a doomed fetch that falls back to English, so this is a note, not a request.
• Process: the CHANGES_REQUESTED from 10:10 was fixed two commits ago but never dismissed, which is what mergeStateStatus: BLOCKED currently is. Worth clearing so the real state is visible.

---

Verdict: Comment — no blockers. The feature is correct, the motivating gap is real and reachable, and every guard it adds is now pinned by a test that fails when the guard is removed — including the posting boundary, which is what round 2 was waiting on. The two open items are a strictly-better replacement for one existing test and a follow-up that would make the description's "caller cannot toggle the register" claim literally true; neither needs to block this merge.

中文说明

复审 — 第 3 轮 @ 9148756e

我验证期间 HEAD 发生了移动（082b6f3a → 9148756e），因此本轮针对新 HEAD 复审。**第 2 轮的阻塞项已修复，且我用变异测试而非阅读来确认这一点。**措辞 nit 也已按建议的原样采纳。

在隔离 worktree 中于 9148756e 的基线：143/143（104 个 compose-review + 39 个 submit），五个改动的 TS 文件 eslint --max-warnings 0 干净，prettier --check 干净，PR 上 Test (ubuntu 22.x) 为绿。

对 9148756e 施加的变异	PR 自带的 143 个测试
从 submit.ts:295 解构中移除 prBodyFetcher	🔴 1 失败 ⟵ 上一轮为 🟢 142 全通过
把重算到的 body 改成英文（新测试是否真正承重？）	🔴 1 失败
fetchPrBodyViaGh 函数体替换为 return ''	🔴 3 失败
typeof plan?.prDescriptionHasHan === 'boolean' → === true	🔴 1 失败
移除兜底 fetch 外层的 try/catch	🔴 1 失败
--json body → --json bodyText	🔴 1 失败
把 setGhHost(host) 移到 handler 中 composeReview(…) 调用之后	🟢 143 全通过 ⟵ 遗留
isPositivePrNumber(plan?.prNumber) → plan?.prNumber !== undefined	🟢 143 全通过

新增的 submit.test.ts:711 是真货，而不是"长得像测试的注释"：移除剥离它会变红；把重算到的 body 换成英文、或掏空生产读取器，它同样变红——说明它确实驱动了 runSubmit → compose → bilingualFromPlan → gh pr view，并从实际发布的 payload 里读出折叠块。这是本 PR 中唯一 CI 看不见的守卫，现在能看见了。

本轮复核、结果干净、且设计确实依赖的四条性质：

• 记录为 false 确实代表「读到了」。 新增注释声称「fetch-pr 取过 body 并且没有汉字」。如果 fetch-pr 会为一个读取失败的 body 记录 false，那么「false 具权威性」的短路会让兜底在它恰恰为之存在的场景中永远不可达。它不会：元数据取数失败会抛错且不写报告（fetch-pr.ts:206-215），且该标记用的是与兜底完全相同的 \p{Script=Han} 正则、作用于同一个 body 字段（fetch-pr.ts:375），因此两条路径对同一段文本不可能给出不同答案。
• 本地评审不产生新的网络请求。 只有当调用方解析出了 PR 时，plan-diff 才写 ownerRepo/prNumber（plan-diff.ts:86-88），因此 /code-review 的 plan 会在 !ownerRepo || !prNumber 处返回，不发起任何取数。
• CI 中不会新增真实 gh 子进程。 只有 compose-review.test.ts 与 submit.test.ts 会构造带 ownerRepo 的 plan，二者都 mock 了 ./lib/gh.js；其他任何地方都无法在未 mock 的情况下走到 fetchPrBodyViaGh。
• 这里不调用 ensureAuthenticated 是对的。 加上它会把一个「无需授权」的命令变成硬失败；对语言信号而言，catch → 英文 才是正确形状。

---

建议 — --host 测试断言的是 setter，不是路由

packages/cli/src/commands/review/compose-review.test.ts:832、compose-review.ts:1232

commit 4 的信息说该改动"由一个用 --host 驱动 handler 并断言路由生效的测试覆盖"。该测试实际断言的是 handler 返回之后 getGhHost() === 'github.example.com'——即模块 env 的末态。两个后果：

1. 它的输入是 { modelId }、没有 planPath，于是 bilingualFromPlan 在 if (!planPath) 处就返回了，该测试里根本没有发生任何 gh 调用。这个选项唯一为之存在的代码路径，它从未执行到。
2. 把 setGhHost(host) 从 handler 顶部移到 composeReview(…) 调用之后，143/143 仍全绿——而在一个 plan 缺少标记的 GHE 评审上，兜底的 gh pr view 此时没有 GH_HOST，打到 github.com、失败，组出的纯英文正文与 submit 实际发布的正文不一致。这正是 commit 4 要防止的 bug，且带着全绿 CI 上线。

在这里，"重排"不是硬造的变异：同目录下 cleanup.ts:304,369 已经确立了围绕 setGhHost 的保存—恢复模式，把这套卫生习惯套到本 handler 上是很自然的后续改动，而位置极易放错。

在调用发生的那一刻捕获 host 可同时关掉两者。我在 9148756e 上实测：HEAD 上为绿，在"移除"变异下为红，在"重排"变异下也为红。代码见上方英文部分。

它严格覆盖了当前测试的全部内容，因此可以替换该测试，而不必并列新增。

---

建议（遗留）— 在发布路径上，register 仍然握在 caller 手里

compose-review.ts:1118-1131、submit.ts:295 对比 submit.ts:438

本 HEAD 未改变此事，故仅作简述。submit 从不检查 planPath 指向的文件是否由 CLI 写入，也不检查 plan 的 PR 身份是否与它已经校验过的 --repo/--pr 一致。驱动 runSubmit 对一个 caller 自写的 plan：{"prDescriptionHasHan": false} 会组出英文正文且零网络请求——权威 false 的短路是比 prBodyFetcher 更廉价的杠杆——而 {"ownerRepo": "attacker/elsewhere", "prNumber": "1"} 会把语言探测发往 submit 未被授权的仓库。两者都不是回归，但描述把这个兜底宣传为强化"caller 不能切换 register"；在发布路径上有干净的做法让它成真：把 submit 已校验的 args.repo/args.pr 传入 composeReview，兜底触发时优先采用。这同时解决第二层问题：plan.ownerRepo 抵达 gh --repo 时，完全没有经过 submit 在第 438 行对自己 --repo 所做的 isRepo() 形状校验——今天是无害的（execFileSync、无 shell、取数失败即回退英文），但这正是本文件已为同一个值确立过的守卫。

---

Nits

• SKILL.md:654 仍把 Step 6 描述为"它不写、不发布、也不需要授权，因此每次 high-effort 评审都可以跑"。在缺标记路径上它现在会发出一次 gh pr view。本 PR 更新了规则 2 与 Step 7 段落、并给 Step 6 命令块加了 --host 注释，却漏掉了唯一告诉模型这个命令做什么的那句话——与刚刚修好的规则 2 措辞属于同一类过时。
• commit 2 的存在是为了让 PR 号规则只有一份定义；但调用点对它的使用没有被钉住——把 isPositivePrNumber(plan?.prNumber) 换成 plan?.prNumber !== undefined，143 个测试依然全绿。后果仅是一次注定失败、随后回退英文的取数，所以这是备注而非要求。
• 流程：10:10 那条 CHANGES_REQUESTED 在两个 commit 前就已被修好，但一直没被 dismiss，这就是当前 mergeStateStatus: BLOCKED 的原因。建议清掉，让真实状态可见。

---

结论：Comment —— 无阻塞项。 功能正确，动机中的缺口真实可达，且它新增的每一个守卫现在都被"移除守卫即变红"的测试钉住了——包括发布边界，那正是第 2 轮在等的东西。两个遗留项分别是：对一个既有测试的严格更优替换；以及一个能让描述里"caller 不能切换 register"这一说法字面成真的后续改动。二者都不必阻塞本次合并。

[yiliang114]

审查意见

这是一个设计精良的修复，解决了 compose-review 在 plan 缺少 prDescriptionHasHan 标志时无法正确渲染双语 review 的问题。

✅ 正确性

核心逻辑在 bilingualFromPlan 中实现得当：

• 已记录的 true/false 是权威值，不会触发网络请求
• 仅在标志缺失且 plan 包含 PR 身份时才回退到实时获取
• 所有错误路径都 fail-safe 回退到英文

if (typeof plan?.prDescriptionHasHan === 'boolean') {
  return plan.prDescriptionHasHan;  // 权威值，不联网
}
// ... recovery logic ...
try {
  return /\p{Script=Han}/u.test(fetchPrBody(ownerRepo, prNumber));
} catch {
  return false;  // fail-safe
}

✅ 安全性

正确处理了潜在的调用方篡改风险：

compose-review handler:

delete parsed.prBodyFetcher;  // 阻止模型注入假 fetcher

submit.ts:

const { env: _dropped, prBodyFetcher: _droppedFetcher, ...rest } = state;

这防止了恶意调用方通过传入 prBodyFetcher: 'suppress' 来绕过双语渲染。

✅ 测试覆盖

测试用例全面覆盖了关键路径：

测试场景	覆盖
恢复出中文 → 渲染双语	✅
恢复出英文 → 保持英文	✅
已记录 false → 不联网	✅
无 PR 身份 → 不联网	✅
fetch 抛错 → 回退英文	✅
生产路径 gh pr view 参数	✅
剥离调用方注入的 fetcher	✅

✅ 代码质量

• 注释清晰解释了设计决策和背景
• Fail-safe 设计原则正确：语言绝不能拖垮 review
• 遵循现有代码模式和风格
• --host 选项正确传递给 GitHub Enterprise 支持

💡 轻微建议（可选）

1. 性能考虑：每个缺少标志的 review 会多一次 gh pr view 调用。但这已被合理缓解：

   • 仅在标志缺失时触发
   • fetch-pr 总是写入该标志，所以只影响 edge case

2. 竞态条件：PR 描述可能在 fetch-pr 和 compose-review 之间被修改。但这是可接受的：

   • 原始描述的语言才是关键
   • 回退到英文是正确的 fail-safe 行为

总结

推荐合并 🚀

修复设计合理，测试充分，安全考量周全。代码质量和文档都符合项目标准。

[wenshao]

@qwen-code /triage

Dismissing this stale CHANGES_REQUESTED (from commit 431cb75): the concern it raised was fixed and verified in the autofix rounds since (round 2 added the load-bearing submit-strip test; round 3 confirms no blockers), and the current triage at 9148756 is an approve pending green CI. Clearing so the real merge state is visible.

[qwen-code-ci-bot]

Triage re-run completed without a new review.

⚠️ The bot has no review of its own on 9148756e1aec62f1da04f3d1befc1d60929f00bc. If this re-run was meant to approve, it did not — an approval left by another account is a separate vote and does not count as the bot's own.

The stage comments above were updated with the latest result. View workflow run.

[qwen-code-ci-bot]

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

中文说明

⚠️ 已从批准降级为评论：CI still running。 已审查。

— qwen3.7-max via Qwen Code /review

[qwen-code-ci-bot]

LGTM, looks ready to ship — CI landed green after the review. ✅

[yiliang114]

[P2] Silent catch on the recovery path — no observability when the fallback fires

The design is correct ("the language must never take the review down"), but this catch is completely silent. When gh pr view fails (auth expiry, network, rate limit, GHE misconfiguration), the bilingual fold disappears with no trace in stderr or logs. A maintainer debugging "why did this Chinese PR get an English-only review?" would have no signal — the same symptom that motivated this PR (#7686) would recur, just through a different door.

Consider adding a one-line stderr warning so the fallback is observable:

} catch (err) {
  process.stderr.write(
    `compose-review: bilingual recovery failed (${ownerRepo}#${prNumber}): ${err instanceof Error ? err.message : String(err)}\n`,
  );
  return false;
}

This keeps the fail-safe (English-only, review still posts) while giving the operator a breadcrumb. The plan-read catch on line 1115 could get the same treatment for consistency, though it fires less often in practice.

[yiliang114]

[P2] gh() carries transient-error retry — up to 9 s added to compose-review on a flaky network

fetchPrBodyViaGh delegates to gh(), which retries up to 2 times with 3 s / 6 s sleeps on HTTP 5xx. This is the right behaviour for idempotent reads in the happy path, but this is a best-effort fallback — the caller already accepts English-only on failure. On a flaky network (the exact condition that makes transient retries fire), the user waits up to 9 s extra just to decide the body language, then gets English anyway.

Two options, either is fine:

1. Accept the latency as a cost of correctness — the fallback only fires when prDescriptionHasHan is missing from the plan, which is the uncommon path. Document this tradeoff in the JSDoc so future readers know.
2. Use a non-retry variant (like ghWithInput does for submit) — e.g., a ghNoRetry(...) wrapper that throws immediately on any error. The catch in bilingualFromPlan already handles the failure gracefully.

Option 1 is simpler and the path is rare enough that this is a suggestion, not a blocker.

[yiliang114]

[Nit] Strong test battery for the recovery path

Seven tests covering the fallback matrix: Han present/absent in recovered body, recorded false short-circuits (no network), no PR identity short-circuits, fetch failure → English, production reader verifies exact gh args, and the handler strip guard against a model-supplied prBodyFetcher. The recordingFetcher pattern (proving it was NOT reached in the short-circuit tests) is a nice touch.

One gap worth considering: there's no test for the case where prDescriptionHasHan is present but with a non-boolean type (e.g. "true" as string, or null). The code correctly falls through to the recovery path in that case (line 1119 typeof === 'boolean' check), but a test asserting "string \"true\" triggers recovery, not silent acceptance" would guard against a future loosening of the type check.

[yiliang114]

Review Summary

Verdict: APPROVE — the fix is correct, well-scoped, and thoroughly tested. Two P2 suggestions and one nit inline, none blocking.

What this does well

1. Root cause is correctly identified and fixed. The bilingual body register depended solely on prDescriptionHasHan in the plan, but plan-diff plans and improvised pipelines never write that field. Recovering from the live PR (gh pr view) when the flag is absent is the right fallback — the live description is authoritative and the caller cannot forge it.

2. Fail-safe design is correct. Every failure mode (no plan, unreadable plan, no PR identity, fetch error) falls through to English-only. "The language must never take the review down" is the right invariant and it holds on every path.

3. Defense in depth on prBodyFetcher. Stripping the test seam from the input JSON in both compose-review handler (line 1258) and submit.ts (line 295) closes the alternate attack surface where a model-supplied non-function value would throw and drop the Chinese fold through the catch. Good belt-and-suspenders.

4. Han detection is robust. \p{Script=Han} via Unicode property escapes is the standard approach — covers simplified + traditional Chinese, ignores Japanese kana and Korean hangul. Applied to the PR body (raw markdown), which is the right signal source.

5. Test coverage is excellent. 7 tests in the new describe block cover: Han recovery, no-Han recovery, recorded-false short-circuit, no-PR-identity short-circuit, fetch failure, production reader arg verification, and handler strip guard. The recordingFetcher pattern proving non-invocation is clean.

6. GHE host routing is properly threaded through --host → setGhHost(), consistent with the other review subcommands.

Inline findings (all non-blocking)

• P2 on compose-review.ts:1131 — silent catch on the recovery path; a stderr warning would make the fallback observable
• P2 on compose-review.ts:1068 — gh() carries transient-error retry (up to 9s); acceptable for this rare path but worth documenting
• Nit on test file — missing test for non-boolean prDescriptionHasHan (e.g. string "true") falling through to recovery

[doudouOUC]

Reviewed. Not reviewed: Agent 1a: Line-by-line correctness, Agent 2: Security, Agent 3: Code quality, Agent 4: Performance & efficiency, Agent 5: Test coverage, Agent 6a: Undirected audit — attacker mindset, Agent 6b: Undirected audit — 3 AM oncall mindset, Agent 6c: Undirected audit — six-months-later maintainer, Agent 1b: Removed-behavior audit, Agent 1c: Cross-file tracer, Agent 7: Build & test verification — its prompt was built, but no agent on record was launched with it. Not reviewed: reverse audit — an auditor ran and opened its brief, but no agent was launched with the prompt the CLI built — the launch was written by hand, and what the agent was actually asked is not what this skill certifies.

— qwen3.7-max via Qwen Code /review

[yiliang114]

PR Review: Recover Bilingual Register from Live PR

Summary

This PR fixes a real issue where Chinese-authored PRs received English-only reviews when the plan (from plan-diff or an improvised pipeline) lacked the prDescriptionHasHan flag. The fix is well-designed with proper fail-safe behavior.

---

✅ Correctness

The core logic in bilingualFromPlan() is correct:

1. Authoritative values are respected: If prDescriptionHasHan is explicitly true or false, use it directly — no network call
2. Recovery path triggers correctly: Only when the flag is absent AND the plan carries PR identity (ownerRepo + prNumber)
3. Fail-safe everywhere: Every error path (unreadable plan, no PR identity, fetch failure, JSON parse error) falls back to English — "language must never take the review down" is the right invariant

The fetchPrBodyViaGh implementation correctly:

• Uses --json body (not bodyText) to get raw markdown
• Returns empty string for missing body field
• Wraps everything in try/catch

---

✅ Security

Good defense-in-depth on the prBodyFetcher test seam:

compose-review handler (line 1258):

delete parsed.prBodyFetcher;

submit.ts (line 295):

const { env: _dropped, prBodyFetcher: _droppedFetcher, ...rest } = state;

This prevents a model/caller from injecting a non-function value (e.g., "suppress") that would throw when called and drop the Chinese fold through the fail-safe catch. The same pattern used for env is correctly applied here.

The recovery signal comes from gh pr view — the real PR body that the caller cannot forge — which correctly "tightens the caller cannot toggle the register" property.

---

✅ Test Coverage

Excellent coverage in compose-review.test.ts:

Scenario	Covered
Recovery finds Han → bilingual body	✅
Recovery finds no Han → English only	✅
Recorded false → no network call	✅
No PR identity → no network call	✅
Fetch throws → fail-safe to English	✅
Production gh pr view arg verification	✅
Handler strips injected prBodyFetcher	✅
GHE --host routing	✅

The recordingFetcher helper proving non-invocation is a clean pattern.

---

✅ Code Quality

• Comments are thorough and explain the "why" — the background on perf(core): Lazy-load first-use dependencies #7686, the gap with plan-diff, and the security reasoning
• Fail-safe design is consistent throughout
• isPositivePrNumber export avoids duplicating PR number validation logic
• --host option correctly added for GitHub Enterprise, consistent with other review subcommands
• SKILL.md updated to document the recovery path

---

💡 Minor Observations (Non-blocking)

1. P2 - Silent fallback: The catch block at line 1131 silently falls back to English. Consider logging a warning to stderr so the fallback is observable in production.

2. P2 - Double fetch on absent-flag path: When the flag is absent, bilingualFromPlan is called twice (Step 6 compose-review, Step 7 submit). Each gh() call has 2-retry backoff. Acceptable for the rare path, but could accumulate ~9s blocking on GitHub issues.

3. Test suggestion: The --host test asserts getGhHost() after the handler returns but doesn't trigger the recovery path (no planPath in input). Consider extending it to verify GH_HOST is set at the moment gh is called.

---

Verdict

APPROVE — The fix is correct, well-tested, and follows the project's design patterns. The fail-safe behavior is robust and the security considerations are properly handled.

中文说明

PR 评审：从线上 PR 恢复双语语言标记

概述

本 PR 修复了一个真实问题：当 plan（来自 plan-diff 或临时拼装的流水线）缺少 prDescriptionHasHan 标记时，中文作者的 PR 会收到纯英文评审。修复设计合理，有正确的 fail-safe 行为。

---

✅ 正确性

bilingualFromPlan() 的核心逻辑正确：

1. 权威值被尊重：如果 prDescriptionHasHan 显式为 true 或 false，直接使用 — 无网络请求
2. 恢复路径正确触发：仅当标记缺失且 plan 带有 PR 身份（ownerRepo + prNumber）时
3. 处处 fail-safe：每个错误路径（plan 不可读、无 PR 身份、取数失败、JSON 解析错误）都回退到英文 — "语言绝不能拖垮评审" 是正确的原则

fetchPrBodyViaGh 实现正确：

• 使用 --json body（非 bodyText）获取原始 markdown
• 缺少 body 字段时返回空字符串
• 所有路径都包裹在 try/catch 中

---

✅ 安全性

对 prBodyFetcher 测试接缝有良好的纵深防御：

compose-review handler（第 1258 行）：

delete parsed.prBodyFetcher;

submit.ts（第 295 行）：

const { env: _dropped, prBodyFetcher: _droppedFetcher, ...rest } = state;

这防止了模型/调用方注入非函数值（如 "suppress"）在调用时抛错、通过 fail-safe catch 丢掉中文折叠。与 env 相同的模式被正确应用。

恢复信号来自 gh pr view —— 调用方无法伪造的真实 PR 正文 —— 正确地"强化了调用方无法切换语言标记"这一属性。

---

✅ 测试覆盖

compose-review.test.ts 覆盖出色：

场景	覆盖
恢复出中文 → 双语正文	✅
恢复出无中文 → 纯英文	✅
已记录 false → 不联网	✅
无 PR 身份 → 不联网	✅
取数抛错 → fail-safe 到英文	✅
生产 gh pr view 参数验证	✅
Handler 剥离注入的 prBodyFetcher	✅
GHE --host 路由	✅

recordingFetcher 辅助函数证明非调用是简洁的模式。

---

✅ 代码质量

• 注释详尽，解释了"为什么" —— perf(core): Lazy-load first-use dependencies #7686 背景、plan-diff 缺口、安全考量
• Fail-safe 设计贯穿始终
• isPositivePrNumber 导出避免重复 PR 号验证逻辑
• --host 选项正确添加 GitHub Enterprise 支持，与其他 review 子命令一致
• SKILL.md 已更新记录恢复路径

---

💡 轻微观察（非阻塞）

1. P2 - 静默回退：第 1131 行的 catch 块静默回退到英文。建议在 stderr 输出警告以便生产环境可观察。

2. P2 - 缺标记路径双次取数：标记缺失时，bilingualFromPlan 被调用两次（Step 6 compose-review，Step 7 submit）。每次 gh() 调用有 2 次重试退避。对于罕见路径可接受，但 GitHub 故障时可能累积约 9 秒阻塞。

3. 测试建议：--host 测试在 handler 返回后断言 getGhHost()，但未触发恢复路径（输入无 planPath）。建议扩展以验证 gh 被调用时 GH_HOST 已设置。

---

结论

批准 —— 修复正确、测试充分、遵循项目设计模式。Fail-safe 行为健壮，安全考量处理得当。

[qwen-code-ci-bot]

Released in v0.21.1.

[yiliang114]

⚠️ Failed to process this request. Please re-mention the bot to retry.