chore(review): close the mutation-verified test-pin gaps from PR #9096 review rounds 5-6

[wenshao]

Follow-up to PR #9096 (review platform subcommands). Across review rounds 5–6 the automated reviewer flagged a consistent class of hardening gaps that are real but non-blocking: tests that under-pin their stated contract (a mutation in the production code ships the suite green). These are test-robustness and a few minor code-hygiene items, not behavior bugs — landing them is worthwhile but should not hold the PR.

Pin-completeness gaps (the dominant theme — each was mutation-verified by the reviewer)

Extend the new subcommand tests so the assertion actually fails when the named invariant is broken:

• Full emitted-command prefix: several pins match only from the subcommand name, not the "${QWEN_CODE_CLI:-qwen}" review wrapper (pr-context inline/issue/review kinds, agent-prompt weld, SKILL.test.ts).
• Negative assertions: rule-4 guard lacks not.toContain('gh issue view'); the 422 drift guard is positive-only (no headRefOid absence check).
• Ordering invariants pinned half-way: the host/auth/gh ordering pins host < min(auth, gh) but not auth < gh; mkdir-before-write ordering is unpinned in fetch-diff/comment-body/issue-context/pr-context.
• Write-target path never asserted: tests read writeFileSyncMock.mock.calls[0][1] (content) but never calls[0][0] (path) — a redirected write ships green.
• Error-message assertions dropped/never pinned: several catch/guard stderr writes are unasserted (writeStderrLineSafe mocked anonymously).
• Numeric-guard Number.isInteger half untested in comment-body/issue-context (only non-positive cases).
• Whitespace-only --host refusal unpinned in handler tests (setGhHost is mocked wholesale, so the real validator never runs in-handler).
• Omitted-host path (setGhHost(undefined)) unpinned in some handlers.
• Success-path extras header wording ('NOT in the closing set') never positively asserted; unfetchable/JSON↔file agreement partially unpinned.
• pr-context host routing: setGhHost(host) handler call had no assertion (addressed) — audit siblings for the same gap.
• meta exit-1 classification for a HOSTNAME_RE-rejected discovered host is untested; the error message's GH_HOST env attribution branch is missing (three-way source attribution needed).

Code-hygiene items worth a second look

• compose-review planPrIdentity re-validates the plan host with HOSTNAME_RE WITHOUT trimming — now consistent because fetch-pr records the trimmed host (round-6), but the reader-side trim that agent-prompt does is the more robust pattern; consider aligning.
• Design doc (docs/design/2026-08-13-…) "no behavior change" invariant was falsified by three probe-confirmed regressions during the PR (ghRaw utf8 decode, resolveRepo parent hop, publish-assets whitespace host) — correct the invariant wording or add a caveat.
• yargs-layer usage errors exit 1 (missing required arg / invalid --kind), not the exit-2 the handler contract implies — the global .fail() in config.ts governs this; tree-wide exit-2 semantics would need a scoped change there.
• plan-diff records --repo/--pr into the plan without isOwnerRepo/numeric validation at record time (the agent-prompt weld re-validates before interpolating, so this is defense-in-depth, not a live hole).
• pr-context handler calls setGhHost(host) without the try/catch TypeError→exit-2 classification its siblings have.
• Render-adjudication carve-out (Step 4) availability is tied to the PR's Enterprise host in the prose, but the call targets the user-designated QWEN_REVIEW_SCRATCH_REPO, whose host is independent.
• Stale comment/test-title: agent-prompt weld comment says "fetch-pr records the raw flag" but fetch-pr now records the trimmed host.

Each item above links to a specific inline comment on PR #9096 (rounds 5–6, reviewer qwen-code-ci-bot); see those threads for the probe evidence and exact file:line.

[qwen-code-ci-bot]

✅ Qwen Triage finished — view run. See the stage comments in this thread for the result.

✅ Qwen Triage 已完成 —— 查看运行。结果见本线程中的各阶段评论。

[qwen-code-ci-bot]

Triage

• Type: enhancement (chore) — maintainer follow-up to PR feat(review): absorb prose gh commands into platform-backed subcommands #9096 (review platform subcommands): mutation-verified test-pin gaps + minor code-hygiene items
• Labels: type/enhancement, scope/testing, scope/cli, category/cli, priority/P3
• Next: pick up after PR feat(review): absorb prose gh commands into platform-backed subcommands #9096 merges — every item targets code introduced or modified by that PR

Result

• Admissible: yes. Maintainer-filed and clearly scoped; the issue itself declares all items real but non-blocking (should not hold PR feat(review): absorb prose gh commands into platform-backed subcommands #9096), so no action on the PR itself is required.
• Scope: 11 test-pin gap items (assertions that would not fail when the named invariant breaks — full emitted-command prefix, negative assertions, ordering invariants, write-target paths, stderr messages, numeric guards, host validation) + 7 code-hygiene items.
• Context check (static): comment-body, fetch-diff, issue-context, and meta exist only in the PR feat(review): absorb prose gh commands into platform-backed subcommands #9096 branch (added there); pr-context, plan-diff, agent-prompt, fetch-pr, publish-assets, and SKILL.test.ts exist on main but are modified by feat(review): absorb prose gh commands into platform-backed subcommands #9096. The PR is still OPEN (not merged), so this follow-up must land after it — or be based on its branch.
• Note: the yargs-layer exit-2 item touches the global .fail() in config.ts — a tree-wide change per the issue itself, best split from the test-only work.
• Evidence: static verification only (unattended CI run; no code built or executed). PR/file state checked via the GitHub API; main checked at c396fe3.

中文说明

• 类型: 增强（chore）—— 维护者提交的 PR feat(review): absorb prose gh commands into platform-backed subcommands #9096（review 平台子命令）后续跟进 issue：经 mutation 验证的测试断言缺口 + 少量代码卫生项
• 标签: type/enhancement、scope/testing、scope/cli、category/cli、priority/P3
• 下一步: 待 PR feat(review): absorb prose gh commands into platform-backed subcommands #9096 合入后跟进 —— 所有条目都针对该 PR 新增或修改的代码

结果

• 可受理: 是。由维护者创建，范围清晰；issue 本身声明所有条目真实但不阻塞（不应拖累 PR feat(review): absorb prose gh commands into platform-backed subcommands #9096），因此无需对该 PR 采取行动。
• 范围: 11 项测试断言缺口（相应不变量被破坏时断言不会失败 —— 完整命令前缀、负向断言、顺序不变量、写入目标路径、stderr 消息、数值守卫、host 校验）+ 7 项代码卫生项。
• 上下文核实（静态）: comment-body、fetch-diff、issue-context、meta 仅存在于 PR feat(review): absorb prose gh commands into platform-backed subcommands #9096 分支（由该 PR 新增）；pr-context、plan-diff、agent-prompt、fetch-pr、publish-assets、SKILL.test.ts 已在 main 上但被 feat(review): absorb prose gh commands into platform-backed subcommands #9096 修改。该 PR 目前仍为 OPEN（未合入），因此本跟进须在其合入后进行（或基于该 PR 分支开展）。
• 注意: yargs 层 exit-2 一项涉及 config.ts 中的全局 .fail() —— issue 本身已指出这是全仓库范围的改动，建议与纯测试工作拆分处理。
• 证据: 仅静态核实（无人值守 CI 运行；未构建或执行任何代码）。PR/文件状态通过 GitHub API 核查；main 核查于 c396fe3。

--- Qwen Code

[wenshao]

Round-9 additions (2026-08-15): the bot approved PR #9096 this round, but it also left 7 Suggestion-class test-pin gaps that were deferred here per the ~5-round discipline:

• comment-body R8-2 duplicated---kind guard unpinned (no array-kind test)
• duplicated string-flag arrays not normalized across the new handlers (7 probe-confirmed locations)
• plan-diff usage validation runs after the diff-file read (classification order)
• comment-body stdout-channel assertion doesn't spy process.stdout.write
• agent-prompt non-string-host (host: 42) guard has no tamper test
• plan-diff no-record assertion can't distinguish never-wrote vs overwrote-before-failing
• fetch-diff byte fidelity severed at the reader: plan-diff reads the diff with readFileSync(..., 'utf8'), so the latin1 write is only realized if the read path also uses latin1 — worth aligning or documenting

No Criticals in round 9; the PR is approved and ready to ship. These remain non-blocking hardening/pin items.