feat(review): back comment-status and presubmit for Aone Code targets - #9627
Conversation
A second `--comment` round on an Aone MR re-posted every still-valid finding as a new comment and never downgraded a self-MR review — both flows were skipped for lack of a1 backing. Route Aone targets at the a1 reads (mr view / mr status / mr comment list / auth whoami) through the same pure classification cores the GitHub path pins, so the report schemas and the Step-7 downgrade semantics stay one contract: parentNoteId threading, closed → resolved, outdated → stale (a rewritten line stays re-postable), no commit anchors (code facts degrade to unknown), and drift with no compare API fails safe. The context-unavailable verdict cap stays until pr-context lands. Closes #9613
|
Re-run at the merged head Template: complete ✓ — all sections filled, Evidence legitimately N/A (CLI subcommands + skill docs, no TUI surface), Chinese translation present, closes #9613. Problem: real, not theoretical. Under AGit-Flow a CR is one commit amended in place, so amend → re-review is the normal loop, and without this backing every repeat Direction: aligned — it completes a designed slice of the platform-provider abstraction instead of inventing a new surface, and both platforms feed the same pure classification cores, so the report contract stays one computation. Size: ~1,213 production-logic lines (comment-status.ts 506, presubmit.ts 464, aone.ts 243), ~1,572 test lines, ~31 docs lines. It is cross-package (the bundled review skill markdown under Approach: the shape is right — a shared report writer plus per-platform runners, so the report schema and the downgrade semantics are computed once. The merge composition with #9621 in the shared files is verified clean in Stage 2. No drive-by changes. Risk: no high-risk-path hits in the revert-history signal. Moving on to code review. 🔍 中文说明在合并后的 head **模板:**完整 ✓——各节均已填写,Evidence 合理地为 N/A(CLI 子命令 + skill 文档,无 TUI 界面),含中文翻译,关闭 #9613。 **问题:**真实存在,并非理论性的。AGit-Flow 下一个 CR 就是一个原地 amend 的提交,amend → 重新审查是常态循环;没有这个支撑,每一轮重复的 **方向:**对齐——它完成的是平台抽象设计中已规划的一片,而不是发明新表面,且两个平台共用同一套纯分类核心,报告契约只有一份计算。 **规模:**生产逻辑约 1,213 行(comment-status.ts 506、presubmit.ts 464、aone.ts 243),测试约 1,572 行,文档约 31 行。跨包( **方案:**形态正确——共享报告写入器 + 平台 runner,报告 schema 与降级语义只计算一次。与 #9621 在共享文件中的合并组合已在 Stage 2 验证干净。无顺手改动。 **风险:**回滚历史信号中无高风险路径命中。进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewWhy this re-pass exists: the head moved from The merge composition is correct. In No Critical findings at this head. Two non-blocking follow-ups, both artifacts of two PRs developed in parallel rather than of this merge itself: (1) TestingCI evidence — the PR's own checks on the reviewed commit, fetched through the API (no PR code was executed in this review):
Both 中文说明**本轮复审的缘由:**head 从 合并组合是正确的。 **该 head 上无 Critical 问题。**两个非阻塞跟进项,都是两个并行开发的 PR 的产物而非本合并引入:(1) 测试:以上 CI 表格引自该提交在 GitHub 上的真实检查结果(经 API 拉取,本评审未运行任何 PR 代码)。两个 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — the re-review delta was exactly what a merge-of-main should be: zero lines moved in the dedup core, the composition with #9621 verified clean in every shared file, and CI fully green on the reviewed commit. The honest reservation is the standing one: live Stepping back over the whole run: this PR does one thing — close the Phase 3 dedup/self-PR slice of the platform-provider design — and does it the right way, routing both platforms through the same pure classification cores so the report contract and the downgrade semantics stay one computation instead of two copies that could drift. The fail-safe discipline is the part I'd thank the author for in six months: unreadable gate state reads as pending, never as all-clear; drift with no compare API fails anchors-at-risk; an unknown identity never self-matches. The two dual-helper divergences the parallel development left in Verdict: approve — both 中文说明置信度:4/5——本轮复审的增量正是一次 main 合并该有的样子:去重核心零行变动,与 #9621 的组合在所有共享文件中验证干净,该提交上 CI 全绿。唯一的保留意见仍是那个一贯的:线上 a1 载荷形状从未实测(已披露——由宽容提取与保守失败兜底,且本 head 上已有一次 整体回看:这个 PR 只做一件事——关闭平台抽象设计 Phase 3 的 dedup/self-PR 切片——而且做法正确:两个平台走同一套纯分类核心,报告契约与降级语义只有一份计算,而不是两份可能漂移的拷贝。保守失败的纪律是六个月后会让人感谢作者的部分:不可读的门禁状态读作 pending、绝不读作放行;没有 compare API 的漂移一律判锚点有风险;未知身份绝不自匹配。并行开发在 结论:批准——该提交上两个 — Qwen Code · qwen3.8-max Reviewed at |
|
@qwen-code /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. Remove the 中文说明🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。移除 |
Upstream #9461 moved the ledger-id shape regex from a presubmit-local constant into lib/ledger.js; the overlap with the a1-backing imports is mechanical — both imports kept, no behavior change on either path.
|
Qwen Code review did not complete successfully. The review pipeline failed before a review could be posted. A transient error is retried automatically; if you are seeing this, retry with |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
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 and its suite did not run locally.
Not explored to full depth (tool budget reached): chunk 6: executing packages/cli/src/commands/review/presubmit.aone.test.ts under vitest (worktree has no installed dependencies; every assertion was instead verified sta….
中文说明
仅完成部分审查,审查缺口已披露。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度(达到工具调用预算):chunk 6:executing packages/cli/src/commands/review/presubmit.aone.test.ts under vitest (worktree has no installed dependencies; every assertion was instead verified sta…。
— qwen3.8-max via Qwen Code /review (v0.21.15)
| } | ||
| } | ||
| for (const container of containers) { | ||
| const checks = (container as Record<string, unknown>)['checks']; |
There was a problem hiding this comment.
[Critical] extractStatusChecks throws a TypeError on a null a1 payload instead of returning the designed undefined ("unreadable gate state"): containers is seeded [null] — the if (out !== null && ...) guard only protects the nesting scan, not the seed — and the loop then reads null['checks']. The sole production caller runPresubmitAone calls getMrStatusChecks unguarded (only getMrAuthorAndHead sits in a try/catch), and the yargs handler has no catch. So when a1 repo mr status answers null — a payload shape this same hunk treats as routine: the sibling listMrComments types its call a1Json<AoneMrComment[] | null> and pins a "tolerating null" test — qwen review presubmit crashes with a stack trace and no report file, stalling Step 7 of an Aone review, instead of capping the verdict the way a still-running check does.
Witness (probe in a scratch tree, a1Json mocked → null):
unmodified PR: {"kind":"threw","value":"TypeError: Cannot read properties of null (reading 'checks')"}
with the guard below: {"kind":"returned"} → undefined → all_pending; all 73 aone tests still pass
| const checks = (container as Record<string, unknown>)['checks']; | |
| if (container === null || typeof container !== 'object') { | |
| continue; | |
| } | |
| const checks = (container as Record<string, unknown>)['checks']; |
中文说明
extractStatusChecks 在 a1 返回 null 载荷时抛出 TypeError,而不是按设计返回 undefined("门禁状态不可读"):containers 以 [null] 初始化——if (out !== null && ...) 守卫只保护嵌套扫描,不保护这个种子——循环随后读取 null['checks']。唯一的生产调用方 runPresubmitAone 未加守卫地调用 getMrStatusChecks(只有 getMrAuthorAndHead 包在 try/catch 中),且 yargs handler 没有 catch。因此当 a1 repo mr status 返回 null(同一 hunk 视为常态的载荷形状:同组的 listMrComments 把调用类型标为 a1Json<AoneMrComment[] | null> 并有 "tolerating null" 测试钉住)时,qwen review presubmit 会以堆栈跟踪崩溃且不写报告文件,使 Aone 审查在第 7 步卡住,而不是像"检查仍在运行"那样给裁决加上限。探针证据(scratch tree,将 a1Json mock 为 null):未修改的 PR 抛出 TypeError: Cannot read properties of null (reading 'checks');加上下方守卫后返回 undefined → all_pending,且全部 73 个 aone 测试仍通过。
— qwen3.8-max via Qwen Code /review (v0.21.15)
| throw new Error('owner_repo must look like "owner/repo"'); | ||
| } | ||
| const mrId = Number(prNumber); | ||
| if (!Number.isInteger(mrId) || mrId <= 0) { |
There was a problem hiding this comment.
[Critical] This pr_number guard accepts tokens fetch-pr deliberately refuses — Number.isInteger(Number(x)) passes '012', '1e3', '0x1f', ' 12', '12.0' — reintroducing the exact label/content divergence fetch-pr's own validation comment documents: "Number('1e3') is 1000, so an unvalidated token would fetch a DIFFERENT PR's head while the ref/worktree/report all carry the caller's label". Witness (probe, unmodified PR): comment-status 012 owner/repo coerces to MR 12 but worktreePath('012') resolves to .qwen/tmp/review-pr-012, which does not exist → worktreeMissing: true and every thread's code facts silently unknown in a SUCCESS report with no error field, whose warning advises "run fetch-pr first" — which already ran and refuses the caller's spelling; comment-status 1e3 ... queries MR 1000 under the label '1e3' while fetch-pr refuses the same token. Flip-check: tightening both Aone runners to /^[1-9]\d*$/ rejects all five tokens. The skill path is guarded by parse-args' digit grammar; exposure is the direct-CLI surface. The twin guard in presubmit.ts carries its own comment.
| if (!Number.isInteger(mrId) || mrId <= 0) { | |
| if (!/^[1-9]\d*$/.test(prNumber)) { |
中文说明
这个 pr_number 守卫接受了 fetch-pr 刻意拒绝的 token——Number.isInteger(Number(x)) 会让 '012'、'1e3'、'0x1f'、' 12'、'12.0' 通过——重新引入了 fetch-pr 自身校验注释明确记载的"标签/内容错位":"Number('1e3') 是 1000,未校验的 token 会去取另一个 PR 的 head,而 ref/worktree/报告却都带着调用方的标签"。探针证据(未修改的 PR):comment-status 012 owner/repo 强转后查询 MR 12,但 worktreePath('012') 解析到不存在的 .qwen/tmp/review-pr-012 → worktreeMissing: true,每个线程的代码事实悄悄变成 unknown,而这是一份没有 error 字段的 SUCCESS 报告,其警告还建议"先运行 fetch-pr"——可 fetch-pr 早已运行过且拒绝调用方的这种写法;comment-status 1e3 ... 会以标签 '1e3' 查询 MR 1000,而 fetch-pr 拒绝同一个 token。翻转验证:把两个 Aone runner 收紧为 /^[1-9]\d*$/ 后五个 token 全部被拒绝。skill 流程由 parse-args 的纯数字语法保护;暴露面是直接 CLI 调用。presubmit.ts 中的孪生守卫另有行内评论。
— qwen3.8-max via Qwen Code /review (v0.21.15)
| throw new Error('owner_repo must look like "owner/repo"'); | ||
| } | ||
| const mrId = Number(prNumber); | ||
| if (!Number.isInteger(mrId) || mrId <= 0) { |
There was a problem hiding this comment.
[Critical] Twin of the comment-status.ts guard (see the comment there for the probe evidence): Number.isInteger(Number(x)) accepts '012'/'1e3'/'0x1f'/' 12'/'12.0', all of which fetch-pr's /^[1-9]\d*$/ grammar refuses. For presubmit the '1e3' class is the sharper one: it computes overlap/resolved/repost state from MR 1000's comments while the report is labeled '1e3' — dedup decisions built from an unrelated MR under the caller's label. Same fix: validate the raw token against /^[1-9]\d*$/ before Number().
| if (!Number.isInteger(mrId) || mrId <= 0) { | |
| if (!/^[1-9]\d*$/.test(prNumber)) { |
中文说明
与 comment-status.ts 中的守卫是孪生问题(探针证据见该处评论):Number.isInteger(Number(x)) 接受 '012'/'1e3'/'0x1f'/' 12'/'12.0',而 fetch-pr 的 /^[1-9]\d*$/ 语法全部拒绝。对 presubmit 而言 '1e3' 一类更尖锐:它会用 MR 1000 的评论计算 overlap/resolved/repost 状态,而报告标签却是 '1e3'——用无关 MR 的数据、顶着调用方的标签做去重决策。修复相同:在 Number() 之前用 /^[1-9]\d*$/ 校验原始 token。
— qwen3.8-max via Qwen Code /review (v0.21.15)
| ? c.created_at | ||
| : '', | ||
| subject_type: | ||
| c.path && (line !== null || c.outdated === true) ? 'line' : 'file', |
There was a problem hiding this comment.
[Suggestion] Two of the four subject_type combinations are pinned by no test, and both mutants survive the suite (probe-proven in a scratch tree; a sanity mutant forcing 'line' turns the suite red, so the harness is alive): (a) a pathless (MR-level) comment WITH outdated: true — the test announcing "pathless comments are never outdated" only uses a pathless fixture without the flag, so the mutant dropping c.path && survives, and an MR-level summary comment with outdated: true would then map to subject_type: 'line' with line: null, computing outdated: true on an empty path — the precise misread the mapper's own doc forbids ("A pathless comment is an MR-level one ... so it never reads as outdated"); (b) the live path+line+non-outdated shape — every ordinary posted Aone finding — where the mutant narrowing to c.path && c.outdated === true survives and would flip anchor.isFileLevel to true on every live line-anchored Aone thread, so a jq consumer selecting .anchor.isFileLevel == false over this machine-shaped report drops every live finding thread.
Fix: add a pathless fixture WITH the flag to the "never outdated" test ({ id: 31, note: 'an MR-level summary', outdated: true, ... }, asserting isFileLevel === true AND outdated === false), and one assertion pinning the live case, e.g. expect(thread(21).anchor.isFileLevel).toBe(false);.
中文说明
subject_type 的四种组合中有两种没有任何测试钉住,且两个变异体都在测试套件中存活(scratch tree 中探针证实;强制 'line' 的哨兵变异体会让套件变红,说明测试框架是活的):(a) 带 outdated: true 的无路径(MR 级)评论——宣称"无路径评论永不 outdated"的测试只用了一个不带该标志的无路径 fixture,因此去掉 c.path && 的变异体可以存活;此时一个带 outdated: true 的 MR 级总结评论会映射为 subject_type: 'line' 且 line: null,在空路径上算出 outdated: true——正是 mapper 自身文档禁止的误读("无路径评论是 MR 级的……因此永远不会读作 outdated");(b) 活跃的"有路径+有行号+非 outdated"形状——即每条已发布的普通 Aone finding——把表达式收窄为 c.path && c.outdated === true 的变异体可以存活,并会把每条活跃行锚定 Aone 线程的 anchor.isFileLevel 翻转为 true,于是用 .anchor.isFileLevel == false 筛选这份机器可读报告的 jq 使用者会丢掉所有活跃 finding 线程。修复:在 "never outdated" 测试中补一个带该标志的无路径 fixture(断言 isFileLevel === true 且 outdated === false),再加一条钉住活跃场景的断言,例如 expect(thread(21).anchor.isFileLevel).toBe(false);。
— qwen3.8-max via Qwen Code /review (v0.21.15)
| type: 'string', | ||
| describe: | ||
| 'GitHub host for this PR (GitHub Enterprise). Routes every gh call in this command via GH_HOST; omit for github.com.', | ||
| 'Host for this PR (GitHub Enterprise, or an Aone host to select the a1 backend). Routes every gh call in this command via GH_HOST; omit for github.com.', |
There was a problem hiding this comment.
[Suggestion] This --host describe (edited by this diff) still ends with "omit for github.com", but the new handler dispatches via detectPlatformKind({ host }), which — with --host omitted — falls back to the cwd clone's origin and selects the a1 backend when that origin is an Aone-family host (probe-proven: omitted-host + Aone origin → 'aone'; omitted-host + github origin → 'github'). The base handler had no detection at all (setGhHost(host) unconditionally), so omission previously always meant gh/github.com — this diff changed that while leaving the promise in place. A reviewer in a clone whose origin is an Aone host (the setup this PR exists for) who runs this subcommand against a GitHub PR without --host gets a1 calls against a GitHub coordinate and an exit-0 degraded a1-error report instead of the GitHub index. Every sibling that gained detection describes the real chain ("omitted: detected from the clone's origin, else GitHub" — meta, fetch-diff, issue-context, comment-body); presubmit.ts carries the identical stale wording.
Fix: replace the trailing sentence with the sibling wording, e.g. "omitted: detected from the clone's origin, else GitHub."
中文说明
这条 --host 描述(由本 diff 编辑过)仍以 "omit for github.com" 结尾,但新的 handler 通过 detectPlatformKind({ host }) 分发:省略 --host 时会回退到当前 clone 的 origin,若该 origin 属于 Aone 主机族就选择 a1 后端(探针证实:省略 host + Aone origin → 'aone';省略 host + github origin → 'github')。改动前的 handler 完全没有检测逻辑(无条件 setGhHost(host)),因此省略过去总是意味着 gh/github.com——本 diff 改变了这一点却保留了原来的承诺。一位在 origin 为 Aone 主机的 clone 中工作(正是本 PR 服务的场景)的评审者,若不带 --host 对 GitHub PR 运行此子命令,会对 GitHub 坐标发起 a1 调用,得到一份带 a1 错误的 exit-0 降级报告,而不是 GitHub 索引。所有获得检测能力的兄弟子命令都描述了真实链路("omitted: detected from the clone's origin, else GitHub"——meta、fetch-diff、issue-context、comment-body);presubmit.ts 有一模一样的过期措辞。修复:把结尾句替换为兄弟子命令的措辞。
— qwen3.8-max via Qwen Code /review (v0.21.15)
| the GitHub path pins — see the Phase 3 note in | ||
| `2026-08-13-review-platform-provider-abstraction.md` for the shape mapping | ||
| (parentNoteId threading, `closed` → resolved, `outdated` → stale, no | ||
| commit anchors, drift with no compare API). Only `pr-context` remains |
There was a problem hiding this comment.
[Suggestion] The added claim "Only pr-context remains unbacked" is contradicted by the user doc updated in this same diff — docs/users/features/code-review.md states "test-plan is unbacked" alongside pr-context — and by reality: test-plan.ts contains no Aone/platform-detection path, and SKILL.md in this same diff keeps "unbacked on Aone; treat the Test Plan as unchecked". The design doc never mentions test-plan, so a maintainer or automation planning the remaining backing work from it concludes the gap is one item, or spends effort reconciling which of the two docs touched by this PR is correct.
| commit anchors, drift with no compare API). Only `pr-context` remains | |
| commit anchors, drift with no compare API). Of the flows deferred in this section, only `pr-context` remains |
中文说明
新增的 "Only pr-context remains unbacked" 断言与同一 diff 更新的用户文档矛盾——docs/users/features/code-review.md 在 pr-context 之外明确写着 "test-plan is unbacked"——也与事实不符:test-plan.ts 没有任何 Aone/平台检测路径,且同一 diff 中的 SKILL.md 仍保留 "unbacked on Aone; treat the Test Plan as unchecked"。设计文档通篇未提 test-plan,因此据它规划剩余支撑工作的维护者或自动化会以为缺口只剩一项,或者需要花精力判断本 PR 触碰的两份文档哪份正确。修复(见 suggestion):把句子限定为本节范围,如 "Of the flows deferred in this section, only pr-context remains unbacked"。
— qwen3.8-max via Qwen Code /review (v0.21.15)
| }); | ||
| }); | ||
|
|
||
| it('a recognized verdict beats an unrecognized lifecycle word in another key', () => { |
There was a problem hiding this comment.
[Suggestion] This test never exercises aoneCheckState's continue-scan/fallback branch: in all three of its cases (and the tolerant-spelling test) the recognized word sits in conclusion, the FIRST key of the iteration order ['conclusion', 'result', 'state', 'status'], or is the only key present. Witness (probe in a scratch tree): replacing the set-membership scan with "return the first non-empty normalized value" leaves all 97 Aone-suite tests green, while the discriminator flips — { result: 'inconclusive', status: 'passed' } classifies all_pass in the real code but all_pending under the mutant; { conclusion: 'inconclusive', state: 'failed' } reads any_failure vs all_pending. In production that mutant shape means a mixed-key gate payload reads as still-running and caps an Approve on a green MR indefinitely — the exact cost aoneCheckState's own comment warns about. Fix: add a case where the earlier key carries the unrecognized word, e.g. expect(classifyAoneChecks([{ name: 'test', result: 'inconclusive', status: 'passed' }])).toMatchObject({ class: 'all_pass', totalChecks: 1 });
中文说明
该测试从未触及 aoneCheckState 的继续扫描/回退分支:它的三个用例(以及宽容拼写测试)中被识别的词都位于迭代顺序 ['conclusion', 'result', 'state', 'status'] 的第一个键 conclusion 上,或是唯一出现的键。探针证据(scratch tree):把集合成员扫描替换为"返回第一个非空归一化值"后,全部 97 个 Aone 套件测试仍为绿色,而判别探针翻转——{ result: 'inconclusive', status: 'passed' } 在真实代码中分类为 all_pass,在变异体下变为 all_pending;{ conclusion: 'inconclusive', state: 'failed' } 则为 any_failure 对 all_pending。在生产中,该变异形状意味着混合键的门禁载荷会被读作"仍在运行",从而无限期地给绿色 MR 的 Approve 加上限——正是 aoneCheckState 自身注释警告的代价。修复:补一个较早键带未识别词的用例。
— qwen3.8-max via Qwen Code /review (v0.21.15)
There was a problem hiding this comment.
Deferred to the next round (this round was batch-bounded at the 3 Critical fixes + 5 small items). The finding stands as reported — the continue-scan branch is unpinned and the suggested { result: 'inconclusive', status: 'passed' } case is the right discriminator; it will land with its own mutation probe next round.
中文说明
顺延到下一轮(本轮批次上限为 3 条 Critical 修复 + 5 条小项)。该 finding 所述属实——继续扫描分支确实没有测试钉住,建议的 { result: 'inconclusive', status: 'passed' } 用例是正确的判别探针;下一轮会连同它自己的变异探针一起补上。
| }); | ||
| }); | ||
|
|
||
| it('reads tolerant key spellings for name and state', () => { |
There was a problem hiding this comment.
[Suggestion] This test's title promises tolerant spellings "for name and state", but only the STATE half is pinned: the context-keyed case asserts only { class: 'all_pass', totalChecks: 1 } (passing checks never collect names), and every other name assertion in the file uses name:-keyed inputs or the check-1 placeholder. aoneCheckName's tolerant name keys (context/check/title) are asserted nowhere — no other file references classifyAoneChecks (grep-verified). Witness (probe): a mutant narrowing the key list to ['name'] leaves all 97 tests green, and a context-keyed FAILED gate then degrades to check-N placeholders inside failedCheckNames/downgrade reasons (CI failing: check-2 instead of CI failing: lint), so Step 7 and human readers cannot identify which gate blocks — and the regression ships with CI green. Fix: expect(classifyAoneChecks([{ context: 'lint', status: 'failed' }])).toMatchObject({ class: 'any_failure', failedCheckNames: ['lint'] });
中文说明
该测试标题承诺对 name 与 state 都做宽容拼写,但实际只钉住了 STATE 一半:context 键用例只断言 { class: 'all_pass', totalChecks: 1 }(通过的检查不收集名字),文件中其他所有名字断言都使用 name: 键输入或 check-1 占位符。aoneCheckName 的宽容名字键(context/check/title)没有任何断言覆盖——也没有其他文件引用 classifyAoneChecks(grep 证实)。探针证据:把键列表收窄为 ['name'] 的变异体让全部 97 个测试保持绿色,此时 context 键的 FAILED 门禁会在 failedCheckNames/降级原因中退化为 check-N 占位符(CI failing: check-2 而不是 CI failing: lint),第 7 步和人类读者都无法识别是哪个门禁在阻塞——而回归会带着绿色 CI 上线。修复见建议断言。
— qwen3.8-max via Qwen Code /review (v0.21.15)
There was a problem hiding this comment.
Deferred to the next round (round batch bound). The gap is real — only the STATE half of the tolerant-spelling test is pinned, and a context-keyed FAILED gate degrading to check-N placeholders would ship with green CI; the suggested assertion lands next round with a key-list-narrowing mutation probe.
中文说明
顺延到下一轮(本轮批次上限)。缺口属实——宽容拼写测试只钉住了 STATE 一半,context 键的失败门禁退化为 check-N 占位符会带着绿色 CI 上线;下一轮会补上建议的断言,并附带收窄键列表的变异探针。
| }); | ||
|
|
||
| describe('aoneCommentToPresubmitComment (a1 → GitHub-shaped input)', () => { | ||
| it('falls back to `body` when `note` is absent (shape-drift tolerance)', () => { |
There was a problem hiding this comment.
[Suggestion] The mappers' text-field priority (note beats body; twin at comment-status.ts aoneCommentToStatusComment) is unpinned: every fixture in the Aone suites passes exactly ONE of the two keys (census: zero fixtures pass both keys, zero pass body: '' beside note), so the inverted mutant c.body ?? c.note ?? '' leaves all 112 tests green (probe-proven on both mappers) — despite AoneMrComment's doc naming the direction: "the text is in note, body stays empty on known a1 versions but is tolerated". If a1 ever serializes that tolerated empty body as body: '' (an empty string, not an absent key), the inverted order returns '' for every comment — ?? does not coalesce empty strings — so every qwen comment fails both recognition arms, all five dedup buckets come back empty, and Step 7 re-posts every finding already on the MR: the exact duplicate storm this backing exists to prevent, silently. Fix: one fixture passing both keys pins the direction in each suite, e.g. { note: '**[Critical]** x…' + FOOTER, body: '' } → expect(mapped.body).toContain('**[Critical]**').
中文说明
mapper 的文本字段优先级(note 优先于 body;孪生处在 comment-status.ts 的 aoneCommentToStatusComment)没有被测试钉住:Aone 套件中所有 fixture 都只传两个键中的一个(清点:没有任何 fixture 同时传两个键,也没有任何 fixture 在 note 旁传 body: ''),因此反转后的变异体 c.body ?? c.note ?? '' 让全部 112 个测试保持绿色(两个 mapper 均探针证实)——尽管 AoneMrComment 的文档写明了方向:"文本在 note 中,已知 a1 版本里 body 保持为空但予以容忍"。如果 a1 未来把这个被容忍的空 body 序列化为 body: ''(空字符串而非缺键),反转的顺序会对每条评论返回 ''——?? 不会对空字符串做合并——于是每条 qwen 评论在两个识别分支上都失败,五个去重桶全部为空,第 7 步会把 MR 上已有的每条 finding 重新发布:正是这套支撑要避免的重复风暴,且悄无声息。修复:在每个套件加一个同时传两个键的 fixture 即可钉住方向。
— qwen3.8-max via Qwen Code /review (v0.21.15)
There was a problem hiding this comment.
Deferred to the next round (round batch bound). The finding stands — no fixture passes both note and body, so the inverted c.body ?? c.note ?? '' mutant survives both mappers; next round adds a both-keys fixture to each suite (presubmit and comment-status) with the inversion probe.
中文说明
顺延到下一轮(本轮批次上限)。该 finding 属实——没有任何 fixture 同时传 note 与 body,因此反转后的 c.body ?? c.note ?? '' 变异体在两个 mapper 上都存活;下一轮会在两个套件(presubmit 与 comment-status)各补一个双键 fixture,并附带反转探针。
| 'sha-reviewed', | ||
| ); | ||
| expect(mapped.commit_id).toBe(''); | ||
| const live = aoneCommentToPresubmitComment( |
There was a problem hiding this comment.
[Suggestion] The mapper's parentNoteId → in_reply_to_id line (presubmit.ts aoneCommentToPresubmitComment) is pinned by no test: this describe never asserts mapped.in_reply_to_id, and the GitHub suite pins its equivalent with a mutation-verified reply-guard test. Witness (probe in a scratch tree): deleting the mapping line leaves all 24 presubmit.aone tests green; the end-to-end probe turns red — with the mapping, an own-account finding-shaped REPLY is excluded (total=1, overlap=0, blockOnExistingComments=false, a genuinely new finding at the same location posts); without it, the reply is mis-recognized as a posted finding (total=2, overlap=1, blockOnExistingComments=true), silently withholding the new finding. The one handler test that uses parentNoteId (line ~350) uses a different-account, marker-less reply, so it cannot catch the mutant. Fix: expect(aoneCommentToPresubmitComment({ id: 4, note: '**[Critical]** confirmed', path: 'a.ts', line: 42, parentNoteId: 1, author: { username: 'reviewer' } }, 'sha').in_reply_to_id).toBe(1) plus the undefined case.
中文说明
mapper 的 parentNoteId → in_reply_to_id 一行(presubmit.ts 的 aoneCommentToPresubmitComment)没有任何测试钉住:这个 describe 从未断言 mapped.in_reply_to_id,而 GitHub 套件用经过变异验证的回复守卫测试钉住了等价行为。探针证据(scratch tree):删除该映射行后全部 24 个 presubmit.aone 测试仍为绿色;端到端探针变红——有映射时,本账号 finding 形状的"回复"被排除(total=1, overlap=0, blockOnExistingComments=false,同一位置的全新 finding 正常发布);删除后,回复被误认为已发布的 finding(total=2, overlap=1, blockOnExistingComments=true),悄无声息地压住新 finding。唯一使用 parentNoteId 的 handler 测试(约第 350 行)用的是其他账号、无标记的回复,抓不到这个变异体。修复:补断言 in_reply_to_id 为 1 及 undefined 两种情况。
— qwen3.8-max via Qwen Code /review (v0.21.15)
There was a problem hiding this comment.
Deferred to the next round (round batch bound). The mapping line is indeed unpinned — deleting it leaves all 24 presubmit.aone tests green while mis-recognizing a finding-shaped reply as a posted finding; next round adds the in_reply_to_id assertions (set and undefined cases) with the line-deletion probe.
中文说明
顺延到下一轮(本轮批次上限)。该映射行确实没有测试钉住——删除它后全部 24 个 presubmit.aone 测试仍为绿色,而 finding 形状的回复会被误认为已发布的 finding;下一轮会补上 in_reply_to_id 断言(赋值与 undefined 两种情况),并附带删除映射行的探针。
|
@qwen-code /resolve |
Address round-1 review findings on the Aone backing of comment-status and presubmit: - extractStatusChecks no longer throws a TypeError when a1 answers a bare null to `mr status`; the payload now reads as the designed unreadable gate state (undefined), capping the verdict like a still-running check instead of crashing presubmit with no report. - comment-status and presubmit validate pr_number with fetch-pr's /^[1-9]\d*$/ grammar before Number() coercion, refusing '012'/'1e3'/ '0x1f'/' 12'/'12.0' tokens that would query a different MR than the caller's label carries. - Pin the two subject_type combinations no test covered (pathless comment WITH outdated:true; the live path+line shape) with mutation-probed assertions. - Align the --host describes with the sibling commands' detection wording (omission no longer promises github.com), name the real bucket (`resolved`) in the review skill's Aone dedup note, and scope the design doc's remaining-unbacked claim to its own section.
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 1/100 轮)。改动内容与我反驳保留之处如下: Autofix round summary — PR #9627 (review round 2)Commit No base conflict ( Critical findings — reproduced, fixed, mutation-probedFor each claim that current behavior is wrong, the defect was reproduced on the unmodified code first (focused failing test, recorded red), then fixed minimally, then probe-verified (temporarily reverting the fix or applying the reviewer's mutant makes the new test fail again; restoring returns green).
Suggestions addressed this round
Deferred to the next round (replies posted on each thread)
All four are genuine test-gap findings verified as real by the reviewer's probes; they are deferred only for this round's size bound, not declined. Needs maintainer/workflow action (cannot be done from this bot)
Review-level / issue-level items
Environment repair (disclosed)The checkout's VerificationEvery command below was actually run this round (nothing asserted from reading the diff alone):
中文说明Autofix 轮次总结 — PR #9627(review 第 2 轮)
无 base 冲突( Critical findings——已复现、已修复、已做变异探针对每个"当前行为有误"的论断,都先在未修改的代码上复现缺陷(写出聚焦的失败测试并记录红色),再做最小修复,然后做探针验证(临时回退修复或套用 reviewer 的变异体,新测试会再次变红;恢复后变绿)。
本轮处理的 Suggestion
顺延到下一轮(各线程已回复)
这四条都是经 reviewer 探针证实的真实测试缺口 finding;仅因本轮批次上限顺延,并非拒绝处理。 需要 maintainer/workflow 操作(本 bot 无法执行)
Review 级 / Issue 级条目
环境修复(披露)本 checkout 的 验证以下每条命令都是本轮真实执行过的(没有任何仅凭读 diff 的断言):
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 attempted to resolve merge conflicts but the run did not complete successfully. Check the workflow run for full logs. |
|
@qwen-code /resolve |
# Conflicts: # docs/design/2026-08-13-review-platform-provider-abstraction.md # docs/users/features/code-review.md # packages/core/src/skills/bundled/review/SKILL.md
…nto HEAD # Conflicts: # packages/core/src/skills/bundled/review/SKILL.md
|
Qwen Code attempted to resolve merge conflicts but the run did not complete successfully. Check the workflow run for full logs. |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. 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.
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
docs/design/2026-08-15-review-aone-provider.md:68 — [review] fact-table row omits parentNoteId/isDraft consumed by the landed code beside the Landed notepackages/cli/src/commands/review/presubmit.aone.test.ts:243 — [probe] ensureAoneAuthenticated gate unpinned by the new Aone presubmit suite (mutant probe: gate deleted, suite stays green)
中文说明
仅完成部分审查,审查缺口已披露。 建议见行内评论。 1 条建议级发现无法锚定到改动行,已丢弃;此处无需进一步处理。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
收敛姿态下延后(第 2 轮,非阻断)——已记录,本轮不要求修改:共 2 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.21.15)
| const isSelfPr = | ||
| mrAuthor !== '' && mrAuthor.toLowerCase() === me.toLowerCase(); |
There was a problem hiding this comment.
[Suggestion] R1-7: Carried from round 1 — still stands: this self-PR detection (MR author vs a1 auth whoami, feeding downgradeApprove/downgradeRequestChanges) implements the entire scope of open sibling issue (9616) — its body asks for exactly this wiring — but the PR description still says only "Closes #9613". Issue (9616) stays open and tracked as unimplemented while its implementation ships here, and the doc caveat (9616) cites is deleted by this same diff. Fix: edit the PR description to also close the sibling issue ("Closes #9616") — a GitHub-side write the author/maintainer must perform.
中文说明
承接第 1 轮——仍然存在:这个 self-PR 检测(MR 作者与 a1 auth whoami 对比,驱动 downgradeApprove/downgradeRequestChanges)完整实现了开放的同源 issue (9616) 的范围——该 issue 要求的正是这套接线——但 PR 描述仍只写了 "Closes #9613"。实现已在本 PR 落地,issue (9616) 却仍以未实现的状态开放着,且 (9616) 所引用的文档警告段落已被本 diff 删除。修复:编辑 PR 描述以同时关闭该兄弟 issue("Closes #9616")——这是需要作者/维护者执行的 GitHub 侧写操作。
— qwen3.8-max via Qwen Code /review (v0.21.15)
There was a problem hiding this comment.
Escalated to the maintainer — this fix is a GitHub-side write this loop cannot perform, and its premise shifted during this round's conflict resolution. The self-MR wiring this finding names (MR author vs a1 auth whoami feeding downgradeApprove/downgradeRequestChanges) landed on main via #9629 — its commit message explicitly names #9616 — and this branch merged it in this round (the merge also consolidated #9629's mrPresubmitFacts into this PR's getMrAuthorAndHead). Whether #9616 is still open is not readable from inside this loop (no GitHub access). Question for the maintainer: is #9616 still open? If yes, please add Closes #9616 to this PR's description (or confirm #9629 closed it); if #9629 already closed it, this finding is moot and can be dismissed.
升级给维护者——该修复是 GitHub 侧写操作(编辑 PR 描述),本循环无法执行,且其前提在本轮冲突解决中已发生变化:本发现点名的 self-MR 接线(MR 作者与 a1 auth whoami 对比,驱动 downgradeApprove/downgradeRequestChanges)已经通过 #9629 落地 main——其提交信息明确点名 #9616——并在本轮合并进本分支(合并同时将 #9629 的 mrPresubmitFacts 整合进本 PR 的 getMrAuthorAndHead)。#9616 当前是否仍开放,在本循环内无法读取(无 GitHub 访问权限)。给维护者的问题: #9616 是否仍然开放?若是,请在本 PR 描述中补充 Closes #9616(或确认已由 #9629 关闭);若 #9629 已将其关闭,则本发现已失效,可直接关闭。
| }); | ||
| }); | ||
|
|
||
| it('a recognized verdict beats an unrecognized lifecycle word in another key', () => { |
There was a problem hiding this comment.
[Suggestion] R1-10: Carried from round 1 — still stands (deferred by the round-1 reply, not addressed since): the classifyAoneChecks tests still never exercise aoneCheckState's continue-scan/fallback branch — no case feeds an unrecognized value in an EARLIER key beside a recognized verdict in a LATER key (the suggested { result: 'inconclusive', status: 'passed' } shape is still absent). If a regression of the key-scan order returned on the first PRESENT key regardless of recognition, every current test would stay green while a gate whose only readable verdict sits in a later key silently degrades to pending.
expect(
classifyAoneChecks([
{ name: 'test', result: 'inconclusive', status: 'passed' },
]),
).toMatchObject({ class: 'all_pass' });中文说明
承接第 1 轮——仍然存在(第 1 轮回复中顺延,此后未处理):classifyAoneChecks 测试仍未覆盖 aoneCheckState 的继续扫描/回退分支——没有任何用例在"较早键为不可识别值、较晚键带可识别结论"的形状上测试(建议的 { result: 'inconclusive', status: 'passed' } 形状仍缺失)。如果键扫描顺序回归为"遇到第一个存在的键就返回、不论是否可识别",现有测试将全绿,而唯一可读结论位于较晚键的门禁会悄悄降级为 pending。修复:补上面的继续扫描用例。
— qwen3.8-max via Qwen Code /review (v0.21.15)
| }); | ||
| }); | ||
|
|
||
| it('reads tolerant key spellings for name and state', () => { |
There was a problem hiding this comment.
[Suggestion] R1-11: Carried from round 1 — still stands (deferred by the round-1 reply, not addressed since): this test's title promises tolerant spellings "for name and state", but only the STATE half is pinned — the context-keyed case is a PASSING check asserting only { class: 'all_pass', totalChecks: 1 }, and passing checks never collect names, so context-keyed NAME extraction stays unpinned. A regression that stops reading context for the check name ships green: a context-keyed FAILED gate degrades to check-N placeholders in failedCheckNames, and the operator log names gates nobody can match to the platform's UI.
expect(
classifyAoneChecks([{ context: 'lint', state: 'failed' }]),
).toMatchObject({
class: 'any_failure',
failedCheckNames: ['lint'],
});中文说明
承接第 1 轮——仍然存在(第 1 轮回复中顺延,此后未处理):该测试标题承诺 "for name and state" 的宽容键名,但只有 STATE 一半被钉住——context 键的用例是一个"通过"的门禁,只断言 { class: 'all_pass', totalChecks: 1 },而通过的门禁从不收集名称,因此 context 键的名称提取仍未被钉住。若回归不再从 context 读取门禁名称,测试仍全绿:context 键的"失败"门禁会退化为 failedCheckNames 中的 check-N 占位符,运维日志里的门禁名无法与平台 UI 对应。修复:补上面的 context 键失败用例。
— qwen3.8-max via Qwen Code /review (v0.21.15)
| }); | ||
|
|
||
| describe('aoneCommentToPresubmitComment (a1 → GitHub-shaped input)', () => { | ||
| it('falls back to `body` when `note` is absent (shape-drift tolerance)', () => { |
There was a problem hiding this comment.
[Suggestion] R1-12: Carried from round 1 — still stands (deferred by the round-1 reply, not addressed since): the mappers' text-field priority (note beats body; twins in presubmit.ts aoneCommentToPresubmitComment and comment-status.ts aoneCommentToStatusComment) is still unpinned — no fixture passes BOTH keys, so the inverted mutant c.body ?? c.note ?? '' leaves every test green. If a1 ever serializes the tolerated empty body as body: '' (an empty string, not an absent key), the inverted order returns '' for every comment — ?? does not coalesce empty strings — every qwen comment fails both recognition arms, all five dedup buckets come back empty, and Step 7 re-posts every finding already on the MR: the exact duplicate storm this backing exists to prevent, silently.
const mapped = aoneCommentToPresubmitComment(
{
id: 5,
note: `**[Critical]** both keys\n\n${FOOTER}`,
body: '',
path: 'a.ts',
line: 42,
author: { username: 'reviewer' },
},
'sha-reviewed',
);
expect(mapped.body).toContain('**[Critical]** both keys');中文说明
承接第 1 轮——仍然存在(第 1 轮回复中顺延,此后未处理):mapper 的文本字段优先级(note 优先于 body;孪生处在 presubmit.ts 的 aoneCommentToPresubmitComment 与 comment-status.ts 的 aoneCommentToStatusComment)仍未被钉住——没有任何 fixture 同时传两个键,因此反转变异体 c.body ?? c.note ?? '' 能让全部测试保持绿色。如果 a1 未来把这个被容忍的空 body 序列化为 body: ''(空字符串而非缺键),反转的顺序会对每条评论返回 ''——?? 不会对空字符串做合并——于是每条 qwen 评论在两个识别分支上都失败,五个去重桶全部为空,第 7 步会把 MR 上已有的每条 finding 重新发布:正是这套支撑要避免的重复风暴,且悄无声息。修复:在每个套件补一个双键 fixture(如上)钉住方向。
— qwen3.8-max via Qwen Code /review (v0.21.15)
| expect(mapped.body).toContain('via Qwen Code /review'); | ||
| }); | ||
|
|
||
| it('an OUTDATED thread rides an empty commit_id (the stale bucket)', () => { |
There was a problem hiding this comment.
[Suggestion] R1-13: Carried from round 1 — still stands (deferred by the round-1 reply, not addressed since): the mapper's parentNoteId → in_reply_to_id line in aoneCommentToPresubmitComment is pinned by no test — this suite never asserts mapped.in_reply_to_id, while the GitHub suite pins its equivalent with a mutation-verified reply-guard test. Deleting the mapping line leaves every presubmit.aone test green while a finding-shaped own REPLY is no longer excluded via thread-root resolution and is mis-recognized as a posted finding (overlap: 1, blockOnExistingComments: true), silently withholding a genuinely new finding at the same location.
expect(
aoneCommentToPresubmitComment(
{
id: 4,
note: '**[Critical]** confirmed',
path: 'a.ts',
line: 42,
parentNoteId: 1,
author: { username: 'reviewer' },
},
'sha-reviewed',
).in_reply_to_id,
).toBe(1);中文说明
承接第 1 轮——仍然存在(第 1 轮回复中顺延,此后未处理):mapper 的 parentNoteId → in_reply_to_id 一行(aoneCommentToPresubmitComment)没有任何测试钉住——本套件从未断言 mapped.in_reply_to_id,而 GitHub 套件用经过变异验证的回复守卫测试钉住了等价行为。删除该映射行后全部 presubmit.aone 测试仍为绿色,而 finding 形状的"本账号回复"不再通过线程根解析被排除,会被误认为已发布的 finding(overlap: 1、blockOnExistingComments: true),悄无声息地压住同一位置的全新 finding。修复:补上面的 in_reply_to_id 断言(含 undefined 情形)。
— qwen3.8-max via Qwen Code /review (v0.21.15)
#9629 landed the self-MR half of the Aone presubmit while this branch carried the full backing; the merge consolidates the two designs: - one MR-facts seam: getMrAuthorAndHead subsumes mrPresubmitFacts (tolerant aoneAccountName author read + the canonical aoneHeadSha head read), shared by presubmit and comment-status - the Aone presubmit runner keeps the full backing (merge-gate classification + comment dedup) and adopts #9629's argv gates and single-spawn whoami gate (ensureAoneAuthenticated doubles as the account read, before the MR fetch) - the shared report writer supersedes the PresubmitReport interface — both platform paths emit one literal through one function - docs/SKILL describe the full backing; the SKILL.md revert guard and the Aone presubmit suite are re-pinned on the merged wording
Four mutation-verified pins on the existing Aone backing, each closing a round-2 Suggestion: - classifyAoneChecks: the continue-scan cell of aoneCheckState — an unrecognized value in an earlier key beside a recognized verdict in a later key reads the verdict, not pending (a first-present-key mutant now fails) - classifyAoneChecks: a context-keyed FAILED gate carries its name — the passing context-keyed case pinned nothing because passing gates never collect names - both comment mappers: `note` beats `body` when BOTH keys are present (`??` does not coalesce `body: ''`, so an inverted priority would blank every recognition signal and re-post the whole review) - aoneCommentToPresubmitComment: parentNoteId maps onto in_reply_to_id, including the absent-stays-unset half No source changes; each pin fails under its named mutant and passes on the current code.
d605814
|
@qwen-code /triage |
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
|
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: 145 passed · 0 failed · 145 total Flakiness gate: ✅ 6 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:145 通过 · 0 失败 · 145 总计 抖动门:✅ 6 changed test file(s) x 5 identical rounds, no divergence Verification reportPR 9627 Deep Verification (follow-up round) — feat(review): back comment-status and presubmit for Aone Code targetsVerdict: 中文 — 判定:✅ 通过 · 可合入(agent 判定)· 跟进轮结论:
Previous-finding status (follow-up round)The previous round verified head
No finding from the previous round worsened; none was declined. Central claim and A/B proofCentral claim. On an Aone Code target, the two comment-aware pre-write flows run instead of being skipped/neutral: A/B design. The a1 transport is
The a1 call log also pins the transport contract on head: one whoami per presubmit run (the gate doubles as the account read), one Secondary claim — GitHub path unchanged. The shared cores ( Reviewer Test Plan, per step.
CorrectionsNone — re-reading this round's measurements against the previous report and the bot threads found no inaccurate claim to correct (the previous round's base-behavior descriptions were re-measured and hold). FindingsNo blocking findings. Three informational notes:
Mutation matrix (vacuity of the new pins)Baseline green first: 271/271 across the five pin-relevant files (
8/8 killed, 0 survivors. M8 doubles as the harness-level positive control (the runner can make suites fail) and fails with expected-vs-actual assertion messages, not import/crash artifacts. The Targeted gates
Delta probes this round (the d605814 merge and the moved base)20/20 scripted checks (
Not covered
MethodologyEnvironment: the CI verify container ( Flakiness gate logEvidence imagesHarness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
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. ✅
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: reverse audit — stopped before round 5 by the review time budget.
Deferred under the convergence posture (round 8, not a blocker) — recorded, not requested in this round:
docs/design/2026-08-15-review-aone-provider.md:65 — [review] fact table omits parentNoteId/isDraft/createdAt the landed code consumespackages/cli/src/commands/review/comment-status.ts:374 — [probe] created_at mapper arms unpinned — drop/inversion mutants ship greenpackages/cli/src/commands/review/comment-status.aone.test.ts:8 — [probe] comment-status cwd-origin routing arm unpinned — mutant ships greenpackages/cli/src/commands/review/presubmit.aone.test.ts:93 — [probe] presubmit cwd-origin routing arm unpinned — mutant ships greenpackages/cli/src/commands/review/presubmit.ts:1274 (+2 locations) — [review] --host describe claims family-wildcard a1 routing the canonical-only guard rejectspackages/cli/src/commands/review/comment-status.aone.test.ts:236 — [probe] 'probe never handed a SHA' pin misses merge-base — anchor mutant ships greenpackages/cli/src/commands/review/lib/platform/aone.test.ts:1753 — [probe] non-numeric-id drop in listMrComments unpinned — guard mutant ships greenpackages/cli/src/commands/review/comment-status.aone.test.ts:313 — [probe] resolveMe empty-string fallback arm unpinned — mutant ships greenpackages/cli/src/commands/review/presubmit.aone.test.ts:386 — [probe] foreign-account HTML-marker shape unpinned on the Aone filter — mutant ships greenpackages/cli/src/commands/review/presubmit.ts:1173 — [probe] merge gates classified as CI — mislabeled downgrade, circular approver_number cap
中文说明
仅完成部分审查,审查缺口已披露。
未审查:反向审计——评审时间预算不足,未能开始第 5 轮。
收敛姿态下延后(第 8 轮,非阻断)——已记录,本轮不要求修改:共 10 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| const comments = listMrComments(mrId, ownerRepo).map( | ||
| aoneCommentToStatusComment, | ||
| ); |
There was a problem hiding this comment.
[Critical] The Aone thread index this PR adds promotes qwen's own review summary comments to blocker threads that can never be cleared again. submitAoneReview posts the review body as a pathless global comment (no --file/--line), and compose renders body-listed Criticals with the literal **[Critical]** prefix intact; the next --comment round then runs buildThreadStatuses over that root, whose isBlockerBody check has a carriesBlockerSignal channel matching /\[critical\]/ with no authorship gate — so the pipeline's own summary is counted in summary.blockers. Because that thread has no path and is never outdated, Step 6's re-check can never rule it fixed (no location to read → cannot tell), and cannot-tell forbids Approve: the pipeline manufactures one permanent, irrefutable blocker per round that body-lists a Critical. Today the forced context-unavailable cap masks verdict gating on Aone, but the mis-stated blocker set ships in the report JSON now, and any future pr-context backing inherits it.
Witness — probe driving the real handler with an own-account pathless summary carrying **[Critical]** R1-1: …:
BASE: {"summaryBlockers":1, threads:[{rootId:500, author:"reviewer", isBlocker:true, path:"", isFileLevel:true, outdated:false}]}
MUTANT (own-account exclusion from the ungated channel): summaryBlockers 0
Fix sketch: exclude the pipeline's own summary from blocker promotion (or from the index) before buildThreadStatuses — e.g. drop pathless roots authored by the reviewing account or carrying the via Qwen Code /review attribution footer, mirroring the footer match presubmit's qwenComments filter already uses.
中文说明
本 PR 新增的 Aone 线程索引会把 qwen 自己的评审总结评论提升为永远无法清除的 blocker 线程。submitAoneReview 以无路径的全局评论形式发布评审正文(不带 --file/--line),而 compose 渲染正文级 Critical 时保留字面 **[Critical]** 前缀;下一轮 --comment 的 buildThreadStatuses 会对该根评论运行 isBlockerBody,其 carriesBlockerSignal 通道以 /\[critical\]/ 匹配且不做作者门禁——于是流水线自己的总结被计入 summary.blockers。该线程没有路径、永不失效,Step 6 复查永远无法判其已修复(无位置可读 → cannot tell),而 cannot-tell 禁止 Approve:每当一轮在正文中列出 Critical,流水线就自造一个永久且不可反驳的 blocker。当前 Aone 的强制 context-unavailable 上限掩盖了裁决层影响,但错误的 blocker 集合现在就写进报告 JSON,未来的 pr-context 支撑落地后会直接继承。
探针证据(驱动真实 handler,投喂携带 **[Critical]** R1-1: … 的本账号无路径总结):未修改代码 summaryBlockers: 1(线程无路径、文件级、永不失效、计为 blocker);加入「本账号评论排除」变异后 summaryBlockers: 0。
修复思路:在 buildThreadStatuses 之前把流水线自己的总结排除出 blocker 提升(或索引)——例如丢弃由评审账号发布、或携带 via Qwen Code /review 署名尾注的无路径根评论,复用 presubmit 的 qwenComments 过滤器已有的尾注匹配。
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
🤖 Addressed the latest review feedback (round 6/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 6/100 轮)。改动内容与我反驳保留之处如下: Autofix round — address review (critical-only)Critical-only mode is active (5 change-producing rounds already complete), so this Feedback triage[Critical] rc:3837070145 — the Aone thread index promotes the pipeline's own review summary to a permanent blocker → RESOLVED IN CODE. Reproduced first on the current code: Fix (minimal, root cause): in [Suggestion] rv:5000526830 #1 — The string-prefix check ( [Suggestion] rv:5000526830 #2 — clarify that The requested clarification already exists at the single provider read site, Notes
VerificationCommands actually run this round (all from the repo root unless noted):
中文说明Autofix 轮次 — 处理评审(仅 Critical 模式)当前已激活仅处理 Critical 的模式(此前已完成 5 个产生改动的轮次),因此本轮只处理唯一一条可执行的 Critical。审计区中被延后的非 Critical 条目按要求未做改动。 反馈分类[Critical] rc:3837070145 — Aone 线程索引把流水线自己的评审总结提升为永久 blocker → 已在代码中解决。 先在现有代码上复现: 修复(最小、治本):在 [Suggestion] rv:5000526830 #1 — 该字符串前缀检查( [Suggestion] rv:5000526830 #2 — 说明 所请求的说明已存在于唯一的 provider 读取点 说明
验证本轮实际执行的命令(除注明外均在仓库根目录):
Deferred non-Critical feedbackCritical-only mode is active: 5 change-producing rounds are complete. 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. (
中文说明已进入仅处理 Critical 的模式:已完成 5 个产生改动的轮次。本轮可执行区域已排除下方非 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 /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: 145 passed · 0 failed · 145 total Flakiness gate: ✅ 7 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:145 通过 · 0 失败 · 145 总计 抖动门:✅ 7 changed test file(s) x 5 identical rounds, no divergence Verification report<!-- qwen-triage:verify --> Sandboxed verification: ✅ passed — merge-ready (agent verdict) 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: 145 passed · 0 failed · 145 total 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:145 通过 · 0 失败 · 145 总计 Verification reportPR 9627 Deep Verification (follow-up round 2) — feat(review): back comment-status and presubmit for Aone Code targetsVerdict: 中文 — 判定:✅ 通过 · 可合入(agent 判定)· 跟进轮 2结论:
Previous-finding status (follow-up round)The previous round verified head
No finding from the previous round worsened; none was declined. Central claim and A/B proofCentral claim (carried). On an Aone Code target, the two comment-aware pre-write flows run instead of being skipped/neutral: Delta claim (this round's commit). The pipeline's own pathless Aone summary is excluded from blocker promotion (it would otherwise be a permanent, self-made blocker: a pathless thread never goes outdated and gives Step 6 no location to re-read), while path-bearing own findings and other accounts' pathless blockers keep their promotion, and an unknown identity excludes nothing. A/B design. The a1 transport is
Delta load-bearing proof (three-arm separation). Reverting ONLY the guard hunk in the built head dist (single-occurrence anchor, interface-preserving) and re-running the b1 payload through the real CLI reproduces the hazard; restoring (sha256-verified) closes it again —
Secondary claim — GitHub path unchanged. The shared cores are exercised untouched by the pre-existing suites: 101 review test files, 4676 pass / 0 fail at head; the M8 positive control (stale-priority inversion in the shared core) killed 41 tests across both platforms' suites, proving the Aone tests traverse the same core. Reviewer Test Plan, per step.
CorrectionsNone — re-reading this round's measurements against the previous report and the bot threads found no inaccurate claim to correct. FindingsNo blocking findings. Three informational notes:
Mutation matrix (vacuity of the pins)Baseline green first: 273/273 across the five pin-relevant files (
9/11 killed, 2 survivors, both adjudicated as coverage gaps with named pinning fixtures; no mutant regressed a kill; M8 is the harness-level positive control. The delta's own new test (M9) pins the reported shape by construction; the three-arm dist control above pins it at the wire level. Targeted gates
Not covered
MethodologyEnvironment: the CI verify container ( Evidence imagesHarness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification Flakiness gate logEvidence imagesHarness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
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 and its suite did not run locally.
Not reviewed: reverse audit — reached the 5-round cap without two consecutive dry rounds; every round surfaced new candidates, all verified.
Not explored to full depth (tool budget reached): "agent invariant-c (packages/cli/src/commands/review/comment…": none — no check was cut short.; chunk 2: executing comment-status.aone.test.ts under vitest (review worktree has no node_modules); mock-graph soundness verified statically against HEAD sources and th….
Deferred under the convergence posture (round 9, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/review/comment-status.ts:190 — [probe] ownPathlessRoot meLc !== '' clause unpinned — deletion mutant ships greenpackages/cli/src/commands/review/lib/platform/aone.ts:682 — [probe] aoneAccountName empty-key fall-through guard unpinned — mutant ships green across 283 testspackages/cli/src/commands/review/comment-status.ts:192 — [probe] ownPathlessRoot case-folding unpinned — fold-dropping mutant ships 38/38 green
Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had none either, so the next review re-reads the whole diff and will keep doing so until a round's marker carries an anchor again. (Stated, not acted on — this changes nothing about what the round posts.)
中文说明
仅完成部分审查,审查缺口已披露。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查:reverse audit — reached the 5-round cap without two consecutive dry rounds; every round surfaced new candidates, all verified。
未探索到全部深度(达到工具调用预算):"agent invariant-c (packages/cli/src/commands/review/comment…":none — no check was cut short.;chunk 2:executing comment-status.aone.test.ts under vitest (review worktree has no node_modules); mock-graph soundness verified statically against HEAD sources and th…。
收敛姿态下延后(第 9 轮,非阻断)——已记录,本轮不要求修改:共 3 条(原文未翻译,列表见上方英文部分)。
机制健康:本轮未能干净收尾,因而扣留了增量锚点,而它恢复到的那一轮也没有锚点,因此下一次评审将重读整个 diff——并会一直如此,直到某一轮的标记重新带上锚点。(仅陈述,不据此行动——这不改变本轮发布的任何内容。)
— qwen3.8-max via Qwen Code /review (v0.22.0)
… backing Merges origin/main (ec8a8a1) into the Aone dedup PR. Main's #9621 backed pr-context on Aone while this PR (#9627) backed comment-status + presubmit dedup; the two are complementary, so the merged state backs pr-context, comment-status, and presubmit (self-PR, head drift, merge-gate CI, and existing-comment dedup) together. Conflict resolutions: - aone.ts: kept the merged aoneHeadSha docstring naming the real read sites (getMrAuthorAndHead + getReviewContext); fixed getReviewContext's authorLogin to use the tolerant aoneCommentAuthor (the merge left author typed unknown but accessed .username). - submit.ts: dropped the now-unused aoneWrite param from compose() — with pr-context backed the forced context-unavailable cap is gone; an Aone run's contextUnavailable claim carries the same meaning as GitHub's, and approve fires when the run read the MR's context. - SKILL.md: the Aone section now lists pr-context + comment-status + presubmit as a1-backed, keeps main's anchor-check failure shape and the approve-fires-when-context-read wording, and carries this PR's dedup paragraph in place of the stale "no dedup backing yet" caveat. - code-review.md: same combination — pr-context backed, comment-status + presubmit a1-backed with dedup, approve fires when context read. Verified: npm run build green, tsc clean on packages/cli, 745 review tests pass (presubmit GitHub+Aone, comment-status, aone/aone-client, submit GitHub+Aone, pr-context, cleanup), and core SKILL.test.ts 30/30.
…nto fix/review-aone-dedup-backing
|
@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: 328 passed · 0 failed · 328 total Flakiness gate: ✅ 7 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:328 通过 · 0 失败 · 328 总计 抖动门:✅ 7 changed test file(s) x 5 identical rounds, no divergence Verification reportPR 9627 Deep Verification (follow-up round 3) — feat(review): back comment-status and presubmit for Aone Code targetsVerdict: 中文 — 判定:✅ 通过 · 可合入(agent 判定)· 跟进轮 3结论:
Previous-finding status (follow-up round)The previous round verified head
No finding from the previous round worsened; none was declined. Central claim and A/B proofCentral claim (carried). On an Aone Code target, the two comment-aware pre-write flows run instead of being skipped/neutral: Round delta. The two commits since the last verified head are merges (main into branch, branch sync); the source delta is not locally separable at depth 2, so the aggregate diff was verified. The NEW verification surface this round is the interaction with #9621, which the merge pulled into both arms: this PR changed A/B design. The a1 transport is
Delta load-bearing proof (three-arm separation, re-run this round). Reverting ONLY the guard hunk in the built head dist (single-occurrence anchor, interface-preserving) and re-running the b1 payload through the real CLI reproduces the hazard; restoring (sha256-verified) closes it again —
Secondary claim — GitHub path unchanged. The shared cores are exercised untouched by the pre-existing suites: 101 review test files, 4716 pass / 4 skipped / 0 fail at head; the M8 positive control (stale-priority inversion in the shared core) killed 41 tests across both platforms' suites, proving the Aone tests traverse the same core. Reviewer Test Plan, per step.
CorrectionsNone — re-reading this round's measurements against the previous report and the bot threads found no inaccurate claim to correct. Mutation matrix (vacuity of the pins)Baseline green first: 290/290 across the five pin-relevant files (
9/11 killed, 2 survivors, both adjudicated as coverage gaps with named pinning fixtures (green on head, red under their mutant, sibling green in both directions); no mutant regressed a kill; M8 is the harness-level positive control. The delta's own new test (M9) pins the reported shape by construction; the three-arm dist control above pins it at the wire level. The two survivors and their adjudication are byte-for-byte the same outcome as the previous round — the suites and the guards were unchanged by this round's merges, and the re-run confirms it rather than assuming it. FindingsNo blocking findings. Five informational notes:
Not covered
MethodologyEnvironment: the CI verify container ( Flakiness gate logEvidence imagesHarness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
…reference files (QwenLM#9804) * refactor(review): split SKILL.md into core body plus verdict-gated references (QwenLM#9787) The bundled review skill's SKILL.md (381,984 bytes, ~95k tokens) was injected whole on every run, although large stretches are conditional territory a given run never touches. Split it into a core body plus reference files the orchestrator reads on demand, gated on the parse-args verdict it already holds: - references/posting.md — whole Step 7 (authorisation gate, presubmit, anchors, submit, 422/head-drift recovery, publish-assets). Loaded only when posting is live (comment.effective or a same-session post request; PR + high only). Its compose-state field list relocates verbatim to Step 6's Verdict section, because a report-only run still writes that state for compose-review without ever loading posting.md. - references/persistence.md — whole Step 8 (tail batching, report, artifact registration, incremental cache). Loaded before Step 8 on every run except cross-repo lightweight mode. - references/aone.md — the self-contained Aone blocks of Step 1 (clone and two-host rules, a1-backed surface, the five submit failure shapes, dedup shape notes). Loaded before match-remote when the host/meta says Aone. The split moves whole steps; incident-backed rules stay with the step they guard. The write prohibition and the posting gates remain in the injected core so they bind runs that never load a file. No enterprise.md: the GHE host notes are sentences woven into universal paragraphs, and extracting them would strip rules from steps that remain in core. Injected prompt: 381,125 -> 304,427 body bytes. Typical non-posting runs (local/file/PR, any effort) save ~58 KB (~15%); lightweight runs ~77 KB (~20%); posting runs load posting.md back and save only the Aone block. The issue's "roughly a third" estimate is unreachable under its own whole-step guardrail — Steps 1 and 6 dominate the core and interleaving forbids fragmenting them; Step 5 / Step 3C effort-gated splits are the natural follow-up. Drive-by, verified against QwenLM#9627's revert-guard test and the a1 implementation: three stale sentences still claiming comment-status "has no Aone backing" are aligned with the a1-backed behavior that landed in QwenLM#9627. Tests: SKILL.test.ts revert guards now govern the full corpus (SKILL.md + references), with new pins for the gates, the core-retained invariants and the no-duplication invariant; run-skill-parity reads the corpus oracle; bundled-skills integration pins the shipped reference files. Verified by build + bundle, all review-skill suites, and a real-model E2E run of the split skill (verdict-gated reads observed: persistence.md loaded before Step 8, posting.md and aone.md correctly skipped). * fix(review): drop uninterpolated template tokens from skill references (QwenLM#9804) * test(review): guard stems oracle by persistence.md, pin gate clauses to bullets (QwenLM#9804) * fix(review): close Step 7 reference doc gaps from reverse audit (QwenLM#9804) --------- Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>











What this PR does
When the review skill reviews an Aone Code MR, the two comment-aware pre-write flows now work instead of being skipped. The comment-status subcommand builds the existing-comment thread index from the a1 CLI (threading, resolved/outdated state, blocker signal), and the presubmit subcommand classifies prior qwen comments against the findings about to post, detects a self-MR, reads the MR's merge-gate/CI state, and checks head drift — emitting the exact same JSON report schema the GitHub path produces, because both platforms feed the same pure classification cores. The Aone shape maps onto the GitHub inputs: replies ride the parent-note field, a resolved thread lands in the engaged bucket like a replied-to thread on GitHub, a thread whose line no longer maps after an amend lands in the stale bucket (so a genuinely new finding at the rewritten line still posts), comments carry no commit anchor (per-thread code facts degrade to
unknown), and head drift — with no compare API on Aone — fails the anchor-risk ruling safe so the skill re-reviews the amended head. The skill docs, user docs, and both design docs are updated: the Aone skip list shrinks to the one remaining unbacked flow, and the two repeat-round caveats (no dedup, no self-PR detection) are removed.Why it's needed
Under AGit-Flow a CR is one commit amended in place, so amend → re-review is the normal loop. Without this backing, every repeat
--commentround on an Aone MR re-posted every still-valid finding as a new comment — the MR accumulated a duplicate of the whole review per round, burying genuinely new findings and flooding the discussion gate — and a review of the user's own MR got no self-PR downgrade. This closes the Phase 3 "dedup/self-PR backing for Aone" item from the platform-provider design doc.Reviewer Test Plan
How to verify
The GitHub path is unchanged — its behavior is pinned by the pre-existing suites passing untouched (the shared-core extraction is line-for-line, audited by a removed-behavior review pass). For the Aone path:
cd packages/cli && npx vitest run src/commands/review/comment-status.aone.test.ts src/commands/review/presubmit.aone.test.ts src/commands/review/lib/platform/aone.test.ts— expect all green.(path, line)lands inoverlapand setsblockOnExistingComments; anoutdatedthread lands instale(re-postable); aclosedthread lands inresolved(engaged, not blocking); an unpublished draft never reaches either bucket.--commentfrom inside a clone (origin ongitlab.alibaba-inc.com) and confirm the second round drops the already-reported findings (logged with the existing comment id) and posts only the new ones.Evidence (Before & After)
N/A — non-UI change (CLI subcommands + skill docs); no TUI output changes.
Tested on
Environment (optional)
Unit tests only (
npx vitest runin packages/cli,npm run build,npx tsc --noEmit). Note: live a1 probing was not possible on the author's machine (corporate SSO credential failure), so a1 payload shapes follow the facts table verified against real calls in the parent design doc, with tolerant extraction and fail-closed classification everywhere.Risk & Scope
Linked Issues
Closes #9613
中文说明
这个 PR 做了什么
审查 Aone Code MR 时,两个依赖评论列表的写前流程从"跳过"变为可用。comment-status 子命令通过 a1 CLI 构建已有评论的线程索引(线程归属、已解决/已失效状态、blocker 信号);presubmit 子命令把此前的 qwen 评论与即将发布的 findings 对账、检测 self-MR、读取 MR 的合并门禁/CI 状态并检查 head 漂移——输出与 GitHub 路径完全一致的 JSON 报告结构,因为两个平台复用同一套纯分类核心。Aone 形状到 GitHub 输入的映射:回复走 parent-note 字段;已解决的线程与 GitHub 上被回复的线程一样进入"已处理"桶;amend 后行号不再映射的线程进入"过期"桶(因此改写行上的全新 finding 仍会发布);评论没有 commit 锚点(每个线程的代码事实降级为
unknown);head 漂移在 Aone 没有 compare API,anchor-risk 判定保守失败、让 skill 回到 amend 后的新 head 重新审查。skill 文档、用户文档与两份设计文档同步更新:Aone 跳过列表收缩到唯一仍无支撑的流程,重复轮的两个已知限制(无去重、无 self-PR 检测)被移除。为什么需要
AGit-Flow 下 CR 是单 commit 原地 amend,amend → 重新审查是常态循环。没有这个支撑时,Aone MR 的每一轮重复
--comment都会把仍然有效的 finding 全部作为新评论重发——MR 每轮累积一份完整重复,真正的新发现被淹没,discussion gate 也被刷屏;且用户审查自己的 MR 时不会触发 self-PR 降级。本 PR 关闭平台抽象设计文档 Phase 3 中 "dedup/self-PR backing for Aone" 一项。评审者测试计划
如何验证
GitHub 路径未变——其行为由既有测试套件原样通过钉死(共享核心的抽取是逐行忠实的,已经过一轮"移除行为"审计)。Aone 路径:
cd packages/cli && npx vitest run src/commands/review/comment-status.aone.test.ts src/commands/review/presubmit.aone.test.ts src/commands/review/lib/platform/aone.test.ts——应全绿。(path, line)的既往 finding 进入overlap并置blockOnExistingComments;outdated线程进入stale(可重发);closed线程进入resolved(已处理、不阻塞);未发布的草稿永远不进入任何桶。gitlab.alibaba-inc.com)对同一 MR 执行两轮--comment审查,确认第二轮丢弃已上报的 findings(日志带已有评论 id),只发布新 findings。前后对比证据
N/A——非 UI 变更(CLI 子命令 + skill 文档),无 TUI 输出变化。
测试环境
运行环境(可选)
仅单元测试(packages/cli 内
npx vitest run、npm run build、npx tsc --noEmit)。说明:作者机器上无法实际调用 a1(公司 SSO 凭证失败),因此 a1 载荷形状遵循父设计文档中对真实调用验证过的事实表,代码各处采用宽容提取与保守失败分类。风险与范围
关联 Issue
Closes #9613